-
-
Notifications
You must be signed in to change notification settings - Fork 332
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add stacked KUBECONFIG
support
#411
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
delimit comment + option nits
looking great. you are really cleaning up the place.
kube/src/config/file_config.rs
Outdated
} | ||
match self.extensions.as_mut() { | ||
Some(mut base_exts) => { | ||
if let Some(next_exts) = next.extensions.take() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feels like the type of thing where extensions
should be Vec<String>
with [serde(default, skip_serialize_if = "Option::is_none")]
rather than Option<Vec<String>>
since we treat missing as empty anyway.
Though it seems we have a convention for Option<Vec<.>>
atm so it's a bit of a breaking change 🤔
d495274
to
f466c52
Compare
I think it's good enough now. I'll merge after cleaning up the commits. |
f466c52
to
8dce23d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looking great. got tests, super readable now.
there's enough to make a new release with this, are you at a good point for 0.50.0?
Yeah 👍 I don't think there's anything else I'd like to include in the next release and want to unblock users with this fix. |
@clux Thank you for being receptive and quick to respond. It's been a pleasure to work on I know I went a little crazy the past week :) |
It's been amazing. The huge hyper/tower hotswap, connection upgrading, tls rewrites, config rewrites, plus all the other hard stuff you've tackled like websockets subresources and schemas. Really appreciate all the work you've done here. It's made the library a lot more usable and the rust kubernetes story a lot more attractive (even the crd reception was huge). Looking forward to hearing what issues you uncover on the controller side. I am back at work now as well, but reviewing your prs has been the easiest thing to do on the side, always happy to look at more :-) |
config::util
current_context
inKubeConfig
is nowOption<String>
Kubeconfig
are now remapped to absolute path after reading to support relative paths in stacked config. Assumes paths are unicode (should be fine because Go doesn't have OsString concept).kube
only needscurrent_context
,contexts
,clusters
, andauth_infos
. If we don't need them, we should remove other fields fromKubeconfig
as well because it'll be misleading. I think it's fine to keep.Option<Preferences>
andOption<Vec<NamedExtension>>
, we're just taking the firstSome
because it's unclear what should be done.apiVersion
/kind
is set and mismatches. Not sure if this is correct behavior.Closes #132