feat(appset): Policies create-only, create-update, sync per ApplicationSet

[speedfl]

Currently create-only, create-update, sync policies are only part of the controller launch parameters which does not allow to selectively disable deletion/updates.

Motivations

• Provide a way to temporarily to deactivate the Applications update (ie: Disable AutoSync on one of the generated Applications) without impacting all projects/teams

• Provide a way to deactivate Applications deletion on a set of ApplicationSet and enable it on other without several instances of the ApplicationSet controller

Signed-off-by: gmuselli [email protected]

Closes #11073

Note on DCO:

If the DCO action in the integration test fails, one or more of your commits are not signed off. Please click on the Details link next to the DCO action for instructions on how to resolve this.

Note: I am not able to remove the kubebuilder annotations: go-swagger/go-swagger#2687

Checklist:

• Either (a) I've created an enhancement proposal and discussed it with the community, (b) this is a bug fix, or (c) this does not need to be in the release notes.
• The title of the PR states what changed and the related issues number (used for the release note).
• I've included "Closes [ISSUE #]" or "Fixes [ISSUE #]" in the description to automatically close the associated issue.
• I've updated both the CLI and UI to expose my feature, or I plan to submit a second PR with them.
• Does this PR require documentation updates?
• I've updated documentation as required by this PR.
• Optional. My organization is added to USERS.md.
• I have signed off all my commits as required by DCO
• I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged.
• My build is green (troubleshooting builds).

[codecov]

Codecov Report

Patch coverage: 54.54% and project coverage change: +0.01 🎉

Comparison is base (99d8024) 49.56% compared to head (58cbf24) 49.58%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #11462      +/-   ##
==========================================
+ Coverage   49.56%   49.58%   +0.01%     
==========================================
  Files         256      256              
  Lines       43920    43913       -7     
==========================================
+ Hits        21770    21774       +4     
+ Misses      19987    19976      -11     
  Partials     2163     2163              

Impacted Files	Coverage Δ
applicationset/utils/policy.go	0.00% <0.00%> (ø)
...cationset/controllers/applicationset_controller.go	62.72% <100.00%> (ø)
.../apis/application/v1alpha1/applicationset_types.go	31.46% <100.00%> (+3.22%)	⬆️

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[crenshaw-dev]

If an Argo CD operator uses the sync policy as part of their security model, this could be a breaking change.

For example, if the admin enforces a controller-level create-only rule, this would allow the user to escalate their privileges to create/update/delete.

Could we add an addendum to the admin-defined policy to enable user-specified policies? For example create-only;allow-per-appset-override.

That would allow us to 1) make this feature opt-in and 2) still specify a default.

[speedfl]

@crenshaw-dev could you please help me on this ? I am not sure to understand what should I do.
I guess there is something related to the rbac configuration but I don't know where and how should I perform my changes. If you maybe have some examples

[crenshaw-dev]

@speedfl you're asking for clarification in reference to my suggested change in policy override behavior? Or is your question regarding something else?

[speedfl]

@crenshaw-dev sorry it was not really clear.
I was asking for clarification in reference to your suggested change in policy override behavior.

[crenshaw-dev]

@speedfl ah cool. So, right now we're pulling a global sync policy which is stored on r.Policy. Then we're checking things like if utils.DefaultPolicy(applicationSetInfo.Spec.SyncPolicy, r.Policy).Update() to gate behavior.

I think wherever we're setting r.Policy we should also set a r.AllowPolicyOverrides. The source of truth for that new parameter could be anything really, but I think it would make sense to toggle on the presence of ;allow-policy-overrides at the end of the string which sets r.Policy.

Then we could do a check like if utils.DefaultPolicy(applicationSetInfo.Spec.SyncPolicy, r.Policy, r.AllowPolicyOverrides).Update(), where utils.DefaultPolicy ignores the ApplicationSet's "override" policy unless r.AllowPolicyOverrides is true.

[speedfl]

@crenshaw-dev I added the --allow-policy-override which is disabled by default if --policy is specified.

Added as well unit tests.
Integration tests are ok Locally

[crenshaw-dev]

@speedfl can you rebase?

[speedfl]

@crenshaw-dev I see that create-delete sync policy has been added. I added one e2e and one unit test for this new use case

[speedfl]

Integration test are failing because of TestSyncOptionValidateFalse which also fails on master.

Pending to have it solved

[crenshaw-dev]

@speedfl fails on master locally or in CI? Could be an environment issue.

[speedfl]

@crenshaw-dev this morning everything worked. The CI was green.

[speedfl]

@ishitasequeira all done :)

[ishitasequeira]

LGTM!! Just added a quick documentation change that I missed out earlier.

@crenshaw-dev could you take another look at the PR?

[speedfl]

@ishitasequeira and @crenshaw-dev I fix the unit tests after label argocd.argoproj.io/application-set-name was added. I also changed the ALLOW_POLICY_OVERRIDE with ENABLE_POLICY_OVERRIDE which seems more in line with the environment variables I saw in the controller (ARGOCD_APPLICATIONSET_CONTROLLER_ENABLE_PROGRESSIVE_SYNCS, ARGOCD_APPLICATIONSET_CONTROLLER_ENABLE_NEW_GIT_FILE_GLOBBING, ARGOCD_APPLICATIONSET_CONTROLLER_ENABLE_LEADER_ELECTION)

[crenshaw-dev]

lfg, thanks as always @speedfl!