fix(certificatemanager): unable to set removal policy on DnsValidatedCertificate by corymhall · Pull Request #22040 · aws/aws-cdk

corymhall · 2022-09-14T17:45:32Z

This PR adds a method override for applyRemovalPolicy which allows the user to specify a removal policy for the DnsValidatedCertificate construct. Since this construct is backed by a custom resource, the lambda handler was updated to no longer delete the certificate if the RemovalPolicy is set to retain.

This is also needed to allow for an easier migration from DnsValidatedCertificate -> Certificate

fixes #20649

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

…Certificate This PR adds a method override for `applyRemovalPolicy` which allows the user to specify a removal policy for the `DnsValidatedCertificate` construct. Since this construct is backed by a custom resource, the lambda handler was updated to no longer delete the certificate if the `RemovalPolicy` is set to `retain`. This is also needed to allow for an easier migration from `DnsValidatedCertificate` -> `Certificate` fixes #20649

gitpod-io · 2022-09-14T17:45:39Z

mergify · 2022-09-15T14:34:43Z

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 · 2022-09-15T15:11:16Z

AWS CodeBuild CI Report

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

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

mergify · 2022-09-15T15:11:36Z

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

…alidatedCertificate (#22040)" (#22056) This reverts commit b3c9464. ---- ### 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*

…Certificate (#22122) This PR adds a method override for applyRemovalPolicy which allows the user to specify a removal policy for the DnsValidatedCertificate construct. Since this construct is backed by a custom resource, the lambda handler was updated to no longer delete the certificate if the RemovalPolicy is set to retain. This is also needed to allow for an easier migration from DnsValidatedCertificate -> Certificate reroll of #22040 This has the same changes as #22040 with the addition of some logic to handle only processing updates for certain parameters. If `RemovalPolicy` is changed for example, the update will not be processed. I also added an integration test with some manual instructions. In order to test ACM certificates I also updated the integ-runner to handle some additional special env variables. fixes #20649, fixes #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*

…Certificate (aws#22122) This PR adds a method override for applyRemovalPolicy which allows the user to specify a removal policy for the DnsValidatedCertificate construct. Since this construct is backed by a custom resource, the lambda handler was updated to no longer delete the certificate if the RemovalPolicy is set to retain. This is also needed to allow for an easier migration from DnsValidatedCertificate -> Certificate reroll of aws#22040 This has the same changes as aws#22040 with the addition of some logic to handle only processing updates for certain parameters. If `RemovalPolicy` is changed for example, the update will not be processed. I also added an integration test with some manual instructions. In order to test ACM certificates I also updated the integ-runner to handle some additional special env variables. fixes aws#20649, fixes aws#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*

…Certificate (aws#22040) This PR adds a method override for `applyRemovalPolicy` which allows the user to specify a removal policy for the `DnsValidatedCertificate` construct. Since this construct is backed by a custom resource, the lambda handler was updated to no longer delete the certificate if the `RemovalPolicy` is set to `retain`. This is also needed to allow for an easier migration from `DnsValidatedCertificate` -> `Certificate` fixes aws#20649 ---- ### 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*

…alidatedCertificate (aws#22040)" (aws#22056) This reverts commit b3c9464. ---- ### 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*

…Certificate (aws#22122) This PR adds a method override for applyRemovalPolicy which allows the user to specify a removal policy for the DnsValidatedCertificate construct. Since this construct is backed by a custom resource, the lambda handler was updated to no longer delete the certificate if the RemovalPolicy is set to retain. This is also needed to allow for an easier migration from DnsValidatedCertificate -> Certificate reroll of aws#22040 This has the same changes as aws#22040 with the addition of some logic to handle only processing updates for certain parameters. If `RemovalPolicy` is changed for example, the update will not be processed. I also added an integration test with some manual instructions. In order to test ACM certificates I also updated the integ-runner to handle some additional special env variables. fixes aws#20649, fixes aws#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*

aws-cdk-automation requested a review from a team September 14, 2022 17:45

github-actions bot added bug This issue is a bug. effort/small Small work item – less than a day of effort p2 labels Sep 14, 2022

mergify bot added the contribution/core This is a PR that came from AWS. label Sep 14, 2022

Naumel approved these changes Sep 15, 2022

View reviewed changes

Merge branch 'main' into corymhall/dnsvalidatedcertificate-retention

f7c03cc

TheRealAmazonKendra added the pr/do-not-merge This PR should not be merged at this time. label Sep 15, 2022

mergify bot merged commit b3c9464 into aws:main Sep 15, 2022

TheRealAmazonKendra removed the pr/do-not-merge This PR should not be merged at this time. label Sep 15, 2022

corymhall mentioned this pull request Sep 19, 2022

fix(certificatemanager): unable to set removal policy on DnsValidatedCertificate #22122

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(certificatemanager): unable to set removal policy on DnsValidatedCertificate#22040

fix(certificatemanager): unable to set removal policy on DnsValidatedCertificate#22040
mergify[bot] merged 2 commits intoaws:mainfrom
corymhall:corymhall/dnsvalidatedcertificate-retention

corymhall commented Sep 14, 2022

Uh oh!

gitpod-io bot commented Sep 14, 2022

Uh oh!

mergify bot commented Sep 15, 2022

Uh oh!

aws-cdk-automation commented Sep 15, 2022

Uh oh!

mergify bot commented Sep 15, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

corymhall commented Sep 14, 2022

All Submissions:

Adding new Unconventional Dependencies:

New Features

Uh oh!

gitpod-io bot commented Sep 14, 2022

Uh oh!

mergify bot commented Sep 15, 2022

Uh oh!

aws-cdk-automation commented Sep 15, 2022

AWS CodeBuild CI Report

Uh oh!

mergify bot commented Sep 15, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants