From 5d0caaa1a667bcd0dd6e1868beb9c17b2fc558ab Mon Sep 17 00:00:00 2001 From: yongruilin Date: Tue, 18 Mar 2025 00:04:16 +0000 Subject: [PATCH] feat: Add alpha feature verification to feature gates Implement a new function, verifyAlphaFeatures, to ensure that alpha features cannot be enabled by default. Update the verifyOrUpdateFeatureList function to call this new verification. Add corresponding unit tests to validate the behavior of alpha feature handling. --- .../cmd/feature_gates.go | 17 ++++++- .../cmd/feature_gates_test.go | 50 +++++++++++++++++++ 2 files changed, 66 insertions(+), 1 deletion(-) diff --git a/test/compatibility_lifecycle/cmd/feature_gates.go b/test/compatibility_lifecycle/cmd/feature_gates.go index 955750367f1..f8833200e4f 100644 --- a/test/compatibility_lifecycle/cmd/feature_gates.go +++ b/test/compatibility_lifecycle/cmd/feature_gates.go @@ -134,6 +134,10 @@ func verifyOrUpdateFeatureList(rootPath, featureListFile string, currentVersion } featureList = append(featureList, features...) + if err := verifyAlphaFeatures(featureList); err != nil { + return err + } + sort.Slice(featureList, func(i, j int) bool { return strings.ToLower(featureList[i].Name) < strings.ToLower(featureList[j].Name) }) @@ -175,7 +179,7 @@ func verifyOrUpdateFeatureList(rootPath, featureListFile string, currentVersion } func dedupeFeatureList(featureList []featureInfo) ([]featureInfo, error) { - if featureList == nil || len(featureList) < 1 { + if len(featureList) < 1 { return featureList, nil } last := featureList[0] @@ -262,6 +266,17 @@ func verifyFeatureRemoval(featureList []featureInfo, baseFeatureList []featureIn return nil } +func verifyAlphaFeatures(featureList []featureInfo) error { + for _, f := range featureList { + for _, spec := range f.VersionedSpecs { + if spec.PreRelease == "Alpha" && spec.Default { + return fmt.Errorf("alpha feature %s cannot be enabled by default", f.Name) + } + } + } + return nil +} + func searchPathForFeatures(path string) ([]featureInfo, error) { allFeatures := []featureInfo{} // Create a FileSet to work with diff --git a/test/compatibility_lifecycle/cmd/feature_gates_test.go b/test/compatibility_lifecycle/cmd/feature_gates_test.go index eabc287a86b..a0652852b2a 100644 --- a/test/compatibility_lifecycle/cmd/feature_gates_test.go +++ b/test/compatibility_lifecycle/cmd/feature_gates_test.go @@ -894,3 +894,53 @@ func TestVerifyFeatureRemoval(t *testing.T) { }) } } + +func TestVerifyAlphaFeatures(t *testing.T) { + tests := []struct { + name string + featureList []featureInfo + expectErr bool + expectedErrMsg string + }{ + { + name: "no alpha features", + featureList: []featureInfo{ + {Name: "FeatureB", VersionedSpecs: []featureSpec{{Version: "1.0", PreRelease: "Beta"}}}, + {Name: "FeatureC", VersionedSpecs: []featureSpec{{Version: "1.0", PreRelease: "GA", LockToDefault: true}}}, + }, + }, + { + name: "alpha feature disabled", + featureList: []featureInfo{ + {Name: "FeatureA", VersionedSpecs: []featureSpec{{Version: "1.0", PreRelease: "Alpha", Default: false}}}, + {Name: "FeatureB", VersionedSpecs: []featureSpec{{Version: "1.0", PreRelease: "Beta"}}}, + }, + }, + { + name: "alpha feature enabled", + featureList: []featureInfo{ + {Name: "FeatureA", VersionedSpecs: []featureSpec{{Version: "1.0", PreRelease: "Alpha", Default: true}}}, + {Name: "FeatureB", VersionedSpecs: []featureSpec{{Version: "1.0", PreRelease: "Beta"}}}, + }, + expectErr: true, + expectedErrMsg: "alpha feature FeatureA cannot be enabled by default", + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + err := verifyAlphaFeatures(tc.featureList) + if tc.expectErr { + if err == nil { + t.Fatalf("expected error, got nil") + } + if !strings.Contains(err.Error(), tc.expectedErrMsg) { + t.Fatalf("expected error message to contain %q, got %q", tc.expectedErrMsg, err.Error()) + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + }) + } +}