fix: Update Arn parsing logic and fix some edge cases/bug fixes for r…

…emote invoke (aws#5295)

* Fix some edge cases and bug fixes for remote invoke and update Arn parsing logic

* Address feedback

* Add unit test for s3 with no region/accoint_id provided

* Renamed command to sam remote invoke

samcli/cli/command.py

@@ -30,7 +30,7 @@
     "samcli.commands.pipeline.pipeline",
     "samcli.commands.list.list",
     "samcli.commands.docs",
-    # "samcli.commands.cloud.cloud",
+    # "samcli.commands.remote.remote",
     # We intentionally do not expose the `bootstrap` command for now. We might open it up later
     # "samcli.commands.bootstrap",
 ]

samcli/commands/cloud/exceptions.py → samcli/commands/remote/exceptions.py

@@ -22,3 +22,7 @@ class UnsupportedServiceForRemoteInvoke(UserException):
 
 class NoExecutorFoundForRemoteInvoke(UserException):
     pass
+
+
+class InvalidStackNameProvidedForRemoteInvoke(UserException):
+    pass

samcli/commands/cloud/invoke/cli.py → samcli/commands/remote/invoke/cli.py

@@ -68,7 +68,7 @@ def cli(
     config_env: str,
 ) -> None:
     """
-    `sam cloud invoke` command entry point
+    `sam remote invoke` command entry point
     """
 
     do_cli(
@@ -100,8 +100,8 @@ def do_cli(
     """
     Implementation of the ``cli`` method
     """
-    from samcli.commands.cloud.remote_invoke_context import RemoteInvokeContext
     from samcli.commands.exceptions import UserException
+    from samcli.commands.remote.remote_invoke_context import RemoteInvokeContext
     from samcli.lib.remote_invoke.exceptions import (
         ErrorBotoApiCallException,
         InvalideBotoResponseException,

samcli/commands/cloud/cloud.py → samcli/commands/remote/remote.py

@@ -1,11 +1,11 @@
 """
-Command group for "cloud" suite for commands. It provides common CLI arguments for interacting with
+Command group for "remote" suite for commands. It provides common CLI arguments for interacting with
 cloud resources such as Lambda Function.
 """
 
 import click
 
-from samcli.commands.cloud.invoke.cli import cli as invoke_cli
+from samcli.commands.remote.invoke.cli import cli as invoke_cli
 
 
 @click.group()

samcli/commands/cloud/remote_invoke_context.py → samcli/commands/remote/remote_invoke_context.py

@@ -4,9 +4,12 @@
 import logging
 from typing import Optional, cast
 
-from samcli.commands.cloud.exceptions import (
+from botocore.exceptions import ClientError
+
+from samcli.commands.remote.exceptions import (
     AmbiguousResourceForRemoteInvoke,
     InvalidRemoteInvokeParameters,
+    InvalidStackNameProvidedForRemoteInvoke,
     NoExecutorFoundForRemoteInvoke,
     NoResourceFoundForRemoteInvoke,
     UnsupportedServiceForRemoteInvoke,
@@ -15,7 +18,7 @@
 from samcli.lib.remote_invoke.remote_invoke_executors import RemoteInvokeExecutionInfo
 from samcli.lib.utils import osutils
 from samcli.lib.utils.arn_utils import ARNParts, InvalidArnValue
-from samcli.lib.utils.boto_utils import BotoProviderType
+from samcli.lib.utils.boto_utils import BotoProviderType, get_client_error_code
 from samcli.lib.utils.cloudformation import (
     CloudFormationResourceSummary,
     get_resource_summaries,
@@ -105,20 +108,28 @@ def _populate_resource_summary(self) -> None:
         if not self._stack_name and not self._resource_id:
             raise InvalidRemoteInvokeParameters("Either --stack-name or --resource-id parameter should be provided")
 
-        if not self._resource_id:
-            # no resource id provided, list all resources from stack and try to find one
-            self._resource_summary = self._get_single_resource_from_stack()
-            self._resource_id = self._resource_summary.logical_resource_id
-            return
-
-        if not self._stack_name:
-            # no stack name provided, resource id should be physical id so that we can use it
-            self._resource_summary = self._get_from_physical_resource_id()
-            return
-
-        self._resource_summary = get_resource_summary(
-            self._boto_resource_provider, self._boto_client_provider, self._stack_name, self._resource_id
-        )
+        try:
+            if not self._resource_id:
+                # no resource id provided, list all resources from stack and try to find one
+                self._resource_summary = self._get_single_resource_from_stack()
+                self._resource_id = self._resource_summary.logical_resource_id
+                return
+
+            if not self._stack_name:
+                # no stack name provided, resource id should be physical id so that we can use it
+                self._resource_summary = self._get_from_physical_resource_id()
+                return
+
+            self._resource_summary = get_resource_summary(
+                self._boto_resource_provider, self._boto_client_provider, self._stack_name, self._resource_id
+            )
+        except ClientError as ex:
+            error_code = get_client_error_code(ex)
+            if error_code == "ValidationError":
+                raise InvalidStackNameProvidedForRemoteInvoke(
+                    f"Invalid --stack-name parameter. Stack with id '{self._stack_name}' does not exist"
+                )
+            raise ex
 
     def _get_single_resource_from_stack(self) -> CloudFormationResourceSummary:
         """

samcli/lib/cli_validation/remote_invoke_options_validations.py

@@ -22,7 +22,7 @@ def event_and_event_file_options_validation(func):
     Parameters
     ----------
     func :
-        Command that would be executed, in this case it is 'sam cloud invoke'
+        Command that would be executed, in this case it is 'sam remote invoke'
 
     Returns
     -------
@@ -64,7 +64,7 @@ def stack_name_or_resource_id_atleast_one_option_validation(func):
     Parameters
     ----------
     func :
-        Command that would be executed, in this case it is 'sam cloud invoke'
+        Command that would be executed, in this case it is 'sam remote invoke'
 
     Returns
     -------

samcli/lib/pipeline/bootstrap/resource.py

@@ -99,10 +99,9 @@ def get_uri(self) -> Optional[str]:
         # ECR's resource_id contains the resource-type("resource") which is excluded from the URL
         # from docs: https://docs.aws.amazon.com/AmazonECR/latest/userguide/security_iam_service-with-iam.html
         # ECR's ARN: arn:${Partition}:ecr:${Region}:${Account}:repository/${Repository-name}
-        if not arn_parts.resource_id.startswith("repository/"):
+        if arn_parts.resource_type != "repository":
             raise ValueError(f"Invalid ECR ARN ({self.arn}), can't extract the URL from it.")
-        i = len("repository/")
-        repo_name = arn_parts.resource_id[i:]
+        repo_name = arn_parts.resource_id
         return f"{arn_parts.account_id}.dkr.ecr.{arn_parts.region}.amazonaws.com/{repo_name}"
 
 

samcli/lib/remote_invoke/lambda_invoke_executors.py

@@ -52,9 +52,7 @@ def validate_action_parameters(self, parameters: dict) -> None:
             if parameter_key == FUNCTION_NAME:
                 LOG.warning("FunctionName is defined using the value provided for --resource-id option.")
             elif parameter_key == PAYLOAD:
-                LOG.warning(
-                    "Payload is defined using the value provided for either --payload or --payload-file options."
-                )
+                LOG.warning("Payload is defined using the value provided for either --event or --event-file options.")
             else:
                 self.request_parameters[parameter_key] = parameter_value
 

samcli/lib/utils/arn_utils.py

@@ -1,6 +1,7 @@
 """
 Module for utilities for ARN (Amazon Resource Names)
 """
+import re
 
 
 class InvalidArnValue(ValueError):
@@ -31,11 +32,43 @@ class ARNParts:
     service: str
     region: str
     account_id: str
+    resource_type: str
     resource_id: str
 
     def __init__(self, arn: str) -> None:
-        parts = arn.split(":")
-        try:
-            [_, self.partition, self.service, self.region, self.account_id, self.resource_id] = parts
-        except ValueError as ex:
-            raise InvalidArnValue(f"Invalid ARN ({arn})") from ex
+        # Regex pattern formed based on the 3 ARN general formats found here:
+        # https://docs.aws.amazon.com/IAM/latest/UserGuide/reference-arns.html
+        arn_pattern = (
+            r"arn:([a-zA-Z0-9_-]+):"  # Pattern for partition
+            r"([a-zA-Z0-9_-]+):"  # Pattern for service
+            r"([a-zA-Z0-9_-]*):"  # Pattern for region
+            r"([a-zA-Z0-9_-]*)"  # Pattern for account_id
+            r"(?::([a-zA-Z0-9_-]+))?"  # Pattern for resource_type if it exists
+            r"(?::(.+))"  # Pattern for resource_id if it exists
+        )
+
+        matched_arn = re.match(arn_pattern, arn)
+        if not matched_arn:
+            raise InvalidArnValue(f"Invalid ARN ({arn}) provided")
+
+        self.partition = matched_arn.group(1)
+        self.service = matched_arn.group(2)
+        self.region = matched_arn.group(3) if matched_arn.group(3) else ""
+        self.account_id = matched_arn.group(4) if matched_arn.group(4) else ""
+        self.resource_type = matched_arn.group(5) if matched_arn.group(5) else ""
+        if matched_arn.group(5):
+            # This handles the Arns of services with the format:
+            # arn:partition:service:region:account-id:resource-type:resource-id
+            self.resource_type = matched_arn.group(5)
+            self.resource_id = matched_arn.group(6) if matched_arn.group(6) else ""
+        elif "/" in matched_arn.group(6):
+            # This handles the Arns of services with the format:
+            # arn:partition:service:region:account-id:resource-type/resource-id
+            split_resource_type_and_id = matched_arn.group(6).split("/", 1)
+            self.resource_type = split_resource_type_and_id[0]
+            self.resource_id = split_resource_type_and_id[1]
+        else:
+            # This handles the Arns of services with the format:
+            # arn:partition:service:region:account-id:resource-id
+            self.resource_type = ""
+            self.resource_id = matched_arn.group(6) if matched_arn.group(6) else ""

tests/unit/commands/cloud/invoke/test_cli.py → tests/unit/commands/remote/invoke/test_cli.py

@@ -3,7 +3,7 @@
 
 from parameterized import parameterized
 
-from samcli.commands.cloud.invoke.cli import do_cli
+from samcli.commands.remote.invoke.cli import do_cli
 from samcli.lib.remote_invoke.remote_invoke_executors import RemoteInvokeOutputFormat
 from samcli.lib.remote_invoke.exceptions import (
     ErrorBotoApiCallException,
@@ -37,7 +37,7 @@ def setUp(self) -> None:
     @patch("samcli.lib.remote_invoke.remote_invoke_executors.RemoteInvokeExecutionInfo")
     @patch("samcli.lib.utils.boto_utils.get_boto_client_provider_with_config")
     @patch("samcli.lib.utils.boto_utils.get_boto_resource_provider_with_config")
-    @patch("samcli.commands.cloud.remote_invoke_context.RemoteInvokeContext")
+    @patch("samcli.commands.remote.remote_invoke_context.RemoteInvokeContext")
     def test_remote_invoke_command(
         self,
         event,
@@ -112,7 +112,7 @@ def test_remote_invoke_command(
             (InvalidResourceBotoParameterException,),
         ]
     )
-    @patch("samcli.commands.cloud.remote_invoke_context.RemoteInvokeContext")
+    @patch("samcli.commands.remote.remote_invoke_context.RemoteInvokeContext")
     def test_raise_user_exception_invoke_not_successfull(self, exeception_to_raise, mock_invoke_context):
 
         context_mock = Mock()

tests/unit/commands/cloud/test_remote_invoke_context.py → tests/unit/commands/remote/test_remote_invoke_context.py

@@ -2,14 +2,15 @@
 from unittest.mock import Mock, patch
 from uuid import uuid4
 
-from samcli.commands.cloud.exceptions import (
+from samcli.commands.remote.exceptions import (
     InvalidRemoteInvokeParameters,
     AmbiguousResourceForRemoteInvoke,
     NoResourceFoundForRemoteInvoke,
     UnsupportedServiceForRemoteInvoke,
     NoExecutorFoundForRemoteInvoke,
+    InvalidStackNameProvidedForRemoteInvoke,
 )
-from samcli.commands.cloud.remote_invoke_context import RemoteInvokeContext, SUPPORTED_SERVICES
+from samcli.commands.remote.remote_invoke_context import RemoteInvokeContext, SUPPORTED_SERVICES
 from samcli.lib.utils.cloudformation import CloudFormationResourceSummary
 
 
@@ -32,23 +33,31 @@ def test_no_stack_name_and_no_resource_id_should_fail(self):
             with self._get_remote_invoke_context():
                 pass
 
-    @patch("samcli.commands.cloud.remote_invoke_context.get_resource_summaries")
+    @patch("samcli.commands.remote.remote_invoke_context.get_resource_summaries")
+    def test_invalid_stack_name_with_no_resource_should_fail(self, patched_resource_summaries):
+        self.resource_id = None
+        patched_resource_summaries.side_effect = InvalidStackNameProvidedForRemoteInvoke("Invalid stack-name")
+        with self.assertRaises(InvalidStackNameProvidedForRemoteInvoke):
+            with self._get_remote_invoke_context():
+                pass
+
+    @patch("samcli.commands.remote.remote_invoke_context.get_resource_summaries")
     def test_only_stack_name_with_no_resource_should_fail(self, patched_resource_summaries):
         self.resource_id = None
         patched_resource_summaries.return_value = {}
         with self.assertRaises(NoResourceFoundForRemoteInvoke):
             with self._get_remote_invoke_context():
                 pass
 
-    @patch("samcli.commands.cloud.remote_invoke_context.get_resource_summaries")
+    @patch("samcli.commands.remote.remote_invoke_context.get_resource_summaries")
     def test_only_stack_name_with_multiple_resource_should_fail(self, patched_resource_summaries):
         self.resource_id = None
         patched_resource_summaries.return_value = {"resource1": Mock(), "resource2": Mock()}
         with self.assertRaises(AmbiguousResourceForRemoteInvoke):
             with self._get_remote_invoke_context():
                 pass
 
-    @patch("samcli.commands.cloud.remote_invoke_context.get_resource_summaries")
+    @patch("samcli.commands.remote.remote_invoke_context.get_resource_summaries")
     def test_only_stack_name_with_single_resource_should_be_valid(self, patched_resource_summaries):
         self.resource_id = None
         resource_summary = Mock(logical_resource_id=self.resource_id)
@@ -58,15 +67,15 @@ def test_only_stack_name_with_single_resource_should_be_valid(self, patched_reso
 
     def test_only_resource_id_unsupported_service_arn_should_fail(self):
         self.stack_name = None
-        self.resource_id = "arn:aws:unsupported-service:region:account:resource_id"
+        self.resource_id = "arn:aws:unsupported-service:region:account:resource_type:resource_id"
         with self.assertRaises(UnsupportedServiceForRemoteInvoke):
             with self._get_remote_invoke_context():
                 pass
 
     def test_only_resource_id_supported_service_arn_should_be_valid(self):
         self.stack_name = None
         service = "lambda"
-        self.resource_id = f"arn:aws:{service}:region:account:{self.resource_id}"
+        self.resource_id = f"arn:aws:{service}:region:account:resource_type:{self.resource_id}"
         with self._get_remote_invoke_context() as remote_invoke_context:
             self.assertEqual(
                 remote_invoke_context._resource_summary,
@@ -75,45 +84,45 @@ def test_only_resource_id_supported_service_arn_should_be_valid(self):
                 ),
             )
 
-    @patch("samcli.commands.cloud.remote_invoke_context.get_resource_summary_from_physical_id")
+    @patch("samcli.commands.remote.remote_invoke_context.get_resource_summary_from_physical_id")
     def test_only_resource_id_as_invalid_physical_id_should_fail(self, patched_resource_summary_from_physical_id):
         self.stack_name = None
         patched_resource_summary_from_physical_id.return_value = None
         with self.assertRaises(AmbiguousResourceForRemoteInvoke):
             with self._get_remote_invoke_context():
                 pass
 
-    @patch("samcli.commands.cloud.remote_invoke_context.get_resource_summary")
+    @patch("samcli.commands.remote.remote_invoke_context.get_resource_summary")
     def test_if_no_resource_found_with_given_stack_and_resource_id_should_fail(self, patched_get_resource_summary):
         patched_get_resource_summary.return_value = None
         with self.assertRaises(AmbiguousResourceForRemoteInvoke):
             with self._get_remote_invoke_context() as remote_invoke_context:
                 remote_invoke_context.run(Mock())
 
-    @patch("samcli.commands.cloud.remote_invoke_context.get_resource_summary_from_physical_id")
+    @patch("samcli.commands.remote.remote_invoke_context.get_resource_summary_from_physical_id")
     def test_only_resource_id_as_valid_physical_id_should_be_valid(self, patched_resource_summary_from_physical_id):
         self.stack_name = None
         resource_summary = Mock()
         patched_resource_summary_from_physical_id.return_value = resource_summary
         with self._get_remote_invoke_context() as remote_invoke_context:
             self.assertEqual(remote_invoke_context._resource_summary, resource_summary)
 
-    @patch("samcli.commands.cloud.remote_invoke_context.get_resource_summary")
+    @patch("samcli.commands.remote.remote_invoke_context.get_resource_summary")
     def test_running_without_resource_summary_should_raise_exception(self, patched_get_resource_summary):
         patched_get_resource_summary.return_value = None
         with self._get_remote_invoke_context() as remote_invoke_context:
             with self.assertRaises(AmbiguousResourceForRemoteInvoke):
                 remote_invoke_context.run(Mock())
 
-    @patch("samcli.commands.cloud.remote_invoke_context.get_resource_summary")
+    @patch("samcli.commands.remote.remote_invoke_context.get_resource_summary")
     def test_running_with_unsupported_resource_should_raise_exception(self, patched_get_resource_summary):
         patched_get_resource_summary.return_value = Mock(resource_type="UnSupportedResource")
         with self._get_remote_invoke_context() as remote_invoke_context:
             with self.assertRaises(NoExecutorFoundForRemoteInvoke):
                 remote_invoke_context.run(Mock())
 
-    @patch("samcli.commands.cloud.remote_invoke_context.RemoteInvokeExecutorFactory")
-    @patch("samcli.commands.cloud.remote_invoke_context.get_resource_summary")
+    @patch("samcli.commands.remote.remote_invoke_context.RemoteInvokeExecutorFactory")
+    @patch("samcli.commands.remote.remote_invoke_context.get_resource_summary")
     def test_running_should_execute_remote_invoke_executor_instance(
         self, patched_get_resource_summary, patched_remote_invoke_executor_factory
     ):