From d2e243ba36b80e5dbf42718199734064f3dca9e0 Mon Sep 17 00:00:00 2001 From: Matt Craddock <5796417+craddm@users.noreply.github.com> Date: Thu, 23 May 2024 11:32:21 +0000 Subject: [PATCH 01/33] Add more informative error messages to context commands --- data_safe_haven/commands/context.py | 34 ++++++++++++++++++++++++----- 1 file changed, 28 insertions(+), 6 deletions(-) diff --git a/data_safe_haven/commands/context.py b/data_safe_haven/commands/context.py index ca4e80f54d..33a3e77b39 100644 --- a/data_safe_haven/commands/context.py +++ b/data_safe_haven/commands/context.py @@ -22,7 +22,7 @@ def show() -> None: try: settings = ContextSettings.from_file() except DataSafeHavenConfigError as exc: - print("No context configuration file.") + print("No context configuration file. Run `dsh context add` to create one.") raise typer.Exit(code=1) from exc current_context_key = settings.selected @@ -39,7 +39,11 @@ def show() -> None: @context_command_group.command() def available() -> None: """Show the available contexts.""" - settings = ContextSettings.from_file() + try: + settings = ContextSettings.from_file() + except DataSafeHavenConfigError as exc: + print("No context configuration file. Run `dsh context add` to create one.") + raise typer.Exit(code=1) from exc current_context_key = settings.selected available = settings.available @@ -56,7 +60,11 @@ def switch( key: Annotated[str, typer.Argument(help="Key of the context to switch to.")] ) -> None: """Switch the selected context.""" - settings = ContextSettings.from_file() + try: + settings = ContextSettings.from_file() + except DataSafeHavenConfigError as exc: + print("No context configuration file. Run `dsh context add` to create one.") + raise typer.Exit(code=1) from exc settings.selected = key settings.write() @@ -145,7 +153,12 @@ def update( ] = None, ) -> None: """Update the selected context settings.""" - settings = ContextSettings.from_file() + try: + settings = ContextSettings.from_file() + except DataSafeHavenConfigError as exc: + print("No context configuration file. Run `dsh context add` to create one.") + raise typer.Exit(code=1) from exc + settings.update( admin_group_id=admin_group, location=location, @@ -160,7 +173,11 @@ def remove( key: Annotated[str, typer.Argument(help="Name of the context to remove.")], ) -> None: """Removes a context.""" - settings = ContextSettings.from_file() + try: + settings = ContextSettings.from_file() + except DataSafeHavenConfigError as exc: + print("No context configuration file, so no contexts to remove.") + raise typer.Exit(code=1) from exc settings.remove(key) settings.write() @@ -168,7 +185,12 @@ def remove( @context_command_group.command() def create() -> None: """Create Data Safe Haven context infrastructure.""" - context = ContextSettings.from_file().assert_context() + try: + settings = ContextSettings.from_file().assert_context() + except DataSafeHavenConfigError as exc: + print("No context configuration file. Run `dsh context add` before creating infrastructure.") + raise typer.Exit(code=1) from exc + context_infra = ContextInfrastructure(context) context_infra.create() From 6abd1f623ce17d7774b22fd17dededa42099f6c6 Mon Sep 17 00:00:00 2001 From: Matt Craddock <5796417+craddm@users.noreply.github.com> Date: Thu, 23 May 2024 15:29:06 +0000 Subject: [PATCH 02/33] Catch non-existent context file for teardown --- data_safe_haven/commands/context.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/data_safe_haven/commands/context.py b/data_safe_haven/commands/context.py index 33a3e77b39..454abb73b3 100644 --- a/data_safe_haven/commands/context.py +++ b/data_safe_haven/commands/context.py @@ -186,7 +186,7 @@ def remove( def create() -> None: """Create Data Safe Haven context infrastructure.""" try: - settings = ContextSettings.from_file().assert_context() + context = ContextSettings.from_file().assert_context() except DataSafeHavenConfigError as exc: print("No context configuration file. Run `dsh context add` before creating infrastructure.") raise typer.Exit(code=1) from exc @@ -198,6 +198,10 @@ def create() -> None: @context_command_group.command() def teardown() -> None: """Tear down Data Safe Haven context infrastructure.""" - context = ContextSettings.from_file().assert_context() + try: + context = ContextSettings.from_file().assert_context() + except DataSafeHavenConfigError as exc: + print("No context configuration file. A context must be found before it can be torn down infrastructure.") + raise typer.Exit(code=1) from exc context_infra = ContextInfrastructure(context) context_infra.teardown() From f08ea7290320a95e297e47fa7c2a7cb104ab8fdc Mon Sep 17 00:00:00 2001 From: Matt Craddock <5796417+craddm@users.noreply.github.com> Date: Thu, 23 May 2024 15:29:33 +0000 Subject: [PATCH 03/33] Add error when not logged in to az cli --- data_safe_haven/context_infrastructure/infrastructure.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/data_safe_haven/context_infrastructure/infrastructure.py b/data_safe_haven/context_infrastructure/infrastructure.py index 03710015d0..f6142da581 100644 --- a/data_safe_haven/context_infrastructure/infrastructure.py +++ b/data_safe_haven/context_infrastructure/infrastructure.py @@ -10,6 +10,12 @@ def __init__(self, context: Context) -> None: self.azure_api_: AzureApi | None = None self.context = context self.tags = {"component": "context"} | context.tags + try: + if self.azure_api.subscription_id: + pass + except Exception as exc: + msg = "Not logged in. \n Authenticate using az login before creating or tearing down context infrastructure." + raise DataSafeHavenAzureError(msg) from exc @property def azure_api(self) -> AzureApi: From 32abe7784671d220ce551cd279e7c57e27ffa5a6 Mon Sep 17 00:00:00 2001 From: Matt Craddock <5796417+craddm@users.noreply.github.com> Date: Thu, 23 May 2024 15:35:23 +0000 Subject: [PATCH 04/33] Fixlinting errors --- data_safe_haven/commands/context.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/data_safe_haven/commands/context.py b/data_safe_haven/commands/context.py index 454abb73b3..4218bb586f 100644 --- a/data_safe_haven/commands/context.py +++ b/data_safe_haven/commands/context.py @@ -188,7 +188,9 @@ def create() -> None: try: context = ContextSettings.from_file().assert_context() except DataSafeHavenConfigError as exc: - print("No context configuration file. Run `dsh context add` before creating infrastructure.") + print( + "No context configuration file. Run `dsh context add` before creating infrastructure." + ) raise typer.Exit(code=1) from exc context_infra = ContextInfrastructure(context) @@ -201,7 +203,9 @@ def teardown() -> None: try: context = ContextSettings.from_file().assert_context() except DataSafeHavenConfigError as exc: - print("No context configuration file. A context must be found before it can be torn down infrastructure.") + print( + "No context configuration file. A context must be found before it can be torn down infrastructure." + ) raise typer.Exit(code=1) from exc context_infra = ContextInfrastructure(context) context_infra.teardown() From e242821e493b6618b9c0c8b7e701b611549336df Mon Sep 17 00:00:00 2001 From: Matt Craddock <5796417+craddm@users.noreply.github.com> Date: Tue, 28 May 2024 11:01:58 +0000 Subject: [PATCH 05/33] fix typo --- data_safe_haven/commands/context.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/data_safe_haven/commands/context.py b/data_safe_haven/commands/context.py index 4218bb586f..66a284c446 100644 --- a/data_safe_haven/commands/context.py +++ b/data_safe_haven/commands/context.py @@ -204,7 +204,7 @@ def teardown() -> None: context = ContextSettings.from_file().assert_context() except DataSafeHavenConfigError as exc: print( - "No context configuration file. A context must be found before it can be torn down infrastructure." + "No context configuration file. A context must be configured before it can be torn down." ) raise typer.Exit(code=1) from exc context_infra = ContextInfrastructure(context) From 533512f8d2767f2ad28ce3d7556ee92ae8dff8f9 Mon Sep 17 00:00:00 2001 From: Matt Craddock <5796417+craddm@users.noreply.github.com> Date: Tue, 28 May 2024 11:02:59 +0000 Subject: [PATCH 06/33] Remove unnecessary whitespace --- data_safe_haven/context_infrastructure/infrastructure.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/data_safe_haven/context_infrastructure/infrastructure.py b/data_safe_haven/context_infrastructure/infrastructure.py index f6142da581..88483c83c2 100644 --- a/data_safe_haven/context_infrastructure/infrastructure.py +++ b/data_safe_haven/context_infrastructure/infrastructure.py @@ -14,7 +14,7 @@ def __init__(self, context: Context) -> None: if self.azure_api.subscription_id: pass except Exception as exc: - msg = "Not logged in. \n Authenticate using az login before creating or tearing down context infrastructure." + msg = "Not logged in. \nAuthenticate using az login before creating or tearing down context infrastructure." raise DataSafeHavenAzureError(msg) from exc @property From b9da2b218d50e7665c1e551ed338ba5c3b37942b Mon Sep 17 00:00:00 2001 From: Matt Craddock <5796417+craddm@users.noreply.github.com> Date: Tue, 28 May 2024 11:15:39 +0000 Subject: [PATCH 07/33] Clearer explanation of missing context for teardown --- data_safe_haven/commands/context.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/data_safe_haven/commands/context.py b/data_safe_haven/commands/context.py index 66a284c446..c2908dec7b 100644 --- a/data_safe_haven/commands/context.py +++ b/data_safe_haven/commands/context.py @@ -204,7 +204,7 @@ def teardown() -> None: context = ContextSettings.from_file().assert_context() except DataSafeHavenConfigError as exc: print( - "No context configuration file. A context must be configured before it can be torn down." + "No context configuration file. A context configuration must exist before the context can be torn down." ) raise typer.Exit(code=1) from exc context_infra = ContextInfrastructure(context) From 06ac61b5802d0bfe0c7efc7eaaf0a4856371fa62 Mon Sep 17 00:00:00 2001 From: Matt Craddock <5796417+craddm@users.noreply.github.com> Date: Tue, 28 May 2024 11:16:15 +0000 Subject: [PATCH 08/33] Fuller explanation of error when az login has expired --- data_safe_haven/context_infrastructure/infrastructure.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/data_safe_haven/context_infrastructure/infrastructure.py b/data_safe_haven/context_infrastructure/infrastructure.py index 88483c83c2..511ea43e26 100644 --- a/data_safe_haven/context_infrastructure/infrastructure.py +++ b/data_safe_haven/context_infrastructure/infrastructure.py @@ -14,7 +14,7 @@ def __init__(self, context: Context) -> None: if self.azure_api.subscription_id: pass except Exception as exc: - msg = "Not logged in. \nAuthenticate using az login before creating or tearing down context infrastructure." + msg = "Not logged in or login has expired. \nAuthenticate using az login before creating or tearing down context infrastructure." raise DataSafeHavenAzureError(msg) from exc @property From 0e78a96031c4fb5a3807473953d9a5bafab8e0d0 Mon Sep 17 00:00:00 2001 From: Matt Craddock <5796417+craddm@users.noreply.github.com> Date: Tue, 28 May 2024 11:17:10 +0000 Subject: [PATCH 09/33] Catch attempted teardown when no context infrastructure deployed --- data_safe_haven/external/api/azure_api.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/data_safe_haven/external/api/azure_api.py b/data_safe_haven/external/api/azure_api.py index 87b692fed0..28b0849c25 100644 --- a/data_safe_haven/external/api/azure_api.py +++ b/data_safe_haven/external/api/azure_api.py @@ -845,6 +845,11 @@ def remove_resource_group(self, resource_group_name: str) -> None: self.logger.debug( f"Removing resource group [green]{resource_group_name}[/] if it exists...", ) + if not resource_client.resource_groups.check_existence(resource_group_name): + self.logger.error( + f"Resource group [green]{resource_group_name}[/] does not exist. There is no context infrastructure to teardown.", + ) + return poller = resource_client.resource_groups.begin_delete( resource_group_name, ) From 873d6f1d507d16032bc7810cb8dda75d0b5f89df Mon Sep 17 00:00:00 2001 From: Matt Craddock <5796417+craddm@users.noreply.github.com> Date: Wed, 29 May 2024 14:51:58 +0000 Subject: [PATCH 10/33] Handle errors when no context selected --- data_safe_haven/commands/context.py | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/data_safe_haven/commands/context.py b/data_safe_haven/commands/context.py index c2908dec7b..21ac4f22e4 100644 --- a/data_safe_haven/commands/context.py +++ b/data_safe_haven/commands/context.py @@ -188,9 +188,14 @@ def create() -> None: try: context = ContextSettings.from_file().assert_context() except DataSafeHavenConfigError as exc: - print( - "No context configuration file. Run `dsh context add` before creating infrastructure." - ) + if exc.args[0] == "No context selected": + print( + "No context selected. Use `dsh context switch KEY` to select one." + ) + else: + print( + "No context configuration file. Run `dsh context add` before creating infrastructure." + ) raise typer.Exit(code=1) from exc context_infra = ContextInfrastructure(context) @@ -203,9 +208,14 @@ def teardown() -> None: try: context = ContextSettings.from_file().assert_context() except DataSafeHavenConfigError as exc: - print( - "No context configuration file. A context configuration must exist before the context can be torn down." - ) + if exc.args[0] == "No context selected": + print( + "No context selected. Use `dsh context switch KEY` to select one." + ) + else: + print( + "No context configuration file. Run `dsh context add` before creating infrastructure." + ) raise typer.Exit(code=1) from exc context_infra = ContextInfrastructure(context) context_infra.teardown() From 0d341023bbe6bb0afc40905bde2306ea712bf499 Mon Sep 17 00:00:00 2001 From: Jim Madge Date: Fri, 31 May 2024 10:23:26 +0100 Subject: [PATCH 11/33] Improve resource group removal log messages --- data_safe_haven/commands/context.py | 8 ++------ .../context_infrastructure/infrastructure.py | 5 +++++ data_safe_haven/external/api/azure_api.py | 12 ++++++------ 3 files changed, 13 insertions(+), 12 deletions(-) diff --git a/data_safe_haven/commands/context.py b/data_safe_haven/commands/context.py index 21ac4f22e4..3af9301d89 100644 --- a/data_safe_haven/commands/context.py +++ b/data_safe_haven/commands/context.py @@ -189,9 +189,7 @@ def create() -> None: context = ContextSettings.from_file().assert_context() except DataSafeHavenConfigError as exc: if exc.args[0] == "No context selected": - print( - "No context selected. Use `dsh context switch KEY` to select one." - ) + print("No context selected. Use `dsh context switch KEY` to select one.") else: print( "No context configuration file. Run `dsh context add` before creating infrastructure." @@ -209,9 +207,7 @@ def teardown() -> None: context = ContextSettings.from_file().assert_context() except DataSafeHavenConfigError as exc: if exc.args[0] == "No context selected": - print( - "No context selected. Use `dsh context switch KEY` to select one." - ) + print("No context selected. Use `dsh context switch KEY` to select one.") else: print( "No context configuration file. Run `dsh context add` before creating infrastructure." diff --git a/data_safe_haven/context_infrastructure/infrastructure.py b/data_safe_haven/context_infrastructure/infrastructure.py index 511ea43e26..f58d8c6b2f 100644 --- a/data_safe_haven/context_infrastructure/infrastructure.py +++ b/data_safe_haven/context_infrastructure/infrastructure.py @@ -1,6 +1,7 @@ from data_safe_haven.context import Context from data_safe_haven.exceptions import DataSafeHavenAzureError from data_safe_haven.external import AzureApi +from data_safe_haven.utility import LoggingSingleton class ContextInfrastructure: @@ -94,7 +95,11 @@ def teardown(self) -> None: Raises: DataSafeHavenAzureError if any resources cannot be destroyed """ + logger = LoggingSingleton try: + logger.info( + f"Removing context {self.context.name} resource group {self.context.resource_group_name}" + ) self.azure_api.remove_resource_group(self.context.resource_group_name) except Exception as exc: msg = f"Failed to destroy context resources.\n{exc}" diff --git a/data_safe_haven/external/api/azure_api.py b/data_safe_haven/external/api/azure_api.py index 28b0849c25..a6a9289c1d 100644 --- a/data_safe_haven/external/api/azure_api.py +++ b/data_safe_haven/external/api/azure_api.py @@ -841,15 +841,15 @@ def remove_resource_group(self, resource_group_name: str) -> None: self.credential, self.subscription_id ) - # Ensure that resource group exists - self.logger.debug( - f"Removing resource group [green]{resource_group_name}[/] if it exists...", - ) if not resource_client.resource_groups.check_existence(resource_group_name): - self.logger.error( - f"Resource group [green]{resource_group_name}[/] does not exist. There is no context infrastructure to teardown.", + self.logger.warning( + f"Resource group [green]{resource_group_name}[/] does not exist.", ) return + # Ensure that resource group exists + self.logger.debug( + f"Attempting to remove resource group [green]{resource_group_name}[/]", + ) poller = resource_client.resource_groups.begin_delete( resource_group_name, ) From 1ebe3dc2bb4d8d03e825fac8097b329c8d76ade1 Mon Sep 17 00:00:00 2001 From: Jim Madge Date: Fri, 31 May 2024 10:31:38 +0100 Subject: [PATCH 12/33] Remove from exc from typer.Exit --- data_safe_haven/commands/context.py | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/data_safe_haven/commands/context.py b/data_safe_haven/commands/context.py index 3af9301d89..c3c1ec56ed 100644 --- a/data_safe_haven/commands/context.py +++ b/data_safe_haven/commands/context.py @@ -21,9 +21,9 @@ def show() -> None: """Show information about the selected context.""" try: settings = ContextSettings.from_file() - except DataSafeHavenConfigError as exc: + except DataSafeHavenConfigError: print("No context configuration file. Run `dsh context add` to create one.") - raise typer.Exit(code=1) from exc + raise typer.Exit(code=1) current_context_key = settings.selected current_context = settings.context @@ -41,9 +41,9 @@ def available() -> None: """Show the available contexts.""" try: settings = ContextSettings.from_file() - except DataSafeHavenConfigError as exc: + except DataSafeHavenConfigError: print("No context configuration file. Run `dsh context add` to create one.") - raise typer.Exit(code=1) from exc + raise typer.Exit(code=1) current_context_key = settings.selected available = settings.available @@ -62,9 +62,9 @@ def switch( """Switch the selected context.""" try: settings = ContextSettings.from_file() - except DataSafeHavenConfigError as exc: + except DataSafeHavenConfigError: print("No context configuration file. Run `dsh context add` to create one.") - raise typer.Exit(code=1) from exc + raise typer.Exit(code=1) settings.selected = key settings.write() @@ -155,9 +155,9 @@ def update( """Update the selected context settings.""" try: settings = ContextSettings.from_file() - except DataSafeHavenConfigError as exc: + except DataSafeHavenConfigError: print("No context configuration file. Run `dsh context add` to create one.") - raise typer.Exit(code=1) from exc + raise typer.Exit(code=1) settings.update( admin_group_id=admin_group, @@ -175,9 +175,9 @@ def remove( """Removes a context.""" try: settings = ContextSettings.from_file() - except DataSafeHavenConfigError as exc: + except DataSafeHavenConfigError: print("No context configuration file, so no contexts to remove.") - raise typer.Exit(code=1) from exc + raise typer.Exit(code=1) settings.remove(key) settings.write() @@ -194,7 +194,7 @@ def create() -> None: print( "No context configuration file. Run `dsh context add` before creating infrastructure." ) - raise typer.Exit(code=1) from exc + raise typer.Exit(code=1) context_infra = ContextInfrastructure(context) context_infra.create() @@ -212,6 +212,6 @@ def teardown() -> None: print( "No context configuration file. Run `dsh context add` before creating infrastructure." ) - raise typer.Exit(code=1) from exc + raise typer.Exit(code=1) context_infra = ContextInfrastructure(context) context_infra.teardown() From 3e076e7be7bd408b310fc29481daf286f7dbd424 Mon Sep 17 00:00:00 2001 From: Jim Madge Date: Fri, 31 May 2024 11:17:25 +0100 Subject: [PATCH 13/33] Make messages more consistent --- data_safe_haven/commands/context.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/data_safe_haven/commands/context.py b/data_safe_haven/commands/context.py index c3c1ec56ed..88862daa9a 100644 --- a/data_safe_haven/commands/context.py +++ b/data_safe_haven/commands/context.py @@ -189,7 +189,7 @@ def create() -> None: context = ContextSettings.from_file().assert_context() except DataSafeHavenConfigError as exc: if exc.args[0] == "No context selected": - print("No context selected. Use `dsh context switch KEY` to select one.") + print("No context selected. Run `dsh context switch` to select one.") else: print( "No context configuration file. Run `dsh context add` before creating infrastructure." @@ -207,7 +207,7 @@ def teardown() -> None: context = ContextSettings.from_file().assert_context() except DataSafeHavenConfigError as exc: if exc.args[0] == "No context selected": - print("No context selected. Use `dsh context switch KEY` to select one.") + print("No context selected. Run `dsh context switch` to select one.") else: print( "No context configuration file. Run `dsh context add` before creating infrastructure." From d9b91fedc23c098b48f33124f2e8087c31d37836 Mon Sep 17 00:00:00 2001 From: Jim Madge Date: Fri, 31 May 2024 11:34:21 +0100 Subject: [PATCH 14/33] Catch exception if Azure API auth fails --- data_safe_haven/commands/context.py | 14 +++++++++++--- .../context_infrastructure/infrastructure.py | 6 ------ data_safe_haven/exceptions/__init__.py | 8 ++++++++ .../external/interface/azure_authenticator.py | 5 +++-- 4 files changed, 22 insertions(+), 11 deletions(-) diff --git a/data_safe_haven/commands/context.py b/data_safe_haven/commands/context.py index 88862daa9a..65b93240e5 100644 --- a/data_safe_haven/commands/context.py +++ b/data_safe_haven/commands/context.py @@ -11,7 +11,7 @@ ContextSettings, ) from data_safe_haven.context_infrastructure import ContextInfrastructure -from data_safe_haven.exceptions import DataSafeHavenConfigError +from data_safe_haven.exceptions import DataSafeHavenAzureAPIAuthenticationError, DataSafeHavenConfigError context_command_group = typer.Typer() @@ -197,7 +197,11 @@ def create() -> None: raise typer.Exit(code=1) context_infra = ContextInfrastructure(context) - context_infra.create() + try: + context_infra.create() + except DataSafeHavenAzureAPIAuthenticationError: + print("Failed to authenticate with the Azure API. You may not be logged into the Azure CLI, or your login may have expired. Try running `az login`.") + raise typer.Exit(1) @context_command_group.command() @@ -214,4 +218,8 @@ def teardown() -> None: ) raise typer.Exit(code=1) context_infra = ContextInfrastructure(context) - context_infra.teardown() + try: + context_infra.teardown() + except DataSafeHavenAzureAPIAuthenticationError: + print("Failed to authenticate with the Azure API. You may not be logged into the Azure CLI, or your login may have expired. Try running `az login`.") + raise typer.Exit(1) diff --git a/data_safe_haven/context_infrastructure/infrastructure.py b/data_safe_haven/context_infrastructure/infrastructure.py index f58d8c6b2f..d427314e0e 100644 --- a/data_safe_haven/context_infrastructure/infrastructure.py +++ b/data_safe_haven/context_infrastructure/infrastructure.py @@ -11,12 +11,6 @@ def __init__(self, context: Context) -> None: self.azure_api_: AzureApi | None = None self.context = context self.tags = {"component": "context"} | context.tags - try: - if self.azure_api.subscription_id: - pass - except Exception as exc: - msg = "Not logged in or login has expired. \nAuthenticate using az login before creating or tearing down context infrastructure." - raise DataSafeHavenAzureError(msg) from exc @property def azure_api(self) -> AzureApi: diff --git a/data_safe_haven/exceptions/__init__.py b/data_safe_haven/exceptions/__init__.py index 3ef0bbcac8..1adb40a67e 100644 --- a/data_safe_haven/exceptions/__init__.py +++ b/data_safe_haven/exceptions/__init__.py @@ -42,6 +42,14 @@ class DataSafeHavenAzureError(DataSafeHavenCloudError): pass +class DataSafeHavenAzureAPIError(DataSafeHavenAzureError): + pass + + +class DataSafeHavenAzureAPIAuthenticationError(DataSafeHavenAzureAPIError): + pass + + class DataSafeHavenUserHandlingError(DataSafeHavenInternalError): pass diff --git a/data_safe_haven/external/interface/azure_authenticator.py b/data_safe_haven/external/interface/azure_authenticator.py index 838833cb69..a5b7bda458 100644 --- a/data_safe_haven/external/interface/azure_authenticator.py +++ b/data_safe_haven/external/interface/azure_authenticator.py @@ -9,6 +9,7 @@ from data_safe_haven.exceptions import ( DataSafeHavenAzureError, + DataSafeHavenAzureAPIAuthenticationError, DataSafeHavenInputError, ) @@ -63,8 +64,8 @@ def login(self) -> None: self.tenant_id_ = subscription.tenant_id break except ClientAuthenticationError as exc: - msg = f"Failed to authenticate with Azure.\n{exc}" - raise DataSafeHavenAzureError(msg) from exc + msg = f"Failed to authenticate with Azure API.\n{exc}" + raise DataSafeHavenAzureAPIAuthenticationError(msg) from exc if not (self.subscription_id and self.tenant_id): msg = f"Could not find subscription '{self.subscription_name}'" raise DataSafeHavenInputError(msg) From f65f45bd3b3a86320cf533d91553ab3f2a792847 Mon Sep 17 00:00:00 2001 From: Jim Madge Date: Fri, 31 May 2024 11:39:17 +0100 Subject: [PATCH 15/33] Fix linting --- data_safe_haven/commands/context.py | 31 ++++++++++++------- .../external/interface/azure_authenticator.py | 2 +- 2 files changed, 20 insertions(+), 13 deletions(-) diff --git a/data_safe_haven/commands/context.py b/data_safe_haven/commands/context.py index 65b93240e5..990f7e3d4c 100644 --- a/data_safe_haven/commands/context.py +++ b/data_safe_haven/commands/context.py @@ -11,7 +11,10 @@ ContextSettings, ) from data_safe_haven.context_infrastructure import ContextInfrastructure -from data_safe_haven.exceptions import DataSafeHavenAzureAPIAuthenticationError, DataSafeHavenConfigError +from data_safe_haven.exceptions import ( + DataSafeHavenAzureAPIAuthenticationError, + DataSafeHavenConfigError, +) context_command_group = typer.Typer() @@ -23,7 +26,7 @@ def show() -> None: settings = ContextSettings.from_file() except DataSafeHavenConfigError: print("No context configuration file. Run `dsh context add` to create one.") - raise typer.Exit(code=1) + raise typer.Exit(code=1) from None current_context_key = settings.selected current_context = settings.context @@ -43,7 +46,7 @@ def available() -> None: settings = ContextSettings.from_file() except DataSafeHavenConfigError: print("No context configuration file. Run `dsh context add` to create one.") - raise typer.Exit(code=1) + raise typer.Exit(code=1) from None current_context_key = settings.selected available = settings.available @@ -64,7 +67,7 @@ def switch( settings = ContextSettings.from_file() except DataSafeHavenConfigError: print("No context configuration file. Run `dsh context add` to create one.") - raise typer.Exit(code=1) + raise typer.Exit(code=1) from None settings.selected = key settings.write() @@ -157,7 +160,7 @@ def update( settings = ContextSettings.from_file() except DataSafeHavenConfigError: print("No context configuration file. Run `dsh context add` to create one.") - raise typer.Exit(code=1) + raise typer.Exit(code=1) from None settings.update( admin_group_id=admin_group, @@ -177,7 +180,7 @@ def remove( settings = ContextSettings.from_file() except DataSafeHavenConfigError: print("No context configuration file, so no contexts to remove.") - raise typer.Exit(code=1) + raise typer.Exit(code=1) from None settings.remove(key) settings.write() @@ -194,14 +197,16 @@ def create() -> None: print( "No context configuration file. Run `dsh context add` before creating infrastructure." ) - raise typer.Exit(code=1) + raise typer.Exit(code=1) from None context_infra = ContextInfrastructure(context) try: context_infra.create() except DataSafeHavenAzureAPIAuthenticationError: - print("Failed to authenticate with the Azure API. You may not be logged into the Azure CLI, or your login may have expired. Try running `az login`.") - raise typer.Exit(1) + print( + "Failed to authenticate with the Azure API. You may not be logged into the Azure CLI, or your login may have expired. Try running `az login`." + ) + raise typer.Exit(1) from None @context_command_group.command() @@ -216,10 +221,12 @@ def teardown() -> None: print( "No context configuration file. Run `dsh context add` before creating infrastructure." ) - raise typer.Exit(code=1) + raise typer.Exit(code=1) from None context_infra = ContextInfrastructure(context) try: context_infra.teardown() except DataSafeHavenAzureAPIAuthenticationError: - print("Failed to authenticate with the Azure API. You may not be logged into the Azure CLI, or your login may have expired. Try running `az login`.") - raise typer.Exit(1) + print( + "Failed to authenticate with the Azure API. You may not be logged into the Azure CLI, or your login may have expired. Try running `az login`." + ) + raise typer.Exit(1) from None diff --git a/data_safe_haven/external/interface/azure_authenticator.py b/data_safe_haven/external/interface/azure_authenticator.py index a5b7bda458..4d7a4641b0 100644 --- a/data_safe_haven/external/interface/azure_authenticator.py +++ b/data_safe_haven/external/interface/azure_authenticator.py @@ -8,8 +8,8 @@ from azure.mgmt.resource.subscriptions.models import Subscription from data_safe_haven.exceptions import ( - DataSafeHavenAzureError, DataSafeHavenAzureAPIAuthenticationError, + DataSafeHavenAzureError, DataSafeHavenInputError, ) From 0938502878d584cb6c89bd02e457d32756f8a7b0 Mon Sep 17 00:00:00 2001 From: Jim Madge Date: Fri, 31 May 2024 11:41:18 +0100 Subject: [PATCH 16/33] Correct logger construction --- data_safe_haven/context_infrastructure/infrastructure.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/data_safe_haven/context_infrastructure/infrastructure.py b/data_safe_haven/context_infrastructure/infrastructure.py index d427314e0e..d4c22d73a7 100644 --- a/data_safe_haven/context_infrastructure/infrastructure.py +++ b/data_safe_haven/context_infrastructure/infrastructure.py @@ -89,7 +89,7 @@ def teardown(self) -> None: Raises: DataSafeHavenAzureError if any resources cannot be destroyed """ - logger = LoggingSingleton + logger = LoggingSingleton() try: logger.info( f"Removing context {self.context.name} resource group {self.context.resource_group_name}" From e1e41ff919e874b4a6c49277e033c037151a1ae2 Mon Sep 17 00:00:00 2001 From: Jim Madge Date: Fri, 31 May 2024 14:00:18 +0100 Subject: [PATCH 17/33] Add tests for updated context commands --- data_safe_haven/commands/context.py | 2 +- tests/commands/test_context.py | 40 +++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/data_safe_haven/commands/context.py b/data_safe_haven/commands/context.py index 990f7e3d4c..d9df969943 100644 --- a/data_safe_haven/commands/context.py +++ b/data_safe_haven/commands/context.py @@ -179,7 +179,7 @@ def remove( try: settings = ContextSettings.from_file() except DataSafeHavenConfigError: - print("No context configuration file, so no contexts to remove.") + print("No context configuration file.") raise typer.Exit(code=1) from None settings.remove(key) settings.write() diff --git a/tests/commands/test_context.py b/tests/commands/test_context.py index d1359e1c3d..f64e3a4db6 100644 --- a/tests/commands/test_context.py +++ b/tests/commands/test_context.py @@ -33,6 +33,11 @@ def test_available_none(self, runner_none): assert "acme_deployment" in result.stdout assert "gems" in result.stdout + def test_no_context_file(self, runner_no_context_file): + result = runner_no_context_file.invoke(context_command_group, ["available"]) + assert result.exit_code == 1 + assert "No context configuration file." in result.stdout + class TestSwitch: def test_switch(self, runner): @@ -49,6 +54,11 @@ def test_invalid_switch(self, runner): # Unable to check error as this is written outside of any Typer # assert "Context 'invalid' is not defined " in result.stdout + def test_no_context_file(self, runner_no_context_file): + result = runner_no_context_file.invoke(context_command_group, ["switch", "context"]) + assert result.exit_code == 1 + assert "No context configuration file." in result.stdout + class TestAdd: def test_add(self, runner): @@ -160,6 +170,11 @@ def test_update(self, runner): assert result.exit_code == 0 assert "Name: New Name" in result.stdout + def test_no_context_file(self, runner_no_context_file): + result = runner_no_context_file.invoke(context_command_group, ["update", "--name", "New Name"]) + assert result.exit_code == 1 + assert "No context configuration file." in result.stdout + class TestRemove: def test_remove(self, runner): @@ -175,6 +190,11 @@ def test_remove_invalid(self, runner): # Unable to check error as this is written outside of any Typer # assert "No context with key 'invalid'." in result.stdout + def test_no_context_file(self, runner_no_context_file): + result = runner_no_context_file.invoke(context_command_group, ["remove", "gems"]) + assert result.exit_code == 1 + assert "No context configuration file." in result.stdout + class TestCreate: def test_create(self, runner, monkeypatch): @@ -187,6 +207,16 @@ def mock_create(self): # noqa: ARG001 assert "mock create" in result.stdout assert result.exit_code == 0 + def test_no_context_file(self, runner_no_context_file): + result = runner_no_context_file.invoke(context_command_group, ["create"]) + assert result.exit_code == 1 + assert "No context configuration file." in result.stdout + + def test_show_none(self, runner_none): + result = runner_none.invoke(context_command_group, ["create"]) + assert result.exit_code == 1 + assert "No context selected." in result.stdout + class TestTeardown: def test_teardown(self, runner, monkeypatch): @@ -198,3 +228,13 @@ def mock_teardown(self): # noqa: ARG001 result = runner.invoke(context_command_group, ["teardown"]) assert "mock teardown" in result.stdout assert result.exit_code == 0 + + def test_no_context_file(self, runner_no_context_file): + result = runner_no_context_file.invoke(context_command_group, ["teardown"]) + assert result.exit_code == 1 + assert "No context configuration file." in result.stdout + + def test_show_none(self, runner_none): + result = runner_none.invoke(context_command_group, ["teardown"]) + assert result.exit_code == 1 + assert "No context selected." in result.stdout From d228e5f36f39092dd7cb2477fdd25992084da855 Mon Sep 17 00:00:00 2001 From: Jim Madge Date: Fri, 31 May 2024 14:03:13 +0100 Subject: [PATCH 18/33] Run lint:fmt --- tests/commands/test_context.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/tests/commands/test_context.py b/tests/commands/test_context.py index f64e3a4db6..0a0408b7b2 100644 --- a/tests/commands/test_context.py +++ b/tests/commands/test_context.py @@ -55,7 +55,9 @@ def test_invalid_switch(self, runner): # assert "Context 'invalid' is not defined " in result.stdout def test_no_context_file(self, runner_no_context_file): - result = runner_no_context_file.invoke(context_command_group, ["switch", "context"]) + result = runner_no_context_file.invoke( + context_command_group, ["switch", "context"] + ) assert result.exit_code == 1 assert "No context configuration file." in result.stdout @@ -171,7 +173,9 @@ def test_update(self, runner): assert "Name: New Name" in result.stdout def test_no_context_file(self, runner_no_context_file): - result = runner_no_context_file.invoke(context_command_group, ["update", "--name", "New Name"]) + result = runner_no_context_file.invoke( + context_command_group, ["update", "--name", "New Name"] + ) assert result.exit_code == 1 assert "No context configuration file." in result.stdout @@ -191,7 +195,9 @@ def test_remove_invalid(self, runner): # assert "No context with key 'invalid'." in result.stdout def test_no_context_file(self, runner_no_context_file): - result = runner_no_context_file.invoke(context_command_group, ["remove", "gems"]) + result = runner_no_context_file.invoke( + context_command_group, ["remove", "gems"] + ) assert result.exit_code == 1 assert "No context configuration file." in result.stdout From 528aab9cafd1d72cfcea223c5a7620e9ad966baa Mon Sep 17 00:00:00 2001 From: Jim Madge Date: Fri, 31 May 2024 14:44:27 +0100 Subject: [PATCH 19/33] Restructure exceptions so auth failure can be caught --- data_safe_haven/commands/context.py | 2 + .../context_infrastructure/infrastructure.py | 4 +- data_safe_haven/exceptions/__init__.py | 2 +- data_safe_haven/external/api/azure_api.py | 44 +++++++++---------- tests/commands/test_context.py | 22 ++++++++++ 5 files changed, 49 insertions(+), 25 deletions(-) diff --git a/data_safe_haven/commands/context.py b/data_safe_haven/commands/context.py index d9df969943..971c4f9408 100644 --- a/data_safe_haven/commands/context.py +++ b/data_safe_haven/commands/context.py @@ -222,7 +222,9 @@ def teardown() -> None: "No context configuration file. Run `dsh context add` before creating infrastructure." ) raise typer.Exit(code=1) from None + context_infra = ContextInfrastructure(context) + try: context_infra.teardown() except DataSafeHavenAzureAPIAuthenticationError: diff --git a/data_safe_haven/context_infrastructure/infrastructure.py b/data_safe_haven/context_infrastructure/infrastructure.py index d4c22d73a7..b1498170b2 100644 --- a/data_safe_haven/context_infrastructure/infrastructure.py +++ b/data_safe_haven/context_infrastructure/infrastructure.py @@ -79,7 +79,7 @@ def create(self) -> None: key_name=self.context.pulumi_encryption_key_name, key_vault_name=keyvault.name, ) - except Exception as exc: + except DataSafeHavenAzureError as exc: msg = f"Failed to create context resources.\n{exc}" raise DataSafeHavenAzureError(msg) from exc @@ -95,6 +95,6 @@ def teardown(self) -> None: f"Removing context {self.context.name} resource group {self.context.resource_group_name}" ) self.azure_api.remove_resource_group(self.context.resource_group_name) - except Exception as exc: + except DataSafeHavenAzureError as exc: msg = f"Failed to destroy context resources.\n{exc}" raise DataSafeHavenAzureError(msg) from exc diff --git a/data_safe_haven/exceptions/__init__.py b/data_safe_haven/exceptions/__init__.py index 1adb40a67e..1064ad63df 100644 --- a/data_safe_haven/exceptions/__init__.py +++ b/data_safe_haven/exceptions/__init__.py @@ -42,7 +42,7 @@ class DataSafeHavenAzureError(DataSafeHavenCloudError): pass -class DataSafeHavenAzureAPIError(DataSafeHavenAzureError): +class DataSafeHavenAzureAPIError(DataSafeHavenError): pass diff --git a/data_safe_haven/external/api/azure_api.py b/data_safe_haven/external/api/azure_api.py index a6a9289c1d..0467be3e61 100644 --- a/data_safe_haven/external/api/azure_api.py +++ b/data_safe_haven/external/api/azure_api.py @@ -139,7 +139,7 @@ def download_blob( ) # Download the requested file return str(blob_client.download_blob(encoding="utf-8").readall()) - except Exception as exc: + except DataSafeHavenAzureError as exc: msg = f"Blob file '{blob_name}' could not be downloaded from '{storage_account_name}'\n{exc}." raise DataSafeHavenAzureError(msg) from exc @@ -179,7 +179,7 @@ def ensure_dns_txt_record( f"Ensured that DNS record {record_name} exists in zone {zone_name}.", ) return record_set - except Exception as exc: + except DataSafeHavenAzureError as exc: msg = ( f"Failed to create DNS record {record_name} in zone {zone_name}.\n{exc}" ) @@ -258,7 +258,7 @@ def ensure_keyvault( f"Ensured that key vault [green]{key_vaults[0].name}[/] exists.", ) return key_vaults[0] - except Exception as exc: + except DataSafeHavenAzureError as exc: msg = f"Failed to create key vault {key_vault_name}.\n{exc}" raise DataSafeHavenAzureError(msg) from exc @@ -295,7 +295,7 @@ def ensure_keyvault_key( f"Ensured that key [green]{key_name}[/] exists.", ) return key - except Exception as exc: + except DataSafeHavenAzureError as exc: msg = f"Failed to create key {key_name}.\n{exc}" raise DataSafeHavenAzureError(msg) from exc @@ -329,7 +329,7 @@ def ensure_managed_identity( f"Ensured that managed identity [green]{identity_name}[/] exists.", ) return managed_identity - except Exception as exc: + except DataSafeHavenAzureError as exc: msg = f"Failed to create managed identity {identity_name}.\n{exc}" raise DataSafeHavenAzureError(msg) from exc @@ -371,7 +371,7 @@ def ensure_resource_group( f" in [green]{resource_groups[0].location}[/].", ) return resource_groups[0] - except Exception as exc: + except DataSafeHavenAzureError as exc: msg = f"Failed to create resource group {resource_group_name}.\n{exc}" raise DataSafeHavenAzureError(msg) from exc @@ -413,7 +413,7 @@ def ensure_storage_account( f"Ensured that storage account [green]{storage_account.name}[/] exists.", ) return storage_account - except Exception as exc: + except DataSafeHavenAzureError as exc: msg = f"Failed to create storage account {storage_account_name}.\n{exc}" raise DataSafeHavenAzureError(msg) from exc @@ -471,7 +471,7 @@ def get_keyvault_certificate( # Ensure that certificate exists try: return certificate_client.get_certificate(certificate_name) - except Exception as exc: + except DataSafeHavenAzureError as exc: msg = f"Failed to retrieve certificate {certificate_name}.\n{exc}" raise DataSafeHavenAzureError(msg) from exc @@ -492,7 +492,7 @@ def get_keyvault_key(self, key_name: str, key_vault_name: str) -> KeyVaultKey: # Ensure that certificate exists try: return key_client.get_key(key_name) - except Exception as exc: + except DataSafeHavenAzureError as exc: msg = f"Failed to retrieve key {key_name}.\n{exc}" raise DataSafeHavenAzureError(msg) from exc @@ -516,7 +516,7 @@ def get_keyvault_secret(self, key_vault_name: str, secret_name: str) -> str: return str(secret.value) msg = f"Secret {secret_name} has no value." raise DataSafeHavenAzureError(msg) - except Exception as exc: + except DataSafeHavenAzureError as exc: msg = f"Failed to retrieve secret {secret_name}.\n{exc}" raise DataSafeHavenAzureError(msg) from exc @@ -537,7 +537,7 @@ def get_locations(self) -> list[str]: ), ) ] - except Exception as exc: + except DataSafeHavenAzureError as exc: msg = f"Azure locations could not be loaded.\n{exc}" raise DataSafeHavenAzureError(msg) from exc @@ -577,7 +577,7 @@ def get_storage_account_keys( msg = f"No keys were retrieved for {msg_sa} in {msg_rg}." raise DataSafeHavenAzureError(msg) return keys - except Exception as exc: + except DataSafeHavenAzureError as exc: msg = f"Keys could not be loaded for {msg_sa} in {msg_rg}.\n{exc}" raise DataSafeHavenAzureError(msg) from exc @@ -621,7 +621,7 @@ def import_keyvault_certificate( f"Imported certificate [green]{certificate_name}[/].", ) return certificate - except Exception as exc: + except DataSafeHavenAzureError as exc: msg = f"Failed to import certificate '{certificate_name}'.\n{exc}" raise DataSafeHavenAzureError(msg) from exc @@ -649,7 +649,7 @@ def list_available_vm_skus(self, location: str) -> dict[str, dict[str, Any]]: ): skus[resource_sku.name][capability.name] = capability.value return skus - except Exception as exc: + except DataSafeHavenAzureError as exc: msg = f"Failed to load available VM sizes for Azure location {location}.\n{exc}" raise DataSafeHavenAzureError(msg) from exc @@ -686,7 +686,7 @@ def purge_keyvault_certificate( self.logger.info( f"Purged certificate [green]{certificate_name}[/] from Key Vault [green]{key_vault_name}[/].", ) - except Exception as exc: + except DataSafeHavenAzureError as exc: msg = f"Failed to remove certificate '{certificate_name}' from Key Vault '{key_vault_name}'.\n{exc}" raise DataSafeHavenAzureError(msg) from exc @@ -724,7 +724,7 @@ def remove_blob( self.logger.info( f"Removed file [green]{blob_name}[/] from blob storage.", ) - except Exception as exc: + except DataSafeHavenAzureError as exc: msg = f"Blob file [green]'{blob_name}'[/] could not be removed from [green]'{storage_account_name}'[/].\n{exc}" raise DataSafeHavenAzureError(msg) from exc @@ -768,7 +768,7 @@ def remove_dns_txt_record( self.logger.info( f"Ensured that DNS record [green]{record_name}[/] is removed from zone [green]{zone_name}[/].", ) - except Exception as exc: + except DataSafeHavenAzureError as exc: msg = f"Failed to remove DNS record [green]{record_name}[/] from zone [green]{zone_name}[/].\n{exc}" raise DataSafeHavenAzureError(msg) from exc @@ -825,7 +825,7 @@ def remove_keyvault_certificate( self.logger.info( f"Removed certificate [green]{certificate_name}[/] from Key Vault [green]{key_vault_name}[/].", ) - except Exception as exc: + except DataSafeHavenAzureError as exc: msg = f"Failed to remove certificate '{certificate_name}' from Key Vault '{key_vault_name}'.\n{exc}" raise DataSafeHavenAzureError(msg) from exc @@ -869,7 +869,7 @@ def remove_resource_group(self, resource_group_name: str) -> None: self.logger.info( f"Ensured that resource group [green]{resource_group_name}[/] does not exist.", ) - except Exception as exc: + except DataSafeHavenAzureError as exc: msg = f"Failed to remove resource group {resource_group_name}.\n{exc}" raise DataSafeHavenAzureError(msg) from exc @@ -921,7 +921,7 @@ def run_remote_script( result = cast(RunCommandResult, poller.result()) # Return any stdout/stderr from the command return str(result.value[0].message) if result.value else "" - except Exception as exc: + except DataSafeHavenAzureError as exc: msg = f"Failed to run command on '{vm_name}'.\n{exc}" raise DataSafeHavenAzureError(msg) from exc @@ -1002,7 +1002,7 @@ def set_blob_container_acl( directory_client = file_system_client._get_root_directory_client() # Set the desired ACL directory_client.set_access_control_recursive(acl=desired_acl) - except Exception as exc: + except DataSafeHavenAzureError as exc: msg = f"Failed to set ACL '{desired_acl}' on container '{container_name}'.\n{exc}" raise DataSafeHavenAzureError(msg) from exc @@ -1034,6 +1034,6 @@ def upload_blob( self.logger.info( f"Uploaded file [green]{blob_name}[/] to blob storage.", ) - except Exception as exc: + except DataSafeHavenAzureError as exc: msg = f"Blob file '{blob_name}' could not be uploaded to '{storage_account_name}'\n{exc}." raise DataSafeHavenAzureError(msg) from exc diff --git a/tests/commands/test_context.py b/tests/commands/test_context.py index 0a0408b7b2..710f37054e 100644 --- a/tests/commands/test_context.py +++ b/tests/commands/test_context.py @@ -1,5 +1,7 @@ from data_safe_haven.commands.context import context_command_group from data_safe_haven.context_infrastructure import ContextInfrastructure +from data_safe_haven.exceptions import DataSafeHavenAzureAPIAuthenticationError +from data_safe_haven.external.interface.azure_authenticator import AzureAuthenticator class TestShow: @@ -223,6 +225,16 @@ def test_show_none(self, runner_none): assert result.exit_code == 1 assert "No context selected." in result.stdout + def test_auth_failure(self, runner, mocker): + def mock_login(self): # noqa: ARG001 + raise DataSafeHavenAzureAPIAuthenticationError + + mocker.patch.object(AzureAuthenticator, "login", mock_login) + + result = runner.invoke(context_command_group, ["create"]) + assert result.exit_code == 1 + assert "Failed to authenticate with the Azure API." in result.stdout + class TestTeardown: def test_teardown(self, runner, monkeypatch): @@ -244,3 +256,13 @@ def test_show_none(self, runner_none): result = runner_none.invoke(context_command_group, ["teardown"]) assert result.exit_code == 1 assert "No context selected." in result.stdout + + def test_auth_failure(self, runner, mocker): + def mock_login(self): # noqa: ARG001 + raise DataSafeHavenAzureAPIAuthenticationError + + mocker.patch.object(AzureAuthenticator, "login", mock_login) + + result = runner.invoke(context_command_group, ["teardown"]) + assert result.exit_code == 1 + assert "Failed to authenticate with the Azure API." in result.stdout From 82c9b42b1d9858529a34be181f0abb7462680cca Mon Sep 17 00:00:00 2001 From: Jim Madge Date: Fri, 31 May 2024 14:56:16 +0100 Subject: [PATCH 20/33] Fix KeyClient exceptions --- data_safe_haven/external/api/azure_api.py | 2 +- tests/external/api/test_azure_api.py | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/data_safe_haven/external/api/azure_api.py b/data_safe_haven/external/api/azure_api.py index 0467be3e61..3036810f00 100644 --- a/data_safe_haven/external/api/azure_api.py +++ b/data_safe_haven/external/api/azure_api.py @@ -492,7 +492,7 @@ def get_keyvault_key(self, key_name: str, key_vault_name: str) -> KeyVaultKey: # Ensure that certificate exists try: return key_client.get_key(key_name) - except DataSafeHavenAzureError as exc: + except (ResourceNotFoundError, HttpResponseError) as exc: msg = f"Failed to retrieve key {key_name}.\n{exc}" raise DataSafeHavenAzureError(msg) from exc diff --git a/tests/external/api/test_azure_api.py b/tests/external/api/test_azure_api.py index 49e1571019..20e435d1d9 100644 --- a/tests/external/api/test_azure_api.py +++ b/tests/external/api/test_azure_api.py @@ -1,4 +1,5 @@ import pytest +from azure.core.exceptions import ResourceNotFoundError from pytest import fixture import data_safe_haven.external.api.azure_api @@ -17,7 +18,7 @@ def get_key(self, key_name): if key_name == "exists": return f"key: {key_name}" else: - raise Exception + raise ResourceNotFoundError monkeypatch.setattr( data_safe_haven.external.api.azure_api, "KeyClient", MockKeyClient From 14eb66c7d591503bfc6553dd9b36ee02b3224a60 Mon Sep 17 00:00:00 2001 From: Jim Madge Date: Fri, 31 May 2024 15:01:41 +0100 Subject: [PATCH 21/33] Use Azure SDK base exception where appropriate --- data_safe_haven/external/api/azure_api.py | 45 ++++++++++++----------- 1 file changed, 23 insertions(+), 22 deletions(-) diff --git a/data_safe_haven/external/api/azure_api.py b/data_safe_haven/external/api/azure_api.py index 3036810f00..7078e67832 100644 --- a/data_safe_haven/external/api/azure_api.py +++ b/data_safe_haven/external/api/azure_api.py @@ -5,6 +5,7 @@ from typing import Any, cast from azure.core.exceptions import ( + AzureError, HttpResponseError, ResourceExistsError, ResourceNotFoundError, @@ -139,7 +140,7 @@ def download_blob( ) # Download the requested file return str(blob_client.download_blob(encoding="utf-8").readall()) - except DataSafeHavenAzureError as exc: + except AzureError as exc: msg = f"Blob file '{blob_name}' could not be downloaded from '{storage_account_name}'\n{exc}." raise DataSafeHavenAzureError(msg) from exc @@ -179,7 +180,7 @@ def ensure_dns_txt_record( f"Ensured that DNS record {record_name} exists in zone {zone_name}.", ) return record_set - except DataSafeHavenAzureError as exc: + except AzureError as exc: msg = ( f"Failed to create DNS record {record_name} in zone {zone_name}.\n{exc}" ) @@ -258,7 +259,7 @@ def ensure_keyvault( f"Ensured that key vault [green]{key_vaults[0].name}[/] exists.", ) return key_vaults[0] - except DataSafeHavenAzureError as exc: + except AzureError as exc: msg = f"Failed to create key vault {key_vault_name}.\n{exc}" raise DataSafeHavenAzureError(msg) from exc @@ -295,7 +296,7 @@ def ensure_keyvault_key( f"Ensured that key [green]{key_name}[/] exists.", ) return key - except DataSafeHavenAzureError as exc: + except AzureError as exc: msg = f"Failed to create key {key_name}.\n{exc}" raise DataSafeHavenAzureError(msg) from exc @@ -329,7 +330,7 @@ def ensure_managed_identity( f"Ensured that managed identity [green]{identity_name}[/] exists.", ) return managed_identity - except DataSafeHavenAzureError as exc: + except AzureError as exc: msg = f"Failed to create managed identity {identity_name}.\n{exc}" raise DataSafeHavenAzureError(msg) from exc @@ -371,7 +372,7 @@ def ensure_resource_group( f" in [green]{resource_groups[0].location}[/].", ) return resource_groups[0] - except DataSafeHavenAzureError as exc: + except AzureError as exc: msg = f"Failed to create resource group {resource_group_name}.\n{exc}" raise DataSafeHavenAzureError(msg) from exc @@ -413,7 +414,7 @@ def ensure_storage_account( f"Ensured that storage account [green]{storage_account.name}[/] exists.", ) return storage_account - except DataSafeHavenAzureError as exc: + except AzureError as exc: msg = f"Failed to create storage account {storage_account_name}.\n{exc}" raise DataSafeHavenAzureError(msg) from exc @@ -471,7 +472,7 @@ def get_keyvault_certificate( # Ensure that certificate exists try: return certificate_client.get_certificate(certificate_name) - except DataSafeHavenAzureError as exc: + except AzureError as exc: msg = f"Failed to retrieve certificate {certificate_name}.\n{exc}" raise DataSafeHavenAzureError(msg) from exc @@ -516,7 +517,7 @@ def get_keyvault_secret(self, key_vault_name: str, secret_name: str) -> str: return str(secret.value) msg = f"Secret {secret_name} has no value." raise DataSafeHavenAzureError(msg) - except DataSafeHavenAzureError as exc: + except AzureError as exc: msg = f"Failed to retrieve secret {secret_name}.\n{exc}" raise DataSafeHavenAzureError(msg) from exc @@ -537,7 +538,7 @@ def get_locations(self) -> list[str]: ), ) ] - except DataSafeHavenAzureError as exc: + except AzureError as exc: msg = f"Azure locations could not be loaded.\n{exc}" raise DataSafeHavenAzureError(msg) from exc @@ -577,7 +578,7 @@ def get_storage_account_keys( msg = f"No keys were retrieved for {msg_sa} in {msg_rg}." raise DataSafeHavenAzureError(msg) return keys - except DataSafeHavenAzureError as exc: + except AzureError as exc: msg = f"Keys could not be loaded for {msg_sa} in {msg_rg}.\n{exc}" raise DataSafeHavenAzureError(msg) from exc @@ -621,7 +622,7 @@ def import_keyvault_certificate( f"Imported certificate [green]{certificate_name}[/].", ) return certificate - except DataSafeHavenAzureError as exc: + except AzureError as exc: msg = f"Failed to import certificate '{certificate_name}'.\n{exc}" raise DataSafeHavenAzureError(msg) from exc @@ -649,7 +650,7 @@ def list_available_vm_skus(self, location: str) -> dict[str, dict[str, Any]]: ): skus[resource_sku.name][capability.name] = capability.value return skus - except DataSafeHavenAzureError as exc: + except AzureError as exc: msg = f"Failed to load available VM sizes for Azure location {location}.\n{exc}" raise DataSafeHavenAzureError(msg) from exc @@ -686,7 +687,7 @@ def purge_keyvault_certificate( self.logger.info( f"Purged certificate [green]{certificate_name}[/] from Key Vault [green]{key_vault_name}[/].", ) - except DataSafeHavenAzureError as exc: + except AzureError as exc: msg = f"Failed to remove certificate '{certificate_name}' from Key Vault '{key_vault_name}'.\n{exc}" raise DataSafeHavenAzureError(msg) from exc @@ -724,7 +725,7 @@ def remove_blob( self.logger.info( f"Removed file [green]{blob_name}[/] from blob storage.", ) - except DataSafeHavenAzureError as exc: + except AzureError as exc: msg = f"Blob file [green]'{blob_name}'[/] could not be removed from [green]'{storage_account_name}'[/].\n{exc}" raise DataSafeHavenAzureError(msg) from exc @@ -768,7 +769,7 @@ def remove_dns_txt_record( self.logger.info( f"Ensured that DNS record [green]{record_name}[/] is removed from zone [green]{zone_name}[/].", ) - except DataSafeHavenAzureError as exc: + except AzureError as exc: msg = f"Failed to remove DNS record [green]{record_name}[/] from zone [green]{zone_name}[/].\n{exc}" raise DataSafeHavenAzureError(msg) from exc @@ -825,7 +826,7 @@ def remove_keyvault_certificate( self.logger.info( f"Removed certificate [green]{certificate_name}[/] from Key Vault [green]{key_vault_name}[/].", ) - except DataSafeHavenAzureError as exc: + except AzureError as exc: msg = f"Failed to remove certificate '{certificate_name}' from Key Vault '{key_vault_name}'.\n{exc}" raise DataSafeHavenAzureError(msg) from exc @@ -869,7 +870,7 @@ def remove_resource_group(self, resource_group_name: str) -> None: self.logger.info( f"Ensured that resource group [green]{resource_group_name}[/] does not exist.", ) - except DataSafeHavenAzureError as exc: + except AzureError as exc: msg = f"Failed to remove resource group {resource_group_name}.\n{exc}" raise DataSafeHavenAzureError(msg) from exc @@ -921,7 +922,7 @@ def run_remote_script( result = cast(RunCommandResult, poller.result()) # Return any stdout/stderr from the command return str(result.value[0].message) if result.value else "" - except DataSafeHavenAzureError as exc: + except AzureError as exc: msg = f"Failed to run command on '{vm_name}'.\n{exc}" raise DataSafeHavenAzureError(msg) from exc @@ -949,7 +950,7 @@ def run_remote_script_waiting( vm_name=vm_name, ) break - except DataSafeHavenAzureError as exc: + except AzureError as exc: if all( reason not in str(exc) for reason in ( @@ -1002,7 +1003,7 @@ def set_blob_container_acl( directory_client = file_system_client._get_root_directory_client() # Set the desired ACL directory_client.set_access_control_recursive(acl=desired_acl) - except DataSafeHavenAzureError as exc: + except AzureError as exc: msg = f"Failed to set ACL '{desired_acl}' on container '{container_name}'.\n{exc}" raise DataSafeHavenAzureError(msg) from exc @@ -1034,6 +1035,6 @@ def upload_blob( self.logger.info( f"Uploaded file [green]{blob_name}[/] to blob storage.", ) - except DataSafeHavenAzureError as exc: + except AzureError as exc: msg = f"Blob file '{blob_name}' could not be uploaded to '{storage_account_name}'\n{exc}." raise DataSafeHavenAzureError(msg) from exc From 3d98eb75665259a0000268420f83bb947a09d0b0 Mon Sep 17 00:00:00 2001 From: Matt Craddock <5796417+craddm@users.noreply.github.com> Date: Tue, 4 Jun 2024 09:51:22 +0000 Subject: [PATCH 22/33] ignore python virtual environments --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 59ba2114dd..15607a9be1 100644 --- a/.gitignore +++ b/.gitignore @@ -14,6 +14,7 @@ environment_configs/package_lists/dependency-cache.json # Python build caches __pycache__/ +.venv/ # Development tools .vscode From 16cd09dad8c7ef8d1b599cfb52ced6d07de24eec Mon Sep 17 00:00:00 2001 From: Matt Craddock <5796417+craddm@users.noreply.github.com> Date: Tue, 4 Jun 2024 12:14:03 +0000 Subject: [PATCH 23/33] Change from run to use --- data_safe_haven/commands/context.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/data_safe_haven/commands/context.py b/data_safe_haven/commands/context.py index b08c9c6981..12b2f8b994 100644 --- a/data_safe_haven/commands/context.py +++ b/data_safe_haven/commands/context.py @@ -25,7 +25,7 @@ def show() -> None: try: settings = ContextSettings.from_file() except DataSafeHavenConfigError: - print("No context configuration file. Run `dsh context add` to create one.") + print("No context configuration file. Use `dsh context add` to create one.") raise typer.Exit(code=1) from None current_context_key = settings.selected @@ -45,7 +45,7 @@ def available() -> None: try: settings = ContextSettings.from_file() except DataSafeHavenConfigError: - print("No context configuration file. Run `dsh context add` to create one.") + print("No context configuration file. Use `dsh context add` to create one.") raise typer.Exit(code=1) from None current_context_key = settings.selected @@ -66,7 +66,7 @@ def switch( try: settings = ContextSettings.from_file() except DataSafeHavenConfigError: - print("No context configuration file. Run `dsh context add` to create one.") + print("No context configuration file. Use `dsh context add` to create one.") raise typer.Exit(code=1) from None settings.selected = key settings.write() @@ -160,7 +160,7 @@ def update( try: settings = ContextSettings.from_file() except DataSafeHavenConfigError: - print("No context configuration file. Run `dsh context add` to create one.") + print("No context configuration file. Use `dsh context add` to create one.") raise typer.Exit(code=1) from None settings.update( @@ -193,10 +193,10 @@ def create() -> None: context = ContextSettings.from_file().assert_context() except DataSafeHavenConfigError as exc: if exc.args[0] == "No context selected": - print("No context selected. Run `dsh context switch` to select one.") + print("No context selected. Use `dsh context switch` to select one.") else: print( - "No context configuration file. Run `dsh context add` before creating infrastructure." + "No context configuration file. Use `dsh context add` before creating infrastructure." ) raise typer.Exit(code=1) from None @@ -217,10 +217,10 @@ def teardown() -> None: context = ContextSettings.from_file().assert_context() except DataSafeHavenConfigError as exc: if exc.args[0] == "No context selected": - print("No context selected. Run `dsh context switch` to select one.") + print("No context selected. Use `dsh context switch` to select one.") else: print( - "No context configuration file. Run `dsh context add` before creating infrastructure." + "No context configuration file. Use `dsh context add` before creating infrastructure." ) raise typer.Exit(code=1) from None From fc1161b1659666ba9ca7d5653fadc80098820cd3 Mon Sep 17 00:00:00 2001 From: Matt Craddock Date: Tue, 4 Jun 2024 15:27:38 +0100 Subject: [PATCH 24/33] Update data_safe_haven/commands/context.py Co-authored-by: James Robinson --- data_safe_haven/commands/context.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/data_safe_haven/commands/context.py b/data_safe_haven/commands/context.py index 12b2f8b994..4a9f293bcc 100644 --- a/data_safe_haven/commands/context.py +++ b/data_safe_haven/commands/context.py @@ -228,8 +228,8 @@ def teardown() -> None: try: context_infra.teardown() - except DataSafeHavenAzureAPIAuthenticationError: + except DataSafeHavenAzureAPIAuthenticationError as exc: print( "Failed to authenticate with the Azure API. You may not be logged into the Azure CLI, or your login may have expired. Try running `az login`." ) - raise typer.Exit(1) from None + raise typer.Exit(1) from exc From 23c9e716efb129c4a8aff7d930365395a48259d8 Mon Sep 17 00:00:00 2001 From: Matt Craddock Date: Tue, 4 Jun 2024 15:38:51 +0100 Subject: [PATCH 25/33] Update data_safe_haven/commands/context.py Co-authored-by: James Robinson --- data_safe_haven/commands/context.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/data_safe_haven/commands/context.py b/data_safe_haven/commands/context.py index 4a9f293bcc..302c399f4a 100644 --- a/data_safe_haven/commands/context.py +++ b/data_safe_haven/commands/context.py @@ -44,9 +44,9 @@ def available() -> None: """Show the available contexts.""" try: settings = ContextSettings.from_file() - except DataSafeHavenConfigError: + except DataSafeHavenConfigError as exc: print("No context configuration file. Use `dsh context add` to create one.") - raise typer.Exit(code=1) from None + raise typer.Exit(code=1) from exc current_context_key = settings.selected available = settings.available From fb3050a05cc8c2b02cfb6735ab827cd74ec5fbaa Mon Sep 17 00:00:00 2001 From: Matt Craddock Date: Tue, 4 Jun 2024 15:38:56 +0100 Subject: [PATCH 26/33] Update data_safe_haven/commands/context.py Co-authored-by: James Robinson --- data_safe_haven/commands/context.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/data_safe_haven/commands/context.py b/data_safe_haven/commands/context.py index 302c399f4a..670701b7c9 100644 --- a/data_safe_haven/commands/context.py +++ b/data_safe_haven/commands/context.py @@ -179,9 +179,9 @@ def remove( """Removes a context.""" try: settings = ContextSettings.from_file() - except DataSafeHavenConfigError: + except DataSafeHavenConfigError as exc: print("No context configuration file.") - raise typer.Exit(code=1) from None + raise typer.Exit(code=1) from exc settings.remove(key) settings.write() From fc1e649b18939d18669e6e3b09354a799b0aef40 Mon Sep 17 00:00:00 2001 From: Matt Craddock Date: Tue, 4 Jun 2024 15:39:05 +0100 Subject: [PATCH 27/33] Update data_safe_haven/commands/context.py Co-authored-by: James Robinson --- data_safe_haven/commands/context.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/data_safe_haven/commands/context.py b/data_safe_haven/commands/context.py index 670701b7c9..da2370461b 100644 --- a/data_safe_haven/commands/context.py +++ b/data_safe_haven/commands/context.py @@ -198,7 +198,7 @@ def create() -> None: print( "No context configuration file. Use `dsh context add` before creating infrastructure." ) - raise typer.Exit(code=1) from None + raise typer.Exit(code=1) from exc context_infra = ContextInfrastructure(context) try: From 82cd6f8bf35267b5487bd96cbaa01c799149ca0d Mon Sep 17 00:00:00 2001 From: Matt Craddock Date: Tue, 4 Jun 2024 15:39:15 +0100 Subject: [PATCH 28/33] Update data_safe_haven/commands/context.py Co-authored-by: James Robinson --- data_safe_haven/commands/context.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/data_safe_haven/commands/context.py b/data_safe_haven/commands/context.py index da2370461b..3ec4a9b466 100644 --- a/data_safe_haven/commands/context.py +++ b/data_safe_haven/commands/context.py @@ -222,7 +222,7 @@ def teardown() -> None: print( "No context configuration file. Use `dsh context add` before creating infrastructure." ) - raise typer.Exit(code=1) from None + raise typer.Exit(code=1) from exc context_infra = ContextInfrastructure(context) From 86beb5657e6bbd904ef9d581ce2fb6d5cfe65c4b Mon Sep 17 00:00:00 2001 From: Matt Craddock Date: Tue, 4 Jun 2024 15:39:20 +0100 Subject: [PATCH 29/33] Update data_safe_haven/commands/context.py Co-authored-by: James Robinson --- data_safe_haven/commands/context.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/data_safe_haven/commands/context.py b/data_safe_haven/commands/context.py index 3ec4a9b466..a491e662c1 100644 --- a/data_safe_haven/commands/context.py +++ b/data_safe_haven/commands/context.py @@ -24,9 +24,9 @@ def show() -> None: """Show information about the selected context.""" try: settings = ContextSettings.from_file() - except DataSafeHavenConfigError: + except DataSafeHavenConfigError as exc: print("No context configuration file. Use `dsh context add` to create one.") - raise typer.Exit(code=1) from None + raise typer.Exit(code=1) from exc current_context_key = settings.selected current_context = settings.context From 1be93bbe3355622f8a8734149b310fc31967c0f5 Mon Sep 17 00:00:00 2001 From: Matt Craddock Date: Tue, 4 Jun 2024 15:39:26 +0100 Subject: [PATCH 30/33] Update data_safe_haven/commands/context.py Co-authored-by: James Robinson --- data_safe_haven/commands/context.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/data_safe_haven/commands/context.py b/data_safe_haven/commands/context.py index a491e662c1..f28a404782 100644 --- a/data_safe_haven/commands/context.py +++ b/data_safe_haven/commands/context.py @@ -203,11 +203,11 @@ def create() -> None: context_infra = ContextInfrastructure(context) try: context_infra.create() - except DataSafeHavenAzureAPIAuthenticationError: + except DataSafeHavenAzureAPIAuthenticationError as exc: print( "Failed to authenticate with the Azure API. You may not be logged into the Azure CLI, or your login may have expired. Try running `az login`." ) - raise typer.Exit(1) from None + raise typer.Exit(1) from exc @context_command_group.command() From 74f6f60bdc7b823db84a7625cc6bca349671c0c6 Mon Sep 17 00:00:00 2001 From: Matt Craddock Date: Tue, 4 Jun 2024 15:39:34 +0100 Subject: [PATCH 31/33] Update data_safe_haven/commands/context.py Co-authored-by: James Robinson --- data_safe_haven/commands/context.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/data_safe_haven/commands/context.py b/data_safe_haven/commands/context.py index f28a404782..511d8d0063 100644 --- a/data_safe_haven/commands/context.py +++ b/data_safe_haven/commands/context.py @@ -159,9 +159,9 @@ def update( """Update the selected context settings.""" try: settings = ContextSettings.from_file() - except DataSafeHavenConfigError: + except DataSafeHavenConfigError as exc: print("No context configuration file. Use `dsh context add` to create one.") - raise typer.Exit(code=1) from None + raise typer.Exit(code=1) from exc settings.update( admin_group_id=admin_group, From 9da2cff034d6cc0ef0953e5125365f2938fa48f2 Mon Sep 17 00:00:00 2001 From: Matt Craddock Date: Tue, 4 Jun 2024 15:39:43 +0100 Subject: [PATCH 32/33] Update data_safe_haven/commands/context.py Co-authored-by: James Robinson --- data_safe_haven/commands/context.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/data_safe_haven/commands/context.py b/data_safe_haven/commands/context.py index 511d8d0063..c7c5ae45ca 100644 --- a/data_safe_haven/commands/context.py +++ b/data_safe_haven/commands/context.py @@ -65,9 +65,9 @@ def switch( """Switch the selected context.""" try: settings = ContextSettings.from_file() - except DataSafeHavenConfigError: + except DataSafeHavenConfigError as exc: print("No context configuration file. Use `dsh context add` to create one.") - raise typer.Exit(code=1) from None + raise typer.Exit(code=1) from exc settings.selected = key settings.write() From 89ab2c420beb88fc8369ecdac1c527752f5509d0 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Thu, 6 Jun 2024 14:03:57 +0100 Subject: [PATCH 33/33] :loud_sound: Use LoggingSingleton for error message before exit --- data_safe_haven/commands/context.py | 49 ++++++++++++++++++----------- 1 file changed, 31 insertions(+), 18 deletions(-) diff --git a/data_safe_haven/commands/context.py b/data_safe_haven/commands/context.py index c7c5ae45ca..94a35fef58 100644 --- a/data_safe_haven/commands/context.py +++ b/data_safe_haven/commands/context.py @@ -3,7 +3,6 @@ from typing import Annotated, Optional import typer -from rich import print from data_safe_haven import validators from data_safe_haven.context import ( @@ -15,8 +14,10 @@ DataSafeHavenAzureAPIAuthenticationError, DataSafeHavenConfigError, ) +from data_safe_haven.utility import LoggingSingleton context_command_group = typer.Typer() +logger = LoggingSingleton() @context_command_group.command() @@ -25,18 +26,20 @@ def show() -> None: try: settings = ContextSettings.from_file() except DataSafeHavenConfigError as exc: - print("No context configuration file. Use `dsh context add` to create one.") + logger.critical( + "No context configuration file. Use `dsh context add` to create one." + ) raise typer.Exit(code=1) from exc current_context_key = settings.selected current_context = settings.context - print(f"Current context: [green]{current_context_key}") + logger.info(f"Current context: [green]{current_context_key}") if current_context is not None: - print(f"\tName: {current_context.name}") - print(f"\tAdmin Group ID: {current_context.admin_group_id}") - print(f"\tSubscription name: {current_context.subscription_name}") - print(f"\tLocation: {current_context.location}") + logger.info(f"\tName: {current_context.name}") + logger.info(f"\tAdmin Group ID: {current_context.admin_group_id}") + logger.info(f"\tSubscription name: {current_context.subscription_name}") + logger.info(f"\tLocation: {current_context.location}") @context_command_group.command() @@ -45,7 +48,9 @@ def available() -> None: try: settings = ContextSettings.from_file() except DataSafeHavenConfigError as exc: - print("No context configuration file. Use `dsh context add` to create one.") + logger.critical( + "No context configuration file. Use `dsh context add` to create one." + ) raise typer.Exit(code=1) from exc current_context_key = settings.selected @@ -55,7 +60,7 @@ def available() -> None: available.remove(current_context_key) available = [f"[green]{current_context_key}*[/]", *available] - print("\n".join(available)) + logger.info("\n".join(available)) @context_command_group.command() @@ -66,7 +71,9 @@ def switch( try: settings = ContextSettings.from_file() except DataSafeHavenConfigError as exc: - print("No context configuration file. Use `dsh context add` to create one.") + logger.critical( + "No context configuration file. Use `dsh context add` to create one." + ) raise typer.Exit(code=1) from exc settings.selected = key settings.write() @@ -160,7 +167,9 @@ def update( try: settings = ContextSettings.from_file() except DataSafeHavenConfigError as exc: - print("No context configuration file. Use `dsh context add` to create one.") + logger.critical( + "No context configuration file. Use `dsh context add` to create one." + ) raise typer.Exit(code=1) from exc settings.update( @@ -180,7 +189,7 @@ def remove( try: settings = ContextSettings.from_file() except DataSafeHavenConfigError as exc: - print("No context configuration file.") + logger.critical("No context configuration file.") raise typer.Exit(code=1) from exc settings.remove(key) settings.write() @@ -193,9 +202,11 @@ def create() -> None: context = ContextSettings.from_file().assert_context() except DataSafeHavenConfigError as exc: if exc.args[0] == "No context selected": - print("No context selected. Use `dsh context switch` to select one.") + logger.critical( + "No context selected. Use `dsh context switch` to select one." + ) else: - print( + logger.critical( "No context configuration file. Use `dsh context add` before creating infrastructure." ) raise typer.Exit(code=1) from exc @@ -204,7 +215,7 @@ def create() -> None: try: context_infra.create() except DataSafeHavenAzureAPIAuthenticationError as exc: - print( + logger.critical( "Failed to authenticate with the Azure API. You may not be logged into the Azure CLI, or your login may have expired. Try running `az login`." ) raise typer.Exit(1) from exc @@ -217,9 +228,11 @@ def teardown() -> None: context = ContextSettings.from_file().assert_context() except DataSafeHavenConfigError as exc: if exc.args[0] == "No context selected": - print("No context selected. Use `dsh context switch` to select one.") + logger.critical( + "No context selected. Use `dsh context switch` to select one." + ) else: - print( + logger.critical( "No context configuration file. Use `dsh context add` before creating infrastructure." ) raise typer.Exit(code=1) from exc @@ -229,7 +242,7 @@ def teardown() -> None: try: context_infra.teardown() except DataSafeHavenAzureAPIAuthenticationError as exc: - print( + logger.critical( "Failed to authenticate with the Azure API. You may not be logged into the Azure CLI, or your login may have expired. Try running `az login`." ) raise typer.Exit(1) from exc