Skip to content
Merged
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
13 changes: 13 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
@@ -1,6 +1,19 @@
Changelog
=========

Changes in Apache Libcloud 3.3.2
--------------------------------

Storage
~~~~~~~

- [Azure Blobs] Enable the Azure storage driver to be used with
Azure Government, Azure China, and Azure Private Link by setting
the driver host argument to the endpoint suffix for the environment.

Reported by Melissa Kersh - @mkcello96
(GITHUB-1551)

Changes in Apache Libcloud 3.3.1
--------------------------------

Expand Down
8 changes: 8 additions & 0 deletions docs/examples/storage/azure/instantiate_gov.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
from libcloud.storage.types import Provider
from libcloud.storage.providers import get_driver

cls = get_driver(Provider.AZURE_BLOBS)

driver = cls(key='your storage account name',
secret='your access key',
host='blob.core.usgovcloudapi.net')
16 changes: 16 additions & 0 deletions docs/storage/drivers/azure_blobs.rst
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,19 @@ below.
.. literalinclude:: /examples/storage/azure/instantiate.py
:language: python

Connecting to Azure Government
------------------------------

To target an `Azure Government`_ storage account, you can instantiate the driver
by setting a custom storage host argument as shown below.

.. literalinclude:: /examples/storage/azure/instantiate_gov.py
:language: python

Setting a custom host argument can also be leveraged to customize the blob
endpoint and connect to a storage account in `Azure China`_ or
`Azure Private Link`_.

Connecting to self-hosted Azure Storage implementations
-------------------------------------------------------

Expand All @@ -51,3 +64,6 @@ Azure Storage implementations such as `Azure Blob Storage on IoT Edge`_.
.. _`BlockBlobStorage accounts`: https://docs.microsoft.com/en-us/azure/storage/common/storage-account-overview#blockblobstorage-accounts
.. _`Azurite storage emulator`: https://github.com/Azure/Azurite
.. _`Azure Blob Storage on IoT Edge`: https://docs.microsoft.com/en-us/azure/iot-edge/how-to-store-data-blob
.. _`Azure Government`: https://docs.microsoft.com/en-us/azure/azure-government/documentation-government-developer-guide
.. _`Azure China`: https://docs.microsoft.com/en-us/azure/china/resources-developer-guide
.. _`Azure Private Link`: https://docs.microsoft.com/en-us/azure/private-link/private-link-overview
20 changes: 11 additions & 9 deletions libcloud/common/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -984,29 +984,31 @@ def __init__(self, key, secret=None, secure=True, host=None, port=None,
self.key = key
self.secret = secret
self.secure = secure
self.api_version = api_version
self.region = region

conn_kwargs = self._ex_connection_class_kwargs()
conn_kwargs.update({'timeout': kwargs.pop('timeout', None),
'retry_delay': kwargs.pop('retry_delay', None),
'backoff': kwargs.pop('backoff', None),
'proxy_url': kwargs.pop('proxy_url', None)})

args = [self.key]

if self.secret is not None:
args.append(self.secret)

args.append(secure)

host = conn_kwargs.pop('host', None) or host

if host is not None:
args.append(host)

if port is not None:
args.append(port)

self.api_version = api_version
self.region = region

conn_kwargs = self._ex_connection_class_kwargs()
conn_kwargs.update({'timeout': kwargs.pop('timeout', None),
'retry_delay': kwargs.pop('retry_delay', None),
'backoff': kwargs.pop('backoff', None),
'proxy_url': kwargs.pop('proxy_url', None)})
self.connection = self.connectionCls(*args, **conn_kwargs)

self.connection.driver = self
self.connection.connect()

Expand Down
47 changes: 35 additions & 12 deletions libcloud/storage/drivers/azure_blobs.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,9 @@
)

AZURE_STORAGE_HOST_SUFFIX = 'blob.core.windows.net'
AZURE_STORAGE_HOST_SUFFIX_CHINA = 'blob.core.chinacloudapi.cn'
AZURE_STORAGE_HOST_SUFFIX_GOVERNMENT = 'blob.core.usgovcloudapi.net'
AZURE_STORAGE_HOST_SUFFIX_PRIVATELINK = 'privatelink.blob.core.windows.net'

AZURE_STORAGE_CDN_URL_DATE_FORMAT = '%Y-%m-%dT%H:%M:%SZ'

Expand Down Expand Up @@ -173,11 +176,12 @@ class AzureBlobsConnection(AzureConnection):
these deployments, the parameter ``account_prefix`` must be set on the
connection. This is done by instantiating the driver with arguments such
as ``host='somewhere.tld'`` and ``key='theaccount'``. To specify a custom
host without an account prefix, e.g. for use-cases where the custom host
implements an auditing proxy or similar, the driver can be instantiated
with ``host='theaccount.somewhere.tld'`` and ``key=''``.
host without an account prefix, e.g. to connect to Azure Government or
Azure China, the driver can be instantiated with the appropriate storage
endpoint suffix, e.g. ``host='blob.core.usgovcloudapi.net'`` and
``key='theaccount'``.

