Merge pull request #108331 from humblec/dns

csi: validate the secretnames in CSI spec against NameIsDNSSubdomain

pkg/apis/core/validation/validation.go

@@ -1531,13 +1531,16 @@ 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, fldPath *field.Path) field.ErrorList {
+func validatePVSecretReference(secretRef *core.SecretReference, allowDNSSubDomainSecretName bool, 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"))...)
 	}
+
 	if len(secretRef.Namespace) == 0 {
 		allErrs = append(allErrs, field.Required(fldPath.Child("namespace"), ""))
 	} else {
@@ -1564,7 +1567,7 @@ func ValidateCSIDriverName(driverName string, fldPath *field.Path) field.ErrorLi
 	return allErrs
 }
 
-func validateCSIPersistentVolumeSource(csi *core.CSIPersistentVolumeSource, fldPath *field.Path) field.ErrorList {
+func validateCSIPersistentVolumeSource(csi *core.CSIPersistentVolumeSource, allowDNSSubDomainSecretName bool, fldPath *field.Path) field.ErrorList {
 	allErrs := field.ErrorList{}
 
 	allErrs = append(allErrs, ValidateCSIDriverName(csi.Driver, fldPath.Child("driver"))...)
@@ -1573,16 +1576,16 @@ func validateCSIPersistentVolumeSource(csi *core.CSIPersistentVolumeSource, fldP
 		allErrs = append(allErrs, field.Required(fldPath.Child("volumeHandle"), ""))
 	}
 	if csi.ControllerPublishSecretRef != nil {
-		allErrs = append(allErrs, validatePVSecretReference(csi.ControllerPublishSecretRef, fldPath.Child("controllerPublishSecretRef"))...)
+		allErrs = append(allErrs, validatePVSecretReference(csi.ControllerPublishSecretRef, allowDNSSubDomainSecretName, fldPath.Child("controllerPublishSecretRef"))...)
 	}
 	if csi.ControllerExpandSecretRef != nil {
-		allErrs = append(allErrs, validatePVSecretReference(csi.ControllerExpandSecretRef, fldPath.Child("controllerExpandSecretRef"))...)
+		allErrs = append(allErrs, validatePVSecretReference(csi.ControllerExpandSecretRef, allowDNSSubDomainSecretName, fldPath.Child("controllerExpandSecretRef"))...)
 	}
 	if csi.NodePublishSecretRef != nil {
-		allErrs = append(allErrs, validatePVSecretReference(csi.NodePublishSecretRef, fldPath.Child("nodePublishSecretRef"))...)
+		allErrs = append(allErrs, validatePVSecretReference(csi.NodePublishSecretRef, allowDNSSubDomainSecretName, fldPath.Child("nodePublishSecretRef"))...)
 	}
 	if csi.NodeExpandSecretRef != nil {
-		allErrs = append(allErrs, validatePVSecretReference(csi.NodeExpandSecretRef, fldPath.Child("nodeExpandSecretRef"))...)
+		allErrs = append(allErrs, validatePVSecretReference(csi.NodeExpandSecretRef, allowDNSSubDomainSecretName, fldPath.Child("nodeExpandSecretRef"))...)
 	}
 	return allErrs
 }
@@ -1644,6 +1647,8 @@ var allowedPVCTemplateObjectMetaFields = 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
@@ -1659,6 +1664,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,
 	}
 	if oldPv == nil {
 		// If there's no old PV, use the options based solely on feature enablement
@@ -1668,9 +1674,21 @@ 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{}
 
@@ -1925,7 +1943,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, fldPath.Child("csi"))...)
+			allErrs = append(allErrs, validateCSIPersistentVolumeSource(pvSpec.CSI, opts.AllowDNSSubDomainSecretName, fldPath.Child("csi"))...)
 		}
 	}
 

pkg/apis/core/validation/validation_test.go

