Sceptre/sceptre

ConnectionManager's call() method is too easy to misuse by not passing enough variables.

jfalkenstein opened this issue · 2 comments

Subject of the issue

The ConnectionManager is used by many hooks, plugins, and template managers. It is essential for interacting with AWS using the StackConfig's values. However, as a result of how it operates, it can (and often is) misused accidentally.

The source of the issue is here:
image

The problem is that this code assumes that if someone wants to use the Stack default configurations, one would pass None for region, profile, and sceptre_role. If all three are None, it is assumed that the stack values ought to be utilized. But if any one of those is not None, then it is assumed that all three have been specified and all three are used.

The result of this is we end up with outdated hooks and resolvers (such as the SSM Resolver) that are passing the profile and region, but not the sceptre_role (aka iam_role). Since region is always specified (by necessity), it will mean the iam_role parameter is overridden and set to None.

Fundamentally, the issue is that there isn't here a distinction between setting a value to None to nullify a setting (meaning "don't use any iam_role") and setting a value to None to retain the stack's configuration.

The solution to this is set the default values for call() to a third constant that is interpreted as the Stack default, similar to how the new get_session() method operates:

image

This is awesome. Thanks for this!
Will this go right into the next release of sceptre 3.2.XXX. ?

@SpyderDave, I'm working to get this into the v4 release. It's a major version, as there are some potentially breaking changes for a few edge cases. However, it'll be pretty feature-packed. You can see the current changelog here: https://github.com/Sceptre/sceptre/pull/1292/files#diff-06572a96a58dc510037d5efa622f9bec8519bc1beab13c9f251e97e657a9d4ed. This will get included there once I can get #1300 merged.