Skip to content

Commit

Permalink
Merge pull request kubernetes#4898 from gmarek/client2
Browse files Browse the repository at this point in the history 
Loosen label and annotation validation and related tests
  • Loading branch information
@thockin
thockin committed Mar 6, 2015
2 parents 788d999 + 26b7fbe commit ca265c5
Show file tree
Hide file tree
Showing 7 changed files with 124 additions and 119 deletions.
2 changes: 1 addition & 1 deletion examples/guestbook/frontend-controller.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
},
"labels": {
"name": "frontend",
"uses": "redis-slave,redis-master",
"uses": "redis-slave-or-redis-master",
"app": "frontend"
}
}},
Expand Down
8 changes: 8 additions & 0 deletions pkg/api/errors/validation.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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 ""
Expand Down Expand Up @@ -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.
Expand Down
20 changes: 17 additions & 3 deletions pkg/api/validation/validation.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,38 +30,52 @@ 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"

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
}

// 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
}
Expand Down
118 changes: 52 additions & 66 deletions pkg/api/validation/validation_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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 {
Expand All @@ -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) {
Expand All @@ -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")
Expand All @@ -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)
}
}
}
}
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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},
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion pkg/labels/selector_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Loading

0 comments on commit ca265c5

Please sign in to comment.