Skip to content

Commit

Permalink
[sample-apiserver] Fix: Use Correct Effective Version for kube (#125941)
Browse files Browse the repository at this point in the history
* Fix slice copy of VersionedSpecs in FeatureGate.

Signed-off-by: Siyuan Zhang <sizhang@google.com>

* Update wardle to kube version mapping

Signed-off-by: Siyuan Zhang <sizhang@google.com>
Signed-off-by: Feilian Xie <fxie@redhat.com>
Co-authored-by: Feilian Xie <fxie@redhat.com>

* Add cap to wardleEmulationVersionToKubeEmulationVersion.

Signed-off-by: Siyuan Zhang <sizhang@google.com>

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

Signed-off-by: Siyuan Zhang <sizhang@google.com>

---------

Signed-off-by: Siyuan Zhang <sizhang@google.com>
Signed-off-by: Feilian Xie <fxie@redhat.com>
Co-authored-by: Siyuan Zhang <sizhang@google.com>

Kubernetes-commit: ebdca538058d2265cb8dc528d0145faea0a6a7cf
  • Loading branch information
fxierh authored and k8s-publishing-bot committed Jul 26, 2024
1 parent 763ac17 commit 95f30f1
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 27 deletions.
33 changes: 14 additions & 19 deletions featuregate/feature_gate.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand Down
45 changes: 43 additions & 2 deletions featuregate/feature_gate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand All @@ -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": {
Expand Down Expand Up @@ -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")))
Expand Down Expand Up @@ -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"))
Expand Down
4 changes: 2 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down

0 comments on commit 95f30f1

Please sign in to comment.