Consolidate validation for beta features

pkg/apis/pipeline/v1/pipeline_types_test.go

@@ -276,18 +276,6 @@ func TestPipelineTask_ValidateRegularTask_Failure(t *testing.T) {
 			Message: `invalid value: taskRef must specify name`,
 			Paths:   []string{"taskRef.name"},
 		},
-	}, {
-		name: "pipeline task - use of resolver without the feature flag set",
-		task: PipelineTask{
-			TaskRef: &TaskRef{Name: "boo", ResolverRef: ResolverRef{Resolver: "bar"}},
-		},
-		expectedError: *apis.ErrDisallowedFields("taskref.resolver"),
-	}, {
-		name: "pipeline task - use of resolver params without the feature flag set",
-		task: PipelineTask{
-			TaskRef: &TaskRef{Name: "boo", ResolverRef: ResolverRef{Params: Params{{}}}},
-		},
-		expectedError: *apis.ErrDisallowedFields("taskref.params"),
 	}}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {

pkg/apis/pipeline/v1/pipeline_validation.go

@@ -70,7 +70,7 @@ func (ps *PipelineSpec) Validate(ctx context.Context) (errs *apis.FieldError) {
 	errs = errs.Also(validatePipelineContextVariables(ps.Tasks).ViaField("tasks"))
 	errs = errs.Also(validatePipelineContextVariables(ps.Finally).ViaField("finally"))
 	errs = errs.Also(validateExecutionStatusVariables(ps.Tasks, ps.Finally))
-	errs = errs.Also(ps.ValidateBetaFeaturesEnabledForParamArrayIndexing(ctx))
+	errs = errs.Also(ps.validateBetaFields(ctx))
 	// Validate the pipeline's workspaces.
 	errs = errs.Also(validatePipelineWorkspacesDeclarations(ps.Workspaces))
 	// Validate the pipeline's results
@@ -84,6 +84,58 @@ func (ps *PipelineSpec) Validate(ctx context.Context) (errs *apis.FieldError) {
 	return errs
 }
 
+// validateBetaFields returns an error if the Pipeline spec uses beta features but does not
+// have "enable-api-fields" set to "alpha" or "beta".
+func (ps *PipelineSpec) validateBetaFields(ctx context.Context) *apis.FieldError {
+	var errs *apis.FieldError
+	// Object parameters
+	for i, p := range ps.Params {
+		if p.Type == ParamTypeObject {
+			errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "object type parameter", config.BetaAPIFields).ViaFieldIndex("params", i))
+		}
+	}
+	// Indexing into array parameters
+	arrayParamIndexingRefs := ps.GetIndexingReferencesToArrayParams()
+	if len(arrayParamIndexingRefs) != 0 && !config.CheckAlphaOrBetaAPIFields(ctx) {
+		errs = errs.Also(apis.ErrGeneric(fmt.Sprintf("cannot index into array parameters when 'enable-api-fields' is 'stable', but found indexing references: %s", arrayParamIndexingRefs)))
+	}
+	// array and object results
+	for i, result := range ps.Results {
+		switch result.Type {
+		case ResultsTypeObject:
+			errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "object results", config.BetaAPIFields).ViaFieldIndex("results", i))
+		case ResultsTypeArray:
+			errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "array results", config.BetaAPIFields).ViaFieldIndex("results", i))
+		case ResultsTypeString:
+		default:
+		}
+	}
+	for i, pt := range ps.Tasks {
+		errs = errs.Also(pt.validateBetaFields(ctx).ViaFieldIndex("tasks", i))
+	}
+	for i, pt := range ps.Finally {
+		errs = errs.Also(pt.validateBetaFields(ctx).ViaFieldIndex("tasks", i))
+	}
+
+	return errs
+}
+
+// validateBetaFields returns an error if the PipelineTask uses beta features but does not
+// have "enable-api-fields" set to "alpha" or "beta".
+func (pt *PipelineTask) validateBetaFields(ctx context.Context) *apis.FieldError {
+	var errs *apis.FieldError
+	if pt.TaskRef != nil {
+		// Resolvers
+		if pt.TaskRef.Resolver != "" {
+			errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "taskref.resolver", config.BetaAPIFields))
+		}
+		if len(pt.TaskRef.Params) > 0 {
+			errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "taskref.params", config.BetaAPIFields))
+		}
+	}
+	return errs
+}
+
 // ValidatePipelineTasks ensures that pipeline tasks has unique label, pipeline tasks has specified one of
 // taskRef or taskSpec, and in case of a pipeline task with taskRef, it has a reference to a valid task (task name)
 func ValidatePipelineTasks(ctx context.Context, tasks []PipelineTask, finalTasks []PipelineTask) *apis.FieldError {
@@ -253,7 +305,6 @@ func (pt PipelineTask) validateCustomTask() (errs *apis.FieldError) {
 
 // validateTask validates a pipeline task or a final task for taskRef and taskSpec
 func (pt PipelineTask) validateTask(ctx context.Context) (errs *apis.FieldError) {
-	cfg := config.FromContextOrDefaults(ctx)
 	// Validate TaskSpec if it's present
 	if pt.TaskSpec != nil {
 		errs = errs.Also(pt.TaskSpec.Validate(ctx).ViaField("taskSpec"))
@@ -267,15 +318,6 @@ func (pt PipelineTask) validateTask(ctx context.Context) (errs *apis.FieldError)
 		} else if pt.TaskRef.Resolver == "" {
 			errs = errs.Also(apis.ErrInvalidValue("taskRef must specify name", "taskRef.name"))
 		}
-		if cfg.FeatureFlags.EnableAPIFields != config.BetaAPIFields && cfg.FeatureFlags.EnableAPIFields != config.AlphaAPIFields {
-			// fail if resolver or resource are present when enable-api-fields is false.
-			if pt.TaskRef.Resolver != "" {
-				errs = errs.Also(apis.ErrDisallowedFields("taskref.resolver"))
-			}
-			if len(pt.TaskRef.Params) > 0 {
-				errs = errs.Also(apis.ErrDisallowedFields("taskref.params"))
-			}
-		}
 	}
 	return errs
 }
@@ -712,20 +754,6 @@ func (ps *PipelineSpec) ValidateParamArrayIndex(ctx context.Context, params Para
 	return validateOutofBoundArrayParams(arrayIndexParamRefs, arrayParamsLengths)
 }
 
-// ValidateBetaFeaturesEnabledForParamArrayIndexing validates that "enable-api-fields" is set to "alpha" or "beta" if the pipeline spec
-// contains indexing references to array params.
-// This can be removed when array param indexing is moved to "stable".
-func (ps *PipelineSpec) ValidateBetaFeaturesEnabledForParamArrayIndexing(ctx context.Context) (errs *apis.FieldError) {
-	if config.CheckAlphaOrBetaAPIFields(ctx) {
-		return nil
-	}
-	arrayParamIndexingRefs := ps.GetIndexingReferencesToArrayParams()
-	if len(arrayParamIndexingRefs) == 0 {
-		return nil
-	}
-	return apis.ErrGeneric(fmt.Sprintf("cannot index into array parameters when 'enable-api-fields' is 'stable', but found indexing references: %s", arrayParamIndexingRefs))
-}
-
 // GetIndexingReferencesToArrayParams returns all strings referencing indices of PipelineRun array parameters
 // from parameters, workspaces, and when expressions defined in the Pipeline's Tasks and Finally Tasks.
 // For example, if a Task in the Pipeline has a parameter with a value "$(params.array-param-name[1])",

pkg/apis/pipeline/v1/pipeline_validation_test.go

@@ -3483,6 +3483,75 @@ func TestPipelineWithBetaFields(t *testing.T) {
 				TaskRef: &TaskRef{Name: "foo"},
 			}},
 		},
