From 2636aa35e3b9d1cf05ca3d4f96e1a1cfda77a019 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Sat, 14 Dec 2024 09:14:07 -0500 Subject: [PATCH] Require canonicalization of NetworkDeviceData IPs There's no reason to allow non-standard or non-canonical IP values in new APIs. --- pkg/apis/resource/validation/validation.go | 17 ++++++++++++----- .../validation/validation_resourceclaim_test.go | 8 ++++++-- 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/pkg/apis/resource/validation/validation.go b/pkg/apis/resource/validation/validation.go index 2f58721ddc1..24fe8cfbdcb 100644 --- a/pkg/apis/resource/validation/validation.go +++ b/pkg/apis/resource/validation/validation.go @@ -802,17 +802,24 @@ func validateNetworkDeviceData(networkDeviceData *resource.NetworkDeviceData, fl allErrs = append(allErrs, validateSet(networkDeviceData.IPs, maxIPs, func(address string, fldPath *field.Path) field.ErrorList { - return validation.IsValidCIDR(fldPath, address) - }, - func(address string) (string, string) { // reformat CIDR to handle different ways IPs can be written // (e.g. 2001:db8::1/64 == 2001:0db8::1/64) ip, ipNet, err := netutils.ParseCIDRSloppy(address) if err != nil { - return "", "" // will fail at IsValidCIDR + // must fail + return validation.IsValidCIDR(fldPath, address) } maskSize, _ := ipNet.Mask.Size() - return fmt.Sprintf("%s/%d", ip.String(), maskSize), "" + canonical := fmt.Sprintf("%s/%d", ip.String(), maskSize) + if address != canonical { + return field.ErrorList{ + field.Invalid(fldPath, address, fmt.Sprintf("must be in canonical form (%s)", canonical)), + } + } + return nil + }, + func(address string) (string, string) { + return address, "" }, fldPath.Child("ips"))...) return allErrs diff --git a/pkg/apis/resource/validation/validation_resourceclaim_test.go b/pkg/apis/resource/validation/validation_resourceclaim_test.go index c48a5bd3079..68c416a517d 100644 --- a/pkg/apis/resource/validation/validation_resourceclaim_test.go +++ b/pkg/apis/resource/validation/validation_resourceclaim_test.go @@ -1039,7 +1039,7 @@ func TestValidateClaimStatusUpdate(t *testing.T) { NetworkData: &resource.NetworkDeviceData{ IPs: []string{ "2001:db8::1/64", - "2001:0db8::1/64", + "2001:db8::1/64", }, }, }, @@ -1058,6 +1058,8 @@ func TestValidateClaimStatusUpdate(t *testing.T) { field.TooLong(field.NewPath("status", "devices").Index(0).Child("networkData", "interfaceName"), "", interfaceNameMaxLength), field.TooLong(field.NewPath("status", "devices").Index(0).Child("networkData", "hardwareAddress"), "", hardwareAddressMaxLength), field.Invalid(field.NewPath("status", "devices").Index(0).Child("networkData", "ips").Index(0), "300.9.8.0/24", "must be a valid CIDR value, (e.g. 10.9.8.0/24 or 2001:db8::/64)"), + field.Invalid(field.NewPath("status", "devices").Index(0).Child("networkData", "ips").Index(1), "010.009.008.000/24", "must be in canonical form (10.9.8.0/24)"), + field.Invalid(field.NewPath("status", "devices").Index(0).Child("networkData", "ips").Index(2), "2001:0db8::1/64", "must be in canonical form (2001:db8::1/64)"), }, oldClaim: func() *resource.ResourceClaim { return validAllocatedClaim }(), update: func(claim *resource.ResourceClaim) *resource.ResourceClaim { @@ -1071,6 +1073,8 @@ func TestValidateClaimStatusUpdate(t *testing.T) { HardwareAddress: strings.Repeat("x", hardwareAddressMaxLength+1), IPs: []string{ "300.9.8.0/24", + "010.009.008.000/24", + "2001:0db8::1/64", }, }, }, @@ -1168,7 +1172,7 @@ func TestValidateClaimStatusUpdate(t *testing.T) { NetworkData: &resource.NetworkDeviceData{ IPs: []string{ "2001:db8::1/64", - "2001:0db8::1/64", + "2001:db8::1/64", }, }, },