From 03f5eed7820d1a9e6916d17d62be1ea5874be780 Mon Sep 17 00:00:00 2001 From: Alex Harvey Date: Fri, 24 Nov 2023 19:16:59 +1100 Subject: [PATCH 01/13] Add a unit test This adds a unit test demonstrating the behaviour of a badly-formatted resolver. --- tests/test_resolvers/test_stack_output.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/test_resolvers/test_stack_output.py b/tests/test_resolvers/test_stack_output.py index 60f437ce3..478958727 100644 --- a/tests/test_resolvers/test_stack_output.py +++ b/tests/test_resolvers/test_stack_output.py @@ -50,6 +50,16 @@ def test_resolver(self, mock_get_output_value): sceptre_role="dependency_sceptre_role", ) + @patch("sceptre.resolvers.stack_output.StackOutput._get_output_value") + def test_resolver__badly_formatted(self, mock_get_output_value): + stack = MagicMock(spec=Stack) + stack.name = "my/stack" + + stack_output_resolver = StackOutput("account/dev/vpc.yaml", stack) + + with pytest.raises(ValueError): + stack_output_resolver.setup() + @patch("sceptre.resolvers.stack_output.StackOutput._get_output_value") def test_resolver_with_existing_dependencies(self, mock_get_output_value): stack = MagicMock(spec=Stack) From 9bf832ee867831948c45c8f19ff1cc9041ac18cb Mon Sep 17 00:00:00 2001 From: Alex Harvey Date: Sat, 25 Nov 2023 02:05:53 +1100 Subject: [PATCH 02/13] more --- tests/test_resolvers/test_stack_output.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_resolvers/test_stack_output.py b/tests/test_resolvers/test_stack_output.py index 478958727..2828947be 100644 --- a/tests/test_resolvers/test_stack_output.py +++ b/tests/test_resolvers/test_stack_output.py @@ -55,7 +55,7 @@ def test_resolver__badly_formatted(self, mock_get_output_value): stack = MagicMock(spec=Stack) stack.name = "my/stack" - stack_output_resolver = StackOutput("account/dev/vpc.yaml", stack) + stack_output_resolver = StackOutput("not_a_valid_stack_output", stack) with pytest.raises(ValueError): stack_output_resolver.setup() From 54c790efc3c7e9b69913cd30a9f63eaa46404ee6 Mon Sep 17 00:00:00 2001 From: Alex Harvey Date: Tue, 28 Nov 2023 00:54:34 +1100 Subject: [PATCH 03/13] more-tests --- tests/test_resolvers/test_stack_output.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/test_resolvers/test_stack_output.py b/tests/test_resolvers/test_stack_output.py index 2828947be..195c901f7 100644 --- a/tests/test_resolvers/test_stack_output.py +++ b/tests/test_resolvers/test_stack_output.py @@ -181,6 +181,18 @@ def test_resolve(self, mock_get_output_value): ) assert stack.dependencies == [] + @patch("sceptre.resolvers.stack_output.StackOutput._get_output_value") + def test_resolve__badly_formatted(self, mock_get_output_value): + stack = MagicMock(spec=Stack) + stack.name = "my/stack" + + stack_output_external_resolver = StackOutputExternal( + "not_a_valid_stack_output", stack + ) + + with pytest.raises(ValueError): + stack_output_external_resolver.resolve() + @patch("sceptre.resolvers.stack_output.StackOutputExternal._get_output_value") def test_resolve_with_args(self, mock_get_output_value): stack = MagicMock(spec=Stack) From fcecf27cf5a0e3d631a447a635ee821a73888393 Mon Sep 17 00:00:00 2001 From: Alex Harvey Date: Tue, 28 Nov 2023 02:03:29 +1100 Subject: [PATCH 04/13] more --- tests/test_resolvers/test_stack_output.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/test_resolvers/test_stack_output.py b/tests/test_resolvers/test_stack_output.py index 195c901f7..f4aef0158 100644 --- a/tests/test_resolvers/test_stack_output.py +++ b/tests/test_resolvers/test_stack_output.py @@ -5,6 +5,8 @@ from sceptre.exceptions import DependencyStackMissingOutputError from sceptre.exceptions import StackDoesNotExistError +from sceptre.exceptions import SceptreException + from botocore.exceptions import ClientError from sceptre.connection_manager import ConnectionManager @@ -57,7 +59,7 @@ def test_resolver__badly_formatted(self, mock_get_output_value): stack_output_resolver = StackOutput("not_a_valid_stack_output", stack) - with pytest.raises(ValueError): + with pytest.raises(SceptreException, match="STACK_NAME::OUTPUT_KEY"): stack_output_resolver.setup() @patch("sceptre.resolvers.stack_output.StackOutput._get_output_value") @@ -190,7 +192,7 @@ def test_resolve__badly_formatted(self, mock_get_output_value): "not_a_valid_stack_output", stack ) - with pytest.raises(ValueError): + with pytest.raises(SceptreException, match="STACK_NAME::OUTPUT_KEY"): stack_output_external_resolver.resolve() @patch("sceptre.resolvers.stack_output.StackOutputExternal._get_output_value") From 763d9f2132df7b3e23c7f5d87f02aaad5c863cb6 Mon Sep 17 00:00:00 2001 From: Alex Harvey Date: Tue, 28 Nov 2023 02:05:01 +1100 Subject: [PATCH 05/13] more --- tests/test_resolvers/test_stack_output.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/test_resolvers/test_stack_output.py b/tests/test_resolvers/test_stack_output.py index f4aef0158..ff20effd9 100644 --- a/tests/test_resolvers/test_stack_output.py +++ b/tests/test_resolvers/test_stack_output.py @@ -5,7 +5,6 @@ from sceptre.exceptions import DependencyStackMissingOutputError from sceptre.exceptions import StackDoesNotExistError -from sceptre.exceptions import SceptreException from botocore.exceptions import ClientError @@ -59,7 +58,7 @@ def test_resolver__badly_formatted(self, mock_get_output_value): stack_output_resolver = StackOutput("not_a_valid_stack_output", stack) - with pytest.raises(SceptreException, match="STACK_NAME::OUTPUT_KEY"): + with pytest.raises(ValueError, match="not enough values to unpack"): stack_output_resolver.setup() @patch("sceptre.resolvers.stack_output.StackOutput._get_output_value") @@ -192,7 +191,7 @@ def test_resolve__badly_formatted(self, mock_get_output_value): "not_a_valid_stack_output", stack ) - with pytest.raises(SceptreException, match="STACK_NAME::OUTPUT_KEY"): + with pytest.raises(ValueError, match="not enough values to unpack"): stack_output_external_resolver.resolve() @patch("sceptre.resolvers.stack_output.StackOutputExternal._get_output_value") From 67d40d41c3d9b5eaa4744306ee1ad973142c0128 Mon Sep 17 00:00:00 2001 From: Alex Harvey Date: Tue, 28 Nov 2023 02:06:07 +1100 Subject: [PATCH 06/13] [Resolves #1388] Handle failures gracefully in stack outputs This adds exception handling in the case of failures to parse the given arguments to !stack_output and !stack_output_external. --- sceptre/resolvers/stack_output.py | 42 +++++++++++++++++------ tests/test_resolvers/test_stack_output.py | 5 +-- 2 files changed, 34 insertions(+), 13 deletions(-) diff --git a/sceptre/resolvers/stack_output.py b/sceptre/resolvers/stack_output.py index 6b2254157..aef84cef2 100644 --- a/sceptre/resolvers/stack_output.py +++ b/sceptre/resolvers/stack_output.py @@ -6,7 +6,9 @@ from botocore.exceptions import ClientError -from sceptre.exceptions import DependencyStackMissingOutputError, StackDoesNotExistError +from sceptre.exceptions import DependencyStackMissingOutputError, StackDoesNotExistError, \ + SceptreException + from sceptre.helpers import normalise_path, sceptreise_path from sceptre.resolvers import Resolver @@ -108,7 +110,11 @@ def setup(self): """ Adds dependency to a Stack. """ - dep_stack_name, self.output_key = self.argument.split("::") + try: + dep_stack_name, self.output_key = self.argument.split("::") + except ValueError as err: + raise SceptreException("StackOutput argument should match STACK_NAME::OUTPUT_KEY") from err + self.dependency_stack_name = sceptreise_path(normalise_path(dep_stack_name)) self.stack.dependencies.append(self.dependency_stack_name) @@ -120,14 +126,16 @@ def resolve(self): :rtype: str """ self.logger.debug("Resolving Stack output: {0}".format(self.argument)) - friendly_stack_name = self.dependency_stack_name.replace(TEMPLATE_EXTENSION, "") - stack = next( - stack - for stack in self.stack.dependencies - if stack.name == friendly_stack_name - ) + try: + stack = next( + stack + for stack in self.stack.dependencies + if stack.name == friendly_stack_name + ) + except StopIteration as err: + raise SceptreException(f"Stack '{friendly_stack_name}' not found in dependencies") from err stack_name = "-".join( [stack.project_code, friendly_stack_name.replace("/", "-")] @@ -170,10 +178,22 @@ def resolve(self): stack_argument = arguments[0] if len(arguments) > 1: - extra_args = arguments[1].split("::", 2) - profile, region, sceptre_role = extra_args + (3 - len(extra_args)) * [None] + try: + extra_args = arguments[1].split("::", 2) + profile, region, sceptre_role = extra_args + (3 - len(extra_args)) * [None] + except ValueError as err: + message = ( + "!stack_output_external second arg should be " + "in the format 'profile::region::sceptre_role'" + ) + raise SceptreException(message) from err + + try: + dependency_stack_name, output_key = stack_argument.split("::") + except ValueError as err: + message = "!stack_output_external arg should match STACK_NAME::OUTPUT_KEY" + raise SceptreException(message) from err - dependency_stack_name, output_key = stack_argument.split("::") return self._get_output_value( dependency_stack_name, output_key, diff --git a/tests/test_resolvers/test_stack_output.py b/tests/test_resolvers/test_stack_output.py index ff20effd9..f4aef0158 100644 --- a/tests/test_resolvers/test_stack_output.py +++ b/tests/test_resolvers/test_stack_output.py @@ -5,6 +5,7 @@ from sceptre.exceptions import DependencyStackMissingOutputError from sceptre.exceptions import StackDoesNotExistError +from sceptre.exceptions import SceptreException from botocore.exceptions import ClientError @@ -58,7 +59,7 @@ def test_resolver__badly_formatted(self, mock_get_output_value): stack_output_resolver = StackOutput("not_a_valid_stack_output", stack) - with pytest.raises(ValueError, match="not enough values to unpack"): + with pytest.raises(SceptreException, match="STACK_NAME::OUTPUT_KEY"): stack_output_resolver.setup() @patch("sceptre.resolvers.stack_output.StackOutput._get_output_value") @@ -191,7 +192,7 @@ def test_resolve__badly_formatted(self, mock_get_output_value): "not_a_valid_stack_output", stack ) - with pytest.raises(ValueError, match="not enough values to unpack"): + with pytest.raises(SceptreException, match="STACK_NAME::OUTPUT_KEY"): stack_output_external_resolver.resolve() @patch("sceptre.resolvers.stack_output.StackOutputExternal._get_output_value") From f6b48d2c9bac6222480040ce317c0653eaacd9d6 Mon Sep 17 00:00:00 2001 From: Alex Harvey Date: Tue, 28 Nov 2023 02:12:25 +1100 Subject: [PATCH 07/13] linter --- sceptre/resolvers/stack_output.py | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/sceptre/resolvers/stack_output.py b/sceptre/resolvers/stack_output.py index aef84cef2..55375d415 100644 --- a/sceptre/resolvers/stack_output.py +++ b/sceptre/resolvers/stack_output.py @@ -6,8 +6,11 @@ from botocore.exceptions import ClientError -from sceptre.exceptions import DependencyStackMissingOutputError, StackDoesNotExistError, \ - SceptreException +from sceptre.exceptions import ( + DependencyStackMissingOutputError, + StackDoesNotExistError, + SceptreException, +) from sceptre.helpers import normalise_path, sceptreise_path from sceptre.resolvers import Resolver @@ -113,7 +116,9 @@ def setup(self): try: dep_stack_name, self.output_key = self.argument.split("::") except ValueError as err: - raise SceptreException("StackOutput argument should match STACK_NAME::OUTPUT_KEY") from err + raise SceptreException( + "StackOutput argument should match STACK_NAME::OUTPUT_KEY" + ) from err self.dependency_stack_name = sceptreise_path(normalise_path(dep_stack_name)) self.stack.dependencies.append(self.dependency_stack_name) @@ -135,7 +140,9 @@ def resolve(self): if stack.name == friendly_stack_name ) except StopIteration as err: - raise SceptreException(f"Stack '{friendly_stack_name}' not found in dependencies") from err + raise SceptreException( + f"Stack '{friendly_stack_name}' not found in dependencies" + ) from err stack_name = "-".join( [stack.project_code, friendly_stack_name.replace("/", "-")] @@ -180,7 +187,9 @@ def resolve(self): if len(arguments) > 1: try: extra_args = arguments[1].split("::", 2) - profile, region, sceptre_role = extra_args + (3 - len(extra_args)) * [None] + profile, region, sceptre_role = extra_args + (3 - len(extra_args)) * [ + None + ] except ValueError as err: message = ( "!stack_output_external second arg should be " From 94e43711dbf195748ace4476dc448b8f8500e4bc Mon Sep 17 00:00:00 2001 From: Alex Harvey Date: Tue, 28 Nov 2023 10:18:51 +1100 Subject: [PATCH 08/13] correction --- sceptre/resolvers/stack_output.py | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/sceptre/resolvers/stack_output.py b/sceptre/resolvers/stack_output.py index 55375d415..72aa6d9d7 100644 --- a/sceptre/resolvers/stack_output.py +++ b/sceptre/resolvers/stack_output.py @@ -133,16 +133,11 @@ def resolve(self): self.logger.debug("Resolving Stack output: {0}".format(self.argument)) friendly_stack_name = self.dependency_stack_name.replace(TEMPLATE_EXTENSION, "") - try: - stack = next( - stack - for stack in self.stack.dependencies - if stack.name == friendly_stack_name - ) - except StopIteration as err: - raise SceptreException( - f"Stack '{friendly_stack_name}' not found in dependencies" - ) from err + stack = next( + stack + for stack in self.stack.dependencies + if stack.name == friendly_stack_name + ) stack_name = "-".join( [stack.project_code, friendly_stack_name.replace("/", "-")] From 21969b7586e342a0e37c06ba3561b21e9d0708b3 Mon Sep 17 00:00:00 2001 From: Alex Harvey Date: Tue, 28 Nov 2023 10:20:20 +1100 Subject: [PATCH 09/13] correction --- sceptre/resolvers/stack_output.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sceptre/resolvers/stack_output.py b/sceptre/resolvers/stack_output.py index 72aa6d9d7..3ffcecffd 100644 --- a/sceptre/resolvers/stack_output.py +++ b/sceptre/resolvers/stack_output.py @@ -117,7 +117,7 @@ def setup(self): dep_stack_name, self.output_key = self.argument.split("::") except ValueError as err: raise SceptreException( - "StackOutput argument should match STACK_NAME::OUTPUT_KEY" + "!stack_output arg should match STACK_NAME::OUTPUT_KEY" ) from err self.dependency_stack_name = sceptreise_path(normalise_path(dep_stack_name)) From 97d1f4c6c80a995a0e37666982957e9bd874ae61 Mon Sep 17 00:00:00 2001 From: Alex Harvey Date: Thu, 30 Nov 2023 21:24:56 +1100 Subject: [PATCH 10/13] cleanup --- sceptre/resolvers/stack_output.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/sceptre/resolvers/stack_output.py b/sceptre/resolvers/stack_output.py index 3ffcecffd..9e8fe814d 100644 --- a/sceptre/resolvers/stack_output.py +++ b/sceptre/resolvers/stack_output.py @@ -179,16 +179,22 @@ def resolve(self): arguments = shlex.split(self.argument) stack_argument = arguments[0] + if len(arguments) > 1: try: extra_args = arguments[1].split("::", 2) - profile, region, sceptre_role = extra_args + (3 - len(extra_args)) * [ - None - ] + + if len(extra_args) > 0: + profile = extra_args[0] + if len(extra_args) > 1: + region = extra_args[1] + if len(extra_args) > 2: + sceptre_role = extra_args[2] + except ValueError as err: message = ( "!stack_output_external second arg should be " - "in the format 'profile::region::sceptre_role'" + "in the format 'PROFILE[::REGION[::SCEPTRE_ROLE]]'" ) raise SceptreException(message) from err From 6c44d8f517db4c16d5974aaa3f0d334c1bfeabcd Mon Sep 17 00:00:00 2001 From: Alex Harvey Date: Thu, 30 Nov 2023 21:33:08 +1100 Subject: [PATCH 11/13] tests --- tests/test_resolvers/test_stack_output.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/tests/test_resolvers/test_stack_output.py b/tests/test_resolvers/test_stack_output.py index f4aef0158..0d671ac61 100644 --- a/tests/test_resolvers/test_stack_output.py +++ b/tests/test_resolvers/test_stack_output.py @@ -59,7 +59,10 @@ def test_resolver__badly_formatted(self, mock_get_output_value): stack_output_resolver = StackOutput("not_a_valid_stack_output", stack) - with pytest.raises(SceptreException, match="STACK_NAME::OUTPUT_KEY"): + with pytest.raises( + SceptreException, + match="!stack_output arg should match STACK_NAME::OUTPUT_KEY", + ): stack_output_resolver.setup() @patch("sceptre.resolvers.stack_output.StackOutput._get_output_value") @@ -192,7 +195,10 @@ def test_resolve__badly_formatted(self, mock_get_output_value): "not_a_valid_stack_output", stack ) - with pytest.raises(SceptreException, match="STACK_NAME::OUTPUT_KEY"): + with pytest.raises( + SceptreException, + match="!stack_output_external arg should match STACK_NAME::OUTPUT_KEY", + ): stack_output_external_resolver.resolve() @patch("sceptre.resolvers.stack_output.StackOutputExternal._get_output_value") From 261d4865627b6d32cff2b1afcf5df7896bd92d97 Mon Sep 17 00:00:00 2001 From: Alex Harvey Date: Sun, 3 Dec 2023 23:30:00 +1100 Subject: [PATCH 12/13] Refactor to use an iterator --- sceptre/resolvers/stack_output.py | 48 ++++++++++++++++--------------- 1 file changed, 25 insertions(+), 23 deletions(-) diff --git a/sceptre/resolvers/stack_output.py b/sceptre/resolvers/stack_output.py index 9e8fe814d..994636cbc 100644 --- a/sceptre/resolvers/stack_output.py +++ b/sceptre/resolvers/stack_output.py @@ -173,41 +173,43 @@ def resolve(self): """ self.logger.debug("Resolving external Stack output: {0}".format(self.argument)) - profile = None - region = None - sceptre_role = None arguments = shlex.split(self.argument) + if not arguments: + message = "!stack_output_external requires at least one argument" + raise SceptreException(message) + stack_argument = arguments[0] + stack_args = iter(stack_argument.split("::")) + + try: + dependency_stack_name = next(stack_args) + output_key = next(stack_args) + + except StopIteration as err: + message = "!stack_output_external arg should match STACK_NAME::OUTPUT_KEY" + raise SceptreException(message) from err + + profile = region = sceptre_role = None if len(arguments) > 1: - try: - extra_args = arguments[1].split("::", 2) + extra_args = iter(arguments[1].split("::")) - if len(extra_args) > 0: - profile = extra_args[0] - if len(extra_args) > 1: - region = extra_args[1] - if len(extra_args) > 2: - sceptre_role = extra_args[2] + profile = next(extra_args, None) + region = next(extra_args, None) + sceptre_role = next(extra_args, None) - except ValueError as err: + try: + next(extra_args) message = ( "!stack_output_external second arg should be " "in the format 'PROFILE[::REGION[::SCEPTRE_ROLE]]'" ) - raise SceptreException(message) from err + raise SceptreException(message) - try: - dependency_stack_name, output_key = stack_argument.split("::") - except ValueError as err: - message = "!stack_output_external arg should match STACK_NAME::OUTPUT_KEY" - raise SceptreException(message) from err + except StopIteration: + pass return self._get_output_value( - dependency_stack_name, - output_key, - profile or None, - region or None, - sceptre_role or None, + dependency_stack_name, output_key, profile, region, sceptre_role ) From f00dfdf437f0f3ffbe8fd96da4e7122bc89e563e Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Fri, 26 Jan 2024 21:00:00 +0000 Subject: [PATCH 13/13] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- tests/test_resolvers/test_stack_output.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/test_resolvers/test_stack_output.py b/tests/test_resolvers/test_stack_output.py index f19215e57..0d671ac61 100644 --- a/tests/test_resolvers/test_stack_output.py +++ b/tests/test_resolvers/test_stack_output.py @@ -63,7 +63,6 @@ def test_resolver__badly_formatted(self, mock_get_output_value): SceptreException, match="!stack_output arg should match STACK_NAME::OUTPUT_KEY", ): - stack_output_resolver.setup() @patch("sceptre.resolvers.stack_output.StackOutput._get_output_value")