Merge branch 'main' into report-yaml-error-properly-as-comment

Author: chmouel

pkg/matcher/annotation_matcher.go

@@ -226,6 +226,15 @@ func MatchPipelinerunByAnnotation(ctx context.Context, logger *zap.SugaredLogger
 		if event.EventType == opscomments.NoOpsCommentEventType.String() || event.EventType == opscomments.OnCommentEventType.String() {
 			continue
 		}
+
+		// If the event is a pull_request and the event type is label_update, but the PipelineRun
+		// does not contain an 'on-label' annotation, do not match this PipelineRun, as it is not intended for this event.
+		_, ok := prun.GetObjectMeta().GetAnnotations()[keys.OnLabel]
+		if event.TriggerTarget == triggertype.PullRequest && event.EventType == string(triggertype.LabelUpdate) && !ok {
+			logger.Infof("label update event, PipelineRun %s does not have a on-label for any of those labels: %s", prName, strings.Join(event.PullRequestLabel, "|"))
+			continue
+		}
+
 		if celExpr, ok := prun.GetObjectMeta().GetAnnotations()[keys.OnCelExpression]; ok {
 			out, err := celEvaluate(ctx, celExpr, event, vcx)
 			if err != nil {
@@ -278,9 +287,6 @@ func MatchPipelinerunByAnnotation(ctx context.Context, logger *zap.SugaredLogger
 				}
 				logger.Infof("matched PipelineRun with name: %s, annotation Label: %q", prName, key)
 				prMatch.Config["label"] = key
-			} else if event.EventType == string(triggertype.LabelUpdate) {
-				logger.Infof("label update event, PipelineRun %s does not have a on-label for any of those labels: %s", prName, strings.Join(event.PullRequestLabel, "|"))
-				continue
 			}
 
 			if key, ok := prun.GetObjectMeta().GetAnnotations()[keys.OnPathChangeIgnore]; ok {

pkg/matcher/annotation_matcher_test.go

@@ -1640,6 +1640,37 @@ func TestMatchPipelinerunByAnnotation(t *testing.T) {
 				"matching pipelineruns to event: URL=https://hello/moto, target-branch=main, source-branch=source, target-event=pull_request, labels=documentation, pull-request=10",
 			},
 		},
+		{
+			name: "no-on-label-annotation-on-pr",
+			args: args{
+				pruns: []*tektonv1.PipelineRun{
+					{
+						ObjectMeta: metav1.ObjectMeta{
+							Name: "pipeline-label",
+							Annotations: map[string]string{
+								keys.OnEvent:        "[pull_request]",
+								keys.OnTargetBranch: "[main]",
+							},
+						},
+					},
+					pipelineGood,
+				},
+				runevent: info.Event{
+					URL:               "https://hello/moto",
+					TriggerTarget:     triggertype.PullRequest,
+					EventType:         string(triggertype.LabelUpdate),
+					HeadBranch:        "source",
+					BaseBranch:        "main",
+					PullRequestNumber: 10,
+					PullRequestLabel:  []string{"documentation"},
+				},
+			},
+			wantErr: true,
+			wantLog: []string{
+				"label update event, PipelineRun pipeline-label does not have a on-label for any of those labels: documentation",
+				"label update event, PipelineRun pipeline-good does not have a on-label for any of those labels: documentation",
+			},
+		},
 		{
 			name: "match-on-comment",
 			args: args{

pkg/pipelineascode/cancel_pipelineruns.go

@@ -28,9 +28,9 @@ var cancelMergePatch = map[string]any{
 	},
 }
 
-// cancelAllInProgressBelongingToPullRequest cancels all in-progress PipelineRuns
+// cancelAllInProgressBelongingToClosedPullRequest cancels all in-progress PipelineRuns
 // that belong to a specific pull request in the given repository.
-func (p *PacRun) cancelAllInProgressBelongingToPullRequest(ctx context.Context, repo *v1alpha1.Repository) error {
+func (p *PacRun) cancelAllInProgressBelongingToClosedPullRequest(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)),
@@ -70,7 +70,9 @@ func (p *PacRun) cancelInProgressMatchingPR(ctx context.Context, matchPR *tekton
 		return nil
 	}
 
-	prName, ok := matchPR.GetAnnotations()[keys.OriginalPRName]
+	// As PipelineRuns are filtered by name, OriginalPRName should be taken from
+	// labels instead of annotations because of constraints imposed by kube API.
+	prName, ok := matchPR.GetLabels()[keys.OriginalPRName]
 	if !ok {
 		return nil
 	}

pkg/pipelineascode/cancel_pipelineruns_test.go

@@ -354,7 +354,7 @@ func TestCancelInProgressMatchingPR(t *testing.T) {
 					ObjectMeta: metav1.ObjectMeta{
 						Name:      "pr-foo",
 						Namespace: "foo",
-						Labels:    fooRepoLabels,
+						Labels:    map[string]string{},
 						Annotations: map[string]string{
 							keys.CancelInProgress: "true",
 						},
@@ -523,9 +523,10 @@ func TestCancelInProgressMatchingPR(t *testing.T) {
 						Namespace: "foo",
 						Labels:    fooRepoLabels,
 						Annotations: map[string]string{
-							keys.CancelInProgress: "true", keys.OriginalPRName: "pr-foo",
-							keys.Repository:   "foo",
-							keys.SourceBranch: "head",
+							keys.CancelInProgress: "true",
+							keys.OriginalPRName:   "pr-foo",
+							keys.Repository:       "foo",
+							keys.SourceBranch:     "head",
 						},
 					},
 					Spec: pipelinev1.PipelineRunSpec{},
@@ -537,6 +538,54 @@ func TestCancelInProgressMatchingPR(t *testing.T) {
 			},
 			wantLog: "cancel-in-progress: cancelling pipelinerun foo/",
 		},
+		{
+			name: "match/cancel in progress on PipelineRun generateName",
+			event: &info.Event{
+				Repository:        "foo",
+				SHA:               "foosha",
+				HeadBranch:        "head",
+				EventType:         string(triggertype.PullRequest),
+				TriggerTarget:     triggertype.PullRequest,
+				PullRequestNumber: pullReqNumber,
+			},
+			pipelineRuns: []*pipelinev1.PipelineRun{
+				{
+					ObjectMeta: metav1.ObjectMeta{
+						GenerateName: "pr-foo-",
+						Name:         "pr-foo-1",
+						Namespace:    "foo",
+						Labels:       fooRepoLabels,
+						Annotations: map[string]string{
+							keys.CancelInProgress: "true",
+							keys.OriginalPRName:   "pr-foo",
+							keys.Repository:       "foo",
+							keys.SourceBranch:     "head",
+						},
+					},
+					Spec: pipelinev1.PipelineRunSpec{},
+				},
+				{
+					ObjectMeta: metav1.ObjectMeta{
+						GenerateName: "pr-foo-",
+						Name:         "pr-foo-2",
+						Namespace:    "foo",
+						Labels:       fooRepoLabels,
+						Annotations: map[string]string{
+							keys.CancelInProgress: "true",
+							keys.OriginalPRName:   "pr-foo",
+							keys.Repository:       "foo",
+							keys.SourceBranch:     "head",
+						},
+					},
+					Spec: pipelinev1.PipelineRunSpec{},
+				},
+			},
+			repo: fooRepo,
+			cancelledPipelineRuns: map[string]bool{
+				"pr-foo-2": true,
+			},
+			wantLog: "cancel-in-progress: cancelling pipelinerun foo/pr-foo-2",
+		},
 		{
 			name: "match/cancel in progress from /retest",
 			event: &info.Event{
@@ -889,7 +938,7 @@ func TestCancelAllInProgressBelongingToPullRequest(t *testing.T) {
 				},
 			}
 			pac := NewPacs(tt.event, nil, cs, &info.PacOpts{}, nil, logger, nil)
-			err := pac.cancelAllInProgressBelongingToPullRequest(ctx, tt.repo)
+			err := pac.cancelAllInProgressBelongingToClosedPullRequest(ctx, tt.repo)
 			assert.NilError(t, err)
 
 			got, err := cs.Clients.Tekton.TektonV1().PipelineRuns("foo").List(ctx, metav1.ListOptions{})

pkg/pipelineascode/pipelineascode.go

@@ -58,7 +58,7 @@ 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 {
+		if err := p.cancelAllInProgressBelongingToClosedPullRequest(ctx, repo); err != nil {
 			return fmt.Errorf("error cancelling in progress pipelineRuns belonging to pull request %d: %w", p.event.PullRequestNumber, err)
 		}
 		return nil

test/github_pullrequest_test.go

@@ -386,6 +386,32 @@ func TestGithubSecondCancelInProgressPRClosed(t *testing.T) {
 	assert.Equal(t, res.CheckRuns[0].GetConclusion(), "cancelled")
 }
 
+func TestGithubPullRequestNoOnLabelAnnotation(t *testing.T) {
+	ctx := context.Background()
+	g := &tgithub.PRTest{
+		Label:     "Github PullRequest",
+		YamlFiles: []string{"testdata/pipelinerun-pr-cel-expression.yaml"},
+	}
+	g.RunPullRequest(ctx, t)
+	defer g.TearDown(ctx, t)
+
+	g.Cnx.Clients.Log.Infof("Creating a label bug on PullRequest")
+	_, _, err := g.Provider.Client.Issues.AddLabelsToIssue(ctx,
+		g.Options.Organization,
+		g.Options.Repo, g.PRNumber,
+		[]string{"bug"})
+	assert.NilError(t, err)
+
+	// let's wait 10 secs and check every second that a PipelineRun is created or not.
+	for i := 0; i < 10; i++ {
+		prs, err := g.Cnx.Clients.Tekton.TektonV1().PipelineRuns(g.TargetNamespace).List(ctx, metav1.ListOptions{})
+		assert.NilError(t, err)
+		// after adding a label on the PR we need to make sure that it doesn't trigger another PipelineRun.
+		assert.Equal(t, len(prs.Items), 1)
+		time.Sleep(1 * time.Second)
+	}
+}
+
 // Local Variables:
 // compile-command: "go test -tags=e2e -v -info TestGithubPullRequest$ ."
 // End:

test/gitlab_merge_request_test.go

@@ -421,15 +421,17 @@ func TestGitlabCancelInProgressOnPRClose(t *testing.T) {
 	})
 	assert.NilError(t, err)
 
-	runcnx.Clients.Log.Infof("Sleeping for 10 seconds to let the pipelinerun to be canceled")
-	time.Sleep(10 * time.Second)
+	err = twait.UntilPipelineRunHasReason(ctx, runcnx.Clients, v1.PipelineRunReasonCancelled, waitOpts)
+	assert.NilError(t, err)
 
 	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{})
+	// failing on `true` condition because for cancelled PipelineRun we want `false` condition.
+	waitOpts.FailOnRepoCondition = corev1.ConditionTrue
+	repo, err := twait.UntilRepositoryUpdated(ctx, runcnx.Clients, waitOpts)
 	assert.NilError(t, err)
 
 	laststatus := repo.Status[len(repo.Status)-1]

test/pkg/wait/wait.go

@@ -16,12 +16,13 @@ import (
 )
 
 type Opts struct {
-	RepoName        string
-	Namespace       string
-	MinNumberStatus int
-	PollTimeout     time.Duration
-	AdminNS         string
-	TargetSHA       string
+	RepoName            string
+	Namespace           string
+	MinNumberStatus     int
+	PollTimeout         time.Duration
+	AdminNS             string
+	TargetSHA           string
+	FailOnRepoCondition corev1.ConditionStatus
 }
 
 func UntilMinPRAppeared(ctx context.Context, clients clients.Clients, opts Opts, minNumber int) error {
@@ -58,7 +59,11 @@ func UntilRepositoryUpdated(ctx context.Context, clients clients.Clients, opts O
 		}
 		if len(prs.Items) > 0 {
 			prConditions := prs.Items[0].Status.Conditions
-			if len(prConditions) != 0 && prConditions[0].Status == corev1.ConditionFalse {
+			if opts.FailOnRepoCondition == "" {
+				opts.FailOnRepoCondition = corev1.ConditionFalse
+			}
+
+			if len(prConditions) != 0 && prConditions[0].Status == opts.FailOnRepoCondition {
 				return true, fmt.Errorf("pipelinerun has failed")
 			}
 		}

test/testdata/pipelinerun-pr-cel-expression.yaml

@@ -0,0 +1,18 @@
+---
+apiVersion: tekton.dev/v1beta1
+kind: PipelineRun
+metadata:
+  name: "\\ .PipelineName //"
+  annotations:
+    pipelinesascode.tekton.dev/target-namespace: "\\ .TargetNamespace //"
+    pipelinesascode.tekton.dev/on-cel-expression: event == "\\ .TargetEvent //" && target_branch == "\\ .TargetBranch //"
+spec:
+  pipelineSpec:
+    tasks:
+      - name: task
+        displayName: "The Task name is Task"
+        taskSpec:
+          steps:
+            - name: task
+              image: registry.access.redhat.com/ubi9/ubi-micro
+              command: ["/bin/echo", "HELLOMOTO"]