Merge pull request #1226 from AhmedGrati/feat-support-dynamic-values-nfr-labels

feat: support dynamic values for labels in the NodeFeatureRule

docs/usage/customization-guide.md

@@ -475,6 +475,38 @@ The `.name` field is required and used as an identifier of the rule.
 
 The `.labels` is a map of the node labels to create if the rule matches.
 
+Take this rule as a referential example:
+
+```yaml
+apiVersion: nfd.k8s-sigs.io/v1alpha1
+kind: NodeFeatureRule
+metadata:
+  name: my-sample-rule-object
+spec:
+  rules:
+    - name: "my dynamic label value rule"
+      labels:
+        linux-lsm-enabled: "@kernel.config.LSM"
+        custom-label: "customlabel"
+```
+
+Label `linux-lsm-enabled` uses the `@` notation for dynamic values.
+The value of the label will be the value of the attribute `LSM`
+of the feature `kernel.config`.
+
+The `@<feature-name>.<element-name>` format can be used to inject values of
+detected features to the label. See
+[available features](#available-features) for possible values to use.
+
+This will yield into the following node label:
+
+```yaml
+  labels:
+    ...
+    feature.node.kubernetes.io/linux-lsm-enabled: apparmor
+    feature.node.kubernetes.io/custom-label: "customlabel"
+```
+
 #### Labels template
 
 The `.labelsTemplate` field specifies a text template for dynamically creating

pkg/nfd-master/nfd-master-internal_test.go

@@ -36,6 +36,7 @@ import (
 	meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	k8sclient "k8s.io/client-go/kubernetes"
 	"sigs.k8s.io/node-feature-discovery/pkg/apihelper"
+	"sigs.k8s.io/node-feature-discovery/pkg/apis/nfd/v1alpha1"
 	nfdv1alpha1 "sigs.k8s.io/node-feature-discovery/pkg/apis/nfd/v1alpha1"
 	"sigs.k8s.io/node-feature-discovery/pkg/labeler"
 	"sigs.k8s.io/node-feature-discovery/pkg/utils"
@@ -438,6 +439,34 @@ func TestSetLabels(t *testing.T) {
 	})
 }
 
