diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go index bcbf344963a..686d184781f 100644 --- a/pkg/apis/core/validation/validation.go +++ b/pkg/apis/core/validation/validation.go @@ -5870,9 +5870,7 @@ func ValidateNode(node *core.Node) field.ErrorList { // all PodCIDRs should be valid ones for idx, value := range node.Spec.PodCIDRs { - if _, err := ValidateCIDR(value); err != nil { - allErrs = append(allErrs, field.Invalid(podCIDRsField.Index(idx), node.Spec.PodCIDRs, "must be valid CIDR")) - } + allErrs = append(allErrs, validation.IsValidCIDR(podCIDRsField.Index(idx), value)...) } // if more than PodCIDR then @@ -7300,15 +7298,6 @@ func validateVolumeNodeAffinity(nodeAffinity *core.VolumeNodeAffinity, fldPath * return true, allErrs } -// ValidateCIDR validates whether a CIDR matches the conventions expected by net.ParseCIDR -func ValidateCIDR(cidr string) (*net.IPNet, error) { - _, net, err := netutils.ParseCIDRSloppy(cidr) - if err != nil { - return nil, err - } - return net, nil -} - func IsDecremented(update, old *int32) bool { if update == nil && old != nil { return true diff --git a/pkg/apis/networking/validation/validation.go b/pkg/apis/networking/validation/validation.go index 581c72e2819..0750851dd85 100644 --- a/pkg/apis/networking/validation/validation.go +++ b/pkg/apis/networking/validation/validation.go @@ -216,27 +216,30 @@ func ValidateNetworkPolicyUpdate(update, old *networking.NetworkPolicy, opts Net // ValidateIPBlock validates a cidr and the except fields of an IpBlock NetworkPolicyPeer func ValidateIPBlock(ipb *networking.IPBlock, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} - if len(ipb.CIDR) == 0 || ipb.CIDR == "" { + if ipb.CIDR == "" { allErrs = append(allErrs, field.Required(fldPath.Child("cidr"), "")) return allErrs } - cidrIPNet, err := apivalidation.ValidateCIDR(ipb.CIDR) + allErrs = append(allErrs, validation.IsValidCIDR(fldPath.Child("cidr"), ipb.CIDR)...) + _, cidrIPNet, err := netutils.ParseCIDRSloppy(ipb.CIDR) if err != nil { - allErrs = append(allErrs, field.Invalid(fldPath.Child("cidr"), ipb.CIDR, "not a valid CIDR")) + // Implies validation would have failed so we already added errors for it. return allErrs } - exceptCIDR := ipb.Except - for i, exceptIP := range exceptCIDR { + + for i, exceptCIDRStr := range ipb.Except { exceptPath := fldPath.Child("except").Index(i) - exceptCIDR, err := apivalidation.ValidateCIDR(exceptIP) + allErrs = append(allErrs, validation.IsValidCIDR(exceptPath, exceptCIDRStr)...) + _, exceptCIDR, err := netutils.ParseCIDRSloppy(exceptCIDRStr) if err != nil { - allErrs = append(allErrs, field.Invalid(exceptPath, exceptIP, "not a valid CIDR")) - return allErrs + // Implies validation would have failed so we already added errors for it. + continue } + cidrMaskLen, _ := cidrIPNet.Mask.Size() exceptMaskLen, _ := exceptCIDR.Mask.Size() if !cidrIPNet.Contains(exceptCIDR.IP) || cidrMaskLen >= exceptMaskLen { - allErrs = append(allErrs, field.Invalid(exceptPath, exceptIP, "must be a strict subset of `cidr`")) + allErrs = append(allErrs, field.Invalid(exceptPath, exceptCIDRStr, "must be a strict subset of `cidr`")) } } return allErrs diff --git a/staging/src/k8s.io/apimachinery/pkg/util/validation/validation.go b/staging/src/k8s.io/apimachinery/pkg/util/validation/validation.go index 0eb33e68aaf..94c3fdb77f4 100644 --- a/staging/src/k8s.io/apimachinery/pkg/util/validation/validation.go +++ b/staging/src/k8s.io/apimachinery/pkg/util/validation/validation.go @@ -378,6 +378,16 @@ func IsValidIPv6Address(fldPath *field.Path, value string) field.ErrorList { return allErrors } +// IsValidCIDR tests that the argument is a valid CIDR value. +func IsValidCIDR(fldPath *field.Path, value string) field.ErrorList { + var allErrors field.ErrorList + _, _, err := netutils.ParseCIDRSloppy(value) + if err != nil { + allErrors = append(allErrors, field.Invalid(fldPath, value, "must be a valid CIDR value, (e.g. 10.9.8.0/24 or 2001:db8::/64)")) + } + return allErrors +} + const percentFmt string = "[0-9]+%" const percentErrMsg string = "a valid percent string must be a numeric string followed by an ending '%'" diff --git a/staging/src/k8s.io/apimachinery/pkg/util/validation/validation_test.go b/staging/src/k8s.io/apimachinery/pkg/util/validation/validation_test.go index 2ce11d560f7..da62083db72 100644 --- a/staging/src/k8s.io/apimachinery/pkg/util/validation/validation_test.go +++ b/staging/src/k8s.io/apimachinery/pkg/util/validation/validation_test.go @@ -496,6 +496,128 @@ func TestIsValidIP(t *testing.T) { } } +func TestIsValidCIDR(t *testing.T) { + for _, tc := range []struct { + name string + in string + err string + }{ + // GOOD VALUES + { + name: "ipv4", + in: "1.0.0.0/8", + }, + { + name: "ipv4, all IPs", + in: "0.0.0.0/0", + }, + { + name: "ipv4, single IP", + in: "1.1.1.1/32", + }, + { + name: "ipv6", + in: "2001:4860:4860::/48", + }, + { + name: "ipv6, all IPs", + in: "::/0", + }, + { + name: "ipv6, single IP", + in: "::1/128", + }, + + // GOOD, THOUGH NON-CANONICAL, VALUES + { + name: "ipv6, extra 0s (non-canonical)", + in: "2a00:79e0:2:0::/64", + }, + { + name: "ipv6, capital letters (non-canonical)", + in: "2001:DB8::/64", + }, + + // BAD VALUES WE CURRENTLY CONSIDER GOOD + { + name: "ipv4 with leading 0s", + in: "1.1.01.0/24", + }, + { + name: "ipv4-in-ipv6 with ipv4-sized prefix", + in: "::ffff:1.1.1.0/24", + }, + { + name: "ipv4-in-ipv6 with ipv6-sized prefix", + in: "::ffff:1.1.1.0/120", + }, + { + name: "ipv4 with bits past prefix", + in: "1.2.3.4/24", + }, + { + name: "ipv6 with bits past prefix", + in: "2001:db8::1/64", + }, + { + name: "prefix length with leading 0s", + in: "192.168.0.0/016", + }, + + // BAD VALUES + { + name: "empty string", + in: "", + err: "must be a valid CIDR value", + }, + { + name: "junk", + in: "aaaaaaa", + err: "must be a valid CIDR value", + }, + { + name: "IP address", + in: "1.2.3.4", + err: "must be a valid CIDR value", + }, + { + name: "partial URL", + in: "192.168.0.1/healthz", + err: "must be a valid CIDR value", + }, + { + name: "partial URL 2", + in: "192.168.0.1/0/99", + err: "must be a valid CIDR value", + }, + { + name: "negative prefix length", + in: "192.168.0.0/-16", + err: "must be a valid CIDR value", + }, + { + name: "prefix length with sign", + in: "192.168.0.0/+16", + err: "must be a valid CIDR value", + }, + } { + t.Run(tc.name, func(t *testing.T) { + errs := IsValidCIDR(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) + } + } + }) + } +} + func TestIsHTTPHeaderName(t *testing.T) { goodValues := []string{ // Common ones