feat(certificatemanager): deprecate DnsValidatedCertificate

[corymhall]

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

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

---

All Submissions:

• Have you followed the guidelines in our Contributing guide?

Adding new Unconventional Dependencies:

• This PR adds new unconventional dependencies following the process described here

New Features

• Have you added the new feature to an integration test?
  • 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

[gitpod-io]

[rix0rrr]

This seems almost too good to be true. Is it really that simple?

Small nit on the example (probably need to transport the string value itself instead of the StringParameter object).

[rix0rrr]

If this empty object is really required, mind making that optional as well?

[corymhall]

In the uniqueResourceName method?

[rix0rrr]

Is this going to work? As in, is going to reference the actual hard-coded parameter name? (As opposed to generating an illegal cross-stack reference?)

[corymhall]

Yeah it works since I'm creating a physical name for the parameter.

[rix0rrr]

Don't forget to add a stack dependency

[joostvanderborg]

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 solution reads to me as cross region use is not supported. I’d be annoyed to have to create a stack in us-east-1 just because the certificate has to be there, while the actual cloudfront distribution can be created in a stack in (in our case) eu-west-1. Sure, it works, but it seems to be a regression in functionality before the equivalent functionality is actually implemented ‘natively’. In practice, we never deploy cloudfront without creating a certificate, so this will always need to be split between several stacks.

I get that having to use a custom resource is not great, so if deprecation means more available time for a real fix, I'd be all for it.

[corymhall]

The solution reads to me as cross region use is not supported. I’d be annoyed to have to create a stack in us-east-1 just because the certificate has to be there, while the actual cloudfront distribution can be created in a stack in (in our case) eu-west-1. Sure, it works, but it seems to be a regression in functionality before the equivalent functionality is actually implemented ‘natively’. In practice, we never deploy cloudfront without creating a certificate, so this will always need to be split between several stacks.

Yeah cross region is technically not supported by CloudFormation in this case. There is an open issue in their roadmap for the Certificate resource to work cross region, which is what the long term solution should be.

I get that having to use a custom resource is not great, so if deprecation means more available time for a real fix, I'd be all for it.

Deprecation means that it may be removed in the next major version of CDK (v3.0.0), so it will continue to be available until then. It also means that we will not work on enhancements.

[corymhall]

hmm looks like DnsValidatedCertificate is currently being used by the route53-patterns library. I'll need to somehow replace that with Certificate before merging this.

[a-bigelow]

@corymhall Does this flag result in an annotation at synth time? e.g. to warn users and nudge them towards the new shiny Cfn functionality.

[taylorb-syd]

As stated in #8934 I do not believe that we should deprecate this until we find a viable replacement for multi-region certificates. I understand that this will likely be removed in 3.0.0, which is some time away, however I ask that we put a blocker on this particular feature until we have discussed and found a solution that we're happy that will allow customers to deploy their CloudFront distributions.

Short of feature request enhancements to either ACM or CloudFormation here are some possible solutions (ideally more than one).

• Generalized cross-region reference method using Custom Resources or Similar. I see you're using Parameters in the Docs example on this PR, but I don't see any reason why CDK can't handle this automatically.
• Documented and fully encouraged process to perform cross-region references using CodePipeline.

[gshpychka]

@corymhall Simply curious - any reason we can't scope this down to a single SSM parameter?

[taylorb-syd]

In theory, yes, since we're using a named parameter. If I were doing this in native CFN I'd do it using the following Fn::Sub...

arn:${AWS::Partition}:ssm:us-east-1:${AWS::AccountId}:parameter/${output.parameterName}

For best practice we should probably limit the policy scope to this resource.

[TheRealAmazonKendra]

Just one question inline but I also want to ask about tests for this one. It looks like no new tests were added, just adjusted from test to testDeprecated. Is there a reason to not add new tests?

[corymhall]

It looks like no new tests were added, just adjusted from test to testDeprecated. Is there a reason to not add new tests?

@TheRealAmazonKendra I should have included it in the description, but I added tests in previous PRs to test the new recommended behavior so this PR is only deprecating the existing behavior.

