Skip to content

Commit

Permalink
TEP-0138 New features to use Per-feature flag struct
Browse files Browse the repository at this point in the history 
This commit uses the Per-feature flag struct for features added in
between the window that the changes per TEP0138 was made. All new
features should conform to the new Per-feature flag struct.

/kind misc
fixes: #7285
  • Loading branch information
@JeromeJu @tekton-robot
JeromeJu authored and tekton-robot committed Feb 12, 2024
1 parent 7f86dfb commit 2c11092
Show file tree
Hide file tree
Showing 2 changed files with 73 additions and 18 deletions.
61 changes: 47 additions & 14 deletions pkg/apis/config/feature_flags.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,20 +96,13 @@ const (
DefaultCoschedule = CoscheduleWorkspaces
// KeepPodOnCancel is the flag used to enable cancelling a pod using the entrypoint, and keep pod on cancel
KeepPodOnCancel = "keep-pod-on-cancel"
// DefaultEnableKeepPodOnCancel is the default value for "keep-pod-on-cancel"
DefaultEnableKeepPodOnCancel = false
// EnableCELInWhenExpression is the flag to enabled CEL in WhenExpression
EnableCELInWhenExpression = "enable-cel-in-whenexpression"
// DefaultEnableCELInWhenExpression is the default value for EnableCELInWhenExpression
DefaultEnableCELInWhenExpression = false
// EnableStepActions is the flag to enable the use of StepActions in Steps
EnableStepActions = "enable-step-actions"
// DefaultEnableStepActions is the default value for EnableStepActions
DefaultEnableStepActions = false
// EnableParamEnum is the flag to enabled enum in params
EnableParamEnum = "enable-param-enum"
// DefaultEnableParamEnum is the default value for EnableParamEnum
DefaultEnableParamEnum = false

disableAffinityAssistantKey = "disable-affinity-assistant"
disableCredsInitKey = "disable-creds-init"
Expand All @@ -129,7 +122,36 @@ const (
)

// DefaultFeatureFlags holds all the default configurations for the feature flags configmap.
var DefaultFeatureFlags, _ = NewFeatureFlagsFromMap(map[string]string{})
var (
DefaultFeatureFlags, _ = NewFeatureFlagsFromMap(map[string]string{})

// DefaultEnableKeepPodOnCancel is the default PerFeatureFlag value for "keep-pod-on-cancel"
DefaultEnableKeepPodOnCancel = PerFeatureFlag{
Name: KeepPodOnCancel,
Stability: AlphaAPIFields,
Enabled: DefaultAlphaFeatureEnabled,
}

// DefaultEnableCELInWhenExpression is the default PerFeatureFlag value for EnableCELInWhenExpression
DefaultEnableCELInWhenExpression = PerFeatureFlag{
Name: EnableCELInWhenExpression,
Stability: AlphaAPIFields,
Enabled: DefaultAlphaFeatureEnabled,
}

// DefaultEnableStepActions is the default PerFeatureFlag value for EnableStepActions
DefaultEnableStepActions = PerFeatureFlag{
Name: EnableStepActions,
Stability: AlphaAPIFields,
Enabled: DefaultAlphaFeatureEnabled}

// DefaultEnableParamEnum is the default PerFeatureFlag value for EnableParamEnum
DefaultEnableParamEnum = PerFeatureFlag{
Name: EnableParamEnum,
Stability: AlphaAPIFields,
Enabled: DefaultAlphaFeatureEnabled,
}
)

// FeatureFlags holds the features configurations
// +k8s:deepcopy-gen=true
Expand Down Expand Up @@ -174,6 +196,19 @@ func GetFeatureFlagsConfigName() string {

// NewFeatureFlagsFromMap returns a Config given a map corresponding to a ConfigMap
func NewFeatureFlagsFromMap(cfgMap map[string]string) (*FeatureFlags, error) {
setPerFeatureFlag := func(key string, defaultValue PerFeatureFlag, feature *bool) error {
if cfg, ok := cfgMap[key]; ok {
value, err := strconv.ParseBool(cfg)
if err != nil {
return fmt.Errorf("failed parsing feature flags config %q: %w for feature %s", cfg, err, key)
}
*feature = value
return nil
}
*feature = defaultValue.Enabled
return nil
}

setFeature := func(key string, defaultValue bool, feature *bool) error {
if cfg, ok := cfgMap[key]; ok {
value, err := strconv.ParseBool(cfg)
Expand Down Expand Up @@ -221,7 +256,7 @@ func NewFeatureFlagsFromMap(cfgMap map[string]string) (*FeatureFlags, error) {
if err := setMaxResultSize(cfgMap, DefaultMaxResultSize, &tc.MaxResultSize); err != nil {
return nil, err
}
if err := setFeature(KeepPodOnCancel, DefaultEnableKeepPodOnCancel, &tc.EnableKeepPodOnCancel); err != nil {
if err := setPerFeatureFlag(KeepPodOnCancel, DefaultEnableKeepPodOnCancel, &tc.EnableKeepPodOnCancel); err != nil {
return nil, err
}
if err := setEnforceNonFalsifiability(cfgMap, &tc.EnforceNonfalsifiability); err != nil {
Expand All @@ -233,13 +268,13 @@ func NewFeatureFlagsFromMap(cfgMap map[string]string) (*FeatureFlags, error) {
if err := setCoschedule(cfgMap, DefaultCoschedule, tc.DisableAffinityAssistant, &tc.Coschedule); err != nil {
return nil, err
}
if err := setFeature(EnableCELInWhenExpression, DefaultEnableCELInWhenExpression, &tc.EnableCELInWhenExpression); err != nil {
if err := setPerFeatureFlag(EnableCELInWhenExpression, DefaultEnableCELInWhenExpression, &tc.EnableCELInWhenExpression); err != nil {
return nil, err
}
if err := setFeature(EnableStepActions, DefaultEnableStepActions, &tc.EnableStepActions); err != nil {
if err := setPerFeatureFlag(EnableStepActions, DefaultEnableStepActions, &tc.EnableStepActions); err != nil {
return nil, err
}
if err := setFeature(EnableParamEnum, DefaultEnableParamEnum, &tc.EnableParamEnum); err != nil {
if err := setPerFeatureFlag(EnableParamEnum, DefaultEnableParamEnum, &tc.EnableParamEnum); err != nil {
return nil, err
}
// Given that they are alpha features, Tekton Bundles and Custom Tasks should be switched on if
Expand Down Expand Up @@ -381,8 +416,6 @@ func IsSpireEnabled(ctx context.Context) bool {
return FromContextOrDefaults(ctx).FeatureFlags.EnforceNonfalsifiability == EnforceNonfalsifiabilityWithSpire
}

// TODO(#7285): Patch the default values of new features that were added after
// `enable-api-fields` was no longer used.
type PerFeatureFlag struct {
// Name of the feature flag
Name string
Expand Down
30 changes: 26 additions & 4 deletions pkg/apis/config/feature_flags_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,10 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) {
SetSecurityContext: config.DefaultSetSecurityContext,
Coschedule: config.DefaultCoschedule,
EnforceNonfalsifiability: config.DefaultEnforceNonfalsifiability,
EnableKeepPodOnCancel: config.DefaultEnableKeepPodOnCancel.Enabled,
EnableCELInWhenExpression: config.DefaultEnableCELInWhenExpression.Enabled,
EnableStepActions: config.DefaultEnableStepActions.Enabled,
EnableParamEnum: config.DefaultEnableParamEnum.Enabled,
},
fileName: config.GetFeatureFlagsConfigName(),
},
Expand Down Expand Up @@ -98,6 +102,10 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) {
MaxResultSize: config.DefaultMaxResultSize,
SetSecurityContext: config.DefaultSetSecurityContext,
Coschedule: config.DefaultCoschedule,
EnableKeepPodOnCancel: config.DefaultEnableKeepPodOnCancel.Enabled,
EnableCELInWhenExpression: config.DefaultEnableCELInWhenExpression.Enabled,
EnableStepActions: config.DefaultEnableStepActions.Enabled,
EnableParamEnum: config.DefaultEnableParamEnum.Enabled,
},
fileName: "feature-flags-enable-api-fields-overrides-bundles-and-custom-tasks",
},
Expand All @@ -118,6 +126,7 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) {
MaxResultSize: config.DefaultMaxResultSize,
SetSecurityContext: config.DefaultSetSecurityContext,
Coschedule: config.DefaultCoschedule,
EnableParamEnum: config.DefaultEnableParamEnum.Enabled,
},
fileName: "feature-flags-bundles-and-custom-tasks",
},
Expand All @@ -138,6 +147,7 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) {
MaxResultSize: config.DefaultMaxResultSize,
SetSecurityContext: config.DefaultSetSecurityContext,
Coschedule: config.DefaultCoschedule,
EnableParamEnum: config.DefaultEnableParamEnum.Enabled,
},
fileName: "feature-flags-beta-api-fields",
},
Expand All @@ -154,6 +164,10 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) {
MaxResultSize: config.DefaultMaxResultSize,
SetSecurityContext: config.DefaultSetSecurityContext,
Coschedule: config.DefaultCoschedule,
EnableKeepPodOnCancel: config.DefaultEnableKeepPodOnCancel.Enabled,
EnableCELInWhenExpression: config.DefaultEnableCELInWhenExpression.Enabled,
EnableStepActions: config.DefaultEnableStepActions.Enabled,
EnableParamEnum: config.DefaultEnableParamEnum.Enabled,
},
fileName: "feature-flags-enforce-nonfalsifiability-spire",
},
Expand All @@ -169,6 +183,10 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) {
MaxResultSize: 8192,
SetSecurityContext: config.DefaultSetSecurityContext,
Coschedule: config.DefaultCoschedule,
EnableKeepPodOnCancel: config.DefaultEnableKeepPodOnCancel.Enabled,
EnableCELInWhenExpression: config.DefaultEnableCELInWhenExpression.Enabled,
EnableStepActions: config.DefaultEnableStepActions.Enabled,
EnableParamEnum: config.DefaultEnableParamEnum.Enabled,
},
fileName: "feature-flags-results-via-sidecar-logs",
},
Expand Down Expand Up @@ -201,6 +219,10 @@ func TestNewFeatureFlagsFromEmptyConfigMap(t *testing.T) {
MaxResultSize: config.DefaultMaxResultSize,
SetSecurityContext: config.DefaultSetSecurityContext,
Coschedule: config.DefaultCoschedule,
EnableKeepPodOnCancel: config.DefaultEnableKeepPodOnCancel.Enabled,
EnableCELInWhenExpression: config.DefaultEnableCELInWhenExpression.Enabled,
EnableStepActions: config.DefaultEnableStepActions.Enabled,
EnableParamEnum: config.DefaultEnableParamEnum.Enabled,
}
verifyConfigFileWithExpectedFeatureFlagsConfig(t, FeatureFlagsConfigEmptyName, expectedConfig)
}
Expand Down Expand Up @@ -265,7 +287,7 @@ func TestNewFeatureFlagsConfigMapErrors(t *testing.T) {
want: `invalid value for feature flag "coschedule": "invalid"`,
}, {
fileName: "feature-flags-invalid-keep-pod-on-cancel",
want: `failed parsing feature flags config "invalid": strconv.ParseBool: parsing "invalid": invalid syntax`,
want: `failed parsing feature flags config "invalid": strconv.ParseBool: parsing "invalid": invalid syntax for feature keep-pod-on-cancel`,
}, {
fileName: "feature-flags-invalid-running-in-environment-with-injected-sidecars",
want: `failed parsing feature flags config "invalid-boolean": strconv.ParseBool: parsing "invalid-boolean": invalid syntax`,
Expand All @@ -274,13 +296,13 @@ func TestNewFeatureFlagsConfigMapErrors(t *testing.T) {
want: `failed parsing feature flags config "truee": strconv.ParseBool: parsing "truee": invalid syntax`,
}, {
fileName: "feature-flags-invalid-enable-cel-in-whenexpression",
want: `failed parsing feature flags config "invalid": strconv.ParseBool: parsing "invalid": invalid syntax`,
want: `failed parsing feature flags config "invalid": strconv.ParseBool: parsing "invalid": invalid syntax for feature enable-cel-in-whenexpression`,
}, {
fileName: "feature-flags-invalid-enable-step-actions",
want: `failed parsing feature flags config "invalid": strconv.ParseBool: parsing "invalid": invalid syntax`,
want: `failed parsing feature flags config "invalid": strconv.ParseBool: parsing "invalid": invalid syntax for feature enable-step-actions`,
}, {
fileName: "feature-flags-invalid-enable-param-enum",
want: `failed parsing feature flags config "invalid": strconv.ParseBool: parsing "invalid": invalid syntax`,
want: `failed parsing feature flags config "invalid": strconv.ParseBool: parsing "invalid": invalid syntax for feature enable-param-enum`,
}} {
t.Run(tc.fileName, func(t *testing.T) {
cm := test.ConfigMapFromTestFile(t, tc.fileName)
Expand Down

0 comments on commit 2c11092

Please sign in to comment.