Skip to content

Commit

Permalink
Issue 1819 cloudfront distribution origin s3 domain (ansible-collecti…
Browse files Browse the repository at this point in the history
…ons#1821)

Issue 1819 cloudfront distribution origin s3 domain

SUMMARY
Fixes ansible-collections#1819
As per Origin Domain Name spec now the S3 domain names are in the form {name}.s3.{region}.amazonaws.com, so the string fragment .s3.amazonaws.com no longer occurs in them, and therefore they aren't recognised as S3 origin domains.
Consequentially, the origin is treated as a custom one, so a custom_origin_config member is generated into it, which collides with the s3_origin_config and produces an error:

botocore.errorfactory.InvalidOrigin: An error occurred (InvalidOrigin) when calling the CreateDistribution operation: You must specify either a CustomOrigin or an S3Origin. You cannot specify both.

The backward-compatible way is to recognise both {name}.s3.amazonaws.com and {name}.s3.{domain}.amazonaws.com, but for this a regular expression is the most effective solution.
ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME
cloudfront_distribution
ADDITIONAL INFORMATION
The breakdown of the regex I used: \.s3(?:\.[^.]+)?\.amazonaws\.com$

\.s3 matches ".s3"
\.[^.]+ would match a dot followed by at least one, possibly more non-dot characters
(\.[^]+) would match the same, just grouped, so we could treat it as an atom
(?:\.[^]+) would match the same, just grouped in a non-capturing fashion (we don't want to extract the matched characters)
(?:\.[^]+)? matches the same, occuring 0 or 1 times
\.amazonaws\.com matches ".amazonaws.com"
$ matches the end of the input string

Reviewed-by: Markus Bergholz <[email protected]>
Reviewed-by: Alina Buzachis
  • Loading branch information
gsimon75 authored Jun 22, 2023
1 parent dbba718 commit 509ccad
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 6 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
bugfixes:
- cloudfront_distribution - The origins recognises the s3 domains with region part now (https://github.com/ansible-collections/community.aws/issues/1819).
13 changes: 7 additions & 6 deletions plugins/modules/cloudfront_distribution.py
Original file line number Diff line number Diff line change
Expand Up @@ -1417,6 +1417,7 @@

from collections import OrderedDict
import datetime
import re

try:
import botocore
Expand Down Expand Up @@ -1676,7 +1677,7 @@ def __init__(self, module):
"http2and3",
]
)
self.__s3_bucket_domain_identifier = ".s3.amazonaws.com"
self.__s3_bucket_domain_regex = re.compile(r"\.s3(?:\.[^.]+)?\.amazonaws\.com$")

def add_missing_key(self, dict_object, key_to_set, value_to_set):
if key_to_set not in dict_object and value_to_set is not None:
Expand Down Expand Up @@ -1818,7 +1819,7 @@ def validate_origin(self, client, existing_config, origin, default_origin_path):
)
else:
origin_shield_region = origin_shield_region.lower()
if self.__s3_bucket_domain_identifier in origin.get("domain_name").lower():
if self.__s3_bucket_domain_regex.search(origin.get("domain_name").lower()):
if origin.get("s3_origin_access_identity_enabled") is not None:
if origin["s3_origin_access_identity_enabled"]:
s3_origin_config = self.validate_s3_origin_configuration(client, existing_config, origin)
Expand All @@ -1834,10 +1835,10 @@ def validate_origin(self, client, existing_config, origin, default_origin_path):

origin["s3_origin_config"] = dict(origin_access_identity=oai)

if "custom_origin_config" in origin:
self.module.fail_json(
msg="s3_origin_access_identity_enabled and custom_origin_config are mutually exclusive"
)
if "custom_origin_config" in origin:
self.module.fail_json(
msg="s3 origin domains and custom_origin_config are mutually exclusive"
)
else:
origin = self.add_missing_key(
origin, "custom_origin_config", existing_config.get("custom_origin_config", {})
Expand Down
37 changes: 37 additions & 0 deletions tests/integration/targets/cloudfront_distribution/tasks/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -568,6 +568,43 @@
ignore_errors: true

- name: check that custom origin with origin access identity fails
# "s3 origin domains and custom_origin_config are mutually exclusive"
assert:
that:
- update_origin_to_s3_with_origin_access_and_with_custom_origin_config.failed

- name: check that custom_origin_config can't be used with an region-agnostic S3 domain
cloudfront_distribution:
distribution_id: "{{ distribution_id }}"
origins:
- domain_name: "{{ resource_prefix }}-bucket.s3.{{ aws_region }}.amazonaws.com"
id: "{{ resource_prefix }}3.example.com"
custom_origin_config:
http_port: 8080
state: present
register: update_origin_to_s3_with_origin_access_and_with_custom_origin_config
ignore_errors: true

- name: check that custom origin with region-agnostic S3 domain fails
# "s3 origin domains and custom_origin_config are mutually exclusive"
assert:
that:
- update_origin_to_s3_with_origin_access_and_with_custom_origin_config.failed

- name: check that custom_origin_config can't be used with an region-aware S3 domain
cloudfront_distribution:
distribution_id: "{{ distribution_id }}"
origins:
- domain_name: "{{ resource_prefix }}-bucket.s3.amazonaws.com"
id: "{{ resource_prefix }}3.example.com"
custom_origin_config:
http_port: 8080
state: present
register: update_origin_to_s3_with_origin_access_and_with_custom_origin_config
ignore_errors: true

- name: check that custom origin with region-aware S3 domain fails
# "s3 origin domains and custom_origin_config are mutually exclusive"
assert:
that:
- update_origin_to_s3_with_origin_access_and_with_custom_origin_config.failed
Expand Down

0 comments on commit 509ccad

Please sign in to comment.