Provide cluster info to exec plugins

[aviramha]

Closes #1328

[codecov]

Codecov Report

Merging #1331 (5f67daa) into main (bdda76e) will decrease coverage by 0.0%.
The diff coverage is 60.9%.

Additional details and impacted files

@@           Coverage Diff           @@
##            main   #1331     +/-   ##
=======================================
- Coverage   72.1%   72.1%   -0.0%     
=======================================
  Files         75      75             
  Lines       6390    6434     +44     
=======================================
+ Hits        4606    4635     +29     
- Misses      1784    1799     +15     

Files	Coverage Δ
kube-client/src/config/file_loader.rs	86.3% <100.0%> (+1.2%)	⬆️
kube-client/src/config/mod.rs	45.4% <ø> (ø)
kube-client/src/client/auth/mod.rs	50.3% <0.0%> (-0.4%)	⬇️
kube-client/src/config/file_config.rs	75.5% <63.2%> (-2.0%)	⬇️

[aviramha]

Hey @clux - I believe this is ready for review - I haven't tested it properly (nor written tests) - I plan to test it manually soon, then write some tests when you thumbs up the implementation as I took some decisions that can be controversial (how we store the cluster info, how we translate it, when we do it, etc.)

[clux]

is the controversial bit the extra struct stored on ExecConfig (the ExecAuthCluster part?)

if this is deviating from the upstream ExecConfig (which it looks like it does), it might be worth considering storing this in the preferences section of the kubeconfig if that is possible (because it allows arbitrary data storage in there without breaking support for kubectl).

[aviramha]

@clux I don't think it belongs into the preferences as it's not serialized/deserialized and it'd be weird to pass it that way.
Note that I borrowed the idea that I didn't like from upstream Go implementation:
https://github.com/kubernetes/client-go/blob/30eba26adb82c17a65cb10dbd9534c742a496404/tools/clientcmd/api/types.go#L206

[aviramha]

Regarding tests, I think the best thing would to incorporate the code change in file_loader as well but it's not exposed to the crate (so we can see data is being set correctly in auth_info).
Any suggestion?

[aviramha]

Short update - the impacted user tested my changed and it worked :)

[clux]

i think this in general makes sense.
some questions initially

[clux]

The one thing i would like to have here before merging is a serialization test case; some yaml that users have that is passed to the deserialize impl for the struct (just so we know what we are dealing with down the line during inevitable refactorings).

[aviramha]

The one thing i would like to have here before merging is a serialization test case; some yaml that users have that is passed to the deserialize impl for the struct (just so we know what we are dealing with down the line during inevitable refactorings).

Done! :)

[clux]

Thank you!

[clux]

Forgot to merge this in during the release setup for 0.87, but it's in now - for 0.88.
Thanks again!

[aviramha]

Thank you @clux for the great maintenance!! ❤️