From 6c9fdd02c0d15c0a9e690169f6ec410d30a99b34 Mon Sep 17 00:00:00 2001 From: timothy-farestad <72991529+timothy-farestad@users.noreply.github.com> Date: Thu, 15 Apr 2021 06:39:35 -0500 Subject: [PATCH] feat(certificatemanager): allow tagging DnsValidatedCertificate (#13990) Closes #12382 Attempting to implement the fix suggested in #12382 to allow the DnsValidatedCertificate resource to be taggable. Currently, only the custom lambda that is created is tagged, but the certificate provisioned by the lambda is not tagged. This would allow the lambda to pass tags through to the certificate, too. *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../lib/index.js | 45 +++--- .../test/handler.test.js | 133 +++++++++++++++++- .../lib/dns-validated-certificate.ts | 18 ++- .../test/dns-validated-certificate.test.ts | 33 ++++- 4 files changed, 205 insertions(+), 24 deletions(-) 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 866e9405b049e..d34f7922f4e7e 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 @@ -2,7 +2,7 @@ const aws = require('aws-sdk'); -const defaultSleep = function(ms) { +const defaultSleep = function (ms) { return new Promise(resolve => setTimeout(resolve, ms)); }; @@ -24,7 +24,7 @@ let maxAttempts = 10; * @param {string} [reason] reason for failure, if any, to convey to the user * @returns {Promise} Promise that is resolved on success, or rejected on connection error or HTTP error response */ -let report = function(event, context, responseStatus, physicalResourceId, responseData, reason) { +let report = function (event, context, responseStatus, physicalResourceId, responseData, reason) { return new Promise((resolve, reject) => { const https = require('https'); const { URL } = require('url'); @@ -75,12 +75,13 @@ let report = function(event, context, responseStatus, physicalResourceId, respon * @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, hostedZoneId, region, route53Endpoint) { +const requestCertificate = async function (requestId, domainName, subjectAlternativeNames, hostedZoneId, region, route53Endpoint, tags) { const crypto = require('crypto'); const acm = new aws.ACM({ region }); - const route53 = route53Endpoint ? new aws.Route53({endpoint: route53Endpoint}) : new aws.Route53(); + 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; @@ -97,6 +98,16 @@ const requestCertificate = async function(requestId, domainName, subjectAlternat 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; @@ -129,6 +140,7 @@ const requestCertificate = async function(requestId, domainName, subjectAlternat 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({ @@ -180,7 +192,7 @@ const requestCertificate = async function(requestId, domainName, subjectAlternat * * @param {string} arn The certificate ARN */ -const deleteCertificate = async function(arn, region) { +const deleteCertificate = async function (arn, region) { const acm = new aws.ACM({ region }); try { @@ -224,7 +236,7 @@ const deleteCertificate = async function(arn, region) { /** * Main handler, invoked by Lambda */ -exports.certificateRequestHandler = async function(event, context) { +exports.certificateRequestHandler = async function (event, context) { var responseData = {}; var physicalResourceId; var certificateArn; @@ -240,6 +252,7 @@ exports.certificateRequestHandler = async function(event, context) { event.ResourceProperties.HostedZoneId, event.ResourceProperties.Region, event.ResourceProperties.Route53Endpoint, + event.ResourceProperties.Tags, ); responseData.Arn = physicalResourceId = certificateArn; break; @@ -267,69 +280,69 @@ exports.certificateRequestHandler = async function(event, context) { /** * @private */ -exports.withReporter = function(reporter) { +exports.withReporter = function (reporter) { report = reporter; }; /** * @private */ -exports.withDefaultResponseURL = function(url) { +exports.withDefaultResponseURL = function (url) { defaultResponseURL = url; }; /** * @private */ -exports.withWaiter = function(w) { +exports.withWaiter = function (w) { waiter = w; }; /** * @private */ -exports.resetWaiter = function() { +exports.resetWaiter = function () { waiter = undefined; }; /** * @private */ -exports.withSleep = function(s) { +exports.withSleep = function (s) { sleep = s; } /** * @private */ -exports.resetSleep = function() { +exports.resetSleep = function () { sleep = defaultSleep; } /** * @private */ -exports.withRandom = function(r) { +exports.withRandom = function (r) { random = r; } /** * @private */ -exports.resetRandom = function() { +exports.resetRandom = function () { random = Math.random; } /** * @private */ -exports.withMaxAttempts = function(ma) { +exports.withMaxAttempts = function (ma) { maxAttempts = ma; } /** * @private */ -exports.resetMaxAttempts = function() { +exports.resetMaxAttempts = function () { maxAttempts = 10; } 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 3e93f09680a91..5b53bc0a46301 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 @@ -20,13 +20,15 @@ describe('DNS Validated Certificate Handler', () => { const testRRValue = '_x2.acm-validations.aws'; const testAltRRName = '_3639ac514e785e898d2646601fa951d5.foo.example.com'; const testAltRRValue = '_x3.acm-validations.aws'; - const spySleep = sinon.spy(function(ms) { + const testTags = { Tag1: 'Test1', Tag2: 'Test2' }; + const testTagsValue = [{ Key: 'Tag1', Value: 'Test1' }, { Key: 'Tag2', Value: 'Test2' }]; + const spySleep = sinon.spy(function (ms) { return Promise.resolve(); }); beforeEach(() => { handler.withDefaultResponseURL(ResponseURL); - handler.withWaiter(function() { + handler.withWaiter(function () { // Mock waiter is merely a self-fulfilling promise return { promise: () => { @@ -37,7 +39,7 @@ describe('DNS Validated Certificate Handler', () => { }; }); handler.withSleep(spySleep); - console.log = function() { }; + console.log = function () { }; }); afterEach(() => { // Restore waiters and logger @@ -99,6 +101,8 @@ describe('DNS Validated Certificate Handler', () => { } }); + const addTagsToCertificateFake = sinon.fake.resolves({}); + const changeResourceRecordSetsFake = sinon.fake.resolves({ ChangeInfo: { Id: 'bogus' @@ -108,6 +112,7 @@ describe('DNS Validated Certificate Handler', () => { 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'; @@ -121,6 +126,7 @@ describe('DNS Validated Certificate Handler', () => { DomainName: testDomainName, HostedZoneId: testHostedZoneId, Region: 'us-east-1', + Tags: testTags } }) .expectResolve(() => { @@ -144,6 +150,10 @@ describe('DNS Validated Certificate Handler', () => { }, HostedZoneId: testHostedZoneId })); + sinon.assert.calledWith(addTagsToCertificateFake, sinon.match({ + "CertificateArn": testCertificateArn, + "Tags": testTagsValue, + })); expect(request.isDone()).toBe(true); }); }); @@ -182,14 +192,21 @@ describe('DNS Validated Certificate Handler', () => { } }); + const addTagsToCertificateFake = sinon.fake.resolves({ + Certificate: testCertificateArn, + Tags: testTags, + }); + const changeResourceRecordSetsFake = sinon.fake.resolves({ ChangeInfo: { Id: 'bogus' } }); + AWS.mock('ACM', 'requestCertificate', requestCertificateFake); AWS.mock('ACM', 'describeCertificate', describeCertificateFake); + AWS.mock('ACM', 'addTagsToCertificate', addTagsToCertificateFake); AWS.mock('Route53', 'changeResourceRecordSets', changeResourceRecordSetsFake); const request = nock(ResponseURL).put('/', body => { @@ -205,6 +222,7 @@ describe('DNS Validated Certificate Handler', () => { SubjectAlternativeNames: [testSubjectAlternativeName], HostedZoneId: testHostedZoneId, Region: 'us-east-1', + Tags: testTags, } }) .expectResolve(() => { @@ -241,6 +259,10 @@ describe('DNS Validated Certificate Handler', () => { }, HostedZoneId: testHostedZoneId })); + sinon.assert.calledWith(addTagsToCertificateFake, sinon.match({ + "CertificateArn": testCertificateArn, + "Tags": testTagsValue, + })); expect(request.isDone()).toBe(true); }); }); @@ -286,6 +308,11 @@ describe('DNS Validated Certificate Handler', () => { } }); + const addTagsToCertificateFake = sinon.fake.resolves({ + Certificate: testCertificateArn, + Tags: testTags, + }); + const changeResourceRecordSetsFake = sinon.fake.resolves({ ChangeInfo: { Id: 'bogus' @@ -294,6 +321,7 @@ describe('DNS Validated Certificate Handler', () => { AWS.mock('ACM', 'requestCertificate', requestCertificateFake); AWS.mock('ACM', 'describeCertificate', describeCertificateFake); + AWS.mock('ACM', 'addTagsToCertificate', addTagsToCertificateFake); AWS.mock('Route53', 'changeResourceRecordSets', changeResourceRecordSetsFake); const request = nock(ResponseURL).put('/', body => { @@ -308,6 +336,7 @@ describe('DNS Validated Certificate Handler', () => { DomainName: testDomainName, HostedZoneId: testHostedZoneId, Region: 'us-east-1', + Tags: testTags, } }) .expectResolve(() => { @@ -343,6 +372,10 @@ describe('DNS Validated Certificate Handler', () => { }, HostedZoneId: testHostedZoneId })); + sinon.assert.calledWith(addTagsToCertificateFake, sinon.match({ + "CertificateArn": testCertificateArn, + "Tags": testTagsValue, + })); expect(request.isDone()).toBe(true); }); }); @@ -453,6 +486,12 @@ describe('DNS Validated Certificate Handler', () => { }); AWS.mock('ACM', 'describeCertificate', describeCertificateFake); + const addTagsToCertificateFake = sinon.fake.resolves({ + Certificate: testCertificateArn, + Tags: testTags, + }); + AWS.mock('ACM', 'addTagsToCertificate', addTagsToCertificateFake); + const changeResourceRecordSetsFake = sinon.fake.resolves({ ChangeInfo: { Id: 'bogus' @@ -472,6 +511,7 @@ describe('DNS Validated Certificate Handler', () => { DomainName: testDomainName, HostedZoneId: testHostedZoneId, Region: 'us-east-1', + Tags: testTags, } }) .expectResolve(() => { @@ -479,6 +519,93 @@ describe('DNS Validated Certificate Handler', () => { sinon.assert.calledWith(describeCertificateFake, sinon.match({ CertificateArn: testCertificateArn, })); + sinon.assert.calledWith(addTagsToCertificateFake, sinon.match({ + "CertificateArn": testCertificateArn, + "Tags": testTagsValue, + })); + expect(request.isDone()).toBe(true); + }); + }); + + test('Create operation succeeds with no tags passed', () => { + 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 changeResourceRecordSetsFake = sinon.fake.resolves({ + ChangeInfo: { + Id: 'bogus' + } + }); + + const addTagsToCertificateFake = sinon.fake.resolves({ + Certificate: testCertificateArn, + }); + + 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: 'Create', + RequestId: testRequestId, + ResourceProperties: { + DomainName: testDomainName, + HostedZoneId: testHostedZoneId, + Region: 'us-east-1' + } + }) + .expectResolve(() => { + sinon.assert.calledWith(requestCertificateFake, sinon.match({ + DomainName: testDomainName, + ValidationMethod: 'DNS' + })); + sinon.assert.calledWith(changeResourceRecordSetsFake, sinon.match({ + ChangeBatch: { + Changes: [{ + Action: 'UPSERT', + ResourceRecordSet: { + Name: testRRName, + Type: 'CNAME', + TTL: 60, + ResourceRecords: [{ + Value: testRRValue + }] + } + }] + }, + HostedZoneId: testHostedZoneId + })); + sinon.assert.neverCalledWith(addTagsToCertificateFake, sinon.match({ + "CertificateArn": testCertificateArn, + "Tags": testTagsValue, + })); 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 6bb389afea97e..8d7fa32c94c9e 100644 --- a/packages/@aws-cdk/aws-certificatemanager/lib/dns-validated-certificate.ts +++ b/packages/@aws-cdk/aws-certificatemanager/lib/dns-validated-certificate.ts @@ -46,6 +46,7 @@ export interface DnsValidatedCertificateProps extends CertificateProps { * @default - A new role will be created */ readonly customResourceRole?: iam.IRole; + } /** @@ -55,8 +56,15 @@ export interface DnsValidatedCertificateProps extends CertificateProps { * @resource AWS::CertificateManager::Certificate * @experimental */ -export class DnsValidatedCertificate extends cdk.Resource implements ICertificate { +export class DnsValidatedCertificate extends cdk.Resource implements ICertificate, cdk.ITaggable { public readonly certificateArn: string; + + /** + * Resource Tags. + * @see https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-certificatemanager-certificate.html#cfn-certificatemanager-certificate-tags + */ + + public readonly tags: cdk.TagManager; private normalizedZoneName: string; private hostedZoneId: string; private domainName: string; @@ -73,6 +81,7 @@ export class DnsValidatedCertificate extends cdk.Resource implements ICertificat // Remove any `/hostedzone/` prefix from the Hosted Zone ID this.hostedZoneId = props.hostedZone.hostedZoneId.replace(/^\/hostedzone\//, ''); + this.tags = new cdk.TagManager(cdk.TagType.MAP, 'AWS::CertificateManager::Certificate'); const requestorFunction = new lambda.Function(this, 'CertificateRequestorFunction', { code: lambda.Code.fromAsset(path.resolve(__dirname, '..', 'lambda-packages', 'dns_validated_certificate_handler', 'lib')), @@ -82,7 +91,7 @@ export class DnsValidatedCertificate extends cdk.Resource implements ICertificat role: props.customResourceRole, }); requestorFunction.addToRolePolicy(new iam.PolicyStatement({ - actions: ['acm:RequestCertificate', 'acm:DescribeCertificate', 'acm:DeleteCertificate'], + actions: ['acm:RequestCertificate', 'acm:DescribeCertificate', 'acm:DeleteCertificate', 'acm:AddTagsToCertificate'], resources: ['*'], })); requestorFunction.addToRolePolicy(new iam.PolicyStatement({ @@ -102,6 +111,7 @@ export class DnsValidatedCertificate extends cdk.Resource implements ICertificat HostedZoneId: this.hostedZoneId, Region: props.region, Route53Endpoint: props.route53Endpoint, + Tags: cdk.Lazy.list({ produce: () => this.tags.renderTags() }), }, }); @@ -112,8 +122,8 @@ export class DnsValidatedCertificate extends cdk.Resource implements ICertificat const errors: string[] = []; // Ensure the zone name is a parent zone of the certificate domain name if (!cdk.Token.isUnresolved(this.normalizedZoneName) && - this.domainName !== this.normalizedZoneName && - !this.domainName.endsWith('.' + this.normalizedZoneName)) { + this.domainName !== this.normalizedZoneName && + !this.domainName.endsWith('.' + this.normalizedZoneName)) { errors.push(`DNS zone ${this.normalizedZoneName} is not authoritative for certificate domain name ${this.domainName}`); } return errors; 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 5074a69b3ac62..1b0a667b52738 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 @@ -2,7 +2,7 @@ import '@aws-cdk/assert-internal/jest'; import { SynthUtils } from '@aws-cdk/assert-internal'; import * as iam from '@aws-cdk/aws-iam'; import { HostedZone, PublicHostedZone } from '@aws-cdk/aws-route53'; -import { App, Stack, Token } from '@aws-cdk/core'; +import { App, Stack, Token, Tags } from '@aws-cdk/core'; import { DnsValidatedCertificate } from '../lib/dns-validated-certificate'; test('creates CloudFormation Custom Resource', () => { @@ -49,6 +49,7 @@ test('creates CloudFormation Custom Resource', () => { 'acm:RequestCertificate', 'acm:DescribeCertificate', 'acm:DeleteCertificate', + 'acm:AddTagsToCertificate', ], Effect: 'Allow', Resource: '*', @@ -136,6 +137,36 @@ test('test root certificate', () => { }); }); +test('test tags are passed to customresource', () => { + const stack = new Stack(); + Tags.of(stack).add('Key1', 'Value1'); + + const exampleDotComZone = new PublicHostedZone(stack, 'ExampleDotCom', { + zoneName: 'example.com', + }); + + new DnsValidatedCertificate(stack, 'Cert', { + domainName: 'example.com', + hostedZone: exampleDotComZone, + }); + + expect(stack).toHaveResource('AWS::CloudFormation::CustomResource', { + ServiceToken: { + 'Fn::GetAtt': [ + 'CertCertificateRequestorFunction98FDF273', + 'Arn', + ], + }, + DomainName: 'example.com', + HostedZoneId: { + Ref: 'ExampleDotCom4D1B83AA', + }, + Tags: { + Key1: 'Value1', + }, + }); +}); + test('works with imported zone', () => { // GIVEN const app = new App();