fix: PipelineRun cancel-in-progress issue when generateName is used

When PipelineRun is specified using generateName field, PAC unable
to filter out and cancel PipelineRuns based on the generateName field
due to trailing hyphen in the generateName field, only alphanumeric
characters are allowed at begining and end of Kubernetes resources labels
thus getting error when filtering PipelineRuns based on originalPRName
label. Thus this PR uses originalPRName label instead of annotation to
filter the PipelineRun.

https://issues.redhat.com/browse/SRVKP-7337

Signed-off-by: Zaki Shaikh <[email protected]>

Author: zakisk

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