Merge pull request #1970 from jemrobinson/remove-update-sections

Remove unused code

data_safe_haven/administration/users/guacamole_users.py

@@ -42,7 +42,7 @@ def __init__(
             / "remote_desktop"
             / "postgresql"
         )
-        self.group_name = f"Data Safe Haven SRE {config.safe_name} Users"
+        self.group_name = f"Data Safe Haven SRE {config.name} Users"
 
     def list(self) -> Sequence[ResearchUser]:
         """List all Guacamole users"""

data_safe_haven/commands/sre.py

@@ -1,6 +1,6 @@
 """Command-line application for managing SRE infrastructure."""
 
-from typing import Annotated, Optional
+from typing import Annotated
 
 import typer
 
@@ -10,7 +10,7 @@
     SHMConfig,
     SREConfig,
 )
-from data_safe_haven.exceptions import DataSafeHavenError, DataSafeHavenPulumiError
+from data_safe_haven.exceptions import DataSafeHavenError
 from data_safe_haven.external import GraphApi
 from data_safe_haven.infrastructure import SREProjectManager
 from data_safe_haven.logging import get_logger
@@ -22,25 +22,23 @@
 @sre_command_group.command()
 def deploy(
     name: Annotated[str, typer.Argument(help="Name of SRE to deploy")],
-    force: Annotated[
-        Optional[bool],  # noqa: UP007
+    force: Annotated[  # noqa: FBT002
+        bool,
         typer.Option(
             "--force",
             "-f",
             help="Force this operation, cancelling any others that are in progress.",
         ),
-    ] = None,
+    ] = False,
 ) -> None:
     """Deploy a Secure Research Environment"""
     logger = get_logger()
     try:
+        # Load context and SHM config
         context = ContextManager.from_file().assert_context()
-        pulumi_config = DSHPulumiConfig.from_remote(context)
         shm_config = SHMConfig.from_remote(context)
-        sre_config = SREConfig.from_remote_by_name(context, name)
 
