Update PipelineSpec Task name and TaskRef name validation to prevents errors at runtime

Author: Vincent-DeSousa-Tereso

docs/pipelines.md

@@ -139,8 +139,11 @@ spec:
 ### Pipeline Tasks
 
 A `Pipeline` will execute a graph of [`Tasks`](tasks.md) (see
-[ordering](#ordering) for how to express this graph). At a minimum, this
-declaration must include a reference to the [`Task`](tasks.md):
+[ordering](#ordering) for how to express this graph). A valid `Pipeline` 
+declaration must include a reference to at least one [`Task`](tasks.md). Each
+`Task` within a `Pipeline` must have a
+[valid](https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names)
+name and task reference, for example:
 
 ```yaml
 tasks:

pkg/apis/pipeline/v1alpha1/pipeline_validation.go

@@ -19,10 +19,12 @@ package v1alpha1
 import (
 	"context"
 	"fmt"
+	"strings"
 
 	"github.com/tektoncd/pipeline/pkg/list"
 	"golang.org/x/xerrors"
 	"k8s.io/apimachinery/pkg/api/equality"
+	"k8s.io/apimachinery/pkg/util/validation"
 	"knative.dev/pkg/apis"
 )
 
@@ -123,9 +125,18 @@ func (ps *PipelineSpec) Validate(ctx context.Context) *apis.FieldError {
 
 	// Names cannot be duplicated
 	taskNames := map[string]struct{}{}
-	for _, t := range ps.Tasks {
+	for i, t := range ps.Tasks {
+		// Task names are appended to the container name, which must exist and
+		// must be a valid k8s name
+		if errSlice := validation.IsQualifiedName(t.Name); len(errSlice) != 0 {
+			return apis.ErrInvalidValue(strings.Join(errSlice, ","), fmt.Sprintf("spec.tasks[%d].name", i))
+		}
+		// TaskRef name must be a valid k8s name
+		if errSlice := validation.IsQualifiedName(t.TaskRef.Name); len(errSlice) != 0 {
+			return apis.ErrInvalidValue(strings.Join(errSlice, ","), fmt.Sprintf("spec.tasks[%d].taskRef.name", i))
+		}
 		if _, ok := taskNames[t.Name]; ok {
-			return apis.ErrMultipleOneOf("spec.tasks.name")
+			return apis.ErrMultipleOneOf(fmt.Sprintf("spec.tasks[%d].name", i))
 		}
 		taskNames[t.Name] = struct{}{}
 	}

pkg/apis/pipeline/v1alpha1/pipeline_validation_test.go

@@ -66,6 +66,24 @@ func TestPipeline_Validate(t *testing.T) {
 			tb.PipelineTask("foo", "foo-task"),
 		)),
 		failureExpected: true,
+	}, {
+		name: "pipeline spec empty task name",
+		p: tb.Pipeline("pipeline", "namespace", tb.PipelineSpec(
+			tb.PipelineTask("", "foo-task"),
+		)),
+		failureExpected: true,
+	}, {
+		name: "pipeline spec invalid task name",
+		p: tb.Pipeline("pipeline", "namespace", tb.PipelineSpec(
+			tb.PipelineTask("_foo", "foo-task"),
+		)),
+		failureExpected: true,
+	}, {
+		name: "pipeline spec invalid taskref name",
+		p: tb.Pipeline("pipeline", "namespace", tb.PipelineSpec(
+			tb.PipelineTask("foo", "_foo-task"),
+		)),
+		failureExpected: true,
 	}}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {