Repository: akuity/kargo

Issue:
Ability to disable namespace auto-deletion
# Checklist

* [x] I've searched the issue queue to verify this is not a duplicate feature request.
* [x] I've pasted the output of `kargo version`, if applicable.
* [x] I've pasted logs, if applicable.

# Proposed Feature

Ability to disable namespace auto-deletion when a Kargo project gets deleted.

# Motivation

Most of the users have the namespace created already through other sources like for Argo applications, helm charts or through self-service when onboarding apps/users to a cluster.

Since Kargo will act as an add-on to these existing namespaces, it shouldn’t take ownership or attempt to delete them, even if the associated Kargo project is removed. I think it would be a reasonable to add an annotation to control the deletion behaviour? More similar to what we have now for namespace adoption `kargo.akuity.io/delete-namespace: "false"`

Discussion:
@krancour: This sounds ok, but I want to think through the implications a little bit more.

One thing I am curious about @Kavinraja-G, can you say more precisely, what is the nature of these namespaces that already exist?

The adoption feature was added because of the high likelihood of orgs where namespaces are created and managed only by highly-privileged admins or by some policy agent or homegrown controller that ensures namespaces comply with some sort of organizational standards.

What we were never anticipating and strongly recommend people do _not_ do is attempt to have Kargo projects share a namespace with any _workloads_. e.g. If your Project's namespace has Deployments, Services, etc. that's not something we recommend... Just curious as to whether something like that is at all a factor in the feature request.@mogsie: One use case is when someone (like me) did not understand the implications of making a project with the same name as an existing namespace, and went ahead and adopted it.  Now I'm in the unlucky owner of an undeletable project.  The workaround would be to uninstall kargo, delete the project and then reinstall kargo, and hope it doesn't magically delete the namespace.

I wanted to re-use a namespace because it had pre-existing secrets (think git credentials and ECR) because it was being used for some CI pipelines

Backing projects with namespaces is great for RBAC and so on, but namespaces are just that, namespaces.  We'd want _ArgoCD_ to create and manage those Stage resources, right?  ArgoCD would probably also be rolling out secrets into it, and perhaps an AnalysisTemplate.  Maybe even a workflow (Argo Workflows and Events).  I, at least, might want a lot of these things in the same namespace, since they would all participate in the delivery of the software.@krancour: A bit of a tangent from the overall discussion, but...

> did not understand the implications of making a project with the same name as an existing namespace, and went ahead and adopted it.

Adopting a namespace is supposed to require the namespace to be labeled a certain way. Did that not work? If this is so, I would obviously like to fix that.

> Now I'm in the unlucky owner of an undeletable project.

The label requirement was meant to safeguard against this scenario, but now that you're here:

Scale the management controller to 0, edit the Project to remove Kargo's finalizer, then delete the Project while the management controller is still scaled to 0. You can scale the management controller back up to 1 after verifying the Project resource in question no longer exists.

>  We'd want ArgoCD to create and manage...

Internally, we GitOps our Kargo Projects and everything in them and that's our best recommendation on how users should manage Projects on an ongoing basis also. (Clickops is for playing around only, imho.)

We've not yet encountered any difficult GitOps'ing Projects.

When GitOps'ing a Project, you don't need to (and probably shouldn't) use Argo CD's built-in option for creating the namespace.

Leaving Kargo out of the picture entirely for a moment, if you handed Argo CD one manifest file with a `Namespace` resource at the top and everything that goes _in_ the namespace below it, things work fine. Or if you need or prefer to use more than one file, just make sure the namespace is at the top of the alphabetically first file. You can force a file to be alphabetically first by naming it something like `_namespace.yaml`...

The same thing works for `Project`s. You don't need to use Argo CD's built-in option for creating the namespace. You also don't need to put the `Namespace` resource in your manifests. As long as the `Project` is the first resource Argo CD applies, you'll be fine. The namespace is created _synchronously_ by a defaulting webhook, so by the time Argo CD moves on to syncing all the resources that need to go in that namespace, it is guaranteed to exist.

@Kavinraja-G: > One thing I am curious about @Kavinraja-G, can you say more precisely, what is the nature of these namespaces that already exist?

@krancour Sure, I can give you an overview. In our clusters, namespaces are provisioned and owned outside of Kargo (well we are still in the evaluation phase..) When a new team, say `Team X` -- is onboarded, the platform team applies an Argo CD application along with the Argo `Project X` that GitOps creates the namespace along with its RBAC, baseline secrets, and supporting controllers, then continues to manage that Argo Projects & of-course the namespace’s lifecycle. This "namespace-first” pattern (often defined in Helm/Kustomize) is pretty common.

What we need is to deploy Kargo resources into those pre-existing namespaces without letting Kargo claim or delete them if the Kargo project itself is later removed.@krancour: Thanks @Kavinraja-G that is perfectly in-line with how we imagined adoption being used.

I sort of wanted to validate that the concern for keeping the ns around wasn't motivated by having placed workloads in the ns.

I don't see any problem with adding something like a "keep" annotation.@Kavinraja-G: I can pick this up if we can confirm on the ~~annotation~~ label. Something like:
1. `kargo.akuity.io/delete-namespace: "false"`
2. `kargo.akuity.io/retain-namespace: "false"`

Open for suggestions. :)@hiddeco: Given it is an annotation to define an abnormality, I would prefer something that has a value of `true` when enabled.@krancour: Helm uses `helm.sh/resource-policy: keep`. "keep" sounds right to me. "resource policy" does not.

