diff --git a/samcli/lib/package/artifact_exporter.py b/samcli/lib/package/artifact_exporter.py index 8ef0652f47..c0f2b94576 100644 --- a/samcli/lib/package/artifact_exporter.py +++ b/samcli/lib/package/artifact_exporter.py @@ -36,7 +36,13 @@ ) from samcli.lib.package.s3_uploader import S3Uploader from samcli.lib.package.uploaders import Uploaders -from samcli.lib.package.utils import is_local_folder, make_abs_path, is_s3_url, is_local_file, mktempfile +from samcli.lib.package.utils import ( + is_local_folder, + make_abs_path, + is_local_file, + mktempfile, + is_s3_url, +) from samcli.lib.utils.packagetype import ZIP from samcli.yamlhelper import yaml_parse, yaml_dump @@ -62,12 +68,7 @@ def do_export(self, resource_id, resource_dict, parent_dir): template_path = resource_dict.get(self.PROPERTY_NAME, None) - if ( - template_path is None - or is_s3_url(template_path) - or template_path.startswith(self.uploader.s3.meta.endpoint_url) - or template_path.startswith("https://s3.amazonaws.com/") - ): + if template_path is None or is_s3_url(template_path): # Nothing to do return diff --git a/samcli/lib/package/packageable_resources.py b/samcli/lib/package/packageable_resources.py index edb99074d8..937b451a28 100644 --- a/samcli/lib/package/packageable_resources.py +++ b/samcli/lib/package/packageable_resources.py @@ -20,7 +20,7 @@ copy_to_temp_dir, upload_local_artifacts, upload_local_image_artifacts, - is_s3_url, + is_s3_protocol_url, is_path_value_valid, ) @@ -466,7 +466,7 @@ def include_transform_export_handler(template_dict, uploader, parent_dir): return template_dict include_location = template_dict.get("Parameters", {}).get("Location", None) - if not include_location or not is_path_value_valid(include_location) or is_s3_url(include_location): + if not include_location or not is_path_value_valid(include_location) or is_s3_protocol_url(include_location): # `include_location` is either empty, or not a string, or an S3 URI return template_dict diff --git a/samcli/lib/package/utils.py b/samcli/lib/package/utils.py index 54bb81ba97..6317c35a48 100644 --- a/samcli/lib/package/utils.py +++ b/samcli/lib/package/utils.py @@ -4,6 +4,7 @@ import logging import os import platform +import re import shutil import tempfile import uuid @@ -22,6 +23,29 @@ LOG = logging.getLogger(__name__) +# https://docs.aws.amazon.com/AmazonS3/latest/dev-retired/UsingBucket.html +_REGION_PATTERN = r"[a-zA-Z0-9-]+" +_DOT_AMAZONAWS_COM_PATTERN = r"\.amazonaws\.com" +_S3_URL_REGEXS = [ + # Path-Style (and ipv6 dualstack) + # - https://s3.Region.amazonaws.com/bucket-name/key name + # - https://s3.amazonaws.com/bucket-name/key name (old, without region) + # - https://s3.dualstack.us-west-2.amazonaws.com/... + re.compile(rf"http(s)?://s3(.dualstack)?(\.{_REGION_PATTERN})?{_DOT_AMAZONAWS_COM_PATTERN}/.+/.+"), + # Virtual Hosted-Style (including two legacies) + # https://docs.aws.amazon.com/AmazonS3/latest/userguide/VirtualHosting.html + # - Virtual Hosted-Style: https://bucket-name.s3.Region.amazonaws.com/key name + # - Virtual Hosted-Style (Legacy: a dash between S3 and the Region): https://bucket-name.s3-Region.amazonaws.com/... + # - Virtual Hosted-Style (Legacy Global Endpoint): https://my-bucket.s3.amazonaws.com/... + re.compile(rf"http(s)?://.+\.s3((.|-){_REGION_PATTERN})?{_DOT_AMAZONAWS_COM_PATTERN}/.+"), + # S3 access point: + # - https://AccessPointName-AccountId.s3-accesspoint.region.amazonaws.com + re.compile(rf"http(s)?://.+-\d+\.s3-accesspoint\.{_REGION_PATTERN}{_DOT_AMAZONAWS_COM_PATTERN}/.+/.+"), + # S3 protocol URL: + # - s3://bucket-name/key-name + re.compile(r"s3://.+/.+"), +] + def is_path_value_valid(path): return isinstance(path, str) @@ -33,7 +57,10 @@ def make_abs_path(directory, path): return path -def is_s3_url(url): +def is_s3_protocol_url(url): + """ + Check whether url is a valid path in the form of "s3://..." + """ try: S3Uploader.parse_s3_url(url) return True @@ -41,6 +68,14 @@ def is_s3_url(url): return False +def is_s3_url(url: str) -> bool: + """ + Check whether a URL is a S3 access URL + specified at https://docs.aws.amazon.com/AmazonS3/latest/dev-retired/UsingBucket.html + """ + return any(regex.match(url) for regex in _S3_URL_REGEXS) + + def is_local_folder(path): return is_path_value_valid(path) and os.path.isdir(path) @@ -122,7 +157,7 @@ def upload_local_artifacts( # Build the root directory and upload to S3 local_path = parent_dir - if is_s3_url(local_path): + if is_s3_protocol_url(local_path): # A valid CloudFormation template will specify artifacts as S3 URLs. # This check is supporting the case where your resource does not # refer to local artifacts diff --git a/tests/unit/lib/package/test_artifact_exporter.py b/tests/unit/lib/package/test_artifact_exporter.py index f1a793ecf3..7cc20f6be7 100644 --- a/tests/unit/lib/package/test_artifact_exporter.py +++ b/tests/unit/lib/package/test_artifact_exporter.py @@ -24,7 +24,7 @@ ServerlessApplicationResource, ) from samcli.lib.package.packageable_resources import ( - is_s3_url, + is_s3_protocol_url, is_local_file, upload_local_artifacts, Resource, @@ -198,10 +198,10 @@ def test_is_s3_url(self): self._assert_is_invalid_s3_url(url) def _assert_is_valid_s3_url(self, url): - self.assertTrue(is_s3_url(url), "{0} should be valid".format(url)) + self.assertTrue(is_s3_protocol_url(url), "{0} should be valid".format(url)) def _assert_is_invalid_s3_url(self, url): - self.assertFalse(is_s3_url(url), "{0} should be valid".format(url)) + self.assertFalse(is_s3_protocol_url(url), "{0} should be valid".format(url)) def test_parse_s3_url(self): diff --git a/tests/unit/lib/package/test_utils.py b/tests/unit/lib/package/test_utils.py new file mode 100644 index 0000000000..2907d7c479 --- /dev/null +++ b/tests/unit/lib/package/test_utils.py @@ -0,0 +1,44 @@ +from unittest import TestCase + +from parameterized import parameterized + +from samcli.lib.package import utils + + +class TestPackageUtils(TestCase): + @parameterized.expand( + [ + # path like + "https://s3.us-west-2.amazonaws.com/bucket-name/some/path/object.html", + "http://s3.amazonaws.com/bucket-name/some/path/object.html", + "https://s3.dualstack.us-west-2.amazonaws.com/bucket-name/some/path/object.html", + # virual host + "http://bucket-name.s3.us-west-2.amazonaws.com/some/path/object.html", + "https://bucket-name.s3-us-west-2.amazonaws.com/some/path/object.html", + "https://bucket-name.s3.amazonaws.com/some/path/object.html", + # access point + "https://access-name-123456.s3-accesspoint.us-west-2.amazonaws.com/some/path/object.html", + "http://access-name-899889.s3-accesspoint.us-east-1.amazonaws.com/some/path/object.html", + # s3:// + "s3://bucket-name/path/to/object", + ] + ) + def test_is_s3_url(self, url): + self.assertTrue(utils.is_s3_url(url)) + + @parameterized.expand( + [ + # path like + "https://s3.$region.amazonaws.com/bucket-name/some/path/object.html", # invalid region + "https://s3.amazonaws.com/object.html", # no bucket + # virual host + "https://bucket-name.s3-us-west-2.amazonaws.com/", # no object + # access point + "https://access-name.s3-accesspoint.us-west-2.amazonaws.com/some/path/object.html", # no account id + # s3:// + "s3://bucket-name", # no object + "s3:://bucket-name", # typo + ] + ) + def test_is_not_s3_url(self, url): + self.assertFalse(utils.is_s3_url(url))