feat: custom domain name support for private endpoints #3719
Conversation
| basepath_mapping = ApiGatewayBasePathMappingV2( | ||
| logical_id, attributes=self.passthrough_resource_attributes | ||
| ) | ||
| basepath_mapping.DomainNameArn = ref(domain_name_arn) | ||
| basepath_mapping.RestApiId = ref(rest_api.logical_id) | ||
| basepath_mapping.Stage = ref(rest_api.logical_id + ".Stage") | ||
| basepath_mapping.BasePath = path if normalize_basepath else basepath |
There was a problem hiding this comment.
The creation of this can be moved to a function too, to then call it with some parameters (to be used in line 674-680 too).
There was a problem hiding this comment.
This comment meant that you should have created a function similar to what you did with create_basepath_mapping, but that you can actually use it here too (probably you need to pass a few more parameters, to make sure that the same function works for both cases).
| record_set_group = Route53RecordSetGroup(logical_id, attributes=self.passthrough_resource_attributes) | ||
| if "HostedZoneId" in route53: | ||
| record_set_group.HostedZoneId = route53.get("HostedZoneId") | ||
| if "HostedZoneName" in route53: | ||
| record_set_group.HostedZoneName = route53.get("HostedZoneName") | ||
| record_set_group.RecordSets = [] |
There was a problem hiding this comment.
All of this can be a method "get_record_set_group(..)"
| self, rest_api: ApiGatewayRestApi, route53_record_set_groups: Any | ||
| ) -> ApiDomainResponseV2: | ||
| """ | ||
| Constructs and returns the ApiGateway Domain and BasepathMapping |
There was a problem hiding this comment.
Maybe update your comment to show the difference between this and the v1 (I guess it's "returns the APIGateway Domain v2 and BasepathMapping"
|
|
||
| self._set_optional_domain_properties(domain) | ||
|
|
||
| basepaths: Optional[List[str]] = self._get_basepaths() |
There was a problem hiding this comment.
You should call this in the original _construct_api_domain (non v2) method too.
| @@ -667,6 +805,17 @@ def _construct_alias_target(self, domain: Dict[str, Any], api_domain_name: str, | |||
| alias_target["DNSName"] = route53.get("DistributionDomainName") | |||
| return alias_target | |||
|
|
|||
| def _create_basepath_mapping( | |||
There was a problem hiding this comment.
call it _v2, because in theory you could be doing the same for v1, but this one explicitly uses v2.
(It would be good if you actually create another method that does this for v1 and use it in the other version)
| basepath_mapping = ApiGatewayBasePathMappingV2( | ||
| logical_id, attributes=self.passthrough_resource_attributes | ||
| ) | ||
| basepath_mapping.DomainNameArn = ref(domain_name_arn) | ||
| basepath_mapping.RestApiId = ref(rest_api.logical_id) | ||
| basepath_mapping.Stage = ref(rest_api.logical_id + ".Stage") | ||
| basepath_mapping.BasePath = path if normalize_basepath else basepath |
There was a problem hiding this comment.
This comment meant that you should have created a function similar to what you did with create_basepath_mapping, but that you can actually use it here too (probably you need to pass a few more parameters, to make sure that the same function works for both cases).
| ) | ||
|
|
||
| if not record_set_group: | ||
| record_set_group = self._get_record_set_group(logical_id, route53) |
There was a problem hiding this comment.
You can use this function in the original _construct_api_domain too.
| # Create the Route53 RecordSetGroup resource | ||
| record_set_group = None | ||
| route53 = self.domain.get("Route53") | ||
| if route53 is not None: | ||
| sam_expect(route53, self.logical_id, "Domain.Route53").to_be_a_map() | ||
| if route53.get("HostedZoneId") is None and route53.get("HostedZoneName") is None: | ||
| raise InvalidResourceException( | ||
| self.logical_id, | ||
| "HostedZoneId or HostedZoneName is required to enable Route53 support on Custom Domains.", | ||
| ) | ||
|
|
||
| logical_id_suffix = LogicalIdGenerator( | ||
| "", route53.get("HostedZoneId") or route53.get("HostedZoneName") | ||
| ).gen() | ||
| logical_id = "RecordSetGroup" + logical_id_suffix | ||
|
|
||
| record_set_group = route53_record_set_groups.get(logical_id) | ||
|
|
||
| if route53.get("SeparateRecordSetGroup"): | ||
| sam_expect( | ||
| route53.get("SeparateRecordSetGroup"), self.logical_id, "Domain.Route53.SeparateRecordSetGroup" | ||
| ).to_be_a_bool() | ||
| return ApiDomainResponseV2( | ||
| domain, | ||
| basepath_resource_list, | ||
| self._construct_single_record_set_group(self.domain, domain_name, route53), | ||
| ) | ||
|
|
||
| if not record_set_group: | ||
| record_set_group = self._get_record_set_group(logical_id, route53) | ||
| route53_record_set_groups[logical_id] = record_set_group | ||
|
|
||
| record_set_group.RecordSets += self._construct_record_sets_for_domain(self.domain, domain_name, route53) | ||
|
|
||
| return ApiDomainResponseV2(domain, basepath_resource_list, record_set_group) |
There was a problem hiding this comment.
It pains me to see that all this code it's the same as the original _construct_api_domain except by returning ApiDomainResponseV2 instead of V1 (and most of the full function really)
I would really like to just take this to another function that we can call in both places.
- either a function that can returns either a v1 or a v2 resource
- a function where you pass the "Class" of the object that needs to be created.
- a function that returns "the parameters", and then you create the object outside.
Now the problem here is that the domain and basepath mapping resource can also be either v1 or v2, so it gets harder to get the types right.
Maybe we can chat about it to see if there's a good solution.
There was a problem hiding this comment.
Discussed offline that it's better to leave this as is for now. Will investigate further refactoring opportunities, but that will be part of another PR.
Issue #, if available
Description of changes
The AWS API Gateway now supports the ability to use custom domain names for private endpoints. Two new resources have been introduced to enable this feature - AWS::ApiGateway::DomainNameV2 and AWS::ApiGateway::BasePathMappingV2.
Description of how you validated changes
Checklist
Examples?
Please reach out in the comments if you want to add an example. Examples will be
added to
sam initthrough aws/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.