How about something like `kargo.akuity.io/keep-namespace: "true"`?@krancour: Also I was imagining this annotation going on the Project, but because I can _so easily_ imagine someone mistakenly thinking it goes on the ns, and because the consequences of that mistake are so high, I'm thinking it might be best to skip ns deletion if _either_ the Project or ns has this annotation.

@hiddeco would you agree?@nitishfy: I would like to take this up.@krancour: @Kavinraja-G [beat you to it](https://github.com/akuity/kargo/issues/4202#issuecomment-2905469737).

He has right of first refusal.@krancour: _Tentatively_ in v1.6.0. It can be deferred if timelines require.@mogsie: Thanks for the detailed instructions.  And for the "Danger zone" warning when deleting the project.  Very on point.

> Adopting a namespace is supposed to require the namespace to be labeled a certain way. Did that not work? If this is so, I would obviously like to fix that.

It did work. It was *I* who decided to adopt, and added the required metadata.  Kargo is fine. 😄 

But as far as I *now* understand from the documentation, these namespaces *should* not be used for other "stuff", and we should operate under the assumption that *they will be reclaimed* if we ever delete the kargo project.  This would definitely influence the naming strategy we'll have to adopt; the kargo projects should have names that don't conflict with our existing namespace naming strategy.

All that said, I second the ability to undo a simple mistake like I did.  Maybe the "delete project" UI flow should get a little checkbox like ArgoCD has when you delete a project; you can opt in or out of "recursive deletion" so to speak.

**Maybe** Kargo projects ought to function the same way?  "kubectl delete project" doesn't delete the namespace unless there's a finalizer there.  The "Danger Zone" UI and "kargo delete project" CLI can have options to opt in/out of deleting the namespace, and they do that by adding the finalizer before deletion.@krancour: > Thanks for the detailed instructions.

My pleasure.

> But as far as I now understand from the documentation, these namespaces should not be used for other "stuff"

We never thought there would be much temptation to combine them. Maybe we were wrong. My own rationale:

Consider a simple test --> uat --> prod pipeline. Most operators wouldn't consider putting all these three instances of an application in the same namespace. In most orgs, they wouldn't even all be in the same _cluster_. (And when that's the case, it's also likely that the Kargo control plane lives in an entirely separate "management cluster" as well.) So, except in the most trivial of cases, one very quickly stumbles into the _impossibility_ of keeping workload stuff in the Project's namespace.

Docs may need to more explicitly state that it's not recommended and often not even possible.

> This would definitely influence the naming strategy we'll have to adopt; the kargo projects should have names that don't conflict with our existing namespace naming strategy.

I think I'd have recommended that regardless, and again I think most people stumble into doing the right thing here. Assuming the test instance of an app x is in x-test, the uat instance is in x-uat, and so forth and so on (I realize this is on the very simple end of possible naming conventions, but it gets the point across) x, kargo-x, project-x, etc. all are up for grabs and seem like reasonable places to put all the Kargo goodness that manages promotion through all the other x-* application instances.

> Maybe the "delete project" UI flow should get a little checkbox like ArgoCD has when you delete a project; you can opt in or out of "recursive deletion" so to speak.

That's not a bad idea at all. Please consider opening a separate issue if you want to request that.
@mogsie: > > ... these namespaces should not be used for other "stuff"
> 
> We never thought there would be much temptation to combine them. Maybe we were wrong. My own rationale:
> 
> Consider a simple test --> uat --> prod pipeline.

There are other workloads in play than the actual business workloads.  Imagine you use TektonCI or Argo Workflows to build your images (in kubernetes).  Those namespaces already have the "plumbing" in place to access git and container repositories.

I agree that the docs/UI should be more explicit about the recommendation against using the namespace for other things.  Namespaces are cheap, just pick kargo project names that give it its own namespace.@ftmiro: > What we were never anticipating and strongly recommend people do _not_ do is attempt to have Kargo projects share a namespace with any _workloads_. e.g., If your Project's namespace has Deployments, Services, etc., that's not something we recommend... Just curious as to whether something like that a factor in the feature request?

Hi @krancour, regarding this part, at least in our organization, most people will have the following patterns:

* A repository named <my-service>, that deploys to a <my-service> namespace. Naturally, people would create a Kargo project <my-service>, which would, 99% of the time, have to adopt the existing namespace.

* A repository named <my-service> deploys on a namespace that is also used to deploy related other services (we discourage that, but we have many legacy or mono-repos doing that).

Having the project deleted would be harmful in a scenario where people want to opt in/out of Kargo. Also, it would be difficult to educate people to create Kargo projects with naming suffixes to prevent namespace collision, but we can enforce that with a GitOps approach instead of allowing it via UI. 

Last, we are not allowing ArgoCD to create namespaces, which also would not be allowed to Kargo, as the namespaces in our clusters must honour a few labeling constraints that, for example, control quotas, allowed ingress classes, etc.

Solution patch:
diff --git a/api/v1alpha1/annotations.go b/api/v1alpha1/annotations.go
index 2f15fd7b..b162e921 100644
--- a/api/v1alpha1/annotations.go
+++ b/api/v1alpha1/annotations.go
@@ -52,4 +52,14 @@ const (
 	// The value of the annotation is a JSON object that maps migration types to
 	// booleans indicating whether the migration has been performed.
 	AnnotationKeyMigrated = "kargo.akuity.io/migrated"
+
+	// AnnotationKeyKeepNamespace is an annotation key that can be set on Project
+	// or Namespace to disable the automatic deletion of the namespace when
+	// the Project is deleted. This is useful for cases where the namespace
+	// contains resources that should not be deleted.
+	AnnotationKeyKeepNamespace = "kargo.akuity.io/keep-namespace"
+
+	// AnnotationValueTrue is the value used to indicate that an annotation
+	// is set to true.
+	AnnotationValueTrue = "true"
 )
diff --git a/docs/docs/50-user-guide/20-how-to-guides/20-working-with-projects.md b/docs/docs/50-user-guide/20-how-to-guides/20-working-with-projects.md
index 3ab83a3f..4cec8890 100644
--- a/docs/docs/50-user-guide/20-how-to-guides/20-working-with-projects.md
+++ b/docs/docs/50-user-guide/20-how-to-guides/20-working-with-projects.md
@@ -243,6 +243,17 @@ spec:
   # ...
 ```
 
+## Preventing Namespace Deletion
+
+By default, when a Project is deleted, Kargo will attempt to delete the
+corresponding Namespace. However, there are scenarios where you may want to
+retain the namespace after the associated Project is removed whether it was
+created by Kargo or adopted from an existing setup.
+
+To achieve this, you can apply the following annotation to the `Namespace` or
+the corresponding `Project` resource with
+`kargo.akuity.io/keep-namespace: "true"`.
+
 ## Interacting with Projects
 
 Kargo provides tools to manage `Project`s using either its UI or
diff --git a/internal/controller/management/projects/projects.go b/internal/controller/management/projects/projects.go
index 1adf161d..0b27ec44 100644
--- a/internal/controller/management/projects/projects.go
+++ b/internal/controller/management/projects/projects.go
@@ -93,6 +93,12 @@ type reconciler struct {
 		...client.CreateOption,
 	) error
 
+	deleteNamespaceFn func(
+		context.Context,
+		client.Object,
+		...client.DeleteOption,
+	) error
+
 	patchOwnerReferencesFn func(
 		context.Context,
 		client.Client,
@@ -105,6 +111,12 @@ type reconciler struct {
 		client.Object,
 	) (bool, error)
 
+	removeFinalizerFn func(
+		context.Context,
+		client.Client,
+		client.Object,
+	) error
+
 	ensureSystemPermissionsFn func(context.Context, *kargoapi.Project) error
 
 	ensureControllerPermissionsFn func(context.Context, *kargoapi.Project) error
@@ -194,8 +206,10 @@ func newReconciler(kubeClient client.Client, cfg ReconcilerConfig) *reconciler {
 	r.patchProjectStatusFn = r.patchProjectStatus
 	r.getNamespaceFn = r.client.Get
 	r.createNamespaceFn = r.client.Create
+	r.deleteNamespaceFn = r.client.Delete
 	r.patchOwnerReferencesFn = api.PatchOwnerReferences
 	r.ensureFinalizerFn = api.EnsureFinalizer
+	r.removeFinalizerFn = api.RemoveFinalizer
 	r.ensureSystemPermissionsFn = r.ensureSystemPermissions
 	r.ensureControllerPermissionsFn = r.ensureControllerPermissions
 	r.ensureDefaultUserRolesFn = r.ensureDefaultUserRoles
@@ -227,8 +241,20 @@ func (r *reconciler) Reconcile(
 		return ctrl.Result{}, nil
 	}
 
+	// ensure the finalizer is present on the Project
+	updated, err := r.ensureFinalizerFn(ctx, r.client, project)
+	if err != nil {
+		return ctrl.Result{}, fmt.Errorf("error ensuring finalizer on Project %q: %w", project.Name, err)
+	}
+	if updated {
+		logger.Debug("added finalizer to Project")
+	}
+
 	if project.DeletionTimestamp != nil {
-		logger.Debug("Project is being deleted; nothing to do")
+		if err = r.cleanupProject(ctx, project); err != nil {
+			return ctrl.Result{}, err
+		}
+
 		return ctrl.Result{}, nil
 	}
 
@@ -327,6 +353,67 @@ func (r *reconciler) reconcile(
 	return status, nil
 }
 
+// cleanupProject handles the deletion and cleanup of a Project's associated
+// resources.
+func (r *reconciler) cleanupProject(ctx context.Context, project *kargoapi.Project) error {
+	logger := logging.LoggerFromContext(ctx)
+
+	// Get namespace for the Project
+	ns := &corev1.Namespace{}
+	err := r.getNamespaceFn(ctx, types.NamespacedName{Name: project.Name}, ns)
+	if err != nil {
+		if kubeerr.IsNotFound(err) {
+			// Namespace already deleted or never existed, just remove finalizer
+			// from project
+			logger.Debug("namespace not found, removing project finalizer")
+			if err = r.removeFinalizerFn(ctx, r.client, project); err != nil {
+				return fmt.Errorf("failed to remove finalizer from project %q: %w", project.Name, err)
+			}
+			return nil
+		}
+		return fmt.Errorf("error getting namespace %q: %w", project.Name, err)
+	}
+
+	if shouldKeepNamespace(project, ns) {
+		logger.Debug("keeping namespace due to keep-namespace annotation")
+
+		// Remove only this Project's OwnerReference from the Namespace
+		var newOwnerRefs []metav1.OwnerReference
+		for _, ref := range ns.OwnerReferences {
+			if ref.UID != project.UID {
+				newOwnerRefs = append(newOwnerRefs, ref)
+			}
+		}
+
+		// Only update owner references if we actually found and removed one
+		if len(newOwnerRefs) < len(ns.OwnerReferences) {
+			ns.OwnerReferences = newOwnerRefs
+			if err = r.patchOwnerReferencesFn(ctx, r.client, ns); err != nil {
+				return fmt.Errorf("failed to patch namespace %q owner references: %w", ns.Name, err)
+			}
+			logger.Debug("removed project owner reference from namespace")
+		}
+
+		// Remove finalizer from namespace
+		if err = r.removeFinalizerFn(ctx, r.client, ns); err != nil {
+			return fmt.Errorf("failed to remove finalizer from namespace %q: %w", ns.Name, err)
+		}
+	} else {
+		// Delete the namespace
+		logger.Debug("deleting namespace")
+		if err = r.deleteNamespaceFn(ctx, ns); err != nil && !kubeerr.IsNotFound(err) {
+			return fmt.Errorf("failed to delete namespace %q: %w", ns.Name, err)
+		}
+		logger.Debug("deleted namespace", "namespace", ns.Name)
+	}
+
+	// Remove finalizer from Project
+	if err = r.removeFinalizerFn(ctx, r.client, project); err != nil {
+		return fmt.Errorf("failed to remove finalizer from project %q: %w", project.Name, err)
+	}
+	return nil
+}
+
 // syncProject ensures the existence of the Project's namespace and any
 // resources that are required for the Project to function properly. It
 // returns an updated ProjectStatus.
@@ -438,6 +525,7 @@ func (r *reconciler) ensureNamespace(ctx context.Context, project *kargoapi.Proj
 		kargoapi.GroupVersion.WithKind("Project"),
 	)
 	ownerRef.BlockOwnerDeletion = ptr.To(false)
+	ownerRef.Controller = nil
 
 	ns := &corev1.Namespace{}
 	if err := r.getNamespaceFn(
@@ -455,29 +543,45 @@ func (r *reconciler) ensureNamespace(ctx context.Context, project *kargoapi.Proj
 				project.Name, project.Name, errProjectNamespaceExists,
 			)
 		}
-		for _, ownerRef := range ns.OwnerReferences {
+
+		// always ensure finalizer is present on the namespace
+		updated, err := r.ensureFinalizerFn(ctx, r.client, ns)
+		if err != nil {
+			return fmt.Errorf("error ensuring finalizer on namespace %q: %w", project.Name, err)
+		}
+		if updated {
+			logger.Debug("added finalizer to namespace")
+		}
+
+		for i, ownerRef := range ns.OwnerReferences {
 			if ownerRef.UID == project.UID {
 				logger.Debug("namespace exists and is already owned by this Project")
+				if ownerRef.Controller != nil {
+					logger.Debug("owner reference requires update")
+					ns.OwnerReferences[i].Controller = nil // Update in place
+					if err = r.patchOwnerReferencesFn(ctx, r.client, ns); err != nil {
+						return fmt.Errorf(
+							"error patching namespace %q owner references: %w",
+							project.Name, err,
+						)
+					}
+					logger.Debug("updated owner reference")
+				}
 				return nil
 			}
 		}
+
 		// If we get to here, the Project is not already an owner of the existing
 		// namespace.
 		logger.Debug(
 			"namespace exists, is not owned by this Project, but has the " +
 				"project label; Project will adopt it",
 		)
+
 		// Note: We allow multiple owners of a namespace due to the not entirely
 		// uncommon scenario where an organization has its own controller that
 		// creates and initializes namespaces to ensure compliance with
 		// internal policies. Such a controller might already own the namespace.
-		updated, err := r.ensureFinalizerFn(ctx, r.client, ns)
-		if err != nil {
-			return fmt.Errorf("error ensuring finalizer on namespace %q: %w", project.Name, err)
-		}
-		if updated {
-			logger.Debug("added finalizer to namespace")
-		}
 		ns.OwnerReferences = append(ns.OwnerReferences, *ownerRef)
 		if err = r.patchOwnerReferencesFn(ctx, r.client, ns); err != nil {
 			return fmt.Errorf(
@@ -944,6 +1048,14 @@ func (r *reconciler) migrateSpecToProjectConfig(
 	return true, nil
 }
 
+// shouldKeepNamespace determines if a Namespace should be kept during Project
+// cleanup based on the keep-namespace annotation on either the Project or
+// Namespace.
+func shouldKeepNamespace(project *kargoapi.Project, ns *corev1.Namespace) bool {
+	return project.Annotations[kargoapi.AnnotationKeyKeepNamespace] == kargoapi.AnnotationValueTrue ||
+		ns.Annotations[kargoapi.AnnotationKeyKeepNamespace] == kargoapi.AnnotationValueTrue
+}
+
 func getRoleBindingName(serviceAccountName string) string {
 	return fmt.Sprintf("%s-read-secrets", serviceAccountName)
 }



Test patch:
diff --git a/internal/controller/management/projects/projects_test.go b/internal/controller/management/projects/projects_test.go
index 5d5bcb45..0316b326 100644
--- a/internal/controller/management/projects/projects_test.go
+++ b/internal/controller/management/projects/projects_test.go
@@ -36,8 +36,10 @@ func TestNewReconciler(t *testing.T) {
 	require.NotNil(t, r.patchProjectStatusFn)
 	require.NotNil(t, r.getNamespaceFn)
 	require.NotNil(t, r.createNamespaceFn)
+	require.NotNil(t, r.deleteNamespaceFn)
 	require.NotNil(t, r.patchOwnerReferencesFn)
 	require.NotNil(t, r.ensureFinalizerFn)
+	require.NotNil(t, r.removeFinalizerFn)
 	require.NotNil(t, r.ensureSystemPermissionsFn)
 	require.NotNil(t, r.ensureControllerPermissionsFn)
 	require.NotNil(t, r.ensureDefaultUserRolesFn)
@@ -99,10 +101,47 @@ func TestReconciler_Reconcile(t *testing.T) {
 				) (*kargoapi.Project, error) {
 					return &kargoapi.Project{
 						ObjectMeta: metav1.ObjectMeta{
+							Name:              "test-project",
 							DeletionTimestamp: &metav1.Time{},
+							Finalizers:        []string{kargoapi.FinalizerName},
 						},
 					}, nil
 				},
+				ensureFinalizerFn: func(
+					context.Context,
+					client.Client,
+					client.Object,
+				) (bool, error) {
+					return false, nil
+				},
+				getNamespaceFn: func(
+					_ context.Context,
+					_ types.NamespacedName,
+					obj client.Object,
+					_ ...client.GetOption,
+				) error {
+					ns, ok := obj.(*corev1.Namespace)
+					require.True(t, ok)
+					ns.Labels = map[string]string{
+						kargoapi.LabelKeyProject: kargoapi.LabelValueTrue,
+					}
+					ns.Finalizers = []string{kargoapi.FinalizerName}
+					return nil
+				},
+				deleteNamespaceFn: func(
+					_ context.Context,
+					_ client.Object,
+					_ ...client.DeleteOption,
+				) error {
+					return nil
+				},
+				removeFinalizerFn: func(
+					_ context.Context,
+					_ client.Client,
+					_ client.Object,
+				) error {
+					return nil
+				},
 			},
 			assertions: func(t *testing.T, result ctrl.Result, err error) {
 				require.NoError(t, err)
@@ -131,6 +170,13 @@ func TestReconciler_Reconcile(t *testing.T) {
 				) (kargoapi.ProjectStatus, error) {
 					return kargoapi.ProjectStatus{}, errors.New("something went wrong")
 				},
+				ensureFinalizerFn: func(
+					context.Context,
+					client.Client,
+					client.Object,
+				) (bool, error) {
+					return false, nil
+				},
 				patchProjectStatusFn: func(
 					_ context.Context,
 					_ *kargoapi.Project,
@@ -153,6 +199,13 @@ func TestReconciler_Reconcile(t *testing.T) {
 				) (*kargoapi.Project, error) {
 					return &kargoapi.Project{}, nil
 				},
+				ensureFinalizerFn: func(
+					context.Context,
+					client.Client,
+					client.Object,
+				) (bool, error) {
+					return false, nil
+				},
 				reconcileFn: func(
 					context.Context,
 					*kargoapi.Project,
@@ -412,6 +465,376 @@ func TestReconciler_reconcile(t *testing.T) {
 	}
 }
 
+func TestReconciler_cleanupProject(t *testing.T) {
+	testCases := []struct {
+		name       string
+		project    *kargoapi.Project
+		reconciler *reconciler
+		assertions func(*testing.T, error)
+	}{
+		{
+			name: "error getting namespace",
+			project: &kargoapi.Project{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "test-project",
+				},
+			},
+			reconciler: &reconciler{
+				getNamespaceFn: func(
+					context.Context,
+					types.NamespacedName,
+					client.Object,
+					...client.GetOption,
+				) error {
+					return errors.New("something went wrong")
+				},
+			},
+			assertions: func(t *testing.T, err error) {
+				require.ErrorContains(t, err, "error getting namespace")
+				require.ErrorContains(t, err, "something went wrong")
+			},
+		},
+		{
+			name: "namespace not found - success",
+			project: &kargoapi.Project{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "test-project",
+				},
+			},
+			reconciler: &reconciler{
+				getNamespaceFn: func(
+					context.Context,
+					types.NamespacedName,
+					client.Object,
+					...client.GetOption,
+				) error {
+					return kubeerr.NewNotFound(schema.GroupResource{}, "test-project")
+				},
+				removeFinalizerFn: func(
+					context.Context,
+					client.Client,
+					client.Object,
+				) error {
+					return nil
+				},
+			},
+			assertions: func(t *testing.T, err error) {
+				require.NoError(t, err)
+			},
+		},
+		{
+			name: "namespace not found - error removing project finalizer",
+			project: &kargoapi.Project{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "test-project",
+				},
+			},
+			reconciler: &reconciler{
+				getNamespaceFn: func(
+					context.Context,
+					types.NamespacedName,
+					client.Object,
+					...client.GetOption,
+				) error {
+					return kubeerr.NewNotFound(schema.GroupResource{}, "test-project")
+				},
+				removeFinalizerFn: func(
+					context.Context,
+					client.Client,
+					client.Object,
+				) error {
+					return errors.New("finalizer removal failed")
+				},
+			},
+			assertions: func(t *testing.T, err error) {
+				require.ErrorContains(t, err, "failed to remove finalizer from project")
+				require.ErrorContains(t, err, "finalizer removal failed")
+			},
+		},
+		{
+			name: "keep namespace - success",
+			project: &kargoapi.Project{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "test-project",
+					UID:  "project-uid",
+					Annotations: map[string]string{
+						kargoapi.AnnotationKeyKeepNamespace: kargoapi.AnnotationValueTrue,
+					},
+				},
+			},
+			reconciler: &reconciler{
+				getNamespaceFn: func(
+					_ context.Context,
+					_ types.NamespacedName,
+					obj client.Object,
+					_ ...client.GetOption,
+				) error {
+					ns, ok := obj.(*corev1.Namespace)
+					require.True(t, ok)
+					ns.Name = "test-project"
+					ns.OwnerReferences = []metav1.OwnerReference{
+						{UID: "project-uid"},
+						{UID: "other-uid"},
+					}
+					return nil
+				},
+				patchOwnerReferencesFn: func(
+					context.Context,
+					client.Client,
+					client.Object,
+				) error {
+					return nil
+				},
+				removeFinalizerFn: func(
+					context.Context,
+					client.Client,
+					client.Object,
+				) error {
+					return nil
+				},
+			},
+			assertions: func(t *testing.T, err error) {
+				require.NoError(t, err)
+			},
+		},
+		{
+			name: "keep namespace - error patching owner references",
+			project: &kargoapi.Project{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "test-project",
+					UID:  "project-uid",
+					Annotations: map[string]string{
+						kargoapi.AnnotationKeyKeepNamespace: kargoapi.AnnotationValueTrue,
+					},
+				},
+			},
+			reconciler: &reconciler{
+				getNamespaceFn: func(
+					_ context.Context,
+					_ types.NamespacedName,
+					obj client.Object,
+					_ ...client.GetOption,
+				) error {
+					ns, ok := obj.(*corev1.Namespace)
+					require.True(t, ok)
+					ns.Name = "test-project"
+					ns.OwnerReferences = []metav1.OwnerReference{
+						{UID: "project-uid"},
+					}
+					return nil
+				},
+				patchOwnerReferencesFn: func(
+					context.Context,
+					client.Client,
+					client.Object,
+				) error {
+					return errors.New("patch failed")
+				},
+			},
+			assertions: func(t *testing.T, err error) {
+				require.ErrorContains(t, err, "failed to patch namespace")
+				require.ErrorContains(t, err, "patch failed")
+			},
+		},
+		{
+			name: "keep namespace - no owner reference to remove",
+			project: &kargoapi.Project{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "test-project",
+					UID:  "project-uid",
+					Annotations: map[string]string{
+						kargoapi.AnnotationKeyKeepNamespace: kargoapi.AnnotationValueTrue,
+					},
+				},
+			},
+			reconciler: &reconciler{
+				getNamespaceFn: func(
+					_ context.Context,
+					_ types.NamespacedName,
+					obj client.Object,
+					_ ...client.GetOption,
+				) error {
+					ns, ok := obj.(*corev1.Namespace)
+					require.True(t, ok)
+					ns.Name = "test-project"
+					ns.OwnerReferences = []metav1.OwnerReference{
+						{UID: "other-uid"},
+					}
+					return nil
+				},
+				removeFinalizerFn: func(
+					context.Context,
+					client.Client,
+					client.Object,
+				) error {
+					return nil
+				},
+			},
+			assertions: func(t *testing.T, err error) {
+				require.NoError(t, err)
+			},
+		},
+		{
+			name: "delete namespace - success",
+			project: &kargoapi.Project{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:        "test-project",
+					Annotations: map[string]string{},
+				},
+			},
+			reconciler: &reconciler{
+				getNamespaceFn: func(
+					_ context.Context,
+					_ types.NamespacedName,
+					obj client.Object,
+					_ ...client.GetOption,
+				) error {
+					ns, ok := obj.(*corev1.Namespace)
+					require.True(t, ok)
+					ns.Name = "test-project"
+					return nil
+				},
+				deleteNamespaceFn: func(
+					context.Context,
+					client.Object,
+					...client.DeleteOption,
+				) error {
+					return nil
+				},
+				removeFinalizerFn: func(
+					context.Context,
+					client.Client,
+					client.Object,
+				) error {
+					return nil
+				},
+			},
+			assertions: func(t *testing.T, err error) {
+				require.NoError(t, err)
+			},
+		},
+		{
+			name: "delete namespace - namespace already deleted",
+			project: &kargoapi.Project{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:        "test-project",
+					Annotations: map[string]string{},
+				},
+			},
+			reconciler: &reconciler{
+				getNamespaceFn: func(
+					_ context.Context,
+					_ types.NamespacedName,
+					obj client.Object,
+					_ ...client.GetOption,
+				) error {
+					ns, ok := obj.(*corev1.Namespace)
+					require.True(t, ok)
+					ns.Name = "test-project"
+					return nil
+				},
+				deleteNamespaceFn: func(
+					context.Context,
+					client.Object,
+					...client.DeleteOption,
+				) error {
+					return kubeerr.NewNotFound(schema.GroupResource{}, "test-project")
+				},
+				removeFinalizerFn: func(
+					context.Context,
+					client.Client,
+					client.Object,
+				) error {
+					return nil
+				},
+			},
+			assertions: func(t *testing.T, err error) {
+				require.NoError(t, err)
+			},
+		},
+		{
+			name: "delete namespace - error deleting",
+			project: &kargoapi.Project{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:        "test-project",
+					Annotations: map[string]string{},
+				},
+			},
+			reconciler: &reconciler{
+				getNamespaceFn: func(
+					_ context.Context,
+					_ types.NamespacedName,
+					obj client.Object,
+					_ ...client.GetOption,
+				) error {
+					ns, ok := obj.(*corev1.Namespace)
+					require.True(t, ok)
+					ns.Name = "test-project"
+					return nil
+				},
+				deleteNamespaceFn: func(
+					context.Context,
+					client.Object,
+					...client.DeleteOption,
+				) error {
+					return errors.New("delete failed")
+				},
+			},
+			assertions: func(t *testing.T, err error) {
+				require.ErrorContains(t, err, "failed to delete namespace")
+				require.ErrorContains(t, err, "delete failed")
+			},
+		},
+		{
+			name: "error removing project finalizer",
+			project: &kargoapi.Project{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:        "test-project",
+					Annotations: map[string]string{},
+				},
+			},
+			reconciler: &reconciler{
+				getNamespaceFn: func(
+					_ context.Context,
+					_ types.NamespacedName,
+					obj client.Object,
+					_ ...client.GetOption,
+				) error {
+					ns, ok := obj.(*corev1.Namespace)
+					require.True(t, ok)
+					ns.Name = "test-project"
+					return nil
+				},
+				deleteNamespaceFn: func(
+					context.Context,
+					client.Object,
+					...client.DeleteOption,
+				) error {
+					return nil
+				},
+				removeFinalizerFn: func(
+					context.Context,
+					client.Client,
+					client.Object,
+				) error {
+					return errors.New("finalizer removal failed")
+				},
+			},
+			assertions: func(t *testing.T, err error) {
+				require.ErrorContains(t, err, "failed to remove finalizer from project")
+				require.ErrorContains(t, err, "finalizer removal failed")
+			},
+		},
+	}
+
+	for _, testCase := range testCases {
+		t.Run(testCase.name, func(t *testing.T) {
+			err := testCase.reconciler.cleanupProject(context.Background(), testCase.project)
+			testCase.assertions(t, err)
+		})
+	}
+}
+
 func TestReconciler_syncProject(t *testing.T) {
 	testCases := []struct {
 		name       string
@@ -677,6 +1100,13 @@ func TestReconciler_ensureNamespace(t *testing.T) {
 				},
 			},
 			reconciler: &reconciler{
+				ensureFinalizerFn: func(
+					context.Context,
+					client.Client,
+					client.Object,
+				) (bool, error) {
+					return false, nil
+				},
 				getNamespaceFn: func(
 					_ context.Context,
 					_ types.NamespacedName,
@@ -1466,3 +1896,128 @@ func TestMigrateSpecToProjectConfig(t *testing.T) {
 		})
 	}
 }
+
+func Test_shouldKeepNamespace(t *testing.T) {
+	testCases := []struct {
+		name      string
+		project   *kargoapi.Project
+		namespace *corev1.Namespace
+		expected  bool
+	}{
+		{
+			name: "no keep annotation on either",
+			project: &kargoapi.Project{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{},
+				},
+			},
+			namespace: &corev1.Namespace{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{},
+				},
+			},
+			expected: false,
+		},
+		{
+			name: "keep annotation on project",
+			project: &kargoapi.Project{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{
+						kargoapi.AnnotationKeyKeepNamespace: kargoapi.AnnotationValueTrue,
+					},
+				},
+			},
+			namespace: &corev1.Namespace{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{},
+				},
+			},
+			expected: true,
+		},
+		{
+			name: "keep annotation on namespace",
+			project: &kargoapi.Project{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{},
+				},
+			},
+			namespace: &corev1.Namespace{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{
+						kargoapi.AnnotationKeyKeepNamespace: kargoapi.AnnotationValueTrue,
+					},
+				},
+			},
+			expected: true,
+		},
+		{
+			name: "keep annotation on both",
+			project: &kargoapi.Project{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{
+						kargoapi.AnnotationKeyKeepNamespace: kargoapi.AnnotationValueTrue,
+					},
+				},
+			},
+			namespace: &corev1.Namespace{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{
+						kargoapi.AnnotationKeyKeepNamespace: kargoapi.AnnotationValueTrue,
+					},
+				},
+			},
+			expected: true,
+		},
+		{
+			name: "false value on project",
+			project: &kargoapi.Project{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{
+						kargoapi.AnnotationKeyKeepNamespace: "false",
+					},
+				},
+			},
+			namespace: &corev1.Namespace{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{},
+				},
+			},
+			expected: false,
+		},
+		{
+			name: "nil annotations on project",
+			project: &kargoapi.Project{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: nil,
+				},
+			},
+			namespace: &corev1.Namespace{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{},
+				},
+			},
+			expected: false,
+		},
+		{
+			name: "nil annotations on namespace",
+			project: &kargoapi.Project{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{},
+				},
+			},
+			namespace: &corev1.Namespace{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: nil,
+				},
+			},
+			expected: false,
+		},
+	}
+
+	for _, testCase := range testCases {
+		t.Run(testCase.name, func(t *testing.T) {
+			result := shouldKeepNamespace(testCase.project, testCase.namespace)
+			require.Equal(t, testCase.expected, result)
+		})
+	}
+}