Merge pull request #24800 from thockin/validation_pt8-3

Automatic merge from submit-queue

Make name validators return string slices

Part of the larger validation PR, broken out for easier review and merge.  Builds on previous PRs in the series.

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) {
+var ValidateClusterName = validation.NameIsDNSSubdomain
-	return validation.NameIsDNSSubdomain(name, prefix)
-}
 
 func ValidateClusterSpec(spec *federation.ClusterSpec, fieldPath *field.Path) field.ErrorList {
 	allErrs := field.ErrorList{}

pkg/api/rest/resttest/resttest.go

@@ -367,7 +367,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)
 		}
 	}
 
@@ -379,7 +379,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)
 		}
 	}
 }

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) {
+	for _, msg := range validation.IsQualifiedName(labelName) {
-		allErrs = append(allErrs, field.Invalid(fldPath, labelName, err))
+		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) {
+		for _, msg := range validation.IsValidLabelValue(v) {
-			allErrs = append(allErrs, field.Invalid(fldPath, v, err))
+			allErrs = append(allErrs, field.Invalid(fldPath, v, msg))
 		}
 	}
 	return allErrs

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 {

pkg/api/validation/name_test.go

@@ -25,32 +25,27 @@ 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: "",
 		},
 
@@ -58,92 +53,80 @@ func TestValidatePathSegmentName(t *testing.T) {
 		"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)
+		msgs := ValidatePathSegmentName(tc.Name, tc.Prefix)
-		if ok != tc.ExpectedOK {
+		if len(tc.ExpectedMsg) == 0 && len(msgs) > 0 {
-			t.Errorf("%s: expected ok=%v, got %v", k, tc.ExpectedOK, ok)
+			t.Errorf("%s: expected no message, got %v", k, msgs)
 		}
-		if len(tc.ExpectedMsg) == 0 && len(msg) > 0 {
+		if len(tc.ExpectedMsg) > 0 && len(msgs) == 0 {
-			t.Errorf("%s: expected no message, got %v", k, msg)
+			t.Errorf("%s: expected error message, got none", k)
 		}
-		if len(tc.ExpectedMsg) > 0 && !strings.Contains(msg, tc.ExpectedMsg) {
+		if len(tc.ExpectedMsg) > 0 && !strings.Contains(msgs[0], tc.ExpectedMsg) {
-			t.Errorf("%s: expected message containing %q, got %v", k, tc.ExpectedMsg, msg)
+			t.Errorf("%s: expected message containing %q, got %v", k, tc.ExpectedMsg, msgs[0])
 		}
 	}
 }

pkg/api/validation/validation.go

@@ -92,8 +92,8 @@ func ValidateAnnotations(annotations map[string]string, fldPath *field.Path) fie
 	allErrs := field.ErrorList{}
 	var totalSize int64
 	for k, v := range annotations {
-		for _, err := range validation.IsQualifiedName(strings.ToLower(k)) {
+		for _, msg := range validation.IsQualifiedName(strings.ToLower(k)) {
-			allErrs = append(allErrs, field.Invalid(fldPath, k, err))
+			allErrs = append(allErrs, field.Invalid(fldPath, k, msg))
 		}
 		totalSize += (int64)(len(k)) + (int64)(len(v))
 	}
@@ -166,9 +166,11 @@ func ValidateOwnerReferences(ownerReferences []api.OwnerReference, fldPath *fiel
 }
 
 // ValidateNameFunc validates that the provided name is valid for a given resource type.
-// Not all resources have the same validation rules for names. Prefix is true if the
+// Not all resources have the same validation rules for names. Prefix is true
-// name will have a value appended to it.
+// if the name will have a value appended to it.  If the names is not valid,
-type ValidateNameFunc func(name string, prefix bool) (bool, string)
+// this returns a list of descriptions of individual characteristics of the
+// value that were not valid.  Otherwise this returns an empty list or nil.
+type ValidateNameFunc func(name string, prefix bool) []string
 
 // maskTrailingDash replaces the final character of a string with a subdomain safe
 // value if is a dash.
@@ -182,106 +184,86 @@ func maskTrailingDash(name string) string {
 // ValidatePodName can be used to check whether the given pod name is valid.
 // Prefix indicates this name will be used as part of generation, in which case
 // trailing dashes are allowed.
-func ValidatePodName(name string, prefix bool) (bool, string) {
+var ValidatePodName = NameIsDNSSubdomain
-	return NameIsDNSSubdomain(name, prefix)
-}
 
 // ValidateReplicationControllerName can be used to check whether the given replication
 // controller name is valid.
 // Prefix indicates this name will be used as part of generation, in which case
 // trailing dashes are allowed.
-func ValidateReplicationControllerName(name string, prefix bool) (bool, string) {
+var ValidateReplicationControllerName = NameIsDNSSubdomain
-	return NameIsDNSSubdomain(name, prefix)
-}
 
 // ValidateServiceName can be used to check whether the given service name is valid.
 // Prefix indicates this name will be used as part of generation, in which case
 // trailing dashes are allowed.
-func ValidateServiceName(name string, prefix bool) (bool, string) {
+var ValidateServiceName = NameIsDNS952Label
-	return NameIsDNS952Label(name, prefix)
-}
 
 // ValidateNodeName can be used to check whether the given node name is valid.
 // Prefix indicates this name will be used as part of generation, in which case
 // trailing dashes are allowed.
-func ValidateNodeName(name string, prefix bool) (bool, string) {
+var ValidateNodeName = NameIsDNSSubdomain
-	return NameIsDNSSubdomain(name, prefix)
-}
 
 // ValidateNamespaceName can be used to check whether the given namespace name is valid.
 // Prefix indicates this name will be used as part of generation, in which case
 // trailing dashes are allowed.
-func ValidateNamespaceName(name string, prefix bool) (bool, string) {
+var ValidateNamespaceName = NameIsDNSLabel
-	return NameIsDNSLabel(name, prefix)
-}
 
 // ValidateLimitRangeName can be used to check whether the given limit range name is valid.
 // Prefix indicates this name will be used as part of generation, in which case
 // trailing dashes are allowed.
-func ValidateLimitRangeName(name string, prefix bool) (bool, string) {
+var ValidateLimitRangeName = NameIsDNSSubdomain
-	return NameIsDNSSubdomain(name, prefix)
-}
 
 // ValidateResourceQuotaName can be used to check whether the given
 // resource quota name is valid.
 // Prefix indicates this name will be used as part of generation, in which case
 // trailing dashes are allowed.
-func ValidateResourceQuotaName(name string, prefix bool) (bool, string) {
+var ValidateResourceQuotaName = NameIsDNSSubdomain
-	return NameIsDNSSubdomain(name, prefix)
-}
 
 // ValidateSecretName can be used to check whether the given secret name is valid.
 // Prefix indicates this name will be used as part of generation, in which case
 // trailing dashes are allowed.
-func ValidateSecretName(name string, prefix bool) (bool, string) {
+var ValidateSecretName = NameIsDNSSubdomain
-	return NameIsDNSSubdomain(name, prefix)
-}
 
 // ValidateServiceAccountName can be used to check whether the given service account name is valid.
 // Prefix indicates this name will be used as part of generation, in which case
 // trailing dashes are allowed.
-func ValidateServiceAccountName(name string, prefix bool) (bool, string) {
+var ValidateServiceAccountName = NameIsDNSSubdomain
-	return NameIsDNSSubdomain(name, prefix)
-}
 
 // ValidateEndpointsName can be used to check whether the given endpoints name is valid.
 // Prefix indicates this name will be used as part of generation, in which case
 // trailing dashes are allowed.
-func ValidateEndpointsName(name string, prefix bool) (bool, string) {
+var ValidateEndpointsName = NameIsDNSSubdomain
-	return NameIsDNSSubdomain(name, prefix)
-}
 
 // NameIsDNSSubdomain is a ValidateNameFunc for names that must be a DNS subdomain.
-func NameIsDNSSubdomain(name string, prefix bool) (bool, string) {
+func NameIsDNSSubdomain(name string, prefix bool) []string {
 	if prefix {
 		name = maskTrailingDash(name)
 	}
 	if validation.IsDNS1123Subdomain(name) {
-		return true, ""
+		return nil
 	}
-	return false, DNSSubdomainErrorMsg
+	return []string{DNSSubdomainErrorMsg}
 }
 
 // NameIsDNSLabel is a ValidateNameFunc for names that must be a DNS 1123 label.
-func NameIsDNSLabel(name string, prefix bool) (bool, string) {
+func NameIsDNSLabel(name string, prefix bool) []string {
 	if prefix {
 		name = maskTrailingDash(name)
 	}
 	if validation.IsDNS1123Label(name) {
-		return true, ""
+		return nil
 	}
-	return false, DNS1123LabelErrorMsg
+	return []string{DNS1123LabelErrorMsg}
 }
 
 // NameIsDNS952Label is a ValidateNameFunc for names that must be a DNS 952 label.
-func NameIsDNS952Label(name string, prefix bool) (bool, string) {
+func NameIsDNS952Label(name string, prefix bool) []string {
 	if prefix {
 		name = maskTrailingDash(name)
 	}
 	if validation.IsDNS952Label(name) {
-		return true, ""
+		return nil
 	}
-	return false, DNS952LabelErrorMsg
+	return []string{DNS952LabelErrorMsg}
 }
 
 // Validates that given value is not negative.
@@ -318,8 +300,8 @@ func ValidateObjectMeta(meta *api.ObjectMeta, requiresNamespace bool, nameFn Val
 	allErrs := field.ErrorList{}
 
 	if len(meta.GenerateName) != 0 {
-		if ok, qualifier := nameFn(meta.GenerateName, true); !ok {
+		for _, msg := range nameFn(meta.GenerateName, true) {
-			allErrs = append(allErrs, field.Invalid(fldPath.Child("generateName"), meta.GenerateName, qualifier))
+			allErrs = append(allErrs, field.Invalid(fldPath.Child("generateName"), meta.GenerateName, msg))
 		}
 	}
 	// If the generated name validates, but the calculated value does not, it's a problem with generation, and we
@@ -328,15 +310,17 @@ func ValidateObjectMeta(meta *api.ObjectMeta, requiresNamespace bool, nameFn Val
 	if len(meta.Name) == 0 {
 		allErrs = append(allErrs, field.Required(fldPath.Child("name"), "name or generateName is required"))
 	} else {
-		if ok, qualifier := nameFn(meta.Name, false); !ok {
+		for _, msg := range nameFn(meta.Name, false) {
-			allErrs = append(allErrs, field.Invalid(fldPath.Child("name"), meta.Name, qualifier))
+			allErrs = append(allErrs, field.Invalid(fldPath.Child("name"), meta.Name, msg))
 		}
 	}
 	if requiresNamespace {
 		if len(meta.Namespace) == 0 {
 			allErrs = append(allErrs, field.Required(fldPath.Child("namespace"), ""))
-		} else if ok, _ := ValidateNamespaceName(meta.Namespace, false); !ok {
+		} else {
-			allErrs = append(allErrs, field.Invalid(fldPath.Child("namespace"), meta.Namespace, DNS1123LabelErrorMsg))
+			for _, msg := range ValidateNamespaceName(meta.Namespace, false) {
+				allErrs = append(allErrs, field.Invalid(fldPath.Child("namespace"), meta.Namespace, msg))
+			}
 		}
 	} else {
 		if len(meta.Namespace) != 0 {
@@ -820,9 +804,9 @@ func validateAzureFile(azure *api.AzureFileVolumeSource, fldPath *field.Path) fi
 	return allErrs
 }
 
-func ValidatePersistentVolumeName(name string, prefix bool) (bool, string) {
+// ValidatePersistentVolumeName checks that a name is appropriate for a
-	return NameIsDNSSubdomain(name, prefix)
+// PersistentVolumeName object.
-}
+var ValidatePersistentVolumeName = NameIsDNSSubdomain
 
 var supportedAccessModes = sets.NewString(string(api.ReadWriteOnce), string(api.ReadOnlyMany), string(api.ReadWriteMany))
 
@@ -1546,13 +1530,13 @@ func ValidatePodSpec(spec *api.PodSpec, fldPath *field.Path) field.ErrorList {
 	allErrs = append(allErrs, ValidatePodSecurityContext(spec.SecurityContext, spec, fldPath, fldPath.Child("securityContext"))...)
 	allErrs = append(allErrs, validateImagePullSecrets(spec.ImagePullSecrets, fldPath.Child("imagePullSecrets"))...)
 	if len(spec.ServiceAccountName) > 0 {
-		if ok, msg := ValidateServiceAccountName(spec.ServiceAccountName, false); !ok {
+		for _, msg := range ValidateServiceAccountName(spec.ServiceAccountName, false) {
 			allErrs = append(allErrs, field.Invalid(fldPath.Child("serviceAccountName"), spec.ServiceAccountName, msg))
 		}
 	}
 
 	if len(spec.NodeName) > 0 {
-		if ok, msg := ValidateNodeName(spec.NodeName, false); !ok {
+		for _, msg := range ValidateNodeName(spec.NodeName, false) {
 			allErrs = append(allErrs, field.Invalid(fldPath.Child("nodeName"), spec.NodeName, msg))
 		}
 	}
@@ -1646,8 +1630,8 @@ func validatePodAffinityTerm(podAffinityTerm api.PodAffinityTerm, allowEmptyTopo
 	allErrs := field.ErrorList{}
 	allErrs = append(allErrs, unversionedvalidation.ValidateLabelSelector(podAffinityTerm.LabelSelector, fldPath.Child("matchExpressions"))...)
 	for _, name := range podAffinityTerm.Namespaces {
-		if ok, _ := ValidateNamespaceName(name, false); !ok {
+		for _, msg := range ValidateNamespaceName(name, false) {
-			allErrs = append(allErrs, field.Invalid(fldPath.Child("namespace"), name, DNS1123LabelErrorMsg))
+			allErrs = append(allErrs, field.Invalid(fldPath.Child("namespace"), name, msg))
 		}
 	}
 	if !allowEmptyTopologyKey && len(podAffinityTerm.TopologyKey) == 0 {
@@ -2296,8 +2280,8 @@ func ValidateNodeUpdate(node, oldNode *api.Node) field.ErrorList {
 // Refer to docs/design/resources.md for more details.
 func validateResourceName(value string, fldPath *field.Path) field.ErrorList {
 	allErrs := field.ErrorList{}
-	for _, err := range validation.IsQualifiedName(value) {
+	for _, msg := range validation.IsQualifiedName(value) {
-		allErrs = append(allErrs, field.Invalid(fldPath, value, err))
+		allErrs = append(allErrs, field.Invalid(fldPath, value, msg))
 	}
 	if len(allErrs) != 0 {
 		return allErrs
@@ -2339,8 +2323,8 @@ func validateResourceQuotaResourceName(value string, fldPath *field.Path) field.
 // Validate limit range types
 func validateLimitRangeTypeName(value string, fldPath *field.Path) field.ErrorList {
 	allErrs := field.ErrorList{}
-	for _, err := range validation.IsQualifiedName(value) {
+	for _, msg := range validation.IsQualifiedName(value) {
-		allErrs = append(allErrs, field.Invalid(fldPath, value, err))
+		allErrs = append(allErrs, field.Invalid(fldPath, value, msg))
 	}
 	if len(allErrs) != 0 {
 		return allErrs
@@ -2599,9 +2583,7 @@ func ValidateSecretUpdate(newSecret, oldSecret *api.Secret) field.ErrorList {
 // ValidateConfigMapName can be used to check whether the given ConfigMap name is valid.
 // Prefix indicates this name will be used as part of generation, in which case
 // trailing dashes are allowed.
-func ValidateConfigMapName(name string, prefix bool) (bool, string) {
+var ValidateConfigMapName = NameIsDNSSubdomain
-	return NameIsDNSSubdomain(name, prefix)
-}
 
 // ValidateConfigMap tests whether required fields in the ConfigMap are set.
 func ValidateConfigMap(cfg *api.ConfigMap) field.ErrorList {
@@ -2813,8 +2795,8 @@ func ValidateNamespace(namespace *api.Namespace) field.ErrorList {
 // Validate finalizer names
 func validateFinalizerName(stringValue string, fldPath *field.Path) field.ErrorList {
 	allErrs := field.ErrorList{}
-	for _, err := range validation.IsQualifiedName(stringValue) {
+	for _, msg := range validation.IsQualifiedName(stringValue) {
-		allErrs = append(allErrs, field.Invalid(fldPath, stringValue, err))
+		allErrs = append(allErrs, field.Invalid(fldPath, stringValue, msg))
 	}
 	if len(allErrs) != 0 {
 		return allErrs
@@ -3020,8 +3002,8 @@ func ValidateLoadBalancerStatus(status *api.LoadBalancerStatus, fldPath *field.P
 			}
 		}
 		if len(ingress.Hostname) > 0 {
-			if valid, errMsg := NameIsDNSSubdomain(ingress.Hostname, false); !valid {
+			for _, msg := range NameIsDNSSubdomain(ingress.Hostname, false) {
-				allErrs = append(allErrs, field.Invalid(idxPath.Child("hostname"), ingress.Hostname, errMsg))
+				allErrs = append(allErrs, field.Invalid(idxPath.Child("hostname"), ingress.Hostname, msg))
 			}
 			if isIP := (net.ParseIP(ingress.Hostname) != nil); isIP {
 				allErrs = append(allErrs, field.Invalid(idxPath.Child("hostname"), ingress.Hostname, "must be a DNS name, not an IP address"))

pkg/api/validation/validation_test.go

@@ -47,11 +47,11 @@ func TestValidateObjectMetaCustomName(t *testing.T) {
 	errs := ValidateObjectMeta(
 		&api.ObjectMeta{Name: "test", GenerateName: "foo"},
 		false,
-		func(s string, prefix bool) (bool, string) {
+		func(s string, prefix bool) []string {
 			if s == "test" {
-				return true, ""
+				return nil
 			}
-			return false, "name-gen"
+			return []string{"name-gen"}
 		},
 		field.NewPath("field"))
 	if len(errs) != 1 {
@@ -67,8 +67,8 @@ func TestValidateObjectMetaNamespaces(t *testing.T) {
 	errs := ValidateObjectMeta(
 		&api.ObjectMeta{Name: "test", Namespace: "foo.bar"},
 		true,
-		func(s string, prefix bool) (bool, string) {
+		func(s string, prefix bool) []string {
-			return true, ""
+			return nil
 		},
 		field.NewPath("field"))
 	if len(errs) != 1 {
@@ -86,8 +86,8 @@ func TestValidateObjectMetaNamespaces(t *testing.T) {
 	errs = ValidateObjectMeta(
 		&api.ObjectMeta{Name: "test", Namespace: string(b)},
 		true,
-		func(s string, prefix bool) (bool, string) {
+		func(s string, prefix bool) []string {
-			return true, ""
+			return nil
 		},
 		field.NewPath("field"))
 	if len(errs) != 1 {
@@ -132,8 +132,8 @@ func TestValidateObjectMetaOwnerReferences(t *testing.T) {
 		errs := ValidateObjectMeta(
 			&api.ObjectMeta{Name: "test", Namespace: "test", OwnerReferences: tc.ownerReferences},
 			true,
-			func(s string, prefix bool) (bool, string) {
+			func(s string, prefix bool) []string {
-				return true, ""
+				return nil
 			},
 			field.NewPath("field"))
 		if len(errs) != 0 && !tc.expectError {

pkg/apis/apps/validation/validation.go

@@ -28,11 +28,10 @@ import (
 	"k8s.io/kubernetes/pkg/util/validation/field"
 )
 
-// ValidatePetSetName can be used to check whether the given PetSet
+// ValidatePetSetName can be used to check whether the given PetSet name is valid.
-// name is valid.
 // Prefix indicates this name will be used as part of generation, in which case
 // trailing dashes are allowed.
-func ValidatePetSetName(name string, prefix bool) (bool, string) {
+func ValidatePetSetName(name string, prefix bool) []string {
 	// TODO: Validate that there's name for the suffix inserted by the pets.
 	// Currently this is just "-index". In the future we may allow a user
 	// specified list of suffixes and we need  to validate the longest one.

pkg/apis/autoscaling/validation/validation.go

@@ -39,10 +39,7 @@ func ValidateScale(scale *autoscaling.Scale) field.ErrorList {
 
 // ValidateHorizontalPodAutoscaler can be used to check whether the given autoscaler name is valid.
 // Prefix indicates this name will be used as part of generation, in which case trailing dashes are allowed.
-func ValidateHorizontalPodAutoscalerName(name string, prefix bool) (bool, string) {
+var ValidateHorizontalPodAutoscalerName = apivalidation.ValidateReplicationControllerName
-	// TODO: finally move it to pkg/api/validation and use nameIsDNSSubdomain function
-	return apivalidation.ValidateReplicationControllerName(name, prefix)
-}
 
 func validateHorizontalPodAutoscalerSpec(autoscaler autoscaling.HorizontalPodAutoscalerSpec, fldPath *field.Path) field.ErrorList {
 	allErrs := field.ErrorList{}
@@ -68,14 +65,18 @@ func ValidateCrossVersionObjectReference(ref autoscaling.CrossVersionObjectRefer
 	allErrs := field.ErrorList{}
 	if len(ref.Kind) == 0 {
 		allErrs = append(allErrs, field.Required(fldPath.Child("kind"), ""))
-	} else if ok, msg := apivalidation.IsValidPathSegmentName(ref.Kind); !ok {
+	} else {
-		allErrs = append(allErrs, field.Invalid(fldPath.Child("kind"), ref.Kind, msg))
+		for _, msg := range apivalidation.IsValidPathSegmentName(ref.Kind) {
+			allErrs = append(allErrs, field.Invalid(fldPath.Child("kind"), ref.Kind, msg))
+		}
 	}
 
 	if len(ref.Name) == 0 {
 		allErrs = append(allErrs, field.Required(fldPath.Child("name"), ""))
-	} else if ok, msg := apivalidation.IsValidPathSegmentName(ref.Name); !ok {
+	} else {
-		allErrs = append(allErrs, field.Invalid(fldPath.Child("name"), ref.Name, msg))
+		for _, msg := range apivalidation.IsValidPathSegmentName(ref.Name) {
+			allErrs = append(allErrs, field.Invalid(fldPath.Child("name"), ref.Name, msg))
+		}
 	}
 
 	return allErrs

pkg/apis/extensions/validation/validation.go

@@ -43,21 +43,21 @@ func ValidateThirdPartyResourceUpdate(update, old *extensions.ThirdPartyResource
 	return allErrs
 }
 
-func ValidateThirdPartyResourceName(name string, prefix bool) (bool, string) {
+func ValidateThirdPartyResourceName(name string, prefix bool) []string {
 	// Make sure it's a valid DNS subdomain
-	if ok, msg := apivalidation.NameIsDNSSubdomain(name, prefix); !ok {
+	if msgs := apivalidation.NameIsDNSSubdomain(name, prefix); len(msgs) != 0 {
-		return ok, msg
+		return msgs
 	}
 
 	// Make sure it's at least three segments (kind + two-segment group name)
 	if !prefix {
 		parts := strings.Split(name, ".")
 		if len(parts) < 3 {
-			return false, "must be at least three segments long: <kind>.<domain>.<tld>"
+			return []string{"must be at least three segments long: <kind>.<domain>.<tld>"}
 		}
 	}
 
-	return true, ""
+	return nil
 }
 
 func ValidateThirdPartyResource(obj *extensions.ThirdPartyResource) field.ErrorList {
@@ -137,14 +137,10 @@ func ValidateDaemonSetSpec(spec *extensions.DaemonSetSpec, fldPath *field.Path)
 // ValidateDaemonSetName can be used to check whether the given daemon set name is valid.
 // Prefix indicates this name will be used as part of generation, in which case
 // trailing dashes are allowed.
-func ValidateDaemonSetName(name string, prefix bool) (bool, string) {
+var ValidateDaemonSetName = apivalidation.NameIsDNSSubdomain
-	return apivalidation.NameIsDNSSubdomain(name, prefix)
-}
 
 // Validates that the given name can be used as a deployment name.
-func ValidateDeploymentName(name string, prefix bool) (bool, string) {
+var ValidateDeploymentName = apivalidation.NameIsDNSSubdomain
-	return apivalidation.NameIsDNSSubdomain(name, prefix)
-}
 
 func ValidatePositiveIntOrPercent(intOrPercent intstr.IntOrString, fldPath *field.Path) field.ErrorList {
 	allErrs := field.ErrorList{}
@@ -305,9 +301,7 @@ func ValidateIngress(ingress *extensions.Ingress) field.ErrorList {
 }
 
 // ValidateIngressName validates that the given name can be used as an Ingress name.
-func ValidateIngressName(name string, prefix bool) (bool, string) {
+var ValidateIngressName = apivalidation.NameIsDNSSubdomain
-	return apivalidation.NameIsDNSSubdomain(name, prefix)
-}
 
 func validateIngressTLS(spec *extensions.IngressSpec, fldPath *field.Path) field.ErrorList {
 	allErrs := field.ErrorList{}
@@ -357,8 +351,8 @@ func validateIngressRules(IngressRules []extensions.IngressRule, fldPath *field.
 		if len(ih.Host) > 0 {
 			// TODO: Ports and ips are allowed in the host part of a url
 			// according to RFC 3986, consider allowing them.
-			if valid, errMsg := apivalidation.NameIsDNSSubdomain(ih.Host, false); !valid {
+			for _, msg := range apivalidation.NameIsDNSSubdomain(ih.Host, false) {
-				allErrs = append(allErrs, field.Invalid(fldPath.Index(i).Child("host"), ih.Host, errMsg))
+				allErrs = append(allErrs, field.Invalid(fldPath.Index(i).Child("host"), ih.Host, msg))
 			}
 			if isIP := (net.ParseIP(ih.Host) != nil); isIP {
 				allErrs = append(allErrs, field.Invalid(fldPath.Index(i).Child("host"), ih.Host, "must be a DNS name, not an IP address"))
@@ -413,8 +407,10 @@ func validateIngressBackend(backend *extensions.IngressBackend, fldPath *field.P
 	// All backends must reference a single local service by name, and a single service port by name or number.
 	if len(backend.ServiceName) == 0 {
 		return append(allErrs, field.Required(fldPath.Child("serviceName"), ""))
-	} else if ok, errMsg := apivalidation.ValidateServiceName(backend.ServiceName, false); !ok {
+	} else {
-		allErrs = append(allErrs, field.Invalid(fldPath.Child("serviceName"), backend.ServiceName, errMsg))
+		for _, msg := range apivalidation.ValidateServiceName(backend.ServiceName, false) {
+			allErrs = append(allErrs, field.Invalid(fldPath.Child("serviceName"), backend.ServiceName, msg))
+		}
 	}
 	if backend.ServicePort.Type == intstr.String {
 		if !validation.IsDNS1123Label(backend.ServicePort.StrVal) {
@@ -444,9 +440,7 @@ func ValidateScale(scale *extensions.Scale) field.ErrorList {
 // name is valid.
 // Prefix indicates this name will be used as part of generation, in which case
 // trailing dashes are allowed.
-func ValidateReplicaSetName(name string, prefix bool) (bool, string) {
+var ValidateReplicaSetName = apivalidation.NameIsDNSSubdomain
-	return apivalidation.NameIsDNSSubdomain(name, prefix)
-}
 
 // ValidateReplicaSet tests if required fields in the ReplicaSet are set.
 func ValidateReplicaSet(rs *extensions.ReplicaSet) field.ErrorList {
@@ -526,9 +520,7 @@ func ValidatePodTemplateSpecForReplicaSet(template *api.PodTemplateSpec, selecto
 // pod security policy name is valid.
 // Prefix indicates this name will be used as part of generation, in which case
 // trailing dashes are allowed.
-func ValidatePodSecurityPolicyName(name string, prefix bool) (bool, string) {
+var ValidatePodSecurityPolicyName = apivalidation.NameIsDNSSubdomain
-	return apivalidation.NameIsDNSSubdomain(name, prefix)
-}
 
 func ValidatePodSecurityPolicy(psp *extensions.PodSecurityPolicy) field.ErrorList {
 	allErrs := field.ErrorList{}

pkg/apis/rbac/validation/validation.go

@@ -25,7 +25,7 @@ import (
 // Minimal validation of names for roles and bindings. Identical to the validation for Openshift. See:
 // * https://github.com/kubernetes/kubernetes/blob/60db50/pkg/api/validation/name.go
 // * https://github.com/openshift/origin/blob/388478/pkg/api/helpers.go
-func minimalNameRequirements(name string, prefix bool) (bool, string) {
+func minimalNameRequirements(name string, prefix bool) []string {
 	return validation.IsValidPathSegmentName(name)
 }
 
@@ -86,16 +86,16 @@ func validateRoleBinding(roleBinding *rbac.RoleBinding, isNamespaced bool, fldPa
 
 	// roleRef namespace is empty when referring to global policy.
 	if len(roleBinding.RoleRef.Namespace) > 0 {
-		if ok, reason := validation.ValidateNamespaceName(roleBinding.RoleRef.Namespace, false); !ok {
+		for _, msg := range validation.ValidateNamespaceName(roleBinding.RoleRef.Namespace, false) {
-			allErrs = append(allErrs, field.Invalid(fldPath.Child("roleRef", "namespace"), roleBinding.RoleRef.Namespace, reason))
+			allErrs = append(allErrs, field.Invalid(fldPath.Child("roleRef", "namespace"), roleBinding.RoleRef.Namespace, msg))
 		}
 	}
 
 	if len(roleBinding.RoleRef.Name) == 0 {
 		allErrs = append(allErrs, field.Required(fldPath.Child("roleRef", "name"), ""))
 	} else {
-		if valid, err := minimalNameRequirements(roleBinding.RoleRef.Name, false); !valid {
+		for _, msg := range minimalNameRequirements(roleBinding.RoleRef.Name, false) {
-			allErrs = append(allErrs, field.Invalid(fldPath.Child("roleRef", "name"), roleBinding.RoleRef.Name, err))
+			allErrs = append(allErrs, field.Invalid(fldPath.Child("roleRef", "name"), roleBinding.RoleRef.Name, msg))
 		}
 	}
 
@@ -119,8 +119,10 @@ func validateRoleBindingSubject(subject rbac.Subject, isNamespaced bool, fldPath
 
 	switch subject.Kind {
 	case rbac.ServiceAccountKind:
-		if valid, reason := validation.ValidateServiceAccountName(subject.Name, false); len(subject.Name) > 0 && !valid {
+		if len(subject.Name) > 0 {
-			allErrs = append(allErrs, field.Invalid(fldPath.Child("name"), subject.Name, reason))
+			for _, msg := range validation.ValidateServiceAccountName(subject.Name, false) {
+				allErrs = append(allErrs, field.Invalid(fldPath.Child("name"), subject.Name, msg))
+			}
 		}
 		if !isNamespaced && len(subject.Namespace) == 0 {
 			allErrs = append(allErrs, field.Required(fldPath.Child("namespace"), ""))

pkg/client/restclient/request.go

@@ -179,8 +179,8 @@ func (r *Request) Resource(resource string) *Request {
 		r.err = fmt.Errorf("resource already set to %q, cannot change to %q", r.resource, resource)
 		return r
 	}
-	if ok, msg := validation.IsValidPathSegmentName(resource); !ok {
+	if msgs := validation.IsValidPathSegmentName(resource); len(msgs) != 0 {
-		r.err = fmt.Errorf("invalid resource %q: %s", resource, msg)
+		r.err = fmt.Errorf("invalid resource %q: %v", resource, msgs)
 		return r
 	}
 	r.resource = resource
@@ -199,8 +199,8 @@ func (r *Request) SubResource(subresources ...string) *Request {
 		return r
 	}
 	for _, s := range subresources {
-		if ok, msg := validation.IsValidPathSegmentName(s); !ok {
+		if msgs := validation.IsValidPathSegmentName(s); len(msgs) != 0 {
-			r.err = fmt.Errorf("invalid subresource %q: %s", s, msg)
+			r.err = fmt.Errorf("invalid subresource %q: %v", s, msgs)
 			return r
 		}
 	}
@@ -221,8 +221,8 @@ func (r *Request) Name(resourceName string) *Request {
 		r.err = fmt.Errorf("resource name already set to %q, cannot change to %q", r.resourceName, resourceName)
 		return r
 	}
-	if ok, msg := validation.IsValidPathSegmentName(resourceName); !ok {
+	if msgs := validation.IsValidPathSegmentName(resourceName); len(msgs) != 0 {
-		r.err = fmt.Errorf("invalid resource name %q: %s", resourceName, msg)
+		r.err = fmt.Errorf("invalid resource name %q: %v", resourceName, msgs)
 		return r
 	}
 	r.resourceName = resourceName
@@ -238,8 +238,8 @@ func (r *Request) Namespace(namespace string) *Request {
 		r.err = fmt.Errorf("namespace already set to %q, cannot change to %q", r.namespace, namespace)
 		return r
 	}
-	if ok, msg := validation.IsValidPathSegmentName(namespace); !ok {
+	if msgs := validation.IsValidPathSegmentName(namespace); len(msgs) != 0 {
-		r.err = fmt.Errorf("invalid namespace %q: %s", namespace, msg)
+		r.err = fmt.Errorf("invalid namespace %q: %v", namespace, msgs)
 		return r
 	}
 	r.namespaceSet = true

pkg/controller/controller_utils.go

@@ -399,7 +399,7 @@ func getPodsAnnotationSet(template *api.PodTemplateSpec, object runtime.Object)
 func getPodsPrefix(controllerName string) string {
 	// use the dash (if the name isn't too long) to make the pod name a bit prettier
 	prefix := fmt.Sprintf("%s-", controllerName)
-	if ok, _ := validation.ValidatePodName(prefix, true); !ok {
+	if len(validation.ValidatePodName(prefix, true)) != 0 {
 		prefix = controllerName
 	}
 	return prefix

pkg/kubelet/client/kubelet_client.go

@@ -21,6 +21,7 @@ import (
 	"fmt"
 	"net"
 	"net/http"
+	"strings"
 	"time"
 
 	"k8s.io/kubernetes/pkg/api"
@@ -98,8 +99,8 @@ func NewStaticKubeletClient(config *KubeletClientConfig) (KubeletClient, error)
 
 // In default HTTPKubeletClient ctx is unused.
 func (c *HTTPKubeletClient) GetConnectionInfo(ctx api.Context, nodeName string) (string, uint, http.RoundTripper, error) {
-	if ok, msg := validation.ValidateNodeName(nodeName, false); !ok {
+	if errs := validation.ValidateNodeName(nodeName, false); len(errs) != 0 {
-		return "", 0, nil, fmt.Errorf("invalid node name: %s", msg)
+		return "", 0, nil, fmt.Errorf("invalid node name: %s", strings.Join(errs, ";"))
 	}
 	scheme := "http"
 	if c.Config.EnableHttps {

pkg/registry/generic/registry/store.go

@@ -19,6 +19,7 @@ package registry
 import (
 	"fmt"
 	"reflect"
+	"strings"
 	"sync"
 
 	"k8s.io/kubernetes/pkg/api"
@@ -137,8 +138,8 @@ func NamespaceKeyFunc(ctx api.Context, prefix string, name string) (string, erro
 	if len(name) == 0 {
 		return "", kubeerr.NewBadRequest("Name parameter required.")
 	}
-	if ok, msg := validation.IsValidPathSegmentName(name); !ok {
+	if msgs := validation.IsValidPathSegmentName(name); len(msgs) != 0 {
-		return "", kubeerr.NewBadRequest(fmt.Sprintf("Name parameter invalid: %v.", msg))
+		return "", kubeerr.NewBadRequest(fmt.Sprintf("Name parameter invalid: %q: %s", name, strings.Join(msgs, ";")))
 	}
 	key = key + "/" + name
 	return key, nil
@@ -149,8 +150,8 @@ func NoNamespaceKeyFunc(ctx api.Context, prefix string, name string) (string, er
 	if len(name) == 0 {
 		return "", kubeerr.NewBadRequest("Name parameter required.")
 	}
-	if ok, msg := validation.IsValidPathSegmentName(name); !ok {
+	if msgs := validation.IsValidPathSegmentName(name); len(msgs) != 0 {
-		return "", kubeerr.NewBadRequest(fmt.Sprintf("Name parameter invalid: %v.", msg))
+		return "", kubeerr.NewBadRequest(fmt.Sprintf("Name parameter invalid: %q: %s", name, strings.Join(msgs, ";")))
 	}
 	key := prefix + "/" + name
 	return key, nil

pkg/serviceaccount/util.go

@@ -52,10 +52,10 @@ func SplitUsername(username string) (string, string, error) {
 		return "", "", invalidUsernameErr
 	}
 	namespace, name := parts[0], parts[1]
-	if ok, _ := validation.ValidateNamespaceName(namespace, false); !ok {
+	if len(validation.ValidateNamespaceName(namespace, false)) != 0 {
 		return "", "", invalidUsernameErr
 	}
-	if ok, _ := validation.ValidateServiceAccountName(name, false); !ok {
+	if len(validation.ValidateServiceAccountName(name, false)) != 0 {
 		return "", "", invalidUsernameErr
 	}
 	return namespace, name, nil

pkg/storage/util.go

@@ -71,8 +71,8 @@ func NamespaceKeyFunc(prefix string, obj runtime.Object) (string, error) {
 		return "", err
 	}
 	name := meta.GetName()
-	if ok, msg := validation.IsValidPathSegmentName(name); !ok {
+	if msgs := validation.IsValidPathSegmentName(name); len(msgs) != 0 {
-		return "", fmt.Errorf("invalid name: %v", msg)
+		return "", fmt.Errorf("invalid name: %v", msgs)
 	}
 	return prefix + "/" + meta.GetNamespace() + "/" + name, nil
 }
@@ -83,8 +83,8 @@ func NoNamespaceKeyFunc(prefix string, obj runtime.Object) (string, error) {
 		return "", err
 	}
 	name := meta.GetName()
-	if ok, msg := validation.IsValidPathSegmentName(name); !ok {
+	if msgs := validation.IsValidPathSegmentName(name); len(msgs) != 0 {
-		return "", fmt.Errorf("invalid name: %v", msg)
+		return "", fmt.Errorf("invalid name: %v", msgs)
 	}
 	return prefix + "/" + name, nil
 }