From 8a4d3161cf8be621df8cd4ae405de00709f36108 Mon Sep 17 00:00:00 2001
From: Markus Lehtonen <markus.lehtonen@intel.com>
Date: Wed, 24 Nov 2021 12:51:45 +0200
Subject: [PATCH] pkg/apis/nfd: stricter format checking for template labels

Require that the expanded LabelsTemplate has values. That is, the
(expanded) template must consist of key=value pairs separated by
newlines. No default value will be assigned and we now return an error
if a (non-empty) line not conforming with the key=value format is
encountered.

Commit c8d73666d described that the value defaults to "true" if not
specified. That was not the case and we defaulted to an empty string,
instead.

An example:

  - name: "my rule"
    labelsTemplate: |
      my.label.1=foo
      my.label.2=

Would create these labels:

  "my.label.1": "foo"
  "my.label.2": ""

Further, the following:

  - name: "my failing rule"
    labelsTemplate: |
      my.label.3

will cause an error in the rule processing.
---
 pkg/apis/nfd/v1alpha1/rule.go      |  6 +++---
 pkg/apis/nfd/v1alpha1/rule_test.go | 32 ++++++++++++++++++++++++++++++
 2 files changed, 35 insertions(+), 3 deletions(-)

diff --git a/pkg/apis/nfd/v1alpha1/rule.go b/pkg/apis/nfd/v1alpha1/rule.go
index 40aad3714..704b3d01d 100644
--- a/pkg/apis/nfd/v1alpha1/rule.go
+++ b/pkg/apis/nfd/v1alpha1/rule.go
@@ -85,14 +85,14 @@ func (r *Rule) executeLabelsTemplate(in matchedFeatures, out map[string]string)
 	if r.labelsTemplate == nil {
 		t, err := newTemplateHelper(r.LabelsTemplate)
 		if err != nil {
-			return err
+			return fmt.Errorf("failed to parse LabelsTemplate: %w", err)
 		}
 		r.labelsTemplate = t
 	}
 
 	labels, err := r.labelsTemplate.expandMap(in)
 	if err != nil {
-		return err
+		return fmt.Errorf("failed to expand LabelsTemplate: %w", err)
 	}
 	for k, v := range labels {
 		out[k] = v
@@ -212,7 +212,7 @@ func (h *templateHelper) expandMap(data interface{}) (map[string]string, error)
 		if trimmed := strings.TrimSpace(item); trimmed != "" {
 			split := strings.SplitN(trimmed, "=", 2)
 			if len(split) == 1 {
-				out[split[0]] = ""
+				return nil, fmt.Errorf("missing value in expanded template line %q, (format must be '<key>=<value>')", trimmed)
 			} else {
 				out[split[0]] = split[1]
 			}
diff --git a/pkg/apis/nfd/v1alpha1/rule_test.go b/pkg/apis/nfd/v1alpha1/rule_test.go
index 6ec28673d..0cadd0676 100644
--- a/pkg/apis/nfd/v1alpha1/rule_test.go
+++ b/pkg/apis/nfd/v1alpha1/rule_test.go
@@ -244,6 +244,8 @@ func TestTemplating(t *testing.T) {
 	r1 := Rule{
 		Labels: map[string]string{"label-1": "label-val-1"},
 		LabelsTemplate: `
+label-1=will-be-overridden
+label-2=
 {{range .domain_1.kf_1}}kf-{{.Name}}=present
 {{end}}
 {{range .domain_1.vf_1}}vf-{{.Name}}=vf-{{.Value}}
@@ -280,6 +282,7 @@ func TestTemplating(t *testing.T) {
 
 	expectedLabels := map[string]string{
 		"label-1": "label-val-1",
+		"label-2": "",
 		// From kf_1 template
 		"kf-key-a": "present",
 		"kf-key-c": "present",
@@ -295,4 +298,33 @@ func TestTemplating(t *testing.T) {
 	m, err := r1.Execute(f)
 	assert.Nilf(t, err, "unexpected error: %v", err)
 	assert.Equal(t, expectedLabels, m, "instances should have matched")
+
+	//
+	// Test error cases
+	//
+	r2 := Rule{
+		MatchFeatures: FeatureMatcher{
+			// We need at least one matcher to match to execute the template.
+			// Use a simple empty matchexpression set to match anything.
+			FeatureMatcherTerm{
+				Feature:          "domain_1.kf_1",
+				MatchExpressions: MatchExpressionSet{},
+			},
+		},
+	}
+
+	r2.LabelsTemplate = "foo=bar"
+	m, err = r2.Execute(f)
+	assert.Nil(t, err)
+	assert.Equal(t, map[string]string{"foo": "bar"}, m, "instances should have matched")
+
+	r2.labelsTemplate = nil
+	r2.LabelsTemplate = "foo"
+	_, err = r2.Execute(f)
+	assert.Error(t, err)
+
+	r2.labelsTemplate = nil
+	r2.LabelsTemplate = "{{"
+	_, err = r2.Execute(f)
+	assert.Error(t, err)
 }