aws-certificatemanager: DnsValidatedCertificate instances throw on applyRemovalPolicy

[IggyMi]

Describe the bug

Calling applyRemovalPolicy on DnsValidatedCertificate instances throws CfnResource error and fails to synthesize the stack.

Expected Behavior

Chosen retention policy should be applied to DnsValidatedCertificate created resources.

Current Behavior

Currently calling applyRemovalPolicy on DnsValidatedCertificate instance will throw

Error: Cannot apply RemovalPolicy: no child or not a CfnResource. Apply the removal policy on the CfnResource directly.
    at DnsValidatedCertificate.applyRemovalPolicy (/Users/ignotasmikalauskas/Projects/resq-infra/certificates/node_modules/aws-cdk-lib/core/lib/resource.ts:227:13)

Reproduction Steps

The error is thrown while attempting to run the following construct

import { Construct } from 'constructs';
import { DnsValidatedCertificate } from 'aws-cdk-lib/aws-certificatemanager';
import { RemovalPolicy } from 'aws-cdk-lib';
import { IHostedZone } from 'aws-cdk-lib/aws-route53';

export interface CertificateProps {
  hostedZone: IHostedZone;
  domainName: string;
  region: string;
}

export class Certificate extends Construct {
  constructor(scope: Construct, id: string, props: CertificateProps) {
    super(scope, id);

    const { domainName, region, hostedZone } = props;

    const certificate = new DnsValidatedCertificate(this, 'Certificate', {
      domainName: `*.${domainName}`,
      hostedZone,
      region
    });
    certificate.applyRemovalPolicy(RemovalPolicy.RETAIN);

   // ... business logic ...
  }
}

Possible Solution

No response

Additional Information/Context

No response

CDK CLI Version

2.24.1 (build 585f9ca)

Framework Version

No response

Node.js Version

v16.14.0

OS

macOS Monterey 12.2.1

Language

Typescript

Language Version

3.9.7

Other information

No response

[peterwoodworth]

This happens because the DnsValidatedCertificate doesn't leverage the CfnCertificate resource, but rather utilizes a custom resource. As a result, custom resources can't have removal policy set the same way as when using the CloudFormation resource directly.

We should call this out in the docs, and potentially support setting removal policy through the custom resource if possible

[ruebroad]

@peterwoodworth how is this not a bug? If the method is available and should work but doesn't - isn't that a bug?

[peterwoodworth]

I think there are two problems here @ruebroad, one bug and one feature request

1. The bug: DnsValidatedCertificate allows you to call applyRemovalPolicy(). This is a consequence of the construct extending CertificateBase which extends Resource
   I think we still ultimately want this construct to extend resource, so you will still be able to call this method. I think that we could override this method on DnsValidatedCertificate to give a more clear error message about how there is no underlying CfnResource for this construct.
2. The FR: You cannot set the removal policy of DnsValidatedCertificate at all. This is a consequence of not being able to set the removal policy of custom resources the same way as most other Cfn resources, so it's not something that should work (though you're right, the method is available so we do claim to support it). We handle the deletion of the Certificate inside the custom resource here

   aws-cdk/packages/@aws-cdk/aws-certificatemanager/lambda-packages/dns_validated_certificate_handler/lib/index.js

   Lines 298 to 310 in 83f1668

   	case 'Delete':
   	physicalResourceId = event.PhysicalResourceId;
   	// If the resource didn't create correctly, the physical resource ID won't be the
   	// certificate ARN, so don't try to delete it in that case.
   	if (physicalResourceId.startsWith('arn:')) {
   	await deleteCertificate(
   	physicalResourceId,
   	event.ResourceProperties.Region,
   	event.ResourceProperties.HostedZoneId,
   	event.ResourceProperties.Route53Endpoint,
   	event.ResourceProperties.CleanupRecords === "true",
   	);
   	}

   
   We could do something like create a removalPolicy prop for DnsValidatedCertificate which ends up getting parsed in the custom resource here.

[vechorko]

just want to follow up this topic and know when it will be added in the next release

[peterwoodworth]

This isn't something the team will be able to get to soon. The quickest way to see this in a new release is for the community to contribute it 🙂

[github-actions]

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