Skip to content

Commit

Permalink
Merge pull request #2266 from craddm/shm-check
Browse files Browse the repository at this point in the history
Add confirmation checks and check for deployed SREs before teardown operations
  • Loading branch information
craddm authored Oct 30, 2024
2 parents 868d41c + 938bc78 commit 51de416
Show file tree
Hide file tree
Showing 6 changed files with 103 additions and 7 deletions.
18 changes: 15 additions & 3 deletions data_safe_haven/commands/shm.py
Original file line number Diff line number Diff line change
Expand Up @@ -140,9 +140,21 @@ def teardown() -> None:

# Teardown Data Safe Haven SHM infrastructure.
try:
config = SHMConfig.from_remote(context)
shm_infra = ImperativeSHM(context, config)
shm_infra.teardown()
if SHMConfig.remote_exists(context):
config = SHMConfig.from_remote(context)
shm_infra = ImperativeSHM(context, config)
console.print(
"Tearing down the Safe Haven Management environment will permanently delete all associated resources, including remotely stored configurations."
)
if not console.confirm(
"Do you wish to continue tearing down the SHM?", default_to_yes=False
):
console.print("SHM teardown cancelled by user.")
raise typer.Exit(0)
shm_infra.teardown()
else:
logger.critical(f"No deployed SHM found for context [green]{context.name}.")
raise typer.Exit(1)
except DataSafeHavenError as exc:
logger.critical("Could not teardown Safe Haven Management environment.")
raise typer.Exit(1) from exc
12 changes: 12 additions & 0 deletions data_safe_haven/commands/sre.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import typer

