github/kubernetes
1
0
Fork 0
mirror of https://github.com/k3s-io/kubernetes.git synced 2025-07-30 15:05:27 +00:00
Code Issues Projects Releases Wiki Activity

Slightly improve EndpointSlice address validation

Browse Source 
Because it used both IsValidIPv4Address and ValidateEndpointIP,
EndpointSlice validation produced duplicate error messages when given
an invalid IP. Fix this by calling IsValidIP first, and only doing the
other checks if that one fails.

Also, since no one else was using the IsValidIPv4Address and
IsValidIPv6Address methods anyway, just inline them into the
EndpointSlice validation, so we don't have to worry about "should they
do legacy or strict validation" later.
This commit is contained in:
Dan Winship 2025-02-28 18:14:44 -05:00
parent fc4bb4fdb9
commit ba189de78f
4 changed files with 47 additions and 86 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
pkg/apis/discovery/validation/validation.go
View File

@ -28,6 +28,7 @@ import (
api "k8s.io/kubernetes/pkg/apis/core"
apivalidation "k8s.io/kubernetes/pkg/apis/core/validation"
"k8s.io/kubernetes/pkg/apis/discovery"
netutils "k8s.io/utils/net"
)
var (
@ -99,11 +100,25 @@ func validateEndpoints(endpoints []discovery.Endpoint, addrType discovery.Addres
// and do not get validated.
switch addrType {
case discovery.AddressTypeIPv4:
allErrs = append(allErrs, validation.IsValidIPv4Address(addressPath.Index(i), address)...)
allErrs = append(allErrs, apivalidation.ValidateEndpointIP(address, addressPath.Index(i))...)
ipErrs := validation.IsValidIP(addressPath.Index(i), address)
if len(ipErrs) > 0 {
allErrs = append(allErrs, ipErrs...)
} else {
if !netutils.IsIPv4String(address) {
allErrs = append(allErrs, field.Invalid(addressPath, address, "must be an IPv4 address"))
}
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.ValidateEndpointIP(address, addressPath.Index(i))...)
ipErrs := validation.IsValidIP(addressPath.Index(i), address)
if len(ipErrs) > 0 {
allErrs = append(allErrs, ipErrs...)
} else {
if !netutils.IsIPv6String(address) {
allErrs = append(allErrs, field.Invalid(addressPath, address, "must be an IPv6 address"))
}
allErrs = append(allErrs, apivalidation.ValidateEndpointIP(address, addressPath.Index(i))...)
}
case discovery.AddressTypeFQDN:
allErrs = append(allErrs, validation.IsFullyQualifiedDomainName(addressPath.Index(i), address)...)
}

6
pkg/apis/discovery/validation/validation_test.go
View File

@ -408,7 +408,7 @@ func TestValidateEndpointSlice(t *testing.T) {
},
},
"bad-ip": {
expectedErrors: 2,
expectedErrors: 1,
endpointSlice: &discovery.EndpointSlice{
ObjectMeta: standardMeta,
AddressType: discovery.AddressTypeIPv4,
@ -423,7 +423,7 @@ func TestValidateEndpointSlice(t *testing.T) {
},
},
"bad-ipv4": {
expectedErrors: 3,
expectedErrors: 2,
endpointSlice: &discovery.EndpointSlice{
ObjectMeta: standardMeta,
AddressType: discovery.AddressTypeIPv4,
@ -438,7 +438,7 @@ func TestValidateEndpointSlice(t *testing.T) {
},
},
"bad-ipv6": {
expectedErrors: 4,
expectedErrors: 2,
endpointSlice: &discovery.EndpointSlice{
ObjectMeta: standardMeta,
AddressType: discovery.AddressTypeIPv6,

20
staging/src/k8s.io/apimachinery/pkg/util/validation/ip.go
View File

@ -65,26 +65,6 @@ func GetWarningsForIP(fldPath *field.Path, value string) []string {
return nil
}
// IsValidIPv4Address tests that the argument is a valid IPv4 address.
func IsValidIPv4Address(fldPath *field.Path, value string) field.ErrorList {
var allErrors field.ErrorList
ip := netutils.ParseIPSloppy(value)
if ip == nil || ip.To4() == nil {
allErrors = append(allErrors, field.Invalid(fldPath, value, "must be a valid IPv4 address"))
}
return allErrors
}
// IsValidIPv6Address tests that the argument is a valid IPv6 address.
func IsValidIPv6Address(fldPath *field.Path, value string) field.ErrorList {
var allErrors field.ErrorList
ip := netutils.ParseIPSloppy(value)
if ip == nil || ip.To4() != nil {
allErrors = append(allErrors, field.Invalid(fldPath, value, "must be a valid IPv6 address"))
}
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

84
staging/src/k8s.io/apimachinery/pkg/util/validation/ip_test.go
View File

@ -26,70 +26,58 @@ import (
func TestIsValidIP(t *testing.T) {
for _, tc := range []struct {
name string
in string
family int
err string
name string
in string
err string
}{
// GOOD VALUES
{
name: "ipv4",
in: "1.2.3.4",
family: 4,
name: "ipv4",
in: "1.2.3.4",
},
{
name: "ipv4, all zeros",
in: "0.0.0.0",
family: 4,
name: "ipv4, all zeros",
in: "0.0.0.0",
},
{
name: "ipv4, max",
in: "255.255.255.255",
family: 4,
name: "ipv4, max",
in: "255.255.255.255",
},
{
name: "ipv6",
in: "1234::abcd",
family: 6,
name: "ipv6",
in: "1234::abcd",
},
{
name: "ipv6, all zeros, collapsed",
in: "::",
family: 6,
name: "ipv6, all zeros, collapsed",
in: "::",
},
{
name: "ipv6, max",
in: "ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff",
family: 6,
name: "ipv6, max",
in: "ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff",
},
// 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, all zeros, expanded (non-canonical)",
in: "0:0:0:0:0:0:0:0",
},
{
name: "ipv6, leading 0s (non-canonical)",
in: "0001:002:03:4::",
family: 6,
name: "ipv6, leading 0s (non-canonical)",
in: "0001:002:03:4::",
},
{
name: "ipv6, capital letters (non-canonical)",
in: "1234::ABCD",
family: 6,
name: "ipv6, capital letters (non-canonical)",
in: "1234::ABCD",
},
// BAD VALUES WE CURRENTLY CONSIDER GOOD
{
name: "ipv4 with leading 0s",
in: "1.1.1.01",
family: 4,
name: "ipv4 with leading 0s",
in: "1.1.1.01",
},
{
name: "ipv4-in-ipv6 value",
in: "::ffff:1.1.1.1",
family: 4,
name: "ipv4-in-ipv6 value",
in: "::ffff:1.1.1.1",
},
// BAD VALUES
@ -172,28 +160,6 @@ func TestIsValidIP(t *testing.T) {
t.Errorf("expected error for %q to contain %q but got: %q", tc.in, tc.err, errs[0].Detail)
}
}
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)
}
}
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)
}
}
})
}
}