From 3d9e383ff55f19719392dd69c8d2f7ef8958f1ba Mon Sep 17 00:00:00 2001
From: Takumi Sue <23391543+mikutas@users.noreply.github.com>
Date: Fri, 8 Dec 2023 23:20:01 +0900
Subject: [PATCH] fix(appset): Always remove ownerReferences when appset policy
 doesn't allow app's deletion (#12172) (#16506)

* fix(appset): remove unnecessary condition

Signed-off-by: mikutas <23391543+mikutas@users.noreply.github.com>

* docs: update explanation about policy

Signed-off-by: mikutas <23391543+mikutas@users.noreply.github.com>

---------

Signed-off-by: mikutas <23391543+mikutas@users.noreply.github.com>
---
 .../controllers/applicationset_controller.go   | 18 ++++++++----------
 .../Controlling-Resource-Modification.md       |  6 ++----
 2 files changed, 10 insertions(+), 14 deletions(-)

diff --git a/applicationset/controllers/applicationset_controller.go b/applicationset/controllers/applicationset_controller.go
index 527a4ae1e3883..4f5ac66fc016d 100644
--- a/applicationset/controllers/applicationset_controller.go
+++ b/applicationset/controllers/applicationset_controller.go
@@ -108,16 +108,14 @@ func (r *ApplicationSetReconciler) Reconcile(ctx context.Context, req ctrl.Reque
 
 	// Do not attempt to further reconcile the ApplicationSet if it is being deleted.
 	if applicationSetInfo.ObjectMeta.DeletionTimestamp != nil {
-		if controllerutil.ContainsFinalizer(&applicationSetInfo, argov1alpha1.ResourcesFinalizerName) {
-			deleteAllowed := utils.DefaultPolicy(applicationSetInfo.Spec.SyncPolicy, r.Policy, r.EnablePolicyOverride).AllowDelete()
-			if !deleteAllowed {
-				if err := r.removeOwnerReferencesOnDeleteAppSet(ctx, applicationSetInfo); err != nil {
-					return ctrl.Result{}, err
-				}
-				controllerutil.RemoveFinalizer(&applicationSetInfo, argov1alpha1.ResourcesFinalizerName)
-				if err := r.Update(ctx, &applicationSetInfo); err != nil {
-					return ctrl.Result{}, err
-				}
+		deleteAllowed := utils.DefaultPolicy(applicationSetInfo.Spec.SyncPolicy, r.Policy, r.EnablePolicyOverride).AllowDelete()
+		if !deleteAllowed {
+			if err := r.removeOwnerReferencesOnDeleteAppSet(ctx, applicationSetInfo); err != nil {
+				return ctrl.Result{}, err
+			}
+			controllerutil.RemoveFinalizer(&applicationSetInfo, argov1alpha1.ResourcesFinalizerName)
+			if err := r.Update(ctx, &applicationSetInfo); err != nil {
+				return ctrl.Result{}, err
 			}
 		}
 		return ctrl.Result{}, nil
diff --git a/docs/operator-manual/applicationset/Controlling-Resource-Modification.md b/docs/operator-manual/applicationset/Controlling-Resource-Modification.md
index 44b6797b24d11..d72cee60ad401 100644
--- a/docs/operator-manual/applicationset/Controlling-Resource-Modification.md
+++ b/docs/operator-manual/applicationset/Controlling-Resource-Modification.md
@@ -32,15 +32,13 @@ spec:
 
 ```
 
-- Policy `create-only`: Prevents ApplicationSet controller from modifying or deleting Applications.
-- Policy `create-update`: Prevents ApplicationSet controller from deleting Applications. Update is allowed.
+- Policy `create-only`: Prevents ApplicationSet controller from modifying or deleting Applications. Prevents Application controller from deleting Applications according to [ownerReferences](https://kubernetes.io/docs/concepts/overview/working-with-objects/owners-dependents/).
+- Policy `create-update`: Prevents ApplicationSet controller from deleting Applications. Update is allowed. Prevents Application controller from deleting Applications according to [ownerReferences](https://kubernetes.io/docs/concepts/overview/working-with-objects/owners-dependents/).
 - Policy `create-delete`: Prevents ApplicationSet controller from modifying Applications. Delete is allowed.
 - Policy `sync`: Update and Delete are allowed.
 
 If the controller parameter `--policy` is set, it takes precedence on the field `applicationsSync`. It is possible to allow per ApplicationSet sync policy by setting variable `ARGOCD_APPLICATIONSET_CONTROLLER_ENABLE_POLICY_OVERRIDE` to argocd-cmd-params-cm `applicationsetcontroller.enable.policy.override` or directly with controller parameter `--enable-policy-override` (default to `false`).
 
-This does not prevent deletion of Applications if the ApplicationSet is deleted
-
 ### Controller parameter
 
 To allow the ApplicationSet controller to *create* `Application` resources, but prevent any further modification, such as deletion, or modification of Application fields, add this parameter in the ApplicationSet controller: