Allow AAD on storage credentials

- Resolves #179

config_templates/credentials.yaml

@@ -1,6 +1,6 @@
 credentials:
   # global aad credentials: use only if a common set of aad credentials
-  # can access batch, management and keyvault endpoints.
+  # can access batch, management, storage, and keyvault endpoints.
   aad:
     authority_url: https://login.microsoftonline.com
     directory_id: 01234567-89ab-cdef-0123-456789abcdef
@@ -31,11 +31,25 @@ credentials:
     resource_group: resource-group-of-batch-account
   # storage credentials
   storage:
+    aad:
+      authority_url: https://login.microsoftonline.com
+      endpoint: https://batch.core.windows.net/
+      directory_id: 01234567-89ab-cdef-0123-456789abcdef
+      application_id: 01234567-89ab-cdef-0123-456789abcdef
+      auth_key: 01234...
+      rsa_private_key_pem: some/path/privatekey.pem
+      x509_cert_sha1_thumbprint: 01234...
+      user: aad_username
+      password: aad_user_password
+      token_cache:
+        enabled: true
+        filename: some/path/token.cache
     mystorageaccount:
       account: storage_account_name
       account_key: 01234...
       account_key_keyvault_secret_id: https://<vault_name>.vault.azure.net/secrets/<secret_id>
       endpoint: core.windows.net
+      resource_group: resource-group-of-storage-account
   # docker private registry credentials
   docker_registry:
     hub:

config_templates/pool.yaml

@@ -84,7 +84,7 @@ pool_specification:
   virtual_network:
     arm_subnet_id: /subscriptions/<subscription_id>/resourceGroups/<resource_group>/providers/Microsoft.Network/virtualNetworks/<virtual_network_name>/subnets/<subnet_name>
     name: myvnet
-    resource_group: vnet-in-another-rg
+    resource_group: resource-group-of-vnet
     create_nonexistant: false
     address_space: 10.0.0.0/16
     subnet:

convoy/clients.py

@@ -40,6 +40,7 @@
 import azure.mgmt.compute
 import azure.mgmt.network
 import azure.mgmt.resource
+import azure.mgmt.storage
 import azure.storage.blob as azureblob
 # local imports
 from . import aad
@@ -131,6 +132,35 @@ def _create_network_client(
         credentials, subscription_id, base_url=endpoint)
 
 
+def _create_storage_mgmt_client(
+        ctx, credentials=None, subscription_id=None, endpoint=None):
+    # type: (CliContext, object, str) ->
+    #        azure.mgmt.storage.StorageManagementClient
+    """Create storage management client
+    :param CliContext ctx: Cli Context
+    :param object credentials: credentials object
+    :param str subscription_id: subscription id
+    :param str endpoint: endpoint
+    :rtype: azure.mgmt.storage.StorageManagementClient
+    :return: storage management client
+    """
+    storage_aad = None
+    if credentials is None:
+        storage_aad = settings.credentials_storage_aad(ctx.config)
+        credentials = aad.create_aad_credentials(ctx, storage_aad)
+    if util.is_none_or_empty(subscription_id):
+        try:
+            subid = storage_aad.subscription_id
+        except Exception:
+            subid = settings.credentials_management(
+                ctx.config).aad.subscription_id
+        subscription_id = ctx.subscription_id or subid
+    if endpoint is None:
+        endpoint = ctx.aad_endpoint or storage_aad.endpoint
+    return azure.mgmt.storage.StorageManagementClient(
+        credentials, subscription_id, base_url=endpoint)
+
+
 def _create_batch_mgmt_client(
         ctx, credentials=None, subscription_id=None, endpoint=None):
     # type: (CliContext, object, str, str) ->