Tests added:

aws-cdk/packages/@aws-cdk/aws-route53-patterns/test/bucket-website-target.test.ts

Lines 157 to 294 in ba14612

	describe('Uses Certificate when @aws-cdk/aws-route53-patters:useCertificate=true', () => {
	test('explicit different region', () => {
	// GIVEN
	const app = new App({
	context: {
	[ROUTE53_PATTERNS_USE_CERTIFICATE]: true,
	},
	});
	// WHEN
	const stack = new Stack(app, 'test', { env: { region: 'us-east-2' }, crossRegionReferences: true });
	new HttpsRedirect(stack, 'Redirect', {
	recordNames: ['foo.example.com'],
	targetDomain: 'bar.example.com',
	zone: HostedZone.fromHostedZoneAttributes(stack, 'HostedZone', {
	hostedZoneId: 'ID',
	zoneName: 'example.com',
	}),
	});
	// THEN
	const certStack = app.node.findChild(`certificate-redirect-stack-${stack.node.addr}`) as Stack;
	Template.fromStack(certStack).hasResourceProperties('AWS::CertificateManager::Certificate', {
	DomainName: 'foo.example.com',
	});
	Template.fromStack(stack).hasResourceProperties('AWS::CloudFront::Distribution', {
	DistributionConfig: {
	ViewerCertificate: {
	AcmCertificateArn: {
	'Fn::GetAtt': [
	'ExportsReader8B249524',
	'/cdk/exports/test/certificateredirectstackc8e2763df63c0f7e0c9afe0394e299bb731e281e8euseast1RefRedirectCertificatec8693e36481e135aa76e35c2db892ec6a33a94c7461E1B6E15A36EB7DA',
	],
	},
	},
	},
	});
	});
	test('explicit same region', () => {
	// GIVEN
	const app = new App({
	context: {
	[ROUTE53_PATTERNS_USE_CERTIFICATE]: true,
	},
	});
	// WHEN
	const stack = new Stack(app, 'test', { env: { region: 'us-east-1' }, crossRegionReferences: true });
	new HttpsRedirect(stack, 'Redirect', {
	recordNames: ['foo.example.com'],
	targetDomain: 'bar.example.com',
	zone: HostedZone.fromHostedZoneAttributes(stack, 'HostedZone', {
	hostedZoneId: 'ID',
	zoneName: 'example.com',
	}),
	});
	// THEN
	const certStack = app.node.tryFindChild(`certificate-redirect-stack-${stack.node.addr}`);
	expect(certStack).toBeUndefined();
	Template.fromStack(stack).hasResourceProperties('AWS::CertificateManager::Certificate', {
	DomainName: 'foo.example.com',
	});
	Template.fromStack(stack).hasResourceProperties('AWS::CloudFront::Distribution', {
	DistributionConfig: {
	ViewerCertificate: {
	AcmCertificateArn: {
	Ref: 'RedirectRedirectCertificateB4F2F130',
	},
	},
	},
	});
	});
	test('same support stack used for multiple certificates', () => {
	// GIVEN
	const app = new App({
	context: {
	[ROUTE53_PATTERNS_USE_CERTIFICATE]: true,
	},
	});
	// WHEN
	const stack = new Stack(app, 'test', { env: { region: 'us-east-2' }, crossRegionReferences: true });
	new HttpsRedirect(stack, 'Redirect', {
	recordNames: ['foo.example.com'],
	targetDomain: 'bar.example.com',
	zone: HostedZone.fromHostedZoneAttributes(stack, 'HostedZone', {
	hostedZoneId: 'ID',
	zoneName: 'example.com',
	}),
	});
	new HttpsRedirect(stack, 'Redirect2', {
	recordNames: ['foo2.example.com'],
	targetDomain: 'bar2.example.com',
	zone: HostedZone.fromHostedZoneAttributes(stack, 'HostedZone2', {
	hostedZoneId: 'ID',
	zoneName: 'example.com',
	}),
	});
	// THEN
	const certStack = app.node.tryFindChild(`certificate-redirect-stack-${stack.node.addr}`) as Stack;
	Template.fromStack(certStack).hasResourceProperties('AWS::CertificateManager::Certificate', {
	DomainName: 'foo.example.com',
	});
	Template.fromStack(certStack).hasResourceProperties('AWS::CertificateManager::Certificate', {
	DomainName: 'foo2.example.com',
	});
	});
	test('unresolved region throws', () => {
	// GIVEN
	const app = new App({
	context: {
	[ROUTE53_PATTERNS_USE_CERTIFICATE]: true,
	},
	});
	// WHEN
	const stack = new Stack(app, 'test');
	// THEN
	expect(() => {
	new HttpsRedirect(stack, 'Redirect', {
	recordNames: ['foo.example.com'],
	targetDomain: 'bar.example.com',
	zone: HostedZone.fromHostedZoneAttributes(stack, 'HostedZone', {
	hostedZoneId: 'ID',
	zoneName: 'example.com',
	}),
	});
	}).toThrow(/When @aws-cdk\/aws-route53-patters:useCertificate is enabled, a region must be defined on the Stack/);

https://github.com/aws/aws-cdk/blob/main/packages/%40aws-cdk/aws-route53-patterns/test/integ.hosted-redirect-same-region.ts
https://github.com/aws/aws-cdk/blob/main/packages/%40aws-cdk/aws-cloudfront/test/integ.cloudfront-cross-region-cert.ts

[mergify]

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

[aws-cdk-automation]

AWS CodeBuild CI Report

• CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
• Commit ID: 2e4e47c
• Result: SUCCEEDED
• Build Logs (available for 30 days)

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

[mergify]

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

[belinde]

Is the multi-stack solution for Cloudfront supposed to be definitive? It's really cumbersome when applied on our codebase, and if there's a possibility to wait for a better solution we'll endure the annoying deprecation notices.

[jskrt]

I think issue #8934 might be incorrectly marked as fixed with this PR. It doesn't seem to me that Cloudformation supports cross-account ACM validation. There is no field to pass a role to Cloudformation for the cross-account call.

[damogallagher]

@corymhall / @jskrt do you happen to know when using Certificate instead of DnsValidatedCertificate - is it possible to specify the location of the certificate.
I am deploying infrastructure in eu-west-1.
In order to validate the cert in CloudFront, the cert needs to be in us-east-1.
With DnsValidatedCertificate, I was able to specify the cert region, however I want to remove DnsValidatedCertificate from my code as it is deprecated.

[ChrisLane]

@damogallagher I think I asked essentially the same question here #23952.
Just putting that out there so there's an easy link.

[therightstuff]

The answer to @damogallagher's question is "no". We now need to create a separate us-east-1 stack containing the certificate before we use the certificate in another region.

[taylorb-syd]

Deprecation of this resource without addressing cross-region references in an automatic way is going to make this difficult for a lot of customers.

[rehanvdm]

It sucks that this creates a new Certificate with a new ARN that then also needs to be validated. We just can not make this change. Please do not deprecate this as it means we would not be able to update to V3 when that eventually drops.

Maybe changing the name to include "Legacy" or something like that is an acceptable solution?

[corymhall]

Deprecation of this resource without addressing cross-region references in an automatic way is going to make this difficult for a lot of customers.

The only additional requirement that you have now is to explicitly create a stack in us-east-1. You can create a new issue for creating some functionality to automatically create a stack for you in us-east-1.

CloudFormation is a regional service. In order to create a resource in a specific region, you must create a stack in that region. I 100% agree that it is not a great experience to be allowed to create a CloudFront distribution in any region, but are required to create an ACM certificate in us-east-1, but that is an issue that should be resolved by the AWS::CertificateManager::Certificate resource. We created the DnsValidatedCertificate resource when the Certificate resource did not support dns validation. Over time we essentially began to manage our own CloudFormation resource which is not something we should do.

[corymhall]

It sucks that this creates a new Certificate with a new ARN that then also needs to be validated. We just can not make this change. Please do not deprecate this as it means we would not be able to update to V3 when that eventually drops.

Why is that a problem?

[rehanvdm]

I 100% understand why you want to deprecate to it and I am all for the advantages it brings.

My problem comes in that we have:

• Multiple certificates created in multiple regions in stack A
• They are shared via SSM parameters to multiple other stacks let's say in Stack B
• The hosted zone for the certificates is in a different AWS account than all these stacks that create and hold the certificate.

There is no strong dependency between these. What is going to happen when we switch to the new Cert CFN construct?

• CDK will remove the old custom resource without knowing other resources are dependent on it? What happens to the ALBs and CloudFronts that depend on it in this transition period to the new Cert?
• It will do the above (delete the custom resource) before the new cert is validated?

When I say multiple, it is about 4+ regions, with easily 100+ stacks per region spread over three different AWS accounts.

As I am writing this, I think the problem is that the old DnsValidatedCertificate construct is being deprecated in favor of the new Certificate construct. They almost have identical arguments, but it should not be "positioned" this way, it is not a drop in replacement change. Just making the name change and the small construct prop change can have big consequences? Unless the questions I asked above are all covered to not allow those situations to happen.

It should rather be approached from a multistage migration standpoint. Where you:

• Create the new Cert construct alongside the old one, deploy, so that both exist at the same time.
• Then change all dependencies to use the new ARN, deploy.
• Then go back and clean up by deleting the old resource, deploy.

Am I maybe missing something and completely overthinking it?

[corymhall]

It should rather be approached from a multistage migration standpoint. Where you:

Create the new Cert construct alongside the old one, deploy, so that both exist at the same time.
Then change all dependencies to use the new ARN, deploy.
Then go back and clean up by deleting the old resource, deploy.

Am I maybe missing something and completely overthinking it?

You could definitely do this in a multistage deployment process. CloudFormation first applies all updates before applying any deletions, so if you were to do it in a single operation it will first create the certificate and then update the CloudFront distributions to reference the new certificate and then delete the old certificate.

[rehanvdm]

CloudFormation first applies all updates before applying any deletions,

Okay that is good to know(or remember 😅)

so if you were to do it in a single operation it will first create the certificate and then update the CloudFront distributions to reference the new certificate and then delete the old certificate.

The problem here is that the Cert and let's use the CloudFront for this example, is not in the same stack. CloudFormation does not know about where the Cert ARN is used because the Cert ARN is stored in an SSM parameter and then read by the other stacks. Basically:

• Network stack creates the Cert, and exports the ARN with new ssm.StringParameter( ... , stringValue: uiCertificate.certificateArn, ... )
• Application stacks (independent deployments, think different repo from the network stack) reads the Cert ARN using const certificateArn = ssm.StringParameter.fromStringParameterAttributes( ... ).stringValue and then imports the cert with const certificate = acm.Certificate.fromCertificateArn(this, 'Certificate', certificateArn) to be used in the application stacks.

Because these are not in the same stack and not in the same deployment, CFN can not know this weak reference (not CFN Exports) relationship exists so I fear that when it tries to delete the Cert it will either succeed and then the CloudFronts and ALBs will break? Or it will prevent the deletion of the cert because it is "in use" of which I think is complicated to determine so I am not sure if AWS actually does this check. If they don't and the cert is deleted what happens to the resources that depended on it?

[johngreen-dev]

As mentioned by a number of people,
We are deploying our stacks to eu-west*
With DnsValidatedCertificate we could just specify the cert region, it seems we would now need to create a new stack in us east and is going to make it incredibly difficult to manage.
This PR should not have included the depreciation until the new functionality contains the ability to run this from any region.

[kschlesselmann]

What about https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_certificatemanager.DnsValidatedCertificate.html#cleanuproute53records ? Currently we really like this feature for our disaster recovery tests where we just deploy and destroy our stacks.

[belinde]

Just to let everybody realize how disrupting this change is: in order to use crossRegionReferences: true, the region must be explicitly setted in the stack. This means we have to redeploy ALL the stack because the references must be updated.

I understand the reasons of the deprecation, but it's waaaay too early.

[luxaritas]

@rehanvdm Maybe I'm missing something here, but wouldn't the solution be the same for any instance of replacing a resource with "weak" references, ie create the new cert alongside the old one, update the SSM value, redeploy the new stacks, then remove the old cert? It's obviously extra steps, but that just seems like the limitations you incur by relying on ARN references, no?

[rehanvdm]

@luxaritas Yes, 100% correct. But the assumption that this deprecation is based on is wrong:

CloudFormation first applies all updates before applying any deletions, so if you were to do it in a single operation it will first create the certificate and then update the CloudFront distributions to reference the new certificate and then delete the old certificate.

My problem is that the deprecation is pointing to the new Certificate construct and is positioning it as a drop-in replacement. When it is not, certificates will get destroyed, this might be fine if your cert and the service using the cert is in the same stack, then the logic above holds. But for many, a cert is used across many applications/stacks/regions, so this is not the case and you will only realize this once it is too late.

https://github.com/aws/aws-cdk/pull/21982/files#diff-fc4f56929c24e8d8193c0fda6bfce2ea82d53be211e9eff63d59dda38ba68c9eR69

Removing just that one line, as in the PR would have been my suggestion as well. It will prompt people to actually go and investigate and scrutinize their architecture to decide if they can just use the new construct or do a migration as I mentioned.

[luxaritas]

Ah, so your concern is not so much around the deprecation as how the forward-migration is described?

It's still useful for it to be clear that Certificate is (except in limited cases namely with the cross-region issue) a drop-in replacement as far as API and functionality (that indicates to me when swapping one out for the other that there aren't any other surprise changes in functionality or options that I wouldn't know about without looking deeper into the change details or implementation). I could see the utility in being more explicit about the potential implications, though on the same token I'd think it'd be a given that if you replace a construct with a different one, you have a high probability of resources being replaced even if they function the same. Just my 2c.

[taylorb-syd]

I could see the utility in being more explicit about the potential implications, though on the same token I'd think it'd be a given that if you replace a construct with a different one, you have a high probability of resources being replaced even if they function the same. Just my 2c.

I think a long term solution to this will be to manage stack policies as part of CDK. By utilizing this feature, we can get users to mark a construct with constraints, like preventing deletion or replacement update. That way, if a consumer updates a construct with a new one and it isn't a slot in replacement, the update will fail due to a policy violation, which will encourage the customer to think about migration and, when ready, provide an override clause.

I'll talk internally to the CDK team about modeling this.

[luxaritas]

@taylorb-syd From a user level that would also be awesome if that means I could get extra protections on say an EC2 instance such that I don't get surprised by it being replaced by changing a property I didn't expect to lead to replacement. I didn't know I needed this until now 🙂

[elhedran]

@luxaritas Yes, 100% correct. But the assumption that this deprecation is based on is wrong:

CloudFormation first applies all updates before applying any deletions, so if you were to do it in a single operation it will first create the certificate and then update the CloudFront distributions to reference the new certificate and then delete the old certificate.

My problem is that the deprecation is pointing to the new Certificate construct and is positioning it as a drop-in replacement. When it is not, certificates will get destroyed, this might be fine if your cert and the service using the cert is in the same stack, then the logic above holds. But for many, a cert is used across many applications/stacks/regions, so this is not the case and you will only realize this once it is too late.

I'm a little confused (came here following breadcrumbs around the construct being deprecated). Could you just have two certificates active for a while in the CDK code? e.g. "add new class, keep old, deploy". Wait till everything changes over to new class, "remove old class, deploy" ? Intuitively that's how I'd deal with the migration - curious now as to whether that would work for you (I'm not up against the grindstone on this one yet, so have the luxury of being curious).

[exalted]

What about https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_certificatemanager.DnsValidatedCertificate.html#cleanuproute53records ? Currently we really like this feature for our disaster recovery tests where we just deploy and destroy our stacks.

Ditto this 👆.

What's the replacement for this now deprecated and seemingly non-existing feature which was previously available please?

Thanks.