Merge pull request #2331 from craddm/remove-data-provider-internet

Remove support for `Internet` Service Tag for Data Provider IP addresses

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

data_safe_haven/infrastructure/components/composite/nfsv3_storage_account.py

@@ -7,23 +7,20 @@
 from data_safe_haven.infrastructure.components.wrapped import (
     WrappedLogAnalyticsWorkspace,
 )
-from data_safe_haven.types import AzureServiceTag
 
 
 class NFSV3StorageAccountProps:
     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],
         subnet_id: Input[str],
     ):
         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(

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,

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,

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="[email protected]",
-            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(