From db96da71d90bb5751546c01d30b91bea66d46c01 Mon Sep 17 00:00:00 2001 From: Gilles Wijnker Date: Fri, 11 Jun 2021 13:44:26 +0200 Subject: [PATCH 1/3] [Resolves 961] Fixed Hook having reference to wrong Stack when multiple YAMLs are processed --- sceptre/hooks/__init__.py | 18 +++--- sceptre/hooks/asg_scaling_processes.py | 16 +++--- sceptre/hooks/cmd.py | 2 +- .../test_hooks/test_asg_scaling_processes.py | 18 +++--- tests/test_hooks/test_cmd.py | 11 ++-- tests/test_hooks/test_hooks.py | 56 +++++++++++++++++-- 6 files changed, 83 insertions(+), 38 deletions(-) diff --git a/sceptre/hooks/__init__.py b/sceptre/hooks/__init__.py index e987d336c..f7f213dc4 100644 --- a/sceptre/hooks/__init__.py +++ b/sceptre/hooks/__init__.py @@ -11,15 +11,12 @@ class Hook(object): :param argument: The argument of the hook. :type argument: str - :param stack: The associated stack of the hook. - :type stack: sceptre.stack.Stack """ __metaclass__ = abc.ABCMeta - def __init__(self, argument=None, stack=None): + def __init__(self, argument=None): self.logger = logging.getLogger(__name__) self.argument = argument - self.stack = stack def setup(self): """ @@ -29,10 +26,13 @@ def setup(self): pass # pragma: no cover @abc.abstractmethod - def run(self): + def run(self, stack): """ run is an abstract method which must be overwritten by all inheriting classes. Run should execute the logic of the hook. + + :param stack: The associated stack of the hook. + :type stack: sceptre.stack.Stack """ pass # pragma: no cover @@ -74,7 +74,7 @@ def setup(attr, key, value): setattr(instance, self.name, value) -def execute_hooks(hooks): +def execute_hooks(hooks, stack): """ Searches through dictionary or list for Resolver objects and replaces them with the resolved value. Supports nested dictionaries and lists. @@ -88,7 +88,7 @@ def execute_hooks(hooks): if isinstance(hooks, list): for hook in hooks: if isinstance(hook, Hook): - hook.run() + hook.run(stack) def add_stack_hooks(func): @@ -100,9 +100,9 @@ def add_stack_hooks(func): """ @wraps(func) def decorated(self, *args, **kwargs): - execute_hooks(self.stack.hooks.get("before_" + func.__name__)) + execute_hooks(self.stack.hooks.get("before_" + func.__name__), self.stack) response = func(self, *args, **kwargs) - execute_hooks(self.stack.hooks.get("after_" + func.__name__)) + execute_hooks(self.stack.hooks.get("after_" + func.__name__), self.stack) return response diff --git a/sceptre/hooks/asg_scaling_processes.py b/sceptre/hooks/asg_scaling_processes.py index f377265c5..0d5ad8d21 100644 --- a/sceptre/hooks/asg_scaling_processes.py +++ b/sceptre/hooks/asg_scaling_processes.py @@ -17,7 +17,7 @@ class ASGScalingProcesses(Hook): def __init__(self, *args, **kwargs): super(ASGScalingProcesses, self).__init__(*args, **kwargs) - def run(self): + def run(self, stack): """ Either suspends or resumes any scaling processes on all autoscaling groups within the current stack. @@ -51,9 +51,9 @@ def run(self): action += "_processes" - autoscaling_group_names = self._find_autoscaling_groups() + autoscaling_group_names = self._find_autoscaling_groups(stack) for autoscaling_group in autoscaling_group_names: - self.stack.connection_manager.call( + stack.connection_manager.call( service="autoscaling", command=action, kwargs={ @@ -62,25 +62,25 @@ def run(self): } ) - def _get_stack_resources(self): + def _get_stack_resources(self, stack): """ Retrieves all resources in stack. :return: list """ - response = self.stack.connection_manager.call( + response = stack.connection_manager.call( service="cloudformation", command="describe_stack_resources", - kwargs={"StackName": self.stack.external_name} + kwargs={"StackName": stack.external_name} ) return response.get("StackResources", []) - def _find_autoscaling_groups(self): + def _find_autoscaling_groups(self, stack): """ Retrieves all the autoscaling groups :return: list [str] """ asg_names = [] - resources = self._get_stack_resources() + resources = self._get_stack_resources(stack) resource_type = "AWS::AutoScaling::AutoScalingGroup" for resource in resources: if resource.get("ResourceType", False) == resource_type: diff --git a/sceptre/hooks/cmd.py b/sceptre/hooks/cmd.py index 4cf6a1f8b..d20556471 100644 --- a/sceptre/hooks/cmd.py +++ b/sceptre/hooks/cmd.py @@ -11,7 +11,7 @@ class Cmd(Hook): def __init__(self, *args, **kwargs): super(Cmd, self).__init__(*args, **kwargs) - def run(self): + def run(self, stack): """ Runs the argument string in a subprocess. diff --git a/tests/test_hooks/test_asg_scaling_processes.py b/tests/test_hooks/test_asg_scaling_processes.py index 99c634728..699d14f36 100644 --- a/tests/test_hooks/test_asg_scaling_processes.py +++ b/tests/test_hooks/test_asg_scaling_processes.py @@ -16,7 +16,7 @@ def setup_method(self, test_method): self.stack = MagicMock(spec=Stack) self.stack.connection_manager = MagicMock(spec=ConnectionManager) self.stack.external_name = "external_name" - self.asg_scaling_processes = ASGScalingProcesses(None, self.stack) + self.asg_scaling_processes = ASGScalingProcesses(None) def test_get_stack_resources_sends_correct_request(self): self.stack.connection_manager.call.return_value = { @@ -27,7 +27,7 @@ def test_get_stack_resources_sends_correct_request(self): } ] } - self.asg_scaling_processes._get_stack_resources() + self.asg_scaling_processes._get_stack_resources(self.stack) self.stack.connection_manager.call.assert_called_with( service="cloudformation", command="describe_stack_resources", @@ -51,7 +51,7 @@ def test_find_autoscaling_groups_with_stack_with_asgs( 'StackId': 'arn:aws:...', 'StackName': 'cloudreach-examples-dev-vpc' }] - response = self.asg_scaling_processes._find_autoscaling_groups() + response = self.asg_scaling_processes._find_autoscaling_groups(self.stack) assert response == ["cloudreach-examples-asg"] @@ -63,7 +63,7 @@ def test_find_autoscaling_groups_with_stack_without_asgs( self, mock_get_stack_resources ): mock_get_stack_resources.return_value = [] - response = self.asg_scaling_processes._find_autoscaling_groups() + response = self.asg_scaling_processes._find_autoscaling_groups(self.stack) assert response == [] @@ -74,7 +74,7 @@ def test_find_autoscaling_groups_with_stack_without_asgs( def test_run_with_resume_argument(self, mock_find_autoscaling_groups): self.asg_scaling_processes.argument = u"resume::ScheduledActions" mock_find_autoscaling_groups.return_value = ["autoscaling_group_1"] - self.asg_scaling_processes.run() + self.asg_scaling_processes.run(self.stack) self.stack.connection_manager.call.assert_called_once_with( service="autoscaling", command="resume_processes", @@ -93,7 +93,7 @@ def test_run_with_resume_argument(self, mock_find_autoscaling_groups): def test_run_with_suspend_argument(self, mock_find_autoscaling_groups): self.asg_scaling_processes.argument = "suspend::ScheduledActions" mock_find_autoscaling_groups.return_value = ["autoscaling_group_1"] - self.asg_scaling_processes.run() + self.asg_scaling_processes.run(self.stack) self.stack.connection_manager.call.assert_called_once_with( service="autoscaling", command="suspend_processes", @@ -115,7 +115,7 @@ def test_run_with_invalid_string_argument( self.asg_scaling_processes.argument = u"invalid_string" mock_find_autoscaling_groups.return_value = ["autoscaling_group_1"] with pytest.raises(InvalidHookArgumentSyntaxError): - self.asg_scaling_processes.run() + self.asg_scaling_processes.run(self.stack) @patch( "sceptre.hooks.asg_scaling_processes" @@ -125,7 +125,7 @@ def test_run_with_unsupported_argument(self, mock_find_autoscaling_groups): self.asg_scaling_processes.argument = "start::Healthcheck" mock_find_autoscaling_groups.return_value = ["autoscaling_group_1"] with pytest.raises(InvalidHookArgumentValueError): - self.asg_scaling_processes.run() + self.asg_scaling_processes.run(self.stack) @patch( "sceptre.hooks.asg_scaling_processes" @@ -135,4 +135,4 @@ def test_run_with_non_string_argument(self, mock_find_autoscaling_groups): self.asg_scaling_processes.argument = 10 mock_find_autoscaling_groups.return_value = ["autoscaling_group_1"] with pytest.raises(InvalidHookArgumentTypeError): - self.asg_scaling_processes.run() + self.asg_scaling_processes.run(self.stack) diff --git a/tests/test_hooks/test_cmd.py b/tests/test_hooks/test_cmd.py index bd8bdfbbd..eddf08552 100644 --- a/tests/test_hooks/test_cmd.py +++ b/tests/test_hooks/test_cmd.py @@ -1,11 +1,12 @@ # -*- coding: utf-8 -*- +import subprocess + import pytest from mock import patch -import subprocess -from sceptre.hooks.cmd import Cmd from sceptre.exceptions import InvalidHookArgumentTypeError +from sceptre.hooks.cmd import Cmd class TestCmd(object): @@ -15,12 +16,12 @@ def setup_method(self, test_method): def test_run_with_non_str_argument(self): self.cmd.argument = None with pytest.raises(InvalidHookArgumentTypeError): - self.cmd.run() + self.cmd.run(None) @patch('sceptre.hooks.cmd.subprocess.check_call') def test_run_with_str_argument(self, mock_call): self.cmd.argument = u"echo hello" - self.cmd.run() + self.cmd.run(None) mock_call.assert_called_once_with(u"echo hello", shell=True) @patch('sceptre.hooks.cmd.subprocess.check_call') @@ -28,4 +29,4 @@ def test_run_with_erroring_command(self, mock_call): mock_call.side_effect = subprocess.CalledProcessError(1, "echo") self.cmd.argument = u"echo hello" with pytest.raises(subprocess.CalledProcessError): - self.cmd.run() + self.cmd.run(None) diff --git a/tests/test_hooks/test_hooks.py b/tests/test_hooks/test_hooks.py index 3d1497a32..a499f9000 100644 --- a/tests/test_hooks/test_hooks.py +++ b/tests/test_hooks/test_hooks.py @@ -2,6 +2,8 @@ from mock import MagicMock from sceptre.hooks import Hook, HookProperty, add_stack_hooks, execute_hooks +from sceptre.plan.actions import StackActions +from sceptre.stack import Stack class MockHook(Hook): @@ -14,7 +16,7 @@ def run(self): class TestHooksFunctions(object): def setup_method(self, test_method): - pass + self.stack = MagicMock(spec=Stack) def test_add_stack_hooks(self): mock_hook_before = MagicMock(spec=Hook) @@ -39,23 +41,23 @@ def mock_function(self): assert mock_hook_after.run.call_count == 1 def test_execute_hooks_with_not_a_list(self): - execute_hooks(None) + execute_hooks(None, self.stack) def test_execute_hooks_with_empty_list(self): - execute_hooks([]) + execute_hooks([], self.stack) def test_execute_hooks_with_list_with_non_hook_objects(self): - execute_hooks([None, "string", 1, True]) + execute_hooks([None, "string", 1, True], self.stack) def test_execute_hooks_with_single_hook(self): hook = MagicMock(spec=Hook) - execute_hooks([hook]) + execute_hooks([hook], self.stack) hook.run.called_once_with() def test_execute_hooks_with_multiple_hook(self): hook_1 = MagicMock(spec=Hook) hook_2 = MagicMock(spec=Hook) - execute_hooks([hook_1, hook_2]) + execute_hooks([hook_1, hook_2], self.stack) hook_1.run.called_once_with() hook_2.run.called_once_with() @@ -87,3 +89,45 @@ def test_setting_hook_property(self): def test_getting_hook_property(self): self.mock_object._hook_property = self.mock_object assert self.mock_object.hook_property == self.mock_object + + +class MultipleHook(Hook): + def __init__(self, *args, **kwargs): + super(MultipleHook, self).__init__(*args, **kwargs) + + def run(self, stack): + pass + + +class TestHooksMultipleConfig(object): + def setup_method(self, test_method): + pass + + def test_add_stack_hooks(self): + sub_stack_one = MagicMock(spec=StackActions) + sub_stack_one.stack = MagicMock(spec=Stack) + sub_stack_one.name = 'sub_stack_one' + sub_stack_two = MagicMock(spec=StackActions) + sub_stack_two.stack = MagicMock(spec=Stack) + sub_stack_two.name = 'sub_stack_two' + hook_before = MagicMock(MultipleHook) + hook_after = MagicMock(MultipleHook) + + sub_stack_one.stack.hooks = sub_stack_two.stack.hooks = { + 'before_mock_function': [hook_before], + 'after_mock_function': [hook_after] + } + + def mock_function(self): + pass + + sub_stack_one.mock_function = sub_stack_two.mock_function = mock_function + sub_stack_one.mock_function.__name__ = sub_stack_two.mock_function.__name__ = 'mock_function' + + add_stack_hooks(sub_stack_one.mock_function)(sub_stack_one) + hook_before.run.assert_called_with(sub_stack_one.stack) + hook_after.run.assert_called_with(sub_stack_one.stack) + + add_stack_hooks(sub_stack_two.mock_function)(sub_stack_two) + hook_before.run.assert_called_with(sub_stack_two.stack) + hook_after.run.assert_called_with(sub_stack_two.stack) From 727617cb9716ffc0e1c112cdf404996dec8b109c Mon Sep 17 00:00:00 2001 From: Gilles Wijnker Date: Fri, 11 Jun 2021 13:47:20 +0200 Subject: [PATCH 2/3] [Resolves 961] Fixed tests to actually assert condition --- tests/test_hooks/test_hooks.py | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/tests/test_hooks/test_hooks.py b/tests/test_hooks/test_hooks.py index a499f9000..12c9c51d1 100644 --- a/tests/test_hooks/test_hooks.py +++ b/tests/test_hooks/test_hooks.py @@ -52,14 +52,14 @@ def test_execute_hooks_with_list_with_non_hook_objects(self): def test_execute_hooks_with_single_hook(self): hook = MagicMock(spec=Hook) execute_hooks([hook], self.stack) - hook.run.called_once_with() + hook.run.assert_called_once_with(self.stack) def test_execute_hooks_with_multiple_hook(self): hook_1 = MagicMock(spec=Hook) hook_2 = MagicMock(spec=Hook) execute_hooks([hook_1, hook_2], self.stack) - hook_1.run.called_once_with() - hook_2.run.called_once_with() + hook_1.run.assert_called_once_with(self.stack) + hook_2.run.assert_called_once_with(self.stack) class TestHook(object): @@ -91,15 +91,7 @@ def test_getting_hook_property(self): assert self.mock_object.hook_property == self.mock_object -class MultipleHook(Hook): - def __init__(self, *args, **kwargs): - super(MultipleHook, self).__init__(*args, **kwargs) - - def run(self, stack): - pass - - -class TestHooksMultipleConfig(object): +class TestHooksMultipleStacks(object): def setup_method(self, test_method): pass @@ -110,8 +102,8 @@ def test_add_stack_hooks(self): sub_stack_two = MagicMock(spec=StackActions) sub_stack_two.stack = MagicMock(spec=Stack) sub_stack_two.name = 'sub_stack_two' - hook_before = MagicMock(MultipleHook) - hook_after = MagicMock(MultipleHook) + hook_before = MagicMock(MockHook) + hook_after = MagicMock(MockHook) sub_stack_one.stack.hooks = sub_stack_two.stack.hooks = { 'before_mock_function': [hook_before], From bea0fd62803e4174948c285c68ea3da6fda7e5fb Mon Sep 17 00:00:00 2001 From: Gilles Wijnker Date: Fri, 11 Jun 2021 14:57:55 +0200 Subject: [PATCH 3/3] [Resolves 961] Updated documentation --- docs/_source/docs/hooks.rst | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/docs/_source/docs/hooks.rst b/docs/_source/docs/hooks.rst index 3d6e59252..d478925c8 100644 --- a/docs/_source/docs/hooks.rst +++ b/docs/_source/docs/hooks.rst @@ -116,10 +116,10 @@ integrate additional functionality into Sceptre projects. A hook is a Python class which inherits from abstract base class ``Hook`` found in the ``sceptre.hooks module``. -Hooks are require to implement a ``run()`` function that takes no parameters -and to call the base class initializer. +Hooks are require to implement a ``run(stack)`` function that takes a single parameter +``stack`` and to call the base class initializer. -Hooks may have access to ``argument``, and ``stack`` as object attributes. For example ``self.stack``. +Hooks may have access to the ``argument`` object attribute. For example ``self.argument``. Sceptre uses the ``sceptre.hooks`` entry point to locate hook classes. Your custom hook can be written anywhere and is installed as Python package. @@ -152,26 +152,28 @@ custom_hook.py argument: str The argument is available from the base class and contains the argument defined in the Sceptre config file (see below) - stack: sceptre.stack.Stack - The associated stack of the hook. - connection_manager: sceptre.connection_manager.ConnectionManager - Boto3 Connection Manager - can be used to call boto3 api. """ def __init__(self, *args, **kwargs): super(CustomHook, self).__init__(*args, **kwargs) - def run(self): + def run(self, stack): """ run is the method called by Sceptre. It should carry out the work intended by this hook. + Parameters + ---------- + stack: sceptre.stack.Stack + The associated stack of the hook. + stack.connection_manager: sceptre.connection_manager.ConnectionManager + Boto3 Connection Manager - can be used to call boto3 api. + To use instance attribute self.. Examples -------- self.argument - self.stack_config """ print(self.argument)