refactor: implement Validatable for step

Add `StepList` with a `Validate` method and implement a `Validate`
method on `Step`.

Move up check for duplicated Step names into `StepList.Validate` to be
consistent with the check for duplicated Task names in
`pipeline_validation.go`.

Issue #8700.

Signed-off-by: Stanislav Jakuschevskij <stas@two-giants.com>

Author: twoGiants

pkg/apis/pipeline/v1/container_validation.go

@@ -29,7 +29,6 @@ import (
 	"github.com/tektoncd/pipeline/pkg/apis/config"
 	"github.com/tektoncd/pipeline/pkg/apis/pipeline"
 	"github.com/tektoncd/pipeline/pkg/internal/resultref"
-	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apimachinery/pkg/util/validation"
 	"knative.dev/pkg/apis"
 )
@@ -104,7 +103,8 @@ func RefNameLikeUrl(name string) error {
 	return nil
 }
 
-func validateStep(ctx context.Context, s Step, names sets.String) (errs *apis.FieldError) {
+// Validate implements apis.Validatable
+func (s *Step) Validate(ctx context.Context) (errs *apis.FieldError) {
 	if err := validateArtifactsReferencesInStep(ctx, s); err != nil {
 		return err
 	}
@@ -181,17 +181,13 @@ func validateStep(ctx context.Context, s Step, names sets.String) (errs *apis.Fi
 	}
 
 	if s.Name != "" {
-		if names.Has(s.Name) {
-			errs = errs.Also(apis.ErrMultipleOneOf("name"))
-		}
 		if e := validation.IsDNS1123Label(s.Name); len(e) > 0 {
 			errs = errs.Also(&apis.FieldError{
 				Message: fmt.Sprintf("invalid value %q", s.Name),
 				Paths:   []string{"name"},
 				Details: "Task step name must be a valid DNS Label, For more info refer to https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names",
 			})
 		}
-		names.Insert(s.Name)
 	}
 
 	if s.Timeout != nil {
@@ -254,7 +250,7 @@ func isParamRefs(s string) bool {
 	return strings.HasPrefix(s, "$("+ParamsPrefix)
 }
 
-func validateArtifactsReferencesInStep(ctx context.Context, s Step) *apis.FieldError {
+func validateArtifactsReferencesInStep(ctx context.Context, s *Step) *apis.FieldError {
 	cfg := config.FromContextOrDefaults(ctx)
 	if cfg == nil || cfg.FeatureFlags == nil {
 		cfg = &config.Config{
@@ -295,11 +291,11 @@ func taskArtifactReferenceExists(src string) bool {
 	return len(artifactref.TaskArtifactRegex.FindAllStringSubmatch(src, -1)) > 0 || strings.Contains(src, "$("+artifactref.TaskArtifactPathPattern+")")
 }
 
-func validateStepResultReference(s Step) (errs *apis.FieldError) {
+func validateStepResultReference(s *Step) (errs *apis.FieldError) {
 	errs = errs.Also(errorIfStepResultReferencedInField(s.Name, "name"))
 	errs = errs.Also(errorIfStepResultReferencedInField(s.Image, "image"))
 	errs = errs.Also(errorIfStepResultReferencedInField(s.Script, "script"))
-	errs = errs.Also(errorIfStepResultReferencedInField(string(s.ImagePullPolicy), "imagePullPoliicy"))
+	errs = errs.Also(errorIfStepResultReferencedInField(string(s.ImagePullPolicy), "imagePullPolicy"))
 	errs = errs.Also(errorIfStepResultReferencedInField(s.WorkingDir, "workingDir"))
 	for _, e := range s.EnvFrom {
 		errs = errs.Also(errorIfStepResultReferencedInField(e.Prefix, "envFrom.prefix"))
@@ -333,7 +329,7 @@ func errorIfStepResultReferencedInField(value, fieldName string) (errs *apis.Fie
 	return errs
 }
 
-func validateStepArtifactsReference(s Step) (errs *apis.FieldError) {
+func validateStepArtifactsReference(s *Step) (errs *apis.FieldError) {
 	errs = errs.Also(errorIfStepArtifactReferencedInField(s.Name, "name"))
 	errs = errs.Also(errorIfStepArtifactReferencedInField(s.Image, "image"))
 	errs = errs.Also(errorIfStepArtifactReferencedInField(string(s.ImagePullPolicy), "imagePullPolicy"))

pkg/apis/pipeline/v1/task_types.go

@@ -133,5 +133,8 @@ type TaskList struct {
 	Items           []Task `json:"items"`
 }
 
+// StepList is a list of Steps
+type StepList []Step
+
 // SidecarList is a list of Sidecars
 type SidecarList []Sidecar

pkg/apis/pipeline/v1/task_validation.go

@@ -87,7 +87,7 @@ func (ts *TaskSpec) Validate(ctx context.Context) (errs *apis.FieldError) {
 		})
 	}
 
-	errs = errs.Also(validateSteps(ctx, mergedSteps).ViaField("steps"))
+	errs = errs.Also(StepList(mergedSteps).Validate(ctx).ViaField("steps"))
 	errs = errs.Also(SidecarList(ts.Sidecars).Validate(ctx).ViaField("sidecars"))
 	errs = errs.Also(ValidateParameterTypes(ctx, ts.Params).ViaField("params"))
 	errs = errs.Also(ValidateParameterVariables(ctx, ts.Steps, ts.Params))
@@ -213,11 +213,20 @@ func ValidateVolumes(volumes []corev1.Volume) (errs *apis.FieldError) {
 	return errs
 }
 
-func validateSteps(ctx context.Context, steps []Step) (errs *apis.FieldError) {
+// Validate implements apis.Validatable
+func (l StepList) Validate(ctx context.Context) (errs *apis.FieldError) {
 	// Task must not have duplicate step names.
 	names := sets.NewString()
-	for idx, s := range steps {
-		errs = errs.Also(validateStep(ctx, s, names).ViaIndex(idx))
+	for idx, s := range l {
+		// names cannot be duplicated - checking that Step names are unique
+		if s.Name != "" {
+			if names.Has(s.Name) {
+				errs = errs.Also(apis.ErrMultipleOneOf("name").ViaIndex(idx))
+			}
+			names.Insert(s.Name)
+		}
+
+		errs = errs.Also(s.Validate(ctx).ViaIndex(idx))
 		if s.Results != nil {
 			errs = errs.Also(ValidateStepResultsVariables(ctx, s.Results, s.Script).ViaIndex(idx))
 			errs = errs.Also(ValidateStepResults(ctx, s.Results).ViaIndex(idx).ViaField("results"))

pkg/apis/pipeline/v1/zz_generated.deepcopy.go

@@ -287,7 +287,7 @@ func errorIfStepArtifactReferencedInField(value, fieldName string) (errs *apis.F
 func validateStepArtifactsReference(s Step) (errs *apis.FieldError) {
 	errs = errs.Also(errorIfStepArtifactReferencedInField(s.Name, "name"))
 	errs = errs.Also(errorIfStepArtifactReferencedInField(s.Image, "image"))
-	errs = errs.Also(errorIfStepArtifactReferencedInField(string(s.ImagePullPolicy), "imagePullPoliicy"))
+	errs = errs.Also(errorIfStepArtifactReferencedInField(string(s.ImagePullPolicy), "imagePullPolicy"))
 	errs = errs.Also(errorIfStepArtifactReferencedInField(s.WorkingDir, "workingDir"))
 	for _, e := range s.EnvFrom {
 		errs = errs.Also(errorIfStepArtifactReferencedInField(e.Prefix, "envFrom.prefix"))
@@ -314,7 +314,7 @@ func validateStepResultReference(s Step) (errs *apis.FieldError) {
 	errs = errs.Also(errorIfStepResultReferenceinField(s.Name, "name"))
 	errs = errs.Also(errorIfStepResultReferenceinField(s.Image, "image"))
 	errs = errs.Also(errorIfStepResultReferenceinField(s.Script, "script"))
-	errs = errs.Also(errorIfStepResultReferenceinField(string(s.ImagePullPolicy), "imagePullPoliicy"))
+	errs = errs.Also(errorIfStepResultReferenceinField(string(s.ImagePullPolicy), "imagePullPolicy"))
 	errs = errs.Also(errorIfStepResultReferenceinField(s.WorkingDir, "workingDir"))
 	for _, e := range s.EnvFrom {
 		errs = errs.Also(errorIfStepResultReferenceinField(e.Prefix, "envFrom.prefix"))