Handle policy dependencies of Gatekeeper types

controllers/templatesync/template_sync.go

@@ -1146,13 +1146,7 @@ func (r *PolicyReconciler) processDependencies(
 	var dependencyFailures []depclient.ObjectIdentifier
 
 	for dep := range templateDeps {
-		depGvk := schema.GroupVersionKind{
-			Group:   dep.Group,
-			Version: dep.Version,
-			Kind:    dep.Kind,
-		}
-
-		rsrc, _, err := utils.GVRFromGVK(discoveryClient, depGvk)
+		rsrc, namespaced, err := utils.GVRFromGVK(discoveryClient, dep.GroupVersionKind())
 		if err != nil {
 			tLogger.Error(err, "Could not find an API mapping for the dependency", "object", dep)
 
@@ -1161,24 +1155,57 @@ func (r *PolicyReconciler) processDependencies(
 			continue
 		}
 
-		// set up namespace for replicated policy dependencies
-		ns := dep.Namespace
-		if ns == "" {
-			ns = r.ClusterNamespace
-		}
+		var res dynamic.ResourceInterface
 
-		// query object and compare compliance status to desired
-		res := dClient.Resource(rsrc).Namespace(ns)
+		if namespaced {
+			ns := dep.Namespace
+			if ns == "" && dep.Group == policiesv1.GroupVersion.Group {
+				// ocm policies should always be in the cluster namespace
+				ns = r.ClusterNamespace
+			}
+
+			res = dClient.Resource(rsrc).Namespace(ns)
+		} else {
+			res = dClient.Resource(rsrc)
+		}
 
 		depObj, err := res.Get(ctx, dep.Name, metav1.GetOptions{})
 		if err != nil {
+			if dep.Group == utils.GvkConstraintTemplate.Group && templateDeps[dep] != "Compliant" {
+				continue // in this (strange) case, the policy wants the ConstraintTemplate to not be found
+			}
+
 			tLogger.Info("Failed to get dependency object", "object", dep)
 
 			dependencyFailures = append(dependencyFailures, dep)
-		} else {
+
+			continue
+		}
+
+		switch dep.Group {
+		case utils.GvkConstraintTemplate.Group:
+			if templateDeps[dep] != "Compliant" {
+				// The ConstraintTemplate was found, but the policy wants it to not be found
+				tLogger.Info("Compliance mismatch for dependency object", "object", dep)
+
+				dependencyFailures = append(dependencyFailures, dep)
+			}
+		case utils.GConstraint:
+			violations, found, err := unstructured.NestedInt64(depObj.Object, "status", "totalViolations")
+			if err != nil || !found {
+				// Note that not finding the field is *not* considered "Compliant"
+				tLogger.Info("Failed to get compliance for dependency object", "object", dep, "error", err)
+
+				dependencyFailures = append(dependencyFailures, dep)
+			} else if (violations == 0) != (templateDeps[dep] == "Compliant") {
+				tLogger.Info("Compliance mismatch for dependency object", "object", dep)
+
+				dependencyFailures = append(dependencyFailures, dep)
+			}
+		default:
 			depCompliance, found, err := unstructured.NestedString(depObj.Object, "status", "compliant")
 			if err != nil || !found {
-				tLogger.Info("Failed to get compliance for dependency object", "object", dep)
+				tLogger.Info("Failed to get compliance for dependency object", "object", dep, "error", err)
 
 				dependencyFailures = append(dependencyFailures, dep)
 			} else if depCompliance != templateDeps[dep] {
@@ -1523,13 +1550,10 @@ func finalizerCleanup(
 			continue
 		}
 
-		// Instantiate a dynamic client
-		res := dClient.Resource(rsrc)
-
 		// Delete clusterwide objects
 		if !namespaced {
 			// Delete object, ignoring not found errors
-			err := res.Delete(ctx, tName, metav1.DeleteOptions{})
+			err := dClient.Resource(rsrc).Delete(ctx, tName, metav1.DeleteOptions{})
 			if err != nil && !k8serrors.IsNotFound(err) {
 				policySystemErrorsCounter.WithLabelValues(pol.Name, tName, "delete-error").Inc()
 

test/e2e/case17_gatekeeper_sync_test.go

@@ -495,4 +495,213 @@ var _ = Describe("Test Gatekeeper ConstraintTemplate and constraint sync", Order
 			"", false, defaultTimeoutSeconds,
 		)
 	})
+
+	Describe("Test policy ordering with gatekeeper objects", func() {
+		const (
+			waitForTemplateName   = "case17-gk-dep-on-tmpl"
+			waitForTemplateYaml   = yamlBasePath + waitForTemplateName + ".yaml"
+			waitForConstraintName = "case17-gk-dep-on-constraint"
+			waitForConstraintYaml = yamlBasePath + waitForConstraintName + ".yaml"
+		)
+
+		BeforeAll(func(ctx context.Context) {
+			By("Deleting any ConfigMaps in the test namespace, to prevent any initial violations")
+			err := clientManaged.CoreV1().ConfigMaps(configMapNamespace).DeleteCollection(
+				ctx, metav1.DeleteOptions{}, metav1.ListOptions{},
+			)
+			Expect(err).ToNot(HaveOccurred())
+		})
+
+		AfterAll(func() {
+			for _, pName := range []string{waitForTemplateName, waitForConstraintName} {
+				By("Deleting policy " + pName + " on the hub in ns:" + clusterNamespaceOnHub)
+				err := clientHubDynamic.Resource(gvrPolicy).Namespace(clusterNamespaceOnHub).Delete(
+					context.TODO(), pName, metav1.DeleteOptions{},
+				)
+				if !k8serrors.IsNotFound(err) {
+					Expect(err).ToNot(HaveOccurred())
+				}
+
+				By("Cleaning up the events for the policy " + pName)
+				_, err = kubectlManaged(
+					"delete",
+					"events",
+					"-n",
+					clusterNamespace,
+					"--field-selector=involvedObject.name="+pName,
+					"--ignore-not-found",
+				)
+				Expect(err).ToNot(HaveOccurred())
+			}
+		})
+
+		It("should not progress until the ConstraintTemplate is created", func() {
+			By("Creating policy " + waitForTemplateName + " on the hub in ns:" + clusterNamespaceOnHub)
+			_, err := kubectlHub("apply", "-f", waitForTemplateYaml, "-n", clusterNamespaceOnHub)
+			Expect(err).ShouldNot(HaveOccurred())
+			Expect(propagatorutils.GetWithTimeout(
+				clientManagedDynamic,
+				gvrPolicy,
+				waitForTemplateName,
+				clusterNamespace,
+				true,
+				defaultTimeoutSeconds,
+			)).NotTo(BeNil())
+
+			By("Checking that the configuration policy is not found, because it should be pending")
+			Consistently(func() interface{} {
+				return propagatorutils.GetWithTimeout(
+					clientManagedDynamic,
+					gvrConfigurationPolicy,
+					waitForTemplateName,
+					clusterNamespace,
+					false,
+					defaultTimeoutSeconds)
+			}, 10, 1).Should(BeNil())
+
+			By("Creating the policy with the ConstraintTemplate")
+			_, err = kubectlHub("apply", "-f", policyYaml, "-n", clusterNamespaceOnHub)
+			Expect(err).ShouldNot(HaveOccurred())
+			Expect(propagatorutils.GetWithTimeout(
+				clientManagedDynamic,
+				gvrPolicy,
+				policyName,
+				clusterNamespace,
+				true,
+				defaultTimeoutSeconds,
+			)).NotTo(BeNil())
+
+			By("Checking that the configuration policy can now be found")
+			Expect(propagatorutils.GetWithTimeout(
+				clientManagedDynamic,
+				gvrConfigurationPolicy,
+				waitForTemplateName,
+				clusterNamespace,
+				true,
+				defaultTimeoutSeconds,
+			)).NotTo(BeNil())
+		})
+
+		It("should progress initially when the constraint has no violations", func() {
+			By("Verifying that the policy status of the constraint is compliant")
+			Eventually(func(g Gomega) {
+				plc := propagatorutils.GetWithTimeout(
+					clientManagedDynamic,
+					gvrPolicy,
+					policyName,
+					clusterNamespace,
+					true,
+					defaultTimeoutSeconds,
+				)
+
+				compliance, found, err := unstructured.NestedString(plc.Object, "status", "compliant")
+				g.Expect(err).ToNot(HaveOccurred())
+				g.Expect(found).To(BeTrue())
+				g.Expect(compliance).To(Equal("Compliant"))
+			}, gkAuditFrequency*3, 1).Should(Succeed())
+
+			By("Creating policy " + waitForConstraintName + " on the hub in ns:" + clusterNamespaceOnHub)
+			_, err := kubectlHub("apply", "-f", waitForConstraintYaml, "-n", clusterNamespaceOnHub)
+			Expect(err).ShouldNot(HaveOccurred())
+			Expect(propagatorutils.GetWithTimeout(
+				clientManagedDynamic,
+				gvrPolicy,
+				waitForConstraintName,
+				clusterNamespace,
+				true,
+				defaultTimeoutSeconds,
+			)).NotTo(BeNil())
+
+			By("Checking that the configuration policy is found; the policy is not pending")
+			Expect(propagatorutils.GetWithTimeout(
+				clientManagedDynamic,
+				gvrConfigurationPolicy,
+				waitForConstraintName,
+				clusterNamespace,
+				true,
+				defaultTimeoutSeconds,
+			)).NotTo(BeNil())
+		})
+
+		It("should become Pending when there are violations on the constraint", func() {
+			By("Adding a ConfigMap that violates the constraint")
+			configMap := &corev1.ConfigMap{
+				TypeMeta: metav1.TypeMeta{
+					Kind:       "ConfigMap",
+					APIVersion: "v1",
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      configMapName,
+					Namespace: configMapNamespace,
+				},
+			}
+
+			_, err := clientManaged.CoreV1().ConfigMaps(configMapNamespace).Create(
+				context.TODO(), configMap, metav1.CreateOptions{},
+			)
+			Expect(err).ToNot(HaveOccurred())
+
+			By("Waiting for policy status of the constraint to be noncompliant")
+			Eventually(func(g Gomega) {
+				plc := propagatorutils.GetWithTimeout(
+					clientManagedDynamic,
+					gvrPolicy,
+					policyName,
+					clusterNamespace,
+					true,
+					defaultTimeoutSeconds,
+				)
+
+				compliance, found, err := unstructured.NestedString(plc.Object, "status", "compliant")
+				g.Expect(err).ToNot(HaveOccurred())
+				g.Expect(found).To(BeTrue())
+				g.Expect(compliance).To(Equal("NonCompliant"))
+			}, gkAuditFrequency*3, 1).Should(Succeed())
+
+			By("Checking that the configuration policy is not found, because it should be pending")
+			Expect(propagatorutils.GetWithTimeout(
+				clientManagedDynamic,
+				gvrConfigurationPolicy,
+				waitForConstraintName,
+				clusterNamespace,
+				false,
+				defaultTimeoutSeconds,
+			)).To(BeNil())
+		})
+
+		It("should progress again when the violations are addressed", func() {
+			By("Deleting the ConfigMap causing the violation")
+			err := clientManaged.CoreV1().ConfigMaps(configMapNamespace).Delete(
+				context.TODO(), configMapName, metav1.DeleteOptions{},
+			)
+			Expect(err).ToNot(HaveOccurred())
+
+			By("Waiting for policy status of the constraint to be compliant")
+			Eventually(func(g Gomega) {
+				plc := propagatorutils.GetWithTimeout(
+					clientManagedDynamic,
+					gvrPolicy,
+					policyName,
+					clusterNamespace,
+					true,
+					defaultTimeoutSeconds,
+				)
+
+				compliance, found, err := unstructured.NestedString(plc.Object, "status", "compliant")
+				g.Expect(err).ToNot(HaveOccurred())
+				g.Expect(found).To(BeTrue())
+				g.Expect(compliance).To(Equal("Compliant"))
+			}, gkAuditFrequency*3, 1).Should(Succeed())
+
+			By("Checking that the configuration policy is found; the policy is no longer pending")
+			Expect(propagatorutils.GetWithTimeout(
+				clientManagedDynamic,
+				gvrConfigurationPolicy,
+				waitForConstraintName,
+				clusterNamespace,
+				true,
+				defaultTimeoutSeconds,
+			)).NotTo(BeNil())
+		})
+	})
 })

test/resources/case17_gatekeeper_sync/case17-gk-dep-on-constraint.yaml

@@ -0,0 +1,32 @@
+apiVersion: policy.open-cluster-management.io/v1
+kind: Policy
+metadata:
+  name: case17-gk-dep-on-constraint
+spec:
+  remediationAction: inform
+  disabled: false
+  dependencies:
+    - apiVersion: constraints.gatekeeper.sh/v1beta1
+      kind: Case17ConstraintTemplate
+      name: case17-gk-constraint
+      namespace: ""
+      compliance: Compliant
+  policy-templates:
+    - objectDefinition:
+        apiVersion: policy.open-cluster-management.io/v1
+        kind: ConfigurationPolicy
+        metadata:
+          name: case17-gk-dep-on-constraint
+        spec:
+          remediationAction: inform
+          object-templates:
+            - complianceType: musthave
+              objectDefinition:
+                apiVersion: v1
+                kind: Pod
+                metadata:
+                  name: nginx-pod-e2e
+                  namespace: default
+                spec:
+                  containers:
+                    - name: nginx

test/resources/case17_gatekeeper_sync/case17-gk-dep-on-tmpl.yaml

@@ -0,0 +1,32 @@
+apiVersion: policy.open-cluster-management.io/v1
+kind: Policy
+metadata:
+  name: case17-gk-dep-on-tmpl
+spec:
+  remediationAction: inform
+  disabled: false
+  dependencies:
+    - apiVersion: templates.gatekeeper.sh/v1
+      kind: ConstraintTemplate
+      name: case17constrainttemplate
+      namespace: ""
+      compliance: Compliant
+  policy-templates:
+    - objectDefinition:
+        apiVersion: policy.open-cluster-management.io/v1
+        kind: ConfigurationPolicy
+        metadata:
+          name: case17-gk-dep-on-tmpl
+        spec:
+          remediationAction: inform
+          object-templates:
+            - complianceType: musthave
+              objectDefinition:
+                apiVersion: v1
+                kind: Pod
+                metadata:
+                  name: nginx-pod-e2e
+                  namespace: default
+                spec:
+                  containers:
+                    - name: nginx