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

proxy: validate each CIDR config seperately and check for errors

Browse Source 
This commit revises validateProxyNodePortAddress and
validateExcludeCIDRS to report on the exact CIDR that is
invalid within the array of strings. Previously we would just return
the whole block of addresses and now we identify the exact address
within the block to eliminate confusion. I also removed the break from
validateProxyNodeAddress so that we can report on all addresses that
may not be valid.

The tests for each function have also been revised to check the errors
explicitly upon validating. This also will properly catch occasions
where we should be returning multiple errors if more than one CIDR is invalid.

Signed-off-by: Christopher M. Luciano <cmluciano@us.ibm.com>
This commit is contained in:
Christopher M. Luciano 2020-09-24 12:28:13 -04:00
parent 2d9a0b64d1
commit 705ba7b4bc
No known key found for this signature in database
GPG Key ID: 5148DBB31F2843F1
2 changed files with 64 additions and 65 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
pkg/proxy/apis/config/validation/validation.go
View File

@ -276,8 +276,7 @@ func validateKubeProxyNodePortAddress(nodePortAddresses []string, fldPath *field
for i := range nodePortAddresses { for i := range nodePortAddresses {
if _, _, err := net.ParseCIDR(nodePortAddresses[i]); err != nil { if _, _, err := net.ParseCIDR(nodePortAddresses[i]); err != nil {
allErrs = append(allErrs, field.Invalid(fldPath, nodePortAddresses, "must be a valid IP block")) allErrs = append(allErrs, field.Invalid(fldPath.Index(i), nodePortAddresses[i], "must be a valid CIDR"))
break
} }
} }
@ -307,7 +306,7 @@ func validateIPVSExcludeCIDRs(excludeCIDRs []string, fldPath *field.Path) field.
for i := range excludeCIDRs { for i := range excludeCIDRs {
if _, _, err := net.ParseCIDR(excludeCIDRs[i]); err != nil { if _, _, err := net.ParseCIDR(excludeCIDRs[i]); err != nil {
allErrs = append(allErrs, field.Invalid(fldPath, excludeCIDRs, "must be a valid IP block")) allErrs = append(allErrs, field.Invalid(fldPath.Index(i), excludeCIDRs[i], "must be a valid CIDR"))
} }
} }
return allErrs return allErrs

116
pkg/proxy/apis/config/validation/validation_test.go
View File

@ -924,52 +924,51 @@ func TestValidateKubeProxyNodePortAddress(t *testing.T) {
} }
} }
errorCases := []struct { testCases := map[string]struct {
addresses []string addresses []string
msg string expectedErrs field.ErrorList
}{ }{
{ "invalid foo address": {
addresses: []string{"foo"}, addresses: []string{"foo"},
msg: "must be a valid IP block", expectedErrs: field.ErrorList{field.Invalid(newPath.Child("NodePortAddresses[0]"), "foo", "must be a valid CIDR")},
}, },
{ "invalid octet address": {
addresses: []string{"1.2.3"}, addresses: []string{"10.0.0.0/0", "1.2.3"},
msg: "must be a valid IP block", expectedErrs: field.ErrorList{field.Invalid(newPath.Child("NodePortAddresses[1]"), "1.2.3", "must be a valid CIDR")},
}, },
{ "address cannot be 0": {
addresses: []string{""}, addresses: []string{"127.0.0.1/32", "0", "1.2.3.0/24"},
msg: "must be a valid IP block", expectedErrs: field.ErrorList{field.Invalid(newPath.Child("NodePortAddresses[1]"), "0", "must be a valid CIDR")},
}, },
{ "address missing subnet range": {
addresses: []string{"10.20.30.40"}, addresses: []string{"127.0.0.1/32", "10.20.30.40", "1.2.3.0/24"},
msg: "must be a valid IP block", expectedErrs: field.ErrorList{field.Invalid(newPath.Child("NodePortAddresses[1]"), "10.20.30.40", "must be a valid CIDR")},
}, },
{ "missing ipv6 subnet ranges": {
addresses: []string{"::1"}, addresses: []string{"::0", "::1", "2001:db8::/32"},
msg: "must be a valid IP block", expectedErrs: field.ErrorList{field.Invalid(newPath.Child("NodePortAddresses[0]"), "::0", "must be a valid CIDR"),
field.Invalid(newPath.Child("NodePortAddresses[1]"), "::1", "must be a valid CIDR")},
}, },
{ "invalid ipv6 ip format": {
addresses: []string{"2001:db8:1"}, addresses: []string{"::1/128", "2001:db8::/32", "2001:db8:xyz/64"},
msg: "must be a valid IP block", expectedErrs: field.ErrorList{field.Invalid(newPath.Child("NodePortAddresses[2]"), "2001:db8:xyz/64", "must be a valid CIDR")},
},
{
addresses: []string{"2001:db8:xyz/64"},
msg: "must be a valid IP block",
}, },
} }
for _, errorCase := range errorCases { for _, testCase := range testCases {
if errs := validateKubeProxyNodePortAddress(errorCase.addresses, newPath.Child("NodePortAddresses")); len(errs) == 0 { errs := validateKubeProxyNodePortAddress(testCase.addresses, newPath.Child("NodePortAddresses"))
t.Errorf("expected failure for %s", errorCase.msg) if len(testCase.expectedErrs) != len(errs) {
} else if !strings.Contains(errs[0].Error(), errorCase.msg) { t.Errorf("Expected %d errors, got %d errors: %v", len(testCase.expectedErrs), len(errs), errs)
t.Errorf("unexpected error: %v, expected: %s", errs[0], errorCase.msg) }
for i, err := range errs {
if err.Error() != testCase.expectedErrs[i].Error() {
t.Errorf("Expected error: %s, got %s", testCase.expectedErrs[i], err.Error())
}
} }
} }
} }
func TestValidateKubeProxyExcludeCIDRs(t *testing.T) { func TestValidateKubeProxyExcludeCIDRs(t *testing.T) {
// TODO(rramkumar): This test is a copy of TestValidateKubeProxyNodePortAddress.
// Maybe some code can be shared?
newPath := field.NewPath("KubeProxyConfiguration") newPath := field.NewPath("KubeProxyConfiguration")
successCases := []struct { successCases := []struct {
@ -996,45 +995,46 @@ func TestValidateKubeProxyExcludeCIDRs(t *testing.T) {
} }
} }
errorCases := []struct { testCases := map[string]struct {
addresses []string addresses []string
msg string expectedErrs field.ErrorList
}{ }{
{ "invalid foo address": {
addresses: []string{"foo"}, addresses: []string{"foo"},
msg: "must be a valid IP block", expectedErrs: field.ErrorList{field.Invalid(newPath.Child("ExcludeCIDRS[0]"), "foo", "must be a valid CIDR")},
}, },
{ "invalid octet address": {
addresses: []string{"1.2.3"}, addresses: []string{"10.0.0.0/0", "1.2.3"},
msg: "must be a valid IP block", expectedErrs: field.ErrorList{field.Invalid(newPath.Child("ExcludeCIDRS[1]"), "1.2.3", "must be a valid CIDR")},
}, },
{ "address cannot be 0": {
addresses: []string{""}, addresses: []string{"127.0.0.1/32", "0", "1.2.3.0/24"},
msg: "must be a valid IP block", expectedErrs: field.ErrorList{field.Invalid(newPath.Child("ExcludeCIDRS[1]"), "0", "must be a valid CIDR")},
}, },
{ "address missing subnet range": {
addresses: []string{"10.20.30.40"}, addresses: []string{"127.0.0.1/32", "10.20.30.40", "1.2.3.0/24"},
msg: "must be a valid IP block", expectedErrs: field.ErrorList{field.Invalid(newPath.Child("ExcludeCIDRS[1]"), "10.20.30.40", "must be a valid CIDR")},
}, },
{ "missing ipv6 subnet ranges": {
addresses: []string{"::1"}, addresses: []string{"::0", "::1", "2001:db8::/32"},
msg: "must be a valid IP block", expectedErrs: field.ErrorList{field.Invalid(newPath.Child("ExcludeCIDRS[0]"), "::0", "must be a valid CIDR"),
field.Invalid(newPath.Child("ExcludeCIDRS[1]"), "::1", "must be a valid CIDR")},
}, },
{ "invalid ipv6 ip format": {
addresses: []string{"2001:db8:1"}, addresses: []string{"::1/128", "2001:db8::/32", "2001:db8:xyz/64"},
msg: "must be a valid IP block", expectedErrs: field.ErrorList{field.Invalid(newPath.Child("ExcludeCIDRS[2]"), "2001:db8:xyz/64", "must be a valid CIDR")},
},
{
addresses: []string{"2001:db8:xyz/64"},
msg: "must be a valid IP block",
}, },
} }
for _, errorCase := range errorCases { for _, testCase := range testCases {
if errs := validateIPVSExcludeCIDRs(errorCase.addresses, newPath.Child("ExcludeCIDRs")); len(errs) == 0 { errs := validateIPVSExcludeCIDRs(testCase.addresses, newPath.Child("ExcludeCIDRS"))
t.Errorf("expected failure for %s", errorCase.msg) if len(testCase.expectedErrs) != len(errs) {
} else if !strings.Contains(errs[0].Error(), errorCase.msg) { t.Errorf("Expected %d errors, got %d errors: %v", len(testCase.expectedErrs), len(errs), errs)
t.Errorf("unexpected error: %v, expected: %s", errs[0], errorCase.msg) }
for i, err := range errs {
if err.Error() != testCase.expectedErrs[i].Error() {
t.Errorf("Expected error: %s, got %s", testCase.expectedErrs[i], err.Error())
}
} }
} }
} }