+func TestFilterLabels(t *testing.T) {
+	mockHelper := &apihelper.MockAPIHelpers{}
+	mockMaster := newMockMaster(mockHelper)
+
+	Convey("When using dynamic values", t, func() {
+		labelName := "testLabel"
+		labelValue := "@test.feature.LSM"
+		features := nfdv1alpha1.Features{
+			Attributes: map[string]nfdv1alpha1.AttributeFeatureSet{
+				"test.feature": v1alpha1.AttributeFeatureSet{
+					Elements: map[string]string{
+						"LSM": "123",
+					},
+				},
+			},
+		}
+		labelValue, err := mockMaster.filterFeatureLabel(labelName, labelValue, &features)
+
+		Convey("Operation should succeed", func() {
+			So(err, ShouldBeNil)
+		})
+
+		Convey("Label value should change", func() {
+			So(labelValue, ShouldEqual, "123")
+		})
+	})
+}
+
 func TestCreatePatches(t *testing.T) {
 	Convey("When creating JSON patches", t, func() {
 		existingItems := map[string]string{"key-1": "val-1", "key-2": "val-2", "key-3": "val-3"}

pkg/nfd-master/nfd-master.go

@@ -483,13 +483,13 @@ func (m *nfdMaster) updateMasterNode() error {
 // into extended resources. This function also handles proper namespacing of
 // labels and ERs, i.e. adds the possibly missing default namespace for labels
 // arriving through the gRPC API.
-func (m *nfdMaster) filterFeatureLabels(labels Labels) (Labels, ExtendedResources) {
+func (m *nfdMaster) filterFeatureLabels(labels Labels, features *nfdv1alpha1.Features) (Labels, ExtendedResources) {
 	outLabels := Labels{}
 	for name, value := range labels {
 		// Add possibly missing default ns
 		name := addNs(name, nfdv1alpha1.FeatureLabelNs)
 
-		if err := m.filterFeatureLabel(name, value); err != nil {
+		if value, err := m.filterFeatureLabel(name, value, features); err != nil {
 			klog.ErrorS(err, "ignoring label", "labelKey", name, "labelValue", value)
 		} else {
 			outLabels[name] = value
@@ -515,10 +515,11 @@ func (m *nfdMaster) filterFeatureLabels(labels Labels) (Labels, ExtendedResource
 	return outLabels, extendedResources
 }
 
-func (m *nfdMaster) filterFeatureLabel(name, value string) error {
+func (m *nfdMaster) filterFeatureLabel(name, value string, features *nfdv1alpha1.Features) (string, error) {
+
 	//Validate label name
 	if errs := k8svalidation.IsQualifiedName(name); len(errs) > 0 {
-		return fmt.Errorf("invalid name %q: %s", name, strings.Join(errs, "; "))
+		return "", fmt.Errorf("invalid name %q: %s", name, strings.Join(errs, "; "))
 	}
 
 	// Check label namespace, filter out if ns is not whitelisted
@@ -528,22 +529,53 @@ func (m *nfdMaster) filterFeatureLabel(name, value string) error {
 		// If the namespace is denied, and not present in the extraLabelNs, label will be ignored
 		if isNamespaceDenied(ns, m.deniedNs.wildcard, m.deniedNs.normal) {
 			if _, ok := m.config.ExtraLabelNs[ns]; !ok {
-				return fmt.Errorf("namespace %q is not allowed", ns)
+				return "", fmt.Errorf("namespace %q is not allowed", ns)
 			}
 		}
 	}
 
 	// Skip if label doesn't match labelWhiteList
 	if !m.config.LabelWhiteList.Regexp.MatchString(base) {
-		return fmt.Errorf("%s (%s) does not match the whitelist (%s)", base, name, m.config.LabelWhiteList.Regexp.String())
+		return "", fmt.Errorf("%s (%s) does not match the whitelist (%s)", base, name, m.config.LabelWhiteList.Regexp.String())
+	}
+
+	var filteredLabel string
+
+	// Dynamic Value
+	if strings.HasPrefix(value, "@") {
+		dynamicValue, err := getDynamicValue(value, features)
+		if err != nil {
+			return "", err
+		}
+		filteredLabel = dynamicValue
+	} else {
+		filteredLabel = value
 	}
 
 	// Validate the label value
-	if errs := k8svalidation.IsValidLabelValue(value); len(errs) > 0 {
+	if errs := k8svalidation.IsValidLabelValue(filteredLabel); len(errs) > 0 {
-		return fmt.Errorf("invalid value %q: %s", value, strings.Join(errs, "; "))
+		return "", fmt.Errorf("invalid value %q: %s", filteredLabel, strings.Join(errs, "; "))
 	}
+	return filteredLabel, nil
+}
 
-	return nil
+func getDynamicValue(value string, features *nfdv1alpha1.Features) (string, error) {
+	// value is a string in the form of attribute.featureset.elements
+	split := strings.SplitN(value[1:], ".", 3)
+	if len(split) != 3 {
+		return "", fmt.Errorf("value %s is not in the form of '@domain.feature.element'", value)
+	}
+	featureName := split[0] + "." + split[1]
+	elementName := split[2]
+	attrFeatureSet, ok := features.Attributes[featureName]
+	if !ok {
+		return "", fmt.Errorf("feature %s not found", featureName)
+	}
+	element, ok := attrFeatureSet.Elements[elementName]
+	if !ok {
+		return "", fmt.Errorf("element %s not found on feature %s", elementName, featureName)
+	}
+	return element, nil
 }
 
 func filterTaints(taints []corev1.Taint) []corev1.Taint {
@@ -750,26 +782,15 @@ func filterExtendedResource(name, value string, features *nfdv1alpha1.Features)
 
 	// Dynamic Value
 	if strings.HasPrefix(value, "@") {
-		// value is a string in the form of attribute.featureset.elements
+		if element, err := getDynamicValue(value, features); err != nil {
-		split := strings.SplitN(value[1:], ".", 3)
+			return "", err
-		if len(split) != 3 {
+		} else {
-			return "", fmt.Errorf("value %s is not in the form of '@domain.feature.element'", value)
+			q, err := k8sQuantity.ParseQuantity(element)
+			if err != nil {
+				return "", fmt.Errorf("invalid value %s (from %s): %w", element, value, err)
+			}
+			return q.String(), nil
 		}
-		featureName := split[0] + "." + split[1]
-		elementName := split[2]
-		attrFeatureSet, ok := features.Attributes[featureName]
-		if !ok {
-			return "", fmt.Errorf("feature %s not found", featureName)
-		}
-		element, ok := attrFeatureSet.Elements[elementName]
-		if !ok {
-			return "", fmt.Errorf("element %s not found on feature %s", elementName, featureName)
-		}
-		q, err := k8sQuantity.ParseQuantity(element)
-		if err != nil {
-			return "", fmt.Errorf("invalid value %s (from %s): %w", element, value, err)
-		}
-		return q.String(), nil
 	}
 	// Static Value (Pre-Defined at the NodeFeatureRule)
 	q, err := k8sQuantity.ParseQuantity(value)
@@ -794,7 +815,7 @@ func (m *nfdMaster) refreshNodeFeatures(cli *kubernetes.Clientset, nodeName stri
 
 	// Remove labels which are intended to be extended resources via
 	// -resource-labels or their NS is not whitelisted
-	labels, extendedResources := m.filterFeatureLabels(labels)
+	labels, extendedResources := m.filterFeatureLabels(labels, features)
 
 	// Mix in CR-originated extended resources with -resource-labels
 	for k, v := range crExtendedResources {

test/e2e/data/nodefeaturerule-2.yaml

@@ -10,6 +10,7 @@ spec:
     - name: "e2e-matchany-test-1"
       labels:
         e2e-matchany-test-1: "true"
+        dynamic-label: "@rule.matched.e2e-attribute-test-1"
       vars:
         e2e-instance-test-1.not: "false"
       matchFeatures:

test/e2e/node_feature_discovery_test.go

@@ -710,6 +710,7 @@ core:
 					expectedLabels["*"][nfdv1alpha1.FeatureLabelNs+"/e2e-matchany-test-1"] = "true"
 					expectedLabels["*"][nfdv1alpha1.FeatureLabelNs+"/e2e-template-test-1-instance_1"] = "found"
 					expectedLabels["*"][nfdv1alpha1.FeatureLabelNs+"/e2e-template-test-1-instance_2"] = "found"
+					expectedLabels["*"][nfdv1alpha1.FeatureLabelNs+"/dynamic-label"] = "true"
 
 					By("Verifying node labels from NodeFeatureRules #1 and #2")
 					eventuallyNonControlPlaneNodes(ctx, f.ClientSet).Should(MatchLabels(expectedLabels, nodes, false))