Fix route53 record set groups #2408
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #2408 +/- ##
===========================================
+ Coverage 93.58% 94.47% +0.89%
===========================================
Files 90 98 +8
Lines 6124 7280 +1156
Branches 1260 1514 +254
===========================================
+ Hits 5731 6878 +1147
- Misses 183 194 +11
+ Partials 210 208 -2
Continue to review full report at Codecov.
|
| @@ -125,6 +125,7 @@ def translate(self, sam_template, parameter_values, feature_toggle=None, passthr | |||
| shared_api_usage_plan = SharedApiUsagePlan() | |||
| document_errors = [] | |||
| changed_logical_ids = {} | |||
| route53_record_set_groups = {} | |||
There was a problem hiding this comment.
This will combine Http and Rest Api's into one "cache". Will we run into a issue where the key used will collide between the two different resources if they are in the same template?
There was a problem hiding this comment.
The key is the logical IDs of the generated AWS::Route53::RecordSetGroup resources. For both case of Http API and Rest API, the logical ID is generated based on either HostedZoneId or HostedZoneName. So, corresponding record set would still be appended into an existing generated record set group instead of overwriting it.
I added a test for this mixed api case.
|
Will this solve #1539? |
Yes I believe so |
| CertificateArn: arn::cert::abc | ||
| EndpointConfiguration: REGIONAL | ||
| Route53: | ||
| HostedZoneId: "abc123456" |
There was a problem hiding this comment.
What happens in HostedZoneId is an intrinsic function?
There was a problem hiding this comment.
The entire HostedZoneId is used to generate a logical ID hash of the generated RecordSet Group. For example, consider HostedZoneId: !Ref MyParam, HostedZoneId: !Sub "{{MyParam}}", and HostedZoneId: "abc123456". Even if MyParamresolves to"abc123456", these three would yield RecordSet Groups with different logical ID hashes. In the case where only one kind of intrinsics is used for HostedZoneId`, only one RecordSet Group would be generated.
In the case where multiple kinds of intrinsics are used for HostedZoneId, there is no issue at well. Multiple AWS::Route53::RecordSetGroup resources can be created in a template pointing to the same HostedZoneId. For example, the template below can be deployed without any issue:
Resources:
HostedZone:
Type: "AWS::Route53::HostedZone"
Properties:
Name: "my-website.com"
MyRecordSetGroup1:
Type: AWS::Route53::RecordSetGroup
Properties:
HostedZoneId: !Ref HostedZone
RecordSets:
- Name: "my-website.com"
Type: "A"
TTL: "900"
ResourceRecords:
- "101.10.21.3"
MyRecordSetGroup2:
Type: AWS::Route53::RecordSetGroup
Properties:
HostedZoneId: !Ref HostedZone
RecordSets:
- Name: "www.my-website.com"
Type: "CNAME"
TTL: "900"
ResourceRecords:
- "my-website.com"
Issue #, if available:
#1829
Description of changes:
Keep track of created
AWS::Route53::RecordSetGroupresources in a cache. If a logical ID is already contained in the cache, use the created one and append a record set into it. Otherwise, create a newAWS::Route53::RecordSetGroupfor that logical ID and keep it in the cache.Description of how you validated changes:
tests/translator/input/api_with_custom_domain_route53_multiple.yaml) to confirm the template is transformed as expectedChecklist:
make prpassesExamples?
Please reach out in the comments, if you want to add an example. Examples will be
added to
sam initthrough https://github.com/awslabs/aws-sam-cli-app-templates/By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.