diff --git a/packages/@aws-cdk/aws-certificatemanager/lambda-packages/dns_validated_certificate_handler/lib/index.js b/packages/@aws-cdk/aws-certificatemanager/lambda-packages/dns_validated_certificate_handler/lib/index.js index 528dd55eb460a..672b5762dbc15 100644 --- a/packages/@aws-cdk/aws-certificatemanager/lambda-packages/dns_validated_certificate_handler/lib/index.js +++ b/packages/@aws-cdk/aws-certificatemanager/lambda-packages/dns_validated_certificate_handler/lib/index.js @@ -110,26 +110,14 @@ 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. @@ -137,42 +125,13 @@ const requestCertificate = async function (requestId, domainName, subjectAlterna 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,40 +152,59 @@ 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; @@ -234,6 +212,66 @@ const deleteCertificate = async function (arn, region) { } }; +/** + * 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 --git a/packages/@aws-cdk/aws-certificatemanager/lambda-packages/dns_validated_certificate_handler/test/handler.test.js b/packages/@aws-cdk/aws-certificatemanager/lambda-packages/dns_validated_certificate_handler/test/handler.test.js index 9f180f20211e9..a6a0632fd5aeb 100644 --- a/packages/@aws-cdk/aws-certificatemanager/lambda-packages/dns_validated_certificate_handler/test/handler.test.js +++ b/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); + }); + }); + }); }); diff --git a/packages/@aws-cdk/aws-certificatemanager/lib/dns-validated-certificate.ts b/packages/@aws-cdk/aws-certificatemanager/lib/dns-validated-certificate.ts index 60c965e21a6ab..db7dc6f1f8663 100644 --- a/packages/@aws-cdk/aws-certificatemanager/lib/dns-validated-certificate.ts +++ b/packages/@aws-cdk/aws-certificatemanager/lib/dns-validated-certificate.ts @@ -47,6 +47,17 @@ export interface DnsValidatedCertificateProps extends CertificateProps { */ readonly customResourceRole?: iam.IRole; + /** + * When set to true, when the DnsValidatedCertificate is deleted, + * the associated Route53 validation records are removed. + * + * CAUTION: If multiple certificates share the same domains (and same validation records), + * this can cause the other certificates to fail renewal and/or not validate. + * Not recommended for production use. + * + * @default false + */ + readonly cleanupRoute53Records?: boolean; } /** @@ -113,6 +124,8 @@ export class DnsValidatedCertificate extends CertificateBase implements ICertifi HostedZoneId: this.hostedZoneId, Region: props.region, Route53Endpoint: props.route53Endpoint, + // Custom resources properties are always converted to strings; might as well be explict here. + CleanupRecords: props.cleanupRoute53Records ? 'true' : undefined, Tags: cdk.Lazy.list({ produce: () => this.tags.renderTags() }), }, }); diff --git a/packages/@aws-cdk/aws-certificatemanager/test/dns-validated-certificate.test.ts b/packages/@aws-cdk/aws-certificatemanager/test/dns-validated-certificate.test.ts index 935f8680415b4..d6881d61d79ac 100644 --- a/packages/@aws-cdk/aws-certificatemanager/test/dns-validated-certificate.test.ts +++ b/packages/@aws-cdk/aws-certificatemanager/test/dns-validated-certificate.test.ts @@ -14,10 +14,13 @@ test('creates CloudFormation Custom Resource', () => { new DnsValidatedCertificate(stack, 'Certificate', { domainName: 'test.example.com', hostedZone: exampleDotComZone, + subjectAlternativeNames: ['test2.example.com'], + cleanupRoute53Records: true, }); Template.fromStack(stack).hasResourceProperties('AWS::CloudFormation::CustomResource', { DomainName: 'test.example.com', + SubjectAlternativeNames: ['test2.example.com'], ServiceToken: { 'Fn::GetAtt': [ 'CertificateCertificateRequestorFunction5E845413', @@ -27,6 +30,7 @@ test('creates CloudFormation Custom Resource', () => { HostedZoneId: { Ref: 'ExampleDotCom4D1B83AA', }, + CleanupRecords: 'true', }); Template.fromStack(stack).hasResourceProperties('AWS::Lambda::Function', { Handler: 'index.certificateRequestHandler',