Merge pull request #4174 from gmarek/master

Add more information to Validator error messages.

pkg/api/validation/validation.go

@@ -30,12 +30,26 @@ import (
 	"github.com/golang/glog"
 )
 
+const qualifiedNameErrorMsg string = "must match regex [" + util.DNS1123SubdomainFmt + " / ] " + util.DNS1123LabelFmt
+const cIdentifierErrorMsg string = "must match regex " + util.CIdentifierFmt
+const isNegativeErrorMsg string = "value must not be negative"
+
+func intervalErrorMsg(lo, hi int) string {
+	return fmt.Sprintf("must be greater than %d and less than %d", lo, hi)
+}
+
+var dnsSubdomainErrorMsg string = fmt.Sprintf("must have at most %d characters and match regex %s", util.DNS1123SubdomainMaxLength, util.DNS1123SubdomainFmt)
+var dnsLabelErrorMsg string = fmt.Sprintf("must have at most %d characters and match regex %s", util.DNS1123LabelMaxLength, util.DNS1123LabelFmt)
+var dns952LabelErrorMsg string = fmt.Sprintf("must have at most %d characters and match regex %s", util.DNS952LabelMaxLength, util.DNS952LabelFmt)
+var pdPartitionErrorMsg string = intervalErrorMsg(0, 255)
+var portRangeErrorMsg string = intervalErrorMsg(0, 65536)
+
 // ValidateLabels validates that a set of labels are correctly defined.
 func ValidateLabels(labels map[string]string, field string) errs.ValidationErrorList {
 	allErrs := errs.ValidationErrorList{}
 	for k := range labels {
 		if !util.IsQualifiedName(k) {
-			allErrs = append(allErrs, errs.NewFieldInvalid(field, k, ""))
+			allErrs = append(allErrs, errs.NewFieldInvalid(field, k, qualifiedNameErrorMsg))
 		}
 	}
 	return allErrs
@@ -46,7 +60,7 @@ func ValidateAnnotations(annotations map[string]string, field string) errs.Valid
 	allErrs := errs.ValidationErrorList{}
 	for k := range annotations {
 		if !util.IsQualifiedName(strings.ToLower(k)) {
-			allErrs = append(allErrs, errs.NewFieldInvalid(field, k, ""))
+			allErrs = append(allErrs, errs.NewFieldInvalid(field, k, qualifiedNameErrorMsg))
 		}
 	}
 	return allErrs
@@ -103,7 +117,7 @@ func nameIsDNSSubdomain(name string, prefix bool) (bool, string) {
 	if util.IsDNSSubdomain(name) {
 		return true, ""
 	}
-	return false, "name must be lowercase letters and numbers, with inline dashes or periods"
+	return false, dnsSubdomainErrorMsg
 }
 
 // nameIsDNS952Label is a ValidateNameFunc for names that must be a DNS 952 label.
@@ -114,7 +128,7 @@ func nameIsDNS952Label(name string, prefix bool) (bool, string) {
 	if util.IsDNS952Label(name) {
 		return true, ""
 	}
-	return false, "name must be lowercase letters, numbers, and dashes"
+	return false, dns952LabelErrorMsg
 }
 
 // ValidateObjectMeta validates an object's metadata on creation. It expects that name generation has already
@@ -141,7 +155,7 @@ func ValidateObjectMeta(meta *api.ObjectMeta, requiresNamespace bool, nameFn Val
 		if len(meta.Namespace) == 0 {
 			allErrs = append(allErrs, errs.NewFieldRequired("namespace", meta.Namespace))
 		} else if !util.IsDNSSubdomain(meta.Namespace) {
-			allErrs = append(allErrs, errs.NewFieldInvalid("namespace", meta.Namespace, ""))
+			allErrs = append(allErrs, errs.NewFieldInvalid("namespace", meta.Namespace, dnsSubdomainErrorMsg))
 		}
 	} else {
 		if len(meta.Namespace) != 0 {
@@ -194,7 +208,7 @@ func validateVolumes(volumes []api.Volume) (util.StringSet, errs.ValidationError
 		if len(vol.Name) == 0 {
 			el = append(el, errs.NewFieldRequired("name", vol.Name))
 		} else if !util.IsDNSLabel(vol.Name) {
-			el = append(el, errs.NewFieldInvalid("name", vol.Name, ""))
+			el = append(el, errs.NewFieldInvalid("name", vol.Name, dnsLabelErrorMsg))
 		} else if allNames.Has(vol.Name) {
 			el = append(el, errs.NewFieldDuplicate("name", vol.Name))
 		}
@@ -257,7 +271,7 @@ func validateGCEPersistentDisk(PD *api.GCEPersistentDisk) errs.ValidationErrorLi
 		allErrs = append(allErrs, errs.NewFieldRequired("fsType", PD.FSType))
 	}
 	if PD.Partition < 0 || PD.Partition > 255 {
-		allErrs = append(allErrs, errs.NewFieldInvalid("partition", PD.Partition, ""))
+		allErrs = append(allErrs, errs.NewFieldInvalid("partition", PD.Partition, pdPartitionErrorMsg))
 	}
 	return allErrs
 }
@@ -271,8 +285,8 @@ func validatePorts(ports []api.Port) errs.ValidationErrorList {
 	for i, port := range ports {
 		pErrs := errs.ValidationErrorList{}
 		if len(port.Name) > 0 {
-			if len(port.Name) > 63 || !util.IsDNSLabel(port.Name) {
+			if len(port.Name) > util.DNS1123LabelMaxLength || !util.IsDNSLabel(port.Name) {
-				pErrs = append(pErrs, errs.NewFieldInvalid("name", port.Name, ""))
+				pErrs = append(pErrs, errs.NewFieldInvalid("name", port.Name, dnsLabelErrorMsg))
 			} else if allNames.Has(port.Name) {
 				pErrs = append(pErrs, errs.NewFieldDuplicate("name", port.Name))
 			} else {
@@ -280,12 +294,12 @@ func validatePorts(ports []api.Port) errs.ValidationErrorList {
 			}
 		}
 		if port.ContainerPort == 0 {
-			pErrs = append(pErrs, errs.NewFieldRequired("containerPort", port.ContainerPort))
+			pErrs = append(pErrs, errs.NewFieldInvalid("containerPort", port.ContainerPort, portRangeErrorMsg))
 		} else if !util.IsValidPortNum(port.ContainerPort) {
-			pErrs = append(pErrs, errs.NewFieldInvalid("containerPort", port.ContainerPort, ""))
+			pErrs = append(pErrs, errs.NewFieldInvalid("containerPort", port.ContainerPort, portRangeErrorMsg))
 		}
 		if port.HostPort != 0 && !util.IsValidPortNum(port.HostPort) {
-			pErrs = append(pErrs, errs.NewFieldInvalid("hostPort", port.HostPort, ""))
+			pErrs = append(pErrs, errs.NewFieldInvalid("hostPort", port.HostPort, portRangeErrorMsg))
 		}
 		if len(port.Protocol) == 0 {
 			pErrs = append(pErrs, errs.NewFieldRequired("protocol", port.Protocol))
@@ -306,7 +320,7 @@ func validateEnv(vars []api.EnvVar) errs.ValidationErrorList {
 			vErrs = append(vErrs, errs.NewFieldRequired("name", ev.Name))
 		}
 		if !util.IsCIdentifier(ev.Name) {
-			vErrs = append(vErrs, errs.NewFieldInvalid("name", ev.Name, ""))
+			vErrs = append(vErrs, errs.NewFieldInvalid("name", ev.Name, cIdentifierErrorMsg))
 		}
 		allErrs = append(allErrs, vErrs.PrefixIndex(i)...)
 	}
@@ -430,7 +444,7 @@ func validateContainers(containers []api.Container, volumes util.StringSet) errs
 		if len(ctr.Name) == 0 {
 			cErrs = append(cErrs, errs.NewFieldRequired("name", ctr.Name))
 		} else if !util.IsDNSLabel(ctr.Name) {
-			cErrs = append(cErrs, errs.NewFieldInvalid("name", ctr.Name, ""))
+			cErrs = append(cErrs, errs.NewFieldInvalid("name", ctr.Name, dnsLabelErrorMsg))
 		} else if allNames.Has(ctr.Name) {
 			cErrs = append(cErrs, errs.NewFieldDuplicate("name", ctr.Name))
 		} else if ctr.Privileged && !capabilities.AllowPrivileged {
@@ -574,7 +588,7 @@ func ValidateService(service *api.Service) errs.ValidationErrorList {
 	allErrs = append(allErrs, ValidateObjectMeta(&service.ObjectMeta, true, ValidateServiceName).Prefix("metadata")...)
 
 	if !util.IsValidPortNum(service.Spec.Port) {
-		allErrs = append(allErrs, errs.NewFieldInvalid("spec.port", service.Spec.Port, ""))
+		allErrs = append(allErrs, errs.NewFieldInvalid("spec.port", service.Spec.Port, portRangeErrorMsg))
 	}
 	if len(service.Spec.Protocol) == 0 {
 		allErrs = append(allErrs, errs.NewFieldRequired("spec.protocol", service.Spec.Protocol))
@@ -635,7 +649,7 @@ func ValidateReplicationControllerSpec(spec *api.ReplicationControllerSpec) errs
 		allErrs = append(allErrs, errs.NewFieldRequired("selector", spec.Selector))
 	}
 	if spec.Replicas < 0 {
-		allErrs = append(allErrs, errs.NewFieldInvalid("replicas", spec.Replicas, ""))
+		allErrs = append(allErrs, errs.NewFieldInvalid("replicas", spec.Replicas, isNegativeErrorMsg))
 	}
 
 	if spec.Template == nil {
@@ -694,7 +708,7 @@ func ValidateBoundPod(pod *api.BoundPod) errs.ValidationErrorList {
 	if len(pod.Namespace) == 0 {
 		allErrs = append(allErrs, errs.NewFieldRequired("namespace", pod.Namespace))
 	} else if !util.IsDNSSubdomain(pod.Namespace) {
-		allErrs = append(allErrs, errs.NewFieldInvalid("namespace", pod.Namespace, ""))
+		allErrs = append(allErrs, errs.NewFieldInvalid("namespace", pod.Namespace, dnsSubdomainErrorMsg))
 	}
 	allErrs = append(allErrs, ValidatePodSpec(&pod.Spec).Prefix("spec")...)
 	return allErrs
@@ -726,6 +740,7 @@ func ValidateMinionUpdate(oldMinion *api.Node, minion *api.Node) errs.Validation
 	// Clear status
 	oldMinion.Status = minion.Status
 
+	// TODO: Add a 'real' ValidationError type for this error and provide print actual diffs.
 	if !api.Semantic.DeepEqual(oldMinion, minion) {
 		glog.V(4).Infof("Update failed validation %#v vs %#v", oldMinion, minion)
 		allErrs = append(allErrs, fmt.Errorf("update contains more than labels or capacity changes"))
@@ -737,14 +752,15 @@ func ValidateMinionUpdate(oldMinion *api.Node, minion *api.Node) errs.Validation
 
 // Validate compute resource typename.
 // Refer to docs/resources.md for more details.
-func validateResourceName(str string) errs.ValidationErrorList {
+func validateResourceName(value string, field string) errs.ValidationErrorList {
-	if !util.IsQualifiedName(str) {
+	allErrs := errs.ValidationErrorList{}
-		return errs.ValidationErrorList{fmt.Errorf("invalid compute resource typename format %q", str)}
+	if !util.IsQualifiedName(value) {
+		return append(allErrs, errs.NewFieldInvalid(field, value, "resource typename: "+qualifiedNameErrorMsg))
 	}
 
-	if len(strings.Split(str, "/")) == 1 {
+	if len(strings.Split(value, "/")) == 1 {
-		if !api.IsStandardResourceName(str) {
+		if !api.IsStandardResourceName(value) {
-			return errs.ValidationErrorList{fmt.Errorf("invalid compute resource typename. %q is neither a standard resource type nor is fully qualified", str)}
+			return append(allErrs, errs.NewFieldInvalid(field, value, "is neither a standard resource type nor is fully qualified"))
 		}
 	}
 
@@ -757,21 +773,21 @@ func ValidateLimitRange(limitRange *api.LimitRange) errs.ValidationErrorList {
 	if len(limitRange.Name) == 0 {
 		allErrs = append(allErrs, errs.NewFieldRequired("name", limitRange.Name))
 	} else if !util.IsDNSSubdomain(limitRange.Name) {
-		allErrs = append(allErrs, errs.NewFieldInvalid("name", limitRange.Name, ""))
+		allErrs = append(allErrs, errs.NewFieldInvalid("name", limitRange.Name, dnsSubdomainErrorMsg))
 	}
 	if len(limitRange.Namespace) == 0 {
 		allErrs = append(allErrs, errs.NewFieldRequired("namespace", limitRange.Namespace))
 	} else if !util.IsDNSSubdomain(limitRange.Namespace) {
-		allErrs = append(allErrs, errs.NewFieldInvalid("namespace", limitRange.Namespace, ""))
+		allErrs = append(allErrs, errs.NewFieldInvalid("namespace", limitRange.Namespace, dnsSubdomainErrorMsg))
 	}
 	// ensure resource names are properly qualified per docs/resources.md
 	for i := range limitRange.Spec.Limits {
 		limit := limitRange.Spec.Limits[i]
 		for k := range limit.Max {
-			allErrs = append(allErrs, validateResourceName(string(k))...)
+			allErrs = append(allErrs, validateResourceName(string(k), fmt.Sprintf("spec.limits[%d].max[%s]", i, k))...)
 		}
 		for k := range limit.Min {
-			allErrs = append(allErrs, validateResourceName(string(k))...)
+			allErrs = append(allErrs, validateResourceName(string(k), fmt.Sprintf("spec.limits[%d].min[%s]", i, k))...)
 		}
 	}
 	return allErrs
@@ -789,7 +805,7 @@ func validateResourceRequirements(container *api.Container) errs.ValidationError
 	allErrs := errs.ValidationErrorList{}
 	for resourceName, quantity := range container.Resources.Limits {
 		// Validate resource name.
-		errs := validateResourceName(resourceName.String())
+		errs := validateResourceName(resourceName.String(), fmt.Sprintf("resources.limits[%s]", resourceName))
 		if api.IsStandardResourceName(resourceName.String()) {
 			errs = append(errs, validateBasicResource(quantity).Prefix(fmt.Sprintf("Resource %s: ", resourceName))...)
 		}
@@ -805,21 +821,21 @@ func ValidateResourceQuota(resourceQuota *api.ResourceQuota) errs.ValidationErro
 	if len(resourceQuota.Name) == 0 {
 		allErrs = append(allErrs, errs.NewFieldRequired("name", resourceQuota.Name))
 	} else if !util.IsDNSSubdomain(resourceQuota.Name) {
-		allErrs = append(allErrs, errs.NewFieldInvalid("name", resourceQuota.Name, ""))
+		allErrs = append(allErrs, errs.NewFieldInvalid("name", resourceQuota.Name, dnsSubdomainErrorMsg))
 	}
 	if len(resourceQuota.Namespace) == 0 {
 		allErrs = append(allErrs, errs.NewFieldRequired("namespace", resourceQuota.Namespace))
 	} else if !util.IsDNSSubdomain(resourceQuota.Namespace) {
-		allErrs = append(allErrs, errs.NewFieldInvalid("namespace", resourceQuota.Namespace, ""))
+		allErrs = append(allErrs, errs.NewFieldInvalid("namespace", resourceQuota.Namespace, dnsSubdomainErrorMsg))
 	}
 	for k := range resourceQuota.Spec.Hard {
-		allErrs = append(allErrs, validateResourceName(string(k))...)
+		allErrs = append(allErrs, validateResourceName(string(k), string(resourceQuota.TypeMeta.Kind))...)
 	}
 	for k := range resourceQuota.Status.Hard {
-		allErrs = append(allErrs, validateResourceName(string(k))...)
+		allErrs = append(allErrs, validateResourceName(string(k), string(resourceQuota.TypeMeta.Kind))...)
 	}
 	for k := range resourceQuota.Status.Used {
-		allErrs = append(allErrs, validateResourceName(string(k))...)
+		allErrs = append(allErrs, validateResourceName(string(k), string(resourceQuota.TypeMeta.Kind))...)
 	}
 	return allErrs
 }

pkg/api/validation/validation_test.go

@@ -94,6 +94,11 @@ func TestValidateLabels(t *testing.T) {
 		errs := ValidateLabels(errorCases[i], "field")
 		if len(errs) != 1 {
 			t.Errorf("case[%d] expected failure", i)
+		} else {
+			detail := errs[0].(*errors.ValidationError).Detail
+			if detail != qualifiedNameErrorMsg {
+				t.Errorf("error detail %s should be equal %s", detail, qualifiedNameErrorMsg)
+			}
 		}
 	}
 }
@@ -131,6 +136,11 @@ func TestValidateAnnotations(t *testing.T) {
 		errs := ValidateAnnotations(errorCases[i], "field")
 		if len(errs) != 1 {
 			t.Errorf("case[%d] expected failure", i)
+		} else {
+			detail := errs[0].(*errors.ValidationError).Detail
+			if detail != qualifiedNameErrorMsg {
+				t.Errorf("error detail %s should be equal %s", detail, qualifiedNameErrorMsg)
+			}
 		}
 	}
 }
@@ -175,6 +185,10 @@ func TestValidateVolumes(t *testing.T) {
 			if errs[i].(*errors.ValidationError).Field != v.F {
 				t.Errorf("%s: expected errors to have field %s: %v", k, v.F, errs[i])
 			}
+			detail := errs[i].(*errors.ValidationError).Detail
+			if detail != "" && detail != dnsLabelErrorMsg {
+				t.Errorf("%s: expected error detail either empty or %s, got %s", k, dnsLabelErrorMsg, detail)
+			}
 		}
 	}
 }
@@ -203,18 +217,19 @@ func TestValidatePorts(t *testing.T) {
 		P []api.Port
 		T errors.ValidationErrorType
 		F string
+		D string
 	}{
-		"name > 63 characters": {[]api.Port{{Name: strings.Repeat("a", 64), ContainerPort: 80, Protocol: "TCP"}}, errors.ValidationErrorTypeInvalid, "[0].name"},
+		"name > 63 characters": {[]api.Port{{Name: strings.Repeat("a", 64), ContainerPort: 80, Protocol: "TCP"}}, errors.ValidationErrorTypeInvalid, "[0].name", dnsLabelErrorMsg},
-		"name not a DNS label": {[]api.Port{{Name: "a.b.c", ContainerPort: 80, Protocol: "TCP"}}, errors.ValidationErrorTypeInvalid, "[0].name"},
+		"name not a DNS label": {[]api.Port{{Name: "a.b.c", ContainerPort: 80, Protocol: "TCP"}}, errors.ValidationErrorTypeInvalid, "[0].name", dnsLabelErrorMsg},
 		"name not unique": {[]api.Port{
 			{Name: "abc", ContainerPort: 80, Protocol: "TCP"},
 			{Name: "abc", ContainerPort: 81, Protocol: "TCP"},
-		}, errors.ValidationErrorTypeDuplicate, "[1].name"},
+		}, errors.ValidationErrorTypeDuplicate, "[1].name", ""},
-		"zero container port":    {[]api.Port{{ContainerPort: 0, Protocol: "TCP"}}, errors.ValidationErrorTypeRequired, "[0].containerPort"},
+		"zero container port":    {[]api.Port{{ContainerPort: 0, Protocol: "TCP"}}, errors.ValidationErrorTypeInvalid, "[0].containerPort", portRangeErrorMsg},
-		"invalid container port": {[]api.Port{{ContainerPort: 65536, Protocol: "TCP"}}, errors.ValidationErrorTypeInvalid, "[0].containerPort"},
+		"invalid container port": {[]api.Port{{ContainerPort: 65536, Protocol: "TCP"}}, errors.ValidationErrorTypeInvalid, "[0].containerPort", portRangeErrorMsg},
-		"invalid host port":      {[]api.Port{{ContainerPort: 80, HostPort: 65536, Protocol: "TCP"}}, errors.ValidationErrorTypeInvalid, "[0].hostPort"},
+		"invalid host port":      {[]api.Port{{ContainerPort: 80, HostPort: 65536, Protocol: "TCP"}}, errors.ValidationErrorTypeInvalid, "[0].hostPort", portRangeErrorMsg},
-		"invalid protocol":       {[]api.Port{{ContainerPort: 80, Protocol: "ICMP"}}, errors.ValidationErrorTypeNotSupported, "[0].protocol"},
+		"invalid protocol":       {[]api.Port{{ContainerPort: 80, Protocol: "ICMP"}}, errors.ValidationErrorTypeNotSupported, "[0].protocol", ""},
-		"protocol required":      {[]api.Port{{Name: "abc", ContainerPort: 80}}, errors.ValidationErrorTypeRequired, "[0].protocol"},
+		"protocol required":      {[]api.Port{{Name: "abc", ContainerPort: 80}}, errors.ValidationErrorTypeRequired, "[0].protocol", ""},
 	}
 	for k, v := range errorCases {
 		errs := validatePorts(v.P)
@@ -228,6 +243,10 @@ func TestValidatePorts(t *testing.T) {
 			if errs[i].(*errors.ValidationError).Field != v.F {
 				t.Errorf("%s: expected errors to have field %s: %v", k, v.F, errs[i])
 			}
+			detail := errs[i].(*errors.ValidationError).Detail
+			if detail != v.D {
+				t.Errorf("%s: expected error detail either empty or %s, got %s", k, v.D, detail)
+			}
 		}
 	}
 }
@@ -250,6 +269,13 @@ func TestValidateEnv(t *testing.T) {
 	for k, v := range errorCases {
 		if errs := validateEnv(v); len(errs) == 0 {
 			t.Errorf("expected failure for %s", k)
+		} else {
+			for i := range errs {
+				detail := errs[i].(*errors.ValidationError).Detail
+				if detail != "" && detail != cIdentifierErrorMsg {
+					t.Errorf("%s: expected error detail either empty or %s, got %s", k, cIdentifierErrorMsg, detail)
+				}
+			}
 		}
 	}
 }
@@ -1076,7 +1102,7 @@ func TestValidateService(t *testing.T) {
 					Protocol: "TCP",
 				},
 			},
-			// Should fail because protocol is missing.
+			// Should fail because the session affinity is missing.
 			numErrs: 1,
 		},
 		{
@@ -1397,6 +1423,9 @@ func TestValidateService(t *testing.T) {
 	errs := ValidateService(&svc)
 	if len(errs) != 0 {
 		t.Errorf("Unexpected non-zero error list: %#v", errs)
+		for i := range errs {
+			t.Errorf("Found error: %s", errs[i].Error())
+		}
 	}
 }
 
@@ -2168,38 +2197,46 @@ func TestValidateResourceNames(t *testing.T) {
 		{"kubernetes.io", false},
 		{"kubernetes.io/will/not/work/", false},
 	}
-	for _, item := range table {
+	for k, item := range table {
-		err := validateResourceName(item.input)
+		err := validateResourceName(item.input, "sth")
 		if len(err) != 0 && item.success {
 			t.Errorf("expected no failure for input %q", item.input)
 		} else if len(err) == 0 && !item.success {
 			t.Errorf("expected failure for input %q", item.input)
+			for i := range err {
+				detail := err[i].(*errors.ValidationError).Detail
+				if detail != "" && detail != qualifiedNameErrorMsg {
+					t.Errorf("%s: expected error detail either empty or %s, got %s", k, qualifiedNameErrorMsg, detail)
+				}
+			}
 		}
 	}
 }
 
 func TestValidateLimitRange(t *testing.T) {
+	spec := api.LimitRangeSpec{
+		Limits: []api.LimitRangeItem{
+			{
+				Type: api.LimitTypePod,
+				Max: api.ResourceList{
+					api.ResourceCPU:    resource.MustParse("100"),
+					api.ResourceMemory: resource.MustParse("10000"),
+				},
+				Min: api.ResourceList{
+					api.ResourceCPU:    resource.MustParse("0"),
+					api.ResourceMemory: resource.MustParse("100"),
+				},
+			},
+		},
+	}
+
 	successCases := []api.LimitRange{
 		{
 			ObjectMeta: api.ObjectMeta{
 				Name:      "abc",
 				Namespace: "foo",
 			},
-			Spec: api.LimitRangeSpec{
+			Spec: spec,
-				Limits: []api.LimitRangeItem{
-					{
-						Type: api.LimitTypePod,
-						Max: api.ResourceList{
-							api.ResourceCPU:    resource.MustParse("100"),
-							api.ResourceMemory: resource.MustParse("10000"),
-						},
-						Min: api.ResourceList{
-							api.ResourceCPU:    resource.MustParse("0"),
-							api.ResourceMemory: resource.MustParse("100"),
-						},
-					},
-				},
-			},
 		},
 	}
 
@@ -2209,82 +2246,65 @@ func TestValidateLimitRange(t *testing.T) {
 		}
 	}
 
-	errorCases := map[string]api.LimitRange{
+	errorCases := map[string]struct {
+		R api.LimitRange
+		D string
+	}{
 		"zero-length Name": {
-			ObjectMeta: api.ObjectMeta{
+			api.LimitRange{ObjectMeta: api.ObjectMeta{Name: "", Namespace: "foo"}, Spec: spec},
-				Name:      "",
+			"",
-				Namespace: "foo",
-			},
-			Spec: api.LimitRangeSpec{
-				Limits: []api.LimitRangeItem{
-					{
-						Type: api.LimitTypePod,
-						Max: api.ResourceList{
-							api.ResourceCPU:    resource.MustParse("100"),
-							api.ResourceMemory: resource.MustParse("10000"),
-						},
-						Min: api.ResourceList{
-							api.ResourceCPU:    resource.MustParse("0"),
-							api.ResourceMemory: resource.MustParse("100"),
-						},
-					},
-				},
-			},
 		},
 		"zero-length-namespace": {
-			ObjectMeta: api.ObjectMeta{
+			api.LimitRange{ObjectMeta: api.ObjectMeta{Name: "abc", Namespace: ""}, Spec: spec},
-				Name:      "abc",
+			"",
-				Namespace: "",
+		},
-			},
+		"invalid Name": {
-			Spec: api.LimitRangeSpec{
+			api.LimitRange{ObjectMeta: api.ObjectMeta{Name: "^Invalid", Namespace: "foo"}, Spec: spec},
-				Limits: []api.LimitRangeItem{
+			dnsSubdomainErrorMsg,
-					{
+		},
-						Type: api.LimitTypePod,
+		"invalid Namespace": {
-						Max: api.ResourceList{
+			api.LimitRange{ObjectMeta: api.ObjectMeta{Name: "abc", Namespace: "^Invalid"}, Spec: spec},
-							api.ResourceCPU:    resource.MustParse("100"),
+			dnsSubdomainErrorMsg,
-							api.ResourceMemory: resource.MustParse("10000"),
-						},
-						Min: api.ResourceList{
-							api.ResourceCPU:    resource.MustParse("0"),
-							api.ResourceMemory: resource.MustParse("100"),
-						},
-					},
-				},
-			},
 		},
 	}
 	for k, v := range errorCases {
-		errs := ValidateLimitRange(&v)
+		errs := ValidateLimitRange(&v.R)
 		if len(errs) == 0 {
 			t.Errorf("expected failure for %s", k)
 		}
 		for i := range errs {
 			field := errs[i].(*errors.ValidationError).Field
+			detail := errs[i].(*errors.ValidationError).Detail
 			if field != "name" &&
 				field != "namespace" {
 				t.Errorf("%s: missing prefix for: %v", k, errs[i])
 			}
+			if detail != v.D {
+				t.Errorf("%s: expected error detail either empty or %s, got %s", k, v.D, detail)
+			}
 		}
 	}
 }
 
 func TestValidateResourceQuota(t *testing.T) {
+	spec := api.ResourceQuotaSpec{
+		Hard: api.ResourceList{
+			api.ResourceCPU:                    resource.MustParse("100"),
+			api.ResourceMemory:                 resource.MustParse("10000"),
+			api.ResourcePods:                   resource.MustParse("10"),
+			api.ResourceServices:               resource.MustParse("10"),
+			api.ResourceReplicationControllers: resource.MustParse("10"),
+			api.ResourceQuotas:                 resource.MustParse("10"),
+		},
+	}
+
 	successCases := []api.ResourceQuota{
 		{
 			ObjectMeta: api.ObjectMeta{
 				Name:      "abc",
 				Namespace: "foo",
 			},
-			Spec: api.ResourceQuotaSpec{
+			Spec: spec,
-				Hard: api.ResourceList{
-					api.ResourceCPU:                    resource.MustParse("100"),
-					api.ResourceMemory:                 resource.MustParse("10000"),
-					api.ResourcePods:                   resource.MustParse("10"),
-					api.ResourceServices:               resource.MustParse("10"),
-					api.ResourceReplicationControllers: resource.MustParse("10"),
-					api.ResourceQuotas:                 resource.MustParse("10"),
-				},
-			},
 		},
 	}
 
@@ -2294,51 +2314,42 @@ func TestValidateResourceQuota(t *testing.T) {
 		}
 	}
 
-	errorCases := map[string]api.ResourceQuota{
+	errorCases := map[string]struct {
+		R api.ResourceQuota
+		D string
+	}{
 		"zero-length Name": {
-			ObjectMeta: api.ObjectMeta{
+			api.ResourceQuota{ObjectMeta: api.ObjectMeta{Name: "", Namespace: "foo"}, Spec: spec},
-				Name:      "",
+			"",
-				Namespace: "foo",
-			},
-			Spec: api.ResourceQuotaSpec{
-				Hard: api.ResourceList{
-					api.ResourceCPU:                    resource.MustParse("100"),
-					api.ResourceMemory:                 resource.MustParse("10000"),
-					api.ResourcePods:                   resource.MustParse("10"),
-					api.ResourceServices:               resource.MustParse("10"),
-					api.ResourceReplicationControllers: resource.MustParse("10"),
-					api.ResourceQuotas:                 resource.MustParse("10"),
-				},
-			},
 		},
-		"zero-length-namespace": {
+		"zero-length Namespace": {
-			ObjectMeta: api.ObjectMeta{
+			api.ResourceQuota{ObjectMeta: api.ObjectMeta{Name: "abc", Namespace: ""}, Spec: spec},
-				Name:      "abc",
+			"",
-				Namespace: "",
+		},
-			},
+		"invalid Name": {
-			Spec: api.ResourceQuotaSpec{
+			api.ResourceQuota{ObjectMeta: api.ObjectMeta{Name: "^Invalid", Namespace: "foo"}, Spec: spec},
-				Hard: api.ResourceList{
+			dnsSubdomainErrorMsg,
-					api.ResourceCPU:                    resource.MustParse("100"),
+		},
-					api.ResourceMemory:                 resource.MustParse("10000"),
+		"invalid Namespace": {
-					api.ResourcePods:                   resource.MustParse("10"),
+			api.ResourceQuota{ObjectMeta: api.ObjectMeta{Name: "abc", Namespace: "^Invalid"}, Spec: spec},
-					api.ResourceServices:               resource.MustParse("10"),
+			dnsSubdomainErrorMsg,
-					api.ResourceReplicationControllers: resource.MustParse("10"),
-					api.ResourceQuotas:                 resource.MustParse("10"),
-				},
-			},
 		},
 	}
 	for k, v := range errorCases {
-		errs := ValidateResourceQuota(&v)
+		errs := ValidateResourceQuota(&v.R)
 		if len(errs) == 0 {
 			t.Errorf("expected failure for %s", k)
 		}
 		for i := range errs {
 			field := errs[i].(*errors.ValidationError).Field
+			detail := errs[i].(*errors.ValidationError).Detail
 			if field != "name" &&
 				field != "namespace" {
 				t.Errorf("%s: missing prefix for: %v", k, errs[i])
 			}
+			if detail != v.D {
+				t.Errorf("%s: expected error detail either empty or %s, got %s", k, v.D, detail)
+			}
 		}
 	}
 }

pkg/util/validation.go

@@ -33,45 +33,45 @@ func IsDNSSubdomain(value string) bool {
 	return IsDNS1123Subdomain(value)
 }
 
-const dns1123LabelFmt string = "[a-z0-9]([-a-z0-9]*[a-z0-9])?"
+const DNS1123LabelFmt string = "[a-z0-9]([-a-z0-9]*[a-z0-9])?"
 
-var dns1123LabelRegexp = regexp.MustCompile("^" + dns1123LabelFmt + "$")
+var dns1123LabelRegexp = regexp.MustCompile("^" + DNS1123LabelFmt + "$")
 
-const dns1123LabelMaxLength int = 63
+const DNS1123LabelMaxLength int = 63
 
 // IsDNS1123Label tests for a string that conforms to the definition of a label in
 // DNS (RFC 1123).
 func IsDNS1123Label(value string) bool {
-	return len(value) <= dns1123LabelMaxLength && dns1123LabelRegexp.MatchString(value)
+	return len(value) <= DNS1123LabelMaxLength && dns1123LabelRegexp.MatchString(value)
 }
 
-const dns1123SubdomainFmt string = dns1123LabelFmt + "(\\." + dns1123LabelFmt + ")*"
+const DNS1123SubdomainFmt string = DNS1123LabelFmt + "(\\." + DNS1123LabelFmt + ")*"
 
-var dns1123SubdomainRegexp = regexp.MustCompile("^" + dns1123SubdomainFmt + "$")
+var dns1123SubdomainRegexp = regexp.MustCompile("^" + DNS1123SubdomainFmt + "$")
 
-const dns1123SubdomainMaxLength int = 253
+const DNS1123SubdomainMaxLength int = 253
 
 // IsDNS1123Subdomain tests for a string that conforms to the definition of a
 // subdomain in DNS (RFC 1123).
 func IsDNS1123Subdomain(value string) bool {
-	return len(value) <= dns1123SubdomainMaxLength && dns1123SubdomainRegexp.MatchString(value)
+	return len(value) <= DNS1123SubdomainMaxLength && dns1123SubdomainRegexp.MatchString(value)
 }
 
-const dns952LabelFmt string = "[a-z]([-a-z0-9]*[a-z0-9])?"
+const DNS952LabelFmt string = "[a-z]([-a-z0-9]*[a-z0-9])?"
 
-var dns952LabelRegexp = regexp.MustCompile("^" + dns952LabelFmt + "$")
+var dns952LabelRegexp = regexp.MustCompile("^" + DNS952LabelFmt + "$")
 
-const dns952LabelMaxLength int = 24
+const DNS952LabelMaxLength int = 24
 
 // IsDNS952Label tests for a string that conforms to the definition of a label in
 // DNS (RFC 952).
 func IsDNS952Label(value string) bool {
-	return len(value) <= dns952LabelMaxLength && dns952LabelRegexp.MatchString(value)
+	return len(value) <= DNS952LabelMaxLength && dns952LabelRegexp.MatchString(value)
 }
 
-const cIdentifierFmt string = "[A-Za-z_][A-Za-z0-9_]*"
+const CIdentifierFmt string = "[A-Za-z_][A-Za-z0-9_]*"
 
-var cIdentifierRegexp = regexp.MustCompile("^" + cIdentifierFmt + "$")
+var cIdentifierRegexp = regexp.MustCompile("^" + CIdentifierFmt + "$")
 
 // IsCIdentifier tests for a string that conforms the definition of an identifier
 // in C. This checks the format, but not the length.