Skip to content

Commit

Permalink
Enable beta features by default
Browse files Browse the repository at this point in the history
When users decide to upgrade to v1 APIs, they may not be aware they
are using beta features. If they are using the default value ("stable") of "enable-api-fields",
any beta features they're using will work in beta APIs, but break when when move to v1 APIs.

This commit changes the default value of "enable-api-fields" to "beta", to ensure beta
features continue to work by default when users change API versions.
Independently, we plan to decouple API versioning from feature versioning.

This commit is not backwards compatible.
  • Loading branch information
lbernick committed May 30, 2023
1 parent cb2e07d commit d7fc3eb
Show file tree
Hide file tree
Showing 18 changed files with 59 additions and 27 deletions.
2 changes: 1 addition & 1 deletion config/config-feature-flags.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ data:
enable-tekton-oci-bundles: "false"
# Setting this flag will determine which gated features are enabled.
# Acceptable values are "stable", "beta", or "alpha".
enable-api-fields: "stable"
enable-api-fields: "beta"
# Setting this flag to "true" enables CloudEvents for CustomRuns and Runs, as long as a
# CloudEvents sink is configured in the config-defaults config map
send-cloudevents-for-runs: "false"
Expand Down
7 changes: 4 additions & 3 deletions docs/additional-configs.md
Original file line number Diff line number Diff line change
Expand Up @@ -237,9 +237,10 @@ not running.
and use Workspaces to mount credentials from Secrets instead.
The default is `false`. For more information, see the [associated issue](https://github.com/tektoncd/pipeline/issues/3399).

- `enable-api-fields`: set this flag to "stable" to allow only the
most stable features to be used. Set it to "alpha" to allow [alpha
features](#alpha-features) to be used.
- `enable-api-fields`: When using v1beta1 APIs, setting this field to "stable" or "beta"
enables [beta features](#beta-features). When using v1 APIs, setting this field to "stable"
allows only stable features, and setting it to "beta" allows only beta features.
Set this field to "alpha" to allow [alpha features](#alpha-features) to be used.

- `trusted-resources-verification-no-match-policy`: Setting this flag to `fail` will fail the taskrun/pipelinerun if no matching policies found. Setting to `warn` will skip verification and log a warning if no matching policies are found, but not fail the taskrun/pipelinerun. Setting to `ignore` will skip verification if no matching policies found.
Defaults to "ignore".
Expand Down
2 changes: 1 addition & 1 deletion pkg/apis/config/feature_flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ const (
// DefaultEnableTektonOciBundles is the default value for "enable-tekton-oci-bundles".
DefaultEnableTektonOciBundles = false
// DefaultEnableAPIFields is the default value for "enable-api-fields".
DefaultEnableAPIFields = StableAPIFields
DefaultEnableAPIFields = BetaAPIFields
// DefaultSendCloudEventsForRuns is the default value for "send-cloudevents-for-runs".
DefaultSendCloudEventsForRuns = false
// EnforceNonfalsifiabilityWithSpire is the value used for "enable-nonfalsifiability" when SPIRE is used to enable non-falsifiability.
Expand Down
2 changes: 1 addition & 1 deletion pkg/apis/config/feature_flags_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) {
},
{
expectedConfig: &config.FeatureFlags{
EnableAPIFields: "stable",
EnableAPIFields: config.DefaultEnableAPIFields,
VerificationNoMatchPolicy: config.DefaultNoMatchPolicyConfig,
RunningInEnvWithInjectedSidecars: config.DefaultRunningInEnvWithInjectedSidecars,
AwaitSidecarReadiness: config.DefaultAwaitSidecarReadiness,
Expand Down
5 changes: 3 additions & 2 deletions pkg/apis/pipeline/v1/pipeline_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3704,11 +3704,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}
if err := pipeline.Validate(context.Background()); err == nil {
ctx := config.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 = config.EnableBetaAPIFields(context.Background())
if err := pipeline.Validate(ctx); err != nil {
t.Errorf("unexpected error when using beta field: %s", err)
}
Expand Down
6 changes: 4 additions & 2 deletions pkg/apis/pipeline/v1/pipelineref_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,15 +44,17 @@ func TestPipelineRef_Invalid(t *testing.T) {
Resolver: "foo",
},
},
wantErr: apis.ErrGeneric("resolver requires \"enable-api-fields\" feature gate to be \"alpha\" or \"beta\" but it is \"stable\""),
withContext: config.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",
ref: &v1.PipelineRef{
ResolverRef: v1.ResolverRef{
Params: v1.Params{},
},
},
wantErr: apis.ErrMissingField("resolver").Also(apis.ErrGeneric("resolver params requires \"enable-api-fields\" feature gate to be \"alpha\" or \"beta\" but it is \"stable\"")),
withContext: config.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",
ref: &v1.PipelineRef{
Expand Down
9 changes: 6 additions & 3 deletions pkg/apis/pipeline/v1/pipelinerun_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -931,7 +931,8 @@ func TestPipelineRunSpec_Invalidate(t *testing.T) {
},
},
},
wantErr: apis.ErrGeneric("stepSpecs requires \"enable-api-fields\" feature gate to be \"alpha\" but it is \"stable\"").ViaIndex(0).ViaField("taskRunSpecs"),
withContext: config.EnableStableAPIFields,
wantErr: apis.ErrGeneric("stepSpecs requires \"enable-api-fields\" feature gate to be \"alpha\" but it is \"stable\"").ViaIndex(0).ViaField("taskRunSpecs"),
}, {
name: "sidecarSpec disallowed without alpha feature gate",
spec: v1.PipelineRunSpec{
Expand All @@ -948,7 +949,8 @@ func TestPipelineRunSpec_Invalidate(t *testing.T) {
},
},
},
wantErr: apis.ErrGeneric("sidecarSpecs requires \"enable-api-fields\" feature gate to be \"alpha\" but it is \"stable\"").ViaIndex(0).ViaField("taskRunSpecs"),
withContext: config.EnableStableAPIFields,
wantErr: apis.ErrGeneric("sidecarSpecs requires \"enable-api-fields\" feature gate to be \"alpha\" but it is \"stable\"").ViaIndex(0).ViaField("taskRunSpecs"),
}, {
name: "missing stepSpecs name",
spec: v1.PipelineRunSpec{
Expand Down Expand Up @@ -1035,7 +1037,8 @@ func TestPipelineRunSpec_Invalidate(t *testing.T) {
},
},
},
wantErr: apis.ErrGeneric("computeResources requires \"enable-api-fields\" feature gate to be \"alpha\" but it is \"stable\"").ViaIndex(0).ViaField("taskRunSpecs"),
withContext: config.EnableStableAPIFields,
wantErr: apis.ErrGeneric("computeResources requires \"enable-api-fields\" feature gate to be \"alpha\" but it is \"stable\"").ViaIndex(0).ViaField("taskRunSpecs"),
}}

