From 81cc93b47e4ac0e6ff209e532a06000967e9fe2a Mon Sep 17 00:00:00 2001 From: Matt Craddock <5796417+craddm@users.noreply.github.com> Date: Fri, 6 Dec 2024 11:08:43 +0000 Subject: [PATCH 1/8] Remove AzureServiceTag option from config for data provider ip --- data_safe_haven/config/config_sections.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/data_safe_haven/config/config_sections.py b/data_safe_haven/config/config_sections.py index 62bfec0833..1118121e63 100644 --- a/data_safe_haven/config/config_sections.py +++ b/data_safe_haven/config/config_sections.py @@ -57,7 +57,7 @@ class ConfigSectionSRE(BaseModel, validate_assignment=True): admin_email_address: EmailAddress admin_ip_addresses: list[IpAddress] = [] databases: UniqueList[DatabaseSystem] = [] - data_provider_ip_addresses: list[IpAddress] | AzureServiceTag = [] + data_provider_ip_addresses: list[IpAddress] remote_desktop: ConfigSubsectionRemoteDesktopOpts research_user_ip_addresses: list[IpAddress] | AzureServiceTag = [] storage_quota_gb: ConfigSubsectionStorageQuotaGB From c4b47d32f24caeacd6d74215b62ec3034ff677bb Mon Sep 17 00:00:00 2001 From: Matt Craddock <5796417+craddm@users.noreply.github.com> Date: Fri, 6 Dec 2024 11:09:11 +0000 Subject: [PATCH 2/8] Remove code to handle service tag for data provider IP address --- .../infrastructure/programs/sre/data.py | 21 +++++++------------ 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/data_safe_haven/infrastructure/programs/sre/data.py b/data_safe_haven/infrastructure/programs/sre/data.py index 825861c122..6bfc8fe315 100644 --- a/data_safe_haven/infrastructure/programs/sre/data.py +++ b/data_safe_haven/infrastructure/programs/sre/data.py @@ -49,7 +49,7 @@ def __init__( admin_email_address: Input[str], admin_group_id: Input[str], admin_ip_addresses: Input[Sequence[str]], - data_provider_ip_addresses: Input[list[str]] | AzureServiceTag, + data_provider_ip_addresses: Input[list[str]], dns_private_zones: Input[dict[str, network.PrivateZone]], dns_record: Input[network.RecordSet], dns_server_admin_password: Input[pulumi_random.RandomPassword], @@ -111,18 +111,13 @@ def __init__( child_opts = ResourceOptions.merge(opts, ResourceOptions(parent=self)) child_tags = {"component": "data"} | (tags if tags else {}) - if isinstance(props.data_provider_ip_addresses, list): - data_private_sensitive_service_tag = None - data_private_sensitive_ip_addresses = Output.all( - props.data_configuration_ip_addresses, props.data_provider_ip_addresses - ).apply( - lambda address_lists: { - ip for address_list in address_lists for ip in address_list - } - ) - else: - data_private_sensitive_ip_addresses = None - data_private_sensitive_service_tag = props.data_provider_ip_addresses + data_private_sensitive_ip_addresses = Output.all( + props.data_configuration_ip_addresses, props.data_provider_ip_addresses + ).apply( + lambda address_lists: { + ip for address_list in address_lists for ip in address_list + } + ) # Define Key Vault reader identity_key_vault_reader = managedidentity.UserAssignedIdentity( From 43415368f4ed95bea1b5823f81d89df55e9e2580 Mon Sep 17 00:00:00 2001 From: Matt Craddock <5796417+craddm@users.noreply.github.com> Date: Fri, 6 Dec 2024 11:31:44 +0000 Subject: [PATCH 3/8] Correctly specify config data provider ip address as list in config --- data_safe_haven/config/config_sections.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/data_safe_haven/config/config_sections.py b/data_safe_haven/config/config_sections.py index 1118121e63..f09da25648 100644 --- a/data_safe_haven/config/config_sections.py +++ b/data_safe_haven/config/config_sections.py @@ -57,7 +57,7 @@ class ConfigSectionSRE(BaseModel, validate_assignment=True): admin_email_address: EmailAddress admin_ip_addresses: list[IpAddress] = [] databases: UniqueList[DatabaseSystem] = [] - data_provider_ip_addresses: list[IpAddress] + data_provider_ip_addresses: list[IpAddress] = [] remote_desktop: ConfigSubsectionRemoteDesktopOpts research_user_ip_addresses: list[IpAddress] | AzureServiceTag = [] storage_quota_gb: ConfigSubsectionStorageQuotaGB From c4be76b161107e93ba4bde599c788c66e6f3e6f5 Mon Sep 17 00:00:00 2001 From: Matt Craddock <5796417+craddm@users.noreply.github.com> Date: Fri, 6 Dec 2024 11:32:10 +0000 Subject: [PATCH 4/8] remove service_tag options from storage account objects --- .../composite/nfsv3_storage_account.py | 29 +++++++------------ 1 file changed, 11 insertions(+), 18 deletions(-) diff --git a/data_safe_haven/infrastructure/components/composite/nfsv3_storage_account.py b/data_safe_haven/infrastructure/components/composite/nfsv3_storage_account.py index ca003bbd3d..962ee773a1 100644 --- a/data_safe_haven/infrastructure/components/composite/nfsv3_storage_account.py +++ b/data_safe_haven/infrastructure/components/composite/nfsv3_storage_account.py @@ -7,7 +7,6 @@ from data_safe_haven.infrastructure.components.wrapped import ( WrappedLogAnalyticsWorkspace, ) -from data_safe_haven.types import AzureServiceTag class NFSV3StorageAccountProps: @@ -15,7 +14,6 @@ def __init__( self, account_name: Input[str], allowed_ip_addresses: Input[Sequence[str]] | None, - allowed_service_tag: AzureServiceTag | None, location: Input[str], log_analytics_workspace: Input[WrappedLogAnalyticsWorkspace], resource_group_name: Input[str], @@ -23,7 +21,6 @@ def __init__( ): self.account_name = account_name self.allowed_ip_addresses = allowed_ip_addresses - self.allowed_service_tag = allowed_service_tag self.location = location self.log_analytics_workspace = log_analytics_workspace self.resource_group_name = resource_group_name @@ -54,21 +51,17 @@ def __init__( child_opts = ResourceOptions.merge(opts, ResourceOptions(parent=self)) child_tags = {"component": "data"} | (tags if tags else {}) - if props.allowed_service_tag == AzureServiceTag.INTERNET: - default_action = storage.DefaultAction.ALLOW - ip_rules = [] - else: - default_action = storage.DefaultAction.DENY - ip_rules = Output.from_input(props.allowed_ip_addresses).apply( - lambda ip_ranges: [ - storage.IPRuleArgs( - action=storage.Action.ALLOW, - i_p_address_or_range=str(ip_address), - ) - for ip_range in sorted(ip_ranges) - for ip_address in AzureIPv4Range.from_cidr(ip_range).all_ips() - ] - ) + default_action = storage.DefaultAction.DENY + ip_rules = Output.from_input(props.allowed_ip_addresses).apply( + lambda ip_ranges: [ + storage.IPRuleArgs( + action=storage.Action.ALLOW, + i_p_address_or_range=str(ip_address), + ) + for ip_range in sorted(ip_ranges) + for ip_address in AzureIPv4Range.from_cidr(ip_range).all_ips() + ] + ) # Deploy storage account self.storage_account = storage.StorageAccount( From 0e822010c2adf37579b44021ed8463e7e61d234c Mon Sep 17 00:00:00 2001 From: Matt Craddock <5796417+craddm@users.noreply.github.com> Date: Fri, 6 Dec 2024 11:32:59 +0000 Subject: [PATCH 5/8] Remove service_tag configurations options from sensitive data objects --- data_safe_haven/infrastructure/programs/sre/data.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/data_safe_haven/infrastructure/programs/sre/data.py b/data_safe_haven/infrastructure/programs/sre/data.py index 6bfc8fe315..10732670f5 100644 --- a/data_safe_haven/infrastructure/programs/sre/data.py +++ b/data_safe_haven/infrastructure/programs/sre/data.py @@ -38,7 +38,7 @@ SSLCertificateProps, WrappedLogAnalyticsWorkspace, ) -from data_safe_haven.types import AzureDnsZoneNames, AzureServiceTag +from data_safe_haven.types import AzureDnsZoneNames class SREDataProps: @@ -514,7 +514,6 @@ def __init__( f"{''.join(truncate_tokens(stack_name.split('-'), 11))}sensitivedata{sha256hash(self._name)}" )[:24], allowed_ip_addresses=data_private_sensitive_ip_addresses, - allowed_service_tag=data_private_sensitive_service_tag, location=props.location, log_analytics_workspace=props.log_analytics_workspace, subnet_id=props.subnet_data_private_id, From 64142b778f06732a0c6b7757c710187c885d4e67 Mon Sep 17 00:00:00 2001 From: Matt Craddock <5796417+craddm@users.noreply.github.com> Date: Fri, 6 Dec 2024 11:33:27 +0000 Subject: [PATCH 6/8] Remove service tag related tests for data provider config section --- tests/config/test_config_sections.py | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/tests/config/test_config_sections.py b/tests/config/test_config_sections.py index 7d9a0ba873..6528b130fa 100644 --- a/tests/config/test_config_sections.py +++ b/tests/config/test_config_sections.py @@ -170,24 +170,6 @@ def test_all_databases_must_be_unique(self) -> None: databases=[DatabaseSystem.POSTGRESQL, DatabaseSystem.POSTGRESQL], ) - def test_data_provider_tag_internet( - self, - config_subsection_remote_desktop: ConfigSubsectionRemoteDesktopOpts, - config_subsection_storage_quota_gb: ConfigSubsectionStorageQuotaGB, - ): - sre_config = ConfigSectionSRE( - admin_email_address="admin@example.com", - remote_desktop=config_subsection_remote_desktop, - storage_quota_gb=config_subsection_storage_quota_gb, - data_provider_ip_addresses="Internet", - ) - assert isinstance(sre_config.data_provider_ip_addresses, AzureServiceTag) - assert sre_config.data_provider_ip_addresses == "Internet" - - def test_data_provider_tag_invalid(self): - with pytest.raises(ValueError, match="Input should be 'Internet'"): - ConfigSectionSRE(data_provider_ip_addresses="Not a tag") - def test_ip_overlap_admin(self): with pytest.raises(ValueError, match="IP addresses must not overlap."): ConfigSectionSRE( From 333e7f05642f5ef7dc6b3323ac6ca56f7061105d Mon Sep 17 00:00:00 2001 From: Matt Craddock <5796417+craddm@users.noreply.github.com> Date: Fri, 6 Dec 2024 11:44:03 +0000 Subject: [PATCH 7/8] Remove unneeded service tag argument --- data_safe_haven/infrastructure/programs/sre/desired_state.py | 1 - 1 file changed, 1 deletion(-) diff --git a/data_safe_haven/infrastructure/programs/sre/desired_state.py b/data_safe_haven/infrastructure/programs/sre/desired_state.py index 20f4e357f1..7b638502a0 100644 --- a/data_safe_haven/infrastructure/programs/sre/desired_state.py +++ b/data_safe_haven/infrastructure/programs/sre/desired_state.py @@ -113,7 +113,6 @@ def __init__( f"{''.join(truncate_tokens(stack_name.split('-'), 11))}desiredstate{sha256hash(self._name)}" )[:24], allowed_ip_addresses=props.admin_ip_addresses, - allowed_service_tag=None, location=props.location, log_analytics_workspace=props.log_analytics_workspace, resource_group_name=props.resource_group_name, From d9d3f587a97a58603bb1b3082b25da24c8451e66 Mon Sep 17 00:00:00 2001 From: Matt Craddock <5796417+craddm@users.noreply.github.com> Date: Mon, 9 Dec 2024 13:59:40 +0000 Subject: [PATCH 8/8] remove unnecessary variable --- .../components/composite/nfsv3_storage_account.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/data_safe_haven/infrastructure/components/composite/nfsv3_storage_account.py b/data_safe_haven/infrastructure/components/composite/nfsv3_storage_account.py index 962ee773a1..59a2f4e50b 100644 --- a/data_safe_haven/infrastructure/components/composite/nfsv3_storage_account.py +++ b/data_safe_haven/infrastructure/components/composite/nfsv3_storage_account.py @@ -51,7 +51,6 @@ def __init__( child_opts = ResourceOptions.merge(opts, ResourceOptions(parent=self)) child_tags = {"component": "data"} | (tags if tags else {}) - default_action = storage.DefaultAction.DENY ip_rules = Output.from_input(props.allowed_ip_addresses).apply( lambda ip_ranges: [ storage.IPRuleArgs( @@ -77,7 +76,7 @@ def __init__( minimum_tls_version=storage.MinimumTlsVersion.TLS1_2, network_rule_set=storage.NetworkRuleSetArgs( bypass=storage.Bypass.AZURE_SERVICES, - default_action=default_action, + default_action=storage.DefaultAction.DENY, ip_rules=ip_rules, virtual_network_rules=[ storage.VirtualNetworkRuleArgs(