client_ext for Client::get and Client::list

[clux]

AKA Api calls without Api for #1032

This is a smaller scope variant of #1037 that solves the basic Api like functionality on Client directly (opt-in feature) without getting bogged down in other requirements around apicaps and scope dependencies by using indirection traits instead.

This PR primarily adds:

• Client::get and Client::list that can be used by all resources that allow for a scoped operation
• Namespace newtype have From impls from &str, String and k8s_openapi::...::Namespace.
• Cluster empty marker type

Where you have to supply the scope as either &Cluster or &Namespace(String):

// across-namespace list
for j in client.list::<Job>(&lp, &Cluster).await? {
    println!("Found job {} in {}", j.name_any(), j.namespace().unwrap());
}

// namespaced get
let default: Namespace = "default".into();
let svc = client.get::<Service>("kubernetes", &default).await?;
assert_eq!(svc.name_unchecked(), "kubernetes");

// global get
let ca = client.get::<ClusterRole>("cluster-admin", &Cluster).await?;
assert_eq!(ca.name_unchecked(), "cluster-admin");
// NB: leaving GetParams compatible get_with for later

There are also some trait magic underneath the hood to ensure you can only do cluster level listing on a cluster scoped object, and only namespace level listing on namespaced objects, but we still retain compatibility with dynamic objects thanks to a bit of magic from @nightkr. Most of this is private. Maybe it can be made public later, but feels early now.

The node_watcher shows how this can be used (showing we can avoid the clone and api construction) for a .list.

Choices

kube::client::scope for scope markers

Currently kube::client::scope is exported. In the future we will likely move this into kube_core once we stabilise it.

naming

We need to avoid clashes with similar names:

• kube::discovery::Scope - what you get from discovery
• Resource::Scope - raw scope type sometimes used to constain Api ctors
• actual scope trait implementors in kube::core

Current is slightly awkward, and am open to suggestions, but i think we can start with this and eventually make this easier later.

impl Client over async trait

impl Client methods that are pulled in via a module feature. Allows for better default visibility, less imports needed, puts less constraints on our code), but it does mean that all the docs get inlined along with the constructors and raw, low-level methods on Client.

exploration

As you can see, you can document each impl Client block ^ (and the blocks do appear in order in the main body), but on the left you see the Client::get and Client::list methods get sorted in with the full list of low-level client methods, which is not great. Maybe that is more of a rust-doc bug, but it makes the docs look a bit cluttered.

Investigated the alternative in in #1375 (comment) and we talked about this in the meet and none of us seemed particularly keen on the trait.

[codecov]

Codecov Report

Attention: Patch coverage is 88.37209% with 5 lines in your changes are missing coverage. Please review.

Project coverage is 72.3%. Comparing base (df37c99) to head (87ec044).

Additional details and impacted files

@@           Coverage Diff           @@
##            main   #1375     +/-   ##
=======================================
+ Coverage   72.2%   72.3%   +0.2%     
=======================================
  Files         75      76      +1     
  Lines       6485    6527     +42     
=======================================
+ Hits        4680    4717     +37     
- Misses      1805    1810      +5     

Files	Coverage Δ
kube-client/src/client/mod.rs	72.0% <ø> (ø)
kube-client/src/config/mod.rs	45.4% <ø> (ø)
kube-client/src/discovery/apigroup.rs	88.1% <ø> (ø)
kube-client/src/lib.rs	75.2% <ø> (ø)
kube-client/src/client/client_ext.rs	88.4% <88.4%> (ø)

... and 1 file with indirect coverage changes

[clux]

This is in a state that could benefit from a review to see that we are at least on the same page.

Not bothering with full details on docs/tests here yet until we agree (just did one basic one), and MSRV failures due to home (error: package home v0.5.9 cannot be built because it requires rustc 1.70.0 or newer, while the currently active rustc version is 1.65.0) can be fixed in a separate PR.

[clux]

Have added tests and docs for the new public interfaces now so am not planning on changing this PR more without feedback.

It's only doing list and get, but there are the 3 small contentious bits that could go in other directions noted above, so it would be nice to get your inputs @Dav1dde / @nightkr 🙏

[clux]

Updated and edited post body. Incorporated a bunch of suggestions from @nightkr with renames and tweaks. Have tried to make the docs look nice (which is a slight challenge when the methods merge with the normal client ctors). I'll mention this tomorrow in the meet.

[clux]

I spent an houlr playing around with async traits as an investigation for whether it would make docs better, and am now under the impression that they are not significantly more helpful for what we are trying to accomplish.

If we convert the impl Client into a trait such as:

pub trait ClientExt {
    async fn get<K>(&self, name: &str, scope: &impl ObjectUrl<K>) -> Result<K>
    where
        K: Resource + DeserializeOwned + Clone + Debug,
        <K as Resource>::DynamicType: Default;
    async fn list<K>(&self, lp: &ListParams, scope: &impl CollectionUrl<K>) -> Result<ObjectList<K>>
    where
        K: Resource + DeserializeOwned + Clone + Debug,
        <K as Resource>::DynamicType: Default;
}

and then blanket impl later:

impl ClientExt for Client {
    async fn get<K>(&self, name: &str, scope: &impl ObjectUrl<K>) -> Result<K>
    where
        K: Resource + DeserializeOwned + Clone + Debug,
        <K as Resource>::DynamicType: Default,
    {
        let mut req = Request::new(scope.url_path())
            .get(name, &GetParams::default())
            .map_err(Error::BuildRequest)?;
        req.extensions_mut().insert("get");
        self.request::<K>(req).await
    }
    // ...
}

then we can indeed export ClientExt and import it in the user land and use it, and docs.rs ends up with a more specialised page for just these methods:

but it has a couple of downsides:

1. The trait output at the top of the page is very verbose due to complex trait constraints, and makes the trait the focus (as if the users was going to implement it) when it's really the guaranteed methods that is the focus (note "required methods").
2. Trait constraints duplicated when they are specialized. Seems insufficient to only put it in one place and can't omit them from the trait because of the requirement from ObjectUrl and CollectionUrl
3. We get a huge warning that tells us not to use this:

warning: use of `async fn` in public traits is discouraged as auto trait bounds cannot be specified
   --> kube-client/src/client/client_ext.rs:157:5
    |
157 |     async fn get<K>(&self, name: &str, scope: &impl ObjectUrl<K>) -> Result<K>
    |     ^^^^^
    |
    = note: you can suppress this lint if you plan to use the trait only in your own code, or do not care about auto traits like `Send` on the `Future`
    = note: `#[warn(async_fn_in_trait)]` on by default
help: you can alternatively desugar to a normal `fn` that returns `impl Future` and add any desired bounds such as `Send`, but these cannot be relaxed without a breaking API change
    |
157 -     async fn get<K>(&self, name: &str, scope: &impl ObjectUrl<K>) -> Result<K>
157 +     fn get<K>(&self, name: &str, scope: &impl ObjectUrl<K>) -> impl std::future::Future<Output = Result<K>> + Send
    |

so am not hugely enthusiastic about the trait approach either. leaving this comment as a justification and linking to it above. ultimately this stuff can be changed down the line later since it's under an unstable flag.