[sample-apiserver] Fix: Use Correct Effective Version for kube (kubernetes#125941)

* Fix slice copy of VersionedSpecs in FeatureGate.

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

* Update wardle to kube version mapping

Signed-off-by: Siyuan Zhang <[email protected]>
Signed-off-by: Feilian Xie <[email protected]>
Co-authored-by: Feilian Xie <[email protected]>

* Add cap to wardleEmulationVersionToKubeEmulationVersion.

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

* Add integration test for default BanFlunder behavior in version 1.2 without Wardle feature gate.

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

---------

Signed-off-by: Siyuan Zhang <[email protected]>
Signed-off-by: Feilian Xie <[email protected]>
Co-authored-by: Siyuan Zhang <[email protected]>

Author: fxierh

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

@@ -115,9 +115,6 @@ type FeatureGate interface {
 	// set on the copy without mutating the original. This is useful for validating
 	// config against potential feature gate changes before committing those changes.
 	DeepCopy() MutableVersionedFeatureGate
-	// CopyKnownFeatures returns a partial copy of the FeatureGate object, with all the known features and overrides.
-	// This is useful for creating a new instance of feature gate without inheriting all the enabled configurations of the base feature gate.
-	CopyKnownFeatures() MutableVersionedFeatureGate
 	// Validate checks if the flag gates are valid at the emulated version.
 	Validate() []error
 }
@@ -189,6 +186,10 @@ type MutableVersionedFeatureGate interface {
 	ExplicitlySet(name Feature) bool
 	// ResetFeatureValueToDefault resets the value of the feature back to the default value.
 	ResetFeatureValueToDefault(name Feature) error
+	// DeepCopyAndReset copies all the registered features of the FeatureGate object, with all the known features and overrides,
+	// and resets all the enabled status of the new feature gate.
+	// This is useful for creating a new instance of feature gate without inheriting all the enabled configurations of the base feature gate.
+	DeepCopyAndReset() MutableVersionedFeatureGate
 }
 
 // featureGate implements FeatureGate as well as pflag.Value for flag parsing.
@@ -423,10 +424,7 @@ func (f *featureGate) AddVersioned(features map[Feature]VersionedSpecs) error {
 	}
 
 	// Copy existing state
-	known := map[Feature]VersionedSpecs{}
-	for k, v := range f.known.Load().(map[Feature]VersionedSpecs) {
-		known[k] = v
-	}
+	known := f.GetAllVersioned()
 
 	for name, specs := range features {
 		sort.Sort(specs)
@@ -458,11 +456,8 @@ func (f *featureGate) OverrideDefaultAtVersion(name Feature, override bool, ver
 		return fmt.Errorf("cannot override default for feature %q: gates already added to a flag set", name)
 	}
 
-	known := map[Feature]VersionedSpecs{}
-	for k, v := range f.known.Load().(map[Feature]VersionedSpecs) {
-		sort.Sort(v)
-		known[k] = v
-	}
+	// Copy existing state
+	known := f.GetAllVersioned()
 
 	specs, ok := known[name]
 	if !ok {
@@ -509,7 +504,9 @@ func (f *featureGate) GetAll() map[Feature]FeatureSpec {
 func (f *featureGate) GetAllVersioned() map[Feature]VersionedSpecs {
 	retval := map[Feature]VersionedSpecs{}
 	for k, v := range f.known.Load().(map[Feature]VersionedSpecs) {
-		retval[k] = v
+		vCopy := make([]FeatureSpec, len(v))
+		_ = copy(vCopy, v)
+		retval[k] = vCopy
 	}
 	return retval
 }
@@ -660,9 +657,10 @@ func (f *featureGate) KnownFeatures() []string {
 	return known
 }
 
-// CopyKnownFeatures returns a partial copy of the FeatureGate object, with all the known features and overrides.
+// DeepCopyAndReset copies all the registered features of the FeatureGate object, with all the known features and overrides,
+// and resets all the enabled status of the new feature gate.
 // This is useful for creating a new instance of feature gate without inheriting all the enabled configurations of the base feature gate.
-func (f *featureGate) CopyKnownFeatures() MutableVersionedFeatureGate {
+func (f *featureGate) DeepCopyAndReset() MutableVersionedFeatureGate {
 	fg := NewVersionedFeatureGate(f.EmulationVersion())
 	known := f.GetAllVersioned()
 	fg.known.Store(known)
@@ -676,10 +674,7 @@ func (f *featureGate) DeepCopy() MutableVersionedFeatureGate {
 	f.lock.Lock()
 	defer f.lock.Unlock()
 	// Copy existing state.
-	known := map[Feature]VersionedSpecs{}
-	for k, v := range f.known.Load().(map[Feature]VersionedSpecs) {
-		known[k] = v
-	}
+	known := f.GetAllVersioned()
 	enabled := map[Feature]bool{}
 	for k, v := range f.enabled.Load().(map[Feature]bool) {
 		enabled[k] = v

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

@@ -600,7 +600,7 @@ func TestFeatureGateOverrideDefault(t *testing.T) {
 		f := NewFeatureGate()
 		require.NoError(t, f.Add(map[Feature]FeatureSpec{"TestFeature": {Default: false}}))
 		require.NoError(t, f.OverrideDefault("TestFeature", true))
-		fcopy := f.CopyKnownFeatures()
+		fcopy := f.DeepCopyAndReset()
 		if !f.Enabled("TestFeature") {
 			t.Error("TestFeature should be enabled by override")
 		}
@@ -609,6 +609,19 @@ func TestFeatureGateOverrideDefault(t *testing.T) {
 		}
 	})
 
+	t.Run("overrides are not passed over after CopyKnownFeatures", func(t *testing.T) {
+		f := NewFeatureGate()
+		require.NoError(t, f.Add(map[Feature]FeatureSpec{"TestFeature": {Default: false}}))
+		fcopy := f.DeepCopyAndReset()
+		require.NoError(t, f.OverrideDefault("TestFeature", true))
+		if !f.Enabled("TestFeature") {
+			t.Error("TestFeature should be enabled by override")
+		}
+		if fcopy.Enabled("TestFeature") {
+			t.Error("default override should not be passed over after CopyKnownFeatures")
+		}
+	})
+
 	t.Run("reflected in known features", func(t *testing.T) {
 		f := NewFeatureGate()
 		if err := f.Add(map[Feature]FeatureSpec{"TestFeature": {
@@ -1351,6 +1364,34 @@ func TestVersionedFeatureGateOverrideDefault(t *testing.T) {
 		}
 	})
 
+	t.Run("overrides are not passed over after deep copies", func(t *testing.T) {
+		f := NewVersionedFeatureGate(version.MustParse("1.29"))
+		require.NoError(t, f.SetEmulationVersion(version.MustParse("1.28")))
+		if err := f.AddVersioned(map[Feature]VersionedSpecs{
+			"TestFeature": {
+				{Version: version.MustParse("1.28"), Default: false},
+				{Version: version.MustParse("1.29"), Default: true},
+			},
+		}); err != nil {
+			t.Fatal(err)
+		}
+		assert.False(t, f.Enabled("TestFeature"))
+
+		fcopy := f.DeepCopy()
+		require.NoError(t, f.OverrideDefault("TestFeature", true))
+		require.NoError(t, f.OverrideDefaultAtVersion("TestFeature", false, version.MustParse("1.29")))
+		assert.True(t, f.Enabled("TestFeature"))
+		assert.False(t, fcopy.Enabled("TestFeature"))
+
+		require.NoError(t, f.SetEmulationVersion(version.MustParse("1.29")))
+		assert.False(t, f.Enabled("TestFeature"))
+		assert.False(t, fcopy.Enabled("TestFeature"))
+
+		require.NoError(t, fcopy.SetEmulationVersion(version.MustParse("1.29")))
+		assert.False(t, f.Enabled("TestFeature"))
+		assert.True(t, fcopy.Enabled("TestFeature"))
+	})
+
 	t.Run("reflected in known features", func(t *testing.T) {
 		f := NewVersionedFeatureGate(version.MustParse("1.29"))
 		require.NoError(t, f.SetEmulationVersion(version.MustParse("1.28")))
@@ -1536,7 +1577,7 @@ func TestCopyKnownFeatures(t *testing.T) {
 	require.NoError(t, f.Add(map[Feature]FeatureSpec{"FeatureA": {Default: false}, "FeatureB": {Default: false}}))
 	require.NoError(t, f.Set("FeatureA=true"))
 	require.NoError(t, f.OverrideDefault("FeatureB", true))
-	fcopy := f.CopyKnownFeatures()
+	fcopy := f.DeepCopyAndReset()
 	require.NoError(t, f.Add(map[Feature]FeatureSpec{"FeatureC": {Default: false}}))
 
 	assert.True(t, f.Enabled("FeatureA"))

staging/src/k8s.io/sample-apiserver/pkg/cmd/server/start.go

@@ -36,6 +36,7 @@ import (
 	utilfeature "k8s.io/apiserver/pkg/util/feature"
 	utilversion "k8s.io/apiserver/pkg/util/version"
 	"k8s.io/component-base/featuregate"
+	baseversion "k8s.io/component-base/version"
 	"k8s.io/sample-apiserver/pkg/admission/plugin/banflunder"
 	"k8s.io/sample-apiserver/pkg/admission/wardleinitializer"
 	"k8s.io/sample-apiserver/pkg/apis/wardle/v1alpha1"
@@ -59,14 +60,18 @@ type WardleServerOptions struct {
 	AlternateDNS []string
 }
 
-func wardleEmulationVersionToKubeEmulationVersion(ver *version.Version) *version.Version {
+func WardleVersionToKubeVersion(ver *version.Version) *version.Version {
 	if ver.Major() != 1 {
 		return nil
 	}
 	kubeVer := utilversion.DefaultKubeEffectiveVersion().BinaryVersion()
-	// "1.1" maps to kubeVer
-	offset := int(ver.Minor()) - 1
-	return kubeVer.OffsetMinor(offset)
+	// "1.2" maps to kubeVer
+	offset := int(ver.Minor()) - 2
+	mappedVer := kubeVer.OffsetMinor(offset)
+	if mappedVer.GreaterThan(kubeVer) {
+		return kubeVer
+	}
+	return mappedVer
 }
 
 // NewWardleServerOptions returns a new WardleServerOptions
@@ -112,19 +117,44 @@ func NewCommandStartWardleServer(ctx context.Context, defaults *WardleServerOpti
 	flags := cmd.Flags()
 	o.RecommendedOptions.AddFlags(flags)
 
-	wardleEffectiveVersion := utilversion.NewEffectiveVersion("1.2")
-	wardleFeatureGate := utilfeature.DefaultFeatureGate.CopyKnownFeatures()
+	// The following lines demonstrate how to configure version compatibility and feature gates
+	// for the "Wardle" component, as an example of KEP-4330.
+
+	// Create an effective version object for the "Wardle" component.
+	// This initializes the binary version, the emulation version and the minimum compatibility version.
+	//
+	// Note:
+	// - The binary version represents the actual version of the running source code.
+	// - The emulation version is the version whose capabilities are being emulated by the binary.
+	// - The minimum compatibility version specifies the minimum version that the component remains compatible with.
+	//
+	// Refer to KEP-4330 for more details: https://github.com/kubernetes/enhancements/blob/master/keps/sig-architecture/4330-compatibility-versions
+	defaultWardleVersion := "1.2"
+	// Register the "Wardle" component with the global component registry,
+	// associating it with its effective version and feature gate configuration.
+	// Will skip if the component has been registered, like in the integration test.
+	_, wardleFeatureGate := utilversion.DefaultComponentGlobalsRegistry.ComponentGlobalsOrRegister(
+		apiserver.WardleComponentName, utilversion.NewEffectiveVersion(defaultWardleVersion),
+		featuregate.NewVersionedFeatureGate(version.MustParse(defaultWardleVersion)))
+
+	// Add versioned feature specifications for the "BanFlunder" feature.
+	// These specifications, together with the effective version, determine if the feature is enabled.
 	utilruntime.Must(wardleFeatureGate.AddVersioned(map[featuregate.Feature]featuregate.VersionedSpecs{
 		"BanFlunder": {
-			{Version: version.MustParse("1.2"), Default: true, PreRelease: featuregate.GA},
-			{Version: version.MustParse("1.1"), Default: false, PreRelease: featuregate.Beta},
+			{Version: version.MustParse("1.2"), Default: true, PreRelease: featuregate.GA, LockToDefault: true},
+			{Version: version.MustParse("1.1"), Default: true, PreRelease: featuregate.Beta},
 			{Version: version.MustParse("1.0"), Default: false, PreRelease: featuregate.Alpha},
 		},
 	}))
-	utilruntime.Must(utilversion.DefaultComponentGlobalsRegistry.Register(apiserver.WardleComponentName, wardleEffectiveVersion, wardleFeatureGate))
-	_, _ = utilversion.DefaultComponentGlobalsRegistry.ComponentGlobalsOrRegister(
-		utilversion.DefaultKubeComponent, utilversion.DefaultKubeEffectiveVersion(), utilfeature.DefaultMutableFeatureGate)
-	utilruntime.Must(utilversion.DefaultComponentGlobalsRegistry.SetEmulationVersionMapping(apiserver.WardleComponentName, utilversion.DefaultKubeComponent, wardleEmulationVersionToKubeEmulationVersion))
+
+	// Register the default kube component if not already present in the global registry.
+	_, _ = utilversion.DefaultComponentGlobalsRegistry.ComponentGlobalsOrRegister(utilversion.DefaultKubeComponent,
+		utilversion.NewEffectiveVersion(baseversion.DefaultKubeBinaryVersion), utilfeature.DefaultMutableFeatureGate)
+
+	// Set the emulation version mapping from the "Wardle" component to the kube component.
+	// This ensures that the emulation version of the latter is determined by the emulation version of the former.
+	utilruntime.Must(utilversion.DefaultComponentGlobalsRegistry.SetEmulationVersionMapping(apiserver.WardleComponentName, utilversion.DefaultKubeComponent, WardleVersionToKubeVersion))
+
 	utilversion.DefaultComponentGlobalsRegistry.AddFlags(flags)
 
 	return cmd
@@ -177,7 +207,7 @@ func (o *WardleServerOptions) Config() (*apiserver.Config, error) {
 	serverConfig.OpenAPIV3Config.Info.Title = "Wardle"
 	serverConfig.OpenAPIV3Config.Info.Version = "0.1"
 
-	serverConfig.FeatureGate = utilversion.DefaultComponentGlobalsRegistry.FeatureGateFor(apiserver.WardleComponentName)
+	serverConfig.FeatureGate = utilversion.DefaultComponentGlobalsRegistry.FeatureGateFor(utilversion.DefaultKubeComponent)
 	serverConfig.EffectiveVersion = utilversion.DefaultComponentGlobalsRegistry.EffectiveVersionFor(apiserver.WardleComponentName)
 
 	if err := o.RecommendedOptions.ApplyTo(serverConfig); err != nil {

staging/src/k8s.io/sample-apiserver/pkg/cmd/server/start_test.go

@@ -35,13 +35,23 @@ func TestWardleEmulationVersionToKubeEmulationVersion(t *testing.T) {
 	}{
 		{
 			desc:                     "same version as than kube binary",
-			wardleEmulationVer:       version.MajorMinor(1, 1),
+			wardleEmulationVer:       version.MajorMinor(1, 2),
 			expectedKubeEmulationVer: defaultKubeEffectiveVersion.BinaryVersion(),
 		},
 		{
-			desc:                     "1 version higher than kube binary",
-			wardleEmulationVer:       version.MajorMinor(1, 2),
-			expectedKubeEmulationVer: defaultKubeEffectiveVersion.BinaryVersion().OffsetMinor(1),
+			desc:                     "1 version lower than kube binary",
+			wardleEmulationVer:       version.MajorMinor(1, 1),
+			expectedKubeEmulationVer: defaultKubeEffectiveVersion.BinaryVersion().OffsetMinor(-1),
+		},
+		{
+			desc:                     "2 versions lower than kube binary",
+			wardleEmulationVer:       version.MajorMinor(1, 0),
+			expectedKubeEmulationVer: defaultKubeEffectiveVersion.BinaryVersion().OffsetMinor(-2),
+		},
+		{
+			desc:                     "capped at kube binary",
+			wardleEmulationVer:       version.MajorMinor(1, 3),
+			expectedKubeEmulationVer: defaultKubeEffectiveVersion.BinaryVersion(),
 		},
 		{
 			desc:               "no mapping",
@@ -51,7 +61,7 @@ func TestWardleEmulationVersionToKubeEmulationVersion(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run(tc.desc, func(t *testing.T) {
-			mappedKubeEmulationVer := wardleEmulationVersionToKubeEmulationVersion(tc.wardleEmulationVer)
+			mappedKubeEmulationVer := WardleVersionToKubeVersion(tc.wardleEmulationVer)
 			assert.True(t, mappedKubeEmulationVer.EqualTo(tc.expectedKubeEmulationVer))
 		})
 	}