Refactoring of image compatibility node validator.

Additionally, testcases have been separated into distinct functions for better readability. Signed-off-by: Marcin Franczyk <[email protected]>
kubernetes-sigs · Jan 23, 2025 · c402435 · c402435
1 parent ab1bfd1
commit c402435
Show file tree

Hide file tree

Showing 2 changed files with 311 additions and 246 deletions.
diff --git 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()
@@ -97,90 +98,99 @@ func (nv *nodeValidator) Execute(ctx context.Context) ([]*CompatibilityStatus, e
 }
 
 func evaluateRuleStatus(rule *nfdv1alpha1.Rule, matchStatus *nodefeaturerule.MatchStatus) ProcessedRuleStatus {
-	var matchedFeatureTerms nfdv1alpha1.FeatureMatcher
 	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 matchStatus.MatchFeatureStatus != nil {
+		matchedFeatureTerms = matchStatus.MatchFeatureStatus.MatchedFeaturesTerms
+	}
+	out.MatchedExpressions = 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 matchStatus.MatchAny[i].MatchedFeaturesTerms != nil {
+			matchedFeatureTermsAny = matchStatus.MatchAny[i].MatchedFeaturesTerms
 		}
+		matchedExpressions := 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 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, matchExpressions(term, matchedFeatureTerms)...)
+		}
+		if term.MatchName != nil {
+			out = append(out, 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 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 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)