Update APIs and adjust tests

Signed-off-by: zhucan <zhucan.k8s@gmail.com>
Signed-off-by: Humble Chirammal <humble.devassy@gmail.com>

api/openapi-spec/v3/api__v1_openapi.json

@@ -446,7 +446,7 @@
                 "$ref": "#/components/schemas/io.k8s.api.core.v1.SecretReference"
               }
             ],
-            "description": "nodeExpandSecretRef is a reference to the secret object containing sensitive information to pass to the CSI driver to complete the CSI NodeExpandVolume call. This is a beta field which is enabled default by CSINodeExpandSecret feature gate. This field is optional, may be omitted if no secret is required. If the secret object contains more than one secret, all secrets are passed."
+            "description": "nodeExpandSecretRef is a reference to the secret object containing sensitive information to pass to the CSI driver to complete the CSI NodeExpandVolume call. This field is optional, may be omitted if no secret is required. If the secret object contains more than one secret, all secrets are passed."
           },
           "nodePublishSecretRef": {
             "allOf": [

api/openapi-spec/v3/apis__storage.k8s.io__v1_openapi.json

@@ -126,7 +126,7 @@
                 "$ref": "#/components/schemas/io.k8s.api.core.v1.SecretReference"
               }
             ],
-            "description": "nodeExpandSecretRef is a reference to the secret object containing sensitive information to pass to the CSI driver to complete the CSI NodeExpandVolume call. This is a beta field which is enabled default by CSINodeExpandSecret feature gate. This field is optional, may be omitted if no secret is required. If the secret object contains more than one secret, all secrets are passed."
+            "description": "nodeExpandSecretRef is a reference to the secret object containing sensitive information to pass to the CSI driver to complete the CSI NodeExpandVolume call. This field is optional, may be omitted if no secret is required. If the secret object contains more than one secret, all secrets are passed."
           },
           "nodePublishSecretRef": {
             "allOf": [

pkg/api/persistentvolume/util.go

@@ -34,11 +34,6 @@ const (
 // DropDisabledSpecFields removes disabled fields from the pv spec.
 // This should be called from PrepareForCreate/PrepareForUpdate for all resources containing a pv spec.
 func DropDisabledSpecFields(pvSpec *api.PersistentVolumeSpec, oldPVSpec *api.PersistentVolumeSpec) {
-	if !utilfeature.DefaultFeatureGate.Enabled(features.CSINodeExpandSecret) && !hasNodeExpansionSecrets(oldPVSpec) {
-		if pvSpec.CSI != nil {
-			pvSpec.CSI.NodeExpandSecretRef = nil
-		}
-	}
 	if !utilfeature.DefaultFeatureGate.Enabled(features.VolumeAttributesClass) {
 		if oldPVSpec == nil || oldPVSpec.VolumeAttributesClassName == nil {
 			pvSpec.VolumeAttributesClassName = nil
@@ -54,17 +49,6 @@ func DropDisabledStatusFields(oldStatus, newStatus *api.PersistentVolumeStatus)
 	}
 }
 
-func hasNodeExpansionSecrets(oldPVSpec *api.PersistentVolumeSpec) bool {
-	if oldPVSpec == nil || oldPVSpec.CSI == nil {
-		return false
-	}
-
-	if oldPVSpec.CSI.NodeExpandSecretRef != nil {
-		return true
-	}
-	return false
-}
-
 func GetWarningsForPersistentVolume(pv *api.PersistentVolume) []string {
 	if pv == nil {
 		return nil

pkg/api/persistentvolume/util_test.go

@@ -32,62 +32,15 @@ import (
 )
 
 func TestDropDisabledFields(t *testing.T) {
-	secretRef := &api.SecretReference{
-		Name:      "expansion-secret",
-		Namespace: "default",
-	}
 	vacName := ptr.To("vac")
 
 	tests := map[string]struct {
-		oldSpec             *api.PersistentVolumeSpec
-		newSpec             *api.PersistentVolumeSpec
-		expectOldSpec       *api.PersistentVolumeSpec
-		expectNewSpec       *api.PersistentVolumeSpec
-		csiExpansionEnabled bool
-		vacEnabled          bool
+		oldSpec       *api.PersistentVolumeSpec
+		newSpec       *api.PersistentVolumeSpec
+		expectOldSpec *api.PersistentVolumeSpec
+		expectNewSpec *api.PersistentVolumeSpec
+		vacEnabled    bool
 	}{
-		"disabled csi expansion clears secrets": {
-			csiExpansionEnabled: false,
-			newSpec:             specWithCSISecrets(secretRef),
-			expectNewSpec:       specWithCSISecrets(nil),
-			oldSpec:             nil,
-			expectOldSpec:       nil,
-		},
-		"enabled csi expansion preserve secrets": {
-			csiExpansionEnabled: true,
-			newSpec:             specWithCSISecrets(secretRef),
-			expectNewSpec:       specWithCSISecrets(secretRef),
-			oldSpec:             nil,
-			expectOldSpec:       nil,
-		},
-		"enabled csi expansion preserve secrets when both old and new have it": {
-			csiExpansionEnabled: true,
-			newSpec:             specWithCSISecrets(secretRef),
-			expectNewSpec:       specWithCSISecrets(secretRef),
-			oldSpec:             specWithCSISecrets(secretRef),
-			expectOldSpec:       specWithCSISecrets(secretRef),
-		},
-		"disabled csi expansion old pv had secrets": {
-			csiExpansionEnabled: false,
-			newSpec:             specWithCSISecrets(secretRef),
-			expectNewSpec:       specWithCSISecrets(secretRef),
-			oldSpec:             specWithCSISecrets(secretRef),
-			expectOldSpec:       specWithCSISecrets(secretRef),
-		},
-		"enabled csi expansion preserves secrets when old pv did not had secrets": {
-			csiExpansionEnabled: true,
-			newSpec:             specWithCSISecrets(secretRef),
-			expectNewSpec:       specWithCSISecrets(secretRef),
-			oldSpec:             specWithCSISecrets(nil),
-			expectOldSpec:       specWithCSISecrets(nil),
-		},
-		"disabled csi expansion neither new pv nor old pv had secrets": {
-			csiExpansionEnabled: false,
-			newSpec:             specWithCSISecrets(nil),
-			expectNewSpec:       specWithCSISecrets(nil),
-			oldSpec:             specWithCSISecrets(nil),
-			expectOldSpec:       specWithCSISecrets(nil),
-		},
 		"disabled vac clears volume attributes class name": {
 			vacEnabled:    false,
 			newSpec:       specWithVACName(vacName),
@@ -134,7 +87,6 @@ func TestDropDisabledFields(t *testing.T) {
 
 	for name, tc := range tests {
 		t.Run(name, func(t *testing.T) {
-			defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSINodeExpandSecret, tc.csiExpansionEnabled)()
 			defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.VolumeAttributesClass, tc.vacEnabled)()
 
 			DropDisabledSpecFields(tc.newSpec, tc.oldSpec)
@@ -148,22 +100,6 @@ func TestDropDisabledFields(t *testing.T) {
 	}
 }
 
-func specWithCSISecrets(secret *api.SecretReference) *api.PersistentVolumeSpec {
-	pvSpec := &api.PersistentVolumeSpec{
-		PersistentVolumeSource: api.PersistentVolumeSource{
-			CSI: &api.CSIPersistentVolumeSource{
-				Driver:       "com.google.gcepd",
-				VolumeHandle: "foobar",
-			},
-		},
-	}
-
-	if secret != nil {
-		pvSpec.CSI.NodeExpandSecretRef = secret
-	}
-	return pvSpec
-}
-
 func specWithVACName(vacName *string) *api.PersistentVolumeSpec {
 	pvSpec := &api.PersistentVolumeSpec{
 		PersistentVolumeSource: api.PersistentVolumeSource{

pkg/apis/core/types.go

@@ -1879,10 +1879,8 @@ type CSIPersistentVolumeSource struct {
 	// NodeExpandSecretRef is a reference to the secret object containing
 	// sensitive information to pass to the CSI driver to complete the CSI
 	// NodeExpandVolume call.
-	// This is a beta field which is enabled default by CSINodeExpandSecret feature gate.
 	// This field is optional, may be omitted if no secret is required. If the
 	// secret object contains more than one secret, all secrets are passed.
-	// +featureGate=CSINodeExpandSecret
 	// +optional
 	NodeExpandSecretRef *SecretReference
 }

pkg/features/kube_features.go

@@ -163,7 +163,7 @@ const (
 	// kep: https://kep.k8s.io/3171
 	// alpha: v1.25
 	// beta: v1.27
-	// GA: 1.29
+	// GA: v1.29
 	// Enables SecretRef field in CSI NodeExpandVolume request.
 	CSINodeExpandSecret featuregate.Feature = "CSINodeExpandSecret"
 
@@ -1006,9 +1006,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS
 
 	CSIMigrationRBD: {Default: false, PreRelease: featuregate.Deprecated}, //  deprecated in 1.28, remove in 1.31
 
-	CSIMigrationvSphere: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.29
-
-	CSINodeExpandSecret: {Default: true, PreRelease: featuregate.GA},
+	CSINodeExpandSecret: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.31
 
 	CSIVolumeHealth: {Default: false, PreRelease: featuregate.Alpha},
 

pkg/registry/core/persistentvolume/strategy.go

@@ -19,6 +19,7 @@ package persistentvolume
 import (
 	"context"
 	"fmt"
+
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	utilfeature "k8s.io/apiserver/pkg/util/feature"
 	"k8s.io/kubernetes/pkg/features"
@@ -74,8 +75,6 @@ func (persistentvolumeStrategy) PrepareForCreate(ctx context.Context, obj runtim
 		now := NowFunc()
 		pv.Status.LastPhaseTransitionTime = &now
 	}
-
-	pvutil.DropDisabledSpecFields(&pv.Spec, nil)
 }
 
 func (persistentvolumeStrategy) Validate(ctx context.Context, obj runtime.Object) field.ErrorList {
@@ -103,8 +102,6 @@ func (persistentvolumeStrategy) PrepareForUpdate(ctx context.Context, obj, old r
 	newPv := obj.(*api.PersistentVolume)
 	oldPv := old.(*api.PersistentVolume)
 	newPv.Status = oldPv.Status
-
-	pvutil.DropDisabledSpecFields(&newPv.Spec, &oldPv.Spec)
 }
 
 func (persistentvolumeStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) field.ErrorList {

pkg/volume/csi/expander.go

@@ -23,9 +23,7 @@ import (
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
 	api "k8s.io/api/core/v1"
-	utilfeature "k8s.io/apiserver/pkg/util/feature"
 	"k8s.io/klog/v2"
-	"k8s.io/kubernetes/pkg/features"
 	"k8s.io/kubernetes/pkg/volume"
 	"k8s.io/kubernetes/pkg/volume/util"
 	volumetypes "k8s.io/kubernetes/pkg/volume/util/types"
@@ -83,13 +81,12 @@ func (c *csiPlugin) nodeExpandWithClient(
 	}
 	nodeExpandSecrets := map[string]string{}
 	expandClient := c.host.GetKubeClient()
-	if utilfeature.DefaultFeatureGate.Enabled(features.CSINodeExpandSecret) {
-		if csiSource.NodeExpandSecretRef != nil {
-			nodeExpandSecrets, err = getCredentialsFromSecret(expandClient, csiSource.NodeExpandSecretRef)
-			if err != nil {
-				return false, fmt.Errorf("expander.NodeExpand failed to get NodeExpandSecretRef %s/%s: %v",
-					csiSource.NodeExpandSecretRef.Namespace, csiSource.NodeExpandSecretRef.Name, err)
-			}
+
+	if csiSource.NodeExpandSecretRef != nil {
+		nodeExpandSecrets, err = getCredentialsFromSecret(expandClient, csiSource.NodeExpandSecretRef)
+		if err != nil {
+			return false, fmt.Errorf("expander.NodeExpand failed to get NodeExpandSecretRef %s/%s: %v",
+				csiSource.NodeExpandSecretRef.Namespace, csiSource.NodeExpandSecretRef.Name, err)
 		}
 	}
 

pkg/volume/csi/expander_test.go

@@ -27,9 +27,6 @@ import (
 	api "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	utilfeature "k8s.io/apiserver/pkg/util/feature"
-	featuregatetesting "k8s.io/component-base/featuregate/testing"
-	"k8s.io/kubernetes/pkg/features"
 	"k8s.io/kubernetes/pkg/volume"
 	volumetypes "k8s.io/kubernetes/pkg/volume/util/types"
 )
@@ -118,7 +115,6 @@ func TestNodeExpand(t *testing.T) {
 	}
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
-			defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSINodeExpandSecret, tc.enableCSINodeExpandSecret)()
 			plug, tmpDir := newTestPlugin(t, nil)
 			defer os.RemoveAll(tmpDir)
 

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

@@ -1968,10 +1968,8 @@ type CSIPersistentVolumeSource struct {
 	// nodeExpandSecretRef is a reference to the secret object containing
 	// sensitive information to pass to the CSI driver to complete the CSI
 	// NodeExpandVolume call.
-	// This is a beta field which is enabled default by CSINodeExpandSecret feature gate.
 	// This field is optional, may be omitted if no secret is required. If the
 	// secret object contains more than one secret, all secrets are passed.
-	// +featureGate=CSINodeExpandSecret
 	// +optional
 	NodeExpandSecretRef *SecretReference `json:"nodeExpandSecretRef,omitempty" protobuf:"bytes,10,opt,name=nodeExpandSecretRef"`
 }