Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Merged
merged 2 commits into from
Oct 2, 2022
Merged

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

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a531990
fix(certificatemanager): unable to set removal policy on DnsValidated…
corymhall Sep 19, 2022
c9ae40b
Merge branch 'main' into corymhall/dnsvalidatedcertificate-retention-2
mergify[bot] Oct 2, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 64 additions & 24 deletions ...cdk/aws-certificatemanager/lambda-packages/dns_validated_certificate_handler/lib/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,23 @@ let report = function (event, context, responseStatus, physicalResourceId, respo
});
};

/**
* Adds tags to an existing certificate
*
* @param {string} certificateArn the ARN of the certificate to add tags to
* @param {string} region the region the certificate exists in
* @param {map} tags Tags to add to the requested certificate
*/
const addTags = async function(certificateArn, region, tags) {
const result = Array.from(Object.entries(tags)).map(([Key, Value]) => ({ Key, Value }))
const acm = new aws.ACM({ region });

await acm.addTagsToCertificate({
CertificateArn: certificateArn,
Tags: result,
}).promise();
}

/**
* Requests a public certificate from AWS Certificate Manager, using DNS validation.
* The hosted zone ID must refer to a **public** Route53-managed DNS zone that is authoritative
Expand All @@ -75,10 +92,9 @@ let report = function (event, context, responseStatus, physicalResourceId, respo
* @param {string} requestId the CloudFormation request ID
* @param {string} domainName the Common Name (CN) field for the requested certificate
* @param {string} hostedZoneId the Route53 Hosted Zone ID
* @param {map} tags Tags to add to the requested certificate
* @returns {string} Validated certificate ARN
*/
const requestCertificate = async function (requestId, domainName, subjectAlternativeNames, certificateTransparencyLoggingPreference, hostedZoneId, region, route53Endpoint, tags) {
const requestCertificate = async function (requestId, domainName, subjectAlternativeNames, certificateTransparencyLoggingPreference, hostedZoneId, region, route53Endpoint) {
const crypto = require('crypto');
const acm = new aws.ACM({ region });
const route53 = route53Endpoint ? new aws.Route53({ endpoint: route53Endpoint }) : new aws.Route53();
Expand All @@ -101,16 +117,6 @@ const requestCertificate = async function (requestId, domainName, subjectAlterna

console.log(`Certificate ARN: ${reqCertResponse.CertificateArn}`);


if (!!tags) {
const result = Array.from(Object.entries(tags)).map(([Key, Value]) => ({ Key, Value }))

await acm.addTagsToCertificate({
CertificateArn: reqCertResponse.CertificateArn,
Tags: result,
}).promise();
}

console.log('Waiting for ACM to provide DNS records for validation...');

let records = [];
Expand Down Expand Up @@ -275,35 +281,69 @@ async function commitRoute53Records(route53, records, hostedZoneId, action = 'UP
}).promise();
}

/**
* Determines whether an update request should request a new certificate
*
* @param {map} oldParams the previously process request parameters
* @param {map} newParams the current process request parameters
* @param {string} physicalResourceId the physicalResourceId
* @returns {boolean} whether or not to request a new certificate
*/
function shouldUpdate(oldParams, newParams, physicalResourceId) {
if (!oldParams) return true;
if (oldParams.DomainName !== newParams.DomainName) return true;
if (oldParams.SubjectAlternativeNames !== newParams.SubjectAlternativeNames) return true;
if (oldParams.CertificateTransparencyLoggingPreference !== newParams.CertificateTransparencyLoggingPreference) return true;
if (oldParams.HostedZoneId !== newParams.HostedZoneId) return true;
if (oldParams.Region !== newParams.Region) return true;
if (!physicalResourceId || !physicalResourceId.startsWith('arn:')) return true;
return false;
}

/**
* Main handler, invoked by Lambda
*/
exports.certificateRequestHandler = async function (event, context) {
var responseData = {};
var physicalResourceId;
var certificateArn;
async function processRequest() {
certificateArn = await requestCertificate(
event.RequestId,
event.ResourceProperties.DomainName,
event.ResourceProperties.SubjectAlternativeNames,
event.ResourceProperties.CertificateTransparencyLoggingPreference,
event.ResourceProperties.HostedZoneId,
event.ResourceProperties.Region,
event.ResourceProperties.Route53Endpoint,
);
responseData.Arn = physicalResourceId = certificateArn;
}

try {
switch (event.RequestType) {
case 'Create':
await processRequest();
if (event.ResourceProperties.Tags && physicalResourceId.startsWith('arn:')) {
await addTags(physicalResourceId, event.ResourceProperties.Region, event.ResourceProperties.Tags);
}
break;
case 'Update':
certificateArn = await requestCertificate(
event.RequestId,
event.ResourceProperties.DomainName,
event.ResourceProperties.SubjectAlternativeNames,
event.ResourceProperties.CertificateTransparencyLoggingPreference,
event.ResourceProperties.HostedZoneId,
event.ResourceProperties.Region,
event.ResourceProperties.Route53Endpoint,
event.ResourceProperties.Tags,
);
responseData.Arn = physicalResourceId = certificateArn;
if (shouldUpdate(event.OldResourceProperties, event.ResourceProperties, event.PhysicalResourceId)) {
await processRequest();
} else {
responseData.Arn = physicalResourceId = event.PhysicalResourceId;
}
if (event.ResourceProperties.Tags && physicalResourceId.startsWith('arn:')) {
await addTags(physicalResourceId, event.ResourceProperties.Region, event.ResourceProperties.Tags);
}
break;
case 'Delete':
physicalResourceId = event.PhysicalResourceId;
const removalPolicy = event.ResourceProperties.RemovalPolicy ?? 'destroy';
// 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:')) {
if (physicalResourceId.startsWith('arn:') && removalPolicy === 'destroy') {
await deleteCertificate(
physicalResourceId,
event.ResourceProperties.Region,
Expand Down
237 changes: 237 additions & 0 deletions ...certificatemanager/lambda-packages/dns_validated_certificate_handler/test/handler.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -869,6 +869,243 @@ describe('DNS Validated Certificate Handler', () => {
});
});

test('Update operation requests a certificate', () => {
const requestCertificateFake = sinon.fake.resolves({
CertificateArn: testCertificateArn,
});

const describeCertificateFake = sinon.stub();
describeCertificateFake.onFirstCall().resolves({
Certificate: {
CertificateArn: testCertificateArn
}
});
describeCertificateFake.resolves({
Certificate: {
CertificateArn: testCertificateArn,
DomainValidationOptions: [{
ValidationStatus: 'SUCCESS',
ResourceRecord: {
Name: testRRName,
Type: 'CNAME',
Value: testRRValue
}
}]
}
});

const addTagsToCertificateFake = sinon.fake.resolves({});

const changeResourceRecordSetsFake = sinon.fake.resolves({
ChangeInfo: {
Id: 'bogus'
}
});

AWS.mock('ACM', 'requestCertificate', requestCertificateFake);
AWS.mock('ACM', 'describeCertificate', describeCertificateFake);
AWS.mock('Route53', 'changeResourceRecordSets', changeResourceRecordSetsFake);
AWS.mock('ACM', 'addTagsToCertificate', addTagsToCertificateFake);

const request = nock(ResponseURL).put('/', body => {
return body.Status === 'SUCCESS';
}).reply(200);

return LambdaTester(handler.certificateRequestHandler)
.event({
RequestType: 'Update',
RequestId: testRequestId,
OldResourceProperties: {
DomainName: 'example.com',
HostedZoneId: testHostedZoneId,
Region: 'us-east-1',
Tags: testTags
},
ResourceProperties: {
DomainName: testDomainName,
HostedZoneId: testHostedZoneId,
Region: 'us-east-1',
Tags: testTags
}
})
.expectResolve(() => {
sinon.assert.calledWith(requestCertificateFake, sinon.match({
DomainName: testDomainName,
ValidationMethod: 'DNS',
Options: {
CertificateTransparencyLoggingPreference: undefined
}
}));
sinon.assert.calledWith(changeResourceRecordSetsFake, sinon.match({
ChangeBatch: {
Changes: [{
Action: 'UPSERT',
ResourceRecordSet: {
Name: testRRName,
Type: 'CNAME',
TTL: 60,
ResourceRecords: [{
Value: testRRValue
}]
}
}]
},
HostedZoneId: testHostedZoneId
}));
sinon.assert.calledWith(addTagsToCertificateFake, sinon.match({
"CertificateArn": testCertificateArn,
"Tags": testTagsValue,
}));
expect(request.isDone()).toBe(true);
});
});

test('Update operation updates tags only', () => {
const requestCertificateFake = sinon.fake.resolves({
CertificateArn: testCertificateArn,
});

const describeCertificateFake = sinon.stub();
describeCertificateFake.onFirstCall().resolves({
Certificate: {
CertificateArn: testCertificateArn
}
});
describeCertificateFake.resolves({
Certificate: {
CertificateArn: testCertificateArn,
DomainValidationOptions: [{
ValidationStatus: 'SUCCESS',
ResourceRecord: {
Name: testRRName,
Type: 'CNAME',
Value: testRRValue
}
}]
}
});

const addTagsToCertificateFake = sinon.fake.resolves({});

const changeResourceRecordSetsFake = sinon.fake.resolves({
ChangeInfo: {
Id: 'bogus'
}
});

AWS.mock('ACM', 'requestCertificate', requestCertificateFake);
AWS.mock('ACM', 'describeCertificate', describeCertificateFake);
AWS.mock('Route53', 'changeResourceRecordSets', changeResourceRecordSetsFake);
AWS.mock('ACM', 'addTagsToCertificate', addTagsToCertificateFake);

const request = nock(ResponseURL).put('/', body => {
return body.Status === 'SUCCESS';
}).reply(200);

return LambdaTester(handler.certificateRequestHandler)
.event({
RequestType: 'Update',
RequestId: testRequestId,
PhysicalResourceId: testCertificateArn,
OldResourceProperties: {
DomainName: testDomainName,
HostedZoneId: testHostedZoneId,
Region: 'us-east-1',
Tags: testTags,
},
ResourceProperties: {
DomainName: testDomainName,
HostedZoneId: testHostedZoneId,
Region: 'us-east-1',
Tags: {
...testTags,
Tag4: 'Value4',
},
}
})
.expectResolve(() => {
sinon.assert.notCalled(requestCertificateFake);
sinon.assert.notCalled(changeResourceRecordSetsFake);
sinon.assert.calledWith(addTagsToCertificateFake, sinon.match({
"CertificateArn": testCertificateArn,
"Tags": [{ Key: 'Tag1', Value: 'Test1' }, { Key: 'Tag2', Value: 'Test2' }, { Key: 'Tag4', Value: 'Value4' }],
}));
expect(request.isDone()).toBe(true);
});
});

test('Update operation does not request certificate if removal policy is changed', () => {
const requestCertificateFake = sinon.fake.resolves({
CertificateArn: testCertificateArn,
});

const describeCertificateFake = sinon.stub();
describeCertificateFake.onFirstCall().resolves({
Certificate: {
CertificateArn: testCertificateArn
}
});
describeCertificateFake.resolves({
Certificate: {
CertificateArn: testCertificateArn,
DomainValidationOptions: [{
ValidationStatus: 'SUCCESS',
ResourceRecord: {
Name: testRRName,
Type: 'CNAME',
Value: testRRValue
}
}]
}
});

const addTagsToCertificateFake = sinon.fake.resolves({});

const changeResourceRecordSetsFake = sinon.fake.resolves({
ChangeInfo: {
Id: 'bogus'
}
});

AWS.mock('ACM', 'requestCertificate', requestCertificateFake);
AWS.mock('ACM', 'describeCertificate', describeCertificateFake);
AWS.mock('Route53', 'changeResourceRecordSets', changeResourceRecordSetsFake);
AWS.mock('ACM', 'addTagsToCertificate', addTagsToCertificateFake);

const request = nock(ResponseURL).put('/', body => {
return body.Status === 'SUCCESS';
}).reply(200);

return LambdaTester(handler.certificateRequestHandler)
.event({
RequestType: 'Update',
RequestId: testRequestId,
PhysicalResourceId: testCertificateArn,
OldResourceProperties: {
DomainName: testDomainName,
HostedZoneId: testHostedZoneId,
Region: 'us-east-1',
Tags: testTags,
},
ResourceProperties: {
DomainName: testDomainName,
HostedZoneId: testHostedZoneId,
Region: 'us-east-1',
Tags: testTags,
RemovalPolicy: 'retain',
}
})
.expectResolve(() => {
sinon.assert.notCalled(requestCertificateFake);
sinon.assert.notCalled(changeResourceRecordSetsFake);
sinon.assert.calledWith(addTagsToCertificateFake, sinon.match({
"CertificateArn": testCertificateArn,
"Tags": testTagsValue,
}));
expect(request.isDone()).toBe(true);
});
});

test('Delete operation succeeds if certificate becomes not-in-use', () => {
const usedByArn = 'arn:aws:cloudfront::123456789012:distribution/d111111abcdef8';

Expand Down
Loading