follow up on CFN template-url S3 URI (localstack#9329)

bentsku · web-flow · commit c1293760b943 · 2023-10-11T16:19:17.000+02:00
diff --git a/localstack/services/cloudformation/api_utils.py b/localstack/services/cloudformation/api_utils.py
@@ -86,15 +86,15 @@ def convert_s3_to_local_url(url: str) -> str:
     headers = {"host": url_parsed.netloc}
     bucket_name, key_name = extract_bucket_name_and_key_from_headers_and_path(headers, path)
 
-    if not bucket_name or not key_name:
-        if url_parsed.scheme != "s3" and not (
-            url_parsed.netloc.startswith("s3.") or ".s3." in url_parsed.netloc
-        ):
-            raise ValidationError("TemplateURL must be a supported URL.")
+    if url_parsed.scheme == "s3":
         raise ValidationError(
             f"S3 error: Domain name specified in {url_parsed.netloc} is not a valid S3 domain"
         )
 
+    if not bucket_name or not key_name:
+        if not (url_parsed.netloc.startswith("s3.") or ".s3." in url_parsed.netloc):
+            raise ValidationError("TemplateURL must be a supported URL.")
+
     # note: make sure to normalize the bucket name here!
     bucket_name = normalize_bucket_name(bucket_name)
     local_url = f"{config.service_url('s3')}/{bucket_name}/{key_name}"
diff --git a/tests/aws/services/cloudformation/api/test_templates.py b/tests/aws/services/cloudformation/api/test_templates.py
@@ -55,15 +55,15 @@ def test_create_stack_from_s3_template_url(
     """
     )
 
-    aws_client.s3.put_object(Bucket=bucket_name, Key="template.yml", Body=to_bytes(template))
+    aws_client.s3.put_object(Bucket=bucket_name, Key="test/template.yml", Body=to_bytes(template))
 
     match url_style:
         case "s3_url":
-            template_url = f"s3://{bucket_name}/template.yml"
+            template_url = f"s3://{bucket_name}/test/template.yml"
         case "http_path":
-            template_url = f"https://s3.amazonaws.com/{bucket_name}/template.yml"
+            template_url = f"https://s3.amazonaws.com/{bucket_name}/test/template.yml"
         case "http_host":
-            template_url = f"https://{bucket_name}.s3.amazonaws.com/template.yml"
+            template_url = f"https://{bucket_name}.s3.amazonaws.com/test/template.yml"
         case "http_invalid":
             # note: using an invalid (non-existing) URL here, but in fact all non-S3 HTTP URLs are invalid in real AWS
             template_url = "https://example.com/dummy.yml"
diff --git a/tests/aws/services/cloudformation/api/test_templates.snapshot.json b/tests/aws/services/cloudformation/api/test_templates.snapshot.json
@@ -25,7 +25,7 @@
     }
   },
   "tests/aws/services/cloudformation/api/test_templates.py::test_create_stack_from_s3_template_url[s3_url]": {
-    "recorded-date": "08-10-2023, 16:15:04",
+    "recorded-date": "11-10-2023, 00:03:44",
     "recorded-content": {
       "create-error": {
         "Error": {
@@ -41,7 +41,7 @@
     }
   },
   "tests/aws/services/cloudformation/api/test_templates.py::test_create_stack_from_s3_template_url[http_path]": {
-    "recorded-date": "08-10-2023, 16:15:14",
+    "recorded-date": "11-10-2023, 00:03:53",
     "recorded-content": {
       "matching-topic": [
         {
@@ -51,7 +51,7 @@
     }
   },
   "tests/aws/services/cloudformation/api/test_templates.py::test_create_stack_from_s3_template_url[http_host]": {
-    "recorded-date": "08-10-2023, 16:15:24",
+    "recorded-date": "11-10-2023, 00:04:02",
     "recorded-content": {
       "matching-topic": [
         {
@@ -61,7 +61,7 @@
     }
   },
   "tests/aws/services/cloudformation/api/test_templates.py::test_create_stack_from_s3_template_url[http_invalid]": {
-    "recorded-date": "08-10-2023, 16:15:26",
+    "recorded-date": "11-10-2023, 00:04:04",
     "recorded-content": {
       "create-error": {
         "Error": {

Original file line number	Diff line number	Diff line change
`@@ -25,7 +25,7 @@`
`25`	`25`	`}`
`26`	`26`	`},`
`27`	`27`	`"tests/aws/services/cloudformation/api/test_templates.py::test_create_stack_from_s3_template_url[s3_url]": {`
`28`		`- "recorded-date": "08-10-2023, 16:15:04",`
	`28`	`+ "recorded-date": "11-10-2023, 00:03:44",`
`29`	`29`	`"recorded-content": {`
`30`	`30`	`"create-error": {`
`31`	`31`	`"Error": {`
`@@ -41,7 +41,7 @@`
`41`	`41`	`}`
`42`	`42`	`},`
`43`	`43`	`"tests/aws/services/cloudformation/api/test_templates.py::test_create_stack_from_s3_template_url[http_path]": {`
`44`		`- "recorded-date": "08-10-2023, 16:15:14",`
	`44`	`+ "recorded-date": "11-10-2023, 00:03:53",`
`45`	`45`	`"recorded-content": {`
`46`	`46`	`"matching-topic": [`
`47`	`47`	`{`
`@@ -51,7 +51,7 @@`
`51`	`51`	`}`
`52`	`52`	`},`
`53`	`53`	`"tests/aws/services/cloudformation/api/test_templates.py::test_create_stack_from_s3_template_url[http_host]": {`
`54`		`- "recorded-date": "08-10-2023, 16:15:24",`
	`54`	`+ "r 7832 ecorded-date": "11-10-2023, 00:04:02",`
`55`	`55`	`"recorded-content": {`
`56`	`56`	`"matching-topic": [`
`57`	`57`	`{`
`@@ -61,7 +61,7 @@`
`61`	`61`	`}`
`62`	`62`	`},`
`63`	`63`	`"tests/aws/services/cloudformation/api/test_templates.py::test_create_stack_from_s3_template_url[http_invalid]": {`
`64`		`- "recorded-date": "08-10-2023, 16:15:26",`
	`64`	`+ "recorded-date": "11-10-2023, 00:04:04",`
`65`	`65`	`"recorded-content": {`
`66`	`66`	`"create-error": {`
`67`	`67`	`"Error": {`