From 26388cdf677f12c2f8c88ec89a933e1e22554a49 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Tue, 22 Aug 2023 12:09:21 +0100 Subject: [PATCH 1/4] :alien: Use workspace.who_am_i() if available --- data_safe_haven/pulumi/pulumi_stack.py | 30 +++++--------------------- 1 file changed, 5 insertions(+), 25 deletions(-) diff --git a/data_safe_haven/pulumi/pulumi_stack.py b/data_safe_haven/pulumi/pulumi_stack.py index 982f3ff3ac..1b53225afb 100644 --- a/data_safe_haven/pulumi/pulumi_stack.py +++ b/data_safe_haven/pulumi/pulumi_stack.py @@ -198,10 +198,12 @@ def login(self) -> None: """Login to Pulumi.""" try: # Check whether we're already logged in + # Note that we cannot retrieve self.stack without being logged in with suppress(DataSafeHavenPulumiError): - username = self.whoami() - self.logger.info(f"Logged into Pulumi as [green]{username}[/]") - return + result = self.stack.workspace.who_am_i() + if result.user: + self.logger.info(f"Logged into Pulumi as [green]{result.user}[/]") + return # Otherwise log in to Pulumi try: # Ensure we are authenticated with the Azure CLI @@ -298,28 +300,6 @@ def update(self) -> None: msg = f"Pulumi update failed.\n{exc}" raise DataSafeHavenPulumiError(msg) from exc - def whoami(self) -> str: - """Check current Pulumi user.""" - try: - AzureCli().login() # this is needed to read the encryption key from the keyvault - self.work_dir.mkdir(parents=True, exist_ok=True) - try: - process = subprocess.run( - ["pulumi", "whoami"], - capture_output=True, - check=True, - cwd=self.work_dir, - encoding="UTF-8", - env={**os.environ, **self.env}, - ) - return process.stdout.strip() - except (subprocess.CalledProcessError, FileNotFoundError) as exc: - msg = f"No Pulumi user found.\n{exc}." - raise DataSafeHavenPulumiError(msg) from exc - except Exception as exc: - msg = f"Pulumi user check failed.\n{exc}." - raise DataSafeHavenPulumiError(msg) from exc - class PulumiSHMStack(PulumiStack): """Interact with an SHM using Pulumi""" From fb21d25fc5d5b1551235367623d06ed339b93f00 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Tue, 22 Aug 2023 13:02:46 +0100 Subject: [PATCH 2/4] :bug: Fix subprocess command which was being broken by nested quotes --- data_safe_haven/pulumi/pulumi_stack.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/data_safe_haven/pulumi/pulumi_stack.py b/data_safe_haven/pulumi/pulumi_stack.py index 1b53225afb..35e9d20947 100644 --- a/data_safe_haven/pulumi/pulumi_stack.py +++ b/data_safe_haven/pulumi/pulumi_stack.py @@ -213,13 +213,13 @@ def login(self) -> None: [ "pulumi", "login", - f"'azblob://{self.cfg.pulumi.storage_container_name}'", + f"azblob://{self.cfg.pulumi.storage_container_name}", ], - env={**os.environ, **self.env}, - encoding="UTF-8", capture_output=True, check=True, cwd=self.work_dir, + encoding="UTF-8", + env={**os.environ, **self.env}, ) self.logger.info(process.stdout) except (subprocess.CalledProcessError, FileNotFoundError) as exc: From 19bfefccad6f92b0956053aa17b76fd515247693 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Tue, 22 Aug 2023 13:43:46 +0100 Subject: [PATCH 3/4] :alien: Ensure we always check for AzureCli login to avoid an opaque error where Pulumi doesn't have permission to read from the keyvault --- data_safe_haven/external/api/azure_cli.py | 19 +++++++++++++++---- data_safe_haven/pulumi/pulumi_stack.py | 6 +++--- 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/data_safe_haven/external/api/azure_cli.py b/data_safe_haven/external/api/azure_cli.py index cc0e406593..ae1fbc8b41 100644 --- a/data_safe_haven/external/api/azure_cli.py +++ b/data_safe_haven/external/api/azure_cli.py @@ -17,17 +17,28 @@ def login(self) -> None: """Force log in via the Azure CLI""" try: self.logger.debug("Attempting to login using Azure CLI.") + # We do not use `check` in subprocess as this raises a CalledProcessError + # which would break the loop. Instead we check the return code of + # `az account show` which will be 0 on success. while True: - process = subprocess.run(["az", "account", "show"], capture_output=True) + # Check whether we are already logged in + process = subprocess.run( + ["az", "account", "show"], capture_output=True, check=False + ) if process.returncode == 0: break + # Note that subprocess.run will block until the process terminates so + # we need to print the guidance first. self.logger.info( - "Please login in your web browser at https://login.microsoftonline.com/organizations/oauth2/v2.0/authorize." + "Please login in your web browser at [bold]https://login.microsoftonline.com/organizations/oauth2/v2.0/authorize[/]." ) self.logger.info( - "If no web browser is available, please run `az login --use-device-code` in a command line window." + "If no web browser is available, please run [bold]az login --use-device-code[/] in a command line window." + ) + # Attempt to log in at the command line + process = subprocess.run( + ["az", "login"], capture_output=True, check=False ) - subprocess.run(["az", "login"], capture_output=True) except (FileNotFoundError, subprocess.CalledProcessError) as exc: msg = f"Please ensure that the Azure CLI is installed.\n{exc}" raise DataSafeHavenAzureError(msg) from exc diff --git a/data_safe_haven/pulumi/pulumi_stack.py b/data_safe_haven/pulumi/pulumi_stack.py index 35e9d20947..7644215679 100644 --- a/data_safe_haven/pulumi/pulumi_stack.py +++ b/data_safe_haven/pulumi/pulumi_stack.py @@ -197,6 +197,9 @@ def install_plugins(self) -> None: def login(self) -> None: """Login to Pulumi.""" try: + # Ensure we are authenticated with the Azure CLI + # Without this, we cannot read the encryption key from the keyvault + AzureCli().login() # Check whether we're already logged in # Note that we cannot retrieve self.stack without being logged in with suppress(DataSafeHavenPulumiError): @@ -206,9 +209,6 @@ def login(self) -> None: return # Otherwise log in to Pulumi try: - # Ensure we are authenticated with the Azure CLI - # Without this, we cannot read the encryption key from the keyvault - AzureCli().login() process = subprocess.run( [ "pulumi", From cd40dac2cba4b08b99977485cec7139cfb56240c Mon Sep 17 00:00:00 2001 From: James Robinson Date: Tue, 22 Aug 2023 13:46:05 +0100 Subject: [PATCH 4/4] :coffin: Remove CalledProcessError which can no longer occur --- data_safe_haven/external/api/azure_cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/data_safe_haven/external/api/azure_cli.py b/data_safe_haven/external/api/azure_cli.py index ae1fbc8b41..54c86e6689 100644 --- a/data_safe_haven/external/api/azure_cli.py +++ b/data_safe_haven/external/api/azure_cli.py @@ -39,6 +39,6 @@ def login(self) -> None: process = subprocess.run( ["az", "login"], capture_output=True, check=False ) - except (FileNotFoundError, subprocess.CalledProcessError) as exc: + except FileNotFoundError as exc: msg = f"Please ensure that the Azure CLI is installed.\n{exc}" raise DataSafeHavenAzureError(msg) from exc