diff --git a/pkg/client-nfd/compat/node-validator/node-validator.go b/pkg/client-nfd/compat/node-validator/node-validator.go index a7c5840704..bd39f598d6 100644 --- a/pkg/client-nfd/compat/node-validator/node-validator.go +++ b/pkg/client-nfd/compat/node-validator/node-validator.go @@ -18,6 +18,7 @@ package nodevalidator import ( "context" + "fmt" "slices" "sort" @@ -62,12 +63,12 @@ func New(opts ...NodeValidatorOpts) nodeValidator { func (nv *nodeValidator) Execute(ctx context.Context) ([]*CompatibilityStatus, error) { spec, err := nv.artifactClient.FetchCompatibilitySpec(ctx) if err != nil { - return nil, err + return nil, fmt.Errorf("failed to fetch compatibility spec: %w", err) } for _, s := range nv.sources { if err := s.Discover(); err != nil { - return nil, err + return nil, fmt.Errorf("error during discovery of source %s: %w", s.Name(), err) } } features := source.GetAllFeatures() @@ -84,7 +85,7 @@ func (nv *nodeValidator) Execute(ctx context.Context) ([]*CompatibilityStatus, e if err != nil { return nil, err } - compat.Rules = append(compat.Rules, evaluateRuleStatus(&r, ruleOut.MatchStatus)) + compat.Rules = append(compat.Rules, nv.evaluateRuleStatus(&r, ruleOut.MatchStatus)) // Add the 'rule.matched' feature for backreference functionality features.InsertAttributeFeatures(nfdv1alpha1.RuleBackrefDomain, nfdv1alpha1.RuleBackrefFeature, ruleOut.Labels) @@ -96,91 +97,100 @@ func (nv *nodeValidator) Execute(ctx context.Context) ([]*CompatibilityStatus, e return compats, nil } -func evaluateRuleStatus(rule *nfdv1alpha1.Rule, matchStatus *nodefeaturerule.MatchStatus) ProcessedRuleStatus { - var matchedFeatureTerms nfdv1alpha1.FeatureMatcher +func (nv *nodeValidator) evaluateRuleStatus(rule *nfdv1alpha1.Rule, matchStatus *nodefeaturerule.MatchStatus) ProcessedRuleStatus { out := ProcessedRuleStatus{Name: rule.Name, IsMatch: matchStatus.IsMatch} - evaluateFeatureMatcher := func(featureMatcher, matchedFeatureTerms nfdv1alpha1.FeatureMatcher) []MatchedExpression { - out := []MatchedExpression{} - for _, term := range featureMatcher { - if term.MatchExpressions != nil { - for name, exp := range *term.MatchExpressions { - isMatch := false - - // Check if the expression matches - for _, processedTerm := range matchedFeatureTerms { - if term.Feature != processedTerm.Feature || processedTerm.MatchExpressions == nil { - continue - } - pexp, ok := (*processedTerm.MatchExpressions)[name] - if isMatch = ok && exp.Op == pexp.Op && slices.Equal(exp.Value, pexp.Value); isMatch { - break - } - } - - out = append(out, MatchedExpression{ - Feature: term.Feature, - Name: name, - Expression: exp, - MatcherType: MatchExpressionType, - IsMatch: isMatch, - }) - } - } + matchedFeatureTerms := nfdv1alpha1.FeatureMatcher{} + if m := matchStatus.MatchFeatureStatus; m != nil { + matchedFeatureTerms = m.MatchedFeaturesTerms + } + out.MatchedExpressions = nv.matchFeatureExpressions(rule.MatchFeatures, matchedFeatureTerms) - if term.MatchName != nil { - isMatch := false - for _, processedTerm := range matchStatus.MatchedFeaturesTerms { - if term.Feature != processedTerm.Feature || processedTerm.MatchName == nil { - continue - } - isMatch = term.MatchName.Op == processedTerm.MatchName.Op && slices.Equal(term.MatchName.Value, processedTerm.MatchName.Value) - if isMatch { - break - } - } - out = append(out, MatchedExpression{ - Feature: term.Feature, - Name: "", - Expression: term.MatchName, - MatcherType: MatchNameType, - IsMatch: isMatch, - }) - } + for i, matchAnyElem := range rule.MatchAny { + matchedFeatureTermsAny := nfdv1alpha1.FeatureMatcher{} + if t := matchStatus.MatchAny[i].MatchedFeaturesTerms; t != nil { + matchedFeatureTermsAny = t } + matchedExpressions := nv.matchFeatureExpressions(matchAnyElem.MatchFeatures, matchedFeatureTermsAny) + out.MatchedAny = append(out.MatchedAny, MatchAnyElem{MatchedExpressions: matchedExpressions}) + } - // For reproducible output sort by name, feature, expression. - sort.Slice(out, func(i, j int) bool { - if out[i].Feature != out[j].Feature { - return out[i].Feature < out[j].Feature - } - if out[i].Name != out[j].Name { - return out[i].Name < out[j].Name - } - return out[i].Expression.String() < out[j].Expression.String() - }) + return out +} - return out - } +func (nv *nodeValidator) matchFeatureExpressions(featureMatcher, matchedFeatureTerms nfdv1alpha1.FeatureMatcher) []MatchedExpression { + var out []MatchedExpression - if matchFeatures := rule.MatchFeatures; matchFeatures != nil { - if matchStatus.MatchFeatureStatus != nil { - matchedFeatureTerms = matchStatus.MatchFeatureStatus.MatchedFeaturesTerms + for _, term := range featureMatcher { + if term.MatchExpressions != nil { + out = append(out, nv.matchExpressions(term, matchedFeatureTerms)...) + } + if term.MatchName != nil { + out = append(out, nv.matchName(term, matchedFeatureTerms)) } - out.MatchedExpressions = evaluateFeatureMatcher(matchFeatures, matchedFeatureTerms) } - for i, matchAnyElem := range rule.MatchAny { - if matchStatus.MatchAny[i].MatchedFeaturesTerms != nil { - matchedFeatureTerms = matchStatus.MatchAny[i].MatchedFeaturesTerms + // For reproducible output sort by name, feature, expression. + sort.Slice(out, func(i, j int) bool { + if out[i].Feature != out[j].Feature { + return out[i].Feature < out[j].Feature } - matchedExpressions := evaluateFeatureMatcher(matchAnyElem.MatchFeatures, matchedFeatureTerms) - out.MatchedAny = append(out.MatchedAny, MatchAnyElem{MatchedExpressions: matchedExpressions}) + if out[i].Name != out[j].Name { + return out[i].Name < out[j].Name + } + return out[i].Expression.String() < out[j].Expression.String() + }) + + return out +} + +func (nodeValidator) matchExpressions(term nfdv1alpha1.FeatureMatcherTerm, matchedFeatureTerms nfdv1alpha1.FeatureMatcher) []MatchedExpression { + var out []MatchedExpression + + for name, exp := range *term.MatchExpressions { + isMatch := false + for _, processedTerm := range matchedFeatureTerms { + if term.Feature != processedTerm.Feature || processedTerm.MatchExpressions == nil { + continue + } + pexp, ok := (*processedTerm.MatchExpressions)[name] + if isMatch = ok && exp.Op == pexp.Op && slices.Equal(exp.Value, pexp.Value); isMatch { + break + } + } + + out = append(out, MatchedExpression{ + Feature: term.Feature, + Name: name, + Expression: exp, + MatcherType: MatchExpressionType, + IsMatch: isMatch, + }) } return out } +func (nodeValidator) matchName(term nfdv1alpha1.FeatureMatcherTerm, matchedFeatureTerms nfdv1alpha1.FeatureMatcher) MatchedExpression { + isMatch := false + for _, processedTerm := range matchedFeatureTerms { + if term.Feature != processedTerm.Feature || processedTerm.MatchName == nil { + continue + } + isMatch = term.MatchName.Op == processedTerm.MatchName.Op && slices.Equal(term.MatchName.Value, processedTerm.MatchName.Value) + if isMatch { + break + } + } + return MatchedExpression{ + Feature: term.Feature, + Name: "", + Expression: term.MatchName, + MatcherType: MatchNameType, + IsMatch: isMatch, + } +} + // NodeValidatorOpts applies certain options to the node validator. type NodeValidatorOpts interface { apply(*nodeValidator) diff --git a/pkg/client-nfd/compat/node-validator/node-validator_test.go b/pkg/client-nfd/compat/node-validator/node-validator_test.go index c0cbdaf5c2..13938a22d0 100644 --- a/pkg/client-nfd/compat/node-validator/node-validator_test.go +++ b/pkg/client-nfd/compat/node-validator/node-validator_test.go @@ -34,228 +34,283 @@ func init() { fs.SetConfig(fs.NewConfig()) } +func buildDefaultSpec(rules []v1alpha1.Rule) *compatv1alpha1.Spec { + return &compatv1alpha1.Spec{ + Version: compatv1alpha1.Version, + Compatibilties: []compatv1alpha1.Compatibility{ + { + Description: "Fake compatibility", + Rules: rules, + }, + }, + } +} + +func buildDefaultExpectedOutput(status []ProcessedRuleStatus) []*CompatibilityStatus { + return []*CompatibilityStatus{ + { + Description: "Fake compatibility", + Rules: status, + }, + } +} + +func assertOutput(ctx context.Context, spec *compatv1alpha1.Spec, expectedOutput []*CompatibilityStatus) { + validator := New( + WithArgs(&Args{}), + WithArtifactClient(newMock(ctx, spec)), + WithSources(map[string]source.FeatureSource{fake.Name: source.GetFeatureSource(fake.Name)}), + ) + output, err := validator.Execute(ctx) + + So(err, ShouldBeNil) + So(output, ShouldEqual, expectedOutput) +} + func TestNodeValidator(t *testing.T) { ctx := context.Background() - Convey("With a single compatibility set that contains flags, attributes and instances", t, func() { - spec := &compatv1alpha1.Spec{ - Version: compatv1alpha1.Version, - Compatibilties: []compatv1alpha1.Compatibility{ + Convey("With a single compatibility set", t, func() { + + Convey("That contains flag which results in match", func() { + spec := buildDefaultSpec([]v1alpha1.Rule{ { - Description: "Fake compatibility", - Rules: []v1alpha1.Rule{ + Name: "fake_1", + MatchFeatures: v1alpha1.FeatureMatcher{ { - Name: "fake_1", - MatchFeatures: v1alpha1.FeatureMatcher{ - { - Feature: "fake.flag", - MatchName: &v1alpha1.MatchExpression{Op: v1alpha1.MatchInRegexp, Value: v1alpha1.MatchValue{"^flag"}}, - }, + Feature: "fake.flag", + MatchName: &v1alpha1.MatchExpression{Op: v1alpha1.MatchInRegexp, Value: v1alpha1.MatchValue{"^flag"}}, + }, + }, + }, + }) + + expectedOutput := buildDefaultExpectedOutput([]ProcessedRuleStatus{ + { + Name: "fake_1", + IsMatch: true, + MatchedExpressions: []MatchedExpression{ + { + Feature: "fake.flag", + Name: "", + Expression: &v1alpha1.MatchExpression{Op: v1alpha1.MatchInRegexp, Value: v1alpha1.MatchValue{"^flag"}}, + MatcherType: MatchNameType, + IsMatch: true, + }, + }, + }, + }) + + assertOutput(ctx, spec, expectedOutput) + }) + + Convey("That contains flags and attribute which result in mismatch", func() { + spec := buildDefaultSpec([]v1alpha1.Rule{ + { + Name: "fake_2", + MatchFeatures: v1alpha1.FeatureMatcher{ + { + Feature: "fake.flag", + MatchExpressions: &v1alpha1.MatchExpressionSet{ + "flag_unknown": &v1alpha1.MatchExpression{Op: v1alpha1.MatchExists}, }, }, { - Name: "fake_2", - MatchFeatures: v1alpha1.FeatureMatcher{ - { - Feature: "fake.flag", - MatchExpressions: &v1alpha1.MatchExpressionSet{ - "flag_unknown": &v1alpha1.MatchExpression{Op: v1alpha1.MatchExists}, - }, - }, - { - Feature: "fake.attribute", - MatchExpressions: &v1alpha1.MatchExpressionSet{ - "attr_1": &v1alpha1.MatchExpression{Op: v1alpha1.MatchIn, Value: v1alpha1.MatchValue{"true"}}, - }, - }, + Feature: "fake.attribute", + MatchExpressions: &v1alpha1.MatchExpressionSet{ + "attr_1": &v1alpha1.MatchExpression{Op: v1alpha1.MatchIn, Value: v1alpha1.MatchValue{"true"}}, + }, + }, + }, + }, + }) + + expectedOutput := buildDefaultExpectedOutput([]ProcessedRuleStatus{ + { + Name: "fake_2", + IsMatch: false, + MatchedExpressions: []MatchedExpression{ + { + Feature: "fake.attribute", + Name: "attr_1", + Expression: &v1alpha1.MatchExpression{Op: v1alpha1.MatchIn, Value: v1alpha1.MatchValue{"true"}}, + MatcherType: MatchExpressionType, + IsMatch: true, + }, + { + Feature: "fake.flag", + Name: "flag_unknown", + Expression: &v1alpha1.MatchExpression{Op: v1alpha1.MatchExists}, + MatcherType: MatchExpressionType, + IsMatch: false, + }, + }, + }, + }) + + assertOutput(ctx, spec, expectedOutput) + }) + + Convey("That contains instances which results in mismatch", func() { + spec := buildDefaultSpec([]v1alpha1.Rule{ + { + Name: "fake_3", + MatchFeatures: v1alpha1.FeatureMatcher{ + { + Feature: "fake.instance", + MatchExpressions: &v1alpha1.MatchExpressionSet{ + "name": &v1alpha1.MatchExpression{Op: v1alpha1.MatchIn, Value: v1alpha1.MatchValue{"instance_1"}}, + "attr_1": &v1alpha1.MatchExpression{Op: v1alpha1.MatchIn, Value: v1alpha1.MatchValue{"true"}}, }, }, { - Name: "fake_3", + Feature: "fake.instance", + MatchExpressions: &v1alpha1.MatchExpressionSet{ + "name": &v1alpha1.MatchExpression{Op: v1alpha1.MatchIn, Value: v1alpha1.MatchValue{"instance_2"}}, + "attr_1": &v1alpha1.MatchExpression{Op: v1alpha1.MatchIn, Value: v1alpha1.MatchValue{"false"}}, + }, + }, + }, + }, + }) + + expectedOutput := buildDefaultExpectedOutput([]ProcessedRuleStatus{ + { + Name: "fake_3", + IsMatch: false, + MatchedExpressions: []MatchedExpression{ + { + Feature: "fake.instance", + Name: "attr_1", + Expression: &v1alpha1.MatchExpression{Op: v1alpha1.MatchIn, Value: v1alpha1.MatchValue{"false"}}, + MatcherType: MatchExpressionType, + IsMatch: false, + }, + { + Feature: "fake.instance", + Name: "attr_1", + Expression: &v1alpha1.MatchExpression{Op: v1alpha1.MatchIn, Value: v1alpha1.MatchValue{"true"}}, + MatcherType: MatchExpressionType, + IsMatch: true, + }, + { + Feature: "fake.instance", + Name: "name", + Expression: &v1alpha1.MatchExpression{Op: v1alpha1.MatchIn, Value: v1alpha1.MatchValue{"instance_1"}}, + MatcherType: MatchExpressionType, + IsMatch: true, + }, + { + Feature: "fake.instance", + Name: "name", + Expression: &v1alpha1.MatchExpression{Op: v1alpha1.MatchIn, Value: v1alpha1.MatchValue{"instance_2"}}, + MatcherType: MatchExpressionType, + IsMatch: true, + }, + }, + }, + }) + + assertOutput(ctx, spec, expectedOutput) + }) + + Convey("That contains instances which results in match", func() { + spec := buildDefaultSpec([]v1alpha1.Rule{ + { + Name: "fake_4", + MatchAny: []v1alpha1.MatchAnyElem{ + { MatchFeatures: v1alpha1.FeatureMatcher{ { Feature: "fake.instance", MatchExpressions: &v1alpha1.MatchExpressionSet{ - "name": &v1alpha1.MatchExpression{Op: v1alpha1.MatchIn, Value: v1alpha1.MatchValue{"instance_1"}}, - "attr_1": &v1alpha1.MatchExpression{Op: v1alpha1.MatchIn, Value: v1alpha1.MatchValue{"true"}}, + "name": &v1alpha1.MatchExpression{Op: v1alpha1.MatchIn, Value: v1alpha1.MatchValue{"instance_1"}}, }, }, + }, + }, + { + MatchFeatures: v1alpha1.FeatureMatcher{ { Feature: "fake.instance", MatchExpressions: &v1alpha1.MatchExpressionSet{ - "name": &v1alpha1.MatchExpression{Op: v1alpha1.MatchIn, Value: v1alpha1.MatchValue{"instance_2"}}, - "attr_1": &v1alpha1.MatchExpression{Op: v1alpha1.MatchIn, Value: v1alpha1.MatchValue{"false"}}, + "name": &v1alpha1.MatchExpression{Op: v1alpha1.MatchIn, Value: v1alpha1.MatchValue{"instance_unknown"}}, }, }, }, }, + }, + }, + }) + + expectedOutput := buildDefaultExpectedOutput([]ProcessedRuleStatus{ + { + Name: "fake_4", + IsMatch: true, + MatchedAny: []MatchAnyElem{ { - Name: "fake_4", - MatchAny: []v1alpha1.MatchAnyElem{ + MatchedExpressions: []MatchedExpression{ { - MatchFeatures: v1alpha1.FeatureMatcher{ - { - Feature: "fake.instance", - MatchExpressions: &v1alpha1.MatchExpressionSet{ - "name": &v1alpha1.MatchExpression{Op: v1alpha1.MatchIn, Value: v1alpha1.MatchValue{"instance_1"}}, - }, - }, - }, - }, - { - MatchFeatures: v1alpha1.FeatureMatcher{ - { - Feature: "fake.instance", - MatchExpressions: &v1alpha1.MatchExpressionSet{ - "name": &v1alpha1.MatchExpression{Op: v1alpha1.MatchIn, Value: v1alpha1.MatchValue{"instance_unknown"}}, - }, - }, - }, + Feature: "fake.instance", + Name: "name", + Expression: &v1alpha1.MatchExpression{Op: v1alpha1.MatchIn, Value: v1alpha1.MatchValue{"instance_1"}}, + MatcherType: MatchExpressionType, + IsMatch: true, }, }, }, { - Name: "fake_5", - MatchFeatures: v1alpha1.FeatureMatcher{ + MatchedExpressions: []MatchedExpression{ { - Feature: "unknown.unknown", - MatchExpressions: &v1alpha1.MatchExpressionSet{ - "name": &v1alpha1.MatchExpression{Op: v1alpha1.MatchIn, Value: v1alpha1.MatchValue{"instance_1"}}, - }, + Feature: "fake.instance", + Name: "name", + Expression: &v1alpha1.MatchExpression{Op: v1alpha1.MatchIn, Value: v1alpha1.MatchValue{"instance_unknown"}}, + MatcherType: MatchExpressionType, + IsMatch: false, }, }, }, }, }, - }, - } + }) - // The output contains expressions in alphabetical order over the feature, name and expression string. - expectedOutput := []*CompatibilityStatus{ - { - Description: "Fake compatibility", - Rules: []ProcessedRuleStatus{ - { - Name: "fake_1", - IsMatch: true, - MatchedExpressions: []MatchedExpression{ - { - Feature: "fake.flag", - Name: "", - Expression: &v1alpha1.MatchExpression{Op: v1alpha1.MatchInRegexp, Value: v1alpha1.MatchValue{"^flag"}}, - MatcherType: MatchNameType, - IsMatch: true, - }, - }, - }, - { - Name: "fake_2", - IsMatch: false, - MatchedExpressions: []MatchedExpression{ - { - Feature: "fake.attribute", - Name: "attr_1", - Expression: &v1alpha1.MatchExpression{Op: v1alpha1.MatchIn, Value: v1alpha1.MatchValue{"true"}}, - MatcherType: MatchExpressionType, - IsMatch: true, - }, - { - Feature: "fake.flag", - Name: "flag_unknown", - Expression: &v1alpha1.MatchExpression{Op: v1alpha1.MatchExists}, - MatcherType: MatchExpressionType, - IsMatch: false, - }, - }, - }, - { - Name: "fake_3", - IsMatch: false, - MatchedExpressions: []MatchedExpression{ - { - Feature: "fake.instance", - Name: "attr_1", - Expression: &v1alpha1.MatchExpression{Op: v1alpha1.MatchIn, Value: v1alpha1.MatchValue{"false"}}, - MatcherType: MatchExpressionType, - IsMatch: false, - }, - { - Feature: "fake.instance", - Name: "attr_1", - Expression: &v1alpha1.MatchExpression{Op: v1alpha1.MatchIn, Value: v1alpha1.MatchValue{"true"}}, - MatcherType: MatchExpressionType, - IsMatch: true, - }, - { - Feature: "fake.instance", - Name: "name", - Expression: &v1alpha1.MatchExpression{Op: v1alpha1.MatchIn, Value: v1alpha1.MatchValue{"instance_1"}}, - MatcherType: MatchExpressionType, - IsMatch: true, - }, - { - Feature: "fake.instance", - Name: "name", - Expression: &v1alpha1.MatchExpression{Op: v1alpha1.MatchIn, Value: v1alpha1.MatchValue{"instance_2"}}, - MatcherType: MatchExpressionType, - IsMatch: true, - }, - }, - }, - { - Name: "fake_4", - IsMatch: true, - MatchedAny: []MatchAnyElem{ - { - MatchedExpressions: []MatchedExpression{ - { - Feature: "fake.instance", - Name: "name", - Expression: &v1alpha1.MatchExpression{Op: v1alpha1.MatchIn, Value: v1alpha1.MatchValue{"instance_1"}}, - MatcherType: MatchExpressionType, - IsMatch: true, - }, - }, - }, - { - MatchedExpressions: []MatchedExpression{ - { - Feature: "fake.instance", - Name: "name", - Expression: &v1alpha1.MatchExpression{Op: v1alpha1.MatchIn, Value: v1alpha1.MatchValue{"instance_unknown"}}, - MatcherType: MatchExpressionType, - IsMatch: false, - }, - }, + assertOutput(ctx, spec, expectedOutput) + }) + + Convey("That contains spec with zero matches which results in mismatch", func() { + spec := buildDefaultSpec([]v1alpha1.Rule{ + { + Name: "fake_5", + MatchFeatures: v1alpha1.FeatureMatcher{ + { + Feature: "unknown.unknown", + MatchExpressions: &v1alpha1.MatchExpressionSet{ + "name": &v1alpha1.MatchExpression{Op: v1alpha1.MatchIn, Value: v1alpha1.MatchValue{"instance_1"}}, }, }, }, - { - Name: "fake_5", - IsMatch: false, - MatchedExpressions: []MatchedExpression{ - { - Feature: "unknown.unknown", - Name: "name", - Expression: &v1alpha1.MatchExpression{Op: v1alpha1.MatchIn, Value: v1alpha1.MatchValue{"instance_1"}}, - MatcherType: MatchExpressionType, - IsMatch: false, - }, + }, + }) + + expectedOutput := buildDefaultExpectedOutput([]ProcessedRuleStatus{ + { + Name: "fake_5", + IsMatch: false, + MatchedExpressions: []MatchedExpression{ + { + Feature: "unknown.unknown", + Name: "name", + Expression: &v1alpha1.MatchExpression{Op: v1alpha1.MatchIn, Value: v1alpha1.MatchValue{"instance_1"}}, + MatcherType: MatchExpressionType, + IsMatch: false, }, }, }, - }, - } + }) - validator := New( - WithArgs(&Args{}), - WithArtifactClient(newMock(ctx, spec)), - WithSources(map[string]source.FeatureSource{fake.Name: source.GetFeatureSource(fake.Name)}), - ) - output, err := validator.Execute(ctx) + assertOutput(ctx, spec, expectedOutput) + }) - So(err, ShouldBeNil) - So(output, ShouldEqual, expectedOutput) }) Convey("With multiple compatibility sets", t, func() {