diff --git a/pkg/apis/storage/validation/validation.go b/pkg/apis/storage/validation/validation.go index e103b613aae..751e0a3074f 100644 --- a/pkg/apis/storage/validation/validation.go +++ b/pkg/apis/storage/validation/validation.go @@ -44,15 +44,9 @@ const ( maxAttachedVolumeMetadataSize = 256 * (1 << 10) // 256 kB maxVolumeErrorMessageSize = 1024 - csiNodeIDMaxLength = 192 - csiNodeIDLongerMaxLength = 256 + csiNodeIDMaxLength = 256 ) -// 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")) @@ -303,15 +297,15 @@ func validateAllowedTopologies(topologies []api.TopologySelectorTerm, fldPath *f } // ValidateCSINode validates a CSINode. -func ValidateCSINode(csiNode *storage.CSINode, validationOpts CSINodeValidationOptions) field.ErrorList { +func ValidateCSINode(csiNode *storage.CSINode) field.ErrorList { allErrs := apivalidation.ValidateObjectMeta(&csiNode.ObjectMeta, false, apivalidation.ValidateNodeName, field.NewPath("metadata")) - allErrs = append(allErrs, validateCSINodeSpec(&csiNode.Spec, field.NewPath("spec"), validationOpts)...) + allErrs = append(allErrs, validateCSINodeSpec(&csiNode.Spec, field.NewPath("spec"))...) return allErrs } // ValidateCSINodeUpdate validates a CSINode. -func ValidateCSINodeUpdate(new, old *storage.CSINode, validationOpts CSINodeValidationOptions) field.ErrorList { - allErrs := ValidateCSINode(new, validationOpts) +func ValidateCSINodeUpdate(new, old *storage.CSINode) field.ErrorList { + allErrs := ValidateCSINode(new) // Validate modifying fields inside an existing CSINodeDriver entry for _, oldDriver := range old.Spec.Drivers { @@ -339,39 +333,35 @@ func ValidateCSINodeUpdate(new, old *storage.CSINode, validationOpts CSINodeVali } // ValidateCSINodeSpec tests that the specified CSINodeSpec has valid data. -func validateCSINodeSpec( - spec *storage.CSINodeSpec, fldPath *field.Path, validationOpts CSINodeValidationOptions) field.ErrorList { +func validateCSINodeSpec(spec *storage.CSINodeSpec, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} - allErrs = append(allErrs, validateCSINodeDrivers(spec.Drivers, fldPath.Child("drivers"), validationOpts)...) + allErrs = append(allErrs, validateCSINodeDrivers(spec.Drivers, fldPath.Child("drivers"))...) return allErrs } // ValidateCSINodeDrivers tests that the specified CSINodeDrivers have valid data. -func validateCSINodeDrivers(drivers []storage.CSINodeDriver, fldPath *field.Path, validationOpts CSINodeValidationOptions) field.ErrorList { +func validateCSINodeDrivers(drivers []storage.CSINodeDriver, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} driverNamesInSpecs := sets.New[string]() for i, driver := range drivers { idxPath := fldPath.Index(i) - allErrs = append(allErrs, validateCSINodeDriver(driver, driverNamesInSpecs, idxPath, validationOpts)...) + allErrs = append(allErrs, validateCSINodeDriver(driver, driverNamesInSpecs, idxPath)...) } return allErrs } // validateCSINodeDriverNodeID tests if Name in CSINodeDriver is a valid node id. -func validateCSINodeDriverNodeID(nodeID string, fldPath *field.Path, validationOpts CSINodeValidationOptions) field.ErrorList { +func validateCSINodeDriverNodeID(nodeID string, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} // nodeID is always required if len(nodeID) == 0 { allErrs = append(allErrs, field.Required(fldPath, nodeID)) } - 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", maxLength))) + + if len(nodeID) > csiNodeIDMaxLength { + allErrs = append(allErrs, field.Invalid(fldPath, nodeID, fmt.Sprintf("must be %d characters or less", csiNodeIDMaxLength))) } return allErrs } @@ -389,12 +379,11 @@ func validateCSINodeDriverAllocatable(a *storage.VolumeNodeResources, fldPath *f } // validateCSINodeDriver tests if CSINodeDriver has valid entries -func validateCSINodeDriver(driver storage.CSINodeDriver, driverNamesInSpecs sets.Set[string], fldPath *field.Path, - validationOpts CSINodeValidationOptions) field.ErrorList { +func validateCSINodeDriver(driver storage.CSINodeDriver, driverNamesInSpecs sets.Set[string], fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} allErrs = append(allErrs, apivalidation.ValidateCSIDriverName(driver.Name, fldPath.Child("name"))...) - allErrs = append(allErrs, validateCSINodeDriverNodeID(driver.NodeID, fldPath.Child("nodeID"), validationOpts)...) + allErrs = append(allErrs, validateCSINodeDriverNodeID(driver.NodeID, fldPath.Child("nodeID"))...) 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 61b0c99d7de..ec493b8429f 100644 --- a/pkg/apis/storage/validation/validation_test.go +++ b/pkg/apis/storage/validation/validation_test.go @@ -46,12 +46,6 @@ var ( }, }, } - longerIDValidateOption = CSINodeValidationOptions{ - AllowLongNodeID: true, - } - shorterIDValidationOption = CSINodeValidationOptions{ - AllowLongNodeID: false, - } ) func TestValidateStorageClass(t *testing.T) { @@ -936,9 +930,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" // 88 chars + longName := strings.Repeat("a", 88) nodeID := "nodeA" - longID := longName + longName + "abcdefghijklmnopqrstuvwxyz" // 202 chars + longID := strings.Repeat("a", 257) successCases := []storage.CSINode{{ // driver name: dot only ObjectMeta: metav1.ObjectMeta{Name: "foo1"}, @@ -1085,27 +1079,11 @@ func TestCSINodeValidation(t *testing.T) { }} for _, csiNode := range successCases { - if errs := ValidateCSINode(&csiNode, shorterIDValidationOption); len(errs) != 0 { + if errs := ValidateCSINode(&csiNode); 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 ObjectMeta: metav1.ObjectMeta{Name: "foo1"}, @@ -1254,21 +1232,26 @@ func TestCSINodeValidation(t *testing.T) { TopologyKeys: []string{"Company.Com/zone1", "company.com/zone2"}, }}, }, + }, { + ObjectMeta: metav1.ObjectMeta{Name: "foo15"}, + Spec: storage.CSINodeSpec{ + Drivers: []storage.CSINodeDriver{{ + Name: driverName, + NodeID: longID, + TopologyKeys: []string{"company.com/zone1", "company.com/zone2"}, + }}, + }, }, - nodeIDCase, } for _, csiNode := range errorCases { - if errs := ValidateCSINode(&csiNode, shorterIDValidationOption); len(errs) == 0 { + if errs := ValidateCSINode(&csiNode); len(errs) == 0 { t.Errorf("Expected failure for test: %v", csiNode) } } } func TestCSINodeUpdateValidation(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" nodeID := "nodeA" // Test with feature gate disabled featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.MutableCSINodeAllocatableCount, false) @@ -1351,7 +1334,7 @@ func TestCSINodeUpdateValidation(t *testing.T) { }} for _, csiNode := range successCases { - if errs := ValidateCSINodeUpdate(&csiNode, &old, shorterIDValidationOption); len(errs) != 0 { + if errs := ValidateCSINodeUpdate(&csiNode, &old); len(errs) != 0 { t.Errorf("expected success: %+v", errs) } } @@ -1449,7 +1432,7 @@ func TestCSINodeUpdateValidation(t *testing.T) { }} for _, csiNode := range errorCases { - if errs := ValidateCSINodeUpdate(&csiNode, &old, shorterIDValidationOption); len(errs) == 0 { + if errs := ValidateCSINodeUpdate(&csiNode, &old); len(errs) == 0 { t.Errorf("Expected failure for test: %+v", csiNode) } } @@ -1506,13 +1489,13 @@ func TestCSINodeUpdateValidation(t *testing.T) { }} for _, csiNode := range errorCases { - if errs := ValidateCSINodeUpdate(&csiNode, &old, shorterIDValidationOption); len(errs) == 0 { + if errs := ValidateCSINodeUpdate(&csiNode, &old); len(errs) == 0 { t.Errorf("Expected failure for test: %+v", csiNode) } } for _, csiNode := range successCases { - if errs := ValidateCSINodeUpdate(&csiNode, &old, shorterIDValidationOption); len(errs) != 0 { + if errs := ValidateCSINodeUpdate(&csiNode, &old); len(errs) != 0 { t.Errorf("expected success with feature gate enabled: %+v", errs) } } diff --git a/pkg/registry/storage/csinode/strategy.go b/pkg/registry/storage/csinode/strategy.go index 5341f1bd36d..3caf9395518 100644 --- a/pkg/registry/storage/csinode/strategy.go +++ b/pkg/registry/storage/csinode/strategy.go @@ -47,13 +47,7 @@ func (csiNodeStrategy) PrepareForCreate(ctx context.Context, obj runtime.Object) func (csiNodeStrategy) Validate(ctx context.Context, obj runtime.Object) field.ErrorList { csiNode := obj.(*storage.CSINode) - validateOptions := validation.CSINodeValidationOptions{ - AllowLongNodeID: true, - } - - errs := validation.ValidateCSINode(csiNode, validateOptions) - - return errs + return validation.ValidateCSINode(csiNode) } // WarningsOnCreate returns warnings for the creation of the given object. @@ -74,12 +68,7 @@ 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) - validateOptions := validation.CSINodeValidationOptions{ - AllowLongNodeID: true, - } - - errorList := validation.ValidateCSINode(newCSINodeObj, validateOptions) - return append(errorList, validation.ValidateCSINodeUpdate(newCSINodeObj, oldCSINodeObj, validateOptions)...) + return validation.ValidateCSINodeUpdate(newCSINodeObj, oldCSINodeObj) } // WarningsOnUpdate returns warnings for the given update.