Skip to content

Commit

Permalink
[Azure][Storage] Update Error response for accessing private containe…
Browse files Browse the repository at this point in the history
…r without access (#3807)

* update error handling and smoke test

* format

* nit

* add comment

* update comment

* add print_exception_no_traceback

* revert check for private container without access
  • Loading branch information
landscapepainter committed Sep 5, 2024
1 parent e3b9763 commit 455e065
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 19 deletions.
17 changes: 11 additions & 6 deletions sky/adaptors/azure.py
Original file line number Diff line number Diff line change
Expand Up @@ -177,19 +177,24 @@ def get_client(name: str,
container_client = blob.ContainerClient.from_container_url(
container_url, credential)
try:
# Suppress noisy logs from Azure SDK when attempting
# to run exists() on private container without access.
# Reference:
# https://github.com/Azure/azure-sdk-for-python/issues/9422
azure_logger = logging.getLogger('azure')
original_level = azure_logger.getEffectiveLevel()
azure_logger.setLevel(logging.CRITICAL)
container_client.exists()
azure_logger.setLevel(original_level)
return container_client
except exceptions().ClientAuthenticationError as e:
# Caught when user attempted to use private container
# without access rights.
# without access rights. Raised error is handled at the
# upstream.
# Reference: https://learn.microsoft.com/en-us/troubleshoot/azure/entra/entra-id/app-integration/error-code-aadsts50020-user-account-identity-provider-does-not-exist # pylint: disable=line-too-long
if 'ERROR: AADSTS50020' in str(e):
with ux_utils.print_exception_no_traceback():
raise sky_exceptions.StorageBucketGetError(
'Attempted to fetch a non-existent public '
'container name: '
f'{container_client.container_name}. '
'Please check if the name is correct.')
raise e
with ux_utils.print_exception_no_traceback():
raise sky_exceptions.StorageBucketGetError(
'Failed to retreive the container client for the '
Expand Down
21 changes: 16 additions & 5 deletions sky/data/storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -2541,11 +2541,22 @@ def _get_bucket(self) -> Tuple[str, bool]:
container_url = data_utils.AZURE_CONTAINER_URL.format(
storage_account_name=self.storage_account_name,
container_name=self.name)
container_client = data_utils.create_az_client(
client_type='container',
container_url=container_url,
storage_account_name=self.storage_account_name,
resource_group_name=self.resource_group_name)
try:
container_client = data_utils.create_az_client(
client_type='container',
container_url=container_url,
storage_account_name=self.storage_account_name,
resource_group_name=self.resource_group_name)
except azure.exceptions().ClientAuthenticationError as e:
if 'ERROR: AADSTS50020' in str(e):
# Caught when failing to obtain container client due to
# lack of permission to passed given private container.
if self.resource_group_name is None:
with ux_utils.print_exception_no_traceback():
raise exceptions.StorageBucketGetError(
_BUCKET_FAIL_TO_CONNECT_MESSAGE.format(
name=self.name))
raise
if container_client.exists():
is_private = (True if
container_client.get_container_properties().get(
Expand Down
12 changes: 4 additions & 8 deletions tests/test_smoke.py
Original file line number Diff line number Diff line change
Expand Up @@ -5069,14 +5069,10 @@ def test_private_bucket(self, private_bucket):
private_bucket).path.strip('/')
else:
private_bucket_name = urllib.parse.urlsplit(private_bucket).netloc
match_str = storage_lib._BUCKET_FAIL_TO_CONNECT_MESSAGE.format(
name=private_bucket_name)
if store_type == 'https':
# Azure blob uses a different error string since container may
# not exist even though the bucket name is ok.
match_str = 'Attempted to fetch a non-existent public container'
with pytest.raises(sky.exceptions.StorageBucketGetError,
match=match_str):
with pytest.raises(
sky.exceptions.StorageBucketGetError,
match=storage_lib._BUCKET_FAIL_TO_CONNECT_MESSAGE.format(
name=private_bucket_name)):
storage_obj = storage_lib.Storage(source=private_bucket)

@pytest.mark.no_fluidstack
Expand Down

0 comments on commit 455e065

Please sign in to comment.