Skip to content

Commit fc4bb4f

Browse files
danwinshipdanwinship
committed
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.
1 parent f79bccf commit fc4bb4f

File tree

4 files changed

+137
-25
lines changed

4 files changed

+137
-25
lines changed

‎pkg/apis/resource/validation/validation.go‎

Lines changed: 2 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@ import (
3737
"k8s.io/dynamic-resource-allocation/structured"
3838
corevalidation "k8s.io/kubernetes/pkg/apis/core/validation"
3939
"k8s.io/kubernetes/pkg/apis/resource"
40-
netutils "k8s.io/utils/net"
4140
)
4241

4342
var (
@@ -890,25 +889,7 @@ func validateNetworkDeviceData(networkDeviceData *resource.NetworkDeviceData, fl
890889

891890
allErrs = append(allErrs, validateSet(networkDeviceData.IPs, resource.NetworkDeviceDataMaxIPs,
892891
func(address string, fldPath *field.Path) field.ErrorList {
893-
// reformat CIDR to handle different ways IPs can be written
894-
// (e.g. 2001:db8::1/64 == 2001:0db8::1/64)
895-
ip, ipNet, err := netutils.ParseCIDRSloppy(address)
896-
if err != nil {
897-
// must fail
898-
return validation.IsValidCIDR(fldPath, address)
899-
}
900-
maskSize, _ := ipNet.Mask.Size()
901-
canonical := fmt.Sprintf("%s/%d", ip.String(), maskSize)
902-
if address != canonical {
903-
return field.ErrorList{
904-
field.Invalid(fldPath, address, fmt.Sprintf("must be in canonical form (%s)", canonical)),
905-
}
906-
}
907-
return nil
908-
},
909-
func(address string) (string, string) {
910-
return address, ""
911-
},
912-
fldPath.Child("ips"))...)
892+
return validation.IsValidInterfaceAddress(fldPath, address)
893+
}, stringKey, fldPath.Child("ips"))...)
913894
return allErrs
914895
}

