Skip to content

Avoid caching kubeconfig for exec-based auth in AsyncKubernetesHook #61738

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.

Sign up for GitHub

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

Closed
Codingaditya17 wants to merge 6 commits into from
Closed

Avoid caching kubeconfig for exec-based auth in AsyncKubernetesHook #61738

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a4c89ae
Fix: avoid caching kubeconfig for exec-based auth in AsyncKubernetesHook
Feb 10, 2026
2403e87
Fix static checks (formatting and whitespace)
Feb 10, 2026
5f9326e
Fix: apply ruff formatting
Feb 10, 2026
69505b4
Fix: properly scope kubeconfig caching to non-exec auth in AsyncKuber…
Feb 11, 2026
2212776
Fix: properly scope kubeconfig caching to non-exec auth in AsyncKuber…
Feb 12, 2026
ed63309
Apply suggestions from code review
amoghrajesh Mar 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 51 additions & 20 deletions providers/cncf/kubernetes/src/airflow/providers/cncf/kubernetes/hooks/kubernetes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -818,6 +818,19 @@ def __init__(
self._event_polling_fallback = False
self._config_loaded = False

def _uses_exec_auth(self, kubeconfig_data: dict) -> bool:
"""
Detect if kubeconfig uses exec-based authentication.

Exec plugins return short-lived tokens (EKS, GKE, etc).
"""
users = kubeconfig_data.get("users", [])
for user in users:
user_auth = user.get("user", {})
if "exec" in user_auth:
return True
return False
Comment on lines +821 to +832

@amoghrajesh amoghrajesh Feb 27, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the conditional to be true with any user using exec auth here?

@amoghrajesh amoghrajesh Mar 27, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still needs to be addressed.

Comment on lines +821 to +832

Copilot AI Feb 27, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The exec auth detection only checks if any user has exec auth, not which user is currently active. A kubeconfig can have multiple users defined, but only one is active based on the current context. The current logic returns True if any user has exec auth, even if the active context uses a different user with static credentials.

Consider checking which user is referenced by the current context (or the cluster_context parameter) and only disable caching if that specific user uses exec auth. This would allow caching when the active user doesn't use exec auth, improving performance.

Copilot uses AI. Check for mistakes.

@amoghrajesh amoghrajesh Mar 27, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 needs addressing

Comment on lines +821 to +832

Copilot AI Feb 27, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing test coverage for exec auth detection logic. The new _uses_exec_auth method and the conditional caching behavior based on exec auth are critical functionality that prevents authentication failures. However, there are no tests verifying:

  1. That exec auth is correctly detected in kubeconfig
  2. That _config_loaded is not set when exec auth is present
  3. That _load_config is called multiple times for exec auth configs
  4. That _config_loaded is still set for non-exec auth configs

The test file at providers/cncf/kubernetes/tests/unit/cncf/kubernetes/hooks/test_kubernetes.py has comprehensive tests for AsyncKubernetesHook but doesn't cover this new logic. Add tests to ensure this critical bugfix works correctly and doesn't regress in future changes.

Copilot uses AI. Check for mistakes.

@amoghrajesh amoghrajesh Mar 27, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 needs addressing.


async def _load_config(self):
"""Load Kubernetes configuration once per hook instance."""
if self._config_loaded:
Expand Down Expand Up @@ -846,50 +859,68 @@ async def _load_config(self):
self._config_loaded = True
return

# If above block does not return, we are not in a cluster.
self._is_in_cluster = False

if self.config_dict:
self.log.debug(LOADING_KUBE_CONFIG_FILE_RESOURCE.format("config dictionary"))
await async_config.load_kube_config_from_dict(self.config_dict, context=cluster_context)
self._config_loaded = True
return

await async_config.load_kube_config_from_dict(
self.config_dict,
context=cluster_context,
)

if not self._uses_exec_auth(self.config_dict):
self._config_loaded = True

return
if kubeconfig_path is not None:
self.log.debug("loading kube_config from: %s", kubeconfig_path)

await async_config.load_kube_config(
config_file=kubeconfig_path,
client_configuration=self.client_configuration,
context=cluster_context,
)
self._config_loaded = True
return

try:
async with aiofiles.open(kubeconfig_path) as f:
content = await f.read()
data = yaml.safe_load(content)

if not self._uses_exec_auth(data):
self._config_loaded = True
except Exception as exc:
self.log.warning(
"Error while parsing kube_config from %s to detect exec auth; "
"continuing without caching the config: %s",
kubeconfig_path,
exc,
)

return
if kubeconfig is not None:
async with aiofiles.tempfile.NamedTemporaryFile() as temp_config:
self.log.debug(
"Reading kubernetes configuration file from connection "
"object and writing temporary config file with its content",
)
if isinstance(kubeconfig, dict):
self.log.debug(
LOADING_KUBE_CONFIG_FILE_RESOURCE.format(
"connection kube_config dictionary (serializing)"
)
)
kubeconfig = json.dumps(kubeconfig)
await temp_config.write(kubeconfig.encode())
kubeconfig_data = kubeconfig
kubeconfig_str = json.dumps(kubeconfig)
else:
kubeconfig_data = None
kubeconfig_str = kubeconfig
Comment thread
amoghrajesh marked this conversation as resolved.
Comment on lines +905 to +907

@amoghrajesh amoghrajesh Mar 27, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
else:
kubeconfig_data = None
kubeconfig_str = kubeconfig
else:
try:
kubeconfig_data = yaml.safe_load(kubeconfig)
except Exception as exc:
self.log.debug(
"Could not parse kubeconfig string to detect exec auth; "
"config will not be cached: %s",
exc,
)
kubeconfig_data = None
kubeconfig_str = kubeconfig


await temp_config.write(kubeconfig_str.encode())
await temp_config.flush()

await async_config.load_kube_config(
config_file=temp_config.name,
client_configuration=self.client_configuration,
context=cluster_context,
)
self._config_loaded = True
return

if kubeconfig_data and not self._uses_exec_auth(kubeconfig_data):
self._config_loaded = True
Comment thread
amoghrajesh marked this conversation as resolved.

return
self.log.debug(LOADING_KUBE_CONFIG_FILE_RESOURCE.format("default configuration file"))

await async_config.load_kube_config(
client_configuration=self.client_configuration,
context=cluster_context,
Expand Down
Loading