Implement client native object reference fetching

[Danil-Grigorev]

Motivation

One of the ways custom resources are declaring dependency on each other, is by providing and ObjectReference field. Having one, still requires to manually prepare an appropriate client/api setup, extract namespace and name and validate the kind, api_version fields. This PR allows to make ObjectReferences native to client, and simplify their use. Additionally it provides similar handling for OwnerReferences.

Solution

Implement a client.fetch method, allowing for the use of the following syntax:

// Actual ObjectReference content is intended to come from a CR
let svc: Service = client.fetch(&ObjectReference::default())
let node: Node = client.fetch(&OwnerReference::default()).await?;
let pod: Pod = client.fetch(&OwnerReference::default().within("default".into())).await?;
// Still the same pod
let pod: DynamicObject = client.fetch(&pod.object_ref(&())).await?;
let local_ref = LocalObjectReference {name: svc.name_any().into()};
let svc: Service = client.fetch(&local_ref.within(svc.namespace())).await?;

Current implementation also constants ability for specifying namespace for the OwnerReference from the resource. This is only allowed on non-cluster scoped resources.

[codecov]

Codecov Report

Attention: Patch coverage is 78.70370% with 23 lines in your changes missing coverage. Please review.

Project coverage is 75.1%. Comparing base (5aa8f83) to head (191435e).

Additional details and impacted files

@@           Coverage Diff           @@
##            main   #1511     +/-   ##
=======================================
+ Coverage   75.0%   75.1%   +0.2%     
=======================================
  Files         78      78             
  Lines       6872    6980    +108     
=======================================
+ Hits        5151    5240     +89     
- Misses      1721    1740     +19     

Files	Coverage Δ
kube-core/src/gvk.rs	75.5% <53.4%> (-7.1%)	⬇️
kube-client/src/client/client_ext.rs	84.5% <82.8%> (-3.6%)	⬇️

... and 4 files with indirect coverage changes

[Danil-Grigorev]

Marking this a WIP, as I’m trying to expand it to OwnerReferences, and impl Into<Option<smth>> is not cooperating with linter or Option into Option (generally looks like is not needed).

Edit: Updated, does not look ideal, but would like to know WDYT.

[clux]

i like the idea of this. possible suggestion included.

[clux]

I guess this will be a new required trait to use for people doing dynamic object ref -> object url inferences.

I think this is a good solution, but probably one that makes us perhaps want to consider a prelude down the road.

[clux]

Conversion here makes sense, but wondering if will it work for other "objectreference-like things" with less information. In particular, would we be able to handle:

• TypedLocalObjectReference - namespace implied same as the object we got it from
• LocalObjectReference - harder, since this needs to get all type data from parent type

not saying we need to, but it's good to know the limits of the design. what would we need to do support LocalObjectReference? Would it even make sense in this design?

[Danil-Grigorev]

I believe that both of them are quite ambiguous. I pushed a change which tries to support for both, but for LocalObjectReference there is a need of a real field which uses it, which can come only from CR. Also the definitions are open to interpretation. TypedLocalObjectReference has an api_group which is confusing, as it is different sometimes from api_version (which is actually group/version always). Forcing some definition there seems a bit questionable to me, need to think it through. Here are some examples:

• as a group/version
• as a group

I’d like to try if the exported trait can be implemented externally, and if possibly wrapping a LocalObjectReference and implementing the NamespaceRef trait for the wrapped type, providing From implementation + some kopium changes to recognize selectively LocalObjectReference can be a thing.

[clux]

LocalObjectReference there is a need of a real field which uses it

i found one use-case on PodSpec::image_pull_secrets which implicitly references Secret object as per kubernetes docs. this is more awkward than I expected because it's not the same resource as the one it was found on.

TypedLocalObjectReference has an api_group which is confusing

..yeah. if that's the case then it's probably not super important to worry about it. people can write converters from one object ref to the one they want to disambiguate. that said though, i would say the first example there is wrong. apigroup should be just the group, and the usage of TypedLocalObjectReference is an indication that people should do discovery to make the version concrete.

[Danil-Grigorev]

So after some consideration I removed the use of TypedLocalObjectReference. This ambiguity translates to kube-rs as well. What the library needs, I think, is a rough equivalent of CR scheme.go, where a type registers itself and provides a bunch of helpers, some of them using discovery client, some of them in-memory. This would allow to look up the version for the GroupKind the TypedLocalObjectReference represents, probably even without using discovery, as a key from GroupKind<->GroupVersionKind mapping.

I’d also love to have this bits being autogenerated by the “derive” feature sometime in the future. WDYT?

[clux]

Yeah, I think this is fair. The scheme registration I am not sure how it would work, but some user input feels necessary to solve the ambiguity. Happy to look at PRs or help sketch on issues for it (hard to envision it all from my POV now).

[clux]

I think there are 2 more types that effectively we could implement for free as they generally have enough information:

• TypedObjectReference
• CrossVersionObjectReference

perhaps good to do one/two of those to show the system is extensible enough.

[clux]

it would be good to see have an error test as well, to see what happens if people type inference the wrong thing (e.g. :Service with ObjectRef data for a ClusterRole say). might not be able to do something useful, but if we could reduce confusion, then that's less issues for us down the line.

[clux]

I think this is very useful, and provides an elegant way to solve a generic problem. Thanks a lot for thinking up this (and making fetch happen).

Any follow-up stories feel free to write up if you have any, I will write up a few tomorrow; client_ext module setup (file is getting big), and a prelude sketch. If you have any other things you like to clean up feel free to, but I am at least happy to send this out as is (under the unstable feature).