fix: package/deploy failure when Location/TemplateURL is virtual host S3 URL

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
 

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
 

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,14 +57,25 @@ 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
     except ValueError:
         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

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):
 

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))