From c571295980d6cb8d140e32a07cfe9c802eee3b86 Mon Sep 17 00:00:00 2001
From: Rico Huijbers <rix0rrr@gmail.com>
Date: Fri, 27 Dec 2019 15:38:42 +0100
Subject: [PATCH 01/10] Make it possible for Policy not to materialize

---
 packages/@aws-cdk/aws-iam/lib/policy.ts       |  76 ++++++++++--
 packages/@aws-cdk/aws-iam/test/policy.test.ts | 111 ++++++++++++------
 packages/@aws-cdk/core/lib/construct.ts       |   9 ++
 packages/@aws-cdk/core/test/test.construct.ts |  13 ++
 4 files changed, 161 insertions(+), 48 deletions(-)

diff --git a/packages/@aws-cdk/aws-iam/lib/policy.ts b/packages/@aws-cdk/aws-iam/lib/policy.ts
index 0afee0220a5f7..9a5d9ac04d06f 100644
--- a/packages/@aws-cdk/aws-iam/lib/policy.ts
+++ b/packages/@aws-cdk/aws-iam/lib/policy.ts
@@ -56,6 +56,22 @@ export interface PolicyProps {
    * @default - No statements.
    */
   readonly statements?: PolicyStatement[];
+
+  /**
+   * Whether an `AWS::IAM::Policy` must be created
+   *
+   * Unless set to `true`, this `Policy` construct will not materialize to an
+   * `AWS::IAM::Policy` CloudFormation resource in case it would have no effect
+   * (for example, if it remains unattached to an IAM identity or if it has no
+   * statements). This is generally desired behavior, since it prevents
+   * creating invalid--and hence undeployable--CloudFormation templates.
+   *
+   * In cases where you know the policy must be created and it is actually
+   * an error if no statements have been added to it, you can se this to `true`.
+   *
+   * @default false
+   */
+  readonly mustCreate?: boolean;
 }
 
 /**
@@ -79,16 +95,12 @@ export class Policy extends Resource implements IPolicy {
    */
   public readonly document = new PolicyDocument();
 
-  /**
-   * The name of this policy.
-   *
-   * @attribute
-   */
-  public readonly policyName: string;
-
+  private readonly _policyName: string;
   private readonly roles = new Array<IRole>();
   private readonly users = new Array<IUser>();
   private readonly groups = new Array<IGroup>();