@@ -160,21 +190,23 @@ def _create_batch_mgmt_client(
     return batch_mgmt_client
 
 
-def create_arm_clients(ctx, batch_clients=False):
+def create_all_clients(ctx, batch_clients=False):
     # type: (CliContext, bool) ->
     #        Tuple[azure.mgmt.resource.resources.ResourceManagementClient,
     #              azure.mgmt.compute.ComputeManagementClient,
     #              azure.mgmt.network.NetworkManagementClient,
+    #              azure.mgmt.storage.StorageManagementClient,
     #              azure.mgmt.batch.BatchManagementClient,
     #              azure.batch.batch_service_client.BatchServiceClient]
-    """Create resource, compute and network clients
+    """Create all arm clients and batch service client
     :param CliContext ctx: Cli Context
     :param bool batch_clients: create batch clients
     :rtype: tuple
     :return: (
         azure.mgmt.resource.resources.ResourceManagementClient,
         azure.mgmt.compute.ComputeManagementClient,
         azure.mgmt.network.NetworkManagementClient,
+        azure.mgmt.storage.StorageManagementClient,
         azure.mgmt.batch.BatchManagementClient,
         azure.batch.batch_service_client.BatchServiceClient)
     """
@@ -186,6 +218,7 @@ def create_arm_clients(ctx, batch_clients=False):
         resource_client = None
         compute_client = None
         network_client = None
+        storage_mgmt_client = None
     else:
         # subscription_id must be of type 'str' due to python management
         # library type checking, but can be read as 'unicode' from json
@@ -208,6 +241,11 @@ def create_arm_clients(ctx, batch_clients=False):
             endpoint=endpoint)
         network_client.config.add_user_agent(
             'batch-shipyard/{}'.format(__version__))
+        storage_mgmt_client = _create_storage_mgmt_client(
+            ctx, credentials=credentials, subscription_id=subscription_id,
+            endpoint=endpoint)
+        storage_mgmt_client.config.add_user_agent(
+            'batch-shipyard/{}'.format(__version__))
     if batch_clients:
         try:
             if credentials is None:
@@ -225,8 +263,8 @@ def create_arm_clients(ctx, batch_clients=False):
         batch_mgmt_client = None
         batch_client = None
     return (
-        resource_client, compute_client, network_client, batch_mgmt_client,
-        batch_client
+        resource_client, compute_client, network_client, storage_mgmt_client,
+        batch_mgmt_client, batch_client
     )
 
 

convoy/fleet.py

@@ -240,6 +240,20 @@ def fetch_secrets_from_keyvault(keyvault_client, config):
         keyvault.parse_secret_ids(keyvault_client, config)
 
 
+def fetch_storage_account_keys_from_aad(
+        storage_mgmt_client, config, fs_storage):
+    # type: (azure.mgmt.storage.StorageManagementClient, dict, bool) -> None
+    """Fetch secrets with secret ids in config from keyvault
+    :param azure.mgmt.storage.StorageManagementClient storage_mgmt_client:
+        storage client
+    :param dict config: configuration dict
+    :param bool fs_storage: adjust for fs context
+    """
+    if storage.populate_storage_account_keys_from_aad(
+            storage_mgmt_client, config):
+        populate_global_settings(config, fs_storage)
+
+
 def _setup_nvidia_driver_package(blob_client, config, vm_size):
     # type: (azure.storage.blob.BlockBlobService, dict, str) -> pathlib.Path
     """Set up the nvidia driver package
@@ -726,7 +740,7 @@ def _pick_node_agent_for_vm(batch_client, config, pool_settings):
             'version': pool_settings.vm_configuration.version,
         }, 'batch.node.windows amd64')
     # pick latest sku
