Add validation.IsValidInterfaceAddress

Split "ifaddr"-style ("192.168.1.5/24") validation out of IsValidCIDR.
Since there is currently only one field that uses this format, and it
already requires canonical form, IsValidInterfaceAddress requires
canonical form unconditionally.
This commit is contained in:
Dan Winship 2025-02-28 16:16:51 -05:00
parent f79bccf4d9
commit fc4bb4fdb9
4 changed files with 137 additions and 25 deletions

View File

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

View File

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

View File

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

View File

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