Resolve lookups in hook args

[ITProKyle]

closes#688

This change allows for the resolution of all stacker.lookups passed into a hook's args without needing to add a handler to each hook.

Previously, only lookups to the provided .env were resolvable or logic had to be added to hooks individually to handle lookups.

Example Usage

pre_build:
testing-hook-lookup-resolve:
path: hooks.test_hook.testargs:
rxref_lookup: ${rxref vpc::VpcId}nested_dict:
xref_lookup: ${xref vpc::VpcCidrBlock}list_lookups:
- ${default env_var::default_value}

Output

[2019-03-09T19:11:54] Using default AWS provider mode
[2019-03-09T19:11:54] Executing pre_build hooks: hooks.test_hook.test
[2019-03-09T19:11:54] rxref_lookup: vpc-xxxxxx
[2019-03-09T19:11:54] list_lookups: ['default_value']
[2019-03-09T19:11:54] nested_dict: {'xref_lookup': '10.0.0.0/16'}
[2019-03-09T19:11:54] ignore-this-stack: skipped (disabled)

[Spareo]

Hope this gets merged soon, this would be a super useful feature.

[phobologic]

Sorry for the long delay - dug into this this weekend, and it looks pretty good. One big concern (which I put inline) and also I think we'll need docs to detail how to use this, it's limitations, etc.

Thanks!

[phobologic]

Any idea how this would work with the output lookup, which changes the order of execution? I think it will likely break- especially if it's a pre_build hook that runs before the graph gets executed. Even with that, I think this is still good - we just might want to handle that case, and explicitly indicate somehow that lookups that change the order of execution can only run as post_build hooks.

[ITProKyle]

I tested output with a post_build hook, it works as expected. However, you are right that it does not work in pre_build. It raises FailedVariableLookup and uses the closest key in args as the variable in the exception.

I added a log output that explains the use of output and similar lookups in args. With the condition used it won't trigger for failed lookups that include another exception like StackDoesNotExist. If you have a better idea, please let me know. I avoided checking the lookups provided here before trying to resolve them here to not add time. could maybe pass stage to resolve_variables/Variable.resolve() if we do want to add validation logic around it?

[phobologic]

Hey @ITProKyle - thanks for this. I think we might want to hold off on it in favor of #722 however, as that does this + adds the ability for hooks to be put directly into the execution graph, so you no longer only have pre/post build hooks. What do you think?

[ITProKyle]

@phobologic - that works. I haven't dug too deep into #722 yet to see how it's being done but as long as lookups can be resolved natively in hook args - that's all that matters.

[Spareo]

Is there something still holding this back from getting merged? This would make lookups 1000% more useful in my opinion.

[josjaf]

I'd love to see this merged as well!

[Spareo]

Any updates on this or details on what it will take to get it merged? If its not much I can try to finish it up.

[ITProKyle]

@Spareo this was paused because of a pending refactor of how hooks work that should take of this.

In the meantime, I have been added something similar to what is below to all my custom hooks which effectively does the same thing as this PR. However, it wouldn't help with any of the built-in hooks.

importloggingfromstacker.variablesimportVariable, resolve_variablesLOGGER=logging.getLogger(__name__)
defget_variables(variables, provider, context):
converted_variables= [
Variable(k, v) fork, vinvariables.items()
]
resolve_variables(
converted_variables, context, provider
)
return {v.name: v.valueforvinconverted_variables}
defexample_hook(provider, context, **kwargs):
"""The function being called as a hook."""variables=get_variables(kwargs, provider, context)
fork, vinvariables.items():
LOGGER.info('%s: %s', k, v)
returnvariables