for _, ps := range tests {
Expand Down
5 changes: 3 additions & 2 deletions pkg/apis/pipeline/v1/task_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1925,11 +1925,12 @@ func TestTaskSpecBetaFields(t *testing.T) {
}}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if err := tt.spec.Validate(context.Background()); err == nil {
ctx := config.EnableStableAPIFields(context.Background())
if err := tt.spec.Validate(ctx); err == nil {
t.Errorf("no error when using beta field when `enable-api-fields` is stable")
}

ctx := config.EnableBetaAPIFields(context.Background())
ctx = config.EnableBetaAPIFields(context.Background())
if err := tt.spec.Validate(ctx); err != nil {
t.Errorf("unexpected error when using beta field: %s", err)
}
Expand Down
2 changes: 2 additions & 0 deletions pkg/apis/pipeline/v1/taskref_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ func TestTaskRef_Invalid(t *testing.T) {
Resolver: "git",
},
},
wc: config.EnableStableAPIFields,
wantErr: apis.ErrGeneric("resolver requires \"enable-api-fields\" feature gate to be \"alpha\" or \"beta\" but it is \"stable\""),
}, {
name: "taskref params disallowed without beta feature gate",
Expand All @@ -100,6 +101,7 @@ func TestTaskRef_Invalid(t *testing.T) {
Params: v1.Params{},
},
},
wc: config.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: "taskref params disallowed without resolver",
Expand Down
4 changes: 4 additions & 0 deletions pkg/apis/pipeline/v1/taskrun_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -640,6 +640,7 @@ func TestTaskRunSpec_Invalidate(t *testing.T) {
Breakpoint: []string{"onFailure"},
},
},
wc: config.EnableStableAPIFields,
wantErr: apis.ErrGeneric("debug requires \"enable-api-fields\" feature gate to be \"alpha\" but it is \"stable\""),
}, {
name: "invalid breakpoint",
Expand All @@ -666,6 +667,7 @@ func TestTaskRunSpec_Invalidate(t *testing.T) {
},
}},
},
wc: config.EnableStableAPIFields,
wantErr: apis.ErrGeneric("stepSpecs requires \"enable-api-fields\" feature gate to be \"alpha\" but it is \"stable\""),
}, {
name: "sidecarSpec disallowed without alpha feature gate",
Expand All @@ -680,6 +682,7 @@ func TestTaskRunSpec_Invalidate(t *testing.T) {
},
}},
},
wc: config.EnableStableAPIFields,
wantErr: apis.ErrGeneric("sidecarSpecs requires \"enable-api-fields\" feature gate to be \"alpha\" but it is \"stable\""),
}, {
name: "duplicate stepSpecs names",
Expand Down Expand Up @@ -776,6 +779,7 @@ func TestTaskRunSpec_Invalidate(t *testing.T) {
},
},
},
wc: config.EnableStableAPIFields,
wantErr: apis.ErrGeneric("computeResources requires \"enable-api-fields\" feature gate to be \"alpha\" but it is \"stable\""),
}}

