Don't set ownership for namespaces

api/v1alpha1/gitopsset_types.go

@@ -194,7 +194,7 @@ type GitOpsSetSpec struct {
 // GitOpsSetStatus defines the observed state of GitOpsSet
 type GitOpsSetStatus struct {
 	meta.ReconcileRequestStatus `json:",inline"`
-	
+
 	// ObservedGeneration is the last observed generation of the HelmRepository
 	// object.
 	// +optional

api/v1alpha1/inventory.go

@@ -1,5 +1,12 @@
 package v1alpha1
 
+import (
+	"fmt"
+
+	runtime "k8s.io/apimachinery/pkg/runtime"
+	"sigs.k8s.io/cli-utils/pkg/object"
+)
+
 // ResourceInventory contains a list of Kubernetes resource object references that have been applied by a Kustomization.
 type ResourceInventory struct {
 	// Entries of Kubernetes resource object references.
@@ -15,3 +22,16 @@ type ResourceRef struct {
 	// Version is the API version of the Kubernetes resource object's kind.
 	Version string `json:"v"`
 }
+
+// ResourceRefFromObject returns a ResourceRef from a runtime.Object.
+func ResourceRefFromObject(obj runtime.Object) (ResourceRef, error) {
+	objMeta, err := object.RuntimeToObjMeta(obj)
+	if err != nil {
+		return ResourceRef{}, fmt.Errorf("failed to parse object Metadata: %w", err)
+	}
+
+	return ResourceRef{
+		ID:      objMeta.String(),
+		Version: obj.GetObjectKind().GroupVersionKind().Version,
+	}, nil
+}

controllers/gitopsset_controller.go

@@ -173,7 +173,7 @@ func (r *GitOpsSetReconciler) renderAndReconcile(ctx context.Context, logger log
 
 	entries := sets.New[templatesv1.ResourceRef]()
 	for _, newResource := range resources {
-		ref, err := resourceRefFromObject(newResource)
+		ref, err := templatesv1.ResourceRefFromObject(newResource)
 		if err != nil {
 			return nil, fmt.Errorf("failed to update inventory: %w", err)
 		}
@@ -206,8 +206,11 @@ func (r *GitOpsSetReconciler) renderAndReconcile(ctx context.Context, logger log
 			}
 		}
 
-		if err := controllerutil.SetOwnerReference(gitOpsSet, newResource, r.Scheme); err != nil {
-			return nil, fmt.Errorf("failed to set ownership: %w", err)
+		// cluster-scoped resources must not have a namespace-scoped owner
+		if templates.IsNamespacedObject(newResource) {
+			if err := controllerutil.SetOwnerReference(gitOpsSet, newResource, r.Scheme); err != nil {
+				return nil, fmt.Errorf("failed to set ownership: %w", err)
+			}
 		}
 
 		if err := logResourceMessage(logger, "creating new resource", newResource); err != nil {
@@ -429,18 +432,6 @@ func unstructuredFromResourceRef(ref templatesv1.ResourceRef) (*unstructured.Uns
 	return &u, nil
 }
 
-func resourceRefFromObject(obj runtime.Object) (templatesv1.ResourceRef, error) {
-	objMeta, err := object.RuntimeToObjMeta(obj)
-	if err != nil {
-		return templatesv1.ResourceRef{}, fmt.Errorf("failed to parse object Metadata: %w", err)
-	}
-
-	return templatesv1.ResourceRef{
-		ID:      objMeta.String(),
-		Version: obj.GetObjectKind().GroupVersionKind().Version,
-	}, nil
-}
-
 func copyUnstructuredContent(existing, newValue *unstructured.Unstructured) *unstructured.Unstructured {
 	result := unstructured.Unstructured{}
 	existing.DeepCopyInto(&result)

controllers/gitopsset_controller_test.go

@@ -103,7 +103,7 @@ func TestReconciliation(t *testing.T) {
 			test.MakeTestKustomization(nsn("default", "engineering-prod-demo")),
 			test.MakeTestKustomization(nsn("default", "engineering-preprod-demo")),
 		}
-		assertInventoryHasItems(t, updated, want...)
+		test.AssertInventoryHasItems(t, updated, want...)
 		assertGitOpsSetCondition(t, updated, meta.ReadyCondition, "3 resources created")
 		assertKustomizationsExist(t, k8sClient, "default", "engineering-dev-demo", "engineering-prod-demo", "engineering-preprod-demo")
 	})
@@ -189,7 +189,7 @@ func TestReconciliation(t *testing.T) {
 		test.AssertNoError(t, k8sClient.Create(ctx, test.ToUnstructured(t, devKS)))
 		defer deleteAllKustomizations(t, k8sClient)
 
-		ref, err := resourceRefFromObject(devKS)
+		ref, err := templatesv1.ResourceRefFromObject(devKS)
 		test.AssertNoError(t, err)
 
 		gs.Status.Inventory = &templatesv1.ResourceInventory{
@@ -206,7 +206,7 @@ func TestReconciliation(t *testing.T) {
 			test.MakeTestKustomization(nsn("default", "engineering-prod-demo")),
 			test.MakeTestKustomization(nsn("default", "engineering-preprod-demo")),
 		}
-		assertInventoryHasItems(t, updated, want...)
+		test.AssertInventoryHasItems(t, updated, want...)
 		assertResourceDoesNotExist(t, k8sClient, devKS)
 	})
 
@@ -251,7 +251,7 @@ func TestReconciliation(t *testing.T) {
 		test.AssertNoError(t, k8sClient.Create(ctx, gs))
 		defer cleanupResource(t, k8sClient, gs)
 
-		ref, err := resourceRefFromObject(devKS)
+		ref, err := templatesv1.ResourceRefFromObject(devKS)
 		test.AssertNoError(t, err)
 		gs.Status.Inventory = &templatesv1.ResourceInventory{
 			Entries: []templatesv1.ResourceRef{ref},
@@ -286,7 +286,7 @@ func TestReconciliation(t *testing.T) {
 		want := []runtime.Object{
 			wantUpdated,
 		}
-		assertInventoryHasItems(t, updated, want...)
+		test.AssertInventoryHasItems(t, updated, want...)
 
 		kustomization := &unstructured.Unstructured{}
 		kustomization.SetGroupVersionKind(kustomizationGVK)
@@ -324,7 +324,7 @@ func TestReconciliation(t *testing.T) {
 		test.AssertNoError(t, k8sClient.Create(ctx, gs))
 		defer cleanupResource(t, k8sClient, gs)
 
-		ref, err := resourceRefFromObject(devKS)
+		ref, err := templatesv1.ResourceRefFromObject(devKS)
 		test.AssertNoError(t, err)
 
 		gs.Status.Inventory = &templatesv1.ResourceInventory{
@@ -388,7 +388,7 @@ func TestReconciliation(t *testing.T) {
 		test.AssertNoError(t, k8sClient.Create(ctx, gs))
 		defer cleanupResource(t, k8sClient, gs)
 
-		ref, err := resourceRefFromObject(devKS)
+		ref, err := templatesv1.ResourceRefFromObject(devKS)
 		test.AssertNoError(t, err)
 		gs.Status.Inventory = &templatesv1.ResourceInventory{
 			Entries: []templatesv1.ResourceRef{ref},
@@ -412,7 +412,7 @@ func TestReconciliation(t *testing.T) {
 		want := []runtime.Object{
 			test.MakeTestKustomization(nsn("default", "engineering-dev-demo")),
 		}
-		assertInventoryHasItems(t, updated, want...)
+		test.AssertInventoryHasItems(t, updated, want...)
 		assertGitOpsSetCondition(t, updated, meta.ReadyCondition, "1 resources created")
 		assertKustomizationsExist(t, k8sClient, "default", "engineering-dev-demo")
 	})
@@ -445,7 +445,7 @@ func TestReconciliation(t *testing.T) {
 			test.MakeTestKustomization(nsn("default", "engineering-prod-demo")),
 			test.MakeTestKustomization(nsn("default", "engineering-preprod-demo")),
 		}
-		assertInventoryHasItems(t, updated, want...)
+		test.AssertInventoryHasItems(t, updated, want...)
 		assertGitOpsSetCondition(t, updated, meta.ReadyCondition, "3 resources created")
 		assertKustomizationsExist(t, k8sClient, "default", "engineering-dev-demo", "engineering-prod-demo", "engineering-preprod-demo")
 	})
@@ -481,7 +481,7 @@ func TestReconciliation(t *testing.T) {
 			test.MakeTestKustomization(nsn("default", "engineering-prod-demo")),
 			test.MakeTestKustomization(nsn("default", "engineering-preprod-demo")),
 		}
-		assertInventoryHasItems(t, updated, want...)
+		test.AssertInventoryHasItems(t, updated, want...)
 		assertGitOpsSetCondition(t, updated, meta.ReadyCondition, "3 resources created")
 		assertKustomizationsExist(t, k8sClient, "default", "engineering-dev-demo", "engineering-prod-demo", "engineering-preprod-demo")
 	})
@@ -529,7 +529,7 @@ func TestReconciliation(t *testing.T) {
 			Verbs:     []string{"get", "list", "watch"},
 		})
 
-		ref, err := resourceRefFromObject(devKS)
+		ref, err := templatesv1.ResourceRefFromObject(devKS)
 		test.AssertNoError(t, err)
 		gs.Status.Inventory = &templatesv1.ResourceInventory{
 			Entries: []templatesv1.ResourceRef{ref},
@@ -575,7 +575,7 @@ func TestReconciliation(t *testing.T) {
 		want := []runtime.Object{
 			wantUpdated,
 		}
-		assertInventoryHasItems(t, updated, want...)
+		test.AssertInventoryHasItems(t, updated, want...)
 
 		kustomization := &unstructured.Unstructured{}
 		kustomization.SetGroupVersionKind(kustomizationGVK)
@@ -927,29 +927,6 @@ func assertGitOpsSetCondition(t *testing.T, gs *templatesv1.GitOpsSet, condType,
 	}
 }
 
-func assertInventoryHasItems(t *testing.T, gs *templatesv1.GitOpsSet, objs ...runtime.Object) {
-	t.Helper()
-	if l := len(gs.Status.Inventory.Entries); l != len(objs) {
-		t.Errorf("expected %d items, got %v", len(objs), l)
-	}
-	entries := []templatesv1.ResourceRef{}
-	for _, obj := range objs {
-		ref, err := resourceRefFromObject(obj)
-		if err != nil {
-			t.Fatal(err)
-		}
-		entries = append(entries, ref)
-	}
-
-	sort.Slice(entries, func(i, j int) bool {
-		return entries[i].ID < entries[j].ID
-	})
-	want := &templatesv1.ResourceInventory{Entries: entries}
-	if diff := cmp.Diff(want, gs.Status.Inventory); diff != "" {
-		t.Errorf("failed to get inventory:\n%s", diff)
-	}
-}
-
 func assertInventoryHasNoItems(t *testing.T, gs *templatesv1.GitOpsSet) {
 	t.Helper()
 	if gs.Status.Inventory == nil {

controllers/templates/namespaced_object.go

@@ -0,0 +1,19 @@
+package templates
+
+import (
+	"k8s.io/apimachinery/pkg/runtime"
+)
+
+// IsNamespacedObject returns true if the provided Object's Kind is a resource
+// that is Namespaced.
+//
+// TODO: This should get the CRD for custom CRs but this requires privilege.
+// https://kubernetes.io/docs/reference/using-api/api-concepts/#resource-uris
+func IsNamespacedObject(obj runtime.Object) bool {
+	return kind(obj) != "Namespace"
+
+}
+
+func kind(o runtime.Object) string {
+	return o.GetObjectKind().GroupVersionKind().Kind
+}

controllers/templates/namespaced_object_test.go

@@ -0,0 +1,32 @@
+package templates
+
+import (
+	"fmt"
+	"testing"
+
+	"k8s.io/apimachinery/pkg/runtime"
+)
+
+func TestIsNamespacedObject(t *testing.T) {
+	nsTests := []struct {
+		obj  runtime.Object
+		want bool
+	}{
+		{
+			obj:  makeTestNamespace("testing"),
+			want: false,
+		},
+		{
+			obj:  makeTestService(nsn("demo", "engineering-dev-demo1")),
+			want: true,
+		},
+	}
+
+	for _, tt := range nsTests {
+		t.Run(fmt.Sprintf("Kind %s", kind(tt.obj)), func(t *testing.T) {
+			if is := IsNamespacedObject(tt.obj); is != tt.want {
+				t.Fatalf("IsNamespacedObject(%s) got %v, want %v", kind(tt.obj), is, tt.want)
+			}
+		})
+	}
+}

controllers/templates/renderer.go

@@ -132,7 +132,7 @@ func renderTemplateParams(tmpl templatesv1.GitOpsSetTemplate, params map[string]
 			delete(unstructuredMap, "status")
 			uns := &unstructured.Unstructured{Object: unstructuredMap}
 
-			if uns.GetKind() != "Namespace" {
+			if IsNamespacedObject(uns) {
 				uns.SetNamespace(name.Namespace)
 
 				// Add source labels

test/inventory.go

@@ -0,0 +1,36 @@
+package test
+
+import (
+	"sort"
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	"k8s.io/apimachinery/pkg/runtime"
+
+	templatesv1 "github.com/weaveworks/gitopssets-controller/api/v1alpha1"
+)
+
+// AssertInventoryHasItems will ensure that each of the provided objects is
+// listed in the Inventory of the provided GitOpsSet.
+func AssertInventoryHasItems(t *testing.T, gs *templatesv1.GitOpsSet, objs ...runtime.Object) {
+	t.Helper()
+	if l := len(gs.Status.Inventory.Entries); l != len(objs) {
+		t.Errorf("expected %d items, got %v", len(objs), l)
+	}
+	entries := []templatesv1.ResourceRef{}
+	for _, obj := range objs {
+		ref, err := templatesv1.ResourceRefFromObject(obj)
+		if err != nil {
+			t.Fatal(err)
+		}
+		entries = append(entries, ref)
+	}
+
+	sort.Slice(entries, func(i, j int) bool {
+		return entries[i].ID < entries[j].ID
+	})
+	want := &templatesv1.ResourceInventory{Entries: entries}
+	if diff := cmp.Diff(want, gs.Status.Inventory); diff != "" {
+		t.Errorf("failed to get inventory:\n%s", diff)
+	}
+}