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

(certificatemanager): stack loses track of DnsValidatedCertificate #17349

Closed
williamputraintan opened this issue Nov 5, 2021 · 3 comments
Closed
Labels
@aws-cdk/aws-certificatemanager Related to Amazon Certificate Manager bug This issue is a bug. effort/small Small work item – less than a day of effort needs-reproduction This issue needs reproduction. p1

Comments

@williamputraintan
Copy link

williamputraintan commented Nov 5, 2021

What is the problem?

After aws_certificatemanager.DnsValidatedCertificate has been deployed and modifying aws_certificatemanager.DnsValidatedCertificate while it still has a pending status it will just create a new one and old ones are being forgotten. As a result, a lot of certificates are created but does not belong to a cdk stack

Reproduction Steps

  1. Construct aws_certificatemanager.DnsValidatedCertificate with a pending status (Reason for this, it needs manual validation from a different account, typo on alternative domain name)
  2. Modify the aws_certificatemanager.DnsValidatedCertificate (for example add a new alternative domain name, or fix typo domain name)

What did you expect to happen?

The previous certificate is deleted and is replaced with the new certificate with the new configuration. Or at least a warning of the previous certificate is not deleted.

What actually happened?

New certificates keep on regenerating with the old one is not tracked with the cdk anymore.

CDK CLI Version

1.130.0 (build 9c094ae)

Framework Version

No response

Node.js Version

N/A

OS

Windows

Language

Python

Language Version

No response

Other information

No response

@williamputraintan williamputraintan added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Nov 5, 2021
@github-actions github-actions bot added the @aws-cdk/aws-certificatemanager Related to Amazon Certificate Manager label Nov 5, 2021
@peterwoodworth peterwoodworth changed the title CDK lose track of aws_certificatemanager.DnsValidatedCertificate (certificatemanager): stack loses track of DnsValidatedCertificate Nov 5, 2021
@ryparker ryparker added effort/small Small work item – less than a day of effort needs-reproduction This issue needs reproduction. and removed needs-triage This issue or PR still needs to be triaged. labels Nov 9, 2021
@njlynch
Copy link
Contributor

njlynch commented Dec 1, 2021

Thanks for this bug report. It does indeed look like the custom resource handler always requests a new certificate (for a create/update), without deleting any existing certificates. Given each update -- sans metadata like tags -- requires a new certificate, this is a likely scenario and should probably be addressed.

In the meantime, I always strongly recommend you use the Certificate construct if you can; the only valid reason to use DnsValidatedCertificate is for cross-region use cases.

@njlynch njlynch removed their assignment Dec 1, 2021
@ChrisLane
Copy link

@njlynch A lot of people use DnsValidatedCertificate over Certificate because of the extra properties available for the construct.
It didn't seem clear to me in the docs that Certificate should be preferred.

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/small Small work item – less than a day of effort needs-reproduction This issue needs reproduction. p1
Projects
None yet
Development

No branches or pull requests

6 participants