Make name validators return string slices

332054781 · May 18, 2016 · 152c86a · 152c86a
1 parent 66d0d87
commit 152c86a
Show file tree

Hide file tree

Showing 17 changed files with 140 additions and 181 deletions.
diff --git a/federation/apis/federation/validation/validation.go b/federation/apis/federation/validation/validation.go
@@ -22,9 +22,7 @@ import (
 	"k8s.io/kubernetes/pkg/util/validation/field"
 )
 
-func ValidateClusterName(name string, prefix bool) (bool, string) {
-	return validation.NameIsDNSSubdomain(name, prefix)
-}
+var ValidateClusterName = validation.NameIsDNSSubdomain
 
 func ValidateClusterSpec(spec *federation.ClusterSpec, fieldPath *field.Path) field.ErrorList {
 	allErrs := field.ErrorList{}

diff --git a/pkg/api/rest/resttest/resttest.go b/pkg/api/rest/resttest/resttest.go
@@ -366,7 +366,7 @@ func (t *Tester) testCreateValidatesNames(valid runtime.Object) {
 		ctx := t.TestContext()
 		_, err := t.storage.(rest.Creater).Create(ctx, objCopy)
 		if !errors.IsInvalid(err) {
-			t.Errorf("%s: Expected to get an invalid resource error, got %v", invalidName, err)
+			t.Errorf("%s: Expected to get an invalid resource error, got '%v'", invalidName, err)
 		}
 	}
 
@@ -378,7 +378,7 @@ func (t *Tester) testCreateValidatesNames(valid runtime.Object) {
 		ctx := t.TestContext()
 		_, err := t.storage.(rest.Creater).Create(ctx, objCopy)
 		if !errors.IsInvalid(err) {
-			t.Errorf("%s: Expected to get an invalid resource error, got %v", invalidSuffix, err)
+			t.Errorf("%s: Expected to get an invalid resource error, got '%v'", invalidSuffix, err)
 		}
 	}
 }

diff --git a/pkg/api/unversioned/validation/validation.go b/pkg/api/unversioned/validation/validation.go
@@ -55,8 +55,8 @@ func ValidateLabelSelectorRequirement(sr unversioned.LabelSelectorRequirement, f
 // ValidateLabelName validates that the label name is correctly defined.
 func ValidateLabelName(labelName string, fldPath *field.Path) field.ErrorList {
 	allErrs := field.ErrorList{}
-	for _, err := range validation.IsQualifiedName(labelName) {
-		allErrs = append(allErrs, field.Invalid(fldPath, labelName, err))
+	for _, msg := range validation.IsQualifiedName(labelName) {
+		allErrs = append(allErrs, field.Invalid(fldPath, labelName, msg))
 	}
 	return allErrs
 }
@@ -66,8 +66,8 @@ func ValidateLabels(labels map[string]string, fldPath *field.Path) field.ErrorLi
 	allErrs := field.ErrorList{}
 	for k, v := range labels {
 		allErrs = append(allErrs, ValidateLabelName(k, fldPath)...)
-		for _, err := range validation.IsValidLabelValue(v) {
-			allErrs = append(allErrs, field.Invalid(fldPath, v, err))
+		for _, msg := range validation.IsValidLabelValue(v) {
+			allErrs = append(allErrs, field.Invalid(fldPath, v, msg))
 		}
 	}
 	return allErrs

diff --git a/pkg/api/validation/name.go b/pkg/api/validation/name.go
@@ -28,36 +28,36 @@ var NameMayNotBe = []string{".", ".."}
 var NameMayNotContain = []string{"/", "%"}
 
 // IsValidPathSegmentName validates the name can be safely encoded as a path segment
-func IsValidPathSegmentName(name string) (bool, string) {
+func IsValidPathSegmentName(name string) []string {
 	for _, illegalName := range NameMayNotBe {
 		if name == illegalName {
-			return false, fmt.Sprintf(`name may not be %q`, illegalName)
+			return []string{fmt.Sprintf(`may not be '%s'`, illegalName)}
 		}
 	}
 
 	for _, illegalContent := range NameMayNotContain {
 		if strings.Contains(name, illegalContent) {
-			return false, fmt.Sprintf(`name may not contain %q`, illegalContent)
+			return []string{fmt.Sprintf(`may not contain '%s'`, illegalContent)}
 		}
 	}
 
-	return true, ""
+	return nil
 }
 
 // IsValidPathSegmentPrefix validates the name can be used as a prefix for a name which will be encoded as a path segment
 // It does not check for exact matches with disallowed names, since an arbitrary suffix might make the name valid
-func IsValidPathSegmentPrefix(name string) (bool, string) {
+func IsValidPathSegmentPrefix(name string) []string {
 	for _, illegalContent := range NameMayNotContain {
 		if strings.Contains(name, illegalContent) {
-			return false, fmt.Sprintf(`name may not contain %q`, illegalContent)
+			return []string{fmt.Sprintf(`may not contain '%s'`, illegalContent)}
 		}
 	}
 
-	return true, ""
+	return nil
 }
 
 // ValidatePathSegmentName validates the name can be safely encoded as a path segment
-func ValidatePathSegmentName(name string, prefix bool) (bool, string) {
+func ValidatePathSegmentName(name string, prefix bool) []string {
 	if prefix {
 		return IsValidPathSegmentPrefix(name)
 	} else {

diff --git a/pkg/api/validation/name_test.go b/pkg/api/validation/name_test.go
@@ -25,125 +25,108 @@ func TestValidatePathSegmentName(t *testing.T) {
 	testcases := map[string]struct {
 		Name        string
 		Prefix      bool
-		ExpectedOK  bool
 		ExpectedMsg string
 	}{
 		"empty": {
 			Name:        "",
 			Prefix:      false,
-			ExpectedOK:  true,
 			ExpectedMsg: "",
 		},
 		"empty,prefix": {
 			Name:        "",
 			Prefix:      true,
-			ExpectedOK:  true,
 			ExpectedMsg: "",
 		},
 
 		"valid": {
 			Name:        "foo.bar.baz",
 			Prefix:      false,
-			ExpectedOK:  true,
 			ExpectedMsg: "",
 		},
 		"valid,prefix": {
 			Name:        "foo.bar.baz",
 			Prefix:      true,
-			ExpectedOK:  true,
 			ExpectedMsg: "",
 		},
 
 		// Make sure mixed case, non DNS subdomain characters are tolerated
 		"valid complex": {
 			Name:        "sha256:ABCDEF012345@ABCDEF012345",
 			Prefix:      false,
-			ExpectedOK:  true,
 			ExpectedMsg: "",
 		},
 		// Make sure non-ascii characters are tolerated
 		"valid extended charset": {
 			Name:        "Iñtërnâtiônàlizætiøn",
 			Prefix:      false,
-			ExpectedOK:  true,
 			ExpectedMsg: "",
 		},
 
 		"dot": {
 			Name:        ".",
 			Prefix:      false,
-			ExpectedOK:  false,
 			ExpectedMsg: ".",
 		},
 		"dot leading": {
 			Name:        ".test",
 			Prefix:      false,
-			ExpectedOK:  true,
 			ExpectedMsg: "",
 		},
 		"dot,prefix": {
 			Name:        ".",
 			Prefix:      true,
-			ExpectedOK:  true, // allowed because a suffix could make it valid
 			ExpectedMsg: "",
 		},
 
 		"dot dot": {
 			Name:        "..",
 			Prefix:      false,
-			ExpectedOK:  false,
 			ExpectedMsg: "..",
 		},
 		"dot dot leading": {
 			Name:        "..test",
 			Prefix:      false,
-			ExpectedOK:  true,
 			ExpectedMsg: "",
 		},
 		"dot dot,prefix": {
 			Name:        "..",
 			Prefix:      true,
-			ExpectedOK:  true, // allowed because a suffix could make it valid
 			ExpectedMsg: "",
 		},
 
 		"slash": {
 			Name:        "foo/bar",
 			Prefix:      false,
-			ExpectedOK:  false,
 			ExpectedMsg: "/",
 		},
 		"slash,prefix": {
 			Name:        "foo/bar",
 			Prefix:      true,
-			ExpectedOK:  false,
 			ExpectedMsg: "/",
 		},
 
 		"percent": {
 			Name:        "foo%bar",
 			Prefix:      false,
-			ExpectedOK:  false,
 			ExpectedMsg: "%",
 		},
 		"percent,prefix": {
 			Name:        "foo%bar",
 			Prefix:      true,
-			ExpectedOK:  false,
 			ExpectedMsg: "%",
 		},
 	}
 
 	for k, tc := range testcases {
-		ok, msg := ValidatePathSegmentName(tc.Name, tc.Prefix)
-		if ok != tc.ExpectedOK {
-			t.Errorf("%s: expected ok=%v, got %v", k, tc.ExpectedOK, ok)
+		msgs := ValidatePathSegmentName(tc.Name, tc.Prefix)
+		if len(tc.ExpectedMsg) == 0 && len(msgs) > 0 {
+			t.Errorf("%s: expected no message, got %v", k, msgs)
 		}
-		if len(tc.ExpectedMsg) == 0 && len(msg) > 0 {
-			t.Errorf("%s: expected no message, got %v", k, msg)
+		if len(tc.ExpectedMsg) > 0 && len(msgs) == 0 {
+			t.Errorf("%s: expected error message, got none", k)
 		}
-		if len(tc.ExpectedMsg) > 0 && !strings.Contains(msg, tc.ExpectedMsg) {
-			t.Errorf("%s: expected message containing %q, got %v", k, tc.ExpectedMsg, msg)
+		if len(tc.ExpectedMsg) > 0 && !strings.Contains(msgs[0], tc.ExpectedMsg) {
+			t.Errorf("%s: expected message containing %q, got %v", k, tc.ExpectedMsg, msgs[0])
 		}
 	}
 }