From fddefead13b4f6d4fa0ea867119412ef6dfb98bf Mon Sep 17 00:00:00 2001 From: Jim Madge Date: Wed, 16 Oct 2024 15:02:03 +0100 Subject: [PATCH 1/5] Raise exception if unchangable values are changed --- data_safe_haven/infrastructure/project_manager.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/data_safe_haven/infrastructure/project_manager.py b/data_safe_haven/infrastructure/project_manager.py index dcb3941af2..6778731ffd 100644 --- a/data_safe_haven/infrastructure/project_manager.py +++ b/data_safe_haven/infrastructure/project_manager.py @@ -288,10 +288,17 @@ def destroy(self) -> None: def ensure_config(self, name: str, value: str, *, secret: bool) -> None: """Ensure that config values have been set, setting them if they do not exist""" try: - self.stack.get_config(name) + existing_value = self.stack.get_config(name) except automation.CommandError: self.set_config(name, value, secret=secret) + if existing_value != value: + msg = ( + f"Unchangeable configuration variable '{name}' not consistent, " + f"your configuration: '{value}', Pulumi workspace: '{existing_value}'." + ) + raise DataSafeHavenPulumiError(msg) + def evaluate(self, result: str) -> None: """Evaluate a Pulumi operation.""" if result == "succeeded": From 01037af082ac238e49792f4be33a01ca68dddb49 Mon Sep 17 00:00:00 2001 From: Jim Madge Date: Thu, 17 Oct 2024 10:28:27 +0100 Subject: [PATCH 2/5] Compare config value, not config object --- data_safe_haven/infrastructure/project_manager.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/data_safe_haven/infrastructure/project_manager.py b/data_safe_haven/infrastructure/project_manager.py index 6778731ffd..7cb4deb458 100644 --- a/data_safe_haven/infrastructure/project_manager.py +++ b/data_safe_haven/infrastructure/project_manager.py @@ -288,7 +288,7 @@ def destroy(self) -> None: def ensure_config(self, name: str, value: str, *, secret: bool) -> None: """Ensure that config values have been set, setting them if they do not exist""" try: - existing_value = self.stack.get_config(name) + existing_value = self.stack.get_config(name).value except automation.CommandError: self.set_config(name, value, secret=secret) From 1839464574d66a1f93ae4abe79892ced2ea57426 Mon Sep 17 00:00:00 2001 From: Jim Madge Date: Thu, 17 Oct 2024 13:30:36 +0100 Subject: [PATCH 3/5] Change variable to option --- data_safe_haven/infrastructure/project_manager.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/data_safe_haven/infrastructure/project_manager.py b/data_safe_haven/infrastructure/project_manager.py index 7cb4deb458..c81b52b8fe 100644 --- a/data_safe_haven/infrastructure/project_manager.py +++ b/data_safe_haven/infrastructure/project_manager.py @@ -294,7 +294,7 @@ def ensure_config(self, name: str, value: str, *, secret: bool) -> None: if existing_value != value: msg = ( - f"Unchangeable configuration variable '{name}' not consistent, " + f"Unchangeable configuration option '{name}' not consistent, " f"your configuration: '{value}', Pulumi workspace: '{existing_value}'." ) raise DataSafeHavenPulumiError(msg) From 98942dc827e72645bb4281022b2285ebd30089ea Mon Sep 17 00:00:00 2001 From: Jim Madge Date: Thu, 17 Oct 2024 13:50:03 +0100 Subject: [PATCH 4/5] Add tests for ensure_config --- tests/infrastructure/test_project_manager.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/infrastructure/test_project_manager.py b/tests/infrastructure/test_project_manager.py index 259c5f1b37..8c74a83808 100644 --- a/tests/infrastructure/test_project_manager.py +++ b/tests/infrastructure/test_project_manager.py @@ -49,6 +49,22 @@ def test_cleanup( ) assert "Purged Azure Key Vault shmacmedsresandbosecrets." in stdout + def test_ensure_config(self, sre_project_manager): + sre_project_manager.ensure_config( + "azure-native:location", "uksouth", secret=False + ) + sre_project_manager.ensure_config("data-safe-haven:variable", "8", secret=False) + + def test_ensure_config_exception(self, sre_project_manager): + + with raises( + DataSafeHavenPulumiError, + match=r"Unchangeable configuration option 'azure-native:location'.*your configuration: 'ukwest', Pulumi workspace: 'uksouth'", + ): + sre_project_manager.ensure_config( + "azure-native:location", "ukwest", secret=False + ) + def test_new_project( self, context_no_secrets, From b16256f281ee5152fadc93eb199c599186e925ff Mon Sep 17 00:00:00 2001 From: Jim Madge Date: Thu, 17 Oct 2024 13:54:11 +0100 Subject: [PATCH 5/5] Update docstring and comments --- data_safe_haven/infrastructure/project_manager.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/data_safe_haven/infrastructure/project_manager.py b/data_safe_haven/infrastructure/project_manager.py index c81b52b8fe..6008f26cf8 100644 --- a/data_safe_haven/infrastructure/project_manager.py +++ b/data_safe_haven/infrastructure/project_manager.py @@ -286,12 +286,22 @@ def destroy(self) -> None: raise DataSafeHavenPulumiError(msg) from exc def ensure_config(self, name: str, value: str, *, secret: bool) -> None: - """Ensure that config values have been set, setting them if they do not exist""" + """ + Ensure that config values have been set. + + Values will be set if they do not exist. + + If the value is already set and does not match the `value` argument, + `DataSafeHavenPulumiError` will be raised. + """ try: existing_value = self.stack.get_config(name).value except automation.CommandError: + # Set value if it does not already exist self.set_config(name, value, secret=secret) + # If the value does already exist, ensure it is consistent with the declared + # value if existing_value != value: msg = ( f"Unchangeable configuration option '{name}' not consistent, "