+  private readonly mustCreate: boolean;
+  private referenceTaken = false;
 
   constructor(scope: Construct, id: string, props: PolicyProps = {}) {
     super(scope, id, {
@@ -107,7 +119,8 @@ export class Policy extends Resource implements IPolicy {
       groups: undefinedIfEmpty(() => this.groups.map(g => g.groupName)),
     });
 
-    this.policyName = this.physicalName!;
+    this._policyName = this.physicalName!;
+    this.mustCreate = props.mustCreate !== undefined ? props.mustCreate : false;
 
     if (props.users) {
       props.users.forEach(u => this.attachToUser(u));
@@ -160,19 +173,60 @@ export class Policy extends Resource implements IPolicy {
     group.attachInlinePolicy(this);
   }
 
+  /**
+   * The name of this policy.
+   *
+   * @attribute
+   */
+  public get policyName(): string {
+    this.referenceTaken = true;
+    return this._policyName;
+  }
+
   protected validate(): string[] {
     const result = new Array<string>();
 
     // validate that the policy document is not empty
     if (this.document.isEmpty) {
-      result.push('Policy is empty. You must add statements to the policy');
+      if (this.mustCreate) {
+        result.push('Policy created with mustCreate=true is empty. You must add statements to the policy');
+      }
+      if (!this.mustCreate && this.referenceTaken) {
+        result.push('Policy name has been read of empty policy. You must add statements to the policy so it can exist.');
+      }
     }
 
     // validate that the policy is attached to at least one principal (role, user or group).
-    if (this.groups.length + this.users.length + this.roles.length === 0) {
-      result.push(`Policy must be attached to at least one principal: user, group or role`);
+    if (!this.isAttached) {
+      if (this.mustCreate) {
+        result.push(`Policy created with mustCreate=true must be attached to at least one principal: user, group or role`);
+      }
+      if (!this.mustCreate && this.referenceTaken) {
+        result.push('Policy name has been read of unattached policy. Attach to at least one principal: user, group or role.');
+      }
     }
 
     return result;
   }
+
+  protected prepare() {
+    // Remove the resource if it shouldn't exist. This will prevent it from being rendered to the template.
+    if (!this.shouldExist) {
+      this.node.removeChild('Resource');
+    }
+  }
+
+  /**
+   * Whether the policy resource has been attached to any identity
+   */
+  private get isAttached() {
+    return this.groups.length + this.users.length + this.roles.length > 0;
+  }
+
+  /**
+   * Whether the policy resource should be created
+   */
+  private get shouldExist() {
+    return this.mustCreate || this.referenceTaken || (!this.document.isEmpty && this.isAttached);
+  }
 }
diff --git a/packages/@aws-cdk/aws-iam/test/policy.test.ts b/packages/@aws-cdk/aws-iam/test/policy.test.ts
index 39780ff746c6f..ada8edab05212 100644
--- a/packages/@aws-cdk/aws-iam/test/policy.test.ts
+++ b/packages/@aws-cdk/aws-iam/test/policy.test.ts
@@ -1,22 +1,26 @@
+import { ResourcePart } from '@aws-cdk/assert';
 import '@aws-cdk/assert/jest';
-import { App, Stack } from '@aws-cdk/core';
+import { App, CfnResource, Stack } from '@aws-cdk/core';
 import { AnyPrincipal, CfnPolicy, Group, Policy, PolicyStatement, Role, ServicePrincipal, User } from '../lib';
 
 // tslint:disable:object-literal-key-quotes
 
 describe('IAM policy', () => {
-  test('fails when policy is empty', () => {
-    const app = new App();
-    const stack = new Stack(app, 'MyStack');
-    new Policy(stack, 'MyPolicy');
+  let app: App;
+  let stack: Stack;
 
-    expect(() => app.synth()).toThrow(/Policy is empty/);
+  beforeEach(() => {
+    app = new App();
+    stack = new Stack(app, 'MyStack');
   });
 
-  test('policy with statements', () => {
-    const app = new App();
-    const stack = new Stack(app, 'MyStack');
+  test('fails when "mustCreate" policy is empty', () => {
+    new Policy(stack, 'MyPolicy', { mustCreate: true });
+
+    expect(() => app.synth()).toThrow(/is empty/);
+  });
 
+  test('policy with statements', () => {
     const policy = new Policy(stack, 'MyPolicy', { policyName: 'MyPolicyName' });
     policy.addStatements(new PolicyStatement({ resources: ['*'], actions: ['sqs:SendMessage'] }));
     policy.addStatements(new PolicyStatement({ resources: ['arn'], actions: ['sns:Subscribe'] }));
@@ -39,9 +43,6 @@ describe('IAM policy', () => {
   });
 
   test('policy name can be omitted, in which case the logical id will be used', () => {
-    const app = new App();
-    const stack = new Stack(app, 'MyStack');
-
     const policy = new Policy(stack, 'MyPolicy');
     policy.addStatements(new PolicyStatement({ resources: ['*'], actions: ['sqs:SendMessage'] }));
     policy.addStatements(new PolicyStatement({ resources: ['arn'], actions: ['sns:Subscribe'] }));
@@ -64,10 +65,6 @@ describe('IAM policy', () => {
   });
 
   test('policy can be attached users, groups and roles and added permissions via props', () => {
-    const app = new App();
-
-    const stack = new Stack(app, 'MyStack');
-
     const user1 = new User(stack, 'User1');
     const group1 = new Group(stack, 'Group1');
     const role1 = new Role(stack, 'Role1', {
@@ -108,8 +105,6 @@ describe('IAM policy', () => {
   });
 
   test('idempotent if a principal (user/group/role) is attached twice', () => {
-    const app = new App();
-    const stack = new Stack(app, 'MyStack');
     const p = new Policy(stack, 'MyPolicy');
     p.addStatements(new PolicyStatement({ actions: ['*'], resources: ['*'] }));
 
@@ -130,10 +125,6 @@ describe('IAM policy', () => {
   });
 
   test('users, groups, roles and permissions can be added using methods', () => {
-    const app = new App();
-
-    const stack = new Stack(app, 'MyStack');
-
     const p = new Policy(stack, 'MyTestPolicy', {
       policyName: 'Foo',
     });
@@ -171,9 +162,6 @@ describe('IAM policy', () => {
   });
 
   test('policy can be attached to users, groups or role via methods on the principal', () => {
-    const app = new App();
-    const stack = new Stack(app, 'MyStack');
-
     const policy = new Policy(stack, 'MyPolicy');
     const user = new User(stack, 'MyUser');
     const group = new Group(stack, 'MyGroup');
@@ -210,9 +198,6 @@ describe('IAM policy', () => {
   });
 
   test('fails if policy name is not unique within a user/group/role', () => {
-    const app = new App();
-    const stack = new Stack(app, 'MyStack');
-
     // create two policies named Foo and attach them both to the same user/group/role
     const p1 = new Policy(stack, 'P1', { policyName: 'Foo' });
     const p2 = new Policy(stack, 'P2', { policyName: 'Foo' });
@@ -236,16 +221,12 @@ describe('IAM policy', () => {
     p3.attachToRole(role);
   });
 
-  test('fails if policy is not attached to a principal', () => {
-    const app = new App();
-    const stack = new Stack(app, 'MyStack');
-    new Policy(stack, 'MyPolicy');
-    expect(() => app.synth()).toThrow(/Policy must be attached to at least one principal: user, group or role/);
+  test('fails if "mustCreate" policy is not attached to a principal', () => {
+    new Policy(stack, 'MyPolicy', { mustCreate: true });
+    expect(() => app.synth()).toThrow(/attached to at least one principal: user, group or role/);
   });
 
   test("generated policy name is the same as the logical id if it's shorter than 128 characters", () => {
-    const stack = new Stack();
-
     createPolicyWithLogicalId(stack, 'Foo');
 
     expect(stack).toHaveResourceLike('AWS::IAM::Policy', {
@@ -254,8 +235,6 @@ describe('IAM policy', () => {
   });
 
   test('generated policy name only uses the last 128 characters of the logical id', () => {
-    const stack = new Stack();
-
     const logicalId128 = 'a' + dup(128 - 2) + 'a';
     const logicalIdOver128 = 'PREFIX' + logicalId128;
 
@@ -273,6 +252,64 @@ describe('IAM policy', () => {
       return r;
     }
   });
+
+  test('mustCreate=false, dependency on empty Policy never materializes', () => {
+    // GIVEN
+    const pol = new Policy(stack, 'Pol', { mustCreate: false });
+
+    const res = new CfnResource(stack, 'Resource', {
+      type: 'Some::Resource',
+    });
+
+    // WHEN
+    res.node.addDependency(pol);
+
+    // THEN
+    expect(stack).toMatchTemplate({
+      Resources: {
+        Resource: {
+          Type: 'Some::Resource',
+        }
+      }
+    });
+  });
+
+  test('mustCreate=false, dependency on attached and non-empty Policy can be taken', () => {
+    // GIVEN
+    const pol = new Policy(stack, 'Pol', { mustCreate: false });
+    pol.addStatements(new PolicyStatement({
+      actions: ['s3:*'],
+      resources: ['*'],
+    }));
+    pol.attachToUser(new User(stack, 'User'));
+
+    const res = new CfnResource(stack, 'Resource', {
+      type: 'Some::Resource',
+    });
+
+    // WHEN
+    res.node.addDependency(pol);
+
+    // THEN
+    expect(stack).toHaveResource("Some::Resource", {
+      Type: "Some::Resource",
+      DependsOn: [ "Pol0FE9AD5D" ]
+    }, ResourcePart.CompleteDefinition);
+  });
+
+  test('empty policy is OK if mustCreate=false', () => {
+    new Policy(stack, 'Pol', { mustCreate: false });
+
+    app.synth();
+    // If we got here, all OK
+  });
+
+  test('reading policyName forces a Policy to materialize', () => {
+    const pol = new Policy(stack, 'Pol', { mustCreate: false });
+    Array.isArray(pol.policyName);
+
+    expect(() => app.synth()).toThrow(/empty policy/);
+  });
 });
 
 function createPolicyWithLogicalId(stack: Stack, logicalId: string): void {
diff --git a/packages/@aws-cdk/core/lib/construct.ts b/packages/@aws-cdk/core/lib/construct.ts
index 84812dd9d6e8d..f70471bafa022 100644
--- a/packages/@aws-cdk/core/lib/construct.ts
+++ b/packages/@aws-cdk/core/lib/construct.ts
@@ -438,6 +438,15 @@ export class ConstructNode {
     return ret;
   }
 
+  /**
+   * Remove the child with the given name, if present.
+   *
+   * It is not an error if a child with the given name does not exist.
+   */
+  public removeChild(childName: string) {
+    delete this._children[childName];
+  }
+
   /**
    * Locks this construct from allowing more children to be added. After this
    * call, no more children can be added to this construct or to any children.
diff --git a/packages/@aws-cdk/core/test/test.construct.ts b/packages/@aws-cdk/core/test/test.construct.ts
index 194cafa207f26..ea4c837f80c78 100644
--- a/packages/@aws-cdk/core/test/test.construct.ts
+++ b/packages/@aws-cdk/core/test/test.construct.ts
@@ -121,6 +121,19 @@ export = {
     test.done();
   },
 
+  'can remove children from the tree using removeChild()'(test: Test) {
+    const root = new Root();
+    const childrenBeforeAdding = root.node.children.length; // Invariant to adding 'Metadata' resource or not
+
+    // Add & remove
+    const child = new Construct(root, 'Construct');
+    root.node.removeChild(child.node.id);
+
+    test.equals(undefined, root.node.tryFindChild(child.node.id));
+    test.equals(childrenBeforeAdding, root.node.children.length);
+    test.done();
+  },
+
   'construct.toString() and construct.toTreeString() can be used for diagnostics'(test: Test) {
     const t = createTree();
 

From e017fb6edbaa7c54cacfc1f07f9f1b3dfaf271b8 Mon Sep 17 00:00:00 2001
From: Rico Huijbers <rix0rrr@gmail.com>
Date: Fri, 27 Dec 2019 15:53:59 +0100
Subject: [PATCH 02/10] Add immutable role

---
 .../@aws-cdk/aws-iam/lib/immutable-role.ts    | 53 +++++++++++
 packages/@aws-cdk/aws-iam/lib/index.ts        |  1 +
 .../aws-iam/test/immutable-role.test.ts       | 87 +++++++++++++++++++
 3 files changed, 141 insertions(+)
 create mode 100644 packages/@aws-cdk/aws-iam/lib/immutable-role.ts
 create mode 100644 packages/@aws-cdk/aws-iam/test/immutable-role.test.ts

diff --git a/packages/@aws-cdk/aws-iam/lib/immutable-role.ts b/packages/@aws-cdk/aws-iam/lib/immutable-role.ts
new file mode 100644
index 0000000000000..f954f3a8076e4
--- /dev/null
+++ b/packages/@aws-cdk/aws-iam/lib/immutable-role.ts
@@ -0,0 +1,53 @@
+import { Grant } from './grant';
+import { IManagedPolicy } from './managed-policy';
+import { IPolicy } from './policy';
+import { PolicyStatement } from './policy-statement';
+import { IPrincipal } from './principals';
+import { IRole } from './role';
+
+/**
+ * An immutable wrapper around an IRole
+ *
+ * This wrapper ignores all mutating operations, like attaching policies or
+ * adding policy statements.
+ *
+ * Useful in cases where you want to turn off CDK's automatic permissions
+ * management, and instead have full control over all permissions.
+ *
+ * Note: if you want to ignore all mutations for an externally defined role
+ * which was imported into the CDK with {@link Role.fromRoleArn}, you don't have to use this class -
+ * simply pass the property mutable = false when calling {@link Role.fromRoleArn}.
+ */
+export class ImmutableRole implements IRole {
+  public readonly assumeRoleAction = this.role.assumeRoleAction;
+  public readonly policyFragment = this.role.policyFragment;
+  public readonly grantPrincipal = this.role.grantPrincipal;
+  public readonly roleArn = this.role.roleArn;
+  public readonly roleName = this.role.roleName;
+  public readonly node = this.role.node;
+  public readonly stack = this.role.stack;
+
+  constructor(private readonly role: IRole) {
+  }
+
+  public attachInlinePolicy(_policy: IPolicy): void {
+    // do nothing
+  }
+
+  public addManagedPolicy(_policy: IManagedPolicy): void {
+    // do nothing
+  }
+
+  public addToPolicy(_statement: PolicyStatement): boolean {
+    // Not really added, but for the purposes of consumer code pretend that it was.
+    return true;
+  }
+
+  public grant(grantee: IPrincipal, ...actions: string[]): Grant {
+    return this.role.grant(grantee, ...actions);
+  }
+
+  public grantPassRole(grantee: IPrincipal): Grant {
+    return this.role.grantPassRole(grantee);
+  }
+}
\ No newline at end of file
diff --git a/packages/@aws-cdk/aws-iam/lib/index.ts b/packages/@aws-cdk/aws-iam/lib/index.ts
index 0d0804ed8faca..6cd51fb524803 100644
--- a/packages/@aws-cdk/aws-iam/lib/index.ts
+++ b/packages/@aws-cdk/aws-iam/lib/index.ts
@@ -10,6 +10,7 @@ export * from './principals';
 export * from './identity-base';
 export * from './grant';
 export * from './unknown-principal';
+export * from './immutable-role';
 
 // AWS::IAM CloudFormation Resources:
 export * from './iam.generated';
diff --git a/packages/@aws-cdk/aws-iam/test/immutable-role.test.ts b/packages/@aws-cdk/aws-iam/test/immutable-role.test.ts
new file mode 100644
index 0000000000000..dd850434d7722
--- /dev/null
+++ b/packages/@aws-cdk/aws-iam/test/immutable-role.test.ts
@@ -0,0 +1,87 @@
+import '@aws-cdk/assert/jest';
+import { Stack } from '@aws-cdk/core';
+import iam = require('../lib');
+
+// tslint:disable:object-literal-key-quotes
+
+describe('ImmutableRole', () => {
+  let stack: Stack;
+  let mutableRole: iam.IRole;
+  let immutableRole: iam.IRole;
+
+  beforeEach(() => {
+    stack = new Stack();
+    mutableRole = new iam.Role(stack, 'MutableRole', {
+      assumedBy: new iam.AnyPrincipal(),
+    });
+    immutableRole = new iam.ImmutableRole(mutableRole);
+  });
+
+  test('ignores calls to attachInlinePolicy', () => {
+    const user = new iam.User(stack, 'User');
+    const policy = new iam.Policy(stack, 'Policy', {
+      statements: [new iam.PolicyStatement({
+        resources: ['*'],
+        actions: ['s3:*'],
+      })],
+      users: [user],
+    });
+
+    immutableRole.attachInlinePolicy(policy);
+
+    expect(stack).toHaveResource('AWS::IAM::Policy', {
+      "PolicyDocument": {
+        "Statement": [
+          {
+            "Action": "s3:*",
+            "Resource": "*",
+            "Effect": "Allow",
+          },
+        ],
+        "Version": "2012-10-17",
+      },
+      "PolicyName": "Policy23B91518",
+      "Users": [
+        {
+          "Ref": "User00B015A1",
+        },
+      ],
+    });
+  });
+
+  test('ignores calls to addManagedPolicy', () => {
+    mutableRole.addManagedPolicy({ managedPolicyArn: 'Arn1' });
+
+    immutableRole.addManagedPolicy({ managedPolicyArn: 'Arn2' });
+
+    expect(stack).toHaveResourceLike('AWS::IAM::Role', {
+      "ManagedPolicyArns": [
+        'Arn1',
+      ],
+    });
+  });
+
+  test('ignores calls to addToPolicy', () => {
+    mutableRole.addToPolicy(new iam.PolicyStatement({
+      resources: ['*'],
+      actions: ['s3:*'],
+    }));
+
+    immutableRole.addToPolicy(new iam.PolicyStatement({
+      resources: ['*'],
+      actions: ['iam:*'],
+    }));
+
+    expect(stack).toHaveResourceLike('AWS::IAM::Policy', {
+      "PolicyDocument": {
+        "Statement": [
+          {
+            "Resource": "*",
+            "Action": "s3:*",
+            "Effect": "Allow",
+          },
+        ],
+      },
+    });
+  });
+});
\ No newline at end of file

From f81c312cc5d740251ece7fb9675945c745643c1b Mon Sep 17 00:00:00 2001
From: Rico Huijbers <rix0rrr@gmail.com>
Date: Fri, 27 Dec 2019 16:00:38 +0100
Subject: [PATCH 03/10] Fix Project for use with immutable roles

---
 .../@aws-cdk/aws-codebuild/lib/project.ts     |  9 ++-
 .../aws-codebuild/test/test.project.ts        | 64 ++++++++++++++++++-
 2 files changed, 66 insertions(+), 7 deletions(-)

diff --git a/packages/@aws-cdk/aws-codebuild/lib/project.ts b/packages/@aws-cdk/aws-codebuild/lib/project.ts
index 7c694c7eed627..4bc48caa0ffdb 100644
--- a/packages/@aws-cdk/aws-codebuild/lib/project.ts
+++ b/packages/@aws-cdk/aws-codebuild/lib/project.ts
@@ -7,7 +7,7 @@ import * as iam from '@aws-cdk/aws-iam';
 import * as kms from '@aws-cdk/aws-kms';
 import * as s3 from '@aws-cdk/aws-s3';
 import * as secretsmanager from '@aws-cdk/aws-secretsmanager';
-import { Aws, CfnResource, Construct, Duration, IResource, Lazy, PhysicalName, Resource, Stack } from '@aws-cdk/core';
+import { Aws, Construct, Duration, IResource, Lazy, PhysicalName, Resource, Stack } from '@aws-cdk/core';
 import { IArtifacts } from './artifacts';
 import { BuildSpec } from './build-spec';
 import { Cache } from './cache';
@@ -973,10 +973,9 @@ export class Project extends ProjectBase {
     this.role.attachInlinePolicy(policy);
 
     // add an explicit dependency between the EC2 Policy and this Project -
-    // otherwise, creating the Project fails,
-    // as it requires these permissions to be already attached to the Project's Role
-    const cfnPolicy = policy.node.findChild('Resource') as CfnResource;
-    project.addDependsOn(cfnPolicy);
+    // otherwise, creating the Project fails, as it requires these permissions
+    // to be already attached to the Project's Role
+    project.node.addDependency(policy);
   }
 
   private validateCodePipelineSettings(artifacts: IArtifacts) {
diff --git a/packages/@aws-cdk/aws-codebuild/test/test.project.ts b/packages/@aws-cdk/aws-codebuild/test/test.project.ts
index 9e288f00601fb..5388eafe5a5ed 100644
--- a/packages/@aws-cdk/aws-codebuild/test/test.project.ts
+++ b/packages/@aws-cdk/aws-codebuild/test/test.project.ts
@@ -1,4 +1,4 @@
-import { expect, haveResource, haveResourceLike, not } from '@aws-cdk/assert';
+import { countResources, expect, haveResource, haveResourceLike, not, ResourcePart } from '@aws-cdk/assert';
 import * as ec2 from '@aws-cdk/aws-ec2';
 import * as iam from '@aws-cdk/aws-iam';
 import * as s3 from '@aws-cdk/aws-s3';
@@ -271,4 +271,64 @@ export = {
 
     test.done();
   },
-};
+
+  'can use an imported Role with mutable = false for a Project within a VPC'(test: Test) {
+    const stack = new cdk.Stack();
+
+    const importedRole = iam.Role.fromRoleArn(stack, 'Role',
+        'arn:aws:iam::1234567890:role/service-role/codebuild-bruiser-service-role', {
+      mutable: false,
+    });
+    const vpc = new ec2.Vpc(stack, 'Vpc');
+
+    new codebuild.Project(stack, 'Project', {
+      source: codebuild.Source.gitHubEnterprise({
+        httpsCloneUrl: 'https://mygithub-enterprise.com/myuser/myrepo',
+      }),
+      role: importedRole,
+      vpc,
+    });
+
+    expect(stack).to(countResources('AWS::IAM::Policy', 0));
+
+    // Check that the CodeBuild project does not have a DependsOn
+    expect(stack).to(haveResource('AWS::CodeBuild::Project', (res: any) => {
+      if (res.DependsOn && res.DependsOn.length > 0) {
+        throw new Error(`CodeBuild project should have no DependsOn, but got: ${JSON.stringify(res, undefined, 2)}`);
+      }
+      return true;
+    }, ResourcePart.CompleteDefinition));
+
+    test.done();
+  },
+
+  'can use an ImmutableRole for a Project within a VPC'(test: Test) {
+    const stack = new cdk.Stack();
+
+    const role = new iam.Role(stack, 'Role', {
+      assumedBy: new iam.ServicePrincipal('codebuild.amazonaws.com')
+    });
+
+    const vpc = new ec2.Vpc(stack, 'Vpc');
+
+    new codebuild.Project(stack, 'Project', {
+      source: codebuild.Source.gitHubEnterprise({
+        httpsCloneUrl: 'https://mygithub-enterprise.com/myuser/myrepo',
+      }),
+      role: new iam.ImmutableRole(role),
+      vpc,
+    });
+
+    expect(stack).to(countResources('AWS::IAM::Policy', 0));
+
+    // Check that the CodeBuild project does not have a DependsOn
+    expect(stack).to(haveResource('AWS::CodeBuild::Project', (res: any) => {
+      if (res.DependsOn && res.DependsOn.length > 0) {
+        throw new Error(`CodeBuild project should have no DependsOn, but got: ${JSON.stringify(res, undefined, 2)}`);
+      }
+      return true;
+    }, ResourcePart.CompleteDefinition));
+
+    test.done();
+  },
+};
\ No newline at end of file

From 7349f2ad374f8642791e9f8859e3744a29ec700f Mon Sep 17 00:00:00 2001
From: Rico Huijbers <rix0rrr@gmail.com>
Date: Fri, 27 Dec 2019 16:27:13 +0100
Subject: [PATCH 04/10] attachInlinePolicy takes a 'Policy' not an 'IPolicy'

---
 packages/@aws-cdk/aws-iam/lib/immutable-role.ts | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/packages/@aws-cdk/aws-iam/lib/immutable-role.ts b/packages/@aws-cdk/aws-iam/lib/immutable-role.ts
index f954f3a8076e4..1ed65fff9dc9a 100644
--- a/packages/@aws-cdk/aws-iam/lib/immutable-role.ts
+++ b/packages/@aws-cdk/aws-iam/lib/immutable-role.ts
@@ -1,6 +1,6 @@
 import { Grant } from './grant';
 import { IManagedPolicy } from './managed-policy';
-import { IPolicy } from './policy';
+import { Policy } from './policy';
 import { PolicyStatement } from './policy-statement';
 import { IPrincipal } from './principals';
 import { IRole } from './role';
@@ -30,7 +30,7 @@ export class ImmutableRole implements IRole {
   constructor(private readonly role: IRole) {
   }
 
-  public attachInlinePolicy(_policy: IPolicy): void {
+  public attachInlinePolicy(_policy: Policy): void {
     // do nothing
   }
 

From 6e0d1e33edb8a80e0c8fc348845d329652541c20 Mon Sep 17 00:00:00 2001
From: Rico Huijbers <rix0rrr@gmail.com>
Date: Fri, 27 Dec 2019 16:39:55 +0100
Subject: [PATCH 05/10] Don't allow require() style import

---
 packages/@aws-cdk/aws-iam/test/immutable-role.test.ts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/@aws-cdk/aws-iam/test/immutable-role.test.ts b/packages/@aws-cdk/aws-iam/test/immutable-role.test.ts
index dd850434d7722..31f37533ec54c 100644
--- a/packages/@aws-cdk/aws-iam/test/immutable-role.test.ts
+++ b/packages/@aws-cdk/aws-iam/test/immutable-role.test.ts
@@ -1,6 +1,6 @@
 import '@aws-cdk/assert/jest';
 import { Stack } from '@aws-cdk/core';
-import iam = require('../lib');
+import * as iam from '../lib';
 
 // tslint:disable:object-literal-key-quotes
 

From 10c3eb07d50410b267a30a407b756582cacfc75f Mon Sep 17 00:00:00 2001
From: Rico Huijbers <huijbers@amazon.com>
Date: Tue, 7 Jan 2020 14:43:29 +0100
Subject: [PATCH 06/10] Rewrote imported Role using ImmutableRole, add README

---
 packages/@aws-cdk/aws-iam/README.md   | 76 ++++++++++++++++++++++++++-
 packages/@aws-cdk/aws-iam/lib/role.ts | 60 ++++++++-------------
 2 files changed, 97 insertions(+), 39 deletions(-)

diff --git a/packages/@aws-cdk/aws-iam/README.md b/packages/@aws-cdk/aws-iam/README.md
index aefeaef3c028d..047a28491a0ea 100644
--- a/packages/@aws-cdk/aws-iam/README.md
+++ b/packages/@aws-cdk/aws-iam/README.md
@@ -47,9 +47,83 @@ The `grant*` methods accept an `IGrantable` object. This interface is implemente
 
 You can find which `grant*` methods exist for a resource in the [AWS CDK API Reference](https://docs.aws.amazon.com/cdk/api/latest/docs/aws-construct-library.html).
 
+### Roles
+
+Many AWS resources require *Roles* to operate. These Roles define the AWS API
+calls an instance or other AWS service is allowed to make.
+
+Creating Roles and populating them with the right permissions *Statements* is
+a necessary but tedious part of setting up AWS infrastructure. In order to
+help you focus on your business logic, CDK will take care of creating
+roles and populating them with least-privilege permissions automatically.
+
+All constructs that require Roles will create one for you if don't specify
+one at construction time. Permissions will be added to that role
+automatically if you associate the construct with other constructs from the
+AWS Construct Library (for example, if you tell an *AWS CodePipeline* to trigger
+an *AWS Lambda Function*, the Pipeline's Role will automatically get
+`lambda:InvokeFunction` permissions on that particular Lambda Function),
+or if you explicitly grant permissions using `grant` functions (see the
+previous section).
+
+#### Opting out of automatic permissions management
+
+You may prefer to manage a Role's permissions yourself instead of having the
+CDK automatically manage them for you. This may happen in one of the
+following cases:
+
+* You don't like the permissions that CDK automatically generates and
+  want to substitute your own set.
+* The least-permissions policy that the CDK generates is becoming too
+  big for IAM to store, and you need to add some wildcards to keep the
+  policy size down.
+
+To this end, you can create a `Role` yourself and wrap the object in
+an instance of the `ImmutableRole` wrapper class when you pass the
+role to a construct.
+
+For example, to have an AWS CodePipeline *not* automatically add the required
+permissions to trigger the expected targets, do the following:
+
+```ts
+const role = new iam.Role(this, 'Role', {
+  assumedBy: new iam.ServicePrincipal('codepipeline.amazonaws.com'),
+});
+
+new codepipeline.Pipeline(this, 'Pipeline', {
+  // Give the Pipeline an immutable view of the Role
+  role: new iam.ImmutableRole(role),
+});
+
+// You now have to manage the Role policies yourself
+role.addToPolicy(new iam.PolicyStatement({
+  action: [/* whatever actions you want */],
+  resource: [/* whatever resources you intend to touch */],
+});
+```
+
+#### Using existing roles
+
+If there are Roles in your account that have already been created which you
+would like to use in your CDK application, you can use `Role.fromRoleArn` to
+import them, as follows:
+
+```ts
+const role = iam.Role.fromRoleArn(this, 'Role', 'arn:aws:iam::123456789012:role/MyExistingRole', {
+  // Set 'mutable' to 'false' to use the role as-is and prevent adding new
+  // policies to it. The default is 'true', which means the role may be
+  // modified as part of the deployment.
+  mutable: false,
+});
+```
+
+In the previous example, passing `mutable: false` does the same as wrapping
+the Role in an `ImmutableRole`, but is more convenient to use in the case of
+using existing Roles.
+
 ### Configuring an ExternalId
 
-If you need to create roles that will be assumed by 3rd parties, it is generally a good idea to [require an `ExternalId`
+If you need to create Roles that will be assumed by 4rd parties, it is generally a good idea to [require an `ExternalId`
 to assume them](https://docs.aws.amazon.com/IAM/latest/UserGuide/id_roles_create_for-user_externalid.html).  Configuring
 an `ExternalId` works like this:
 
diff --git a/packages/@aws-cdk/aws-iam/lib/role.ts b/packages/@aws-cdk/aws-iam/lib/role.ts
index 88ee7151c1f6e..47716dd8de865 100644
--- a/packages/@aws-cdk/aws-iam/lib/role.ts
+++ b/packages/@aws-cdk/aws-iam/lib/role.ts
@@ -2,6 +2,7 @@ import { Construct, Duration, Lazy, Resource, Stack, Token } from '@aws-cdk/core
 import { Grant } from './grant';
 import { CfnRole } from './iam.generated';
 import { IIdentity } from './identity-base';
+import { ImmutableRole } from './immutable-role';
 import { IManagedPolicy } from './managed-policy';
 import { Policy } from './policy';
 import { PolicyDocument } from './policy-document';
@@ -157,16 +158,32 @@ export class Role extends Resource implements IRole {
     const parsedArn = scopeStack.parseArn(roleArn);
     const roleName = parsedArn.resourceName!;
 
-    abstract class Import extends Resource implements IRole {
+    class Import extends Resource implements IRole {
       public readonly grantPrincipal: IPrincipal = this;
       public readonly assumeRoleAction: string = 'sts:AssumeRole';
       public readonly policyFragment = new ArnPrincipal(roleArn).policyFragment;
       public readonly roleArn = roleArn;
       public readonly roleName = roleName;
+      private readonly attachedPolicies = new AttachedPolicies();
+      private defaultPolicy?: Policy;
 
-      public abstract addToPolicy(statement: PolicyStatement): boolean;
+      public addToPolicy(statement: PolicyStatement): boolean {
+        if (!this.defaultPolicy) {
+          this.defaultPolicy = new Policy(this, 'Policy');
+          this.attachInlinePolicy(this.defaultPolicy);
+        }
+        this.defaultPolicy.addStatements(statement);
+        return true;
+      }
 
-      public abstract attachInlinePolicy(policy: Policy): void;
+      public attachInlinePolicy(policy: Policy): void {
+        const policyAccount = Stack.of(policy).account;
+
+        if (accountsAreEqualOrOneIsUnresolved(policyAccount, roleAccount)) {
+          this.attachedPolicies.attach(policy);
+          policy.attachToRole(this);
+        }
+      }
 
       public addManagedPolicy(_policy: IManagedPolicy): void {
         // FIXME: Add warning that we're ignoring this
@@ -194,44 +211,11 @@ export class Role extends Resource implements IRole {
 
     const roleAccount = parsedArn.account;
 
-    class MutableImport extends Import {
-      private readonly attachedPolicies = new AttachedPolicies();
-      private defaultPolicy?: Policy;
-
-      public addToPolicy(statement: PolicyStatement): boolean {
-        if (!this.defaultPolicy) {
-          this.defaultPolicy = new Policy(this, 'Policy');
-          this.attachInlinePolicy(this.defaultPolicy);
-        }
-        this.defaultPolicy.addStatements(statement);
-        return true;
-      }
-
-      public attachInlinePolicy(policy: Policy): void {
-        const policyAccount = Stack.of(policy).account;
-
-        if (accountsAreEqualOrOneIsUnresolved(policyAccount, roleAccount)) {
-          this.attachedPolicies.attach(policy);
-          policy.attachToRole(this);
-        }
-      }
-    }
-
-    class ImmutableImport extends Import {
-      public addToPolicy(_statement: PolicyStatement): boolean {
-        return true;
-      }
-
-      public attachInlinePolicy(_policy: Policy): void {
-        // do nothing
-      }
-    }
-
     const scopeAccount = scopeStack.account;
 
     return options.mutable !== false && accountsAreEqualOrOneIsUnresolved(scopeAccount, roleAccount)
-      ? new MutableImport(scope, id)
-      : new ImmutableImport(scope, id);
+      ? new Import(scope, id)
+      : new ImmutableRole(new Import(scope, id));
 
     function accountsAreEqualOrOneIsUnresolved(account1: string | undefined,
                                                account2: string | undefined): boolean {

From 19734090215d2e90ede6685fc71f3bf674062b4b Mon Sep 17 00:00:00 2001
From: Rico Huijbers <huijbers@amazon.com>
Date: Tue, 7 Jan 2020 15:03:04 +0100
Subject: [PATCH 07/10] Make ImmutableRole implement DependableTrait

---
 packages/@aws-cdk/aws-iam/lib/immutable-role.ts | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/packages/@aws-cdk/aws-iam/lib/immutable-role.ts b/packages/@aws-cdk/aws-iam/lib/immutable-role.ts
index 1ed65fff9dc9a..78d9366d8b885 100644
--- a/packages/@aws-cdk/aws-iam/lib/immutable-role.ts
+++ b/packages/@aws-cdk/aws-iam/lib/immutable-role.ts
@@ -4,6 +4,7 @@ import { Policy } from './policy';
 import { PolicyStatement } from './policy-statement';
 import { IPrincipal } from './principals';
 import { IRole } from './role';
+import { DependableTrait } from '@aws-cdk/core';
 
 /**
  * An immutable wrapper around an IRole
@@ -28,6 +29,10 @@ export class ImmutableRole implements IRole {
   public readonly stack = this.role.stack;
 
   constructor(private readonly role: IRole) {
+    // implement IDependable privately
+    DependableTrait.implement(this, {
+      dependencyRoots: [ role ]
+    });
   }
 
   public attachInlinePolicy(_policy: Policy): void {

From 334cb8c3fec0e8e011e6e31b94fe9ce7d436a437 Mon Sep 17 00:00:00 2001
From: Rico Huijbers <huijbers@amazon.com>
Date: Wed, 8 Jan 2020 09:57:44 +0100
Subject: [PATCH 08/10] Fix import ordering

---
 packages/@aws-cdk/aws-iam/lib/immutable-role.ts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/@aws-cdk/aws-iam/lib/immutable-role.ts b/packages/@aws-cdk/aws-iam/lib/immutable-role.ts
index 78d9366d8b885..dc287f0cc683e 100644
--- a/packages/@aws-cdk/aws-iam/lib/immutable-role.ts
+++ b/packages/@aws-cdk/aws-iam/lib/immutable-role.ts
@@ -1,10 +1,10 @@
+import { DependableTrait } from '@aws-cdk/core';
 import { Grant } from './grant';
 import { IManagedPolicy } from './managed-policy';
 import { Policy } from './policy';
 import { PolicyStatement } from './policy-statement';
 import { IPrincipal } from './principals';
 import { IRole } from './role';
-import { DependableTrait } from '@aws-cdk/core';
 
 /**
  * An immutable wrapper around an IRole

From 717d052550f06ac252b9558470e6607a8b45cc4b Mon Sep 17 00:00:00 2001
From: Rico Huijbers <rix0rrr@gmail.com>
Date: Fri, 10 Jan 2020 17:01:58 +0100
Subject: [PATCH 09/10] Hide 'ImmutableRole' behind factory function on 'Role'

---
 packages/@aws-cdk/aws-iam/README.md               | 11 +++--------
 packages/@aws-cdk/aws-iam/lib/index.ts            |  1 -
 .../aws-iam/lib/{ => private}/immutable-role.ts   | 12 ++++++------
 packages/@aws-cdk/aws-iam/lib/role.ts             | 15 ++++++++++++++-
 .../@aws-cdk/aws-iam/test/immutable-role.test.ts  |  4 ++--
 5 files changed, 25 insertions(+), 18 deletions(-)
 rename packages/@aws-cdk/aws-iam/lib/{ => private}/immutable-role.ts (87%)

diff --git a/packages/@aws-cdk/aws-iam/README.md b/packages/@aws-cdk/aws-iam/README.md
index 047a28491a0ea..1e783fb628870 100644
--- a/packages/@aws-cdk/aws-iam/README.md
+++ b/packages/@aws-cdk/aws-iam/README.md
@@ -78,9 +78,8 @@ following cases:
   big for IAM to store, and you need to add some wildcards to keep the
   policy size down.
 
-To this end, you can create a `Role` yourself and wrap the object in
-an instance of the `ImmutableRole` wrapper class when you pass the
-role to a construct.
+To prevent constructs from updating your Role's policy, pass the object
+returned by `myRole.withoutPolicyUpdates()` instead of `myRole` itself.
 
 For example, to have an AWS CodePipeline *not* automatically add the required
 permissions to trigger the expected targets, do the following:
@@ -92,7 +91,7 @@ const role = new iam.Role(this, 'Role', {
 
 new codepipeline.Pipeline(this, 'Pipeline', {
   // Give the Pipeline an immutable view of the Role
-  role: new iam.ImmutableRole(role),
+  role: role.withoutPolicyUpdates(),
 });
 
 // You now have to manage the Role policies yourself
@@ -117,10 +116,6 @@ const role = iam.Role.fromRoleArn(this, 'Role', 'arn:aws:iam::123456789012:role/
 });
 ```
 
-In the previous example, passing `mutable: false` does the same as wrapping
-the Role in an `ImmutableRole`, but is more convenient to use in the case of
-using existing Roles.
-
 ### Configuring an ExternalId
 
 If you need to create Roles that will be assumed by 4rd parties, it is generally a good idea to [require an `ExternalId`
diff --git a/packages/@aws-cdk/aws-iam/lib/index.ts b/packages/@aws-cdk/aws-iam/lib/index.ts
index 6cd51fb524803..0d0804ed8faca 100644
--- a/packages/@aws-cdk/aws-iam/lib/index.ts
+++ b/packages/@aws-cdk/aws-iam/lib/index.ts
@@ -10,7 +10,6 @@ export * from './principals';
 export * from './identity-base';
 export * from './grant';
 export * from './unknown-principal';
-export * from './immutable-role';
 
 // AWS::IAM CloudFormation Resources:
 export * from './iam.generated';
diff --git a/packages/@aws-cdk/aws-iam/lib/immutable-role.ts b/packages/@aws-cdk/aws-iam/lib/private/immutable-role.ts
similarity index 87%
rename from packages/@aws-cdk/aws-iam/lib/immutable-role.ts
rename to packages/@aws-cdk/aws-iam/lib/private/immutable-role.ts
index dc287f0cc683e..d894cc8dbc722 100644
--- a/packages/@aws-cdk/aws-iam/lib/immutable-role.ts
+++ b/packages/@aws-cdk/aws-iam/lib/private/immutable-role.ts
@@ -1,10 +1,10 @@
 import { DependableTrait } from '@aws-cdk/core';
-import { Grant } from './grant';
-import { IManagedPolicy } from './managed-policy';
-import { Policy } from './policy';
-import { PolicyStatement } from './policy-statement';
-import { IPrincipal } from './principals';
-import { IRole } from './role';
+import { Grant } from '../grant';
+import { IManagedPolicy } from '../managed-policy';
+import { Policy } from '../policy';
+import { PolicyStatement } from '../policy-statement';
+import { IPrincipal } from '../principals';
+import { IRole } from '../role';
 
 /**
  * An immutable wrapper around an IRole
diff --git a/packages/@aws-cdk/aws-iam/lib/role.ts b/packages/@aws-cdk/aws-iam/lib/role.ts
index 8503aa835b006..7eb580c70661c 100644
--- a/packages/@aws-cdk/aws-iam/lib/role.ts
+++ b/packages/@aws-cdk/aws-iam/lib/role.ts
@@ -2,12 +2,12 @@ import { Construct, Duration, Lazy, Resource, Stack, Token } from '@aws-cdk/core
 import { Grant } from './grant';
 import { CfnRole } from './iam.generated';
 import { IIdentity } from './identity-base';
-import { ImmutableRole } from './immutable-role';
 import { IManagedPolicy } from './managed-policy';
 import { Policy } from './policy';
 import { PolicyDocument } from './policy-document';
 import { PolicyStatement } from './policy-statement';
 import { ArnPrincipal, IPrincipal, PrincipalPolicyFragment } from './principals';
+import { ImmutableRole } from './private/immutable-role';
 import { AttachedPolicies } from './util';
 
 export interface RoleProps {
@@ -369,6 +369,19 @@ export class Role extends Resource implements IRole {
   public grantPassRole(identity: IPrincipal) {
     return this.grant(identity, 'iam:PassRole');
   }
+
+  /**
+   * Return a copy of this Role object whose Policies will not be updated
+   *
+   * Use the object returned by this method if you want this Role to be used by
+   * a construct without it automatically updating the Role's Policies.
+   *
+   * If you do, you are responsible for adding the correct statements to the
+   * Role's policies yourself.
+   */
+  public withoutPolicyUpdates(): IRole {
+    return new ImmutableRole(this);
+  }
 }
 
 /**
diff --git a/packages/@aws-cdk/aws-iam/test/immutable-role.test.ts b/packages/@aws-cdk/aws-iam/test/immutable-role.test.ts
index 31f37533ec54c..7a39b761bbb7f 100644
--- a/packages/@aws-cdk/aws-iam/test/immutable-role.test.ts
+++ b/packages/@aws-cdk/aws-iam/test/immutable-role.test.ts
@@ -6,7 +6,7 @@ import * as iam from '../lib';
 
 describe('ImmutableRole', () => {
   let stack: Stack;
-  let mutableRole: iam.IRole;
+  let mutableRole: iam.Role;
   let immutableRole: iam.IRole;
 
   beforeEach(() => {
@@ -14,7 +14,7 @@ describe('ImmutableRole', () => {
     mutableRole = new iam.Role(stack, 'MutableRole', {
       assumedBy: new iam.AnyPrincipal(),
     });
-    immutableRole = new iam.ImmutableRole(mutableRole);
+    immutableRole = mutableRole.withoutPolicyUpdates();
   });
 
   test('ignores calls to attachInlinePolicy', () => {

From 8ef7ab5f8e48e68ed6de6261c1cebe25ae02f083 Mon Sep 17 00:00:00 2001
From: Rico Huijbers <rix0rrr@gmail.com>
Date: Fri, 10 Jan 2020 17:29:37 +0100
Subject: [PATCH 10/10] Also update test.project.ts

---
 packages/@aws-cdk/aws-codebuild/test/test.project.ts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/@aws-cdk/aws-codebuild/test/test.project.ts b/packages/@aws-cdk/aws-codebuild/test/test.project.ts
index 5388eafe5a5ed..17402b5ae1395 100644
--- a/packages/@aws-cdk/aws-codebuild/test/test.project.ts
+++ b/packages/@aws-cdk/aws-codebuild/test/test.project.ts
@@ -315,7 +315,7 @@ export = {
       source: codebuild.Source.gitHubEnterprise({
         httpsCloneUrl: 'https://mygithub-enterprise.com/myuser/myrepo',
       }),
-      role: new iam.ImmutableRole(role),
+      role: role.withoutPolicyUpdates(),
       vpc,
     });