Add .ObjectName template context variable

Allows leveraging the results from the `objectSelector`. ref: https://issues.redhat.com/browse/ACM-15809 Signed-off-by: Dale Haiducek <[email protected]>
open-cluster-management-io · Nov 26, 2024 · 6f66d7e · 6f66d7e
1 parent b8f19d4
commit 6f66d7e
Show file tree

Hide file tree

Showing 6 changed files with 260 additions and 12 deletions.
diff --git a/controllers/configurationpolicy_controller.go b/controllers/configurationpolicy_controller.go
@@ -88,6 +88,7 @@ var (
 	commonSprigFuncMap template.FuncMap
 
 	templateHasObjectNamespaceRegex = regexp.MustCompile(`(\.ObjectNamespace)`)
+	templateHasObjectNameRegex      = regexp.MustCompile(`(\.ObjectName)\W`)
 )
 
 func init() {
@@ -1379,19 +1380,19 @@ func (r *ConfigurationPolicyReconciler) determineDesiredObjects(
 	if len(relevantNsNames) == 0 {
 		log.Info(
 			"The object template is namespaced but no namespace is specified. Cannot process.",
-			"name", parsedMinMetadata.Metadata.Name,
+			"name", desiredName,
 			"kind", objGVK.Kind,
 		)
 
 		var space string
-		if parsedMinMetadata.Metadata.Name != "" {
+		if desiredName != "" {
 			space = " "
 		}
 
 		// namespaced but none specified, generate violation
 		msg := fmt.Sprintf("namespaced object%s%s of kind %s has no namespace specified "+
 			"from the policy namespaceSelector nor the object metadata",
-			space, parsedMinMetadata.Metadata.Name, objGVK.Kind,
+			space, desiredName, objGVK.Kind,
 		)
 
 		errEvent := &objectTmplEvalEvent{false, "K8s missing namespace", msg}
@@ -1457,9 +1458,11 @@ func (r *ConfigurationPolicyReconciler) determineDesiredObjects(
 	// Detect templates
 	hasTemplate := templates.HasTemplate(objectT.ObjectDefinition.Raw, "", true)
 	needsPerNamespaceTemplating := false
+	needsPerNameTemplating := false
 
-	// Detect .ObjectNamespace template context variables
+	// Detect .objectNamespace and .ObjectName template context variables
 	if hasTemplate {
+		needsPerNameTemplating = templateHasObjectNameRegex.Match(objectT.ObjectDefinition.Raw)
 		needsPerNamespaceTemplating = templateHasObjectNamespaceRegex.Match(objectT.ObjectDefinition.Raw)
 	}
 
@@ -1474,17 +1477,23 @@ func (r *ConfigurationPolicyReconciler) determineDesiredObjects(
 		for nameIdx, name := range names {
 			var rawDesiredObject []byte
 
-			// Process templating only on the first loop if the ObjectNamespace
-			// template variable isn't used.
-			//
 			// If object-templates-raw was used, the templates were already resolved
 			// and a resolver is not passed in for this function to use since parsing
-			// templates again is undesirable.
-			if nameIdx == 0 && tmplResolver != nil &&
-				hasTemplate && (desiredObj == nil || needsPerNamespaceTemplating) {
+			// templates again is undesirable. Process templating when the templates
+			// and resolver are present, and:
+			// - Every time if the ObjectName variable is used
+			// - Only on the first name (inner) loop if the ObjectName variable isn't
+			//   used but ObjectNamespace is
+			// - Only on the first namespace (outer) loop if the ObjectNamespace
+			//   template variable isn't used
+			if tmplResolver != nil && hasTemplate &&
+				(needsPerNameTemplating || (nameIdx == 0 && needsPerNamespaceTemplating) || desiredObj == nil) {
 				r.processedPolicyCache.Delete(plc.GetUID())
 
-				templateContext := struct{ ObjectNamespace string }{ObjectNamespace: ns}
+				templateContext := struct {
+					ObjectNamespace string
+					ObjectName      string
+				}{ObjectNamespace: ns, ObjectName: name}
 
 				resolvedTemplate, err := tmplResolver.ResolveTemplate(
 					objectT.ObjectDefinition.Raw, templateContext, resolveOptions,
@@ -1549,7 +1558,8 @@ func (r *ConfigurationPolicyReconciler) determineDesiredObjects(
 			desiredObj.SetKind(strings.TrimSpace(desiredObj.GetKind()))
 			desiredObj.SetNamespace(strings.TrimSpace(desiredObj.GetNamespace()))
 
-			if desiredObj.GetNamespace() != desiredNs && desiredObj.GetNamespace() != ns {
+			// If the namespace doesn't match the original, return an error
+			if needsPerNamespaceTemplating && desiredObj.GetNamespace() != ns {
 				errEvent := &objectTmplEvalEvent{
 					compliant: false,
 					reason:    reasonTemplateError,
@@ -1560,6 +1570,18 @@ func (r *ConfigurationPolicyReconciler) determineDesiredObjects(
 				return nil, &scopedGVR, errEvent, nil
 			}
 
+			// If the name doesn't match the original, return an error
+			if needsPerNameTemplating && desiredObj.GetName() != name {
+				errEvent := &objectTmplEvalEvent{
+					compliant: false,
+					reason:    reasonTemplateError,
+					message: "The object definition's name must match the selected name after template " +
+						"resolution",
+				}
+
+				return nil, &scopedGVR, errEvent, nil
+			}
+
 			// Populate the namespace and name if they're not empty strings
 			if name != "" {
 				desiredObj.SetName(strings.TrimSpace(name))

diff --git a/test/e2e/case13_templatization_test.go b/test/e2e/case13_templatization_test.go
@@ -5,6 +5,7 @@ package e2e
 
 import (
 	"context"
+	"fmt"
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
@@ -567,4 +568,128 @@ var _ = Describe("Test templatization", Ordered, func() {
 			utils.KubectlDelete("-f", preReqs)
 		})
 	})
+
+	Describe("Policy with the ObjectName variable", Ordered, func() {
+		const (
+			preReqs           = case13RsrcPath + "/case13_objectname_var_prereqs.yaml"
+			policyYAML        = case13RsrcPath + "/case13_objectname_var.yaml"
+			policyName        = "case13-objectname-var"
+			e2eBaseName       = "case13-e2e-objectname-var"
+			invalidPolicyYAML = case13RsrcPath + "/case13_objectname_var_invalid_name.yaml"
+			invalidPolicyName = "case13-invalid-name"
+			emptyPolicyYAML   = case13RsrcPath + "/case13_objectname_var_empty_name.yaml"
+			emptyPolicyName   = "case13-empty-name"
+			skipPolicyYAML    = case13RsrcPath + "/case13_objectname_var_skip_name.yaml"
+			skipPolicyName    = "case13-skip-name"
+		)
+
+		It("Should enforce the labels on the case13-e2e-objectname-var* ConfigMaps", func(ctx SpecContext) {
+			By("Creating the prerequisites")
+			utils.Kubectl("apply", "-f", preReqs)
+
+			By("Applying the " + policyName + " ConfigurationPolicy")
+			utils.Kubectl("apply", "-n", testNamespace, "-f", policyYAML)
+
+			By("By verifying that the ConfigurationPolicy is compliant and has the correct related objects")
+			Eventually(func(g Gomega) {
+				managedPlc := utils.GetWithTimeout(
+					clientManagedDynamic, gvrConfigPolicy, policyName, testNamespace, true, defaultTimeoutSeconds,
+				)
+
+				utils.CheckComplianceStatus(g, managedPlc, "Compliant")
+				g.Expect(utils.GetStatusMessage(managedPlc)).Should(Equal(fmt.Sprintf(
+					"configmaps [case13-e2e-objectname-var1] found as specified in namespace %[1]s; "+
+						"configmaps [case13-e2e-objectname-var2] found as specified in namespace %[1]s; "+
+						"configmaps [case13-e2e-objectname-var3] found as specified in namespace %[1]s", e2eBaseName)))
+
+				relatedObjects, _, _ := unstructured.NestedSlice(managedPlc.Object, "status", "relatedObjects")
+				g.Expect(relatedObjects).To(HaveLen(3))
+
+				for idx := range relatedObjects {
+					relatedObject, ok := relatedObjects[idx].(map[string]interface{})
+					g.Expect(ok).To(BeTrue(), "Related object is not a map")
+					relatedObject1NS, _, _ := unstructured.NestedString(relatedObject, "object", "metadata", "name")
+					g.Expect(relatedObject1NS).To(
+						Equal(fmt.Sprintf("%s%d", e2eBaseName, idx+1)),
+						"Related object name should match")
+				}
+			}, defaultTimeoutSeconds, 1).Should(Succeed())
+
+			By("By verifying the ConfigMaps")
+			configMaps, err := clientManaged.CoreV1().ConfigMaps(e2eBaseName).List(
+				ctx, metav1.ListOptions{
+					LabelSelector: "case13",
+				},
+			)
+			Expect(err).ToNot(HaveOccurred())
+
+			for _, cm := range configMaps.Items {
+				Expect(cm.ObjectMeta.Labels).To(HaveKeyWithValue("case13", "passed"))
+				Expect(cm.ObjectMeta.Labels).To(HaveKeyWithValue("name", cm.GetName()))
+				Expect(cm.ObjectMeta.Labels).To(HaveKeyWithValue("namespace", cm.GetNamespace()))
+			}
+		})
+
+		It("Should fail when the name is empty after template resolution", func(ctx SpecContext) {
+			By("Creating the prerequisites")
+			utils.Kubectl("apply", "-f", preReqs)
+
+			By("Applying the " + emptyPolicyName + " ConfigurationPolicy")
+			utils.Kubectl("apply", "-n", testNamespace, "-f", emptyPolicyYAML)
+
+			By("By verifying that the ConfigurationPolicy is noncompliant")
+			Eventually(func(g Gomega) {
+				managedPlc := utils.GetWithTimeout(
+					clientManagedDynamic,
+					gvrConfigPolicy,
+					emptyPolicyName,
+					testNamespace,
+					true,
+					defaultTimeoutSeconds,
+				)
+
+				utils.CheckComplianceStatus(g, managedPlc, "NonCompliant")
+				g.Expect(utils.GetStatusMessage(managedPlc)).To(Equal(
+					"The object definition's name must match the selected name after template resolution",
+				))
+			}, defaultTimeoutSeconds, 1).Should(Succeed())
+		})
+
+		It("Should fail when the name doesn't match after template resolution", func(ctx SpecContext) {
+			By("Creating the prerequisites")
+			utils.Kubectl("apply", "-f", preReqs)
+
+			By("Applying the " + invalidPolicyName + " ConfigurationPolicy")
+			utils.Kubectl("apply", "-n", testNamespace, "-f", invalidPolicyYAML)
+
+			By("By verifying that the ConfigurationPolicy is noncompliant")
+			Eventually(func(g Gomega) {
+				managedPlc := utils.GetWithTimeout(
+					clientManagedDynamic,
+					gvrConfigPolicy,
+					invalidPolicyName,
+					testNamespace,
+					true,
+					defaultTimeoutSeconds,
+				)
+
+				utils.CheckComplianceStatus(g, managedPlc, "NonCompliant")
+				g.Expect(utils.GetStatusMessage(managedPlc)).To(Equal(
+					"The object definition's name must match the selected name after template resolution",
+				))
+			}, defaultTimeoutSeconds, 1).Should(Succeed())
+		})
+
+		AfterEach(func() {
+			utils.KubectlDelete("configurationpolicy", policyName, "-n", testNamespace)
+			utils.KubectlDelete("configurationpolicy", invalidPolicyName, "-n", testNamespace)
+			utils.KubectlDelete("configurationpolicy", emptyPolicyName, "-n", testNamespace)
+			utils.KubectlDelete("configurationpolicy", skipPolicyName, "-n", testNamespace)
+			utils.KubectlDelete("configmaps", "-n", e2eBaseName, "--all")
+		})
+
+		AfterAll(func() {
+			utils.KubectlDelete("-f", preReqs)
+		})
+	})
 })
diff --git a/test/resources/case13_templatization/case13_objectname_var.yaml b/test/resources/case13_templatization/case13_objectname_var.yaml
@@ -0,0 +1,25 @@
+apiVersion: policy.open-cluster-management.io/v1
+kind: ConfigurationPolicy
+metadata:
+  name: case13-objectname-var
+spec:
+  remediationAction: enforce
+  namespaceSelector:
+    include:
+      - case13-e2e-objectname-var
+  object-templates:
+    - complianceType: musthave
+      objectSelector:
+        matchExpressions:
+          - key: case13
+            operator: Exists
+      objectDefinition:
+        apiVersion: v1
+        kind: ConfigMap
+        metadata:
+          name: "{{ .ObjectName }}"
+          namespace: "{{ .ObjectNamespace }}"
+          labels:
+            case13: passed
+            name: "{{ .ObjectName }}"
+            namespace: "{{ .ObjectNamespace }}"
diff --git a/test/resources/case13_templatization/case13_objectname_var_empty_name.yaml b/test/resources/case13_templatization/case13_objectname_var_empty_name.yaml
@@ -0,0 +1,21 @@
+apiVersion: policy.open-cluster-management.io/v1
+kind: ConfigurationPolicy
+metadata:
+  name: case13-empty-name
+spec:
+  remediationAction: enforce
+  namespaceSelector:
+    include:
+      - case13-e2e-objectname-var
+  object-templates:
+    - complianceType: musthave
+      objectSelector:
+        matchExpressions:
+          - key: case13
+            operator: Exists
+      objectDefinition:
+        apiVersion: v1
+        kind: ConfigMap
+        metadata:
+          name: "{{ if false }}{{ .ObjectName }}{{ end }}"
+          namespace: "{{ .ObjectNamespace }}"
diff --git a/test/resources/case13_templatization/case13_objectname_var_invalid_name.yaml b/test/resources/case13_templatization/case13_objectname_var_invalid_name.yaml
@@ -0,0 +1,21 @@
+apiVersion: policy.open-cluster-management.io/v1
+kind: ConfigurationPolicy
+metadata:
+  name: case13-invalid-name
+spec:
+  remediationAction: enforce
+  namespaceSelector:
+    include:
+      - case13-e2e-objectname-var
+  object-templates:
+    - complianceType: musthave
+      objectSelector:
+        matchExpressions:
+          - key: case13
+            operator: Exists
+      objectDefinition:
+        apiVersion: v1
+        kind: ConfigMap
+        metadata:
+          name: "{{ .ObjectName }}-some-random-suffix"
+          namespace: "{{ .ObjectNamespace }}"
diff --git a/test/resources/case13_templatization/case13_objectname_var_prereqs.yaml b/test/resources/case13_templatization/case13_objectname_var_prereqs.yaml
@@ -0,0 +1,34 @@
+apiVersion: v1
+kind: Namespace
+metadata:
+  name: case13-e2e-objectname-var
+---
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  labels:
+    case13: case13-e2e-objectname-var1
+  name: case13-e2e-objectname-var1
+  namespace: case13-e2e-objectname-var
+---
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  labels:
+    case13: case13-e2e-objectname-var2
+  name: case13-e2e-objectname-var2
+  namespace: case13-e2e-objectname-var
+---
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  labels:
+    case13: case13-e2e-objectname-var3
+  name: case13-e2e-objectname-var3
+  namespace: case13-e2e-objectname-var
+---
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: case13-e2e-objectname-var4
+  namespace: case13-e2e-objectname-var