Set the cluster namespace on OperatorPolicy dependencies

Without this, the governance-policy-framework panics because it doesn't have permission to view OperatorPolicy in all namespaces. Signed-off-by: mprahl <mprahl@users.noreply.github.com>
open-cluster-management-io · Jul 1, 2024 · c2681fe · c2681fe
1 parent 1378b0e
commit c2681fe
Show file tree

Hide file tree

Showing 2 changed files with 84 additions and 23 deletions.
diff --git a/controllers/templatesync/template_sync.go b/controllers/templatesync/template_sync.go
@@ -241,17 +241,10 @@ func (r *PolicyReconciler) Reconcile(ctx context.Context, request reconcile.Requ
 			Group:     dep.GroupVersionKind().Group,
 			Version:   dep.GroupVersionKind().Version,
 			Kind:      dep.GroupVersionKind().Kind,
-			Namespace: dep.Namespace,
+			Namespace: getDepNamespace(r.ClusterNamespace, dep),
 			Name:      dep.Name,
 		}
 
-		// Use cluster namespace for known policy types when the namespace is blank
-		if depID.Namespace == "" && depID.Group == policiesv1.GroupVersion.Group &&
-			depID.Version == policiesv1.GroupVersion.Version &&
-			strings.HasSuffix(depID.Kind, "Policy") {
-			depID.Namespace = request.Namespace
-		}
-
 		existingDep, ok := topLevelDeps[depID]
 		if ok && existingDep != string(dep.Compliance) {
 			err := fmt.Errorf("dependency on %s has conflicting compliance states", dep.Name)
@@ -322,17 +315,10 @@ func (r *PolicyReconciler) Reconcile(ctx context.Context, request reconcile.Requ
 				Group:     dep.GroupVersionKind().Group,
 				Version:   dep.GroupVersionKind().Version,
 				Kind:      dep.GroupVersionKind().Kind,
-				Namespace: dep.Namespace,
+				Namespace: getDepNamespace(r.ClusterNamespace, dep),
 				Name:      dep.Name,
 			}
 
-			// Use cluster namespace for known policy types when the namespace is blank
-			if depID.Namespace == "" && depID.Group == policiesv1.GroupVersion.Group &&
-				depID.Version == policiesv1.GroupVersion.Version &&
-				strings.HasSuffix(depID.Kind, "Policy") {
-				depID.Namespace = request.Namespace
-			}
-
 			existingDep, ok := templateDeps[depID]
 			if ok && existingDep != string(dep.Compliance) {
 				// dependency conflict, fire error
@@ -1287,6 +1273,18 @@ const (
 	DepFailWrongCompliance = "Compliance mismatch on the dependency object"
 )
 
+// getDepNamespace will return the cluster namespace if the namespace is a policy, otherwise it returns the value
+// of dep.Namespace.
+func getDepNamespace(clusterNamespace string, dep policiesv1.PolicyDependency) string {
+	// Use cluster namespace for known policy types since the governance-policy-framework doesn't have permission
+	// to read outside of the cluster namespace.
+	if dep.GroupVersionKind().Group == policiesv1.GroupVersion.Group && strings.HasSuffix(dep.Kind, "Policy") {
+		return clusterNamespace
+	}
+
+	return dep.Namespace
+}
+
 // processDependencies iterates through all dependencies of a template and returns an map of
 // unmet dependencies to the reason that dependency was not satisfied.
 func (r *PolicyReconciler) processDependencies(
@@ -1310,13 +1308,7 @@ func (r *PolicyReconciler) processDependencies(
 		var res dynamic.ResourceInterface
 
 		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)
+			res = dClient.Resource(rsrc).Namespace(dep.Namespace)
 		} else {
 			res = dClient.Resource(rsrc)
 		}

diff --git a/controllers/templatesync/template_sync_test.go b/controllers/templatesync/template_sync_test.go
@@ -332,3 +332,72 @@ func TestEquivalentTemplatesOperatorPolicyComplianceConfig(t *testing.T) {
 		t.Fatal("Expected the templates to be equivalent")
 	}
 }
+
+func TestGetDepNamespace(t *testing.T) {
+	t.Parallel()
+
+	tests := map[string]struct {
+		gvk       schema.GroupVersionKind
+		namespace string
+		expected  string
+	}{
+		"operator-policy-no-ns": {
+			gvk: schema.GroupVersionKind{
+				Group: policiesv1.GroupVersion.Group, Version: "v1beta1", Kind: "OperatorPolicy",
+			},
+			namespace: "",
+			expected:  "local-cluster",
+		},
+		"operator-policy-with-ns": {
+			gvk: schema.GroupVersionKind{
+				Group: policiesv1.GroupVersion.Group, Version: "v1beta1", Kind: "OperatorPolicy",
+			},
+			namespace: "something",
+			expected:  "local-cluster",
+		},
+		"config-policy-no-ns": {
+			gvk: schema.GroupVersionKind{
+				Group: policiesv1.GroupVersion.Group, Version: "v1", Kind: "ConfigurationPolicy",
+			},
+			namespace: "",
+			expected:  "local-cluster",
+		},
+		"config-policy-with-ns": {
+			gvk: schema.GroupVersionKind{
+				Group: policiesv1.GroupVersion.Group, Version: "v1", Kind: "ConfigurationPolicy",
+			},
+			namespace: "something",
+			expected:  "local-cluster",
+		},
+		"other-with-ns": {
+			gvk: schema.GroupVersionKind{
+				Group: "other", Version: "v1", Kind: "ConfigurationPolicy",
+			},
+			namespace: "something",
+			expected:  "something",
+		},
+	}
+
+	for testName, test := range tests {
+		test := test
+
+		t.Run(testName, func(t *testing.T) {
+			t.Parallel()
+
+			dep := policiesv1.PolicyDependency{
+				TypeMeta: metav1.TypeMeta{
+					Kind:       test.gvk.Kind,
+					APIVersion: test.gvk.GroupVersion().String(),
+				},
+				Name:      "some-policy",
+				Namespace: test.namespace,
+			}
+
+			depNamespace := getDepNamespace("local-cluster", dep)
+
+			if depNamespace != test.expected {
+				t.Fatalf("Expected namespace %s got %s", test.expected, depNamespace)
+			}
+		})
+	}
+}