Add Validation to versioned feature specs.

Co-authored-by: Jordan Liggitt <[email protected]>
Co-authored-by: Siyuan Zhang <[email protected]>

Signed-off-by: Siyuan Zhang <[email protected]>

Author: siyuanfoundation

cmd/kube-controller-manager/app/options/options_test.go

@@ -747,8 +747,8 @@ func TestEmulatedVersion(t *testing.T) {
 		fg := featuregate.NewVersionedFeatureGate(version.MustParse("1.32"))
 		utilruntime.Must(fg.AddVersioned(map[featuregate.Feature]featuregate.VersionedSpecs{
 			"kubeA": {
-				{Version: version.MustParse("1.32"), Default: true, LockToDefault: true, PreRelease: featuregate.GA},
 				{Version: version.MustParse("1.30"), Default: false, PreRelease: featuregate.Beta},
+				{Version: version.MustParse("1.32"), Default: true, LockToDefault: true, PreRelease: featuregate.GA},
 			},
 			"kubeB": {
 				{Version: version.MustParse("1.31"), Default: false, PreRelease: featuregate.Alpha},

cmd/kube-scheduler/app/server_test.go

@@ -29,6 +29,7 @@ import (
 
 	"github.com/google/go-cmp/cmp"
 	"github.com/spf13/pflag"
+
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -446,8 +447,8 @@ leaderElection:
 			fg := feature.DefaultFeatureGate.DeepCopy()
 			utilruntime.Must(fg.AddVersioned(map[featuregate.Feature]featuregate.VersionedSpecs{
 				"kubeA": {
-					{Version: version.MustParse("1.32"), Default: true, LockToDefault: true, PreRelease: featuregate.GA},
 					{Version: version.MustParse("1.30"), Default: false, PreRelease: featuregate.Beta},
+					{Version: version.MustParse("1.32"), Default: true, LockToDefault: true, PreRelease: featuregate.GA},
 				},
 				"kubeB": {
 					{Version: version.MustParse("1.31"), Default: false, PreRelease: featuregate.Alpha},

pkg/features/versioned_kube_features.go

@@ -37,18 +37,22 @@ import (
 // Entries are alphabetized.
 var defaultVersionedKubernetesFeatureGates = map[featuregate.Feature]featuregate.VersionedSpecs{
 	AllowDNSOnlyNodeCSR: {
+		{Version: version.MustParse("1.0"), Default: true, PreRelease: featuregate.GA},
 		{Version: version.MustParse("1.31"), Default: false, PreRelease: featuregate.Deprecated},
 	},
 
 	AllowInsecureKubeletCertificateSigningRequests: {
+		{Version: version.MustParse("1.0"), Default: true, PreRelease: featuregate.GA},
 		{Version: version.MustParse("1.31"), Default: false, PreRelease: featuregate.Deprecated},
 	},
 
 	AllowOverwriteTerminationGracePeriodSeconds: {
+		{Version: version.MustParse("1.0"), Default: true, PreRelease: featuregate.GA},
 		{Version: version.MustParse("1.32"), Default: false, PreRelease: featuregate.Deprecated},
 	},
 
 	AllowServiceLBStatusOnNonLB: {
+		{Version: version.MustParse("1.0"), Default: true, PreRelease: featuregate.GA},
 		{Version: version.MustParse("1.29"), Default: false, PreRelease: featuregate.Deprecated},
 		{Version: version.MustParse("1.32"), Default: false, PreRelease: featuregate.Deprecated, LockToDefault: true}, // remove in 1.35
 	},
@@ -291,6 +295,7 @@ var defaultVersionedKubernetesFeatureGates = map[featuregate.Feature]featuregate
 	},
 
 	genericfeatures.KMSv1: {
+		{Version: version.MustParse("1.0"), Default: true, PreRelease: featuregate.GA},
 		{Version: version.MustParse("1.28"), Default: true, PreRelease: featuregate.Deprecated},
 		{Version: version.MustParse("1.29"), Default: false, PreRelease: featuregate.Deprecated},
 	},
@@ -474,6 +479,7 @@ var defaultVersionedKubernetesFeatureGates = map[featuregate.Feature]featuregate
 	},
 
 	KubeletRegistrationGetOnExistsOnly: {
+		{Version: version.MustParse("1.0"), Default: true, PreRelease: featuregate.GA},
 		{Version: version.MustParse("1.32"), Default: false, PreRelease: featuregate.Deprecated},
 	},
 

staging/src/k8s.io/apiserver/pkg/features/kube_features.go

@@ -356,6 +356,7 @@ var defaultVersionedKubernetesFeatureGates = map[featuregate.Feature]featuregate
 	},
 
 	KMSv1: {
+		{Version: version.MustParse("1.0"), Default: true, PreRelease: featuregate.GA},
 		{Version: version.MustParse("1.28"), Default: true, PreRelease: featuregate.Deprecated},
 		{Version: version.MustParse("1.29"), Default: false, PreRelease: featuregate.Deprecated},
 	},

staging/src/k8s.io/component-base/featuregate/feature_gate.go

@@ -318,7 +318,6 @@ func (f *featureGate) unsafeSetFromMap(enabled map[Feature]bool, m map[string]bo
 	// Copy existing state
 	known := map[Feature]VersionedSpecs{}
 	for k, v := range f.known.Load().(map[Feature]VersionedSpecs) {
-		sort.Sort(v)
 		known[k] = v
 	}
 
@@ -421,19 +420,52 @@ func (f *featureGate) AddVersioned(features map[Feature]VersionedSpecs) error {
 	if f.closed {
 		return fmt.Errorf("cannot add a feature gate after adding it to the flag set")
 	}
-
 	// Copy existing state
 	known := f.GetAllVersioned()
 
 	for name, specs := range features {
-		sort.Sort(specs)
 		if existingSpec, found := known[name]; found {
-			sort.Sort(existingSpec)
 			if reflect.DeepEqual(existingSpec, specs) {
 				continue
 			}
 			return fmt.Errorf("feature gate %q with different spec already exists: %v", name, existingSpec)
 		}
+		// Validate new specs are well-formed
+		var lastVersion *version.Version
+		var wasBeta, wasGA, wasDeprecated bool
+		for i, spec := range specs {
+			if spec.Version == nil {
+				return fmt.Errorf("feature %q did not provide a version", name)
+			}
+			if len(spec.Version.Components()) != 2 {
+				return fmt.Errorf("feature %q specified patch version: %s", name, spec.Version.String())
+
+			}
+			// gates that begin as deprecated must indicate their prior state
+			if i == 0 && spec.PreRelease == Deprecated && spec.Version.Minor() != 0 {
+				return fmt.Errorf("feature %q introduced as deprecated must provide a 1.0 entry indicating initial state", name)
+			}
+			if i > 0 {
+				// versions must strictly increase
+				if !lastVersion.LessThan(spec.Version) {
+					return fmt.Errorf("feature %q lists version transitions in non-increasing order (%s <= %s)", name, spec.Version, lastVersion)
+				}
+				// stability must not regress from ga --> {beta,alpha} or beta --> alpha, and
+				// Deprecated state must be the terminal state
+				switch {
+				case spec.PreRelease != Deprecated && wasDeprecated:
+					return fmt.Errorf("deprecated feature %q must not resurrect from its terminal state", name)
+				case spec.PreRelease == Alpha && (wasBeta || wasGA):
+					return fmt.Errorf("feature %q regresses stability from more stable level to %s in %s", name, spec.PreRelease, spec.Version)
+				case spec.PreRelease == Beta && wasGA:
+					return fmt.Errorf("feature %q regresses stability from more stable level to %s in %s", name, spec.PreRelease, spec.Version)
+				}
+			}
+			lastVersion = spec.Version
+			wasBeta = wasBeta || spec.PreRelease == Beta
+			wasGA = wasGA || spec.PreRelease == GA
+			wasDeprecated = wasDeprecated || spec.PreRelease == Deprecated
+		}
 		known[name] = specs
 	}