kube icon indicating copy to clipboard operation
kube copied to clipboard

Potential extension of PatchParams::validate to cover for invalid patch footgun

Open jmintb opened this issue 1 year ago • 4 comments

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 partial Pod. It is therefore an invalid patch as the full structure of a resource, Pod in this case, is required when patching. 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:

use k8s_openapi::api::core::v1::{Pod, PodSpec};
use kube::{Api, api::{PatchParams, Patch}};

# async fn wrapper() -> Result<(), Box<dyn std::error::Error>> {
# let client = kube::Client::try_default().await?;
let pods: Api<Pod> = Api::namespaced(client, "apps");
let pp = PatchParams::default();

let invalid_patch: PodSpec = serde_json::from_value(serde_json::json!({
                "activeDeadlineSeconds": 5
}))?;

// This will have no effect on mypod.
pods.patch("mypod", &pp, &Patch::Strategic(invalid_patch)).await?;

let valid_patch: Pod = serde_json::from_value(serde_json::json!({
         "spec": {
                "activeDeadlineSeconds": 5
           }
}))?;

// This will set activeDeadlineSeconds to 5.
pods.patch("mypod", &pp, &Patch::Strategic(invalid_patch)).await?;

# Ok(())
# }

Describe the solution you'd like

Extend PatchParams::validate with the necessary checks to avoid "silently" invalid patches.

PatchParams::validate_strictseems to already be able to cover for this case, so it might make sense to reuse that functionality :thinking:

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 that PatchParams::validate_strict can mitigate this issue. The docs for the #1153 already indclude a mention of this.~~ this it not a solution, as validation_strict is supposed to only affect server-side style patches.

~~I think having some builtin validation in PatchParams::validate is preferable so user's can avoid invalid patches without requiring extra steps or knowledge of PatchParams::validate_strict.~~

Documentation, Adoption, Migration Strategy

No response

Target crate for feature

kube-client

jmintb avatar Mar 18 '23 08:03 jmintb