aws · skinny85 · Oct 14, 2019 · Oct 23, 2019 · Oct 28, 2019 · Oct 30, 2019
diff --git a/packages/@aws-cdk/aws-codebuild/lib/project.ts b/packages/@aws-cdk/aws-codebuild/lib/project.ts
@@ -7,7 +7,7 @@ import iam = require('@aws-cdk/aws-iam');
 import kms = require('@aws-cdk/aws-kms');
 import s3 = require('@aws-cdk/aws-s3');
 import secretsmanager = require('@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';
@@ -954,8 +954,7 @@ export class Project extends ProjectBase {
       },
     }));
 
-    const policy = new iam.Policy(this, 'PolicyDocument', {
-      policyName: 'CodeBuildEC2Policy',
+    const policy = this.role.addPolicy('CodeBuildVpcPolicy', {
       statements: [
         new iam.PolicyStatement({
           resources: ['*'],
@@ -971,13 +970,11 @@ export class Project extends ProjectBase {
         }),
       ],
     });
-    this.role.attachInlinePolicy(policy);
-
-    // add an explicit dependency between the EC2 Policy and this Project -
+    // add an explicit dependency between the VPC 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);
+    // const cfnPolicy = policy.node.defaultChild as CfnResource;
+    project.node.addDependency(policy);
   }
 
   private validateCodePipelineSettings(artifacts: IArtifacts) {

diff --git a/packages/@aws-cdk/aws-codebuild/test/integ.project-vpc.expected.json b/packages/@aws-cdk/aws-codebuild/test/integ.project-vpc.expected.json
@@ -343,7 +343,7 @@
         ]
       }
     },
-    "MyProjectPolicyDocument646EE0F2": {
+    "MyProjectRoleCodeBuildVpcPolicyE806ED54": {
       "Type": "AWS::IAM::Policy",
       "Properties": {
         "PolicyDocument": {
@@ -364,7 +364,7 @@
           ],
           "Version": "2012-10-17"
         },
-        "PolicyName": "CodeBuildEC2Policy",
+        "PolicyName": "MyProjectRoleCodeBuildVpcPolicyE806ED54",
         "Roles": [
           {
             "Ref": "MyProjectRole9BBE5233"
@@ -414,7 +414,7 @@
         }
       },
       "DependsOn": [
-        "MyProjectPolicyDocument646EE0F2"
+        "MyProjectRoleCodeBuildVpcPolicyE806ED54"
       ]
     }
   }

diff --git 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 ec2 = require('@aws-cdk/aws-ec2');
 import iam = require('@aws-cdk/aws-iam');
 import s3 = require('@aws-cdk/aws-s3');
@@ -272,4 +272,34 @@ 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();
+  },
 };
diff --git a/packages/@aws-cdk/aws-iam/lib/group.ts b/packages/@aws-cdk/aws-iam/lib/group.ts
@@ -2,7 +2,7 @@ import { Construct, Lazy, Resource, Stack } from '@aws-cdk/core';
 import { CfnGroup } from './iam.generated';
 import { IIdentity } from './identity-base';
 import { IManagedPolicy } from './managed-policy';
-import { Policy } from './policy';
+import { IPolicy, Policy, PolicyProps } from './policy';
 import { PolicyStatement } from './policy-statement';
 import { ArnPrincipal, IPrincipal, PrincipalPolicyFragment } from './principals';
 import { IUser } from './user';
@@ -73,23 +73,29 @@ abstract class GroupBase extends Resource implements IGroup {
     return new ArnPrincipal(this.groupArn).policyFragment;
   }
 
+  public addPolicy(id: string, props?: PolicyProps): IPolicy {
+    const policy = new Policy(this, id, props);
+    this.attachInlinePolicy(policy);
+    return policy;
+  }
+
   /**
    * Attaches a policy to this group.
    * @param policy The policy to attach.
    */
-  public attachInlinePolicy(policy: Policy) {
+  public attachInlinePolicy(policy: IPolicy): void {
     this.attachedPolicies.attach(policy);
     policy.attachToGroup(this);
   }
 
-  public addManagedPolicy(_policy: IManagedPolicy) {
+  public addManagedPolicy(_policy: IManagedPolicy): void {
     // drop
   }
 
   /**
    * Adds a user to this group.
    */
-  public addUser(user: IUser) {
+  public addUser(user: IUser): void {
     user.addToGroup(this);
   }
 

diff --git a/packages/@aws-cdk/aws-iam/lib/identity-base.ts b/packages/@aws-cdk/aws-iam/lib/identity-base.ts
@@ -1,18 +1,36 @@
 import { IResource } from '@aws-cdk/core';
 import { IManagedPolicy } from './managed-policy';
-import { Policy } from "./policy";
+import { IPolicy, PolicyProps } from './policy';
 import { IPrincipal } from "./principals";
 
 /**
  * A construct that represents an IAM principal, such as a user, group or role.
  */
 export interface IIdentity extends IPrincipal, IResource {
   /**
-   * Attaches an inline policy to this principal.
-   * This is the same as calling `policy.addToXxx(principal)`.
-   * @param policy The policy resource to attach to this principal [disable-awslint:ref-via-interface]
+   * Creates and attaches a Policy to this principal, if it is mutable.
+   * If this principal is immutable, does nothing, and returns undefined.
+   *
+   * @param id the logical identifier to use for the new policy resource.
+   *   It will be created in the scope of this principal (if it's mutable)
+   * @param props the construction properties of the new policy
+   * @returns the newly created Policy resource if this principal is mutable,
+   *   or undefined if this principal is immutable
    */
-  attachInlinePolicy(policy: Policy): void;
+  addPolicy(id: string, props?: PolicyProps): IPolicy;
+
+  /**
+   * Attaches an inline policy to this principal, if this principal is mutable.
+   * For a mutable principal, it is the same as calling `policy.addToXxx(principal)`.
+   * For an immutable principal, this method does nothing.
+   *
+   * @param policy The policy resource to attach to this principal
+   *
+   * @deprecated for immutable principals, this will leave the policy passed as the first parameter
+   *   possibly not attached to any principal, which is invalid in CloudFormation.
+   *   Use the {@link addPolicy} method instead
+   */
+  attachInlinePolicy(policy: IPolicy): void;
 
   /**
    * Attaches a managed policy to this principal.

diff --git a/packages/@aws-cdk/aws-iam/lib/immutable-role.ts b/packages/@aws-cdk/aws-iam/lib/immutable-role.ts
@@ -0,0 +1,86 @@
+import { ConstructNode, Stack } from "@aws-cdk/core";
+import { Grant } from './grant';
+import { IGroup } from './group';
+import { IManagedPolicy } from './managed-policy';
+import { IPolicy, PolicyProps } from './policy';
+import { PolicyStatement } from './policy-statement';
+import { IPrincipal } from './principals';
+import { IRole } from './role';
+import { IUser } from './user';
+
+/**
+ * An immutable wrapper around an IRole that 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 addPolicy(_id: string, _props?: PolicyProps): IPolicy {
+    return new ImmutablePolicy();
+  }
+
+  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);
+  }
+}
+
+class ImmutablePolicy implements IPolicy {
+  public addStatements(..._statements: PolicyStatement[]): void {
+    // do nothing
+  }
+
+  public attachToGroup(_group: IGroup): void {
+    // do nothing
+  }
+
+  public attachToRole(_role: IRole): void {
+    // do nothing
+  }
+
+  public attachToUser(_user: IUser): void {
+    // do nothing
+  }
+
+  public get node(): ConstructNode {
+    throw new Error('IConstruct.node is not implemented for ImmutablePolicy');
+  }
+
+  public get policyName(): string {
+    throw new Error('IPolicy.policyName is not implemented for ImmutablePolicy');
+  }
+
+  public get stack(): Stack {
+    throw new Error('IResource.stack is not implemented for ImmutablePolicy');
+  }
+}
diff --git a/packages/@aws-cdk/aws-iam/lib/index.ts b/packages/@aws-cdk/aws-iam/lib/index.ts
@@ -2,6 +2,7 @@ export * from './policy-document';
 export * from './policy-statement';
 export * from './managed-policy';
 export * from './role';
+export * from './immutable-role';
 export * from './policy';
 export * from './user';
 export * from './group';

diff --git a/packages/@aws-cdk/aws-iam/lib/lazy-policy.ts b/packages/@aws-cdk/aws-iam/lib/lazy-policy.ts
@@ -0,0 +1,47 @@
+import { Lazy } from '@aws-cdk/core';
+import { IPolicy, Policy } from './policy';
+
+export class LazyPolicy extends Policy implements IPolicy {
+  private _policyName!: string;
+
+  public get policyName(): string {
+    return Lazy.stringValue({ produce: () => {
+      if (this.isAttached) {
+        return super.policyName;
+      } else {
+        return this._policyName;
+      }
+    }});
+  }
+
+  public set policyName(value: string) {
+    this._policyName = value;
+  }
+
+  public get ref(): string {
+    return Lazy.stringValue({ produce: () => {
+      if (this.isAttached) {
+        return super.policyName;
+      } else {
+        throw Error('Cannot get ref of unattached/empty LazyPolicy');
+      }
+    }});
+  }
+
+  protected prepare() {
+    if (!this.isMeaningful) {
+      this.node.deleteChild('Resource');
+    }
+  }
+
+  protected validate(): string[] {
+    // Inherited validate would validate that we are attached and
+    // have statements. This version of policy does not validate that,
+    // it just won't render.
+    return [];
+  }
+
+  private get isMeaningful() {
+    return this.document.statementCount > 0 && this.isAttached;
+  }
+}
diff --git a/packages/@aws-cdk/aws-iam/lib/lazy-role.ts b/packages/@aws-cdk/aws-iam/lib/lazy-role.ts
@@ -1,7 +1,7 @@
 import cdk = require('@aws-cdk/core');
 import { Grant } from './grant';
 import { IManagedPolicy } from './managed-policy';
-import { Policy } from './policy';
+import { IPolicy, Policy, PolicyProps } from './policy';
 import { PolicyStatement } from './policy-statement';
 import { IPrincipal, PrincipalPolicyFragment } from './principals';
 import { IRole, Role, RoleProps } from './role';
@@ -28,7 +28,7 @@ export class LazyRole extends cdk.Resource implements IRole {
 
   private role?: Role;
   private readonly statements = new Array<PolicyStatement>();
-  private readonly policies = new Array<Policy>();
+  private readonly policies = new Array<IPolicy>();
   private readonly managedPolicies = new Array<IManagedPolicy>();
 
   constructor(scope: cdk.Construct, id: string, private readonly props: LazyRoleProps) {
@@ -49,11 +49,17 @@ export class LazyRole extends cdk.Resource implements IRole {
     }
   }
 
+  public addPolicy(id: string, props?: PolicyProps): IPolicy {
+    const policy = new Policy(this, id, props);
+    this.attachInlinePolicy(policy);
+    return policy;
+  }
+
   /**
    * Attaches a policy to this role.
    * @param policy The policy to attach
    */
-  public attachInlinePolicy(policy: Policy): void {
+  public attachInlinePolicy(policy: IPolicy): void {
     if (this.role) {
       this.role.attachInlinePolicy(policy);
     } else {