-        # Load GraphAPI as this may require user-interaction that is not possible as
-        # part of a Pulumi declarative command
+        # Load GraphAPI as this may require user-interaction
         graph_api = GraphApi(
             tenant_id=shm_config.shm.entra_tenant_id,
             default_scopes=[
@@ -51,6 +49,12 @@ def deploy(
             ],
         )
 
+        # Load Pulumi and SRE configs
+        pulumi_config = DSHPulumiConfig.from_remote_or_create(
+            context, encrypted_key=None, projects={}
+        )
+        sre_config = SREConfig.from_remote_by_name(context, name)
+
         # Initialise Pulumi stack
         stack = SREProjectManager(
             context=context,
@@ -89,30 +93,27 @@ def deploy(
         )
 
         # Deploy Azure infrastructure with Pulumi
-        if force is None:
-            stack.deploy()
-        else:
+        try:
             stack.deploy(force=force)
+        finally:
+            # Upload Pulumi config to blob storage
+            pulumi_config.upload(context)
 
         # Provision SRE with anything that could not be done in Pulumi
         manager = SREProvisioningManager(
             graph_api_token=graph_api.token,
             location=sre_config.azure.location,
-            sre_name=sre_config.safe_name,
+            sre_name=sre_config.name,
             sre_stack=stack,
             subscription_name=context.subscription_name,
             timezone=sre_config.sre.timezone,
         )
         manager.run()
     except DataSafeHavenError as exc:
         logger.critical(
-            f"Could not deploy Secure Research Environment {sre_config.safe_name}."
+            f"Could not deploy Secure Research Environment '[green]{name}[/]'."
         )
         raise typer.Exit(code=1) from exc
-    finally:
-        # Upload Pulumi config to blob storage
-        if pulumi_config:
-            pulumi_config.upload(context)
 
 
 @sre_command_group.command()
@@ -122,30 +123,28 @@ def teardown(
     """Tear down a deployed a Secure Research Environment."""
     logger = get_logger()
     try:
+        # Load context and SHM config
         context = ContextManager.from_file().assert_context()
-        pulumi_config = DSHPulumiConfig.from_remote(context)
         shm_config = SHMConfig.from_remote(context)
-        sre_config = SREConfig.from_remote_by_name(context, name)
 
-        # Load GraphAPI as this may require user-interaction that is not possible as
-        # part of a Pulumi declarative command
+        # Load GraphAPI as this may require user-interaction
         graph_api = GraphApi(
             tenant_id=shm_config.shm.entra_tenant_id,
             default_scopes=["Application.ReadWrite.All", "Group.ReadWrite.All"],
         )
 
+        # Load Pulumi and SRE configs
+        pulumi_config = DSHPulumiConfig.from_remote(context)
+        sre_config = SREConfig.from_remote_by_name(context, name)
+
         # Remove infrastructure deployed with Pulumi
-        try:
-            stack = SREProjectManager(
-                context=context,
-                config=sre_config,
-                pulumi_config=pulumi_config,
-                graph_api_token=graph_api.token,
-            )
-            stack.teardown()
-        except Exception as exc:
-            msg = "Unable to teardown Pulumi infrastructure."
-            raise DataSafeHavenPulumiError(msg) from exc
+        stack = SREProjectManager(
+            context=context,
+            config=sre_config,
+            pulumi_config=pulumi_config,
+            graph_api_token=graph_api.token,
+        )
+        stack.teardown()
 
         # Remove Pulumi project from Pulumi config file
         del pulumi_config[name]
@@ -154,6 +153,6 @@ def teardown(
         pulumi_config.upload(context)
     except DataSafeHavenError as exc:
         logger.critical(
-            f"Could not teardown Secure Research Environment '{sre_config.safe_name}'."
+            f"Could not teardown Secure Research Environment '[green]{name}[/]'."
         )
         raise typer.Exit(1) from exc

data_safe_haven/commands/users.py

@@ -130,9 +130,8 @@ def register(
 
         # Load SREConfig
         sre_config = SREConfig.from_remote_by_name(context, sre)
-        sre_name = sre_config.safe_name
-        if sre_name not in pulumi_config.project_names:
-            msg = f"Could not load Pulumi settings for '{sre_name}'. Have you deployed the SRE?"
+        if sre_config.name not in pulumi_config.project_names:
+            msg = f"Could not load Pulumi settings for '{sre_config.name}'. Have you deployed the SRE?"
             logger.error(msg)
             raise DataSafeHavenError(msg)
 
@@ -143,7 +142,7 @@ def register(
         )
 
         logger.debug(
-            f"Preparing to register {len(usernames)} user(s) with SRE '{sre_name}'"
+            f"Preparing to register {len(usernames)} user(s) with SRE '{sre_config.name}'"
         )
 
         # List users
@@ -158,7 +157,7 @@ def register(
                     f"Username '{username}' does not belong to this Data Safe Haven deployment."
                     " Please use 'dsh users add' to create it."
                 )
-        users.register(sre_name, usernames_to_register)
+        users.register(sre_config.name, usernames_to_register)
     except DataSafeHavenError as exc:
         logger.critical(f"Could not register Data Safe Haven users with SRE '{sre}'.")
         raise typer.Exit(1) from exc
@@ -236,9 +235,8 @@ def unregister(
 
         # Load SREConfig
         sre_config = SREConfig.from_remote_by_name(context, sre)
-        sre_name = sre_config.safe_name
-        if sre_name not in pulumi_config.project_names:
-            msg = f"Could not load Pulumi settings for '{sre_name}'. Have you deployed the SRE?"
+        if sre_config.name not in pulumi_config.project_names:
+            msg = f"Could not load Pulumi settings for '{sre_config.name}'. Have you deployed the SRE?"
             logger.error(msg)
             raise DataSafeHavenError(msg)
 
@@ -249,7 +247,7 @@ def unregister(
         )
 
         logger.debug(
-            f"Preparing to unregister {len(usernames)} users with SRE '{sre_config.safe_name}'"
+            f"Preparing to unregister {len(usernames)} users with SRE '{sre_config.name}'"
         )
 
         # List users
@@ -265,9 +263,9 @@ def unregister(
                     " Please use 'dsh users add' to create it."
                 )
         for group_name in (
-            f"{sre_config.safe_name} Users",
-            f"{sre_config.safe_name} Privileged Users",
-            f"{sre_config.safe_name} Administrators",
+            f"{sre_config.name} Users",
+            f"{sre_config.name} Privileged Users",
+            f"{sre_config.name} Administrators",
         ):
             users.unregister(group_name, usernames_to_unregister)
     except DataSafeHavenError as exc:

data_safe_haven/config/config_sections.py

@@ -7,7 +7,6 @@
     Field,
 )
 
-from data_safe_haven.logging import get_logger
 from data_safe_haven.types import (
     AzureLocation,
     AzureVmSku,
@@ -33,68 +32,11 @@ class ConfigSectionSHM(BaseModel, validate_assignment=True):
     entra_tenant_id: Guid
     fqdn: Fqdn
 
-    def update(
-        self,
-        *,
-        admin_group_id: str | None = None,
-        entra_tenant_id: str | None = None,
-        fqdn: str | None = None,
-    ) -> None:
-        """Update SHM settings
-
-        Args:
-            admin_group_id: ID of a security group that contains all Azure infrastructure admins.
-            entra_tenant_id: Tenant ID for the Entra ID used to manage TRE users
-            fqdn: Fully-qualified domain name to use for this TRE
-        """
-        logger = get_logger()
-        # Set admin group ID
-        if admin_group_id:
-            self.admin_group_id = admin_group_id
-        logger.debug(
-            f"[bold]Admin group ID[/] will be [green]{self.admin_group_id}[/]."
-        )
-        # Set Entra tenant ID
-        if entra_tenant_id:
-            self.entra_tenant_id = entra_tenant_id
-        logger.debug(
-            f"[bold]Entra tenant ID[/] will be [green]{self.entra_tenant_id}[/]."
-        )
-        # Set fully-qualified domain name
-        if fqdn:
-            self.fqdn = fqdn
-        logger.debug(
-            f"[bold]Fully-qualified domain name[/] will be [green]{self.fqdn}[/]."
-        )
-
 
 class ConfigSubsectionRemoteDesktopOpts(BaseModel, validate_assignment=True):
     allow_copy: bool = False
     allow_paste: bool = False
 
-    def update(
-        self, *, allow_copy: bool | None = None, allow_paste: bool | None = None
-    ) -> None:
-        """Update SRE remote desktop settings
-
-        Args:
-            allow_copy: Allow/deny copying text out of the SRE
-            allow_paste: Allow/deny pasting text into the SRE
-        """
-        logger = get_logger()
-        # Set whether copying text out of the SRE is allowed
-        if allow_copy:
-            self.allow_copy = allow_copy
-        logger.debug(
-            f"[bold]Copying text out of the SRE[/] will be [green]{'allowed' if self.allow_copy else 'forbidden'}[/]."
-        )
-        # Set whether pasting text into the SRE is allowed
-        if allow_paste:
-            self.allow_paste = allow_paste
-        logger.debug(
-            f"[bold]Pasting text into the SRE[/] will be [green]{'allowed' if self.allow_paste else 'forbidden'}[/]."
-        )
-
 
 class ConfigSectionSRE(BaseModel, validate_assignment=True):
     admin_email_address: EmailAddress
@@ -114,77 +56,3 @@ class ConfigSectionSRE(BaseModel, validate_assignment=True):
     software_packages: SoftwarePackageCategory = SoftwarePackageCategory.NONE
     timezone: TimeZone = "Etc/UTC"
     workspace_skus: list[AzureVmSku] = Field(..., default_factory=list[AzureVmSku])
-
-    def update(
-        self,
-        *,
-        admin_email_address: str | None = None,
-        admin_ip_addresses: list[str] | None = None,
-        data_provider_ip_addresses: list[IpAddress] | None = None,
-        databases: list[DatabaseSystem] | None = None,
-        software_packages: SoftwarePackageCategory | None = None,
-        timezone: TimeZone | None = None,
-        user_ip_addresses: list[IpAddress] | None = None,
-        workspace_skus: list[AzureVmSku] | None = None,
-    ) -> None:
-        """Update SRE settings
-
-        Args:
-            admin_email_address: Email address shared by all administrators
-            admin_ip_addresses: List of IP addresses belonging to administrators
-            databases: List of database systems to deploy
-            data_provider_ip_addresses: List of IP addresses belonging to data providers
-            software_packages: Whether to allow packages from external repositories
-            timezone: Timezone in pytz format (eg. Europe/London)
-            user_ip_addresses: List of IP addresses belonging to users
-            workspace_skus: List of Azure VM SKUs - see cloudprice.net for list of valid SKUs
-        """
-        logger = get_logger()
-        # Set admin email address
-        if admin_email_address:
-            self.admin_email_address = admin_email_address
-        logger.debug(
-            f"[bold]Admin email address[/] will be [green]{self.admin_email_address}[/]."
-        )
-        # Set admin IP addresses
-        if admin_ip_addresses:
-            self.admin_ip_addresses = admin_ip_addresses
-        logger.debug(
-            f"[bold]IP addresses used by administrators[/] will be [green]{self.admin_ip_addresses}[/]."
-        )
-        # Set data provider IP addresses
-        if data_provider_ip_addresses:
-            self.data_provider_ip_addresses = data_provider_ip_addresses
-        logger.debug(
-            f"[bold]IP addresses used by data providers[/] will be [green]{self.data_provider_ip_addresses}[/]."
-        )
-        # Set which databases to deploy
-        if databases:
-            self.databases = sorted(set(databases))
-            if len(self.databases) != len(databases):
-                logger.warning("Discarding duplicate values for 'database'.")
-        logger.debug(
-            f"[bold]Databases available to users[/] will be [green]{[database.value for database in self.databases]}[/]."
-        )
-        # Select which software packages can be installed by users
-        if software_packages:
-            self.software_packages = software_packages
-        logger.debug(
-            f"[bold]Software packages[/] from [green]{self.software_packages.value}[/] sources will be installable."
-        )
-        # Set timezone
-        if timezone:
-            self.timezone = timezone
-        logger.info(f"[bold]Timezone[/] will be [green]{self.timezone}[/].")
-        # Set user IP addresses
-        if user_ip_addresses:
-            self.research_user_ip_addresses = user_ip_addresses
-        logger.debug(
-            f"[bold]IP addresses used by users[/] will be [green]{self.research_user_ip_addresses}[/]."
-        )
-        # Set workspace desktop SKUs
-        if workspace_skus:
-            self.workspace_skus = workspace_skus
-        logger.debug(
-            f"[bold]Workspace SKUs[/] will be [green]{self.workspace_skus}[/]."
-        )

data_safe_haven/config/sre_config.py

@@ -33,11 +33,6 @@ def filename(self) -> str:
         """Construct a canonical filename for this SREConfig."""
         return sre_config_name(self.name)
 
-    @property
-    def safe_name(self) -> str:
-        """Construct a JSON-safe version of the name of this SRE."""
-        return json_safe(self.name)
-
     @classmethod
     def from_remote_by_name(
         cls: type[Self], context: ContextBase, sre_name: str

data_safe_haven/external/api/azure_api.py

@@ -124,7 +124,7 @@ def blob_exists(
         except DataSafeHavenAzureError:
             exists = False
         response = "exists" if exists else "does not exist"
-        self.logger.info(
+        self.logger.debug(
             f"File [green]{blob_name}[/] {response} in blob storage.",
         )
         return exists