chore: audit controller and management controller permissions #2230
Conversation
✅ Deploy Preview for docs-kargo-akuity-io ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
| @@ -50,7 +50,6 @@ rules: | |||
| - list | |||
| - patch | |||
| - promote | |||
| - update | |||
There was a problem hiding this comment.
The only update to Stages involves removing a finalizer and that can quite easily be done with a patch.
| @@ -40,7 +40,6 @@ rules: | |||
| - get | |||
| - list | |||
| - watch | |||
| - patch | |||
There was a problem hiding this comment.
The controller doesn't need to patch Freight or Promotions at all.
| @@ -57,7 +57,6 @@ rules: | |||
| verbs: | |||
| - get | |||
| - list | |||
| - patch | |||
There was a problem hiding this comment.
The controller does not need to patch Warehouses at all.
| @@ -67,7 +67,6 @@ rules: | |||
| - warehouses/finalizers | |||
| - warehouses/status | |||
| verbs: | |||
| - update | |||
There was a problem hiding this comment.
The controller only ever patches status subresources.
| @@ -114,7 +114,6 @@ rules: | |||
| - create | |||
| - get | |||
| - list | |||
| - patch | |||
There was a problem hiding this comment.
I don't believe there is actually any need to patch AnalysisRuns. Everything works correctly e2e without this.
@hiddeco maybe you can give me a sanity check on this one, however.
There was a problem hiding this comment.
nm. I just noticed the DM you sent me about this hours ago.
It's for aborting a verification's underlying AnalysisRun.
I'll fix this in the morning.
| - apiGroups: | ||
| - argoproj.io | ||
| resources: | ||
| - analysistemplates | ||
| verbs: | ||
| - get | ||
| - list | ||
| - watch |
There was a problem hiding this comment.
These permissions are better off down here where they will not be granted in the event that Rollouts integration is completely disabled.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2230 +/- ##
==========================================
+ Coverage 46.30% 46.36% +0.05%
==========================================
Files 242 242
Lines 16777 16810 +33
==========================================
+ Hits 7769 7794 +25
- Misses 8636 8640 +4
- Partials 372 376 +4 ☔ View full report in Codecov by Sentry. |
| @@ -16,7 +16,6 @@ rules: | |||
| - get | |||
| - list | |||
| - patch | |||
| - update | |||
There was a problem hiding this comment.
This was only ever used for updating owners and finalizers, but those are easy enough to do with patches.
| @@ -46,5 +46,4 @@ rules: | |||
| - projects/status | |||
| verbs: | |||
| - patch | |||
| - update | |||
There was a problem hiding this comment.
This permission was already unused.
api/v1alpha1/helpers.go
Outdated
| func ClearAnnotations(ctx context.Context, c client.Client, obj client.Object, keys ...string) error { | ||
| kvs := make(map[string]*string, len(keys)) | ||
| for _, k := range keys { | ||
| kvs[k] = nil | ||
| } | ||
| return patchAnnotations(ctx, c, obj, kvs) | ||
| } |
There was a problem hiding this comment.
Seems we stopped clearing refresh annotation quite some time ago and that was the only thing we were ever using this for.
krancour
force-pushed
the
krancour/permissions-audit
branch
from
e1e6aca
to
e310cbc
Compare
Signed-off-by: Kent Rancourt <[email protected]>
Signed-off-by: Kent Rancourt <[email protected]>
Signed-off-by: Kent Rancourt <[email protected]>
…patch is sufficient) Signed-off-by: Kent Rancourt <[email protected]>
Signed-off-by: Kent Rancourt <[email protected]>
Signed-off-by: Kent Rancourt <[email protected]>
…tch is sufficient) Signed-off-by: Kent Rancourt <[email protected]>
Signed-off-by: Kent Rancourt <[email protected]>
Signed-off-by: Kent Rancourt <[email protected]>
krancour
force-pushed
the
krancour/permissions-audit
branch
from
e310cbc
to
415edab
Compare
No description provided.