From e743a6d14df5b190164f1cd884cfa98ce0143590 Mon Sep 17 00:00:00 2001 From: Jan Safranek Date: Tue, 3 Jan 2023 14:19:30 +0100 Subject: [PATCH] Allow SecretReference.Name in PVs to have 253 characters PV.Spec.CSI.*SecretReference.Name should be allowed to have up to be limited to 253 characters (DNS1123Subdomain) and not to 63 characters (DNS1123Label), so all possible Secrets names can be used as secrets in a PV. This is continuation of https://github.com/kubernetes/kubernetes/pull/108331 / Kubernetes 1.25, which allowed updating PVs with long secret names, if the previous PV had long secret name too. This makes sure downgrade from 1.27 to 1.26 works well and allows PVs created in 1.27 to be updated in 1.26. Now the long secret names are accepted during PV creation too. --- pkg/apis/core/validation/validation.go | 35 ++----- pkg/apis/core/validation/validation_test.go | 101 +++++++------------- 2 files changed, 44 insertions(+), 92 deletions(-) diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go index 741057db71c..25f65dd6087 100644 --- a/pkg/apis/core/validation/validation.go +++ b/pkg/apis/core/validation/validation.go @@ -1531,14 +1531,12 @@ func validateStorageOSPersistentVolumeSource(storageos *core.StorageOSPersistent // validatePVSecretReference check whether provided SecretReference object is valid in terms of secret name and namespace. -func validatePVSecretReference(secretRef *core.SecretReference, allowDNSSubDomainSecretName bool, fldPath *field.Path) field.ErrorList { +func validatePVSecretReference(secretRef *core.SecretReference, fldPath *field.Path) field.ErrorList { var allErrs field.ErrorList if len(secretRef.Name) == 0 { allErrs = append(allErrs, field.Required(fldPath.Child("name"), "")) - } else if allowDNSSubDomainSecretName { - allErrs = append(allErrs, ValidateDNS1123Subdomain(secretRef.Name, fldPath.Child("name"))...) } else { - allErrs = append(allErrs, ValidateDNS1123Label(secretRef.Name, fldPath.Child("name"))...) + allErrs = append(allErrs, ValidateDNS1123Subdomain(secretRef.Name, fldPath.Child("name"))...) } if len(secretRef.Namespace) == 0 { @@ -1567,7 +1565,7 @@ func ValidateCSIDriverName(driverName string, fldPath *field.Path) field.ErrorLi return allErrs } -func validateCSIPersistentVolumeSource(csi *core.CSIPersistentVolumeSource, allowDNSSubDomainSecretName bool, fldPath *field.Path) field.ErrorList { +func validateCSIPersistentVolumeSource(csi *core.CSIPersistentVolumeSource, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} allErrs = append(allErrs, ValidateCSIDriverName(csi.Driver, fldPath.Child("driver"))...) @@ -1576,16 +1574,16 @@ func validateCSIPersistentVolumeSource(csi *core.CSIPersistentVolumeSource, allo allErrs = append(allErrs, field.Required(fldPath.Child("volumeHandle"), "")) } if csi.ControllerPublishSecretRef != nil { - allErrs = append(allErrs, validatePVSecretReference(csi.ControllerPublishSecretRef, allowDNSSubDomainSecretName, fldPath.Child("controllerPublishSecretRef"))...) + allErrs = append(allErrs, validatePVSecretReference(csi.ControllerPublishSecretRef, fldPath.Child("controllerPublishSecretRef"))...) } if csi.ControllerExpandSecretRef != nil { - allErrs = append(allErrs, validatePVSecretReference(csi.ControllerExpandSecretRef, allowDNSSubDomainSecretName, fldPath.Child("controllerExpandSecretRef"))...) + allErrs = append(allErrs, validatePVSecretReference(csi.ControllerExpandSecretRef, fldPath.Child("controllerExpandSecretRef"))...) } if csi.NodePublishSecretRef != nil { - allErrs = append(allErrs, validatePVSecretReference(csi.NodePublishSecretRef, allowDNSSubDomainSecretName, fldPath.Child("nodePublishSecretRef"))...) + allErrs = append(allErrs, validatePVSecretReference(csi.NodePublishSecretRef, fldPath.Child("nodePublishSecretRef"))...) } if csi.NodeExpandSecretRef != nil { - allErrs = append(allErrs, validatePVSecretReference(csi.NodeExpandSecretRef, allowDNSSubDomainSecretName, fldPath.Child("nodeExpandSecretRef"))...) + allErrs = append(allErrs, validatePVSecretReference(csi.NodeExpandSecretRef, fldPath.Child("nodeExpandSecretRef"))...) } return allErrs } @@ -1647,8 +1645,6 @@ var allowedTemplateObjectMetaFields = map[string]bool{ type PersistentVolumeSpecValidationOptions struct { // Allow spec to contain the "ReadWiteOncePod" access mode AllowReadWriteOncePod bool - // Allow the secretRef Name field to be of DNSSubDomain Format - AllowDNSSubDomainSecretName bool } // ValidatePersistentVolumeName checks that a name is appropriate for a @@ -1663,8 +1659,7 @@ var supportedVolumeModes = sets.NewString(string(core.PersistentVolumeBlock), st func ValidationOptionsForPersistentVolume(pv, oldPv *core.PersistentVolume) PersistentVolumeSpecValidationOptions { opts := PersistentVolumeSpecValidationOptions{ - AllowReadWriteOncePod: utilfeature.DefaultFeatureGate.Enabled(features.ReadWriteOncePod), - AllowDNSSubDomainSecretName: false, + AllowReadWriteOncePod: utilfeature.DefaultFeatureGate.Enabled(features.ReadWriteOncePod), } if oldPv == nil { // If there's no old PV, use the options based solely on feature enablement @@ -1674,21 +1669,9 @@ func ValidationOptionsForPersistentVolume(pv, oldPv *core.PersistentVolume) Pers // If the old object allowed "ReadWriteOncePod", continue to allow it in the new object opts.AllowReadWriteOncePod = true } - if oldCSI := oldPv.Spec.CSI; oldCSI != nil { - opts.AllowDNSSubDomainSecretName = - secretRefRequiresSubdomainSecretName(oldCSI.ControllerExpandSecretRef) || - secretRefRequiresSubdomainSecretName(oldCSI.ControllerPublishSecretRef) || - secretRefRequiresSubdomainSecretName(oldCSI.NodeStageSecretRef) || - secretRefRequiresSubdomainSecretName(oldCSI.NodePublishSecretRef) - } return opts } -func secretRefRequiresSubdomainSecretName(secretRef *core.SecretReference) bool { - // ref and name were specified and name didn't fit within label validation - return secretRef != nil && len(secretRef.Name) > 0 && len(validation.IsDNS1123Label(secretRef.Name)) > 0 -} - func ValidatePersistentVolumeSpec(pvSpec *core.PersistentVolumeSpec, pvName string, validateInlinePersistentVolumeSpec bool, fldPath *field.Path, opts PersistentVolumeSpecValidationOptions) field.ErrorList { allErrs := field.ErrorList{} @@ -1943,7 +1926,7 @@ func ValidatePersistentVolumeSpec(pvSpec *core.PersistentVolumeSpec, pvName stri allErrs = append(allErrs, field.Forbidden(fldPath.Child("csi"), "may not specify more than 1 volume type")) } else { numVolumes++ - allErrs = append(allErrs, validateCSIPersistentVolumeSource(pvSpec.CSI, opts.AllowDNSSubDomainSecretName, fldPath.Child("csi"))...) + allErrs = append(allErrs, validateCSIPersistentVolumeSource(pvSpec.CSI, fldPath.Child("csi"))...) } } diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index c11184e9dca..5ca335e0990 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/pkg/apis/core/validation/validation_test.go @@ -763,7 +763,7 @@ func TestValidatePersistentVolumeSourceUpdate(t *testing.T) { newVolume: getCSIVolumeWithSecret(validCSIVolume, shortSecretRef, "controllerExpand"), }, "csi-expansion-enabled-with-longSecretRef": { - isExpectedFailure: true, + isExpectedFailure: false, // updating controllerExpandSecretRef is allowed only from nil oldVolume: validCSIVolume, newVolume: getCSIVolumeWithSecret(validCSIVolume, longSecretRef, "controllerExpand"), }, @@ -773,7 +773,7 @@ func TestValidatePersistentVolumeSourceUpdate(t *testing.T) { newVolume: getCSIVolumeWithSecret(validCSIVolume, shortSecretRef, "controllerExpand"), }, "csi-expansion-enabled-from-shortSecretRef-to-longSecretRef": { - isExpectedFailure: true, + isExpectedFailure: true, // updating controllerExpandSecretRef is allowed only from nil oldVolume: getCSIVolumeWithSecret(validCSIVolume, shortSecretRef, "controllerExpand"), newVolume: getCSIVolumeWithSecret(validCSIVolume, longSecretRef, "controllerExpand"), }, @@ -798,7 +798,7 @@ func TestValidatePersistentVolumeSourceUpdate(t *testing.T) { newVolume: getCSIVolumeWithSecret(validCSIVolume, shortSecretRef, "controllerPublish"), }, "csi-cntrlpublish-enabled-from-shortSecretRef-to-longSecretRef": { - isExpectedFailure: true, + isExpectedFailure: true, // updating secretRef will fail as the object is immutable eventhough the secretRef is valid oldVolume: getCSIVolumeWithSecret(validCSIVolume, shortSecretRef, "controllerPublish"), newVolume: getCSIVolumeWithSecret(validCSIVolume, longSecretRef, "controllerPublish"), }, @@ -843,7 +843,7 @@ func TestValidatePersistentVolumeSourceUpdate(t *testing.T) { newVolume: getCSIVolumeWithSecret(validCSIVolume, longSecretRef, "nodeStage"), }, "csi-nodestage-enabled-from-shortSecretRef-to-longSecretRef": { - isExpectedFailure: true, + isExpectedFailure: true, // updating secretRef will fail as the object is immutable eventhough the secretRef is valid oldVolume: getCSIVolumeWithSecret(validCSIVolume, shortSecretRef, "nodeStage"), newVolume: getCSIVolumeWithSecret(validCSIVolume, longSecretRef, "nodeStage"), }, @@ -2891,11 +2891,10 @@ func TestValidateCSIVolumeSource(t *testing.T) { func TestValidateCSIPersistentVolumeSource(t *testing.T) { testCases := []struct { - name string - csi *core.CSIPersistentVolumeSource - errtype field.ErrorType - errfield string - allowDNSSubDomainSecretName bool + name string + csi *core.CSIPersistentVolumeSource + errtype field.ErrorType + errfield string }{ { name: "all required fields ok", @@ -3060,81 +3059,51 @@ func TestValidateCSIPersistentVolumeSource(t *testing.T) { // tests with allowDNSSubDomainSecretName flag on/off { - name: "valid nodeExpandSecretRef with allow flag off", - csi: &core.CSIPersistentVolumeSource{Driver: "com.google.gcepd", VolumeHandle: "foobar", NodeExpandSecretRef: &core.SecretReference{Name: strings.Repeat("g", 63), Namespace: "default"}}, - allowDNSSubDomainSecretName: false, + name: "valid nodeExpandSecretRef", + csi: &core.CSIPersistentVolumeSource{Driver: "com.google.gcepd", VolumeHandle: "foobar", NodeExpandSecretRef: &core.SecretReference{Name: strings.Repeat("g", 63), Namespace: "default"}}, }, { - name: "Invalid nodeExpandSecretRef with allow flag off", - csi: &core.CSIPersistentVolumeSource{Driver: "com.google.gcepd", VolumeHandle: "foobar", NodeExpandSecretRef: &core.SecretReference{Name: strings.Repeat("g", 65), Namespace: "default"}}, - allowDNSSubDomainSecretName: false, - errtype: field.ErrorTypeInvalid, - errfield: "nodeExpandSecretRef.name", + name: "valid long nodeExpandSecretRef", + csi: &core.CSIPersistentVolumeSource{Driver: "com.google.gcepd", VolumeHandle: "foobar", NodeExpandSecretRef: &core.SecretReference{Name: strings.Repeat("g", 65), Namespace: "default"}}, }, { - name: "valid nodeExpandSecretRef with allow flag on", - csi: &core.CSIPersistentVolumeSource{Driver: "com.google.gcepd", VolumeHandle: "foobar", NodeExpandSecretRef: &core.SecretReference{Name: strings.Repeat("g", 65), Namespace: "default"}}, - allowDNSSubDomainSecretName: true, + name: "Invalid nodeExpandSecretRef", + csi: &core.CSIPersistentVolumeSource{Driver: "com.google.gcepd", VolumeHandle: "foobar", NodeExpandSecretRef: &core.SecretReference{Name: strings.Repeat("g", 255), Namespace: "default"}}, + errtype: field.ErrorTypeInvalid, + errfield: "nodeExpandSecretRef.name", }, { - name: "Invalid nodeExpandSecretRef with allow flag on", - csi: &core.CSIPersistentVolumeSource{Driver: "com.google.gcepd", VolumeHandle: "foobar", NodeExpandSecretRef: &core.SecretReference{Name: strings.Repeat("g", 255), Namespace: "default"}}, - allowDNSSubDomainSecretName: true, - errtype: field.ErrorTypeInvalid, - errfield: "nodeExpandSecretRef.name", + name: "valid nodePublishSecretRef", + csi: &core.CSIPersistentVolumeSource{Driver: "com.google.gcepd", VolumeHandle: "foobar", NodePublishSecretRef: &core.SecretReference{Name: strings.Repeat("g", 63), Namespace: "default"}}, }, { - name: "valid nodePublishSecretRef with allow flag off", - csi: &core.CSIPersistentVolumeSource{Driver: "com.google.gcepd", VolumeHandle: "foobar", NodePublishSecretRef: &core.SecretReference{Name: strings.Repeat("g", 63), Namespace: "default"}}, - allowDNSSubDomainSecretName: false, + name: "valid long nodePublishSecretRef", + csi: &core.CSIPersistentVolumeSource{Driver: "com.google.gcepd", VolumeHandle: "foobar", NodePublishSecretRef: &core.SecretReference{Name: strings.Repeat("g", 65), Namespace: "default"}}, }, { - name: "Invalid nodePublishSecretRef with allow flag off", - csi: &core.CSIPersistentVolumeSource{Driver: "com.google.gcepd", VolumeHandle: "foobar", NodePublishSecretRef: &core.SecretReference{Name: strings.Repeat("g", 65), Namespace: "default"}}, - allowDNSSubDomainSecretName: false, - errtype: field.ErrorTypeInvalid, - errfield: "nodePublishSecretRef.name", + name: "Invalid nodePublishSecretRef", + csi: &core.CSIPersistentVolumeSource{Driver: "com.google.gcepd", VolumeHandle: "foobar", NodePublishSecretRef: &core.SecretReference{Name: strings.Repeat("g", 255), Namespace: "default"}}, + errtype: field.ErrorTypeInvalid, + errfield: "nodePublishSecretRef.name", }, { - name: "valid nodePublishSecretRef with allow flag on", - csi: &core.CSIPersistentVolumeSource{Driver: "com.google.gcepd", VolumeHandle: "foobar", NodePublishSecretRef: &core.SecretReference{Name: strings.Repeat("g", 65), Namespace: "default"}}, - allowDNSSubDomainSecretName: true, + name: "valid ControllerExpandSecretRef", + csi: &core.CSIPersistentVolumeSource{Driver: "com.google.gcepd", VolumeHandle: "foobar", ControllerExpandSecretRef: &core.SecretReference{Name: strings.Repeat("g", 63), Namespace: "default"}}, }, { - name: "Invalid nodePublishSecretRef with allow flag on", - csi: &core.CSIPersistentVolumeSource{Driver: "com.google.gcepd", VolumeHandle: "foobar", NodePublishSecretRef: &core.SecretReference{Name: strings.Repeat("g", 255), Namespace: "default"}}, - allowDNSSubDomainSecretName: true, - errtype: field.ErrorTypeInvalid, - errfield: "nodePublishSecretRef.name", + name: "valid long ControllerExpandSecretRef", + csi: &core.CSIPersistentVolumeSource{Driver: "com.google.gcepd", VolumeHandle: "foobar", ControllerExpandSecretRef: &core.SecretReference{Name: strings.Repeat("g", 65), Namespace: "default"}}, }, { - name: "valid ControllerExpandSecretRef with allow flag off", - csi: &core.CSIPersistentVolumeSource{Driver: "com.google.gcepd", VolumeHandle: "foobar", ControllerExpandSecretRef: &core.SecretReference{Name: strings.Repeat("g", 63), Namespace: "default"}}, - allowDNSSubDomainSecretName: false, - }, - { - name: "Invalid ControllerExpandSecretRef with allow flag off", - csi: &core.CSIPersistentVolumeSource{Driver: "com.google.gcepd", VolumeHandle: "foobar", ControllerExpandSecretRef: &core.SecretReference{Name: strings.Repeat("g", 65), Namespace: "default"}}, - allowDNSSubDomainSecretName: false, - errtype: field.ErrorTypeInvalid, - errfield: "controllerExpandSecretRef.name", - }, - { - name: "valid ControllerExpandSecretRef with allow flag on", - csi: &core.CSIPersistentVolumeSource{Driver: "com.google.gcepd", VolumeHandle: "foobar", ControllerExpandSecretRef: &core.SecretReference{Name: strings.Repeat("g", 65), Namespace: "default"}}, - allowDNSSubDomainSecretName: true, - }, - { - name: "Invalid ControllerExpandSecretRef with allow flag on", - csi: &core.CSIPersistentVolumeSource{Driver: "com.google.gcepd", VolumeHandle: "foobar", ControllerExpandSecretRef: &core.SecretReference{Name: strings.Repeat("g", 255), Namespace: "default"}}, - allowDNSSubDomainSecretName: true, - errtype: field.ErrorTypeInvalid, - errfield: "controllerExpandSecretRef.name", + name: "Invalid ControllerExpandSecretRef", + csi: &core.CSIPersistentVolumeSource{Driver: "com.google.gcepd", VolumeHandle: "foobar", ControllerExpandSecretRef: &core.SecretReference{Name: strings.Repeat("g", 255), Namespace: "default"}}, + errtype: field.ErrorTypeInvalid, + errfield: "controllerExpandSecretRef.name", }, } for i, tc := range testCases { - errs := validateCSIPersistentVolumeSource(tc.csi, tc.allowDNSSubDomainSecretName, field.NewPath("field")) + errs := validateCSIPersistentVolumeSource(tc.csi, field.NewPath("field")) if len(errs) > 0 && tc.errtype == "" { t.Errorf("[%d: %q] unexpected error(s): %v", i, tc.name, errs) @@ -21709,7 +21678,7 @@ func TestValidatePVSecretReference(t *testing.T) { name: "invalid secret ref name", args: args{&core.SecretReference{Name: "$%^&*#", Namespace: "default"}, rootFld}, expectError: true, - expectedError: "name.name: Invalid value: \"$%^&*#\": " + dnsLabelErrMsg, + expectedError: "name.name: Invalid value: \"$%^&*#\": " + dnsSubdomainLabelErrMsg, }, { name: "invalid secret ref namespace", @@ -21738,7 +21707,7 @@ func TestValidatePVSecretReference(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - errs := validatePVSecretReference(tt.args.secretRef, false, tt.args.fldPath) + errs := validatePVSecretReference(tt.args.secretRef, tt.args.fldPath) if tt.expectError && len(errs) == 0 { t.Errorf("Unexpected success") }