graduate CSIServiceAccountToken to beta

pkg/apis/storage/types.go

@@ -274,6 +274,9 @@ type CSIDriverSpec struct {
 	// If the CSIDriverRegistry feature gate is enabled and the value is
 	// specified to false, the attach operation will be skipped.
 	// Otherwise the attach operation will be called.
+	//
+	// This field is immutable.
+	//
 	// +optional
 	AttachRequired *bool
 
@@ -282,6 +285,9 @@ type CSIDriverSpec struct {
 	// Refer to the specific FSGroupPolicy values for additional details.
 	// This field is alpha-level, and is only honored by servers
 	// that enable the CSIVolumeFSGroupPolicy feature gate.
+	//
+	// This field is immutable.
+	//
 	// +optional
 	FSGroupPolicy *FSGroupPolicy
 
@@ -309,6 +315,9 @@ type CSIDriverSpec struct {
 	// As Kubernetes 1.15 doesn't support this field, drivers can only support one mode when
 	// deployed on such a cluster and the deployment determines which mode that is, for example
 	// via a command line parameter of the driver.
+	//
+	// This field is immutable.
+	//
 	// +optional
 	PodInfoOnMount *bool
 
@@ -324,6 +333,9 @@ type CSIDriverSpec struct {
 	// https://kubernetes-csi.github.io/docs/ephemeral-local-volumes.html
 	// A driver can support one or more of these mode and
 	// more modes may be added in the future.
+	//
+	// This field is immutable.
+	//
 	// +optional
 	VolumeLifecycleModes []VolumeLifecycleMode
 
@@ -341,6 +353,8 @@ type CSIDriverSpec struct {
 	// unset or false and it can be flipped later when storage
 	// capacity information has been published.
 	//
+	// This field is immutable.
+	//
 	// This is a beta field and only available when the CSIStorageCapacity
 	// feature is enabled. The default is false.
 	//
@@ -363,7 +377,7 @@ type CSIDriverSpec struct {
 	// most one token is empty string. To receive a new token after expiry,
 	// RequiresRepublish can be used to trigger NodePublishVolume periodically.
 	//
-	// This is an alpha feature and only available when the
+	// This is a beta feature and only available when the
 	// CSIServiceAccountToken feature is enabled.
 	//
 	// +optional
@@ -378,7 +392,7 @@ type CSIDriverSpec struct {
 	// to NodePublishVolume should only update the contents of the volume. New
 	// mount points will not be seen by a running container.
 	//
-	// This is an alpha feature and only available when the
+	// This is a beta feature and only available when the
 	// CSIServiceAccountToken feature is enabled.
 	//
 	// +optional

pkg/apis/storage/validation/validation.go

@@ -425,11 +425,14 @@ func ValidateCSIDriver(csiDriver *storage.CSIDriver) field.ErrorList {
 func ValidateCSIDriverUpdate(new, old *storage.CSIDriver) field.ErrorList {
 	allErrs := apivalidation.ValidateObjectMetaUpdate(&new.ObjectMeta, &old.ObjectMeta, field.NewPath("metadata"))
 
-	// Spec is read-only
-	// If this ever relaxes in the future, make sure to increment the Generation number in PrepareForUpdate
-	if !apiequality.Semantic.DeepEqual(old.Spec, new.Spec) {
-		allErrs = append(allErrs, field.Invalid(field.NewPath("spec"), new.Spec, "field is immutable"))
-	}
+	// immutable fields should not be mutated.
+	allErrs = append(allErrs, apimachineryvalidation.ValidateImmutableField(new.Spec.AttachRequired, old.Spec.AttachRequired, field.NewPath("spec", "attachedRequired"))...)
+	allErrs = append(allErrs, apimachineryvalidation.ValidateImmutableField(new.Spec.FSGroupPolicy, old.Spec.FSGroupPolicy, field.NewPath("spec", "fsGroupPolicy"))...)
+	allErrs = append(allErrs, apimachineryvalidation.ValidateImmutableField(new.Spec.PodInfoOnMount, old.Spec.PodInfoOnMount, field.NewPath("spec", "podInfoOnMount"))...)
+	allErrs = append(allErrs, apimachineryvalidation.ValidateImmutableField(new.Spec.VolumeLifecycleModes, old.Spec.VolumeLifecycleModes, field.NewPath("spec", "volumeLifecycleModes"))...)
+	allErrs = append(allErrs, apimachineryvalidation.ValidateImmutableField(new.Spec.StorageCapacity, old.Spec.StorageCapacity, field.NewPath("spec", "storageCapacity"))...)
+
+	allErrs = append(allErrs, validateTokenRequests(new.Spec.TokenRequests, field.NewPath("spec", "tokenRequests"))...)
 	return allErrs
 }
 

pkg/apis/storage/validation/validation_test.go

@@ -1927,11 +1927,11 @@ func TestCSIDriverValidationUpdate(t *testing.T) {
 	podInfoOnMount := true
 	storageCapacity := true
 	notPodInfoOnMount := false
+	gcp := "gcp"
+	requiresRepublish := true
 	notRequiresRepublish := false
 	notStorageCapacity := false
 	resourceVersion := "1"
-	invalidFSGroupPolicy := storage.ReadWriteOnceWithFSTypeFSGroupPolicy
-	invalidFSGroupPolicy = "invalid-mode"
 	old := storage.CSIDriver{
 		ObjectMeta: metav1.ObjectMeta{Name: driverName, ResourceVersion: resourceVersion},
 		Spec: storage.CSIDriverSpec{
@@ -1946,30 +1946,35 @@ func TestCSIDriverValidationUpdate(t *testing.T) {
 		},
 	}
 
-	// Currently we compare the object against itself
-	// and ensure updates succeed
-	successCases := []storage.CSIDriver{
-		old,
-		// An invalid FSGroupPolicy should still pass
+	successCases := []struct {
+		name   string
+		modify func(new *storage.CSIDriver)
+	}{
 		{
-			ObjectMeta: metav1.ObjectMeta{Name: driverName, ResourceVersion: resourceVersion},
-			Spec: storage.CSIDriverSpec{
-				AttachRequired: &attachNotRequired,
-				PodInfoOnMount: &notPodInfoOnMount,
-				VolumeLifecycleModes: []storage.VolumeLifecycleMode{
-					storage.VolumeLifecycleEphemeral,
-					storage.VolumeLifecyclePersistent,
-				},
-				FSGroupPolicy:   &invalidFSGroupPolicy,
-				StorageCapacity: &storageCapacity,
+			name:   "no change",
+			modify: func(new *storage.CSIDriver) {},
+		},
+		{
+			name: "change TokenRequests",
+			modify: func(new *storage.CSIDriver) {
+				new.Spec.TokenRequests = []storage.TokenRequest{{Audience: gcp}}
+			},
+		},
+		{
+			name: "change RequiresRepublish",
+			modify: func(new *storage.CSIDriver) {
+				new.Spec.RequiresRepublish = &requiresRepublish
 			},
 		},
 	}
-	for _, csiDriver := range successCases {
-		newDriver := csiDriver.DeepCopy()
-		if errs := ValidateCSIDriverUpdate(&csiDriver, newDriver); len(errs) != 0 {
-			t.Errorf("expected success for %+v: %v", csiDriver, errs)
-		}
+	for _, test := range successCases {
+		t.Run(test.name, func(t *testing.T) {
+			new := old.DeepCopy()
+			test.modify(new)
+			if errs := ValidateCSIDriverUpdate(new, &old); len(errs) != 0 {
+				t.Errorf("Expected success for %+v: %v", new, errs)
+			}
+		})
 	}
 
 	// Each test case changes exactly one field. None of that is valid.
@@ -1996,15 +2001,15 @@ func TestCSIDriverValidationUpdate(t *testing.T) {
 			},
 		},
 		{
-			name: "PodInfoOnMount not set",
+			name: "AttachRequired changed",
 			modify: func(new *storage.CSIDriver) {
-				new.Spec.PodInfoOnMount = nil
+				new.Spec.AttachRequired = &attachRequired
 			},
 		},
 		{
-			name: "AttachRequired changed",
+			name: "PodInfoOnMount not set",
 			modify: func(new *storage.CSIDriver) {
-				new.Spec.AttachRequired = &attachRequired
+				new.Spec.PodInfoOnMount = nil
 			},
 		},
 		{
@@ -2064,14 +2069,20 @@ func TestCSIDriverValidationUpdate(t *testing.T) {
 				new.Spec.StorageCapacity = &notStorageCapacity
 			},
 		},
+		{
+			name: "TokenRequests invalidated",
+			modify: func(new *storage.CSIDriver) {
+				new.Spec.TokenRequests = []storage.TokenRequest{{Audience: gcp}, {Audience: gcp}}
+			},
+		},
 	}
 
 	for _, test := range errorCases {
 		t.Run(test.name, func(t *testing.T) {
 			new := old.DeepCopy()
 			test.modify(new)
 			if errs := ValidateCSIDriverUpdate(new, &old); len(errs) == 0 {
-				t.Errorf("Expected failure for test: %v", new)
+				t.Errorf("Expected failure for test: %+v", new)
 			}
 		})
 	}
@@ -2253,7 +2264,7 @@ func TestCSIServiceAccountToken(t *testing.T) {
 			wantErr: true,
 		},
 		{
-			desc: "invalid - TokenRequests has tokens with ExpirationSeconds less than 10min",
+			desc: "invalid - TokenRequests has tokens with ExpirationSeconds longer than 1<<32 min",
 			csiDriver: &storage.CSIDriver{
 				ObjectMeta: metav1.ObjectMeta{Name: driverName},
 				Spec: storage.CSIDriverSpec{

pkg/features/kube_features.go

@@ -656,6 +656,7 @@ const (
 
 	// owner: @zshihang
 	// alpha: v1.20
+	// beta: v1.21
 	//
 	// Enable kubelet to pass pod's service account token to NodePublishVolume
 	// call of CSI driver which is mounting volumes for that pod.
@@ -801,7 +802,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS
 	BalanceAttachedNodeVolumes:                     {Default: false, PreRelease: featuregate.Alpha},
 	CSIInlineVolume:                                {Default: true, PreRelease: featuregate.Beta},
 	CSIStorageCapacity:                             {Default: true, PreRelease: featuregate.Beta},
-	CSIServiceAccountToken:                         {Default: false, PreRelease: featuregate.Alpha},
+	CSIServiceAccountToken:                         {Default: true, PreRelease: featuregate.Beta},
 	GenericEphemeralVolume:                         {Default: true, PreRelease: featuregate.Beta},
 	CSIVolumeFSGroupPolicy:                         {Default: true, PreRelease: featuregate.Beta},
 	RuntimeClass:                                   {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.23

pkg/registry/storage/csidriver/strategy.go

@@ -19,6 +19,7 @@ package csidriver
 import (
 	"context"
 
+	apiequality "k8s.io/apimachinery/pkg/api/equality"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	"k8s.io/apiserver/pkg/storage/names"
@@ -64,10 +65,7 @@ func (csiDriverStrategy) PrepareForCreate(ctx context.Context, obj runtime.Objec
 func (csiDriverStrategy) Validate(ctx context.Context, obj runtime.Object) field.ErrorList {
 	csiDriver := obj.(*storage.CSIDriver)
 
-	errs := validation.ValidateCSIDriver(csiDriver)
-	errs = append(errs, validation.ValidateCSIDriver(csiDriver)...)
-
-	return errs
+	return validation.ValidateCSIDriver(csiDriver)
 }
 
 // Canonicalize normalizes the object after validation.
@@ -82,31 +80,33 @@ func (csiDriverStrategy) AllowCreateOnUpdate() bool {
 // existing object does not already have that field set. This allows the field to remain when
 // downgrading to a version that has the feature disabled.
 func (csiDriverStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Object) {
-	if old.(*storage.CSIDriver).Spec.StorageCapacity == nil &&
+	newCSIDriver := obj.(*storage.CSIDriver)
+	oldCSIDriver := old.(*storage.CSIDriver)
+
+	if oldCSIDriver.Spec.StorageCapacity == nil &&
 		!utilfeature.DefaultFeatureGate.Enabled(features.CSIStorageCapacity) {
-		newCSIDriver := obj.(*storage.CSIDriver)
 		newCSIDriver.Spec.StorageCapacity = nil
 	}
-	if old.(*storage.CSIDriver).Spec.VolumeLifecycleModes == nil &&
+	if oldCSIDriver.Spec.VolumeLifecycleModes == nil &&
 		!utilfeature.DefaultFeatureGate.Enabled(features.CSIInlineVolume) {
-		newCSIDriver := obj.(*storage.CSIDriver)
 		newCSIDriver.Spec.VolumeLifecycleModes = nil
 	}
-	if old.(*storage.CSIDriver).Spec.FSGroupPolicy == nil &&
+	if oldCSIDriver.Spec.FSGroupPolicy == nil &&
 		!utilfeature.DefaultFeatureGate.Enabled(features.CSIVolumeFSGroupPolicy) {
-		newCSIDriver := obj.(*storage.CSIDriver)
 		newCSIDriver.Spec.FSGroupPolicy = nil
 	}
-	if old.(*storage.CSIDriver).Spec.TokenRequests == nil &&
+	if oldCSIDriver.Spec.TokenRequests == nil &&
 		!utilfeature.DefaultFeatureGate.Enabled(features.CSIServiceAccountToken) {
-		csiDriver := obj.(*storage.CSIDriver)
-		csiDriver.Spec.TokenRequests = nil
+		newCSIDriver.Spec.TokenRequests = nil
 	}
-
-	if old.(*storage.CSIDriver).Spec.RequiresRepublish == nil &&
+	if oldCSIDriver.Spec.RequiresRepublish == nil &&
 		!utilfeature.DefaultFeatureGate.Enabled(features.CSIServiceAccountToken) {
-		csiDriver := obj.(*storage.CSIDriver)
-		csiDriver.Spec.RequiresRepublish = nil
+		newCSIDriver.Spec.RequiresRepublish = nil
+	}
+
+	// Any changes to the mutable fields increment the generation number.
+	if !apiequality.Semantic.DeepEqual(oldCSIDriver.Spec.TokenRequests, newCSIDriver.Spec.TokenRequests) || !apiequality.Semantic.DeepEqual(oldCSIDriver.Spec.RequiresRepublish, newCSIDriver.Spec.RequiresRepublish) {
+		newCSIDriver.Generation = oldCSIDriver.Generation + 1
 	}
 }
 

pkg/registry/storage/csidriver/strategy_test.go

@@ -275,6 +275,7 @@ func TestCSIDriverPrepareForUpdate(t *testing.T) {
 		wantModes                     []storage.VolumeLifecycleMode
 		wantTokenRequests             []storage.TokenRequest
 		wantRequiresRepublish         *bool
+		wantGeneration                int64
 	}{
 		{
 			name:                      "capacity feature enabled, before: none, update: enabled",
@@ -321,6 +322,7 @@ func TestCSIDriverPrepareForUpdate(t *testing.T) {
 			update:                        driverWithServiceAccountTokenGCP,
 			wantTokenRequests:             []storage.TokenRequest{{Audience: gcp}},
 			wantRequiresRepublish:         &enabled,
+			wantGeneration:                1,
 		},
 		{
 			name:                  "service account token feature disabled, before: none, update: audience=gcp",
@@ -335,6 +337,7 @@ func TestCSIDriverPrepareForUpdate(t *testing.T) {
 			update:                driverWithServiceAccountTokenGCP,
 			wantTokenRequests:     []storage.TokenRequest{{Audience: gcp}},
 			wantRequiresRepublish: &enabled,
+			wantGeneration:        1,
 		},
 	}
 
@@ -346,6 +349,7 @@ func TestCSIDriverPrepareForUpdate(t *testing.T) {
 
 			csiDriver := test.update.DeepCopy()
 			Strategy.PrepareForUpdate(ctx, csiDriver, test.old)
+			require.Equal(t, test.wantGeneration, csiDriver.GetGeneration())
 			require.Equal(t, test.wantCapacity, csiDriver.Spec.StorageCapacity)
 			require.Equal(t, test.wantModes, csiDriver.Spec.VolumeLifecycleModes)
 			require.Equal(t, test.wantTokenRequests, csiDriver.Spec.TokenRequests)