From 3495b53ed489865488c41962ca2b4a0fa0f9ac35 Mon Sep 17 00:00:00 2001 From: Chmouel Boudjnah Date: Fri, 13 Dec 2024 14:47:55 +0100 Subject: [PATCH 1/2] feat: auto-cancel PipelineRuns on PR close The pipelinesascode.tekton.dev/cancel-in-progress: "true" feature annotation has now been enhanced to include automatic cancellation of PipelineRuns when the associated pull request is closed or merged. Jira: https://issues.redhat.com/browse/SRVKP-6908 Signed-off-by: Chmouel Boudjnah --- pkg/formatting/pipelinerun.go | 4 + pkg/formatting/pipelinerun_test.go | 18 +++ pkg/params/triggertype/types.go | 15 +-- pkg/pipelineascode/cancel_pipelineruns.go | 105 +++++++++++------- .../cancel_pipelineruns_test.go | 96 +++++++++++++++- pkg/pipelineascode/match.go | 2 +- pkg/pipelineascode/pipelineascode.go | 9 +- pkg/policy/policy.go | 2 +- pkg/provider/gitea/detect.go | 3 +- pkg/provider/gitea/gitea.go | 2 +- pkg/provider/gitea/parse_payload.go | 3 + pkg/provider/github/detect.go | 4 + pkg/provider/github/parse_payload.go | 6 + pkg/provider/github/parse_payload_test.go | 32 ++++-- pkg/provider/github/status.go | 3 + pkg/provider/gitlab/detect.go | 13 ++- pkg/provider/gitlab/parse_payload.go | 3 + pkg/reconciler/reconciler.go | 2 +- test/gitea_access_control_test.go | 4 +- test/gitea_test.go | 55 +++++++-- test/github_pullrequest_test.go | 67 +++++++++-- test/github_push_retest_test.go | 2 +- test/gitlab_incoming_webhook_test.go | 2 +- test/gitlab_merge_request_test.go | 82 +++++++++++++- test/pkg/gitea/scm.go | 2 +- test/pkg/gitea/test.go | 3 +- test/pkg/scm/scm.go | 7 +- .../pipelinerun-cancel-in-progress.yaml | 2 +- 28 files changed, 448 insertions(+), 100 deletions(-) diff --git a/pkg/formatting/pipelinerun.go b/pkg/formatting/pipelinerun.go index b61007c2d..eb7ed97e2 100644 --- a/pkg/formatting/pipelinerun.go +++ b/pkg/formatting/pipelinerun.go @@ -3,6 +3,7 @@ package formatting import ( tektonv1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" corev1 "k8s.io/api/core/v1" + "knative.dev/pkg/apis" ) // PipelineRunStatus return status of PR success failed or skipped. @@ -10,6 +11,9 @@ func PipelineRunStatus(pr *tektonv1.PipelineRun) string { if len(pr.Status.Conditions) == 0 { return "neutral" } + if pr.Status.GetCondition(apis.ConditionSucceeded).GetReason() == tektonv1.PipelineRunSpecStatusCancelled { + return "cancelled" + } if pr.Status.Conditions[0].Status == corev1.ConditionFalse { return "failure" } diff --git a/pkg/formatting/pipelinerun_test.go b/pkg/formatting/pipelinerun_test.go index 4351178b3..4f558cc21 100644 --- a/pkg/formatting/pipelinerun_test.go +++ b/pkg/formatting/pipelinerun_test.go @@ -6,6 +6,7 @@ import ( tektonv1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" "gotest.tools/v3/assert" corev1 "k8s.io/api/core/v1" + "knative.dev/pkg/apis" knativeduckv1 "knative.dev/pkg/apis/duck/v1" ) @@ -30,6 +31,23 @@ func TestPipelineRunStatus(t *testing.T) { }, }, }, + { + name: "cancelled", + pr: &tektonv1.PipelineRun{ + Status: tektonv1.PipelineRunStatus{ + Status: knativeduckv1.Status{ + Conditions: knativeduckv1.Conditions{ + { + Status: corev1.ConditionTrue, + Reason: tektonv1.PipelineRunSpecStatusCancelled, + Message: "Cancelled", + Type: apis.ConditionSucceeded, + }, + }, + }, + }, + }, + }, { name: "failure", pr: &tektonv1.PipelineRun{ diff --git a/pkg/params/triggertype/types.go b/pkg/params/triggertype/types.go index 9a9e61994..4726ea279 100644 --- a/pkg/params/triggertype/types.go +++ b/pkg/params/triggertype/types.go @@ -43,14 +43,15 @@ func StringToType(s string) Trigger { } const ( - OkToTest Trigger = "ok-to-test" - Retest Trigger = "retest" - Push Trigger = "push" - PullRequest Trigger = "pull_request" - LabelUpdate Trigger = "label_update" Cancel Trigger = "cancel" - CheckSuiteRerequested Trigger = "check-suite-rerequested" CheckRunRerequested Trigger = "check-run-rerequested" - Incoming Trigger = "incoming" + CheckSuiteRerequested Trigger = "check-suite-rerequested" Comment Trigger = "comment" + Incoming Trigger = "incoming" + LabelUpdate Trigger = "label_update" + OkToTest Trigger = "ok-to-test" + PullRequestClosed Trigger = "pull_request_closed" + PullRequest Trigger = "pull_request" // it's should be "pull_request_opened_updated" but let's keep it simple. + Push Trigger = "push" + Retest Trigger = "retest" ) diff --git a/pkg/pipelineascode/cancel_pipelineruns.go b/pkg/pipelineascode/cancel_pipelineruns.go index 12834a523..7a312acb8 100644 --- a/pkg/pipelineascode/cancel_pipelineruns.go +++ b/pkg/pipelineascode/cancel_pipelineruns.go @@ -20,19 +20,47 @@ import ( "github.com/openshift-pipelines/pipelines-as-code/pkg/params/triggertype" ) +type matchingCond func(pr tektonv1.PipelineRun) bool + var cancelMergePatch = map[string]interface{}{ "spec": map[string]interface{}{ "status": tektonv1.PipelineRunSpecStatusCancelledRunFinally, }, } -// cancelInProgress cancels all PipelineRuns associated with a given repository and pull request, +func (p *PacRun) cancelAllInProgressBelongingToPullRequest(ctx context.Context, repo *v1alpha1.Repository) error { + labelSelector := getLabelSelector(map[string]string{ + keys.URLRepository: formatting.CleanValueKubernetes(p.event.Repository), + keys.PullRequest: strconv.Itoa(int(p.event.PullRequestNumber)), + }) + prs, err := p.run.Clients.Tekton.TektonV1().PipelineRuns(repo.Namespace).List(ctx, metav1.ListOptions{ + LabelSelector: labelSelector, + }) + if err != nil { + return fmt.Errorf("failed to list pipelineRuns : %w", err) + } + + if len(prs.Items) == 0 { + msg := fmt.Sprintf("no pipelinerun found for repository: %v and pullRequest %v", + p.event.Repository, p.event.PullRequestNumber) + p.eventEmitter.EmitMessage(repo, zap.InfoLevel, "RepositoryPipelineRun", msg) + return nil + } + + p.cancelPipelineRuns(ctx, prs, repo, func(_ tektonv1.PipelineRun) bool { + return true + }) + + return nil +} + +// cancelInProgressMatchingPR cancels all PipelineRuns associated with a given repository and pull request, // except for the one that triggered the cancellation. It first checks if the cancellation is in progress // and if the repository has a concurrency limit. If a concurrency limit is set, it returns an error as // cancellation is not supported with concurrency limits. It then retrieves the original pull request name // from the annotations and lists all PipelineRuns with matching labels. For each PipelineRun that is not // already done, cancelled, or gracefully stopped, it patches the PipelineRun to cancel it. -func (p *PacRun) cancelInProgress(ctx context.Context, matchPR *tektonv1.PipelineRun, repo *v1alpha1.Repository) error { +func (p *PacRun) cancelInProgressMatchingPR(ctx context.Context, matchPR *tektonv1.PipelineRun, repo *v1alpha1.Repository) error { if matchPR == nil { return nil } @@ -67,11 +95,9 @@ func (p *PacRun) cancelInProgress(ctx context.Context, matchPR *tektonv1.Pipelin if err != nil { return fmt.Errorf("failed to list pipelineRuns : %w", err) } - var wg sync.WaitGroup - for _, pr := range prs.Items { - if pr.GetName() == matchPR.GetName() { - continue - } + + p.cancelPipelineRuns(ctx, prs, repo, func(pr tektonv1.PipelineRun) bool { + // skip our own for cancellation if sourceBranch, ok := pr.GetAnnotations()[keys.SourceBranch]; ok { // NOTE(chmouel): Every PR has their own branch and so is every push to different branch // it means we only cancel pipelinerun of the same name that runs to @@ -79,39 +105,18 @@ func (p *PacRun) cancelInProgress(ctx context.Context, matchPR *tektonv1.Pipelin // comes from in git jargon. if sourceBranch != p.event.HeadBranch { p.logger.Infof("cancel-in-progress: skipping pipelinerun %v/%v as it is not from the same branch, annotation source-branch: %s event headbranch: %s", pr.GetNamespace(), pr.GetName(), sourceBranch, p.event.HeadBranch) - continue + return false } } - if pr.IsPending() { - p.logger.Infof("cancel-in-progress: skipping pipelinerun %v/%v as it is pending", pr.GetNamespace(), pr.GetName()) - } - - if pr.IsDone() { - p.logger.Infof("cancel-in-progress: skipping pipelinerun %v/%v as it is done", pr.GetNamespace(), pr.GetName()) - continue - } - if pr.IsCancelled() || pr.IsGracefullyCancelled() || pr.IsGracefullyStopped() { - p.logger.Infof("cancel-in-progress: skipping pipelinerun %v/%v as it is already in %v state", pr.GetNamespace(), pr.GetName(), pr.Spec.Status) - continue - } - - p.logger.Infof("cancel-in-progress: cancelling pipelinerun %v/%v", pr.GetNamespace(), pr.GetName()) - wg.Add(1) - go func(ctx context.Context, pr tektonv1.PipelineRun) { - defer wg.Done() - if _, err := action.PatchPipelineRun(ctx, p.logger, "cancel patch", p.run.Clients.Tekton, &pr, cancelMergePatch); err != nil { - errMsg := fmt.Sprintf("failed to cancel pipelineRun %s/%s: %s", pr.GetNamespace(), pr.GetName(), err.Error()) - p.eventEmitter.EmitMessage(repo, zap.ErrorLevel, "RepositoryPipelineRun", errMsg) - } - }(ctx, pr) - } - wg.Wait() - + return pr.GetName() != matchPR.GetName() + }) return nil } -func (p *PacRun) cancelPipelineRuns(ctx context.Context, repo *v1alpha1.Repository) error { +// cancelPipelineRunsOpsComment cancels all PipelineRuns associated with a given repository and pull request. +// when the user issue a cancel comment. +func (p *PacRun) cancelPipelineRunsOpsComment(ctx context.Context, repo *v1alpha1.Repository) error { labelSelector := getLabelSelector(map[string]string{ keys.URLRepository: formatting.CleanValueKubernetes(p.event.Repository), keys.SHA: formatting.CleanValueKubernetes(p.event.SHA), @@ -137,22 +142,40 @@ func (p *PacRun) cancelPipelineRuns(ctx context.Context, repo *v1alpha1.Reposito return nil } - var wg sync.WaitGroup - for _, pr := range prs.Items { + p.cancelPipelineRuns(ctx, prs, repo, func(pr tektonv1.PipelineRun) bool { if p.event.TargetCancelPipelineRun != "" { if prName, ok := pr.GetAnnotations()[keys.OriginalPRName]; !ok || prName != p.event.TargetCancelPipelineRun { - continue + return false } } - if pr.IsDone() { - p.logger.Infof("pipelinerun %v/%v is done, skipping cancellation", pr.GetNamespace(), pr.GetName()) + return true + }) + + return nil +} + +func (p *PacRun) cancelPipelineRuns(ctx context.Context, prs *tektonv1.PipelineRunList, repo *v1alpha1.Repository, condition matchingCond) { + var wg sync.WaitGroup + for _, pr := range prs.Items { + if !condition(pr) { continue } + if pr.IsCancelled() || pr.IsGracefullyCancelled() || pr.IsGracefullyStopped() { - p.logger.Infof("pipelinerun %v/%v is already in %v state", pr.GetNamespace(), pr.GetName(), pr.Spec.Status) + p.logger.Infof("cancel-in-progress: skipping cancelling pipelinerun %v/%v, already in %v state", pr.GetNamespace(), pr.GetName(), pr.Spec.Status) continue } + if pr.IsDone() { + p.logger.Infof("cancel-in-progress: skipping cancelling pipelinerun %v/%v, already done", pr.GetNamespace(), pr.GetName()) + continue + } + + if pr.IsPending() { + p.logger.Infof("cancel-in-progress: skipping cancelling pipelinerun %v/%v in pending state", pr.GetNamespace(), pr.GetName()) + } + + p.logger.Infof("cancel-in-progress: cancelling pipelinerun %v/%v", pr.GetNamespace(), pr.GetName()) wg.Add(1) go func(ctx context.Context, pr tektonv1.PipelineRun) { defer wg.Done() @@ -163,8 +186,6 @@ func (p *PacRun) cancelPipelineRuns(ctx context.Context, repo *v1alpha1.Reposito }(ctx, pr) } wg.Wait() - - return nil } func getLabelSelector(labelsMap map[string]string) string { diff --git a/pkg/pipelineascode/cancel_pipelineruns_test.go b/pkg/pipelineascode/cancel_pipelineruns_test.go index c497f49d0..5df9759a3 100644 --- a/pkg/pipelineascode/cancel_pipelineruns_test.go +++ b/pkg/pipelineascode/cancel_pipelineruns_test.go @@ -71,7 +71,7 @@ var ( } ) -func TestCancelPipelinerun(t *testing.T) { +func TestCancelPipelinerunOpsComment(t *testing.T) { observer, _ := zapobserver.New(zap.InfoLevel) logger := zap.New(observer).Sugar() tests := []struct { @@ -300,7 +300,7 @@ func TestCancelPipelinerun(t *testing.T) { }, } pac := NewPacs(tt.event, nil, cs, &info.PacOpts{}, nil, logger, nil) - err := pac.cancelPipelineRuns(ctx, tt.repo) + err := pac.cancelPipelineRunsOpsComment(ctx, tt.repo) assert.NilError(t, err) got, err := cs.Clients.Tekton.TektonV1().PipelineRuns("foo").List(ctx, metav1.ListOptions{}) @@ -318,7 +318,7 @@ func TestCancelPipelinerun(t *testing.T) { } } -func TestCancelInProgress(t *testing.T) { +func TestCancelInProgressMatchingPR(t *testing.T) { observer, catcher := zapobserver.New(zap.InfoLevel) logger := zap.New(observer).Sugar() tests := []struct { @@ -789,7 +789,7 @@ func TestCancelInProgress(t *testing.T) { if len(tt.pipelineRuns) > 0 { firstPr = tt.pipelineRuns[0] } - err := pac.cancelInProgress(ctx, firstPr, tt.repo) + err := pac.cancelInProgressMatchingPR(ctx, firstPr, tt.repo) if tt.wantErrString != "" { assert.ErrorContains(t, err, tt.wantErrString) return @@ -818,6 +818,94 @@ func TestCancelInProgress(t *testing.T) { } } +func TestCancelAllInProgressBelongingToPullRequest(t *testing.T) { + observer, _ := zapobserver.New(zap.InfoLevel) + logger := zap.New(observer).Sugar() + + tests := []struct { + name string + event *info.Event + repo *v1alpha1.Repository + pipelineRuns []*pipelinev1.PipelineRun + cancelledPipelineRuns map[string]bool + }{ + { + name: "cancel all in progress PipelineRuns", + event: &info.Event{ + Repository: "foo", + TriggerTarget: "pull_request", + PullRequestNumber: pullReqNumber, + }, + repo: fooRepo, + pipelineRuns: []*pipelinev1.PipelineRun{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "pr-foo-1", + Namespace: "foo", + Labels: fooRepoLabels, + }, + Spec: pipelinev1.PipelineRunSpec{}, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "pr-foo-2", + Namespace: "foo", + Labels: fooRepoLabels, + }, + Spec: pipelinev1.PipelineRunSpec{}, + }, + }, + cancelledPipelineRuns: map[string]bool{ + "pr-foo-1": true, + "pr-foo-2": true, + }, + }, + { + name: "no PipelineRuns to cancel", + event: &info.Event{ + Repository: "foo", + TriggerTarget: "pull_request", + PullRequestNumber: pullReqNumber, + }, + repo: fooRepo, + pipelineRuns: []*pipelinev1.PipelineRun{}, + cancelledPipelineRuns: map[string]bool{}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx, _ := rtesting.SetupFakeContext(t) + + tdata := testclient.Data{ + PipelineRuns: tt.pipelineRuns, + } + stdata, _ := testclient.SeedTestData(t, ctx, tdata) + cs := ¶ms.Run{ + Clients: clients.Clients{ + Log: logger, + Tekton: stdata.Pipeline, + Kube: stdata.Kube, + }, + } + pac := NewPacs(tt.event, nil, cs, &info.PacOpts{}, nil, logger, nil) + err := pac.cancelAllInProgressBelongingToPullRequest(ctx, tt.repo) + assert.NilError(t, err) + + got, err := cs.Clients.Tekton.TektonV1().PipelineRuns("foo").List(ctx, metav1.ListOptions{}) + assert.NilError(t, err) + + for _, pr := range got.Items { + if _, ok := tt.cancelledPipelineRuns[pr.Name]; ok { + assert.Equal(t, string(pr.Spec.Status), pipelinev1.PipelineRunSpecStatusCancelledRunFinally) + } else { + assert.Assert(t, string(pr.Spec.Status) != pipelinev1.PipelineRunSpecStatusCancelledRunFinally) + } + } + }) + } +} + func TestGetLabelSelector(t *testing.T) { tests := []struct { name string diff --git a/pkg/pipelineascode/match.go b/pkg/pipelineascode/match.go index 68ca9c7e5..85cd3dd3e 100644 --- a/pkg/pipelineascode/match.go +++ b/pkg/pipelineascode/match.go @@ -30,7 +30,7 @@ func (p *PacRun) matchRepoPR(ctx context.Context) ([]matcher.Match, *v1alpha1.Re } if p.event.CancelPipelineRuns { - return nil, repo, p.cancelPipelineRuns(ctx, repo) + return nil, repo, p.cancelPipelineRunsOpsComment(ctx, repo) } matchedPRs, err := p.getPipelineRunsFromRepo(ctx, repo) diff --git a/pkg/pipelineascode/pipelineascode.go b/pkg/pipelineascode/pipelineascode.go index 66f8608ec..835339b4d 100644 --- a/pkg/pipelineascode/pipelineascode.go +++ b/pkg/pipelineascode/pipelineascode.go @@ -17,6 +17,7 @@ import ( "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/params/settings" + "github.com/openshift-pipelines/pipelines-as-code/pkg/params/triggertype" "github.com/openshift-pipelines/pipelines-as-code/pkg/provider" "github.com/openshift-pipelines/pipelines-as-code/pkg/secrets" tektonv1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" @@ -55,6 +56,12 @@ func NewPacs(event *info.Event, vcx provider.Interface, run *params.Run, pacInfo func (p *PacRun) Run(ctx context.Context) error { matchedPRs, repo, err := p.matchRepoPR(ctx) + if repo != nil && p.event.TriggerTarget == triggertype.PullRequestClosed { + if err := p.cancelAllInProgressBelongingToPullRequest(ctx, repo); err != nil { + return fmt.Errorf("error cancelling in progress pipelineRuns belonging to pull request %d: %w", p.event.PullRequestNumber, err) + } + return nil + } if err != nil { createStatusErr := p.vcx.CreateStatus(ctx, p.event, provider.StatusOpts{ Status: CompletedStatus, @@ -116,7 +123,7 @@ func (p *PacRun) Run(ctx context.Context) error { } } p.manager.AddPipelineRun(pr) - if err := p.cancelInProgress(ctx, pr, repo); err != nil { + if err := p.cancelInProgressMatchingPR(ctx, pr, repo); err != nil { p.eventEmitter.EmitMessage(repo, zap.ErrorLevel, "RepositoryPipelineRun", fmt.Sprintf("error cancelling in progress pipelineRuns: %s", err)) } }(match, i) diff --git a/pkg/policy/policy.go b/pkg/policy/policy.go index 77f9b5cd6..dff60bb06 100644 --- a/pkg/policy/policy.go +++ b/pkg/policy/policy.go @@ -45,7 +45,7 @@ func (p *Policy) checkAllowed(ctx context.Context, tType triggertype.Trigger) (R sType = settings.Policy.OkToTest // apply the same policy for PullRequest and comment // we don't support comments on PRs yet but if we do on the future we will need our own policy - case triggertype.PullRequest, triggertype.Comment, triggertype.LabelUpdate: + case triggertype.PullRequest, triggertype.Comment, triggertype.LabelUpdate, triggertype.PullRequestClosed: sType = settings.Policy.PullRequest // NOTE: not supported yet, will imp if it gets requested and reasonable to implement case triggertype.Push, triggertype.Cancel, triggertype.CheckSuiteRerequested, triggertype.CheckRunRerequested, triggertype.Incoming: diff --git a/pkg/provider/gitea/detect.go b/pkg/provider/gitea/detect.go index 45884119d..cde0f5a46 100644 --- a/pkg/provider/gitea/detect.go +++ b/pkg/provider/gitea/detect.go @@ -14,6 +14,7 @@ import ( var ( pullRequestOpenSyncEvent = []string{"opened", "synchronize", "synchronized", "reopened"} pullRequestLabelUpdated = "label_updated" + pullRequestLabelClosed = "closed" ) // Detect processes event and detect if it is a gitea event, whether to process or reject it @@ -56,7 +57,7 @@ func detectTriggerTypeFromPayload(ghEventType string, eventInt any) (triggertype } return "", "invalid payload: no pusher in event" case *giteaStructs.PullRequestPayload: - if provider.Valid(string(event.Action), append(pullRequestOpenSyncEvent, pullRequestLabelUpdated)) { + if provider.Valid(string(event.Action), append(pullRequestOpenSyncEvent, pullRequestLabelUpdated, pullRequestLabelClosed)) { return triggertype.PullRequest, "" } return "", fmt.Sprintf("pull_request: unsupported action \"%s\"", event.Action) diff --git a/pkg/provider/gitea/gitea.go b/pkg/provider/gitea/gitea.go index 58e4adf2a..5328bf121 100644 --- a/pkg/provider/gitea/gitea.go +++ b/pkg/provider/gitea/gitea.go @@ -341,7 +341,7 @@ func (v *Provider) GetFiles(_ context.Context, runevent *info.Event) (changedfil //nolint:exhaustive // we don't need to handle all cases switch runevent.TriggerTarget { - case triggertype.PullRequest: + case triggertype.PullRequest, triggertype.PullRequestClosed: opt := gitea.ListPullRequestFilesOptions{ListOptions: gitea.ListOptions{Page: 1, PageSize: 50}} shouldGetNextPage := false for { diff --git a/pkg/provider/gitea/parse_payload.go b/pkg/provider/gitea/parse_payload.go index 5cc424378..dd8784249 100644 --- a/pkg/provider/gitea/parse_payload.go +++ b/pkg/provider/gitea/parse_payload.go @@ -57,6 +57,9 @@ func (v *Provider) ParsePayload(_ context.Context, _ *params.Run, request *http. for _, label := range gitEvent.PullRequest.Labels { processedEvent.PullRequestLabel = append(processedEvent.PullRequestLabel, label.Name) } + if gitEvent.Action == giteaStructs.HookIssueClosed { + processedEvent.TriggerTarget = triggertype.PullRequestClosed + } case *giteaStructs.PushPayload: processedEvent = info.NewEvent() processedEvent.SHA = gitEvent.HeadCommit.ID diff --git a/pkg/provider/github/detect.go b/pkg/provider/github/detect.go index 51be80164..b63ab32b5 100644 --- a/pkg/provider/github/detect.go +++ b/pkg/provider/github/detect.go @@ -64,6 +64,10 @@ func (v *Provider) detectTriggerTypeFromPayload(ghEventType string, eventInt any } return "", "no pusher in payload" case *github.PullRequestEvent: + if event.GetAction() == "closed" { + return triggertype.PullRequestClosed, "" + } + if provider.Valid(event.GetAction(), pullRequestOpenSyncEvent) || provider.Valid(event.GetAction(), pullRequestLabelEvent) { return triggertype.PullRequest, "" } diff --git a/pkg/provider/github/parse_payload.go b/pkg/provider/github/parse_payload.go index b969573fd..50c298a5c 100644 --- a/pkg/provider/github/parse_payload.go +++ b/pkg/provider/github/parse_payload.go @@ -290,9 +290,15 @@ func (v *Provider) processEvent(ctx context.Context, event *info.Event, eventInt processedEvent.HeadURL = gitEvent.GetPullRequest().Head.GetRepo().GetHTMLURL() processedEvent.Sender = gitEvent.GetPullRequest().GetUser().GetLogin() processedEvent.EventType = event.EventType + if gitEvent.Action != nil && provider.Valid(*gitEvent.Action, pullRequestLabelEvent) { processedEvent.EventType = string(triggertype.LabelUpdate) } + + if gitEvent.GetAction() == "closed" { + processedEvent.TriggerTarget = triggertype.PullRequestClosed + } + processedEvent.PullRequestNumber = gitEvent.GetPullRequest().GetNumber() processedEvent.PullRequestTitle = gitEvent.GetPullRequest().GetTitle() // getting the repository ids of the base and head of the pull request diff --git a/pkg/provider/github/parse_payload_test.go b/pkg/provider/github/parse_payload_test.go index ab827becc..14738eae8 100644 --- a/pkg/provider/github/parse_payload_test.go +++ b/pkg/provider/github/parse_payload_test.go @@ -91,7 +91,8 @@ var samplePRAnother = github.PullRequest{ func TestParsePayLoad(t *testing.T) { samplePRNoRepo := samplePRevent samplePRNoRepo.Repo = nil - + samplePrEventClosed := samplePRevent + samplePrEventClosed.Action = github.String("closed") tests := []struct { name string wantErrString string @@ -208,7 +209,7 @@ func TestParsePayLoad(t *testing.T) { name: "good/rerequest check_run on pull request", eventType: "check_run", githubClient: true, - triggerTarget: "issue-recheck", + triggerTarget: string(triggertype.PullRequest), payloadEventStruct: github.CheckRunEvent{ Action: github.String("rerequested"), Repo: sampleRepo, @@ -226,7 +227,7 @@ func TestParsePayLoad(t *testing.T) { name: "good/rerequest check_suite on pull request", eventType: "check_suite", githubClient: true, - triggerTarget: "issue-recheck", + triggerTarget: string(triggertype.PullRequest), payloadEventStruct: github.CheckSuiteEvent{ Action: github.String("rerequested"), Repo: sampleRepo, @@ -238,10 +239,9 @@ func TestParsePayLoad(t *testing.T) { shaRet: "samplePRsha", }, { - name: "good/rerequest on push", - eventType: "check_run", - githubClient: true, - triggerTarget: "issue-recheck", + name: "good/rerequest on push", + eventType: "check_run", + githubClient: true, payloadEventStruct: github.CheckRunEvent{ Action: github.String("rerequested"), Repo: sampleRepo, @@ -281,10 +281,17 @@ func TestParsePayLoad(t *testing.T) { { name: "good/pull request", eventType: "pull_request", - triggerTarget: "pull_request", + triggerTarget: triggertype.PullRequest.String(), payloadEventStruct: samplePRevent, shaRet: "sampleHeadsha", }, + { + name: "good/pull request closed", + eventType: "pull_request", + triggerTarget: triggertype.PullRequestClosed.String(), + payloadEventStruct: samplePrEventClosed, + shaRet: "sampleHeadsha", + }, { name: "good/push", eventType: "push", @@ -301,7 +308,7 @@ func TestParsePayLoad(t *testing.T) { { name: "good/issue comment for retest", eventType: "issue_comment", - triggerTarget: "pull_request", + triggerTarget: triggertype.PullRequest.String(), githubClient: true, payloadEventStruct: github.IssueCommentEvent{ Action: github.String("created"), @@ -322,7 +329,7 @@ func TestParsePayLoad(t *testing.T) { { name: "good/issue comment for cancel all", eventType: "issue_comment", - triggerTarget: "pull_request", + triggerTarget: triggertype.PullRequest.String(), githubClient: true, payloadEventStruct: github.IssueCommentEvent{ Action: github.String("created"), @@ -342,7 +349,7 @@ func TestParsePayLoad(t *testing.T) { { name: "good/issue comment for cancel a pr", eventType: "issue_comment", - triggerTarget: "pull_request", + triggerTarget: triggertype.PullRequest.String(), githubClient: true, payloadEventStruct: github.IssueCommentEvent{ Action: github.String("created"), @@ -606,7 +613,7 @@ func TestParsePayLoad(t *testing.T) { assert.NilError(t, err) assert.Assert(t, ret != nil) assert.Equal(t, tt.shaRet, ret.SHA) - if tt.eventType == "pull_request" { + if tt.eventType == triggertype.PullRequest.String() { assert.Equal(t, "my first PR", ret.PullRequestTitle) } if tt.eventType == "commit_comment" { @@ -620,6 +627,7 @@ func TestParsePayLoad(t *testing.T) { if tt.targetCancelPipelinerun != "" { assert.Equal(t, tt.targetCancelPipelinerun, ret.TargetCancelPipelineRun) } + assert.Equal(t, tt.triggerTarget, string(ret.TriggerTarget)) }) } } diff --git a/pkg/provider/github/status.go b/pkg/provider/github/status.go index 6fbff6702..cccea9320 100644 --- a/pkg/provider/github/status.go +++ b/pkg/provider/github/status.go @@ -366,6 +366,9 @@ func (v *Provider) CreateStatus(ctx context.Context, runevent *info.Event, statu // for unauthorized user set title as Pending approval statusOpts.Summary = "is waiting for approval." } + case "cancelled": + statusOpts.Title = "Cancelled" + statusOpts.Summary = "has been cancelled." case "neutral": statusOpts.Title = "Unknown" statusOpts.Summary = "doesn't know what happened with this commit." diff --git a/pkg/provider/gitlab/detect.go b/pkg/provider/gitlab/detect.go index 3ad30f1c0..0abbce2ee 100644 --- a/pkg/provider/gitlab/detect.go +++ b/pkg/provider/gitlab/detect.go @@ -40,8 +40,17 @@ func (v *Provider) Detect(req *http.Request, payload string, logger *zap.Sugared if provider.Valid(gitEvent.ObjectAttributes.Action, []string{"open", "reopen", "update"}) { return setLoggerAndProceed(true, "", nil) } - return setLoggerAndProceed(false, fmt.Sprintf("not a merge event we care about: \"%s\"", - gitEvent.ObjectAttributes.Action), nil) + + // on a MR Update only react when there is Oldrev set, since this means + // there is a Push of commit in there + if gitEvent.ObjectAttributes.Action == "update" && gitEvent.ObjectAttributes.OldRev != "" { + return setLoggerAndProceed(true, "", nil) + } + if provider.Valid(gitEvent.ObjectAttributes.Action, []string{"open", "reopen", "close"}) { + return setLoggerAndProceed(true, "", nil) + } + + return setLoggerAndProceed(false, fmt.Sprintf("not a merge event we care about: \"%s\"", gitEvent.ObjectAttributes.Action), nil) case *gitlab.PushEvent, *gitlab.TagEvent: return setLoggerAndProceed(true, "", nil) case *gitlab.MergeCommentEvent: diff --git a/pkg/provider/gitlab/parse_payload.go b/pkg/provider/gitlab/parse_payload.go index 3350a918b..cd05bc9db 100644 --- a/pkg/provider/gitlab/parse_payload.go +++ b/pkg/provider/gitlab/parse_payload.go @@ -69,6 +69,9 @@ func (v *Provider) ParsePayload(_ context.Context, _ *params.Run, request *http. for _, label := range gitEvent.Labels { processedEvent.PullRequestLabel = append(processedEvent.PullRequestLabel, label.Title) } + if gitEvent.ObjectAttributes.Action == "close" { + processedEvent.TriggerTarget = triggertype.PullRequestClosed + } case *gitlab.TagEvent: // GitLab sends same event for both Tag creation and deletion i.e. "Tag Push Hook". // if gitEvent.After is containing all zeros and gitEvent.CheckoutSHA is empty diff --git a/pkg/reconciler/reconciler.go b/pkg/reconciler/reconciler.go index 2c635efd4..60d6f3bdc 100644 --- a/pkg/reconciler/reconciler.go +++ b/pkg/reconciler/reconciler.go @@ -73,7 +73,7 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, pr *tektonv1.PipelineRun return r.queuePipelineRun(ctx, logger, pr) } - if !pr.IsDone() { + if !pr.IsDone() && !pr.IsCancelled() { return nil } diff --git a/test/gitea_access_control_test.go b/test/gitea_access_control_test.go index cc32c444a..57ed23cfc 100644 --- a/test/gitea_access_control_test.go +++ b/test/gitea_access_control_test.go @@ -423,13 +423,13 @@ func TestGiteaPolicyAllowedOwnerFiles(t *testing.T) { BaseRefName: topts.DefaultBranch, } // push OWNERS file to main - scm.PushFilesToRefGit(t, scmOpts, entries) + _ = scm.PushFilesToRefGit(t, scmOpts, entries) scmOpts.TargetRefName = targetRef newyamlFiles := map[string]string{".tekton/pr.yaml": "testdata/pipelinerun.yaml"} newEntries, err := payload.GetEntries(newyamlFiles, topts.TargetNS, topts.DefaultBranch, topts.TargetEvent, map[string]string{}) assert.NilError(t, err) - scm.PushFilesToRefGit(t, scmOpts, newEntries) + _ = scm.PushFilesToRefGit(t, scmOpts, newEntries) npr := tgitea.CreateForkPullRequest(t, topts, allowedCnx, "") waitOpts := twait.Opts{ diff --git a/test/gitea_test.go b/test/gitea_test.go index c864578f8..950ee8ae4 100644 --- a/test/gitea_test.go +++ b/test/gitea_test.go @@ -271,7 +271,7 @@ func TestGiteaConcurrencyExclusivenessMultipleRuns(t *testing.T) { }) assert.NilError(t, err) entries := map[string]string{".tekton/pr.yaml": processed} - scm.PushFilesToRefGit(t, scmOpts, entries) + _ = scm.PushFilesToRefGit(t, scmOpts, entries) processed, err = payload.ApplyTemplate("testdata/pipelinerun-alt.yaml", map[string]string{ "TargetNamespace": topts.TargetNS, @@ -282,7 +282,7 @@ func TestGiteaConcurrencyExclusivenessMultipleRuns(t *testing.T) { }) assert.NilError(t, err) entries = map[string]string{".tekton/pr.yaml": processed} - scm.PushFilesToRefGit(t, scmOpts, entries) + _ = scm.PushFilesToRefGit(t, scmOpts, entries) // loop until we get the status gotPipelineRunPending := false @@ -341,7 +341,7 @@ func TestGiteaRetestAfterPush(t *testing.T) { BaseRefName: topts.DefaultBranch, PushForce: true, } - scm.PushFilesToRefGit(t, scmOpts, entries) + _ = scm.PushFilesToRefGit(t, scmOpts, entries) topts.CheckForStatus = "success" tgitea.WaitForStatus(t, topts, "heads/"+topts.TargetRefName, "", false) } @@ -417,7 +417,7 @@ func TestGiteaConfigCancelInProgress(t *testing.T) { BaseRefName: topts.DefaultBranch, NoCheckOutFromBase: true, } - scm.PushFilesToRefGit(t, scmOpts, entries) + _ = scm.PushFilesToRefGit(t, scmOpts, entries) pr, _, err := topts.GiteaCNX.Client.CreatePullRequest(topts.Opts.Organization, topts.Opts.Repo, gitea.CreatePullRequestOption{ Title: "Test Pull Request - " + targetRef, @@ -458,6 +458,45 @@ func TestGiteaConfigCancelInProgress(t *testing.T) { assert.Equal(t, cancelledPr, 2, "tweo pr should have been canceled") } +func TestGiteaConfigCancelInProgressAfterPRClosed(t *testing.T) { + prmap := map[string]string{".tekton/pr.yaml": "testdata/pipelinerun-cancel-in-progress.yaml"} + topts := &tgitea.TestOpts{ + TargetEvent: triggertype.PullRequest.String(), + YAMLFiles: prmap, + CheckForStatus: "", + ExpectEvents: false, + Regexp: nil, + } + _, f := tgitea.TestPR(t, topts) + defer f() + + time.Sleep(3 * time.Second) // “Evil does not sleep. It waits.” - Galadriel + waitOpts := twait.Opts{ + RepoName: topts.TargetRefName, + Namespace: topts.TargetNS, + MinNumberStatus: 1, + PollTimeout: twait.DefaultTimeout, + TargetSHA: topts.SHA, + } + err := twait.UntilPipelineRunCreated(context.Background(), topts.ParamsRun.Clients, waitOpts) + assert.NilError(t, err) + + closed := gitea.StateClosed + _, _, err = topts.GiteaCNX.Client.EditPullRequest(topts.Opts.Organization, topts.Opts.Repo, topts.PullRequest.Index, gitea.EditPullRequestOption{ + State: &closed, + }) + assert.NilError(t, err) + + topts.ParamsRun.Clients.Log.Info("Waiting 10 seconds to check things has been canceled") + time.Sleep(10 * time.Second) // “Evil does not sleep. It waits.” - Galadriel + + prs, err := topts.ParamsRun.Clients.Tekton.TektonV1().PipelineRuns(topts.TargetNS).List(context.Background(), metav1.ListOptions{}) + assert.NilError(t, err) + assert.Equal(t, len(prs.Items), 1, "should have only one pipelinerun, but we have: %d", len(prs.Items)) + + assert.Equal(t, prs.Items[0].GetStatusCondition().GetCondition(apis.ConditionSucceeded).GetReason(), "Cancelled", "should have been canceled") +} + func TestGiteaPush(t *testing.T) { topts := &tgitea.TestOpts{ Regexp: successRegexp, @@ -985,12 +1024,12 @@ func verifyProvenance(t *testing.T, topts *tgitea.TestOpts, expectedOutput, cNam BaseRefName: topts.DefaultBranch, NoCheckOutFromBase: true, } - scm.PushFilesToRefGit(t, scmOpts, entries) + _ = scm.PushFilesToRefGit(t, scmOpts, entries) prmap = map[string]string{".tekton/notgonnatobetested.yaml": "testdata/pipelinerun-provenance-test.yaml"} entries, err = payload.GetEntries(prmap, topts.TargetNS, topts.DefaultBranch, topts.TargetEvent, map[string]string{}) assert.NilError(t, err) scmOpts.TargetRefName = targetRef - scm.PushFilesToRefGit(t, scmOpts, entries) + _ = scm.PushFilesToRefGit(t, scmOpts, entries) pr, _, err := topts.GiteaCNX.Client.CreatePullRequest(topts.Opts.Organization, targetRef, gitea.CreatePullRequestOption{ Title: "Test Pull Request - " + targetRef, @@ -1068,10 +1107,10 @@ func TestGiteaPushToTagGreedy(t *testing.T) { TargetRefName: topts.DefaultBranch, BaseRefName: topts.DefaultBranch, } - scm.PushFilesToRefGit(t, scmOpts, entries) + _ = scm.PushFilesToRefGit(t, scmOpts, entries) scmOpts.TargetRefName = "refs/tags/v1.0.0" - scm.PushFilesToRefGit(t, scmOpts, map[string]string{"README.md": "hello new version from tag"}) + _ = scm.PushFilesToRefGit(t, scmOpts, map[string]string{"README.md": "hello new version from tag"}) waitOpts := twait.Opts{ RepoName: topts.TargetNS, Namespace: topts.TargetNS, diff --git a/test/github_pullrequest_test.go b/test/github_pullrequest_test.go index dd88e75b2..0aa956f38 100644 --- a/test/github_pullrequest_test.go +++ b/test/github_pullrequest_test.go @@ -13,19 +13,19 @@ import ( "time" "github.com/google/go-github/v66/github" - "github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/keys" - "github.com/openshift-pipelines/pipelines-as-code/pkg/opscomments" + "gotest.tools/v3/assert" + "gotest.tools/v3/assert/cmp" + "gotest.tools/v3/golden" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "knative.dev/pkg/apis" + "github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/keys" + "github.com/openshift-pipelines/pipelines-as-code/pkg/opscomments" "github.com/openshift-pipelines/pipelines-as-code/pkg/params/settings" "github.com/openshift-pipelines/pipelines-as-code/pkg/params/triggertype" tgithub "github.com/openshift-pipelines/pipelines-as-code/test/pkg/github" "github.com/openshift-pipelines/pipelines-as-code/test/pkg/options" - twait "github.com/openshift-pipelines/pipelines-as-code/test/pkg/wait" - "gotest.tools/v3/assert" - "gotest.tools/v3/assert/cmp" - "gotest.tools/v3/golden" ) func TestGithubPullRequest(t *testing.T) { @@ -277,7 +277,7 @@ func TestGithubSecondTestExplicitelyNoMatchedPipelineRun(t *testing.T) { func TestGithubSecondCancelInProgress(t *testing.T) { ctx := context.Background() g := tgithub.PRTest{ - Label: "Github test implicit comment", + Label: "Github cancel in progress", YamlFiles: []string{"testdata/pipelinerun-cancel-in-progress.yaml"}, SecondController: true, NoStatusCheck: true, @@ -342,3 +342,56 @@ func TestGithubSecondCancelInProgress(t *testing.T) { } assert.Assert(t, foundCancelled, "No Pipelines has been found cancedl in NS %s", g.TargetNamespace) } + +func TestGithubSecondCancelInProgressPRClosed(t *testing.T) { + ctx := context.Background() + g := tgithub.PRTest{ + Label: "Github cancel in progress while pr is closed", + YamlFiles: []string{"testdata/pipelinerun-cancel-in-progress.yaml"}, + SecondController: true, + NoStatusCheck: true, + } + g.RunPullRequest(ctx, t) + defer g.TearDown(ctx, t) + + g.Cnx.Clients.Log.Infof("Waiting for the two pipelinerun to be created") + waitOpts := twait.Opts{ + RepoName: g.TargetNamespace, + Namespace: g.TargetNamespace, + MinNumberStatus: 1, + PollTimeout: twait.DefaultTimeout, + TargetSHA: g.SHA, + } + err := twait.UntilPipelineRunCreated(ctx, g.Cnx.Clients, waitOpts) + assert.NilError(t, err) + + g.Cnx.Clients.Log.Infof("Closing the PullRequest") + _, _, err = g.Provider.Client.PullRequests.Edit(ctx, g.Options.Organization, g.Options.Repo, g.PRNumber, &github.PullRequest{ + State: github.String("closed"), + }) + assert.NilError(t, err) + + g.Cnx.Clients.Log.Infof("Sleeping for 10 seconds to let the pipelinerun to be canceled") + time.Sleep(10 * time.Second) + + g.Cnx.Clients.Log.Infof("Checking that the pipelinerun has been canceled") + + prs, err := g.Cnx.Clients.Tekton.TektonV1().PipelineRuns(g.TargetNamespace).List(context.Background(), metav1.ListOptions{}) + assert.NilError(t, err) + assert.Equal(t, len(prs.Items), 1, "should have only one pipelinerun, but we have: %d", len(prs.Items)) + + assert.Equal(t, prs.Items[0].GetStatusCondition().GetCondition(apis.ConditionSucceeded).GetReason(), "Cancelled", "should have been canceled") + + res, resp, err := g.Provider.Client.Checks.ListCheckRunsForRef(ctx, g.Options.Organization, g.Options.Repo, g.SHA, &github.ListCheckRunsOptions{ + AppID: g.Provider.ApplicationID, + ListOptions: github.ListOptions{}, + }) + assert.NilError(t, err) + assert.Equal(t, resp.StatusCode, 200) + + assert.Equal(t, res.CheckRuns[0].GetConclusion(), "cancelled") +} + +// Local Variables: +// compile-command: "go test -tags=e2e -v -info TestGithubPullRequest$ ." +// End: diff --git a/test/github_push_retest_test.go b/test/github_push_retest_test.go index cc15c7f34..6f5a4aae0 100644 --- a/test/github_push_retest_test.go +++ b/test/github_push_retest_test.go @@ -122,7 +122,7 @@ func TestGithubPushRequestGitOpsCommentCancel(t *testing.T) { // this went too fast so at least we check it was requested for it if !cancelled { - reg := regexp.MustCompile(".*pipelinerun.*is done.*skipping cancellation.*") + reg := regexp.MustCompile(".*pipelinerun.*skipping cancelling pipelinerun.*on-push.*already done.*") err = twait.RegexpMatchingInControllerLog(ctx, g.Cnx, *reg, 10, "controller", github.Int64(20)) if err != nil { t.Errorf("neither a cancelled pipelinerun in repo status or a request to skip the cancellation in the controller log was found: %s", err.Error()) diff --git a/test/gitlab_incoming_webhook_test.go b/test/gitlab_incoming_webhook_test.go index 0ba0e7ee9..bd990fbcd 100644 --- a/test/gitlab_incoming_webhook_test.go +++ b/test/gitlab_incoming_webhook_test.go @@ -76,7 +76,7 @@ func TestGitlabIncomingWebhook(t *testing.T) { BaseRefName: projectinfo.DefaultBranch, CommitTitle: title, } - scm.PushFilesToRefGit(t, scmOpts, entries) + _ = scm.PushFilesToRefGit(t, scmOpts, entries) runcnx.Clients.Log.Infof("Branch %s has been created and pushed with files", randomedString) url := fmt.Sprintf("%s/incoming?repository=%s&branch=%s&pipelinerun=%s&secret=%s", opts.ControllerURL, diff --git a/test/gitlab_merge_request_test.go b/test/gitlab_merge_request_test.go index 621ed6b5b..a14d24938 100644 --- a/test/gitlab_merge_request_test.go +++ b/test/gitlab_merge_request_test.go @@ -25,6 +25,7 @@ import ( "gotest.tools/v3/assert" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "knative.dev/pkg/apis" ) func TestGitlabMergeRequest(t *testing.T) { @@ -65,7 +66,7 @@ func TestGitlabMergeRequest(t *testing.T) { TargetRefName: targetRefName, BaseRefName: projectinfo.DefaultBranch, } - scm.PushFilesToRefGit(t, scmOpts, entries) + _ = scm.PushFilesToRefGit(t, scmOpts, entries) runcnx.Clients.Log.Infof("Branch %s has been created and pushed with files", targetRefName) mrTitle := "TestMergeRequest - " + targetRefName @@ -81,7 +82,7 @@ func TestGitlabMergeRequest(t *testing.T) { triggertype.PullRequest.String(), map[string]string{}) assert.NilError(t, err) scmOpts.BaseRefName = targetRefName - scm.PushFilesToRefGit(t, scmOpts, entries) + _ = scm.PushFilesToRefGit(t, scmOpts, entries) sopt := twait.SuccessOpt{ Title: mrTitle, @@ -232,7 +233,7 @@ func TestGitlabOnComment(t *testing.T) { TargetRefName: targetRefName, BaseRefName: projectinfo.DefaultBranch, } - scm.PushFilesToRefGit(t, scmOpts, entries) + _ = scm.PushFilesToRefGit(t, scmOpts, entries) runcnx.Clients.Log.Infof("Branch %s has been created and pushed with files", targetRefName) mrTitle := "TestMergeRequest - " + targetRefName @@ -276,6 +277,79 @@ func TestGitlabOnComment(t *testing.T) { assert.NilError(t, err) } +func TestGitlabCancelInProgressOnPRClose(t *testing.T) { + targetNS := names.SimpleNameGenerator.RestrictLengthWithRandomSuffix("pac-e2e-ns") + ctx := context.Background() + runcnx, opts, glprovider, err := tgitlab.Setup(ctx) + assert.NilError(t, err) + ctx, err = cctx.GetControllerCtxInfo(ctx, runcnx) + assert.NilError(t, err) + runcnx.Clients.Log.Info("Testing Gitlab cancel in progress on pr close") + projectinfo, resp, err := glprovider.Client.Projects.GetProject(opts.ProjectID, nil) + assert.NilError(t, err) + if resp != nil && resp.StatusCode == http.StatusNotFound { + t.Errorf("Repository %s not found in %s", opts.Organization, opts.Repo) + } + + err = tgitlab.CreateCRD(ctx, projectinfo, runcnx, targetNS, nil) + assert.NilError(t, err) + + entries, err := payload.GetEntries(map[string]string{ + ".tekton/in-progress.yaml": "testdata/pipelinerun-cancel-in-progress.yaml", + }, targetNS, projectinfo.DefaultBranch, + triggertype.PullRequest.String(), map[string]string{}) + assert.NilError(t, err) + targetRefName := names.SimpleNameGenerator.RestrictLengthWithRandomSuffix("pac-e2e-test") + + gitCloneURL, err := scm.MakeGitCloneURL(projectinfo.WebURL, opts.UserName, opts.Password) + assert.NilError(t, err) + mrTitle := "TestCancelInProgress - " + targetRefName + scmOpts := &scm.Opts{ + GitURL: gitCloneURL, + Log: runcnx.Clients.Log, + WebURL: projectinfo.WebURL, + TargetRefName: targetRefName, + BaseRefName: projectinfo.DefaultBranch, + CommitTitle: mrTitle, + } + + sha := scm.PushFilesToRefGit(t, scmOpts, entries) + runcnx.Clients.Log.Infof("Branch %s has been created and pushed with files", targetRefName) + mrID, err := tgitlab.CreateMR(glprovider.Client, opts.ProjectID, targetRefName, projectinfo.DefaultBranch, mrTitle) + assert.NilError(t, err) + runcnx.Clients.Log.Infof("MergeRequest %s/-/merge_requests/%d has been created", projectinfo.WebURL, mrID) + defer tgitlab.TearDown(ctx, t, runcnx, glprovider, mrID, targetRefName, targetNS, opts.ProjectID) + + runcnx.Clients.Log.Infof("Waiting for the two pipelinerun to be created") + waitOpts := twait.Opts{ + RepoName: targetNS, + Namespace: targetNS, + MinNumberStatus: 1, + PollTimeout: twait.DefaultTimeout, + TargetSHA: sha, + } + err = twait.UntilPipelineRunCreated(ctx, runcnx.Clients, waitOpts) + assert.NilError(t, err) + _, _, err = glprovider.Client.MergeRequests.UpdateMergeRequest(opts.ProjectID, mrID, &clientGitlab.UpdateMergeRequestOptions{ + StateEvent: clientGitlab.Ptr("close"), + }) + assert.NilError(t, err) + + runcnx.Clients.Log.Infof("Sleeping for 10 seconds to let the pipelinerun to be canceled") + time.Sleep(10 * time.Second) + + prs, err := runcnx.Clients.Tekton.TektonV1().PipelineRuns(targetNS).List(context.Background(), metav1.ListOptions{}) + assert.NilError(t, err) + assert.Equal(t, len(prs.Items), 1, "should have only one pipelinerun, but we have: %d", len(prs.Items)) + assert.Equal(t, prs.Items[0].GetStatusCondition().GetCondition(apis.ConditionSucceeded).GetReason(), "Cancelled", "should have been canceled") + + repo, err := runcnx.Clients.PipelineAsCode.PipelinesascodeV1alpha1().Repositories(targetNS).Get(ctx, targetNS, metav1.GetOptions{}) + assert.NilError(t, err) + + laststatus := repo.Status[len(repo.Status)-1] + assert.Equal(t, "Cancelled", laststatus.Conditions[0].Reason) +} + func TestGitlabIssueGitopsComment(t *testing.T) { targetNS := names.SimpleNameGenerator.RestrictLengthWithRandomSuffix("pac-e2e-ns") ctx := context.Background() @@ -312,7 +386,7 @@ func TestGitlabIssueGitopsComment(t *testing.T) { BaseRefName: projectinfo.DefaultBranch, CommitTitle: mrTitle, } - scm.PushFilesToRefGit(t, scmOpts, entries) + _ = scm.PushFilesToRefGit(t, scmOpts, entries) runcnx.Clients.Log.Infof("Branch %s has been created and pushed with files", targetRefName) mrID, err := tgitlab.CreateMR(glprovider.Client, opts.ProjectID, targetRefName, projectinfo.DefaultBranch, mrTitle) diff --git a/test/pkg/gitea/scm.go b/test/pkg/gitea/scm.go index b26812f29..c51f75677 100644 --- a/test/pkg/gitea/scm.go +++ b/test/pkg/gitea/scm.go @@ -264,7 +264,7 @@ func PushToPullRequest(t *testing.T, topts *TestOpts, secondcnx pgitea.Provider, TargetRefName: topts.TargetRefName, BaseRefName: topts.DefaultBranch, } - scm.PushFilesToRefGit(t, scmOpts, entries) + _ = scm.PushFilesToRefGit(t, scmOpts, entries) } func CreateAccess(topts *TestOpts, touser, accessMode string) error { diff --git a/test/pkg/gitea/test.go b/test/pkg/gitea/test.go index cc51ed2aa..15ec7d3bb 100644 --- a/test/pkg/gitea/test.go +++ b/test/pkg/gitea/test.go @@ -61,6 +61,7 @@ type TestOpts struct { ExpectEvents bool InternalGiteaURL string Token string + SHA string FileChanges []scm.FileChange } @@ -199,7 +200,7 @@ func TestPR(t *testing.T, topts *TestOpts) (context.Context, func()) { TargetRefName: topts.TargetRefName, BaseRefName: topts.DefaultBranch, } - scm.PushFilesToRefGit(t, scmOpts, entries) + topts.SHA = scm.PushFilesToRefGit(t, scmOpts, entries) topts.ParamsRun.Clients.Log.Infof("Creating PullRequest") for i := 0; i < 5; i++ { diff --git a/test/pkg/scm/scm.go b/test/pkg/scm/scm.go index 8e0ae713b..e1ce8d458 100644 --- a/test/pkg/scm/scm.go +++ b/test/pkg/scm/scm.go @@ -68,7 +68,7 @@ func gitPushPullRetry(t *testing.T, opts *Opts, path string) { } } -func PushFilesToRefGit(t *testing.T, opts *Opts, entries map[string]string) { +func PushFilesToRefGit(t *testing.T, opts *Opts, entries map[string]string) string { tmpdir := fs.NewDir(t, t.Name()) defer (func() { if os.Getenv("TEST_NOCLEANUP") == "" { @@ -125,7 +125,12 @@ func PushFilesToRefGit(t *testing.T, opts *Opts, entries map[string]string) { assert.NilError(t, err) } + // get sha + sha, err := git.RunGit(path, "rev-parse", "HEAD") + assert.NilError(t, err) + gitPushPullRetry(t, opts, path) + return sha } func ChangeFilesRefGit(t *testing.T, opts *Opts, fileChanges []FileChange) { diff --git a/test/testdata/pipelinerun-cancel-in-progress.yaml b/test/testdata/pipelinerun-cancel-in-progress.yaml index 84642b416..e0a82da0c 100644 --- a/test/testdata/pipelinerun-cancel-in-progress.yaml +++ b/test/testdata/pipelinerun-cancel-in-progress.yaml @@ -17,4 +17,4 @@ spec: - name: task image: registry.access.redhat.com/ubi9/ubi-micro script: | - sleep 60 + sleep 120 From f08617877107ceef9168d0dc92d04667c494c067 Mon Sep 17 00:00:00 2001 From: Chmouel Boudjnah Date: Fri, 20 Dec 2024 18:30:26 +0100 Subject: [PATCH 2/2] add support for bitbucketcloud TBC on Monday the christmas FML Signed-off-by: Chmouel Boudjnah --- pkg/provider/bitbucketcloud/detect.go | 15 ++++- pkg/provider/bitbucketcloud/parse_payload.go | 11 ++-- pkg/provider/bitbucketcloud/types/types.go | 1 + test/bitbucket_cloud_pullrequest_test.go | 65 ++++++++++++++++++-- test/pkg/bitbucketcloud/pr.go | 12 ++-- 5 files changed, 85 insertions(+), 19 deletions(-) diff --git a/pkg/provider/bitbucketcloud/detect.go b/pkg/provider/bitbucketcloud/detect.go index 7726ca745..2f96b8233 100644 --- a/pkg/provider/bitbucketcloud/detect.go +++ b/pkg/provider/bitbucketcloud/detect.go @@ -10,6 +10,12 @@ import ( "go.uber.org/zap" ) +var ( + pullRequestsClosed = []string{"pullrequest:closed", "pullrequest:fulfilled", "pullrequest:rejected"} + pullRequestsCreated = []string{"pullrequest:created", "pullrequest:updated"} + pullRequestsCommentCreated = []string{"pullrequest:comment_created"} +) + func (v *Provider) Detect(req *http.Request, payload string, logger *zap.SugaredLogger) (bool, bool, *zap.SugaredLogger, string, error) { isBitCloud := false reqHeader := req.Header @@ -37,10 +43,15 @@ func (v *Provider) Detect(req *http.Request, payload string, logger *zap.Sugared switch e := eventInt.(type) { case *types.PullRequestEvent: - if provider.Valid(event, []string{"pullrequest:created", "pullrequest:updated"}) { + if provider.Valid(event, pullRequestsClosed) { + return setLoggerAndProceed(true, "", nil) + } + + if provider.Valid(event, pullRequestsCreated) { return setLoggerAndProceed(true, "", nil) } - if provider.Valid(event, []string{"pullrequest:comment_created"}) { + + if provider.Valid(event, pullRequestsCommentCreated) { if provider.IsTestRetestComment(e.Comment.Content.Raw) { return setLoggerAndProceed(true, "", nil) } diff --git a/pkg/provider/bitbucketcloud/parse_payload.go b/pkg/provider/bitbucketcloud/parse_payload.go index df25a5858..c45a4e356 100644 --- a/pkg/provider/bitbucketcloud/parse_payload.go +++ b/pkg/provider/bitbucketcloud/parse_payload.go @@ -125,15 +125,16 @@ func (v *Provider) ParsePayload(ctx context.Context, run *params.Run, request *h } processedEvent.Event = eventInt - switch e := eventInt.(type) { case *types.PullRequestEvent: - if provider.Valid(event, []string{"pullrequest:created", "pullrequest:updated"}) { - processedEvent.TriggerTarget = triggertype.PullRequest + processedEvent.TriggerTarget = triggertype.PullRequest + switch { + case provider.Valid(event, pullRequestsCreated): processedEvent.EventType = triggertype.PullRequest.String() - } else if provider.Valid(event, []string{"pullrequest:comment_created"}) { - processedEvent.TriggerTarget = triggertype.PullRequest + case provider.Valid(event, pullRequestsCommentCreated): opscomments.SetEventTypeAndTargetPR(processedEvent, e.Comment.Content.Raw) + case provider.Valid(event, pullRequestsClosed): + processedEvent.EventType = string(triggertype.PullRequestClosed) } processedEvent.Organization = e.Repository.Workspace.Slug processedEvent.Repository = strings.Split(e.Repository.FullName, "/")[1] diff --git a/pkg/provider/bitbucketcloud/types/types.go b/pkg/provider/bitbucketcloud/types/types.go index 29a2354c0..dd34db076 100644 --- a/pkg/provider/bitbucketcloud/types/types.go +++ b/pkg/provider/bitbucketcloud/types/types.go @@ -55,6 +55,7 @@ type PullRequest struct { ID int `json:"id"` Links Links Title string `json:"title"` + State string `json:"state"` } type PullRequestEvent struct { diff --git a/test/bitbucket_cloud_pullrequest_test.go b/test/bitbucket_cloud_pullrequest_test.go index d51e4f025..d54094243 100644 --- a/test/bitbucket_cloud_pullrequest_test.go +++ b/test/bitbucket_cloud_pullrequest_test.go @@ -5,11 +5,15 @@ package test import ( "context" + "fmt" "testing" + "github.com/ktrysmt/go-bitbucket" "github.com/openshift-pipelines/pipelines-as-code/pkg/params/triggertype" tbb "github.com/openshift-pipelines/pipelines-as-code/test/pkg/bitbucketcloud" - "github.com/openshift-pipelines/pipelines-as-code/test/pkg/wait" + "github.com/openshift-pipelines/pipelines-as-code/test/pkg/options" + "github.com/openshift-pipelines/pipelines-as-code/test/pkg/payload" + twait "github.com/openshift-pipelines/pipelines-as-code/test/pkg/wait" "github.com/tektoncd/pipeline/pkg/names" "gotest.tools/v3/assert" ) @@ -27,13 +31,18 @@ func TestBitbucketCloudPullRequest(t *testing.T) { targetRefName := names.SimpleNameGenerator.RestrictLengthWithRandomSuffix("pac-e2e-test") title := "TestPullRequest - " + targetRefName - pr, repobranch := tbb.MakePR(t, bprovider, runcnx, bcrepo, opts, title, targetNS, targetRefName) + entries, err := payload.GetEntries( + map[string]string{".tekton/pipelinerun.yaml": "testdata/pipelinerun.yaml"}, + targetNS, options.MainBranch, triggertype.PullRequest.String(), map[string]string{}) + assert.NilError(t, err) + + pr, repobranch := tbb.MakePR(t, bprovider, runcnx, bcrepo, opts, title, targetRefName, entries) defer tbb.TearDown(ctx, t, runcnx, bprovider, opts, pr.ID, targetRefName, targetNS) hash, ok := repobranch.Target["hash"].(string) assert.Assert(t, ok) - sopt := wait.SuccessOpt{ + sopt := twait.SuccessOpt{ TargetNS: targetNS, OnEvent: triggertype.PullRequest.String(), NumberofPRMatch: 1, @@ -41,7 +50,55 @@ func TestBitbucketCloudPullRequest(t *testing.T) { Title: title, MinNumberStatus: 1, } - wait.Succeeded(ctx, t, runcnx, opts, sopt) + twait.Succeeded(ctx, t, runcnx, opts, sopt) +} + +func TestBitbucketCloudPullRequestCancelInProgressMerged(t *testing.T) { + targetNS := names.SimpleNameGenerator.RestrictLengthWithRandomSuffix("pac-e2e-ns") + ctx := context.Background() + + runcnx, opts, bprovider, err := tbb.Setup(ctx) + if err != nil { + t.Skip(err.Error()) + return + } + bcrepo := tbb.CreateCRD(ctx, t, bprovider, runcnx, opts, targetNS) + targetRefName := names.SimpleNameGenerator.RestrictLengthWithRandomSuffix("pac-e2e-test") + title := "TestPullRequest - " + targetRefName + + entries, err := payload.GetEntries( + map[string]string{".tekton/pipelinerun-on-label.yaml": "testdata/pipelinerun-cancel-in-progress.yaml"}, + targetNS, options.MainBranch, triggertype.PullRequest.String(), map[string]string{}) + assert.NilError(t, err) + + pr, repobranch := tbb.MakePR(t, bprovider, runcnx, bcrepo, opts, title, targetRefName, entries) + defer tbb.TearDown(ctx, t, runcnx, bprovider, opts, pr.ID, targetRefName, targetNS) + + sha, ok := repobranch.Target["hash"].(string) + assert.Assert(t, ok) + + waitOpts := twait.Opts{ + RepoName: targetNS, + Namespace: targetNS, + MinNumberStatus: 1, + PollTimeout: twait.DefaultTimeout, + TargetSHA: sha, + } + err = twait.UntilPipelineRunCreated(ctx, runcnx.Clients, waitOpts) + assert.NilError(t, err) + + po := &bitbucket.PullRequestsOptions{ + RepoSlug: opts.Repo, + Owner: opts.Organization, + ID: fmt.Sprintf("%d", pr.ID), + } + _, err = bprovider.Client.Repositories.PullRequests.Decline(po) + assert.NilError(t, err) + + // _, _, err = glprovider.Client.MergeRequests.UpdateMergeRequest(opts.ProjectID, mrID, &clientGitlab.UpdateMergeRequestOptions{ + // StateEvent: clientGitlab.Ptr("close"), + // }) + // assert.NilError(t, err) } // Local Variables: diff --git a/test/pkg/bitbucketcloud/pr.go b/test/pkg/bitbucketcloud/pr.go index 3059d6f2d..8fc858ac4 100644 --- a/test/pkg/bitbucketcloud/pr.go +++ b/test/pkg/bitbucketcloud/pr.go @@ -7,27 +7,23 @@ import ( "github.com/ktrysmt/go-bitbucket" "github.com/mitchellh/mapstructure" "github.com/openshift-pipelines/pipelines-as-code/pkg/params" - "github.com/openshift-pipelines/pipelines-as-code/pkg/params/triggertype" "github.com/openshift-pipelines/pipelines-as-code/pkg/provider/bitbucketcloud" "github.com/openshift-pipelines/pipelines-as-code/pkg/provider/bitbucketcloud/types" "github.com/openshift-pipelines/pipelines-as-code/test/pkg/options" - "github.com/openshift-pipelines/pipelines-as-code/test/pkg/payload" "gotest.tools/v3/assert" "gotest.tools/v3/fs" ) -func MakePR(t *testing.T, bprovider bitbucketcloud.Provider, runcnx *params.Run, bcrepo *bitbucket.Repository, opts options.E2E, title, targetNS, targetRefName string) (*types.PullRequest, *bitbucket.RepositoryBranch) { +func MakePR(t *testing.T, bprovider bitbucketcloud.Provider, runcnx *params.Run, bcrepo *bitbucket.Repository, opts options.E2E, title, targetRefName string, + entries map[string]string, +) (*types.PullRequest, *bitbucket.RepositoryBranch) { commitAuthor := "OpenShift Pipelines E2E test" commitEmail := "e2e-pipelines@redhat.com" - entries, err := payload.GetEntries( - map[string]string{".tekton/pipelinerun.yaml": "testdata/pipelinerun.yaml"}, - targetNS, options.MainBranch, triggertype.PullRequest.String(), map[string]string{}) - assert.NilError(t, err) tmpfile := fs.NewFile(t, "pipelinerun", fs.WithContent(entries[".tekton/pipelinerun.yaml"])) defer tmpfile.Remove() - err = bprovider.Client.Workspaces.Repositories.Repository.WriteFileBlob(&bitbucket.RepositoryBlobWriteOptions{ + err := bprovider.Client.Workspaces.Repositories.Repository.WriteFileBlob(&bitbucket.RepositoryBlobWriteOptions{ Owner: opts.Organization, RepoSlug: opts.Repo, Files: []bitbucket.File{