Fix bug in cluster scoped ansible operators (#4850)

Dependent object creation passing through the proxy failed to add an
owner annotation, causing the objects not to be watched.

Signed-off-by: austin <[email protected]>

Author: asmacdo

changelog/fragments/ansible-fix-cluster-annotations.yaml

@@ -0,0 +1,41 @@
+# entries is a list of entries to include in
+# release notes and/or the migration guide
+entries:
+  - description: >
+      For Anible-based operators, fixed a bug that caused to be unable watch
+      cluster scoped dependent resources and dependent resources in other
+      namespaces.
+
+    # kind is one of:
+    # - addition
+    # - change
+    # - deprecation
+    # - removal
+    # - bugfix
+    kind: "bugfix"
+
+    # Is this a breaking change?
+    breaking: false
+
+    # NOTE: ONLY USE `pull_request_override` WHEN ADDING THIS
+    # FILE FOR A PREVIOUSLY MERGED PULL_REQUEST!
+    #
+    # The generator auto-detects the PR number from the commit
+    # message in which this file was originally added.
+    #
+    # What is the pull request number (without the "#")?
+    # pull_request_override: 0
+
+
+    # Migration can be defined to automatically add a section to
+    # the migration guide. This is required for breaking changes.
+    migration:
+      header: Ensure that existing dependent resources have owner annotations
+      body: >
+        Ansible-based operators use owner references (for resources in the same
+        namespace) and owner annotations(for cluster scoped resources and
+        resources in other namespaces). If dependent resources were created
+        without the references/annotations, they cannot be cleaned up by the
+        operator, and update/deletes will not trigger a reconcile. Follow these
+        instructions to add owner references/annotations to existing objects.
+        https://sdk.operatorframework.io/docs/building-operators/ansible/reference/retroactively-owned-resources/

hack/generate/samples/internal/ansible/advanced_molecule.go

@@ -134,6 +134,7 @@ func (ma *AdvancedMolecule) updateConfig() {
       - ""
     resources:
       - configmaps
+      - namespaces
     verbs:
       - create
       - delete
@@ -479,19 +480,56 @@ func (ma *AdvancedMolecule) updatePlaybooks() {
 		originalPlaybookFragment,
 		subresourcesPlaybook)
 	pkg.CheckError("adding playbook for subresourcestest", err)
+
+	log.Infof("adding playbook for clusterannotationtest")
+	const clusterAnnotationTest = `---
+- hosts: localhost
+  gather_facts: no
+  collections:
+    - community.kubernetes
+  tasks:
+
+    - name: create externalnamespace
+      k8s:
+        name: "externalnamespace"
+        api_version: v1
+        kind: "Namespace"
+        definition:
+          metadata:
+            labels:
+              foo: bar
+
+    - name: create configmap
+      k8s:
+        definition:
+          apiVersion: v1
+          kind: ConfigMap
+          metadata:
+            namespace: "externalnamespace"
+            name: '{{ meta.name }}'
+          data:
+            foo: bar
+`
+	err = util.ReplaceInFile(
+		filepath.Join(ma.ctx.Dir, "playbooks", "clusterannotationtest.yml"),
+		originalPlaybookFragment,
+		clusterAnnotationTest)
+	pkg.CheckError("adding playbook for clusterannotationtest", err)
+
 }
 
 func (ma *AdvancedMolecule) addPlaybooks() {
 	allPlaybookKinds := []string{
 		"ArgsTest",
 		"CaseTest",
 		"CollectionTest",
+		"ClusterAnnotationTest",
 		"ReconciliationTest",
 		"SelectorTest",
 		"SubresourcesTest",
 	}
 
-	// Crate API
+	// Create API
 	for _, k := range allPlaybookKinds {
 		logMsgForKind := fmt.Sprintf("creating an API %s", k)
 		log.Infof(logMsgForKind)

hack/generate/samples/internal/ansible/testdata/tasks/clusterannotationtest_test.yml

@@ -0,0 +1,51 @@
+---
+- name: Create the test.example.com/v1alpha1.ClusterAnnotationTest
+  k8s:
+    state: present
+    namespace: '{{ namespace }}'
+    definition: "{{ lookup('template', '/'.join([samples_dir, cr_file])) | from_yaml }}"
+    wait: yes
+    wait_timeout: 300
+    wait_condition:
+      type: Running
+      reason: Successful
+      status: "True"
+  vars:
+    cr_file: 'test_v1alpha1_clusterannotationtest.yaml'
+
+- name: retrieve configmap
+  k8s_info:
+    api_version: v1
+    kind: ConfigMap
+    namespace: "externalnamespace"
+    name: "clusterannotationtest-sample"
+  register: configmap
+  until: (configmap.resources | length) == 1
+
+- assert:
+    that:
+      - configmap.resources[0].metadata.annotations["operator-sdk/primary-resource"] == primary
+      - configmap.resources[0].metadata.annotations["operator-sdk/primary-resource-type"] == primary_type
+  vars:
+    primary: "osdk-test/clusterannotationtest-sample"
+    primary_type: "ClusterAnnotationTest.test.example.com"
+
+- name: change the namespace labels
+  k8s:
+    name: "externalnamespace"
+    api_version: v1
+    kind: "Namespace"
+    wait: yes
+    definition:
+      metadata:
+        labels:
+          foo: baz
+
+- name: Make sure the label is changed back
+  k8s_info:
+    api_version: v1
+    kind: Namespace
+    name: "externalnamespace"
+  register: external_namespace
+  until: external_namespace.resources[0].metadata.labels["foo"] == "bar"
+

hack/generate/samples/internal/ansible/testdata/watches.yaml

@@ -60,4 +60,12 @@
   playbook: playbooks/reconciliationtest.yml
   vars:
     meta: '{{ ansible_operator_meta }}'
+
+- version: v1alpha1
+  group: test.example.com
+  kind: ClusterAnnotationTest
+  playbook: playbooks/clusterannotationtest.yml
+  watchClusterScopedResources: true
+  vars:
+    meta: '{{ ansible_operator_meta }}'
 #+kubebuilder:scaffold:watch

internal/ansible/proxy/inject_owner.go

@@ -146,7 +146,7 @@ func (i *injectOwnerReferenceHandler) ServeHTTP(w http.ResponseWriter, req *http
 			if addOwnerRef {
 				data.SetOwnerReferences(append(data.GetOwnerReferences(), owner.OwnerReference))
 			} else {
-				err := handler.SetOwnerAnnotations(data, ownerObject)
+				err := handler.SetOwnerAnnotations(ownerObject, data)
 				if err != nil {
 					m := "Could not set owner annotations"
 					log.Error(err, m)