Move feature flag related functions to config pkg

- getContextBasedOnFeatureFlag - enableAlphaAPIFields Those two feature flag related functions are defined in v1beta1_test packages but used across v1beta1 and v1beta1_test. Since we cannot import v1beta1_test from v1beta1, moving it to the config package will be a solution for it to be used across different packages. Also getContextBasedOnFeatureFlag is overlapping with enableAlphaAPIFields. So it's sufficient to only keep enableAlphaAPIFields.
tektoncd · Jul 8, 2022 · df4d68b · df4d68b
1 parent dc9ea6c
commit df4d68b
Show file tree

Hide file tree

Showing 7 changed files with 68 additions and 63 deletions.
diff --git a/pkg/apis/config/feature_flags.go b/pkg/apis/config/feature_flags.go
@@ -17,6 +17,7 @@ limitations under the License.
 package config
 
 import (
+	"context"
 	"fmt"
 	"os"
 	"strconv"
@@ -187,3 +188,17 @@ func setEmbeddedStatus(cfgMap map[string]string, defaultValue string, feature *s
 func NewFeatureFlagsFromConfigMap(config *corev1.ConfigMap) (*FeatureFlags, error) {
 	return NewFeatureFlagsFromMap(config.Data)
 }
+
+// EnableAlphaAPIFields enables alpha feature in an existing context (for use in testing)
+func EnableAlphaAPIFields(ctx context.Context) context.Context {
+	featureFlags, _ := NewFeatureFlagsFromMap(map[string]string{
+		"enable-api-fields": "alpha",
+	})
+	cfg := &Config{
+		Defaults: &Defaults{
+			DefaultTimeoutMinutes: 60,
+		},
+		FeatureFlags: featureFlags,
+	}
+	return ToContext(ctx, cfg)
+}
diff --git a/pkg/apis/pipeline/v1beta1/pipelineref_validation_test.go b/pkg/apis/pipeline/v1beta1/pipelineref_validation_test.go
@@ -86,7 +86,7 @@ func TestPipelineRef_Invalid(t *testing.T) {
 			},
 		},
 		wantErr:     apis.ErrMissingField("resolver"),
-		withContext: enableAlphaAPIFields,
+		withContext: config.EnableAlphaAPIFields,
 	}, {
 		name: "pipelineref resolver disallowed in conjunction with pipelineref name",
 		ref: &v1beta1.PipelineRef{
@@ -96,7 +96,7 @@ func TestPipelineRef_Invalid(t *testing.T) {
 			},
 		},
 		wantErr:     apis.ErrMultipleOneOf("name", "resolver"),
-		withContext: enableAlphaAPIFields,
+		withContext: config.EnableAlphaAPIFields,
 	}, {
 		name: "pipelineref resolver disallowed in conjunction with pipelineref bundle",
 		ref: &v1beta1.PipelineRef{
@@ -106,7 +106,7 @@ func TestPipelineRef_Invalid(t *testing.T) {
 			},
 		},
 		wantErr:     apis.ErrMultipleOneOf("bundle", "resolver"),
-		withContext: enableAlphaAPIFields,
+		withContext: config.EnableAlphaAPIFields,
 	}, {
 		name: "pipelineref resource disallowed in conjunction with pipelineref name",
 		ref: &v1beta1.PipelineRef{
@@ -119,7 +119,7 @@ func TestPipelineRef_Invalid(t *testing.T) {
 			},
 		},
 		wantErr:     apis.ErrMultipleOneOf("name", "resource").Also(apis.ErrMissingField("resolver")),
-		withContext: enableAlphaAPIFields,
+		withContext: config.EnableAlphaAPIFields,
 	}, {
 		name: "pipelineref resource disallowed in conjunction with pipelineref bundle",
 		ref: &v1beta1.PipelineRef{
@@ -132,7 +132,7 @@ func TestPipelineRef_Invalid(t *testing.T) {
 			},
 		},
 		wantErr:     apis.ErrMultipleOneOf("bundle", "resource").Also(apis.ErrMissingField("resolver")),
-		withContext: enableAlphaAPIFields,
+		withContext: config.EnableAlphaAPIFields,
 	}}
 
 	for _, tc := range tests {
@@ -160,7 +160,7 @@ func TestPipelineRef_Valid(t *testing.T) {
 	}, {
 		name: "alpha feature: valid resolver",
 		ref:  &v1beta1.PipelineRef{ResolverRef: v1beta1.ResolverRef{Resolver: "git"}},
-		wc:   enableAlphaAPIFields,
+		wc:   config.EnableAlphaAPIFields,
 	}, {
 		name: "alpha feature: valid resolver with resource parameters",
 		ref: &v1beta1.PipelineRef{ResolverRef: v1beta1.ResolverRef{Resolver: "git", Resource: []v1beta1.ResolverParam{{
@@ -170,7 +170,7 @@ func TestPipelineRef_Valid(t *testing.T) {
 			Name:  "branch",
 			Value: "baz",
 		}}}},
-		wc: enableAlphaAPIFields,
+		wc: config.EnableAlphaAPIFields,
 	}}
 
 	for _, ts := range tests {

diff --git a/pkg/apis/pipeline/v1beta1/pipelinerun_validation_test.go b/pkg/apis/pipeline/v1beta1/pipelinerun_validation_test.go
@@ -289,7 +289,7 @@ func TestPipelineRun_Validate(t *testing.T) {
 				},
 			},
 		},
