feat: syncing to a different revision requires override privilege#22858
Conversation
Currenttly, it is possible to sync an application to any commit disregarding the revision definied in the application. Similar to syncing to local manifests, this should be considered as 'override'. Signed-off-by: Andreas Schramm <andreas.jabs@gmail.com>
❌ Preview Environment deleted from BunnyshellAvailable commands (reply to this comment):
|
Signed-off-by: Andreas Schramm <andreas.jabs@gmail.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #22858 +/- ##
==========================================
+ Coverage 60.71% 60.81% +0.09%
==========================================
Files 350 350
Lines 60392 60430 +38
==========================================
+ Hits 36669 36749 +80
+ Misses 20797 20766 -31
+ Partials 2926 2915 -11 ☔ View full report in Codecov by Sentry. |
…erage Signed-off-by: Andreas Schramm <andreas.jabs@gmail.com>
Signed-off-by: Andreas Schramm <andreas.jabs@gmail.com>
Signed-off-by: Andreas Schramm <andreas.jabs@gmail.com>
|
Few typos, but LGTM |
Signed-off-by: Andreas Schramm <andreas.jabs@gmail.com>
blakepettersson
left a comment
There was a problem hiding this comment.
I think this generally LGTM at a glance, but would need to take a deeper look at the change itself when I have some more time.
Signed-off-by: Andreas Schramm <andreas.jabs@gmail.com>
Signed-off-by: Andreas Schramm <andreas.jabs@gmail.com>
Signed-off-by: Andreas Schramm <andreas.jabs@gmail.com>
4ae4258 to
e11c57a
Compare
|
We also require this feature. |
Signed-off-by: Andreas Schramm <andreas.jabs@gmail.com>
Signed-off-by: Andreas Schramm <andreas.jabs@gmail.com>
|
pretty please, can we get this reviewed and merged. I will gladly adapt to any recommendations, but I think we should really implement this change soon, as the current behavior is contrary to gitops... |
ppapapetrou76
left a comment
There was a problem hiding this comment.
Overall LGTM but there are few things we need to consider IMHO
split into multiple tests, as recommended. add error case unparseable value Signed-off-by: Andreas Schramm <andreas.jabs@gmail.com>
Signed-off-by: Andreas Schramm <andreas.jabs@gmail.com>
Signed-off-by: Andreas Schramm <andreas.jabs@gmail.com>
Signed-off-by: Andreas Schramm <andreas.jabs@gmail.com>
361ee7d to
567bb8b
Compare
Signed-off-by: Andreas Schramm <andreas.jabs@gmail.com>
|
i have renamed the flag, in order to make the intent clearer. |
Signed-off-by: Andreas Schramm <andreas.jabs@gmail.com>
|
@schraax I think this LGTM, but you'd need to fix the conflicts |
Signed-off-by: Andreas Schramm <andreas.jabs@gmail.com>
|
ok, i have removed the conflict. I have fulfilled any and all requests, and will continue to do so, but please, lets get this merged. |
|
@agaudreault , @blakepettersson : please, lets get this merged with 3.2. |
agaudreault
left a comment
There was a problem hiding this comment.
small nitpicks, but LGTM.
Signed-off-by: Andreas Schramm <andreas.jabs@gmail.com>
Currently, it is possible to sync an application to any commit disregarding the revision definied in the application. Similar to syncing to arbitrary local manifests, this should be considered as 'override', because it is a deviation from gitops principle.
Reasoning:
ArgoCD and GitOps offer a way to compare application state against a 'desired' state. The desired state is defined in the Application object. The application object can be protected against editing via RBAC, but this protection is rendered useless by the possibility to sync to any revision.
Fixes: #21696,#22857
This is implemented with a toggle, so it will nnot break any existing behavior.
Hint: Once the feature is toggle on, users wanting to sync to different revisions need to be granted 'override' rights on the application.
Checklist: