feat: enable metadata to be set on namespaces (argoproj#10672)

* namespace labels Signed-off-by: pashavictorovich <[email protected]> * create namespace should support annotations Signed-off-by: pashavictorovich <[email protected]> * handle also modification hook Signed-off-by: pashavictorovich <[email protected]> * regenerate entity on modify hook Signed-off-by: pashavictorovich <[email protected]> * manifests Signed-off-by: pashavictorovich <[email protected]> * feat: enable metadata to be set on namespaces This builds upon the work that @pasha-codefresh did in argoproj#10288. The main differences between this PR and the previous one is that we use SSA to diff between different versions of the namespace, as well as having a slightly different API in gitops-engine for setting the namespace modifier. We now also set the ownership of the namespace in ArgoCD. Closes argoproj#4628 Closes argoproj#6215 Closes argoproj#7799 Signed-off-by: Blake Pettersson <[email protected]> * fix: don't always track namespaces For now, only allow namespaces managed with `managedNamespaceMetadata` to have tracking set by Argo. Ideally we'd like new namespaces to also be tracked by Argo, but there's currently an issue with a failing integration test. Also wrap error message if setting the app instance errors on the namespace. Signed-off-by: Blake Pettersson <[email protected]> * fix: always return true with `hasManagedMetadata` If `hasManagedMetadata` is set, `true` should always be returned. Signed-off-by: Blake Pettersson <[email protected]> * docs: add clarifying docs on resource tracking Signed-off-by: Blake Pettersson <[email protected]> * style: pr tweaks Signed-off-by: Blake Pettersson <[email protected]> * fix: re-add label unsetting Signed-off-by: Blake Pettersson <[email protected]> * Update gitops-engine to current master Signed-off-by: Leonardo Luz Almeida <[email protected]> Signed-off-by: pashavictorovich <[email protected]> Signed-off-by: Blake Pettersson <[email protected]> Signed-off-by: Leonardo Luz Almeida <[email protected]> Co-authored-by: pashavictorovich <[email protected]> Co-authored-by: Leonardo Luz Almeida <[email protected]>
tkasuz · Nov 4, 2022 · 7773021 · 7773021
1 parent ebf2682
commit 7773021
Show file tree

Hide file tree

Showing 25 changed files with 3,198 additions and 735 deletions.
diff --git a/assets/swagger.json b/assets/swagger.json
@@ -6544,6 +6544,23 @@
         }
       }
     },
