Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Resolves #961]: Hook.stack is not necessarily the stack actually being updated #1057

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 11 additions & 9 deletions docs/_source/docs/hooks.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the contribution however I don't think we can break every Sceptre custom hook out in the wild. I would prefer a more general solution that would allow hooks or resolvers to access the stack object.

If this is the only solution then i think it would need to be done with a new interface and then deprecating the old one.

"""
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.<attribute_name>.

Examples
--------
self.argument
self.stack_config

"""
print(self.argument)
Expand Down
18 changes: 9 additions & 9 deletions sceptre/hooks/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
"""
Expand All @@ -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

Expand Down Expand Up @@ -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.
Expand All @@ -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):
Expand All @@ -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

Expand Down
16 changes: 8 additions & 8 deletions sceptre/hooks/asg_scaling_processes.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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={
Expand All @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion sceptre/hooks/cmd.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
18 changes: 9 additions & 9 deletions tests/test_hooks/test_asg_scaling_processes.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand All @@ -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",
Expand All @@ -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"]

Expand All @@ -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 == []

Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -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"
Expand All @@ -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"
Expand All @@ -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)
11 changes: 6 additions & 5 deletions tests/test_hooks/test_cmd.py
Original file line number Diff line number Diff line change
@@ -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):
Expand All @@ -15,17 +16,17 @@ 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')
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)
54 changes: 45 additions & 9 deletions tests/test_hooks/test_hooks.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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)
Expand All @@ -39,25 +41,25 @@ 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])
hook.run.called_once_with()
execute_hooks([hook], self.stack)
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])
hook_1.run.called_once_with()
hook_2.run.called_once_with()
execute_hooks([hook_1, hook_2], self.stack)
hook_1.run.assert_called_once_with(self.stack)
hook_2.run.assert_called_once_with(self.stack)


class TestHook(object):
Expand Down Expand Up @@ -87,3 +89,37 @@ 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 TestHooksMultipleStacks(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(MockHook)
hook_after = MagicMock(MockHook)

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)