feat(certificatemanager): native CloudFormation DNS validated certificate #8552

jogold · 2020-06-15T19:09:45Z

Automatically adding Amazon Route 53 CNAME records for DNS validation is
now natively supported by CloudFormation.

Add a validation prop to Certificate to handle both email and DNS
validation. DnsValidatedCertificate is now only useful for cross-region
certificate creation.

The default remains email validation (non-breaking).

Closes #5831
Closes #5835
Closes #6081
Closes #6516
Closes #7150
Closes #7941
Closes #7995
Closes #7996
Closes #8282
Closes #8659
Closes #8783

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

…cate Automatically adding Amazon Route 53 CNAME records for DNS validation is now natively supported by CloudFormation. Add a `validation` prop to `Certificate` to handle both email and DNS validation. Deprecate `DnsValidatedCertificate`. The default remains email validation (non-breaking). Closes aws#5831 Closes aws#5835 Closes aws#6081 Closes aws#6516 Closes aws#7150 Closes aws#7941 Closes aws#7995 Closes aws#7996

jogold · 2020-06-15T20:03:06Z

What should be done here?

aws-cdk/packages/@aws-cdk/aws-route53-patterns/lib/website-redirect.ts

Lines 45 to 50 in 8a90000

    
           const redirectCertArn = props.certificate ? props.certificate.certificateArn : new DnsValidatedCertificate(this, 'RedirectCertificate', { 
        
             domainName: domainNames[0], 
        
             subjectAlternativeNames: domainNames, 
        
             hostedZone: props.zone, 
        
             region: 'us-east-1', 
        
           }).certificateArn;

If we replace DnsValidatedCertificate with Certificate the construct will now only work if deployed in us-est-1.

See also aws-cloudformation/cloudformation-coverage-roadmap#523

AlexCheema · 2020-06-19T18:51:24Z

Any news on this? Recently a lot of DnsValidatedCertificate resources have been failing to deploy for us. Would be great to replace it with the native Cloudformation version.

njlynch · 2020-07-09T09:09:13Z

packages/@aws-cdk/aws-certificatemanager/README.md

