Merge pull request #2514 from thockin/label-ns

Add label key namespacing
This commit is contained in:
Tim Hockin 2014-12-02 13:38:46 -08:00
commit f4b031d025
10 changed files with 171 additions and 84 deletions

View File

@ -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

View File

@ -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"`
}

View File

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

View File

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

View File

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

View File

@ -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

View File

@ -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 {

View File

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

View File

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

View File

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