[Resolve #1299] Making ConnectionManager.call() work the way we all w…

…ant it to (#1300)

sceptre/connection_manager.py

@@ -13,7 +13,7 @@
 import threading
 import time
 import warnings
-from typing import Optional, Dict
+from typing import Optional, Dict, Tuple, Any
 
 import boto3
 import deprecation
@@ -72,11 +72,6 @@ def decorated(*args, **kwargs):
     return decorated
 
 
-# STACK_DEFAULT is a sentinel value meaning "default to the stack's configuration". This is in
-# contrast with passing None, which would mean "use no value".
-STACK_DEFAULT = "[STACK DEFAULT]"
-
-
 class ConnectionManager(object):
     """
     The Connection Manager is used to create boto3 clients for
@@ -89,6 +84,10 @@ class ConnectionManager(object):
     :param sceptre_role_session_duration: The duration to assume the specified sceptre_role per session.
     """
 
+    # STACK_DEFAULT is a sentinel value meaning "default to the stack's configuration". This is in
+    # contrast with passing None, which would mean "use no value".
+    STACK_DEFAULT = "[STACK DEFAULT]"
+
     _session_lock = threading.Lock()
     _client_lock = threading.Lock()
     _boto_sessions = {}
@@ -169,17 +168,24 @@ def get_session(
         :returns: The Boto3 session.
         :raises: botocore.exceptions.ClientError
         """
-        profile = self.profile if profile == STACK_DEFAULT else profile
-        region = self.region if region == STACK_DEFAULT else region
-        sceptre_role = (
-            self.sceptre_role if sceptre_role == STACK_DEFAULT else sceptre_role
+        profile, region, sceptre_role = self._determine_session_args(
+            profile, region, sceptre_role, iam_role
         )
-        if sceptre_role == STACK_DEFAULT and iam_role != STACK_DEFAULT:
-            self._emit_iam_role_deprecation_warning()
-            sceptre_role = iam_role
 
         return self._get_session(profile, region, sceptre_role)
 
+    def _determine_session_args(
+        self, profile: str, region: str, sceptre_role: str, iam_role: str
+    ) -> Tuple[str, str, str]:
+        profile = self.profile if profile == self.STACK_DEFAULT else profile
+        region = self.region if region == self.STACK_DEFAULT else region
+        sceptre_role = self._coalesce_sceptre_role(iam_role, sceptre_role)
+        sceptre_role = (
+            self.sceptre_role if sceptre_role == self.STACK_DEFAULT else sceptre_role
+        )
+
+        return profile, region, sceptre_role
+
     def _emit_iam_role_deprecation_warning(self):
         warnings.warn(
             deprecation.DeprecatedWarning(
@@ -369,43 +375,95 @@ def _get_client(self, service, region, profile, stack_name, sceptre_role):
     @_retry_boto_call
     def call(
         self,
-        service,
-        command,
-        kwargs=None,
-        profile=None,
-        region=None,
-        stack_name=None,
-        sceptre_role=None,
+        service: str,
+        command: str,
+        kwargs: Dict[str, Any] = None,
+        profile: Optional[str] = STACK_DEFAULT,
+        region: Optional[str] = STACK_DEFAULT,
+        stack_name: Optional[str] = None,
+        sceptre_role: Optional[str] = STACK_DEFAULT,
         *,
-        iam_role=None,
+        iam_role: Optional[str] = STACK_DEFAULT,
     ):
         """
         Makes a thread-safe Boto3 client call.
 
         Equivalent to ``boto3.client(<service>).<command>(**kwargs)``.
 
+        | Note regarding the profile, region, and sceptre_role parameters:
+        |    We will interpret each parameter individually this way:
+        |      * If the value passed is the STACK_DEFAULT constant, we'll assume it to mean we ought
+        |        to use the target stack's value of that parameter.
+        |      * If the value passed is None, we will interpret that as an explicit request to nullify
+        |        the target stack's setting. Note: While this is valid for profile and sceptre_role,
+        |        it will likely blow up if doing this for region, since AWS almost always requires that.
+        |      * Otherwise, any value that has been specified will override the target stack's
+        |        configuration, regardless of what has been passed for other parameters.
+
         :param service: The Boto3 service to return a client for.
-        :type service: str
         :param command: The Boto3 command to call.
-        :type command: str
         :param kwargs: The keyword arguments to supply to <command>.
-        :type kwargs: dict
+        :param profile: The profile to use when invoking the command; Defaults to the stack's configuration
+        :param region: The region to use when invoking the command; Default's to the stack's configuration
+        :param stack_name: The name of the stack whose configuration to use. Defaults to the current stack
+        :param sceptre_role: The IAM Role ARN to assume in order to invoke the command; Defaults to
+            the stack's configuration.
+        :param iam_role: DEPRECATED. Use sceptre_role instead.
         :returns: The response from the Boto3 call.
         """
-        if iam_role is not None:
-            self._emit_iam_role_deprecation_warning()
-            sceptre_role = iam_role
-
-        if region is None and profile is None and sceptre_role is None:
-            if stack_name and stack_name in self._stack_keys:
-                region, profile, sceptre_role = self._stack_keys[stack_name]
+        # If stack_name has been specified and we've already cached the region/profile/role
+        # configured for that stack, the "defaults" we'll use will be those of that stack rather then
+        # the defaults for the current ConnectionManager instance.
+        #
+        # stack_name is not used often and only really makes sense when we are acting inside Stack A
+        # but needing to interact with Stack B using Stack B's configurations. This is mostly only
+        # done when we're getting Stack B's outputs to resolve for Stack A's configuration.
+        if stack_name and stack_name in self._stack_keys:
+            stack_region, stack_profile, stack_sceptre_role = self._stack_keys[
+                stack_name
+            ]
+            sceptre_role = self._coalesce_sceptre_role(iam_role, sceptre_role)
+            # For historical/legacy purposes, if `None` is explicitly passed for all three parameters,
+            # this will be interpreted to mean we're going to use the profile/region/role configuration
+            # of the stack name. This could potentially interfere with an explicit attempt to nullify
+            # a setting; However, in that case, we'd need to be setting the region to None... which
+            # is unlikely, since that is a required stack configuration. This is the way this
+            # function has always operated, so to change this behavior could break or cause
+            # unexpected behavior elsewhere.
+            if (region, profile, sceptre_role) == (None, None, None):
+                region, profile, sceptre_role = (
+                    stack_region,
+                    stack_profile,
+                    stack_sceptre_role,
+                )
+            # In every other circumstance, we will interpret each parameter individually according
+            # to the way described in the docstring.
             else:
-                region = self.region
-                profile = self.profile
-                sceptre_role = self.sceptre_role
+                region = stack_region if region == self.STACK_DEFAULT else region
+                profile = stack_profile if profile == self.STACK_DEFAULT else profile
+                sceptre_role = (
+                    stack_sceptre_role
+                    if sceptre_role == self.STACK_DEFAULT
+                    else sceptre_role
+                )
+        # In most cases, we won't be targeting another stack's configurations. Instead, we'll want
+        # to be using the configurations of the CURRENT stack.
+        else:
+            profile, region, sceptre_role = self._determine_session_args(
+                profile, region, sceptre_role, iam_role
+            )
 
         if kwargs is None:  # pragma: no cover
             kwargs = {}
 
         client = self._get_client(service, region, profile, stack_name, sceptre_role)
         return getattr(client, command)(**kwargs)
+
+    def _coalesce_sceptre_role(self, iam_role: str, sceptre_role: str) -> str:
+        """Evaluates the iam_role and sceptre_role parameters as passed to determine which value to
+        use.
+        """
+        if sceptre_role == self.STACK_DEFAULT and iam_role != self.STACK_DEFAULT:
+            self._emit_iam_role_deprecation_warning()
+            sceptre_role = iam_role
+        return sceptre_role