Add label key namespacing

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 labels follow a slightly modified RFC952 format:  24 characters or less, all lowercase, begins with alpha, dashes (-) are allowed, and ends with alphanumeric.
+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.
+
+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/types.go

@@ -107,12 +107,21 @@ type ObjectMeta struct {
 	CreationTimestamp util.Time `json:"creationTimestamp,omitempty" yaml:"creationTimestamp,omitempty"`
 
 	// Labels are key value pairs that may be used to scope and select individual resources.
+	// Label keys are of the form:
+	//     label-key ::= prefixed-name | name
+	//     prefixed-name ::= prefix '/' name
+	//     prefix ::= DNS_SUBDOMAIN
+	//     name ::= DNS_LABEL
+	// The prefix is optional.  If the prefix is not specified, the key is assumed to be private
+	// to the user.  Other system components that wish to use labels must specify a prefix.  The
+	// "kubernetes.io/" prefix is reserved for use by kubernetes components.
 	// TODO: replace map[string]string with labels.LabelSet type
 	Labels map[string]string `json:"labels,omitempty" yaml:"labels,omitempty"`
 
 	// Annotations are unstructured key value data stored with a resource that may be set by
 	// external tooling. They are not queryable and should be preserved when modifying
-	// objects.
+	// objects.  Annotation keys have the same formatting restrictions as Label keys. See the
+	// comments on Labels for details.
 	Annotations map[string]string `json:"annotations,omitempty" yaml:"annotations,omitempty"`
 }
 

pkg/api/validation/validation.go

@@ -363,7 +363,7 @@ func ValidatePod(pod *api.Pod) errs.ValidationErrorList {
 		allErrs = append(allErrs, errs.NewFieldInvalid("namespace", pod.Namespace, ""))
 	}
 	allErrs = append(allErrs, ValidatePodSpec(&pod.Spec).Prefix("spec")...)
-	allErrs = append(allErrs, validateLabels(pod.Labels)...)
+	allErrs = append(allErrs, validateLabels(pod.Labels, "labels")...)
 	return allErrs
 }
 
@@ -378,15 +378,27 @@ func ValidatePodSpec(spec *api.PodSpec) errs.ValidationErrorList {
 	allErrs = append(allErrs, vErrs.Prefix("volumes")...)
 	allErrs = append(allErrs, validateContainers(spec.Containers, allVolumes).Prefix("containers")...)
 	allErrs = append(allErrs, validateRestartPolicy(&spec.RestartPolicy).Prefix("restartPolicy")...)
-	allErrs = append(allErrs, validateLabels(spec.NodeSelector).Prefix("nodeSelector")...)
+	allErrs = append(allErrs, validateLabels(spec.NodeSelector, "nodeSelector")...)
 	return allErrs
 }
 
-func validateLabels(labels map[string]string) errs.ValidationErrorList {
+func validateLabels(labels map[string]string, field string) errs.ValidationErrorList {
 	allErrs := errs.ValidationErrorList{}
 	for k := range labels {
-		if !util.IsDNS952Label(k) {
-			allErrs = append(allErrs, errs.NewFieldNotSupported("label", k))
+		var n, ns string
+		parts := strings.Split(k, "/")
+		switch len(parts) {
+		case 1:
+			n = parts[0]
+		case 2:
+			ns = parts[0]
+			n = parts[1]
+		default:
+			allErrs = append(allErrs, errs.NewFieldInvalid(field, k, ""))
+			continue
+		}
+		if (ns != "" && !util.IsDNSSubdomain(ns)) || !util.IsDNSLabel(n) {
+			allErrs = append(allErrs, errs.NewFieldInvalid(field, k, ""))
 		}
 	}
 	return allErrs
@@ -456,8 +468,8 @@ func ValidateService(service *api.Service, lister ServiceLister, ctx api.Context
 			}
 		}
 	}
-	allErrs = append(allErrs, validateLabels(service.Labels)...)
-	allErrs = append(allErrs, validateLabels(service.Spec.Selector)...)
+	allErrs = append(allErrs, validateLabels(service.Labels, "labels")...)
+	allErrs = append(allErrs, validateLabels(service.Spec.Selector, "selector")...)
 	return allErrs
 }
 
@@ -471,7 +483,7 @@ func ValidateReplicationController(controller *api.ReplicationController) errs.V
 		allErrs = append(allErrs, errs.NewFieldInvalid("namespace", controller.Namespace, ""))
 	}
 	allErrs = append(allErrs, ValidateReplicationControllerSpec(&controller.Spec).Prefix("spec")...)
-	allErrs = append(allErrs, validateLabels(controller.Labels)...)
+	allErrs = append(allErrs, validateLabels(controller.Labels, "labels")...)
 	return allErrs
 }
 
@@ -494,7 +506,6 @@ func ValidateReplicationControllerSpec(spec *api.ReplicationControllerSpec) errs
 		if !selector.Matches(labels) {
 			allErrs = append(allErrs, errs.NewFieldInvalid("template.labels", spec.Template.Labels, "selector does not match template"))
 		}
