-
Notifications
You must be signed in to change notification settings - Fork 397
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
Issue 1819 cloudfront distribution origin s3 domain #1821
Issue 1819 cloudfront distribution origin s3 domain #1821
Conversation
Build succeeded. ✔️ ansible-galaxy-importer SUCCESS in 3m 42s |
changelogs/fragments/1819-cloudfront-distribution-s3-domain-recognise.yaml
Outdated
Show resolved
Hide resolved
A test (integration or unit) would be nice. But not a blocker for me. |
Build failed. ❌ ansible-galaxy-importer FAILURE in 4m 31s |
Build failed. ❌ ansible-galaxy-importer FAILURE in 4m 11s |
Build succeeded. ✔️ ansible-galaxy-importer SUCCESS in 3m 48s |
regate |
regate |
regate |
regate |
regate |
regate |
regate |
regate |
Build succeeded. ✔️ ansible-galaxy-importer SUCCESS in 4m 09s |
Build succeeded (gate pipeline). ✔️ ansible-galaxy-importer SUCCESS in 4m 33s |
Backport to stable-5: 💔 cherry-picking failed — conflicts found❌ Failed to cleanly apply 509ccad on top of patchback/backports/stable-5/509ccad9fdf8961d2c1e7fde672f45979de6f1eb/pr-1821 Backporting merged PR #1821 into main
🤖 @patchback |
Backport to stable-6: 💚 backport PR created✅ Backport PR branch: Backported as #1849 🤖 @patchback |
Issue 1819 cloudfront distribution origin s3 domain SUMMARY Fixes #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 (cherry picked from commit 509ccad)
lint - cloudfront_distribution - apply black formatting SUMMARY #1821 introduced some bad formatting ISSUE TYPE Bugfix Pull Request COMPONENT NAME cloudfront_distribution ADDITIONAL INFORMATION
…tion origin s3 domain (#1849) [PR #1821/509ccad9 backport][stable-6] Issue 1819 cloudfront distribution origin s3 domain This is a backport of PR #1821 as merged into main (509ccad). SUMMARY Fixes #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: Mark Chappell
SUMMARY
Fixes #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 thes3_origin_config
and produces an error: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
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