argo-cd
argo-cd copied to clipboard
docs(proposal): decoupling app sync from control plane user w/ impersonation
Proposal for decoupling the application sync process from the Application controller service account.
Addresses issue https://github.com/argoproj/argo-cd/issues/7689 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.
Checklist:
- [x] 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.
- [x] The title of the PR states what changed and the related issues number (used for the release note).
- [x] The title of the PR conforms to the Toolchain Guide
- [ ] 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.
- [x] Optional. My organization is added to USERS.md.
- [x] 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).
- [ ] My new feature complies with the feature status guidelines.
- [ ] I have added a brief description of why this PR is necessary and/or what this PR solves.
Please see Contribution FAQs if you have questions about your pull-request.
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Comparison is base (
55713b3
) 49.26% compared to head (d2c3cdf
) 49.25%. Report is 6 commits behind head on master.
Additional details and impacted files
@@ Coverage Diff @@
## master #14255 +/- ##
==========================================
- Coverage 49.26% 49.25% -0.02%
==========================================
Files 274 274
Lines 48158 48170 +12
==========================================
- Hits 23727 23724 -3
- Misses 22085 22099 +14
- Partials 2346 2347 +1
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
@anandf the proposal as it stands is 👨🍳 💋, LGTM! Could you remove the docs/decouple-application-sync-user-using-impersonation
doc since it was a bit confusing to follow and the actual proposal is located in docs/proposals/decouple-application-sync-user-using-impersonation
?
A question on the proposal, how would this help with fixing issues like https://github.com/argoproj/argo-cd/issues/15027, https://github.com/argoproj/argo-cd/pull/10897, https://github.com/argoproj/argo-cd/issues/9606 and https://github.com/argoproj/argo-cd/issues/9515. I see this proposal being mentioned as the fix, but I can't figure out how as all examples point to ArgoCD managing the same cluster where it is also deployed and not remote clusters. I think https://github.com/argoproj/argo-cd/pull/12755 goes into a better direction to fix the mentioned issues, as long as Argo can differentiate clusters with the same cluster url.
Isn't it also assuming that ArgoCD will have the permission to impersonate all the needed service accounts which might be difficult in multi-tenant clusters where the cluster is managed by a team different than the team managing ArgoCD. The examples in the docs do not really setup the permissions, so am I missing something here? Don't you have to grant the impersonate
verb to ArgoCD so it can impersonate other SAs?
A question on the proposal, how would this help with fixing issues like #15027, #10897, #9606 and #9515. I see this proposal being mentioned as the fix, but I can't figure out how as all examples point to ArgoCD managing the same cluster where it is also deployed and not remote clusters. I think #12755 goes into a better direction to fix the mentioned issues, as long as Argo can differentiate clusters with the same cluster url.
I see the issues #15027, #10897, #9606 and #9515 are all about managing multiple cluster URLs with different namespace. Probably one namespace per tenant and each tenant having its own set of cluster secrets.
Though this proposal might not directly address, it might help to remove the need for adding multiple cluster secrets per namespace. You can login to the remote cluster as cluster-admin
user, for all the tenants, and for sync operations each tenant (cluster + namespace combination) would use a separate service account. That way you can manage with a single cluster secret and instead control the access to each application or tenant using appropriate RBAC policies. I can add an example of how to manage a remote cluster.
You can login to the remote cluster as cluster-admin user
That is exactly the problem in our case and I assume for a lot of other people judging by the feedback on the linked issues. I do not own the multi-tenant cluster, so I am not able to get a cluster-admin service account to setup in Argo. Tenants get a service account that has privileges in their own namespace only. Due to this, I don't see this proposal as a solution for that particular issue at least. The proposal in https://github.com/argoproj/argo-cd/pull/12755 (or similar) should still be implemented as that allows for least-privilege principle to be implemented in an easy way. Impersonation can still be used as needed, whether with a cluster-admin SA or a namespace SA.