OCPBUGS-73945: Allow custom feature gates to be exempted from upgrade blocking

pkg/admin/prerun/featuregate_lock.go

@@ -21,9 +21,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
@@ -142,7 +142,7 @@ func validateFeatureGateLockFile(cfg *config.Config) error {
 		return fmt.Errorf("failed to get current version: %w", err)
 	}
 
-	if lockFile.Version != currentVersion {
+	if lockFile.Version.Major != currentVersion.Major || lockFile.Version.Minor != currentVersion.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"+

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,23 +33,23 @@ func TestFeatureGateLockFile_Marshal(t *testing.T) {
 			name: "TechPreviewNoUpgrade",
 			lockFile: featureGateLockFile{
 				FeatureSet:      config.FeatureSetTechPreviewNoUpgrade,
-				CustomNoUpgrade: config.CustomNoUpgrade{},
+				CustomNoUpgrade: config.EnableDisableFeatures{},
 			},
 			wantErr: false,
 		},
 		{
 			name: "DevPreviewNoUpgrade",
 			lockFile: featureGateLockFile{
 				FeatureSet:      config.FeatureSetDevPreviewNoUpgrade,
-				CustomNoUpgrade: config.CustomNoUpgrade{},
+				CustomNoUpgrade: config.EnableDisableFeatures{},
 			},
 			wantErr: false,
 		},
 		{
 			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,
 		},
@@ -151,7 +151,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"},
 				},
@@ -217,14 +217,14 @@ func TestCompareFeatureGates(t *testing.T) {
 			name: "identical custom feature gates",
 			lockFile: featureGateLockFile{
 				FeatureSet: config.FeatureSetCustomNoUpgrade,
-				CustomNoUpgrade: config.CustomNoUpgrade{
+				CustomNoUpgrade: config.EnableDisableFeatures{
 					Enabled:  []string{"FeatureA", "FeatureB"},
 					Disabled: []string{"FeatureC"},
 				},
 			},
 			current: config.FeatureGates{
 				FeatureSet: config.FeatureSetCustomNoUpgrade,
-				CustomNoUpgrade: config.CustomNoUpgrade{
+				CustomNoUpgrade: config.EnableDisableFeatures{
 					Enabled:  []string{"FeatureA", "FeatureB"},
 					Disabled: []string{"FeatureC"},
 				},
@@ -235,13 +235,13 @@ func TestCompareFeatureGates(t *testing.T) {
 			name: "different enabled features",
 			lockFile: featureGateLockFile{
 				FeatureSet: config.FeatureSetCustomNoUpgrade,
-				CustomNoUpgrade: config.CustomNoUpgrade{
+				CustomNoUpgrade: config.EnableDisableFeatures{
 					Enabled: []string{"FeatureA"},
 				},
 			},
 			current: config.FeatureGates{
 				FeatureSet: config.FeatureSetCustomNoUpgrade,
-				CustomNoUpgrade: config.CustomNoUpgrade{
+				CustomNoUpgrade: config.EnableDisableFeatures{
 					Enabled: []string{"FeatureB"},
 				},
 			},
@@ -312,7 +312,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"},
 				},
 			},
@@ -366,7 +366,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"},
 		},
 	}
@@ -379,7 +379,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
 				},
 			},
@@ -393,59 +393,114 @@ func TestFeatureGateLockManagement_ConfigChange(t *testing.T) {
 }
 
 func TestFeatureGateLockManagement_VersionChange(t *testing.T) {
-	// Create a temporary directory for testing
-	tmpDir, err := os.MkdirTemp("", "featuregate-lockFile-test-*")
-	if err != nil {
-		t.Fatal(err)
-	}
-	defer os.RemoveAll(tmpDir)
-
-	// Override the lockFile file path for testing
-	originalPath := featureGateLockFilePath
-	featureGateLockFilePath = filepath.Join(tmpDir, "no-upgrade")
-	defer func() { featureGateLockFilePath = originalPath }()
-
-	// Override version file path for testing
-	originalVersionPath := versionFilePath
-	versionFilePath = filepath.Join(tmpDir, "version")
-	defer func() { versionFilePath = originalVersionPath }()
-
-	// Create a version file with NEW version (simulating upgrade) (version file uses JSON format)
-	versionData := versionFile{
-		Version: versionMetadata{Major: 4, Minor: 19, Patch: 0}, // Newer version
-		BootID:  "test-boot",
-	}
-	versionJSON, _ := json.Marshal(versionData)
-	if err := os.WriteFile(versionFilePath, versionJSON, 0600); err != nil {
-		t.Fatal(err)
-	}
-
-	// Create lockFile file with OLD version
-	lockFile := featureGateLockFile{
-		FeatureSet: config.FeatureSetCustomNoUpgrade,
-		CustomNoUpgrade: config.CustomNoUpgrade{
-			Enabled: []string{"FeatureA"},
+	tests := []struct {
+		name        string
+		lockFileVer versionMetadata
+		currentVer  versionMetadata
+		wantErr     bool
+		description string
+	}{
+		{
+			name:        "minor version upgrade should fail",
+			lockFileVer: versionMetadata{Major: 4, Minor: 21, Patch: 0},
+			currentVer:  versionMetadata{Major: 4, Minor: 22, Patch: 0},
+			wantErr:     true,
+			description: "Minor version upgrade (4.21.0 -> 4.22.0) should be blocked",
+		},
+		{
+			name:        "major version upgrade should fail",
+			lockFileVer: versionMetadata{Major: 4, Minor: 21, Patch: 0},
+			currentVer:  versionMetadata{Major: 5, Minor: 0, Patch: 0},
+			wantErr:     true,
+			description: "Major version upgrade (4.21.0 -> 5.0.0) should be blocked",
+		},
+		{
+			name:        "patch version change should succeed",
+			lockFileVer: versionMetadata{Major: 4, Minor: 21, Patch: 0},
+			currentVer:  versionMetadata{Major: 4, Minor: 21, Patch: 1},
+			wantErr:     false,
+			description: "Patch version change (4.21.0 -> 4.21.1) should be allowed",
+		},
+		{
+			name:        "same version should succeed",
+			lockFileVer: versionMetadata{Major: 4, Minor: 21, Patch: 0},
+			currentVer:  versionMetadata{Major: 4, Minor: 21, Patch: 0},
+			wantErr:     false,
+			description: "Same version (4.21.0 -> 4.21.0) should be allowed",
+		},
+		{
+			name:        "minor version downgrade should fail",
+			lockFileVer: versionMetadata{Major: 4, Minor: 22, Patch: 0},
+			currentVer:  versionMetadata{Major: 4, Minor: 21, Patch: 0},
+			wantErr:     true,
+			description: "Minor version downgrade (4.22.0 -> 4.21.0) should be blocked",
+		},
+		{
+			name:        "major version downgrade should fail",
+			lockFileVer: versionMetadata{Major: 5, Minor: 0, Patch: 0},
+			currentVer:  versionMetadata{Major: 4, Minor: 21, Patch: 0},
+			wantErr:     true,
+			description: "Major version downgrade (5.0.0 -> 4.21.0) should be blocked",
 		},
-		Version: versionMetadata{Major: 4, Minor: 18, Patch: 0}, // Older version
-	}
-	if err := writeFeatureGateLockFile(featureGateLockFilePath, lockFile); err != nil {
-		t.Fatal(err)
 	}
 
-	cfg := &config.Config{
-		ApiServer: config.ApiServer{
-			FeatureGates: config.FeatureGates{
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			// Create a temporary directory for testing
+			tmpDir, err := os.MkdirTemp("", "featuregate-lockFile-test-*")
+			if err != nil {
+				t.Fatal(err)
+			}
+			defer os.RemoveAll(tmpDir)
+
+			// Override the lockFile file path for testing
+			originalPath := featureGateLockFilePath
+			featureGateLockFilePath = filepath.Join(tmpDir, "no-upgrade")
+			defer func() { featureGateLockFilePath = originalPath }()
+
+			// Override version file path for testing
+			originalVersionPath := versionFilePath
+			versionFilePath = filepath.Join(tmpDir, "version")
+			defer func() { versionFilePath = originalVersionPath }()
+
+			// Create a version file with current version (version file uses JSON format)
+			versionData := versionFile{
+				Version: tt.currentVer,
+				BootID:  "test-boot",
+			}
+			versionJSON, _ := json.Marshal(versionData)
+			if err := os.WriteFile(versionFilePath, versionJSON, 0600); err != nil {
+				t.Fatal(err)
+			}
+
+			// Create lockFile file with locked version
+			lockFile := featureGateLockFile{
 				FeatureSet: config.FeatureSetCustomNoUpgrade,
-				CustomNoUpgrade: config.CustomNoUpgrade{
+				CustomNoUpgrade: config.EnableDisableFeatures{
 					Enabled: []string{"FeatureA"},
 				},
-			},
-		},
-	}
+				Version: tt.lockFileVer,
+			}
+			if err := writeFeatureGateLockFile(featureGateLockFilePath, lockFile); err != nil {
+				t.Fatal(err)
+			}
 
-	err = FeatureGateLockManagement(cfg)
-	if err == nil {
-		t.Error("FeatureGateLockManagement() should have failed with version change")
+			cfg := &config.Config{
+				ApiServer: config.ApiServer{
+					FeatureGates: config.FeatureGates{
+						FeatureSet: config.FeatureSetCustomNoUpgrade,
+						CustomNoUpgrade: config.EnableDisableFeatures{
+							Enabled: []string{"FeatureA"},
+						},
+					},
+				},
+			}
+
+			err = FeatureGateLockManagement(cfg)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("FeatureGateLockManagement() error = %v, wantErr %v. %s", err, tt.wantErr, tt.description)
+			}
+		})
 	}
 }