kube icon indicating copy to clipboard operation
kube copied to clipboard

Add shortnames field in kube::discovery::ApiResource

Open imuxin opened this issue 2 years ago • 7 comments

Would you like to work on this feature?

No response

What problem are you trying to solve?

Use shortnames like kubectl request

Describe the solution you'd like

support shortname in resolve_api_resource function

fn resolve_api_resource(
    discovery: &Discovery,
    name: &str,
) -> Option<(ApiResource, ApiCapabilities)> {
    // iterate through groups to find matching kind/plural names at recommended versions
    // and then take the minimal match by group.name (equivalent to sorting groups by group.name).
    // this is equivalent to kubectl's api group preference
    discovery
        .groups()
        .flat_map(|group| {
            group
                .recommended_resources()
                .into_iter()
                .map(move |res| (group, res))
        })
        .filter(|(_, (res, _))| {
            // match on both resource name and kind name
            // ideally we should allow shortname matches as well
            name.eq_ignore_ascii_case(&res.kind) || name.eq_ignore_ascii_case(&res.plural)
        })
        .min_by_key(|(group, _res)| group.name())
        .map(|(_, res)| res)
}

Describe alternatives you've considered

Not yet.

Documentation, Adoption, Migration Strategy

No response

Target crate for feature

kube-core

imuxin avatar Sep 08 '22 15:09 imuxin

Thanks for the issue! This is an oversight on our end. It should have been added when we made ApiResource. It exists on the native type APIResource so shouldn't be too hard to populate it from discovery where it's made in parse_apiresource.

clux avatar Sep 08 '22 18:09 clux

If possible I would like to work on this. From a quick glance it looks like I need to:

  1. Add a short_names field to ApiResource.
  2. Update the code using ApiResource.
  3. Feed the short names to the ApiResource in parse_apiresource.

Does that sound about right?

jmintb avatar Sep 18 '22 14:09 jmintb

I think shortname should go into ApiCapabilities, because:

  1. ApiResource can be constructed bypassing API discovery, e.g. using ApiResource::erase, and in those functions you won't have enough information to fill in short_names.
  2. shortname is not necessary for talking to the k8s API.

On the other hand, ApiCapabilities was initially designed exactly for all those discovered things which are not that essential (like resource scope or supported verbs)

MikailBag avatar Sep 18 '22 20:09 MikailBag

Interesting, I think it makes more sense to include short names as an Option<Vec<String>> in ApiResource mirroring what APIResource does.

I don't think short names belong in ApiCapabilties since a short name does not describe an API capability, but rather the API resource itself, at least to me.

I am of course not intimately familiar with the project so feel free to correct me. :)

jmintb avatar Sep 21 '22 11:09 jmintb

Slow response on this, but the question on where to put this was not immediately obvious. Have written a larger thing in #1036 on direction for these two structs. I'll wait a little bit to see what others say about it, but my vote is to put this as an Option<Vec<String>> inside ApiResource and default construct it in the places where it does not fall out of discovery (and then later merge the remaining fields + methods of ApiCapabilities into ApiResource.

clux avatar Sep 28 '22 20:09 clux

Because of how close this was and how much restructuring was required, I ended up doing the two lines necessary on top of it in https://github.com/kube-rs/kube/pull/1038 to make sure the abstraction was sound. Going to unmark as help wanted. Sorry.

clux avatar Oct 01 '22 11:10 clux

No worries, that makes sense.

jmintb avatar Oct 01 '22 15:10 jmintb