Merge pull request #97615 from wojtek-t/immutable_secrets_ga

Promote Immutable Secrets/ConfigMaps to Stable

api/openapi-spec/swagger.json

@@ -5661,7 +5661,7 @@
           "type": "object"
         },
         "immutable": {
-          "description": "Immutable, if set to true, ensures that data stored in the ConfigMap cannot 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.",
+          "description": "Immutable, if set to true, ensures that data stored in the ConfigMap cannot be updated (only object metadata can be modified). If not set to true, the field can be modified at any time. Defaulted to nil.",
           "type": "boolean"
         },
         "kind": {
@@ -9900,7 +9900,7 @@
           "type": "object"
         },
         "immutable": {
-          "description": "Immutable, if set to true, ensures that data stored in the Secret cannot 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.",
+          "description": "Immutable, if set to true, ensures that data stored in the Secret cannot be updated (only object metadata can be modified). If not set to true, the field can be modified at any time. Defaulted to nil.",
           "type": "boolean"
         },
         "kind": {

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
 

pkg/features/kube_features.go

@@ -559,7 +559,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"
@@ -776,7 +777,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

pkg/kubelet/util/manager/BUILD

@@ -10,7 +10,6 @@ go_library(
     importpath = "k8s.io/kubernetes/pkg/kubelet/util/manager",
     visibility = ["//visibility:public"],
     deps = [
-        "//pkg/features:go_default_library",
         "//pkg/kubelet/util:go_default_library",
         "//staging/src/k8s.io/api/core/v1:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library",
@@ -23,7 +22,6 @@ go_library(
         "//staging/src/k8s.io/apimachinery/pkg/util/wait:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/watch:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/storage/etcd3:go_default_library",
-        "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library",
         "//staging/src/k8s.io/client-go/tools/cache:go_default_library",
         "//vendor/k8s.io/klog/v2:go_default_library",
     ],
@@ -39,7 +37,6 @@ go_test(
     deps = [
         "//pkg/api/v1/pod:go_default_library",
         "//pkg/apis/core/v1:go_default_library",
-        "//pkg/features:go_default_library",
         "//staging/src/k8s.io/api/core/v1:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/api/equality:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library",
@@ -49,11 +46,9 @@ go_test(
         "//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/util/wait:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/watch:go_default_library",
-        "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library",
         "//staging/src/k8s.io/client-go/kubernetes:go_default_library",
         "//staging/src/k8s.io/client-go/kubernetes/fake:go_default_library",
         "//staging/src/k8s.io/client-go/testing:go_default_library",
-        "//staging/src/k8s.io/component-base/featuregate/testing:go_default_library",
         "//vendor/github.com/stretchr/testify/assert:go_default_library",
     ],
 )

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)
 			}

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{

pkg/registry/core/configmap/BUILD

@@ -17,7 +17,6 @@ go_library(
         "//pkg/api/legacyscheme:go_default_library",
         "//pkg/apis/core:go_default_library",
         "//pkg/apis/core/validation:go_default_library",
-        "//pkg/features:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/fields:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/labels:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library",
@@ -26,7 +25,6 @@ go_library(
         "//staging/src/k8s.io/apiserver/pkg/registry/rest:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/storage:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/storage/names:go_default_library",
-        "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library",
     ],
 )
 

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 {

pkg/registry/core/secret/BUILD

@@ -17,7 +17,6 @@ go_library(
         "//pkg/api/legacyscheme:go_default_library",
         "//pkg/apis/core:go_default_library",
         "//pkg/apis/core/validation:go_default_library",
-        "//pkg/features:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/fields:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/labels:go_default_library",
@@ -27,7 +26,6 @@ go_library(
         "//staging/src/k8s.io/apiserver/pkg/registry/rest:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/storage:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/storage/names:go_default_library",
-        "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library",
     ],
 )
 

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 {

staging/src/k8s.io/api/core/v1/generated.proto

@@ -461,7 +461,6 @@ message ConfigMap {
   // 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
   optional bool immutable = 4;
 
@@ -4419,7 +4418,6 @@ message Secret {
   // 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
   optional bool immutable = 5;
 

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"`
 

staging/src/k8s.io/api/core/v1/types_swagger_doc_generated.go

@@ -252,7 +252,7 @@ func (ComponentStatusList) SwaggerDoc() map[string]string {
 var map_ConfigMap = map[string]string{
 	"":           "ConfigMap holds configuration data for pods to consume.",
 	"metadata":   "Standard object's metadata. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#metadata",
-	"immutable":  "Immutable, if set to true, ensures that data stored in the ConfigMap cannot 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.",
+	"immutable":  "Immutable, if set to true, ensures that data stored in the ConfigMap cannot be updated (only object metadata can be modified). If not set to true, the field can be modified at any time. Defaulted to nil.",
 	"data":       "Data contains the configuration data. Each key must consist of alphanumeric characters, '-', '_' or '.'. Values with non-UTF-8 byte sequences must use the BinaryData field. The keys stored in Data must not overlap with the keys in the BinaryData field, this is enforced during validation process.",
 	"binaryData": "BinaryData contains the binary data. Each key must consist of alphanumeric characters, '-', '_' or '.'. BinaryData can contain byte sequences that are not in the UTF-8 range. The keys stored in BinaryData must not overlap with the ones in the Data field, this is enforced during validation process. Using this field will require 1.10+ apiserver and kubelet.",
 }
@@ -2060,7 +2060,7 @@ func (SeccompProfile) SwaggerDoc() map[string]string {
 var map_Secret = map[string]string{
 	"":           "Secret holds secret data of a certain type. The total bytes of the values in the Data field must be less than MaxSecretSize bytes.",
 	"metadata":   "Standard object's metadata. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#metadata",
-	"immutable":  "Immutable, if set to true, ensures that data stored in the Secret cannot 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.",
+	"immutable":  "Immutable, if set to true, ensures that data stored in the Secret cannot be updated (only object metadata can be modified). If not set to true, the field can be modified at any time. Defaulted to nil.",
 	"data":       "Data contains the secret data. Each key must consist of alphanumeric characters, '-', '_' or '.'. The serialized form of the secret data is a base64 encoded string, representing the arbitrary (possibly non-string) data value here. Described in https://tools.ietf.org/html/rfc4648#section-4",
 	"stringData": "stringData allows specifying non-binary secret data in string form. It is provided as a write-only convenience method. All keys and values are merged into the data field on write, overwriting any existing values. It is never output when reading from the API.",
 	"type":       "Used to facilitate programmatic handling of secret data.",

test/conformance/testdata/conformance.yaml

@@ -2162,6 +2162,16 @@
     The content MUST be accessible from all the mapped volume mounts.
   release: v1.9
   file: test/e2e/common/configmap_volume.go
+- testname: ConfigMap Volume, immutability
+  codename: '[sig-storage] ConfigMap should be immutable if `immutable` field is set
+    [Conformance]'
+  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.
+  release: v1.21
+  file: test/e2e/common/configmap_volume.go
 - testname: ConfigMap Volume, update
   codename: '[sig-storage] ConfigMap updates should be reflected in volume [NodeConformance]
     [Conformance]'
@@ -2771,6 +2781,16 @@
     the secret from the both the mounted volumes from the two specified custom paths.
   release: v1.9
   file: test/e2e/common/secrets_volume.go
+- testname: Secrets Volume, immutability
+  codename: '[sig-storage] Secrets should be immutable if `immutable` field is set
+    [Conformance]'
+  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.
+  release: v1.21
+  file: test/e2e/common/secrets_volume.go
 - testname: 'SubPath: Reading content from a configmap volume.'
   codename: '[sig-storage] Subpath Atomic writer volumes should support subpaths with
     configmap pod [LinuxOnly] [Conformance]'

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.
+		Release: v1.21
-	ginkgo.It("should be immutable if `immutable` field is set", func() {
+		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)
 

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.
+		Release: v1.21
-	ginkgo.It("should be immutable if `immutable` field is set", func() {
+		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)