search

kube-rs / kube

Rust Kubernetes client and controller runtime
https://kube.rs
Apache License 2.0
2.91k stars 304 forks source link

`Api::get("")` (get with empty name), will send a command to the k8s-api-server that looks like a `list` and thus will result in a serde error. #1540

Closed xMAC94x closed 2 months ago

xMAC94x commented 2 months ago

Current and expected behavior

When callind Api::get() with a name:&str which is "" kube will generate an Api call to the k8s server which is equal to the list call (prob because the trailing / is ignored here https://github.com/kube-rs/kube/blob/0.92.1/kube-core/src/request.rs#L84 . The ApiServer will send a List, e.g. PodList (or whatever) back instead a single instance. As a result serde will always fail here https://github.com/kube-rs/kube/blob/0.92.1/kube-client/src/client/mod.rs#L249

while we could argue that the behavior from kube side is completely valid, it is a bit inconvenient and makes debugging a tiny bit harder.

as an empty string is probably a invalid name for a Kubernetes ressource (needs to be verified), we can fail early on and don't need to do the actually request, especially dont fail with that serde error. For performance reason maybe only in debug mode.

If you tell me where to make the change I can also provide an PR. (In case this bug is valid for you at all)

Possible solution

Check &name within Api::get() and return an error, maybe here: https://github.com/kube-rs/kube/blob/0.92.1/kube-core/src/request.rs#L83 and return Error::Validation

Additional context

No response

Environment

AWS cluster: Server Version: v1.30.2-eks-db838b0

should be independent of this though

Configuration and features

kube = { version = "0.92", features = ["runtime", "client", "derive"] }
k8s-openapi = { version = "0.22", features = ["v1_30"] }

Affected crates

kube-core, kube-client

Would you like to work on fixing this bug?

yes

clux commented 2 months ago

Thanks for the report. I agree we should fail early on this.

I imagine this should be handled using the Error::Validation variant exposed in kube-core/../request.rs. This variant commonly brings up user errors and is typically passed up early. We could probably return it in all the get_ functions implemented on Request in request.rs (which I assume will error similarly) if name.is_empty().

xMAC94x commented 2 months ago

I am working on a PR, Interesting notes:

Edit:

Api::get("") -> executes at list, fails locally parsing result list as single entry Api::delete("", &DeleteParams::default()) -> executes as delete everything!, fails locally parsing result Api::patch("", &PatchParams::default(), somepatch) -> fails serverside with 405 method not allowed Api::replace("", &PostParams::default(), somepod) -> fails serverside with 405 method not allowed Api::get_subresource("status", "") -> fails serverside with 400 name must be provided Api::get_subresource("", "") -> executes at list, fails locally parsing, like get("") Api::get_subresource("validPodName", "") -> fails serverside with 404 the server could not find the requested resource Api::create_subresource() -> not testable, i couldn't reproduce a working scenario Api::patch_subresource("status", "") -> fails serverside with 400 name must be provided Api::replace_subresource("status", "") -> fails serverside with 400 name must be provided Api::patch_metadata("", &PatchParams::default(), somepatch) -> fails serverside with 405 method not allowed Api::get_metadata("") -> fails serverside with 406 not acceptable (message: "you requested PartialObjectMetadata, but the requested object is a list (*apps.DeploymentList)")

TLDR: