diff --git a/test/featuregates_linter/cmd/feature_gates.go b/test/featuregates_linter/cmd/feature_gates.go index 4c37b3ec8e2..0a4f56abd4e 100644 --- a/test/featuregates_linter/cmd/feature_gates.go +++ b/test/featuregates_linter/cmd/feature_gates.go @@ -162,15 +162,15 @@ func verifyOrUpdateFeatureList(rootPath, featureListFile string, update, version } } + featureListBytes, err := yaml.Marshal(featureList) + if err != nil { + return err + } if update { - data, err := yaml.Marshal(featureList) - if err != nil { - return err - } - return os.WriteFile(filePath, data, 0644) + return os.WriteFile(filePath, featureListBytes, 0644) } - if diff := cmp.Diff(featureList, baseFeatureList); diff != "" { + if diff := cmp.Diff(featureListBytes, baseFeatureListBytes); diff != "" { if versioned { return fmt.Errorf("detected diff in versioned feature list (%s), diff: \n%s", versionedFeatureListFile, diff) } else { diff --git a/test/featuregates_linter/cmd/feature_gates_test.go b/test/featuregates_linter/cmd/feature_gates_test.go index 1ebb61672c3..14fb8097a8e 100644 --- a/test/featuregates_linter/cmd/feature_gates_test.go +++ b/test/featuregates_linter/cmd/feature_gates_test.go @@ -24,7 +24,6 @@ import ( "os" "path/filepath" "reflect" - "strings" "testing" "github.com/google/go-cmp/cmp" @@ -111,6 +110,7 @@ func TestVerifyOrUpdateFeatureListUnversioned(t *testing.T) { tests := []struct { name string goFileContent string + featureListFileContent string updatedFeatureListFileContent string expectVerifyErr bool expectUpdateErr bool @@ -132,8 +132,48 @@ var otherFeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{ AppArmorFields: {Default: true, PreRelease: featuregate.Beta}, } `, + featureListFileContent: featureListFileContent, updatedFeatureListFileContent: featureListFileContent, }, + { + name: "semantically equivalent, formatting wrong", + goFileContent: ` +package features + +import ( + "k8s.io/component-base/featuregate" +) +var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{ + AppArmorFields: {Default: true, PreRelease: featuregate.Beta}, + CPUCFSQuotaPeriod: {Default: false, PreRelease: featuregate.Alpha}, + ClusterTrustBundleProjection: {Default: false, PreRelease: featuregate.Alpha}, +} +var otherFeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{ + AppArmorFields: {Default: true, PreRelease: featuregate.Beta}, +} +`, + featureListFileContent: `- name: AppArmorFields + versionedSpecs: + - default: true + lockToDefault: false + preRelease: Beta + version: "" +- name: ClusterTrustBundleProjection + versionedSpecs: + - default: false + lockToDefault: false + preRelease: Alpha + version: "" +- name: CPUCFSQuotaPeriod + versionedSpecs: + - default: false + lockToDefault: false + preRelease: Alpha + version: "" +`, + updatedFeatureListFileContent: featureListFileContent, + expectVerifyErr: true, + }, { name: "same feature added twice with different lifecycle", goFileContent: ` @@ -151,8 +191,9 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS AppArmorFields: {Default: true, PreRelease: featuregate.Alpha}, } `, - expectVerifyErr: true, - expectUpdateErr: true, + featureListFileContent: featureListFileContent, + expectVerifyErr: true, + expectUpdateErr: true, }, { name: "new feature added", @@ -169,8 +210,9 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS SELinuxMount: {Default: false, PreRelease: featuregate.Alpha}, } `, - expectVerifyErr: true, - expectUpdateErr: true, + featureListFileContent: featureListFileContent, + expectVerifyErr: true, + expectUpdateErr: true, }, { name: "delete feature", @@ -185,7 +227,8 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS ClusterTrustBundleProjection: {Default: false, PreRelease: featuregate.Alpha}, } `, - expectVerifyErr: true, + featureListFileContent: featureListFileContent, + expectVerifyErr: true, updatedFeatureListFileContent: `- name: AppArmorFields versionedSpecs: - default: true @@ -214,13 +257,14 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS ClusterTrustBundleProjection: {Default: false, PreRelease: featuregate.Alpha}, } `, - expectVerifyErr: true, - expectUpdateErr: true, + featureListFileContent: featureListFileContent, + expectVerifyErr: true, + expectUpdateErr: true, }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - featureListFile := writeContentToTmpFile(t, "", "feature_list.yaml", strings.TrimSpace(featureListFileContent)) + featureListFile := writeContentToTmpFile(t, "", "feature_list.yaml", tc.featureListFileContent) tmpDir := filepath.Dir(featureListFile.Name()) _ = writeContentToTmpFile(t, tmpDir, "pkg/new_features.go", tc.goFileContent) err := verifyOrUpdateFeatureList(tmpDir, filepath.Base(featureListFile.Name()), false, false) @@ -229,7 +273,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS } err = verifyOrUpdateFeatureList(tmpDir, filepath.Base(featureListFile.Name()), true, false) if tc.expectUpdateErr != (err != nil) { - t.Errorf("expectVerifyErr=%v, got err: %s", tc.expectVerifyErr, err) + t.Errorf("expectUpdateErr=%v, got err: %s", tc.expectUpdateErr, err) } if tc.expectUpdateErr { return @@ -272,6 +316,7 @@ func TestVerifyOrUpdateFeatureListVersioned(t *testing.T) { tests := []struct { name string goFileContent string + featureListFileContent string updatedFeatureListFileContent string expectVerifyErr bool expectUpdateErr bool @@ -303,8 +348,62 @@ var otherFeatureGates = map[featuregate.Feature]featuregate.VersionedSpecs{ }, } `, + featureListFileContent: featureListFileContent, updatedFeatureListFileContent: featureListFileContent, }, + { + name: "semantically equivalent, formatting wrong", + goFileContent: ` +package features + +import ( + "k8s.io/apimachinery/pkg/util/version" + "k8s.io/component-base/featuregate" +) +var defaultVersionedKubernetesFeatureGates = map[featuregate.Feature]featuregate.VersionedSpecs{ + AppArmorFields: { + {Version: version.MajorMinor(1, 30), Default: true, PreRelease: featuregate.Beta}, + }, + CPUCFSQuotaPeriod: { + {Version: version.MustParse("1.30"), Default: false, PreRelease: featuregate.Alpha}, + {Version: version.MustParse("1.31"), Default: true, PreRelease: featuregate.Beta}, + }, + genericfeatures.APIListChunking: { + {Version: version.MustParse("1.30"), Default: true, PreRelease: featuregate.GA, LockToDefault: true}, + }, +} +var otherFeatureGates = map[featuregate.Feature]featuregate.VersionedSpecs{ + AppArmorFields: { + {Version: version.MajorMinor(1, 30), Default: true, PreRelease: featuregate.Beta}, + }, +} +`, + featureListFileContent: `- name: APIListChunking + versionedSpecs: + - default: true + lockToDefault: true + preRelease: GA + version: "1.30" +- name: AppArmorFields + versionedSpecs: + - default: true + lockToDefault: false + preRelease: Beta + version: "1.30" +- name: CPUCFSQuotaPeriod + versionedSpecs: + - default: false + lockToDefault: false + preRelease: Alpha + version: "1.30" + - default: true + lockToDefault: false + preRelease: Beta + version: "1.31" +`, + updatedFeatureListFileContent: featureListFileContent, + expectVerifyErr: true, + }, { name: "same feature added twice with different lifecycle", goFileContent: ` @@ -332,8 +431,9 @@ var otherFeatureGates = map[featuregate.Feature]featuregate.VersionedSpecs{ }, } `, - expectVerifyErr: true, - expectUpdateErr: true, + featureListFileContent: featureListFileContent, + expectVerifyErr: true, + expectUpdateErr: true, }, { name: "VersionedSpecs not ordered by version", @@ -357,8 +457,9 @@ var defaultVersionedKubernetesFeatureGates = map[featuregate.Feature]featuregate }, } `, - expectVerifyErr: true, - expectUpdateErr: true, + featureListFileContent: featureListFileContent, + expectVerifyErr: true, + expectUpdateErr: true, }, { name: "add new feature", @@ -385,7 +486,8 @@ var defaultVersionedKubernetesFeatureGates = map[featuregate.Feature]featuregate }, } `, - expectVerifyErr: true, + expectVerifyErr: true, + featureListFileContent: featureListFileContent, updatedFeatureListFileContent: `- name: APIListChunking versionedSpecs: - default: true @@ -435,7 +537,8 @@ var defaultVersionedKubernetesFeatureGates = map[featuregate.Feature]featuregate }, } `, - expectVerifyErr: true, + expectVerifyErr: true, + featureListFileContent: featureListFileContent, updatedFeatureListFileContent: `- name: APIListChunking versionedSpecs: - default: true @@ -477,7 +580,8 @@ var defaultVersionedKubernetesFeatureGates = map[featuregate.Feature]featuregate }, } `, - expectVerifyErr: true, + expectVerifyErr: true, + featureListFileContent: featureListFileContent, updatedFeatureListFileContent: `- name: APIListChunking versionedSpecs: - default: true @@ -509,7 +613,7 @@ var defaultVersionedKubernetesFeatureGates = map[featuregate.Feature]featuregate } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - featureListFile := writeContentToTmpFile(t, "", "feature_list.yaml", strings.TrimSpace(featureListFileContent)) + featureListFile := writeContentToTmpFile(t, "", "feature_list.yaml", tc.featureListFileContent) tmpDir := filepath.Dir(featureListFile.Name()) _ = writeContentToTmpFile(t, tmpDir, "pkg/new_features.go", tc.goFileContent) err := verifyOrUpdateFeatureList(tmpDir, filepath.Base(featureListFile.Name()), false, true) diff --git a/test/featuregates_linter/test_data/versioned_feature_list.yaml b/test/featuregates_linter/test_data/versioned_feature_list.yaml index f77b99b1b24..65feff4cf21 100644 --- a/test/featuregates_linter/test_data/versioned_feature_list.yaml +++ b/test/featuregates_linter/test_data/versioned_feature_list.yaml @@ -978,10 +978,10 @@ version: "1.32" - name: PodLogsQuerySplitStreams versionedSpecs: - - default: false - lockToDefault: false - preRelease: Alpha - version: "1.32" + - default: false + lockToDefault: false + preRelease: Alpha + version: "1.32" - name: PodReadyToStartContainersCondition versionedSpecs: - default: false