apis/nfd: fix multiple matcher terms targeting the same feature

Fix NodeFeatureRule templating in cases where multiple matchFeatures
terms are targeting the same feature. Previously, only matched feature
elements from the last matcher terms were used as the input to the
template. However, the input should contain all matched elements from
all matcher terms.

For example, consider the example rule snippet below:

  ...
  labelsTemplate: |
    {{ range .pci.device }}vendor.io/pci-device.{{ .class }}-{{ .device }}=exists
    {{ end }}
  matchFeatures:
    - feature: pci.device
      matchExpressions:
        class: {op: InRegexp, value: ["^03"]}
        vendor: {op: In, value: ["1234"]}
    - feature: pci.device
      matchExpressions:
        class: {op: InRegexp, value: ["^12"]}

This rule matches if both a pci device of class 03 from vendor 1234
exists and a pci device of class 12 (from any vendor) exists.
Previously, the template would only generate labels from the devices in
class 12 (as that's the last term). With this patch the template creates
device labels from devices in both classes 03 and 12.

pkg/apis/nfd/v1alpha1/expression.go

@@ -288,16 +288,15 @@ func (m *MatchExpressionSet) MatchKeys(keys map[string]Nil) (bool, error) {
 	return matched, err
 }
 
-// MatchedKey holds one matched key.
-type MatchedKey struct {
-	Name string
-}
+// MatchedElement holds one matched Instance.
+// +k8s:deepcopy-gen=false
+type MatchedElement map[string]string
 
 // MatchGetKeys evaluates the MatchExpressionSet against a set of keys and
 // returns all matched keys or nil if no match was found. Note that an empty
 // MatchExpressionSet returns a match with an empty slice of matched features.
-func (m *MatchExpressionSet) MatchGetKeys(keys map[string]Nil) (bool, []MatchedKey, error) {
-	ret := make([]MatchedKey, 0, len(*m))
+func (m *MatchExpressionSet) MatchGetKeys(keys map[string]Nil) (bool, []MatchedElement, error) {
+	ret := make([]MatchedElement, 0, len(*m))
 
 	for n, e := range *m {
 		match, err := e.MatchKeys(n, keys)
@@ -307,10 +306,8 @@ func (m *MatchExpressionSet) MatchGetKeys(keys map[string]Nil) (bool, []MatchedK
 		if !match {
 			return false, nil, nil
 		}
-		ret = append(ret, MatchedKey{Name: n})
+		ret = append(ret, MatchedElement{"Name": n})
 	}
-	// Sort for reproducible output
-	sort.Slice(ret, func(i, j int) bool { return ret[i].Name < ret[j].Name })
 	return true, ret, nil
 }
 
@@ -320,17 +317,11 @@ func (m *MatchExpressionSet) MatchValues(values map[string]string) (bool, error)
 	return matched, err
 }
 
-// MatchedValue holds one matched key-value pair.
-type MatchedValue struct {
-	Name  string
-	Value string
-}
-
 // MatchGetValues evaluates the MatchExpressionSet against a set of key-value
 // pairs and returns all matched key-value pairs. Note that an empty
 // MatchExpressionSet returns a match with an empty slice of matched features.
-func (m *MatchExpressionSet) MatchGetValues(values map[string]string) (bool, []MatchedValue, error) {
-	ret := make([]MatchedValue, 0, len(*m))
+func (m *MatchExpressionSet) MatchGetValues(values map[string]string) (bool, []MatchedElement, error) {
+	ret := make([]MatchedElement, 0, len(*m))
 
 	for n, e := range *m {
 		match, err := e.MatchValues(n, values)
@@ -340,10 +331,8 @@ func (m *MatchExpressionSet) MatchGetValues(values map[string]string) (bool, []M
 		if !match {
 			return false, nil, nil
 		}
-		ret = append(ret, MatchedValue{Name: n, Value: values[n]})
+		ret = append(ret, MatchedElement{"Name": n, "Value": values[n]})
 	}
-	// Sort for reproducible output
-	sort.Slice(ret, func(i, j int) bool { return ret[i].Name < ret[j].Name })
 	return true, ret, nil
 }
 
@@ -355,15 +344,12 @@ func (m *MatchExpressionSet) MatchInstances(instances []InstanceFeature) (bool,
 	return len(v) > 0, err
 }
 
-// MatchedInstance holds one matched Instance.
-type MatchedInstance map[string]string
-
 // MatchGetInstances evaluates the MatchExpressionSet against a set of instance
 // features, each of which is an individual set of key-value pairs
 // (attributes). A slice containing all matching instances is returned. An
 // empty (non-nil) slice is returned if no matching instances were found.
-func (m *MatchExpressionSet) MatchGetInstances(instances []InstanceFeature) ([]MatchedInstance, error) {
-	ret := []MatchedInstance{}
+func (m *MatchExpressionSet) MatchGetInstances(instances []InstanceFeature) ([]MatchedElement, error) {
+	ret := []MatchedElement{}
 
 	for _, i := range instances {
 		if match, err := m.MatchValues(i.Attributes); err != nil {

pkg/apis/nfd/v1alpha1/expression_test.go

@@ -290,8 +290,8 @@ func TestMatchValues(t *testing.T) {
 
 func TestMESMatchKeys(t *testing.T) {
 	type I = map[string]api.Nil
-	type MK = api.MatchedKey
-	type O = []MK
+	type ME = api.MatchedElement
+	type O = []ME
 	type TC struct {
 		mes    string
 		input  I
@@ -312,7 +312,7 @@ foo: { op: DoesNotExist }
 bar: { op: Exists }
 `,
 			input:  I{"bar": {}, "baz": {}, "buzz": {}},
-			output: O{MK{Name: "bar"}, MK{Name: "foo"}},
+			output: O{ME{"Name": "bar"}, ME{"Name": "foo"}},
 			result: assert.Truef, err: assert.Nilf},
 
 		{mes: `
@@ -351,8 +351,8 @@ bar: { op: Exists }
 
 func TestMESMatchValues(t *testing.T) {
 	type I = map[string]string
-	type MV = api.MatchedValue
-	type O = []MV
+	type ME = api.MatchedElement
+	type O = []ME
 	type TC struct {
 		mes    string
 		input  I
@@ -382,7 +382,7 @@ bar: { op: In, value: ["val", "wal"] }
 baz: { op: Gt, value: ["10"] }
 `,
 			input:  I{"foo": "1", "bar": "val", "baz": "123", "buzz": "light"},
-			output: O{MV{Name: "bar", Value: "val"}, MV{Name: "baz", Value: "123"}, MV{Name: "foo", Value: "1"}},
+			output: O{ME{"Name": "bar", "Value": "val"}, ME{"Name": "baz", "Value": "123"}, ME{"Name": "foo", "Value": "1"}},
 			result: assert.Truef, err: assert.Nilf},
 
 		{mes: `
@@ -413,8 +413,8 @@ baz: { op: Gt, value: ["10"] }
 
 func TestMESMatchInstances(t *testing.T) {
 	type I = api.InstanceFeature
-	type MI = api.MatchedInstance
-	type O = []MI
+	type ME = api.MatchedElement
+	type O = []ME
 	type A = map[string]string
 	type TC struct {
 		mes    string

pkg/apis/nfd/v1alpha1/rule.go

@@ -155,7 +155,7 @@ func (r *Rule) executeVarsTemplate(in matchedFeatures, out map[string]string) er
 
 type matchedFeatures map[string]domainMatchedFeatures
 
-type domainMatchedFeatures map[string]interface{}
+type domainMatchedFeatures map[string][]MatchedElement
 
 func (e *MatchAnyElem) match(features *Features) (bool, matchedFeatures, error) {
 	return e.MatchFeatures.match(features)
@@ -175,30 +175,26 @@ func (m *FeatureMatcher) match(features *Features) (bool, matchedFeatures, error
 			nameSplit = []string{featureName, ""}
 		}
 
-		if _, ok := matches[nameSplit[0]]; !ok {
-			matches[nameSplit[0]] = make(domainMatchedFeatures)
+		dom := nameSplit[0]
+		nam := nameSplit[1]
+		if _, ok := matches[dom]; !ok {
+			matches[dom] = make(domainMatchedFeatures)
 		}
 
 		var isMatch bool
+		var matchedElems []MatchedElement
 		var err error
 		if f, ok := features.Flags[featureName]; ok {
-			m, v, e := term.MatchExpressions.MatchGetKeys(f.Elements)
-			isMatch = m
-			err = e
-			matches[nameSplit[0]][nameSplit[1]] = v
+			isMatch, matchedElems, err = term.MatchExpressions.MatchGetKeys(f.Elements)
 		} else if f, ok := features.Attributes[featureName]; ok {
-			m, v, e := term.MatchExpressions.MatchGetValues(f.Elements)
-			isMatch = m
-			err = e
-			matches[nameSplit[0]][nameSplit[1]] = v
+			isMatch, matchedElems, err = term.MatchExpressions.MatchGetValues(f.Elements)
 		} else if f, ok := features.Instances[featureName]; ok {
-			v, e := term.MatchExpressions.MatchGetInstances(f.Elements)
-			isMatch = len(v) > 0
-			err = e
-			matches[nameSplit[0]][nameSplit[1]] = v
+			matchedElems, err = term.MatchExpressions.MatchGetInstances(f.Elements)
+			isMatch = len(matchedElems) > 0
 		} else {
 			return false, nil, fmt.Errorf("feature %q not available", featureName)
 		}
+		matches[dom][nam] = append(matches[dom][nam], matchedElems...)
 
 		if err != nil {
 			return false, nil, err

pkg/apis/nfd/v1alpha1/rule_test.go

@@ -246,6 +246,13 @@ func TestTemplating(t *testing.T) {
 							"attr-2": "val-200",
 						},
 					},
+					{
+						Attributes: map[string]string{
+							"attr-1": "1000",
+							"attr-2": "val-2000",
+							"attr-3": "3000",
+						},
+					},
 				},
 			},
 		},
@@ -290,6 +297,14 @@ var-2=
 					"attr-1": MustCreateMatchExpression(MatchLt, "100"),
 				},
 			},
+			FeatureMatcherTerm{
+				Feature: "domain_1.if_1",
+				MatchExpressions: MatchExpressionSet{
+					"attr-1": MustCreateMatchExpression(MatchExists),
+					"attr-2": MustCreateMatchExpression(MatchExists),
+					"attr-3": MustCreateMatchExpression(MatchExists),
+				},
+			},
 		},
 	}
 
@@ -309,8 +324,9 @@ var-2=
 		"vf-key-1": "vf-val-1",
 		"vf-bar":   "vf-",
 		// From if_1 template
-		"if-1_val-2":   "present",
-		"if-10_val-20": "present",
+		"if-1_val-2":       "present",
+		"if-10_val-20":     "present",
+		"if-1000_val-2000": "present",
 	}
 	expectedVars := map[string]string{
 		"var-1": "var-val-1",