diff --git a/data_safe_haven/config/config_sections.py b/data_safe_haven/config/config_sections.py index 62bfec0833..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] | AzureServiceTag = [] + data_provider_ip_addresses: list[IpAddress] = [] remote_desktop: ConfigSubsectionRemoteDesktopOpts research_user_ip_addresses: list[IpAddress] | AzureServiceTag = [] storage_quota_gb: ConfigSubsectionStorageQuotaGB 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..59a2f4e50b 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,16 @@ 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() - ] - ) + 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( @@ -84,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( diff --git a/data_safe_haven/infrastructure/programs/sre/data.py b/data_safe_haven/infrastructure/programs/sre/data.py index 825861c122..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: @@ -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( @@ -519,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, 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, 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(