Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Loosen label and annotation validation and related tests #4898

Merged
merged 3 commits into from
Mar 6, 2015
Merged

Loosen label and annotation validation and related tests #4898

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
726a5af
Loosen label and annotation validation and related tests
Feb 27, 2015
bb8a4f5
apply comments
Mar 2, 2015
26b7fbe
apply comments
Mar 6, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion examples/guestbook/frontend-controller.json
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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