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/validation/validation.go b/pkg/api/validation/validation.go index 54b25d6447f5d..8a1e14bfe7c27 100644 --- a/pkg/api/validation/validation.go +++ b/pkg/api/validation/validation.go @@ -30,7 +30,6 @@ import ( "github.com/golang/glog" ) -const invalidAnnotationValueErrorMsg string = "must match regex " + util.AnnotationValueFmt const cIdentifierErrorMsg string = "must match regex " + util.CIdentifierFmt const isNegativeErrorMsg string = "value must not be negative" @@ -46,6 +45,8 @@ var dns952LabelErrorMsg string = fmt.Sprintf("must have at most %d characters an 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{} @@ -69,11 +70,11 @@ func ValidateAnnotations(annotations map[string]string, field string) errs.Valid allErrs = append(allErrs, errs.NewFieldInvalid(field, k, qualifiedNameErrorMsg)) } if !util.IsValidAnnotationValue(v) { - allErrs = append(allErrs, errs.NewFieldInvalid(field, k, invalidAnnotationValueErrorMsg)) + allErrs = append(allErrs, errs.NewFieldInvalid(field, k, "")) } totalSize += (int64)(len(k)) + (int64)(len(v)) } - if totalSize > (int64)(util.TotalAnnotationSizeB) { + if totalSize > (int64)(totalAnnotationSizeLimitB) { allErrs = append(allErrs, errs.NewFieldTooLong("annotations", "")) } return allErrs @@ -881,7 +882,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 51d6a706d2404..f2203ff799a97 100644 --- a/pkg/api/validation/validation_test.go +++ b/pkg/api/validation/validation_test.go @@ -76,7 +76,7 @@ func TestValidateLabels(t *testing.T) { {"1234/5678": "bar"}, {"1.2.3.4/5678": "bar"}, {"UpperCaseAreOK123": "bar"}, - {"goodvalue": "123_-.BaR,"}, + {"goodvalue": "123_-.BaR"}, } for i := range successCases { errs := ValidateLabels(successCases[i], "field") @@ -106,6 +106,7 @@ 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 { @@ -136,7 +137,11 @@ func TestValidateAnnotations(t *testing.T) { {"1234/5678": "bar"}, {"1.2.3.4/5678": "bar"}, {"UpperCase123": "bar"}, - {"a": strings.Repeat("b", 64*(1<<10)-1)}, + {"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") @@ -144,7 +149,7 @@ func TestValidateAnnotations(t *testing.T) { t.Errorf("case[%d] expected success, got %#v", i, errs) } } - + nameErrorCases := []map[string]string{ {"nospecialchars^=@": "bar"}, {"cantendwithadash-": "bar"}, @@ -161,11 +166,15 @@ func TestValidateAnnotations(t *testing.T) { t.Errorf("error detail %s should be equal %s", detail, qualifiedNameErrorMsg) } } - errorCases := []map[string]string{ - {"a": strings.Repeat("b", 64*(1<<10))}, + 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 errorCases { - errs := ValidateAnnotations(errorCases[i], "field") + for i := range totalSizeErrorCases { + errs := ValidateAnnotations(totalSizeErrorCases[i], "field") if len(errs) != 1 { t.Errorf("case[%d] expected failure", i) } @@ -2274,7 +2283,7 @@ func TestValidateResourceNames(t *testing.T) { {".", false}, {"..", false}, {"my.favorite.app.co/12345", true}, - {"my.favorite.app.co/_12345", true}, + {"my.favorite.app.co/_12345", false}, {"my.favorite.app.co/12345_", false}, {"kubernetes.io/..", false}, {"kubernetes.io/" + longString, true}, diff --git a/pkg/labels/selector_test.go b/pkg/labels/selector_test.go index 4455e0f39e255..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, true}, //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 fc57372e94e7b..06e1ca0a65c89 100644 --- a/pkg/util/validation.go +++ b/pkg/util/validation.go @@ -20,7 +20,7 @@ import ( "regexp" ) -const LabelValueFmt string = "[A-Za-z0-9_\\-\\.,]*" +const LabelValueFmt string = "[-A-Za-z0-9_.]*" var labelValueRegexp = regexp.MustCompile("^" + LabelValueFmt + "$") @@ -30,17 +30,15 @@ func IsValidLabelValue(value string) bool { return (len(value) <= LabelValueMaxLength && labelValueRegexp.MatchString(value)) } -const AnnotationValueFmt string = ".*" - -var annotationValueRegexp = regexp.MustCompile("^" + AnnotationValueFmt + "$") - -const TotalAnnotationSizeB int = 64 * (1 << 10) // 64 kB - +// Annotation values are opaque. func IsValidAnnotationValue(value string) bool { - return annotationValueRegexp.MatchString(value) + return true } -const QualifiedNameFmt string = "([A-Za-z0-9_\\-\\.]+/)?[A-Za-z0-9_\\-\\.]*[A-Za-z0-9]" +const kubeToken string = "[A-Za-z0-9]" +const extendedKubeToken string = "[-A-Za-z0-9_.]" +const qualifiedNamePiece string = "(" + kubeToken + extendedKubeToken + "*)?" + kubeToken +const QualifiedNameFmt string = "(" + qualifiedNamePiece + "/)?" + qualifiedNamePiece var qualifiedNameRegexp = regexp.MustCompile("^" + QualifiedNameFmt + "$") diff --git a/pkg/util/validation_test.go b/pkg/util/validation_test.go index a215e7e179c8f..828c3cafd3003 100644 --- a/pkg/util/validation_test.go +++ b/pkg/util/validation_test.go @@ -169,7 +169,6 @@ func TestIsQualifiedName(t *testing.T) { "1234/5678", "1.2.3.4/5678", "UppercaseIsOK123", - "-canstartwithadash", } for i := range successCases { if !IsQualifiedName(successCases[i]) { @@ -182,6 +181,7 @@ func TestIsQualifiedName(t *testing.T) { "cantendwithadash-", "only/one/slash", strings.Repeat("a", 254), + "-cantstartwithadash", } for i := range errorCases { if IsQualifiedName(errorCases[i]) {