Merge pull request #114776 from jsafrane/pv-secret-validation

Allow SecretReference.Name in PVs to have 253 characters
This commit is contained in:
Kubernetes Prow Robot 2023-01-18 10:54:34 -08:00 committed by GitHub
commit bdaa6bb617
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 44 additions and 92 deletions

View File

@ -1531,14 +1531,12 @@ func validateStorageOSPersistentVolumeSource(storageos *core.StorageOSPersistent
// validatePVSecretReference check whether provided SecretReference object is valid in terms of secret name and namespace. // 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 var allErrs field.ErrorList
if len(secretRef.Name) == 0 { if len(secretRef.Name) == 0 {
allErrs = append(allErrs, field.Required(fldPath.Child("name"), "")) allErrs = append(allErrs, field.Required(fldPath.Child("name"), ""))
} else if allowDNSSubDomainSecretName {
allErrs = append(allErrs, ValidateDNS1123Subdomain(secretRef.Name, fldPath.Child("name"))...)
} else { } else {
allErrs = append(allErrs, ValidateDNS1123Label(secretRef.Name, fldPath.Child("name"))...) allErrs = append(allErrs, ValidateDNS1123Subdomain(secretRef.Name, fldPath.Child("name"))...)
} }
if len(secretRef.Namespace) == 0 { if len(secretRef.Namespace) == 0 {
@ -1567,7 +1565,7 @@ func ValidateCSIDriverName(driverName string, fldPath *field.Path) field.ErrorLi
return allErrs 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 := field.ErrorList{}
allErrs = append(allErrs, ValidateCSIDriverName(csi.Driver, fldPath.Child("driver"))...) 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"), "")) allErrs = append(allErrs, field.Required(fldPath.Child("volumeHandle"), ""))
} }
if csi.ControllerPublishSecretRef != nil { 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 { 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 { 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 { 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 return allErrs
} }
@ -1647,8 +1645,6 @@ var allowedTemplateObjectMetaFields = map[string]bool{
type PersistentVolumeSpecValidationOptions struct { type PersistentVolumeSpecValidationOptions struct {
// Allow spec to contain the "ReadWiteOncePod" access mode // Allow spec to contain the "ReadWiteOncePod" access mode
AllowReadWriteOncePod bool AllowReadWriteOncePod bool
// Allow the secretRef Name field to be of DNSSubDomain Format
AllowDNSSubDomainSecretName bool
} }
// ValidatePersistentVolumeName checks that a name is appropriate for a // 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 { func ValidationOptionsForPersistentVolume(pv, oldPv *core.PersistentVolume) PersistentVolumeSpecValidationOptions {
opts := PersistentVolumeSpecValidationOptions{ opts := PersistentVolumeSpecValidationOptions{
AllowReadWriteOncePod: utilfeature.DefaultFeatureGate.Enabled(features.ReadWriteOncePod), AllowReadWriteOncePod: utilfeature.DefaultFeatureGate.Enabled(features.ReadWriteOncePod),
AllowDNSSubDomainSecretName: false,
} }
if oldPv == nil { if oldPv == nil {
// If there's no old PV, use the options based solely on feature enablement // 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 // If the old object allowed "ReadWriteOncePod", continue to allow it in the new object
opts.AllowReadWriteOncePod = true 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 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 { func ValidatePersistentVolumeSpec(pvSpec *core.PersistentVolumeSpec, pvName string, validateInlinePersistentVolumeSpec bool, fldPath *field.Path, opts PersistentVolumeSpecValidationOptions) field.ErrorList {
allErrs := 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")) allErrs = append(allErrs, field.Forbidden(fldPath.Child("csi"), "may not specify more than 1 volume type"))
} else { } else {
numVolumes++ numVolumes++
allErrs = append(allErrs, validateCSIPersistentVolumeSource(pvSpec.CSI, opts.AllowDNSSubDomainSecretName, fldPath.Child("csi"))...) allErrs = append(allErrs, validateCSIPersistentVolumeSource(pvSpec.CSI, fldPath.Child("csi"))...)
} }
} }

View File

@ -763,7 +763,7 @@ func TestValidatePersistentVolumeSourceUpdate(t *testing.T) {
newVolume: getCSIVolumeWithSecret(validCSIVolume, shortSecretRef, "controllerExpand"), newVolume: getCSIVolumeWithSecret(validCSIVolume, shortSecretRef, "controllerExpand"),
}, },
"csi-expansion-enabled-with-longSecretRef": { "csi-expansion-enabled-with-longSecretRef": {
isExpectedFailure: true, isExpectedFailure: false, // updating controllerExpandSecretRef is allowed only from nil
oldVolume: validCSIVolume, oldVolume: validCSIVolume,
newVolume: getCSIVolumeWithSecret(validCSIVolume, longSecretRef, "controllerExpand"), newVolume: getCSIVolumeWithSecret(validCSIVolume, longSecretRef, "controllerExpand"),
}, },
@ -773,7 +773,7 @@ func TestValidatePersistentVolumeSourceUpdate(t *testing.T) {
newVolume: getCSIVolumeWithSecret(validCSIVolume, shortSecretRef, "controllerExpand"), newVolume: getCSIVolumeWithSecret(validCSIVolume, shortSecretRef, "controllerExpand"),
}, },
"csi-expansion-enabled-from-shortSecretRef-to-longSecretRef": { "csi-expansion-enabled-from-shortSecretRef-to-longSecretRef": {
isExpectedFailure: true, isExpectedFailure: true, // updating controllerExpandSecretRef is allowed only from nil
oldVolume: getCSIVolumeWithSecret(validCSIVolume, shortSecretRef, "controllerExpand"), oldVolume: getCSIVolumeWithSecret(validCSIVolume, shortSecretRef, "controllerExpand"),
newVolume: getCSIVolumeWithSecret(validCSIVolume, longSecretRef, "controllerExpand"), newVolume: getCSIVolumeWithSecret(validCSIVolume, longSecretRef, "controllerExpand"),
}, },
@ -798,7 +798,7 @@ func TestValidatePersistentVolumeSourceUpdate(t *testing.T) {
newVolume: getCSIVolumeWithSecret(validCSIVolume, shortSecretRef, "controllerPublish"), newVolume: getCSIVolumeWithSecret(validCSIVolume, shortSecretRef, "controllerPublish"),
}, },
"csi-cntrlpublish-enabled-from-shortSecretRef-to-longSecretRef": { "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"), oldVolume: getCSIVolumeWithSecret(validCSIVolume, shortSecretRef, "controllerPublish"),
newVolume: getCSIVolumeWithSecret(validCSIVolume, longSecretRef, "controllerPublish"), newVolume: getCSIVolumeWithSecret(validCSIVolume, longSecretRef, "controllerPublish"),
}, },
@ -843,7 +843,7 @@ func TestValidatePersistentVolumeSourceUpdate(t *testing.T) {
newVolume: getCSIVolumeWithSecret(validCSIVolume, longSecretRef, "nodeStage"), newVolume: getCSIVolumeWithSecret(validCSIVolume, longSecretRef, "nodeStage"),
}, },
"csi-nodestage-enabled-from-shortSecretRef-to-longSecretRef": { "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"), oldVolume: getCSIVolumeWithSecret(validCSIVolume, shortSecretRef, "nodeStage"),
newVolume: getCSIVolumeWithSecret(validCSIVolume, longSecretRef, "nodeStage"), newVolume: getCSIVolumeWithSecret(validCSIVolume, longSecretRef, "nodeStage"),
}, },
@ -2891,11 +2891,10 @@ func TestValidateCSIVolumeSource(t *testing.T) {
func TestValidateCSIPersistentVolumeSource(t *testing.T) { func TestValidateCSIPersistentVolumeSource(t *testing.T) {
testCases := []struct { testCases := []struct {
name string name string
csi *core.CSIPersistentVolumeSource csi *core.CSIPersistentVolumeSource
errtype field.ErrorType errtype field.ErrorType
errfield string errfield string
allowDNSSubDomainSecretName bool
}{ }{
{ {
name: "all required fields ok", name: "all required fields ok",
@ -3060,81 +3059,51 @@ func TestValidateCSIPersistentVolumeSource(t *testing.T) {
// tests with allowDNSSubDomainSecretName flag on/off // tests with allowDNSSubDomainSecretName flag on/off
{ {
name: "valid nodeExpandSecretRef with allow flag off", name: "valid nodeExpandSecretRef",
csi: &core.CSIPersistentVolumeSource{Driver: "com.google.gcepd", VolumeHandle: "foobar", NodeExpandSecretRef: &core.SecretReference{Name: strings.Repeat("g", 63), Namespace: "default"}}, 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", name: "valid long nodeExpandSecretRef",
csi: &core.CSIPersistentVolumeSource{Driver: "com.google.gcepd", VolumeHandle: "foobar", NodeExpandSecretRef: &core.SecretReference{Name: strings.Repeat("g", 65), Namespace: "default"}}, 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", name: "Invalid nodeExpandSecretRef",
csi: &core.CSIPersistentVolumeSource{Driver: "com.google.gcepd", VolumeHandle: "foobar", NodeExpandSecretRef: &core.SecretReference{Name: strings.Repeat("g", 65), Namespace: "default"}}, 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: "Invalid nodeExpandSecretRef with allow flag on", name: "valid nodePublishSecretRef",
csi: &core.CSIPersistentVolumeSource{Driver: "com.google.gcepd", VolumeHandle: "foobar", NodeExpandSecretRef: &core.SecretReference{Name: strings.Repeat("g", 255), Namespace: "default"}}, csi: &core.CSIPersistentVolumeSource{Driver: "com.google.gcepd", VolumeHandle: "foobar", NodePublishSecretRef: &core.SecretReference{Name: strings.Repeat("g", 63), Namespace: "default"}},
allowDNSSubDomainSecretName: true,
errtype: field.ErrorTypeInvalid,
errfield: "nodeExpandSecretRef.name",
}, },
{ {
name: "valid nodePublishSecretRef with allow flag off", name: "valid long nodePublishSecretRef",
csi: &core.CSIPersistentVolumeSource{Driver: "com.google.gcepd", VolumeHandle: "foobar", NodePublishSecretRef: &core.SecretReference{Name: strings.Repeat("g", 63), Namespace: "default"}}, csi: &core.CSIPersistentVolumeSource{Driver: "com.google.gcepd", VolumeHandle: "foobar", NodePublishSecretRef: &core.SecretReference{Name: strings.Repeat("g", 65), Namespace: "default"}},
allowDNSSubDomainSecretName: false,
}, },
{ {
name: "Invalid nodePublishSecretRef with allow flag off", name: "Invalid nodePublishSecretRef",
csi: &core.CSIPersistentVolumeSource{Driver: "com.google.gcepd", VolumeHandle: "foobar", NodePublishSecretRef: &core.SecretReference{Name: strings.Repeat("g", 65), Namespace: "default"}}, csi: &core.CSIPersistentVolumeSource{Driver: "com.google.gcepd", VolumeHandle: "foobar", NodePublishSecretRef: &core.SecretReference{Name: strings.Repeat("g", 255), Namespace: "default"}},
allowDNSSubDomainSecretName: false, errtype: field.ErrorTypeInvalid,
errtype: field.ErrorTypeInvalid, errfield: "nodePublishSecretRef.name",
errfield: "nodePublishSecretRef.name",
}, },
{ {
name: "valid nodePublishSecretRef with allow flag on", name: "valid ControllerExpandSecretRef",
csi: &core.CSIPersistentVolumeSource{Driver: "com.google.gcepd", VolumeHandle: "foobar", NodePublishSecretRef: &core.SecretReference{Name: strings.Repeat("g", 65), Namespace: "default"}}, csi: &core.CSIPersistentVolumeSource{Driver: "com.google.gcepd", VolumeHandle: "foobar", ControllerExpandSecretRef: &core.SecretReference{Name: strings.Repeat("g", 63), Namespace: "default"}},
allowDNSSubDomainSecretName: true,
}, },
{ {
name: "Invalid nodePublishSecretRef with allow flag on", name: "valid long ControllerExpandSecretRef",
csi: &core.CSIPersistentVolumeSource{Driver: "com.google.gcepd", VolumeHandle: "foobar", NodePublishSecretRef: &core.SecretReference{Name: strings.Repeat("g", 255), Namespace: "default"}}, csi: &core.CSIPersistentVolumeSource{Driver: "com.google.gcepd", VolumeHandle: "foobar", ControllerExpandSecretRef: &core.SecretReference{Name: strings.Repeat("g", 65), Namespace: "default"}},
allowDNSSubDomainSecretName: true,
errtype: field.ErrorTypeInvalid,
errfield: "nodePublishSecretRef.name",
}, },
{ {
name: "valid ControllerExpandSecretRef with allow flag off", name: "Invalid ControllerExpandSecretRef",
csi: &core.CSIPersistentVolumeSource{Driver: "com.google.gcepd", VolumeHandle: "foobar", ControllerExpandSecretRef: &core.SecretReference{Name: strings.Repeat("g", 63), Namespace: "default"}}, csi: &core.CSIPersistentVolumeSource{Driver: "com.google.gcepd", VolumeHandle: "foobar", ControllerExpandSecretRef: &core.SecretReference{Name: strings.Repeat("g", 255), Namespace: "default"}},
allowDNSSubDomainSecretName: false, errtype: field.ErrorTypeInvalid,
}, errfield: "controllerExpandSecretRef.name",
{
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 { 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 == "" { if len(errs) > 0 && tc.errtype == "" {
t.Errorf("[%d: %q] unexpected error(s): %v", i, tc.name, errs) 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", name: "invalid secret ref name",
args: args{&core.SecretReference{Name: "$%^&*#", Namespace: "default"}, rootFld}, args: args{&core.SecretReference{Name: "$%^&*#", Namespace: "default"}, rootFld},
expectError: true, expectError: true,
expectedError: "name.name: Invalid value: \"$%^&*#\": " + dnsLabelErrMsg, expectedError: "name.name: Invalid value: \"$%^&*#\": " + dnsSubdomainLabelErrMsg,
}, },
{ {
name: "invalid secret ref namespace", name: "invalid secret ref namespace",
@ -21766,7 +21735,7 @@ func TestValidatePVSecretReference(t *testing.T) {
} }
for _, tt := range tests { for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) { 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 { if tt.expectError && len(errs) == 0 {
t.Errorf("Unexpected success") t.Errorf("Unexpected success")
} }