From c9898f55fb90b081f18899516404db3a26e1fedf Mon Sep 17 00:00:00 2001 From: Jon Cope Date: Wed, 28 Jan 2026 16:09:55 -0600 Subject: [PATCH 1/3] aligned microshift featuregate validations with openshift --- Makefile | 1 + pkg/admin/prerun/featuregate_lock.go | 78 ++++++-------------- pkg/admin/prerun/featuregate_lock_test.go | 86 +++++++++-------------- pkg/config/apiserver.go | 70 +++++++++--------- pkg/config/config_test.go | 22 ------ 5 files changed, 92 insertions(+), 165 deletions(-) diff --git a/Makefile b/Makefile index 286560609e..3cae8d5f6f 100644 --- a/Makefile +++ b/Makefile @@ -97,6 +97,7 @@ GO_BUILD_FLAGS :=-tags 'include_gcs include_oss containers_image_openpgp gssapi # Set variables for test-unit target GO_TEST_FLAGS=$(GO_BUILD_FLAGS) +GO_TEST_ARGS=$(GO_LD_FLAGS) GO_TEST_PACKAGES=./cmd/... ./pkg/... # Enable CGO when building microshift binary for access to local libraries. diff --git a/pkg/admin/prerun/featuregate_lock.go b/pkg/admin/prerun/featuregate_lock.go index 641c7671f2..e17d621ea5 100644 --- a/pkg/admin/prerun/featuregate_lock.go +++ b/pkg/admin/prerun/featuregate_lock.go @@ -5,7 +5,6 @@ import ( "fmt" "os" "path/filepath" - "reflect" "github.com/openshift/microshift/pkg/config" "github.com/openshift/microshift/pkg/util" @@ -45,58 +44,27 @@ func featureGateLockManagement(cfg *config.Config) error { if err != nil { return fmt.Errorf("failed to check if lock file exists: %w", err) } - // Lock file exists - validate configuration if lockExists { - return validateFeatureGateLockFile(cfg) + return runValidationsChecks(cfg) } - // No lock file exists yet and custom feature gates are configured, so this is the first time configuring custom feature gates - if hasCustomFeatureGates(cfg.ApiServer.FeatureGates) { - klog.InfoS("Custom feature gates detected", "featureSet", cfg.ApiServer.FeatureGates.FeatureSet) + if cfg.ApiServer.FeatureGates.FeatureSet != "" { return createFeatureGateLockFile(cfg) } - // No lock file and no custom feature gates - normal operation - klog.InfoS("No custom feature gates configured - skipping lock file management") return nil } -// hasCustomFeatureGates checks if any custom feature gates are configured -func hasCustomFeatureGates(fg config.FeatureGates) bool { - // Empty feature set means no custom feature gates - if fg.FeatureSet == "" { - return false - } - - // TechPreviewNoUpgrade and DevPreviewNoUpgrade are considered custom - if fg.FeatureSet == config.FeatureSetTechPreviewNoUpgrade || - fg.FeatureSet == config.FeatureSetDevPreviewNoUpgrade { - return true - } - - // CustomNoUpgrade requires actual enabled or disabled features - if fg.FeatureSet == config.FeatureSetCustomNoUpgrade { - return len(fg.CustomNoUpgrade.Enabled) > 0 || len(fg.CustomNoUpgrade.Disabled) > 0 - } - - return false -} - // createFeatureGateLockFile creates the lock file with current configuration func createFeatureGateLockFile(cfg *config.Config) error { klog.InfoS("Creating feature gate lock file - this cluster can no longer be upgraded", "path", featureGateLockFilePath) // Get current version from version file - currentVersion, err := getVersionOfData() + currentVersion, err := GetVersionOfExecutable() if err != nil { - // If version file doesn't exist yet, get executable version - klog.InfoS("Version file does not exist yet, using executable version") - currentVersion, err = GetVersionOfExecutable() - if err != nil { - return fmt.Errorf("failed to get version: %w", err) - } + return fmt.Errorf("failed to get version: %w", err) } lockFile := featureGateLockFile{ @@ -116,9 +84,9 @@ func createFeatureGateLockFile(cfg *config.Config) error { return nil } -// validateFeatureGateLockFile validates that the current configuration matches the lock file -// and that no version upgrade has occurred -func validateFeatureGateLockFile(cfg *config.Config) error { +// runValidationsChecks validates the feature gate lock file and the current configuration +// It returns an error if the configuration is invalid or if an x or y stream version upgrade has occurred. +func runValidationsChecks(cfg *config.Config) error { klog.InfoS("Validating feature gate lock file", "path", featureGateLockFilePath) lockFile, err := readFeatureGateLockFile(featureGateLockFilePath) @@ -127,9 +95,8 @@ func validateFeatureGateLockFile(cfg *config.Config) error { } // Check if feature gate configuration has changed - if err := compareFeatureGates(lockFile, cfg.ApiServer.FeatureGates); err != nil { - return fmt.Errorf("feature gate configuration has changed: %w\n\n"+ - "Custom feature gates cannot be modified or reverted once applied.\n"+ + if err := configValidationChecksPass(lockFile, cfg.ApiServer.FeatureGates); err != nil { + return fmt.Errorf("detected invalid changes in feature gate configuration: %w\n\n"+ "To restore MicroShift to a supported state, you must:\n"+ "1. Run: sudo microshift-cleanup-data --all\n"+ "2. Remove custom feature gates from /etc/microshift/config.yaml\n"+ @@ -137,12 +104,12 @@ func validateFeatureGateLockFile(cfg *config.Config) error { } // Check if version has changed (upgrade attempted) - currentVersion, err := getVersionOfData() + currentExecutableVersion, err := getVersionOfData() if err != nil { return fmt.Errorf("failed to get current version: %w", err) } - if lockFile.Version.Major != currentVersion.Major || lockFile.Version.Minor != currentVersion.Minor { + if lockFile.Version.Major != currentExecutableVersion.Major || lockFile.Version.Minor != currentExecutableVersion.Minor { return fmt.Errorf("version upgrade detected with custom feature gates: locked version %s, current version %s\n\n"+ "Upgrades are not supported when custom feature gates are configured.\n"+ "Custom feature gates (%s) were configured in version %s.\n"+ @@ -151,7 +118,7 @@ func validateFeatureGateLockFile(cfg *config.Config) error { "2. Run: sudo microshift-cleanup-data --all\n"+ "3. Remove custom feature gates from /etc/microshift/config.yaml\n"+ "4. Restart MicroShift: sudo systemctl restart microshift", - lockFile.Version.String(), currentVersion.String(), + lockFile.Version.String(), currentExecutableVersion.String(), lockFile.FeatureSet, lockFile.Version.String(), lockFile.Version.String()) } @@ -159,21 +126,16 @@ func validateFeatureGateLockFile(cfg *config.Config) error { return nil } -// compareFeatureGates compares the lock file with current configuration -func compareFeatureGates(lockFile featureGateLockFile, current config.FeatureGates) error { - var errs []error - - if lockFile.FeatureSet != current.FeatureSet { - errs = append(errs, fmt.Errorf("feature set changed: locked config has %q, current config has %q", - lockFile.FeatureSet, current.FeatureSet)) +func configValidationChecksPass(prev featureGateLockFile, current config.FeatureGates) error { + if prev.FeatureSet != "" && current.FeatureSet == "" { + // Disallow changing from feature set to no feature set + return fmt.Errorf("cannot unset feature set. Previous config had feature set %q, current config has no feature set configured", prev.FeatureSet) } - - if !reflect.DeepEqual(lockFile.CustomNoUpgrade, current.CustomNoUpgrade) { - errs = append(errs, fmt.Errorf("custom feature gates changed: locked config has %#v, current config has %#v", - lockFile.CustomNoUpgrade, current.CustomNoUpgrade)) + if prev.FeatureSet == config.FeatureSetCustomNoUpgrade && current.FeatureSet != config.FeatureSetCustomNoUpgrade { + // Disallow changing from custom feature gates to any other feature set + return fmt.Errorf("cannot change CustomNoUpgrade feature set. Previous feature set was %q, current feature set is %q", prev.FeatureSet, current.FeatureSet) } - - return errors.Join(errs...) + return nil } // writeFeatureGateLockFile writes the lock file to disk in YAML format diff --git a/pkg/admin/prerun/featuregate_lock_test.go b/pkg/admin/prerun/featuregate_lock_test.go index 163a1ae177..5340bde1ed 100644 --- a/pkg/admin/prerun/featuregate_lock_test.go +++ b/pkg/admin/prerun/featuregate_lock_test.go @@ -125,9 +125,15 @@ func TestIsCustomFeatureGatesConfigured(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got := hasCustomFeatureGates(tt.fg) - if got != tt.want { - t.Errorf("isCustomFeatureGatesConfigured() = %v, want %v", got, tt.want) + err := configValidationChecksPass(featureGateLockFile{ + FeatureSet: tt.fg.FeatureSet, + CustomNoUpgrade: tt.fg.CustomNoUpgrade, + }, tt.fg) + if err != nil { + t.Errorf("featureValidationsPass() error = %v", err) + } + if err != nil { + t.Errorf("featureValidationsPass() error = %v", err) } }) } @@ -206,75 +212,40 @@ func TestFeatureGateLockFile_ReadNonExistent(t *testing.T) { } } -func TestCompareFeatureGates(t *testing.T) { +func TestConfigValidationChecksPass(t *testing.T) { tests := []struct { - name string - lockFile featureGateLockFile - current config.FeatureGates - wantMatch bool + name string + lockFile featureGateLockFile + current config.FeatureGates + wantErr bool }{ { - name: "identical custom feature gates", + name: "unset any feature set", lockFile: featureGateLockFile{ FeatureSet: config.FeatureSetCustomNoUpgrade, - CustomNoUpgrade: config.CustomNoUpgrade{ - Enabled: []string{"FeatureA", "FeatureB"}, - Disabled: []string{"FeatureC"}, - }, }, current: config.FeatureGates{ - FeatureSet: config.FeatureSetCustomNoUpgrade, - CustomNoUpgrade: config.CustomNoUpgrade{ - Enabled: []string{"FeatureA", "FeatureB"}, - Disabled: []string{"FeatureC"}, - }, + FeatureSet: "", }, - wantMatch: true, + wantErr: true, }, { - name: "different enabled features", + name: "change CustomNoUpgrade to any other feature set", lockFile: featureGateLockFile{ FeatureSet: config.FeatureSetCustomNoUpgrade, - CustomNoUpgrade: config.CustomNoUpgrade{ - Enabled: []string{"FeatureA"}, - }, - }, - current: config.FeatureGates{ - FeatureSet: config.FeatureSetCustomNoUpgrade, - CustomNoUpgrade: config.CustomNoUpgrade{ - Enabled: []string{"FeatureB"}, - }, - }, - wantMatch: false, - }, - { - name: "different feature sets", - lockFile: featureGateLockFile{ - FeatureSet: config.FeatureSetTechPreviewNoUpgrade, - }, - current: config.FeatureGates{ - FeatureSet: config.FeatureSetDevPreviewNoUpgrade, - }, - wantMatch: false, - }, - { - name: "identical TechPreviewNoUpgrade", - lockFile: featureGateLockFile{ - FeatureSet: config.FeatureSetTechPreviewNoUpgrade, }, current: config.FeatureGates{ FeatureSet: config.FeatureSetTechPreviewNoUpgrade, }, - wantMatch: true, + wantErr: true, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - err := compareFeatureGates(tt.lockFile, tt.current) - gotMatch := err == nil - if gotMatch != tt.wantMatch { - t.Errorf("compareFeatureGates() match = %v, want %v, error = %v", gotMatch, tt.wantMatch, err) + err := configValidationChecksPass(tt.lockFile, tt.current) + if (err != nil) != tt.wantErr { + t.Errorf("configValidationChecksPass() error = %v, wantErr %v", err, tt.wantErr) } }) } @@ -299,8 +270,13 @@ func TestFeatureGateLockManagement_FirstRun(t *testing.T) { defer func() { versionFilePath = originalVersionPath }() // Create a version file to simulate existing data (version file uses JSON format) + version, err := GetVersionOfExecutable() + if err != nil { + t.Fatal(err) + } + t.Logf("version: %s", version.String()) versionData := versionFile{ - Version: versionMetadata{Major: 4, Minor: 18, Patch: 0}, + Version: version, BootID: "test-boot", } versionJSON, _ := json.Marshal(versionData) @@ -354,8 +330,12 @@ func TestFeatureGateLockManagement_ConfigChange(t *testing.T) { defer func() { versionFilePath = originalVersionPath }() // Create a version file to simulate existing data (version file uses JSON format) + version, err := GetVersionOfExecutable() + if err != nil { + t.Fatal(err) + } versionData := versionFile{ - Version: versionMetadata{Major: 4, Minor: 18, Patch: 0}, + Version: version, BootID: "test-boot", } versionJSON, _ := json.Marshal(versionData) diff --git a/pkg/config/apiserver.go b/pkg/config/apiserver.go index 4ed96c2166..efbd1b4f9a 100644 --- a/pkg/config/apiserver.go +++ b/pkg/config/apiserver.go @@ -157,12 +157,14 @@ type FeatureGates struct { // 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() - 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)) +addFeatures := func(features []string, enabled bool) { + for _, feature := range features { + ret.Insert(fmt.Sprintf("%s=%t", feature, enabled)) + } } + + addFeatures(fg.CustomNoUpgrade.Enabled, true) + addFeatures(fg.CustomNoUpgrade.Disabled, false) return ret.List(), nil } @@ -171,49 +173,53 @@ func (fg FeatureGates) GoString() string { return fmt.Sprintf("FeatureGates{FeatureSet: %q, CustomNoUpgrade: %#v}", fg.FeatureSet, fg.CustomNoUpgrade) } +// checkFeatureGateConflict checks if two sets of feature gates have any intersection and returns an error if they do. +func checkFeatureGateConflict(a, b sets.Set[string], errorMsg string) error { + if intersect := a.Intersection(b); intersect.Len() > 0 { + return fmt.Errorf("%s: %s", errorMsg, intersect.UnsortedList()) + } + return nil +} + +// validateFeatureGates validates the FeatureGates struct according to the following rules: +// 1. FeatureGates may be unset. +// 2. FeatureSet must be empty or CustomNoUpgrade. +// 3. If FeatureSet is DevPreviewNoUpgrade or TechPreviewNoUpgrade, return an error. +// 4. If FeatureSet is CustomNoUpgrade, CustomNoUpgrade.Enabled/Disabled lists may be set but are not required. +// 5. Required feature gates cannot be disabled. +// 6. Feature gates cannot be both enabled and disabled within the same object. func (fg *FeatureGates) validateFeatureGates() error { - // FG is unset if fg == nil || reflect.DeepEqual(*fg, FeatureGates{}) { return nil } - // FeatureSet must be empty or CustomNoUpgrade. If empty, CustomNoUpgrade.Enabled/Disabled lists must be empty. - switch fg.FeatureSet { +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 + // Valid - continue with validation 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) } - var errs = make(sets.Set[error], 0) - for _, requiredFG := range RequiredFeatureGates { - // Edge case: Users must not be allowed to explicitly disable required feature gates. - if sets.NewString(fg.CustomNoUpgrade.Disabled...).Has(requiredFG) { - errs.Insert(fmt.Errorf("required feature gate %s cannot be disabled: %s", requiredFG, fg.CustomNoUpgrade.Disabled)) - } - // Edge case: Users must not be allowed to explicitly enable required feature gates or else the config would be locked and the cluster - // would not be able to be upgraded. - if sets.New(fg.CustomNoUpgrade.Enabled...).Has(requiredFG) { - errs.Insert(fmt.Errorf("feature gate %s is explicitly enabled and cannot be enabled by the user", requiredFG)) - } - } - if errs.Len() > 0 { - return fmt.Errorf("invalid feature gates: %s", errs.UnsortedList()) + enabledCustom := sets.New(fg.CustomNoUpgrade.Enabled...) + disabledCustom := sets.New(fg.CustomNoUpgrade.Disabled...) + + conflictChecks := []struct { + setA sets.Set[string] + setB sets.Set[string] + msg string + }{ + {disabledCustom, sets.New(RequiredFeatureGates...), "required feature gates cannot be disabled"}, + {enabledCustom, disabledCustom, "feature gates cannot be both enabled and disabled"}, } - // Must not have any feature gates that are enabled and disabled at the same time - enabledSet := sets.New(fg.CustomNoUpgrade.Enabled...) - disabledSet := sets.New(fg.CustomNoUpgrade.Disabled...) - inBothSets := enabledSet.Intersection(disabledSet) - if inBothSets.Len() > 0 { - return fmt.Errorf("featuregates cannot be enabled and disabled at the same time: %s", inBothSets.UnsortedList()) + for _, check := range conflictChecks { + if err := checkFeatureGateConflict(check.setA, check.setB, check.msg); err != nil { + return err + } } return nil diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index 497fe49953..9352ab5428 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -829,17 +829,6 @@ func TestValidate(t *testing.T) { }(), expectErr: false, }, - { - name: "feature-gates-custom-no-upgrade-with-feature-set-empty", - config: func() *Config { - c := mkDefaultConfig() - c.ApiServer.FeatureGates.FeatureSet = "" - c.ApiServer.FeatureGates.CustomNoUpgrade.Enabled = []string{"feature1"} - c.ApiServer.FeatureGates.CustomNoUpgrade.Disabled = []string{"feature2"} - return c - }(), - expectErr: true, - }, { name: "feature-gates-custom-no-upgrade-enabled-and-disabled-have-same-feature-gate", config: func() *Config { @@ -862,17 +851,6 @@ func TestValidate(t *testing.T) { }(), 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 { From a27e2406b50af3c5f2ee38fa52004a9e9cdb0a97 Mon Sep 17 00:00:00 2001 From: Jon Cope Date: Wed, 28 Jan 2026 14:37:24 -0600 Subject: [PATCH 2/3] fixed: prerun validation should check execuatble version removed: RF tests related to version detection. these are covered sufficiently by unit tests --- pkg/admin/prerun/featuregate_lock.go | 4 +-- test/suites/standard2/feature-gates.robot | 37 ++--------------------- 2 files changed, 5 insertions(+), 36 deletions(-) diff --git a/pkg/admin/prerun/featuregate_lock.go b/pkg/admin/prerun/featuregate_lock.go index e17d621ea5..752f1fadfe 100644 --- a/pkg/admin/prerun/featuregate_lock.go +++ b/pkg/admin/prerun/featuregate_lock.go @@ -104,9 +104,9 @@ func runValidationsChecks(cfg *config.Config) error { } // Check if version has changed (upgrade attempted) - currentExecutableVersion, err := getVersionOfData() + currentExecutableVersion, err := GetVersionOfExecutable() if err != nil { - return fmt.Errorf("failed to get current version: %w", err) + return fmt.Errorf("failed to get current executable version: %w", err) } if lockFile.Version.Major != currentExecutableVersion.Major || lockFile.Version.Minor != currentExecutableVersion.Minor { diff --git a/test/suites/standard2/feature-gates.robot b/test/suites/standard2/feature-gates.robot index c4744a20d8..1ee84c20d8 100644 --- a/test/suites/standard2/feature-gates.robot +++ b/test/suites/standard2/feature-gates.robot @@ -1,6 +1,6 @@ *** Settings *** -Documentation Tests for feature gate configuration - +Documentation Verify that that MicroShift feature gate configuration is correctly integrated into the kube-apiserver. +... Validation testing is handled by prerun unit tests and is not included here. Resource ../../resources/common.resource Resource ../../resources/microshift-config.resource Resource ../../resources/microshift-process.resource @@ -23,13 +23,6 @@ ${CUSTOM_FEATURE_GATES} SEPARATOR=\n ... \ \ \ \ \ \ \ \ - TestFeatureEnabled ... \ \ \ \ \ \ disabled: ... \ \ \ \ \ \ \ \ - TestFeatureDisabled -${DIFFERENT_FEATURE_GATES} SEPARATOR=\n -... apiServer: -... \ \ featureGates: -... \ \ \ \ featureSet: CustomNoUpgrade -... \ \ \ \ customNoUpgrade: -... \ \ \ \ \ \ enabled: -... \ \ \ \ \ \ \ \ - DifferentTestFeature ${FEATURE_GATE_LOCK_FILE} /var/lib/microshift/no-upgrade @@ -39,7 +32,7 @@ Custom Feature Gates Are Passed To Kube APIServer ... kube-apiserver. This test verifies that arbitrary feature gate values are correctly propagated from the ... MicroShift configuration to the kube-apiserver, regardless of whether the feature gates are valid or have any effect. ... Also verify that feature gate lock file is created when custom feature gates are configured. - ... The lock file prevents upgrades and configuration changes when CustomNoUpgrade feature set is used. + ... The lock file prevents upgrades and configuration changes when CustomNoUpgrade feature set is used.j [Setup] Setup Custom Feature Gates Test Wait Until Keyword Succeeds 2 min 5 sec ... Pattern Should Appear In Log Output ${CURSOR} kube:feature-gates=.*TestFeatureEnabled=true @@ -50,20 +43,6 @@ Custom Feature Gates Are Passed To Kube APIServer Feature Gate Lock File Should Contain Feature Gates CustomNoUpgrade TestFeatureEnabled [Teardown] Teardown Custom Feature Gates Test -Feature Gate Config Change Blocked After Lock Created - [Documentation] Verify that changing feature gate config is blocked after lock file exists. - ... MicroShift must refuse to start if feature gates change after CustomNoUpgrade is set. - [Setup] Setup Custom Feature Gates Test - Stop MicroShift - Drop In MicroShift Config ${DIFFERENT_FEATURE_GATES} 10-featuregates - Save Journal Cursor - MicroShift Should Fail To Start - Pattern Should Appear In Log Output ${CURSOR} feature gate configuration has changed - # Restore original config and verify that MicroShift starts - Drop In MicroShift Config ${CUSTOM_FEATURE_GATES} 10-featuregates - Start MicroShift - [Teardown] Teardown Custom Feature Gates Test - Feature Gate Lock File Persists Across Restarts With Same Config [Documentation] Verify that feature gate lock file persists and validation succeeds across restarts ... when the same feature gate configuration is maintained. @@ -123,13 +102,3 @@ Feature Gate Lock File Should Contain Feature Gates ${contents}= Command Should Work cat ${FEATURE_GATE_LOCK_FILE} Should Contain ${contents} ${feature_set} Should Contain ${contents} ${feature_name} - -MicroShift Should Fail To Start - [Documentation] Verify that MicroShift fails to start and returns a non-zero exit code. - ... This keyword is unique and differs from a composite keyword like - ... Run Keyword And Expect Error 1 != 0 Start MicroShift - ... because there is no need to poll the service for an "active" state, which Start MicroShift does. - ${stdout} ${stderr} ${rc}= Execute Command sudo systemctl start microshift.service - ... sudo=True return_stdout=True return_stderr=True return_rc=True - Log Many ${stdout} ${stderr} ${rc} - Should Be Equal As Integers 1 ${rc} From 18ec2090cd51518e74b030c6c106616e41e0dfa8 Mon Sep 17 00:00:00 2001 From: Jon Cope Date: Wed, 28 Jan 2026 16:32:30 -0600 Subject: [PATCH 3/3] add feature exemption lists --- pkg/admin/prerun/featuregate_lock.go | 6 +- pkg/admin/prerun/featuregate_lock_test.go | 67 +++++++++++++++++++---- pkg/config/apiserver.go | 18 ++++-- 3 files changed, 71 insertions(+), 20 deletions(-) diff --git a/pkg/admin/prerun/featuregate_lock.go b/pkg/admin/prerun/featuregate_lock.go index 752f1fadfe..f8cb37de41 100644 --- a/pkg/admin/prerun/featuregate_lock.go +++ b/pkg/admin/prerun/featuregate_lock.go @@ -20,9 +20,9 @@ var ( // featureGateLockFile represents the structure of the lock file // that tracks custom feature gate configuration and prevents changes/upgrades type featureGateLockFile struct { - FeatureSet string `json:"featureSet"` - CustomNoUpgrade config.CustomNoUpgrade `json:"customNoUpgrade"` - Version versionMetadata `json:"version"` + FeatureSet string `json:"featureSet"` + CustomNoUpgrade config.EnableDisableFeatures `json:"customNoUpgrade"` + Version versionMetadata `json:"version"` } // FeatureGateLockManagement manages the feature gate lock file diff --git a/pkg/admin/prerun/featuregate_lock_test.go b/pkg/admin/prerun/featuregate_lock_test.go index 5340bde1ed..10275d9a68 100644 --- a/pkg/admin/prerun/featuregate_lock_test.go +++ b/pkg/admin/prerun/featuregate_lock_test.go @@ -22,7 +22,7 @@ func TestFeatureGateLockFile_Marshal(t *testing.T) { name: "custom feature gates with enabled and disabled", lockFile: featureGateLockFile{ FeatureSet: config.FeatureSetCustomNoUpgrade, - CustomNoUpgrade: config.CustomNoUpgrade{ + CustomNoUpgrade: config.EnableDisableFeatures{ Enabled: []string{"FeatureA", "FeatureB"}, Disabled: []string{"FeatureC"}, }, @@ -33,7 +33,7 @@ func TestFeatureGateLockFile_Marshal(t *testing.T) { name: "TechPreviewNoUpgrade", lockFile: featureGateLockFile{ FeatureSet: config.FeatureSetTechPreviewNoUpgrade, - CustomNoUpgrade: config.CustomNoUpgrade{}, + CustomNoUpgrade: config.EnableDisableFeatures{}, }, wantErr: false, }, @@ -41,7 +41,7 @@ func TestFeatureGateLockFile_Marshal(t *testing.T) { name: "DevPreviewNoUpgrade", lockFile: featureGateLockFile{ FeatureSet: config.FeatureSetDevPreviewNoUpgrade, - CustomNoUpgrade: config.CustomNoUpgrade{}, + CustomNoUpgrade: config.EnableDisableFeatures{}, }, wantErr: false, }, @@ -49,7 +49,7 @@ func TestFeatureGateLockFile_Marshal(t *testing.T) { name: "empty feature gates", lockFile: featureGateLockFile{ FeatureSet: "", - CustomNoUpgrade: config.CustomNoUpgrade{}, + CustomNoUpgrade: config.EnableDisableFeatures{}, }, wantErr: false, }, @@ -86,7 +86,7 @@ func TestIsCustomFeatureGatesConfigured(t *testing.T) { name: "CustomNoUpgrade with enabled features", fg: config.FeatureGates{ FeatureSet: config.FeatureSetCustomNoUpgrade, - CustomNoUpgrade: config.CustomNoUpgrade{ + CustomNoUpgrade: config.EnableDisableFeatures{ Enabled: []string{"FeatureA"}, }, }, @@ -117,7 +117,7 @@ func TestIsCustomFeatureGatesConfigured(t *testing.T) { name: "CustomNoUpgrade without any enabled/disabled", fg: config.FeatureGates{ FeatureSet: config.FeatureSetCustomNoUpgrade, - CustomNoUpgrade: config.CustomNoUpgrade{}, + CustomNoUpgrade: config.EnableDisableFeatures{}, }, want: false, }, @@ -157,7 +157,7 @@ func TestFeatureGateLockFile_ReadWrite(t *testing.T) { name: "write and read custom feature gates", lockFile: featureGateLockFile{ FeatureSet: config.FeatureSetCustomNoUpgrade, - CustomNoUpgrade: config.CustomNoUpgrade{ + CustomNoUpgrade: config.EnableDisableFeatures{ Enabled: []string{"FeatureA", "FeatureB"}, Disabled: []string{"FeatureC"}, }, @@ -223,9 +223,23 @@ func TestConfigValidationChecksPass(t *testing.T) { name: "unset any feature set", lockFile: featureGateLockFile{ FeatureSet: config.FeatureSetCustomNoUpgrade, +<<<<<<< HEAD }, current: config.FeatureGates{ FeatureSet: "", +======= + CustomNoUpgrade: config.EnableDisableFeatures{ + Enabled: []string{"FeatureA", "FeatureB"}, + Disabled: []string{"FeatureC"}, + }, + }, + current: config.FeatureGates{ + FeatureSet: config.FeatureSetCustomNoUpgrade, + CustomNoUpgrade: config.EnableDisableFeatures{ + Enabled: []string{"FeatureA", "FeatureB"}, + Disabled: []string{"FeatureC"}, + }, +>>>>>>> 35dcb7969 (add feature exemption lists) }, wantErr: true, }, @@ -233,6 +247,35 @@ func TestConfigValidationChecksPass(t *testing.T) { name: "change CustomNoUpgrade to any other feature set", lockFile: featureGateLockFile{ FeatureSet: config.FeatureSetCustomNoUpgrade, +<<<<<<< HEAD +======= + CustomNoUpgrade: config.EnableDisableFeatures{ + Enabled: []string{"FeatureA"}, + }, + }, + current: config.FeatureGates{ + FeatureSet: config.FeatureSetCustomNoUpgrade, + CustomNoUpgrade: config.EnableDisableFeatures{ + Enabled: []string{"FeatureB"}, + }, + }, + wantMatch: false, + }, + { + name: "different feature sets", + lockFile: featureGateLockFile{ + FeatureSet: config.FeatureSetTechPreviewNoUpgrade, + }, + current: config.FeatureGates{ + FeatureSet: config.FeatureSetDevPreviewNoUpgrade, + }, + wantMatch: false, + }, + { + name: "identical TechPreviewNoUpgrade", + lockFile: featureGateLockFile{ + FeatureSet: config.FeatureSetTechPreviewNoUpgrade, +>>>>>>> 35dcb7969 (add feature exemption lists) }, current: config.FeatureGates{ FeatureSet: config.FeatureSetTechPreviewNoUpgrade, @@ -288,7 +331,7 @@ func TestFeatureGateLockManagement_FirstRun(t *testing.T) { ApiServer: config.ApiServer{ FeatureGates: config.FeatureGates{ FeatureSet: config.FeatureSetCustomNoUpgrade, - CustomNoUpgrade: config.CustomNoUpgrade{ + CustomNoUpgrade: config.EnableDisableFeatures{ Enabled: []string{"FeatureA"}, }, }, @@ -346,7 +389,7 @@ func TestFeatureGateLockManagement_ConfigChange(t *testing.T) { // Create lockFile file with initial config initialLock := featureGateLockFile{ FeatureSet: config.FeatureSetCustomNoUpgrade, - CustomNoUpgrade: config.CustomNoUpgrade{ + CustomNoUpgrade: config.EnableDisableFeatures{ Enabled: []string{"FeatureA"}, }, } @@ -359,7 +402,7 @@ func TestFeatureGateLockManagement_ConfigChange(t *testing.T) { ApiServer: config.ApiServer{ FeatureGates: config.FeatureGates{ FeatureSet: config.FeatureSetCustomNoUpgrade, - CustomNoUpgrade: config.CustomNoUpgrade{ + CustomNoUpgrade: config.EnableDisableFeatures{ Enabled: []string{"FeatureB"}, // Different feature }, }, @@ -456,7 +499,7 @@ func TestFeatureGateLockManagement_VersionChange(t *testing.T) { // Create lockFile file with locked version lockFile := featureGateLockFile{ FeatureSet: config.FeatureSetCustomNoUpgrade, - CustomNoUpgrade: config.CustomNoUpgrade{ + CustomNoUpgrade: config.EnableDisableFeatures{ Enabled: []string{"FeatureA"}, }, Version: tt.lockFileVer, @@ -469,7 +512,7 @@ func TestFeatureGateLockManagement_VersionChange(t *testing.T) { ApiServer: config.ApiServer{ FeatureGates: config.FeatureGates{ FeatureSet: config.FeatureSetCustomNoUpgrade, - CustomNoUpgrade: config.CustomNoUpgrade{ + CustomNoUpgrade: config.EnableDisableFeatures{ Enabled: []string{"FeatureA"}, }, }, diff --git a/pkg/config/apiserver.go b/pkg/config/apiserver.go index efbd1b4f9a..b0c1ab4147 100644 --- a/pkg/config/apiserver.go +++ b/pkg/config/apiserver.go @@ -139,7 +139,7 @@ const ( FeatureSetDevPreviewNoUpgrade = "DevPreviewNoUpgrade" ) -type CustomNoUpgrade struct { +type EnableDisableFeatures struct { Enabled []string `json:"enabled"` Disabled []string `json:"disabled"` } @@ -149,15 +149,21 @@ type CustomNoUpgrade struct { var RequiredFeatureGates = []string{"UserNamespacesSupport", "UserNamespacesPodSecurityStandards"} type FeatureGates struct { - FeatureSet string `json:"featureSet"` - CustomNoUpgrade CustomNoUpgrade `json:"customNoUpgrade"` + FeatureSet string `json:"featureSet"` + // CustomNoUpgrade is used to enable/disable feature gates. When the enabled or disable lists are not empty, x- and y-stream upgrades will be blocked. + // Use this field exclusively for custom feature gates, unless you are certain that the feature gate is a SpecialHandlingSupportExceptionRequired feature. + CustomNoUpgrade EnableDisableFeatures `json:"customNoUpgrade"` + // SpecialHandlingSupportExceptionRequired is used to enable/disable feature gates without blocking x- and y-stream upgrades. + // A SpecialHandlingSupportExceptionRequired feature will be given precedence over the same feature (if set) in CustomNoUpgrade features. + SpecialHandlingSupportExceptionRequired EnableDisableFeatures `json:"specialHandlingSupportExceptionRequired"` } // 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() -addFeatures := func(features []string, enabled bool) { + + addFeatures := func(features []string, enabled bool) { for _, feature := range features { ret.Insert(fmt.Sprintf("%s=%t", feature, enabled)) } @@ -165,6 +171,8 @@ addFeatures := func(features []string, enabled bool) { addFeatures(fg.CustomNoUpgrade.Enabled, true) addFeatures(fg.CustomNoUpgrade.Disabled, false) + addFeatures(fg.SpecialHandlingSupportExceptionRequired.Enabled, true) + addFeatures(fg.SpecialHandlingSupportExceptionRequired.Disabled, false) return ret.List(), nil } @@ -193,7 +201,7 @@ func (fg *FeatureGates) validateFeatureGates() error { return nil } -switch fg.FeatureSet { + switch fg.FeatureSet { case "": return nil case FeatureSetCustomNoUpgrade: