Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Nov 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 11 additions & 25 deletions pkg/apis/nfd/v1alpha1/expression.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
}

Expand All @@ -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)
Expand All @@ -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
}

Expand All @@ -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 {
Expand Down
16 changes: 8 additions & 8 deletions pkg/apis/nfd/v1alpha1/expression_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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: `
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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: `
Expand Down Expand Up @@ -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
Expand Down
26 changes: 11 additions & 15 deletions pkg/apis/nfd/v1alpha1/rule.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
Expand Down
20 changes: 18 additions & 2 deletions pkg/apis/nfd/v1alpha1/rule_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
},
},
},
},
},
Expand Down Expand Up @@ -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),
},
},
},
}

Expand All @@ -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",
Expand Down
51 changes: 0 additions & 51 deletions pkg/apis/nfd/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.