Skip to content

Commit

Permalink
Move feature flags testing functions into testing package
Browse files Browse the repository at this point in the history
Several functions in the config package are used to set the values of feature flags in tests.
This commit moves them into an existing package used only in testing,
to help ensure that these functions will only be used in testing contexts.
  • Loading branch information
lbernick committed Jul 17, 2023
1 parent 9c249d6 commit e9433d3
Show file tree
Hide file tree
Showing 18 changed files with 175 additions and 167 deletions.
28 changes: 0 additions & 28 deletions pkg/apis/config/feature_flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -345,21 +345,6 @@ func NewFeatureFlagsFromConfigMap(config *corev1.ConfigMap) (*FeatureFlags, erro
return NewFeatureFlagsFromMap(config.Data)
}

// EnableAlphaAPIFields enables alpha features in an existing context (for use in testing)
func EnableAlphaAPIFields(ctx context.Context) context.Context {
return setEnableAPIFields(ctx, AlphaAPIFields)
}

// EnableBetaAPIFields enables beta features in an existing context (for use in testing)
func EnableBetaAPIFields(ctx context.Context) context.Context {
return setEnableAPIFields(ctx, BetaAPIFields)
}

// EnableStableAPIFields enables stable features in an existing context (for use in testing)
func EnableStableAPIFields(ctx context.Context) context.Context {
return setEnableAPIFields(ctx, StableAPIFields)
}

