fix(providers/cncf-kubernetes): apply verify_ssl=False to returned ApiClient in KubernetesHook - #63478
Merged
potiuk merged 3 commits intoMar 14, 2026
Conversation
When disable_verify_ssl=True, _disable_verify_ssl() sets the global Configuration default via Configuration.set_default(). However, config.load_kube_config() called immediately after with client_configuration=None creates a fresh Configuration instance, loads the kubeconfig into it (with verify_ssl=True), and overwrites the global default via Configuration.set_default() again. The _TimeoutK8sApiClient() created without arguments then calls Configuration.get_default_copy() and gets verify_ssl=True. Fix by ensuring that when disable_verify_ssl=True, self.client_configuration is initialized from the updated default (with verify_ssl=False) before calling load_kube_config(), then re-applying verify_ssl=False after the load (since load_and_set() may overwrite it), and finally passing the configuration explicitly to _TimeoutK8sApiClient. Closes apache#56432 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
antonio-mello-ai
requested review from
hussein-awala,
jedcunningham and
jscheffl
as code owners
March 12, 2026 21:01
boring-cyborg
Bot
added
area:providers
provider:cncf-kubernetes
2 tasks
jscheffl
reviewed
Mar 12, 2026
Replace `assert` with conditional check to satisfy both mypy (union-attr on Optional type) and ruff (S101 no-assert rule). Fixes apache#56432 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ructor Move verify_ssl handling into _TimeoutK8sApiClient.__init__ so callers pass disable_verify_ssl as a flag instead of repeating the config adjustment at every call site. Reduces risk of future inconsistency. Addresses review feedback from jscheffl. Fixes apache#56432 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Member
|
@jscheffl — Could you please check whether your review feedback on this PR has been addressed? @antonio-mello-ai appears to have responded to your comments. @antonio-mello-ai, do you believe the reviewer's concerns have been resolved? If the concerns are resolved, please resolve the conversation threads. Thank you! |
jscheffl
approved these changes
Mar 13, 2026
jscheffl
left a comment
jscheffl
left a comment
Contributor
There was a problem hiding this comment.
Looks good for me, tests are all green. As this is a security related change I'd request another pair of eyes from a second maintainer.
potiuk
approved these changes
Mar 14, 2026
abhijeets25012-tech
pushed a commit
to abhijeets25012-tech/airflow
that referenced
this pull request
Apr 9, 2026
…iClient in KubernetesHook (apache#63478) * Fix KubernetesHook verify_ssl flag not applied to returned ApiClient When disable_verify_ssl=True, _disable_verify_ssl() sets the global Configuration default via Configuration.set_default(). However, config.load_kube_config() called immediately after with client_configuration=None creates a fresh Configuration instance, loads the kubeconfig into it (with verify_ssl=True), and overwrites the global default via Configuration.set_default() again. The _TimeoutK8sApiClient() created without arguments then calls Configuration.get_default_copy() and gets verify_ssl=True. Fix by ensuring that when disable_verify_ssl=True, self.client_configuration is initialized from the updated default (with verify_ssl=False) before calling load_kube_config(), then re-applying verify_ssl=False after the load (since load_and_set() may overwrite it), and finally passing the configuration explicitly to _TimeoutK8sApiClient. Closes apache#56432 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: narrow client_configuration type for mypy union-attr check Replace `assert` with conditional check to satisfy both mypy (union-attr on Optional type) and ruff (S101 no-assert rule). Fixes apache#56432 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor: centralize disable_verify_ssl in _TimeoutK8sApiClient constructor Move verify_ssl handling into _TimeoutK8sApiClient.__init__ so callers pass disable_verify_ssl as a flag instead of repeating the config adjustment at every call site. Reduces risk of future inconsistency. Addresses review feedback from jscheffl. Fixes apache#56432 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
KubernetesHookso thatdisable_verify_ssl=Trueactually disables SSL verification on the returnedApiClientload_kube_config()overwritesConfiguration.set_default()with a freshConfiguration(verify_ssl=True), so the earlier_disable_verify_ssl()call is lostverify_ssl=Falseafter each config load, and pass the configuration explicitly to_TimeoutK8sApiClientRoot Cause
_disable_verify_ssl()callsConfiguration.set_default()to registerverify_ssl=Falseglobally. However,load_kube_config()(called right after) creates a newConfigurationinstance withverify_ssl=Trueby default and overwrites the global default. The_TimeoutK8sApiClient()then getsverify_ssl=Truefromget_default_copy().Test Plan
kube_config_path,kube_config,config_dict,default_client)prek)Closes #56432
🤖 Generated with Claude Code
Co-Authored-By: Claude Opus 4.6 noreply@anthropic.com