Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(aws-certificatemanager): DNS Validation fails when alternates are in different hosted zones #15217

Closed
codeedog opened this issue Jun 20, 2021 · 12 comments
Labels
@aws-cdk/aws-certificatemanager Related to Amazon Certificate Manager bug This issue is a bug. effort/medium Medium work item – several days of effort p2

Comments

@codeedog
Copy link

Although SSL Certificates may contain alternate subject domains with different hosted zones and it is possible to create such certificates via Web Console, aws command line and, in fact, CDK (using the Certificate class), it is not possible to do this with DnsValidatedCertificate. The certificate is, in fact, actually created. However, the CDK management code triggers an error causing a rollback of the Stack creation or update process despite the successful creation of the certificate.

Reproduction Steps

const hzMap = { "www.example.com": "example.com", "www.example.net": "example.net" };

cert = new acm.DnsValidatedCertificate(this, "cert", {
  domainName: "www.example.com",
  hostedZone: "example.com",
  validation: acm.CertificateValidation.fromDnsMultiZone(hzMap),
  region: 'us-east-1',                       // Required for CloudFront
  subjectAlternativeNames: domains,
});

I coded a stack that triggers the bug. It contains two options:

  1. Triggers the bug
  2. Successfully builds a Certificate in local region using two hosted zones, but cannot select region.

github repo

What did you expect to happen?

I expected that a Certificate containing the two different domains each existing in two separate hosted zones would have resulted in a Certificate created in the region 'us-east-1'. Note, the current region for this was not 'us-east-1'. Then, I would have expected the Stack creation process to have continued successfully to completion.

What actually happened?

A Certificate was created in the 'us-east-1' region. Then, upon an attempt to create A Records, the record for www.example.net was attempted to be created within the hosted zone for example.com. This caused a failure (example.com cannot hold a record for example.net) which triggered a rollback of the stack creation. So, the Certificate was created successfully, but the A record entries were not.

Environment

  • CDK CLI Version : 1.109.0 (build c647e38)
  • Framework Version: 1.109.0
  • Node.js Version: v14.17.0
  • OS : macOS 10.15.7 (19H524)
  • Language (Version): TypeScript (3.9.7)

Other

Note: it is possible to create a Certificate with different hosted zones using the AWS tools and UI. The Web console, the command line tool and CDK's acm.Certificate() method all allow it. Note, in the case of the latter, it looks like the code (if I have the correct file: lambda handler) uses the sdk call acm.requestCertificate (line 92), which according to the documentation allows SubjectAlternativeNames in different hosted zones (www.example.net). Given this and the fact that I can see the certificate continues to exist in the 'us-east-1' region even after rollback, I'm fairly certain that the call to requestCertificate succeeds. Therefore, the problem is almost certainly further down in the code.

Looking lower, Line 146 initiates a change batch call into Route 53 to create the A Records passing the hosted zone ID at Line 163. It uses an array of records collected at Line 130. I believe it is this batch change call that's failing as that's indicated by the error I'm seeing. I don't have a copy of the command line error as the stack creation process erases it, but it mentions something about not allowed to create example.net in example.com hosted zone. This would fit with a fixed hosted zone (Line 163) and varied hosted zones required for the A records.

I think this batch change needs to be broken up into separate hosted zones depending upon the zone matching the domain(s).


This is 🐛 Bug Report

@codeedog codeedog added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jun 20, 2021
@github-actions github-actions bot added the @aws-cdk/aws-certificatemanager Related to Amazon Certificate Manager label Jun 20, 2021
@njlynch njlynch added effort/medium Medium work item – several days of effort p2 and removed needs-triage This issue or PR still needs to be triaged. labels Jun 28, 2021
@njlynch njlynch removed their assignment Jun 28, 2021
@njlynch
Copy link
Contributor

njlynch commented Jun 28, 2021

Thanks for the bug report, and the initial investigation!

I am unassigning and marking this issue as p2, which means that we are unable to work on this immediately.

We use +1s to help prioritize our work, and are happy to revaluate this issue based on community feedback. You can reach out to the cdk.dev community on Slack to solicit support for reprioritization.

@flavioleggio
Copy link
Contributor

flavioleggio commented Aug 24, 2021

We also came up with the same problem, so here is my +1

I think this could be a low effort task, as it would be enough to replace this and this with a map of SANs and corresponding hosted zone ids and loop over it here.

In the meantime, are there any known workarounds? We really need to update the stack containing our certificate, as we upgraded the cdk version.

@codeedog
Copy link
Author

@flavioleggio I tried to figure out a CDK workaround and was unable to do so; workaround coding started to get way too big. It felt like the cons out weighed the pros. In my case, I know when I'm using multiple host zones vs alternates that exist under one zone. In the multiple host zone case I create the cert via the console and drop its arn in a DB which I query during the CDK stack run. If the arn exists, I use Certificate.fromCertificateArn to fetch the certificate.

I happen to have other code that fetches DNS records and assembles (groups) the alternate names into hosted zones so that I can create DNS records at the apex, if those records do not exist. The code is a bit involved. At first, I toyed with the idea of fixing this bug myself, but decided I just didn't understand the CDK well enough to propose a proper fix.

