fix: Only consider resources which supports appropriate verb for any given operation#423
Conversation
…given operation Signed-off-by: jannfis <jann@mistrust.net>
Signed-off-by: jannfis <jann@mistrust.net>
Codecov Report
@@ Coverage Diff @@
## master #423 +/- ##
==========================================
+ Coverage 54.12% 54.37% +0.24%
==========================================
Files 41 41
Lines 3104 3110 +6
==========================================
+ Hits 1680 1691 +11
+ Misses 1259 1252 -7
- Partials 165 167 +2
Continue to review full report at Codecov.
|
pkg/utils/kube/kube.go
Outdated
| } | ||
| for _, r := range resources.APIResources { | ||
| if r.Kind == gvk.Kind { | ||
| if r.Kind == gvk.Kind && isSupportedVerb(&r, verb) { |
There was a problem hiding this comment.
I'm not sure if users will ever run into a misleading message where we report NotFound error in logs, not because the GVK doesn't exist, but rather the verb isn't supported and we did not distinguish between the two. Since it's a pretty rare corner case, I don't think it's necessary but wanted to point it out.
There was a problem hiding this comment.
I actually agree with this comment, @jessesuen. Edge case or not, I'm going to fix this.
There was a problem hiding this comment.
Changed the logic to return MethodNotSupported if the GVK resolves to a resource which does not support the given verb.
Also, added some unit tests around all that.
crenshaw-dev
left a comment
crenshaw-dev
left a comment
There was a problem hiding this comment.
Was puzzled about one thing, but lgtm.
| // isSupportedVerb returns whether or not a APIResource supports a specific verb. | ||
| // The verb will be matched case-insensitive. | ||
| func isSupportedVerb(apiResource *metav1.APIResource, verb string) bool { | ||
| if verb == "" || verb == "*" { |
There was a problem hiding this comment.
Is this something that can happen? As far as I can tell, all instances of the argument are hard-coded as values others than these two.
There was a problem hiding this comment.
Good question. This is intended to allow the caller to just say "I don't care about the support verbs, just resolve it". It might be a valid use-case for some.
Signed-off-by: jannfis <jann@mistrust.net>
|
Kudos, SonarCloud Quality Gate passed!
|
…given operation (#423) * fix: Only consider resources which supports appropriate verb for any given operation Signed-off-by: jannfis <jann@mistrust.net> * Fix unit tests Signed-off-by: jannfis <jann@mistrust.net> * Return MethodNotSupported and add some tests Signed-off-by: jannfis <jann@mistrust.net>
|
I have cherry-picked this change into release-0.7 I want to pull this change into an Argo CD v2.4.z release after the current code freeze period. |
) * Update cluster.go Fixes #423 Signed-off-by: Daniel Urgell <urgell.d@gmail.com> * Use c.log.Info instead of c.log.Warning Signed-off-by: Daniel Urgell <urgell.d@gmail.com> * Changed c.log.Info format to fix type string in argument Signed-off-by: Daniel Urgell <urgell.d@gmail.com> Co-authored-by: Daniel Urgell <daniel@bluelabs.eu>
3 participants
This will consider the verbs supported by the API any GVK resolves to.
Fixes #422
Signed-off-by: jannfis jann@mistrust.net