Rename ValidateNonSpecialIP to ValidateEndpointIP

There is not a single definition of "non-special IP" that makes sense
in all contexts. Rename ValidateNonSpecialIP to ValidateEndpointIP and
clarify that it shouldn't be used for other validations.

Also add a few more unit tests.
This commit is contained in:
Dan Winship 2024-02-16 10:07:39 -05:00
parent e0ab1a16ad
commit 76f1684117
3 changed files with 31 additions and 21 deletions

View File

@ -5943,7 +5943,9 @@ func ValidateService(service *core.Service) field.ErrorList {
if errs := validation.IsValidIP(idxPath, ip); len(errs) != 0 {
allErrs = append(allErrs, errs...)
} else {
allErrs = append(allErrs, ValidateNonSpecialIP(ip, idxPath)...)
// For historical reasons, this uses ValidateEndpointIP even
// though that is not exactly the appropriate set of checks.
allErrs = append(allErrs, ValidateEndpointIP(ip, idxPath)...)
}
}
@ -7489,19 +7491,21 @@ func validateEndpointAddress(address *core.EndpointAddress, fldPath *field.Path)
allErrs = append(allErrs, field.Invalid(fldPath.Child("nodeName"), *address.NodeName, msg).WithOrigin("format=dns-label"))
}
}
allErrs = append(allErrs, ValidateNonSpecialIP(address.IP, fldPath.Child("ip"))...)
allErrs = append(allErrs, ValidateEndpointIP(address.IP, fldPath.Child("ip"))...)
return allErrs
}
// ValidateNonSpecialIP is used to validate Endpoints, EndpointSlices, and
// external IPs. Specifically, this disallows unspecified and loopback addresses
// are nonsensical and link-local addresses tend to be used for node-centric
// purposes (e.g. metadata service).
// ValidateEndpointIP is used to validate Endpoints and EndpointSlice addresses, and also
// (for historical reasons) external IPs. It disallows certain address types that don't
// make sense in those contexts. Note that this function is _almost_, but not exactly,
// equivalent to net.IP.IsGlobalUnicast(). (Unlike IsGlobalUnicast, it allows global
// multicast IPs, which is probably a bug.)
//
// IPv6 references
// - https://www.iana.org/assignments/iana-ipv6-special-registry/iana-ipv6-special-registry.xhtml
// - https://www.iana.org/assignments/ipv6-multicast-addresses/ipv6-multicast-addresses.xhtml
func ValidateNonSpecialIP(ipAddress string, fldPath *field.Path) field.ErrorList {
// This function should not be used for new validations; the exact set of IPs that do and
// don't make sense in a particular field is context-dependent (e.g., localhost makes
// sense in some places; unspecified IPs make sense in fields that are used as bind
// addresses rather than destination addresses).
func ValidateEndpointIP(ipAddress string, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}
ip := netutils.ParseIPSloppy(ipAddress)
if ip == nil {
@ -7520,7 +7524,7 @@ func ValidateNonSpecialIP(ipAddress string, fldPath *field.Path) field.ErrorList
if ip.IsLinkLocalMulticast() {
allErrs = append(allErrs, field.Invalid(fldPath, ipAddress, "may not be in the link-local multicast range (224.0.0.0/24, ff02::/10)"))
}
return allErrs.WithOrigin("format=non-special-ip")
return allErrs.WithOrigin("format=endpoint-ip")
}
func validateEndpointPort(port *core.EndpointPort, requireName bool, fldPath *field.Path) field.ErrorList {

View File

@ -20725,7 +20725,7 @@ func TestValidateEndpointsCreate(t *testing.T) {
}},
},
expectedErrs: field.ErrorList{
field.Invalid(field.NewPath("subsets[0].addresses[0].ip"), nil, "").WithOrigin("format=non-special-ip"),
field.Invalid(field.NewPath("subsets[0].addresses[0].ip"), nil, "").WithOrigin("format=endpoint-ip"),
},
},
"Address is link-local": {
@ -20737,7 +20737,7 @@ func TestValidateEndpointsCreate(t *testing.T) {
}},
},
expectedErrs: field.ErrorList{
field.Invalid(field.NewPath("subsets[0].addresses[0].ip"), nil, "").WithOrigin("format=non-special-ip"),
field.Invalid(field.NewPath("subsets[0].addresses[0].ip"), nil, "").WithOrigin("format=endpoint-ip"),
},
},
"Address is link-local multicast": {
@ -20749,7 +20749,7 @@ func TestValidateEndpointsCreate(t *testing.T) {
}},
},
expectedErrs: field.ErrorList{
field.Invalid(field.NewPath("subsets[0].addresses[0].ip"), nil, "").WithOrigin("format=non-special-ip"),
field.Invalid(field.NewPath("subsets[0].addresses[0].ip"), nil, "").WithOrigin("format=endpoint-ip"),
},
},
"Invalid AppProtocol": {
@ -23641,7 +23641,7 @@ func TestValidateResourceRequirements(t *testing.T) {
}
}
func TestValidateNonSpecialIP(t *testing.T) {
func TestValidateEndpointIP(t *testing.T) {
fp := field.NewPath("ip")
// Valid values.
@ -23652,11 +23652,15 @@ func TestValidateNonSpecialIP(t *testing.T) {
{"ipv4", "10.1.2.3"},
{"ipv4 class E", "244.1.2.3"},
{"ipv6", "2000::1"},
// These probably should not have been allowed, but they are
{"ipv4 multicast", "239.255.255.253"},
{"ipv6 multicast", "ff05::1:3"},
} {
t.Run(tc.desc, func(t *testing.T) {
errs := ValidateNonSpecialIP(tc.ip, fp)
errs := ValidateEndpointIP(tc.ip, fp)
if len(errs) != 0 {
t.Errorf("ValidateNonSpecialIP(%q, ...) = %v; want nil", tc.ip, errs)
t.Errorf("ValidateEndpointIP(%q, ...) = %v; want nil", tc.ip, errs)
}
})
}
@ -23670,13 +23674,15 @@ func TestValidateNonSpecialIP(t *testing.T) {
{"ipv4 localhost", "127.0.0.0"},
{"ipv4 localhost", "127.255.255.255"},
{"ipv6 localhost", "::1"},
{"ipv4 link local", "169.254.169.254"},
{"ipv6 link local", "fe80::"},
{"ipv4 local multicast", "224.0.0.251"},
{"ipv6 local multicast", "ff02::"},
} {
t.Run(tc.desc, func(t *testing.T) {
errs := ValidateNonSpecialIP(tc.ip, fp)
errs := ValidateEndpointIP(tc.ip, fp)
if len(errs) == 0 {
t.Errorf("ValidateNonSpecialIP(%q, ...) = nil; want non-nil (errors)", tc.ip)
t.Errorf("ValidateEndpointIP(%q, ...) = nil; want non-nil (errors)", tc.ip)
}
})
}

View File

@ -100,10 +100,10 @@ func validateEndpoints(endpoints []discovery.Endpoint, addrType discovery.Addres
switch addrType {
case discovery.AddressTypeIPv4:
allErrs = append(allErrs, validation.IsValidIPv4Address(addressPath.Index(i), address)...)
allErrs = append(allErrs, apivalidation.ValidateNonSpecialIP(address, addressPath.Index(i))...)
allErrs = append(allErrs, apivalidation.ValidateEndpointIP(address, addressPath.Index(i))...)
case discovery.AddressTypeIPv6:
allErrs = append(allErrs, validation.IsValidIPv6Address(addressPath.Index(i), address)...)
allErrs = append(allErrs, apivalidation.ValidateNonSpecialIP(address, addressPath.Index(i))...)
allErrs = append(allErrs, apivalidation.ValidateEndpointIP(address, addressPath.Index(i))...)
case discovery.AddressTypeFQDN:
allErrs = append(allErrs, validation.IsFullyQualifiedDomainName(addressPath.Index(i), address)...)
}