feat(apigatewayv2): domain endpoint type, security policy and endpoint migration#17518
feat(apigatewayv2): domain endpoint type, security policy and endpoint migration#17518mergify[bot] merged 9 commits intoaws:masterfrom SmritiVashisth:master
Conversation
|
|
nija-at
left a comment
There was a problem hiding this comment.
Wow. This looks much better. Good job on turning this PR around!
Some comments below.
packages/@aws-cdk/aws-apigatewayv2/test/http/domain-name.test.ts
Outdated
Show resolved
Hide resolved
Pull request has been modified.
nija-at
left a comment
There was a problem hiding this comment.
Just a few comments here.
Please address comments from previous iteration around naming the method and validations.
Pull request has been modified.
| private isDuplicateEndpointType(endpointType: string | undefined) : boolean { | ||
| for (let config of this.domainNameConfigurations) { | ||
| if (endpointType && endpointType == config.endpointType) { | ||
| return true; | ||
| } | ||
| } | ||
| return false; | ||
| } |
There was a problem hiding this comment.
If I'm reading this correctly, this will allow one endpoint type undefined and one endpoint type of REGIONAL. But this should not be allowed, right?
Can you fix this up and add a corresponding test case?
There was a problem hiding this comment.
It won't be that way. If you look at the addEndpoint() method which is the caller for this method, if there is no EndpointType defined by the customer, we set a default 'REGIONAL' endpoint. So this value will never be undefined. Some eslint code-check complained about it though, that's why I had to add undefined in the parameter.
There was a problem hiding this comment.
You can check the last test I have added. When I call dn.addEndpoint(), I am not giving an EndpointType in the input, but the code still figures out that I'm trying to add a REGIONAL endpoint here and throws an error.
Pull request has been modified.
nija-at
left a comment
There was a problem hiding this comment.
Fantastic. Thanks for all the work here! 👏
|
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
|
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
…t migration (aws#17518) Updating the L2 construct for `AWS::ApiGatewayV2::DomainName` resource to add support for DomainNameConfigurations. DomainNameConfigurations is a list of configurations for an API's domain name (CFN equivalent - https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-apigatewayv2-domainname-domainnameconfiguration.html) which includes endpoint type, security policy, ownership certificate. Changes include: - Code update to support the properties mentioned above - Unit test changes for existing tests to account for the updated `domainNameConfigurations` Lazy evaluation. - New unit tests for mutual tls with ownership certificate, and domain name migration that were not present before. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Updating the L2 construct for
AWS::ApiGatewayV2::DomainNameresource to add support for DomainNameConfigurations.DomainNameConfigurations is a list of configurations for an API's domain name (CFN equivalent - https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-apigatewayv2-domainname-domainnameconfiguration.html) which includes endpoint type, security policy, ownership certificate.
Changes include:
domainNameConfigurationsLazy evaluation.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license