fix: do not mutate live when managed namespace enabled (argoproj#11197)

* fix: do not mutate live when managed namespace enabled Signed-off-by: Leonardo Luz Almeida <[email protected]> * fix unit-test Signed-off-by: Leonardo Luz Almeida <[email protected]> * fix unit-test Signed-off-by: Leonardo Luz Almeida <[email protected]> * Fix lint Signed-off-by: Leonardo Luz Almeida <[email protected]> * remove trackingID from e2e test validation Signed-off-by: Leonardo Luz Almeida <[email protected]> * fix e2e Signed-off-by: Leonardo Luz Almeida <[email protected]> * remove unnecessary config Signed-off-by: Leonardo Luz Almeida <[email protected]> Signed-off-by: Leonardo Luz Almeida <[email protected]>
chrevanthreddy · Nov 8, 2022 · fe151a1 · fe151a1
1 parent 149926d
commit fe151a1
Show file tree

Hide file tree

Showing 5 changed files with 92 additions and 137 deletions.
diff --git a/controller/sync_namespace.go b/controller/sync_namespace.go
@@ -1,46 +1,50 @@
 package controller
 
 import (
-	"fmt"
-	cdcommon "github.com/argoproj/argo-cd/v2/common"
 	"github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
 	"github.com/argoproj/argo-cd/v2/util/argo"
 	gitopscommon "github.com/argoproj/gitops-engine/pkg/sync/common"
-	"github.com/argoproj/gitops-engine/pkg/utils/kube"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 )
 
-func syncNamespace(resourceTracking argo.ResourceTracking, appLabelKey string, trackingMethod v1alpha1.TrackingMethod, appName string, syncPolicy *v1alpha1.SyncPolicy) func(un *unstructured.Unstructured) (bool, error) {
-	return func(liveNs *unstructured.Unstructured) (bool, error) {
-		if liveNs != nil && kube.GetAppInstanceLabel(liveNs, cdcommon.LabelKeyAppInstance) != "" {
-			kube.UnsetLabel(liveNs, cdcommon.LabelKeyAppInstance)
-			return true, nil
+// syncNamespace determine if Argo CD should create and/or manage the namespace
+// where the application will be deployed.
+func syncNamespace(resourceTracking argo.ResourceTracking, appLabelKey string, trackingMethod v1alpha1.TrackingMethod, appName string, syncPolicy *v1alpha1.SyncPolicy) func(m, l *unstructured.Unstructured) (bool, error) {
+	// This function must return true for the managed namespace to be synced.
+	return func(managedNs, liveNs *unstructured.Unstructured) (bool, error) {
+		if managedNs == nil {
+			return false, nil
 		}
 
-		isNewNamespace := liveNs != nil && liveNs.GetUID() == "" && liveNs.GetResourceVersion() == ""
+		isNewNamespace := liveNs == nil
+		isManagedNamespace := syncPolicy != nil && syncPolicy.ManagedNamespaceMetadata != nil
 
-		if liveNs != nil && syncPolicy != nil {
-			// managedNamespaceMetadata relies on SSA, and since the diffs are computed by the k8s control plane we
-			// always need to call the k8s api server, so we'll always need to return true if managedNamespaceMetadata is set.
-			hasManagedMetadata := syncPolicy.ManagedNamespaceMetadata != nil
-			if hasManagedMetadata {
-				managedNamespaceMetadata := syncPolicy.ManagedNamespaceMetadata
-				liveNs.SetLabels(managedNamespaceMetadata.Labels)
-				liveNs.SetAnnotations(appendSSAAnnotation(managedNamespaceMetadata.Annotations))
-
-				err := resourceTracking.SetAppInstance(liveNs, appLabelKey, appName, "", trackingMethod)
-				if err != nil {
-					return false, fmt.Errorf("failed to set app instance tracking on the namespace %s: %s", liveNs.GetName(), err)
-				}
+		// should only sync the namespace if it doesn't exist in k8s or if
+		// syncPolicy is defined to manage the metadata
+		if !isManagedNamespace && !isNewNamespace {
+			return false, nil
+		}
 
-				return true, nil
-			}
+		if isManagedNamespace {
+			managedNamespaceMetadata := syncPolicy.ManagedNamespaceMetadata
+			managedNs.SetLabels(managedNamespaceMetadata.Labels)
+			// managedNamespaceMetadata relies on SSA in order to avoid overriding
+			// existing labels and annotations in namespaces
+			managedNs.SetAnnotations(appendSSAAnnotation(managedNamespaceMetadata.Annotations))
 		}
 
-		return isNewNamespace, nil
+		// TODO: https://github.com/argoproj/argo-cd/issues/11196
+		// err := resourceTracking.SetAppInstance(managedNs, appLabelKey, appName, "", trackingMethod)
+		// if err != nil {
+		// 	return false, fmt.Errorf("failed to set app instance tracking on the namespace %s: %s", managedNs.GetName(), err)
+		// }
+
+		return true, nil
 	}
 }
 
+// appendSSAAnnotation will set the managed namespace to be synced
+// with server-side apply
 func appendSSAAnnotation(in map[string]string) map[string]string {
 	r := map[string]string{}
 	for k, v := range in {