Refactor version.ValidateEnabledAPIFields to config pkg

pkg/apis/version/version_validation.go → pkg/apis/config/featureflags_validation.go

@@ -14,40 +14,39 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package version
+package config
 
 import (
 	"context"
 	"fmt"
 
-	"github.com/tektoncd/pipeline/pkg/apis/config"
 	"knative.dev/pkg/apis"
 )
 
 // ValidateEnabledAPIFields checks that the enable-api-fields feature gate is set
 // to a version at most as stable as wantVersion, if not, returns an error stating which feature
 // is dependent on the version and what the current version actually is.
 func ValidateEnabledAPIFields(ctx context.Context, featureName string, wantVersion string) *apis.FieldError {
-	currentVersion := config.FromContextOrDefaults(ctx).FeatureFlags.EnableAPIFields
+	currentVersion := FromContextOrDefaults(ctx).FeatureFlags.EnableAPIFields
 	var errs *apis.FieldError
 	message := `%s requires "enable-api-fields" feature gate to be %s but it is %q`
 	switch wantVersion {
-	case config.StableAPIFields:
+	case StableAPIFields:
 		// If the feature is stable, it doesn't matter what the current version is
-	case config.BetaAPIFields:
+	case BetaAPIFields:
 		// If the feature requires "beta" fields to be enabled, the current version may be "beta" or "alpha"
-		if currentVersion != config.BetaAPIFields && currentVersion != config.AlphaAPIFields {
-			message = fmt.Sprintf(message, featureName, fmt.Sprintf("%q or %q", config.AlphaAPIFields, config.BetaAPIFields), currentVersion)
+		if currentVersion != BetaAPIFields && currentVersion != AlphaAPIFields {
+			message = fmt.Sprintf(message, featureName, fmt.Sprintf("%q or %q", AlphaAPIFields, BetaAPIFields), currentVersion)
 			errs = apis.ErrGeneric(message)
 		}
-	case config.AlphaAPIFields:
+	case AlphaAPIFields:
 		// If the feature requires "alpha" fields to be enabled, the current version must be "alpha"
 		if currentVersion != wantVersion {
-			message = fmt.Sprintf(message, featureName, fmt.Sprintf("%q", config.AlphaAPIFields), currentVersion)
+			message = fmt.Sprintf(message, featureName, fmt.Sprintf("%q", AlphaAPIFields), currentVersion)
 			errs = apis.ErrGeneric(message)
 		}
 	default:
-		errs = apis.ErrGeneric("invalid wantVersion %s, must be one of (%s, %s, %s)", wantVersion, config.AlphaAPIFields, config.BetaAPIFields, config.StableAPIFields)
+		errs = apis.ErrGeneric("invalid wantVersion %s, must be one of (%s, %s, %s)", wantVersion, AlphaAPIFields, BetaAPIFields, StableAPIFields)
 	}
 	return errs
 }

pkg/apis/config/featureflags_validation_test.go

@@ -0,0 +1,116 @@
+/*
+Copyright 2021 The Tekton Authors
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package config_test
+
+import (
+	"context"
+	"testing"
+
+	"github.com/tektoncd/pipeline/pkg/apis/config"
+)
+
+func TestValidateEnabledAPIFields(t *testing.T) {
+	tcs := []struct {
+		name           string
+		wantVersion    string
+		currentVersion string
+	}{{
+		name:           "alpha fields w/ alpha",
+		wantVersion:    "alpha",
+		currentVersion: "alpha",
+	}, {
+		name:           "beta fields w/ alpha",
+		wantVersion:    "beta",
+		currentVersion: "alpha",
+	}, {
+		name:           "beta fields w/ beta",
+		wantVersion:    "beta",
+		currentVersion: "beta",
+	}, {
+		name:           "stable fields w/ alpha",
+		wantVersion:    "stable",
+		currentVersion: "alpha",
+	}, {
+		name:           "stable fields w/ beta",
+		wantVersion:    "stable",
+		currentVersion: "beta",
+	}, {
+		name:           "stable fields w/ stable",
+		wantVersion:    "stable",
+		currentVersion: "stable",
+	}}
+	for _, tc := range tcs {
+		t.Run(tc.name, func(t *testing.T) {
+			flags, err := config.NewFeatureFlagsFromMap(map[string]string{
+				"enable-api-fields": tc.currentVersion,
+			})
+			if err != nil {
+				t.Fatalf("error creating feature flags from map: %v", err)
+			}
+			cfg := &config.Config{
+				FeatureFlags: flags,
+			}
+			ctx := config.ToContext(context.Background(), cfg)
+			if err := config.ValidateEnabledAPIFields(ctx, "test feature", tc.wantVersion); err != nil {
+				t.Errorf("unexpected error for compatible feature gates: %q", err)
+			}
+		})
+	}
+}
+
+func TestValidateEnabledAPIFieldsError(t *testing.T) {
+	tcs := []struct {
+		name           string
+		wantVersion    string
+		currentVersion string
+	}{{
+		name:           "alpha fields w/ stable",
+		wantVersion:    "alpha",
+		currentVersion: "stable",
+	}, {
+		name:           "alpha fields w/ beta",
+		wantVersion:    "alpha",
+		currentVersion: "beta",
+	}, {
+		name:           "beta fields w/ stable",
+		wantVersion:    "beta",
+		currentVersion: "stable",
+	}, {
+		name:           "invalid wantVersion",
+		wantVersion:    "foo",
+		currentVersion: "stable",
+	}}
+	for _, tc := range tcs {
+		t.Run(tc.name, func(t *testing.T) {
+			flags, err := config.NewFeatureFlagsFromMap(map[string]string{
+				"enable-api-fields": tc.currentVersion,
+			})
+			if err != nil {
+				t.Fatalf("error creating feature flags from map: %v", err)
+			}
+			cfg := &config.Config{
+				FeatureFlags: flags,
+			}
+			ctx := config.ToContext(context.Background(), cfg)
+			fieldErr := config.ValidateEnabledAPIFields(ctx, "test feature", tc.wantVersion)
+
+			if fieldErr == nil {
+				t.Errorf("error expected for incompatible feature gates")
+			}
+		})
+	}
+}

pkg/apis/pipeline/v1/pipeline_validation.go

@@ -23,7 +23,6 @@ import (
 
 	"github.com/tektoncd/pipeline/pkg/apis/config"
 	"github.com/tektoncd/pipeline/pkg/apis/validate"
-	"github.com/tektoncd/pipeline/pkg/apis/version"
 	"github.com/tektoncd/pipeline/pkg/reconciler/pipeline/dag"
 	"github.com/tektoncd/pipeline/pkg/substitution"
 	admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
@@ -103,21 +102,21 @@ func (ps *PipelineSpec) ValidateBetaFields(ctx context.Context) *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))
+			errs = errs.Also(config.ValidateEnabledAPIFields(ctx, "object type parameter", config.BetaAPIFields).ViaFieldIndex("params", i))
 		}
 	}
 	// Indexing into array parameters
 	arrayParamIndexingRefs := ps.GetIndexingReferencesToArrayParams()
 	if len(arrayParamIndexingRefs) != 0 {
-		errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "indexing into array parameters", config.BetaAPIFields))
+		errs = errs.Also(config.ValidateEnabledAPIFields(ctx, "indexing into array parameters", config.BetaAPIFields))
 	}
 	// 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))
+			errs = errs.Also(config.ValidateEnabledAPIFields(ctx, "object results", config.BetaAPIFields).ViaFieldIndex("results", i))
 		case ResultsTypeArray:
-			errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "array results", config.BetaAPIFields).ViaFieldIndex("results", i))
+			errs = errs.Also(config.ValidateEnabledAPIFields(ctx, "array results", config.BetaAPIFields).ViaFieldIndex("results", i))
 		case ResultsTypeString:
 		default:
 		}
@@ -139,10 +138,10 @@ func (pt *PipelineTask) validateBetaFields(ctx context.Context) *apis.FieldError
 	if pt.TaskRef != nil {
 		// Resolvers
 		if pt.TaskRef.Resolver != "" {
-			errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "taskref.resolver", config.BetaAPIFields))
+			errs = errs.Also(config.ValidateEnabledAPIFields(ctx, "taskref.resolver", config.BetaAPIFields))
 		}
 		if len(pt.TaskRef.Params) > 0 {
-			errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "taskref.params", config.BetaAPIFields))
+			errs = errs.Also(config.ValidateEnabledAPIFields(ctx, "taskref.params", config.BetaAPIFields))
 		}
 	} else if pt.TaskSpec != nil {
 		errs = errs.Also(pt.TaskSpec.ValidateBetaFields(ctx))
@@ -232,7 +231,7 @@ func (pt *PipelineTask) validateMatrix(ctx context.Context) (errs *apis.FieldErr
 	if pt.IsMatrixed() {
 		// This is an alpha feature and will fail validation if it's used in a pipeline spec
 		// when the enable-api-fields feature gate is anything but "alpha".
-		errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "matrix", config.AlphaAPIFields))
+		errs = errs.Also(config.ValidateEnabledAPIFields(ctx, "matrix", config.AlphaAPIFields))
 		errs = errs.Also(pt.Matrix.validateCombinationsCount(ctx))
 		errs = errs.Also(pt.Matrix.validateUniqueParams())
 	}
@@ -307,11 +306,11 @@ func (pt PipelineTask) validateRefOrSpec(ctx context.Context) (errs *apis.FieldE
 		nonNilFields = append(nonNilFields, taskSpec)
 	}
 	if pt.PipelineRef != nil {
-		errs = errs.Also(version.ValidateEnabledAPIFields(ctx, pipelineRef, config.AlphaAPIFields))
+		errs = errs.Also(config.ValidateEnabledAPIFields(ctx, pipelineRef, config.AlphaAPIFields))
 		nonNilFields = append(nonNilFields, pipelineRef)
 	}
 	if pt.PipelineSpec != nil {
-		errs = errs.Also(version.ValidateEnabledAPIFields(ctx, pipelineSpec, config.AlphaAPIFields))
+		errs = errs.Also(config.ValidateEnabledAPIFields(ctx, pipelineSpec, config.AlphaAPIFields))
 		nonNilFields = append(nonNilFields, pipelineSpec)
 	}
 

pkg/apis/pipeline/v1/pipelineref_validation.go

@@ -20,7 +20,6 @@ import (
 	"context"
 
 	"github.com/tektoncd/pipeline/pkg/apis/config"
-	"github.com/tektoncd/pipeline/pkg/apis/version"
 	"knative.dev/pkg/apis"
 )
 
@@ -33,13 +32,13 @@ func (ref *PipelineRef) Validate(ctx context.Context) (errs *apis.FieldError) {
 
 	if ref.Resolver != "" || ref.Params != nil {
 		if ref.Resolver != "" {
-			errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "resolver", config.BetaAPIFields).ViaField("resolver"))
+			errs = errs.Also(config.ValidateEnabledAPIFields(ctx, "resolver", config.BetaAPIFields).ViaField("resolver"))
 			if ref.Name != "" {
 				errs = errs.Also(apis.ErrMultipleOneOf("name", "resolver"))
 			}
 		}
 		if ref.Params != nil {
-			errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "resolver params", config.BetaAPIFields).ViaField("params"))
+			errs = errs.Also(config.ValidateEnabledAPIFields(ctx, "resolver params", config.BetaAPIFields).ViaField("params"))
 			if ref.Name != "" {
 				errs = errs.Also(apis.ErrMultipleOneOf("name", "params"))
 			}

pkg/apis/pipeline/v1/pipelinerun_validation.go

@@ -23,7 +23,6 @@ import (
 
 	"github.com/tektoncd/pipeline/pkg/apis/config"
 	"github.com/tektoncd/pipeline/pkg/apis/validate"
-	"github.com/tektoncd/pipeline/pkg/apis/version"
 	admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"knative.dev/pkg/apis"
@@ -283,15 +282,15 @@ func (ps *PipelineRunSpec) validatePipelineTimeout(timeout time.Duration, errorM
 
 func validateTaskRunSpec(ctx context.Context, trs PipelineTaskRunSpec) (errs *apis.FieldError) {
 	if trs.StepSpecs != nil {
-		errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "stepSpecs", config.AlphaAPIFields).ViaField("stepSpecs"))
+		errs = errs.Also(config.ValidateEnabledAPIFields(ctx, "stepSpecs", config.AlphaAPIFields).ViaField("stepSpecs"))
 		errs = errs.Also(validateStepSpecs(trs.StepSpecs).ViaField("stepSpecs"))
 	}
 	if trs.SidecarSpecs != nil {
-		errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "sidecarSpecs", config.AlphaAPIFields).ViaField("sidecarSpecs"))
+		errs = errs.Also(config.ValidateEnabledAPIFields(ctx, "sidecarSpecs", config.AlphaAPIFields).ViaField("sidecarSpecs"))
 		errs = errs.Also(validateSidecarSpecs(trs.SidecarSpecs).ViaField("sidecarSpecs"))
 	}
 	if trs.ComputeResources != nil {
-		errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "computeResources", config.AlphaAPIFields).ViaField("computeResources"))
+		errs = errs.Also(config.ValidateEnabledAPIFields(ctx, "computeResources", config.AlphaAPIFields).ViaField("computeResources"))
 		errs = errs.Also(validateTaskRunComputeResources(trs.ComputeResources, trs.StepSpecs))
 	}
 	if trs.PodTemplate != nil {

pkg/apis/pipeline/v1/task_validation.go

@@ -27,7 +27,6 @@ import (
 	"github.com/tektoncd/pipeline/pkg/apis/config"
 	"github.com/tektoncd/pipeline/pkg/apis/pipeline"
 	"github.com/tektoncd/pipeline/pkg/apis/validate"
-	"github.com/tektoncd/pipeline/pkg/apis/version"
 	"github.com/tektoncd/pipeline/pkg/substitution"
 	admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
 	corev1 "k8s.io/api/core/v1"
@@ -109,21 +108,21 @@ func (ts *TaskSpec) ValidateBetaFields(ctx context.Context) *apis.FieldError {
 	// Object parameters
 	for i, p := range ts.Params {
 		if p.Type == ParamTypeObject {
-			errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "object type parameter", config.BetaAPIFields).ViaFieldIndex("params", i))
+			errs = errs.Also(config.ValidateEnabledAPIFields(ctx, "object type parameter", config.BetaAPIFields).ViaFieldIndex("params", i))
 		}
 	}
 	// Indexing into array parameters
 	arrayIndexParamRefs := ts.GetIndexingReferencesToArrayParams()
 	if len(arrayIndexParamRefs) != 0 {
-		errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "indexing into array parameters", config.BetaAPIFields))
+		errs = errs.Also(config.ValidateEnabledAPIFields(ctx, "indexing into array parameters", config.BetaAPIFields))
 	}
 	// Array and object results
 	for i, result := range ts.Results {
 		switch result.Type {
 		case ResultsTypeObject:
-			errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "object results", config.BetaAPIFields).ViaFieldIndex("results", i))
+			errs = errs.Also(config.ValidateEnabledAPIFields(ctx, "object results", config.BetaAPIFields).ViaFieldIndex("results", i))
 		case ResultsTypeArray:
-			errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "array results", config.BetaAPIFields).ViaFieldIndex("results", i))
+			errs = errs.Also(config.ValidateEnabledAPIFields(ctx, "array results", config.BetaAPIFields).ViaFieldIndex("results", i))
 		case ResultsTypeString:
 		default:
 		}
@@ -222,7 +221,7 @@ func validateWorkspaceUsages(ctx context.Context, ts *TaskSpec) (errs *apis.Fiel
 
 	for stepIdx, step := range steps {
 		if len(step.Workspaces) != 0 {
-			errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "step workspaces", config.BetaAPIFields).ViaIndex(stepIdx).ViaField("steps"))
+			errs = errs.Also(config.ValidateEnabledAPIFields(ctx, "step workspaces", config.BetaAPIFields).ViaIndex(stepIdx).ViaField("steps"))
 		}
 		for workspaceIdx, w := range step.Workspaces {
 			if !wsNames.Has(w.Name) {
@@ -233,7 +232,7 @@ func validateWorkspaceUsages(ctx context.Context, ts *TaskSpec) (errs *apis.Fiel
 
 	for sidecarIdx, sidecar := range sidecars {
 		if len(sidecar.Workspaces) != 0 {
-			errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "sidecar workspaces", config.BetaAPIFields).ViaIndex(sidecarIdx).ViaField("sidecars"))
+			errs = errs.Also(config.ValidateEnabledAPIFields(ctx, "sidecar workspaces", config.BetaAPIFields).ViaIndex(sidecarIdx).ViaField("sidecars"))
 		}
 		for workspaceIdx, w := range sidecar.Workspaces {
 			if !wsNames.Has(w.Name) {
@@ -325,19 +324,19 @@ func validateStep(ctx context.Context, s Step, names sets.String) (errs *apis.Fi
 	if s.Script != "" {
 		cleaned := strings.TrimSpace(s.Script)
 		if strings.HasPrefix(cleaned, "#!win") {
-			errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "windows script support", config.AlphaAPIFields).ViaField("script"))
+			errs = errs.Also(config.ValidateEnabledAPIFields(ctx, "windows script support", config.AlphaAPIFields).ViaField("script"))
 		}
 	}
 
 	// StdoutConfig is an alpha feature and will fail validation if it's used in a task spec
 	// when the enable-api-fields feature gate is not "alpha".
 	if s.StdoutConfig != nil {
-		errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "step stdout stream support", config.AlphaAPIFields).ViaField("stdoutconfig"))
+		errs = errs.Also(config.ValidateEnabledAPIFields(ctx, "step stdout stream support", config.AlphaAPIFields).ViaField("stdoutconfig"))
 	}
 	// StderrConfig is an alpha feature and will fail validation if it's used in a task spec
 	// when the enable-api-fields feature gate is not "alpha".
 	if s.StderrConfig != nil {
-		errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "step stderr stream support", config.AlphaAPIFields).ViaField("stderrconfig"))
+		errs = errs.Also(config.ValidateEnabledAPIFields(ctx, "step stderr stream support", config.AlphaAPIFields).ViaField("stderrconfig"))
 	}
 	return errs
 }

pkg/apis/pipeline/v1/taskref_validation.go

@@ -21,7 +21,6 @@ import (
 	"strings"
 
 	"github.com/tektoncd/pipeline/pkg/apis/config"
-	"github.com/tektoncd/pipeline/pkg/apis/version"
 	"k8s.io/apimachinery/pkg/util/validation"
 	"knative.dev/pkg/apis"
 )
@@ -36,13 +35,13 @@ func (ref *TaskRef) Validate(ctx context.Context) (errs *apis.FieldError) {
 	switch {
 	case ref.Resolver != "" || ref.Params != nil:
 		if ref.Resolver != "" {
-			errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "resolver", config.BetaAPIFields).ViaField("resolver"))
+			errs = errs.Also(config.ValidateEnabledAPIFields(ctx, "resolver", config.BetaAPIFields).ViaField("resolver"))
 			if ref.Name != "" {
 				errs = errs.Also(apis.ErrMultipleOneOf("name", "resolver"))
 			}
 		}
 		if ref.Params != nil {
-			errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "resolver params", config.BetaAPIFields).ViaField("params"))
+			errs = errs.Also(config.ValidateEnabledAPIFields(ctx, "resolver params", config.BetaAPIFields).ViaField("params"))
 			if ref.Name != "" {
 				errs = errs.Also(apis.ErrMultipleOneOf("name", "params"))
 			}