-    node_agent_skus = batch_client.account.list_node_agent_skus(config)
+    node_agent_skus = batch_client.account.list_node_agent_skus()
     skus_to_use = [
         (nas, image_ref) for nas in node_agent_skus
         for image_ref in sorted(

convoy/keyvault.py

@@ -224,7 +224,7 @@ def parse_secret_ids(client, config):
             raise ValueError(
                 'storage account key retrieved for secret id {} is '
                 'invalid'.format(secid))
-        settings.set_credentials_storage_account_key(config, ssel, sakey)
+        settings.set_credentials_storage_account(config, ssel, sakey)
     # docker registry passwords
     for reg in settings.credentials_iterate_registry_servers(config, True):
         secid = settings.credentials_registry_password_secret_id(

convoy/settings.py

@@ -229,7 +229,7 @@
 )
 StorageCredentialsSettings = collections.namedtuple(
     'StorageCredentialsSettings', [
-        'account', 'account_key', 'endpoint',
+        'account', 'account_key', 'endpoint', 'resource_group',
     ]
 )
 BatchShipyardSettings = collections.namedtuple(
@@ -1458,6 +1458,33 @@ def set_credentials_batch_account_key(config, bakey):
     config['credentials']['batch']['account_key'] = bakey
 
 
+def credentials_storage_aad(config):
+    # type: (dict) -> AADSettings
+    """Get storage AAD credentials
+    :param dict config: configuration object
+    :rtype: AADSettings
+    :return: storage aad settings
+    """
+    if 'aad' in config['credentials']['storage']:
+        return _aad_credentials(
+            config['credentials'],
+            'storage',
+            default_endpoint='https://management.azure.com/',
+            default_token_cache_file=(
+                '.batch_shipyard_aad_storage_token.json'
+            ),
+        )
+    else:
+        return _aad_credentials(
+            config['credentials'],
+            'management',
+            default_endpoint='https://management.azure.com/',
+            default_token_cache_file=(
+                '.batch_shipyard_aad_storage_token.json'
+            ),
+        )
+
+
 def credentials_storage(config, ssel):
     # type: (dict, str) -> StorageCredentialsSettings
     """Get specific storage credentials
@@ -1473,16 +1500,12 @@ def credentials_storage(config, ssel):
             ('Could not find storage account alias {} in credentials:storage '
              'configuration. Please ensure the storage account alias '
              'exists.').format(ssel))
-    try:
-        ep = conf['endpoint']
-        if util.is_none_or_empty(ep):
-            raise KeyError()
-    except KeyError:
-        ep = 'core.windows.net'
     return StorageCredentialsSettings(
         account=conf['account'],
-        account_key=conf['account_key'],
-        endpoint=ep,
+        account_key=_kv_read_checked(conf, 'account_key'),
+        endpoint=_kv_read_checked(
+            conf, 'endpoint', default='core.windows.net'),
+        resource_group=_kv_read_checked(conf, 'resource_group'),
     )
 
 
@@ -1494,6 +1517,8 @@ def iterate_storage_credentials(config):
     :return: storage selector link
     """
     for conf in config['credentials']['storage']:
+        if conf == 'aad':
+            continue
         yield conf
 
 
@@ -1515,14 +1540,17 @@ def credentials_storage_account_key_secret_id(config, ssel):
     return secid
 
 
-def set_credentials_storage_account_key(config, ssel, sakey):
-    # type: (dict, str, str) -> None
-    """Set Storage account key
+def set_credentials_storage_account(config, ssel, sakey, ep=None):
+    # type: (dict, str, str, str) -> None
+    """Set Storage account key and endpoint
     :param dict config: configuration object
     :param str ssel: storage selector link
     :param str sakey: storage account key
+    :param str ep: endpoint
     """
     config['credentials']['storage'][ssel]['account_key'] = sakey
+    if util.is_not_empty(ep):
+        config['credentials']['storage'][ssel]['endpoint'] = ep
 
 
 def docker_registry_login(config, server):

convoy/storage.py

@@ -149,6 +149,37 @@ def get_storageaccount_endpoint():
     return _STORAGEACCOUNTEP
 
 
+def populate_storage_account_keys_from_aad(storage_mgmt_client, config):
+    # type: (azure.mgmt.storage.StorageManagementClient, dict) -> None
+    """Fetch secrets with secret ids in config from keyvault
+    :param azure.mgmt.storage.StorageManagementClient storage_mgmt_client:
+        storage client
+    :param dict config: configuration dict
+    """
+    modified = False
+    if storage_mgmt_client is None:
+        return modified
+    # iterate all storage accounts, if storage account does not have
+    # a storage account key, then lookup via aad
+    for ssel in settings.iterate_storage_credentials(config):
+        sc = settings.credentials_storage(config, ssel)
+        if util.is_none_or_empty(sc.account_key):
+            if util.is_none_or_empty(sc.resource_group):
+                raise ValueError(
+                    ('resource_group is invalid for storage account {} to '
+                     'be retrieved by aad').format(sc.account))
+            keys = storage_mgmt_client.storage_accounts.list_keys(
+                sc.resource_group, sc.account)
+            props = storage_mgmt_client.storage_accounts.get_properties(
+                sc.resource_group, sc.account)
+            ep = '.'.join(
+                props.primary_endpoints.blob.rstrip('/').split('.')[2:])
+            settings.set_credentials_storage_account(
+                config, ssel, keys.keys[0].value, ep)
+            modified = True
+    return modified
+
+
 def generate_blob_container_uri(storage_settings, container):
     # type: (StorageCredentialsSettings, str) -> str
     """Create a uri to a blob container

docs/11-batch-shipyard-configuration-credentials.md

@@ -35,6 +35,19 @@ credentials:
     account_service_url: https://<batch_account_name>.<region>.batch.azure.com/
     resource_group: resource-group-of-batch-account
   storage:
+    aad:
+      authority_url: https://login.microsoftonline.com
+      endpoint: https://batch.core.windows.net/
+      directory_id: 01234567-89ab-cdef-0123-456789abcdef
+      application_id: 01234567-89ab-cdef-0123-456789abcdef
+      auth_key: 01234...
+      rsa_private_key_pem: some/path/privatekey.pem
+      x509_cert_sha1_thumbprint: 01234...
+      user: aad_username
+      password: aad_user_password
+      token_cache:
+        enabled: true
+        filename: some/path/token.cache
     mystorageaccount:
       account: storage_account_name
       account_key: 01234...
@@ -100,13 +113,13 @@ reviewing the options below.
 ### Azure Active Directory: `aad`
 `aad` can be specified at the "global" level, which would apply to all
 resources that can be accessed through Azure Active Directory: `batch`,
-`keyvault` and `management`. `aad` should only be specified at the "global"
-level if a common set of credentials are permitted to access all three
-resources. The `aad` property can also be specified within each individual
-credential section for `batch`, `keyvault` and `management`. Any `aad`
-properties specified within a credential section will override any "global"
-`aad` setting. Note that certain properties such as `endpoint` and
-`token_cache` are not available at the "global" level.
+`storage`, `keyvault` and `management`. `aad` should only be specified at
+the "global" level if a common set of credentials are permitted to access
+all three resources. The `aad` property can also be specified within each
+individual credential section for `batch`, `keyvault` and `management`.
+Any `aad` properties specified within a credential section will override
+any "global" `aad` setting. Note that certain properties such as `endpoint`
+and `token_cache` are not available at the "global" level.
 
 The `aad` property contains members for Azure Active Directory credentials.
 This section may not be needed or applicable for every credential section.
@@ -186,11 +199,27 @@ different Azure Storage account credentials under the `storage` property. This
 may be needed for more flexible configuration in other configuration files. In
 the example above, we only have one storage account defined which is aliased
 by the property name `mystorageaccount`. The alias (or storage account link
-name) can be the same as the storage account name itself.
-    * (optional) `account_key_keyvault_secret_id` property can be used to
-      reference an Azure KeyVault secret id. Batch Shipyard will contact the
-      specified KeyVault and replace the `account_key` value as returned by
-      Azure KeyVault.
+name) can be the same as the storage account name itself. Note that it is
+possible to not specify an `account_key` directly through the use of `aad`
+or `account_key_keyvault_secret_id`.
+    * (optional) `aad` AAD authentication parameters for Azure Storage.
+    * (required, at least 1) `<account-name-link>` is an arbitrary account
+      name link. This does not necessarily need to be the name of the storage
+      account but the link name to be referred in other configuration files.
+      This link name cannot be named `aad`.
+        * (required) `account` is the storage account name
+        * (required unless `aad` or `account_key_keyvault_secret_id` is
+          specified) `account_key` is the storage account key
+        * (optional) `account_key_keyvault_secret_id` property can be used to
+          reference an Azure KeyVault secret id. Batch Shipyard will contact
+          the specified KeyVault and replace the `account_key` value as
+          returned by Azure KeyVault.
+        * (optional) `endpoint` is the storage endpoint to use. The default
+          if not specified is `core.windows.net` which is the Public Azure
+          default.
+        * (required if `aad` is specified) `resource_group` is the resource
+          group of the storage account. This is required if `account_key`
+          is not specified and `aad` is used instead.
 
 ### Docker and Singularity Registries: `docker_registry` and `singularity_registry`
 * (optional) `docker_registry` or `singularity_registry` property defines

docs/13-batch-shipyard-configuration-pool.md

@@ -92,7 +92,7 @@ pool_specification:
   virtual_network:
     arm_subnet_id: /subscriptions/<subscription_id>/resourceGroups/<resource_group>/providers/Microsoft.Network/virtualNetworks/<virtual_network_name>/subnets/<subnet_name>
     name: myvnet
-    resource_group: vnet-in-another-rg
+    resource_group: resource-group-of-vnet
     create_nonexistant: false
     address_space: 10.0.0.0/16
     subnet: