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.
This commit is contained in:
Jan Safranek 2023-01-03 14:19:30 +01:00
parent 1edbb8cf1a
commit e743a6d14d
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)
@ -21709,7 +21678,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",
@ -21738,7 +21707,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")
} }