@@ -757,7 +757,7 @@ func TestValidatePersistentVolumeSourceUpdate(t *testing.T) {
 			newVolume:         getCSIVolumeWithSecret(validCSIVolume, longSecretRef, "controllerExpand"),
 		},
 		"csi-expansion-enabled-from-longSecretRef-to-longSecretRef": {
-			isExpectedFailure: true,
+			isExpectedFailure: false,
 			oldVolume:         getCSIVolumeWithSecret(validCSIVolume, longSecretRef, "controllerExpand"),
 			newVolume:         getCSIVolumeWithSecret(validCSIVolume, longSecretRef, "controllerExpand"),
 		},
@@ -782,7 +782,7 @@ func TestValidatePersistentVolumeSourceUpdate(t *testing.T) {
 			newVolume:         getCSIVolumeWithSecret(validCSIVolume, longSecretRef, "controllerPublish"),
 		},
 		"csi-cntrlpublish-enabled-from-longSecretRef-to-longSecretRef": {
-			isExpectedFailure: true,
+			isExpectedFailure: false,
 			oldVolume:         getCSIVolumeWithSecret(validCSIVolume, longSecretRef, "controllerPublish"),
 			newVolume:         getCSIVolumeWithSecret(validCSIVolume, longSecretRef, "controllerPublish"),
 		},
@@ -807,7 +807,7 @@ func TestValidatePersistentVolumeSourceUpdate(t *testing.T) {
 			newVolume:         getCSIVolumeWithSecret(validCSIVolume, longSecretRef, "nodePublish"),
 		},
 		"csi-nodepublish-enabled-from-longSecretRef-to-longSecretRef": {
-			isExpectedFailure: true,
+			isExpectedFailure: false,
 			oldVolume:         getCSIVolumeWithSecret(validCSIVolume, longSecretRef, "nodePublish"),
 			newVolume:         getCSIVolumeWithSecret(validCSIVolume, longSecretRef, "nodePublish"),
 		},
@@ -2747,6 +2747,7 @@ func TestValidateCSIPersistentVolumeSource(t *testing.T) {
 		csi                         *core.CSIPersistentVolumeSource
 		errtype                     field.ErrorType
 		errfield                    string
+		allowDNSSubDomainSecretName bool
 	}{
 		{
 			name: "all required fields ok",
@@ -2904,10 +2905,88 @@ func TestValidateCSIPersistentVolumeSource(t *testing.T) {
 			name: "valid nodeExpandSecretRef",
 			csi:  &core.CSIPersistentVolumeSource{Driver: "com.google.gcepd", VolumeHandle: "foobar", NodeExpandSecretRef: &core.SecretReference{Name: "foobar", Namespace: "default"}},
 		},
+		{
+			name: "Invalid nodePublishSecretRef",
+			csi:  &core.CSIPersistentVolumeSource{Driver: "com.google.gcepd", VolumeHandle: "foobar", NodePublishSecretRef: &core.SecretReference{Name: "foobar", Namespace: "default"}},
+		},
+
+		// 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:                        "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 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 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 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:                        "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:                        "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:                        "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 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",
+		},
 	}
 
 	for i, tc := range testCases {
-		errs := validateCSIPersistentVolumeSource(tc.csi, field.NewPath("field"))
+		errs := validateCSIPersistentVolumeSource(tc.csi, tc.allowDNSSubDomainSecretName, field.NewPath("field"))
 
 		if len(errs) > 0 && tc.errtype == "" {
 			t.Errorf("[%d: %q] unexpected error(s): %v", i, tc.name, errs)
@@ -20530,7 +20609,7 @@ func TestValidatePVSecretReference(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			errs := validatePVSecretReference(tt.args.secretRef, tt.args.fldPath)
+			errs := validatePVSecretReference(tt.args.secretRef, false, tt.args.fldPath)
 			if tt.expectError && len(errs) == 0 {
 				t.Errorf("Unexpected success")
 			}