-		wc: enableAlphaAPIFields,
+		wc: config.EnableAlphaAPIFields,
 	}}
 
 	for _, ts := range tests {
@@ -378,7 +378,7 @@ func TestPipelineRunSpec_Invalidate(t *testing.T) {
 			},
 		},
 		wantErr:     apis.ErrMultipleOneOf("taskRunSpecs[0].stepOverrides[1].name"),
-		withContext: enableAlphaAPIFields,
+		withContext: config.EnableAlphaAPIFields,
 	}, {
 		name: "stepOverride disallowed without alpha feature gate",
 		spec: v1beta1.PipelineRunSpec{
@@ -429,7 +429,7 @@ func TestPipelineRunSpec_Invalidate(t *testing.T) {
 			},
 		},
 		wantErr:     apis.ErrMissingField("taskRunSpecs[0].stepOverrides[0].name"),
-		withContext: enableAlphaAPIFields,
+		withContext: config.EnableAlphaAPIFields,
 	}, {
 		name: "duplicate sidecarOverride names",
 		spec: v1beta1.PipelineRunSpec{
@@ -444,7 +444,7 @@ func TestPipelineRunSpec_Invalidate(t *testing.T) {
 			},
 		},
 		wantErr:     apis.ErrMultipleOneOf("taskRunSpecs[0].sidecarOverrides[1].name"),
-		withContext: enableAlphaAPIFields,
+		withContext: config.EnableAlphaAPIFields,
 	}, {
 		name: "missing sidecarOverride name",
 		spec: v1beta1.PipelineRunSpec{
@@ -461,7 +461,7 @@ func TestPipelineRunSpec_Invalidate(t *testing.T) {
 			},
 		},
 		wantErr:     apis.ErrMissingField("taskRunSpecs[0].sidecarOverrides[0].name"),
-		withContext: enableAlphaAPIFields,
+		withContext: config.EnableAlphaAPIFields,
 	}, {
 		name: "invalid both step-level (stepOverrides.resources) and task-level (taskRunSpecs.resources) resource requirements configured",
 		spec: v1beta1.PipelineRunSpec{
@@ -485,7 +485,7 @@ func TestPipelineRunSpec_Invalidate(t *testing.T) {
 			"taskRunSpecs[0].stepOverrides.resources",
 			"taskRunSpecs[0].computeResources",
 		),
-		withContext: enableAlphaAPIFields,
+		withContext: config.EnableAlphaAPIFields,
 	}, {
 		name: "computeResources disallowed without alpha feature gate",
 		spec: v1beta1.PipelineRunSpec{
@@ -547,7 +547,7 @@ func TestPipelineRunSpec_Validate(t *testing.T) {
 				},
 			}},
 		},
-		withContext: enableAlphaAPIFields,
+		withContext: config.EnableAlphaAPIFields,
 	}, {
 		name: "valid sidecar and task-level (taskRunSpecs.resources) resource requirements configured",
 		spec: v1beta1.PipelineRunSpec{
@@ -570,7 +570,7 @@ func TestPipelineRunSpec_Validate(t *testing.T) {
 				}},
 			}},
 		},
-		withContext: enableAlphaAPIFields,
+		withContext: config.EnableAlphaAPIFields,
 	}}
 
 	for _, ps := range tests {
@@ -840,16 +840,3 @@ func TestPipelineRunWithTimeout_Validate(t *testing.T) {
 		})
 	}
 }