from data_safe_haven import console
from data_safe_haven.config import ContextManager, DSHPulumiConfig, SHMConfig, SREConfig
from data_safe_haven.exceptions import DataSafeHavenConfigError, DataSafeHavenError
from data_safe_haven.external import AzureSdk, GraphApi
Expand Down Expand Up @@ -190,6 +191,17 @@ def teardown(
pulumi_config = DSHPulumiConfig.from_remote(context)
sre_config = SREConfig.from_remote_by_name(context, name)

console.print(
"Tearing down the Secure Research Environment will permanently delete all associated resources, "
"including all data stored in the environment.\n"
"Ensure that any desired outputs have been extracted before continuing."
)
if not console.confirm(
"Do you wish to continue tearing down the SRE?", default_to_yes=False
):
console.print("SRE teardown cancelled by user.")
raise typer.Exit(0)

# Check whether current IP address is authorised to take administrator actions
if not ip_address_in_list(sre_config.sre.admin_ip_addresses):
logger.warning(
Expand Down
9 changes: 8 additions & 1 deletion data_safe_haven/infrastructure/programs/imperative_shm.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from data_safe_haven.config import Context, SHMConfig
from data_safe_haven.config import Context, DSHPulumiConfig, SHMConfig
from data_safe_haven.exceptions import (
DataSafeHavenAzureError,
DataSafeHavenMicrosoftGraphError,
Expand Down Expand Up @@ -178,6 +178,13 @@ def teardown(self) -> None:
DataSafeHavenAzureError if any resources cannot be destroyed
"""
logger = get_logger()
if DSHPulumiConfig.remote_exists(self.context):
pulumi_config = DSHPulumiConfig.from_remote(self.context)
deployed = pulumi_config.project_names
if deployed:
logger.info(f"Found deployed SREs: {deployed}.")
msg = "Deployed SREs must be torn down before the SHM can be torn down."
raise DataSafeHavenAzureError(msg)
try:
logger.info(
f"Removing [green]{self.context.description}[/] resource group {self.context.resource_group_name}."
Expand Down
14 changes: 14 additions & 0 deletions tests/commands/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,15 @@ def mock_pulumi_config_from_remote(mocker, pulumi_config):
mocker.patch.object(DSHPulumiConfig, "from_remote", return_value=pulumi_config)


@fixture
def mock_pulumi_config_from_remote_fails(mocker):
mocker.patch.object(
DSHPulumiConfig,
"from_remote",
return_value=DataSafeHavenAzureError("mock from_remote failure"),
)


@fixture
def mock_pulumi_config_from_remote_or_create(mocker, pulumi_config_empty):
mocker.patch.object(
Expand All @@ -114,6 +123,11 @@ def mock_pulumi_config_upload(mocker):
mocker.patch.object(DSHPulumiConfig, "upload", return_value=None)


@fixture
def mock_pulumi_config_remote_exists(mocker):
mocker.patch.object(DSHPulumiConfig, "remote_exists", return_value=True)


@fixture
def mock_shm_config_from_remote(mocker, shm_config):
mocker.patch.object(SHMConfig, "from_remote", return_value=shm_config)
Expand Down
42 changes: 41 additions & 1 deletion tests/commands/test_shm.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,9 @@ def test_teardown(
runner,
mock_imperative_shm_teardown_then_exit, # noqa: ARG002
mock_shm_config_from_remote, # noqa: ARG002
mock_shm_config_remote_exists, # noqa: ARG002
):
result = runner.invoke(shm_command_group, ["teardown"])
result = runner.invoke(shm_command_group, ["teardown"], input="y")
assert result.exit_code == 1
assert "mock teardown" in result.stdout

Expand All @@ -62,9 +63,48 @@ def test_auth_failure(
self,
runner,
mock_azuresdk_get_credential_failure, # noqa: ARG002
mock_shm_config_remote_exists, # noqa: ARG002
):
result = runner.invoke(shm_command_group, ["teardown"])
assert result.exit_code == 1
assert "mock get_credential\n" in result.stdout
assert "mock get_credential error" in result.stdout
assert "Could not teardown Safe Haven Management environment." in result.stdout

def test_teardown_sres_exist(
self,
runner,
mock_azuresdk_get_subscription_name, # noqa: ARG002
mock_pulumi_config_from_remote, # noqa: ARG002
mock_pulumi_config_remote_exists, # noqa: ARG002
mock_shm_config_from_remote, # noqa: ARG002
mock_shm_config_remote_exists, # noqa: ARG002
):
result = runner.invoke(shm_command_group, ["teardown"], input="y")
assert result.exit_code == 1
assert "Found deployed SREs" in result.stdout

def test_teardown_user_cancelled(
self,
runner,
mock_azuresdk_get_subscription_name, # noqa: ARG002
mock_pulumi_config_from_remote, # noqa: ARG002
mock_shm_config_from_remote, # noqa: ARG002
mock_shm_config_remote_exists, # noqa: ARG002
):
result = runner.invoke(shm_command_group, ["teardown"], input="n")
assert result.exit_code == 0
assert "cancelled" in result.stdout

def test_teardown_no_pulumi_config(
self,
runner,
mock_azuresdk_get_subscription_name, # noqa: ARG002
mock_pulumi_config_from_remote_fails, # noqa: ARG002
mock_shm_config_from_remote, # noqa: ARG002
mock_imperative_shm_teardown_then_exit, # noqa: ARG002
mock_shm_config_remote_exists, # noqa: ARG002
):
result = runner.invoke(shm_command_group, ["teardown"], input="y")
assert result.exit_code == 1
assert "mock teardown" in result.stdout
15 changes: 13 additions & 2 deletions tests/commands/test_sre.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,11 +103,10 @@ def test_teardown(
runner: CliRunner,
mock_ip_1_2_3_4, # noqa: ARG002
mock_pulumi_config_from_remote, # noqa: ARG002
mock_shm_config_from_remote, # noqa: ARG002
mock_sre_config_from_remote, # noqa: ARG002
mock_sre_project_manager_teardown_then_exit, # noqa: ARG002
) -> None:
result = runner.invoke(sre_command_group, ["teardown", "sandbox"])
result = runner.invoke(sre_command_group, ["teardown", "sandbox"], input="y")
assert result.exit_code == 1
assert "mock teardown" in result.stdout

Expand Down Expand Up @@ -138,3 +137,15 @@ def test_auth_failure(
assert result.exit_code == 1
assert "mock get_credential\n" in result.stdout
assert "mock get_credential error" in result.stdout

def test_teardown_cancelled(
self,
runner: CliRunner,
mock_ip_1_2_3_4, # noqa: ARG002
mock_pulumi_config_from_remote, # noqa: ARG002
mock_sre_config_from_remote, # noqa: ARG002
mock_sre_project_manager_teardown_then_exit, # noqa: ARG002
) -> None:
result = runner.invoke(sre_command_group, ["teardown", "sandbox"], input="n")
assert result.exit_code == 0
assert "cancelled by user" in result.stdout

0 comments on commit 51de416

Please sign in to comment.