apis/nfd: no error on ops that never match

Return false (i.e. "did not match") but no error when evaluating a match
expression against a "flag" type feature (which don't have any
associated value, just the name) if a MatchOp that never matches is
used.

This is preparation for supporting multi-type features, i.e. one
feature, like "cpu.cpuid", having e.g. "flag" and "attribute" type
features.

Author: marquiz

pkg/apis/nfd/nodefeaturerule/expression-api_test.go

@@ -43,34 +43,46 @@ func TestMatchKeys(t *testing.T) {
 	}
 
 	tcs := []TC{
-		{output: O{}, result: assert.True, err: assert.Nil},
+		{name: "1", output: O{}, result: assert.True, err: assert.Nil},
 
-		{input: I{}, output: O{}, result: assert.True, err: assert.Nil},
+		{name: "2", input: I{}, output: O{}, result: assert.True, err: assert.Nil},
 
-		{input: I{"foo": {}}, output: O{}, result: assert.True, err: assert.Nil},
+		{name: "3", input: I{"foo": {}}, output: O{}, result: assert.True, err: assert.Nil},
 
-		{mes: `
+		{name: "4",
+			mes: `
 foo: { op: DoesNotExist }
 bar: { op: Exists }
 `,
 			input:  I{"bar": {}, "baz": {}, "buzz": {}},
 			output: O{{"Name": "bar"}, {"Name": "foo"}},
 			result: assert.True, err: assert.Nil},
 
-		{mes: `
+		{name: "5",
+			mes: `
 foo: { op: DoesNotExist }
 bar: { op: Exists }
 `,
 			input:  I{"foo": {}, "bar": {}, "baz": {}},
 			output: nil,
 			result: assert.False, err: assert.Nil},
 
-		{mes: `
+		{name: "6",
+			mes: `
 foo: { op: In, value: ["bar"] }
 bar: { op: Exists }
 `,
 			input:  I{"bar": {}, "baz": {}},
 			output: nil,
+			result: assert.False, err: assert.Nil},
+
+		{name: "7",
+			mes: `
+foo: { op: Exists, value: ["bar"] }
+bar: { op: Exists }
+`,
+			input:  I{"bar": {}},
+			output: nil,
 			result: assert.False, err: assert.NotNil},
 	}
 

pkg/apis/nfd/nodefeaturerule/expression.go

@@ -67,7 +67,7 @@ func evaluateMatchExpression(m *nfdv1alpha1.MatchExpression, valid bool, value i
 		return !valid, nil
 	}
 
-	if valid {
+	if valid && value != nil {
 		value := fmt.Sprintf("%v", value)
 		switch m.Op {
 		case nfdv1alpha1.MatchIn:
@@ -161,18 +161,10 @@ func evaluateMatchExpression(m *nfdv1alpha1.MatchExpression, valid bool, value i
 
 // evaluateMatchExpressionKeys evaluates the MatchExpression against a set of keys.
 func evaluateMatchExpressionKeys(m *nfdv1alpha1.MatchExpression, name string, keys map[string]nfdv1alpha1.Nil) (bool, error) {
-	matched := false
-
 	_, ok := keys[name]
-	switch m.Op {
-	case nfdv1alpha1.MatchAny:
-		matched = true
-	case nfdv1alpha1.MatchExists:
-		matched = ok
-	case nfdv1alpha1.MatchDoesNotExist:
-		matched = !ok
-	default:
-		return false, fmt.Errorf("invalid Op %q when matching keys", m.Op)
+	matched, err := evaluateMatchExpression(m, ok, nil)
+	if err != nil {
+		return false, err
 	}
 
 	if klogV := klog.V(3); klogV.Enabled() {

pkg/apis/nfd/nodefeaturerule/expression_test.go

@@ -171,15 +171,15 @@ func TestEvaluateMatchExpressionKeys(t *testing.T) {
 		{name: "7", op: nfdv1alpha1.MatchDoesNotExist, key: "foo", input: I{"bar": {}}, result: assert.True, err: assert.Nil},
 		{name: "8", op: nfdv1alpha1.MatchDoesNotExist, key: "foo", input: I{"bar": {}, "foo": {}}, result: assert.False, err: assert.Nil},
 
-		// All other ops should return an error
-		{name: "9", op: nfdv1alpha1.MatchIn, values: V{"foo"}, key: "foo", result: assert.False, err: assert.NotNil},
-		{name: "10", op: nfdv1alpha1.MatchNotIn, values: V{"foo"}, key: "foo", result: assert.False, err: assert.NotNil},
-		{name: "11", op: nfdv1alpha1.MatchInRegexp, values: V{"foo"}, key: "foo", result: assert.False, err: assert.NotNil},
-		{name: "12", op: nfdv1alpha1.MatchGt, values: V{"1"}, key: "foo", result: assert.False, err: assert.NotNil},
-		{name: "13", op: nfdv1alpha1.MatchLt, values: V{"1"}, key: "foo", result: assert.False, err: assert.NotNil},
-		{name: "14", op: nfdv1alpha1.MatchGtLt, values: V{"1", "10"}, key: "foo", result: assert.False, err: assert.NotNil},
-		{name: "15", op: nfdv1alpha1.MatchIsTrue, key: "foo", result: assert.False, err: assert.NotNil},
-		{name: "16", op: nfdv1alpha1.MatchIsFalse, key: "foo", result: assert.False, err: assert.NotNil},
+		// All other ops should be nop (and return false) for "key" features
+		{name: "9", op: nfdv1alpha1.MatchIn, values: V{"foo"}, key: "foo", result: assert.False, err: assert.Nil},
+		{name: "10", op: nfdv1alpha1.MatchNotIn, values: V{"foo"}, key: "foo", result: assert.False, err: assert.Nil},
+		{name: "11", op: nfdv1alpha1.MatchInRegexp, values: V{"foo"}, key: "foo", result: assert.False, err: assert.Nil},
+		{name: "12", op: nfdv1alpha1.MatchGt, values: V{"1"}, key: "foo", result: assert.False, err: assert.Nil},
+		{name: "13", op: nfdv1alpha1.MatchLt, values: V{"1"}, key: "foo", result: assert.False, err: assert.Nil},
+		{name: "14", op: nfdv1alpha1.MatchGtLt, values: V{"1", "10"}, key: "foo", result: assert.False, err: assert.Nil},
+		{name: "15", op: nfdv1alpha1.MatchIsTrue, key: "foo", result: assert.False, err: assert.Nil},
+		{name: "16", op: nfdv1alpha1.MatchIsFalse, key: "foo", result: assert.False, err: assert.Nil},
 	}
 
 	for _, tc := range tcs {