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
Set defaults from params after loading files, allowing params to override #44142
Conversation
if auth_method == 'params': | ||
return DynamicClient(kubernetes.client.ApiClient(configuration)) | ||
configuration = kubernetes.client.Configuration() | ||
for key, value in iteritems(auth): |
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.
should be auth.items()
for python 2 and 3 compatibility.
Edit: I see iteritems
comes from six
, I'm just not sure why it's needed when auth.items()
will work fine
@@ -139,46 +139,37 @@ def get_api_client(self, **auth_params): | |||
auth_params = auth_params or getattr(self, 'params', {}) | |||
auth = copy.deepcopy(auth_params) | |||
|
|||
configuration = kubernetes.client.Configuration() | |||
# Pull authorization settings from environment variables |
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.
Comment here should be more extensive and state precisely what order we're processing configuration.
The test
|
|
||
kubernetes.client.Configuration.set_default(configuration) | ||
return DynamicClient(kubernetes.client.ApiClient(configuration)) | ||
|
||
|
||
def client_from_kubeconfig(self, config_file, context): |
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.
After this refactor this method is no longer referenced, probably safe to remove?
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.
Yeah, definitely
@maxamillion I've fixed the client issue that caused the build to fail, can we rekick? |
@willthames, mind doing another pass? |
@willthames, any chance you could take a look at this? I'd love to ditch my local patch... |
@willthames @maxamillion can we get this reviewed and merged soon so we can get this into 2.7 before release? |
rebuild_merge |
@fabianvf one this merges, we'll need a backport PR against |
was asked for response 10 days ago, but still no response.
rebuild_merge |
rebuild_merge |
…ride (ansible#44142) * Set defaults from params after loading files, allowing params to override * cleanup, add some comments (cherry picked from commit aa01d9d)
…ride (#45442) * Set defaults from params after loading files, allowing params to override (#44142) * Set defaults from params after loading files, allowing params to override * cleanup, add some comments (cherry picked from commit aa01d9d) * Add client_from_kubeconfig function back for 2.7.0 since it's late in the 2.7 cycle to remove module_util code.
…ride (ansible#45442) * Set defaults from params after loading files, allowing params to override (ansible#44142) * Set defaults from params after loading files, allowing params to override * cleanup, add some comments (cherry picked from commit aa01d9d) * Add client_from_kubeconfig function back for 2.7.0 since it's late in the 2.7 cycle to remove module_util code.
SUMMARY
I think this should give parameters to the module priority over incluster/kubeconfig-based authentication.
Fixes #43845
ISSUE TYPE
COMPONENT NAME
k8s
ANSIBLE VERSION