Report tekton yaml validation error to the user

The validation errors occurring in PipelineRuns as reported by the
tekton controller within the .tekton directory are now reported directly
in the event logs of the user namespaces along with the pipelinerun
name where the error has occurred, this let a user troubleshoot the issue
even if she or he doesn't have access to the controller log.

Signed-off-by: Chmouel Boudjnah <[email protected]>

pkg/pipelineascode/match.go

@@ -168,6 +168,13 @@ func (p *PacRun) getPipelineRunsFromRepo(ctx context.Context, repo *v1alpha1.Rep
 	if err != nil {
 		return nil, err
 	}
+
+	if types.ValidationErrors != nil {
+		for k, v := range types.ValidationErrors {
+			kv := fmt.Sprintf("prun: %s tekton validation error: %s", k, v)
+			p.eventEmitter.EmitMessage(repo, zap.ErrorLevel, "PipelineRunValidationErrors", kv)
+		}
+	}
 	pipelineRuns := types.PipelineRuns
 	if len(pipelineRuns) == 0 {
 		msg := fmt.Sprintf("cannot locate templates in %s/ directory for this repository in %s", tektonDir, p.event.HeadBranch)

pkg/pipelineascode/match_test.go

@@ -8,6 +8,7 @@ import (
 	apipac "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/events"
 	"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"
@@ -116,6 +117,7 @@ func TestGetPipelineRunsFromRepo(t *testing.T) {
 		tektondir             string
 		expectedNumberOfPruns int
 		event                 *info.Event
+		logSnippet            string
 	}{
 		{
 			name: "more than one pipelinerun in .tekton dir",
@@ -143,6 +145,19 @@ func TestGetPipelineRunsFromRepo(t *testing.T) {
 			expectedNumberOfPruns: 1,
 			event:                 pullRequestEvent,
 		},
+		{
+			name: "invalid tekton pipelineruns in directory",
+			repositories: &v1alpha1.Repository{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "testrepo",
+					Namespace: "test",
+				},
+				Spec: v1alpha1.RepositorySpec{},
+			},
+			tektondir:  "testdata/invalid_tekton_yaml",
+			event:      pullRequestEvent,
+			logSnippet: `prun: bad-tekton-yaml tekton validation error: json: cannot unmarshal object into Go struct field PipelineSpec.spec.pipelineSpec.tasks of type []v1beta1.PipelineTask`,
+		},
 		{
 			name: "no-match pipelineruns in .tekton dir, only matched should be returned",
 			repositories: &v1alpha1.Repository{
@@ -176,8 +191,8 @@ func TestGetPipelineRunsFromRepo(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			observer, _ := zapobserver.New(zap.InfoLevel)
-			logger := zap.New(observer).Sugar()
+			observerCore, logCatcher := zapobserver.New(zap.InfoLevel)
+			logger := zap.New(observerCore).Sugar()
 			ctx, _ := rtesting.SetupFakeContext(t)
 			fakeclient, mux, _, teardown := ghtesthelper.SetupGH()
 			defer teardown()
@@ -213,12 +228,16 @@ func TestGetPipelineRunsFromRepo(t *testing.T) {
 				Logger: logger,
 			}
 			p := NewPacs(tt.event, vcx, cs, k8int, logger)
+			p.eventEmitter = events.NewEventEmitter(stdata.Kube, logger)
 			matchedPRs, err := p.getPipelineRunsFromRepo(ctx, tt.repositories)
 			assert.NilError(t, err)
 			matchedPRNames := []string{}
 			for i := range matchedPRs {
 				matchedPRNames = append(matchedPRNames, matchedPRs[i].PipelineRun.GetGenerateName())
 			}
+			if tt.logSnippet != "" {
+				assert.Assert(t, logCatcher.FilterMessageSnippet(tt.logSnippet).Len() > 0, logCatcher.All())
+			}
 			assert.Equal(t, len(matchedPRNames), tt.expectedNumberOfPruns)
 		})
 	}

pkg/pipelineascode/testdata/invalid_tekton_yaml/.tekton/on-comment.yaml

@@ -0,0 +1,18 @@
+---
+apiVersion: tekton.dev/v1beta1
+kind: PipelineRun
+metadata:
+  name: bad-tekton-yaml
+  annotations:
+    pipelinesascode.tekton.dev/max-keep-runs: "2"
+    pipelinesascode.tekton.dev/task: ".tekton/remotetask.yaml"
+    pipelinesascode.tekton.dev/on-comment: "/make-me-a-sandwich"
+spec:
+  pipelineSpec:
+    tasks:
+      taskSpec:
+        steps:
+          - name: noop-task
+            image: registry.access.redhat.com/ubi9/ubi-micro
+            script: |
+              echo "make me a sandwich"

pkg/resolve/resolve.go

@@ -15,18 +15,48 @@ import (
 	tektonv1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
 	tektonv1beta1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
 	"go.uber.org/zap"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	k8scheme "k8s.io/client-go/kubernetes/scheme"
+	yaml "sigs.k8s.io/yaml/goyaml.v2"
 )
 
 type TektonTypes struct {
-	PipelineRuns []*tektonv1.PipelineRun
-	Pipelines    []*tektonv1.Pipeline
-	TaskRuns     []*tektonv1.TaskRun
-	Tasks        []*tektonv1.Task
+	PipelineRuns     []*tektonv1.PipelineRun
+	Pipelines        []*tektonv1.Pipeline
+	TaskRuns         []*tektonv1.TaskRun
+	Tasks            []*tektonv1.Task
+	ValidationErrors map[string]string
+}
+
+func NewTektonTypes() TektonTypes {
+	return TektonTypes{
+		ValidationErrors: map[string]string{},
+	}
 }
 
 var yamlDocSeparatorRe = regexp.MustCompile(`(?m)^---\s*$`)
 
+// detectAtleastNameOrGenerateNameFromPipelineRun detects the name or
+// generateName of a yaml files even if there is an error decoding it as tekton types.
+func detectAtleastNameOrGenerateNameFromPipelineRun(data string) string {
+	var metadataName struct {
+		Metadata metav1.ObjectMeta
+	}
+	err := yaml.Unmarshal([]byte(data), &metadataName)
+	if err != nil {
+		return ""
+	}
+	if metadataName.Metadata.Name != "" {
+		return metadataName.Metadata.Name
+	}
+
+	// TODO: yaml Unmarshal don't want to parse generatename and i have no idea why
+	if metadataName.Metadata.GenerateName != "" {
+		return metadataName.Metadata.GenerateName
+	}
+	return "unknown"
+}
+
 // getTaskRunByName returns the taskrun with the given name the first one found
 // will be matched. It does not handle conflicts so user has fetched multiple
 // taskruns with the same name it will just pick up the first one.
@@ -120,7 +150,7 @@ type Opts struct {
 }
 
 func ReadTektonTypes(ctx context.Context, log *zap.SugaredLogger, data string) (TektonTypes, error) {
-	types := TektonTypes{}
+	types := NewTektonTypes()
 	decoder := k8scheme.Codecs.UniversalDeserializer()
 
 	for _, doc := range yamlDocSeparatorRe.Split(data, -1) {
@@ -130,7 +160,7 @@ func ReadTektonTypes(ctx context.Context, log *zap.SugaredLogger, data string) (
 
 		obj, _, err := decoder.Decode([]byte(doc), nil, nil)
 		if err != nil {
-			log.Infof("skipping yaml document not looking like a kubernetes resources: %v", err)
+			types.ValidationErrors[detectAtleastNameOrGenerateNameFromPipelineRun(doc)] = err.Error()
 			continue
 		}
 		switch o := obj.(type) {

pkg/resolve/resolve_test.go

@@ -1,6 +1,7 @@
 package resolve
 
 import (
+	"context"
 	"fmt"
 	"log"
 	"os"
@@ -201,14 +202,42 @@ func TestNotTektonDocumentIgnore(t *testing.T) {
 	assert.Assert(t, resolved.Spec.PipelineSpec != nil)
 }
 
-func TestNotKubernetesDocumentIgnore(t *testing.T) {
-	resolved, log, err := readTDfile(t, "not-a-kubernetes-yaml", false, true)
-	logs := log.TakeAll()
-	assert.Assert(t, len(logs) > 0)
-	assert.Assert(t, strings.HasPrefix(logs[0].Message, "skipping yaml"), logs[0].Message)
-	assert.Assert(t, resolved.Spec.PipelineSpec != nil)
-	assert.NilError(t, err)
-	assert.Assert(t, resolved.Spec.PipelineSpec != nil)
+func TestReportBadTektonYaml(t *testing.T) {
+	tests := []struct {
+		name           string
+		filename       string
+		wantErr        bool
+		validError     string
+		validErrorName string
+	}{
+		{
+			name:           "bad tekton yaml name",
+			filename:       "bad-tekton-yaml-name",
+			validError:     `json: cannot unmarshal object into Go struct field PipelineSpec.spec.pipelineSpec.tasks of type []v1beta1.PipelineTask`,
+			validErrorName: "bad-name",
+		},
+		{
+			name:           "bad tekton yaml generateName",
+			filename:       "bad-tekton-yaml-generate-name",
+			validError:     `json: cannot unmarshal object into Go struct field PipelineSpec.spec.pipelineSpec.tasks of type []v1beta1.PipelineTask`,
+			validErrorName: "unknown",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			data, err := os.ReadFile("testdata/" + tt.filename + ".yaml")
+			assert.NilError(t, err)
+			types, err := ReadTektonTypes(context.TODO(), nil, string(data))
+			assert.NilError(t, err)
+			if value, ok := types.ValidationErrors[tt.validErrorName]; ok {
+				assert.Equal(t, value, tt.validError, "error message mismatch")
+			} else {
+				t.Errorf("could not find the task %s in the validation errors: %+v", tt.validErrorName, types.ValidationErrors)
+			}
+		})
+	}
+
+	assert.Equal(t, "", detectAtleastNameOrGenerateNameFromPipelineRun("- babdakdja"))
 }
 
 // test if we have the task in .tekton dir not referenced in annotations but taskRef in a task.

pkg/resolve/testdata/bad-tekton-yaml-generate-name.yaml

@@ -0,0 +1,12 @@
+---
+apiVersion: tekton.dev/v1beta1
+kind: PipelineRun
+metadata:
+  generateName: bad-generate-name
+spec:
+  pipelineSpec:
+    tasks:
+      taskSpec:
+        steps:
+          - name: hello-moto
+            image: scratch

pkg/resolve/testdata/bad-tekton-yaml-name.yaml

@@ -0,0 +1,12 @@
+---
+apiVersion: tekton.dev/v1beta1
+kind: PipelineRun
+metadata:
+  name: bad-name
+spec:
+  pipelineSpec:
+    tasks:
+      taskSpec:
+        steps:
+          - name: hello-moto
+            image: scratch