Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove support for Internet Service Tag for Data Provider IP addresses #2331

Merged
2 changes: 1 addition & 1 deletion data_safe_haven/config/config_sections.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand All @@ -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(
Expand Down
24 changes: 9 additions & 15 deletions data_safe_haven/infrastructure/programs/sre/data.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
SSLCertificateProps,
WrappedLogAnalyticsWorkspace,
)
from data_safe_haven.types import AzureDnsZoneNames, AzureServiceTag
from data_safe_haven.types import AzureDnsZoneNames


class SREDataProps:
Expand All @@ -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],
Expand Down Expand Up @@ -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
}
)
JimMadge marked this conversation as resolved.
Show resolved Hide resolved

# Define Key Vault reader
identity_key_vault_reader = managedidentity.UserAssignedIdentity(
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
18 changes: 0 additions & 18 deletions tests/config/test_config_sections.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Loading