change: move service-role path parsing for AmazonSageMaker-ExecutionRole for get_execution_role() into except block of IAM get_role() call and add warning message

Author: ChoiByungWook

src/sagemaker/session.py

@@ -3498,14 +3498,6 @@ def get_caller_identity_arn(self):
             endpoint_url=sts_regional_endpoint(self.boto_region_name),
         ).get_caller_identity()["Arn"]
 
-        if "AmazonSageMaker-ExecutionRole" in assumed_role:
-            role = re.sub(
-                r"^(.+)sts::(\d+):assumed-role/(.+?)/.*$",
-                r"\1iam::\2:role/service-role/\3",
-                assumed_role,
-            )
-            return role
-
         role = re.sub(r"^(.+)sts::(\d+):assumed-role/(.+?)/.*$", r"\1iam::\2:role/\3", assumed_role)
 
         # Call IAM to get the role's path
@@ -3518,6 +3510,22 @@ def get_caller_identity_arn(self):
                 role_name,
             )
 
+            # This conditional has been present since the inception of SageMaker
+            # Guessing this conditional's purpose was to handle lack of IAM permissions
+            # https://github.com/aws/sagemaker-python-sdk/issues/2089#issuecomment-791802713
+            if "AmazonSageMaker-ExecutionRole" in assumed_role:
+                LOGGER.warning('Assuming role was created in SageMaker AWS console, '
+                               'as the name contains `AmazonSageMaker-ExecutionRole`. '
+                               'Defaulting to Role ARN with service-role in path. '
+                               'If this Role ARN is incorrect, please add '
+                               'IAM read permissions to your role or supply the '
+                               'Role Arn directly.')
+                role = re.sub(
+                    r"^(.+)sts::(\d+):assumed-role/(.+?)/.*$",
+                    r"\1iam::\2:role/service-role/\3",
+                    assumed_role,
+                )
+
         return role
 
     def logs_for_job(  # noqa: C901 - suppress complexity warning for this method

tests/unit/test_session.py

@@ -405,6 +405,24 @@ def test_get_caller_identity_arn_from_an_execution_role(sts_regional_endpoint, b
     }
     sess.boto_session.client("iam").get_role.return_value = {"Role": {"Arn": arn}}
 
+    actual = sess.get_caller_identity_arn()
+    assert (
+        actual
+        == "arn:aws:iam::369233609183:role/AmazonSageMaker-ExecutionRole-20171129T072388"
+    )
+
+
+@patch("os.path.exists", side_effect=mock_exists(NOTEBOOK_METADATA_FILE, False))
+@patch("sagemaker.session.sts_regional_endpoint", return_value=STS_ENDPOINT)
+def test_get_caller_identity_arn_from_a_sagemaker_execution_role_with_iam_client_error(sts_regional_endpoint, boto_session):
+    sess = Session(boto_session)
+    arn = "arn:aws:sts::369233609183:assumed-role/AmazonSageMaker-ExecutionRole-20171129T072388/SageMaker"
+    sess.boto_session.client("sts", endpoint_url=STS_ENDPOINT).get_caller_identity.return_value = {
+        "Arn": arn
+    }
+
+    sess.boto_session.client("iam").get_role.side_effect = ClientError({}, {})
+
     actual = sess.get_caller_identity_arn()
     assert (
         actual