+	}, {
+		name: "pipeline tasks - use of resolver without the feature flag set",
+		spec: PipelineSpec{
+			Tasks: []PipelineTask{{
+				Name:    "uses-resolver",
+				TaskRef: &TaskRef{Name: "boo", ResolverRef: ResolverRef{Resolver: "bar"}},
+			}},
+		},
+	}, {
+		name: "pipeline tasks - use of resolver params without the feature flag set",
+		spec: PipelineSpec{
+			Tasks: []PipelineTask{{
+				Name:    "uses-resolver-params",
+				TaskRef: &TaskRef{Name: "boo", ResolverRef: ResolverRef{Params: Params{{}}}},
+			}},
+		},
+	}, {
+		name: "finally tasks - use of resolver without the feature flag set",
+		spec: PipelineSpec{
+			Tasks: []PipelineTask{{
+				Name:    "valid-pipeline-task",
+				TaskRef: &TaskRef{Name: "foo-task"},
+			}},
+			Finally: []PipelineTask{{
+				Name:    "uses-resolver",
+				TaskRef: &TaskRef{Name: "boo", ResolverRef: ResolverRef{Resolver: "bar"}},
+			}},
+		},
+	}, {
+		name: "finally tasks - use of resolver params without the feature flag set",
+		spec: PipelineSpec{
+			Tasks: []PipelineTask{{
+				Name:    "valid-pipeline-task",
+				TaskRef: &TaskRef{Name: "foo-task"},
+			}},
+			Finally: []PipelineTask{{
+				Name:    "uses-resolver-params",
+				TaskRef: &TaskRef{Name: "boo", ResolverRef: ResolverRef{Params: Params{{}}}},
+			}},
+		},
+	}, {
+		name: "object params",
+		spec: PipelineSpec{
+			Params: []ParamSpec{
+				{Name: "first-param", Type: ParamTypeObject, Properties: map[string]PropertySpec{}},
+			},
+			Tasks: []PipelineTask{{
+				Name:    "foo",
+				TaskRef: &TaskRef{Name: "foo"},
+			}},
+		},
+	}, {
+		name: "array results",
+		spec: PipelineSpec{
+			Tasks: []PipelineTask{{
+				Name:    "valid-pipeline-task",
+				TaskRef: &TaskRef{Name: "foo-task"},
+			}},
+			Results: []PipelineResult{{Name: "my-array-result", Type: ResultsTypeArray, Value: *NewStructuredValues("$(tasks.valid-pipeline-task.results.foo[*])")}},
+		},
+	}, {
+		name: "object results",
+		spec: PipelineSpec{
+			Tasks: []PipelineTask{{
+				Name:    "valid-pipeline-task",
+				TaskRef: &TaskRef{Name: "foo-task"},
+			}},
+			Results: []PipelineResult{{Name: "my-object-result", Type: ResultsTypeObject, Value: *NewStructuredValues("$(tasks.valid-pipeline-task.results.foo[*])")}},
+		},
 	}}
 	for _, tt := range tts {
 		t.Run(tt.name, func(t *testing.T) {

pkg/apis/pipeline/v1/result_validation.go

@@ -18,8 +18,6 @@ import (
 	"fmt"
 	"regexp"
 
-	"github.com/tektoncd/pipeline/pkg/apis/config"
-	"github.com/tektoncd/pipeline/pkg/apis/version"
 	"knative.dev/pkg/apis"
 )
 
@@ -35,15 +33,11 @@ func (tr TaskResult) Validate(ctx context.Context) (errs *apis.FieldError) {
 	}
 
 	switch {
-	// Object results is beta feature - check if the feature flag is set to "beta" or "alpha"
 	case tr.Type == ResultsTypeObject:
 		errs := validateObjectResult(tr)
-		errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "results type", config.BetaAPIFields))
 		return errs
-	// Array results is a beta feature - check if the feature flag is set to "beta" or "alpha"
 	case tr.Type == ResultsTypeArray:
-		errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "results type", config.BetaAPIFields))
-		return errs
+		return nil
 	// Resources created before the result. Type was introduced may not have Type set
 	// and should be considered valid
 	case tr.Type == "":

