diff --git a/pkg/apis/resource/validation/validation.go b/pkg/apis/resource/validation/validation.go index 5853e79acef..0a1b1ce429f 100644 --- a/pkg/apis/resource/validation/validation.go +++ b/pkg/apis/resource/validation/validation.go @@ -37,7 +37,6 @@ import ( "k8s.io/dynamic-resource-allocation/structured" corevalidation "k8s.io/kubernetes/pkg/apis/core/validation" "k8s.io/kubernetes/pkg/apis/resource" - netutils "k8s.io/utils/net" ) var ( @@ -890,25 +889,7 @@ func validateNetworkDeviceData(networkDeviceData *resource.NetworkDeviceData, fl allErrs = append(allErrs, validateSet(networkDeviceData.IPs, resource.NetworkDeviceDataMaxIPs, func(address string, fldPath *field.Path) field.ErrorList { - // 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 { - // must fail - return validation.IsValidCIDR(fldPath, address) - } - maskSize, _ := ipNet.Mask.Size() - 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 validation.IsValidInterfaceAddress(fldPath, address) + }, stringKey, 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 1a243920e4f..7224fc82f1f 100644 --- a/pkg/apis/resource/validation/validation_resourceclaim_test.go +++ b/pkg/apis/resource/validation/validation_resourceclaim_test.go @@ -1257,9 +1257,9 @@ func TestValidateClaimStatusUpdate(t *testing.T) { wantFailures: field.ErrorList{ field.TooLong(field.NewPath("status", "devices").Index(0).Child("networkData", "interfaceName"), "", resource.NetworkDeviceDataInterfaceNameMaxLength), field.TooLong(field.NewPath("status", "devices").Index(0).Child("networkData", "hardwareAddress"), "", resource.NetworkDeviceDataHardwareAddressMaxLength), - 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)"), + field.Invalid(field.NewPath("status", "devices").Index(0).Child("networkData", "ips").Index(0), "300.9.8.0/24", "must be a valid address in CIDR form, (e.g. 10.9.8.7/24 or 2001:db8::1/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 { @@ -1390,7 +1390,7 @@ func TestValidateClaimStatusUpdate(t *testing.T) { wantFailures: field.ErrorList{ field.TooLong(field.NewPath("status", "devices").Index(0).Child("networkData", "interfaceName"), "", resource.NetworkDeviceDataInterfaceNameMaxLength), field.TooLong(field.NewPath("status", "devices").Index(0).Child("networkData", "hardwareAddress"), "", resource.NetworkDeviceDataHardwareAddressMaxLength), - 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(0), "300.9.8.0/24", "must be a valid address in CIDR form, (e.g. 10.9.8.7/24 or 2001:db8::1/64)"), }, oldClaim: func() *resource.ResourceClaim { return validAllocatedClaim }(), update: func(claim *resource.ResourceClaim) *resource.ResourceClaim { diff --git a/staging/src/k8s.io/apimachinery/pkg/util/validation/ip.go b/staging/src/k8s.io/apimachinery/pkg/util/validation/ip.go index f073c3668ca..ae9497ff87d 100644 --- a/staging/src/k8s.io/apimachinery/pkg/util/validation/ip.go +++ b/staging/src/k8s.io/apimachinery/pkg/util/validation/ip.go @@ -18,6 +18,7 @@ package validation import ( "fmt" + "net" "net/netip" "k8s.io/apimachinery/pkg/util/validation/field" @@ -137,3 +138,30 @@ func GetWarningsForCIDR(fldPath *field.Path, value string) []string { return warnings } + +// IsValidInterfaceAddress tests that the argument is a valid "ifaddr"-style CIDR value in +// canonical form (e.g., "192.168.1.5/24", with a complete IP address and associated +// subnet length). Use IsValidCIDR for "subnet"/"mask"-style CIDR values (e.g., +// "192.168.1.0/24"). +func IsValidInterfaceAddress(fldPath *field.Path, value string) field.ErrorList { + var allErrors field.ErrorList + ip, ipnet, err := netutils.ParseCIDRSloppy(value) + if err != nil { + allErrors = append(allErrors, field.Invalid(fldPath, value, "must be a valid address in CIDR form, (e.g. 10.9.8.7/24 or 2001:db8::1/64)")) + return allErrors + } + + // The canonical form of `value` is not `ipnet.String()`, because `ipnet` doesn't + // include the bits after the prefix. We need to construct the canonical form + // ourselves from `ip` and `ipnet.Mask`. + maskSize, _ := ipnet.Mask.Size() + if netutils.IsIPv4(ip) && maskSize > net.IPv4len*8 { + // "::ffff:192.168.0.1/120" -> "192.168.0.1/24" + maskSize -= (net.IPv6len - net.IPv4len) * 8 + } + canonical := fmt.Sprintf("%s/%d", ip.String(), maskSize) + if value != canonical { + allErrors = append(allErrors, field.Invalid(fldPath, value, fmt.Sprintf("must be in canonical form (%q)", canonical))) + } + return allErrors +} diff --git a/staging/src/k8s.io/apimachinery/pkg/util/validation/ip_test.go b/staging/src/k8s.io/apimachinery/pkg/util/validation/ip_test.go index 6fb5d4f15cc..9ff4f9ca0f9 100644 --- a/staging/src/k8s.io/apimachinery/pkg/util/validation/ip_test.go +++ b/staging/src/k8s.io/apimachinery/pkg/util/validation/ip_test.go @@ -450,3 +450,106 @@ func TestGetWarningsForCIDR(t *testing.T) { }) } } + +func TestIsValidInterfaceAddress(t *testing.T) { + for _, tc := range []struct { + name string + in string + err string + }{ + // GOOD VALUES + { + name: "ipv4", + in: "1.2.3.4/24", + }, + { + name: "ipv4, single IP", + in: "1.1.1.1/32", + }, + { + name: "ipv6", + in: "2001:4860:4860::1/48", + }, + { + name: "ipv6, single IP", + in: "::1/128", + }, + + // BAD VALUES + { + name: "empty string", + in: "", + err: "must be a valid address in CIDR form", + }, + { + name: "junk", + in: "aaaaaaa", + err: "must be a valid address in CIDR form", + }, + { + name: "IP address", + in: "1.2.3.4", + err: "must be a valid address in CIDR form", + }, + { + name: "partial URL", + in: "192.168.0.1/healthz", + err: "must be a valid address in CIDR form", + }, + { + name: "partial URL 2", + in: "192.168.0.1/0/99", + err: "must be a valid address in CIDR form", + }, + { + name: "negative prefix length", + in: "192.168.0.0/-16", + err: "must be a valid address in CIDR form", + }, + { + name: "prefix length with sign", + in: "192.168.0.0/+16", + err: "must be a valid address in CIDR form", + }, + { + name: "ipv6 non-canonical", + in: "2001:0:0:0::0BCD/64", + err: `must be in canonical form ("2001::bcd/64")`, + }, + { + name: "ipv4 with leading 0s", + in: "1.1.01.002/24", + err: `must be in canonical form ("1.1.1.2/24")`, + }, + { + name: "ipv4-in-ipv6 with ipv4-sized prefix", + in: "::ffff:1.1.1.1/24", + err: `must be in canonical form ("1.1.1.1/24")`, + }, + { + name: "ipv4-in-ipv6 with ipv6-sized prefix", + in: "::ffff:1.1.1.1/120", + err: `must be in canonical form ("1.1.1.1/24")`, + }, + { + name: "prefix length with leading 0s", + in: "192.168.0.5/016", + err: `must be in canonical form ("192.168.0.5/16")`, + }, + } { + t.Run(tc.name, func(t *testing.T) { + errs := IsValidInterfaceAddress(field.NewPath(""), tc.in) + if tc.err == "" { + if len(errs) != 0 { + t.Errorf("expected %q to be valid but got: %v", tc.in, errs) + } + } else { + if len(errs) != 1 { + t.Errorf("expected %q to have 1 error but got: %v", tc.in, errs) + } else if !strings.Contains(errs[0].Detail, tc.err) { + t.Errorf("expected error for %q to contain %q but got: %q", tc.in, tc.err, errs[0].Detail) + } + } + }) + } +}