fix(appset): Always remove ownerReferences when appset policy doesn't…

… allow app's deletion (argoproj#12172) (argoproj#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> Signed-off-by: Kevin Lyda <kevin@lyda.ie>
lyda · Mar 28, 2024 · a5dcbf8 · a5dcbf8
1 parent 265693b
commit a5dcbf8
Show file tree

Hide file tree

Showing 2 changed files with 10 additions and 14 deletions.
diff --git 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
@@ -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: