diff --git a/docs/design/labels.md b/docs/design/labels.md index df904d3a979..8415376d021 100644 --- a/docs/design/labels.md +++ b/docs/design/labels.md @@ -18,9 +18,17 @@ Label selectors may also be used to associate policies with sets of objects. We also [plan](https://github.com/GoogleCloudPlatform/kubernetes/issues/560) to make labels available inside pods and [lifecycle hooks](container-environment.md). -[Namespacing of label keys](https://github.com/GoogleCloudPlatform/kubernetes/issues/1491) is under discussion. +Valid label keys are comprised of two segments - prefix and name - separated +by a slash (`/`). The name segment is required and must be a DNS label: 63 +characters or less, all lowercase, beginning and ending with an alphanumeric +character (`[a-z0-9]`), with dashes (`-`) and alphanumerics between. The +prefix and slash are optional. If specified, the prefix must be a DNS +subdomain (a series of DNS labels separated by dots (`.`), not longer than 253 +characters in total. -Valid labels follow a slightly modified RFC952 format: 24 characters or less, all lowercase, begins with alpha, dashes (-) are allowed, and ends with alphanumeric. +If the prefix is omitted, the label key is presumed to be private to the user. +System components which use labels must specify a prefix. The `kubernetes.io` +prefix is reserved for kubernetes core components. ## Motivation diff --git a/pkg/api/validation/validation.go b/pkg/api/validation/validation.go index 320e3cc971a..a4460d89c87 100644 --- a/pkg/api/validation/validation.go +++ b/pkg/api/validation/validation.go @@ -397,7 +397,7 @@ func validateLabels(labels map[string]string, field string) errs.ValidationError allErrs = append(allErrs, errs.NewFieldInvalid(field, k, "")) continue } - if (ns != "" && !util.IsDNSSubdomain(ns)) || !util.IsDNS952Label(n) { + if (ns != "" && !util.IsDNSSubdomain(ns)) || !util.IsDNSLabel(n) { allErrs = append(allErrs, errs.NewFieldInvalid(field, k, "")) } } diff --git a/pkg/api/validation/validation_test.go b/pkg/api/validation/validation_test.go index 54f78a1dabd..7c23dbe6370 100644 --- a/pkg/api/validation/validation_test.go +++ b/pkg/api/validation/validation_test.go @@ -39,11 +39,16 @@ func TestValidateLabels(t *testing.T) { successCases := []map[string]string{ {"simple": "bar"}, {"now-with-dashes": "bar"}, + {"1-starts-with-num": "bar"}, + {"1234": "bar"}, {"simple/simple": "bar"}, {"now-with-dashes/simple": "bar"}, {"now-with-dashes/now-with-dashes": "bar"}, {"now.with.dots/simple": "bar"}, {"now-with.dashes-and.dots/simple": "bar"}, + {"1-num.2-num/3-num": "bar"}, + {"1234/5678": "bar"}, + {"1.2.3.4/5678": "bar"}, } for i := range successCases { errs := validateLabels(successCases[i], "field") @@ -53,12 +58,11 @@ func TestValidateLabels(t *testing.T) { } errorCases := []map[string]string{ - {"123cantbeginwithnumber": "bar"}, //invalid - {"NoUppercase123": "bar"}, //invalid - {"nospecialchars^=@": "bar"}, //invalid - {"cantendwithadash-": "bar"}, //invalid - {"rfc952-mustbe24charactersorless": "bar"}, //invalid + {"NoUppercase123": "bar"}, + {"nospecialchars^=@": "bar"}, + {"cantendwithadash-": "bar"}, {"only/one/slash": "bar"}, + {strings.Repeat("a", 254): "bar"}, } for i := range errorCases { errs := validateLabels(errorCases[i], "field") @@ -445,19 +449,14 @@ func TestValidatePod(t *testing.T) { Name: "foo", Namespace: api.NamespaceDefault, Labels: map[string]string{ - "123cantbeginwithnumber": "bar", //invalid - "NoUppercase123": "bar", //invalid - "nospecialchars^=@": "bar", //invalid - "cantendwithadash-": "bar", //invalid - "rfc952-mustbe24charactersorless": "bar", //invalid - "rfc952-dash-nodots-lower": "bar", //good label - "rfc952-24chars-orless": "bar", //good label + "1/2/3/4/5": "bar", // invalid + "valid": "bar", // good }, }, Spec: api.PodSpec{}, }) - if len(errs) != 5 { - t.Errorf("Unexpected non-zero error list: %#v", errs) + if len(errs) != 1 { + t.Errorf("Unexpected error list: %#v", errs) } } diff --git a/pkg/kubelet/envvars/envvars.go b/pkg/kubelet/envvars/envvars.go index 2cf5418496f..b06af93fb28 100644 --- a/pkg/kubelet/envvars/envvars.go +++ b/pkg/kubelet/envvars/envvars.go @@ -43,6 +43,10 @@ func FromServices(services *api.ServiceList) []api.EnvVar { } func makeEnvVariableName(str string) string { + // TODO: If we simplify to "all names are DNS1123Subdomains" this + // will need two tweaks: + // 1) Handle leading digits + // 2) Handle dots return strings.ToUpper(strings.Replace(str, "-", "_", -1)) } diff --git a/pkg/labels/selector.go b/pkg/labels/selector.go index 246bcd5fb14..7f9a00e399b 100644 --- a/pkg/labels/selector.go +++ b/pkg/labels/selector.go @@ -428,7 +428,7 @@ func Parse(selector string) (SetBasedSelector, error) { // TODO: unify with validation.validateLabels func validateLabelKey(k string) error { - if !util.IsDNS952Label(k) { + if !util.IsDNSLabel(k) { return errors.NewFieldNotSupported("key", k) } return nil diff --git a/pkg/labels/selector_test.go b/pkg/labels/selector_test.go index 54eb0b15fc2..4a9b7fac542 100644 --- a/pkg/labels/selector_test.go +++ b/pkg/labels/selector_test.go @@ -18,6 +18,7 @@ package labels import ( "reflect" + "strings" "testing" "github.com/GoogleCloudPlatform/kubernetes/pkg/util" @@ -222,8 +223,9 @@ func TestRequirementConstructor(t *testing.T) { {"x", In, util.NewStringSet("foo"), true}, {"x", NotIn, util.NewStringSet("foo"), true}, {"x", Exists, nil, true}, - {"abcdefghijklmnopqrstuvwxy", Exists, nil, false}, //breaks DNS952 rule that len(key) < 25 - {"1foo", In, util.NewStringSet("bar"), false}, //breaks DNS952 rule that keys start with [a-z] + {"1foo", In, util.NewStringSet("bar"), true}, + {"1234", In, util.NewStringSet("bar"), true}, + {strings.Repeat("a", 64), Exists, nil, false}, //breaks DNS rule that len(key) <= 63 } for _, rc := range requirementConstructorTests { if _, err := NewRequirement(rc.Key, rc.Op, rc.Vals); err == nil && !rc.Success { diff --git a/pkg/registry/pod/rest_test.go b/pkg/registry/pod/rest_test.go index cb06b8f63f8..543ee02eff4 100644 --- a/pkg/registry/pod/rest_test.go +++ b/pkg/registry/pod/rest_test.go @@ -518,7 +518,7 @@ func TestPodStorageValidatesCreate(t *testing.T) { pod := &api.Pod{ ObjectMeta: api.ObjectMeta{ Labels: map[string]string{ - "invalid-label-to-cause-validation-failure": "bar", + "invalid/label/to/cause/validation/failure": "bar", }, }, } diff --git a/pkg/util/validation.go b/pkg/util/validation.go index 3f1f0c433e9..a46e6636f81 100644 --- a/pkg/util/validation.go +++ b/pkg/util/validation.go @@ -20,28 +20,52 @@ import ( "regexp" ) -const dnsLabelFmt string = "[a-z0-9]([-a-z0-9]*[a-z0-9])?" - -var dnsLabelRegexp = regexp.MustCompile("^" + dnsLabelFmt + "$") - -const dnsLabelMaxLength int = 63 - // IsDNSLabel tests for a string that conforms to the definition of a label in -// DNS (RFC 1035/1123). +// DNS (RFC 1123). func IsDNSLabel(value string) bool { - return len(value) <= dnsLabelMaxLength && dnsLabelRegexp.MatchString(value) + return IsDNS1123Label(value) } -const dnsSubdomainFmt string = dnsLabelFmt + "(\\." + dnsLabelFmt + ")*" - -var dnsSubdomainRegexp = regexp.MustCompile("^" + dnsSubdomainFmt + "$") - -const dnsSubdomainMaxLength int = 253 - // IsDNSSubdomain tests for a string that conforms to the definition of a -// subdomain in DNS (RFC 1035/1123). +// subdomain in DNS (RFC 1123). func IsDNSSubdomain(value string) bool { - return len(value) <= dnsSubdomainMaxLength && dnsSubdomainRegexp.MatchString(value) + return IsDNS1123Subdomain(value) +} + +const dns1123LabelFmt string = "[a-z0-9]([-a-z0-9]*[a-z0-9])?" + +var dns1123LabelRegexp = regexp.MustCompile("^" + dns1123LabelFmt + "$") + +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) +} + +const dns1123SubdomainFmt string = dns1123LabelFmt + "(\\." + dns1123LabelFmt + ")*" + +var dns1123SubdomainRegexp = regexp.MustCompile("^" + dns1123SubdomainFmt + "$") + +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) +} + +const dns952LabelFmt string = "[a-z]([-a-z0-9]*[a-z0-9])?" + +var dns952LabelRegexp = regexp.MustCompile("^" + dns952LabelFmt + "$") + +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) } const cIdentifierFmt string = "[A-Za-z_][A-Za-z0-9_]*" @@ -58,13 +82,3 @@ func IsCIdentifier(value string) bool { func IsValidPortNum(port int) bool { return 0 < port && port < 65536 } - -const dns952IdentifierFmt string = "[a-z]([-a-z0-9]*[a-z0-9])?" - -var dns952Regexp = regexp.MustCompile("^" + dns952IdentifierFmt + "$") - -const dns952MaxLength = 24 - -func IsDNS952Label(value string) bool { - return len(value) <= dns952MaxLength && dns952Regexp.MatchString(value) -} diff --git a/pkg/util/validation_test.go b/pkg/util/validation_test.go index 3bd66660063..48254c553ee 100644 --- a/pkg/util/validation_test.go +++ b/pkg/util/validation_test.go @@ -21,13 +21,14 @@ import ( "testing" ) -func TestIsDNSLabel(t *testing.T) { +func TestIsDNS1123Label(t *testing.T) { goodValues := []string{ "a", "ab", "abc", "a1", "a-1", "a--1--2--b", "0", "01", "012", "1a", "1-a", "1--a--b--2", + strings.Repeat("a", 63), } for _, val := range goodValues { - if !IsDNSLabel(val) { + if !IsDNS1123Label(val) { t.Errorf("expected true for '%s'", val) } } @@ -41,13 +42,13 @@ func TestIsDNSLabel(t *testing.T) { strings.Repeat("a", 64), } for _, val := range badValues { - if IsDNSLabel(val) { + if IsDNS1123Label(val) { t.Errorf("expected false for '%s'", val) } } } -func TestIsDNSSubdomain(t *testing.T) { +func TestIsDNS1123Subdomain(t *testing.T) { goodValues := []string{ "a", "ab", "abc", "a1", "a-1", "a--1--2--b", "0", "01", "012", "1a", "1-a", "1--a--b--2", @@ -56,9 +57,10 @@ func TestIsDNSSubdomain(t *testing.T) { "0.a", "01.a", "012.a", "1a.a", "1-a.a", "1--a--b--2", "0.1", "01.1", "012.1", "1a.1", "1-a.1", "1--a--b--2.1", "a.b.c.d.e", "aa.bb.cc.dd.ee", "1.2.3.4.5", "11.22.33.44.55", + strings.Repeat("a", 253), } for _, val := range goodValues { - if !IsDNSSubdomain(val) { + if !IsDNS1123Subdomain(val) { t.Errorf("expected true for '%s'", val) } } @@ -78,7 +80,34 @@ func TestIsDNSSubdomain(t *testing.T) { strings.Repeat("a", 254), } for _, val := range badValues { - if IsDNSSubdomain(val) { + if IsDNS1123Subdomain(val) { + t.Errorf("expected false for '%s'", val) + } + } +} + +func TestIsDNS952Label(t *testing.T) { + goodValues := []string{ + "a", "ab", "abc", "a1", "a-1", "a--1--2--b", + strings.Repeat("a", 24), + } + for _, val := range goodValues { + if !IsDNS952Label(val) { + t.Errorf("expected true for '%s'", val) + } + } + + badValues := []string{ + "0", "01", "012", "1a", "1-a", "1--a--b--2", + "", "A", "ABC", "aBc", "A1", "A-1", "1-A", + "-", "a-", "-a", "1-", "-1", + "_", "a_", "_a", "a_b", "1_", "_1", "1_2", + ".", "a.", ".a", "a.b", "1.", ".1", "1.2", + " ", "a ", " a", "a b", "1 ", " 1", "1 2", + strings.Repeat("a", 25), + } + for _, val := range badValues { + if IsDNS952Label(val) { t.Errorf("expected false for '%s'", val) } } @@ -124,26 +153,3 @@ func TestIsValidPortNum(t *testing.T) { } } } - -func TestIsDNS952(t *testing.T) { - goodValues := []string{ - "a", "ab", "abc", "a1", "a-b", "a-1", "a-1-2-b", "abc-123", - } - for _, val := range goodValues { - if !IsDNS952Label(val) { - t.Errorf("expected true for '%s'", val) - } - } - - badValues := []string{ - "", "1", "123", "1a", - "-", "a-", "-a", "1-", "-1", "1-2", - " ", "a ", " a", "a b", "1 ", " 1", "1 2", - "A", "AB", "AbC", "A1", "A-B", "A-1", "A-1-2-B", - } - for _, val := range badValues { - if IsDNS952Label(val) { - t.Errorf("expected false for '%s'", val) - } - } -}