diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go index eb3ac7e43c7..344b746758c 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 e9347dc6be0..b1e20769e98 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) @@ -21737,7 +21706,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", @@ -21766,7 +21735,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") }