:param account_prefix: Optional prefix identifying the sotrage account.
:param account_prefix: Optional prefix identifying the storage account.
Used when connecting to a custom deployment of the
storage service like Azurite or IoT Edge Storage.
:type account_prefix: ``str``
Expand Down Expand Up @@ -206,7 +210,7 @@ class AzureBlobsStorageDriver(StorageDriver):

def __init__(self, key, secret=None, secure=True, host=None, port=None,
**kwargs):
self._host_argument_set = bool(host)
self._host = host

# B64decode() this key and keep it, so that we don't have to do
# so for every request. Minor performance improvement
Expand All @@ -217,15 +221,34 @@ def __init__(self, key, secret=None, secure=True, host=None, port=None,
port=port, **kwargs)

def _ex_connection_class_kwargs(self):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Some unit tests for this method would be good.

Copy link
Copy Markdown
Member Author

@c-w c-w Feb 10, 2021

Choose a reason for hiding this comment

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

Added tests for the Azurite path in c581553 and for the new GovCloud path in 739a6a3.

result = {}

# host argument has precedence
if not self._host_argument_set:
result['host'] = '%s.%s' % (self.key, AZURE_STORAGE_HOST_SUFFIX)
# if the user didn't provide a custom host value, assume we're
# targeting the default Azure Storage endpoints
if self._host is None:
return {'host': '%s.%s' % (self.key, AZURE_STORAGE_HOST_SUFFIX)}

# connecting to a special storage region like Azure Government or
# Azure China requires setting a custom storage endpoint but we
# still use the same scheme to identify a specific account as for
# the standard storage endpoint
try:
host_suffix = next(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is there a situation where we may want to allow end user to specify full host as-is - aka so we don't add any prefix to it, but user already includes that in the host argument?

I think this would give users more flexibility in some situations.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

To clarify - if a new endpoint ever gets added or similar, this would require use to update the code to support it.

If we allowed user to specify a full arbitrary host, they may be able to use that instead (until a new version is released / similar).

For us to be able to support that, we would likely need to add new argument to the constructor - e.g. ex_use_host_as_is / ex_dont_prefix_host (or some better name, can't come up with anything better atm).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The only current Azure Storage version that I'm aware of which wouldn't be covered by the approach proposed in this PR is Azure Stack Hub Storage. However the API version that libcloud uses (2018-11-09) is newer than the latest supported version of Azure Stack (2017-11-09) so libcloud doesn't support it anyways.

I get the appeal of an ex_force_host or similar argument, but I wonder if the simplicity of this explicit approach will be more user-friendly as it doesn't introduce an argument that's specific to just this driver and instead manages the complexity internally. Thoughts?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yeah, I didn't really mean it as an alternative, but in addition to your proposed changes (so the interface for the common case would still be the same, but in case there is a need to use a fully custom host, there is an easy way to do it).

I also don't think that's a blocker for this PR so if you think it's a good idea, feel free to open a new PR with that change in the future.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

In that case I'd say to defer the addition of ex_force_host for now.

host_suffix
for host_suffix in (
AZURE_STORAGE_HOST_SUFFIX_CHINA,
AZURE_STORAGE_HOST_SUFFIX_GOVERNMENT,
AZURE_STORAGE_HOST_SUFFIX_PRIVATELINK,
)
if self._host.endswith(host_suffix)
)
except StopIteration:
pass
else:
result['account_prefix'] = self.key
return {'host': '%s.%s' % (self.key, host_suffix)}

return result
# if the host isn't targeting one of the special storage regions, it
# must be pointing to Azurite or IoT Edge Storage so switch to prefix
# identification
return {'account_prefix': self.key}

def _xml_to_container(self, node):
"""
Expand Down
29 changes: 29 additions & 0 deletions libcloud/test/storage/test_azure_blobs.py
Original file line number Diff line number Diff line change
Expand Up @@ -935,6 +935,35 @@ def test_storage_driver_host(self):
self.assertEqual(host2, 'fakeaccount2.blob.core.windows.net')
self.assertEqual(host3, 'test.foo.bar.com')

def test_storage_driver_host_govcloud(self):
driver1 = self.driver_type(
'fakeaccount1', 'deadbeafcafebabe==',
host='blob.core.usgovcloudapi.net')
driver2 = self.driver_type(
'fakeaccount2', 'deadbeafcafebabe==',
host='fakeaccount2.blob.core.usgovcloudapi.net')

host1 = driver1.connection.host
host2 = driver2.connection.host
account_prefix_1 = driver1.connection.account_prefix
account_prefix_2 = driver2.connection.account_prefix

self.assertEqual(host1, 'fakeaccount1.blob.core.usgovcloudapi.net')
self.assertEqual(host2, 'fakeaccount2.blob.core.usgovcloudapi.net')
self.assertIsNone(account_prefix_1)
self.assertIsNone(account_prefix_2)

def test_storage_driver_host_azurite(self):
driver = self.driver_type(
'fakeaccount1', 'deadbeafcafebabe==',
host='localhost', port=10000, secure=False)

host = driver.connection.host
account_prefix = driver.connection.account_prefix

self.assertEqual(host, 'localhost')
self.assertEqual(account_prefix, 'fakeaccount1')


class AzuriteBlobsTests(AzureBlobsTests):
driver_args = STORAGE_AZURITE_BLOBS_PARAMS
Expand Down