-
-
Notifications
You must be signed in to change notification settings - Fork 305
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Potential extension of PatchParams::validate to cover for invalid patch footgun #1164
Comments
Yeah, I think having some type of validation like this would be good. Putting it on straight on |
I see, I wonder if we can do something that avoids core::Request, it seems unnecessary to me to perform this extra validation for every request kube makes when we are only interested in validating a subset of requests 🤔. I am not familiar with the code so I could be completely of base here. I will experiment a bit and make a draft PR this should help drive the discussion and help me actually know what I am talking about haha. |
For context on why |
Would you like to work on this feature?
yes
What problem are you trying to solve?
When working on #1153 it became apparent that it is possible to submit invalid patches and receive no error. See this example from the
PR docs:edit: the docs have been moved to a new PR.
In the example below the patch contains a
PodSpec
and not a complete or partialPod
.It is therefore an invalid patch as the full structure of a resource,
Pod
in this case, is required whenpatching. The invalid patch will be accepted by the K8S API, but no changes will be made to the cluster.
An example with an invalid and valid patch:
Describe the solution you'd like
Extend
PatchParams::validate
with the necessary checks to avoid "silently" invalid patches.PatchParams::validate_strict
seems to already be able to cover for this case, so it might make sense to reuse that functionality 🤔This issue is meant for discussion I don't have a clear solution yet.
Describe alternatives you've considered
An alternative could be to inform users to thatthis it not a solution, asPatchParams::validate_strict
can mitigate this issue. The docs for the #1153 already indclude a mention of this.validation_strict
is supposed to only affect server-side style patches.I think having some builtin validation inPatchParams::validate
is preferable so user's can avoid invalid patches without requiring extra steps or knowledge ofPatchParams::validate_strict
.Documentation, Adoption, Migration Strategy
No response
Target crate for feature
kube-client
The text was updated successfully, but these errors were encountered: