fix: apply ssa namespace twice

go.mod

@@ -10,7 +10,7 @@ require (
 	github.com/google/gnostic v0.5.7-v3refs
 	github.com/spf13/cobra v1.6.0
 	github.com/stretchr/testify v1.8.1
-	golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4
+	golang.org/x/sync v0.1.0
 	google.golang.org/protobuf v1.28.1
 	k8s.io/api v0.26.4
 	k8s.io/apiextensions-apiserver v0.26.4

go.sum

@@ -410,8 +410,8 @@ golang.org/x/sync v0.0.0-20200625203802-6e8e738ad208/go.mod h1:RxMgew5VJxzue5/jJ
 golang.org/x/sync v0.0.0-20201020160332-67f06af15bc9/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
 golang.org/x/sync v0.0.0-20201207232520-09787c993a3a/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
 golang.org/x/sync v0.0.0-20210220032951-036812b2e83c/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
-golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4 h1:uVc8UZUe6tr40fFVnUP5Oj+veunVezqYl9z7DYw9xzw=
-golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
+golang.org/x/sync v0.1.0 h1:wsuoTGHzEhffawBOhz5CYhcrV4IdKZbEyZjBMuTp12o=
+golang.org/x/sync v0.1.0/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
 golang.org/x/sys v0.0.0-20180830151530-49385e6e1522/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
 golang.org/x/sys v0.0.0-20180905080454-ebe1bf3edb33/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
 golang.org/x/sys v0.0.0-20181116152217-5ac8a444bdc5/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=

pkg/sync/sync_context.go

@@ -397,7 +397,7 @@ func (sc *syncContext) Sync() {
 	} else {
 		// Perform a `kubectl apply --dry-run` against all the manifests. This will detect most (but
 		// not all) validation issues with the user's manifests (e.g. will detect syntax issues, but
-		// will not not detect if they are mutating immutable fields). If anything fails, we will refuse
+		// will not detect if they are mutating immutable fields). If anything fails, we will refuse
 		// to perform the sync. we only wish to do this once per operation, performing additional dry-runs
 		// is harmless, but redundant. The indicator we use to detect if we have already performed
 		// the dry-run for this operation, is if the resource or hook list is empty.
@@ -934,6 +934,23 @@ func (sc *syncContext) applyObject(t *syncTask, dryRun, force, validate bool) (c
 			message, err = sc.resourceOps.CreateResource(context.TODO(), t.targetObj, dryRunStrategy, validate)
 		}
 	} else {
+		targetObjIsSSA := resourceutil.HasAnnotationOption(t.targetObj, common.AnnotationSyncOptions, common.SyncOptionServerSideApply)
+		liveObjIsNotSSA := t.liveObj != nil && !resourceutil.HasAnnotationOption(t.liveObj, common.AnnotationSyncOptions, common.SyncOptionServerSideApply)
+		if t.targetObj.GetKind() == kube.NamespaceKind && t.liveObj != nil && t.liveObj.GetKind() == kube.NamespaceKind && targetObjIsSSA && liveObjIsNotSSA {
+			// If there is a pre-existing live namespace, and if we also want to use managedNamespaceMetadata on an existing namespace,
+			// we need to do `kubectl apply` with the namespace as-is, using a different manager than the default one.
+			// The reason we do this is so that we can preserve existing labels and annotations on the namespace.
+			nsSpec := &v1.Namespace{TypeMeta: metav1.TypeMeta{APIVersion: "v1", Kind: kube.NamespaceKind}, ObjectMeta: metav1.ObjectMeta{Name: t.liveObj.GetName(), Annotations: t.liveObj.GetAnnotations(), Labels: t.liveObj.GetLabels(), Namespace: t.liveObj.GetNamespace()}}
+			liveCopy, err := kube.ToUnstructured(nsSpec)
+			if err != nil {
+				return common.ResultCodeSyncFailed, "failed to convert namespace spec to unstructured: " + err.Error()
+			}
+
+			_, err = sc.resourceOps.ApplyResource(context.TODO(), liveCopy, dryRunStrategy, force, validate, serverSideApply, "argocd-controller-tmp")
+			if err != nil {
+				return common.ResultCodeSyncFailed, "failed to apply existing namespace with temp field manager: " + err.Error()
+			}
+		}
 		message, err = sc.resourceOps.ApplyResource(context.TODO(), t.targetObj, dryRunStrategy, force, validate, serverSideApply, sc.serverSideApplyManager)
 	}
 	if err != nil {

pkg/sync/sync_context_test.go

@@ -798,18 +798,20 @@ func withReplaceAndServerSideApplyAnnotations(un *unstructured.Unstructured) *un
 
 func TestSync_ServerSideApply(t *testing.T) {
 	testCases := []struct {
-		name            string
-		target          *unstructured.Unstructured
-		live            *unstructured.Unstructured
-		commandUsed     string
-		serverSideApply bool
-		manager         string
+		name             string
+		target           *unstructured.Unstructured
+		live             *unstructured.Unstructured
+		commandUsed      string
+		serverSideApply  bool
+		manager          string
+		expectedManagers []string
 	}{
-		{"NoAnnotation", NewPod(), NewPod(), "apply", false, "managerA"},
-		{"ServerSideApplyAnnotationIsSet", withServerSideApplyAnnotation(NewPod()), NewPod(), "apply", true, "managerB"},
-		{"ServerSideApplyAndReplaceAnnotationsAreSet", withReplaceAndServerSideApplyAnnotations(NewPod()), NewPod(), "replace", false, ""},
-		{"ServerSideApplyAndReplaceAnnotationsAreSetNamespace", withReplaceAndServerSideApplyAnnotations(NewNamespace()), NewNamespace(), "update", false, ""},
-		{"LiveObjectMissing", withReplaceAnnotation(NewPod()), nil, "create", false, ""},
+		{"NoAnnotation", NewPod(), NewPod(), "apply", false, "managerA", []string{"managerA", "managerA"}},
+		{"ServerSideApplyAnnotationIsSet", withServerSideApplyAnnotation(NewPod()), NewPod(), "apply", true, "managerB", []string{"managerB", "managerB"}},
+		{"ServerSideApplyAndReplaceAnnotationsAreSet", withReplaceAndServerSideApplyAnnotations(NewPod()), NewPod(), "replace", false, "", nil},
+		{"ServerSideApplyAndReplaceAnnotationsAreSetNamespace", withReplaceAndServerSideApplyAnnotations(NewNamespace()), NewNamespace(), "update", false, "", nil},
+		{"LiveObjectMissing", withReplaceAnnotation(NewPod()), nil, "create", false, "", nil},
+		{"ServerSideApplyWithExistingLiveNonSSAdNamespace", withServerSideApplyAnnotation(NewNamespace()), NewNamespace(), "apply", true, "managerA", []string{"argocd-controller-tmp", "managerA", "argocd-controller-tmp", "managerA"}},
 	}
 
 	for _, tc := range testCases {
@@ -834,6 +836,7 @@ func TestSync_ServerSideApply(t *testing.T) {
 			resourceOps, _ := syncCtx.resourceOps.(*kubetest.MockResourceOps)
 			assert.Equal(t, tc.commandUsed, resourceOps.GetLastResourceCommand(kube.GetResourceKey(tc.target)))
 			assert.Equal(t, tc.serverSideApply, resourceOps.GetLastServerSideApply())
+			assert.Equal(t, tc.expectedManagers, resourceOps.ServerSideApplyManagerCommands[kube.GetResourceKey(tc.target)])
 			assert.Equal(t, tc.manager, resourceOps.GetLastServerSideApplyManager())
 		})
 	}

pkg/utils/kube/kubetest/mock_resource_operations.go

@@ -14,9 +14,10 @@ import (
 )
 
 type MockResourceOps struct {
-	Commands      map[string]KubectlOutput
-	Events        chan watch.Event
-	DynamicClient dynamic.Interface
+	Commands                       map[string]KubectlOutput
+	ServerSideApplyManagerCommands map[kube.ResourceKey][]string
+	Events                         chan watch.Event
+	DynamicClient                  dynamic.Interface
 
 	lastCommandPerResource map[kube.ResourceKey]string
 	lastValidate           bool
@@ -73,6 +74,24 @@ func (r *MockResourceOps) SetLastServerSideApplyManager(manager string) {
 	r.recordLock.Unlock()
 }
 
+func (r *MockResourceOps) AddServerSideApplyManagerCommand(key kube.ResourceKey, manager string) {
+	r.recordLock.Lock()
+	if r.ServerSideApplyManagerCommands == nil {
+
+		r.ServerSideApplyManagerCommands = map[kube.ResourceKey][]string{}
+	}
+
+	strings := r.ServerSideApplyManagerCommands[key]
+
+	if strings == nil {
+		r.ServerSideApplyManagerCommands[key] = []string{manager}
+	} else {
+		r.ServerSideApplyManagerCommands[key] = append(r.ServerSideApplyManagerCommands[key], manager)
+	}
+
+	r.recordLock.Unlock()
+}
+
 func (r *MockResourceOps) SetLastResourceCommand(key kube.ResourceKey, cmd string) {
 	r.recordLock.Lock()
 	if r.lastCommandPerResource == nil {
@@ -96,6 +115,7 @@ func (r *MockResourceOps) ApplyResource(ctx context.Context, obj *unstructured.U
 	r.SetLastServerSideApply(serverSideApply)
 	r.SetLastServerSideApplyManager(manager)
 	r.SetLastResourceCommand(kube.GetResourceKey(obj), "apply")
+	r.AddServerSideApplyManagerCommand(kube.GetResourceKey(obj), manager)
 	command, ok := r.Commands[obj.GetName()]
 	if !ok {
 		return "", nil

pkg/utils/kube/scheme/scheme.go

@@ -1,6 +1,8 @@
 package scheme
 
 import (
+	// TODO: change to this below?
+	// "k8s.io/client-go/kubernetes/scheme"
 	"k8s.io/kubernetes/pkg/api/legacyscheme"
 
 	_ "k8s.io/kubernetes/pkg/apis/admission/install"

specs/design-top-down.md

@@ -115,7 +115,7 @@ The manifests generation is out of scope and should be implemented by the Engine
 
 ### Engine API
 
-> `Sync<Term>` is a place holder to a real name. The name is still under discussion
+> `Sync<Term>` is a placeholder to a real name. The name is still under discussion
 
 The engine API includes three main parts:
 - `Engine` golang interface