feat: Support for DeleteNamespace SyncOption(#4435, #7875)

docs/operator-manual/application.yaml

@@ -155,6 +155,7 @@ spec:
     syncOptions:     # Sync options which modifies sync behavior
     - Validate=false # disables resource validation (equivalent to 'kubectl apply --validate=false') ( true by default ).
     - CreateNamespace=true # Namespace Auto-Creation ensures that namespace specified as the application destination exists in the destination cluster.
+    - DeleteNamespace=true # Delete Auto-Created namespace. Ignored when CreateNamespace=true is not set.
     - PrunePropagationPolicy=foreground # Supported policies are background, foreground and orphan.
     - PruneLast=true # Allow the ability for resource pruning to happen as a final, implicit wave of a sync operation
     managedNamespaceMetadata: # Sets the metadata for the application namespace. Only valid if CreateNamespace=true (see above), otherwise it's a no-op.

docs/user-guide/sync-options.md

@@ -280,6 +280,13 @@ The example above shows how an Argo CD Application can be configured so it will
 
 Note that the namespace to be created must be informed in the `spec.destination.namespace` field of the Application resource. The `metadata.namespace` field in the Application's child manifests must match this value, or can be omitted, so resources are created in the proper destination.
 
+### Delete Namespace
+
+If `DeleteNamespace=true` is set, we can delete an auto-created namespace when an application is deleted.
+Note that the namespace is deleted only when both `CreateNamespace=true` and `DeleteNamespace=true` are set.  
+In addition, the namespace must have a label or an annotation to indicate the application tracks the namespace [see [resource tracking](resource_tracking.md)].  
+Otherwise, the application is deleted, but the namespace remains throwing error.
+
 ### Namespace Metadata
 
 We can also add labels and annotations to the namespace through `managedNamespaceMetadata`. If we extend the example above

server/application/application.go

@@ -1000,6 +1000,19 @@ func (s *Server) Delete(ctx context.Context, q *application.ApplicationDeleteReq
 		return nil, fmt.Errorf("error deleting application: %w", err)
 	}
 	s.logAppEvent(a, ctx, argo.EventReasonResourceDeleted, "deleted application")
+
+	// Delete a namespace when both CreateNamespace and DeleteNamespace are set to true.
+	if a.Spec.SyncPolicy != nil {
+		syncOptions := a.Spec.SyncPolicy.SyncOptions
+		if syncOptions.HasOption("CreateNamespace=true") && syncOptions.HasOption("DeleteNamespace=true") {
+			err := s.deleteNamespace(ctx, a.Name, a.Spec.Destination.Namespace, metav1.DeleteOptions{})
+			if err != nil {
+				return nil, fmt.Errorf("error deleting namespace: %w", err)
+			}
+			s.logAppEvent(a, ctx, argo.EventReasonResourceDeleted, fmt.Sprintf("deleted namespace: %s", a.Spec.Destination.Namespace))
+		}
+	}
+
 	return &application.ApplicationResponse{}, nil
 }
 