Expand Down
1 change: 1 addition & 0 deletions pkg/apis/pipeline/v1/workspace_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,7 @@ func TestWorkspaceBindingValidateInvalid(t *testing.T) {
Driver: "csi-driver",
},
},
wc: config.EnableStableAPIFields,
}, {
name: "Provide csi without a driver",
binding: &v1.WorkspaceBinding{
Expand Down
9 changes: 6 additions & 3 deletions pkg/apis/pipeline/v1beta1/pipelinerun_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1065,7 +1065,8 @@ func TestPipelineRunSpec_Invalidate(t *testing.T) {
},
},
},
wantErr: apis.ErrGeneric("stepOverrides requires \"enable-api-fields\" feature gate to be \"alpha\" but it is \"stable\"").ViaIndex(0).ViaField("taskRunSpecs"),
withContext: config.EnableStableAPIFields,
wantErr: apis.ErrGeneric("stepOverrides requires \"enable-api-fields\" feature gate to be \"alpha\" but it is \"stable\"").ViaIndex(0).ViaField("taskRunSpecs"),
}, {
name: "sidecarOverride disallowed without alpha feature gate",
spec: v1beta1.PipelineRunSpec{
Expand All @@ -1082,7 +1083,8 @@ func TestPipelineRunSpec_Invalidate(t *testing.T) {
},
},
},
wantErr: apis.ErrGeneric("sidecarOverrides requires \"enable-api-fields\" feature gate to be \"alpha\" but it is \"stable\"").ViaIndex(0).ViaField("taskRunSpecs"),
withContext: config.EnableStableAPIFields,
wantErr: apis.ErrGeneric("sidecarOverrides requires \"enable-api-fields\" feature gate to be \"alpha\" but it is \"stable\"").ViaIndex(0).ViaField("taskRunSpecs"),
}, {
name: "missing stepOverride name",
spec: v1beta1.PipelineRunSpec{
Expand Down Expand Up @@ -1169,7 +1171,8 @@ func TestPipelineRunSpec_Invalidate(t *testing.T) {
},
},
},
wantErr: apis.ErrGeneric("computeResources requires \"enable-api-fields\" feature gate to be \"alpha\" but it is \"stable\"").ViaIndex(0).ViaField("taskRunSpecs"),
withContext: config.EnableStableAPIFields,
wantErr: apis.ErrGeneric("computeResources requires \"enable-api-fields\" feature gate to be \"alpha\" but it is \"stable\"").ViaIndex(0).ViaField("taskRunSpecs"),
}}

