diff --git a/pkg/api/errors/validation.go b/pkg/api/errors/validation.go index f3431a495df..ef29780e013 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 b68da232753..54b25d6447f 100644 --- a/pkg/api/validation/validation.go +++ b/pkg/api/validation/validation.go @@ -30,7 +30,7 @@ import ( "github.com/golang/glog" ) -const qualifiedNameErrorMsg string = "must match regex [" + util.DNS1123SubdomainFmt + " / ] " + util.DNS1123LabelFmt +const invalidAnnotationValueErrorMsg string = "must match regex " + util.AnnotationValueFmt const cIdentifierErrorMsg string = "must match regex " + util.CIdentifierFmt const isNegativeErrorMsg string = "value must not be negative" @@ -38,6 +38,8 @@ 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) @@ -47,10 +49,13 @@ var portRangeErrorMsg string = intervalErrorMsg(0, 65536) // 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 +63,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, invalidAnnotationValueErrorMsg)) + } + totalSize += (int64)(len(k)) + (int64)(len(v)) + } + if totalSize > (int64)(util.TotalAnnotationSizeB) { + allErrs = append(allErrs, errs.NewFieldTooLong("annotations", "")) } return allErrs } @@ -868,7 +881,7 @@ func validateResourceName(value string, field string) errs.ValidationErrorList { if len(strings.Split(value, "/")) == 1 { if !api.IsStandardResourceName(value) { - return append(allErrs, errs.NewFieldInvalid(field, value, "is neither a standard resource type nor is fully qualified")) + return append(allErrs, errs.NewFieldInvalid(field, value, "is neither a standard resource type nor is fully qualified") } } diff --git a/pkg/api/validation/validation_test.go b/pkg/api/validation/validation_test.go index a81f03c1e99..51d6a706d24 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,23 @@ func TestValidateLabels(t *testing.T) { } } } + + labelValueErrorCases := []map[string]string{ + {"toolongvalue": strings.Repeat("a", 64)}, + {"backslashesinvalue": "some\\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 +136,7 @@ func TestValidateAnnotations(t *testing.T) { {"1234/5678": "bar"}, {"1.2.3.4/5678": "bar"}, {"UpperCase123": "bar"}, + {"a": strings.Repeat("b", 64*(1<<10)-1)}, } for i := range successCases { errs := ValidateAnnotations(successCases[i], "field") @@ -125,22 +144,30 @@ func TestValidateAnnotations(t *testing.T) { t.Errorf("case[%d] expected success, got %#v", i, errs) } } - - errorCases := []map[string]string{ + + nameErrorCases := []map[string]string{ {"nospecialchars^=@": "bar"}, {"cantendwithadash-": "bar"}, {"only/one/slash": "bar"}, {strings.Repeat("a", 254): "bar"}, } + 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) + } + } + errorCases := []map[string]string{ + {"a": strings.Repeat("b", 64*(1<<10))}, + } for i := range errorCases { errs := ValidateAnnotations(errorCases[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 +867,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 +1482,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 +1828,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 +1940,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 +2074,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 +2243,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) @@ -2297,10 +2274,10 @@ func TestValidateResourceNames(t *testing.T) { {".", false}, {"..", false}, {"my.favorite.app.co/12345", true}, - {"my.favorite.app.co/_12345", false}, + {"my.favorite.app.co/_12345", true}, {"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 +2534,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 071e152cb5b..4455e0f39e2 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", 64), ExistsOperator, nil, true}, //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/util/validation.go b/pkg/util/validation.go index f366393a25f..fc57372e94e 100644 --- a/pkg/util/validation.go +++ b/pkg/util/validation.go @@ -18,19 +18,34 @@ 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 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)) } -// 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 AnnotationValueFmt string = ".*" + +var annotationValueRegexp = regexp.MustCompile("^" + AnnotationValueFmt + "$") + +const TotalAnnotationSizeB int = 64 * (1 << 10) // 64 kB + +func IsValidAnnotationValue(value string) bool { + return annotationValueRegexp.MatchString(value) +} + +const QualifiedNameFmt string = "([A-Za-z0-9_\\-\\.]+/)?[A-Za-z0-9_\\-\\.]*[A-Za-z0-9]" + +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 +72,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 +110,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 47222388c01..a215e7e179c 100644 --- a/pkg/util/validation_test.go +++ b/pkg/util/validation_test.go @@ -168,6 +168,8 @@ func TestIsQualifiedName(t *testing.T) { "1-num.2-num/3-num", "1234/5678", "1.2.3.4/5678", + "UppercaseIsOK123", + "-canstartwithadash", } for i := range successCases { if !IsQualifiedName(successCases[i]) { @@ -176,10 +178,8 @@ func TestIsQualifiedName(t *testing.T) { } errorCases := []string{ - "NoUppercase123", "nospecialchars%^=@", "cantendwithadash-", - "-cantstartwithadash", "only/one/slash", strings.Repeat("a", 254), } @@ -200,20 +200,20 @@ func TestIsValidLabelValue(t *testing.T) { "ends-with-dash-", ".starts.with.dot", "ends.with.dot.", - "\\preserve\\backslash", "1234", // only num strings.Repeat("a", 63), // to the limit } 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", + strings.Repeat("a", 64), // over the limit } for i := range errorCases { if IsValidLabelValue(errorCases[i]) {