+```ts
+new Certificate(this, 'Certificate', {
+  domainName: 'hello.example.com',
+  validation: CertificateValidation.fromDns(),


I feel like the README here should call out the same warning from the fromDns documentation about needing to manually add the DNS records before CloudFormation can succeed. Might even be worth having the Route53 example first (as the thing people are most likely to copy/paste and use), and then call out the outside-DNS example with the warning about the need for manual intervention.

njlynch · 2020-07-09T09:25:05Z

packages/@aws-cdk/aws-certificatemanager/lib/dns-validated-certificate.ts

+ * @deprecated use the `validation` prop with `CertificateValidation.fromDns()`
+ * on `Certificate`.


Counter-argument -- this custom resource-based approach allows for cross-region cert creation, which the native CloudFormation support doesn't allow (AFAIK). It might be worth not deprecating it, and noting this use case in the README. What do you think?

Good point. For information, it's in the CF roadmap aws-cloudformation/cloudformation-coverage-roadmap#523.

This also solves my question here #8552 (comment)

njlynch · 2020-07-09T09:37:09Z

packages/@aws-cdk/aws-certificatemanager/lib/certificate.ts

+   * @param hostedZone the default hosted zone to use for all domains in the certificate
+   * @param hostedZones a map of hosted zones to use for domains in the certificate
+   */
+  public static fromDns(hostedZone?: route53.IHostedZone, hostedZones?: { [domainName: string]: route53.IHostedZone }) {


I'm trying to think of a better way to do this interface. As-is, it's not clear how to use it -- if I need to provide the hostedZones map, do I also need to provide the hostedZone? If so, does it matter which is the "main" zone and which appears in the map? One other thought -- domain names to hosted zones are N:1; you may have 10 domain names with two top-level hosted zones; flipping the map around may be slightly more usable for some customers (I don't have the data to back that up off-hand though).

Maybe breaking this into two would alleviate some of the problems:

public static fromDns(hostedZone?: route53.IHostedZone) {...} public static fromDnsMultiZone(hostedZoneMap: { [domainName: string]: route53.IHostedZone) {...}

Open to better naming and disagreements.

packages/@aws-cdk/aws-certificatemanager/lib/certificate.ts

jogold · 2020-07-09T10:47:00Z

packages/@aws-cdk/aws-certificatemanager/README.md

+Use the `DnsValidatedCertificate` construct for cross-region certificate creation:
+
+```ts
+new DnsValidatedCertificate(this, 'CrossRegionCertificate', {


Should we do:

export class CrossRegionDnsValidatedCertificate extends DnsValidatedCertificate { constructor(scope: cdk.Construct, id: string, props: DnsValidatedCertificateProps) { super(scope, id, props); } }

to get a better naming?

I don't think it's worth it, especially considering cross-region support is on the roadmap for CloudFormation. Let's just leave it as-is and deprecate it once we can.

njlynch · 2020-07-09T15:38:30Z

packages/@aws-cdk/aws-certificatemanager/test/example.dns-validated-request.lit.ts

@@ -1,26 +0,0 @@
-import * as route53 from '@aws-cdk/aws-route53';


Can you un-delete this file, given we are not (yet) deprecating the DnsValidatedCertificate?

Pull request has been modified.

jogold · 2020-07-10T08:38:36Z

mergify bot dismissed njlynch’s stale review 2 minutes ago
Pull request has been modified.

@njlynch can you re-approve?

mergify · 2020-07-10T08:57:37Z

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-cdk-automation · 2020-07-10T09:56:35Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 8b9d7cb
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

mergify · 2020-07-10T09:57:02Z

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).

jogold added 2 commits June 15, 2020 21:07

update ecs-patterns

fb22718

SomayaB assigned skinny85 Jun 15, 2020

magJ mentioned this pull request Jun 19, 2020

fix(certificate-manager): Wait for all subject alternative resource records to be available. #7996

Closed

jogold mentioned this pull request Jun 19, 2020

(aws-certificatemanager): implement new cloudformation feature for DNS validation #8659

Closed

2 tasks

strottos mentioned this pull request Jun 23, 2020

Unable to delete hosted zone with DNS verified certificate #7063

Closed

jogold and others added 3 commits June 23, 2020 14:20

Merge branch 'master' into acm-dns-cf

f1c3328

Merge branch 'master' into acm-dns-cf

dab531c

Merge branch 'master' into acm-dns-cf

d51738d

jogold mentioned this pull request Jul 8, 2020

[aws-certificatemanager] DnsValidatedCertificate support for separate hosted zones and accounts #8934

Closed

skinny85 assigned njlynch and unassigned skinny85 Jul 8, 2020

skinny85 requested a review from njlynch July 8, 2020 16:16

njlynch reviewed Jul 9, 2020

View reviewed changes

jogold and others added 7 commits July 9, 2020 12:04

do not deprecate DnsValidatedCertificate

d3e9c4d

README for DnsValidatedCertificate

fb792ff

README DNS

0248018

fromDnsMultiZone

3b1ee06

JSDoc

01b5941

Merge branch 'master' into acm-dns-cf

a7f1997

AWS Route 53 -> Amazon Route 53

a371fbd

jogold commented Jul 9, 2020

View reviewed changes

njlynch reviewed Jul 9, 2020

View reviewed changes

jogold added 2 commits July 9, 2020 21:35

revert delete example.dns-validated-request.lit.ts

538b658

Merge branch 'master' into acm-dns-cf

785dff6

njlynch previously approved these changes Jul 10, 2020

View reviewed changes

Merge branch 'master' into acm-dns-cf

2742524

njlynch approved these changes Jul 10, 2020

View reviewed changes

Merge branch 'master' into acm-dns-cf

8b9d7cb

mergify bot merged commit 337279f into aws:master Jul 10, 2020

jogold deleted the acm-dns-cf branch July 10, 2020 10:02

ralovely mentioned this pull request Jul 10, 2020

certificatemanager : DnsValidatedCertificate doesn't wait long enough #5831

Closed

njlynch mentioned this pull request Jul 13, 2020

DnsValidatedCertificate doesn't support validationDomains #7777

Closed

jk2l mentioned this pull request Jul 17, 2020

AWS::CloudFormation::Stack - Region aws-cloudformation/cloudformation-coverage-roadmap#562

Open

aereal mentioned this pull request Jul 20, 2020

[certificatemanager] validationDomains ignored if validation method is DNS #9164

Closed

stephenwiebe mentioned this pull request Jul 24, 2020

[aws-certificatemanager] New DNS validation method not working due to route53 error #9248

Closed

unerh mentioned this pull request Jul 6, 2021

Use the new CFN Custom Resource Certificate Validator cdk-cosmos/cosmos#305

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(certificatemanager): native CloudFormation DNS validated certificate #8552

feat(certificatemanager): native CloudFormation DNS validated certificate #8552

jogold commented Jun 15, 2020 •

edited

Loading

jogold commented Jun 15, 2020 •

edited

Loading

AlexCheema commented Jun 19, 2020 •

edited

Loading

njlynch Jul 9, 2020

njlynch Jul 9, 2020

jogold Jul 9, 2020

njlynch Jul 9, 2020

jogold Jul 9, 2020

njlynch Jul 9, 2020

njlynch Jul 9, 2020

jogold commented Jul 10, 2020

mergify bot commented Jul 10, 2020

aws-cdk-automation commented Jul 10, 2020

mergify bot commented Jul 10, 2020

		* @deprecated use the `validation` prop with `CertificateValidation.fromDns()`
		* on `Certificate`.

		@@ -1,26 +0,0 @@
		import * as route53 from '@aws-cdk/aws-route53';

feat(certificatemanager): native CloudFormation DNS validated certificate #8552

feat(certificatemanager): native CloudFormation DNS validated certificate #8552

Conversation

jogold commented Jun 15, 2020 • edited Loading

jogold commented Jun 15, 2020 • edited Loading

AlexCheema commented Jun 19, 2020 • edited Loading

njlynch Jul 9, 2020

Choose a reason for hiding this comment

njlynch Jul 9, 2020

Choose a reason for hiding this comment

jogold Jul 9, 2020

Choose a reason for hiding this comment

njlynch Jul 9, 2020

Choose a reason for hiding this comment

jogold Jul 9, 2020

Choose a reason for hiding this comment

njlynch Jul 9, 2020

Choose a reason for hiding this comment

njlynch Jul 9, 2020

Choose a reason for hiding this comment

jogold commented Jul 10, 2020

mergify bot commented Jul 10, 2020

aws-cdk-automation commented Jul 10, 2020

AWS CodeBuild CI Report

mergify bot commented Jul 10, 2020

jogold commented Jun 15, 2020 •

edited

Loading

jogold commented Jun 15, 2020 •

edited

Loading

AlexCheema commented Jun 19, 2020 •

edited

Loading