From 95f30f136fb21a1be37074e3b2abcfabdab42ba5 Mon Sep 17 00:00:00 2001 From: Feilian Xie Date: Sat, 27 Jul 2024 03:03:52 +0800 Subject: [PATCH] [sample-apiserver] Fix: Use Correct Effective Version for kube (#125941) * Fix slice copy of VersionedSpecs in FeatureGate. Signed-off-by: Siyuan Zhang * Update wardle to kube version mapping Signed-off-by: Siyuan Zhang Signed-off-by: Feilian Xie Co-authored-by: Feilian Xie * Add cap to wardleEmulationVersionToKubeEmulationVersion. Signed-off-by: Siyuan Zhang * Add integration test for default BanFlunder behavior in version 1.2 without Wardle feature gate. Signed-off-by: Siyuan Zhang --------- Signed-off-by: Siyuan Zhang Signed-off-by: Feilian Xie Co-authored-by: Siyuan Zhang Kubernetes-commit: ebdca538058d2265cb8dc528d0145faea0a6a7cf --- featuregate/feature_gate.go | 33 ++++++++++------------- featuregate/feature_gate_test.go | 45 ++++++++++++++++++++++++++++++-- go.mod | 4 +-- go.sum | 8 +++--- 4 files changed, 63 insertions(+), 27 deletions(-) diff --git a/featuregate/feature_gate.go b/featuregate/feature_gate.go index c4d8480..b6f08a6 100644 --- a/featuregate/feature_gate.go +++ b/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 diff --git a/featuregate/feature_gate_test.go b/featuregate/feature_gate_test.go index cd6eeb8..b0e0413 100644 --- a/featuregate/feature_gate_test.go +++ b/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")) diff --git a/go.mod b/go.mod index ed09da1..f7318f8 100644 --- a/go.mod +++ b/go.mod @@ -26,7 +26,7 @@ require ( golang.org/x/sys v0.21.0 gopkg.in/yaml.v2 v2.4.0 k8s.io/apimachinery v0.0.0-20240720202316-95b78024e3fe - k8s.io/client-go v0.0.0-20240725170742-93c6a5bf507f + k8s.io/client-go v0.0.0-20240725210749-4536e5a391f8 k8s.io/klog/v2 v2.130.1 k8s.io/utils v0.0.0-20240711033017-18e509b52bc8 sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd @@ -76,7 +76,7 @@ require ( google.golang.org/protobuf v1.34.2 // indirect gopkg.in/inf.v0 v0.9.1 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect - k8s.io/api v0.0.0-20240724031224-63e21d3bdab9 // indirect + k8s.io/api v0.0.0-20240725200553-fb1fc3084c0e // indirect k8s.io/kube-openapi v0.0.0-20240228011516-70dd3763d340 // indirect sigs.k8s.io/structured-merge-diff/v4 v4.4.1 // indirect sigs.k8s.io/yaml v1.4.0 // indirect diff --git a/go.sum b/go.sum index fbaf229..9383fb9 100644 --- a/go.sum +++ b/go.sum @@ -204,12 +204,12 @@ gopkg.in/yaml.v2 v2.4.0/go.mod h1:RDklbk79AGWmwhnvt/jBztapEOGDOx6ZbXqjP6csGnQ= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= -k8s.io/api v0.0.0-20240724031224-63e21d3bdab9 h1:DsdxdppkdprdzZ/IwMZY+uNpvEcKtpJpKQRgll5PXto= -k8s.io/api v0.0.0-20240724031224-63e21d3bdab9/go.mod h1:ytlEzqC2wOTwYET71W7+J+k7O2V7vrDuzmNLBSpgT+k= +k8s.io/api v0.0.0-20240725200553-fb1fc3084c0e h1:zSGnlOF57ubuWLnmPjHd1c9XRaXJeXdcVsszq+wm17o= +k8s.io/api v0.0.0-20240725200553-fb1fc3084c0e/go.mod h1:ytlEzqC2wOTwYET71W7+J+k7O2V7vrDuzmNLBSpgT+k= k8s.io/apimachinery v0.0.0-20240720202316-95b78024e3fe h1:V9MwpYUwbKlfLKVrhpVuKWiat/LBIhm1pGB9/xdHm5Q= k8s.io/apimachinery v0.0.0-20240720202316-95b78024e3fe/go.mod h1:rsPdaZJfTfLsNJSQzNHQvYoTmxhoOEofxtOsF3rtsMo= -k8s.io/client-go v0.0.0-20240725170742-93c6a5bf507f h1:PwgWcIHZw0QbiNWDz57rSH7GnSgq7d8lOo/kJoHd/DI= -k8s.io/client-go v0.0.0-20240725170742-93c6a5bf507f/go.mod h1:duh6Hk8A52zAEoC179uJBw/MGC05zB74mzfF/nESIyU= +k8s.io/client-go v0.0.0-20240725210749-4536e5a391f8 h1:ne00XpPG8VqvDVLT5nhjP/GpVQSQVthGm3ivE/YCawA= +k8s.io/client-go v0.0.0-20240725210749-4536e5a391f8/go.mod h1:SORxrGJjz+pQkGtVEBP5ilXUslm+iXOJRgvngcKr6+Q= k8s.io/klog/v2 v2.130.1 h1:n9Xl7H1Xvksem4KFG4PYbdQCQxqc/tTUyrgXaOhHSzk= k8s.io/klog/v2 v2.130.1/go.mod h1:3Jpz1GvMt720eyJH1ckRHK1EDfpxISzJ7I9OYgaDtPE= k8s.io/kube-openapi v0.0.0-20240228011516-70dd3763d340 h1:BZqlfIlq5YbRMFko6/PM7FjZpUb45WallggurYhKGag=