Stack attribute in sceptre.hooks.Hook is not thread-safe

[gilleswijnker]

I am using a custom Hook because I want to report the start and end time of updating our stacks. I also want to know the name of the Stack for which the Hook has been called. According to the documentation this is done with self.stack.name. See this simplified example:

class CustomHook(Hook):
    def __init__(self, *args, **kwargs):
        super(CustomHook, self).__init__(*args, **kwargs)

    def run(self):
        print(f'Hook: {self.stack.name}')

The content of self.stack seems to be overwritten by other concurrent running updates, probably because all use the same CustomHook-instance. So when I call self.stack.name I am not getting the information of stack for which the Hook has been called, but the information of the latest stack that has been assigned to the CustomHook-instance.

To clarify this, I have added a print-statement to the add_stack_hooks(func) in hooks.__init__:

def add_stack_hooks(func):
    """
    A function decorator to trigger the before and after hooks, relative
    to the decorated function's name.
    :param func: a function that operates on a stack
    :type func: function
    """
    @wraps(func)
    def decorated(self, *args, **kwargs):
        print(f'before_{func.__name__}: {self.stack.name} {self.stack.hooks}')
        execute_hooks(self.stack.hooks.get("before_" + func.__name__))
        response = func(self, *args, **kwargs)
        execute_hooks(self.stack.hooks.get("after_" + func.__name__))

        return response

    return decorated

I have the following in my config/config.yaml:

profile: [....]
project_code: [....]
hooks:
  before_update:
    - !stack_deploy_time "deploy_start"
  after_update:
    - !stack_deploy_time "deploy_end"

The output will be something like this (I have sanitized it to be readable):

before_update: d/e/f {'before_update': [<hook.hook.CustomHook object at 0x7fdb5ffe2ed0>]} Hook: a/b/c
before_update: x/y/z {'before_update': [<hook.hook.CustomHook object at 0x7fdb5ff09e90>]} Hook: x/y/z
before_update: g/h/i {'before_update': [<hook.hook.CustomHook object at 0x7fdb5ffe2ed0>]} Hook: a/b/c
before_update: j/k/l {'before_update': [<hook.hook.CustomHook object at 0x7fdb5ffe2ed0>]} Hook: a/b/c
before_update: m/n/o {'before_update': [<hook.hook.CustomHook object at 0x7fdb5ffe2ed0>]} Hook: a/b/c

Only one Hook has the correct stack available (the x/y/z). The other four hooks all have a/b/c as stack, which is different than the one being updated. Note how these four all use the same hook-instance, which probably causes this problem.

Because the correct stack is available in the add_stack_hooks-decorator, I suggest adding the stack as parameter to the Hook.run method. The stack in the hook is than always the same as the one in the decorator. Also, sceptre can still get around with a limited amount of hook-instances. This would also mean that Hook.stack is no longer needed:

import abc
import logging
from functools import wraps

from sceptre.helpers import _call_func_on_values


class Hook(object):
    """
    Hook is an abstract base class that should be inherited by all hooks.

    :param argument: The argument of the hook.
    :type argument: str
    """
    __metaclass__ = abc.ABCMeta

    def __init__(self, argument=None):
        self.logger = logging.getLogger(__name__)
        self.argument = argument

    def setup(self):
        """
        setup is a method that may be overwritten by inheriting classes. Allows
        hooks to run so initalisation steps when config is first read.
        """
        pass  # pragma: no cover

    @abc.abstractmethod
    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


class HookProperty(object):
    [... as is ...]


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.
    Does not detect Resolver objects used as keys in dictionaries.

    :param attr: A complex data structure to search through.
    :type attr: dict or list
    :param stack: The associated stack of the hook.
    :type stack: sceptre.stack.Stack
    :return: A complex data structure without Resolver objects.
    :rtype: dict or list
    """
    if isinstance(hooks, list):
        for hook in hooks:
            if isinstance(hook, Hook):
                hook.run(stack)


def add_stack_hooks(func):
    """
    A function decorator to trigger the before and after hooks, relative
    to the decorated function's name.
    :param func: a function that operates on a stack
    :type func: function
    """
    @wraps(func)
    def decorated(self, *args, **kwargs):
        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__), self.stack)

        return response

    return decorated

[craighurley]

Hi @gilleswijnker , do you think you could contribute a PR to help resolve this issue?

[gilleswijnker]

Hi @craighurley , sure I can. It will take some time because I'm quite busy at the moment.

For anyone having the same issue, we have worked around this issue by getting the correct stack from the call stack:

class CustomHook(Hook):
    def run(self):
        stack = self._get_stack()
        self.lambda_handler(self.argument, stack)
    
    # other methods, init etc.

    @staticmethod
    def _get_stack():
        #  Get reference to 'decorated' function in call stack. This is where sceptre hooks are applied.
        #  Moreover, the 'decorated' function has a reference to StackActions containing the correct Stack-instance.
        fr = next(stack for stack in inspect.stack() if stack.function == 'decorated')[0]
        args, _, _, value_dict = inspect.getargvalues(fr)
        instance = value_dict['self'] if len(args) and args[0] == 'self' else None
        return instance.stack if isinstance(instance, StackActions) else None