(route53): RecordSetName incorrectly assumes zone name doesn't end with period when hostedzone.zonename is token

[peterwoodworth]

Describe the bug

An extra, unnecessary period is added when the HostedZone.zoneName is a token. This can occur when you import a HostedZone and set the zoneName with Fn.import values. These imported values contain periods at the end of the name

Expected Behavior

RecordSet is created with correct name

Current Behavior

RecordSet is created with name that ends in two periods, resulting in deployment failure

Reproduction Steps

        const dist = new Distribution(this, 'dist', {
            defaultBehavior: {
                origin: new S3Origin(new Bucket(this, 'bucket'))
            }
        })
        const name = Fn.importValue('name');
        const hzid = Fn.importValue('id');
        const zone = HostedZone.fromHostedZoneAttributes(this, 'hz', {
            hostedZoneId: hzid,
            zoneName: name,
        })
        new RecordSet(this, 'Rs', {
            recordType: RecordType.A,
            target: RecordTarget.fromAlias(new CloudFrontTarget(dist)),
            zone,
            recordName: 'wafv2-staticwebsite.win-deploy.app.xc'
        })

Possible Solution

Add a prop which can forcefully remove the period if desired by the user

Here: https://github.com/aws/aws-cdk/blob/2edcbba8e826a2a12c88d2943a0b19b67dec0e85/packages/%40aws-cdk/aws-route53/lib/util.ts#L66

if (hostedZoneName.isUnresolved() && forceRemovePeriodProp) {
    return `${providedName}${suffix}`;
} else {
    return `${providedName}${suffix}.`;
}

Not sure how else we'd be able to handle this since we can't know what the token's value will be at synth

Additional Information/Context

Originally reported internally P64092469

Can be worked around with escape hatches, but it would be nice if this could just work without that

CDK CLI Version

latest

Framework Version

No response

Node.js Version

16

OS

mac

Language

Typescript

Language Version

No response

Other information

No response

[tejasmr]

What if we do this instead?

if (suffix.endsWith('.'))
  return `${providedName}${suffix}`;
return `${providedName}${suffix}.`

Since

RecordSet is created with name that ends in two periods

means suffix ends with '.'

[peterwoodworth]

This unfortunately won't work in this case. The code before already "ensures" that the suffix won't end in a period - it does that here

https://github.com/aws/aws-cdk/blob/2edcbba8e826a2a12c88d2943a0b19b67dec0e85/packages/%40aws-cdk/aws-route53/lib/util.ts#L57-L59

The issue here is that when the hostedZone.zoneName is passed in as a token, this code can't actually parse the zoneName, and ends up assuming that it doesn't have a period

[tejasmr]

Got it. Thanks. I will start implementing your solution.

[peterwoodworth]

I'm not super confident my solution is the best way to address this, but whip it up and you'll get a review on it!

[tejasmr]

Can you review the PR?

[TheRealAmazonKendra]

Any fix for this is going to have to go under a feature flag. I think the path forward here is to not add that period at the end but I'm torn about removing that functionality altogether or about just removing it for tokens. @comcalvi what are your thoughts here?

[corymhall]

From the docs it looks like Route53 treats records with/without a trailing dot as identical so my preference would be to just remove the trailing dot.

In order to also handle https://github.com/aws/aws-cdk/issues/21306 I think the feature flag behavior would be to assume that the providedName contains the domain name if it is a token. That way can add the domain name if it isn't part of the token. I'm not sure it would be possible to do the other way around.