Merge pull request #634 from mbaldessari/small-fixes

A couple of fixes

Author: mbaldessari

internal/controller/argo.go

@@ -469,7 +469,7 @@ func createOrUpdateArgoCD(client dynamic.Interface, fullClient kubernetes.Interf
 		newArgo := &unstructured.Unstructured{Object: obj}
 		_, err = client.Resource(gvr).Namespace(namespace).Create(context.TODO(), newArgo, metav1.CreateOptions{})
 	} else { // update it
-		oldArgo, errGet := getArgoCDFunc(client, name, namespace)
+		oldArgo, oldUnstructured, errGet := getArgoCDFunc(client, name, namespace)
 		if errGet != nil {
 			return fmt.Errorf("failed to get existing ArgoCD %s/%s: %v", namespace, name, errGet)
 		}
@@ -483,14 +483,11 @@ func createOrUpdateArgoCD(client dynamic.Interface, fullClient kubernetes.Interf
 		// Preserve spec fields not known to this vendored argocd-operator version
 		// (e.g. networkPolicy added in gitops-operator v1.20.3) to avoid
 		// stripping them and causing infinite reconciliation loops.
-		oldUnstructured, errGet2 := client.Resource(gvr).Namespace(namespace).Get(context.TODO(), name, metav1.GetOptions{})
-		if errGet2 == nil {
-			oldSpec, _, _ := unstructured.NestedMap(oldUnstructured.Object, "spec")
-			newSpec, _, _ := unstructured.NestedMap(newArgo.Object, "spec")
-			for key, val := range oldSpec {
-				if _, exists := newSpec[key]; !exists {
-					_ = unstructured.SetNestedField(newArgo.Object, val, "spec", key)
-				}
+		oldSpec, _, _ := unstructured.NestedMap(oldUnstructured.Object, "spec")
+		newSpec, _, _ := unstructured.NestedMap(newArgo.Object, "spec")
+		for key, val := range oldSpec {
+			if _, exists := newSpec[key]; !exists {
+				_ = unstructured.SetNestedField(newArgo.Object, val, "spec", key)
 			}
 		}
 
@@ -555,15 +552,15 @@ func removeConsoleLink(client dynamic.Interface, argoName string) error {
 
 var getArgoCDFunc = getArgoCD
 
-func getArgoCD(client dynamic.Interface, name, namespace string) (*argooperator.ArgoCD, error) {
+func getArgoCD(client dynamic.Interface, name, namespace string) (*argooperator.ArgoCD, *unstructured.Unstructured, error) {
 	gvr := schema.GroupVersionResource{Group: ArgoCDGroup, Version: ArgoCDVersion, Resource: ArgoCDResource}
 	argo := &argooperator.ArgoCD{}
 	unstructuredArgo, err := client.Resource(gvr).Namespace(namespace).Get(context.TODO(), name, metav1.GetOptions{})
 	if err != nil {
-		return nil, err
+		return nil, nil, err
 	}
 	err = runtime.DefaultUnstructuredConverter.FromUnstructured(unstructuredArgo.UnstructuredContent(), argo)
-	return argo, err
+	return argo, unstructuredArgo, err
 }
 
 func newApplicationParameters(p *api.Pattern) []argoapi.HelmParameter {

internal/controller/argo_test.go

@@ -1020,52 +1020,52 @@ var _ = Describe("CreateOrUpdateArgoCD", func() {
 		})
 	})
 
-    Context("when the ArgoCD instance exists with additional spec fields not managed by patterns-operator", func() {
-    BeforeEach(func() {
-        argoCD := &unstructured.Unstructured{
-            Object: map[string]any{
-                "apiVersion": "argoproj.io/v1beta1",
-                "kind":       "ArgoCD",
-                "metadata": map[string]any{
-                    "name":            name,
-                    "namespace":       namespace,
-                    "resourceVersion": "1",
-                },
-                "spec": map[string]any{
-                    "networkPolicy": map[string]any{
-                        "enabled": true,
-                    },
-                    "imageUpdater": map[string]any{
-                        "enabled": false,
-                    },
-                },
-            },
-        }
-        _, err := dynamicClient.Resource(gvr).Namespace(namespace).Create(context.TODO(), argoCD, metav1.CreateOptions{})
-        Expect(err).ToNot(HaveOccurred())
-    })
-
-    It("should preserve spec fields not managed by patterns-operator during update", func() {
-        err := createOrUpdateArgoCD(dynamicClient, nil, name, namespace, patternsOperatorConfig)
-        Expect(err).ToNot(HaveOccurred())
-
-        argoCD, err := dynamicClient.Resource(gvr).Namespace(namespace).Get(context.TODO(), name, metav1.GetOptions{})
-        Expect(err).ToNot(HaveOccurred())
-
-        // networkPolicy should be preserved — it is managed by gitops-operator
-        // not by patterns-operator, so the update must not strip it
-        networkPolicy, found, err := unstructured.NestedMap(argoCD.Object, "spec", "networkPolicy")
-        Expect(err).ToNot(HaveOccurred())
-        Expect(found).To(BeTrue(), "networkPolicy should be preserved after update")
-        Expect(networkPolicy["enabled"]).To(BeTrue())
-
-        // imageUpdater should also be preserved
-        imageUpdater, found, err := unstructured.NestedMap(argoCD.Object, "spec", "imageUpdater")
-        Expect(err).ToNot(HaveOccurred())
-        Expect(found).To(BeTrue(), "imageUpdater should be preserved after update")
-        Expect(imageUpdater["enabled"]).To(BeFalse())
-    })
-})
+	Context("when the ArgoCD instance exists with additional spec fields not managed by patterns-operator", func() {
+		BeforeEach(func() {
+			argoCD := &unstructured.Unstructured{
+				Object: map[string]any{
+					"apiVersion": "argoproj.io/v1beta1",
+					"kind":       "ArgoCD",
+					"metadata": map[string]any{
+						"name":            name,
+						"namespace":       namespace,
+						"resourceVersion": "1",
+					},
+					"spec": map[string]any{
+						"networkPolicy": map[string]any{
+							"enabled": true,
+						},
+						"imageUpdater": map[string]any{
+							"enabled": false,
+						},
+					},
+				},
+			}
+			_, err := dynamicClient.Resource(gvr).Namespace(namespace).Create(context.TODO(), argoCD, metav1.CreateOptions{})
+			Expect(err).ToNot(HaveOccurred())
+		})
+
+		It("should preserve spec fields not managed by patterns-operator during update", func() {
+			err := createOrUpdateArgoCD(dynamicClient, nil, name, namespace, patternsOperatorConfig)
+			Expect(err).ToNot(HaveOccurred())
+
+			argoCD, err := dynamicClient.Resource(gvr).Namespace(namespace).Get(context.TODO(), name, metav1.GetOptions{})
+			Expect(err).ToNot(HaveOccurred())
+
+			// networkPolicy should be preserved — it is managed by gitops-operator
+			// not by patterns-operator, so the update must not strip it
+			networkPolicy, found, err := unstructured.NestedMap(argoCD.Object, "spec", "networkPolicy")
+			Expect(err).ToNot(HaveOccurred())
+			Expect(found).To(BeTrue(), "networkPolicy should be preserved after update")
+			Expect(networkPolicy["enabled"]).To(BeTrue())
+
+			// imageUpdater should also be preserved
+			imageUpdater, found, err := unstructured.NestedMap(argoCD.Object, "spec", "imageUpdater")
+			Expect(err).ToNot(HaveOccurred())
+			Expect(found).To(BeTrue(), "imageUpdater should be preserved after update")
+			Expect(imageUpdater["enabled"]).To(BeFalse())
+		})
+	})
 
 	Context("when there is an error in the getArgoCD fn but there is an argocd", func() {
 
@@ -1083,8 +1083,8 @@ var _ = Describe("CreateOrUpdateArgoCD", func() {
 			}
 			_, err := dynamicClient.Resource(gvr).Namespace(namespace).Create(context.TODO(), argoCD, metav1.CreateOptions{})
 			Expect(err).ToNot(HaveOccurred())
-			getArgoCDFunc = func(_ dynamic.Interface, _, _ string) (*argooperator.ArgoCD, error) {
-				return nil, fmt.Errorf("forced error")
+			getArgoCDFunc = func(_ dynamic.Interface, _, _ string) (*argooperator.ArgoCD, *unstructured.Unstructured, error) {
+				return nil, nil, fmt.Errorf("forced error")
 			}
 		})