Merge pull request #135455 from carlory/csiNodeIDMaxLength

cleanup csiNodeIDMaxLength
This commit is contained in:
Kubernetes Prow Robot 2025-12-17 22:35:44 -08:00 committed by GitHub
commit 8fcb1fd4cf
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 34 additions and 73 deletions

View file

@ -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

View file

@ -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)
}
}

View file

@ -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.