pkg/apis/pipeline/v1/result_validation_test.go

@@ -22,42 +22,35 @@ import (
 
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-cmp/cmp/cmpopts"
-	"github.com/tektoncd/pipeline/pkg/apis/config"
 	v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
 	"github.com/tektoncd/pipeline/test/diff"
 	"knative.dev/pkg/apis"
 )
 
 func TestResultsValidate(t *testing.T) {
 	tests := []struct {
-		name      string
-		Result    v1.TaskResult
-		apiFields string
+		name   string
+		Result v1.TaskResult
 	}{{
 		name: "valid result type empty",
 		Result: v1.TaskResult{
 			Name:        "MY-RESULT",
 			Description: "my great result",
 		},
-		apiFields: "stable",
 	}, {
 		name: "valid result type string",
 		Result: v1.TaskResult{
 			Name:        "MY-RESULT",
 			Type:        v1.ResultsTypeString,
 			Description: "my great result",
 		},
-
-		apiFields: "stable",
 	}, {
 		name: "valid result type array",
 		Result: v1.TaskResult{
 			Name:        "MY-RESULT",
 			Type:        v1.ResultsTypeArray,
 			Description: "my great result",
 		},
-
-		apiFields: "alpha",
 	}, {
 		name: "valid result type object",
 		Result: v1.TaskResult{
@@ -66,14 +59,10 @@ func TestResultsValidate(t *testing.T) {
 			Description: "my great result",
 			Properties:  map[string]v1.PropertySpec{"hello": {Type: v1.ParamTypeString}},
 		},
-		apiFields: "alpha",
 	}}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			ctx := context.Background()
-			if tt.apiFields == "alpha" {
-				ctx = config.EnableAlphaAPIFields(ctx)
-			}
 			if err := tt.Result.Validate(ctx); err != nil {
 				t.Errorf("TaskSpec.Validate() = %v", err)
 			}
@@ -85,57 +74,19 @@ func TestResultsValidateError(t *testing.T) {
 	tests := []struct {
 		name          string
 		Result        v1.TaskResult
-		apiFields     string
 		expectedError apis.FieldError
 	}{{
-		name: "invalid result type in stable",
+		name: "invalid result type",
 		Result: v1.TaskResult{
 			Name:        "MY-RESULT",
 			Type:        "wrong",
 			Description: "my great result",
 		},
-		apiFields: "stable",
 		expectedError: apis.FieldError{
 			Message: `invalid value: wrong`,
 			Paths:   []string{"type"},
 			Details: "type must be string",
 		},
-	}, {
-		name: "invalid result type in alpha",
-		Result: v1.TaskResult{
-			Name:        "MY-RESULT",
-			Type:        "wrong",
-			Description: "my great result",
-		},
-		apiFields: "alpha",
-		expectedError: apis.FieldError{
-			Message: `invalid value: wrong`,
-			Paths:   []string{"type"},
-			Details: "type must be string",
-		},
-	}, {
-		name: "invalid array result type in stable",
-		Result: v1.TaskResult{
-			Name:        "MY-RESULT",
-			Type:        v1.ResultsTypeArray,
-			Description: "my great result",
-		},
-		apiFields: "stable",
-		expectedError: apis.FieldError{
-			Message: "results type requires \"enable-api-fields\" feature gate to be \"alpha\" or \"beta\" but it is \"stable\"",
-		},
-	}, {
-		name: "invalid object result type in stable",
-		Result: v1.TaskResult{
-			Name:        "MY-RESULT",
-			Type:        v1.ResultsTypeObject,
-			Description: "my great result",
-			Properties:  map[string]v1.PropertySpec{"hello": {Type: v1.ParamTypeString}},
-		},
-		apiFields: "stable",
-		expectedError: apis.FieldError{
-			Message: "results type requires \"enable-api-fields\" feature gate to be \"alpha\" or \"beta\" but it is \"stable\"",
-		},
 	}, {
 		name: "invalid object properties type",
 		Result: v1.TaskResult{
@@ -144,7 +95,6 @@ func TestResultsValidateError(t *testing.T) {
 			Description: "my great result",
 			Properties:  map[string]v1.PropertySpec{"hello": {Type: "wrong type"}},
 		},
-		apiFields: "alpha",
 		expectedError: apis.FieldError{
 			Message: "The value type specified for these keys [hello] is invalid, the type must be string",
 			Paths:   []string{"MY-RESULT.properties"},
@@ -156,19 +106,14 @@ func TestResultsValidateError(t *testing.T) {
 			Type:        v1.ResultsTypeObject,
 			Description: "my great result",
 		},
-		apiFields: "alpha",
 		expectedError: apis.FieldError{
 			Message: "missing field(s)",
 			Paths:   []string{"MY-RESULT.properties"},
 		},
 	}}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			ctx := context.Background()
-			if tt.apiFields == "alpha" {
-				ctx = config.EnableAlphaAPIFields(ctx)
-			}
-			err := tt.Result.Validate(ctx)
+			err := tt.Result.Validate(context.Background())
 			if err == nil {
 				t.Fatalf("Expected an error, got nothing for %v", tt.Result)
 			}