From c12f7c38926af796566d97cfc0375828e4b7e7d2 Mon Sep 17 00:00:00 2001 From: Alexander Zeitler Date: Wed, 27 Nov 2019 21:54:40 +0100 Subject: [PATCH 1/3] fix(route53): return plain hosted zone id without /hostedzone/ prefix When creating a HostedZone, the `hostedZoneId` is the ID of this hosted zone, such as "Z23ABC4XYZL05B". Until how, when importing a HostedZone using route53.HostedZone.fromLookup, it did return the `hostedZoneId` in this format: /hostedzone/Z23ABC4XYZL05B. This commit makes the value of `hostedZoneId` consistent between creating a new `HostedZone`, using `HostedZone.fromLookup` and `fromHostedZoneAttributes`. BREAKING CHANGE: value of `hostedZoneId` is now be different compared to previous versions of CDK when using `HostedZone.fromLookup` or `fromHostedZoneAttributes` --- packages/@aws-cdk/aws-route53/lib/hosted-zone.ts | 4 ++-- .../@aws-cdk/aws-route53/test/test.hosted-zone-provider.ts | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/@aws-cdk/aws-route53/lib/hosted-zone.ts b/packages/@aws-cdk/aws-route53/lib/hosted-zone.ts index 324b863eaee1e..ff16e392c995b 100644 --- a/packages/@aws-cdk/aws-route53/lib/hosted-zone.ts +++ b/packages/@aws-cdk/aws-route53/lib/hosted-zone.ts @@ -62,7 +62,7 @@ export class HostedZone extends Resource implements IHostedZone { */ public static fromHostedZoneAttributes(scope: Construct, id: string, attrs: HostedZoneAttributes): IHostedZone { class Import extends Resource implements IHostedZone { - public readonly hostedZoneId = attrs.hostedZoneId; + public readonly hostedZoneId = attrs.hostedZoneId.replace('/hostedzone/', ''); public readonly zoneName = attrs.zoneName; } @@ -74,7 +74,7 @@ export class HostedZone extends Resource implements IHostedZone { */ public static fromLookup(scope: Construct, id: string, query: HostedZoneProviderProps): IHostedZone { const DEFAULT_HOSTED_ZONE: HostedZoneContextResponse = { - Id: '/hostedzone/DUMMY', + Id: 'DUMMY', Name: query.domainName, }; diff --git a/packages/@aws-cdk/aws-route53/test/test.hosted-zone-provider.ts b/packages/@aws-cdk/aws-route53/test/test.hosted-zone-provider.ts index 4043fb02f0d6d..2c2b71f2d52d7 100644 --- a/packages/@aws-cdk/aws-route53/test/test.hosted-zone-provider.ts +++ b/packages/@aws-cdk/aws-route53/test/test.hosted-zone-provider.ts @@ -15,8 +15,9 @@ export = { const missing = SynthUtils.synthesize(stack).assembly.manifest.missing!; test.ok(missing && missing.length === 1); + const fakeZoneId = '11111111111111'; const fakeZone = { - Id: "/hostedzone/11111111111111", + Id: `/hostedzone/${fakeZoneId}`, Name: "example.com.", CallerReference: "TestLates-PublicZo-OESZPDFV7G6A", Config: { From 667f65ff312af2966c383e779bff4d74d3e95324 Mon Sep 17 00:00:00 2001 From: Alexander Zeitler Date: Fri, 29 Nov 2019 10:04:05 +0100 Subject: [PATCH 2/3] test: ensure zone is resolved correctly when using no /hostedzone prefix --- .../test/test.hosted-zone-provider.ts | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/packages/@aws-cdk/aws-route53/test/test.hosted-zone-provider.ts b/packages/@aws-cdk/aws-route53/test/test.hosted-zone-provider.ts index 2c2b71f2d52d7..4d573665bd6cb 100644 --- a/packages/@aws-cdk/aws-route53/test/test.hosted-zone-provider.ts +++ b/packages/@aws-cdk/aws-route53/test/test.hosted-zone-provider.ts @@ -44,5 +44,44 @@ export = { test.deepEqual(zoneRef.hostedZoneId, cdkZone.hostedZoneId); test.done(); }, + 'HostedZoneProvider will return context values if availble when using plain hosted zone id'(test: Test) { + // GIVEN + const stack = new cdk.Stack(undefined, 'TestStack', { env: { account: '12345', region: 'us-east-1' } }); + const filter = {domainName: 'test.com'}; + + HostedZone.fromLookup(stack, 'Ref', filter); + + const missing = SynthUtils.synthesize(stack).assembly.manifest.missing!; + test.ok(missing && missing.length === 1); + + const fakeZoneId = '11111111111111'; + const fakeZone = { + Id: fakeZoneId, + Name: "example.com.", + CallerReference: "TestLates-PublicZo-OESZPDFV7G6A", + Config: { + Comment: "CDK created", + PrivateZone: false + }, + ResourceRecordSetCount: 3 + }; + + const stack2 = new cdk.Stack(undefined, 'TestStack', { env: { account: '12345', region: 'us-east-1' } }); + stack2.node.setContext(missing[0].key, fakeZone); + + const cdkZoneProps: HostedZoneAttributes = { + hostedZoneId: fakeZone.Id, + zoneName: 'example.com', + }; + + const cdkZone = HostedZone.fromHostedZoneAttributes(stack2, 'MyZone', cdkZoneProps); + + // WHEN + const zoneId = cdkZone.hostedZoneId; + + // THEN + test.deepEqual(fakeZoneId, zoneId); + test.done(); + }, } }; From 6b141223ee188c959c06a6492480cfbfa31f8ce1 Mon Sep 17 00:00:00 2001 From: Shiv Lakshminarayan Date: Sat, 7 Dec 2019 18:06:29 -0800 Subject: [PATCH 3/3] modified fix into `fromValue` instead. add hostedZoneArn attribute & tests. --- .../test.load-balanced-fargate-service.ts | 1 + .../aws-route53/lib/hosted-zone-ref.ts | 7 +++ .../@aws-cdk/aws-route53/lib/hosted-zone.ts | 21 ++++++- packages/@aws-cdk/aws-route53/lib/util.ts | 11 ++++ .../test/test.hosted-zone-provider.ts | 62 +++++++++---------- .../aws-route53/test/test.hosted-zone.ts | 32 ++++++++++ 6 files changed, 100 insertions(+), 34 deletions(-) create mode 100644 packages/@aws-cdk/aws-route53/test/test.hosted-zone.ts diff --git a/packages/@aws-cdk/aws-ecs-patterns/test/fargate/test.load-balanced-fargate-service.ts b/packages/@aws-cdk/aws-ecs-patterns/test/fargate/test.load-balanced-fargate-service.ts index 8a80c866b6ac8..171221d942a15 100644 --- a/packages/@aws-cdk/aws-ecs-patterns/test/fargate/test.load-balanced-fargate-service.ts +++ b/packages/@aws-cdk/aws-ecs-patterns/test/fargate/test.load-balanced-fargate-service.ts @@ -329,6 +329,7 @@ export = { domainZone: { hostedZoneId: 'fakeId', zoneName: 'domain.com', + hostedZoneArn: 'arn:aws:route53:::hostedzone/fakeId', stack, node: stack.node, }, diff --git a/packages/@aws-cdk/aws-route53/lib/hosted-zone-ref.ts b/packages/@aws-cdk/aws-route53/lib/hosted-zone-ref.ts index cdc0cfc17bd88..718689a0e32db 100644 --- a/packages/@aws-cdk/aws-route53/lib/hosted-zone-ref.ts +++ b/packages/@aws-cdk/aws-route53/lib/hosted-zone-ref.ts @@ -16,6 +16,13 @@ export interface IHostedZone extends IResource { */ readonly zoneName: string; + /** + * ARN of this hosted zone, such as arn:${Partition}:route53:::hostedzone/${Id} + * + * @attribute + */ + readonly hostedZoneArn: string; + /** * Returns the set of name servers for the specific hosted zone. For example: * ns1.example.com. diff --git a/packages/@aws-cdk/aws-route53/lib/hosted-zone.ts b/packages/@aws-cdk/aws-route53/lib/hosted-zone.ts index ff16e392c995b..fb5cb671e27a0 100644 --- a/packages/@aws-cdk/aws-route53/lib/hosted-zone.ts +++ b/packages/@aws-cdk/aws-route53/lib/hosted-zone.ts @@ -5,7 +5,7 @@ import { HostedZoneProviderProps } from './hosted-zone-provider'; import { HostedZoneAttributes, IHostedZone } from './hosted-zone-ref'; import { CaaAmazonRecord, ZoneDelegationRecord } from './record-set'; import { CfnHostedZone } from './route53.generated'; -import { validateZoneName } from './util'; +import { makeHostedZoneArn, validateZoneName } from './util'; export interface CommonHostedZoneProps { /** @@ -45,6 +45,9 @@ export interface HostedZoneProps extends CommonHostedZoneProps { } export class HostedZone extends Resource implements IHostedZone { + public get hostedZoneArn(): string { + return makeHostedZoneArn(this, this.hostedZoneId); + } public static fromHostedZoneId(scope: Construct, id: string, hostedZoneId: string): IHostedZone { class Import extends Resource implements IHostedZone { @@ -52,6 +55,9 @@ export class HostedZone extends Resource implements IHostedZone { public get zoneName(): string { throw new Error(`HostedZone.fromHostedZoneId doesn't support "zoneName"`); } + public get hostedZoneArn(): string { + return makeHostedZoneArn(this, this.hostedZoneId); + } } return new Import(scope, id); @@ -62,8 +68,11 @@ export class HostedZone extends Resource implements IHostedZone { */ public static fromHostedZoneAttributes(scope: Construct, id: string, attrs: HostedZoneAttributes): IHostedZone { class Import extends Resource implements IHostedZone { - public readonly hostedZoneId = attrs.hostedZoneId.replace('/hostedzone/', ''); + public readonly hostedZoneId = attrs.hostedZoneId; public readonly zoneName = attrs.zoneName; + public get hostedZoneArn(): string { + return makeHostedZoneArn(this, this.hostedZoneId); + } } return new Import(scope, id); @@ -94,6 +103,8 @@ export class HostedZone extends Resource implements IHostedZone { response.Name = response.Name.substring(0, response.Name.length - 1); } + response.Id = response.Id.replace('/hostedzone/', ''); + return HostedZone.fromHostedZoneAttributes(scope, id, { hostedZoneId: response.Id, zoneName: response.Name, @@ -166,6 +177,9 @@ export class PublicHostedZone extends HostedZone implements IPublicHostedZone { class Import extends Resource implements IPublicHostedZone { public readonly hostedZoneId = publicHostedZoneId; public get zoneName(): string { throw new Error(`cannot retrieve "zoneName" from an an imported hosted zone`); } + public get hostedZoneArn(): string { + return makeHostedZoneArn(this, this.hostedZoneId); + } } return new Import(scope, id); } @@ -246,6 +260,9 @@ export class PrivateHostedZone extends HostedZone implements IPrivateHostedZone class Import extends Resource implements IPrivateHostedZone { public readonly hostedZoneId = privateHostedZoneId; public get zoneName(): string { throw new Error(`cannot retrieve "zoneName" from an an imported hosted zone`); } + public get hostedZoneArn(): string { + return makeHostedZoneArn(this, this.hostedZoneId); + } } return new Import(scope, id); } diff --git a/packages/@aws-cdk/aws-route53/lib/util.ts b/packages/@aws-cdk/aws-route53/lib/util.ts index 358f39f2da7ce..a1cd2fce5a6f9 100644 --- a/packages/@aws-cdk/aws-route53/lib/util.ts +++ b/packages/@aws-cdk/aws-route53/lib/util.ts @@ -1,3 +1,4 @@ +import { Construct, Stack } from '@aws-cdk/core'; import { IHostedZone } from './hosted-zone-ref'; /** @@ -56,3 +57,13 @@ export function determineFullyQualifiedDomainName(providedName: string, hostedZo return `${providedName}${suffix}.`; } + +export function makeHostedZoneArn(construct: Construct, hostedZoneId: string): string { + return Stack.of(construct).formatArn({ + account: '', + region: '', + service: 'route53', + resource: 'hostedzone', + resourceName: hostedZoneId + }); +} diff --git a/packages/@aws-cdk/aws-route53/test/test.hosted-zone-provider.ts b/packages/@aws-cdk/aws-route53/test/test.hosted-zone-provider.ts index 4d573665bd6cb..12f8a90d66f9b 100644 --- a/packages/@aws-cdk/aws-route53/test/test.hosted-zone-provider.ts +++ b/packages/@aws-cdk/aws-route53/test/test.hosted-zone-provider.ts @@ -1,14 +1,16 @@ import { SynthUtils } from '@aws-cdk/assert'; import cdk = require('@aws-cdk/core'); import { Test } from 'nodeunit'; -import { HostedZone, HostedZoneAttributes } from '../lib'; +import { HostedZone } from '../lib'; export = { 'Hosted Zone Provider': { - 'HostedZoneProvider will return context values if availble'(test: Test) { + 'HostedZoneProvider will return context values if available'(test: Test) { // GIVEN - const stack = new cdk.Stack(undefined, 'TestStack', { env: { account: '12345', region: 'us-east-1' } }); - const filter = {domainName: 'test.com'}; + const stack = new cdk.Stack(undefined, 'TestStack', { + env: { account: '12345', region: 'us-east-1' } + }); + const filter = { domainName: 'test.com' }; HostedZone.fromLookup(stack, 'Ref', filter); @@ -18,36 +20,35 @@ export = { const fakeZoneId = '11111111111111'; const fakeZone = { Id: `/hostedzone/${fakeZoneId}`, - Name: "example.com.", - CallerReference: "TestLates-PublicZo-OESZPDFV7G6A", + Name: 'example.com.', + CallerReference: 'TestLates-PublicZo-OESZPDFV7G6A', Config: { - Comment: "CDK created", + Comment: 'CDK created', PrivateZone: false }, ResourceRecordSetCount: 3 }; - const stack2 = new cdk.Stack(undefined, 'TestStack', { env: { account: '12345', region: 'us-east-1' } }); + const stack2 = new cdk.Stack(undefined, 'TestStack', { + env: { account: '12345', region: 'us-east-1' } + }); stack2.node.setContext(missing[0].key, fakeZone); - const cdkZoneProps: HostedZoneAttributes = { - hostedZoneId: fakeZone.Id, - zoneName: 'example.com', - }; - - const cdkZone = HostedZone.fromHostedZoneAttributes(stack2, 'MyZone', cdkZoneProps); - // WHEN const zoneRef = HostedZone.fromLookup(stack2, 'MyZoneProvider', filter); // THEN - test.deepEqual(zoneRef.hostedZoneId, cdkZone.hostedZoneId); + test.deepEqual(zoneRef.hostedZoneId, fakeZoneId); test.done(); }, - 'HostedZoneProvider will return context values if availble when using plain hosted zone id'(test: Test) { + 'HostedZoneProvider will return context values if available when using plain hosted zone id'( + test: Test + ) { // GIVEN - const stack = new cdk.Stack(undefined, 'TestStack', { env: { account: '12345', region: 'us-east-1' } }); - const filter = {domainName: 'test.com'}; + const stack = new cdk.Stack(undefined, 'TestStack', { + env: { account: '12345', region: 'us-east-1' } + }); + const filter = { domainName: 'test.com' }; HostedZone.fromLookup(stack, 'Ref', filter); @@ -56,32 +57,29 @@ export = { const fakeZoneId = '11111111111111'; const fakeZone = { - Id: fakeZoneId, - Name: "example.com.", - CallerReference: "TestLates-PublicZo-OESZPDFV7G6A", + Id: `/hostedzone/${fakeZoneId}`, + Name: 'example.com.', + CallerReference: 'TestLates-PublicZo-OESZPDFV7G6A', Config: { - Comment: "CDK created", + Comment: 'CDK created', PrivateZone: false }, ResourceRecordSetCount: 3 }; - const stack2 = new cdk.Stack(undefined, 'TestStack', { env: { account: '12345', region: 'us-east-1' } }); + const stack2 = new cdk.Stack(undefined, 'TestStack', { + env: { account: '12345', region: 'us-east-1' } + }); stack2.node.setContext(missing[0].key, fakeZone); - const cdkZoneProps: HostedZoneAttributes = { - hostedZoneId: fakeZone.Id, - zoneName: 'example.com', - }; - - const cdkZone = HostedZone.fromHostedZoneAttributes(stack2, 'MyZone', cdkZoneProps); + const zone = HostedZone.fromLookup(stack2, 'MyZoneProvider', filter); // WHEN - const zoneId = cdkZone.hostedZoneId; + const zoneId = zone.hostedZoneId; // THEN test.deepEqual(fakeZoneId, zoneId); test.done(); - }, + } } }; diff --git a/packages/@aws-cdk/aws-route53/test/test.hosted-zone.ts b/packages/@aws-cdk/aws-route53/test/test.hosted-zone.ts new file mode 100644 index 0000000000000..1f0b7e42315d5 --- /dev/null +++ b/packages/@aws-cdk/aws-route53/test/test.hosted-zone.ts @@ -0,0 +1,32 @@ +import cdk = require('@aws-cdk/core'); +import { Test } from 'nodeunit'; +import { HostedZone } from '../lib'; + +export = { + 'Hosted Zone': { + 'Hosted Zone constructs the ARN'(test: Test) { + // GIVEN + const stack = new cdk.Stack(undefined, 'TestStack', { + env: { account: '123456789012', region: 'us-east-1' } + }); + + const testZone = new HostedZone(stack, 'HostedZone', { + zoneName: 'testZone' + }); + + test.deepEqual(stack.resolve(testZone.hostedZoneArn), { + 'Fn::Join': [ + '', + [ + 'arn:', + { Ref: 'AWS::Partition' }, + ':route53:::hostedzone/', + { Ref: 'HostedZoneDB99F866' } + ] + ] + }); + + test.done(); + } + } +};