diff --git a/pkg/labels/selector.go b/pkg/labels/selector.go index 3e6eca821aa..659083d1a05 100644 --- a/pkg/labels/selector.go +++ b/pkg/labels/selector.go @@ -22,6 +22,7 @@ import ( "sort" "strings" + "github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors" "github.com/GoogleCloudPlatform/kubernetes/pkg/util" ) @@ -137,6 +138,16 @@ func (t andTerm) String() string { return strings.Join(terms, ",") } +// TODO Support forward and reverse indexing (#1183, #1348). Eliminate uses of Selector.RequiresExactMatch. +// TODO rename to Selector after Selector interface above removed +type SetBasedSelector interface { + // Matches returns true if this selector matches the given set of labels. + Matches(Labels) (bool, error) + + // String returns a human-readable string that represents this selector. + String() (string, error) +} + // Operator represents a key's relationship // to a set of values in a Requirement. type Operator int @@ -171,13 +182,18 @@ type Requirement struct { } // NewRequirement is the constructor for a Requirement. -// If either of these rules is violated, an error is returned: +// If any of these rules is violated, an error is returned: // (1) The operator can only be In, NotIn or Exists. // (2) If the operator is In or NotIn, the values set must // be non-empty. +// (3) The key is invalid due to its length, or sequence +// of characters. See validateLabelKey for more details. // // The empty string is a valid value in the input values set. func NewRequirement(key string, op Operator, vals util.StringSet) (*Requirement, error) { + if err := validateLabelKey(key); err != nil { + return nil, err + } switch op { case In, NotIn: if len(vals) == 0 { @@ -281,16 +297,142 @@ func (lsel *LabelSelector) String() (string, error) { return strings.Join(reqs, ","), nil } -// TODO: Parse takes a string representing a selector and returns -// a selector, or an error. A well-formed input string follows -// the syntax of that which is returned by LabelSelector.String -// and therefore is largely controlled by that which is returned -// by Requirement.String. The returned selector object's type -// should be an interface implemented by LabelSelector. Note that -// this parsing function is different than ParseSelector since -// they parse different selectors with different syntaxes. -// See comments above for LabelSelector and Requirement struct -// definition for more details. +// Parse takes a string representing a selector and returns a selector +// object, or an error. This parsing function differs from ParseSelector +// as they parse different selectors with different syntaxes. +// The input will cause an error if it does not follow this form: +// +// ::= | "," +// ::= KEY +// ::= "" | +// ::= " in " | " not in " +// ::= "(" ")" +// ::= VALUE | VALUE "," +// +// KEY is a sequence of one or more characters that does not contain ',' or ' ' +// [^, ]+ +// VALUE is a sequence of zero or more characters that does not contain ',', ' ' or ')' +// [^, )]* +// +// Example of valid syntax: +// "x in (foo,,baz),y,z not in ()" +// +// Note: +// (1) Inclusion - " in " - denotes that the KEY is equal to any of the +// VALUEs in its requirement +// (2) Exclusion - " not in " - denotes that the KEY is not equal to any +// of the VALUEs in its requirement +// (3) The empty string is a valid VALUE +// (4) A requirement with just a KEY - as in "y" above - denotes that +// the KEY exists and can be any VALUE. +// +// TODO: value validation possibly including duplicate value check, restricting certain characters +func Parse(selector string) (SetBasedSelector, error) { + var items []Requirement + var key string + var op Operator + var vals util.StringSet + const ( + startReq int = iota + inKey + waitOp + inVals + ) + const inPre = "in (" + const notInPre = "not in (" + const pos = "position %d:%s" + + state := startReq + strStart := 0 + for i := 0; i < len(selector); i++ { + switch state { + case startReq: + switch selector[i] { + case ',': + return nil, fmt.Errorf("a requirement can't be empty. "+pos, i, selector) + case ' ': + return nil, fmt.Errorf("white space not allowed before key. "+pos, i, selector) + default: + state = inKey + strStart = i + } + case inKey: + switch selector[i] { + case ',': + state = startReq + if req, err := NewRequirement(selector[strStart:i], Exists, nil); err != nil { + return nil, err + } else { + items = append(items, *req) + } + case ' ': + state = waitOp + key = selector[strStart:i] + } + case waitOp: + if len(selector)-i >= len(inPre) && selector[i:len(inPre)+i] == inPre { + op = In + i += len(inPre) - 1 + } else if len(selector)-i >= len(notInPre) && selector[i:len(notInPre)+i] == notInPre { + op = NotIn + i += len(notInPre) - 1 + } else { + return nil, fmt.Errorf("expected \" in (\"/\" not in (\" after key. "+pos, i, selector) + } + state = inVals + vals = util.NewStringSet() + strStart = i + 1 + case inVals: + switch selector[i] { + case ',': + vals.Insert(selector[strStart:i]) + strStart = i + 1 + case ' ': + return nil, fmt.Errorf("white space not allowed in set strings. "+pos, i, selector) + case ')': + if i+1 == len(selector)-1 && selector[i+1] == ',' { + return nil, fmt.Errorf("expected requirement after comma. "+pos, i+1, selector) + } + if i+1 < len(selector) && selector[i+1] != ',' { + return nil, fmt.Errorf("requirements must be comma-separated. "+pos, i+1, selector) + } + state = startReq + vals.Insert(selector[strStart:i]) + if req, err := NewRequirement(key, op, vals); err != nil { + return nil, err + } else { + items = append(items, *req) + } + if i+1 < len(selector) { + i += 1 //advance past comma + } + } + } + } + + switch state { + case inKey: + if req, err := NewRequirement(selector[strStart:], Exists, nil); err != nil { + return nil, err + } else { + items = append(items, *req) + } + case waitOp: + return nil, fmt.Errorf("input terminated while waiting for operator \"in \"/\"not in \":%s", selector) + case inVals: + return nil, fmt.Errorf("input terminated while waiting for value set:%s", selector) + } + + return &LabelSelector{Requirements: items}, nil +} + +// TODO: unify with validation.validateLabels +func validateLabelKey(k string) error { + if !util.IsDNS952Label(k) { + return errors.NewFieldNotSupported("key", k) + } + return nil +} func try(selectorPiece, op string) (lhs, rhs string, ok bool) { pieces := strings.Split(selectorPiece, op) diff --git a/pkg/labels/selector_test.go b/pkg/labels/selector_test.go index 331dd286fb4..65af12467ae 100644 --- a/pkg/labels/selector_test.go +++ b/pkg/labels/selector_test.go @@ -17,6 +17,7 @@ limitations under the License. package labels import ( + "reflect" "testing" "github.com/GoogleCloudPlatform/kubernetes/pkg/util" @@ -212,6 +213,8 @@ func TestRequirementConstructor(t *testing.T) { {"x", In, util.NewStringSet("foo"), true}, {"x", NotIn, util.NewStringSet("foo"), true}, {"x", Exists, nil, true}, + {"abcdefghijklmnopqrstuvwxy", Exists, nil, false}, //breaks DNS952 rule that len(key) < 25 + {"1foo", In, util.NewStringSet("bar"), false}, //breaks DNS952 rule that keys start with [a-z] } for _, rc := range requirementConstructorTests { if _, err := NewRequirement(rc.Key, rc.Op, rc.Vals); err == nil && !rc.Success { @@ -289,6 +292,67 @@ func TestRequirementLabelSelectorMatching(t *testing.T) { } } +func TestSetSelectorParser(t *testing.T) { + setSelectorParserTests := []struct { + In string + Out SetBasedSelector + Match bool + Valid bool + }{ + {"", &LabelSelector{Requirements: nil}, true, true}, + {"x", &LabelSelector{Requirements: []Requirement{ + getRequirement("x", Exists, nil, t), + }}, true, true}, + {"foo in (abc)", &LabelSelector{Requirements: []Requirement{ + getRequirement("foo", In, util.NewStringSet("abc"), t), + }}, true, true}, + {"x not in (abc)", &LabelSelector{Requirements: []Requirement{ + getRequirement("x", NotIn, util.NewStringSet("abc"), t), + }}, true, true}, + {"x not in (abc,def)", &LabelSelector{Requirements: []Requirement{ + getRequirement("x", NotIn, util.NewStringSet("abc", "def"), t), + }}, true, true}, + {"x in (abc,def)", &LabelSelector{Requirements: []Requirement{ + getRequirement("x", In, util.NewStringSet("abc", "def"), t), + }}, true, true}, + {"x in (abc,)", &LabelSelector{Requirements: []Requirement{ + getRequirement("x", In, util.NewStringSet("abc", ""), t), + }}, true, true}, + {"x in ()", &LabelSelector{Requirements: []Requirement{ + getRequirement("x", In, util.NewStringSet(""), t), + }}, true, true}, + {"x not in (abc,,def),bar,z in (),w", &LabelSelector{Requirements: []Requirement{ + getRequirement("x", NotIn, util.NewStringSet("abc", "", "def"), t), + getRequirement("bar", Exists, nil, t), + getRequirement("z", In, util.NewStringSet(""), t), + getRequirement("w", Exists, nil, t), + }}, true, true}, + {"x,y in (a)", &LabelSelector{Requirements: []Requirement{ + getRequirement("y", In, util.NewStringSet("a"), t), + getRequirement("x", Exists, nil, t), + }}, false, true}, + {"x,,y", nil, true, false}, + {",x,y", nil, true, false}, + {"x, y", nil, true, false}, + {"x nott in (y)", nil, true, false}, + {"x not in ( )", nil, true, false}, + {"x not in (, a)", nil, true, false}, + {"a in (xyz),", nil, true, false}, + {"a in (xyz)b not in ()", nil, true, false}, + {"a ", nil, true, false}, + {"a not in(", nil, true, false}, + } + for _, ssp := range setSelectorParserTests { + if sel, err := Parse(ssp.In); err != nil && ssp.Valid { + t.Errorf("Parse(%s) => %v expected no error", ssp.In, err) + } else if err == nil && !ssp.Valid { + t.Errorf("Parse(%s) => %+v expected error", ssp.In, sel) + } else if ssp.Match && !reflect.DeepEqual(sel, ssp.Out) { + t.Errorf("parse output %+v doesn't match %+v, expected match", sel, ssp.Out) + } + } +} + func getRequirement(key string, op Operator, vals util.StringSet, t *testing.T) Requirement { req, err := NewRequirement(key, op, vals) if err != nil {