diff --git a/packages/@aws-cdk/aws-codebuild/lib/project.ts b/packages/@aws-cdk/aws-codebuild/lib/project.ts index d46dacaa29c46..5c544a668d732 100644 --- a/packages/@aws-cdk/aws-codebuild/lib/project.ts +++ b/packages/@aws-cdk/aws-codebuild/lib/project.ts @@ -901,8 +901,7 @@ export class Project extends ProjectBase { }, })); - const policy = new iam.Policy(this, 'PolicyDocument', { - policyName: 'CodeBuildEC2Policy', + const policy = this.role.createAndAttachPolicy('CodeBuildVpcPolicy', { statements: [ new iam.PolicyStatement({ resources: ['*'], @@ -918,13 +917,13 @@ 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); + if (policy) { + // 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.defaultChild as CfnResource; + project.addDependsOn(cfnPolicy); + } } 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 index 43b22ac3a1d55..51ce356745600 100644 --- 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 index 55a6d4294744f..4193e182f9ce1 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 } from '@aws-cdk/assert'; import ec2 = require('@aws-cdk/aws-ec2'); import iam = require('@aws-cdk/aws-iam'); import { Bucket } from '@aws-cdk/aws-s3'; @@ -255,4 +255,26 @@ 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)); + + test.done(); + }, }; diff --git a/packages/@aws-cdk/aws-iam/lib/group.ts b/packages/@aws-cdk/aws-iam/lib/group.ts index 7a755b85cf26f..283a492ec94d9 100644 --- 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 { Policy, PolicyProps } from './policy'; import { PolicyStatement } from './policy-statement'; import { ArnPrincipal, IPrincipal, PrincipalPolicyFragment } from './principals'; import { IUser } from './user'; @@ -73,6 +73,12 @@ abstract class GroupBase extends Resource implements IGroup { return new ArnPrincipal(this.groupArn).policyFragment; } + public createAndAttachPolicy(id: string, props?: PolicyProps): Policy | undefined { + const policy = new Policy(this, id, props); + this.attachInlinePolicy(policy); + return policy; + } + /** * Attaches a policy to this group. * @param policy The policy to attach. diff --git a/packages/@aws-cdk/aws-iam/lib/identity-base.ts b/packages/@aws-cdk/aws-iam/lib/identity-base.ts index f4cc2e08c5d14..4530c48b9edc1 100644 --- a/packages/@aws-cdk/aws-iam/lib/identity-base.ts +++ b/packages/@aws-cdk/aws-iam/lib/identity-base.ts @@ -1,6 +1,6 @@ import { IResource } from '@aws-cdk/core'; import { IManagedPolicy } from './managed-policy'; -import { Policy } from "./policy"; +import { Policy, PolicyProps } from "./policy"; import { IPrincipal } from "./principals"; /** @@ -8,9 +8,25 @@ import { IPrincipal } from "./principals"; */ export interface IIdentity extends IPrincipal, IResource { /** - * Attaches an inline policy to this principal. - * This is the same as calling `policy.addToXxx(principal)`. + * 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 + */ + createAndAttachPolicy(id: string, props?: PolicyProps): Policy | undefined; + + /** + * 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 [disable-awslint:ref-via-interface] + * + * @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 createAndAttachPolicy} method instead */ attachInlinePolicy(policy: Policy): void; 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..f485000f1cbbd --- /dev/null +++ b/packages/@aws-cdk/aws-iam/lib/immutable-role.ts @@ -0,0 +1,43 @@ +import { Grant } from './grant'; +import { IManagedPolicy } from './managed-policy'; +import { Policy, PolicyProps } from './policy'; +import { PolicyStatement } from './policy-statement'; +import { IPrincipal } from './principals'; +import { IRole } from './role'; + +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 createAndAttachPolicy(_id: string, _props?: PolicyProps): Policy | undefined { + return undefined; + } + + public attachInlinePolicy(_policy: Policy): void { + // do nothing + } + + public addManagedPolicy(_policy: IManagedPolicy): void { + // do nothing + } + + public addToPolicy(_statement: PolicyStatement): boolean { + return false; + } + + public grant(grantee: IPrincipal, ...actions: string[]): Grant { + return this.role.grant(grantee, ...actions); + } + + public grantPassRole(grantee: IPrincipal): Grant { + return this.role.grantPassRole(grantee); + } +} diff --git a/packages/@aws-cdk/aws-iam/lib/index.ts b/packages/@aws-cdk/aws-iam/lib/index.ts index 0d0804ed8faca..377ba2685660f 100644 --- 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-role.ts b/packages/@aws-cdk/aws-iam/lib/lazy-role.ts index e3385d511cbfd..2b21302d298f8 100644 --- 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 { Policy, PolicyProps } from './policy'; import { PolicyStatement } from './policy-statement'; import { IPrincipal, PrincipalPolicyFragment } from './principals'; import { IRole, Role, RoleProps } from './role'; @@ -49,6 +49,12 @@ export class LazyRole extends cdk.Resource implements IRole { } } + public createAndAttachPolicy(id: string, props?: PolicyProps): Policy | undefined { + const policy = new Policy(this, id, props); + this.attachInlinePolicy(policy); + return policy; + } + /** * Attaches a policy to this role. * @param policy The policy to attach diff --git a/packages/@aws-cdk/aws-iam/lib/role.ts b/packages/@aws-cdk/aws-iam/lib/role.ts index 7fb0ceb982765..c38113d66a29e 100644 --- a/packages/@aws-cdk/aws-iam/lib/role.ts +++ b/packages/@aws-cdk/aws-iam/lib/role.ts @@ -3,7 +3,7 @@ import { Grant } from './grant'; import { CfnRole } from './iam.generated'; import { IIdentity } from './identity-base'; import { IManagedPolicy } from './managed-policy'; -import { Policy } from './policy'; +import { Policy, PolicyProps } from './policy'; import { PolicyDocument } from './policy-document'; import { PolicyStatement } from './policy-statement'; import { ArnPrincipal, IPrincipal, PrincipalPolicyFragment } from './principals'; @@ -168,6 +168,8 @@ export class Role extends Resource implements IRole { public abstract attachInlinePolicy(policy: Policy): void; + public abstract createAndAttachPolicy(id: string, props?: PolicyProps): Policy | undefined; + public addManagedPolicy(_policy: IManagedPolicy): void { // FIXME: Add warning that we're ignoring this } @@ -207,6 +209,12 @@ export class Role extends Resource implements IRole { return true; } + public createAndAttachPolicy(identifier: string, props?: PolicyProps): Policy | undefined { + const policy = new Policy(this, identifier, props); + this.attachInlinePolicy(policy); + return policy; + } + public attachInlinePolicy(policy: Policy): void { const policyAccount = Stack.of(policy).account; @@ -222,6 +230,10 @@ export class Role extends Resource implements IRole { return false; } + public createAndAttachPolicy(_id: string, _props?: PolicyProps): Policy | undefined { + return undefined; + } + public attachInlinePolicy(_policy: Policy): void { // do nothing } @@ -353,6 +365,12 @@ export class Role extends Resource implements IRole { this.managedPolicies.push(policy); } + public createAndAttachPolicy(id: string, props?: PolicyProps): Policy | undefined { + const policy = new Policy(this, id, props); + this.attachInlinePolicy(policy); + return policy; + } + /** * Attaches a policy to this role. * @param policy The policy to attach diff --git a/packages/@aws-cdk/aws-iam/lib/user.ts b/packages/@aws-cdk/aws-iam/lib/user.ts index 428b2817ca491..f4d5234da20e0 100644 --- a/packages/@aws-cdk/aws-iam/lib/user.ts +++ b/packages/@aws-cdk/aws-iam/lib/user.ts @@ -3,7 +3,7 @@ import { IGroup } from './group'; import { CfnUser } from './iam.generated'; import { IIdentity } from './identity-base'; import { IManagedPolicy } from './managed-policy'; -import { Policy } from './policy'; +import { Policy, PolicyProps } from './policy'; import { PolicyStatement } from './policy-statement'; import { ArnPrincipal, PrincipalPolicyFragment } from './principals'; import { IPrincipal } from './principals'; @@ -140,6 +140,10 @@ export class User extends Resource implements IIdentity, IUser { throw new Error('Cannot add imported User to Group'); } + public createAndAttachPolicy(_id: string, _props?: PolicyProps): Policy | undefined { + return undefined; + } + public attachInlinePolicy(_policy: Policy): void { throw new Error('Cannot add inline policy to imported User'); } @@ -227,6 +231,12 @@ export class User extends Resource implements IIdentity, IUser { this.managedPolicies.push(policy); } + public createAndAttachPolicy(id: string, props?: PolicyProps): Policy | undefined { + const policy = new Policy(this, id, props); + this.attachInlinePolicy(policy); + return policy; + } + /** * Attaches a policy to this user. */ 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..7aec463e7c6e4 --- /dev/null +++ b/packages/@aws-cdk/aws-iam/test/immutable-role.test.ts @@ -0,0 +1,93 @@ +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 createAndAttachPolicy', () => { + immutableRole.createAndAttachPolicy('Policy'); + + expect(stack).not.toHaveResourceLike('AWS::IAM::Policy'); + }); + + 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", + }, + ], + }, + }); + }); +});