Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 8 additions & 1 deletion sky/backends/backend_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -763,7 +763,14 @@ def write_cluster_config(
keys=('allowed_contexts',),
default_value=None)
if allowed_contexts is None:
excluded_clouds.add(cloud)
# Exclude both Kubernetes and SSH explicitly since:
# 1. isinstance(cloud, clouds.Kubernetes) matches both (SSH
# inherits from Kubernetes)
# 2. Both share the same get_credential_file_mounts() which
# returns the kubeconfig. So if we don't exclude both, the
# unexcluded one will upload the kubeconfig.
excluded_clouds.add(clouds.Kubernetes())
excluded_clouds.add(clouds.SSH())
else:
excluded_clouds.add(cloud)

Expand Down
47 changes: 47 additions & 0 deletions tests/unit_tests/test_backend_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import pathlib
from unittest import mock

from sky import check as sky_check
from sky import clouds
from sky import skypilot_config
from sky.backends import backend_utils
Expand Down Expand Up @@ -283,3 +284,49 @@ def get_request_tasks(*args, **kwargs):

assert len(
backend_utils.get_clusters(refresh=common.StatusRefreshMode.FORCE)) == 2


def test_kubeconfig_upload_with_kubernetes_exclusion():
"""Tests kubeconfig upload behavior with Kubernetes/SSH cloud exclusion.

This is a regression test for a bug where kubeconfig was uploaded even when
`remote_identity: SERVICE_ACCOUNT` was set for a Kubernetes cluster. This
happened because `SSH` inherits from `Kubernetes` and was not being
explicitly excluded, causing it to upload the kubeconfig.
"""
# Mock get_credential_file_mounts on Kubernetes to return kubeconfig.
# SSH inherits from Kubernetes, so it will also return kubeconfig.
kubeconfig_mounts = {'~/.kube/config': '~/.kube/config'}

with mock.patch.object(clouds.Kubernetes, 'get_credential_file_mounts',
return_value=kubeconfig_mounts):
# 1. Test the buggy behavior: only Kubernetes is excluded.
# SSH is not excluded, and since it inherits from Kubernetes, it will
# upload the kubeconfig via the (mocked) inherited method.
excluded_clouds_buggy = {clouds.Kubernetes()}

# Mock os.path functions for the credential collection loop
with mock.patch('os.path.exists', return_value=True), \
mock.patch('os.path.expanduser', side_effect=lambda x: x), \
mock.patch('os.path.realpath', side_effect=lambda x: x):
credentials_buggy = sky_check.get_cloud_credential_file_mounts(
excluded_clouds_buggy)

assert '~/.kube/config' in credentials_buggy, (
'Kubeconfig should be uploaded when only Kubernetes is excluded. '
'This demonstrates the buggy behavior that the fix in '
'write_cluster_config() is meant to prevent.')

# 2. Test the correct behavior: both Kubernetes and SSH are excluded.
# Kubeconfig should not be in the returned credentials.
excluded_clouds_fixed = {clouds.Kubernetes(), clouds.SSH()}

with mock.patch('os.path.exists', return_value=True), \
mock.patch('os.path.expanduser', side_effect=lambda x: x), \
mock.patch('os.path.realpath', side_effect=lambda x: x):
credentials_fixed = sky_check.get_cloud_credential_file_mounts(
excluded_clouds_fixed)

assert '~/.kube/config' not in credentials_fixed, (
'Kubeconfig should not be uploaded when both Kubernetes and SSH '
'are excluded.')
Loading