From 0f88bbe9b1fcb2138cd668948d75358c1a1d9858 Mon Sep 17 00:00:00 2001 From: wojtekt Date: Mon, 7 Dec 2020 13:16:40 +0100 Subject: [PATCH] Promote Immutable Secrets/ConfigMaps feature to GA --- pkg/apis/core/types.go | 2 -- pkg/features/kube_features.go | 5 +++-- pkg/kubelet/util/manager/watch_based_manager.go | 4 +--- .../util/manager/watch_based_manager_test.go | 5 ----- pkg/registry/core/configmap/strategy.go | 9 --------- pkg/registry/core/secret/strategy.go | 9 --------- staging/src/k8s.io/api/core/v1/types.go | 2 -- test/e2e/common/configmap_volume.go | 13 ++++++++++--- test/e2e/common/secrets_volume.go | 13 ++++++++++--- 9 files changed, 24 insertions(+), 38 deletions(-) diff --git a/pkg/apis/core/types.go b/pkg/apis/core/types.go index 72d4fb6db66..094366379d0 100644 --- a/pkg/apis/core/types.go +++ b/pkg/apis/core/types.go @@ -4945,7 +4945,6 @@ type Secret struct { // Immutable field, if set, ensures that data stored in the Secret cannot // be updated (only object metadata can be modified). - // This is a beta field enabled by ImmutableEphemeralVolumes feature gate. // +optional Immutable *bool @@ -5073,7 +5072,6 @@ type ConfigMap struct { // Immutable field, if set, ensures that data stored in the ConfigMap cannot // be updated (only object metadata can be modified). - // This is a beta field enabled by ImmutableEphemeralVolumes feature gate. // +optional Immutable *bool diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index fedddd75f2a..32b5823f7dd 100644 --- a/pkg/features/kube_features.go +++ b/pkg/features/kube_features.go @@ -595,7 +595,8 @@ const ( // owner: @wojtek-t // alpha: v1.18 - // beta: v1.19 + // beta: v1.19 + // ga: v1.21 // // Enables a feature to make secrets and configmaps data immutable. ImmutableEphemeralVolumes featuregate.Feature = "ImmutableEphemeralVolumes" @@ -817,7 +818,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS DaemonSetUpdateSurge: {Default: false, PreRelease: featuregate.Alpha}, ServiceTopology: {Default: false, PreRelease: featuregate.Alpha}, ServiceAppProtocol: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, - ImmutableEphemeralVolumes: {Default: true, PreRelease: featuregate.Beta}, + ImmutableEphemeralVolumes: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.24 HugePageStorageMediumSize: {Default: true, PreRelease: featuregate.Beta}, DownwardAPIHugePages: {Default: false, PreRelease: featuregate.Alpha}, ExternalPolicyForExternalIP: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.22 diff --git a/pkg/kubelet/util/manager/watch_based_manager.go b/pkg/kubelet/util/manager/watch_based_manager.go index 4dfe1037539..30f25b405bc 100644 --- a/pkg/kubelet/util/manager/watch_based_manager.go +++ b/pkg/kubelet/util/manager/watch_based_manager.go @@ -34,8 +34,6 @@ import ( "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/apimachinery/pkg/watch" - utilfeature "k8s.io/apiserver/pkg/util/feature" - "k8s.io/kubernetes/pkg/features" ) type listObjectFunc func(string, metav1.ListOptions) (runtime.Object, error) @@ -208,7 +206,7 @@ func (c *objectCache) Get(namespace, name string) (runtime.Object, error) { // already have it from here // - doing that would require significant refactoring to reflector // we limit ourselves to just quickly stop the reflector here. - if utilfeature.DefaultFeatureGate.Enabled(features.ImmutableEphemeralVolumes) && c.isImmutable(object) { + if c.isImmutable(object) { if item.stop() { klog.V(4).Infof("Stopped watching for changes of %q/%q - object is immutable", namespace, name) } diff --git a/pkg/kubelet/util/manager/watch_based_manager_test.go b/pkg/kubelet/util/manager/watch_based_manager_test.go index 1f103404a9a..d81c4901106 100644 --- a/pkg/kubelet/util/manager/watch_based_manager_test.go +++ b/pkg/kubelet/util/manager/watch_based_manager_test.go @@ -31,14 +31,11 @@ import ( "k8s.io/apimachinery/pkg/util/wait" "k8s.io/apimachinery/pkg/watch" - utilfeature "k8s.io/apiserver/pkg/util/feature" clientset "k8s.io/client-go/kubernetes" "k8s.io/client-go/kubernetes/fake" core "k8s.io/client-go/testing" - featuregatetesting "k8s.io/component-base/featuregate/testing" corev1 "k8s.io/kubernetes/pkg/apis/core/v1" - "k8s.io/kubernetes/pkg/features" "github.com/stretchr/testify/assert" ) @@ -197,8 +194,6 @@ func TestSecretCacheMultipleRegistrations(t *testing.T) { } func TestImmutableSecretStopsTheReflector(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ImmutableEphemeralVolumes, true)() - secret := func(rv string, immutable bool) *v1.Secret { result := &v1.Secret{ ObjectMeta: metav1.ObjectMeta{ diff --git a/pkg/registry/core/configmap/strategy.go b/pkg/registry/core/configmap/strategy.go index d592c181c0c..12f021ce40c 100644 --- a/pkg/registry/core/configmap/strategy.go +++ b/pkg/registry/core/configmap/strategy.go @@ -28,11 +28,9 @@ import ( "k8s.io/apiserver/pkg/registry/rest" pkgstorage "k8s.io/apiserver/pkg/storage" "k8s.io/apiserver/pkg/storage/names" - utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/kubernetes/pkg/api/legacyscheme" api "k8s.io/kubernetes/pkg/apis/core" "k8s.io/kubernetes/pkg/apis/core/validation" - "k8s.io/kubernetes/pkg/features" ) // strategy implements behavior for ConfigMap objects @@ -86,14 +84,7 @@ func (strategy) ValidateUpdate(ctx context.Context, newObj, oldObj runtime.Objec return validation.ValidateConfigMapUpdate(newCfg, oldCfg) } -func isImmutableInUse(configMap *api.ConfigMap) bool { - return configMap != nil && configMap.Immutable != nil -} - func dropDisabledFields(configMap *api.ConfigMap, oldConfigMap *api.ConfigMap) { - if !utilfeature.DefaultFeatureGate.Enabled(features.ImmutableEphemeralVolumes) && !isImmutableInUse(oldConfigMap) { - configMap.Immutable = nil - } } func (strategy) AllowUnconditionalUpdate() bool { diff --git a/pkg/registry/core/secret/strategy.go b/pkg/registry/core/secret/strategy.go index 0d5908d8975..ce2ea2b4d77 100644 --- a/pkg/registry/core/secret/strategy.go +++ b/pkg/registry/core/secret/strategy.go @@ -29,11 +29,9 @@ import ( "k8s.io/apiserver/pkg/registry/rest" pkgstorage "k8s.io/apiserver/pkg/storage" "k8s.io/apiserver/pkg/storage/names" - utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/kubernetes/pkg/api/legacyscheme" api "k8s.io/kubernetes/pkg/apis/core" "k8s.io/kubernetes/pkg/apis/core/validation" - "k8s.io/kubernetes/pkg/features" ) // strategy implements behavior for Secret objects @@ -80,14 +78,7 @@ func (strategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) fie return validation.ValidateSecretUpdate(obj.(*api.Secret), old.(*api.Secret)) } -func isImmutableInUse(secret *api.Secret) bool { - return secret != nil && secret.Immutable != nil -} - func dropDisabledFields(secret *api.Secret, oldSecret *api.Secret) { - if !utilfeature.DefaultFeatureGate.Enabled(features.ImmutableEphemeralVolumes) && !isImmutableInUse(oldSecret) { - secret.Immutable = nil - } } func (strategy) AllowUnconditionalUpdate() bool { diff --git a/staging/src/k8s.io/api/core/v1/types.go b/staging/src/k8s.io/api/core/v1/types.go index 86e91c987c0..e047885411b 100644 --- a/staging/src/k8s.io/api/core/v1/types.go +++ b/staging/src/k8s.io/api/core/v1/types.go @@ -5742,7 +5742,6 @@ type Secret struct { // be updated (only object metadata can be modified). // If not set to true, the field can be modified at any time. // Defaulted to nil. - // This is a beta field enabled by ImmutableEphemeralVolumes feature gate. // +optional Immutable *bool `json:"immutable,omitempty" protobuf:"varint,5,opt,name=immutable"` @@ -5883,7 +5882,6 @@ type ConfigMap struct { // be updated (only object metadata can be modified). // If not set to true, the field can be modified at any time. // Defaulted to nil. - // This is a beta field enabled by ImmutableEphemeralVolumes feature gate. // +optional Immutable *bool `json:"immutable,omitempty" protobuf:"varint,4,opt,name=immutable"` diff --git a/test/e2e/common/configmap_volume.go b/test/e2e/common/configmap_volume.go index 468960f8487..64700f37686 100644 --- a/test/e2e/common/configmap_volume.go +++ b/test/e2e/common/configmap_volume.go @@ -488,9 +488,16 @@ var _ = ginkgo.Describe("[sig-storage] ConfigMap", func() { }) - // It should be forbidden to change data for configmaps marked as immutable, but - // allowed to modify its metadata independently of its state. - ginkgo.It("should be immutable if `immutable` field is set", func() { + /* + Release: v1.21 + Testname: ConfigMap Volume, immutability + Description: Create a ConfigMap. Update it's data field, the update MUST succeed. + Mark the ConfigMap as immutable, the update MUST succeed. Try to update its data, the update MUST fail. + Try to mark the ConfigMap back as not immutable, the update MUST fail. + Try to update the ConfigMap`s metadata (labels), the update must succeed. + Try to delete the ConfigMap, the deletion must succeed. + */ + framework.ConformanceIt("should be immutable if `immutable` field is set", func() { name := "immutable" configMap := newConfigMap(f, name) diff --git a/test/e2e/common/secrets_volume.go b/test/e2e/common/secrets_volume.go index b6b9e32c16c..4bf10122f11 100644 --- a/test/e2e/common/secrets_volume.go +++ b/test/e2e/common/secrets_volume.go @@ -371,9 +371,16 @@ var _ = ginkgo.Describe("[sig-storage] Secrets", func() { gomega.Eventually(pollDeleteLogs, podLogTimeout, framework.Poll).Should(gomega.ContainSubstring("Error reading file /etc/secret-volumes/delete/data-1")) }) - // It should be forbidden to change data for secrets marked as immutable, but - // allowed to modify its metadata independently of its state. - ginkgo.It("should be immutable if `immutable` field is set", func() { + /* + Release: v1.21 + Testname: Secrets Volume, immutability + Description: Create a secret. Update it's data field, the update MUST succeed. + Mark the secret as immutable, the update MUST succeed. Try to update its data, the update MUST fail. + Try to mark the secret back as not immutable, the update MUST fail. + Try to update the secret`s metadata (labels), the update must succeed. + Try to delete the secret, the deletion must succeed. + */ + framework.ConformanceIt("should be immutable if `immutable` field is set", func() { name := "immutable" secret := secretForTest(f.Namespace.Name, name)