RecordSet: determineFullyQualifiedDomainName incorrectly suffixes zone name when param is passed in

[matusfaro]

Describe the bug

Edited for clarification: RecordSet attempts to be smart about the input value of recordName. Typically, for a zone example.com, a FQDN is required for the recordName such as test.example.com.. However, RecordSet's determineFullyQualifiedDomainName method attempts to fix user input in case they forgot to add the zone name turning test into test.example.com. This works great in the usual cases, except for parameterized input such as CfnCondition or imported value from another stack. The determineFullyQualifiedDomainName thinks the input does not end in the zone name and tacks it on. When test.example.com. is passed in via CfnParameter, the resulting record becomes test.example.com.example.com.

Expected Behavior

Since user is expected to pass in FQDN and the helper method is intended to suffix it if missed by the user, the default should be to leave the value as is if input can't be determined and assume user supplied the correct FQDN in parameter/imported value.

Current Behavior

The logic in determineFullyQualifiedDomainName suffixes zone name if it cannot read the parameter/imported value resulting in incorrect value.

Reproduction Steps

determineFullyQualifiedDomainName(
    'dataspray.io',
    {zoneName: 'dataspray.io.'});
=> CORRECT: dataspray.io.

determineFullyQualifiedDomainName(
    CfnParameter.valueAsString(), // Parameter contains dataspray.io
    {zoneName: 'dataspray.io.'});
=> INCORRECT: dataspray.io.dataspray.io.

determineFullyQualifiedDomainName(
    Fn.importValue(), // Imported value contains dataspray.io
    {zoneName: 'dataspray.io.'});
=> INCORRECT: dataspray.io.dataspray.io.

Possible Solution

Workaround is to suffix with "." to make it a FQDN and bypass suffixing:

determineFullyQualifiedDomainName(
    CfnParameter.valueAsString() + '.',
    {zoneName: 'dataspray.io.'});
=> CORRECT: dataspray.io.

But this only works when you can directly supply the recordName. In case of aws-route53-patterns.HttpsRedirect and other third party constructs, the input domain cannot contain a trailing dot as other constructs cannot have FQDN as input. Particularly for HttpsRedirect, a single input parameter is used both for the route53 record set as well as the cloudfront ViewerCertificate. One requires the FQDN, the other fails with it making it impossible to use as is.

Additional Information/Context

No response

CDK CLI Version

2.77.0

Framework Version

No response

Node.js Version

18

OS

osx 14

Language

Typescript

Language Version

4.9.5

Other information

No response

[matusfaro]

This also makes @aws-cdk/aws-route53-patterns unusable with a stack parameter for domain as I have no control over adding the required trailing dot to the RecordSet while not having the dot for the certificate that doesn't allow it.

[pahud]

Agree this is something we should fix though I am not sure what is the best solution off the top of my head. Are you interested to draft a PR for that?

[matusfaro]

@pahud Opened a PR here: #26597