// GetVerificationNoMatchPolicy returns the "trusted-resources-verification-no-match-policy" value
func GetVerificationNoMatchPolicy(ctx context.Context) string {
return FromContextOrDefaults(ctx).FeatureFlags.VerificationNoMatchPolicy
Expand All @@ -375,16 +360,3 @@ func CheckAlphaOrBetaAPIFields(ctx context.Context) bool {
func IsSpireEnabled(ctx context.Context) bool {
return FromContextOrDefaults(ctx).FeatureFlags.EnforceNonfalsifiability == EnforceNonfalsifiabilityWithSpire
}

func setEnableAPIFields(ctx context.Context, want string) context.Context {
featureFlags, _ := NewFeatureFlagsFromMap(map[string]string{
"enable-api-fields": want,
})
cfg := &Config{
Defaults: &Defaults{
DefaultTimeoutMinutes: 60,
},
FeatureFlags: featureFlags,
}
return ToContext(ctx, cfg)
}
7 changes: 4 additions & 3 deletions pkg/apis/config/feature_flags_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (

"github.com/google/go-cmp/cmp"
"github.com/tektoncd/pipeline/pkg/apis/config"
cfgtesting "github.com/tektoncd/pipeline/pkg/apis/config/testing"
test "github.com/tektoncd/pipeline/pkg/reconciler/testing"
"github.com/tektoncd/pipeline/test/diff"
corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -321,15 +322,15 @@ func TestCheckAlphaOrBetaAPIFields(t *testing.T) {
}
testCases := []testCase{{
name: "when enable-api-fields is set to alpha",
c: config.EnableAlphaAPIFields(ctx),
c: cfgtesting.EnableAlphaAPIFields(ctx),
expectedResult: true,
}, {
name: "when enable-api-fields is set to beta",
c: config.EnableBetaAPIFields(ctx),
c: cfgtesting.EnableBetaAPIFields(ctx),
expectedResult: true,
}, {
name: "when enable-api-fields is set to stable",
c: config.EnableStableAPIFields(ctx),
c: cfgtesting.EnableStableAPIFields(ctx),
expectedResult: false,
}}
for _, tc := range testCases {
Expand Down
28 changes: 28 additions & 0 deletions pkg/apis/config/testing/featureflags.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,3 +38,31 @@ func SetFeatureFlags(ctx context.Context, t *testing.T, data map[string]string)
})
return s.ToContext(ctx)
}

// EnableAlphaAPIFields enables alpha features in an existing context (for use in testing)
func EnableAlphaAPIFields(ctx context.Context) context.Context {
return setEnableAPIFields(ctx, config.AlphaAPIFields)
}

// EnableBetaAPIFields enables beta features in an existing context (for use in testing)
func EnableBetaAPIFields(ctx context.Context) context.Context {
return setEnableAPIFields(ctx, config.BetaAPIFields)
}

// EnableStableAPIFields enables stable features in an existing context (for use in testing)
func EnableStableAPIFields(ctx context.Context) context.Context {
return setEnableAPIFields(ctx, config.StableAPIFields)
}

func setEnableAPIFields(ctx context.Context, want string) context.Context {
featureFlags, _ := config.NewFeatureFlagsFromMap(map[string]string{
"enable-api-fields": want,
})
cfg := &config.Config{
Defaults: &config.Defaults{
DefaultTimeoutMinutes: 60,
},
FeatureFlags: featureFlags,
}
return config.ToContext(ctx, cfg)
}
7 changes: 4 additions & 3 deletions pkg/apis/pipeline/v1/pipeline_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"
"github.com/tektoncd/pipeline/pkg/apis/config"
cfgtesting "github.com/tektoncd/pipeline/pkg/apis/config/testing"
"github.com/tektoncd/pipeline/test/diff"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
Expand Down Expand Up @@ -1341,7 +1342,7 @@ func TestValidatePipelineParameterVariables_Success(t *testing.T) {
}}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ctx := config.EnableAlphaAPIFields(context.Background())
ctx := cfgtesting.EnableAlphaAPIFields(context.Background())
err := ValidatePipelineParameterVariables(ctx, tt.tasks, tt.params)
if err != nil {
t.Errorf("Pipeline.ValidatePipelineParameterVariables() returned error for valid pipeline parameters: %v", err)
Expand Down Expand Up @@ -3537,12 +3538,12 @@ func TestPipelineWithBetaFields(t *testing.T) {
for _, tt := range tts {
t.Run(tt.name, func(t *testing.T) {
pipeline := Pipeline{ObjectMeta: metav1.ObjectMeta{Name: "foo"}, Spec: tt.spec}
ctx := config.EnableStableAPIFields(context.Background())
ctx := cfgtesting.EnableStableAPIFields(context.Background())
if err := pipeline.Validate(ctx); err == nil {
t.Errorf("no error when using beta field when `enable-api-fields` is stable")
}

ctx = config.EnableBetaAPIFields(context.Background())
ctx = cfgtesting.EnableBetaAPIFields(context.Background())
if err := pipeline.Validate(ctx); err != nil {
t.Errorf("unexpected error when using beta field: %s", err)
}
Expand Down
20 changes: 10 additions & 10 deletions pkg/apis/pipeline/v1/pipelineref_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import (
"testing"

"github.com/google/go-cmp/cmp"
"github.com/tektoncd/pipeline/pkg/apis/config"
cfgtesting "github.com/tektoncd/pipeline/pkg/apis/config/testing"
v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
"github.com/tektoncd/pipeline/test/diff"
"knative.dev/pkg/apis"
Expand All @@ -44,7 +44,7 @@ func TestPipelineRef_Invalid(t *testing.T) {
Resolver: "foo",
},
},
withContext: config.EnableStableAPIFields,
withContext: cfgtesting.EnableStableAPIFields,
wantErr: apis.ErrGeneric("resolver requires \"enable-api-fields\" feature gate to be \"alpha\" or \"beta\" but it is \"stable\""),
}, {
name: "pipelineref params disallowed without beta feature gate",
Expand All @@ -53,7 +53,7 @@ func TestPipelineRef_Invalid(t *testing.T) {
Params: v1.Params{},
},
},
withContext: config.EnableStableAPIFields,
withContext: cfgtesting.EnableStableAPIFields,
wantErr: apis.ErrMissingField("resolver").Also(apis.ErrGeneric("resolver params requires \"enable-api-fields\" feature gate to be \"alpha\" or \"beta\" but it is \"stable\"")),
}, {
name: "pipelineref params disallowed without resolver",
Expand All @@ -63,7 +63,7 @@ func TestPipelineRef_Invalid(t *testing.T) {
},
},
wantErr: apis.ErrMissingField("resolver"),
withContext: config.EnableBetaAPIFields,
withContext: cfgtesting.EnableBetaAPIFields,
}, {
name: "pipelineref resolver disallowed in conjunction with pipelineref name",
ref: &v1.PipelineRef{
Expand All @@ -73,7 +73,7 @@ func TestPipelineRef_Invalid(t *testing.T) {
},
},
wantErr: apis.ErrMultipleOneOf("name", "resolver"),
withContext: config.EnableBetaAPIFields,
withContext: cfgtesting.EnableBetaAPIFields,
}, {
name: "pipelineref params disallowed in conjunction with pipelineref name",
ref: &v1.PipelineRef{
Expand All @@ -89,7 +89,7 @@ func TestPipelineRef_Invalid(t *testing.T) {
},
},
wantErr: apis.ErrMultipleOneOf("name", "params").Also(apis.ErrMissingField("resolver")),
withContext: config.EnableBetaAPIFields,
withContext: cfgtesting.EnableBetaAPIFields,
}, {
name: "pipelineref param object not allowed in stable",
ref: &v1.PipelineRef{
Expand All @@ -107,7 +107,7 @@ func TestPipelineRef_Invalid(t *testing.T) {
wantErr: apis.ErrGeneric("resolver requires \"enable-api-fields\" feature gate to be \"alpha\" or \"beta\" but it is \"stable\"").Also(
apis.ErrGeneric("resolver params requires \"enable-api-fields\" feature gate to be \"alpha\" or \"beta\" but it is \"stable\"")).Also(
apis.ErrGeneric("object type parameter requires \"enable-api-fields\" feature gate to be \"alpha\" or \"beta\" but it is \"stable\"")),
withContext: config.EnableStableAPIFields,
withContext: cfgtesting.EnableStableAPIFields,
}}

for _, tc := range tests {
Expand Down Expand Up @@ -135,11 +135,11 @@ func TestPipelineRef_Valid(t *testing.T) {
}, {
name: "beta feature: valid resolver",
ref: &v1.PipelineRef{ResolverRef: v1.ResolverRef{Resolver: "git"}},
wc: config.EnableBetaAPIFields,
wc: cfgtesting.EnableBetaAPIFields,
}, {
name: "beta feature: valid resolver with alpha flag",
ref: &v1.PipelineRef{ResolverRef: v1.ResolverRef{Resolver: "git"}},
wc: config.EnableAlphaAPIFields,
wc: cfgtesting.EnableAlphaAPIFields,
}, {
name: "alpha feature: valid resolver with params",
ref: &v1.PipelineRef{ResolverRef: v1.ResolverRef{Resolver: "git", Params: v1.Params{{
Expand All @@ -155,7 +155,7 @@ func TestPipelineRef_Valid(t *testing.T) {
StringVal: "baz",
},
}}}},
wc: config.EnableAlphaAPIFields,
wc: cfgtesting.EnableAlphaAPIFields,
}}

for _, ts := range tests {
Expand Down
Loading

0 comments on commit e9433d3

Please sign in to comment.