feat(certificatemanager): DnsValidatedCertificate DNS record cleanup (#18311)

Adds an option to DnsValidatedCertificate to automatically cleanup the related
DNS validation records when the Certificate is deleted.

This is an opt-in property and discouraged for production use, as there are edge
cases that can cause unintended side effects. The most obvious is that if two or
more certificates exist with the same domain, the same validation record is used
for both. If one certificate is deleted (and deletes the validation record), the
second certificate (with the same domain name) will be unable to automatically
renew.

closes #3333
closes #7063

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

Author: njlynch

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

@@ -110,69 +110,28 @@ const requestCertificate = async function (requestId, domainName, subjectAlterna
 
   console.log('Waiting for ACM to provide DNS records for validation...');
 
-  let records;
-  for (let attempt = 0; attempt < maxAttempts && !records; attempt++) {
+  let records = [];
+  for (let attempt = 0; attempt < maxAttempts && !records.length; attempt++) {
     const { Certificate } = await acm.describeCertificate({
       CertificateArn: reqCertResponse.CertificateArn
     }).promise();
-    const options = Certificate.DomainValidationOptions || [];
-    // Ensure all records are ready; there is (at least a theory there's) a chance of a partial response here in rare cases.
-    if (options.length > 0 && options.every(opt => opt && !!opt.ResourceRecord)) {
-      // some alternative names will produce the same validation record
-      // as the main domain (eg. example.com + *.example.com)
-      // filtering duplicates to avoid errors with adding the same record
-      // to the route53 zone twice
-      const unique = options
-        .map((val) => val.ResourceRecord)
-        .reduce((acc, cur) => {
-          acc[cur.Name] = cur;
-          return acc;
-        }, {});
-      records = Object.keys(unique).sort().map(key => unique[key]);
-    } else {
+
+    records = getDomainValidationRecords(Certificate);
+    if (!records.length) {
       // Exponential backoff with jitter based on 200ms base
       // component of backoff fixed to ensure minimum total wait time on
       // slow targets.
       const base = Math.pow(2, attempt);
       await sleep(random() * base * 50 + base * 150);
     }
   }
-  if (!records) {
+  if (!records.length) {
     throw new Error(`Response from describeCertificate did not contain DomainValidationOptions after ${maxAttempts} attempts.`)
   }
 
-
   console.log(`Upserting ${records.length} DNS records into zone ${hostedZoneId}:`);
 
-  const changeBatch = await route53.changeResourceRecordSets({
-    ChangeBatch: {
-      Changes: records.map((record) => {
-        console.log(`${record.Name} ${record.Type} ${record.Value}`)
-        return {
-          Action: 'UPSERT',
-          ResourceRecordSet: {
-            Name: record.Name,
-            Type: record.Type,
-            TTL: 60,
-            ResourceRecords: [{
-              Value: record.Value
-            }]
-          }
-        };
-      }),
-    },
-    HostedZoneId: hostedZoneId
-  }).promise();
-
-  console.log('Waiting for DNS records to commit...');
-  await route53.waitFor('resourceRecordSetsChanged', {
-    // Wait up to 5 minutes
-    $waiter: {
-      delay: 30,
-      maxAttempts: 10
-    },
-    Id: changeBatch.ChangeInfo.Id
-  }).promise();
+  await commitRoute53Records(route53, records, hostedZoneId);
 
   console.log('Waiting for validation...');
   await acm.waitFor('certificateValidated', {
@@ -193,47 +152,126 @@ const requestCertificate = async function (requestId, domainName, subjectAlterna
  *
  * @param {string} arn The certificate ARN
  */
-const deleteCertificate = async function (arn, region) {
+const deleteCertificate = async function (arn, region, hostedZoneId, route53Endpoint, cleanupRecords) {
   const acm = new aws.ACM({ region });
+  const route53 = route53Endpoint ? new aws.Route53({ endpoint: route53Endpoint }) : new aws.Route53();
+  if (waiter) {
+    // Used by the test suite, since waiters aren't mockable yet
+    route53.waitFor = acm.waitFor = waiter;
+  }
 
   try {
     console.log(`Waiting for certificate ${arn} to become unused`);
 
     let inUseByResources;
+    let records = [];
     for (let attempt = 0; attempt < maxAttempts; attempt++) {
       const { Certificate } = await acm.describeCertificate({
         CertificateArn: arn
       }).promise();
 
+      if (cleanupRecords) {
+        records = getDomainValidationRecords(Certificate);
+      }
       inUseByResources = Certificate.InUseBy || [];
 
-      if (inUseByResources.length) {
+      if (inUseByResources.length || !records.length) {
         // Exponential backoff with jitter based on 200ms base
         // component of backoff fixed to ensure minimum total wait time on
         // slow targets.
         const base = Math.pow(2, attempt);
         await sleep(random() * base * 50 + base * 150);
       } else {
-        break
+        break;
       }
     }
 
     if (inUseByResources.length) {
       throw new Error(`Response from describeCertificate did not contain an empty InUseBy list after ${maxAttempts} attempts.`)
     }
+    if (cleanupRecords && !records.length) {
+      throw new Error(`Response from describeCertificate did not contain DomainValidationOptions after ${maxAttempts} attempts.`)
+    }
 
     console.log(`Deleting certificate ${arn}`);
 
     await acm.deleteCertificate({
       CertificateArn: arn
     }).promise();
+
+    if (cleanupRecords) {
+      console.log(`Deleting ${records.length} DNS records from zone ${hostedZoneId}:`);
+
+      await commitRoute53Records(route53, records, hostedZoneId, 'DELETE');
+    }
+
   } catch (err) {
     if (err.name !== 'ResourceNotFoundException') {
       throw err;
     }
   }
 };
 
+/**
+ * Retrieve the unique domain validation options as records to be upserted (or deleted) from Route53.
+ *
+ * Returns an empty array ([]) if the domain validation options is empty or the records are not yet ready.
+ */
+function getDomainValidationRecords(certificate) {
+  const options = certificate.DomainValidationOptions || [];
+  // Ensure all records are ready; there is (at least a theory there's) a chance of a partial response here in rare cases.
+  if (options.length > 0 && options.every(opt => opt && !!opt.ResourceRecord)) {
+    // some alternative names will produce the same validation record
+    // as the main domain (eg. example.com + *.example.com)
+    // filtering duplicates to avoid errors with adding the same record
+    // to the route53 zone twice
+    const unique = options
+      .map((val) => val.ResourceRecord)
+      .reduce((acc, cur) => {
+        acc[cur.Name] = cur;
+        return acc;
+      }, {});
+    return Object.keys(unique).sort().map(key => unique[key]);
+  }
+  return [];
+}
+
+/**
+ * Execute Route53 ChangeResourceRecordSets for a set of records within a Hosted Zone,
+ * and wait for the records to commit. Defaults to an 'UPSERT' action.
+ */
+async function commitRoute53Records(route53, records, hostedZoneId, action = 'UPSERT') {
+  const changeBatch = await route53.changeResourceRecordSets({
+    ChangeBatch: {
+      Changes: records.map((record) => {
+        console.log(`${record.Name} ${record.Type} ${record.Value}`);
+        return {
+          Action: action,
+          ResourceRecordSet: {
+            Name: record.Name,
+            Type: record.Type,
+            TTL: 60,
+            ResourceRecords: [{
+              Value: record.Value
+            }]
+          }
+        };
+      }),
+    },
+    HostedZoneId: hostedZoneId
+  }).promise();
+
+  console.log('Waiting for DNS records to commit...');
+  await route53.waitFor('resourceRecordSetsChanged', {
+    // Wait up to 5 minutes
+    $waiter: {
+      delay: 30,
+      maxAttempts: 10
+    },
+    Id: changeBatch.ChangeInfo.Id
+  }).promise();
+}
+
 /**
  * Main handler, invoked by Lambda
  */
@@ -262,7 +300,13 @@ exports.certificateRequestHandler = async function (event, context) {
         // 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);
+          await deleteCertificate(
+            physicalResourceId,
+            event.ResourceProperties.Region,
+            event.ResourceProperties.HostedZoneId,
+            event.ResourceProperties.Route53Endpoint,
+            event.ResourceProperties.CleanupRecords === "true",
+          );
         }
         break;
       default:

Diff for: packages/@aws-cdk/aws-certificatemanager/lambda-packages/dns_validated_certificate_handler/test/handler.test.js

@@ -996,4 +996,172 @@ describe('DNS Validated Certificate Handler', () => {
         expect(request.isDone()).toBe(true);
       });
   });
+
+  describe('Delete option record cleanup', () => {
+    let describeCertificateFake;
+    let deleteCertificateFake;
+    let changeResourceRecordSetsFake;
+
+    beforeEach(() => {
+      deleteCertificateFake = sinon.fake.resolves({});
+      AWS.mock('ACM', 'deleteCertificate', deleteCertificateFake);
+      changeResourceRecordSetsFake = sinon.fake.resolves({
+        ChangeInfo: {
+          Id: 'bogus'
+        }
+      });
+      AWS.mock('Route53', 'changeResourceRecordSets', changeResourceRecordSetsFake);
+
+      describeCertificateFake = sinon.fake.resolves({
+        Certificate: {
+          CertificateArn: testCertificateArn,
+          DomainValidationOptions: [{
+            ValidationStatus: 'SUCCESS',
+            ResourceRecord: {
+              Name: testRRName,
+              Type: 'CNAME',
+              Value: testRRValue
+            }
+          }]
+        }
+      });
+      AWS.mock('ACM', 'describeCertificate', describeCertificateFake);
+    });
+
+    test('ignores records if CleanupRecords is not set', () => {
+      const request = nock(ResponseURL).put('/', body => {
+        return body.Status === 'SUCCESS';
+      }).reply(200);
+
+      return LambdaTester(handler.certificateRequestHandler)
+        .event({
+          RequestType: 'Delete',
+          RequestId: testRequestId,
+          PhysicalResourceId: testCertificateArn,
+          ResourceProperties: {
+            Region: 'us-east-1',
+            HostedZoneId: testHostedZoneId,
+          }
+        })
+        .expectResolve(() => {
+          sinon.assert.calledWith(describeCertificateFake, sinon.match({
+            CertificateArn: testCertificateArn
+          }));
+          sinon.assert.calledWith(deleteCertificateFake, sinon.match({
+            CertificateArn: testCertificateArn
+          }));
+          sinon.assert.notCalled(changeResourceRecordSetsFake);
+          expect(request.isDone()).toBe(true);
+        });
+    });
+
+    test('ignores records if CleanupRecords is not set to "true"', () => {
+      const request = nock(ResponseURL).put('/', body => {
+        return body.Status === 'SUCCESS';
+      }).reply(200);
+
+      return LambdaTester(handler.certificateRequestHandler)
+        .event({
+          RequestType: 'Delete',
+          RequestId: testRequestId,
+          PhysicalResourceId: testCertificateArn,
+          ResourceProperties: {
+            Region: 'us-east-1',
+            HostedZoneId: testHostedZoneId,
+            CleanupRecords: 'TRUE', // Not "true"
+          }
+        })
+        .expectResolve(() => {
+          sinon.assert.calledWith(describeCertificateFake, sinon.match({
+            CertificateArn: testCertificateArn
+          }));
+          sinon.assert.calledWith(deleteCertificateFake, sinon.match({
+            CertificateArn: testCertificateArn
+          }));
+          sinon.assert.notCalled(changeResourceRecordSetsFake);
+          expect(request.isDone()).toBe(true);
+        });
+    });
+
+    test('deletes records if CleanupRecords is set to true and records are present', () => {
+      const request = nock(ResponseURL).put('/', body => {
+        return body.Status === 'SUCCESS';
+      }).reply(200);
+
+      AWS.mock('Route53', 'changeResourceRecordSets', changeResourceRecordSetsFake);
+
+      return LambdaTester(handler.certificateRequestHandler)
+        .event({
+          RequestType: 'Delete',
+          RequestId: testRequestId,
+          PhysicalResourceId: testCertificateArn,
+          ResourceProperties: {
+            Region: 'us-east-1',
+            HostedZoneId: testHostedZoneId,
+            CleanupRecords: 'true',
+          },
+        })
+        .expectResolve(() => {
+          sinon.assert.calledWith(describeCertificateFake, sinon.match({
+            CertificateArn: testCertificateArn
+          }));
+          sinon.assert.calledWith(deleteCertificateFake, sinon.match({
+            CertificateArn: testCertificateArn
+          }));
+          sinon.assert.calledWith(changeResourceRecordSetsFake, sinon.match({
+            ChangeBatch: {
+              Changes: [{
+                Action: 'DELETE',
+                ResourceRecordSet: {
+                  Name: testRRName,
+                  Type: 'CNAME',
+                  TTL: 60,
+                  ResourceRecords: [{
+                    Value: testRRValue
+                  }]
+                }
+              }]
+            },
+            HostedZoneId: testHostedZoneId
+          }));
+          expect(request.isDone()).toBe(true);
+        });
+    });
+
+    test('fails if CleanupRecords is set to true and records are not present', () => {
+      describeCertificateFake = sinon.fake.resolves({
+        Certificate: {
+          CertificateArn: testCertificateArn,
+        }
+      });
+      AWS.remock('ACM', 'describeCertificate', describeCertificateFake);
+
+      const request = nock(ResponseURL).put('/', body => {
+        return body.Status === 'FAILED' &&
+        body.Reason.startsWith('Response from describeCertificate did not contain DomainValidationOptions');
+      }).reply(200);
+
+      AWS.mock('Route53', 'changeResourceRecordSets', changeResourceRecordSetsFake);
+
+      return LambdaTester(handler.certificateRequestHandler)
+        .event({
+          RequestType: 'Delete',
+          RequestId: testRequestId,
+          PhysicalResourceId: testCertificateArn,
+          ResourceProperties: {
+            Region: 'us-east-1',
+            HostedZoneId: testHostedZoneId,
+            CleanupRecords: 'true',
+          },
+        })
+        .expectResolve(() => {
+          sinon.assert.calledWith(describeCertificateFake, sinon.match({
+            CertificateArn: testCertificateArn
+          }));
+          sinon.assert.notCalled(deleteCertificateFake);
+          sinon.assert.notCalled(changeResourceRecordSetsFake);
+          expect(request.isDone()).toBe(true);
+        });
+    });
+  });
 });