diff --git a/pkg/reconciler/finalizer.go b/pkg/reconciler/finalizer.go index 13068be7f..58f9cfa34 100644 --- a/pkg/reconciler/finalizer.go +++ b/pkg/reconciler/finalizer.go @@ -2,12 +2,15 @@ package reconciler import ( "context" + "fmt" "strings" "github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/keys" "github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/v1alpha1" "github.com/openshift-pipelines/pipelines-as-code/pkg/kubeinteraction" + "github.com/openshift-pipelines/pipelines-as-code/pkg/provider" tektonv1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" + "go.uber.org/zap" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "knative.dev/pkg/logging" @@ -59,6 +62,37 @@ func (r *Reconciler) FinalizeKind(ctx context.Context, pr *tektonv1.PipelineRun) } return nil } + + // report the PipelineRun as cancelled as it was queued or started but it is deleted + // but its status is still in progress or queued on git provider + if err := r.reportPipelineRunAsCancelled(ctx, logger, repo, pr); err != nil { + logger.Errorf("failed to report started pipeline run as cancelled: %w", err) + return err + } + } + return nil +} + +func (r *Reconciler) reportPipelineRunAsCancelled(ctx context.Context, logger *zap.SugaredLogger, repo *v1alpha1.Repository, pr *tektonv1.PipelineRun) error { + detectedProvider, event, err := r.initGitProviderClient(ctx, logger, repo, pr) + if err != nil { + return err + } + + consoleURL := r.run.Clients.ConsoleUI().DetailURL(pr) + status := provider.StatusOpts{ + Conclusion: "cancelled", + Text: fmt.Sprintf("PipelineRun %s was deleted", pr.GetName()), + DetailsURL: consoleURL, + PipelineRunName: pr.GetName(), + PipelineRun: pr, + OriginalPipelineRunName: pr.GetAnnotations()[keys.OriginalPRName], } + + if err := createStatusWithRetry(ctx, logger, detectedProvider, event, status); err != nil { + return fmt.Errorf("failed to report cancelled status to provider: %w", err) + } + + logger.Infof("updated cancelled status on provider platform for pipelineRun %s", pr.GetName()) return nil } diff --git a/pkg/reconciler/finalizer_test.go b/pkg/reconciler/finalizer_test.go index df481e72f..0330e5394 100644 --- a/pkg/reconciler/finalizer_test.go +++ b/pkg/reconciler/finalizer_test.go @@ -1,16 +1,19 @@ package reconciler import ( + "strings" "testing" "github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/keys" "github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/v1alpha1" + "github.com/openshift-pipelines/pipelines-as-code/pkg/consoleui" "github.com/openshift-pipelines/pipelines-as-code/pkg/kubeinteraction" "github.com/openshift-pipelines/pipelines-as-code/pkg/params" "github.com/openshift-pipelines/pipelines-as-code/pkg/params/clients" "github.com/openshift-pipelines/pipelines-as-code/pkg/params/info" "github.com/openshift-pipelines/pipelines-as-code/pkg/sync" testclient "github.com/openshift-pipelines/pipelines-as-code/pkg/test/clients" + testkubernetestint "github.com/openshift-pipelines/pipelines-as-code/pkg/test/kubernetestint" tektonv1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" "go.uber.org/zap" zapobserver "go.uber.org/zap/zaptest/observer" @@ -29,6 +32,11 @@ var ( Spec: v1alpha1.RepositorySpec{ URL: "https://github.com/sm43/pac-app", ConcurrencyLimit: &concurrency, + GitProvider: &v1alpha1.GitProvider{ + Secret: &v1alpha1.Secret{ + Name: "pac-git-basic-auth-owner-repo", + }, + }, }, } ) @@ -43,8 +51,12 @@ func getTestPR(name, state string) *tektonv1.PipelineRun { Name: name, Namespace: finalizeTestRepo.Namespace, Annotations: map[string]string{ - keys.State: state, - keys.Repository: finalizeTestRepo.Name, + keys.State: state, + keys.Repository: finalizeTestRepo.Name, + keys.GitProvider: "github", + keys.SHA: "123afc", + keys.URLOrg: "sm43", + keys.URLRepository: "pac-app", }, }, Spec: tektonv1.PipelineRunSpec{ @@ -92,6 +104,15 @@ func TestReconciler_FinalizeKind(t *testing.T) { }, skipAddingRepo: true, }, + { + name: "cancelled status reported", + pipelinerun: getTestPR("pr3", kubeinteraction.StateStarted), + addToQueue: []*tektonv1.PipelineRun{ + getTestPR("pr1", kubeinteraction.StateStarted), + getTestPR("pr2", kubeinteraction.StateQueued), + getTestPR("pr3", kubeinteraction.StateQueued), + }, + }, } for _, tt := range tests { @@ -104,18 +125,29 @@ func TestReconciler_FinalizeKind(t *testing.T) { testData.Repositories = []*v1alpha1.Repository{} } stdata, informers := testclient.SeedTestData(t, ctx, testData) + kinterfaceTest := &testkubernetestint.KinterfaceTest{ + GetSecretResult: map[string]string{ + "pac-git-basic-auth-owner-repo": "https://whateveryousayboss", + }, + } + + cs := ¶ms.Run{ + Clients: clients.Clients{ + PipelineAsCode: stdata.PipelineAsCode, + Log: fakelogger, + }, + Info: info.Info{ + Kube: &info.KubeOpts{Namespace: "pac"}, + Controller: &info.ControllerInfo{GlobalRepository: "pac"}, + Pac: info.NewPacOpts(), + }, + } + cs.Clients.SetConsoleUI(consoleui.FallBackConsole{}) r := Reconciler{ repoLister: informers.Repository.Lister(), qm: sync.NewQueueManager(fakelogger), - run: ¶ms.Run{ - Clients: clients.Clients{ - PipelineAsCode: stdata.PipelineAsCode, - }, - Info: info.Info{ - Kube: &info.KubeOpts{Namespace: "pac"}, - Controller: &info.ControllerInfo{GlobalRepository: "pac"}, - }, - }, + run: cs, + kinteract: kinterfaceTest, } if len(tt.addToQueue) != 0 { @@ -125,7 +157,9 @@ func TestReconciler_FinalizeKind(t *testing.T) { } } err := r.FinalizeKind(ctx, tt.pipelinerun) - assert.NilError(t, err) + if err != nil && !strings.Contains(err.Error(), "401 Bad credentials []") { + t.Fatalf("expected no error, got %v", err) + } // if repo was deleted then no queue will be there if tt.skipAddingRepo { diff --git a/pkg/reconciler/reconciler.go b/pkg/reconciler/reconciler.go index 412047fb0..ddc0dff14 100644 --- a/pkg/reconciler/reconciler.go +++ b/pkg/reconciler/reconciler.go @@ -293,40 +293,10 @@ func (r *Reconciler) updatePipelineRunToInProgress(ctx context.Context, logger * if err != nil { return fmt.Errorf("cannot update state: %w", err) } - pacInfo := r.run.Info.GetPacOpts() - detectedProvider, event, err := r.detectProvider(ctx, logger, pr) - if err != nil { - logger.Error(err) - return nil - } - detectedProvider.SetPacInfo(&pacInfo) - - if event.InstallationID > 0 { - event.Provider.WebhookSecret, _ = pac.GetCurrentNSWebhookSecret(ctx, r.kinteract, r.run) - } else { - // secretNS is needed when git provider is other than Github. - secretNS := repo.GetNamespace() - if repo.Spec.GitProvider != nil && repo.Spec.GitProvider.Secret == nil && r.globalRepo != nil && r.globalRepo.Spec.GitProvider != nil && r.globalRepo.Spec.GitProvider.Secret != nil { - secretNS = r.globalRepo.GetNamespace() - } - - secretFromRepo := pac.SecretFromRepository{ - K8int: r.kinteract, - Config: detectedProvider.GetConfig(), - Event: event, - Repo: repo, - WebhookType: pacInfo.WebhookType, - Logger: logger, - Namespace: secretNS, - } - if err := secretFromRepo.Get(ctx); err != nil { - return fmt.Errorf("cannot get secret from repository: %w", err) - } - } - err = detectedProvider.SetClient(ctx, r.run, event, repo, r.eventEmitter) + detectedProvider, event, err := r.initGitProviderClient(ctx, logger, repo, pr) if err != nil { - return fmt.Errorf("cannot set client: %w", err) + return fmt.Errorf("cannot initialize git provider client: %w", err) } consoleURL := r.run.Clients.ConsoleUI().DetailURL(pr) @@ -364,6 +334,45 @@ func (r *Reconciler) updatePipelineRunToInProgress(ctx context.Context, logger * return nil } +func (r *Reconciler) initGitProviderClient(ctx context.Context, logger *zap.SugaredLogger, repo *v1alpha1.Repository, pr *tektonv1.PipelineRun) (provider.Interface, *info.Event, error) { + pacInfo := r.run.Info.GetPacOpts() + detectedProvider, event, err := r.detectProvider(ctx, logger, pr) + if err != nil { + return nil, nil, fmt.Errorf("cannot detect provider: %w", err) + } + detectedProvider.SetPacInfo(&pacInfo) + + if event.InstallationID > 0 { + event.Provider.WebhookSecret, _ = pac.GetCurrentNSWebhookSecret(ctx, r.kinteract, r.run) + } else { + // secretNS is needed when git provider is other than Github. + secretNS := repo.GetNamespace() + if repo.Spec.GitProvider != nil && repo.Spec.GitProvider.Secret == nil && r.globalRepo != nil && r.globalRepo.Spec.GitProvider != nil && r.globalRepo.Spec.GitProvider.Secret != nil { + secretNS = r.globalRepo.GetNamespace() + } + + secretFromRepo := pac.SecretFromRepository{ + K8int: r.kinteract, + Config: detectedProvider.GetConfig(), + Event: event, + Repo: repo, + WebhookType: pacInfo.WebhookType, + Logger: logger, + Namespace: secretNS, + } + if err := secretFromRepo.Get(ctx); err != nil { + return nil, nil, fmt.Errorf("cannot get secret from repository: %w", err) + } + } + + err = detectedProvider.SetClient(ctx, r.run, event, repo, r.eventEmitter) + if err != nil { + return nil, nil, fmt.Errorf("cannot set client: %w", err) + } + + return detectedProvider, event, nil +} + func (r *Reconciler) updatePipelineRunState(ctx context.Context, logger *zap.SugaredLogger, pr *tektonv1.PipelineRun, state string) (*tektonv1.PipelineRun, error) { currentState := pr.GetAnnotations()[keys.State] logger.Infof("updating pipelineRun %v/%v state from %s to %s", pr.GetNamespace(), pr.GetName(), currentState, state) diff --git a/pkg/reconciler/reconciler_test.go b/pkg/reconciler/reconciler_test.go index 3e7b82c8e..10b4f24b2 100644 --- a/pkg/reconciler/reconciler_test.go +++ b/pkg/reconciler/reconciler_test.go @@ -24,6 +24,7 @@ import ( "github.com/openshift-pipelines/pipelines-as-code/pkg/sync" testclient "github.com/openshift-pipelines/pipelines-as-code/pkg/test/clients" ghtesthelper "github.com/openshift-pipelines/pipelines-as-code/pkg/test/github" + testkubernetestint "github.com/openshift-pipelines/pipelines-as-code/pkg/test/kubernetestint" tektontest "github.com/openshift-pipelines/pipelines-as-code/pkg/test/tekton" tektonv1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" "go.uber.org/zap" @@ -315,7 +316,7 @@ func TestUpdatePipelineRunState(t *testing.T) { func TestReconcileKind_SCMReportingLogic(t *testing.T) { observer, _ := zapobserver.New(zap.InfoLevel) - _ = zap.New(observer).Sugar() + logger := zap.New(observer).Sugar() tests := []struct { name string @@ -330,8 +331,12 @@ func TestReconcileKind_SCMReportingLogic(t *testing.T) { Namespace: "test", Name: "test-pr", Annotations: map[string]string{ - keys.State: kubeinteraction.StateQueued, - keys.Repository: "test-repo", + keys.State: kubeinteraction.StateQueued, + keys.Repository: "test-repo", + keys.GitProvider: "github", + keys.SHA: "123afc", + keys.URLOrg: "random", + keys.URLRepository: "app", }, }, Spec: tektonv1.PipelineRunSpec{}, @@ -360,6 +365,10 @@ func TestReconcileKind_SCMReportingLogic(t *testing.T) { keys.State: kubeinteraction.StateStarted, keys.Repository: "test-repo", keys.SCMReportingPLRStarted: "true", + keys.GitProvider: "github", + keys.SHA: "123afc", + keys.URLOrg: "random", + keys.URLRepository: "app", }, }, Spec: tektonv1.PipelineRunSpec{}, @@ -385,8 +394,12 @@ func TestReconcileKind_SCMReportingLogic(t *testing.T) { Namespace: "test", Name: "test-pr", Annotations: map[string]string{ - keys.State: kubeinteraction.StateQueued, - keys.Repository: "test-repo", + keys.State: kubeinteraction.StateQueued, + keys.Repository: "test-repo", + keys.GitProvider: "github", + keys.SHA: "123afc", + keys.URLOrg: "random", + keys.URLRepository: "app", }, }, Spec: tektonv1.PipelineRunSpec{ @@ -420,6 +433,11 @@ func TestReconcileKind_SCMReportingLogic(t *testing.T) { }, Spec: v1alpha1.RepositorySpec{ URL: randomURL, + GitProvider: &v1alpha1.GitProvider{ + Secret: &v1alpha1.Secret{ + Name: "pac-git-basic-auth-owner-repo", + }, + }, }, } @@ -432,19 +450,30 @@ func TestReconcileKind_SCMReportingLogic(t *testing.T) { // Track if updatePipelineRunToInProgress was called by checking state changes originalState := tt.pipelineRun.GetAnnotations()[keys.State] - r := &Reconciler{ - repoLister: informers.Repository.Lister(), - run: ¶ms.Run{ - Clients: clients.Clients{ - Tekton: stdata.Pipeline, - }, - Info: info.Info{ - Pac: &info.PacOpts{ - Settings: settings.Settings{}, - }, + kinterfaceTest := &testkubernetestint.KinterfaceTest{ + GetSecretResult: map[string]string{ + "pac-git-basic-auth-owner-repo": "https://whateveryousayboss", + }, + } + + cs := ¶ms.Run{ + Clients: clients.Clients{ + Tekton: stdata.Pipeline, + Log: logger, + }, + Info: info.Info{ + Pac: &info.PacOpts{ + Settings: settings.Settings{}, }, }, } + cs.Clients.SetConsoleUI(consoleui.FallBackConsole{}) + + r := &Reconciler{ + repoLister: informers.Repository.Lister(), + run: cs, + kinteract: kinterfaceTest, + } err := r.ReconcileKind(ctx, tt.pipelineRun)