‎pkg/apis/resource/validation/validation_resourceclaim_test.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1257,9 +1257,9 @@ func TestValidateClaimStatusUpdate(t *testing.T) {
12571257
wantFailures: field.ErrorList{
12581258
field.TooLong(field.NewPath("status", "devices").Index(0).Child("networkData", "interfaceName"), "", resource.NetworkDeviceDataInterfaceNameMaxLength),
12591259
field.TooLong(field.NewPath("status", "devices").Index(0).Child("networkData", "hardwareAddress"), "", resource.NetworkDeviceDataHardwareAddressMaxLength),
1260-
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)"),
1261-
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)"),
1262-
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)"),
1260+
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)"),
1261+
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\")"),
1262+
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\")"),
12631263
},
12641264
oldClaim: func() *resource.ResourceClaim { return validAllocatedClaim }(),
12651265
update: func(claim *resource.ResourceClaim) *resource.ResourceClaim {
@@ -1390,7 +1390,7 @@ func TestValidateClaimStatusUpdate(t *testing.T) {
13901390
wantFailures: field.ErrorList{
13911391
field.TooLong(field.NewPath("status", "devices").Index(0).Child("networkData", "interfaceName"), "", resource.NetworkDeviceDataInterfaceNameMaxLength),
13921392
field.TooLong(field.NewPath("status", "devices").Index(0).Child("networkData", "hardwareAddress"), "", resource.NetworkDeviceDataHardwareAddressMaxLength),
1393-
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)"),
1393+
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)"),
13941394
},
13951395
oldClaim: func() *resource.ResourceClaim { return validAllocatedClaim }(),
13961396
update: func(claim *resource.ResourceClaim) *resource.ResourceClaim {

‎staging/src/k8s.io/apimachinery/pkg/util/validation/ip.go‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package validation
1818

1919
import (
2020
"fmt"
21+
"net"
2122
"net/netip"
2223

2324
"k8s.io/apimachinery/pkg/util/validation/field"
@@ -137,3 +138,30 @@ func GetWarningsForCIDR(fldPath *field.Path, value string) []string {
137138

138139
return warnings
139140
}
141+
142+
// IsValidInterfaceAddress tests that the argument is a valid "ifaddr"-style CIDR value in
143+
// canonical form (e.g., "192.168.1.5/24", with a complete IP address and associated
144+
// subnet length). Use IsValidCIDR for "subnet"/"mask"-style CIDR values (e.g.,
145+
// "192.168.1.0/24").
146+
func IsValidInterfaceAddress(fldPath *field.Path, value string) field.ErrorList {
147+
var allErrors field.ErrorList
148+
ip, ipnet, err := netutils.ParseCIDRSloppy(value)
149+
if err != nil {
150+
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)"))
151+
return allErrors
152+
}
153+
154+
// The canonical form of `value` is not `ipnet.String()`, because `ipnet` doesn't
155+
// include the bits after the prefix. We need to construct the canonical form
156+
// ourselves from `ip` and `ipnet.Mask`.
157+
maskSize, _ := ipnet.Mask.Size()
158+
if netutils.IsIPv4(ip) && maskSize > net.IPv4len*8 {
159+
// "::ffff:192.168.0.1/120" -> "192.168.0.1/24"
160+
maskSize -= (net.IPv6len - net.IPv4len) * 8
161+
}
162+
canonical := fmt.Sprintf("%s/%d", ip.String(), maskSize)
163+
if value != canonical {
164+
allErrors = append(allErrors, field.Invalid(fldPath, value, fmt.Sprintf("must be in canonical form (%q)", canonical)))
165+
}
166+
return allErrors
167+
}

‎staging/src/k8s.io/apimachinery/pkg/util/validation/ip_test.go‎

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -450,3 +450,106 @@ func TestGetWarningsForCIDR(t *testing.T) {
450450
})
451451
}
452452
}
453+
454+
func TestIsValidInterfaceAddress(t *testing.T) {
455+
for _, tc := range []struct {
456+
name string
457+
in string
458+
err string
459+
}{
460+
// GOOD VALUES
461+
{
462+
name: "ipv4",
463+
in: "1.2.3.4/24",
464+
},
465+
{
466+
name: "ipv4, single IP",
467+
in: "1.1.1.1/32",
468+
},
469+
{
470+
name: "ipv6",
471+
in: "2001:4860:4860::1/48",
472+
},
473+
{
474+
name: "ipv6, single IP",
475+
in: "::1/128",
476+
},
477+
478+
// BAD VALUES
479+
{
480+
name: "empty string",
481+
in: "",
482+
err: "must be a valid address in CIDR form",
483+
},
484+
{
485+
name: "junk",
486+
in: "aaaaaaa",
487+
err: "must be a valid address in CIDR form",
488+
},
489+
{
490+
name: "IP address",
491+
in: "1.2.3.4",
492+
err: "must be a valid address in CIDR form",
493+
},
494+
{
495+
name: "partial URL",
496+
in: "192.168.0.1/healthz",
497+
err: "must be a valid address in CIDR form",
498+
},
499+
{
500+
name: "partial URL 2",
501+
in: "192.168.0.1/0/99",
502+
err: "must be a valid address in CIDR form",
503+
},
504+
{
505+
name: "negative prefix length",
506+
in: "192.168.0.0/-16",
507+
err: "must be a valid address in CIDR form",
508+
},
509+
{
510+
name: "prefix length with sign",
511+
in: "192.168.0.0/+16",
512+
err: "must be a valid address in CIDR form",
513+
},
514+
{
515+
name: "ipv6 non-canonical",
516+
in: "2001:0:0:0::0BCD/64",
517+
err: `must be in canonical form ("2001::bcd/64")`,
518+
},
519+
{
520+
name: "ipv4 with leading 0s",
521+
in: "1.1.01.002/24",
522+
err: `must be in canonical form ("1.1.1.2/24")`,
523+
},
524+
{
525+
name: "ipv4-in-ipv6 with ipv4-sized prefix",
526+
in: "::ffff:1.1.1.1/24",
527+
err: `must be in canonical form ("1.1.1.1/24")`,
528+
},
529+
{
530+
name: "ipv4-in-ipv6 with ipv6-sized prefix",
531+
in: "::ffff:1.1.1.1/120",
532+
err: `must be in canonical form ("1.1.1.1/24")`,
533+
},
534+
{
535+
name: "prefix length with leading 0s",
536+
in: "192.168.0.5/016",
537+
err: `must be in canonical form ("192.168.0.5/16")`,
538+
},
539+
} {
540+
t.Run(tc.name, func(t *testing.T) {
541+
errs := IsValidInterfaceAddress(field.NewPath(""), tc.in)
542+
if tc.err == "" {
543+
if len(errs) != 0 {
544+
t.Errorf("expected %q to be valid but got: %v", tc.in, errs)
545+
}
546+
} else {
547+
if len(errs) != 1 {
548+
t.Errorf("expected %q to have 1 error but got: %v", tc.in, errs)
549+
} else if !strings.Contains(errs[0].Detail, tc.err) {
550+
t.Errorf("expected error for %q to contain %q but got: %q", tc.in, tc.err, errs[0].Detail)
551+
}
552+
}
553+
})
554+
}
555+
}

0 commit comments

Comments
 (0)