-
-func enableAlphaAPIFields(ctx context.Context) context.Context {
-	featureFlags, _ := config.NewFeatureFlagsFromMap(map[string]string{
-		"enable-api-fields": "alpha",
-	})
-	cfg := &config.Config{
-		Defaults: &config.Defaults{
-			DefaultTimeoutMinutes: 60,
-		},
-		FeatureFlags: featureFlags,
-	}
-	return config.ToContext(ctx, cfg)
-}
diff --git a/pkg/apis/pipeline/v1beta1/result_validation_test.go b/pkg/apis/pipeline/v1beta1/result_validation_test.go
@@ -17,10 +17,12 @@ limitations under the License.
 package v1beta1_test
 
 import (
+	"context"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-cmp/cmp/cmpopts"
+	"github.com/tektoncd/pipeline/pkg/apis/config"
 	"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
 	"github.com/tektoncd/pipeline/test/diff"
 	"knative.dev/pkg/apis"
@@ -68,7 +70,10 @@ func TestResultsValidate(t *testing.T) {
 	}}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			ctx := getContextBasedOnFeatureFlag(tt.apiFields)
+			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)
 			}
@@ -133,7 +138,10 @@ func TestResultsValidateError(t *testing.T) {
 	}}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			ctx := getContextBasedOnFeatureFlag(tt.apiFields)
