enhance: add PipelineRun name in pr json marshalling

when a PipelineRun fails json marshalling in match.go
we don't know which PipelineRun has syntax error if
there are multiple PipelineRuns. this PR adds
PipelineRun name along with Json marshalling error so
that it is easy to track the issue.

Signed-off-by: Zaki Shaikh <zashaikh@redhat.com>

Author: zakisk

pkg/pipelineascode/match.go

@@ -396,9 +396,14 @@ func filterRunningPipelineRunOnTargetTest(testPipeline string, prs []*tektonv1.P
 // - the template variable with the one from the event (this includes the remote pipeline that has template variables).
 func (p *PacRun) changePipelineRun(ctx context.Context, repo *v1alpha1.Repository, prs []*tektonv1.PipelineRun) error {
 	for k, pr := range prs {
+		prName := pr.GetName()
+		if prName == "" {
+			prName = pr.GetGenerateName()
+		}
+
 		b, err := json.Marshal(pr)
 		if err != nil {
-			return err
+			return fmt.Errorf("failed to marshal PipelineRun %s: %w", prName, err)
 		}
 
 		name := secrets.GenerateBasicAuthSecretName()
@@ -410,7 +415,7 @@ func (p *PacRun) changePipelineRun(ctx context.Context, repo *v1alpha1.Repositor
 		var np *tektonv1.PipelineRun
 		err = json.Unmarshal([]byte(processed), &np)
 		if err != nil {
-			return err
+			return fmt.Errorf("failed to unmarshal PipelineRun %s: %w", prName, err)
 		}
 		// don't crash when we don't have any annotations
 		if np.Annotations == nil {

pkg/pipelineascode/match_test.go

@@ -58,6 +58,13 @@ func TestPacRun_checkNeedUpdate(t *testing.T) {
 }
 
 func TestChangePipelineRun(t *testing.T) {
+	repo := &v1alpha1.Repository{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "testrepo",
+			Namespace: "test",
+		},
+	}
+
 	prs := []*tektonv1.PipelineRun{
 		{
 			ObjectMeta: metav1.ObjectMeta{
@@ -66,21 +73,59 @@ func TestChangePipelineRun(t *testing.T) {
 			},
 		},
 	}
-	event := info.NewEvent()
-	event.Repository = "testrepo"
-	p := &PacRun{event: event}
-	repo := &v1alpha1.Repository{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      "testrepo",
-			Namespace: "test",
+
+	jsonErrorPRs := []*tektonv1.PipelineRun{
+		{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "json-error-pr",
+				Namespace: "test",
+			},
+			Spec: tektonv1.PipelineRunSpec{
+				Params: []tektonv1.Param{
+					{
+						Name: "my-param",
+						Value: tektonv1.ParamValue{
+							// We are intentionally leaving this empty:
+							// Type: tektonv1.ParamTypeString,
+							StringVal: "some-value",
+						},
+					},
+				},
+			},
+		},
+	}
+
+	tests := []struct {
+		name          string
+		prs           []*tektonv1.PipelineRun
+		expectedError string
+	}{
+		{
+			name: "test with params",
+			prs:  prs,
 		},
+		{
+			name:          "test with json error",
+			prs:           jsonErrorPRs,
+			expectedError: "failed to marshal PipelineRun json-error-pr: json: error calling MarshalJSON for type v1.ParamValue: impossible ParamValues.Type: \"\"",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			event := info.NewEvent()
+			event.Repository = "testrepo"
+			p := &PacRun{event: event}
+			ctx, _ := rtesting.SetupFakeContext(t)
+			err := p.changePipelineRun(ctx, repo, tt.prs)
+			if tt.expectedError != "" {
+				assert.Error(t, err, tt.expectedError)
+				return
+			}
+			assert.Assert(t, strings.HasPrefix(tt.prs[0].GetName(), "pac-gitauth"), tt.prs[0].GetName(), "has no pac-gitauth prefix")
+			assert.Assert(t, tt.prs[0].GetAnnotations()[apipac.GitAuthSecret] != "")
+			assert.Assert(t, tt.prs[0].GetNamespace() == "testrepo", "namespace should be testrepo: %v", tt.prs[0].GetNamespace())
+		})
 	}
-	ctx, _ := rtesting.SetupFakeContext(t)
-	err := p.changePipelineRun(ctx, repo, prs)
-	assert.NilError(t, err)
-	assert.Assert(t, strings.HasPrefix(prs[0].GetName(), "pac-gitauth"), prs[0].GetName(), "has no pac-gitauth prefix")
-	assert.Assert(t, prs[0].GetAnnotations()[apipac.GitAuthSecret] != "")
-	assert.Assert(t, prs[0].GetNamespace() == "testrepo", "namespace should be testrepo: %v", prs[0].GetNamespace())
 }
 
 func TestFilterRunningPipelineRunOnTargetTest(t *testing.T) {