Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Update Arn parsing logic and fix some edge cases/bug fixes for remote invoke #5295

Merged
merged 8 commits into from
Jun 10, 2023
2 changes: 1 addition & 1 deletion samcli/cli/command.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,7 @@ class UnsupportedServiceForRemoteInvoke(UserException):

class NoExecutorFoundForRemoteInvoke(UserException):
pass


class InvalidStackNameProvidedForRemoteInvoke(UserException):
pass
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ def cli(
config_env: str,
) -> None:
"""
`sam cloud invoke` command entry point
`sam remote invoke` command entry point
"""

do_cli(
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
@@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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:
"""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
-------
Expand Down Expand Up @@ -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
-------
Expand Down
5 changes: 2 additions & 3 deletions samcli/lib/pipeline/bootstrap/resource.py
Original file line number Diff line number Diff line change
Expand Up @@ -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}"


Expand Down
4 changes: 1 addition & 3 deletions samcli/lib/remote_invoke/lambda_invoke_executors.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
43 changes: 38 additions & 5 deletions samcli/lib/utils/arn_utils.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
"""
Module for utilities for ARN (Amazon Resource Names)
"""
import re


class InvalidArnValue(ValueError):
Expand Down Expand Up @@ -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 ""
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand All @@ -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)
Expand All @@ -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,
Expand All @@ -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
):
Expand Down
Loading