ArgoCD should support attributes that can explicitly disable application deletion and syncing

[shapirus]

Summary

There should be a way to explicitly block application deletion and syncing, regardless of what sequence of actions leads to them, using attributes in the Application object spec.

Motivation

There are many types of applications whose accidental deletion may result in a disaster: mission-critical applications, the "applications" application, applications that contain persistent volumes, application that manages ArgoCD itself etc.

Human errors happen.

There are multiple possibilities to trigger an unwanted deletion: from deleting a wrong application via the web UI (and even confirming it by typing the app name in the input field, which is a mistake easily made after a long day) to a poorly carried out migration of ArgoCD from cluster to cluster when one of the instances begins removing everything that it manages.

There are also situations when an unwanted sync may lead to a disaster, e.g., manifest path erroneously set to a wrong location and either auto-sync being enabled or the operator not checking the manifest diff before clicking Sync manually.

Application manifests often include namespace objects. If such an object is deleted following an accidental application deletion or syncing to a wrong set of manifests, then it can also be a disaster, such as losing data on persistent volumes that will be deleted following the deletion of the namespace, as well as losing state kept in CRD-managed objects.

The possibility to explicitly disable app deletion and sync would be similar in its purpose to, for example, the instance termination protection or the RDS deletion protection attributes supported by AWS which serve as an additional safeguard against accidental resource termination by both humans and automated resource management tools.

Proposal

apiVersion: argoproj.io/v1alpha1
kind: Application
metadata:
  name: mission-critical-app
spec:
  deletionPolicy:
    automated:
      condition: disable
    cli:
      condition: disable
    web:
      condition: disable
  syncPolicy:
    automated:
      condition: disable
    cli:
      condition: disable
    web:
      condition: extra-confirmation

The automated section is for the decisions made by ArgoCD itself. The rest are for externally triggered actions. The reason why I am proposing to make separate declarations for cli and web is because the former is often used for external CI/CD workflows which are non-interactive with regards to human presence, and the latter is more likely to be used by humans, so they will serve different use cases.

This would create a unified, easy to use and consistent method of specifying a set of human-readable unambiguous attributes as part of the application spec (as opposed to annotations which are generally harder to understand and track) to protect mission-critical resources regardless of whether the applications are standalone or managed by appsets.

There are quite a few related issues already, including, but not limited to:

• Add Lock/Unlock for user "modify" actions on applications #3580
• Annotation on child resources so ArgoCD doesn't delete them when parrent Application gets deleted #5945
• Provide better protection against deleting argocd #7301
• Persistent volume claim are deleted automatically when deleting argocd app #9853
• Set default deletion propagation policy for all apps #10834
• Support annotation to prevent argo from deleting resources if app gets deleted #11227
• option to never terminate 'kind: namespace' #13216

Some are related to either bugs or misunderstanding of how the existing features work:

• Deleting ApplicationSet causes Application deletion even if applicationset-controller policy is 'create-only' #12172
• Application deletion by applicationSet when using ApplicationSet Policy or syncPolicy.applicationsSync #15393

I believe that most of them, as far as the day-to-day practical use cases are concerned, could be solved by adding two simple boolean isDeletionAllowed(app_id) and isSyncAllowed(app_id) functions that check the application's attributes proposed above and using them just before the deletion and sync procedures are normally called.

Besides, this can provide an additional layer of security: currently a malicious actor, having obtained admin access to the ArgoCD web UI, can cause total destruction purely by the UI's own means. If the proposed enhancement is implemented, then there will be no way of destroying or changing protected applications, unless access to the repository holding the app definitions is obtained as well. The option to edit app manifests in the web UI remains, however, but it can (should?) respect either the deletion or sync policy to grant or deny access (see also #9031).

[blakepettersson]

A deny sync window should get you some of the way there already, this would protect against syncs at least. It might be worth extending the sync window functionality to protect against [manual] deletions as well?

[shapirus]

For one thing, sync windows documentation page mentions only AppProjects. Are they applicable to individual Applications too?

Other than that, yes, sync windows, if they can be applied on per-application basis, can pretty much solve the problem, including, it seems, the separation of automated and manual syncs.

Extending this to deletion, even with the same set of possible configuration attributes, would solve the deletion problem too.

And there is likely a good possibility of code reuse to make both syncWindows and deleteWindows use the same code to calculate the allow/deny decision.

[blakepettersson]

For one thing, sync windows documentation page mentions only AppProjects. Are they applicable to individual Applications too?

Sadly no 😞 Implementing #11755 would be a nice way of achieving this, but there's the matter of who/when would implement this...

[blakepettersson]

Duplicate of #10301

[chris-ng-scmp]

This is an important feature, we have managing core system components by ArgoCD
My colleague performed a argocd delete command with a wrong label selector, this removed almost all ArgoCD applications from the cluster including those system components in kube-system namespace

If we can have delete protection for applications, this could prevent any deletion accidentally