From b5bd39e479f342830b2a0fb89e75202ef657cbad Mon Sep 17 00:00:00 2001 From: Luca Pizzini Date: Fri, 25 Aug 2023 17:24:32 +0200 Subject: [PATCH] fix(route53): IHostedZone cannot be used for ses.Identity.publicHostedZone anymore (#26888) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Identity.publicHostedZone` takes an `IPublicHostedZone`, but because of TypeScript structural typing it would also accept an `IHostedZone`. When in [this PR](https://github.com/aws/aws-cdk/pull/26333) the `grantDelegation` method was added to the `IPublicHostedZone` interface, this passing was no longer allowed and code that used to work on accident, no longer works. For example: ``` const zone = HostedZone.fromHostedZoneId(stack, 'Zone', 'hosted-id'); const sesIdentity = ses.Identity.publicHostedZone(zone); ``` It raises an error because the imported `zone` does not implement the `grantDelegation` method. This fix moves the `grantDelegation` method declaration into the `IHostedZone` interface and makes it available to all imported zones. Closes #26872. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- ...-route53-cross-account-integ.template.json | 50 ++++++-- .../integ.cross-account-zone-delegation.ts | 8 +- packages/aws-cdk-lib/aws-route53/README.md | 10 +- .../aws-route53/lib/hosted-zone-ref.ts | 6 + .../aws-route53/lib/hosted-zone.ts | 28 +++-- .../aws-route53/test/hosted-zone.test.ts | 107 +++++++++++++++++- 6 files changed, 178 insertions(+), 31 deletions(-) diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-route53/test/integ.cross-account-zone-delegation.js.snapshot/aws-cdk-route53-cross-account-integ.template.json b/packages/@aws-cdk-testing/framework-integ/test/aws-route53/test/integ.cross-account-zone-delegation.js.snapshot/aws-cdk-route53-cross-account-integ.template.json index 1d8521c793e6e..f5143b937bed8 100644 --- a/packages/@aws-cdk-testing/framework-integ/test/aws-route53/test/integ.cross-account-zone-delegation.js.snapshot/aws-cdk-route53-cross-account-integ.template.json +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-route53/test/integ.cross-account-zone-delegation.js.snapshot/aws-cdk-route53-cross-account-integ.template.json @@ -354,18 +354,44 @@ } }, "Effect": "Allow", - "Resource": { - "Fn::Join": [ - "", - [ - "arn:", - { - "Ref": "AWS::Partition" - }, - ":route53:::hostedzone/imported-public-zone-id" - ] - ] - } + "Resource": [ + { + "Fn::Join": [ + "", + [ + "arn:", + { + "Ref": "AWS::Partition" + }, + ":route53:::hostedzone/imported-private-zone-id" + ] + ] + }, + { + "Fn::Join": [ + "", + [ + "arn:", + { + "Ref": "AWS::Partition" + }, + ":route53:::hostedzone/imported-public-zone-id" + ] + ] + }, + { + "Fn::Join": [ + "", + [ + "arn:", + { + "Ref": "AWS::Partition" + }, + ":route53:::hostedzone/imported-zone-id" + ] + ] + } + ] }, { "Action": "route53:ListHostedZonesByName", diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-route53/test/integ.cross-account-zone-delegation.ts b/packages/@aws-cdk-testing/framework-integ/test/aws-route53/test/integ.cross-account-zone-delegation.ts index e8e5d3d154b1b..d4065c8a7fa3f 100644 --- a/packages/@aws-cdk-testing/framework-integ/test/aws-route53/test/integ.cross-account-zone-delegation.ts +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-route53/test/integ.cross-account-zone-delegation.ts @@ -1,6 +1,6 @@ import * as iam from 'aws-cdk-lib/aws-iam'; import * as cdk from 'aws-cdk-lib'; -import { PublicHostedZone, CrossAccountZoneDelegationRecord } from 'aws-cdk-lib/aws-route53'; +import { PublicHostedZone, CrossAccountZoneDelegationRecord, PrivateHostedZone, HostedZone } from 'aws-cdk-lib/aws-route53'; import { IntegTest } from '@aws-cdk/integ-tests-alpha'; const app = new cdk.App(); @@ -36,9 +36,15 @@ const role = new iam.Role(stack, 'Role', { assumedBy: new iam.AccountRootPrincipal(), }); +const importedZone = HostedZone.fromHostedZoneId(stack, 'ImportedZone', 'imported-zone-id'); +importedZone.grantDelegation(role); + const importedPublicZone = PublicHostedZone.fromPublicHostedZoneId(stack, 'ImportedPublicZone', 'imported-public-zone-id'); importedPublicZone.grantDelegation(role); +const importedPrivateZone = PrivateHostedZone.fromPrivateHostedZoneId(stack, 'ImportedPrivateZone', 'imported-private-zone-id'); +importedPrivateZone.grantDelegation(role); + new IntegTest(app, 'Route53CrossAccountInteg', { testCases: [stack], diffAssets: true, diff --git a/packages/aws-cdk-lib/aws-route53/README.md b/packages/aws-cdk-lib/aws-route53/README.md index af2036ca097fd..91dc7baeee353 100644 --- a/packages/aws-cdk-lib/aws-route53/README.md +++ b/packages/aws-cdk-lib/aws-route53/README.md @@ -289,7 +289,7 @@ const zoneFromAttributes = route53.PublicHostedZone.fromPublicHostedZoneAttribut const zoneFromId = route53.PublicHostedZone.fromPublicHostedZoneId(this, 'MyZone', 'ZOJJZC49E0EPZ'); ``` -You can use `CrossAccountZoneDelegationRecord` on imported Public Hosted Zones with the `grantDelegation` method: +You can use `CrossAccountZoneDelegationRecord` on imported Hosted Zones with the `grantDelegation` method: ```ts const crossAccountRole = new iam.Role(this, 'CrossAccountRole', { @@ -299,8 +299,14 @@ const crossAccountRole = new iam.Role(this, 'CrossAccountRole', { assumedBy: new iam.AccountPrincipal('12345678901'), }); -const zoneFromId = route53.PublicHostedZone.fromPublicHostedZoneId(this, 'MyZone', 'ZOJJZC49E0EPZ'); +const zoneFromId = route53.HostedZone.fromHostedZoneId(this, 'MyZone', 'zone-id'); zoneFromId.grantDelegation(crossAccountRole); + +const publicZoneFromId = route53.PublicHostedZone.fromPublicHostedZoneId(this, 'MyPublicZone', 'public-zone-id'); +publicZoneFromId.grantDelegation(crossAccountRole); + +const privateZoneFromId = route53.PrivateHostedZone.fromPrivateHostedZoneId(this, 'MyPrivateZone', 'private-zone-id'); +privateZoneFromId.grantDelegation(crossAccountRole); ``` ## VPC Endpoint Service Private DNS diff --git a/packages/aws-cdk-lib/aws-route53/lib/hosted-zone-ref.ts b/packages/aws-cdk-lib/aws-route53/lib/hosted-zone-ref.ts index f18ccb608e59c..8672c3f7460fc 100644 --- a/packages/aws-cdk-lib/aws-route53/lib/hosted-zone-ref.ts +++ b/packages/aws-cdk-lib/aws-route53/lib/hosted-zone-ref.ts @@ -1,3 +1,4 @@ +import * as iam from '../../aws-iam'; import { IResource } from '../../core'; /** @@ -32,6 +33,11 @@ export interface IHostedZone extends IResource { * @attribute */ readonly hostedZoneNameServers?: string[]; + + /** + * Grant permissions to add delegation records to this zone + */ + grantDelegation(grantee: iam.IGrantable): iam.Grant; } /** diff --git a/packages/aws-cdk-lib/aws-route53/lib/hosted-zone.ts b/packages/aws-cdk-lib/aws-route53/lib/hosted-zone.ts index cbb8ad6faf524..d9361ee2b2d08 100644 --- a/packages/aws-cdk-lib/aws-route53/lib/hosted-zone.ts +++ b/packages/aws-cdk-lib/aws-route53/lib/hosted-zone.ts @@ -84,6 +84,9 @@ export class HostedZone extends Resource implements IHostedZone { public get hostedZoneArn(): string { return makeHostedZoneArn(this, this.hostedZoneId); } + public grantDelegation(grantee: iam.IGrantable): iam.Grant { + return makeGrantDelegation(grantee, this.hostedZoneArn); + } } return new Import(scope, id); @@ -105,6 +108,9 @@ export class HostedZone extends Resource implements IHostedZone { public get hostedZoneArn(): string { return makeHostedZoneArn(this, this.hostedZoneId); } + public grantDelegation(grantee: iam.IGrantable): iam.Grant { + return makeGrantDelegation(grantee, this.hostedZoneArn); + } } return new Import(scope, id); @@ -192,6 +198,10 @@ export class HostedZone extends Resource implements IHostedZone { public addVpc(vpc: ec2.IVpc) { this.vpcs.push({ vpcId: vpc.vpcId, vpcRegion: vpc.env.region ?? Stack.of(vpc).region }); } + + public grantDelegation(grantee: iam.IGrantable): iam.Grant { + return makeGrantDelegation(grantee, this.hostedZoneArn); + } } /** @@ -238,12 +248,7 @@ export interface PublicHostedZoneProps extends CommonHostedZoneProps { /** * Represents a Route 53 public hosted zone */ -export interface IPublicHostedZone extends IHostedZone { - /** - * Grant permissions to add delegation records to this zone - */ - grantDelegation(grantee: iam.IGrantable): iam.Grant; -} +export interface IPublicHostedZone extends IHostedZone {} /** * Create a Route53 public hosted zone. @@ -271,7 +276,7 @@ export class PublicHostedZone extends HostedZone implements IPublicHostedZone { } public grantDelegation(grantee: iam.IGrantable): iam.Grant { return makeGrantDelegation(grantee, this.hostedZoneArn); - }; + } } return new Import(scope, id); } @@ -294,7 +299,7 @@ export class PublicHostedZone extends HostedZone implements IPublicHostedZone { } public grantDelegation(grantee: iam.IGrantable): iam.Grant { return makeGrantDelegation(grantee, this.hostedZoneArn); - }; + } } return new Import(scope, id); } @@ -364,10 +369,6 @@ export class PublicHostedZone extends HostedZone implements IPublicHostedZone { ttl: opts.ttl, }); } - - public grantDelegation(grantee: iam.IGrantable): iam.Grant { - return makeGrantDelegation(grantee, this.hostedZoneArn); - } } /** @@ -434,6 +435,9 @@ export class PrivateHostedZone extends HostedZone implements IPrivateHostedZone public get hostedZoneArn(): string { return makeHostedZoneArn(this, this.hostedZoneId); } + public grantDelegation(grantee: iam.IGrantable): iam.Grant { + return makeGrantDelegation(grantee, this.hostedZoneArn); + } } return new Import(scope, id); } diff --git a/packages/aws-cdk-lib/aws-route53/test/hosted-zone.test.ts b/packages/aws-cdk-lib/aws-route53/test/hosted-zone.test.ts index a5b2722a8adf5..f08c8e1530dab 100644 --- a/packages/aws-cdk-lib/aws-route53/test/hosted-zone.test.ts +++ b/packages/aws-cdk-lib/aws-route53/test/hosted-zone.test.ts @@ -288,19 +288,18 @@ test('grantDelegation', () => { }); }); -test('grantDelegation on imported public zones', () => { +test('grantDelegation on imported zones', () => { // GIVEN const stack = new cdk.Stack(undefined, 'TestStack', { env: { account: '123456789012', region: 'us-east-1' }, }); const role = new iam.Role(stack, 'Role', { - assumedBy: new iam.AccountPrincipal('22222222222222'), + assumedBy: new iam.AccountRootPrincipal(), }); - const zone = PublicHostedZone.fromPublicHostedZoneId(stack, 'Zone', 'hosted-id'); - // WHEN + const zone = HostedZone.fromHostedZoneId(stack, 'Zone', 'hosted-id'); zone.grantDelegation(role); // THEN @@ -339,6 +338,106 @@ test('grantDelegation on imported public zones', () => { }); }); +test('grantDelegation on public imported zones', () => { + // GIVEN + const stack = new cdk.Stack(undefined, 'TestStack', { + env: { account: '123456789012', region: 'us-east-1' }, + }); + + const role = new iam.Role(stack, 'Role', { + assumedBy: new iam.AccountRootPrincipal(), + }); + + // WHEN + const publicZone = PublicHostedZone.fromPublicHostedZoneId(stack, 'PublicZone', 'public-hosted-id'); + publicZone.grantDelegation(role); + + // THEN + Template.fromStack(stack).hasResourceProperties('AWS::IAM::Policy', { + PolicyDocument: { + Statement: [ + { + Action: 'route53:ChangeResourceRecordSets', + Effect: 'Allow', + Resource: { + 'Fn::Join': [ + '', + [ + 'arn:', + { + Ref: 'AWS::Partition', + }, + ':route53:::hostedzone/public-hosted-id', + ], + ], + }, + Condition: { + 'ForAllValues:StringEquals': { + 'route53:ChangeResourceRecordSetsRecordTypes': ['NS'], + 'route53:ChangeResourceRecordSetsActions': ['UPSERT', 'DELETE'], + }, + }, + }, + { + Action: 'route53:ListHostedZonesByName', + Effect: 'Allow', + Resource: '*', + }, + ], + }, + }); +}); + +test('grantDelegation on private imported zones', () => { + // GIVEN + const stack = new cdk.Stack(undefined, 'TestStack', { + env: { account: '123456789012', region: 'us-east-1' }, + }); + + const role = new iam.Role(stack, 'Role', { + assumedBy: new iam.AccountRootPrincipal(), + }); + + // WHEN + const privateZone = PrivateHostedZone.fromPrivateHostedZoneId(stack, 'PrivateZone', 'private-hosted-id'); + privateZone.grantDelegation(role); + + // THEN + Template.fromStack(stack).hasResourceProperties('AWS::IAM::Policy', { + PolicyDocument: { + Statement: [ + { + Action: 'route53:ChangeResourceRecordSets', + Effect: 'Allow', + Resource: { + 'Fn::Join': [ + '', + [ + 'arn:', + { + Ref: 'AWS::Partition', + }, + ':route53:::hostedzone/private-hosted-id', + ], + ], + }, + Condition: { + 'ForAllValues:StringEquals': { + 'route53:ChangeResourceRecordSetsRecordTypes': ['NS'], + 'route53:ChangeResourceRecordSetsActions': ['UPSERT', 'DELETE'], + }, + }, + }, + { + Action: 'route53:ListHostedZonesByName', + Effect: 'Allow', + Resource: '*', + }, + ], + }, + }); +}); + describe('Hosted Zone with dot', () => { test('Hosted Zone constructs without trailing dot by default', () => { // GIVEN