search

Arnavion / k8s-openapi

Rust definitions of the resource types in the Kubernetes client API
Apache License 2.0
386 stars 41 forks source link

`StatefulSetSpec::default()` already has a `selector` and `serviceName` when serializing #112

Open nightkr opened 2 years ago

nightkr commented 2 years ago

Repro

use k8s_openapi::api::apps::v1::StatefulSetSpec;
println!("{}", serde_yaml::to_string(&StatefulSetSpec::default()).unwrap());

Expected

---
template: {}

Produced

---
selector: {}
serviceName: ""
template: {}

Why is this a problem?

LabelSelector and string are both atomic (see in curl $K8S_API/openapi/v2 | jq '.definitions."io.k8s.apimachinery.pkg.apis.meta.v1.LabelSelector"' that "x-kubernetes-map-type": "atomic"), which means that server-side apply will replace them outright rather than trying to merge them. This prevents server-side apply from updating StatefulSet objects safely.

Versions

[[package]]
name = "k8s-openapi"
version = "0.13.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "4f8de9873b904e74b3533f77493731ee26742418077503683db44e1b3c54aa5c"
dependencies = [
 "base64",
 "bytes",
 "chrono",
 "serde",
 "serde-value",
 "serde_json",
]
nightkr commented 2 years ago

I think this comes down to a larger issue about when things are Optional, and how we handle patches. Currently, (AFAIK) the policy is that all properties that are not required (in the OpenAPI) are wrapped in Option, which makes some sense for standard CRUD operations.

However, this makes less sense for patches, where almost no field is statically required since a missing value just means that it falls through to the old value.

On the other hand, non-atomic values (objects that are not explicitly tagged as atomic, as well as list-maps) don't have much of a point in being Optional either, since a missing value is essentially equivalent to an empty value ({} or [], respectively).

Maybe we will end up needing separate types to handle CRUD vs patch properly (either completely separate like Go handles it, or parametrized with some type mode mechanism). That seems unfortunate (both in terms of compile time and teachability), but I'm not sure I see a way around it... :/

clux commented 2 years ago

Don't know if you've seen, but this is something they had to tackle on the go side as well with Apply* variants of all the structs. Basically an entirely parallel world of structs where everything is optional to get around this problem for server side apply.

Stuff for it is under KEP 2155 and some of my notes under https://github.com/kube-rs/kube-rs/issues/649

Arnavion commented 2 years ago

Stuff for it is under KEP 2155 and some of my notes under kube-rs/kube-rs#649

Thanks for those links.

For my reference: