cloudtools / stacker

An AWS CloudFormation Stack orchestrator/manager.
http://stacker.readthedocs.io/en/stable/
BSD 2-Clause "Simplified" License
711 stars 167 forks source link

[WIP/RFC] Integrate hooks into stack execution graph #722

Open danielkza opened 5 years ago

danielkza commented 5 years ago

Make hooks "first-class" citizens in the Stacker execution engine, allowing them to depend on/be depended upon by stacks and targets.

This is not yet 100% complete, but I'm posting to gather some early reviews and advice on the config changes needed.

There are two new top level config. keys introduced: build_hooks and destroy_hooks. They contain lists of hooks specifications, much like what was already present in {pre,post}_{build,destroy}. But the hooks in there are expected to order themselves in relation to stacks using the requires / required_by keys or lookups (which are now resolved), very much like stacks themselves.

Hooks now also should retrieve a boto3.Session using Provider.get_session, respecting the profile and region options they now have.


Apologies for the size of the changes, but this required some deep refactorings.


There are a couple of additional unrelated changes currently still in this branch. I initially made them to be able to use the AWS Account ID as part of the bucket name in some tests, but I figured out they are not strictly needed. I will refactor them out, but can still make separate PRs if they seem useful.

1) Make lookups be able to return Troposphere objects, and properly generate Joins to concatenate variable parts if needed. 2) Using 1, implement an awsparam lookup that can interpolate CloudFormation pseudo-parameters into strings. ex: ${awsparam AccountID}

This depends on #714. A few tests were refactored to use pytest fixtures when appropriate (for example, to properly handle creation of boto3 clients with different regions).

danielkza commented 5 years ago

Rebased and cleaned up - removed a bunch of unnecessary changes and collapsed some commits to be teasier to follow.

phobologic commented 5 years ago

This looks good from my first pass - I like the way it's laid out. @ejholmes can you take a quick look at the plan/graph stuff, just to be sure we have another set of eyes on it?

Also, it looks like this will need a bit of doc updates for the new config options, etc.

One question: Do you think it's worth separating build/destroy hooks, or instead just having the hook classes themselves know how to behave in each situation (a build/destroy method)? I'm kind of on the fence - if I had a hook that created a bucket in S3 on build, not sure if I would want it to destroy that same bucket on destroy.

Thanks for this @danielkza !

Amir-Ahmad commented 3 years ago

Any chance of getting this one merged?