diff --git a/pkg/apis/storage/validation/validation.go b/pkg/apis/storage/validation/validation.go index 81d74b188b7..f168973e053 100644 --- a/pkg/apis/storage/validation/validation.go +++ b/pkg/apis/storage/validation/validation.go @@ -44,9 +44,15 @@ const ( maxAttachedVolumeMetadataSize = 256 * (1 << 10) // 256 kB maxVolumeErrorMessageSize = 1024 - csiNodeIDMaxLength = 128 + csiNodeIDMaxLength = 128 + csiNodeIDLongerMaxLength = 192 ) +// CSINodeValidationOptions contains the validation options for validating CSINode +type CSINodeValidationOptions struct { + AllowLongNodeID bool +} + // ValidateStorageClass validates a StorageClass. func ValidateStorageClass(storageClass *storage.StorageClass) field.ErrorList { allErrs := apivalidation.ValidateObjectMeta(&storageClass.ObjectMeta, false, apivalidation.ValidateClassName, field.NewPath("metadata")) @@ -291,15 +297,15 @@ func validateAllowedTopologies(topologies []api.TopologySelectorTerm, fldPath *f } // ValidateCSINode validates a CSINode. -func ValidateCSINode(csiNode *storage.CSINode) field.ErrorList { +func ValidateCSINode(csiNode *storage.CSINode, validationOpts CSINodeValidationOptions) field.ErrorList { allErrs := apivalidation.ValidateObjectMeta(&csiNode.ObjectMeta, false, apivalidation.ValidateNodeName, field.NewPath("metadata")) - allErrs = append(allErrs, validateCSINodeSpec(&csiNode.Spec, field.NewPath("spec"))...) + allErrs = append(allErrs, validateCSINodeSpec(&csiNode.Spec, field.NewPath("spec"), validationOpts)...) return allErrs } // ValidateCSINodeUpdate validates a CSINode. -func ValidateCSINodeUpdate(new, old *storage.CSINode) field.ErrorList { - allErrs := ValidateCSINode(new) +func ValidateCSINodeUpdate(new, old *storage.CSINode, validationOpts CSINodeValidationOptions) field.ErrorList { + allErrs := ValidateCSINode(new, validationOpts) // Validate modifying fields inside an existing CSINodeDriver entry is not allowed for _, oldDriver := range old.Spec.Drivers { @@ -317,38 +323,47 @@ func ValidateCSINodeUpdate(new, old *storage.CSINode) field.ErrorList { // ValidateCSINodeSpec tests that the specified CSINodeSpec has valid data. func validateCSINodeSpec( - spec *storage.CSINodeSpec, fldPath *field.Path) field.ErrorList { + spec *storage.CSINodeSpec, fldPath *field.Path, validationOpts CSINodeValidationOptions) field.ErrorList { allErrs := field.ErrorList{} - allErrs = append(allErrs, validateCSINodeDrivers(spec.Drivers, fldPath.Child("drivers"))...) + allErrs = append(allErrs, validateCSINodeDrivers(spec.Drivers, fldPath.Child("drivers"), validationOpts)...) return allErrs } // ValidateCSINodeDrivers tests that the specified CSINodeDrivers have valid data. -func validateCSINodeDrivers(drivers []storage.CSINodeDriver, fldPath *field.Path) field.ErrorList { +func validateCSINodeDrivers(drivers []storage.CSINodeDriver, fldPath *field.Path, validationOpts CSINodeValidationOptions) field.ErrorList { allErrs := field.ErrorList{} driverNamesInSpecs := make(sets.String) for i, driver := range drivers { idxPath := fldPath.Index(i) - allErrs = append(allErrs, validateCSINodeDriver(driver, driverNamesInSpecs, idxPath)...) + allErrs = append(allErrs, validateCSINodeDriver(driver, driverNamesInSpecs, idxPath, validationOpts)...) } return allErrs } // validateCSINodeDriverNodeID tests if Name in CSINodeDriver is a valid node id. -func validateCSINodeDriverNodeID(nodeID string, fldPath *field.Path) field.ErrorList { +func validateCSINodeDriverNodeID(nodeID string, fldPath *field.Path, validationOpts CSINodeValidationOptions) field.ErrorList { allErrs := field.ErrorList{} // nodeID is always required if len(nodeID) == 0 { allErrs = append(allErrs, field.Required(fldPath, nodeID)) } - if len(nodeID) > csiNodeIDMaxLength { + maxLength := csiNodeIDMaxLength + if validationOpts.AllowLongNodeID { + maxLength = csiNodeIDLongerMaxLength + } + if len(nodeID) > maxLength { allErrs = append(allErrs, field.Invalid(fldPath, nodeID, fmt.Sprintf("must be %d characters or less", csiNodeIDMaxLength))) } return allErrs } +// CSINodeLongerID will check if the nodeID is longer than csiNodeIDMaxLength +func CSINodeLongerID(nodeID string) bool { + return len(nodeID) > csiNodeIDMaxLength +} + // validateCSINodeDriverAllocatable tests if Allocatable in CSINodeDriver has valid volume limits. func validateCSINodeDriverAllocatable(a *storage.VolumeNodeResources, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} @@ -362,11 +377,12 @@ func validateCSINodeDriverAllocatable(a *storage.VolumeNodeResources, fldPath *f } // validateCSINodeDriver tests if CSINodeDriver has valid entries -func validateCSINodeDriver(driver storage.CSINodeDriver, driverNamesInSpecs sets.String, fldPath *field.Path) field.ErrorList { +func validateCSINodeDriver(driver storage.CSINodeDriver, driverNamesInSpecs sets.String, fldPath *field.Path, + validationOpts CSINodeValidationOptions) field.ErrorList { allErrs := field.ErrorList{} allErrs = append(allErrs, apivalidation.ValidateCSIDriverName(driver.Name, fldPath.Child("name"))...) - allErrs = append(allErrs, validateCSINodeDriverNodeID(driver.NodeID, fldPath.Child("nodeID"))...) + allErrs = append(allErrs, validateCSINodeDriverNodeID(driver.NodeID, fldPath.Child("nodeID"), validationOpts)...) allErrs = append(allErrs, validateCSINodeDriverAllocatable(driver.Allocatable, fldPath.Child("allocatable"))...) // check for duplicate entries for the same driver in specs diff --git a/pkg/apis/storage/validation/validation_test.go b/pkg/apis/storage/validation/validation_test.go index b26b29d294f..df04e215e7e 100644 --- a/pkg/apis/storage/validation/validation_test.go +++ b/pkg/apis/storage/validation/validation_test.go @@ -46,6 +46,12 @@ var ( }, }, } + longerIDValidateOption = CSINodeValidationOptions{ + AllowLongNodeID: true, + } + shorterIDValidationOption = CSINodeValidationOptions{ + AllowLongNodeID: false, + } ) func TestValidateStorageClass(t *testing.T) { @@ -1026,8 +1032,9 @@ func TestValidateAllowedTopologies(t *testing.T) { func TestCSINodeValidation(t *testing.T) { driverName := "driver-name" driverName2 := "1io.kubernetes-storage-2-csi-driver3" - longName := "my-a-b-c-d-c-f-g-h-i-j-k-l-m-n-o-p-q-r-s-t-u-v-w-x-y-z-ABCDEFGHIJKLMNOPQRSTUVWXYZ-driver" + longName := "my-a-b-c-d-c-f-g-h-i-j-k-l-m-n-o-p-q-r-s-t-u-v-w-x-y-z-ABCDEFGHIJKLMNOPQRSTUVWXYZ-driver" // 88 chars nodeID := "nodeA" + longID := longName + longName // 176 chars successCases := []storage.CSINode{ { // driver name: dot only @@ -1217,10 +1224,29 @@ func TestCSINodeValidation(t *testing.T) { } for _, csiNode := range successCases { - if errs := ValidateCSINode(&csiNode); len(errs) != 0 { + if errs := ValidateCSINode(&csiNode, shorterIDValidationOption); len(errs) != 0 { t.Errorf("expected success: %v", errs) } } + + nodeIDCase := storage.CSINode{ + // node ID length > 128 but < 192 + ObjectMeta: metav1.ObjectMeta{Name: "foo7"}, + Spec: storage.CSINodeSpec{ + Drivers: []storage.CSINodeDriver{ + { + Name: driverName, + NodeID: longID, + TopologyKeys: []string{"company.com/zone1", "company.com/zone2"}, + }, + }, + }, + } + + if errs := ValidateCSINode(&nodeIDCase, longerIDValidateOption); len(errs) != 0 { + t.Errorf("expected success: %v", errs) + } + errorCases := []storage.CSINode{ { // Empty driver name @@ -1414,10 +1440,11 @@ func TestCSINodeValidation(t *testing.T) { }, }, }, + nodeIDCase, } for _, csiNode := range errorCases { - if errs := ValidateCSINode(&csiNode); len(errs) == 0 { + if errs := ValidateCSINode(&csiNode, shorterIDValidationOption); len(errs) == 0 { t.Errorf("Expected failure for test: %v", csiNode) } } @@ -1528,7 +1555,7 @@ func TestCSINodeUpdateValidation(t *testing.T) { } for _, csiNode := range successCases { - if errs := ValidateCSINodeUpdate(&csiNode, &old); len(errs) != 0 { + if errs := ValidateCSINodeUpdate(&csiNode, &old, shorterIDValidationOption); len(errs) != 0 { t.Errorf("expected success: %+v", errs) } } @@ -1651,7 +1678,7 @@ func TestCSINodeUpdateValidation(t *testing.T) { } for _, csiNode := range errorCases { - if errs := ValidateCSINodeUpdate(&csiNode, &old); len(errs) == 0 { + if errs := ValidateCSINodeUpdate(&csiNode, &old, shorterIDValidationOption); len(errs) == 0 { t.Errorf("Expected failure for test: %+v", csiNode) } } diff --git a/pkg/controller/volume/persistentvolume/pv_controller_base.go b/pkg/controller/volume/persistentvolume/pv_controller_base.go index 9e4d8c57f57..8e48d5d8c36 100644 --- a/pkg/controller/volume/persistentvolume/pv_controller_base.go +++ b/pkg/controller/volume/persistentvolume/pv_controller_base.go @@ -366,7 +366,7 @@ func (ctrl *PersistentVolumeController) updateVolumeMigrationAnnotations(volume // updateMigrationAnnotations takes an Annotations map and checks for a // provisioner name using the provisionerKey. It will then add a -// "volume.beta.kubernetes.io/migrated-to" annotation if migration with the CSI +// "pv.kubernetes.io/migrated-to" annotation if migration with the CSI // driver name for that provisioner is "on" based on feature flags, it will also // remove the annotation is migration is "off" for that provisioner in rollback // scenarios. Returns true if the annotations map was modified and false otherwise. diff --git a/pkg/registry/storage/csinode/strategy.go b/pkg/registry/storage/csinode/strategy.go index d7ac8871e4f..6581b1ded07 100644 --- a/pkg/registry/storage/csinode/strategy.go +++ b/pkg/registry/storage/csinode/strategy.go @@ -48,8 +48,13 @@ func (csiNodeStrategy) PrepareForCreate(ctx context.Context, obj runtime.Object) func (csiNodeStrategy) Validate(ctx context.Context, obj runtime.Object) field.ErrorList { csiNode := obj.(*storage.CSINode) - errs := validation.ValidateCSINode(csiNode) - errs = append(errs, validation.ValidateCSINode(csiNode)...) + // in 1.21, on create, set AllowLongNodeID=false + // in 1.22, on create, set AllowLongNodeID=true + validateOptions := validation.CSINodeValidationOptions{ + AllowLongNodeID: false, + } + + errs := validation.ValidateCSINode(csiNode, validateOptions) return errs } @@ -69,8 +74,20 @@ func (csiNodeStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Ob func (csiNodeStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) field.ErrorList { newCSINodeObj := obj.(*storage.CSINode) oldCSINodeObj := old.(*storage.CSINode) - errorList := validation.ValidateCSINode(newCSINodeObj) - return append(errorList, validation.ValidateCSINodeUpdate(newCSINodeObj, oldCSINodeObj)...) + // in 1.21 on update, set AllowLongNodeID to true only if the old object already has a long node ID + // in 1.22 on update, set AllowLongNodeID=true + allowLongNodeID := false + for _, nodeDriver := range oldCSINodeObj.Spec.Drivers { + if validation.CSINodeLongerID(nodeDriver.NodeID) { + allowLongNodeID = true + } + } + validateOptions := validation.CSINodeValidationOptions{ + AllowLongNodeID: allowLongNodeID, + } + + errorList := validation.ValidateCSINode(newCSINodeObj, validateOptions) + return append(errorList, validation.ValidateCSINodeUpdate(newCSINodeObj, oldCSINodeObj, validateOptions)...) } func (csiNodeStrategy) AllowUnconditionalUpdate() bool {