-		allErrs = append(allErrs, validateLabels(spec.Template.Labels).Prefix("template.labels")...)
 		allErrs = append(allErrs, ValidatePodTemplateSpec(spec.Template).Prefix("template")...)
 		// RestartPolicy has already been first-order validated as per ValidatePodTemplateSpec().
 		if spec.Template.Spec.RestartPolicy.Always == nil {
@@ -509,6 +520,7 @@ func ValidateReplicationControllerSpec(spec *api.ReplicationControllerSpec) errs
 // ValidatePodTemplateSpec validates the spec of a pod template
 func ValidatePodTemplateSpec(spec *api.PodTemplateSpec) errs.ValidationErrorList {
 	allErrs := errs.ValidationErrorList{}
+	allErrs = append(allErrs, validateLabels(spec.Labels, "labels")...)
 	allErrs = append(allErrs, ValidatePodSpec(&spec.Spec).Prefix("spec")...)
 	allErrs = append(allErrs, ValidateReadOnlyPersistentDisks(spec.Spec.Volumes).Prefix("spec.volumes")...)
 	return allErrs
@@ -557,7 +569,7 @@ func ValidateMinion(minion *api.Minion) errs.ValidationErrorList {
 	if len(minion.Name) == 0 {
 		allErrs = append(allErrs, errs.NewFieldRequired("name", minion.Name))
 	}
-	allErrs = append(allErrs, validateLabels(minion.Labels)...)
+	allErrs = append(allErrs, validateLabels(minion.Labels, "labels")...)
 	return allErrs
 }
 

pkg/api/validation/validation_test.go

@@ -35,6 +35,43 @@ func expectPrefix(t *testing.T, prefix string, errs errors.ValidationErrorList)
 	}
 }
 
+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")
+		if len(errs) != 0 {
+			t.Errorf("case[%d] expected success, got %#v", i, errs)
+		}
+	}
+
+	errorCases := []map[string]string{
+		{"NoUppercase123": "bar"},
+		{"nospecialchars^=@": "bar"},
+		{"cantendwithadash-": "bar"},
+		{"only/one/slash": "bar"},
+		{strings.Repeat("a", 254): "bar"},
+	}
+	for i := range errorCases {
+		errs := validateLabels(errorCases[i], "field")
+		if len(errs) != 1 {
+			t.Errorf("case[%d] expected failure", i)
+		}
+	}
+}
+
 func TestValidateVolumes(t *testing.T) {
 	successCase := []api.Volume{
 		{Name: "abc"},
@@ -412,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)
 	}
 }
 
@@ -1003,8 +1035,8 @@ func TestValidateReplicationController(t *testing.T) {
 				field != "spec.template" &&
 				field != "GCEPersistentDisk.ReadOnly" &&
 				field != "spec.replicas" &&
-				field != "spec.template.label" &&
-				field != "label" {
+				field != "spec.template.labels" &&
+				field != "labels" {
 				t.Errorf("%s: missing prefix for: %v", k, errs[i])
 			}
 		}
@@ -1080,7 +1112,7 @@ func TestValidateMinion(t *testing.T) {
 		for i := range errs {
 			field := errs[i].(*errors.ValidationError).Field
 			if field != "name" &&
-				field != "label" {
+				field != "labels" {
 				t.Errorf("%s: missing prefix for: %v", k, errs[i])
 			}
 		}

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

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])?"
+// IsDNSLabel tests for a string that conforms to the definition of a label in
+// DNS (RFC 1123).
+func IsDNSLabel(value string) bool {
+	return IsDNS1123Label(value)
+}
+
+// IsDNSSubdomain tests for a string that conforms to the definition of a
+// subdomain in DNS (RFC 1123).
+func IsDNSSubdomain(value string) bool {
+	return IsDNS1123Subdomain(value)
+}
 
-var dnsLabelRegexp = regexp.MustCompile("^" + dnsLabelFmt + "$")
+const dns1123LabelFmt string = "[a-z0-9]([-a-z0-9]*[a-z0-9])?"
 
-const dnsLabelMaxLength int = 63
+var dns1123LabelRegexp = regexp.MustCompile("^" + dns1123LabelFmt + "$")
 
-// IsDNSLabel tests for a string that conforms to the definition of a label in
-// DNS (RFC 1035/1123).
-func IsDNSLabel(value string) bool {
-	return len(value) <= dnsLabelMaxLength && dnsLabelRegexp.MatchString(value)
+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 dnsSubdomainFmt string = dnsLabelFmt + "(\\." + dnsLabelFmt + ")*"
+const dns1123SubdomainFmt string = dns1123LabelFmt + "(\\." + dns1123LabelFmt + ")*"
 
-var dnsSubdomainRegexp = regexp.MustCompile("^" + dnsSubdomainFmt + "$")
+var dns1123SubdomainRegexp = regexp.MustCompile("^" + dns1123SubdomainFmt + "$")
 
-const dnsSubdomainMaxLength int = 253
+const dns1123SubdomainMaxLength int = 253
 
-// IsDNSSubdomain tests for a string that conforms to the definition of a
-// subdomain in DNS (RFC 1035/1123).
-func IsDNSSubdomain(value string) bool {
-	return len(value) <= dnsSubdomainMaxLength && dnsSubdomainRegexp.MatchString(value)
+// 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)
-}