From f6a74e3420a4aa754a07d1a2ad2c32a034d99a4b Mon Sep 17 00:00:00 2001
From: njuptlzf
Date: Thu, 22 Apr 2021 15:10:33 +0800
Subject: [PATCH] parseOperator description is inconsistent with the behavior
---
.../apimachinery/pkg/labels/selector.go | 11 ++++---
.../apimachinery/pkg/labels/selector_test.go | 29 +++++++++++++++++++
2 files changed, 36 insertions(+), 4 deletions(-)
diff --git a/staging/src/k8s.io/apimachinery/pkg/labels/selector.go b/staging/src/k8s.io/apimachinery/pkg/labels/selector.go
index b0865777a29..4b8ca3ec2f6 100644
--- a/staging/src/k8s.io/apimachinery/pkg/labels/selector.go
+++ b/staging/src/k8s.io/apimachinery/pkg/labels/selector.go
@@ -31,12 +31,15 @@ import (
)
var (
- validRequirementOperators = []string{
+ unaryOperators = []string{
+ string(selection.Exists), string(selection.DoesNotExist),
+ }
+ binaryOperators = []string{
string(selection.In), string(selection.NotIn),
string(selection.Equals), string(selection.DoubleEquals), string(selection.NotEquals),
- string(selection.Exists), string(selection.DoesNotExist),
string(selection.GreaterThan), string(selection.LessThan),
}
+ validRequirementOperators = append(binaryOperators, unaryOperators...)
)
// Requirements is AND of all requirements.
@@ -140,7 +143,7 @@ type Requirement struct {
// NewRequirement is the constructor for a Requirement.
// If any of these rules is violated, an error is returned:
-// (1) The operator can only be In, NotIn, Equals, DoubleEquals, NotEquals, Exists, or DoesNotExist.
+// (1) The operator can only be In, NotIn, Equals, DoubleEquals, Gt, Lt, NotEquals, Exists, or DoesNotExist.
// (2) If the operator is In or NotIn, the values set must be non-empty.
// (3) If the operator is Equals, DoubleEquals, or NotEquals, the values set must contain one value.
// (4) If the operator is Exists or DoesNotExist, the value set must be empty.
@@ -753,7 +756,7 @@ func (p *Parser) parseOperator() (op selection.Operator, err error) {
case NotEqualsToken:
op = selection.NotEquals
default:
- return "", fmt.Errorf("found '%s', expected: '=', '!=', '==', 'in', notin'", lit)
+ return "", fmt.Errorf("found '%s', expected: %v", lit, strings.Join(binaryOperators, ", "))
}
return op, nil
}
diff --git a/staging/src/k8s.io/apimachinery/pkg/labels/selector_test.go b/staging/src/k8s.io/apimachinery/pkg/labels/selector_test.go
index 07774bc9a0f..7c061462dd5 100644
--- a/staging/src/k8s.io/apimachinery/pkg/labels/selector_test.go
+++ b/staging/src/k8s.io/apimachinery/pkg/labels/selector_test.go
@@ -17,6 +17,7 @@ limitations under the License.
package labels
import (
+ "fmt"
"reflect"
"strings"
"testing"
@@ -148,6 +149,7 @@ func expectMatchDirect(t *testing.T, selector, ls Set) {
}
//lint:ignore U1000 currently commented out in TODO of TestSetMatches
+//nolint:unused,deadcode
func expectNoMatchDirect(t *testing.T, selector, ls Set) {
if SelectorFromSet(selector).Matches(ls) {
t.Errorf("Wanted '%s' to not match '%s', but it did.", selector, ls)
@@ -304,6 +306,33 @@ func TestParserLookahead(t *testing.T) {
}
}
+func TestParseOperator(t *testing.T) {
+ testcases := []struct {
+ token string
+ expectedError error
+ }{
+ {"in", nil},
+ {"=", nil},
+ {"==", nil},
+ {">", nil},
+ {"<", nil},
+ {"notin", nil},
+ {"!=", nil},
+ {"!", fmt.Errorf("found '%s', expected: %v", selection.DoesNotExist, strings.Join(binaryOperators, ", "))},
+ {"exists", fmt.Errorf("found '%s', expected: %v", selection.Exists, strings.Join(binaryOperators, ", "))},
+ {"(", fmt.Errorf("found '%s', expected: %v", "(", strings.Join(binaryOperators, ", "))},
+ }
+ for _, testcase := range testcases {
+ p := &Parser{l: &Lexer{s: testcase.token, pos: 0}, position: 0}
+ p.scan()
+
+ _, err := p.parseOperator()
+ if ok := reflect.DeepEqual(testcase.expectedError, err); !ok {
+ t.Errorf("\nexpect err [%v], \nactual err [%v]", testcase.expectedError, err)
+ }
+ }
+}
+
func TestRequirementConstructor(t *testing.T) {
requirementConstructorTests := []struct {
Key string