diff --git a/contrib/mesos/pkg/node/node.go b/contrib/mesos/pkg/node/node.go index 93f7d6ed7db..0b726fc804b 100644 --- a/contrib/mesos/pkg/node/node.go +++ b/contrib/mesos/pkg/node/node.go @@ -181,12 +181,12 @@ func SlaveAttributesToLabels(attrs []*mesos.Attribute) map[string]string { } if errs := validation.IsQualifiedName(k); len(errs) != 0 { - log.V(3).Infof("ignoring invalid node label name %q", k, errs) + log.V(3).Infof("ignoring invalid node label %q: %v", k, errs) continue } - if !validation.IsValidLabelValue(v) { - log.V(3).Infof("ignoring invalid node label %s value: %q", k, v) + if errs := validation.IsValidLabelValue(v); len(errs) != 0 { + log.V(3).Infof("ignoring invalid node %s=%q: %v", k, v, errs) continue } diff --git a/pkg/api/unversioned/validation/validation.go b/pkg/api/unversioned/validation/validation.go index 352c4fa1964..3fdd4c2453f 100644 --- a/pkg/api/unversioned/validation/validation.go +++ b/pkg/api/unversioned/validation/validation.go @@ -17,17 +17,11 @@ limitations under the License. package validation import ( - "fmt" - "k8s.io/kubernetes/pkg/api/unversioned" "k8s.io/kubernetes/pkg/util/validation" "k8s.io/kubernetes/pkg/util/validation/field" ) -var ( - labelValueErrorMsg string = fmt.Sprintf(`must have at most %d characters, matching regex %s: e.g. "MyValue" or ""`, validation.LabelValueMaxLength, validation.LabelValueFmt) -) - func ValidateLabelSelector(ps *unversioned.LabelSelector, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} if ps == nil { @@ -72,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)...) - if !validation.IsValidLabelValue(v) { - allErrs = append(allErrs, field.Invalid(fldPath, v, labelValueErrorMsg)) + for _, err := range validation.IsValidLabelValue(v) { + allErrs = append(allErrs, field.Invalid(fldPath, v, err)) } } return allErrs diff --git a/pkg/api/unversioned/validation/validation_test.go b/pkg/api/unversioned/validation/validation_test.go index d05144ef303..ec1264f4eae 100644 --- a/pkg/api/unversioned/validation/validation_test.go +++ b/pkg/api/unversioned/validation/validation_test.go @@ -67,20 +67,22 @@ func TestValidateLabels(t *testing.T) { } } - labelValueErrorCases := []map[string]string{ - {"toolongvalue": strings.Repeat("a", 64)}, - {"backslashesinvalue": "some\\bad\\value"}, - {"nocommasallowed": "bad,value"}, - {"strangecharsinvalue": "?#$notsogood"}, + labelValueErrorCases := []struct { + labels map[string]string + expect string + }{ + {map[string]string{"toolongvalue": strings.Repeat("a", 64)}, "must be no more than"}, + {map[string]string{"backslashesinvalue": "some\\bad\\value"}, "must match the regex"}, + {map[string]string{"nocommasallowed": "bad,value"}, "must match the regex"}, + {map[string]string{"strangecharsinvalue": "?#$notsogood"}, "must match the regex"}, } for i := range labelValueErrorCases { - errs := ValidateLabels(labelValueErrorCases[i], field.NewPath("field")) + errs := ValidateLabels(labelValueErrorCases[i].labels, field.NewPath("field")) if len(errs) != 1 { - t.Errorf("case[%d] expected failure", i) + t.Errorf("case[%d]: expected failure", i) } else { - detail := errs[0].Detail - if detail != labelValueErrorMsg { - t.Errorf("error detail %s should be equal %s", detail, labelValueErrorMsg) + if !strings.Contains(errs[0].Detail, labelValueErrorCases[i].expect) { + t.Errorf("case[%d]: error details do not include %q: %q", i, labelValueErrorCases[i].expect, errs[0].Detail) } } } diff --git a/pkg/api/validation/validation.go b/pkg/api/validation/validation.go index ce5ba70d90f..291f8d3e1ca 100644 --- a/pkg/api/validation/validation.go +++ b/pkg/api/validation/validation.go @@ -58,7 +58,6 @@ func InclusiveRangeErrorMsg(lo, hi int) string { return fmt.Sprintf(`must be between %d and %d, inclusive`, lo, hi) } -var labelValueErrorMsg string = fmt.Sprintf(`must have at most %d characters, matching regex %s: e.g. "MyValue" or ""`, validation.LabelValueMaxLength, validation.LabelValueFmt) var DNSSubdomainErrorMsg string = fmt.Sprintf(`must be a DNS subdomain (at most %d characters, matching regex %s): e.g. "example.com"`, validation.DNS1123SubdomainMaxLength, validation.DNS1123SubdomainFmt) var DNS1123LabelErrorMsg string = fmt.Sprintf(`must be a DNS label (at most %d characters, matching regex %s): e.g. "my-name"`, validation.DNS1123LabelMaxLength, validation.DNS1123LabelFmt) var DNS952LabelErrorMsg string = fmt.Sprintf(`must be a DNS 952 label (at most %d characters, matching regex %s): e.g. "my-name"`, validation.DNS952LabelMaxLength, validation.DNS952LabelFmt) diff --git a/pkg/kubectl/cmd/label.go b/pkg/kubectl/cmd/label.go index b39f7e4cfcc..97f18268112 100644 --- a/pkg/kubectl/cmd/label.go +++ b/pkg/kubectl/cmd/label.go @@ -124,9 +124,12 @@ func parseLabels(spec []string) (map[string]string, []string, error) { for _, labelSpec := range spec { if strings.Index(labelSpec, "=") != -1 { parts := strings.Split(labelSpec, "=") - if len(parts) != 2 || len(parts[1]) == 0 || !validation.IsValidLabelValue(parts[1]) { + if len(parts) != 2 || len(parts[1]) == 0 { return nil, nil, fmt.Errorf("invalid label spec: %v", labelSpec) } + if errs := validation.IsValidLabelValue(parts[1]); len(errs) != 0 { + return nil, nil, fmt.Errorf("invalid label value: %q: %s", labelSpec, strings.Join(errs, ";")) + } labels[parts[0]] = parts[1] } else if strings.HasSuffix(labelSpec, "-") { remove = append(remove, labelSpec[:len(labelSpec)-1]) diff --git a/pkg/kubelet/network/plugins.go b/pkg/kubelet/network/plugins.go index 5eebadc1115..1d52415b22c 100644 --- a/pkg/kubelet/network/plugins.go +++ b/pkg/kubelet/network/plugins.go @@ -120,8 +120,8 @@ func InitNetworkPlugin(plugins []NetworkPlugin, networkPluginName string, host H allErrs := []error{} for _, plugin := range plugins { name := plugin.Name() - if len(validation.IsQualifiedName(name)) != 0 { - allErrs = append(allErrs, fmt.Errorf("network plugin has invalid name: %#v", plugin)) + if errs := validation.IsQualifiedName(name); len(errs) != 0 { + allErrs = append(allErrs, fmt.Errorf("network plugin has invalid name: %q: %s", name, strings.Join(errs, ";"))) continue } diff --git a/pkg/labels/selector.go b/pkg/labels/selector.go index b47dc3746b1..ab64ecc8090 100644 --- a/pkg/labels/selector.go +++ b/pkg/labels/selector.go @@ -764,8 +764,6 @@ func parse(selector string) (internalSelector, error) { return internalSelector(items), err } -var labelValueErrorMsg string = fmt.Sprintf(`must have at most %d characters, matching regex %s: e.g. "MyValue" or ""`, validation.LabelValueMaxLength, validation.LabelValueFmt) - func validateLabelKey(k string) error { if errs := validation.IsQualifiedName(k); len(errs) != 0 { return fmt.Errorf("invalid label key %q: %s", k, strings.Join(errs, "; ")) @@ -774,8 +772,8 @@ func validateLabelKey(k string) error { } func validateLabelValue(v string) error { - if !validation.IsValidLabelValue(v) { - return fmt.Errorf("invalid label value: %s", labelValueErrorMsg) + if errs := validation.IsValidLabelValue(v); len(errs) != 0 { + return fmt.Errorf("invalid label value: %q: %s", v, strings.Join(errs, "; ")) } return nil } diff --git a/pkg/util/validation/validation.go b/pkg/util/validation/validation.go index c8789c8838a..bc92290fb29 100644 --- a/pkg/util/validation/validation.go +++ b/pkg/util/validation/validation.go @@ -67,13 +67,24 @@ func IsQualifiedName(value string) []string { return errs } -const LabelValueFmt string = "(" + qualifiedNameFmt + ")?" +const labelValueFmt string = "(" + qualifiedNameFmt + ")?" const LabelValueMaxLength int = 63 -var labelValueRegexp = regexp.MustCompile("^" + LabelValueFmt + "$") +var labelValueMaxLengthString = strconv.Itoa(LabelValueMaxLength) +var labelValueRegexp = regexp.MustCompile("^" + labelValueFmt + "$") -func IsValidLabelValue(value string) bool { - return (len(value) <= LabelValueMaxLength && labelValueRegexp.MatchString(value)) +// IsValidLabelValue tests whether the value passed is a valid label value. If +// the value is not valid, a list of error strings is returned. Otherwise an +// empty list (or nil) is returned. +func IsValidLabelValue(value string) []string { + var errs []string + if len(value) > LabelValueMaxLength { + errs = append(errs, "must be no more than "+labelValueMaxLengthString+" characters") + } + if !labelValueRegexp.MatchString(value) { + errs = append(errs, "must match the regex "+labelValueFmt+" (e.g. 'MyValue' or 'my_value' or '12345')") + } + return errs } const DNS1123LabelFmt string = "[a-z0-9]([-a-z0-9]*[a-z0-9])?" diff --git a/pkg/util/validation/validation_test.go b/pkg/util/validation/validation_test.go index 884a6c4945e..d0b73e3b65b 100644 --- a/pkg/util/validation/validation_test.go +++ b/pkg/util/validation/validation_test.go @@ -257,8 +257,8 @@ func TestIsValidLabelValue(t *testing.T) { "", // empty value } for i := range successCases { - if !IsValidLabelValue(successCases[i]) { - t.Errorf("case %s expected success", successCases[i]) + if errs := IsValidLabelValue(successCases[i]); len(errs) != 0 { + t.Errorf("case %s expected success: %v", successCases[i], errs) } } @@ -273,7 +273,7 @@ func TestIsValidLabelValue(t *testing.T) { strings.Repeat("a", 64), // over the limit } for i := range errorCases { - if IsValidLabelValue(errorCases[i]) { + if errs := IsValidLabelValue(errorCases[i]); len(errs) == 0 { t.Errorf("case[%d] expected failure", i) } } diff --git a/pkg/volume/plugins.go b/pkg/volume/plugins.go index 22bf1f9e159..c9195f6c28b 100644 --- a/pkg/volume/plugins.go +++ b/pkg/volume/plugins.go @@ -272,8 +272,8 @@ func (pm *VolumePluginMgr) InitPlugins(plugins []VolumePlugin, host VolumeHost) allErrs := []error{} for _, plugin := range plugins { name := plugin.Name() - if len(validation.IsQualifiedName(name)) != 0 { - allErrs = append(allErrs, fmt.Errorf("volume plugin has invalid name: %#v", plugin)) + if errs := validation.IsQualifiedName(name); len(errs) != 0 { + allErrs = append(allErrs, fmt.Errorf("volume plugin has invalid name: %q: %s", name, strings.Join(errs, ";"))) continue } diff --git a/plugin/pkg/scheduler/factory/factory.go b/plugin/pkg/scheduler/factory/factory.go index 0500a273273..50f8a59cfbf 100644 --- a/plugin/pkg/scheduler/factory/factory.go +++ b/plugin/pkg/scheduler/factory/factory.go @@ -307,8 +307,8 @@ func (f *ConfigFactory) CreateFromKeys(predicateKeys, priorityKeys sets.String, failureDomainArgs := strings.Split(f.FailureDomains, ",") for _, failureDomain := range failureDomainArgs { - if len(utilvalidation.IsQualifiedName(failureDomain)) != 0 { - return nil, fmt.Errorf("invalid failure domain: %s", failureDomain) + if errs := utilvalidation.IsQualifiedName(failureDomain); len(errs) != 0 { + return nil, fmt.Errorf("invalid failure domain: %q: %s", failureDomain, strings.Join(errs, ";")) } }