diff --git a/pkg/config/apiserver.go b/pkg/config/apiserver.go index cea5eb3be6..4ed96c2166 100644 --- a/pkg/config/apiserver.go +++ b/pkg/config/apiserver.go @@ -6,7 +6,6 @@ import ( "slices" configv1 "github.com/openshift/api/config/v1" - featuresUtils "github.com/openshift/api/features" "github.com/openshift/library-go/pkg/crypto" "k8s.io/apimachinery/pkg/util/sets" @@ -154,28 +153,15 @@ type FeatureGates struct { CustomNoUpgrade CustomNoUpgrade `json:"customNoUpgrade"` } +// ToApiserverArgs converts the FeatureGates struct to a list of feature-gates arguments for the kube-apiserver. +// Validation checks should be performed before calling this function to ensure the FeatureGates struct is valid. func (fg FeatureGates) ToApiserverArgs() ([]string, error) { ret := sets.NewString() - - switch fg.FeatureSet { - case FeatureSetCustomNoUpgrade: - for _, feature := range fg.CustomNoUpgrade.Enabled { - ret.Insert(fmt.Sprintf("%s=true", feature)) - } - for _, feature := range fg.CustomNoUpgrade.Disabled { - ret.Insert(fmt.Sprintf("%s=false", feature)) - } - case FeatureSetDevPreviewNoUpgrade, FeatureSetTechPreviewNoUpgrade: - fgEnabledDisabled, err := featuresUtils.FeatureSets(featuresUtils.SelfManaged, configv1.FeatureSet(fg.FeatureSet)) - if err != nil { - return nil, fmt.Errorf("failed to get feature set gates: %w", err) - } - for _, f := range fgEnabledDisabled.Enabled { - ret.Insert(fmt.Sprintf("%s=true", f.FeatureGateAttributes.Name)) - } - for _, f := range fgEnabledDisabled.Disabled { - ret.Insert(fmt.Sprintf("%s=false", f.FeatureGateAttributes.Name)) - } + for _, feature := range fg.CustomNoUpgrade.Enabled { + ret.Insert(fmt.Sprintf("%s=true", feature)) + } + for _, feature := range fg.CustomNoUpgrade.Disabled { + ret.Insert(fmt.Sprintf("%s=false", feature)) } return ret.List(), nil } @@ -190,18 +176,21 @@ func (fg *FeatureGates) validateFeatureGates() error { if fg == nil || reflect.DeepEqual(*fg, FeatureGates{}) { return nil } - // Must use a recognized feature set, or else empty - if fg.FeatureSet != "" && fg.FeatureSet != FeatureSetCustomNoUpgrade && fg.FeatureSet != FeatureSetTechPreviewNoUpgrade && fg.FeatureSet != FeatureSetDevPreviewNoUpgrade { + + // FeatureSet must be empty or CustomNoUpgrade. If empty, CustomNoUpgrade.Enabled/Disabled lists must be empty. + switch fg.FeatureSet { + case "": + if len(fg.CustomNoUpgrade.Enabled) > 0 || len(fg.CustomNoUpgrade.Disabled) > 0 { + return fmt.Errorf("CustomNoUpgrade enabled/disabled lists must be empty when FeatureSet is empty") + } + return nil + case FeatureSetCustomNoUpgrade: + // Valid - continue to validate enabled/disabled lists below + case FeatureSetDevPreviewNoUpgrade, FeatureSetTechPreviewNoUpgrade: + return fmt.Errorf("FeatureSet %s is not supported. Use CustomNoUpgrade to enable/disable feature gates", fg.FeatureSet) + default: return fmt.Errorf("invalid feature set: %s", fg.FeatureSet) } - // Must set FeatureSet to CustomNoUpgrade to use custom feature gates - if fg.FeatureSet != FeatureSetCustomNoUpgrade && (len(fg.CustomNoUpgrade.Enabled) > 0 || len(fg.CustomNoUpgrade.Disabled) > 0) { - return fmt.Errorf("CustomNoUpgrade must be empty when FeatureSet is empty") - } - // Must set CustomNoUpgrade enabled or disabled lists when FeatureSet is CustomNoUpgrade - if fg.FeatureSet == FeatureSetCustomNoUpgrade && len(fg.CustomNoUpgrade.Enabled) == 0 && len(fg.CustomNoUpgrade.Disabled) == 0 { - return fmt.Errorf("CustomNoUpgrade enabled or disabled lists must be set when FeatureSet is CustomNoUpgrade") - } var errs = make(sets.Set[error], 0) for _, requiredFG := range RequiredFeatureGates { diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index c41fb9a0c0..497fe49953 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -819,7 +819,7 @@ func TestValidate(t *testing.T) { expectErr: true, }, { - name: "feature-gates-custom-no-upgrade-with-feature-set", + name: "feature-gates-custom-no-upgrade-valid", config: func() *Config { c := mkDefaultConfig() c.ApiServer.FeatureGates.FeatureSet = "CustomNoUpgrade" @@ -827,6 +827,7 @@ func TestValidate(t *testing.T) { c.ApiServer.FeatureGates.CustomNoUpgrade.Disabled = []string{"feature2"} return c }(), + expectErr: false, }, { name: "feature-gates-custom-no-upgrade-with-feature-set-empty", @@ -840,23 +841,54 @@ func TestValidate(t *testing.T) { expectErr: true, }, { - name: "feature-gates-custom-no-upgrade-with-empty-enabled-and-disabled-lists", + name: "feature-gates-custom-no-upgrade-enabled-and-disabled-have-same-feature-gate", config: func() *Config { c := mkDefaultConfig() c.ApiServer.FeatureGates.FeatureSet = "CustomNoUpgrade" - c.ApiServer.FeatureGates.CustomNoUpgrade.Enabled = []string{} - c.ApiServer.FeatureGates.CustomNoUpgrade.Disabled = []string{} + c.ApiServer.FeatureGates.CustomNoUpgrade.Enabled = []string{"feature1"} + c.ApiServer.FeatureGates.CustomNoUpgrade.Disabled = []string{"feature1"} return c }(), expectErr: true, }, { - name: "feature-gates-custom-no-upgrade-enabled-and-disabled-have-same-feature-gate", + name: "feature-gates-required-feature-gate-cannot-be-disabled", config: func() *Config { c := mkDefaultConfig() c.ApiServer.FeatureGates.FeatureSet = "CustomNoUpgrade" c.ApiServer.FeatureGates.CustomNoUpgrade.Enabled = []string{"feature1"} - c.ApiServer.FeatureGates.CustomNoUpgrade.Disabled = []string{"feature1"} + c.ApiServer.FeatureGates.CustomNoUpgrade.Disabled = []string{"UserNamespacesSupport"} + return c + }(), + expectErr: true, + }, + { + name: "feature-gates-required-feature-gate-cannot-be-explicitly-enabled", + config: func() *Config { + c := mkDefaultConfig() + c.ApiServer.FeatureGates.FeatureSet = "CustomNoUpgrade" + c.ApiServer.FeatureGates.CustomNoUpgrade.Enabled = []string{"UserNamespacesSupport"} + c.ApiServer.FeatureGates.CustomNoUpgrade.Disabled = []string{"feature2"} + return c + }(), + expectErr: true, + }, + { + name: "feature-gates-custom-no-upgrade-with-empty-enabled-and-disabled-lists", + config: func() *Config { + c := mkDefaultConfig() + c.ApiServer.FeatureGates.FeatureSet = "CustomNoUpgrade" + c.ApiServer.FeatureGates.CustomNoUpgrade.Enabled = []string{} + c.ApiServer.FeatureGates.CustomNoUpgrade.Disabled = []string{} + return c + }(), + expectErr: false, + }, + { + name: "feature-gates-preview-feature-sets-not-supported", + config: func() *Config { + c := mkDefaultConfig() + c.ApiServer.FeatureGates.FeatureSet = "TechPreviewNoUpgrade" return c }(), expectErr: true, diff --git a/scripts/aws/cf-gen.yaml b/scripts/aws/cf-gen.yaml index a7a58814ce..babea03205 100644 --- a/scripts/aws/cf-gen.yaml +++ b/scripts/aws/cf-gen.yaml @@ -32,8 +32,8 @@ Parameters: Description: Current RHEL AMI to use. Type: AWS::EC2::Image::Id Machinename: - AllowedPattern: ^([a-zA-Z][a-zA-Z0-9\-]{0,26})$ - MaxLength: 27 + AllowedPattern: ^([a-zA-Z][a-zA-Z0-9\-]*)$ + MaxLength: 64 MinLength: 1 ConstraintDescription: Machinename Description: Machinename