Loosen DNS 952 for labels

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
 

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, ""))
 		}
 	}

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"},
-		{"NoUppercase123": "bar"},                  //invalid
+		{"nospecialchars^=@": "bar"},
-		{"nospecialchars^=@": "bar"},               //invalid
+		{"cantendwithadash-": "bar"},
-		{"cantendwithadash-": "bar"},               //invalid
-		{"rfc952-mustbe24charactersorless": "bar"}, //invalid
 		{"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
+				"1/2/3/4/5": "bar", // invalid
-				"NoUppercase123":                  "bar", //invalid
+				"valid":     "bar", // good
-				"nospecialchars^=@":               "bar", //invalid
-				"cantendwithadash-":               "bar", //invalid
-				"rfc952-mustbe24charactersorless": "bar", //invalid
-				"rfc952-dash-nodots-lower":        "bar", //good label
-				"rfc952-24chars-orless":           "bar", //good label
 			},
 		},
 		Spec: api.PodSpec{},
 	})
-	if len(errs) != 5 {
+	if len(errs) != 1 {
-		t.Errorf("Unexpected non-zero error list: %#v", errs)
+		t.Errorf("Unexpected error list: %#v", errs)
 	}
 }
 

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))
 }
 

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

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"), true},
-		{"1foo", In, util.NewStringSet("bar"), false},     //breaks DNS952 rule that keys start with [a-z]
+		{"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 {

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",
 			},
 		},
 	}

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)
-}

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)
-		}
-	}
-}