search

aws / aws-cdk

The AWS Cloud Development Kit is a framework for defining cloud infrastructure in code
https://aws.amazon.com/cdk
Apache License 2.0
11.59k stars 3.89k forks source link

[certificatemanager] CNAME used for SSL Cert. validation is not deleted automatically. #11201

Closed kerbachi closed 1 year ago

kerbachi commented 3 years ago

description of the bug:

aws_certificatemanager.Certificate creates and validate the SSL Certificates with 'CertificateValidation.from_dns' but does not delete the CNAME after 'cdk destroy'

Reproduction Steps

    hosted_zone = aws_route53.HostedZone.from_hosted_zone_attributes(
        self,
        "HostedZone",
        hosted_zone_id='1234567890ABCDEF',
        zone_name='acme.com',
    )

    acm_certificate = aws_certificatemanager.Certificate(
        self,
        id="ACMSubdomain",
        domain_name='api.acme.com',
        subject_alternative_names=["*.api.acme.com"],
        validation=aws_certificatemanager.CertificateValidation.from_dns(hosted_zone=hosted_zone)
    )

What did you expect to happen?

"cdk destroy" should deleted the CNAME record it created in Route53 for DNS validation

What actually happened?

The CNAME record is not deleted automatically after "cdk destroy" or after deleting the CloudFormation Template

Environment

Other

This is :bug: Bug Report

njlynch commented 3 years ago

Related to #3333, with some of the same implications.

This is the built-in CloudFormation behavior, and I suspect it's intentional. The important point to note is that for the same AWS account and domain name, the same CNAME is generated. This means two (or more) certificates can share the same CNAME record for validation, and removing the record when one certificate is removed will impact the others' ability to be renewed.

The only option here -- besides lobbying to change the built-in CloudFormation behavior -- would be the creation of a new option (cleanupRecords=true) that creates a dedicated custom resource to delete the associated records. This is a healthy chunk of work for what appears to be somewhat minor benefits.

@kerbachi, can you provide any details of if/how this is negatively impacting you? Is it mostly the principle of cleanup, or is there some adverse side-effect to these records remaining I might not be aware of?

AntonioAngelino commented 3 years ago

The only option here -- besides lobbying to change the built-in CloudFormation behavior -- would be the creation of a new option (cleanupRecords=true) that creates a dedicated custom resource to delete the associated records. This is a healthy chunk of work for what appears to be somewhat minor benefits.

@njlynch That seems a good solution. Any plan to implement it?

We need to clean up the AWS subaccounts we assign to our customers, therefore it's mandatory that CDK deletes all the created resources.

coltenkrauter commented 2 years ago

Any updates here?

organom commented 1 year ago

was implemented with https://github.com/aws/aws-cdk/pull/18311 in the aws_certificatemanager.DnsValidatedCertificate construct by defining prop cleanupRoute53Records: true.

I guess this issue can also be close

madeline-k commented 1 year ago

Thanks, @organom. I agree with you, this one can be closed. Please also note that DnsValidatedCertificate has been deprecated and replaced with Certificate. If anyone still encounters a problem with this, please open a new issue.

github-actions[bot] commented 1 year ago

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

wv-tud commented 1 year ago

I'm having the exact same issue when using Certificate. Which is now forcing me to go back to using DnsValidatedCertificate.

ericchuawc commented 1 year ago

I'm having the exact same issue when using Certificate. Which is now forcing me to go back to using DnsValidatedCertificate.

I am getting the same issue with Certificate too. Any way to fix it without falling back to the old way?

stoyan-scava commented 1 year ago

@ericchuawc I'm afraid the problem is not in the Certificate construct (CDK) but in the CloudFormation Resource The issue is tracked here

liam-careerhub commented 1 year ago

As per this comment: https://github.com/aws-cloudformation/cloudformation-coverage-roadmap/issues/837#issuecomment-1468258709

Please upvote https://github.com/aws-cloudformation/cloudformation-coverage-roadmap/issues/837 if this is important to you

robzet commented 1 year ago

aws-cloudformation project has closed their issue and aws-cdk has closed theirs. Both projects pointing to eachother but CDK users loosing out on a working destroy command when you run essential infrastructure. Sad.

organom commented 1 year ago

Totally agree with @robzet . At the time or writing my original comment the DnsValidatedCertificate was still not deprecated, and I had no idea CDK would embark on this journey...

Internal AWS team fights are of little interest, and right now, I depend on a feature that was deprecated with no alternative being offered, outside of an excuse saying the other team should do it.

Please get your stuff together... Would it be better that cloudformation team would implement it properly? Sure everyone can agree with that, but as a product, you should only deprecate a feature when it was fixed by the upstream team or when a better alternative can be provided.

Just my sincere opinion that I believe lots of CDK users can relate to

@madeline-k can you please follow this with the cdk team, and hopefully fix or revert the internal decision?

msessa commented 8 months ago

For anyone interested, I have published a construct to mitigate this specific issue until a better fix comes from AWS.

https://www.npmjs.com/package/@servicevic-oss/cdk-cleanup-certificate-validation-records