+    "v1alpha1ManagedNamespaceMetadata": {
+      "type": "object",
+      "properties": {
+        "annotations": {
+          "type": "object",
+          "additionalProperties": {
+            "type": "string"
+          }
+        },
+        "labels": {
+          "type": "object",
+          "additionalProperties": {
+            "type": "string"
+          }
+        }
+      }
+    },
     "v1alpha1MatrixGenerator": {
       "description": "MatrixGenerator generates the cartesian product of two sets of parameters. The parameters are defined by two nested\ngenerators.",
       "type": "object",
@@ -7827,6 +7844,9 @@
         "automated": {
           "$ref": "#/definitions/v1alpha1SyncPolicyAutomated"
         },
+        "managedNamespaceMetadata": {
+          "$ref": "#/definitions/v1alpha1ManagedNamespaceMetadata"
+        },
         "retry": {
           "$ref": "#/definitions/v1alpha1RetryStrategy"
         },

diff --git a/controller/sync.go b/controller/sync.go
@@ -4,6 +4,7 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	cdcommon "github.com/argoproj/argo-cd/v2/common"
 	"os"
 	"strconv"
 	"sync/atomic"
@@ -20,7 +21,6 @@ import (
 	"k8s.io/apimachinery/pkg/util/managedfields"
 	"k8s.io/kubectl/pkg/util/openapi"
 
-	cdcommon "github.com/argoproj/argo-cd/v2/common"
 	"github.com/argoproj/argo-cd/v2/controller/metrics"
 	"github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
 	listersv1alpha1 "github.com/argoproj/argo-cd/v2/pkg/client/listers/application/v1alpha1"
@@ -212,14 +212,7 @@ func (m *appStateManager) SyncAppState(app *v1alpha1.Application, state *v1alpha
 	}
 	trackingMethod := argo.GetTrackingMethod(m.settingsMgr)
 
-	syncCtx, cleanup, err := sync.NewSyncContext(
-		compareResult.syncStatus.Revision,
-		reconciliationResult,
-		restConfig,
-		rawConfig,
-		m.kubectl,
-		app.Spec.Destination.Namespace,
-		openAPISchema,
+	opts := []sync.SyncOpt{
 		sync.WithLogr(logutils.NewLogrusLogger(logEntry)),
 		sync.WithHealthOverride(lua.ResourceHealthOverrides(resourceOverrides)),
 		sync.WithPermissionValidator(func(un *unstructured.Unstructured, res *v1.APIResource) error {
@@ -249,20 +242,28 @@ func (m *appStateManager) SyncAppState(app *v1alpha1.Application, state *v1alpha
 				m.isSelfReferencedObj(live, target, app.GetName(), appLabelKey, trackingMethod)
 		}),
 		sync.WithManifestValidation(!syncOp.SyncOptions.HasOption(common.SyncOptionsDisableValidation)),
-		sync.WithNamespaceCreation(syncOp.SyncOptions.HasOption("CreateNamespace=true"), func(un *unstructured.Unstructured) bool {
-			if un != nil && kube.GetAppInstanceLabel(un, cdcommon.LabelKeyAppInstance) != "" {
-				kube.UnsetLabel(un, cdcommon.LabelKeyAppInstance)
-				return true
-			}
-			return false
-		}),
 		sync.WithSyncWaveHook(delayBetweenSyncWaves),
 		sync.WithPruneLast(syncOp.SyncOptions.HasOption(common.SyncOptionPruneLast)),
 		sync.WithResourceModificationChecker(syncOp.SyncOptions.HasOption("ApplyOutOfSyncOnly=true"), compareResult.diffResultList),
 		sync.WithPrunePropagationPolicy(&prunePropagationPolicy),
 		sync.WithReplace(syncOp.SyncOptions.HasOption(common.SyncOptionReplace)),
 		sync.WithServerSideApply(syncOp.SyncOptions.HasOption(common.SyncOptionServerSideApply)),
 		sync.WithServerSideApplyManager(cdcommon.ArgoCDSSAManager),
+	}
+
+	if syncOp.SyncOptions.HasOption("CreateNamespace=true") {
+		opts = append(opts, sync.WithNamespaceModifier(syncNamespace(m.resourceTracking, appLabelKey, trackingMethod, app.Name, app.Spec.SyncPolicy)))
+	}
+
+	syncCtx, cleanup, err := sync.NewSyncContext(
+		compareResult.syncStatus.Revision,
+		reconciliationResult,
+		restConfig,
+		rawConfig,
+		m.kubectl,
+		app.Spec.Destination.Namespace,
+		openAPISchema,
+		opts...,
 	)
 
 	if err != nil {

diff --git a/controller/sync_namespace.go b/controller/sync_namespace.go
@@ -0,0 +1,51 @@
+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
+		}
+
+		isNewNamespace := liveNs != nil && liveNs.GetUID() == "" && liveNs.GetResourceVersion() == ""
+
+		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)
+				}
+
+				return true, nil
+			}
+		}
+
+		return isNewNamespace, nil
+	}
+}
+
+func appendSSAAnnotation(in map[string]string) map[string]string {
+	r := map[string]string{}
+	for k, v := range in {
+		r[k] = v
+	}
+	r[gitopscommon.AnnotationSyncOptions] = gitopscommon.SyncOptionServerSideApply
+	return r
+}