Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move two feature flag related functions to config pkg #5113

Merged
merged 1 commit into from
Jul 8, 2022
Merged

Move two feature flag related functions to config pkg #5113

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions pkg/apis/config/feature_flags.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package config

import (
"context"
"fmt"
"os"
"strconv"
Expand Down Expand Up @@ -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)
}
14 changes: 7 additions & 7 deletions pkg/apis/pipeline/v1beta1/pipelineref_validation_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -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{
Expand All @@ -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{
Expand All @@ -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{
Expand All @@ -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 {
Expand Down Expand Up @@ -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{{
Expand All @@ -170,7 +170,7 @@ func TestPipelineRef_Valid(t *testing.T) {
Name: "branch",
Value: "baz",
}}}},
wc: enableAlphaAPIFields,
wc: config.EnableAlphaAPIFields,
}}

for _, ts := range tests {
Expand Down
29 changes: 8 additions & 21 deletions pkg/apis/pipeline/v1beta1/pipelinerun_validation_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ func TestPipelineRun_Validate(t *testing.T) {
},
},
},
wc: enableAlphaAPIFields,
wc: config.EnableAlphaAPIFields,
}}

for _, ts := range tests {
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand All @@ -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{
Expand All @@ -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{
Expand All @@ -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{
Expand Down Expand Up @@ -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{
Expand All @@ -570,7 +570,7 @@ func TestPipelineRunSpec_Validate(t *testing.T) {
}},
}},
},
withContext: enableAlphaAPIFields,
withContext: config.EnableAlphaAPIFields,
}}

for _, ps := range tests {
Expand Down Expand Up @@ -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)
}
12 changes: 10 additions & 2 deletions pkg/apis/pipeline/v1beta1/result_validation_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
Expand Down
23 changes: 8 additions & 15 deletions pkg/apis/pipeline/v1beta1/task_validation_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand Down
17 changes: 9 additions & 8 deletions pkg/apis/pipeline/v1beta1/taskref_validation_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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{{
Expand All @@ -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) {
Expand Down Expand Up @@ -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{
Expand All @@ -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{
Expand All @@ -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{
Expand All @@ -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{
Expand All @@ -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) {
Expand Down
Loading