diff --git a/contrib/mesos/pkg/node/node.go b/contrib/mesos/pkg/node/node.go index 92b5114afc7..93f7d6ed7db 100644 --- a/contrib/mesos/pkg/node/node.go +++ b/contrib/mesos/pkg/node/node.go @@ -180,8 +180,8 @@ func SlaveAttributesToLabels(attrs []*mesos.Attribute) map[string]string { v = strconv.FormatFloat(a.GetScalar().GetValue(), 'G', -1, 64) } - if !validation.IsQualifiedName(k) { - log.V(3).Infof("ignoring invalid node label name %q", k) + if errs := validation.IsQualifiedName(k); len(errs) != 0 { + log.V(3).Infof("ignoring invalid node label name %q", k, errs) continue } diff --git a/pkg/api/unversioned/validation/validation.go b/pkg/api/unversioned/validation/validation.go index e0b4f17560d..352c4fa1964 100644 --- a/pkg/api/unversioned/validation/validation.go +++ b/pkg/api/unversioned/validation/validation.go @@ -25,8 +25,7 @@ import ( ) var ( - labelValueErrorMsg string = fmt.Sprintf(`must have at most %d characters, matching regex %s: e.g. "MyValue" or ""`, validation.LabelValueMaxLength, validation.LabelValueFmt) - qualifiedNameErrorMsg string = fmt.Sprintf(`must be a qualified name (at most %d characters, matching regex %s), with an optional DNS subdomain prefix (at most %d characters, matching regex %s) and slash (/): e.g. "MyName" or "example.com/MyName"`, validation.QualifiedNameMaxLength, validation.QualifiedNameFmt, validation.DNS1123SubdomainMaxLength, validation.DNS1123SubdomainFmt) + 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 { @@ -62,8 +61,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{} - if !validation.IsQualifiedName(labelName) { - allErrs = append(allErrs, field.Invalid(fldPath, labelName, qualifiedNameErrorMsg)) + for _, err := range validation.IsQualifiedName(labelName) { + allErrs = append(allErrs, field.Invalid(fldPath, labelName, err)) } return allErrs } diff --git a/pkg/api/unversioned/validation/validation_test.go b/pkg/api/unversioned/validation/validation_test.go index 26be6fb5802..d05144ef303 100644 --- a/pkg/api/unversioned/validation/validation_test.go +++ b/pkg/api/unversioned/validation/validation_test.go @@ -47,20 +47,22 @@ func TestValidateLabels(t *testing.T) { } } - labelNameErrorCases := []map[string]string{ - {"nospecialchars^=@": "bar"}, - {"cantendwithadash-": "bar"}, - {"only/one/slash": "bar"}, - {strings.Repeat("a", 254): "bar"}, + labelNameErrorCases := []struct { + labels map[string]string + expect string + }{ + {map[string]string{"nospecialchars^=@": "bar"}, "must match the regex"}, + {map[string]string{"cantendwithadash-": "bar"}, "must match the regex"}, + {map[string]string{"only/one/slash": "bar"}, "must match the regex"}, + {map[string]string{strings.Repeat("a", 254): "bar"}, "must be no more than"}, } for i := range labelNameErrorCases { - errs := ValidateLabels(labelNameErrorCases[i], field.NewPath("field")) + errs := ValidateLabels(labelNameErrorCases[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 != qualifiedNameErrorMsg { - t.Errorf("error detail %s should be equal %s", detail, qualifiedNameErrorMsg) + if !strings.Contains(errs[0].Detail, labelNameErrorCases[i].expect) { + t.Errorf("case[%d]: error details do not include %q: %q", i, labelNameErrorCases[i].expect, errs[0].Detail) } } } diff --git a/pkg/api/validation/validation.go b/pkg/api/validation/validation.go index 5acba71cb78..ad55b55c128 100644 --- a/pkg/api/validation/validation.go +++ b/pkg/api/validation/validation.go @@ -59,7 +59,6 @@ func InclusiveRangeErrorMsg(lo, hi int) string { } var labelValueErrorMsg string = fmt.Sprintf(`must have at most %d characters, matching regex %s: e.g. "MyValue" or ""`, validation.LabelValueMaxLength, validation.LabelValueFmt) -var qualifiedNameErrorMsg string = fmt.Sprintf(`must be a qualified name (at most %d characters, matching regex %s), with an optional DNS subdomain prefix (at most %d characters, matching regex %s) and slash (/): e.g. "MyName" or "example.com/MyName"`, validation.QualifiedNameMaxLength, validation.QualifiedNameFmt, validation.DNS1123SubdomainMaxLength, validation.DNS1123SubdomainFmt) 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) @@ -94,8 +93,8 @@ func ValidateAnnotations(annotations map[string]string, fldPath *field.Path) fie allErrs := field.ErrorList{} var totalSize int64 for k, v := range annotations { - if !validation.IsQualifiedName(strings.ToLower(k)) { - allErrs = append(allErrs, field.Invalid(fldPath, k, qualifiedNameErrorMsg)) + for _, err := range validation.IsQualifiedName(strings.ToLower(k)) { + allErrs = append(allErrs, field.Invalid(fldPath, k, err)) } totalSize += (int64)(len(k)) + (int64)(len(v)) } @@ -2148,8 +2147,11 @@ 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{} - if !validation.IsQualifiedName(value) { - return append(allErrs, field.Invalid(fldPath, value, qualifiedNameErrorMsg)) + for _, err := range validation.IsQualifiedName(value) { + allErrs = append(allErrs, field.Invalid(fldPath, value, err)) + } + if len(allErrs) != 0 { + return allErrs } if len(strings.Split(value, "/")) == 1 { @@ -2188,8 +2190,11 @@ func validateResourceQuotaResourceName(value string, fldPath *field.Path) field. // Validate limit range types func validateLimitRangeTypeName(value string, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} - if !validation.IsQualifiedName(value) { - return append(allErrs, field.Invalid(fldPath, value, qualifiedNameErrorMsg)) + for _, err := range validation.IsQualifiedName(value) { + allErrs = append(allErrs, field.Invalid(fldPath, value, err)) + } + if len(allErrs) != 0 { + return allErrs } if len(strings.Split(value, "/")) == 1 { @@ -2659,8 +2664,11 @@ func ValidateNamespace(namespace *api.Namespace) field.ErrorList { // Validate finalizer names func validateFinalizerName(stringValue string, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} - if !validation.IsQualifiedName(stringValue) { - return append(allErrs, field.Invalid(fldPath, stringValue, qualifiedNameErrorMsg)) + for _, err := range validation.IsQualifiedName(stringValue) { + allErrs = append(allErrs, field.Invalid(fldPath, stringValue, err)) + } + if len(allErrs) != 0 { + return allErrs } if len(strings.Split(stringValue, "/")) == 1 { diff --git a/pkg/api/validation/validation_test.go b/pkg/api/validation/validation_test.go index 6f7299f0d7c..cd45d79c00a 100644 --- a/pkg/api/validation/validation_test.go +++ b/pkg/api/validation/validation_test.go @@ -310,20 +310,23 @@ func TestValidateAnnotations(t *testing.T) { } } - nameErrorCases := []map[string]string{ - {"nospecialchars^=@": "bar"}, - {"cantendwithadash-": "bar"}, - {"only/one/slash": "bar"}, - {strings.Repeat("a", 254): "bar"}, + nameErrorCases := []struct { + annotations map[string]string + expect string + }{ + {map[string]string{"nospecialchars^=@": "bar"}, "must match the regex"}, + {map[string]string{"cantendwithadash-": "bar"}, "must match the regex"}, + {map[string]string{"only/one/slash": "bar"}, "must match the regex"}, + {map[string]string{strings.Repeat("a", 254): "bar"}, "must be no more than"}, } for i := range nameErrorCases { - errs := ValidateAnnotations(nameErrorCases[i], field.NewPath("field")) + errs := ValidateAnnotations(nameErrorCases[i].annotations, field.NewPath("field")) if len(errs) != 1 { - t.Errorf("case[%d] expected failure", i) - } - detail := errs[0].Detail - if detail != qualifiedNameErrorMsg { - t.Errorf("error detail %s should be equal %s", detail, qualifiedNameErrorMsg) + t.Errorf("case[%d]: expected failure", i) + } else { + if !strings.Contains(errs[0].Detail, nameErrorCases[i].expect) { + t.Errorf("case[%d]: error details do not include %q: %q", i, nameErrorCases[i].expect, errs[0].Detail) + } } } totalSizeErrorCases := []map[string]string{ @@ -4277,23 +4280,24 @@ func TestValidateResourceNames(t *testing.T) { table := []struct { input string success bool + expect string }{ - {"memory", true}, - {"cpu", true}, - {"network", false}, - {"disk", false}, - {"", false}, - {".", false}, - {"..", false}, - {"my.favorite.app.co/12345", true}, - {"my.favorite.app.co/_12345", false}, - {"my.favorite.app.co/12345_", false}, - {"kubernetes.io/..", false}, - {"kubernetes.io/" + strings.Repeat("a", 63), true}, - {"kubernetes.io/" + strings.Repeat("a", 64), false}, - {"kubernetes.io//", false}, - {"kubernetes.io", false}, - {"kubernetes.io/will/not/work/", false}, + {"memory", true, ""}, + {"cpu", true, ""}, + {"network", false, ""}, + {"disk", false, ""}, + {"", false, ""}, + {".", false, ""}, + {"..", false, ""}, + {"my.favorite.app.co/12345", true, ""}, + {"my.favorite.app.co/_12345", false, ""}, + {"my.favorite.app.co/12345_", false, ""}, + {"kubernetes.io/..", false, ""}, + {"kubernetes.io/" + strings.Repeat("a", 63), true, ""}, + {"kubernetes.io/" + strings.Repeat("a", 64), false, ""}, + {"kubernetes.io//", false, ""}, + {"kubernetes.io", false, ""}, + {"kubernetes.io/will/not/work/", false, ""}, } for k, item := range table { err := validateResourceName(item.input, field.NewPath("field")) @@ -4303,8 +4307,8 @@ func TestValidateResourceNames(t *testing.T) { t.Errorf("expected failure for input %q", item.input) for i := range err { detail := err[i].Detail - if detail != "" && detail != qualifiedNameErrorMsg { - t.Errorf("%d: expected error detail either empty or %s, got %s", k, qualifiedNameErrorMsg, detail) + if detail != "" && !strings.Contains(detail, item.expect) { + t.Errorf("%d: expected error detail either empty or %s, got %s", k, item.expect, detail) } } } diff --git a/pkg/kubelet/network/plugins.go b/pkg/kubelet/network/plugins.go index 36da95fcedf..5eebadc1115 100644 --- a/pkg/kubelet/network/plugins.go +++ b/pkg/kubelet/network/plugins.go @@ -120,7 +120,7 @@ func InitNetworkPlugin(plugins []NetworkPlugin, networkPluginName string, host H allErrs := []error{} for _, plugin := range plugins { name := plugin.Name() - if !validation.IsQualifiedName(name) { + if len(validation.IsQualifiedName(name)) != 0 { allErrs = append(allErrs, fmt.Errorf("network plugin has invalid name: %#v", plugin)) continue } diff --git a/pkg/labels/selector.go b/pkg/labels/selector.go index fb48b10e9e0..b47dc3746b1 100644 --- a/pkg/labels/selector.go +++ b/pkg/labels/selector.go @@ -764,12 +764,11 @@ func parse(selector string) (internalSelector, error) { return internalSelector(items), err } -var qualifiedNameErrorMsg string = fmt.Sprintf(`must be a qualified name (at most %d characters, matching regex %s), with an optional DNS subdomain prefix (at most %d characters, matching regex %s) and slash (/): e.g. "MyName" or "example.com/MyName"`, validation.QualifiedNameMaxLength, validation.QualifiedNameFmt, validation.DNS1123SubdomainMaxLength, validation.DNS1123SubdomainFmt) 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 !validation.IsQualifiedName(k) { - return fmt.Errorf("invalid label key: %s", qualifiedNameErrorMsg) + if errs := validation.IsQualifiedName(k); len(errs) != 0 { + return fmt.Errorf("invalid label key %q: %s", k, strings.Join(errs, "; ")) } return nil } diff --git a/pkg/util/validation/validation.go b/pkg/util/validation/validation.go index aada3246559..c8789c8838a 100644 --- a/pkg/util/validation/validation.go +++ b/pkg/util/validation/validation.go @@ -17,20 +17,28 @@ limitations under the License. package validation import ( + "fmt" "math" "net" "regexp" + "strconv" "strings" ) const qnameCharFmt string = "[A-Za-z0-9]" const qnameExtCharFmt string = "[-A-Za-z0-9_.]" -const QualifiedNameFmt string = "(" + qnameCharFmt + qnameExtCharFmt + "*)?" + qnameCharFmt -const QualifiedNameMaxLength int = 63 +const qualifiedNameFmt string = "(" + qnameCharFmt + qnameExtCharFmt + "*)?" + qnameCharFmt +const qualifiedNameMaxLength int = 63 -var qualifiedNameRegexp = regexp.MustCompile("^" + QualifiedNameFmt + "$") +var qualifiedNameMaxLengthString = strconv.Itoa(qualifiedNameMaxLength) +var qualifiedNameRegexp = regexp.MustCompile("^" + qualifiedNameFmt + "$") -func IsQualifiedName(value string) bool { +// IsQualifiedName tests whether the value passed is what Kubernetes calls a +// "qualified name". This is a format used in various places throughout the +// system. If the value is not valid, a list of error strings is returned. +// Otherwise an empty list (or nil) is returned. +func IsQualifiedName(value string) []string { + var errs []string parts := strings.Split(value, "/") var name string switch len(parts) { @@ -39,17 +47,27 @@ func IsQualifiedName(value string) bool { case 2: var prefix string prefix, name = parts[0], parts[1] - if prefix == "" || !IsDNS1123Subdomain(prefix) { - return false + if len(prefix) == 0 { + errs = append(errs, "prefix part must be non-empty") + } else if !IsDNS1123Subdomain(prefix) { + errs = append(errs, fmt.Sprintf("prefix part must be a DNS subdomain (e.g. 'example.com')")) } default: - return false + return append(errs, "must match the regex "+qualifiedNameFmt+" with an optional DNS subdomain prefix and '/' (e.g. 'MyName' or 'example.com/MyName'") } - return name != "" && len(name) <= QualifiedNameMaxLength && qualifiedNameRegexp.MatchString(name) + if len(name) == 0 { + errs = append(errs, "name part must be non-empty") + } else if len(name) > qualifiedNameMaxLength { + errs = append(errs, "name part must be no more than "+qualifiedNameMaxLengthString+" characters") + } + if !qualifiedNameRegexp.MatchString(name) { + errs = append(errs, "name part must match the regex "+qualifiedNameFmt+" (e.g. 'MyName' or 'my.name' or '123-abc')") + } + return errs } -const LabelValueFmt string = "(" + QualifiedNameFmt + ")?" +const LabelValueFmt string = "(" + qualifiedNameFmt + ")?" const LabelValueMaxLength int = 63 var labelValueRegexp = regexp.MustCompile("^" + LabelValueFmt + "$") diff --git a/pkg/util/validation/validation_test.go b/pkg/util/validation/validation_test.go index 79967d20467..884a6c4945e 100644 --- a/pkg/util/validation/validation_test.go +++ b/pkg/util/validation/validation_test.go @@ -222,8 +222,8 @@ func TestIsQualifiedName(t *testing.T) { strings.Repeat("a", 253) + "/" + strings.Repeat("b", 63), } for i := range successCases { - if !IsQualifiedName(successCases[i]) { - t.Errorf("case[%d]: %q: expected success", i, successCases[i]) + if errs := IsQualifiedName(successCases[i]); len(errs) != 0 { + t.Errorf("case[%d]: %q: expected success: %v", i, successCases[i], errs) } } @@ -240,7 +240,7 @@ func TestIsQualifiedName(t *testing.T) { strings.Repeat("a", 254) + "/abc", } for i := range errorCases { - if IsQualifiedName(errorCases[i]) { + if errs := IsQualifiedName(errorCases[i]); len(errs) == 0 { t.Errorf("case[%d]: %q: expected failure", i, errorCases[i]) } } diff --git a/pkg/volume/plugins.go b/pkg/volume/plugins.go index 190342cb360..22bf1f9e159 100644 --- a/pkg/volume/plugins.go +++ b/pkg/volume/plugins.go @@ -272,7 +272,7 @@ func (pm *VolumePluginMgr) InitPlugins(plugins []VolumePlugin, host VolumeHost) allErrs := []error{} for _, plugin := range plugins { name := plugin.Name() - if !validation.IsQualifiedName(name) { + if len(validation.IsQualifiedName(name)) != 0 { allErrs = append(allErrs, fmt.Errorf("volume plugin has invalid name: %#v", plugin)) continue } diff --git a/plugin/pkg/scheduler/factory/factory.go b/plugin/pkg/scheduler/factory/factory.go index e8aa6c03401..0500a273273 100644 --- a/plugin/pkg/scheduler/factory/factory.go +++ b/plugin/pkg/scheduler/factory/factory.go @@ -307,7 +307,7 @@ func (f *ConfigFactory) CreateFromKeys(predicateKeys, priorityKeys sets.String, failureDomainArgs := strings.Split(f.FailureDomains, ",") for _, failureDomain := range failureDomainArgs { - if !utilvalidation.IsQualifiedName(failureDomain) { + if len(utilvalidation.IsQualifiedName(failureDomain)) != 0 { return nil, fmt.Errorf("invalid failure domain: %s", failureDomain) } }