refactor(route53): favor grant method over precreated role (#23083)

`PublicHostedZone` allows creating exactly one role for cross-account delegation.

Because of limitations of trust policy size, some teams inside Amazon are creating a role per opt-in region, in which case the "one precreated role" feature doesn't help at all.

Instead, add a `grantDelegation()` helper which works for both cases.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

Author: rix0rrr

packages/@aws-cdk/aws-iam/lib/grant.ts

@@ -24,6 +24,13 @@ export interface CommonGrantOptions {
    * The resource ARNs to grant to
    */
   readonly resourceArns: string[];
+
+  /**
+   * Any conditions to attach to the grant
+   *
+   * @default - No conditions
+   */
+  readonly conditions?: Record<string, Record<string, unknown>>;
 }
 
 /**
@@ -160,6 +167,7 @@ export class Grant implements IDependable {
     const statement = new PolicyStatement({
       actions: options.actions,
       resources: options.resourceArns,
+      conditions: options.conditions,
     });
 
     const addedToPrincipal = options.grantee.grantPrincipal.addToPrincipalPolicy(statement);
@@ -224,17 +232,27 @@ export class Grant implements IDependable {
   /**
    * The statement that was added to the principal's policy
    *
-   * Can be accessed to (e.g.) add additional conditions to the statement.
+   * @deprecated Use `principalStatements` instead
    */
   public readonly principalStatement?: PolicyStatement;
 
+  /**
+   * The statements that were added to the principal's policy
+   */
+  public readonly principalStatements = new Array<PolicyStatement>();
+
   /**
    * The statement that was added to the resource policy
    *
-   * Can be accessed to (e.g.) add additional conditions to the statement.
+   * @deprecated Use `resourceStatements` instead
    */
   public readonly resourceStatement?: PolicyStatement;
 
+  /**
+   * The statements that were added to the principal's policy
+   */
+  public readonly resourceStatements = new Array<PolicyStatement>();
+
   /**
    * The options originally used to set this result
    *
@@ -243,14 +261,26 @@ export class Grant implements IDependable {
    */
   private readonly options: CommonGrantOptions;
 
+  private readonly dependables = new Array<IDependable>();
+
   private constructor(props: GrantProps) {
     this.options = props.options;
     this.principalStatement = props.principalStatement;
     this.resourceStatement = props.resourceStatement;
+    if (this.principalStatement) {
+      this.principalStatements.push(this.principalStatement);
+    }
+    if (this.resourceStatement) {
+      this.resourceStatements.push(this.resourceStatement);
+    }
+    if (props.policyDependable) {
+      this.dependables.push(props.policyDependable);
+    }
 
+    const self = this;
     Dependable.implement(this, {
       get dependencyRoots() {
-        return props.policyDependable ? Dependable.of(props.policyDependable).dependencyRoots : [];
+        return Array.from(new Set(self.dependables.flatMap(d => Dependable.of(d).dependencyRoots)));
       },
     });
   }
@@ -282,6 +312,24 @@ export class Grant implements IDependable {
       construct.node.addDependency(this);
     }
   }
