Merge pull request #95036 from cmluciano/cml/validateproxycidrs

proxy: validate each CIDR config seperately and check for errors
This commit is contained in:
Kubernetes Prow Robot 2020-11-05 13:12:52 -08:00 committed by GitHub
commit f1a3e4dcce
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 64 additions and 65 deletions

View File

@ -284,8 +284,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
} }
} }
@ -315,7 +314,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

View File

@ -949,52 +949,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 {
@ -1021,45 +1020,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())
}
} }
} }
} }