Skip to content
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

[Storage] Modified StorageRetryPolicy to skip AzureSigningError from bad storage account key #36431

Merged
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
SansIOHTTPPolicy,
)

from .authentication import StorageHttpChallenge
from .authentication import AzureSigningError, StorageHttpChallenge
from .constants import DEFAULT_OAUTH_SCOPE
from .models import LocationMode

Expand Down Expand Up @@ -542,6 +542,8 @@ def send(self, request):
continue
break
except AzureError as err:
weirongw23-msft marked this conversation as resolved.
Show resolved Hide resolved
if isinstance(err, AzureSigningError):
raise
retries_remaining = self.increment(
retry_settings, request=request.http_request, error=err)
if retries_remaining:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
from azure.core.exceptions import AzureError
from azure.core.pipeline.policies import AsyncBearerTokenCredentialPolicy, AsyncHTTPPolicy

from .authentication import StorageHttpChallenge
from .authentication import AzureSigningError, StorageHttpChallenge
from .constants import DEFAULT_OAUTH_SCOPE
from .policies import is_retry, StorageRetryPolicy

Expand Down Expand Up @@ -127,6 +127,8 @@ async def send(self, request):
continue
break
except AzureError as err:
if isinstance(err, AzureSigningError):
raise
retries_remaining = self.increment(
retry_settings, request=request.http_request, error=err)
if retries_remaining:
Expand Down
2 changes: 2 additions & 0 deletions sdk/storage/azure-storage-blob/tests/test_blob_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
# --------------------------------------------------------------------------

import platform
import time
jalauzon-msft marked this conversation as resolved.
Show resolved Hide resolved
from datetime import datetime, timedelta

import pytest
Expand All @@ -19,6 +20,7 @@
VERSION,
)
from azure.storage.blob._shared.base_client import create_configuration
from azure.storage.blob._shared.authentication import AzureSigningError

from devtools_testutils import recorded_by_proxy
from devtools_testutils.storage import StorageRecordedTestCase
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
# --------------------------------------------------------------------------

import platform
import time
from datetime import datetime, timedelta

import pytest
Expand All @@ -20,6 +21,7 @@
ContainerClient,
BlobServiceClient
)
from azure.storage.blob._shared.authentication import AzureSigningError

from devtools_testutils.aio import recorded_by_proxy_async
from devtools_testutils.storage.aio import AsyncStorageRecordedTestCase
Expand Down
27 changes: 27 additions & 0 deletions sdk/storage/azure-storage-blob/tests/test_retry.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@
ServiceResponseError
)
from azure.core.pipeline.transport import RequestsTransport
from azure.storage.blob._shared.authentication import AzureSigningError
from azure.storage.blob import (
BlobClient,
BlobServiceClient,
ExponentialRetry,
LinearRetry,
Expand Down Expand Up @@ -550,4 +552,29 @@ def test_streaming_retry(self, **kwargs):
blob.download_blob()
assert iterator_mock.__next__.call_count == count[0] == 3

@BlobPreparer()
def test_invalid_storage_account_key(self, **kwargs):
storage_account_name = kwargs.pop("storage_account_name")
storage_account_key = "a"

# Arrange
blob_client = self._create_storage_service(
BlobClient,
storage_account_name,
storage_account_key,
container_name="foo",
blob_name="bar"
)

retry_counter = RetryCounter()
retry_callback = retry_counter.simple_count

# Act
with pytest.raises(AzureSigningError) as e:
blob_client.get_blob_properties(retry_hook=retry_callback)

# Assert
assert "Invalid base64-encoded string" in e.value.message
assert retry_counter.count == 0

# ------------------------------------------------------------------------------
28 changes: 27 additions & 1 deletion sdk/storage/azure-storage-blob/tests/test_retry_async.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,9 @@
)
from azure.core.pipeline.transport import AioHttpTransport
from azure.storage.blob import LocationMode
from azure.storage.blob._shared.authentication import AzureSigningError
from azure.storage.blob._shared.policies_async import ExponentialRetry, LinearRetry
from azure.storage.blob.aio import BlobServiceClient
from azure.storage.blob.aio import BlobClient, BlobServiceClient

from devtools_testutils import ResponseCallback, RetryCounter
from devtools_testutils.aio import recorded_by_proxy_async
Expand Down Expand Up @@ -529,4 +530,29 @@ async def test_streaming_retry(self, **kwargs):
await blob.download_blob()
assert stream_reader_read_mock.call_count == count[0] == 4

