diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go index 58274dc86d6..bcbf344963a 100644 --- a/pkg/apis/core/validation/validation.go +++ b/pkg/apis/core/validation/validation.go @@ -3720,9 +3720,7 @@ func validatePodDNSConfig(dnsConfig *core.PodDNSConfig, dnsPolicy *core.DNSPolic allErrs = append(allErrs, field.Invalid(fldPath.Child("nameservers"), dnsConfig.Nameservers, fmt.Sprintf("must not have more than %v nameservers", MaxDNSNameservers))) } for i, ns := range dnsConfig.Nameservers { - if ip := netutils.ParseIPSloppy(ns); ip == nil { - allErrs = append(allErrs, field.Invalid(fldPath.Child("nameservers").Index(i), ns, "must be valid IP address")) - } + allErrs = append(allErrs, validation.IsValidIP(fldPath.Child("nameservers").Index(i), ns)...) } // Validate searches. if len(dnsConfig.Searches) > MaxDNSSearchPaths { @@ -3889,12 +3887,10 @@ func validateOnlyDeletedSchedulingGates(newGates, oldGates []core.PodSchedulingG func ValidateHostAliases(hostAliases []core.HostAlias, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} - for _, hostAlias := range hostAliases { - if ip := netutils.ParseIPSloppy(hostAlias.IP); ip == nil { - allErrs = append(allErrs, field.Invalid(fldPath.Child("ip"), hostAlias.IP, "must be valid IP address")) - } - for _, hostname := range hostAlias.Hostnames { - allErrs = append(allErrs, ValidateDNS1123Subdomain(hostname, fldPath.Child("hostnames"))...) + for i, hostAlias := range hostAliases { + allErrs = append(allErrs, validation.IsValidIP(fldPath.Index(i).Child("ip"), hostAlias.IP)...) + for j, hostname := range hostAlias.Hostnames { + allErrs = append(allErrs, ValidateDNS1123Subdomain(hostname, fldPath.Index(i).Child("hostnames").Index(j))...) } } return allErrs @@ -4031,9 +4027,7 @@ func validatePodIPs(pod *core.Pod) field.ErrorList { // all PodIPs must be valid IPs for i, podIP := range pod.Status.PodIPs { - for _, msg := range validation.IsValidIP(podIP.IP) { - allErrs = append(allErrs, field.Invalid(podIPsField.Index(i), podIP.IP, msg)) - } + allErrs = append(allErrs, validation.IsValidIP(podIPsField.Index(i), podIP.IP)...) } // if we have more than one Pod.PodIP then @@ -4085,9 +4079,7 @@ func validateHostIPs(pod *core.Pod) field.ErrorList { // all HostPs must be valid IPs for i, hostIP := range pod.Status.HostIPs { - for _, msg := range validation.IsValidIP(hostIP.IP) { - allErrs = append(allErrs, field.Invalid(hostIPsField.Index(i), hostIP.IP, msg)) - } + allErrs = append(allErrs, validation.IsValidIP(hostIPsField.Index(i), hostIP.IP)...) } // if we have more than one Pod.HostIP then @@ -5403,10 +5395,8 @@ func ValidateService(service *core.Service) field.ErrorList { ipPath := specPath.Child("externalIPs") for i, ip := range service.Spec.ExternalIPs { idxPath := ipPath.Index(i) - if msgs := validation.IsValidIP(ip); len(msgs) != 0 { - for i := range msgs { - allErrs = append(allErrs, field.Invalid(idxPath, ip, msgs[i])) - } + if errs := validation.IsValidIP(idxPath, ip); len(errs) != 0 { + allErrs = append(allErrs, errs...) } else { allErrs = append(allErrs, ValidateNonSpecialIP(ip, idxPath)...) } @@ -6918,9 +6908,7 @@ func validateEndpointSubsets(subsets []core.EndpointSubset, fldPath *field.Path) func validateEndpointAddress(address *core.EndpointAddress, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} - for _, msg := range validation.IsValidIP(address.IP) { - allErrs = append(allErrs, field.Invalid(fldPath.Child("ip"), address.IP, msg)) - } + allErrs = append(allErrs, validation.IsValidIP(fldPath.Child("ip"), address.IP)...) if len(address.Hostname) > 0 { allErrs = append(allErrs, ValidateDNS1123Label(address.Hostname, fldPath.Child("hostname"))...) } @@ -7266,9 +7254,7 @@ func ValidateLoadBalancerStatus(status *core.LoadBalancerStatus, fldPath *field. for i, ingress := range status.Ingress { idxPath := ingrPath.Index(i) if len(ingress.IP) > 0 { - if isIP := (netutils.ParseIPSloppy(ingress.IP) != nil); !isIP { - allErrs = append(allErrs, field.Invalid(idxPath.Child("ip"), ingress.IP, "must be a valid IP address")) - } + allErrs = append(allErrs, validation.IsValidIP(idxPath.Child("ip"), ingress.IP)...) } if utilfeature.DefaultFeatureGate.Enabled(features.LoadBalancerIPMode) && ingress.IPMode == nil { @@ -7622,11 +7608,9 @@ func ValidateServiceClusterIPsRelatedFields(service *core.Service) field.ErrorLi } // is it valid ip? - errorMessages := validation.IsValidIP(clusterIP) + errorMessages := validation.IsValidIP(clusterIPsField.Index(i), clusterIP) hasInvalidIPs = (len(errorMessages) != 0) || hasInvalidIPs - for _, msg := range errorMessages { - allErrs = append(allErrs, field.Invalid(clusterIPsField.Index(i), clusterIP, msg)) - } + allErrs = append(allErrs, errorMessages...) } // max two diff --git a/pkg/apis/networking/validation/validation.go b/pkg/apis/networking/validation/validation.go index 7df3b56328d..581c72e2819 100644 --- a/pkg/apis/networking/validation/validation.go +++ b/pkg/apis/networking/validation/validation.go @@ -354,9 +354,7 @@ func ValidateIngressLoadBalancerStatus(status *networking.IngressLoadBalancerSta for i, ingress := range status.Ingress { idxPath := fldPath.Child("ingress").Index(i) if len(ingress.IP) > 0 { - if isIP := (netutils.ParseIPSloppy(ingress.IP) != nil); !isIP { - allErrs = append(allErrs, field.Invalid(idxPath.Child("ip"), ingress.IP, "must be a valid IP address")) - } + allErrs = append(allErrs, validation.IsValidIP(idxPath.Child("ip"), ingress.IP)...) } if len(ingress.Hostname) > 0 { for _, msg := range validation.IsDNS1123Subdomain(ingress.Hostname) { 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 0b8a6cb354a..0eb33e68aaf 100644 --- a/staging/src/k8s.io/apimachinery/pkg/util/validation/validation.go +++ b/staging/src/k8s.io/apimachinery/pkg/util/validation/validation.go @@ -19,9 +19,7 @@ package validation import ( "fmt" "math" - "net" "regexp" - "strconv" "strings" "k8s.io/apimachinery/pkg/util/validation/field" @@ -352,11 +350,12 @@ func IsValidPortName(port string) []string { } // IsValidIP tests that the argument is a valid IP address. -func IsValidIP(value string) []string { +func IsValidIP(fldPath *field.Path, value string) field.ErrorList { + var allErrors field.ErrorList if netutils.ParseIPSloppy(value) == nil { - return []string{"must be a valid IP address, (e.g. 10.9.8.7 or 2001:db8::ffff)"} + allErrors = append(allErrors, field.Invalid(fldPath, value, "must be a valid IP address, (e.g. 10.9.8.7 or 2001:db8::ffff)")) } - return nil + return allErrors } // IsValidIPv4Address tests that the argument is a valid IPv4 address. @@ -493,18 +492,3 @@ func hasChDirPrefix(value string) []string { } return errs } - -// IsValidSocketAddr checks that string represents a valid socket address -// as defined in RFC 789. (e.g 0.0.0.0:10254 or [::]:10254)) -func IsValidSocketAddr(value string) []string { - var errs []string - ip, port, err := net.SplitHostPort(value) - if err != nil { - errs = append(errs, "must be a valid socket address format, (e.g. 0.0.0.0:10254 or [::]:10254)") - return errs - } - portInt, _ := strconv.Atoi(port) - errs = append(errs, IsValidPortNum(portInt)...) - errs = append(errs, IsValidIP(ip)...) - return errs -} 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 499b13c737a..2ce11d560f7 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 @@ -323,104 +323,176 @@ func TestIsValidLabelValue(t *testing.T) { } func TestIsValidIP(t *testing.T) { - goodValues := []string{ - "::1", - "2a00:79e0:2:0:f1c3:e797:93c1:df80", - "::", - "2001:4860:4860::8888", - "::fff:1.1.1.1", - "1.1.1.1", - "1.1.1.01", - "255.0.0.1", - "1.0.0.0", - "0.0.0.0", - } - for _, val := range goodValues { - if msgs := IsValidIP(val); len(msgs) != 0 { - t.Errorf("expected true for %q: %v", val, msgs) - } - } + for _, tc := range []struct { + name string + in string + family int + err string + }{ + // GOOD VALUES + { + name: "ipv4", + in: "1.2.3.4", + family: 4, + }, + { + name: "ipv4, all zeros", + in: "0.0.0.0", + family: 4, + }, + { + name: "ipv4, max", + in: "255.255.255.255", + family: 4, + }, + { + name: "ipv6", + in: "1234::abcd", + family: 6, + }, + { + name: "ipv6, all zeros, collapsed", + in: "::", + family: 6, + }, + { + name: "ipv6, max", + in: "ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff", + family: 6, + }, - badValues := []string{ - "[2001:db8:0:1]:80", - "myhost.mydomain", - "-1.0.0.0", - "[2001:db8:0:1]", - "a", - } - for _, val := range badValues { - if msgs := IsValidIP(val); len(msgs) == 0 { - t.Errorf("expected false for %q", val) - } - } -} + // GOOD, THOUGH NON-CANONICAL, VALUES + { + name: "ipv6, all zeros, expanded (non-canonical)", + in: "0:0:0:0:0:0:0:0", + family: 6, + }, + { + name: "ipv6, leading 0s (non-canonical)", + in: "0001:002:03:4::", + family: 6, + }, + { + name: "ipv6, capital letters (non-canonical)", + in: "1234::ABCD", + family: 6, + }, -func TestIsValidIPv4Address(t *testing.T) { - goodValues := []string{ - "1.1.1.1", - "1.1.1.01", - "255.0.0.1", - "1.0.0.0", - "0.0.0.0", - } - for _, val := range goodValues { - if msgs := IsValidIPv4Address(field.NewPath(""), val); len(msgs) != 0 { - t.Errorf("expected %q to be valid IPv4 address: %v", val, msgs) - } - } + // BAD VALUES WE CURRENTLY CONSIDER GOOD + { + name: "ipv4 with leading 0s", + in: "1.1.1.01", + family: 4, + }, + { + name: "ipv4-in-ipv6 value", + in: "::ffff:1.1.1.1", + family: 4, + }, - badValues := []string{ - "[2001:db8:0:1]:80", - "myhost.mydomain", - "-1.0.0.0", - "[2001:db8:0:1]", - "a", - "2001:4860:4860::8888", - "::fff:1.1.1.1", - "::1", - "2a00:79e0:2:0:f1c3:e797:93c1:df80", - "::", - } - for _, val := range badValues { - if msgs := IsValidIPv4Address(field.NewPath(""), val); len(msgs) == 0 { - t.Errorf("expected %q to be invalid IPv4 address", val) - } - } -} + // BAD VALUES + { + name: "empty string", + in: "", + err: "must be a valid IP address", + }, + { + name: "junk", + in: "aaaaaaa", + err: "must be a valid IP address", + }, + { + name: "domain name", + in: "myhost.mydomain", + err: "must be a valid IP address", + }, + { + name: "cidr", + in: "1.2.3.0/24", + err: "must be a valid IP address", + }, + { + name: "ipv4 with out-of-range octets", + in: "1.2.3.400", + err: "must be a valid IP address", + }, + { + name: "ipv4 with negative octets", + in: "-1.0.0.0", + err: "must be a valid IP address", + }, + { + name: "ipv6 with out-of-range segment", + in: "2001:db8::10005", + err: "must be a valid IP address", + }, + { + name: "ipv4:port", + in: "1.2.3.4:80", + err: "must be a valid IP address", + }, + { + name: "ipv6 with brackets", + in: "[2001:db8::1]", + err: "must be a valid IP address", + }, + { + name: "[ipv6]:port", + in: "[2001:db8::1]:80", + err: "must be a valid IP address", + }, + { + name: "host:port", + in: "example.com:80", + err: "must be a valid IP address", + }, + { + name: "ipv6 with zone", + in: "1234::abcd%eth0", + err: "must be a valid IP address", + }, + { + name: "ipv4 with zone", + in: "169.254.0.0%eth0", + err: "must be a valid IP address", + }, + } { + t.Run(tc.name, func(t *testing.T) { + errs := IsValidIP(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 TestIsValidIPv6Address(t *testing.T) { - goodValues := []string{ - "2001:4860:4860::8888", - "2a00:79e0:2:0:f1c3:e797:93c1:df80", - "2001:0db8:85a3:0000:0000:8a2e:0370:7334", - "::fff:1.1.1.1", - "::1", - "::", - } + errs = IsValidIPv4Address(field.NewPath(""), tc.in) + if tc.family == 4 { + if len(errs) != 0 { + t.Errorf("expected %q to pass IsValidIPv4Address but got: %v", tc.in, errs) + } + } else { + if len(errs) == 0 { + t.Errorf("expected %q to fail IsValidIPv4Address", tc.in) + } + } - for _, val := range goodValues { - if msgs := IsValidIPv6Address(field.NewPath(""), val); len(msgs) != 0 { - t.Errorf("expected %q to be valid IPv6 address: %v", val, msgs) - } - } - - badValues := []string{ - "1.1.1.1", - "1.1.1.01", - "255.0.0.1", - "1.0.0.0", - "0.0.0.0", - "[2001:db8:0:1]:80", - "myhost.mydomain", - "2001:0db8:85a3:0000:0000:8a2e:0370:7334:2001:0db8:85a3:0000:0000:8a2e:0370:7334", - "-1.0.0.0", - "[2001:db8:0:1]", - "a", - } - for _, val := range badValues { - if msgs := IsValidIPv6Address(field.NewPath(""), val); len(msgs) == 0 { - t.Errorf("expected %q to be invalid IPv6 address", val) - } + errs = IsValidIPv6Address(field.NewPath(""), tc.in) + if tc.family == 6 { + if len(errs) != 0 { + t.Errorf("expected %q to pass IsValidIPv6Address but got: %v", tc.in, errs) + } + } else { + if len(errs) == 0 { + t.Errorf("expected %q to fail IsValidIPv6Address", tc.in) + } + } + }) } } @@ -710,31 +782,3 @@ func TestIsDomainPrefixedPath(t *testing.T) { } } } - -func TestIsValidSocketAddr(t *testing.T) { - goodValues := []string{ - "0.0.0.0:10254", - "127.0.0.1:8888", - "[2001:db8:1f70::999:de8:7648:6e8]:10254", - "[::]:10254", - } - for _, val := range goodValues { - if errs := IsValidSocketAddr(val); len(errs) != 0 { - t.Errorf("expected no errors for %q: %v", val, errs) - } - } - - badValues := []string{ - "0.0.0.0.0:2020", - "0.0.0.0", - "6.6.6.6:909090", - "2001:db8:1f70::999:de8:7648:6e8:87567:102545", - "", - "*", - } - for _, val := range badValues { - if errs := IsValidSocketAddr(val); len(errs) == 0 { - t.Errorf("expected errors for %q", val) - } - } -}