Merge pull request #122931 from danwinship/ip-validation-cleanup

consistently use IsValidIP for IP validation
This commit is contained in:
Kubernetes Prow Robot 2024-02-06 15:58:15 -08:00 committed by GitHub
commit 052bce26f4
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 183 additions and 173 deletions

View File

@ -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))) 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 { for i, ns := range dnsConfig.Nameservers {
if ip := netutils.ParseIPSloppy(ns); ip == nil { allErrs = append(allErrs, validation.IsValidIP(fldPath.Child("nameservers").Index(i), ns)...)
allErrs = append(allErrs, field.Invalid(fldPath.Child("nameservers").Index(i), ns, "must be valid IP address"))
}
} }
// Validate searches. // Validate searches.
if len(dnsConfig.Searches) > MaxDNSSearchPaths { 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 { func ValidateHostAliases(hostAliases []core.HostAlias, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{} allErrs := field.ErrorList{}
for _, hostAlias := range hostAliases { for i, hostAlias := range hostAliases {
if ip := netutils.ParseIPSloppy(hostAlias.IP); ip == nil { allErrs = append(allErrs, validation.IsValidIP(fldPath.Index(i).Child("ip"), hostAlias.IP)...)
allErrs = append(allErrs, field.Invalid(fldPath.Child("ip"), hostAlias.IP, "must be valid IP address")) for j, hostname := range hostAlias.Hostnames {
} allErrs = append(allErrs, ValidateDNS1123Subdomain(hostname, fldPath.Index(i).Child("hostnames").Index(j))...)
for _, hostname := range hostAlias.Hostnames {
allErrs = append(allErrs, ValidateDNS1123Subdomain(hostname, fldPath.Child("hostnames"))...)
} }
} }
return allErrs return allErrs
@ -4031,9 +4027,7 @@ func validatePodIPs(pod *core.Pod) field.ErrorList {
// all PodIPs must be valid IPs // all PodIPs must be valid IPs
for i, podIP := range pod.Status.PodIPs { for i, podIP := range pod.Status.PodIPs {
for _, msg := range validation.IsValidIP(podIP.IP) { allErrs = append(allErrs, validation.IsValidIP(podIPsField.Index(i), podIP.IP)...)
allErrs = append(allErrs, field.Invalid(podIPsField.Index(i), podIP.IP, msg))
}
} }
// if we have more than one Pod.PodIP then // 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 // all HostPs must be valid IPs
for i, hostIP := range pod.Status.HostIPs { for i, hostIP := range pod.Status.HostIPs {
for _, msg := range validation.IsValidIP(hostIP.IP) { allErrs = append(allErrs, validation.IsValidIP(hostIPsField.Index(i), hostIP.IP)...)
allErrs = append(allErrs, field.Invalid(hostIPsField.Index(i), hostIP.IP, msg))
}
} }
// if we have more than one Pod.HostIP then // if we have more than one Pod.HostIP then
@ -5403,10 +5395,8 @@ func ValidateService(service *core.Service) field.ErrorList {
ipPath := specPath.Child("externalIPs") ipPath := specPath.Child("externalIPs")
for i, ip := range service.Spec.ExternalIPs { for i, ip := range service.Spec.ExternalIPs {
idxPath := ipPath.Index(i) idxPath := ipPath.Index(i)
if msgs := validation.IsValidIP(ip); len(msgs) != 0 { if errs := validation.IsValidIP(idxPath, ip); len(errs) != 0 {
for i := range msgs { allErrs = append(allErrs, errs...)
allErrs = append(allErrs, field.Invalid(idxPath, ip, msgs[i]))
}
} else { } else {
allErrs = append(allErrs, ValidateNonSpecialIP(ip, idxPath)...) 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 { func validateEndpointAddress(address *core.EndpointAddress, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{} allErrs := field.ErrorList{}
for _, msg := range validation.IsValidIP(address.IP) { allErrs = append(allErrs, validation.IsValidIP(fldPath.Child("ip"), address.IP)...)
allErrs = append(allErrs, field.Invalid(fldPath.Child("ip"), address.IP, msg))
}
if len(address.Hostname) > 0 { if len(address.Hostname) > 0 {
allErrs = append(allErrs, ValidateDNS1123Label(address.Hostname, fldPath.Child("hostname"))...) 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 { for i, ingress := range status.Ingress {
idxPath := ingrPath.Index(i) idxPath := ingrPath.Index(i)
if len(ingress.IP) > 0 { if len(ingress.IP) > 0 {
if isIP := (netutils.ParseIPSloppy(ingress.IP) != nil); !isIP { allErrs = append(allErrs, validation.IsValidIP(idxPath.Child("ip"), ingress.IP)...)
allErrs = append(allErrs, field.Invalid(idxPath.Child("ip"), ingress.IP, "must be a valid IP address"))
}
} }
if utilfeature.DefaultFeatureGate.Enabled(features.LoadBalancerIPMode) && ingress.IPMode == nil { if utilfeature.DefaultFeatureGate.Enabled(features.LoadBalancerIPMode) && ingress.IPMode == nil {
@ -7622,11 +7608,9 @@ func ValidateServiceClusterIPsRelatedFields(service *core.Service) field.ErrorLi
} }
// is it valid ip? // is it valid ip?
errorMessages := validation.IsValidIP(clusterIP) errorMessages := validation.IsValidIP(clusterIPsField.Index(i), clusterIP)
hasInvalidIPs = (len(errorMessages) != 0) || hasInvalidIPs hasInvalidIPs = (len(errorMessages) != 0) || hasInvalidIPs
for _, msg := range errorMessages { allErrs = append(allErrs, errorMessages...)
allErrs = append(allErrs, field.Invalid(clusterIPsField.Index(i), clusterIP, msg))
}
} }
// max two // max two

View File

@ -354,9 +354,7 @@ func ValidateIngressLoadBalancerStatus(status *networking.IngressLoadBalancerSta
for i, ingress := range status.Ingress { for i, ingress := range status.Ingress {
idxPath := fldPath.Child("ingress").Index(i) idxPath := fldPath.Child("ingress").Index(i)
if len(ingress.IP) > 0 { if len(ingress.IP) > 0 {
if isIP := (netutils.ParseIPSloppy(ingress.IP) != nil); !isIP { allErrs = append(allErrs, validation.IsValidIP(idxPath.Child("ip"), ingress.IP)...)
allErrs = append(allErrs, field.Invalid(idxPath.Child("ip"), ingress.IP, "must be a valid IP address"))
}
} }
if len(ingress.Hostname) > 0 { if len(ingress.Hostname) > 0 {
for _, msg := range validation.IsDNS1123Subdomain(ingress.Hostname) { for _, msg := range validation.IsDNS1123Subdomain(ingress.Hostname) {

View File

@ -19,9 +19,7 @@ package validation
import ( import (
"fmt" "fmt"
"math" "math"
"net"
"regexp" "regexp"
"strconv"
"strings" "strings"
"k8s.io/apimachinery/pkg/util/validation/field" "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. // 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 { 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. // IsValidIPv4Address tests that the argument is a valid IPv4 address.
@ -493,18 +492,3 @@ func hasChDirPrefix(value string) []string {
} }
return errs 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
}

View File

@ -323,104 +323,176 @@ func TestIsValidLabelValue(t *testing.T) {
} }
func TestIsValidIP(t *testing.T) { func TestIsValidIP(t *testing.T) {
goodValues := []string{ for _, tc := range []struct {
"::1", name string
"2a00:79e0:2:0:f1c3:e797:93c1:df80", in string
"::", family int
"2001:4860:4860::8888", err string
"::fff:1.1.1.1", }{
"1.1.1.1", // GOOD VALUES
"1.1.1.01", {
"255.0.0.1", name: "ipv4",
"1.0.0.0", in: "1.2.3.4",
"0.0.0.0", family: 4,
} },
for _, val := range goodValues { {
if msgs := IsValidIP(val); len(msgs) != 0 { name: "ipv4, all zeros",
t.Errorf("expected true for %q: %v", val, msgs) 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{ // GOOD, THOUGH NON-CANONICAL, VALUES
"[2001:db8:0:1]:80", {
"myhost.mydomain", name: "ipv6, all zeros, expanded (non-canonical)",
"-1.0.0.0", in: "0:0:0:0:0:0:0:0",
"[2001:db8:0:1]", family: 6,
"a", },
} {
for _, val := range badValues { name: "ipv6, leading 0s (non-canonical)",
if msgs := IsValidIP(val); len(msgs) == 0 { in: "0001:002:03:4::",
t.Errorf("expected false for %q", val) family: 6,
} },
} {
} name: "ipv6, capital letters (non-canonical)",
in: "1234::ABCD",
family: 6,
},
func TestIsValidIPv4Address(t *testing.T) { // BAD VALUES WE CURRENTLY CONSIDER GOOD
goodValues := []string{ {
"1.1.1.1", name: "ipv4 with leading 0s",
"1.1.1.01", in: "1.1.1.01",
"255.0.0.1", family: 4,
"1.0.0.0", },
"0.0.0.0", {
} name: "ipv4-in-ipv6 value",
for _, val := range goodValues { in: "::ffff:1.1.1.1",
if msgs := IsValidIPv4Address(field.NewPath(""), val); len(msgs) != 0 { family: 4,
t.Errorf("expected %q to be valid IPv4 address: %v", val, msgs) },
}
}
badValues := []string{ // BAD VALUES
"[2001:db8:0:1]:80", {
"myhost.mydomain", name: "empty string",
"-1.0.0.0", in: "",
"[2001:db8:0:1]", err: "must be a valid IP address",
"a", },
"2001:4860:4860::8888", {
"::fff:1.1.1.1", name: "junk",
"::1", in: "aaaaaaa",
"2a00:79e0:2:0:f1c3:e797:93c1:df80", err: "must be a valid IP address",
"::", },
} {
for _, val := range badValues { name: "domain name",
if msgs := IsValidIPv4Address(field.NewPath(""), val); len(msgs) == 0 { in: "myhost.mydomain",
t.Errorf("expected %q to be invalid IPv4 address", val) 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) { errs = IsValidIPv4Address(field.NewPath(""), tc.in)
goodValues := []string{ if tc.family == 4 {
"2001:4860:4860::8888", if len(errs) != 0 {
"2a00:79e0:2:0:f1c3:e797:93c1:df80", t.Errorf("expected %q to pass IsValidIPv4Address but got: %v", tc.in, errs)
"2001:0db8:85a3:0000:0000:8a2e:0370:7334", }
"::fff:1.1.1.1", } else {
"::1", if len(errs) == 0 {
"::", t.Errorf("expected %q to fail IsValidIPv4Address", tc.in)
} }
}
for _, val := range goodValues { errs = IsValidIPv6Address(field.NewPath(""), tc.in)
if msgs := IsValidIPv6Address(field.NewPath(""), val); len(msgs) != 0 { if tc.family == 6 {
t.Errorf("expected %q to be valid IPv6 address: %v", val, msgs) if len(errs) != 0 {
} t.Errorf("expected %q to pass IsValidIPv6Address but got: %v", tc.in, errs)
} }
} else {
badValues := []string{ if len(errs) == 0 {
"1.1.1.1", t.Errorf("expected %q to fail IsValidIPv6Address", tc.in)
"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)
}
} }
} }
@ -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)
}
}
}