@BlobPreparer()
async def test_invalid_storage_account_key(self, **kwargs):
storage_account_name = kwargs.pop("storage_account_name")
storage_account_key = "a"

# Arrange
blob_client = self._create_storage_service(
BlobClient,
storage_account_name,
storage_account_key,
container_name="foo",
blob_name="bar"
)

retry_counter = RetryCounter()
retry_callback = retry_counter.simple_count

# Act
with pytest.raises(AzureSigningError) as e:
await blob_client.get_blob_properties(retry_hook=retry_callback)

# Assert
assert "Invalid base64-encoded string" in e.value.message
assert retry_counter.count == 0

# ------------------------------------------------------------------------------
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
SansIOHTTPPolicy,
)

from .authentication import StorageHttpChallenge
from .authentication import AzureSigningError, StorageHttpChallenge
from .constants import DEFAULT_OAUTH_SCOPE
from .models import LocationMode

Expand Down Expand Up @@ -544,6 +544,8 @@ def send(self, request):
continue
break
except AzureError as err:
if isinstance(err, AzureSigningError):
raise
retries_remaining = self.increment(
retry_settings, request=request.http_request, error=err)
if retries_remaining:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
from azure.core.exceptions import AzureError
from azure.core.pipeline.policies import AsyncBearerTokenCredentialPolicy, AsyncHTTPPolicy

from .authentication import StorageHttpChallenge
from .authentication import AzureSigningError, StorageHttpChallenge
from .constants import DEFAULT_OAUTH_SCOPE
from .policies import is_retry, StorageRetryPolicy

Expand Down Expand Up @@ -127,6 +127,8 @@ async def send(self, request):
continue
break
except AzureError as err:
if isinstance(err, AzureSigningError):
raise
retries_remaining = self.increment(
retry_settings, request=request.http_request, error=err)
if retries_remaining:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
SansIOHTTPPolicy,
)

from .authentication import StorageHttpChallenge
from .authentication import AzureSigningError, StorageHttpChallenge
from .constants import DEFAULT_OAUTH_SCOPE
from .models import LocationMode

Expand Down Expand Up @@ -541,6 +541,8 @@ def send(self, request):
continue
break
except AzureError as err:
if isinstance(err, AzureSigningError):
raise
retries_remaining = self.increment(
retry_settings, request=request.http_request, error=err)
if retries_remaining:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
from azure.core.exceptions import AzureError
from azure.core.pipeline.policies import AsyncBearerTokenCredentialPolicy, AsyncHTTPPolicy

from .authentication import StorageHttpChallenge
from .authentication import AzureSigningError, StorageHttpChallenge
from .constants import DEFAULT_OAUTH_SCOPE
from .policies import is_retry, StorageRetryPolicy

Expand Down Expand Up @@ -127,6 +127,8 @@ async def send(self, request):
continue
break
except AzureError as err:
if isinstance(err, AzureSigningError):
raise
retries_remaining = self.increment(
retry_settings, request=request.http_request, error=err)
if retries_remaining:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
SansIOHTTPPolicy,
)

from .authentication import StorageHttpChallenge
from .authentication import AzureSigningError, StorageHttpChallenge
from .constants import DEFAULT_OAUTH_SCOPE
from .models import LocationMode

Expand Down Expand Up @@ -547,6 +547,8 @@ def send(self, request):
continue
break
except AzureError as err:
if isinstance(err, AzureSigningError):
raise
retries_remaining = self.increment(
retry_settings, request=request.http_request, error=err)
if retries_remaining:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
from azure.core.exceptions import AzureError
from azure.core.pipeline.policies import AsyncBearerTokenCredentialPolicy, AsyncHTTPPolicy

from .authentication import StorageHttpChallenge
from .authentication import AzureSigningError, StorageHttpChallenge
from .constants import DEFAULT_OAUTH_SCOPE
from .policies import is_retry, StorageRetryPolicy

Expand Down Expand Up @@ -127,6 +127,8 @@ async def send(self, request):
continue
break
except AzureError as err:
if isinstance(err, AzureSigningError):
raise
retries_remaining = self.increment(
retry_settings, request=request.http_request, error=err)
if retries_remaining:
Expand Down
Loading