From be06583b02fb03ea1b96c27bd9facca6c2842c97 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Tue, 26 Mar 2024 17:20:15 +0000 Subject: [PATCH 1/7] :bug: Remove circular dependency in domain_sid extraction --- .../infrastructure/stacks/shm/domain_controllers.py | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/data_safe_haven/infrastructure/stacks/shm/domain_controllers.py b/data_safe_haven/infrastructure/stacks/shm/domain_controllers.py index ab460843a4..9fe20512cd 100644 --- a/data_safe_haven/infrastructure/stacks/shm/domain_controllers.py +++ b/data_safe_haven/infrastructure/stacks/shm/domain_controllers.py @@ -171,13 +171,7 @@ def __init__( ), opts=ResourceOptions.merge( child_opts, - ResourceOptions( - depends_on=[ - primary_domain_controller, - primary_domain_controller_dsc_node, - ], - parent=primary_domain_controller_dsc_node, - ), + ResourceOptions(parent=primary_domain_controller_dsc_node), ), ) From 2cdafcfe9e609c90514779481a96625a9a4437ff Mon Sep 17 00:00:00 2001 From: James Robinson Date: Tue, 26 Mar 2024 12:57:50 +0000 Subject: [PATCH 2/7] :alien: Add a TTL sleep before requesting a new SSL certificate to allow DNS changes to propagate fully --- .../components/dynamic/ssl_certificate.py | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/data_safe_haven/infrastructure/components/dynamic/ssl_certificate.py b/data_safe_haven/infrastructure/components/dynamic/ssl_certificate.py index 3ce4ad217c..deb63ce144 100644 --- a/data_safe_haven/infrastructure/components/dynamic/ssl_certificate.py +++ b/data_safe_haven/infrastructure/components/dynamic/ssl_certificate.py @@ -1,5 +1,6 @@ """Pulumi dynamic component for SSL certificates uploaded to an Azure KeyVault.""" +import time from contextlib import suppress from typing import Any @@ -78,11 +79,9 @@ def create(self, props: dict[str, Any]) -> CreateResult: client.generate_csr() # Request DNS verification tokens and add them to the DNS record azure_api = AzureApi(props["subscription_name"], disable_logging=True) - for ( - record_name, - record_values, - ) in client.request_verification_tokens().items(): - azure_api.ensure_dns_txt_record( + verification_tokens = client.request_verification_tokens().items() + for record_name, record_values in verification_tokens: + record_set = azure_api.ensure_dns_txt_record( record_name=record_name.replace(f".{props['domain_name']}", ""), record_value=record_values[0], resource_group_name=props["networking_resource_group_name"], @@ -94,6 +93,8 @@ def create(self, props: dict[str, Any]) -> CreateResult: ): msg = "DNS propagation failed" raise DataSafeHavenSSLError(msg) + # Wait for the TTL for this record to expire to remove risk of caching + time.sleep(record_set.ttl or 30) # Request a signed certificate try: certificate_bytes = client.request_certificate() @@ -101,6 +102,10 @@ def create(self, props: dict[str, Any]) -> CreateResult: msg = "\n".join( ["ACME validation error:"] + [str(auth_error) for auth_error in exc.failed_authzrs] + + [ + f"TXT record {record_name} is currently set to {record_values}" + for (record_name, record_values) in verification_tokens + ] ) raise DataSafeHavenSSLError(msg) from exc # Although KeyVault will accept a PEM certificate (where we simply prepend From 15ae9c7998b494e2f6b57c14bc9537831af1598b Mon Sep 17 00:00:00 2001 From: James Robinson Date: Wed, 27 Mar 2024 09:54:12 +0000 Subject: [PATCH 3/7] :rotating_light: Update ruff and fix resulting errors --- data_safe_haven/functions/strings.py | 2 +- pyproject.toml | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/data_safe_haven/functions/strings.py b/data_safe_haven/functions/strings.py index d7cc679423..3a9d325281 100644 --- a/data_safe_haven/functions/strings.py +++ b/data_safe_haven/functions/strings.py @@ -63,7 +63,7 @@ def replace_separators(input_string: str, separator: str = "") -> str: def seeded_uuid(seed: str) -> uuid.UUID: """Return a UUID seeded from a given string.""" - generator = random.Random() + generator = random.Random() # noqa: S311 generator.seed(seed) return uuid.UUID(int=generator.getrandbits(128), version=4) diff --git a/pyproject.toml b/pyproject.toml index 90e179c1bd..8b1162d6eb 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -65,7 +65,7 @@ dependencies = [ "black>=24.1.0", "mypy>=1.0.0", "pydantic>=2.4", - "ruff>=0.2.0", + "ruff>=0.3.4", "types-appdirs>=1.4.3.5", "types-chevron>=0.14.2.5", "types-pytz>=2023.3.0.0", @@ -77,7 +77,7 @@ dependencies = [ typing = "mypy {args:data_safe_haven}" style = [ - "ruff {args:data_safe_haven tests_}", + "ruff check {args:data_safe_haven tests_}", "black --check --diff {args:data_safe_haven tests_}", ] fmt = [ From c5e92738eeb9d4d66f0474e879d667f8c9fb7f91 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Tue, 13 Feb 2024 17:03:52 +0000 Subject: [PATCH 4/7] :truck: Rename dsh:sre:SRESoftwareRepositoriesComponent for consistency --- .../infrastructure/stacks/sre/software_repositories.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/data_safe_haven/infrastructure/stacks/sre/software_repositories.py b/data_safe_haven/infrastructure/stacks/sre/software_repositories.py index 073124737e..a411e1ea85 100644 --- a/data_safe_haven/infrastructure/stacks/sre/software_repositories.py +++ b/data_safe_haven/infrastructure/stacks/sre/software_repositories.py @@ -65,7 +65,7 @@ def __init__( opts: ResourceOptions | None = None, tags: Input[Mapping[str, Input[str]]] | None = None, ) -> None: - super().__init__("dsh:sre:SRESoftwareRepositoriesComponent", name, {}, opts) + super().__init__("dsh:sre:SoftwareRepositoriesComponent", name, {}, opts) child_opts = ResourceOptions.merge(opts, ResourceOptions(parent=self)) child_tags = tags if tags else {} From e1f4fe8833a1755d8645874e622119986ddd5563 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Wed, 14 Feb 2024 12:27:47 +0000 Subject: [PATCH 5/7] :see_no_evil: Add pytest cache to gitignore --- .gitignore | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.gitignore b/.gitignore index ba31e22e7e..6021ebf736 100644 --- a/.gitignore +++ b/.gitignore @@ -43,5 +43,8 @@ expanded.yaml # mypy cache .mypy_cache +# ruff cache +.pytest_cache + # ruff cache .ruff_cache From b989fae7b45e244ab48a4413aef154c8491bc5d8 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Thu, 15 Feb 2024 10:31:26 +0000 Subject: [PATCH 6/7] :alien: Cast to avoid incorrect type-hinting in external Azure library --- data_safe_haven/external/api/azure_api.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/data_safe_haven/external/api/azure_api.py b/data_safe_haven/external/api/azure_api.py index 7197d04298..68a47975fc 100644 --- a/data_safe_haven/external/api/azure_api.py +++ b/data_safe_haven/external/api/azure_api.py @@ -713,7 +713,7 @@ def get_storage_account_keys( if not isinstance(storage_keys, StorageAccountListKeysResult): msg = f"Could not connect to {msg_sa} in {msg_rg}." raise DataSafeHavenAzureError(msg) - keys: list[StorageAccountKey] = storage_keys.keys + keys = cast(list[StorageAccountKey], storage_keys.keys) if not keys or not isinstance(keys, list) or len(keys) == 0: msg = f"No keys were retrieved for {msg_sa} in {msg_rg}." raise DataSafeHavenAzureError(msg) @@ -888,7 +888,7 @@ def remove_blob( f"Removed file [green]{blob_name}[/] from blob storage.", ) except Exception as exc: - msg = f"Blob file '{blob_name}' could not be removed from '{storage_account_name}'\n{exc}." + msg = f"Blob file [green]'{blob_name}'[/] could not be removed from [green]'{storage_account_name}'[/].\n{exc}" raise DataSafeHavenAzureError(msg) from exc def remove_dns_txt_record( From 7dd81d447177e5b78d142542daab86ecbf0b397f Mon Sep 17 00:00:00 2001 From: James Robinson Date: Tue, 20 Feb 2024 10:39:10 +0000 Subject: [PATCH 7/7] :alien: Add missing signed_identifiers argument to storage.FileShare --- data_safe_haven/infrastructure/stacks/sre/data.py | 2 ++ data_safe_haven/infrastructure/stacks/sre/gitea_server.py | 2 ++ data_safe_haven/infrastructure/stacks/sre/hedgedoc_server.py | 1 + data_safe_haven/infrastructure/stacks/sre/remote_desktop.py | 1 + .../infrastructure/stacks/sre/software_repositories.py | 3 +++ 5 files changed, 9 insertions(+) diff --git a/data_safe_haven/infrastructure/stacks/sre/data.py b/data_safe_haven/infrastructure/stacks/sre/data.py index 3ce5b932d7..c0f2a0da70 100644 --- a/data_safe_haven/infrastructure/stacks/sre/data.py +++ b/data_safe_haven/infrastructure/stacks/sre/data.py @@ -691,6 +691,7 @@ def __init__( root_squash=storage.RootSquashType.NO_ROOT_SQUASH, share_name="home", share_quota=1024, + signed_identifiers=[], opts=ResourceOptions.merge( child_opts, ResourceOptions(parent=storage_account_data_private_user) ), @@ -704,6 +705,7 @@ def __init__( root_squash=storage.RootSquashType.ROOT_SQUASH, share_name="shared", share_quota=1024, + signed_identifiers=[], opts=ResourceOptions.merge( child_opts, ResourceOptions(parent=storage_account_data_private_user) ), diff --git a/data_safe_haven/infrastructure/stacks/sre/gitea_server.py b/data_safe_haven/infrastructure/stacks/sre/gitea_server.py index 7e665322fd..4107ae3d26 100644 --- a/data_safe_haven/infrastructure/stacks/sre/gitea_server.py +++ b/data_safe_haven/infrastructure/stacks/sre/gitea_server.py @@ -91,6 +91,7 @@ def __init__( resource_group_name=props.storage_account_resource_group_name, share_name="gitea-caddy", share_quota=1, + signed_identifiers=[], opts=child_opts, ) file_share_gitea_gitea = storage.FileShare( @@ -100,6 +101,7 @@ def __init__( resource_group_name=props.storage_account_resource_group_name, share_name="gitea-gitea", share_quota=1, + signed_identifiers=[], opts=child_opts, ) diff --git a/data_safe_haven/infrastructure/stacks/sre/hedgedoc_server.py b/data_safe_haven/infrastructure/stacks/sre/hedgedoc_server.py index 76b34cbc90..89e40be7fa 100644 --- a/data_safe_haven/infrastructure/stacks/sre/hedgedoc_server.py +++ b/data_safe_haven/infrastructure/stacks/sre/hedgedoc_server.py @@ -104,6 +104,7 @@ def __init__( resource_group_name=props.storage_account_resource_group_name, share_name="hedgedoc-caddy", share_quota=1, + signed_identifiers=[], opts=child_opts, ) diff --git a/data_safe_haven/infrastructure/stacks/sre/remote_desktop.py b/data_safe_haven/infrastructure/stacks/sre/remote_desktop.py index 8f975969a4..0ea737e7d5 100644 --- a/data_safe_haven/infrastructure/stacks/sre/remote_desktop.py +++ b/data_safe_haven/infrastructure/stacks/sre/remote_desktop.py @@ -150,6 +150,7 @@ def __init__( resource_group_name=props.storage_account_resource_group_name, share_name="remote-desktop-caddy", share_quota=1, + signed_identifiers=[], opts=child_opts, ) diff --git a/data_safe_haven/infrastructure/stacks/sre/software_repositories.py b/data_safe_haven/infrastructure/stacks/sre/software_repositories.py index a411e1ea85..a1e55052bb 100644 --- a/data_safe_haven/infrastructure/stacks/sre/software_repositories.py +++ b/data_safe_haven/infrastructure/stacks/sre/software_repositories.py @@ -77,6 +77,7 @@ def __init__( resource_group_name=props.storage_account_resource_group_name, share_name="software-repositories-caddy", share_quota=1, + signed_identifiers=[], opts=child_opts, ) file_share_nexus = storage.FileShare( @@ -86,6 +87,7 @@ def __init__( resource_group_name=props.storage_account_resource_group_name, share_name="software-repositories-nexus", share_quota=5120, + signed_identifiers=[], opts=child_opts, ) file_share_nexus_allowlists = storage.FileShare( @@ -95,6 +97,7 @@ def __init__( resource_group_name=props.storage_account_resource_group_name, share_name="software-repositories-nexus-allowlists", share_quota=1, + signed_identifiers=[], opts=child_opts, )