for _, ps := range tests {
Expand Down
4 changes: 4 additions & 0 deletions pkg/apis/pipeline/v1beta1/taskrun_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -628,6 +628,7 @@ func TestTaskRunSpec_Invalidate(t *testing.T) {
Breakpoint: []string{"onFailure"},
},
},
wc: config.EnableStableAPIFields,
wantErr: apis.ErrGeneric("debug requires \"enable-api-fields\" feature gate to be \"alpha\" but it is \"stable\""),
}, {
name: "invalid breakpoint",
Expand All @@ -654,6 +655,7 @@ func TestTaskRunSpec_Invalidate(t *testing.T) {
},
}},
},
wc: config.EnableStableAPIFields,
wantErr: apis.ErrGeneric("stepOverrides requires \"enable-api-fields\" feature gate to be \"alpha\" but it is \"stable\""),
}, {
name: "sidecarOverride disallowed without alpha feature gate",
Expand All @@ -668,6 +670,7 @@ func TestTaskRunSpec_Invalidate(t *testing.T) {
},
}},
},
wc: config.EnableStableAPIFields,
wantErr: apis.ErrGeneric("sidecarOverrides requires \"enable-api-fields\" feature gate to be \"alpha\" but it is \"stable\""),
}, {
name: "duplicate stepOverride names",
Expand Down Expand Up @@ -764,6 +767,7 @@ func TestTaskRunSpec_Invalidate(t *testing.T) {
},
},
},
wc: config.EnableStableAPIFields,
wantErr: apis.ErrGeneric("computeResources requires \"enable-api-fields\" feature gate to be \"alpha\" but it is \"stable\""),
}, {
name: "uses resources",
Expand Down
4 changes: 2 additions & 2 deletions pkg/reconciler/pipelinerun/pipelinerun_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4597,7 +4597,7 @@ status:
provenance:
featureFlags:
RunningInEnvWithInjectedSidecars: true
EnableAPIFields: "stable"
EnableAPIFields: "beta"
AwaitSidecarReadiness: true
VerificationNoMatchPolicy: "ignore"
EnableProvenanceInStatus: true
Expand Down Expand Up @@ -4754,7 +4754,7 @@ status:
provenance:
featureFlags:
RunningInEnvWithInjectedSidecars: true
EnableAPIFields: "stable"
EnableAPIFields: "beta"
AwaitSidecarReadiness: true
VerificationNoMatchPolicy: "ignore"
EnableProvenanceInStatus: true
Expand Down
2 changes: 1 addition & 1 deletion pkg/reconciler/pipelinerun/resources/pipelineref_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,7 @@ func TestGetPipelineFuncSpecAlreadyFetched(t *testing.T) {
}

func TestGetPipelineFunc_RemoteResolution(t *testing.T) {
ctx := context.Background()
ctx := config.EnableStableAPIFields(context.Background())
cfg := config.FromContextOrDefaults(ctx)
ctx = config.ToContext(ctx, cfg)
pipelineRef := &v1beta1.PipelineRef{ResolverRef: v1beta1.ResolverRef{Resolver: "git"}}
Expand Down
2 changes: 1 addition & 1 deletion pkg/reconciler/taskrun/resources/taskref_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -548,7 +548,7 @@ echo hello
}

func TestGetTaskFunc_RemoteResolution(t *testing.T) {
ctx := context.Background()
ctx := config.EnableStableAPIFields(context.Background())
cfg := config.FromContextOrDefaults(ctx)
ctx = config.ToContext(ctx, cfg)
taskRef := &v1beta1.TaskRef{ResolverRef: v1beta1.ResolverRef{Resolver: "git"}}
Expand Down
2 changes: 1 addition & 1 deletion pkg/reconciler/taskrun/taskrun_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1721,7 +1721,7 @@ status:
provenance:
featureFlags:
RunningInEnvWithInjectedSidecars: true
EnableAPIFields: "stable"
EnableAPIFields: "beta"
AwaitSidecarReadiness: true
VerificationNoMatchPolicy: "ignore"
EnableProvenanceInStatus: true
Expand Down
18 changes: 14 additions & 4 deletions test/custom_task_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -721,16 +721,26 @@ func resetConfigMap(ctx context.Context, t *testing.T, c *clients, namespace, co

func getFeatureFlagsBaseOnAPIFlag(t *testing.T) *config.FeatureFlags {
t.Helper()
alphaFeatureFlags, _ := config.NewFeatureFlagsFromMap(map[string]string{
alphaFeatureFlags, err := config.NewFeatureFlagsFromMap(map[string]string{
"enable-api-fields": "alpha",
"results-from": "sidecar-logs",
"enable-tekton-oci-bundles": "true",
})
betaFeatureFlags, _ := config.NewFeatureFlagsFromMap(map[string]string{
if err != nil {
t.Fatalf("error creating alpha feature flags configmap: %v", err)
}
betaFeatureFlags, err := config.NewFeatureFlagsFromMap(map[string]string{
"enable-api-fields": "beta",
})
stableFeatureFlags := config.DefaultFeatureFlags.DeepCopy()

if err != nil {
t.Fatalf("error creating beta feature flags configmap: %v", err)
}
stableFeatureFlags, err := config.NewFeatureFlagsFromMap(map[string]string{
"enable-api-fields": "stable",
})
if err != nil {
t.Fatalf("error creating stable feature flags configmap: %v", err)
}
enabledFeatureGate, err := getAPIFeatureGate()
if err != nil {
t.Fatalf("error reading enabled feature gate: %v", err)
Expand Down

0 comments on commit d7fc3eb

Please sign in to comment.