state-based parser for multi-valued set selector syntax

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:
+//
+//     <selector-syntax> ::= <requirement> | <requirement> "," <selector-syntax>
+//         <requirement> ::= KEY <set-restriction>
+//     <set-restriction> ::= "" | <inclusion-exclusion> <value-set>
+// <inclusion-exclusion> ::= " in " | " not in "
+//           <value-set> ::= "(" <values> ")"
+//              <values> ::= VALUE | VALUE "," <values>
+//
+// 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)

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 {