Skip to content

Commit

Permalink
fix(acm): support Tokens for domainName in Certificate (#4251)
Browse files Browse the repository at this point in the history
Detect when someone tries to use a Token for a `domainName` in
`Certificate`, and require that they supply a validation domain when
that happens (necessary because we won't be able to properly calculate
the apex domain at CDK run time).

This does require that you use the exact same string in the
`validationDomains` map; since different instances of equivalent
tokens might evaluate to a different string, this requires that you
store the stringified token in a variable first.

Fixes #4232.
  • Loading branch information
rix0rrr authored and mergify[bot] committed Sep 27, 2019
1 parent caa0077 commit ee1283d
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 8 deletions.
16 changes: 10 additions & 6 deletions packages/@aws-cdk/aws-certificatemanager/lib/certificate.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Construct, IResource, Resource } from '@aws-cdk/core';
import { Construct, IResource, Resource, Token } from '@aws-cdk/core';
import { CfnCertificate } from './certificatemanager.generated';
import { apexDomain } from './util';

Expand Down Expand Up @@ -103,11 +103,15 @@ export class Certificate extends Resource implements ICertificate {
* Closes over props.
*/
function domainValidationOption(domainName: string): CfnCertificate.DomainValidationOptionProperty {
const overrideDomain = props.validationDomains && props.validationDomains[domainName];
return {
domainName,
validationDomain: overrideDomain || apexDomain(domainName)
};
let validationDomain = props.validationDomains && props.validationDomains[domainName];
if (validationDomain === undefined) {
if (Token.isUnresolved(domainName)) {
throw new Error(`When using Tokens for domain names, 'validationDomains' needs to be supplied`);
}
validationDomain = apexDomain(domainName);
}

return { domainName, validationDomain };
}
}
}
Expand Down
39 changes: 37 additions & 2 deletions packages/@aws-cdk/aws-certificatemanager/test/test.certificate.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { expect, haveResource } from '@aws-cdk/assert';
import { Stack } from '@aws-cdk/core';
import { Lazy, Stack } from '@aws-cdk/core';
import { Test } from 'nodeunit';
import { Certificate, ValidationMethod } from '../lib';

Expand Down Expand Up @@ -54,7 +54,7 @@ export = {
test.done();
},

'can configure validatin method'(test: Test) {
'can configure validation method'(test: Test) {
const stack = new Stack();

new Certificate(stack, 'Certificate', {
Expand All @@ -69,4 +69,39 @@ export = {

test.done();
},

'needs validation domain supplied if domain contains a token'(test: Test) {
const stack = new Stack();

test.throws(() => {
const domainName = Lazy.stringValue({ produce: () => 'example.com' });
new Certificate(stack, 'Certificate', {
domainName,
});
}, /'validationDomains' needs to be supplied/);

test.done();
},

'validationdomains can be given for a Token'(test: Test) {
const stack = new Stack();

const domainName = Lazy.stringValue({ produce: () => 'my.example.com' });
new Certificate(stack, 'Certificate', {
domainName,
validationDomains: {
[domainName]: 'example.com'
}
});

expect(stack).to(haveResource('AWS::CertificateManager::Certificate', {
DomainName: 'my.example.com',
DomainValidationOptions: [{
DomainName: 'my.example.com',
ValidationDomain: 'example.com'
}]
}));

test.done();
},
};

0 comments on commit ee1283d

Please sign in to comment.