aws / aws-cdk

The AWS Cloud Development Kit is a framework for defining cloud infrastructure in code
https://aws.amazon.com/cdk
Apache License 2.0
11.59k stars 3.89k forks source link

HostedZone: Default period at the end should be optional #22406

Closed DavidA94 closed 1 year ago

DavidA94 commented 2 years ago

Describe the bug

In the HostedZone constructor, a period is added to the end of the name on line 163.

This changes the user's input from github.com to github.com.. Although Route53 considers these to be identical, they cannot co-exist together. Thus, for anybody moving from CFN templates to CDK, who did not have a . at the end of their name, this line breaks their CloudFormation deployment.

Consider a user who has a CloudFormation template with the following:

CustomHostedZone:
    Type: AWS::Route53::HostedZone
    Properties:
        Name: github.com
        ...

Who then re-creates their resource in CDK, keeping the same LogicalId for the migration

new HostedZone(this, "CustomHostedZone", {
    zoneName: 'github.com'
    ...
}
overrideLogicalId("CustomHostedZone");

Because the . is added, CloudFormation will see this as a name change, and try to re-create the resource. However, Route53 will see the same domain name and block the creation because the domain already exists. Because this added . is by default, the user is only left with hacky solutions to deploy their code.

Expected Behavior

Give the user an option whether or not the . should be added at the end

Current Behavior

The user has no control over the behavior.

Reproduction Steps

Create a hosted zone with a CFN template with no trailing ., and then try to create the same HostedZone, with the LogicalId overwritten, using CDK.

Possible Solution

At this point, I believe the only viable solution is a flag (addTrailingDot?) added to the properties which is defaulted to true. Unfortunately, removing this behavior outright will break anybody's code who didn't have pre-existing resources.

If github.com. exists in a user's Route53 domain list, and the . is removed from a later release of this package, then their CDK will start deploying as github.com, which will still cause CloudFormation to try to re-create the resource, causing the same issue.

Additional Information/Context

No response

CDK CLI Version

2.43.1

Framework Version

No response

Node.js Version

14

OS

Mac

Language

Typescript

Language Version

No response

Other information

No response

pahud commented 1 year ago

I agree with you. Sounds like we could have addTrailingDot optional prop with default True to allow users opt out. I am making this issue a P2 and any suggested solutions or pull requests are welcome!

github-actions[bot] commented 1 year ago

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see. If you need more assistance, please either tag a team member or open a new issue that references this one. If you wish to keep having a conversation with other community members under this issue feel free to do so.