kube-rs / kube

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

Add a `kube::k8s` re-export module for stable apis #1613

Open clux opened 1 month ago

clux commented 1 month ago

Adds kube_core/src/k8s.rs and makes examples and kube use those imports internally.

grim-area-2024-10-19-17_43_56

Will propagate to kube_client / kube_runtime later if people are happy with this idea and the naming convention (module_name + version no delimiter).

Have been thinking about this before, but was a bit unsure of a) how to do it, and b) if it would make k8s-pb usage harder (it would not - it's kind of necessary for it).

EDIT: sorry, i pinged all of you basically. looking for quick opinions / gut feelings. if you have none, feel free to ignore this.

codecov[bot] commented 1 month ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 75.3%. Comparing base (ecbdafc) to head (6e1dfb4).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1613 +/- ## ===================================== Coverage 75.3% 75.3% ===================================== Files 82 82 Lines 7343 7343 ===================================== Hits 5527 5527 Misses 1816 1816 ``` | [Files with missing lines](https://app.codecov.io/gh/kube-rs/kube/pull/1613?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kube-rs) | Coverage Δ | | |---|---|---| | [kube-core/src/crd.rs](https://app.codecov.io/gh/kube-rs/kube/pull/1613?src=pr&el=tree&filepath=kube-core%2Fsrc%2Fcrd.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kube-rs#diff-a3ViZS1jb3JlL3NyYy9jcmQucnM=) | `83.0% <ø> (ø)` | | | [kube-core/src/dynamic.rs](https://app.codecov.io/gh/kube-rs/kube/pull/1613?src=pr&el=tree&filepath=kube-core%2Fsrc%2Fdynamic.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kube-rs#diff-a3ViZS1jb3JlL3NyYy9keW5hbWljLnJz) | `77.1% <ø> (ø)` | | | [kube-core/src/gvk.rs](https://app.codecov.io/gh/kube-rs/kube/pull/1613?src=pr&el=tree&filepath=kube-core%2Fsrc%2Fgvk.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kube-rs#diff-a3ViZS1jb3JlL3NyYy9ndmsucnM=) | `75.5% <ø> (ø)` | | | [kube-core/src/labels.rs](https://app.codecov.io/gh/kube-rs/kube/pull/1613?src=pr&el=tree&filepath=kube-core%2Fsrc%2Flabels.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kube-rs#diff-a3ViZS1jb3JlL3NyYy9sYWJlbHMucnM=) | `90.3% <ø> (ø)` | | | [kube-core/src/metadata.rs](https://app.codecov.io/gh/kube-rs/kube/pull/1613?src=pr&el=tree&filepath=kube-core%2Fsrc%2Fmetadata.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kube-rs#diff-a3ViZS1jb3JlL3NyYy9tZXRhZGF0YS5ycw==) | `67.7% <ø> (ø)` | | | [kube-core/src/schema.rs](https://app.codecov.io/gh/kube-rs/kube/pull/1613?src=pr&el=tree&filepath=kube-core%2Fsrc%2Fschema.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kube-rs#diff-a3ViZS1jb3JlL3NyYy9zY2hlbWEucnM=) | `90.2% <ø> (ø)` | | | [kube-core/src/util.rs](https://app.codecov.io/gh/kube-rs/kube/pull/1613?src=pr&el=tree&filepath=kube-core%2Fsrc%2Futil.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kube-rs#diff-a3ViZS1jb3JlL3NyYy91dGlsLnJz) | `100.0% <100.0%> (ø)` | | | [kube/src/lib.rs](https://app.codecov.io/gh/kube-rs/kube/pull/1613?src=pr&el=tree&filepath=kube%2Fsrc%2Flib.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kube-rs#diff-a3ViZS9zcmMvbGliLnJz) | `88.5% <ø> (ø)` | |
nightkr commented 1 month ago

Hm, not quite sure I see the intended use case. You'd still need to take a direct dependency on k8s-openapi to set the k8s compat level feature, right? And we wouldn't really gain much in API stability since breaking k8s-openapi upgrades are still breaking.

The one difference I see remaining, then, is that import paths become shorter to type out manually? (As opposed to letting Rust-Analyzer manage imports.)

clux commented 1 month ago

not quite sure I see the intended use case. [..] import paths become shorter to type out manually?

As it stands, yes. The main benefit is an alternative shorter import path structure. Even with rust analyzer I find myself copy pasting strings because it's not always good enough to navigate 4-5 levels deep, but I usually know the apigroup.

You'd still need to take a direct dependency on k8s-openapi to set the k8s compat level feature, right?

With this PR, yes. But we can avoid the peer-dependency this way if we pin k8s-openapi to latest (which I would like to propose separately to reduce the api surface / variability and simplify our builds).

And we wouldn't really gain much in API stability since breaking k8s-openapi upgrades are still breaking.

Yep. We are releasing a minor right now at every k8s-openapi release, so it's kind of equivalent. This is only intended as an alternative import path. My plan is that this can play into k8s-pb type multiplexing as this type of layer has been very helpful so far in https://github.com/kube-rs/kube/pull/1602. I liked using it there, so thought to propose it as a thing on its own first.

Danil-Grigorev commented 4 weeks ago

Personally I think having a re-export of k8s-openapi in kube is useful, as it provides options and allows for extending core k8s types functionality in the crate more straightforward. LGTM

nightkr commented 3 weeks ago

IMO the k8s-pb saga is another case for reducing the specific coupling to k8s-openapi, but I guess some of that also has to do with how that integration would end up working. I should go look into that...

Dav1dde commented 3 weeks ago

I think re-exporting only makes sense if there is additional value provided by kube-rs, if it's just re-export to have shorter imports I don't think this is worth it. It may even be confusing if users get k8s-openapi errors without now directly depending on it (e.g. need to select a feature).

If kube-rs now pins k8s-openapi to latest this becomes a bit nicer.