I'm happy to share that DNS analysis code with you or anyone else either for a possible patch or for your own workaround.

Here are the data and function signatures. If anyone is interested in the actual code, post here or PM me.

export interface HostedZoneSchedule {
  exists: { [key: string]: any },      // hosted zone exists in Route53
  create: { [key: string]: any },      // create these (apex) hosted zones
  domains: { [key: string]: {          // Requires an A or CNAME record
    hostedZone: string,                // Hosted Zone for the record
  } },
};

// Given an unordered domain list, construct an organized mapping of domains
// to existing (in Route 53) hosted zones and new hosted zones to be created.
// New hosted zones will always be apex domains (apex.tld.) whereas existing
// hosted domains could be apex or subdomains.
export async function synthesizeHostedZoneSchedule(domains: string[]): Promise<HostedZoneSchedule> {
...
}

Note, because new DNS entries have to be created, if you want true CDK rollback behavior, the challenge here is to remember that you created the DNS records so they can be rolled back for CDK errors and deletes. I'm sure CDK developers are used to doing this all of the time.

In my stack code, if I see a pre-existing DNS entry, it takes a different code path (route53.HostedZone.fromLookup()) than the ones that need to be created (new route53.HostedZone()) and because I don't keep track that I created the DNS entries, this has the effect of ejecting the DNS record out of the CDK stack on future runs. That means a subsequent CDK delete will not catch the DNS records which will need to be cleaned up by hand.

@winjer
Copy link

winjer commented Aug 24, 2021

In the meantime, are there any known workarounds? We really need to update the stack containing our certificate, as we upgraded the cdk version.

We are manually creating certificates and using their ARNs. It sucks but I've not seen any better way really.

@flavioleggio
Copy link
Contributor

@codeedog
If your problem is only related to the SANs in multiple hosted zones, the Certificate construct is what you need, as you can specify validation with DNS using the CertificateValidation.fromDnsMultiZone()

new Certificate(this, "MyCertificate", {
  domainName: certificateDomainName,
  subjectAlternativeNames: aliases.map(
    alias => alias.domainName
  ),
  validation: CertificateValidation.fromDnsMultiZone(
    aliases.reduce((multiZone: { [domain: string]: IHostedZone }, domain) => {
      multiZone[domain.domainName] = domain.hostedZone;
      return multiZone;
    }, {})
  )
});

Note that in my example aliases is of type Array<{ domainName: string; hostedZone: IHostedZone; }>

The only problem with this solution is that it does not work if you also need to create the certificate in a region which is different from the one in which the stack is deployed; in fact, the Certificate construct does not allow to specify a region, while the DNSValidatedCertificate does. Note that there are use cases where you need to specify a region, for example for services like CloudFront and Amazon ApiGateway, where the certificate must be located in us-east-1 while the stack can be located wherever you prefer.
This is the reason why the DNSValidatedCertificate should implement the same solution for multizone use cases.

@ckifer
Copy link
Contributor

ckifer commented Apr 13, 2022

+1
Just ran into the last paragraph above from @flavioleggio. We have a stack not in us-east-1 that needs a certificate created in us-east-1. Can't use DnsValidatedCertificate because of the above issue and can't use Certificate because the cert needs to be in us-east-1 to associate with a Cloudfront distro.

@codeedog
Copy link
Author

In case my original filing was murky, the entire point of the bug is that certificate creation for multiple apex hosted zones (e.g. example.net and example.com) doesn't work from non us-east-1 regions and the only reason the certificate is being created in the remote us-east-1 region is due to Amazon's runtime deployment requirement that the cert live in that specific us-east-1 region.

@silberistgold
Copy link

+1
Am I right the only workaround for now is to put the whole stack into us-east-1?

@flavioleggio
Copy link
Contributor

+1
Am I right the only workaround for now is to put the whole stack into us-east-1?

yes, AFAIK

@codeedog
Copy link
Author

+1
Am I right the only workaround for now is to put the whole stack into us-east-1?

You can create the Certificate in us-east-1 outside the stack process and refer to it by ARN during stack creation and update.

@taylorb-syd
Copy link

Duplicate of #8934, #20774, and #21040. Consolidating issues into #8934.

mergify bot pushed a commit that referenced this issue Jan 25, 2023
Now that the official CloudFormation resource `AWS::CertificateManager::Certificate` (CDK's `Certificate` construct) supports DNS validation we do not want to recommend using the `DnsValidatedCertificate` construct.

The `DnsValidatedCertificate` construct uses CloudFormation custom resources to perform the certificate creation and this creates a lot of maintenance burden on our team (see the list of linked issues). Currently the primary use case for using `DnsValidatedCertificate` over `Certificate` is for cross region use cases. For this use case I have updated the README to have our suggested solution.

The example in the README is tested in this [integration test](https://github.com/aws/aws-cdk/blob/main/packages/@aws-cdk/aws-cloudfront/test/integ.cloudfront-cross-region-cert.ts)

fixes #8934, #2914, #20698, #17349, #15217, #14519


----

### All Submissions:

* [ ] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions
Copy link

⚠️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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-certificatemanager Related to Amazon Certificate Manager bug This issue is a bug. effort/medium Medium work item – several days of effort p2
Projects
None yet
Development

No branches or pull requests

8 participants