diff --git a/examples/guestbook/frontend-controller.json b/examples/guestbook/frontend-controller.json index 4d6b3ad36a57e..d502e5758d39b 100644 --- a/examples/guestbook/frontend-controller.json +++ b/examples/guestbook/frontend-controller.json @@ -21,7 +21,7 @@ }, "labels": { "name": "frontend", - "uses": "redis-slave,redis-master", + "uses": "redis-slave-or-redis-master", "app": "frontend" } }}, diff --git a/pkg/api/errors/validation.go b/pkg/api/errors/validation.go index f3431a495df8f..ef29780e0133a 100644 --- a/pkg/api/errors/validation.go +++ b/pkg/api/errors/validation.go @@ -52,6 +52,8 @@ const ( // values which would be accepted by some api instances, but which would invoke behavior // not permitted by this api instance (such as due to stricter security policy). ValidationErrorTypeForbidden ValidationErrorType = "FieldValueForbidden" + // ValidationErrorTypeTooLong is used to report that given value is too long. + ValidationErrorTypeTooLong ValidationErrorType = "FieldValueTooLong" ) // String converts a ValidationErrorType into its corresponding error message. @@ -69,6 +71,8 @@ func (t ValidationErrorType) String() string { return "unsupported value" case ValidationErrorTypeForbidden: return "forbidden" + case ValidationErrorTypeTooLong: + return "too long" default: glog.Errorf("unrecognized validation type: %#v", t) return "" @@ -130,6 +134,10 @@ func NewFieldNotFound(field string, value interface{}) *ValidationError { return &ValidationError{ValidationErrorTypeNotFound, field, value, ""} } +func NewFieldTooLong(field string, value interface{}) *ValidationError { + return &ValidationError{ValidationErrorTypeTooLong, field, value, ""} +} + type ValidationErrorList []error // Prefix adds a prefix to the Field of every ValidationError in the list. diff --git a/pkg/api/validation/validation.go b/pkg/api/validation/validation.go index b68da23275349..8a1e14bfe7c27 100644 --- a/pkg/api/validation/validation.go +++ b/pkg/api/validation/validation.go @@ -30,7 +30,6 @@ import ( "github.com/golang/glog" ) -const qualifiedNameErrorMsg string = "must match regex [" + util.DNS1123SubdomainFmt + " / ] " + util.DNS1123LabelFmt const cIdentifierErrorMsg string = "must match regex " + util.CIdentifierFmt const isNegativeErrorMsg string = "value must not be negative" @@ -38,19 +37,26 @@ func intervalErrorMsg(lo, hi int) string { return fmt.Sprintf("must be greater than %d and less than %d", lo, hi) } +var labelValueErrorMsg string = fmt.Sprintf("must have at most %d characters and match regex %s", util.LabelValueMaxLength, util.LabelValueFmt) +var qualifiedNameErrorMsg string = fmt.Sprintf("must have at most %d characters and match regex %s", util.DNS1123SubdomainMaxLength, util.QualifiedNameFmt) var dnsSubdomainErrorMsg string = fmt.Sprintf("must have at most %d characters and match regex %s", util.DNS1123SubdomainMaxLength, util.DNS1123SubdomainFmt) var dnsLabelErrorMsg string = fmt.Sprintf("must have at most %d characters and match regex %s", util.DNS1123LabelMaxLength, util.DNS1123LabelFmt) var dns952LabelErrorMsg string = fmt.Sprintf("must have at most %d characters and match regex %s", util.DNS952LabelMaxLength, util.DNS952LabelFmt) var pdPartitionErrorMsg string = intervalErrorMsg(0, 255) var portRangeErrorMsg string = intervalErrorMsg(0, 65536) +const totalAnnotationSizeLimitB int = 64 * (1 << 10) // 64 kB + // ValidateLabels validates that a set of labels are correctly defined. func ValidateLabels(labels map[string]string, field string) errs.ValidationErrorList { allErrs := errs.ValidationErrorList{} - for k := range labels { + for k, v := range labels { if !util.IsQualifiedName(k) { allErrs = append(allErrs, errs.NewFieldInvalid(field, k, qualifiedNameErrorMsg)) } + if !util.IsValidLabelValue(v) { + allErrs = append(allErrs, errs.NewFieldInvalid(field, v, labelValueErrorMsg)) + } } return allErrs } @@ -58,10 +64,18 @@ func ValidateLabels(labels map[string]string, field string) errs.ValidationError // ValidateAnnotations validates that a set of annotations are correctly defined. func ValidateAnnotations(annotations map[string]string, field string) errs.ValidationErrorList { allErrs := errs.ValidationErrorList{} - for k := range annotations { + var totalSize int64 + for k, v := range annotations { if !util.IsQualifiedName(strings.ToLower(k)) { allErrs = append(allErrs, errs.NewFieldInvalid(field, k, qualifiedNameErrorMsg)) } + if !util.IsValidAnnotationValue(v) { + allErrs = append(allErrs, errs.NewFieldInvalid(field, k, "")) + } + totalSize += (int64)(len(k)) + (int64)(len(v)) + } + if totalSize > (int64)(totalAnnotationSizeLimitB) { + allErrs = append(allErrs, errs.NewFieldTooLong("annotations", "")) } return allErrs } diff --git a/pkg/api/validation/validation_test.go b/pkg/api/validation/validation_test.go index a81f03c1e9951..f2203ff799a97 100644 --- a/pkg/api/validation/validation_test.go +++ b/pkg/api/validation/validation_test.go @@ -75,6 +75,8 @@ func TestValidateLabels(t *testing.T) { {"1-num.2-num/3-num": "bar"}, {"1234/5678": "bar"}, {"1.2.3.4/5678": "bar"}, + {"UpperCaseAreOK123": "bar"}, + {"goodvalue": "123_-.BaR"}, } for i := range successCases { errs := ValidateLabels(successCases[i], "field") @@ -83,15 +85,14 @@ func TestValidateLabels(t *testing.T) { } } - errorCases := []map[string]string{ - {"NoUppercase123": "bar"}, + labelNameErrorCases := []map[string]string{ {"nospecialchars^=@": "bar"}, {"cantendwithadash-": "bar"}, {"only/one/slash": "bar"}, {strings.Repeat("a", 254): "bar"}, } - for i := range errorCases { - errs := ValidateLabels(errorCases[i], "field") + for i := range labelNameErrorCases { + errs := ValidateLabels(labelNameErrorCases[i], "field") if len(errs) != 1 { t.Errorf("case[%d] expected failure", i) } else { @@ -101,6 +102,24 @@ func TestValidateLabels(t *testing.T) { } } } + + labelValueErrorCases := []map[string]string{ + {"toolongvalue": strings.Repeat("a", 64)}, + {"backslashesinvalue": "some\\bad\\value"}, + {"nocommasallowed": "bad,value"}, + {"strangecharsinvalue": "?#$notsogood"}, + } + for i := range labelValueErrorCases { + errs := ValidateLabels(labelValueErrorCases[i], "field") + if len(errs) != 1 { + t.Errorf("case[%d] expected failure", i) + } else { + detail := errs[0].(*errors.ValidationError).Detail + if detail != labelValueErrorMsg { + t.Errorf("error detail %s should be equal %s", detail, labelValueErrorMsg) + } + } + } } func TestValidateAnnotations(t *testing.T) { @@ -118,6 +137,11 @@ func TestValidateAnnotations(t *testing.T) { {"1234/5678": "bar"}, {"1.2.3.4/5678": "bar"}, {"UpperCase123": "bar"}, + {"a": strings.Repeat("b", (64*1024)-1)}, + { + "a": strings.Repeat("b", (32*1024)-1), + "c": strings.Repeat("d", (32*1024)-1), + }, } for i := range successCases { errs := ValidateAnnotations(successCases[i], "field") @@ -126,21 +150,33 @@ func TestValidateAnnotations(t *testing.T) { } } - errorCases := []map[string]string{ + nameErrorCases := []map[string]string{ {"nospecialchars^=@": "bar"}, {"cantendwithadash-": "bar"}, {"only/one/slash": "bar"}, {strings.Repeat("a", 254): "bar"}, } - for i := range errorCases { - errs := ValidateAnnotations(errorCases[i], "field") + for i := range nameErrorCases { + errs := ValidateAnnotations(nameErrorCases[i], "field") + if len(errs) != 1 { + t.Errorf("case[%d] expected failure", i) + } + detail := errs[0].(*errors.ValidationError).Detail + if detail != qualifiedNameErrorMsg { + t.Errorf("error detail %s should be equal %s", detail, qualifiedNameErrorMsg) + } + } + totalSizeErrorCases := []map[string]string{ + {"a": strings.Repeat("b", 64*1024)}, + { + "a": strings.Repeat("b", 32*1024), + "c": strings.Repeat("d", 32*1024), + }, + } + for i := range totalSizeErrorCases { + errs := ValidateAnnotations(totalSizeErrorCases[i], "field") if len(errs) != 1 { t.Errorf("case[%d] expected failure", i) - } else { - detail := errs[0].(*errors.ValidationError).Detail - if detail != qualifiedNameErrorMsg { - t.Errorf("error detail %s should be equal %s", detail, qualifiedNameErrorMsg) - } } } } @@ -840,19 +876,6 @@ func TestValidatePod(t *testing.T) { DNSPolicy: api.DNSClusterFirst, }, }, - "bad annotation": { - ObjectMeta: api.ObjectMeta{ - Name: "abc", - Namespace: "ns", - Annotations: map[string]string{ - "NoUppercaseOrSpecialCharsLike=Equals": "bar", - }, - }, - Spec: api.PodSpec{ - RestartPolicy: api.RestartPolicy{Always: &api.RestartPolicyAlways{}}, - DNSPolicy: api.DNSClusterFirst, - }, - }, } for k, v := range errorCases { if errs := ValidatePod(&v); len(errs) == 0 { @@ -1468,24 +1491,6 @@ func TestValidateService(t *testing.T) { }, numErrs: 1, }, - { - name: "invalid annotation", - svc: api.Service{ - ObjectMeta: api.ObjectMeta{ - Name: "abc123", - Namespace: api.NamespaceDefault, - Annotations: map[string]string{ - "NoUppercaseOrSpecialCharsLike=Equals": "bar", - }, - }, - Spec: api.ServiceSpec{ - Port: 8675, - Protocol: "TCP", - SessionAffinity: "None", - }, - }, - numErrs: 1, - }, { name: "invalid selector", svc: api.Service{ @@ -1832,19 +1837,6 @@ func TestValidateReplicationController(t *testing.T) { Template: &invalidPodTemplate.Spec, }, }, - "invalid_annotation": { - ObjectMeta: api.ObjectMeta{ - Name: "abc-123", - Namespace: api.NamespaceDefault, - Annotations: map[string]string{ - "NoUppercaseOrSpecialCharsLike=Equals": "bar", - }, - }, - Spec: api.ReplicationControllerSpec{ - Selector: validSelector, - Template: &validPodTemplate.Spec, - }, - }, "invalid restart policy 1": { ObjectMeta: api.ObjectMeta{ Name: "abc-123", @@ -1957,12 +1949,6 @@ func TestValidateMinion(t *testing.T) { Labels: invalidSelector, }, }, - "invalid-annotations": { - ObjectMeta: api.ObjectMeta{ - Name: "abc-123", - Annotations: invalidSelector, - }, - }, } for k, v := range errorCases { errs := ValidateMinion(&v) @@ -2097,7 +2083,7 @@ func TestValidateMinionUpdate(t *testing.T) { Name: "foo", Labels: map[string]string{"Foo": "baz"}, }, - }, false}, + }, true}, } for i, test := range tests { errs := ValidateMinionUpdate(&test.oldMinion, &test.minion) @@ -2266,7 +2252,7 @@ func TestValidateServiceUpdate(t *testing.T) { Name: "foo", Labels: map[string]string{"Foo": "baz"}, }, - }, false}, + }, true}, } for i, test := range tests { errs := ValidateServiceUpdate(&test.oldService, &test.service) @@ -2300,7 +2286,7 @@ func TestValidateResourceNames(t *testing.T) { {"my.favorite.app.co/_12345", false}, {"my.favorite.app.co/12345_", false}, {"kubernetes.io/..", false}, - {"kubernetes.io/" + longString, false}, + {"kubernetes.io/" + longString, true}, {"kubernetes.io//", false}, {"kubernetes.io", false}, {"kubernetes.io/will/not/work/", false}, @@ -2557,7 +2543,7 @@ func TestValidateNamespaceUpdate(t *testing.T) { Name: "foo", Labels: map[string]string{"Foo": "baz"}, }, - }, false}, + }, true}, } for i, test := range tests { errs := ValidateNamespaceUpdate(&test.oldNamespace, &test.namespace) diff --git a/pkg/labels/selector_test.go b/pkg/labels/selector_test.go index 071e152cb5b07..d5c1438fe68be 100644 --- a/pkg/labels/selector_test.go +++ b/pkg/labels/selector_test.go @@ -417,7 +417,7 @@ func TestRequirementConstructor(t *testing.T) { {"x", ExistsOperator, nil, true}, {"1foo", InOperator, util.NewStringSet("bar"), true}, {"1234", InOperator, util.NewStringSet("bar"), true}, - {strings.Repeat("a", 64), ExistsOperator, nil, false}, //breaks DNS rule that len(key) <= 63 + {strings.Repeat("a", 254), ExistsOperator, nil, false}, //breaks DNS rule that len(key) <= 253 } for _, rc := range requirementConstructorTests { if _, err := NewRequirement(rc.Key, rc.Op, rc.Vals); err == nil && !rc.Success { diff --git a/pkg/util/validation.go b/pkg/util/validation.go index f366393a25f3f..9ab0dcfe9242d 100644 --- a/pkg/util/validation.go +++ b/pkg/util/validation.go @@ -18,19 +18,33 @@ package util import ( "regexp" - "strings" ) -// 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) +const kubeChar string = "[A-Za-z0-9]" +const extendedKubeChar string = "[-A-Za-z0-9_.]" +const qualifiedToken string = "(" + kubeChar + extendedKubeChar + "*)?" + kubeChar + +const LabelValueFmt string = "((" + kubeChar + extendedKubeChar + "*)?" + kubeChar + ")?" + +var labelValueRegexp = regexp.MustCompile("^" + LabelValueFmt + "$") + +const LabelValueMaxLength int = 63 + +func IsValidLabelValue(value string) bool { + return (len(value) <= LabelValueMaxLength && labelValueRegexp.MatchString(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) +// Annotation values are opaque. +func IsValidAnnotationValue(value string) bool { + return true +} + +const QualifiedNameFmt string = "(" + qualifiedToken + "/)?" + qualifiedToken + +var qualifiedNameRegexp = regexp.MustCompile("^" + QualifiedNameFmt + "$") + +func IsQualifiedName(value string) bool { + return (len(value) <= DNS1123SubdomainMaxLength && qualifiedNameRegexp.MatchString(value)) } const DNS1123LabelFmt string = "[a-z0-9]([-a-z0-9]*[a-z0-9])?" @@ -57,6 +71,18 @@ func IsDNS1123Subdomain(value string) bool { return len(value) <= DNS1123SubdomainMaxLength && dns1123SubdomainRegexp.MatchString(value) } +// 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) +} + const DNS952LabelFmt string = "[a-z]([-a-z0-9]*[a-z0-9])?" var dns952LabelRegexp = regexp.MustCompile("^" + DNS952LabelFmt + "$") @@ -83,33 +109,3 @@ func IsCIdentifier(value string) bool { func IsValidPortNum(port int) bool { return 0 < port && port < 65536 } - -// IsQualifiedName tests whether a string fits the "optionally-namespaced -// name" pattern: [ DNS_SUBDOMAIN "/" ] DNS_LABEL -func IsQualifiedName(value string) bool { - var n, ns string - parts := strings.Split(value, "/") - switch len(parts) { - case 1: - n = parts[0] - case 2: - ns = parts[0] - n = parts[1] - default: - return false - } - if (ns != "" && !IsDNSSubdomain(ns)) || !IsDNSLabel(n) { - return false - } - return true -} - -const LabelValueFmt string = "([A-Za-z0-9_\\-\\\\.]*)" - -var labelValueRegexp = regexp.MustCompile("^" + LabelValueFmt + "$") - -const labelValueMaxLength int = 63 - -func IsValidLabelValue(value string) bool { - return (len(value) <= labelValueMaxLength && labelValueRegexp.MatchString(value)) -} diff --git a/pkg/util/validation_test.go b/pkg/util/validation_test.go index 47222388c0111..61c7061a8d90c 100644 --- a/pkg/util/validation_test.go +++ b/pkg/util/validation_test.go @@ -168,6 +168,7 @@ func TestIsQualifiedName(t *testing.T) { "1-num.2-num/3-num", "1234/5678", "1.2.3.4/5678", + "UppercaseIsOK123", } for i := range successCases { if !IsQualifiedName(successCases[i]) { @@ -176,12 +177,11 @@ func TestIsQualifiedName(t *testing.T) { } errorCases := []string{ - "NoUppercase123", "nospecialchars%^=@", "cantendwithadash-", - "-cantstartwithadash", "only/one/slash", strings.Repeat("a", 254), + "-cantstartwithadash", } for i := range errorCases { if IsQualifiedName(errorCases[i]) { @@ -196,24 +196,25 @@ func TestIsValidLabelValue(t *testing.T) { "now-with-dashes", "1-starts-with-num", "end-with-num-1", - "-starts-with-dash", - "ends-with-dash-", - ".starts.with.dot", - "ends.with.dot.", - "\\preserve\\backslash", "1234", // only num strings.Repeat("a", 63), // to the limit + "", // empty value } for i := range successCases { if !IsValidLabelValue(successCases[i]) { - t.Errorf("case[%d] expected success", i) + t.Errorf("case %s expected success", successCases[i]) } } errorCases := []string{ "nospecialchars%^=@", "Tama-nui-te-rā.is.Māori.sun", - strings.Repeat("a", 65), + "\\backslashes\\are\\bad", + "-starts-with-dash", + "ends-with-dash-", + ".starts.with.dot", + "ends.with.dot.", + strings.Repeat("a", 64), // over the limit } for i := range errorCases { if IsValidLabelValue(errorCases[i]) {