Fix when context variable isn't used for metadata

When the `.ObjectName` template variable was used
outside `.metadata.name`, the controller
complained that the name didn't match.

This also removes an errant E2E test.

Signed-off-by: Dale Haiducek <[email protected]>

Author: dhaiducek

api/v1/configurationpolicy_types.go

@@ -79,6 +79,11 @@ type Target struct {
 	Exclude []NonEmptyString `json:"exclude,omitempty"`
 }
 
+// IsEmpty returns whether the defined Target would always return no objects.
+func (t Target) IsEmpty() bool {
+	return t.LabelSelector == nil && len(t.Include) == 0
+}
+
 // Define String() so that the LabelSelector is dereferenced in the logs
 func (t Target) String() string {
 	fmtSelectorStr := "{include:%s,exclude:%s,matchLabels:%+v,matchExpressions:%+v}"

controllers/configurationpolicy_controller.go

@@ -1584,45 +1584,45 @@ func (r *ConfigurationPolicyReconciler) determineDesiredObjects(
 				return nil, &scopedGVR, errEvent, err
 			}
 
+			// Populate the namespace and name if applicable
+			if desiredObj.GetName() == "" && name != "" {
+				desiredObj.SetName(strings.TrimSpace(name))
+			}
+
+			if desiredObj.GetNamespace() == "" && ns != "" {
+				desiredObj.SetNamespace(strings.TrimSpace(ns))
+			}
+
 			// strings.TrimSpace() is needed here because a multi-line value will have
 			// '\n' in it. This is kept for backwards compatibility.
 			desiredObj.SetName(strings.TrimSpace(desiredObj.GetName()))
 			desiredObj.SetKind(strings.TrimSpace(desiredObj.GetKind()))
 			desiredObj.SetNamespace(strings.TrimSpace(desiredObj.GetNamespace()))
 
-			// If the namespace doesn't match the original, return an error
-			if needsPerNamespaceTemplating && desiredObj.GetNamespace() != ns {
+			// Error if the namespace doesn't match the parsed namespace from the namespaceSelector
+			if !plc.Spec.NamespaceSelector.IsEmpty() && desiredObj.GetNamespace() != ns {
 				errEvent := &objectTmplEvalEvent{
 					compliant: false,
 					reason:    reasonTemplateError,
-					message: "The object definition's namespace must match the selected namespace after template " +
-						"resolution",
+					message: "The object definition's namespace must match the result " +
+						"from the namespace selector after template resolution",
 				}
 
 				return nil, &scopedGVR, errEvent, nil
 			}
 
-			// If the name doesn't match the original, return an error
-			if needsPerNameTemplating && desiredObj.GetName() != name {
+			// Error if the name doesn't match the parsed name from the objectSelector
+			if objectSelector != nil && desiredObj.GetName() != name {
 				errEvent := &objectTmplEvalEvent{
 					compliant: false,
 					reason:    reasonTemplateError,
-					message: "The object definition's name must match the selected name after template " +
-						"resolution",
+					message: "The object definition's name must match the result " +
+						"from the object selector 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))
-			}
-
-			if ns != "" {
-				desiredObj.SetNamespace(strings.TrimSpace(ns))
-			}
-
 			desiredObjects = append(desiredObjects, desiredObj)
 		}
 	}

pkg/common/namespace_selection.go

@@ -179,7 +179,7 @@ func (r *NamespaceSelectorReconciler) Get(objNS string, objName string, t policy
 	r.lock.Unlock()
 
 	// Return no namespaces when both include and label selector are empty
-	if t.LabelSelector == nil && len(t.Include) == 0 {
+	if t.IsEmpty() {
 		log.V(2).Info("Updating selection from Reconcile for empty selector",
 			"namespace", objNS, "policy", objName)
 
@@ -306,7 +306,7 @@ func (r *NamespaceSelectorReconciler) update(namespace string, name string, sel
 func filter(allNSList corev1.NamespaceList, t policyv1.Target) ([]string, error) {
 	// If MatchLabels and MatchExpressions are nil, the resulting label selector
 	// matches all namespaces. This is to guard against that.
-	if t.LabelSelector == nil && len(t.Include) == 0 {
+	if t.IsEmpty() {
 		return []string{}, nil
 	}
 

test/e2e/case13_templatization_test.go

@@ -577,8 +577,8 @@ var _ = Describe("Test templatization", Ordered, func() {
 			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"
+			outsidePolicyYAML    = case13RsrcPath + "/case13_objectname_var_outside_name.yaml"
+			outsidePolicyName    = "case13-outside-name"
 			allSkippedPolicyYAML = case13RsrcPath + "/case13_objectname_var_all_skipped.yaml"
 			allSkippedPolicyName = "case13-objectname-var-all-skipped"
 		)
@@ -636,26 +636,42 @@ var _ = Describe("Test templatization", Ordered, func() {
 			}
 		})
 
-		It("Should fail when the name is empty after template resolution", func(ctx SpecContext) {
-			By("Applying the " + emptyPolicyName + " ConfigurationPolicy")
-			utils.Kubectl("apply", "-n", testNamespace, "-f", emptyPolicyYAML)
+		It("Should succeed when context vars are in use but name/namespace are left empty", func(ctx SpecContext) {
+			By("Applying the " + outsidePolicyName + " ConfigurationPolicy")
+			utils.Kubectl("apply", "-n", testNamespace, "-f", outsidePolicyYAML)
 
-			By("By verifying that the ConfigurationPolicy is noncompliant")
+			By("By verifying that the ConfigurationPolicy is compliant and has the correct related objects")
 			Eventually(func(g Gomega) {
 				managedPlc := utils.GetWithTimeout(
 					clientManagedDynamic,
 					gvrConfigPolicy,
-					emptyPolicyName,
+					outsidePolicyName,
 					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",
-				))
+				utils.CheckComplianceStatus(g, managedPlc, "Compliant")
+				g.Expect(utils.GetStatusMessage(managedPlc)).Should(Equal(fmt.Sprintf(
+					"configmaps [case13-e2e-objectname-var3] found as specified in namespace %s", e2eBaseName)))
+
+				relatedObjects, _, _ := unstructured.NestedSlice(managedPlc.Object, "status", "relatedObjects")
+				g.Expect(relatedObjects).To(HaveLen(1))
+				relatedObject, ok := relatedObjects[0].(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, 3)),
+					"Related object name should match")
 			}, defaultTimeoutSeconds, 1).Should(Succeed())
+
+			By("By verifying the ConfigMaps")
+			cm, err := clientManaged.CoreV1().ConfigMaps(e2eBaseName).Get(
+				ctx, "case13-e2e-objectname-var3", metav1.GetOptions{})
+			Expect(err).ToNot(HaveOccurred())
+			Expect(cm.ObjectMeta.Labels).To(HaveKeyWithValue("case13", "passed"))
+			Expect(cm.ObjectMeta.Labels).To(HaveKeyWithValue("object-name", cm.GetName()))
+			Expect(cm.ObjectMeta.Labels).To(HaveKeyWithValue("object-namespace", cm.GetNamespace()))
 		})
 
 		It("Should fail when the name doesn't match after template resolution", func(ctx SpecContext) {
@@ -705,7 +721,7 @@ var _ = Describe("Test templatization", Ordered, func() {
 		AfterEach(func() {
 			utils.KubectlDelete("configurationpolicy", policyName, "-n", testNamespace)
 			utils.KubectlDelete("configurationpolicy", invalidPolicyName, "-n", testNamespace)
-			utils.KubectlDelete("configurationpolicy", emptyPolicyName, "-n", testNamespace)
+			utils.KubectlDelete("configurationpolicy", outsidePolicyName, "-n", testNamespace)
 			utils.KubectlDelete("configurationpolicy", allSkippedPolicyName, "-n", testNamespace)
 			utils.KubectlDelete("configmaps", "-n", e2eBaseName, "--all")
 		})

test/resources/case13_templatization/case13_objectname_var_empty_name.yaml renamed to test/resources/case13_templatization/case13_objectname_var_outside_name.yaml

@@ -1,7 +1,7 @@
 apiVersion: policy.open-cluster-management.io/v1
 kind: ConfigurationPolicy
 metadata:
-  name: case13-empty-name
+  name: case13-outside-name
 spec:
   remediationAction: enforce
   namespaceSelector:
@@ -17,5 +17,7 @@ spec:
         apiVersion: v1
         kind: ConfigMap
         metadata:
-          name: "{{ if false }}{{ .ObjectName }}{{ end }}"
-          namespace: "{{ .ObjectNamespace }}"
+          labels:
+            case13: passed
+            object-name: '{{ if (hasSuffix "3" .ObjectName) }}{{ .ObjectName }}{{ else }}{{ skipObject }}{{ end }}'
+            object-namespace: "{{ .ObjectNamespace }}"