@@ -2353,3 +2366,41 @@ func (s *Server) appNamespaceOrDefault(appNs string) string {
 func (s *Server) isNamespaceEnabled(namespace string) bool {
 	return security.IsNamespaceEnabled(namespace, s.ns, s.enabledNamespaces)
 }
+
+// The namespace can only be deleted when the namespace has an appropriate label or annotation which shows the namespace is tracked by the application.
+func (s *Server) deleteNamespace(ctx context.Context, appName, namespace string, opts metav1.DeleteOptions) error {
+	canDelete, err := s.isNamespaceTrackedByApplication(ctx, appName, namespace, metav1.GetOptions{})
+	if err != nil {
+		return err
+	}
+
+	if canDelete {
+		err := s.kubeclientset.CoreV1().Namespaces().Delete(ctx, namespace, metav1.DeleteOptions{})
+		return err
+	} else {
+		return fmt.Errorf("error cannot delete namespace %s which is not managed by %s", namespace, appName)
+	}
+}
+
+func (s *Server) isNamespaceTrackedByApplication(ctx context.Context, appName, namespace string, opts metav1.GetOptions) (bool, error) {
+	appLabelKey, err := s.settingsMgr.GetAppInstanceLabelKey()
+	if err != nil {
+		log.Errorf("Could not get appInstanceLabelKey: %v", err)
+		return false, err
+	}
+
+	ns, err := s.kubeclientset.CoreV1().Namespaces().Get(ctx, namespace, opts)
+	if err != nil {
+		return false, err
+	}
+
+	unstructedNs, err := kube.ToUnstructured(ns)
+	if err != nil {
+		return false, err
+	}
+
+	trackingMethod := argo.GetTrackingMethod(s.settingsMgr)
+	nsOwner := argo.NewResourceTracking().GetAppName(unstructedNs, appLabelKey, trackingMethod)
+
+	return nsOwner == appName, nil
+}

server/application/application_test.go

@@ -25,6 +25,7 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/watch"
 	"k8s.io/client-go/kubernetes/fake"
+	k8sfake "k8s.io/client-go/kubernetes/fake"
 	kubetesting "k8s.io/client-go/testing"
 	k8scache "k8s.io/client-go/tools/cache"
 	"k8s.io/utils/pointer"
@@ -277,6 +278,32 @@ spec:
     server: https://cluster-api.com
 `
 
+const fakeAppWithDeleteNamespace = `
+apiVersion: argoproj.io/v1alpha1
+kind: Application
+metadata:
+  name: test-app
+  namespace: default
+spec:
+  source:
+    path: some/path
+    repoURL: https://github.com/argoproj/argocd-example-apps.git
+    targetRevision: HEAD
+    ksonnet:
+      environment: default
+  destination:
+    namespace: ` + test.FakeDestNamespace + `
+    name: fake-cluster
+  syncPolicy:
+    syncOptions:     # Sync options which modifies sync behavior
+    - CreateNamespace=true
+    - DeleteNamespace=true
+`
+
+func newTestAppWithDeleteNamespace(opts ...func(app *appsv1.Application)) *appsv1.Application {
+	return createTestApp(fakeAppWithDeleteNamespace, opts...)
+}
+
 func newTestAppWithDestName(opts ...func(app *appsv1.Application)) *appsv1.Application {
 	return createTestApp(fakeAppWithDestName, opts...)
 }
@@ -615,6 +642,87 @@ func TestDeleteApp_InvalidName(t *testing.T) {
 	assert.True(t, apierrors.IsNotFound(err))
 }
 
+func TestDeleteApp_DeleteNamespace(t *testing.T) {
+	ctx := context.Background()
+	appServer := newTestAppServer()
+	createReq := application.ApplicationCreateRequest{
+		Application: newTestAppWithDeleteNamespace(),
+	}
+	appLabelKey, err := appServer.settingsMgr.GetAppInstanceLabelKey()
+	assert.Nil(t, err)
+
+	app, err := appServer.Create(ctx, &createReq)
+	assert.Nil(t, err)
+	ns := v1.Namespace{}
+
+	// Flag whether Delete is called
+	appDeleted := false
+	nsDeleted := false
+	revertValues := func() {
+		appDeleted = false
+		nsDeleted = false
+	}
+
+	// Fake Application response
+	fakeAppCs := appServer.appclientset.(*apps.Clientset)
+	// this removes the default */* reactor so we can set our own patch/delete reactor
+	fakeAppCs.ReactionChain = nil
+	fakeAppCs.AddReactor("delete", "applications", func(action kubetesting.Action) (handled bool, ret runtime.Object, err error) {
+		appDeleted = true
+		return true, nil, nil
+	})
+	fakeAppCs.AddReactor("get", "applications", func(action kubetesting.Action) (handled bool, ret runtime.Object, err error) {
+		return true, app, nil
+	})
+	appServer.appclientset = fakeAppCs
+
+	// Fake Namespace response
+	fakeKubeCs := appServer.kubeclientset.(*k8sfake.Clientset)
+	// this removes the default */* reactor so we can set our own patch/delete reactor
+	fakeKubeCs.ReactionChain = nil
+	fakeKubeCs.AddReactor("get", "namespaces", func(action kubetesting.Action) (handled bool, ret runtime.Object, err error) {
+		return true, &ns, nil
+	})
+	fakeKubeCs.AddReactor("delete", "namespaces", func(action kubetesting.Action) (handled bool, ret runtime.Object, err error) {
+		nsDeleted = true
+		return true, nil, nil
+	})
+	appServer.kubeclientset = fakeKubeCs
+
+	t.Run("Success to delete namespace", func(t *testing.T) {
+		ns = v1.Namespace{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: test.FakeDestNamespace,
+				Labels: map[string]string{
+					appLabelKey: app.Name,
+				},
+			},
+		}
+		_, err = appServer.Delete(ctx, &application.ApplicationDeleteRequest{Name: &app.Name})
+		assert.Nil(t, err)
+		assert.True(t, appDeleted)
+		assert.True(t, nsDeleted)
+		t.Cleanup(revertValues)
+	})
+
+	t.Run("Fail to delete namespace because of invalid label", func(t *testing.T) {
+		ns = v1.Namespace{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: test.FakeDestNamespace,
+				Labels: map[string]string{
+					appLabelKey: "this-is-invalid",
+				},
+			},
+		}
+		_, err = appServer.Delete(ctx, &application.ApplicationDeleteRequest{Name: &app.Name})
+		assert.NotNil(t, err)
+		assert.Equal(t, fmt.Sprintf("error deleting namespace: error cannot delete namespace %s which is not managed by test-app", test.FakeDestNamespace), err.Error())
+		assert.True(t, appDeleted)
+		assert.False(t, nsDeleted)
+		t.Cleanup(revertValues)
+	})
+}
+
 func TestSyncAndTerminate(t *testing.T) {
 	ctx := context.Background()
 	appServer := newTestAppServer()

test/e2e/app_management_test.go

@@ -2459,3 +2459,122 @@ func TestAnnotationTrackingExtraResources(t *testing.T) {
 		Expect(HealthIs(health.HealthStatusHealthy))
 
 }
+
+// Given
+//   - application is set with --sync-option CreateNamespace=true  --sync-option DeleteNamespace=true
+//   - application --dest-namespace does not exist
+//
+// Verity
+//   - application --dest-namespace is created
+//   - application sync successful
+//   - when application is deleted, --dest-namespace is deleted
+func TestAppDeletionDeleteNamespace(t *testing.T) {
+	appName := "test-app-deletion-delete-namespace"
+	SkipOnEnv(t, "OPENSHIFT")
+	createdNamespace := getNewNamespace(t)
+	Given(t).
+		Timeout(30).
+		Path("guestbook").
+		When().
+		CreateFromFile(func(app *Application) {
+			app.Spec.SyncPolicy = &SyncPolicy{
+				SyncOptions: SyncOptions{"CreateNamespace=true", "DeleteNamespace=true", "PruneLast=true"},
+				ManagedNamespaceMetadata: &ManagedNamespaceMetadata{
+					Labels: map[string]string{"app.kubernetes.io/instance": appName},
+				}}
+		}).
+		Then().
+		Expect(NoNamespace(createdNamespace)).
+		When().
+		AppSet("--dest-namespace", createdNamespace).
+		Sync().
+		Then().
+		Expect(Success("")).
+		Expect(OperationPhaseIs(OperationSucceeded)).
+		Expect(ResourceHealthWithNamespaceIs("Deployment", "guestbook-ui", createdNamespace, health.HealthStatusHealthy)).
+		Expect(ResourceHealthWithNamespaceIs("Deployment", "guestbook-ui", createdNamespace, health.HealthStatusHealthy)).
+		Expect(ResourceSyncStatusWithNamespaceIs("Deployment", "guestbook-ui", createdNamespace, SyncStatusCodeSynced)).
+		Expect(ResourceSyncStatusWithNamespaceIs("Deployment", "guestbook-ui", createdNamespace, SyncStatusCodeSynced)).
+		And(func(app *Application) {
+			// Verify ns is created
+			output, err := Run("", "kubectl", "get", "namespace", createdNamespace)
+			assert.NoError(t, err)
+			assert.Contains(t, output, createdNamespace)
+		}).
+		When().
+		Delete(true).
+		Then().
+		Expect(Success("")).
+		And(func(app *Application) {
+			//Verify app delete the namespace auto created
+			_, err := Run("", "kubectl", "wait", "--for=delete", "namespace", createdNamespace, "--timeout=60s")
+			assert.NoError(t, err)
+		}).
+		Expect(NoNamespace(createdNamespace))
+}
+
+// Given
+//   - application is set with --sync-option CreateNamespace=true  --sync-option DeleteNamespace=true
+//   - namespace does not have an appropriate label ("app.kubernetes.io/instance": application-name)
+//   - application --dest-namespace does not exist
+//
+// Verity
+//   - application --dest-namespace is created
+//   - application sync successful
+//   - when application is deleted, --dest-namespace is not deleted
+//   - deletion process shows error
+func TestAppDeletionDoesNotDeleteNamespace(t *testing.T) {
+	appName := "test-app-deletion-does-not-delete-namespace"
+	SkipOnEnv(t, "OPENSHIFT")
+	createdNamespace := getNewNamespace(t)
+	defer func() {
+		if !t.Skipped() {
+			_, err := Run("", "kubectl", "delete", "namespace", createdNamespace)
+			assert.NoError(t, err)
+		}
+	}()
+	Given(t).
+		Timeout(30).
+		Path("guestbook").
+		When().
+		CreateFromFile(func(app *Application) {
+			app.Spec.SyncPolicy = &SyncPolicy{
+				SyncOptions: SyncOptions{"CreateNamespace=true", "DeleteNamespace=true", "PruneLast=true"},
+				ManagedNamespaceMetadata: &ManagedNamespaceMetadata{
+					Labels: map[string]string{"app.kubernetes.io/instance": appName},
+				}}
+		}).
+		Then().
+		Expect(NoNamespace(createdNamespace)).
+		When().
+		AppSet("--dest-namespace", createdNamespace).
+		Sync().
+		Then().
+		Expect(Success("")).
+		Expect(OperationPhaseIs(OperationSucceeded)).
+		Expect(ResourceHealthWithNamespaceIs("Deployment", "guestbook-ui", createdNamespace, health.HealthStatusHealthy)).
+		Expect(ResourceHealthWithNamespaceIs("Deployment", "guestbook-ui", createdNamespace, health.HealthStatusHealthy)).
+		Expect(ResourceSyncStatusWithNamespaceIs("Deployment", "guestbook-ui", createdNamespace, SyncStatusCodeSynced)).
+		Expect(ResourceSyncStatusWithNamespaceIs("Deployment", "guestbook-ui", createdNamespace, SyncStatusCodeSynced)).
+		When().
+		And(func() {
+			_, err := Run("", "kubectl", "label", "--overwrite", "namespace", createdNamespace, "app.kubernetes.io/instance=wrong-owner")
+			assert.NoError(t, err)
+		}).
+		Then().
+		Expect(Namespace(createdNamespace, func(app *Application, ns *v1.Namespace) {
+			delete(ns.Labels, "kubernetes.io/metadata.name")
+			assert.Equal(t, map[string]string{"app.kubernetes.io/instance": "wrong-owner"}, ns.Labels)
+		})).
+		When().
+		IgnoreErrors().
+		Delete(true).
+		Then().
+		Expect(Error("", "which is not managed by "+appName)).
+		And(func(app *Application) {
+			//Verify delete app does not delete the namespace auto created
+			output, err := Run("", "kubectl", "get", "namespace", createdNamespace)
+			assert.NoError(t, err)
+			assert.Contains(t, output, createdNamespace)
+		})
+}

util/argo/resource_tracking.go

@@ -31,7 +31,7 @@ type ResourceTracking interface {
 	Normalize(config, live *unstructured.Unstructured, labelKey, trackingMethod string) error
 }
 
-//AppInstanceValue store information about resource tracking info
+// AppInstanceValue store information about resource tracking info
 type AppInstanceValue struct {
 	ApplicationName string
 	Group           string
@@ -146,12 +146,12 @@ func (rt *resourceTracking) SetAppInstance(un *unstructured.Unstructured, key, v
 	}
 }
 
-//BuildAppInstanceValue build resource tracking id in format <application-name>;<group>/<kind>/<namespace>/<name>
+// BuildAppInstanceValue build resource tracking id in format <application-name>;<group>/<kind>/<namespace>/<name>
 func (rt *resourceTracking) BuildAppInstanceValue(value AppInstanceValue) string {
 	return fmt.Sprintf("%s:%s/%s:%s/%s", value.ApplicationName, value.Group, value.Kind, value.Namespace, value.Name)
 }
 
-//ParseAppInstanceValue parse resource tracking id from format <application-name>:<group>/<kind>:<namespace>/<name> to struct
+// ParseAppInstanceValue parse resource tracking id from format <application-name>:<group>/<kind>:<namespace>/<name> to struct
 func (rt *resourceTracking) ParseAppInstanceValue(value string) (*AppInstanceValue, error) {
 	var appInstanceValue AppInstanceValue
 	parts := strings.Split(value, ":")