diff --git a/pkg/proxy/apis/config/validation/validation.go b/pkg/proxy/apis/config/validation/validation.go index e111faf2038..0856bc97bd6 100644 --- a/pkg/proxy/apis/config/validation/validation.go +++ b/pkg/proxy/apis/config/validation/validation.go @@ -284,8 +284,7 @@ func validateKubeProxyNodePortAddress(nodePortAddresses []string, fldPath *field for i := range nodePortAddresses { if _, _, err := net.ParseCIDR(nodePortAddresses[i]); err != nil { - allErrs = append(allErrs, field.Invalid(fldPath, nodePortAddresses, "must be a valid IP block")) - break + allErrs = append(allErrs, field.Invalid(fldPath.Index(i), nodePortAddresses[i], "must be a valid CIDR")) } } @@ -315,7 +314,7 @@ func validateIPVSExcludeCIDRs(excludeCIDRs []string, fldPath *field.Path) field. for i := range excludeCIDRs { 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 diff --git a/pkg/proxy/apis/config/validation/validation_test.go b/pkg/proxy/apis/config/validation/validation_test.go index c01252c27e7..3e3eda83ca8 100644 --- a/pkg/proxy/apis/config/validation/validation_test.go +++ b/pkg/proxy/apis/config/validation/validation_test.go @@ -949,52 +949,51 @@ func TestValidateKubeProxyNodePortAddress(t *testing.T) { } } - errorCases := []struct { - addresses []string - msg string + testCases := map[string]struct { + addresses []string + expectedErrs field.ErrorList }{ - { - addresses: []string{"foo"}, - msg: "must be a valid IP block", + "invalid foo address": { + addresses: []string{"foo"}, + expectedErrs: field.ErrorList{field.Invalid(newPath.Child("NodePortAddresses[0]"), "foo", "must be a valid CIDR")}, }, - { - addresses: []string{"1.2.3"}, - msg: "must be a valid IP block", + "invalid octet address": { + addresses: []string{"10.0.0.0/0", "1.2.3"}, + expectedErrs: field.ErrorList{field.Invalid(newPath.Child("NodePortAddresses[1]"), "1.2.3", "must be a valid CIDR")}, }, - { - addresses: []string{""}, - msg: "must be a valid IP block", + "address cannot be 0": { + addresses: []string{"127.0.0.1/32", "0", "1.2.3.0/24"}, + expectedErrs: field.ErrorList{field.Invalid(newPath.Child("NodePortAddresses[1]"), "0", "must be a valid CIDR")}, }, - { - addresses: []string{"10.20.30.40"}, - msg: "must be a valid IP block", + "address missing subnet range": { + addresses: []string{"127.0.0.1/32", "10.20.30.40", "1.2.3.0/24"}, + expectedErrs: field.ErrorList{field.Invalid(newPath.Child("NodePortAddresses[1]"), "10.20.30.40", "must be a valid CIDR")}, }, - { - addresses: []string{"::1"}, - msg: "must be a valid IP block", + "missing ipv6 subnet ranges": { + addresses: []string{"::0", "::1", "2001:db8::/32"}, + 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")}, }, - { - addresses: []string{"2001:db8:1"}, - msg: "must be a valid IP block", - }, - { - addresses: []string{"2001:db8:xyz/64"}, - msg: "must be a valid IP block", + "invalid ipv6 ip format": { + addresses: []string{"::1/128", "2001:db8::/32", "2001:db8:xyz/64"}, + expectedErrs: field.ErrorList{field.Invalid(newPath.Child("NodePortAddresses[2]"), "2001:db8:xyz/64", "must be a valid CIDR")}, }, } - for _, errorCase := range errorCases { - if errs := validateKubeProxyNodePortAddress(errorCase.addresses, newPath.Child("NodePortAddresses")); len(errs) == 0 { - t.Errorf("expected failure for %s", errorCase.msg) - } else if !strings.Contains(errs[0].Error(), errorCase.msg) { - t.Errorf("unexpected error: %v, expected: %s", errs[0], errorCase.msg) + for _, testCase := range testCases { + errs := validateKubeProxyNodePortAddress(testCase.addresses, newPath.Child("NodePortAddresses")) + if len(testCase.expectedErrs) != len(errs) { + t.Errorf("Expected %d errors, got %d errors: %v", len(testCase.expectedErrs), len(errs), errs) + } + 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) { - // TODO(rramkumar): This test is a copy of TestValidateKubeProxyNodePortAddress. - // Maybe some code can be shared? newPath := field.NewPath("KubeProxyConfiguration") successCases := []struct { @@ -1021,45 +1020,46 @@ func TestValidateKubeProxyExcludeCIDRs(t *testing.T) { } } - errorCases := []struct { - addresses []string - msg string + testCases := map[string]struct { + addresses []string + expectedErrs field.ErrorList }{ - { - addresses: []string{"foo"}, - msg: "must be a valid IP block", + "invalid foo address": { + addresses: []string{"foo"}, + expectedErrs: field.ErrorList{field.Invalid(newPath.Child("ExcludeCIDRS[0]"), "foo", "must be a valid CIDR")}, }, - { - addresses: []string{"1.2.3"}, - msg: "must be a valid IP block", + "invalid octet address": { + addresses: []string{"10.0.0.0/0", "1.2.3"}, + expectedErrs: field.ErrorList{field.Invalid(newPath.Child("ExcludeCIDRS[1]"), "1.2.3", "must be a valid CIDR")}, }, - { - addresses: []string{""}, - msg: "must be a valid IP block", + "address cannot be 0": { + addresses: []string{"127.0.0.1/32", "0", "1.2.3.0/24"}, + expectedErrs: field.ErrorList{field.Invalid(newPath.Child("ExcludeCIDRS[1]"), "0", "must be a valid CIDR")}, }, - { - addresses: []string{"10.20.30.40"}, - msg: "must be a valid IP block", + "address missing subnet range": { + addresses: []string{"127.0.0.1/32", "10.20.30.40", "1.2.3.0/24"}, + expectedErrs: field.ErrorList{field.Invalid(newPath.Child("ExcludeCIDRS[1]"), "10.20.30.40", "must be a valid CIDR")}, }, - { - addresses: []string{"::1"}, - msg: "must be a valid IP block", + "missing ipv6 subnet ranges": { + addresses: []string{"::0", "::1", "2001:db8::/32"}, + 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")}, }, - { - addresses: []string{"2001:db8:1"}, - msg: "must be a valid IP block", - }, - { - addresses: []string{"2001:db8:xyz/64"}, - msg: "must be a valid IP block", + "invalid ipv6 ip format": { + addresses: []string{"::1/128", "2001:db8::/32", "2001:db8:xyz/64"}, + expectedErrs: field.ErrorList{field.Invalid(newPath.Child("ExcludeCIDRS[2]"), "2001:db8:xyz/64", "must be a valid CIDR")}, }, } - for _, errorCase := range errorCases { - if errs := validateIPVSExcludeCIDRs(errorCase.addresses, newPath.Child("ExcludeCIDRs")); len(errs) == 0 { - t.Errorf("expected failure for %s", errorCase.msg) - } else if !strings.Contains(errs[0].Error(), errorCase.msg) { - t.Errorf("unexpected error: %v, expected: %s", errs[0], errorCase.msg) + for _, testCase := range testCases { + errs := validateIPVSExcludeCIDRs(testCase.addresses, newPath.Child("ExcludeCIDRS")) + if len(testCase.expectedErrs) != len(errs) { + t.Errorf("Expected %d errors, got %d errors: %v", len(testCase.expectedErrs), len(errs), errs) + } + for i, err := range errs { + if err.Error() != testCase.expectedErrs[i].Error() { + t.Errorf("Expected error: %s, got %s", testCase.expectedErrs[i], err.Error()) + } } } }