From 38905462656a490280ba9955477c11376c7ca4aa Mon Sep 17 00:00:00 2001 From: Humble Chirammal Date: Tue, 31 Oct 2023 09:07:06 +0800 Subject: [PATCH] Update APIs and adjust tests Signed-off-by: zhucan Signed-off-by: Humble Chirammal --- api/openapi-spec/swagger.json | 2 +- api/openapi-spec/v3/api__v1_openapi.json | 2 +- .../v3/apis__storage.k8s.io__v1_openapi.json | 2 +- pkg/api/persistentvolume/util.go | 16 ---- pkg/api/persistentvolume/util_test.go | 74 ++----------------- pkg/apis/core/types.go | 2 - pkg/features/kube_features.go | 6 +- pkg/generated/openapi/zz_generated.openapi.go | 2 +- .../core/persistentvolume/strategy.go | 5 +- pkg/volume/csi/expander.go | 15 ++-- pkg/volume/csi/expander_test.go | 4 - .../src/k8s.io/api/core/v1/generated.proto | 2 - staging/src/k8s.io/api/core/v1/types.go | 2 - .../core/v1/types_swagger_doc_generated.go | 2 +- 14 files changed, 19 insertions(+), 117 deletions(-) diff --git a/api/openapi-spec/swagger.json b/api/openapi-spec/swagger.json index 1460ff8e2df..8ca17196194 100644 --- a/api/openapi-spec/swagger.json +++ b/api/openapi-spec/swagger.json @@ -5039,7 +5039,7 @@ }, "nodeExpandSecretRef": { "$ref": "#/definitions/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": { "$ref": "#/definitions/io.k8s.api.core.v1.SecretReference", diff --git a/api/openapi-spec/v3/api__v1_openapi.json b/api/openapi-spec/v3/api__v1_openapi.json index fc02fc6fb7d..1c235b58d48 100644 --- a/api/openapi-spec/v3/api__v1_openapi.json +++ b/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": [ diff --git a/api/openapi-spec/v3/apis__storage.k8s.io__v1_openapi.json b/api/openapi-spec/v3/apis__storage.k8s.io__v1_openapi.json index 02ea235f055..a3a749ff5d2 100644 --- a/api/openapi-spec/v3/apis__storage.k8s.io__v1_openapi.json +++ b/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": [ diff --git a/pkg/api/persistentvolume/util.go b/pkg/api/persistentvolume/util.go index ef1f83481e6..4afb69c50b8 100644 --- a/pkg/api/persistentvolume/util.go +++ b/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 diff --git a/pkg/api/persistentvolume/util_test.go b/pkg/api/persistentvolume/util_test.go index dec285103ab..b479aff55a7 100644 --- a/pkg/api/persistentvolume/util_test.go +++ b/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{ diff --git a/pkg/apis/core/types.go b/pkg/apis/core/types.go index 3e74fe31d32..41305a58604 100644 --- a/pkg/apis/core/types.go +++ b/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 } diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index 6673d497a2b..816c456ca59 100644 --- a/pkg/features/kube_features.go +++ b/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}, diff --git a/pkg/generated/openapi/zz_generated.openapi.go b/pkg/generated/openapi/zz_generated.openapi.go index ca97bade13f..b37e8a37f2f 100644 --- a/pkg/generated/openapi/zz_generated.openapi.go +++ b/pkg/generated/openapi/zz_generated.openapi.go @@ -17645,7 +17645,7 @@ func schema_k8sio_api_core_v1_CSIPersistentVolumeSource(ref common.ReferenceCall }, "nodeExpandSecretRef": { SchemaProps: spec.SchemaProps{ - 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.", Ref: ref("k8s.io/api/core/v1.SecretReference"), }, }, diff --git a/pkg/registry/core/persistentvolume/strategy.go b/pkg/registry/core/persistentvolume/strategy.go index 515a9b76869..8ca4a921863 100644 --- a/pkg/registry/core/persistentvolume/strategy.go +++ b/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 { diff --git a/pkg/volume/csi/expander.go b/pkg/volume/csi/expander.go index 262b4774f43..408780582f1 100644 --- a/pkg/volume/csi/expander.go +++ b/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) } } diff --git a/pkg/volume/csi/expander_test.go b/pkg/volume/csi/expander_test.go index 2e4ab7ffeb6..788ad4b20ca 100644 --- a/pkg/volume/csi/expander_test.go +++ b/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) diff --git a/staging/src/k8s.io/api/core/v1/generated.proto b/staging/src/k8s.io/api/core/v1/generated.proto index b5254d174ea..a100fb6dc72 100644 --- a/staging/src/k8s.io/api/core/v1/generated.proto +++ b/staging/src/k8s.io/api/core/v1/generated.proto @@ -228,10 +228,8 @@ message CSIPersistentVolumeSource { // 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 optional SecretReference nodeExpandSecretRef = 10; } diff --git a/staging/src/k8s.io/api/core/v1/types.go b/staging/src/k8s.io/api/core/v1/types.go index 7dbb25a1735..3c4f3735553 100644 --- a/staging/src/k8s.io/api/core/v1/types.go +++ b/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"` } diff --git a/staging/src/k8s.io/api/core/v1/types_swagger_doc_generated.go b/staging/src/k8s.io/api/core/v1/types_swagger_doc_generated.go index 2fc669706a0..484393382c5 100644 --- a/staging/src/k8s.io/api/core/v1/types_swagger_doc_generated.go +++ b/staging/src/k8s.io/api/core/v1/types_swagger_doc_generated.go @@ -127,7 +127,7 @@ var map_CSIPersistentVolumeSource = map[string]string{ "nodeStageSecretRef": "nodeStageSecretRef is a reference to the secret object containing sensitive information to pass to the CSI driver to complete the CSI NodeStageVolume and NodeStageVolume and NodeUnstageVolume calls. This field is optional, and may be empty if no secret is required. If the secret object contains more than one secret, all secrets are passed.", "nodePublishSecretRef": "nodePublishSecretRef is a reference to the secret object containing sensitive information to pass to the CSI driver to complete the CSI NodePublishVolume and NodeUnpublishVolume calls. This field is optional, and may be empty if no secret is required. If the secret object contains more than one secret, all secrets are passed.", "controllerExpandSecretRef": "controllerExpandSecretRef is a reference to the secret object containing sensitive information to pass to the CSI driver to complete the CSI ControllerExpandVolume call. This field is optional, and may be empty if no secret is required. If the secret object contains more than one secret, all secrets are passed.", - "nodeExpandSecretRef": "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.", + "nodeExpandSecretRef": "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.", } func (CSIPersistentVolumeSource) SwaggerDoc() map[string]string {