+
+  /**
+   * Combine two grants into a new one
+   */
+  public combine(rhs: Grant) {
+    const combinedPrinc = [...this.principalStatements, ...rhs.principalStatements];
+    const combinedRes = [...this.resourceStatements, ...rhs.resourceStatements];
+
+    const ret = new Grant({
+      options: this.options,
+      principalStatement: combinedPrinc[0],
+      resourceStatement: combinedRes[0],
+    });
+    ret.principalStatements.splice(0, ret.principalStatements.length, ...combinedPrinc);
+    ret.resourceStatements.splice(0, ret.resourceStatements.length, ...combinedRes);
+    ret.dependables.push(...this.dependables, ...rhs.dependables);
+    return ret;
+  }
 }
 
 function describeGrant(options: CommonGrantOptions) {

packages/@aws-cdk/aws-iam/test/grant.test.ts

@@ -1,4 +1,4 @@
-import { Template } from '@aws-cdk/assertions';
+import { Template, Match } from '@aws-cdk/assertions';
 import { CfnResource, Resource, Stack } from '@aws-cdk/core';
 import { Construct } from 'constructs';
 import * as iam from '../lib';
@@ -97,6 +97,34 @@ describe('Grant dependencies', () => {
     expectDependencyOn('RoleDefaultPolicy5FFB7DAB');
     expectDependencyOn('BuckarooPolicy74174DA4');
   });
+
+  test('can combine two grants', () => {
+    // GIVEN
+    const role1 = new iam.Role(stack, 'Role1', {
+      assumedBy: new iam.ServicePrincipal('bla.amazonaws.com'),
+    });
+    const role2 = new iam.Role(stack, 'Role2', {
+      assumedBy: new iam.ServicePrincipal('bla.amazonaws.com'),
+    });
+
+    // WHEN
+    const g1 = iam.Grant.addToPrincipal({
+      actions: ['service:DoAThing'],
+      grantee: role1,
+      resourceArns: ['*'],
+    });
+    const g2 = iam.Grant.addToPrincipal({
+      actions: ['service:DoAThing'],
+      grantee: role2,
+      resourceArns: ['*'],
+    });
+
+    (g1.combine(g2)).applyBefore(resource);
+
+    // THEN
+    expectDependencyOn('Role1DefaultPolicyD3EF4D0A');
+    expectDependencyOn('Role2DefaultPolicy3A7A0A1B');
+  });
 });
 
 function applyGrantWithDependencyTo(principal: iam.IPrincipal) {
@@ -108,8 +136,8 @@ function applyGrantWithDependencyTo(principal: iam.IPrincipal) {
 }
 
 function expectDependencyOn(id: string) {
-  Template.fromStack(stack).hasResource('CDK::Test::SomeResource', (props: any) => {
-    return (props?.DependsOn ?? []).includes(id);
+  Template.fromStack(stack).hasResource('CDK::Test::SomeResource', {
+    DependsOn: Match.arrayWith([id]),
   });
 }
 

packages/@aws-cdk/aws-route53/README.md

@@ -152,16 +152,23 @@ new route53.ARecord(this, 'ARecord', {
 
 ### Cross Account Zone Delegation
 
-To add a NS record to a HostedZone in different account you can do the following:
+If you want to have your root domain hosted zone in one account and your subdomain hosted
+zone in a diferent one, you can use `CrossAccountZoneDelegationRecord` to set up delegation
+between them.
 
 In the account containing the parent hosted zone:
 
 ```ts
 const parentZone = new route53.PublicHostedZone(this, 'HostedZone', {
   zoneName: 'someexample.com',
-  crossAccountZoneDelegationPrincipal: new iam.AccountPrincipal('12345678901'),
-  crossAccountZoneDelegationRoleName: 'MyDelegationRole',
 });
+const crossAccountRole = new iam.Role(this, 'CrossAccountRole', {
+  // The role name must be predictable
+  roleName: 'MyDelegationRole',
+  // The other account
+  assumedBy: new iam.AccountPrincipal('12345678901'),
+});
+parentZone.grantDelegation(crossAccountRole);
 ```
 
 In the account containing the child zone to be delegated:

packages/@aws-cdk/aws-route53/lib/hosted-zone.ts

@@ -213,13 +213,15 @@ export interface PublicHostedZoneProps extends CommonHostedZoneProps {
    * with appropriate permissions for every opt-in region instead.
    *
    * @default - No delegation configuration
+   * @deprecated Create the Role yourself and call `hostedZone.grantDelegation()`.
    */
   readonly crossAccountZoneDelegationPrincipal?: iam.IPrincipal;
 
   /**
    * The name of the role created for cross account delegation
    *
    * @default - A role name is generated automatically
+   * @deprecated Create the Role yourself and call `hostedZone.grantDelegation()`.
    */
   readonly crossAccountZoneDelegationRoleName?: string;
 }
@@ -342,6 +344,30 @@ export class PublicHostedZone extends HostedZone implements IPublicHostedZone {
       ttl: opts.ttl,
     });
   }
+
+  /**
+   * Grant permissions to add delegation records to this zone
+   */
+  public grantDelegation(grantee: iam.IGrantable) {
+    const g1 = iam.Grant.addToPrincipal({
+      grantee,
+      actions: ['route53:ChangeResourceRecordSets'],
+      resourceArns: [this.hostedZoneArn],
+      conditions: {
+        'ForAllValues:StringEquals': {
+          'route53:ChangeResourceRecordSetsRecordTypes': ['NS'],
+          'route53:ChangeResourceRecordSetsActions': ['UPSERT', 'DELETE'],
+        },
+      },
+    });
+    const g2 = iam.Grant.addToPrincipal({
+      grantee,
+      actions: ['route53:ListHostedZonesByName'],
+      resourceArns: ['*'],
+    });
+
+    return g1.combine(g2);
+  }
 }
 
 /**

packages/@aws-cdk/aws-route53/test/hosted-zone.test.ts

@@ -1,6 +1,7 @@
 import { Match, Template } from '@aws-cdk/assertions';
 import * as ec2 from '@aws-cdk/aws-ec2';
 import * as iam from '@aws-cdk/aws-iam';
+import { testDeprecated } from '@aws-cdk/cdk-build-tools';
 import * as cdk from '@aws-cdk/core';
 import { HostedZone, PrivateHostedZone, PublicHostedZone } from '../lib';
 
@@ -59,7 +60,7 @@ describe('hosted zone', () => {
     });
   });
 
-  test('with crossAccountZoneDelegationPrincipal', () => {
+  testDeprecated('with crossAccountZoneDelegationPrincipal', () => {
     // GIVEN
     const stack = new cdk.Stack(undefined, 'TestStack', {
       env: { account: '123456789012', region: 'us-east-1' },
@@ -154,7 +155,7 @@ describe('hosted zone', () => {
     });
   });
 
-  test('with crossAccountZoneDelegationPrincipal, throws if name provided without principal', () => {
+  testDeprecated('with crossAccountZoneDelegationPrincipal, throws if name provided without principal', () => {
     // GIVEN
     const stack = new cdk.Stack(undefined, 'TestStack', {
       env: { account: '123456789012', region: 'us-east-1' },
@@ -229,3 +230,60 @@ describe('Vpc', () => {
     });
   });
 });
+
+test('grantDelegation', () => {
+  // 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'),
+  });
+
+  const zone = new PublicHostedZone(stack, 'Zone', {
+    zoneName: 'banana.com',
+  });
+
+  // WHEN
+  zone.grantDelegation(role);
+
+  // THEN
+  const template = Template.fromStack(stack);
+  template.hasResourceProperties('AWS::IAM::Policy', {
+    PolicyDocument: {
+      Statement: [
+        {
+          Action: 'route53:ChangeResourceRecordSets',
+          Effect: 'Allow',
+          Resource: {
+            'Fn::Join': [
+              '',
+              [
+                'arn:',
+                {
+                  Ref: 'AWS::Partition',
+                },
+                ':route53:::hostedzone/',
+                {
+                  Ref: 'ZoneA5DE4B68',
+                },
+              ],
+            ],
+          },
+          Condition: {
+            'ForAllValues:StringEquals': {
+              'route53:ChangeResourceRecordSetsRecordTypes': ['NS'],
+              'route53:ChangeResourceRecordSetsActions': ['UPSERT', 'DELETE'],
+            },
+          },
+        },
+        {
+          Action: 'route53:ListHostedZonesByName',
+          Effect: 'Allow',
+          Resource: '*',
+        },
+      ],
+    },
+  });
+});

packages/@aws-cdk/aws-route53/test/record-set.test.ts

@@ -1,5 +1,6 @@
 import { Template } from '@aws-cdk/assertions';
 import * as iam from '@aws-cdk/aws-iam';
+import { testDeprecated } from '@aws-cdk/cdk-build-tools';
 import { Duration, RemovalPolicy, Stack } from '@aws-cdk/core';
 import * as route53 from '../lib';
 
@@ -578,7 +579,7 @@ describe('record set', () => {
     });
   });
 
-  test('Cross account zone delegation record with parentHostedZoneId', () => {
+  testDeprecated('Cross account zone delegation record with parentHostedZoneId', () => {
     // GIVEN
     const stack = new Stack();
     const parentZone = new route53.PublicHostedZone(stack, 'ParentHostedZone', {
@@ -630,7 +631,7 @@ describe('record set', () => {
     });
   });
 
-  test('Cross account zone delegation record with parentHostedZoneName', () => {
+  testDeprecated('Cross account zone delegation record with parentHostedZoneName', () => {
     // GIVEN
     const stack = new Stack();
     const parentZone = new route53.PublicHostedZone(stack, 'ParentHostedZone', {
@@ -675,7 +676,7 @@ describe('record set', () => {
     });
   });
 
-  test('Cross account zone delegation record throws when parent id and name both/nither are supplied', () => {
+  testDeprecated('Cross account zone delegation record throws when parent id and name both/nither are supplied', () => {
     // GIVEN
     const stack = new Stack();
     const parentZone = new route53.PublicHostedZone(stack, 'ParentHostedZone', {
@@ -707,7 +708,7 @@ describe('record set', () => {
     }).toThrow(/Only one of parentHostedZoneName and parentHostedZoneId is supported/);
   });
 
-  test('Multiple cross account zone delegation records', () => {
+  testDeprecated('Multiple cross account zone delegation records', () => {
     // GIVEN
     const stack = new Stack();
     const parentZone = new route53.PublicHostedZone(stack, 'ParentHostedZone', {
@@ -773,7 +774,7 @@ describe('record set', () => {
     }
   });
 
-  test('Cross account zone delegation policies', () => {
+  testDeprecated('Cross account zone delegation policies', () => {
     // GIVEN
     const stack = new Stack();
     const parentZone = new route53.PublicHostedZone(stack, 'ParentHostedZone', {
@@ -845,7 +846,7 @@ describe('record set', () => {
     }
   });
 
-  test('Cross account zone context flag', () => {
+  testDeprecated('Cross account zone context flag', () => {
     // GIVEN
     const stack = new Stack();
     stack.node.setContext('@aws-cdk/aws-route53:useRegionalStsEndpoint', true);