+			ctx := context.Background()
+			if tt.apiFields == "alpha" {
+				ctx = config.EnableAlphaAPIFields(ctx)
+			}
 			err := tt.Result.Validate(ctx)
 			if err == nil {
 				t.Fatalf("Expected an error, got nothing for %v", tt.Result)

diff --git a/pkg/apis/pipeline/v1beta1/task_validation_test.go b/pkg/apis/pipeline/v1beta1/task_validation_test.go
@@ -427,7 +427,7 @@ func TestTaskSpecValidate(t *testing.T) {
 				Workspaces:   tt.fields.Workspaces,
 				Results:      tt.fields.Results,
 			}
-			ctx := getContextBasedOnFeatureFlag("alpha")
+			ctx := config.EnableAlphaAPIFields(context.Background())
 			ts.SetDefaults(ctx)
 			if err := ts.Validate(ctx); err != nil {
 				t.Errorf("TaskSpec.Validate() = %v", err)
@@ -1279,7 +1279,7 @@ func TestTaskSpecValidateError(t *testing.T) {
 				Workspaces:   tt.fields.Workspaces,
 				Results:      tt.fields.Results,
 			}
-			ctx := getContextBasedOnFeatureFlag("alpha")
+			ctx := config.EnableAlphaAPIFields(context.Background())
 			ts.SetDefaults(ctx)
 			err := ts.Validate(ctx)
 			if err == nil {
@@ -1326,7 +1326,7 @@ func TestStepAndSidecarWorkspaces(t *testing.T) {
 				Sidecars:   tt.fields.Sidecars,
 				Workspaces: tt.fields.Workspaces,
 			}
-			ctx := getContextBasedOnFeatureFlag("alpha")
+			ctx := config.EnableAlphaAPIFields(context.Background())
 			ts.SetDefaults(ctx)
 			if err := ts.Validate(ctx); err != nil {
 				t.Errorf("TaskSpec.Validate() = %v", err)
@@ -1383,7 +1383,7 @@ func TestStepAndSidecarWorkspacesErrors(t *testing.T) {
 				Sidecars: tt.fields.Sidecars,
 			}
 
-			ctx := getContextBasedOnFeatureFlag("alpha")
+			ctx := config.EnableAlphaAPIFields(context.Background())
 			ts.SetDefaults(ctx)
 			err := ts.Validate(ctx)
 			if err == nil {
@@ -1503,7 +1503,10 @@ func TestIncompatibleAPIVersions(t *testing.T) {
 			testName := fmt.Sprintf("(using %s) %s", version, tt.name)
 			t.Run(testName, func(t *testing.T) {
 				ts := tt.spec
-				ctx := getContextBasedOnFeatureFlag(version)
+				ctx := context.Background()
+				if version == "alpha" {
+					ctx = config.EnableAlphaAPIFields(ctx)
+				}
 
 				ts.SetDefaults(ctx)
 				err := ts.Validate(ctx)
@@ -1520,16 +1523,6 @@ func TestIncompatibleAPIVersions(t *testing.T) {
 	}
 }
 
-func getContextBasedOnFeatureFlag(featureFlag string) context.Context {
-	featureFlags, _ := config.NewFeatureFlagsFromMap(map[string]string{
-		"enable-api-fields": featureFlag,
-	})
-	cfg := &config.Config{
-		FeatureFlags: featureFlags,
-	}
-
-	return config.ToContext(context.Background(), cfg)
-}
 func TestSubstitutedContext(t *testing.T) {
 	type fields struct {
 		Params              []v1beta1.ParamSpec

diff --git a/pkg/apis/pipeline/v1beta1/taskref_validation_test.go b/pkg/apis/pipeline/v1beta1/taskref_validation_test.go
@@ -21,6 +21,7 @@ import (
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
+	"github.com/tektoncd/pipeline/pkg/apis/config"
 	"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
 	"github.com/tektoncd/pipeline/test/diff"
 	"knative.dev/pkg/apis"
@@ -39,7 +40,7 @@ func TestTaskRef_Valid(t *testing.T) {
 	}, {
 		name:    "alpha feature: valid resolver",
 		taskRef: &v1beta1.TaskRef{ResolverRef: v1beta1.ResolverRef{Resolver: "git"}},
-		wc:      enableAlphaAPIFields,
+		wc:      config.EnableAlphaAPIFields,
 	}, {
 		name: "alpha feature: valid resolver with resource parameters",
 		taskRef: &v1beta1.TaskRef{ResolverRef: v1beta1.ResolverRef{Resolver: "git", Resource: []v1beta1.ResolverParam{{
@@ -49,13 +50,13 @@ func TestTaskRef_Valid(t *testing.T) {
 			Name:  "branch",
 			Value: "baz",
 		}}}},
-		wc: enableAlphaAPIFields,
+		wc: config.EnableAlphaAPIFields,
 	}, {
 		name: "valid bundle",
 		taskRef: &v1beta1.TaskRef{
 			Name:   "bundled-task",
 			Bundle: "gcr.io/my-bundle"},
-		wc: enableAlphaAPIFields,
+		wc: config.EnableAlphaAPIFields,
 	}}
 	for _, ts := range tests {
 		t.Run(ts.name, func(t *testing.T) {
@@ -126,7 +127,7 @@ func TestTaskRef_Invalid(t *testing.T) {
 			},
 		},
 		wantErr: apis.ErrMissingField("resolver"),
-		wc:      enableAlphaAPIFields,
+		wc:      config.EnableAlphaAPIFields,
 	}, {
 		name: "taskref resolver disallowed in conjunction with taskref name",
 		taskRef: &v1beta1.TaskRef{
@@ -136,7 +137,7 @@ func TestTaskRef_Invalid(t *testing.T) {
 			},
 		},
 		wantErr: apis.ErrMultipleOneOf("name", "resolver"),
-		wc:      enableAlphaAPIFields,
+		wc:      config.EnableAlphaAPIFields,
 	}, {
 		name: "taskref resolver disallowed in conjunction with taskref bundle",
 		taskRef: &v1beta1.TaskRef{
@@ -146,7 +147,7 @@ func TestTaskRef_Invalid(t *testing.T) {
 			},
 		},
 		wantErr: apis.ErrMultipleOneOf("bundle", "resolver"),
-		wc:      enableAlphaAPIFields,
+		wc:      config.EnableAlphaAPIFields,
 	}, {
 		name: "taskref resource disallowed in conjunction with taskref name",
 		taskRef: &v1beta1.TaskRef{
@@ -159,7 +160,7 @@ func TestTaskRef_Invalid(t *testing.T) {
 			},
 		},
 		wantErr: apis.ErrMultipleOneOf("name", "resource").Also(apis.ErrMissingField("resolver")),
-		wc:      enableAlphaAPIFields,
+		wc:      config.EnableAlphaAPIFields,
 	}, {
 		name: "taskref resource disallowed in conjunction with taskref bundle",
 		taskRef: &v1beta1.TaskRef{
@@ -172,7 +173,7 @@ func TestTaskRef_Invalid(t *testing.T) {
 			},
 		},
 		wantErr: apis.ErrMultipleOneOf("bundle", "resource").Also(apis.ErrMissingField("resolver")),
-		wc:      enableAlphaAPIFields,
+		wc:      config.EnableAlphaAPIFields,
 	}}
 	for _, ts := range tests {
 		t.Run(ts.name, func(t *testing.T) {