From 89c1d8e3c036d146e12717aa7956133866018085 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Tue, 1 Aug 2023 10:24:16 +0100 Subject: [PATCH 1/7] :heavy_plus_sign: Add additional imports --- data_safe_haven/external/api/azure_api.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/data_safe_haven/external/api/azure_api.py b/data_safe_haven/external/api/azure_api.py index 6792a835ee..b7ec358bc9 100644 --- a/data_safe_haven/external/api/azure_api.py +++ b/data_safe_haven/external/api/azure_api.py @@ -21,9 +21,15 @@ from azure.mgmt.automation.models import ( DscCompilationJobCreateParameters, DscConfigurationAssociationProperty, + Module, ) from azure.mgmt.compute import ComputeManagementClient -from azure.mgmt.compute.models import RunCommandInput, RunCommandInputParameter +from azure.mgmt.compute.models import ( + ResourceSkuCapabilities, + RunCommandInput, + RunCommandInputParameter, + RunCommandResult, +) from azure.mgmt.dns import DnsManagementClient from azure.mgmt.dns.models import RecordSet, TxtRecord from azure.mgmt.keyvault import KeyVaultManagementClient @@ -37,8 +43,10 @@ ) from azure.mgmt.msi import ManagedServiceIdentityClient from azure.mgmt.msi.models import Identity -from azure.mgmt.resource import ResourceManagementClient, SubscriptionClient +from azure.mgmt.resource.resources import ResourceManagementClient from azure.mgmt.resource.resources.models import ResourceGroup +from azure.mgmt.resource.subscriptions import SubscriptionClient +from azure.mgmt.resource.subscriptions.models import Location from azure.mgmt.storage import StorageManagementClient from azure.mgmt.storage.models import ( BlobContainer, From 102c5e6fb293df763113cdb8cb539cab135ced10 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Tue, 1 Aug 2023 10:22:28 +0100 Subject: [PATCH 2/7] :alien: Add type casts to correct spurious type hints in Azure libraries --- data_safe_haven/external/api/azure_api.py | 51 ++++++++++++++----- .../external/interface/azure_authenticator.py | 9 +++- 2 files changed, 44 insertions(+), 16 deletions(-) diff --git a/data_safe_haven/external/api/azure_api.py b/data_safe_haven/external/api/azure_api.py index b7ec358bc9..214c618004 100644 --- a/data_safe_haven/external/api/azure_api.py +++ b/data_safe_haven/external/api/azure_api.py @@ -2,7 +2,7 @@ import time from collections.abc import Sequence from contextlib import suppress -from typing import Any +from typing import Any, cast from azure.core.exceptions import ( HttpResponseError, @@ -94,8 +94,12 @@ def compile_desired_state( automation_client = AutomationClient(self.credential, self.subscription_id) # Wait until all modules are available while True: - available_modules = automation_client.module.list_by_automation_account( - resource_group_name, automation_account_name + # Cast to correct spurious type hint in Azure libraries + available_modules = cast( + list[Module], + automation_client.module.list_by_automation_account( + resource_group_name, automation_account_name + ), ) available_module_names = [ module.name @@ -284,8 +288,11 @@ def ensure_keyvault( ), ), ) + # Cast to correct spurious type hint in Azure libraries key_vaults = [ - kv for kv in key_vault_client.vaults.list() if kv.name == key_vault_name + kv + for kv in cast(list[Vault], key_vault_client.vaults.list()) + if kv.name == key_vault_name ] self.logger.info( f"Ensured that key vault [green]{key_vaults[0].name}[/] exists.", @@ -476,9 +483,12 @@ def ensure_resource_group( resource_group_name, ResourceGroup(location=location, tags=tags), ) + # Cast to correct spurious type hint in Azure libraries resource_groups = [ rg - for rg in resource_client.resource_groups.list() + for rg in cast( + list[ResourceGroup], resource_client.resource_groups.list() + ) if rg.name == resource_group_name ] self.logger.info( @@ -623,9 +633,12 @@ def get_locations(self) -> list[str]: try: subscription_client = SubscriptionClient(self.credential) return [ - location.name - for location in subscription_client.subscriptions.list_locations( - subscription_id=self.subscription_id + str(location.name) + for location in cast( + list[Location], + subscription_client.subscriptions.list_locations( + subscription_id=self.subscription_id + ), ) ] except Exception as exc: @@ -680,7 +693,10 @@ def get_vm_sku_details(self, sku: str) -> tuple[str, str, str]: for resource_sku in compute_client.resource_skus.list(): if resource_sku.name == sku: if resource_sku.capabilities: - for capability in resource_sku.capabilities: + # Cast to correct spurious type hint in Azure libraries + for capability in cast( + list[ResourceSkuCapabilities], resource_sku.capabilities + ): if capability.name == "vCPUs": cpus = capability.value if capability.name == "GPUs": @@ -754,7 +770,10 @@ def list_available_vm_skus(self, location: str) -> dict[str, dict[str, Any]]: "GPUs": 0 } # default to 0 GPUs, overriding if appropriate if resource_sku.capabilities: - for capability in resource_sku.capabilities: + # Cast to correct spurious type hint in Azure libraries + for capability in cast( + list[ResourceSkuCapabilities], resource_sku.capabilities + ): skus[resource_sku.name][capability.name] = capability.value return skus except Exception as exc: @@ -897,9 +916,12 @@ def remove_resource_group(self, resource_group_name: str) -> None: ) while not poller.done(): poller.wait(10) + # Cast to correct spurious type hint in Azure libraries resource_groups = [ rg - for rg in resource_client.resource_groups.list() + for rg in cast( + list[ResourceGroup], resource_client.resource_groups.list() + ) if rg.name == resource_group_name ] if resource_groups: @@ -979,9 +1001,10 @@ def run_remote_script( poller = compute_client.virtual_machines.begin_run_command( resource_group_name, vm_name, run_command_parameters ) - result = poller.result() - # Return stdout/stderr from the command - return str(result.value[0].message) + # Cast to correct spurious type hint in Azure libraries + result = cast(RunCommandResult, poller.result()) + # Return any stdout/stderr from the command + return str(result.value[0].message) if result.value else "" except Exception as exc: msg = f"Failed to run command on '{vm_name}'.\n{exc}" raise DataSafeHavenAzureError(msg) from exc diff --git a/data_safe_haven/external/interface/azure_authenticator.py b/data_safe_haven/external/interface/azure_authenticator.py index 850d8459ed..74d28672fd 100644 --- a/data_safe_haven/external/interface/azure_authenticator.py +++ b/data_safe_haven/external/interface/azure_authenticator.py @@ -1,7 +1,10 @@ """Standalone utility class for anything that needs to authenticate against Azure""" +from typing import cast + from azure.core.exceptions import ClientAuthenticationError from azure.identity import DefaultAzureCredential -from azure.mgmt.resource import SubscriptionClient +from azure.mgmt.resource.subscriptions import SubscriptionClient +from azure.mgmt.resource.subscriptions.models import Subscription from data_safe_haven.exceptions import ( DataSafeHavenAzureError, @@ -53,7 +56,9 @@ def login(self) -> None: # Check that the Azure credentials are valid try: - for subscription in list(subscription_client.subscriptions.list()): + for subscription in cast( + list[Subscription], subscription_client.subscriptions.list() + ): if subscription.display_name == self.subscription_name: self.subscription_id_ = subscription.subscription_id self.tenant_id_ = subscription.tenant_id From bb508c98a0c7e2bc63752768ba48e6935293ee2d Mon Sep 17 00:00:00 2001 From: James Robinson Date: Tue, 1 Aug 2023 10:36:59 +0100 Subject: [PATCH 3/7] :memo: Remove outdated comments --- data_safe_haven/external/api/azure_api.py | 26 +++++++++++------------ 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/data_safe_haven/external/api/azure_api.py b/data_safe_haven/external/api/azure_api.py index 214c618004..9a7fcf7f5a 100644 --- a/data_safe_haven/external/api/azure_api.py +++ b/data_safe_haven/external/api/azure_api.py @@ -23,17 +23,17 @@ DscConfigurationAssociationProperty, Module, ) -from azure.mgmt.compute import ComputeManagementClient -from azure.mgmt.compute.models import ( +from azure.mgmt.compute.v2021_07_01 import ComputeManagementClient +from azure.mgmt.compute.v2021_07_01.models import ( ResourceSkuCapabilities, RunCommandInput, RunCommandInputParameter, RunCommandResult, ) -from azure.mgmt.dns import DnsManagementClient -from azure.mgmt.dns.models import RecordSet, TxtRecord -from azure.mgmt.keyvault import KeyVaultManagementClient -from azure.mgmt.keyvault.models import ( +from azure.mgmt.dns.v2018_05_01 import DnsManagementClient +from azure.mgmt.dns.v2018_05_01.models import RecordSet, TxtRecord +from azure.mgmt.keyvault.v2021_06_01_preview import KeyVaultManagementClient +from azure.mgmt.keyvault.v2021_06_01_preview.models import ( AccessPolicyEntry, Permissions, Sku as KeyVaultSku, @@ -41,14 +41,14 @@ VaultCreateOrUpdateParameters, VaultProperties, ) -from azure.mgmt.msi import ManagedServiceIdentityClient -from azure.mgmt.msi.models import Identity -from azure.mgmt.resource.resources import ResourceManagementClient -from azure.mgmt.resource.resources.models import ResourceGroup +from azure.mgmt.msi.v2022_01_31_preview import ManagedServiceIdentityClient +from azure.mgmt.msi.v2022_01_31_preview.models import Identity +from azure.mgmt.resource.resources.v2021_04_01 import ResourceManagementClient +from azure.mgmt.resource.resources.v2021_04_01.models import ResourceGroup from azure.mgmt.resource.subscriptions import SubscriptionClient from azure.mgmt.resource.subscriptions.models import Location -from azure.mgmt.storage import StorageManagementClient -from azure.mgmt.storage.models import ( +from azure.mgmt.storage.v2021_08_01 import StorageManagementClient +from azure.mgmt.storage.v2021_08_01.models import ( BlobContainer, Kind as StorageAccountKind, PublicAccess, @@ -236,7 +236,6 @@ def ensure_keyvault( ) -> Vault: """Ensure that a KeyVault exists - Raises: DataSafeHavenAzureError if the existence of the KeyVault could not be verified """ @@ -444,7 +443,6 @@ def ensure_managed_identity( msi_client = ManagedServiceIdentityClient( self.credential, self.subscription_id ) - # mypy erroneously thinks that create_or_update returns Any rather than Identity managed_identity = msi_client.user_assigned_identities.create_or_update( resource_group_name, identity_name, From 3eaa5c56f0413f9ba1fcafbc6752b1d9ddaa3533 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Tue, 1 Aug 2023 11:28:11 +0100 Subject: [PATCH 4/7] :alien: Workaround for lack of next() in dns.resolver.Answer --- data_safe_haven/external/api/graph_api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/data_safe_haven/external/api/graph_api.py b/data_safe_haven/external/api/graph_api.py index 64c01a1be8..ab0a68d7c9 100644 --- a/data_safe_haven/external/api/graph_api.py +++ b/data_safe_haven/external/api/graph_api.py @@ -858,7 +858,7 @@ def verify_custom_domain( # Check whether all expected nameservers are active with suppress(resolver.NXDOMAIN): active_nameservers = [ - str(ns) for ns in resolver.resolve(domain_name, "NS") + str(ns) for ns in iter(resolver.resolve(domain_name, "NS")) ] self.logger.info("Checking domain verification status.") if all( From 6fa7492ceaebc2293feebebd9d2056ec527a46fa Mon Sep 17 00:00:00 2001 From: James Robinson Date: Tue, 1 Aug 2023 11:30:27 +0100 Subject: [PATCH 5/7] :coffin: Removed unused code --- data_safe_haven/pulumi/components/automation_dsc_node.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/data_safe_haven/pulumi/components/automation_dsc_node.py b/data_safe_haven/pulumi/components/automation_dsc_node.py index c6a13a84a0..b17caba336 100644 --- a/data_safe_haven/pulumi/components/automation_dsc_node.py +++ b/data_safe_haven/pulumi/components/automation_dsc_node.py @@ -1,5 +1,4 @@ """Register a VM as an Azure Automation DSC node""" -import pathlib import time from collections.abc import Sequence @@ -57,7 +56,6 @@ def __init__( ): super().__init__("dsh:common:AutomationDscNode", name, {}, opts) child_opts = ResourceOptions.merge(ResourceOptions(parent=self), opts) - pathlib.Path(__file__).parent.parent.parent / "resources" # Upload the primary domain controller DSC dsc = automation.DscConfiguration( From b49282f35070d13f5c01f36c19d6bdb24f2a9aed Mon Sep 17 00:00:00 2001 From: James Robinson Date: Tue, 1 Aug 2023 11:32:05 +0100 Subject: [PATCH 6/7] :label: Ensure subnet_id is not None --- data_safe_haven/pulumi/components/shm_bastion.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/data_safe_haven/pulumi/components/shm_bastion.py b/data_safe_haven/pulumi/components/shm_bastion.py index 6b09193131..bf8d37ed8b 100644 --- a/data_safe_haven/pulumi/components/shm_bastion.py +++ b/data_safe_haven/pulumi/components/shm_bastion.py @@ -15,7 +15,7 @@ def __init__( # self.automation_account_name = automation_account_name self.location = location self.resource_group_name = resource_group_name - self.subnet_id = Output.from_input(subnet).apply(lambda s: s.id) + self.subnet_id = Output.from_input(subnet).apply(lambda s: s.id if s.id else "") class SHMBastionComponent(ComponentResource): From 01ba73433983dc6ed9c795d0ffad396d94f13cc2 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Tue, 1 Aug 2023 11:38:32 +0100 Subject: [PATCH 7/7] :bug: Process Outputs inside all().apply() --- .../pulumi/components/shm_monitoring.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/data_safe_haven/pulumi/components/shm_monitoring.py b/data_safe_haven/pulumi/components/shm_monitoring.py index fac929437b..c01d2be918 100644 --- a/data_safe_haven/pulumi/components/shm_monitoring.py +++ b/data_safe_haven/pulumi/components/shm_monitoring.py @@ -68,9 +68,10 @@ def __init__( sku=automation.SkuArgs(name=automation.SkuNameEnum.FREE), opts=child_opts, ) - automation_keys = automation.list_key_by_automation_account( - automation_account.name, resource_group_name=resource_group.name - ) + automation_keys = Output.all( + automation_account_name=automation_account.name, + resource_group_name=resource_group.name, + ).apply(lambda kwargs: automation.list_key_by_automation_account(**kwargs)) # List of modules as 'name: (version, SHA256 hash)' # Note that we exclude ComputerManagementDsc which is already present (https://docs.microsoft.com/en-us/azure/automation/shared-resources/modules#default-modules) @@ -156,10 +157,9 @@ def __init__( workspace_name=f"{stack_name}-log", opts=child_opts, ) - log_analytics_keys = operationalinsights.get_shared_keys( - resource_group_name=resource_group.name, - workspace_name=log_analytics.name, - ) + log_analytics_keys = Output.all( + resource_group_name=resource_group.name, workspace_name=log_analytics.name + ).apply(lambda kwargs: operationalinsights.get_shared_keys(**kwargs)) # Set up a private linkscope and endpoint for the log analytics workspace log_analytics_private_link_scope = insights.PrivateLinkScope(