From 4e43f0508f8bb7deba608248afa37029d9cfab82 Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Tue, 11 Jun 2019 15:32:41 +0200 Subject: [PATCH 01/12] refactor(iam): cleanup of IAM library Various changes. BREAKING CHANGE: * **iam**: rename `ImportedResourcePrincipal` to `UnknownPrincipal`. * **iam**: `managedPolicyArns` renamed to `managedPolicies`, takes return value from `ManagedPolicy.fromAwsManagedPolicyName()`. --- .../@aws-cdk/aws-codebuild/lib/project.ts | 4 +- .../@aws-cdk/aws-config/lib/managed-rules.ts | 4 +- packages/@aws-cdk/aws-config/lib/rule.ts | 4 +- packages/@aws-cdk/aws-eks/lib/cluster.ts | 12 ++-- packages/@aws-cdk/aws-iam/lib/group.ts | 19 +++--- .../@aws-cdk/aws-iam/lib/identity-base.ts | 5 +- packages/@aws-cdk/aws-iam/lib/index.ts | 2 +- packages/@aws-cdk/aws-iam/lib/lazy-role.ts | 13 ++-- .../@aws-cdk/aws-iam/lib/managed-policy.ts | 62 ++++++++++++------- packages/@aws-cdk/aws-iam/lib/role.ts | 21 ++++--- ...urce-principal.ts => unknown-principal.ts} | 10 +-- packages/@aws-cdk/aws-iam/lib/user.ts | 13 ++-- .../aws-iam/test/example.managedpolicy.lit.ts | 4 +- .../aws-iam/test/test.managed-policy.ts | 6 +- packages/@aws-cdk/aws-iam/test/test.role.ts | 6 +- packages/@aws-cdk/aws-lambda/lib/function.ts | 10 +-- .../notifications-resource-handler.ts | 10 +-- 17 files changed, 111 insertions(+), 94 deletions(-) rename packages/@aws-cdk/aws-iam/lib/{imported-resource-principal.ts => unknown-principal.ts} (84%) diff --git a/packages/@aws-cdk/aws-codebuild/lib/project.ts b/packages/@aws-cdk/aws-codebuild/lib/project.ts index 381b2c7c2ffff..ed4974501965e 100644 --- a/packages/@aws-cdk/aws-codebuild/lib/project.ts +++ b/packages/@aws-cdk/aws-codebuild/lib/project.ts @@ -546,7 +546,7 @@ export class Project extends ProjectBase { constructor(s: Construct, i: string) { super(s, i); - this.grantPrincipal = new iam.ImportedResourcePrincipal({ resource: this }); + this.grantPrincipal = new iam.UnknownPrincipal({ resource: this }); } } @@ -584,7 +584,7 @@ export class Project extends ProjectBase { resourceName: projectName, }); - this.grantPrincipal = new iam.ImportedResourcePrincipal({ resource: this }); + this.grantPrincipal = new iam.UnknownPrincipal({ resource: this }); this.projectName = projectName; } } diff --git a/packages/@aws-cdk/aws-config/lib/managed-rules.ts b/packages/@aws-cdk/aws-config/lib/managed-rules.ts index d6f6947128e03..71dbfd50fe14e 100644 --- a/packages/@aws-cdk/aws-config/lib/managed-rules.ts +++ b/packages/@aws-cdk/aws-config/lib/managed-rules.ts @@ -85,8 +85,8 @@ export class CloudFormationStackDriftDetectionCheck extends ManagedRule { this.role = props.role || new iam.Role(this, 'Role', { assumedBy: new iam.ServicePrincipal('config.amazonaws.com'), - managedPolicyArns: [ - new iam.AwsManagedPolicy('ReadOnlyAccess', this).policyArn, + managedPolicies: [ + iam.ManagedPolicy.fromAwsManagedPolicyName('ReadOnlyAccess') ] }); } diff --git a/packages/@aws-cdk/aws-config/lib/rule.ts b/packages/@aws-cdk/aws-config/lib/rule.ts index c1d2f39d4083c..c1910f1c23d4b 100644 --- a/packages/@aws-cdk/aws-config/lib/rule.ts +++ b/packages/@aws-cdk/aws-config/lib/rule.ts @@ -345,8 +345,8 @@ export class CustomRule extends RuleNew { }); if (props.lambdaFunction.role) { - props.lambdaFunction.role.attachManagedPolicy( - new iam.AwsManagedPolicy('service-role/AWSConfigRulesExecutionRole', this).policyArn + props.lambdaFunction.role.addManagedPolicy( + iam.ManagedPolicy.fromAwsManagedPolicyName('service-role/AWSConfigRulesExecutionRole') ); } diff --git a/packages/@aws-cdk/aws-eks/lib/cluster.ts b/packages/@aws-cdk/aws-eks/lib/cluster.ts index c63c3eab36d1d..c33c1bdae2fc8 100644 --- a/packages/@aws-cdk/aws-eks/lib/cluster.ts +++ b/packages/@aws-cdk/aws-eks/lib/cluster.ts @@ -208,9 +208,9 @@ export class Cluster extends Resource implements ICluster { this.role = props.role || new iam.Role(this, 'ClusterRole', { assumedBy: new iam.ServicePrincipal('eks.amazonaws.com'), - managedPolicyArns: [ - new iam.AwsManagedPolicy('AmazonEKSClusterPolicy', this).policyArn, - new iam.AwsManagedPolicy('AmazonEKSServicePolicy', this).policyArn, + managedPolicies: [ + iam.ManagedPolicy.fromAwsManagedPolicyName('AmazonEKSClusterPolicy'), + iam.ManagedPolicy.fromAwsManagedPolicyName('AmazonEKSServicePolicy'), ], }); @@ -308,9 +308,9 @@ export class Cluster extends Resource implements ICluster { // FIXME: Add a cfn-signal call once we've sorted out UserData and can write reliable // signaling scripts: https://github.com/awslabs/aws-cdk/issues/623 - autoScalingGroup.role.attachManagedPolicy(new iam.AwsManagedPolicy('AmazonEKSWorkerNodePolicy', this).policyArn); - autoScalingGroup.role.attachManagedPolicy(new iam.AwsManagedPolicy('AmazonEKS_CNI_Policy', this).policyArn); - autoScalingGroup.role.attachManagedPolicy(new iam.AwsManagedPolicy('AmazonEC2ContainerRegistryReadOnly', this).policyArn); + autoScalingGroup.role.addManagedPolicy(iam.ManagedPolicy.fromAwsManagedPolicyName('AmazonEKSWorkerNodePolicy')); + autoScalingGroup.role.addManagedPolicy(iam.ManagedPolicy.fromAwsManagedPolicyName('AmazonEKS_CNI_Policy')); + autoScalingGroup.role.addManagedPolicy(iam.ManagedPolicy.fromAwsManagedPolicyName('AmazonEC2ContainerRegistryReadOnly')); // EKS Required Tags autoScalingGroup.node.applyAspect(new Tag(`kubernetes.io/cluster/${this.clusterName}`, 'owned', { applyToLaunchedInstances: true })); diff --git a/packages/@aws-cdk/aws-iam/lib/group.ts b/packages/@aws-cdk/aws-iam/lib/group.ts index c5de7b2352384..aa56afa03e121 100644 --- a/packages/@aws-cdk/aws-iam/lib/group.ts +++ b/packages/@aws-cdk/aws-iam/lib/group.ts @@ -1,11 +1,12 @@ -import { Construct, Resource, Stack } from '@aws-cdk/cdk'; +import { Construct, Lazy, Resource, Stack } from '@aws-cdk/cdk'; import { CfnGroup } from './iam.generated'; import { IIdentity } from './identity-base'; +import { IManagedPolicy } from './managed-policy'; import { Policy } from './policy'; import { PolicyStatement } from './policy-document'; import { ArnPrincipal, IPrincipal, PrincipalPolicyFragment } from './principals'; import { IUser } from './user'; -import { AttachedPolicies, undefinedIfEmpty } from './util'; +import { AttachedPolicies } from './util'; export interface IGroup extends IIdentity { /** @@ -74,7 +75,7 @@ abstract class GroupBase extends Resource implements IGroup { policy.attachToGroup(this); } - public attachManagedPolicy(_arn: string) { + public addManagedPolicy(_policy: IManagedPolicy) { // drop } @@ -118,16 +119,16 @@ export class Group extends GroupBase { public readonly groupName: string; public readonly groupArn: string; - private readonly managedPolicies: string[]; + private readonly managedPolicies: IManagedPolicy[] = []; constructor(scope: Construct, id: string, props: GroupProps = {}) { super(scope, id); - this.managedPolicies = props.managedPolicyArns || []; + this.managedPolicies.push(...props.managedPolicyArns || []); const group = new CfnGroup(this, 'Resource', { groupName: props.groupName, - managedPolicyArns: undefinedIfEmpty(() => this.managedPolicies), + managedPolicyArns: Lazy.listValue({ produce: () => this.managedPolicies.map(p => p.managedPolicyArn) }, { omitEmpty: true }), path: props.path, }); @@ -137,9 +138,9 @@ export class Group extends GroupBase { /** * Attaches a managed policy to this group. - * @param arn The ARN of the managed policy to attach. + * @param policy The managed policy to attach. */ - public attachManagedPolicy(arn: string) { - this.managedPolicies.push(arn); + public addManagedPolicy(policy: IManagedPolicy) { + this.managedPolicies.push(policy); } } diff --git a/packages/@aws-cdk/aws-iam/lib/identity-base.ts b/packages/@aws-cdk/aws-iam/lib/identity-base.ts index 48dd488b86fac..1e53ed2f58d29 100644 --- a/packages/@aws-cdk/aws-iam/lib/identity-base.ts +++ b/packages/@aws-cdk/aws-iam/lib/identity-base.ts @@ -1,4 +1,5 @@ import { IResource } from '@aws-cdk/cdk'; +import { IManagedPolicy } from './managed-policy'; import { Policy } from "./policy"; import { IPrincipal } from "./principals"; @@ -15,7 +16,7 @@ export interface IIdentity extends IPrincipal, IResource { /** * Attaches a managed policy to this principal. - * @param arn The ARN of the managed policy + * @param policy The managed policy */ - attachManagedPolicy(arn: string): void; + addManagedPolicy(policy: IManagedPolicy): void; } diff --git a/packages/@aws-cdk/aws-iam/lib/index.ts b/packages/@aws-cdk/aws-iam/lib/index.ts index d8566a304f30f..07908afba018b 100644 --- a/packages/@aws-cdk/aws-iam/lib/index.ts +++ b/packages/@aws-cdk/aws-iam/lib/index.ts @@ -8,7 +8,7 @@ export * from './lazy-role'; export * from './principals'; export * from './identity-base'; export * from './grant'; -export * from './imported-resource-principal'; +export * from './unknown-principal'; // AWS::IAM CloudFormation Resources: export * from './iam.generated'; diff --git a/packages/@aws-cdk/aws-iam/lib/lazy-role.ts b/packages/@aws-cdk/aws-iam/lib/lazy-role.ts index 43d21130b311c..5c27ad153edfd 100644 --- a/packages/@aws-cdk/aws-iam/lib/lazy-role.ts +++ b/packages/@aws-cdk/aws-iam/lib/lazy-role.ts @@ -1,5 +1,6 @@ import cdk = require('@aws-cdk/cdk'); import { Grant } from './grant'; +import { IManagedPolicy } from './managed-policy'; import { Policy } from './policy'; import { PolicyStatement } from './policy-document'; import { IPrincipal, PrincipalPolicyFragment } from './principals'; @@ -27,7 +28,7 @@ export class LazyRole extends cdk.Resource implements IRole { private role?: Role; private readonly statements = new Array(); private readonly policies = new Array(); - private readonly managedPolicies = new Array(); + private readonly managedPolicies = new Array(); constructor(scope: cdk.Construct, id: string, private readonly props: LazyRoleProps) { super(scope, id); @@ -61,13 +62,13 @@ export class LazyRole extends cdk.Resource implements IRole { /** * Attaches a managed policy to this role. - * @param arn The ARN of the managed policy to attach. + * @param policy The managed policy to attach. */ - public attachManagedPolicy(arn: string): void { + public addManagedPolicy(policy: IManagedPolicy): void { if (this.role) { - this.role.attachManagedPolicy(arn); + this.role.addManagedPolicy(policy); } else { - this.managedPolicies.push(arn); + this.managedPolicies.push(policy); } } @@ -110,7 +111,7 @@ export class LazyRole extends cdk.Resource implements IRole { const role = new Role(this, 'Default', this.props); this.statements.forEach(role.addToPolicy.bind(role)); this.policies.forEach(role.attachInlinePolicy.bind(role)); - this.managedPolicies.forEach(role.attachManagedPolicy.bind(role)); + this.managedPolicies.forEach(role.addManagedPolicy.bind(role)); this.role = role; } return this.role; diff --git a/packages/@aws-cdk/aws-iam/lib/managed-policy.ts b/packages/@aws-cdk/aws-iam/lib/managed-policy.ts index 726e51624c1c1..7ff8771d5b987 100644 --- a/packages/@aws-cdk/aws-iam/lib/managed-policy.ts +++ b/packages/@aws-cdk/aws-iam/lib/managed-policy.ts @@ -1,30 +1,48 @@ -import cdk = require('@aws-cdk/cdk'); -import { Stack } from '@aws-cdk/cdk'; +import { IResolveContext, Lazy, Stack } from '@aws-cdk/cdk'; /** - * A policy managed by AWS - * - * For this managed policy, you only need to know the name to be able to use it. - * - * Some managed policy names start with "service-role/", some start with - * "job-function/", and some don't start with anything. Do include the - * prefix when constructing this object. + * A managed policy */ -export class AwsManagedPolicy { - constructor(private readonly managedPolicyName: string, private readonly scope: cdk.IConstruct) { - } +export interface IManagedPolicy { + /** + * The ARN of the managed policy + */ + readonly managedPolicyArn: string; +} +/** + * Managed policy + * + * This class is an incomplete placeholder class, and exists only to get access + * to AWS Managed policies. + */ +export class ManagedPolicy { /** - * The Arn of this managed policy + * Construct a managed policy from one of the policies that AWS manages + * + * For this managed policy, you only need to know the name to be able to use it. + * + * Some managed policy names start with "service-role/", some start with + * "job-function/", and some don't start with anything. Do include the + * prefix when constructing this object. */ - public get policyArn(): string { - // the arn is in the form of - arn:aws:iam::aws:policy/ - return Stack.of(this.scope).formatArn({ - service: "iam", - region: "", // no region for managed policy - account: "aws", // the account for a managed policy is 'aws' - resource: "policy", - resourceName: this.managedPolicyName - }); + public static fromAwsManagedPolicyName(managedPolicyName: string): IManagedPolicy { + class AwsManagedPolicy implements IManagedPolicy { + public readonly managedPolicyArn = Lazy.stringValue({ + produce(ctx: IResolveContext) { + return Stack.of(ctx.scope).formatArn({ + service: "iam", + region: "", // no region for managed policy + account: "aws", // the account for a managed policy is 'aws' + resource: "policy", + resourceName: managedPolicyName + }); + } + }); + } + return new AwsManagedPolicy(); + } + + protected constructor() { } } diff --git a/packages/@aws-cdk/aws-iam/lib/role.ts b/packages/@aws-cdk/aws-iam/lib/role.ts index 3f59ae06fcc77..de92aee7d1f14 100644 --- a/packages/@aws-cdk/aws-iam/lib/role.ts +++ b/packages/@aws-cdk/aws-iam/lib/role.ts @@ -1,11 +1,12 @@ -import { Construct, PhysicalName, Resource, ResourceIdentifiers, Stack } from '@aws-cdk/cdk'; +import { Construct, Lazy, PhysicalName, Resource, ResourceIdentifiers, Stack } from '@aws-cdk/cdk'; import { Grant } from './grant'; import { CfnRole } from './iam.generated'; import { IIdentity } from './identity-base'; +import { IManagedPolicy } from './managed-policy'; import { Policy } from './policy'; import { PolicyDocument, PolicyStatement } from './policy-document'; import { ArnPrincipal, IPrincipal, PrincipalPolicyFragment } from './principals'; -import { AttachedPolicies, undefinedIfEmpty } from './util'; +import { AttachedPolicies } from './util'; export interface RoleProps { /** @@ -33,7 +34,7 @@ export interface RoleProps { * * @default - No managed policies. */ - readonly managedPolicyArns?: string[]; + readonly managedPolicies?: IManagedPolicy[]; /** * A list of named policies to inline into this role. These policies will be @@ -125,7 +126,7 @@ export class Role extends Resource implements IRole { // FIXME: Add warning that we're ignoring this } - public attachManagedPolicy(_arn: string): void { + public addManagedPolicy(_policy: IManagedPolicy): void { // FIXME: Add warning that we're ignoring this } @@ -186,7 +187,7 @@ export class Role extends Resource implements IRole { public readonly policyFragment: PrincipalPolicyFragment; private defaultPolicy?: Policy; - private readonly managedPolicyArns: string[]; + private readonly managedPolicies: IManagedPolicy[] = []; private readonly attachedPolicies = new AttachedPolicies(); constructor(scope: Construct, id: string, props: RoleProps) { @@ -195,13 +196,13 @@ export class Role extends Resource implements IRole { }); this.assumeRolePolicy = createAssumeRolePolicy(props.assumedBy, props.externalId); - this.managedPolicyArns = props.managedPolicyArns || [ ]; + this.managedPolicies.push(...props.managedPolicies || []); validateMaxSessionDuration(props.maxSessionDurationSec); const role = new CfnRole(this, 'Resource', { assumeRolePolicyDocument: this.assumeRolePolicy as any, - managedPolicyArns: undefinedIfEmpty(() => this.managedPolicyArns), + managedPolicyArns: Lazy.listValue({ produce: () => this.managedPolicies.map(p => p.managedPolicyArn) }, { omitEmpty: true }), policies: _flatten(props.inlinePolicies), path: props.path, roleName: this.physicalName.value, @@ -252,10 +253,10 @@ export class Role extends Resource implements IRole { /** * Attaches a managed policy to this role. - * @param arn The ARN of the managed policy to attach. + * @param policy The the managed policy to attach. */ - public attachManagedPolicy(arn: string) { - this.managedPolicyArns.push(arn); + public addManagedPolicy(policy: IManagedPolicy) { + this.managedPolicies.push(policy); } /** diff --git a/packages/@aws-cdk/aws-iam/lib/imported-resource-principal.ts b/packages/@aws-cdk/aws-iam/lib/unknown-principal.ts similarity index 84% rename from packages/@aws-cdk/aws-iam/lib/imported-resource-principal.ts rename to packages/@aws-cdk/aws-iam/lib/unknown-principal.ts index 87530e9d3006b..6778dcfa66578 100644 --- a/packages/@aws-cdk/aws-iam/lib/imported-resource-principal.ts +++ b/packages/@aws-cdk/aws-iam/lib/unknown-principal.ts @@ -3,9 +3,9 @@ import { PolicyStatement } from './policy-document'; import { IPrincipal, PrincipalPolicyFragment } from './principals'; /** - * Properties for an ImportedResourcePrincipal + * Properties for an UnknownPrincipal */ -export interface ImportedResourcePrincipalProps { +export interface UnknownPrincipalProps { /** * The resource the role proxy is for */ @@ -13,7 +13,7 @@ export interface ImportedResourcePrincipalProps { } /** - * A principal associated with an imported resource + * A principal for use in resources that need to have a role but it's unknown * * Some resources have roles associated with them which they assume, such as * Lambda Functions, CodeBuild projects, StepFunctions machines, etc. @@ -23,12 +23,12 @@ export interface ImportedResourcePrincipalProps { * instead, which will add user warnings when statements are attempted to be * added to it. */ -export class ImportedResourcePrincipal implements IPrincipal { +export class UnknownPrincipal implements IPrincipal { public readonly assumeRoleAction: string = 'sts:AssumeRole'; public readonly grantPrincipal: IPrincipal; private readonly resource: IConstruct; - constructor(props: ImportedResourcePrincipalProps) { + constructor(props: UnknownPrincipalProps) { this.resource = props.resource; this.grantPrincipal = this; } diff --git a/packages/@aws-cdk/aws-iam/lib/user.ts b/packages/@aws-cdk/aws-iam/lib/user.ts index 759e99854ffbf..17dadf34cbf4b 100644 --- a/packages/@aws-cdk/aws-iam/lib/user.ts +++ b/packages/@aws-cdk/aws-iam/lib/user.ts @@ -1,7 +1,8 @@ -import { Construct, Resource, SecretValue } from '@aws-cdk/cdk'; +import { Construct, Lazy, Resource, SecretValue } from '@aws-cdk/cdk'; import { IGroup } from './group'; import { CfnUser } from './iam.generated'; import { IIdentity } from './identity-base'; +import { IManagedPolicy } from './managed-policy'; import { Policy } from './policy'; import { PolicyStatement } from './policy-document'; import { ArnPrincipal, PrincipalPolicyFragment } from './principals'; @@ -97,7 +98,7 @@ export class User extends Resource implements IIdentity { public readonly policyFragment: PrincipalPolicyFragment; private readonly groups = new Array(); - private readonly managedPolicyArns = new Array(); + private readonly managedPolicies = new Array(); private readonly attachedPolicies = new AttachedPolicies(); private defaultPolicy?: Policy; @@ -107,7 +108,7 @@ export class User extends Resource implements IIdentity { const user = new CfnUser(this, 'Resource', { userName: props.userName, groups: undefinedIfEmpty(() => this.groups), - managedPolicyArns: undefinedIfEmpty(() => this.managedPolicyArns), + managedPolicyArns: Lazy.listValue({ produce: () => this.managedPolicies.map(p => p.managedPolicyArn) }, { omitEmpty: true }), path: props.path, loginProfile: this.parseLoginProfile(props) }); @@ -130,10 +131,10 @@ export class User extends Resource implements IIdentity { /** * Attaches a managed policy to the user. - * @param arn The ARN of the managed policy to attach. + * @param policy The managed policy to attach. */ - public attachManagedPolicy(arn: string) { - this.managedPolicyArns.push(arn); + public addManagedPolicy(policy: IManagedPolicy) { + this.managedPolicies.push(policy); } /** diff --git a/packages/@aws-cdk/aws-iam/test/example.managedpolicy.lit.ts b/packages/@aws-cdk/aws-iam/test/example.managedpolicy.lit.ts index f05c5a2446bf2..67483dfea6e6c 100644 --- a/packages/@aws-cdk/aws-iam/test/example.managedpolicy.lit.ts +++ b/packages/@aws-cdk/aws-iam/test/example.managedpolicy.lit.ts @@ -1,5 +1,5 @@ import cdk = require('@aws-cdk/cdk'); -import { Group } from '../lib'; +import { Group, ManagedPolicy } from '../lib'; export class ExampleConstruct extends cdk.Construct { constructor(scope: cdk.Construct, id: string) { @@ -7,7 +7,7 @@ export class ExampleConstruct extends cdk.Construct { /// !show const group = new Group(this, 'MyGroup'); - group.attachManagedPolicy('arn:aws:iam::aws:policy/AdministratorAccess'); + group.addManagedPolicy(ManagedPolicy.fromAwsManagedPolicyName('policy/AdministratorAccess')); /// !hide } } diff --git a/packages/@aws-cdk/aws-iam/test/test.managed-policy.ts b/packages/@aws-cdk/aws-iam/test/test.managed-policy.ts index ffb6f503ddc19..00d2fa67646a3 100644 --- a/packages/@aws-cdk/aws-iam/test/test.managed-policy.ts +++ b/packages/@aws-cdk/aws-iam/test/test.managed-policy.ts @@ -1,13 +1,13 @@ import cdk = require('@aws-cdk/cdk'); import { Test } from 'nodeunit'; -import { AwsManagedPolicy } from '../lib'; +import { ManagedPolicy } from '../lib'; export = { 'simple managed policy'(test: Test) { const stack = new cdk.Stack(); - const mp = new AwsManagedPolicy("service-role/SomePolicy", stack); + const mp = ManagedPolicy.fromAwsManagedPolicyName("service-role/SomePolicy"); - test.deepEqual(stack.resolve(mp.policyArn), { + test.deepEqual(stack.resolve(mp.managedPolicyArn), { "Fn::Join": ['', [ 'arn:', { Ref: 'AWS::Partition' }, diff --git a/packages/@aws-cdk/aws-iam/test/test.role.ts b/packages/@aws-cdk/aws-iam/test/test.role.ts index c3783e814ba4d..154e1d79492ab 100644 --- a/packages/@aws-cdk/aws-iam/test/test.role.ts +++ b/packages/@aws-cdk/aws-iam/test/test.role.ts @@ -116,10 +116,10 @@ export = { const role = new Role(stack, 'MyRole', { assumedBy: new ServicePrincipal('test.service'), - managedPolicyArns: [ 'managed1', 'managed2' ] + managedPolicies: [ { managedPolicyArn: 'managed1' }, { managedPolicyArn: 'managed2' } ] }); - role.attachManagedPolicy('managed3'); + role.addManagedPolicy({ managedPolicyArn: 'managed3' }); expect(stack).toMatch({ Resources: { MyRoleF48FFE04: { Type: 'AWS::IAM::Role', @@ -261,4 +261,4 @@ export = { test.deepEqual(importedRole.roleName, 'S3Access'); test.done(); } -}; +}; \ No newline at end of file diff --git a/packages/@aws-cdk/aws-lambda/lib/function.ts b/packages/@aws-cdk/aws-lambda/lib/function.ts index 5151cf57eb958..5329ad923ba0d 100644 --- a/packages/@aws-cdk/aws-lambda/lib/function.ts +++ b/packages/@aws-cdk/aws-lambda/lib/function.ts @@ -265,7 +265,7 @@ export class Function extends FunctionBase { constructor(s: Construct, i: string) { super(s, i); - this.grantPrincipal = role || new iam.ImportedResourcePrincipal({ resource: this } ); + this.grantPrincipal = role || new iam.UnknownPrincipal({ resource: this } ); if (attrs.securityGroupId) { this._connections = new ec2.Connections({ @@ -394,19 +394,19 @@ export class Function extends FunctionBase { this.environment = props.environment || { }; - const managedPolicyArns = new Array(); + const managedPolicies = new Array(); // the arn is in the form of - arn:aws:iam::aws:policy/service-role/AWSLambdaBasicExecutionRole - managedPolicyArns.push(new iam.AwsManagedPolicy("service-role/AWSLambdaBasicExecutionRole", this).policyArn); + managedPolicies.push(iam.ManagedPolicy.fromAwsManagedPolicyName("service-role/AWSLambdaBasicExecutionRole")); if (props.vpc) { // Policy that will have ENI creation permissions - managedPolicyArns.push(new iam.AwsManagedPolicy("service-role/AWSLambdaVPCAccessExecutionRole", this).policyArn); + managedPolicies.push(iam.ManagedPolicy.fromAwsManagedPolicyName("service-role/AWSLambdaVPCAccessExecutionRole")); } this.role = props.role || new iam.Role(this, 'ServiceRole', { assumedBy: new iam.ServicePrincipal('lambda.amazonaws.com'), - managedPolicyArns, + managedPolicies }); this.grantPrincipal = this.role; diff --git a/packages/@aws-cdk/aws-s3/lib/notifications-resource/notifications-resource-handler.ts b/packages/@aws-cdk/aws-s3/lib/notifications-resource/notifications-resource-handler.ts index 45cb2b839a661..76e570d53c4d5 100644 --- a/packages/@aws-cdk/aws-s3/lib/notifications-resource/notifications-resource-handler.ts +++ b/packages/@aws-cdk/aws-s3/lib/notifications-resource/notifications-resource-handler.ts @@ -50,14 +50,8 @@ export class NotificationsResourceHandler extends cdk.Construct { const role = new iam.Role(this, 'Role', { assumedBy: new iam.ServicePrincipal('lambda.amazonaws.com'), - managedPolicyArns: [ - Stack.of(this).formatArn({ - service: 'iam', - region: '', // no region for managed policy - account: 'aws', // the account for a managed policy is 'aws' - resource: 'policy', - resourceName: 'service-role/AWSLambdaBasicExecutionRole', - }) + managedPolicies: [ + iam.ManagedPolicy.fromAwsManagedPolicyName('service-role/AWSLambdaBasicExecutionRole') ] }); From 9e21e6eef20399aa84ccc19a450f5cbac63e3488 Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Tue, 11 Jun 2019 16:23:37 +0200 Subject: [PATCH 02/12] Move PolicyStatement to its own file. Remove `postProcess` from Token interface, move opting into it into `IResolvable.resolve()`, so it can be hidden from the `PolicyDocument` public interface. Make addStatements() variadic. --- .../assets-docker/lib/adopted-repository.ts | 2 +- packages/@aws-cdk/aws-ec2/lib/vpc-endpoint.ts | 2 +- packages/@aws-cdk/aws-ecr/lib/repository.ts | 2 +- packages/@aws-cdk/aws-iam/lib/grant.ts | 2 +- packages/@aws-cdk/aws-iam/lib/group.ts | 4 +- packages/@aws-cdk/aws-iam/lib/index.ts | 1 + packages/@aws-cdk/aws-iam/lib/lazy-role.ts | 2 +- .../@aws-cdk/aws-iam/lib/policy-document.ts | 364 +++--------------- .../@aws-cdk/aws-iam/lib/policy-statement.ts | 262 +++++++++++++ packages/@aws-cdk/aws-iam/lib/policy.ts | 9 +- packages/@aws-cdk/aws-iam/lib/principals.ts | 2 +- packages/@aws-cdk/aws-iam/lib/role.ts | 9 +- .../@aws-cdk/aws-iam/lib/unknown-principal.ts | 2 +- packages/@aws-cdk/aws-iam/lib/user.ts | 4 +- .../@aws-cdk/aws-iam/test/integ.policy.ts | 4 +- packages/@aws-cdk/aws-iam/test/integ.role.ts | 2 +- .../aws-iam/test/integ.users-and-groups.ts | 2 +- .../aws-iam/test/test.policy-document.ts | 99 ++--- packages/@aws-cdk/aws-iam/test/test.policy.ts | 14 +- packages/@aws-cdk/aws-kms/lib/key.ts | 2 +- .../aws-logs/lib/cross-account-destination.ts | 2 +- packages/@aws-cdk/aws-s3/lib/bucket.ts | 2 +- packages/@aws-cdk/aws-sns/lib/policy.ts | 12 +- packages/@aws-cdk/aws-sns/lib/topic-base.ts | 2 +- packages/@aws-cdk/aws-sqs/lib/queue-base.ts | 2 +- .../@aws-cdk/cdk/lib/cloudformation-lang.ts | 6 +- packages/@aws-cdk/cdk/lib/private/resolve.ts | 33 +- packages/@aws-cdk/cdk/lib/resolvable.ts | 24 +- packages/@aws-cdk/cdk/lib/util.ts | 9 +- 29 files changed, 428 insertions(+), 454 deletions(-) create mode 100644 packages/@aws-cdk/aws-iam/lib/policy-statement.ts diff --git a/packages/@aws-cdk/assets-docker/lib/adopted-repository.ts b/packages/@aws-cdk/assets-docker/lib/adopted-repository.ts index 397afb6828b0e..d51e1ef95e723 100644 --- a/packages/@aws-cdk/assets-docker/lib/adopted-repository.ts +++ b/packages/@aws-cdk/assets-docker/lib/adopted-repository.ts @@ -88,6 +88,6 @@ export class AdoptedRepository extends ecr.RepositoryBase { * use the custom resource to modify the ECR resource policy if needed. */ public addToResourcePolicy(statement: iam.PolicyStatement) { - this.policyDocument.addStatement(statement); + this.policyDocument.addStatements(statement); } } diff --git a/packages/@aws-cdk/aws-ec2/lib/vpc-endpoint.ts b/packages/@aws-cdk/aws-ec2/lib/vpc-endpoint.ts index 96bb3aab7fb3f..227ab0aac1596 100644 --- a/packages/@aws-cdk/aws-ec2/lib/vpc-endpoint.ts +++ b/packages/@aws-cdk/aws-ec2/lib/vpc-endpoint.ts @@ -40,7 +40,7 @@ export abstract class VpcEndpoint extends Resource implements IVpcEndpoint { this.policyDocument = new iam.PolicyDocument(); } - this.policyDocument.addStatement(statement); + this.policyDocument.addStatements(statement); } } diff --git a/packages/@aws-cdk/aws-ecr/lib/repository.ts b/packages/@aws-cdk/aws-ecr/lib/repository.ts index 9fa3186fedf3a..d8227d75edced 100644 --- a/packages/@aws-cdk/aws-ecr/lib/repository.ts +++ b/packages/@aws-cdk/aws-ecr/lib/repository.ts @@ -364,7 +364,7 @@ export class Repository extends RepositoryBase { if (this.policyDocument === undefined) { this.policyDocument = new iam.PolicyDocument(); } - this.policyDocument.addStatement(statement); + this.policyDocument.addStatements(statement); } /** diff --git a/packages/@aws-cdk/aws-iam/lib/grant.ts b/packages/@aws-cdk/aws-iam/lib/grant.ts index c025e4e45a1b4..4208ee8df9297 100644 --- a/packages/@aws-cdk/aws-iam/lib/grant.ts +++ b/packages/@aws-cdk/aws-iam/lib/grant.ts @@ -1,5 +1,5 @@ import cdk = require('@aws-cdk/cdk'); -import { PolicyStatement } from "./policy-document"; +import { PolicyStatement } from "./policy-statement"; import { IGrantable } from "./principals"; /** diff --git a/packages/@aws-cdk/aws-iam/lib/group.ts b/packages/@aws-cdk/aws-iam/lib/group.ts index aa56afa03e121..0943349591f9f 100644 --- a/packages/@aws-cdk/aws-iam/lib/group.ts +++ b/packages/@aws-cdk/aws-iam/lib/group.ts @@ -3,7 +3,7 @@ import { CfnGroup } from './iam.generated'; import { IIdentity } from './identity-base'; import { IManagedPolicy } from './managed-policy'; import { Policy } from './policy'; -import { PolicyStatement } from './policy-document'; +import { PolicyStatement } from './policy-statement'; import { ArnPrincipal, IPrincipal, PrincipalPolicyFragment } from './principals'; import { IUser } from './user'; import { AttachedPolicies } from './util'; @@ -95,7 +95,7 @@ abstract class GroupBase extends Resource implements IGroup { this.defaultPolicy.attachToGroup(this); } - this.defaultPolicy.addStatement(statement); + this.defaultPolicy.addStatements(statement); return true; } } diff --git a/packages/@aws-cdk/aws-iam/lib/index.ts b/packages/@aws-cdk/aws-iam/lib/index.ts index 07908afba018b..0d0804ed8faca 100644 --- a/packages/@aws-cdk/aws-iam/lib/index.ts +++ b/packages/@aws-cdk/aws-iam/lib/index.ts @@ -1,4 +1,5 @@ export * from './policy-document'; +export * from './policy-statement'; export * from './managed-policy'; export * from './role'; export * from './policy'; diff --git a/packages/@aws-cdk/aws-iam/lib/lazy-role.ts b/packages/@aws-cdk/aws-iam/lib/lazy-role.ts index 5c27ad153edfd..b226b033bfbbf 100644 --- a/packages/@aws-cdk/aws-iam/lib/lazy-role.ts +++ b/packages/@aws-cdk/aws-iam/lib/lazy-role.ts @@ -2,7 +2,7 @@ import cdk = require('@aws-cdk/cdk'); import { Grant } from './grant'; import { IManagedPolicy } from './managed-policy'; import { Policy } from './policy'; -import { PolicyStatement } from './policy-document'; +import { PolicyStatement } from './policy-statement'; import { IPrincipal, PrincipalPolicyFragment } from './principals'; import { IRole, Role, RoleProps } from './role'; diff --git a/packages/@aws-cdk/aws-iam/lib/policy-document.ts b/packages/@aws-cdk/aws-iam/lib/policy-document.ts index aaabe7f18f987..94e9a81167941 100644 --- a/packages/@aws-cdk/aws-iam/lib/policy-document.ts +++ b/packages/@aws-cdk/aws-iam/lib/policy-document.ts @@ -1,66 +1,36 @@ import cdk = require('@aws-cdk/cdk'); -import { AccountPrincipal, AccountRootPrincipal, Anyone, ArnPrincipal, CanonicalUserPrincipal, - FederatedPrincipal, IPrincipal, ServicePrincipal, ServicePrincipalOpts } from './principals'; -import { mergePrincipal } from './util'; - -export class PolicyDocument implements cdk.IResolvableWithPostProcess { - private statements = new Array(); - private _autoAssignSids = false; +import { IPostProcessor } from '@aws-cdk/cdk'; +import { PolicyStatement } from './policy-statement'; +/** + * Properties for a new PolicyDocument + */ +export interface PolicyDocumentProps { /** - * Creates a new IAM policy document. - * @param defaultDocument An IAM policy document to use as an initial - * policy. All statements of this document will be copied in. + * Automatically assign Statement Ids to all statements + * + * @default false */ - constructor(private readonly baseDocument: any = {}) { - } + readonly assignSids?: boolean; +} - /** - * Will automatically assign a unique SID to each statement, unless an SID is provided. - */ - public autoAssignSids() { - this._autoAssignSids = true; - } +/** + * A PolicyDocument is a collection of statements + */ +export class PolicyDocument implements cdk.IResolvable { + private readonly statements = new Array(); + private readonly autoAssignSids: boolean; - public resolve(_context: cdk.IResolveContext): any { - return this.render(); + constructor(props: PolicyDocumentProps = {}) { + this.autoAssignSids = !!props.assignSids; } - /** - * Removes duplicate statements - */ - public postProcess(input: any, _context: cdk.IResolveContext): any { - if (!input || !input.Statement) { - return input; - } - - const jsonStatements = new Set(); - const uniqueStatements: any[] = []; - - for (const statement of input.Statement) { - const jsonStatement = JSON.stringify(statement); - if (!jsonStatements.has(jsonStatement)) { - uniqueStatements.push(statement); - jsonStatements.add(jsonStatement); - } - } - - // assign unique SIDs (the statement index) if `autoAssignSids` is enabled - const statements = uniqueStatements.map((s, i) => { - if (this._autoAssignSids && !s.Sid) { - s.Sid = i.toString(); - } - - return s; - }); - - return { - ...input, - Statement: statements - }; + public resolve(context: cdk.IResolveContext): any { + context.registerPostProcessor(new RemoveDuplicateStatements(this.autoAssignSids)); + return this.render(); } - get isEmpty(): boolean { + public get isEmpty(): boolean { return this.statements.length === 0; } @@ -68,7 +38,7 @@ export class PolicyDocument implements cdk.IResolvableWithPostProcess { * The number of statements already added to this policy. * Can be used, for example, to generate uniuqe "sid"s within the policy. */ - get statementCount(): number { + public get statementCount(): number { return this.statements.length; } @@ -77,30 +47,27 @@ export class PolicyDocument implements cdk.IResolvableWithPostProcess { * * @param statement the statement to add. */ - public addStatement(statement: PolicyStatement): PolicyDocument { - this.statements.push(statement); - return this; + public addStatements(...statement: PolicyStatement[]) { + this.statements.push(...statement); } + /** + * Encode the policy document as a string + */ public toString() { return cdk.Token.asString(this, { displayHint: 'PolicyDocument' }); } - public toJSON() { - return this.render(); - } - - private render() { + private render(): any { if (this.isEmpty) { return undefined; } const doc = { - ...this.baseDocument, - Statement: (this.baseDocument.Statement || []).concat(this.statements), - Version: this.baseDocument.Version || '2012-10-17' + Statement: this.statements, + Version: '2012-10-17' }; return doc; @@ -108,259 +75,40 @@ export class PolicyDocument implements cdk.IResolvableWithPostProcess { } /** - * Represents a statement in an IAM policy document. + * Removes duplicate statements and assign Sids if necessary */ -export class PolicyStatement implements cdk.IResolvable { - public sid?: string; - - private action = new Array(); - private principal: { [key: string]: any[] } = {}; - private resource = new Array(); - private condition: { [key: string]: any } = { }; - private effect?: PolicyStatementEffect; - - constructor(effect: PolicyStatementEffect = PolicyStatementEffect.Allow) { - this.effect = effect; - } - - // - // Actions - // - - public addAction(action: string): PolicyStatement { - this.action.push(action); - return this; +class RemoveDuplicateStatements implements IPostProcessor { + constructor(private readonly autoAssignSids: boolean) { } - public addActions(...actions: string[]): PolicyStatement { - actions.forEach(action => this.addAction(action)); - return this; - } - - // - // Principal - // - - /** - * Indicates if this permission has a "Principal" section. - */ - public get hasPrincipal() { - return Object.keys(this.principal).length > 0; - } - - public addPrincipal(principal: IPrincipal): this { - const fragment = principal.policyFragment; - mergePrincipal(this.principal, fragment.principalJson); - this.addConditions(fragment.conditions); - return this; - } - - public addAwsPrincipal(arn: string): this { - return this.addPrincipal(new ArnPrincipal(arn)); - } - - public addAwsAccountPrincipal(accountId: string): this { - return this.addPrincipal(new AccountPrincipal(accountId)); - } - - public addArnPrincipal(arn: string): this { - return this.addAwsPrincipal(arn); - } - - /** - * Adds a service principal to this policy statement. - * - * @param service the service name for which a service principal is requested (e.g: `s3.amazonaws.com`). - * @param opts options for adding the service principal (such as specifying a principal in a different region) - */ - public addServicePrincipal(service: string, opts?: ServicePrincipalOpts): this { - return this.addPrincipal(new ServicePrincipal(service, opts)); - } - - public addFederatedPrincipal(federated: any, conditions: {[key: string]: any}): this { - return this.addPrincipal(new FederatedPrincipal(federated, conditions)); - } - - public addAccountRootPrincipal(): this { - return this.addPrincipal(new AccountRootPrincipal()); - } - - public addCanonicalUserPrincipal(canonicalUserId: string): this { - return this.addPrincipal(new CanonicalUserPrincipal(canonicalUserId)); - } - - public addAnyPrincipal(): this { - return this.addPrincipal(new Anyone()); - } - - // - // Resources - // - - public addResource(arn: string): PolicyStatement { - this.resource.push(arn); - return this; - } - - /** - * Adds a ``"*"`` resource to this statement. - */ - public addAllResources(): PolicyStatement { - return this.addResource('*'); - } - - public addResources(...arns: string[]): PolicyStatement { - arns.forEach(r => this.addResource(r)); - return this; - } - - /** - * Indicates if this permission as at least one resource associated with it. - */ - public get hasResource() { - return this.resource && this.resource.length > 0; - } - - /** - * @deprecated Use `statement.sid = value` - */ - public describe(sid: string): PolicyStatement { - this.sid = sid; - return this; - } - - // - // Effect - // - - /** - * Sets the permission effect to allow access to resources. - */ - public allow(): PolicyStatement { - this.effect = PolicyStatementEffect.Allow; - return this; - } - - /** - * Sets the permission effect to deny access to resources. - */ - public deny(): PolicyStatement { - this.effect = PolicyStatementEffect.Deny; - return this; - } - - // - // Condition - // - - /** - * Add a condition to the Policy - */ - public addCondition(key: string, value: any): PolicyStatement { - this.condition[key] = value; - return this; - } - - /** - * Add multiple conditions to the Policy - */ - public addConditions(conditions: {[key: string]: any}): PolicyStatement { - Object.keys(conditions).map(key => { - this.addCondition(key, conditions[key]); - }); - return this; - } - - /** - * Add a condition to the Policy. - * - * @deprecated For backwards compatibility. Use addCondition() instead. - */ - public setCondition(key: string, value: any): PolicyStatement { - return this.addCondition(key, value); - } - - public limitToAccount(accountId: string): PolicyStatement { - return this.addCondition('StringEquals', { 'sts:ExternalId': accountId }); - } - - // - // Serialization - // - public resolve(_context: cdk.IResolveContext): any { - return this.toJson(); - } - - public toJson(): any { - return { - Action: _norm(this.action), - Condition: _norm(this.condition), - Effect: _norm(this.effect), - Principal: _normPrincipal(this.principal), - Resource: _norm(this.resource), - Sid: _norm(this.sid), - }; - - function _norm(values: any) { - - if (typeof(values) === 'undefined') { - return undefined; - } - - if (cdk.Token.isUnresolved(values)) { - return values; - } - - if (Array.isArray(values)) { - if (!values || values.length === 0) { - return undefined; - } - - if (values.length === 1) { - return values[0]; - } + public postProcess(input: any, _context: cdk.IResolveContext): any { + if (!input || !input.Statement) { + return input; + } - return values; - } + const jsonStatements = new Set(); + const uniqueStatements: any[] = []; - if (typeof(values) === 'object') { - if (Object.keys(values).length === 0) { - return undefined; - } + for (const statement of input.Statement) { + const jsonStatement = JSON.stringify(statement); + if (!jsonStatements.has(jsonStatement)) { + uniqueStatements.push(statement); + jsonStatements.add(jsonStatement); } - - return values; } - function _normPrincipal(principal: { [key: string]: any[] }) { - const keys = Object.keys(principal); - if (keys.length === 0) { return undefined; } - const result: any = {}; - for (const key of keys) { - const normVal = _norm(principal[key]); - if (normVal) { - result[key] = normVal; - } - } - if (Object.keys(result).length === 1 && result.AWS === '*') { - return '*'; + // assign unique SIDs (the statement index) if `autoAssignSids` is enabled + const statements = uniqueStatements.map((s, i) => { + if (this.autoAssignSids && !s.Sid) { + s.Sid = i.toString(); } - return result; - } - } - public toString() { - return cdk.Token.asString(this, { - displayHint: 'PolicyStatement' + return s; }); - } - public toJSON() { - return this.toJson(); + return { + ...input, + Statement: statements + }; } -} - -export enum PolicyStatementEffect { - Allow = 'Allow', - Deny = 'Deny', -} +} \ No newline at end of file diff --git a/packages/@aws-cdk/aws-iam/lib/policy-statement.ts b/packages/@aws-cdk/aws-iam/lib/policy-statement.ts new file mode 100644 index 0000000000000..2c1bf42d5ec3e --- /dev/null +++ b/packages/@aws-cdk/aws-iam/lib/policy-statement.ts @@ -0,0 +1,262 @@ +import cdk = require('@aws-cdk/cdk'); +import { AccountPrincipal, AccountRootPrincipal, Anyone, ArnPrincipal, CanonicalUserPrincipal, + FederatedPrincipal, IPrincipal, ServicePrincipal, ServicePrincipalOpts } from './principals'; +import { mergePrincipal } from './util'; + +/** + * Represents a statement in an IAM policy document. + */ +export class PolicyStatement implements cdk.IResolvable { + public sid?: string; + + private action = new Array(); + private principal: { [key: string]: any[] } = {}; + private resource = new Array(); + private condition: { [key: string]: any } = { }; + private effect?: PolicyStatementEffect; + + constructor(effect: PolicyStatementEffect = PolicyStatementEffect.Allow) { + this.effect = effect; + } + + // + // Actions + // + + public addAction(action: string): PolicyStatement { + this.action.push(action); + return this; + } + + public addActions(...actions: string[]): PolicyStatement { + actions.forEach(action => this.addAction(action)); + return this; + } + + // + // Principal + // + + /** + * Indicates if this permission has a "Principal" section. + */ + public get hasPrincipal() { + return Object.keys(this.principal).length > 0; + } + + public addPrincipal(principal: IPrincipal): this { + const fragment = principal.policyFragment; + mergePrincipal(this.principal, fragment.principalJson); + this.addConditions(fragment.conditions); + return this; + } + + public addAwsPrincipal(arn: string): this { + return this.addPrincipal(new ArnPrincipal(arn)); + } + + public addAwsAccountPrincipal(accountId: string): this { + return this.addPrincipal(new AccountPrincipal(accountId)); + } + + public addArnPrincipal(arn: string): this { + return this.addAwsPrincipal(arn); + } + + /** + * Adds a service principal to this policy statement. + * + * @param service the service name for which a service principal is requested (e.g: `s3.amazonaws.com`). + * @param opts options for adding the service principal (such as specifying a principal in a different region) + */ + public addServicePrincipal(service: string, opts?: ServicePrincipalOpts): this { + return this.addPrincipal(new ServicePrincipal(service, opts)); + } + + public addFederatedPrincipal(federated: any, conditions: {[key: string]: any}): this { + return this.addPrincipal(new FederatedPrincipal(federated, conditions)); + } + + public addAccountRootPrincipal(): this { + return this.addPrincipal(new AccountRootPrincipal()); + } + + public addCanonicalUserPrincipal(canonicalUserId: string): this { + return this.addPrincipal(new CanonicalUserPrincipal(canonicalUserId)); + } + + public addAnyPrincipal(): this { + return this.addPrincipal(new Anyone()); + } + + // + // Resources + // + + public addResource(arn: string): PolicyStatement { + this.resource.push(arn); + return this; + } + + /** + * Adds a ``"*"`` resource to this statement. + */ + public addAllResources(): PolicyStatement { + return this.addResource('*'); + } + + public addResources(...arns: string[]): PolicyStatement { + arns.forEach(r => this.addResource(r)); + return this; + } + + /** + * Indicates if this permission as at least one resource associated with it. + */ + public get hasResource() { + return this.resource && this.resource.length > 0; + } + + /** + * @deprecated Use `statement.sid = value` + */ + public describe(sid: string): PolicyStatement { + this.sid = sid; + return this; + } + + // + // Effect + // + + /** + * Sets the permission effect to allow access to resources. + */ + public allow(): PolicyStatement { + this.effect = PolicyStatementEffect.Allow; + return this; + } + + /** + * Sets the permission effect to deny access to resources. + */ + public deny(): PolicyStatement { + this.effect = PolicyStatementEffect.Deny; + return this; + } + + // + // Condition + // + + /** + * Add a condition to the Policy + */ + public addCondition(key: string, value: any): PolicyStatement { + this.condition[key] = value; + return this; + } + + /** + * Add multiple conditions to the Policy + */ + public addConditions(conditions: {[key: string]: any}): PolicyStatement { + Object.keys(conditions).map(key => { + this.addCondition(key, conditions[key]); + }); + return this; + } + + /** + * Add a condition to the Policy. + * + * @deprecated For backwards compatibility. Use addCondition() instead. + */ + public setCondition(key: string, value: any): PolicyStatement { + return this.addCondition(key, value); + } + + public limitToAccount(accountId: string): PolicyStatement { + return this.addCondition('StringEquals', { 'sts:ExternalId': accountId }); + } + + // + // Serialization + // + public resolve(_context: cdk.IResolveContext): any { + return this.toJson(); + } + + public toJson(): any { + return { + Action: _norm(this.action), + Condition: _norm(this.condition), + Effect: _norm(this.effect), + Principal: _normPrincipal(this.principal), + Resource: _norm(this.resource), + Sid: _norm(this.sid), + }; + + function _norm(values: any) { + + if (typeof(values) === 'undefined') { + return undefined; + } + + if (cdk.Token.isUnresolved(values)) { + return values; + } + + if (Array.isArray(values)) { + if (!values || values.length === 0) { + return undefined; + } + + if (values.length === 1) { + return values[0]; + } + + return values; + } + + if (typeof(values) === 'object') { + if (Object.keys(values).length === 0) { + return undefined; + } + } + + return values; + } + + function _normPrincipal(principal: { [key: string]: any[] }) { + const keys = Object.keys(principal); + if (keys.length === 0) { return undefined; } + const result: any = {}; + for (const key of keys) { + const normVal = _norm(principal[key]); + if (normVal) { + result[key] = normVal; + } + } + if (Object.keys(result).length === 1 && result.AWS === '*') { + return '*'; + } + return result; + } + } + + public toString() { + return cdk.Token.asString(this, { + displayHint: 'PolicyStatement' + }); + } + + public toJSON() { + return this.toJson(); + } +} + +export enum PolicyStatementEffect { + Allow = 'Allow', + Deny = 'Deny', +} \ No newline at end of file diff --git a/packages/@aws-cdk/aws-iam/lib/policy.ts b/packages/@aws-cdk/aws-iam/lib/policy.ts index 8d3b7f66d7343..e5ef29ec99ff4 100644 --- a/packages/@aws-cdk/aws-iam/lib/policy.ts +++ b/packages/@aws-cdk/aws-iam/lib/policy.ts @@ -1,7 +1,8 @@ import { Construct, IResource, Lazy, Resource } from '@aws-cdk/cdk'; import { IGroup } from './group'; import { CfnPolicy } from './iam.generated'; -import { PolicyDocument, PolicyStatement } from './policy-document'; +import { PolicyDocument } from './policy-document'; +import { PolicyStatement } from './policy-statement'; import { IRole } from './role'; import { IUser } from './user'; import { generatePolicyName, undefinedIfEmpty } from './util'; @@ -118,15 +119,15 @@ export class Policy extends Resource implements IPolicy { } if (props.statements) { - props.statements.forEach(p => this.addStatement(p)); + props.statements.forEach(p => this.addStatements(p)); } } /** * Adds a statement to the policy document. */ - public addStatement(statement: PolicyStatement) { - this.document.addStatement(statement); + public addStatements(...statement: PolicyStatement[]) { + this.document.addStatements(...statement); } /** diff --git a/packages/@aws-cdk/aws-iam/lib/principals.ts b/packages/@aws-cdk/aws-iam/lib/principals.ts index 8a79d26190e02..458f62f86a68d 100644 --- a/packages/@aws-cdk/aws-iam/lib/principals.ts +++ b/packages/@aws-cdk/aws-iam/lib/principals.ts @@ -1,7 +1,7 @@ import cdk = require('@aws-cdk/cdk'); import { Stack } from '@aws-cdk/cdk'; import { Default, RegionInfo } from '@aws-cdk/region-info'; -import { PolicyStatement } from './policy-document'; +import { PolicyStatement } from './policy-statement'; import { mergePrincipal } from './util'; /** diff --git a/packages/@aws-cdk/aws-iam/lib/role.ts b/packages/@aws-cdk/aws-iam/lib/role.ts index de92aee7d1f14..c9304050c8194 100644 --- a/packages/@aws-cdk/aws-iam/lib/role.ts +++ b/packages/@aws-cdk/aws-iam/lib/role.ts @@ -4,7 +4,8 @@ import { CfnRole } from './iam.generated'; import { IIdentity } from './identity-base'; import { IManagedPolicy } from './managed-policy'; import { Policy } from './policy'; -import { PolicyDocument, PolicyStatement } from './policy-document'; +import { PolicyDocument } from './policy-document'; +import { PolicyStatement } from './policy-statement'; import { ArnPrincipal, IPrincipal, PrincipalPolicyFragment } from './principals'; import { AttachedPolicies } from './util'; @@ -247,7 +248,7 @@ export class Role extends Resource implements IRole { this.defaultPolicy = new Policy(this, 'DefaultPolicy'); this.attachInlinePolicy(this.defaultPolicy); } - this.defaultPolicy.addStatement(statement); + this.defaultPolicy.addStatements(statement); return true; } @@ -327,7 +328,9 @@ function createAssumeRolePolicy(principal: IPrincipal, externalId?: string) { statement.addCondition('StringEquals', { 'sts:ExternalId': externalId }); } - return new PolicyDocument().addStatement(statement); + const doc = new PolicyDocument(); + doc.addStatements(statement); + return doc; } function validateMaxSessionDuration(duration?: number) { diff --git a/packages/@aws-cdk/aws-iam/lib/unknown-principal.ts b/packages/@aws-cdk/aws-iam/lib/unknown-principal.ts index 6778dcfa66578..5b94fff6bf643 100644 --- a/packages/@aws-cdk/aws-iam/lib/unknown-principal.ts +++ b/packages/@aws-cdk/aws-iam/lib/unknown-principal.ts @@ -1,5 +1,5 @@ import { IConstruct, Stack } from '@aws-cdk/cdk'; -import { PolicyStatement } from './policy-document'; +import { PolicyStatement } from './policy-statement'; import { IPrincipal, PrincipalPolicyFragment } from './principals'; /** diff --git a/packages/@aws-cdk/aws-iam/lib/user.ts b/packages/@aws-cdk/aws-iam/lib/user.ts index 17dadf34cbf4b..04c8918348f66 100644 --- a/packages/@aws-cdk/aws-iam/lib/user.ts +++ b/packages/@aws-cdk/aws-iam/lib/user.ts @@ -4,7 +4,7 @@ import { CfnUser } from './iam.generated'; import { IIdentity } from './identity-base'; import { IManagedPolicy } from './managed-policy'; import { Policy } from './policy'; -import { PolicyStatement } from './policy-document'; +import { PolicyStatement } from './policy-statement'; import { ArnPrincipal, PrincipalPolicyFragment } from './principals'; import { IPrincipal } from './principals'; import { AttachedPolicies, undefinedIfEmpty } from './util'; @@ -156,7 +156,7 @@ export class User extends Resource implements IIdentity { this.defaultPolicy.attachToUser(this); } - this.defaultPolicy.addStatement(statement); + this.defaultPolicy.addStatements(statement); return true; } diff --git a/packages/@aws-cdk/aws-iam/test/integ.policy.ts b/packages/@aws-cdk/aws-iam/test/integ.policy.ts index 9d23de82c9cdb..0cb682080ac08 100644 --- a/packages/@aws-cdk/aws-iam/test/integ.policy.ts +++ b/packages/@aws-cdk/aws-iam/test/integ.policy.ts @@ -9,11 +9,11 @@ const stack = new Stack(app, 'aws-cdk-iam-policy'); const user = new User(stack, 'MyUser'); const policy = new Policy(stack, 'HelloPolicy', { policyName: 'Default' }); -policy.addStatement(new PolicyStatement().addResource('*').addAction('sqs:SendMessage')); +policy.addStatements(new PolicyStatement().addResource('*').addAction('sqs:SendMessage')); policy.attachToUser(user); const policy2 = new Policy(stack, 'GoodbyePolicy'); -policy2.addStatement(new PolicyStatement().addResource('*').addAction('lambda:InvokeFunction')); +policy2.addStatements(new PolicyStatement().addResource('*').addAction('lambda:InvokeFunction')); policy2.attachToUser(user); app.synth(); diff --git a/packages/@aws-cdk/aws-iam/test/integ.role.ts b/packages/@aws-cdk/aws-iam/test/integ.role.ts index ee548b53d1f1b..cf47c170ac02b 100644 --- a/packages/@aws-cdk/aws-iam/test/integ.role.ts +++ b/packages/@aws-cdk/aws-iam/test/integ.role.ts @@ -12,7 +12,7 @@ const role = new Role(stack, 'TestRole', { role.addToPolicy(new PolicyStatement().addResource('*').addAction('sqs:SendMessage')); const policy = new Policy(stack, 'HelloPolicy', { policyName: 'Default' }); -policy.addStatement(new PolicyStatement().addAction('ec2:*').addResource('*')); +policy.addStatements(new PolicyStatement().addAction('ec2:*').addResource('*')); policy.attachToRole(role); // Role with an external ID diff --git a/packages/@aws-cdk/aws-iam/test/integ.users-and-groups.ts b/packages/@aws-cdk/aws-iam/test/integ.users-and-groups.ts index 4b0260fde84f1..e99c30c3b1eb8 100644 --- a/packages/@aws-cdk/aws-iam/test/integ.users-and-groups.ts +++ b/packages/@aws-cdk/aws-iam/test/integ.users-and-groups.ts @@ -16,6 +16,6 @@ for (let i = 0; i < 5; ++i) { const policy = new Policy(stack, 'MyPolicy'); policy.attachToGroup(g1); -policy.addStatement(new PolicyStatement().addResource(g2.groupArn).addAction('iam:*')); +policy.addStatements(new PolicyStatement().addResource(g2.groupArn).addAction('iam:*')); app.synth(); diff --git a/packages/@aws-cdk/aws-iam/test/test.policy-document.ts b/packages/@aws-cdk/aws-iam/test/test.policy-document.ts index 2ba27fcd6f0ac..6ced727881df4 100644 --- a/packages/@aws-cdk/aws-iam/test/test.policy-document.ts +++ b/packages/@aws-cdk/aws-iam/test/test.policy-document.ts @@ -48,8 +48,8 @@ export = { p2.deny(); p2.addActions('cloudformation:CreateStack'); - doc.addStatement(p1); - doc.addStatement(p2); + doc.addStatements(p1); + doc.addStatements(p2); test.deepEqual(stack.resolve(doc), { Version: '2012-10-17', @@ -60,28 +60,6 @@ export = { test.done(); }, - 'A PolicyDocument can be initialized with an existing policy, which is merged upon serialization'(test: Test) { - const stack = new Stack(); - const base = { - Version: 'Foo', - Something: 123, - Statement: [ - { Statement1: 1 }, - { Statement2: 2 } - ] - }; - const doc = new PolicyDocument(base); - doc.addStatement(new PolicyStatement().addResource('resource').addAction('action')); - - test.deepEqual(stack.resolve(doc), { Version: 'Foo', - Something: 123, - Statement: - [ { Statement1: 1 }, - { Statement2: 2 }, - { Effect: 'Allow', Action: 'action', Resource: 'resource' } ] }); - test.done(); - }, - 'Permission allows specifying multiple actions upon construction'(test: Test) { const stack = new Stack(); const perm = new PolicyStatement().addResource('MyResource').addActions('Action1', 'Action2', 'Action3'); @@ -212,9 +190,9 @@ export = { 'statementCount returns the number of statement in the policy document'(test: Test) { const p = new PolicyDocument(); test.equal(p.statementCount, 0); - p.addStatement(new PolicyStatement().addAction('action1')); + p.addStatements(new PolicyStatement().addAction('action1')); test.equal(p.statementCount, 1); - p.addStatement(new PolicyStatement().addAction('action2')); + p.addStatements(new PolicyStatement().addAction('action2')); test.equal(p.statementCount, 2); test.done(); }, @@ -224,7 +202,7 @@ export = { const stack = new Stack(); const p = new PolicyDocument(); - p.addStatement(new PolicyStatement().addPrincipal(new Anyone())); + p.addStatements(new PolicyStatement().addPrincipal(new Anyone())); test.deepEqual(stack.resolve(p), { Statement: [ @@ -239,7 +217,7 @@ export = { const stack = new Stack(); const p = new PolicyDocument(); - p.addStatement(new PolicyStatement().addPrincipal(new AnyPrincipal())); + p.addStatements(new PolicyStatement().addPrincipal(new AnyPrincipal())); test.deepEqual(stack.resolve(p), { Statement: [ @@ -254,7 +232,7 @@ export = { const stack = new Stack(); const p = new PolicyDocument(); - p.addStatement(new PolicyStatement().addAnyPrincipal()); + p.addStatements(new PolicyStatement().addAnyPrincipal()); test.deepEqual(stack.resolve(p), { Statement: [ @@ -270,9 +248,9 @@ export = { const stack = new Stack(); const p = new PolicyDocument(); - p.addStatement(new PolicyStatement().addAwsPrincipal('111222-A')); - p.addStatement(new PolicyStatement().addArnPrincipal('111222-B')); - p.addStatement(new PolicyStatement().addPrincipal(new ArnPrincipal('111222-C'))); + p.addStatements(new PolicyStatement().addAwsPrincipal('111222-A')); + p.addStatements(new PolicyStatement().addArnPrincipal('111222-B')); + p.addStatements(new PolicyStatement().addPrincipal(new ArnPrincipal('111222-C'))); test.deepEqual(stack.resolve(p), { Statement: [ { @@ -306,8 +284,8 @@ export = { const stack = new Stack(); const p = new PolicyDocument(); - p.addStatement(new PolicyStatement().addCanonicalUserPrincipal('cannonical-user-1')); - p.addStatement(new PolicyStatement().addPrincipal(new CanonicalUserPrincipal('cannonical-user-2'))); + p.addStatements(new PolicyStatement().addCanonicalUserPrincipal('cannonical-user-1')); + p.addStatements(new PolicyStatement().addPrincipal(new CanonicalUserPrincipal('cannonical-user-2'))); test.deepEqual(stack.resolve(p), { Statement: [ @@ -491,9 +469,9 @@ export = { }); // WHEN - p.addStatement(statement); - p.addStatement(statement); - p.addStatement(statement); + p.addStatements(statement); + p.addStatements(statement); + p.addStatements(statement); // THEN test.equal(stack.resolve(p).Statement.length, 1); @@ -513,55 +491,28 @@ export = { .addAction(Lazy.stringValue({ produce: () => 'action' })); // WHEN - p.addStatement(statement1); - p.addStatement(statement2); + p.addStatements(statement1); + p.addStatements(statement2); // THEN test.equal(stack.resolve(p).Statement.length, 1); test.done(); }, - 'with base document'(test: Test) { - // GIVEN - const stack = new Stack(); - - // WHEN - const p = new PolicyDocument({ - Statement: [ - { - Action: 'action', - Effect: 'Allow', - Resource: 'resource' - }, - { - Action: 'action', - Effect: 'Allow', - Resource: 'resource' - } - ] - }); - - p.addStatement(new PolicyStatement() - .addAction('action') - .addResource('resource')); - - // THEN - test.equal(stack.resolve(p).Statement.length, 1); - test.done(); - } }, 'autoAssignSids enables auto-assignment of a unique SID for each statement'(test: Test) { // GIVEN - const doc = new PolicyDocument(); - doc.addStatement(new PolicyStatement().addAction('action1').addResource('resource1')); - doc.addStatement(new PolicyStatement().addAction('action1').addResource('resource1')); - doc.addStatement(new PolicyStatement().addAction('action1').addResource('resource1')); - doc.addStatement(new PolicyStatement().addAction('action1').addResource('resource1')); - doc.addStatement(new PolicyStatement().addAction('action2').addResource('resource2')); + const doc = new PolicyDocument({ + assignSids: true + }); // WHEN - doc.autoAssignSids(); + doc.addStatements(new PolicyStatement().addAction('action1').addResource('resource1')); + doc.addStatements(new PolicyStatement().addAction('action1').addResource('resource1')); + doc.addStatements(new PolicyStatement().addAction('action1').addResource('resource1')); + doc.addStatements(new PolicyStatement().addAction('action1').addResource('resource1')); + doc.addStatements(new PolicyStatement().addAction('action2').addResource('resource2')); // THEN const stack = new Stack(); diff --git a/packages/@aws-cdk/aws-iam/test/test.policy.ts b/packages/@aws-cdk/aws-iam/test/test.policy.ts index bf54cdd94e2d8..e0cc671ec877f 100644 --- a/packages/@aws-cdk/aws-iam/test/test.policy.ts +++ b/packages/@aws-cdk/aws-iam/test/test.policy.ts @@ -19,8 +19,8 @@ export = { const stack = new Stack(app, 'MyStack'); const policy = new Policy(stack, 'MyPolicy', { policyName: 'MyPolicyName' }); - policy.addStatement(new PolicyStatement().addResource('*').addAction('sqs:SendMessage')); - policy.addStatement(new PolicyStatement().addResource('arn').addAction('sns:Subscribe')); + policy.addStatements(new PolicyStatement().addResource('*').addAction('sqs:SendMessage')); + policy.addStatements(new PolicyStatement().addResource('arn').addAction('sns:Subscribe')); const group = new Group(stack, 'MyGroup'); group.attachInlinePolicy(policy); @@ -45,8 +45,8 @@ export = { const stack = new Stack(app, 'MyStack'); const policy = new Policy(stack, 'MyPolicy'); - policy.addStatement(new PolicyStatement().addResource('*').addAction('sqs:SendMessage')); - policy.addStatement(new PolicyStatement().addResource('arn').addAction('sns:Subscribe')); + policy.addStatements(new PolicyStatement().addResource('*').addAction('sqs:SendMessage')); + policy.addStatements(new PolicyStatement().addResource('arn').addAction('sns:Subscribe')); const user = new User(stack, 'MyUser'); user.attachInlinePolicy(policy); @@ -116,7 +116,7 @@ export = { const app = new App(); const stack = new Stack(app, 'MyStack'); const p = new Policy(stack, 'MyPolicy'); - p.addStatement(new PolicyStatement().addAction('*').addResource('*')); + p.addStatements(new PolicyStatement().addAction('*').addResource('*')); const user = new User(stack, 'MyUser'); p.attachToUser(user); @@ -148,7 +148,7 @@ export = { p.attachToUser(new User(stack, 'User2')); p.attachToGroup(new Group(stack, 'Group1')); p.attachToRole(new Role(stack, 'Role1', { assumedBy: new ServicePrincipal('test.service') })); - p.addStatement(new PolicyStatement().addResource('*').addAction('dynamodb:GetItem')); + p.addStatements(new PolicyStatement().addResource('*').addAction('dynamodb:GetItem')); expect(stack).toMatch({ Resources: { MyTestPolicy316BDB50: @@ -190,7 +190,7 @@ export = { group.attachInlinePolicy(policy); role.attachInlinePolicy(policy); - policy.addStatement(new PolicyStatement().addResource('*').addAction('*')); + policy.addStatements(new PolicyStatement().addResource('*').addAction('*')); expect(stack).toMatch({ Resources: { MyPolicy39D66CF6: diff --git a/packages/@aws-cdk/aws-kms/lib/key.ts b/packages/@aws-cdk/aws-kms/lib/key.ts index 9950da6b13d05..a3711bbc564bc 100644 --- a/packages/@aws-cdk/aws-kms/lib/key.ts +++ b/packages/@aws-cdk/aws-kms/lib/key.ts @@ -86,7 +86,7 @@ abstract class KeyBase extends Resource implements IKey { throw new Error(`Unable to add statement to IAM resource policy for KMS key: ${JSON.stringify(stack.resolve(this.keyArn))}`); } - this.policy.addStatement(statement); + this.policy.addStatements(statement); } /** diff --git a/packages/@aws-cdk/aws-logs/lib/cross-account-destination.ts b/packages/@aws-cdk/aws-logs/lib/cross-account-destination.ts index f71765f5aeeb3..68eaa0d4ced04 100644 --- a/packages/@aws-cdk/aws-logs/lib/cross-account-destination.ts +++ b/packages/@aws-cdk/aws-logs/lib/cross-account-destination.ts @@ -80,7 +80,7 @@ export class CrossAccountDestination extends cdk.Construct implements ILogSubscr } public addToPolicy(statement: iam.PolicyStatement) { - this.policyDocument.addStatement(statement); + this.policyDocument.addStatements(statement); } public bind(_scope: Construct, _sourceLogGroup: ILogGroup): LogSubscriptionDestinationConfig { diff --git a/packages/@aws-cdk/aws-s3/lib/bucket.ts b/packages/@aws-cdk/aws-s3/lib/bucket.ts index b73d8725f32e9..5aabf47ccbacf 100644 --- a/packages/@aws-cdk/aws-s3/lib/bucket.ts +++ b/packages/@aws-cdk/aws-s3/lib/bucket.ts @@ -348,7 +348,7 @@ abstract class BucketBase extends Resource implements IBucket { } if (this.policy) { - this.policy.document.addStatement(permission); + this.policy.document.addStatements(permission); } } diff --git a/packages/@aws-cdk/aws-sns/lib/policy.ts b/packages/@aws-cdk/aws-sns/lib/policy.ts index 974a6f03833de..7b289e30a58a8 100644 --- a/packages/@aws-cdk/aws-sns/lib/policy.ts +++ b/packages/@aws-cdk/aws-sns/lib/policy.ts @@ -17,16 +17,16 @@ export class TopicPolicy extends Resource { /** * The IAM policy document for this policy. */ - public readonly document = new PolicyDocument(); - - constructor(scope: Construct, id: string, props: TopicPolicyProps) { - super(scope, id); - + public readonly document = new PolicyDocument({ // statements must be unique, so we use the statement index. // potantially SIDs can change as a result of order change, but this should // not have an impact on the policy evaluation. // https://docs.aws.amazon.com/sns/latest/dg/AccessPolicyLanguage_SpecialInfo.html - this.document.autoAssignSids(); + assignSids: true + }); + + constructor(scope: Construct, id: string, props: TopicPolicyProps) { + super(scope, id); new CfnTopicPolicy(this, 'Resource', { policyDocument: this.document, diff --git a/packages/@aws-cdk/aws-sns/lib/topic-base.ts b/packages/@aws-cdk/aws-sns/lib/topic-base.ts index 7c6c4c842e4fb..dce05fadaa28d 100644 --- a/packages/@aws-cdk/aws-sns/lib/topic-base.ts +++ b/packages/@aws-cdk/aws-sns/lib/topic-base.ts @@ -71,7 +71,7 @@ export abstract class TopicBase extends Resource implements ITopic { } if (this.policy) { - this.policy.document.addStatement(statement); + this.policy.document.addStatements(statement); } } diff --git a/packages/@aws-cdk/aws-sqs/lib/queue-base.ts b/packages/@aws-cdk/aws-sqs/lib/queue-base.ts index 1e855d63d5acc..ae2b134a030b3 100644 --- a/packages/@aws-cdk/aws-sqs/lib/queue-base.ts +++ b/packages/@aws-cdk/aws-sqs/lib/queue-base.ts @@ -134,7 +134,7 @@ export abstract class QueueBase extends Resource implements IQueue { } if (this.policy) { - this.policy.document.addStatement(statement); + this.policy.document.addStatements(statement); } } diff --git a/packages/@aws-cdk/cdk/lib/cloudformation-lang.ts b/packages/@aws-cdk/cdk/lib/cloudformation-lang.ts index cb07b064c594a..596ea89204888 100644 --- a/packages/@aws-cdk/cdk/lib/cloudformation-lang.ts +++ b/packages/@aws-cdk/cdk/lib/cloudformation-lang.ts @@ -1,7 +1,7 @@ import { Lazy } from "./lazy"; import { Intrinsic } from "./private/intrinsic"; import { resolve } from "./private/resolve"; -import { DefaultTokenResolver, IFragmentConcatenator, IResolvable, IResolveContext } from "./resolvable"; +import { DefaultTokenResolver, IFragmentConcatenator, IPostProcessor, IResolvable, IResolveContext } from "./resolvable"; import { TokenizedStringFragments } from "./string-fragments"; import { Token } from "./token"; @@ -45,8 +45,8 @@ export class CloudFormationLang { super(CLOUDFORMATION_CONCAT); } - public resolveToken(t: IResolvable, context: IResolveContext) { - return wrap(super.resolveToken(t, context)); + public resolveToken(t: IResolvable, context: IResolveContext, postProcess: IPostProcessor) { + return wrap(super.resolveToken(t, context, postProcess)); } public resolveString(fragments: TokenizedStringFragments, context: IResolveContext) { return wrap(super.resolveString(fragments, context)); diff --git a/packages/@aws-cdk/cdk/lib/private/resolve.ts b/packages/@aws-cdk/cdk/lib/private/resolve.ts index 70495d8706557..c3d1e173e9294 100644 --- a/packages/@aws-cdk/cdk/lib/private/resolve.ts +++ b/packages/@aws-cdk/cdk/lib/private/resolve.ts @@ -1,5 +1,5 @@ import { IConstruct } from '../construct'; -import { DefaultTokenResolver, IResolvable, IResolveContext, ITokenResolver, StringConcat } from '../resolvable'; +import { DefaultTokenResolver, IPostProcessor, IResolvable, IResolveContext, ITokenResolver, StringConcat } from '../resolvable'; import { TokenizedStringFragments } from '../string-fragments'; import { containsListTokenElement, TokenString, unresolved } from "./encoding"; import { TokenMap } from './token-map'; @@ -36,12 +36,18 @@ export function resolve(obj: any, options: IResolveOptions): any { /** * Make a new resolution context */ - function makeContext(appendPath?: string): IResolveContext { + function makeContext(appendPath?: string): [IResolveContext, IPostProcessor] { const newPrefix = appendPath !== undefined ? prefix.concat([appendPath]) : options.prefix; - return { + + let postProcessor: IPostProcessor | undefined; + + const context: IResolveContext = { scope: options.scope, - resolve(x: any) { return resolve(x, { ...options, prefix: newPrefix }); } + registerPostProcessor(pp) { postProcessor = pp; }, + resolve(x: any) { return resolve(x, { ...options, prefix: newPrefix }); }, }; + + return [context, { postProcess(x) { return postProcessor ? postProcessor.postProcess(x, context) : x; }}]; } // protect against cyclic references by limiting depth. @@ -80,7 +86,7 @@ export function resolve(obj: any, options: IResolveOptions): any { const str = TokenString.forString(obj); if (str.test()) { const fragments = str.split(tokenMap.lookupToken.bind(tokenMap)); - return options.resolver.resolveString(fragments, makeContext()); + return options.resolver.resolveString(fragments, makeContext()[0]); } return obj; } @@ -89,7 +95,7 @@ export function resolve(obj: any, options: IResolveOptions): any { // number - potentially decode Tokenized number // if (typeof(obj) === 'number') { - return resolveNumberToken(obj, makeContext()); + return resolveNumberToken(obj, makeContext()[0]); } // @@ -106,11 +112,11 @@ export function resolve(obj: any, options: IResolveOptions): any { if (Array.isArray(obj)) { if (containsListTokenElement(obj)) { - return options.resolver.resolveList(obj, makeContext()); + return options.resolver.resolveList(obj, makeContext()[0]); } const arr = obj - .map((x, i) => makeContext(`${i}`).resolve(x)) + .map((x, i) => makeContext(`${i}`)[0].resolve(x)) .filter(x => typeof(x) !== 'undefined'); return arr; @@ -121,7 +127,8 @@ export function resolve(obj: any, options: IResolveOptions): any { // if (unresolved(obj)) { - return options.resolver.resolveToken(obj, makeContext()); + const [context, postProcessor] = makeContext(); + return options.resolver.resolveToken(obj, context, postProcessor); } // @@ -137,12 +144,12 @@ export function resolve(obj: any, options: IResolveOptions): any { const result: any = { }; for (const key of Object.keys(obj)) { - const resolvedKey = makeContext().resolve(key); + const resolvedKey = makeContext()[0].resolve(key); if (typeof(resolvedKey) !== 'string') { throw new Error(`"${key}" is used as the key in a map so must resolve to a string, but it resolves to: ${JSON.stringify(resolvedKey)}`); } - const value = makeContext(key).resolve(obj[key]); + const value = makeContext(key)[0].resolve(obj[key]); // skip undefined if (typeof(value) === 'undefined') { @@ -172,9 +179,9 @@ export function findTokens(scope: IConstruct, fn: () => any): IResolvable[] { export class RememberingTokenResolver extends DefaultTokenResolver { private readonly tokensSeen = new Set(); - public resolveToken(t: IResolvable, context: IResolveContext) { + public resolveToken(t: IResolvable, context: IResolveContext, postProcessor: IPostProcessor) { this.tokensSeen.add(t); - return super.resolveToken(t, context); + return super.resolveToken(t, context, postProcessor); } public resolveString(s: TokenizedStringFragments, context: IResolveContext) { diff --git a/packages/@aws-cdk/cdk/lib/resolvable.ts b/packages/@aws-cdk/cdk/lib/resolvable.ts index 646a69f3c4b86..ba7d830573781 100644 --- a/packages/@aws-cdk/cdk/lib/resolvable.ts +++ b/packages/@aws-cdk/cdk/lib/resolvable.ts @@ -16,6 +16,11 @@ export interface IResolveContext { * Resolve an inner object */ resolve(x: any): any; + + /** + * Use this postprocessor after the entire token structure has been resolved + */ + registerPostProcessor(postProcessor: IPostProcessor): void; } /** @@ -40,7 +45,7 @@ export interface IResolvable { /** * A Token that can post-process the complete resolved value, after resolve() has recursed over it */ -export interface IResolvableWithPostProcess extends IResolvable { +export interface IPostProcessor { /** * Process the completely resolved value, after full recursion/resolution has happened */ @@ -54,7 +59,7 @@ export interface ITokenResolver { /** * Resolve a single token */ - resolveToken(t: IResolvable, context: IResolveContext): any; + resolveToken(t: IResolvable, context: IResolveContext, postProcessor: IPostProcessor): any; /** * Resolve a string with at least one stringified token in it @@ -107,15 +112,13 @@ export class DefaultTokenResolver implements ITokenResolver { * Resolve the Token, recurse into whatever it returns, * then finally post-process it. */ - public resolveToken(t: IResolvable, context: IResolveContext) { + public resolveToken(t: IResolvable, context: IResolveContext, postProcessor: IPostProcessor) { let resolved = t.resolve(context); // The token might have returned more values that need resolving, recurse resolved = context.resolve(resolved); - if (isResolvedValuePostProcessor(t)) { - resolved = t.postProcess(resolved, context); - } + resolved = postProcessor.postProcess(resolved, context); return resolved; } @@ -142,11 +145,4 @@ export class DefaultTokenResolver implements ITokenResolver { return fragments.mapTokens({ mapToken: context.resolve }).firstValue; } -} - -/** - * Whether the given object is an `IResolvedValuePostProcessor` - */ -function isResolvedValuePostProcessor(x: any): x is IResolvableWithPostProcess { - return x.postProcess !== undefined; -} +} \ No newline at end of file diff --git a/packages/@aws-cdk/cdk/lib/util.ts b/packages/@aws-cdk/cdk/lib/util.ts index 6eb6c94f08719..a929a36c6f290 100644 --- a/packages/@aws-cdk/cdk/lib/util.ts +++ b/packages/@aws-cdk/cdk/lib/util.ts @@ -1,6 +1,6 @@ import { IConstruct } from "./construct"; import { Intrinsic } from "./private/intrinsic"; -import { IResolvableWithPostProcess, IResolveContext } from "./resolvable"; +import { IPostProcessor, IResolveContext } from "./resolvable"; import { Stack } from "./stack"; /** @@ -78,11 +78,16 @@ export function filterUndefined(obj: any): any { /** * A Token that applies a function AFTER resolve resolution */ -export class PostResolveToken extends Intrinsic implements IResolvableWithPostProcess { +export class PostResolveToken extends Intrinsic implements IPostProcessor { constructor(value: any, private readonly processor: (x: any) => any) { super(value); } + public resolve(context: IResolveContext) { + context.registerPostProcessor(this); + return super.resolve(context); + } + public postProcess(o: any, _context: IResolveContext): any { return this.processor(o); } From 4093679ea9d420005e634672d255b0e706e376e3 Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Tue, 11 Jun 2019 16:43:44 +0200 Subject: [PATCH 03/12] Introduce IPolicyStatement PolicyStatement is no longer IResolvable (doesn't need to be) Remove AwsPrincipal, which was always an alias for ArnPrincipal which is much clearer. Introduce `PolicyStatement.fromAttributes({ ... })` as an alternative to the fluent interface chaining of old `PolicyStatement`. --- packages/@aws-cdk/aws-iam/README.md | 2 +- .../@aws-cdk/aws-iam/lib/policy-document.ts | 17 +++- .../@aws-cdk/aws-iam/lib/policy-statement.ts | 81 ++++++++++++++---- .../aws-iam/test/test.policy-document.ts | 83 ++++++++++--------- packages/@aws-cdk/aws-kms/test/integ.key.ts | 2 +- packages/@aws-cdk/aws-kms/test/test.key.ts | 4 +- 6 files changed, 127 insertions(+), 62 deletions(-) diff --git a/packages/@aws-cdk/aws-iam/README.md b/packages/@aws-cdk/aws-iam/README.md index 30ec76bd767dc..d03c162d306ed 100644 --- a/packages/@aws-cdk/aws-iam/README.md +++ b/packages/@aws-cdk/aws-iam/README.md @@ -71,7 +71,7 @@ If multiple principals are added to the policy statement, they will be merged to const statement = new PolicyStatement(); statement.addServicePrincipal('cloudwatch.amazonaws.com'); statement.addServicePrincipal('ec2.amazonaws.com'); -statement.addAwsPrincipal('arn:aws:boom:boom'); +statement.addArnPrincipal('arn:aws:boom:boom'); ``` Will result in: diff --git a/packages/@aws-cdk/aws-iam/lib/policy-document.ts b/packages/@aws-cdk/aws-iam/lib/policy-document.ts index 94e9a81167941..fd0bafe92cc80 100644 --- a/packages/@aws-cdk/aws-iam/lib/policy-document.ts +++ b/packages/@aws-cdk/aws-iam/lib/policy-document.ts @@ -1,6 +1,6 @@ import cdk = require('@aws-cdk/cdk'); import { IPostProcessor } from '@aws-cdk/cdk'; -import { PolicyStatement } from './policy-statement'; +import { IPolicyStatement } from './policy-statement'; /** * Properties for a new PolicyDocument @@ -18,7 +18,7 @@ export interface PolicyDocumentProps { * A PolicyDocument is a collection of statements */ export class PolicyDocument implements cdk.IResolvable { - private readonly statements = new Array(); + private readonly statements = new Array(); private readonly autoAssignSids: boolean; constructor(props: PolicyDocumentProps = {}) { @@ -47,7 +47,7 @@ export class PolicyDocument implements cdk.IResolvable { * * @param statement the statement to add. */ - public addStatements(...statement: PolicyStatement[]) { + public addStatements(...statement: IPolicyStatement[]) { this.statements.push(...statement); } @@ -60,13 +60,22 @@ export class PolicyDocument implements cdk.IResolvable { }); } + /** + * JSON-ify the document + * + * Used when JSON.stringify() is called + */ + public toJSON() { + return this.render(); + } + private render(): any { if (this.isEmpty) { return undefined; } const doc = { - Statement: this.statements, + Statement: this.statements.map(s => s.toStatementJson()), Version: '2012-10-17' }; diff --git a/packages/@aws-cdk/aws-iam/lib/policy-statement.ts b/packages/@aws-cdk/aws-iam/lib/policy-statement.ts index 2c1bf42d5ec3e..ec5bb844a80c8 100644 --- a/packages/@aws-cdk/aws-iam/lib/policy-statement.ts +++ b/packages/@aws-cdk/aws-iam/lib/policy-statement.ts @@ -3,10 +3,34 @@ import { AccountPrincipal, AccountRootPrincipal, Anyone, ArnPrincipal, Canonical FederatedPrincipal, IPrincipal, ServicePrincipal, ServicePrincipalOpts } from './principals'; import { mergePrincipal } from './util'; +/** + * An abstract PolicyStatement + */ +export interface IPolicyStatement { + /** + * Render the policy statement to JSON + */ + toStatementJson(): any; +} + /** * Represents a statement in an IAM policy document. */ -export class PolicyStatement implements cdk.IResolvable { +export class PolicyStatement implements IPolicyStatement { + public static fromAttributes(attrs: PolicyStatementAttributes): IPolicyStatement { + const st = new PolicyStatement(); + st.addActions(...attrs.actions || []); + (attrs.principals || []).forEach(p => st.addPrincipal(p)); + st.addResources(...attrs.resourceArns || []); + if (attrs.conditions !== undefined) { + st.addConditions(attrs.conditions); + } + return st; + } + + /** + * Statement ID for this statement + */ public sid?: string; private action = new Array(); @@ -51,16 +75,12 @@ export class PolicyStatement implements cdk.IResolvable { return this; } - public addAwsPrincipal(arn: string): this { - return this.addPrincipal(new ArnPrincipal(arn)); - } - public addAwsAccountPrincipal(accountId: string): this { return this.addPrincipal(new AccountPrincipal(accountId)); } public addArnPrincipal(arn: string): this { - return this.addAwsPrincipal(arn); + return this.addPrincipal(new ArnPrincipal(arn)); } /** @@ -180,14 +200,7 @@ export class PolicyStatement implements cdk.IResolvable { return this.addCondition('StringEquals', { 'sts:ExternalId': accountId }); } - // - // Serialization - // - public resolve(_context: cdk.IResolveContext): any { - return this.toJson(); - } - - public toJson(): any { + public toStatementJson(): any { return { Action: _norm(this.action), Condition: _norm(this.condition), @@ -251,12 +264,50 @@ export class PolicyStatement implements cdk.IResolvable { }); } + /** + * JSON-ify the statement + * + * Used when JSON.stringify() is called + */ public toJSON() { - return this.toJson(); + return this.toStatementJson(); } } export enum PolicyStatementEffect { Allow = 'Allow', Deny = 'Deny', +} + +/** + * Interface for creating a policy statement + */ +export interface PolicyStatementAttributes { + /** + * List of actions to add to the statement + * + * @default - no actions + */ + actions?: string[]; + + /** + * List of principals to add to the statement + * + * @default - no principals + */ + principals?: IPrincipal[]; + + /** + * Resource ARNs to add to the statement + * + * @default - no principals + */ + resourceArns?: string[]; + + /** + * Conditions to add to the statement + * + * @default - no condition + */ + conditions?: {[key: string]: any}; } \ No newline at end of file diff --git a/packages/@aws-cdk/aws-iam/test/test.policy-document.ts b/packages/@aws-cdk/aws-iam/test/test.policy-document.ts index 6ced727881df4..2c3f60b6f8484 100644 --- a/packages/@aws-cdk/aws-iam/test/test.policy-document.ts +++ b/packages/@aws-cdk/aws-iam/test/test.policy-document.ts @@ -17,7 +17,7 @@ export = { p.addAwsAccountPrincipal(`my${Token.asString({ account: 'account' })}name`); p.limitToAccount('12221121221'); - test.deepEqual(stack.resolve(p), { Action: + test.deepEqual(stack.resolve(p.toStatementJson()), { Action: [ 'sqs:SendMessage', 'dynamodb:CreateTable', 'dynamodb:DeleteTable' ], @@ -63,7 +63,7 @@ export = { 'Permission allows specifying multiple actions upon construction'(test: Test) { const stack = new Stack(); const perm = new PolicyStatement().addResource('MyResource').addActions('Action1', 'Action2', 'Action3'); - test.deepEqual(stack.resolve(perm), { + test.deepEqual(stack.resolve(perm.toStatementJson()), { Effect: 'Allow', Action: [ 'Action1', 'Action2', 'Action3' ], Resource: 'MyResource' }); @@ -82,7 +82,7 @@ export = { const p = new PolicyStatement(); const canoncialUser = "averysuperduperlongstringfor"; p.addPrincipal(new CanonicalUserPrincipal(canoncialUser)); - test.deepEqual(stack.resolve(p), { + test.deepEqual(stack.resolve(p.toStatementJson()), { Effect: "Allow", Principal: { CanonicalUser: canoncialUser @@ -96,7 +96,7 @@ export = { const p = new PolicyStatement(); p.addAccountRootPrincipal(); - test.deepEqual(stack.resolve(p), { + test.deepEqual(stack.resolve(p.toStatementJson()), { Effect: "Allow", Principal: { AWS: { @@ -120,7 +120,7 @@ export = { const stack = new Stack(); const p = new PolicyStatement(); p.addFederatedPrincipal("com.amazon.cognito", { StringEquals: { key: 'value' }}); - test.deepEqual(stack.resolve(p), { + test.deepEqual(stack.resolve(p.toStatementJson()), { Effect: "Allow", Principal: { Federated: "com.amazon.cognito" @@ -138,7 +138,7 @@ export = { const p = new PolicyStatement(); p.addAwsAccountPrincipal('1234'); p.addAwsAccountPrincipal('5678'); - test.deepEqual(stack.resolve(p), { + test.deepEqual(stack.resolve(p.toStatementJson()), { Effect: 'Allow', Principal: { AWS: [ @@ -181,7 +181,7 @@ export = { 'true if there is a principal'(test: Test) { const p = new PolicyStatement(); - p.addAwsPrincipal('bla'); + p.addArnPrincipal('bla'); test.equal(p.hasPrincipal, true); test.done(); } @@ -244,26 +244,6 @@ export = { } }, - 'addAwsPrincipal/addArnPrincipal are the aliases'(test: Test) { - const stack = new Stack(); - const p = new PolicyDocument(); - - p.addStatements(new PolicyStatement().addAwsPrincipal('111222-A')); - p.addStatements(new PolicyStatement().addArnPrincipal('111222-B')); - p.addStatements(new PolicyStatement().addPrincipal(new ArnPrincipal('111222-C'))); - - test.deepEqual(stack.resolve(p), { - Statement: [ { - Effect: 'Allow', Principal: { AWS: '111222-A' } }, - { Effect: 'Allow', Principal: { AWS: '111222-B' } }, - { Effect: 'Allow', Principal: { AWS: '111222-C' } } - ], - Version: '2012-10-17' - }); - - test.done(); - }, - 'addResources() will not break a list-encoded Token'(test: Test) { const stack = new Stack(); @@ -271,7 +251,7 @@ export = { .addActions(...Lazy.listValue({ produce: () => ['a', 'b', 'c'] })) .addResources(...Lazy.listValue({ produce: () => ['x', 'y', 'z'] })); - test.deepEqual(stack.resolve(statement), { + test.deepEqual(stack.resolve(statement.toStatementJson()), { Effect: 'Allow', Action: ['a', 'b', 'c'], Resource: ['x', 'y', 'z'], @@ -308,7 +288,7 @@ export = { }; const s = new PolicyStatement().addAccountRootPrincipal() .addPrincipal(arrayPrincipal); - test.deepEqual(stack.resolve(s), { + test.deepEqual(stack.resolve(s.toStatementJson()), { Effect: 'Allow', Principal: { AWS: [ @@ -324,12 +304,12 @@ export = { 'policy statements with multiple principal types can be created using multiple addPrincipal calls'(test: Test) { const stack = new Stack(); const s = new PolicyStatement() - .addAwsPrincipal('349494949494') + .addArnPrincipal('349494949494') .addServicePrincipal('test.service') .addResource('resource') .addAction('action'); - test.deepEqual(stack.resolve(s), { + test.deepEqual(stack.resolve(s.toStatementJson()), { Action: 'action', Effect: 'Allow', Principal: { AWS: '349494949494', Service: 'test.service' }, @@ -346,7 +326,7 @@ export = { .addAction('test:Action') .addServicePrincipal('codedeploy.amazonaws.com'); - test.deepEqual(stack.resolve(s), { + test.deepEqual(stack.resolve(s.toStatementJson()), { Effect: 'Allow', Action: 'test:Action', Principal: { Service: 'codedeploy.cn-north-1.amazonaws.com.cn' } @@ -361,7 +341,7 @@ export = { .addAction('test:Action') .addServicePrincipal('codedeploy.amazonaws.com', { region: 'cn-north-1' }); - test.deepEqual(stack.resolve(s), { + test.deepEqual(stack.resolve(s.toStatementJson()), { Effect: 'Allow', Action: 'test:Action', Principal: { Service: 'codedeploy.cn-north-1.amazonaws.com.cn' } @@ -376,7 +356,7 @@ export = { .addAction('test:Action') .addServicePrincipal('test.service-principal.dev'); - test.deepEqual(stack.resolve(s), { + test.deepEqual(stack.resolve(s.toStatementJson()), { Effect: 'Allow', Action: 'test:Action', Principal: { Service: 'test.service-principal.dev' } @@ -392,7 +372,7 @@ export = { const stack = new Stack(); const p = new CompositePrincipal(new ArnPrincipal('i:am:an:arn')); const statement = new PolicyStatement().addPrincipal(p); - test.deepEqual(stack.resolve(statement), { Effect: 'Allow', Principal: { AWS: 'i:am:an:arn' } }); + test.deepEqual(stack.resolve(statement.toStatementJson()), { Effect: 'Allow', Principal: { AWS: 'i:am:an:arn' } }); test.done(); }, @@ -420,10 +400,10 @@ export = { const statement = new PolicyStatement().addPrincipal(p); // add via policy statement - statement.addAwsPrincipal('aws-principal-3'); + statement.addArnPrincipal('aws-principal-3'); statement.addCondition('cond2', { boom: 123 }); - test.deepEqual(stack.resolve(statement), { + test.deepEqual(stack.resolve(statement.toStatementJson()), { Condition: { cond2: { boom: 123 } }, @@ -524,5 +504,30 @@ export = { ], }); test.done(); - } -}; + }, + + 'fromAttributes is equivalent'(test: Test) { + const stack = new Stack(); + + const doc1 = new PolicyDocument(); + doc1.addStatements(new PolicyStatement() + .addActions('action1', 'action2') + .addAllResources() + .addArnPrincipal('arn') + .addCondition('key', { equals: 'value' })); + + const doc2 = new PolicyDocument(); + doc2.addStatements(PolicyStatement.fromAttributes({ + actions: ['action1', 'action2'], + resourceArns: ['*'], + principals: [new ArnPrincipal('arn')], + conditions: { + key: { equals: 'value' } + } + })); + + test.deepEqual(stack.resolve(doc1), stack.resolve(doc2)); + + test.done(); + }, +}; \ No newline at end of file diff --git a/packages/@aws-cdk/aws-kms/test/integ.key.ts b/packages/@aws-cdk/aws-kms/test/integ.key.ts index a1dcfd66a757a..c212903dfefa1 100644 --- a/packages/@aws-cdk/aws-kms/test/integ.key.ts +++ b/packages/@aws-cdk/aws-kms/test/integ.key.ts @@ -11,7 +11,7 @@ const key = new Key(stack, 'MyKey', { retain: false }); key.addToResourcePolicy(new PolicyStatement() .addAllResources() .addAction('kms:encrypt') - .addAwsPrincipal(stack.accountId)); + .addArnPrincipal(stack.accountId)); key.addAlias('alias/bar'); diff --git a/packages/@aws-cdk/aws-kms/test/test.key.ts b/packages/@aws-cdk/aws-kms/test/test.key.ts index 4923791b90093..b8cef36985346 100644 --- a/packages/@aws-cdk/aws-kms/test/test.key.ts +++ b/packages/@aws-cdk/aws-kms/test/test.key.ts @@ -80,7 +80,7 @@ export = { const key = new Key(stack, 'MyKey'); const p = new PolicyStatement().addAllResources().addAction('kms:encrypt'); - p.addAwsPrincipal('arn'); + p.addArnPrincipal('arn'); key.addToResourcePolicy(p); expect(stack).to(exactlyMatchTemplate({ @@ -154,7 +154,7 @@ export = { enabled: false, }); const p = new PolicyStatement().addAllResources().addAction('kms:encrypt'); - p.addAwsPrincipal('arn'); + p.addArnPrincipal('arn'); key.addToResourcePolicy(p); key.node.applyAspect(new Tag('tag1', 'value1')); From 5d9ba0fea97443422100248b971e497a10eea34a Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Thu, 13 Jun 2019 14:05:14 +0200 Subject: [PATCH 04/12] Updating all the call sites --- .../test/test.pipeline-deploy-stack-action.ts | 9 +- .../assets-docker/lib/adopted-repository.ts | 9 +- .../assets-docker/test/test.image-asset.ts | 7 +- .../@aws-cdk/aws-apigateway/lib/restapi.ts | 9 +- .../test/test.auto-scaling-group.ts | 7 +- .../test/test.lifecyclehooks.ts | 7 +- .../lib/dns-validated-certificate.ts | 27 ++- .../lib/aws-custom-resource.ts | 9 +- .../test/test.aws-custom-resource.ts | 7 +- .../test/integ.cloudfront-s3.ts | 10 +- packages/@aws-cdk/aws-cloudtrail/lib/index.ts | 31 ++-- .../@aws-cdk/aws-codebuild/lib/project.ts | 67 ++++---- packages/@aws-cdk/aws-codebuild/lib/source.ts | 7 +- .../lib/lambda/deployment-group.ts | 2 +- .../lib/server/deployment-group.ts | 2 +- .../lib/cloudformation/pipeline-actions.ts | 17 +- .../lib/codebuild/build-action.ts | 9 +- .../lib/codecommit/source-action.ts | 7 +- .../lib/codedeploy/server-deploy-action.ts | 29 ++-- .../lib/ecr/source-action.ts | 9 +- .../lib/ecs/deploy-action.ts | 32 ++-- .../lib/lambda/invoke-action.ts | 22 +-- .../test.cloudformation-pipeline-actions.ts | 2 +- .../integ.pipeline-cfn-wtih-action-role.ts | 8 +- .../@aws-cdk/aws-codepipeline/lib/pipeline.ts | 7 +- .../lib/global-table-coordinator.ts | 22 +-- .../aws-ec2/test/integ.vpc-endpoint.lit.ts | 10 +- .../aws-ec2/test/test.vpc-endpoint.ts | 22 ++- .../@aws-cdk/aws-ecr/test/test.repository.ts | 2 +- packages/@aws-cdk/aws-ecs/lib/cluster.ts | 27 +-- .../lib/drain-hook/instance-drain-hook.ts | 19 ++- .../aws-events-targets/lib/codebuild.ts | 8 +- .../aws-events-targets/lib/codepipeline.ts | 7 +- .../aws-events-targets/lib/ecs-task.ts | 40 +++-- .../aws-events-targets/lib/state-machine.ts | 8 +- packages/@aws-cdk/aws-iam/lib/grant.ts | 25 +-- .../@aws-cdk/aws-iam/lib/policy-document.ts | 15 +- .../@aws-cdk/aws-iam/lib/policy-statement.ts | 157 ++++++------------ packages/@aws-cdk/aws-iam/lib/role.ts | 5 +- .../@aws-cdk/aws-iam/test/example.role.lit.ts | 6 +- .../@aws-cdk/aws-iam/test/integ.policy.ts | 4 +- packages/@aws-cdk/aws-iam/test/integ.role.ts | 4 +- .../aws-iam/test/integ.users-and-groups.ts | 5 +- .../aws-iam/test/test.policy-document.ts | 147 ++++++++-------- packages/@aws-cdk/aws-iam/test/test.policy.ts | 16 +- packages/@aws-cdk/aws-iam/test/test.role.ts | 2 +- packages/@aws-cdk/aws-kms/lib/key.ts | 9 +- packages/@aws-cdk/aws-kms/test/integ.key.ts | 10 +- packages/@aws-cdk/aws-kms/test/test.key.ts | 8 +- .../test/test.via-service-principal.ts | 18 +- packages/@aws-cdk/aws-lambda/lib/function.ts | 14 +- .../@aws-cdk/aws-lambda/lib/log-retention.ts | 16 +- .../@aws-cdk/aws-lambda/test/integ.lambda.ts | 5 +- .../@aws-cdk/aws-lambda/test/test.lambda.ts | 8 +- .../aws-logs/test/test.destination.ts | 5 +- packages/@aws-cdk/aws-rds/lib/instance.ts | 8 +- .../@aws-cdk/aws-s3-notifications/lib/sns.ts | 13 +- .../@aws-cdk/aws-s3-notifications/lib/sqs.ts | 10 +- .../notifications-resource-handler.ts | 7 +- packages/@aws-cdk/aws-s3/test/test.bucket.ts | 20 +-- .../aws-ses/lib/receipt-rule-action.ts | 30 ++-- .../@aws-cdk/aws-sns-subscriptions/lib/sqs.ts | 13 +- packages/@aws-cdk/aws-sns/test/test.sns.ts | 13 +- packages/@aws-cdk/aws-sqs/test/test.sqs.ts | 7 +- .../lib/invoke-function.ts | 8 +- .../lib/publish-to-topic.ts | 8 +- .../lib/run-ecs-task-base.ts | 30 ++-- .../lib/send-to-queue.ts | 8 +- .../test/test.state-machine-resources.ts | 15 +- 69 files changed, 596 insertions(+), 590 deletions(-) diff --git a/packages/@aws-cdk/app-delivery/test/test.pipeline-deploy-stack-action.ts b/packages/@aws-cdk/app-delivery/test/test.pipeline-deploy-stack-action.ts index 3dfaaa12e6391..95feb1fec7e51 100644 --- a/packages/@aws-cdk/app-delivery/test/test.pipeline-deploy-stack-action.ts +++ b/packages/@aws-cdk/app-delivery/test/test.pipeline-deploy-stack-action.ts @@ -222,8 +222,8 @@ export = nodeunit.testCase({ adminPermissions: false, }); // we might need to add permissions - deployAction.addToDeploymentRolePolicy( new iam.PolicyStatement(). - addActions( + deployAction.addToDeploymentRolePolicy(new iam.PolicyStatement({ + actions: [ 'ec2:AuthorizeSecurityGroupEgress', 'ec2:AuthorizeSecurityGroupIngress', 'ec2:DeleteSecurityGroup', @@ -231,8 +231,9 @@ export = nodeunit.testCase({ 'ec2:CreateSecurityGroup', 'ec2:RevokeSecurityGroupEgress', 'ec2:RevokeSecurityGroupIngress' - ). - addAllResources()); + ], + resources: ['*'] + })); // THEN // // there should be 3 policies 1. CodePipeline, 2. Codebuild, 3. diff --git a/packages/@aws-cdk/assets-docker/lib/adopted-repository.ts b/packages/@aws-cdk/assets-docker/lib/adopted-repository.ts index d51e1ef95e723..fc5b9310a5e09 100644 --- a/packages/@aws-cdk/assets-docker/lib/adopted-repository.ts +++ b/packages/@aws-cdk/assets-docker/lib/adopted-repository.ts @@ -41,15 +41,16 @@ export class AdoptedRepository extends ecr.RepositoryBase { timeout: 300 }); - fn.addToRolePolicy(new iam.PolicyStatement() - .addResource(ecr.Repository.arnForLocalRepository(props.repositoryName, this)) - .addActions( + fn.addToRolePolicy(new iam.PolicyStatement({ + resources: [ecr.Repository.arnForLocalRepository(props.repositoryName, this)], + actions: [ 'ecr:GetRepositoryPolicy', 'ecr:SetRepositoryPolicy', 'ecr:DeleteRepository', 'ecr:ListImages', 'ecr:BatchDeleteImage' - )); + ], + })); const adopter = new cfn.CustomResource(this, 'Resource', { resourceType: 'Custom::ECRAdoptedRepository', diff --git a/packages/@aws-cdk/assets-docker/test/test.image-asset.ts b/packages/@aws-cdk/assets-docker/test/test.image-asset.ts index 53fad403b0b2a..b6b4a2ec0a15a 100644 --- a/packages/@aws-cdk/assets-docker/test/test.image-asset.ts +++ b/packages/@aws-cdk/assets-docker/test/test.image-asset.ts @@ -112,9 +112,10 @@ export = { }); // WHEN - asset.repository.addToResourcePolicy(new iam.PolicyStatement() - .addAction('BOOM') - .addPrincipal(new iam.ServicePrincipal('test.service'))); + asset.repository.addToResourcePolicy(new iam.PolicyStatement({ + actions: ['BOOM'], + principals: [new iam.ServicePrincipal('test.service')] + })); // THEN expect(stack).to(haveResource('Custom::ECRAdoptedRepository', { diff --git a/packages/@aws-cdk/aws-apigateway/lib/restapi.ts b/packages/@aws-cdk/aws-apigateway/lib/restapi.ts index 502cbeef35c3b..baba37991b047 100644 --- a/packages/@aws-cdk/aws-apigateway/lib/restapi.ts +++ b/packages/@aws-cdk/aws-apigateway/lib/restapi.ts @@ -343,14 +343,7 @@ export class RestApi extends Resource implements IRestApi { private configureCloudWatchRole(apiResource: CfnRestApi) { const role = new iam.Role(this, 'CloudWatchRole', { assumedBy: new iam.ServicePrincipal('apigateway.amazonaws.com'), - managedPolicyArns: [ Stack.of(this).formatArn({ - service: 'iam', - region: '', - account: 'aws', - resource: 'policy', - sep: '/', - resourceName: 'service-role/AmazonAPIGatewayPushToCloudWatchLogs' - }) ] + managedPolicies: [iam.ManagedPolicy.fromAwsManagedPolicyName('service-role/AmazonAPIGatewayPushToCloudWatchLogs')], }); const resource = new CfnAccount(this, 'Account', { diff --git a/packages/@aws-cdk/aws-autoscaling/test/test.auto-scaling-group.ts b/packages/@aws-cdk/aws-autoscaling/test/test.auto-scaling-group.ts index 72fe66b8dc838..60b44894ebeee 100644 --- a/packages/@aws-cdk/aws-autoscaling/test/test.auto-scaling-group.ts +++ b/packages/@aws-cdk/aws-autoscaling/test/test.auto-scaling-group.ts @@ -231,9 +231,10 @@ export = { vpc }); - fleet.addToRolePolicy(new iam.PolicyStatement() - .addAction('test:SpecialName') - .addAllResources()); + fleet.addToRolePolicy(new iam.PolicyStatement({ + actions: ['test:SpecialName'], + resources: ['*'] + })); expect(stack).to(haveResource('AWS::IAM::Policy', { PolicyDocument: { diff --git a/packages/@aws-cdk/aws-autoscaling/test/test.lifecyclehooks.ts b/packages/@aws-cdk/aws-autoscaling/test/test.lifecyclehooks.ts index e2792d54fb682..d1b56d3328c78 100644 --- a/packages/@aws-cdk/aws-autoscaling/test/test.lifecyclehooks.ts +++ b/packages/@aws-cdk/aws-autoscaling/test/test.lifecyclehooks.ts @@ -71,9 +71,10 @@ export = { class FakeNotificationTarget implements autoscaling.ILifecycleHookTarget { public bind(_scope: Construct, lifecycleHook: autoscaling.ILifecycleHook): autoscaling.LifecycleHookTargetConfig { - lifecycleHook.role.addToPolicy(new iam.PolicyStatement() - .addAction('action:Work') - .addAllResources()); + lifecycleHook.role.addToPolicy(new iam.PolicyStatement({ + actions: ['action:Work'], + resources: ['*'] + })); return { notificationTargetArn: 'target:arn', }; } } diff --git a/packages/@aws-cdk/aws-certificatemanager/lib/dns-validated-certificate.ts b/packages/@aws-cdk/aws-certificatemanager/lib/dns-validated-certificate.ts index e329df9ec4263..25df3ea62aa29 100644 --- a/packages/@aws-cdk/aws-certificatemanager/lib/dns-validated-certificate.ts +++ b/packages/@aws-cdk/aws-certificatemanager/lib/dns-validated-certificate.ts @@ -53,21 +53,18 @@ export class DnsValidatedCertificate extends cdk.Resource implements ICertificat runtime: lambda.Runtime.NodeJS810, timeout: 15 * 60 // 15 minutes }); - requestorFunction.addToRolePolicy( - new iam.PolicyStatement() - .addActions('acm:RequestCertificate', 'acm:DescribeCertificate', 'acm:DeleteCertificate') - .addResource('*') - ); - requestorFunction.addToRolePolicy( - new iam.PolicyStatement() - .addActions('route53:GetChange') - .addResource('*') - ); - requestorFunction.addToRolePolicy( - new iam.PolicyStatement() - .addActions('route53:changeResourceRecordSets') - .addResource(`arn:aws:route53:::hostedzone/${this.hostedZoneId}`) - ); + requestorFunction.addToRolePolicy(new iam.PolicyStatement({ + actions: ['acm:RequestCertificate', 'acm:DescribeCertificate', 'acm:DeleteCertificate'], + resources: ['*'], + })); + requestorFunction.addToRolePolicy(new iam.PolicyStatement({ + actions: ['route53:GetChange'], + resources: ['*'], + })); + requestorFunction.addToRolePolicy(new iam.PolicyStatement({ + actions: ['route53:changeResourceRecordSets'], + resources: [`arn:aws:route53:::hostedzone/${this.hostedZoneId}`], + })); const certificate = new cfn.CustomResource(this, 'CertificateRequestorResource', { provider: cfn.CustomResourceProvider.lambda(requestorFunction), diff --git a/packages/@aws-cdk/aws-cloudformation/lib/aws-custom-resource.ts b/packages/@aws-cdk/aws-cloudformation/lib/aws-custom-resource.ts index 7243d6a1b1686..ce85a012b7ef7 100644 --- a/packages/@aws-cdk/aws-cloudformation/lib/aws-custom-resource.ts +++ b/packages/@aws-cdk/aws-cloudformation/lib/aws-custom-resource.ts @@ -136,11 +136,10 @@ export class AwsCustomResource extends cdk.Construct { } else { // Derive statements from AWS SDK calls for (const call of [props.onCreate, props.onUpdate, props.onDelete]) { if (call) { - provider.addToRolePolicy( - new iam.PolicyStatement() - .addAction(awsSdkToIamAction(call.service, call.action)) - .addAllResources() - ); + provider.addToRolePolicy(new iam.PolicyStatement({ + actions: [awsSdkToIamAction(call.service, call.action)], + resources: ['*'] + })); } } } diff --git a/packages/@aws-cdk/aws-cloudformation/test/test.aws-custom-resource.ts b/packages/@aws-cdk/aws-cloudformation/test/test.aws-custom-resource.ts index 43dd116a5f46e..2e3fc7f05ad86 100644 --- a/packages/@aws-cdk/aws-cloudformation/test/test.aws-custom-resource.ts +++ b/packages/@aws-cdk/aws-cloudformation/test/test.aws-custom-resource.ts @@ -134,9 +134,10 @@ export = { physicalResourceIdPath: 'ETag' }, policyStatements: [ - new iam.PolicyStatement() - .addAction('s3:PutObject') - .addResource('arn:aws:s3:::my-bucket/my-key') + new iam.PolicyStatement({ + actions: ['s3:PutObject'], + resources: ['arn:aws:s3:::my-bucket/my-key'] + }) ] }); diff --git a/packages/@aws-cdk/aws-cloudfront/test/integ.cloudfront-s3.ts b/packages/@aws-cdk/aws-cloudfront/test/integ.cloudfront-s3.ts index 194b021bf75a7..8ccc04736a3bb 100644 --- a/packages/@aws-cdk/aws-cloudfront/test/integ.cloudfront-s3.ts +++ b/packages/@aws-cdk/aws-cloudfront/test/integ.cloudfront-s3.ts @@ -21,10 +21,10 @@ const dist = new cloudfront.CloudFrontWebDistribution(stack, 'Distribution', { }, }] }); -bucket.addToResourcePolicy(new iam.PolicyStatement() - .allow() - .addActions('s3:Get*', 's3:List*') - .addResources(bucket.bucketArn, bucket.arnForObjects('*')) - .addCanonicalUserPrincipal(oai.cloudFrontOriginAccessIdentityS3CanonicalUserId)); +bucket.addToResourcePolicy(new iam.PolicyStatement({ + actions: ['s3:Get*', 's3:List*'], + resources: [bucket.bucketArn, bucket.arnForObjects('*')], + principals: [new iam.CanonicalUserPrincipal(oai.cloudFrontOriginAccessIdentityS3CanonicalUserId)] +})); new cdk.CfnOutput(stack, 'DistributionDomainName', { value: dist.domainName }); diff --git a/packages/@aws-cdk/aws-cloudtrail/lib/index.ts b/packages/@aws-cdk/aws-cloudtrail/lib/index.ts index 61349309500e0..5055e8aad3767 100644 --- a/packages/@aws-cdk/aws-cloudtrail/lib/index.ts +++ b/packages/@aws-cdk/aws-cloudtrail/lib/index.ts @@ -130,16 +130,20 @@ export class Trail extends Resource { const s3bucket = new s3.Bucket(this, 'S3', {encryption: s3.BucketEncryption.Unencrypted}); const cloudTrailPrincipal = "cloudtrail.amazonaws.com"; - s3bucket.addToResourcePolicy(new iam.PolicyStatement() - .addResource(s3bucket.bucketArn) - .addActions('s3:GetBucketAcl') - .addServicePrincipal(cloudTrailPrincipal)); - - s3bucket.addToResourcePolicy(new iam.PolicyStatement() - .addResource(s3bucket.arnForObjects(`AWSLogs/${Stack.of(this).accountId}/*`)) - .addActions("s3:PutObject") - .addServicePrincipal(cloudTrailPrincipal) - .setCondition("StringEquals", {'s3:x-amz-acl': "bucket-owner-full-control"})); + s3bucket.addToResourcePolicy(new iam.PolicyStatement({ + resources: [s3bucket.bucketArn], + actions: ['s3:GetBucketAcl'], + principals: [new iam.ServicePrincipal(cloudTrailPrincipal)] + })); + + s3bucket.addToResourcePolicy(new iam.PolicyStatement({ + resources: [s3bucket.arnForObjects(`AWSLogs/${Stack.of(this).accountId}/*`)], + actions: ["s3:PutObject"], + principals: [new iam.ServicePrincipal(cloudTrailPrincipal)], + conditions: { + StringEquals: {'s3:x-amz-acl': "bucket-owner-full-control"} + } + })); let logGroup: logs.CfnLogGroup | undefined; let logsRole: iam.IRole | undefined; @@ -150,9 +154,10 @@ export class Trail extends Resource { logsRole = new iam.Role(this, 'LogsRole', { assumedBy: new iam.ServicePrincipal(cloudTrailPrincipal) }); - logsRole.addToPolicy(new iam.PolicyStatement() - .addActions("logs:PutLogEvents", "logs:CreateLogStream") - .addResource(logGroup.logGroupArn)); + logsRole.addToPolicy(new iam.PolicyStatement({ + actions: ["logs:PutLogEvents", "logs:CreateLogStream"], + resources: [logGroup.logGroupArn], + })); } if (props.managementEvents) { const managementEvent = { diff --git a/packages/@aws-cdk/aws-codebuild/lib/project.ts b/packages/@aws-cdk/aws-codebuild/lib/project.ts index ed4974501965e..40ff7ea5fe16e 100644 --- a/packages/@aws-cdk/aws-codebuild/lib/project.ts +++ b/packages/@aws-cdk/aws-codebuild/lib/project.ts @@ -814,15 +814,10 @@ export class Project extends ProjectBase { const logGroupStarArn = `${logGroupArn}:*`; - const p = new iam.PolicyStatement(); - p.allow(); - p.addResource(logGroupArn); - p.addResource(logGroupStarArn); - p.addAction('logs:CreateLogGroup'); - p.addAction('logs:CreateLogStream'); - p.addAction('logs:PutLogEvents'); - - return p; + return new iam.PolicyStatement({ + resources: [logGroupArn, logGroupStarArn], + actions: ['logs:CreateLogGroup', 'logs:CreateLogStream', 'logs:PutLogEvents'], + }); } private renderEnvironment(env: BuildEnvironment = {}, @@ -899,26 +894,26 @@ export class Project extends ProjectBase { }); this._securityGroups = [securityGroup]; } - this.addToRoleInlinePolicy(new iam.PolicyStatement() - .addAllResources() - .addActions( - 'ec2:CreateNetworkInterface', - 'ec2:DescribeNetworkInterfaces', - 'ec2:DeleteNetworkInterface', - 'ec2:DescribeSubnets', - 'ec2:DescribeSecurityGroups', - 'ec2:DescribeDhcpOptions', - 'ec2:DescribeVpcs' - )); - this.addToRolePolicy(new iam.PolicyStatement() - .addResource(`arn:aws:ec2:${Aws.region}:${Aws.accountId}:network-interface/*`) - .addCondition('StringEquals', { - "ec2:Subnet": props.vpc - .selectSubnets(props.subnetSelection).subnetIds - .map(si => `arn:aws:ec2:${Aws.region}:${Aws.accountId}:subnet/${si}`), - "ec2:AuthorizedService": "codebuild.amazonaws.com" - }) - .addAction('ec2:CreateNetworkInterfacePermission')); + this.addToRoleInlinePolicy(new iam.PolicyStatement({ + resources: ['*'], + actions: [ + 'ec2:CreateNetworkInterface', 'ec2:DescribeNetworkInterfaces', + 'ec2:DeleteNetworkInterface', 'ec2:DescribeSubnets', + 'ec2:DescribeSecurityGroups', 'ec2:DescribeDhcpOptions', + 'ec2:DescribeVpcs'] + })); + this.addToRolePolicy(new iam.PolicyStatement({ + resources: [`arn:aws:ec2:${Aws.region}:${Aws.accountId}:network-interface/*`], + actions: ['ec2:CreateNetworkInterfacePermission'], + conditions: { + StringEquals: { + "ec2:Subnet": props.vpc + .selectSubnets(props.subnetSelection).subnetIds + .map(si => `arn:aws:ec2:${Aws.region}:${Aws.accountId}:subnet/${si}`), + "ec2:AuthorizedService": "codebuild.amazonaws.com" + } + } + })); return { vpcId: props.vpc.vpcId, subnets: props.vpc.selectSubnets(props.subnetSelection).subnetIds, @@ -1300,12 +1295,10 @@ function extendBuildSpec(buildSpec: any, extend: any) { } function ecrAccessForCodeBuildService(): iam.PolicyStatement { - return new iam.PolicyStatement() - .describe('CodeBuild') - .addServicePrincipal('codebuild.amazonaws.com') - .addActions( - 'ecr:GetDownloadUrlForLayer', - 'ecr:BatchGetImage', - 'ecr:BatchCheckLayerAvailability' - ); + const s = new iam.PolicyStatement({ + principals: [new iam.ServicePrincipal('codebuild.amazonaws.com')], + actions: ['ecr:GetDownloadUrlForLayer', 'ecr:BatchGetImage', 'ecr:BatchCheckLayerAvailability'], + }); + s.sid = 'CodeBuild'; + return s; } diff --git a/packages/@aws-cdk/aws-codebuild/lib/source.ts b/packages/@aws-cdk/aws-codebuild/lib/source.ts index 5595bcc58b065..9bd734fad4faa 100644 --- a/packages/@aws-cdk/aws-codebuild/lib/source.ts +++ b/packages/@aws-cdk/aws-codebuild/lib/source.ts @@ -466,9 +466,10 @@ export class CodeCommitSource extends GitBuildSource { */ public _bind(project: Project) { // https://docs.aws.amazon.com/codebuild/latest/userguide/setting-up.html - project.addToRolePolicy(new iam.PolicyStatement() - .addAction('codecommit:GitPull') - .addResource(this.repo.repositoryArn)); + project.addToRolePolicy(new iam.PolicyStatement({ + actions: ['codecommit:GitPull'], + resources: [this.repo.repositoryArn] + })); } protected toSourceProperty(): any { diff --git a/packages/@aws-cdk/aws-codedeploy/lib/lambda/deployment-group.ts b/packages/@aws-cdk/aws-codedeploy/lib/lambda/deployment-group.ts index ea0d6cc945294..1befdb56f5093 100644 --- a/packages/@aws-cdk/aws-codedeploy/lib/lambda/deployment-group.ts +++ b/packages/@aws-cdk/aws-codedeploy/lib/lambda/deployment-group.ts @@ -151,7 +151,7 @@ export class LambdaDeploymentGroup extends cdk.Resource implements ILambdaDeploy assumedBy: new iam.ServicePrincipal('codedeploy.amazonaws.com') }); - this.role.attachManagedPolicy('arn:aws:iam::aws:policy/service-role/AWSCodeDeployRoleForLambda'); + this.role.addManagedPolicy(iam.ManagedPolicy.fromAwsManagedPolicyName('service-role/AWSCodeDeployRoleForLambda')); const resource = new CfnDeploymentGroup(this, 'Resource', { applicationName: this.application.applicationName, diff --git a/packages/@aws-cdk/aws-codedeploy/lib/server/deployment-group.ts b/packages/@aws-cdk/aws-codedeploy/lib/server/deployment-group.ts index b5f5f5c7eb02c..53cd780aa134f 100644 --- a/packages/@aws-cdk/aws-codedeploy/lib/server/deployment-group.ts +++ b/packages/@aws-cdk/aws-codedeploy/lib/server/deployment-group.ts @@ -272,7 +272,7 @@ export class ServerDeploymentGroup extends ServerDeploymentGroupBase { this.role = props.role || new iam.Role(this, 'Role', { assumedBy: new iam.ServicePrincipal('codedeploy.amazonaws.com'), - managedPolicyArns: ['arn:aws:iam::aws:policy/service-role/AWSCodeDeployRole'], + managedPolicies: [iam.ManagedPolicy.fromAwsManagedPolicyName('service-role/AWSCodeDeployRole')], }); this._autoScalingGroups = props.autoScalingGroups || []; diff --git a/packages/@aws-cdk/aws-codepipeline-actions/lib/cloudformation/pipeline-actions.ts b/packages/@aws-cdk/aws-codepipeline-actions/lib/cloudformation/pipeline-actions.ts index 96db314388eae..683c0dc65e43f 100644 --- a/packages/@aws-cdk/aws-codepipeline-actions/lib/cloudformation/pipeline-actions.ts +++ b/packages/@aws-cdk/aws-codepipeline-actions/lib/cloudformation/pipeline-actions.ts @@ -260,7 +260,10 @@ export abstract class CloudFormationDeployAction extends CloudFormationAction { }); if (this.props.adminPermissions) { - this._deploymentRole.addToPolicy(new iam.PolicyStatement().addAction('*').addAllResources()); + this._deploymentRole.addToPolicy(new iam.PolicyStatement({ + actions: ['*'], + resources: ['*'], + })); } } @@ -448,7 +451,7 @@ class SingletonPolicy extends cdk.Construct { this.statementFor({ actions: ['cloudformation:ExecuteChangeSet'], conditions: { StringEquals: { 'cloudformation:ChangeSetName': props.changeSetName } }, - }).addResource(this.stackArnFromProps(props)); + }).addResources(this.stackArnFromProps(props)); } public grantCreateReplaceChangeSet(props: { stackName: string, changeSetName: string, region?: string }): void { @@ -460,7 +463,7 @@ class SingletonPolicy extends cdk.Construct { 'cloudformation:DescribeStacks', ], conditions: { StringEqualsIfExists: { 'cloudformation:ChangeSetName': props.changeSetName } }, - }).addResource(this.stackArnFromProps(props)); + }).addResources(this.stackArnFromProps(props)); } public grantCreateUpdateStack(props: { stackName: string, replaceOnFailure?: boolean, region?: string }): void { @@ -476,7 +479,7 @@ class SingletonPolicy extends cdk.Construct { if (props.replaceOnFailure) { actions.push('cloudformation:DeleteStack'); } - this.statementFor({ actions }).addResource(this.stackArnFromProps(props)); + this.statementFor({ actions }).addResources(this.stackArnFromProps(props)); } public grantDeleteStack(props: { stackName: string, region?: string }): void { @@ -485,17 +488,17 @@ class SingletonPolicy extends cdk.Construct { 'cloudformation:DescribeStack*', 'cloudformation:DeleteStack', ] - }).addResource(this.stackArnFromProps(props)); + }).addResources(this.stackArnFromProps(props)); } public grantPassRole(role: iam.IRole): void { - this.statementFor({ actions: ['iam:PassRole'] }).addResource(role.roleArn); + this.statementFor({ actions: ['iam:PassRole'] }).addResources(role.roleArn); } private statementFor(template: StatementTemplate): iam.PolicyStatement { const key = keyFor(template); if (!(key in this.statements)) { - this.statements[key] = new iam.PolicyStatement().addActions(...template.actions); + this.statements[key] = new iam.PolicyStatement({ actions: template.actions }); if (template.conditions) { this.statements[key].addConditions(template.conditions); } diff --git a/packages/@aws-cdk/aws-codepipeline-actions/lib/codebuild/build-action.ts b/packages/@aws-cdk/aws-codepipeline-actions/lib/codebuild/build-action.ts index 1c0c31b84379d..12950f0f64d18 100644 --- a/packages/@aws-cdk/aws-codepipeline-actions/lib/codebuild/build-action.ts +++ b/packages/@aws-cdk/aws-codepipeline-actions/lib/codebuild/build-action.ts @@ -90,13 +90,14 @@ export class CodeBuildAction extends codepipeline.Action { protected bind(info: codepipeline.ActionBind): void { // grant the Pipeline role the required permissions to this Project - info.role.addToPolicy(new iam.PolicyStatement() - .addResource(this.props.project.projectArn) - .addActions( + info.role.addToPolicy(new iam.PolicyStatement({ + resources: [this.props.project.projectArn], + actions: [ 'codebuild:BatchGetBuilds', 'codebuild:StartBuild', 'codebuild:StopBuild', - )); + ] + })); // allow the Project access to the Pipeline's artifact Bucket if (this.outputs.length > 0) { diff --git a/packages/@aws-cdk/aws-codepipeline-actions/lib/codecommit/source-action.ts b/packages/@aws-cdk/aws-codepipeline-actions/lib/codecommit/source-action.ts index 3e96e368e710b..32278f12ac0a7 100644 --- a/packages/@aws-cdk/aws-codepipeline-actions/lib/codecommit/source-action.ts +++ b/packages/@aws-cdk/aws-codepipeline-actions/lib/codecommit/source-action.ts @@ -72,8 +72,9 @@ export class CodeCommitSourceAction extends codepipeline.Action { 'codecommit:CancelUploadArchive', ]; - info.role.addToPolicy(new iam.PolicyStatement() - .addResource(this.props.repository.repositoryArn) - .addActions(...actions)); + info.role.addToPolicy(new iam.PolicyStatement({ + resources: [this.props.repository.repositoryArn], + actions + })); } } diff --git a/packages/@aws-cdk/aws-codepipeline-actions/lib/codedeploy/server-deploy-action.ts b/packages/@aws-cdk/aws-codepipeline-actions/lib/codedeploy/server-deploy-action.ts index 6cec74c77a7ef..2456af68e1dc1 100644 --- a/packages/@aws-cdk/aws-codepipeline-actions/lib/codedeploy/server-deploy-action.ts +++ b/packages/@aws-cdk/aws-codepipeline-actions/lib/codedeploy/server-deploy-action.ts @@ -41,25 +41,20 @@ export class CodeDeployServerDeployAction extends codepipeline.Action { // permissions, based on: // https://docs.aws.amazon.com/codedeploy/latest/userguide/auth-and-access-control-permissions-reference.html - info.role.addToPolicy(new iam.PolicyStatement() - .addResource(this.deploymentGroup.application.applicationArn) - .addActions( - 'codedeploy:GetApplicationRevision', - 'codedeploy:RegisterApplicationRevision', - )); + info.role.addToPolicy(new iam.PolicyStatement({ + resources: [this.deploymentGroup.application.applicationArn], + actions: ['codedeploy:GetApplicationRevision', 'codedeploy:RegisterApplicationRevision'] + })); - info.role.addToPolicy(new iam.PolicyStatement() - .addResource(this.deploymentGroup.deploymentGroupArn) - .addActions( - 'codedeploy:CreateDeployment', - 'codedeploy:GetDeployment', - )); + info.role.addToPolicy(new iam.PolicyStatement({ + resources: [this.deploymentGroup.deploymentGroupArn], + actions: ['codedeploy:CreateDeployment', 'codedeploy:GetDeployment'], + })); - info.role.addToPolicy(new iam.PolicyStatement() - .addResource(this.deploymentGroup.deploymentConfig.deploymentConfigArn) - .addActions( - 'codedeploy:GetDeploymentConfig', - )); + info.role.addToPolicy(new iam.PolicyStatement({ + resources: [this.deploymentGroup.deploymentConfig.deploymentConfigArn], + actions: ['codedeploy:GetDeploymentConfig'] + })); // grant the ASG Role permissions to read from the Pipeline Bucket for (const asg of this.deploymentGroup.autoScalingGroups || []) { diff --git a/packages/@aws-cdk/aws-codepipeline-actions/lib/ecr/source-action.ts b/packages/@aws-cdk/aws-codepipeline-actions/lib/ecr/source-action.ts index 04f927719089e..22aa8d7e80ede 100644 --- a/packages/@aws-cdk/aws-codepipeline-actions/lib/ecr/source-action.ts +++ b/packages/@aws-cdk/aws-codepipeline-actions/lib/ecr/source-action.ts @@ -53,11 +53,10 @@ export class EcrSourceAction extends codepipeline.Action { } protected bind(info: codepipeline.ActionBind): void { - info.role.addToPolicy(new iam.PolicyStatement() - .addActions( - 'ecr:DescribeImages', - ) - .addResource(this.props.repository.repositoryArn)); + info.role.addToPolicy(new iam.PolicyStatement({ + actions: ['ecr:DescribeImages'], + resources: [this.props.repository.repositoryArn] + })); this.props.repository.onCloudTrailImagePushed(info.pipeline.node.uniqueId + 'SourceEventRule', { target: new targets.CodePipeline(info.pipeline), diff --git a/packages/@aws-cdk/aws-codepipeline-actions/lib/ecs/deploy-action.ts b/packages/@aws-cdk/aws-codepipeline-actions/lib/ecs/deploy-action.ts index 15db63fcfcddc..1ae0f05333936 100644 --- a/packages/@aws-cdk/aws-codepipeline-actions/lib/ecs/deploy-action.ts +++ b/packages/@aws-cdk/aws-codepipeline-actions/lib/ecs/deploy-action.ts @@ -62,28 +62,30 @@ export class EcsDeployAction extends codepipeline.Action { protected bind(info: codepipeline.ActionBind): void { // permissions based on CodePipeline documentation: // https://docs.aws.amazon.com/codepipeline/latest/userguide/how-to-custom-role.html#how-to-update-role-new-services - info.role.addToPolicy(new iam.PolicyStatement() - .addActions( + info.role.addToPolicy(new iam.PolicyStatement({ + actions: [ 'ecs:DescribeServices', 'ecs:DescribeTaskDefinition', 'ecs:DescribeTasks', 'ecs:ListTasks', 'ecs:RegisterTaskDefinition', 'ecs:UpdateService', - ) - .addAllResources()); + ], + resources: ['*'] + })); - info.role.addToPolicy(new iam.PolicyStatement() - .addActions( - 'iam:PassRole', - ) - .addAllResources() - .addCondition('StringEqualsIfExists', { - 'iam:PassedToService': [ - 'ec2.amazonaws.com', - 'ecs-tasks.amazonaws.com', - ], - })); + info.role.addToPolicy(new iam.PolicyStatement({ + actions: ['iam:PassRole'], + resources: ['*'], + conditions: { + StringEqualsIfExists: { + 'iam:PassedToService': [ + 'ec2.amazonaws.com', + 'ecs-tasks.amazonaws.com', + ], + } + } + })); } } diff --git a/packages/@aws-cdk/aws-codepipeline-actions/lib/lambda/invoke-action.ts b/packages/@aws-cdk/aws-codepipeline-actions/lib/lambda/invoke-action.ts index ca982fcb7c386..4e248b9c76e42 100644 --- a/packages/@aws-cdk/aws-codepipeline-actions/lib/lambda/invoke-action.ts +++ b/packages/@aws-cdk/aws-codepipeline-actions/lib/lambda/invoke-action.ts @@ -78,21 +78,23 @@ export class LambdaInvokeAction extends codepipeline.Action { protected bind(info: codepipeline.ActionBind): void { // allow pipeline to list functions - info.role.addToPolicy(new iam.PolicyStatement() - .addAction('lambda:ListFunctions') - .addAllResources()); + info.role.addToPolicy(new iam.PolicyStatement({ + actions: ['lambda:ListFunctions'], + resources: ['*'] + })); // allow pipeline to invoke this lambda functionn - info.role.addToPolicy(new iam.PolicyStatement() - .addAction('lambda:InvokeFunction') - .addResource(this.props.lambda.functionArn)); + info.role.addToPolicy(new iam.PolicyStatement({ + actions: ['lambda:InvokeFunction'], + resources: [this.props.lambda.functionArn] + })); // allow lambda to put job results for this pipeline // CodePipeline requires this to be granted to '*' // (the Pipeline ARN will not be enough) - this.props.lambda.addToRolePolicy(new iam.PolicyStatement() - .addAllResources() - .addAction('codepipeline:PutJobSuccessResult') - .addAction('codepipeline:PutJobFailureResult')); + this.props.lambda.addToRolePolicy(new iam.PolicyStatement({ + resources: ['*'], + actions: ['codepipeline:PutJobSuccessResult', 'codepipeline:PutJobFailureResult'] + })); } } diff --git a/packages/@aws-cdk/aws-codepipeline-actions/test/cloudformation/test.cloudformation-pipeline-actions.ts b/packages/@aws-cdk/aws-codepipeline-actions/test/cloudformation/test.cloudformation-pipeline-actions.ts index f9fcedf104f00..bfdecd46ea297 100644 --- a/packages/@aws-cdk/aws-codepipeline-actions/test/cloudformation/test.cloudformation-pipeline-actions.ts +++ b/packages/@aws-cdk/aws-codepipeline-actions/test/cloudformation/test.cloudformation-pipeline-actions.ts @@ -58,7 +58,7 @@ export = { /** Deploy! */ // To execute a change set - yes, you probably do need *:* 🤷‍♀️ - changeSetExecRole.addToPolicy(new PolicyStatement().addAllResources().addAction("*")); + changeSetExecRole.addToPolicy(new PolicyStatement({ resources: ['*'], actions: ['*'] })); const stackName = 'BrelandsStack'; const changeSetName = 'MyMagicalChangeSet'; diff --git a/packages/@aws-cdk/aws-codepipeline-actions/test/integ.pipeline-cfn-wtih-action-role.ts b/packages/@aws-cdk/aws-codepipeline-actions/test/integ.pipeline-cfn-wtih-action-role.ts index d71799cb93f78..448e652bbfc85 100644 --- a/packages/@aws-cdk/aws-codepipeline-actions/test/integ.pipeline-cfn-wtih-action-role.ts +++ b/packages/@aws-cdk/aws-codepipeline-actions/test/integ.pipeline-cfn-wtih-action-role.ts @@ -28,10 +28,10 @@ const sourceStage = { const role = new iam.Role(stack, 'ActionRole', { assumedBy: new iam.AccountPrincipal(cdk.Aws.accountId) }); -role.addToPolicy(new iam.PolicyStatement() - .addAction('sqs:*') - .addAllResources() -); +role.addToPolicy(new iam.PolicyStatement({ + actions: ['sqs:*'], + resources: ['*'] +})); const cfnStage = { name: 'CFN', actions: [ diff --git a/packages/@aws-cdk/aws-codepipeline/lib/pipeline.ts b/packages/@aws-cdk/aws-codepipeline/lib/pipeline.ts index 966d70cae181f..bf0836d227a0a 100644 --- a/packages/@aws-cdk/aws-codepipeline/lib/pipeline.ts +++ b/packages/@aws-cdk/aws-codepipeline/lib/pipeline.ts @@ -445,9 +445,10 @@ export class Pipeline extends PipelineBase { // the pipeline role needs assumeRole permissions to the action role if (actionRole) { - this.role.addToPolicy(new iam.PolicyStatement() - .addAction('sts:AssumeRole') - .addResource(actionRole.roleArn)); + this.role.addToPolicy(new iam.PolicyStatement({ + actions: ['sts:AssumeRole'], + resources: [actionRole.roleArn] + })); } return actionRole; diff --git a/packages/@aws-cdk/aws-dynamodb-global/lib/global-table-coordinator.ts b/packages/@aws-cdk/aws-dynamodb-global/lib/global-table-coordinator.ts index 5747247d6d1b9..8bc0e10e04f4f 100644 --- a/packages/@aws-cdk/aws-dynamodb-global/lib/global-table-coordinator.ts +++ b/packages/@aws-cdk/aws-dynamodb-global/lib/global-table-coordinator.ts @@ -40,16 +40,16 @@ export class GlobalTableCoordinator extends cdk.Stack { */ function grantCreateGlobalTableLambda(principal?: iam.IPrincipal): void { if (principal) { - principal.addToPolicy(new iam.PolicyStatement() - .allow() - .addAllResources() - .addAction("iam:CreateServiceLinkedRole") - .addAction("application-autoscaling:DeleteScalingPolicy") - .addAction("application-autoscaling:DeregisterScalableTarget") - .addAction("dynamodb:CreateGlobalTable") - .addAction("dynamodb:DescribeLimits") - .addAction("dynamodb:DeleteTable") - .addAction("dynamodb:DescribeGlobalTable") - .addAction("dynamodb:UpdateGlobalTable")); + principal.addToPolicy(new iam.PolicyStatement({ + resources: ['*'], + actions: [ + "iam:CreateServiceLinkedRole", + "application-autoscaling:DeleteScalingPolicy", + "application-autoscaling:DeregisterScalableTarget", + "dynamodb:CreateGlobalTable", "dynamodb:DescribeLimits", + "dynamodb:DeleteTable", "dynamodb:DescribeGlobalTable", + "dynamodb:UpdateGlobalTable", + ] + })); } } diff --git a/packages/@aws-cdk/aws-ec2/test/integ.vpc-endpoint.lit.ts b/packages/@aws-cdk/aws-ec2/test/integ.vpc-endpoint.lit.ts index b558dfce5bd64..71041a79c6d0e 100644 --- a/packages/@aws-cdk/aws-ec2/test/integ.vpc-endpoint.lit.ts +++ b/packages/@aws-cdk/aws-ec2/test/integ.vpc-endpoint.lit.ts @@ -25,11 +25,11 @@ class VpcEndpointStack extends cdk.Stack { // This allows to customize the endpoint policy dynamoDbEndpoint.addToPolicy( - new iam.PolicyStatement() // Restrict to listing and describing tables - .addAnyPrincipal() - .addActions('dynamodb:DescribeTable', 'dynamodb:ListTables') - .addAllResources() - ); + new iam.PolicyStatement({ // Restrict to listing and describing tables + principals: [new iam.AnyPrincipal()], + actions: ['dynamodb:DescribeTable', 'dynamodb:ListTables'], + resources: ['*'], + })); // Add an interface endpoint const ecrDockerEndpoint = vpc.addInterfaceEndpoint('EcrDockerEndpoint', { diff --git a/packages/@aws-cdk/aws-ec2/test/test.vpc-endpoint.ts b/packages/@aws-cdk/aws-ec2/test/test.vpc-endpoint.ts index 6eb97b3f3653b..69fd0670a8977 100644 --- a/packages/@aws-cdk/aws-ec2/test/test.vpc-endpoint.ts +++ b/packages/@aws-cdk/aws-ec2/test/test.vpc-endpoint.ts @@ -1,5 +1,5 @@ import { expect, haveResource } from '@aws-cdk/assert'; -import { PolicyStatement } from '@aws-cdk/aws-iam'; +import { AnyPrincipal, PolicyStatement } from '@aws-cdk/aws-iam'; import { Stack } from '@aws-cdk/cdk'; import { Test } from 'nodeunit'; // tslint:disable-next-line:max-line-length @@ -127,12 +127,11 @@ export = { }); // WHEN - endpoint.addToPolicy( - new PolicyStatement() - .addAnyPrincipal() - .addActions('s3:GetObject', 's3:ListBucket') - .addAllResources() - ); + endpoint.addToPolicy(new PolicyStatement({ + principals: [new AnyPrincipal()], + actions: ['s3:GetObject', 's3:ListBucket'], + resources: ['*'] + })); // THEN expect(stack).to(haveResource('AWS::EC2::VPCEndpoint', { @@ -164,11 +163,10 @@ export = { }); // THEN - test.throws(() => endpoint.addToPolicy( - new PolicyStatement() - .addActions('s3:GetObject', 's3:ListBucket') - .addAllResources() - ), /`Principal`/); + test.throws(() => endpoint.addToPolicy(new PolicyStatement({ + actions: ['s3:GetObject', 's3:ListBucket'], + resources: ['*'] + })), /`Principal`/); test.done(); }, diff --git a/packages/@aws-cdk/aws-ecr/test/test.repository.ts b/packages/@aws-cdk/aws-ecr/test/test.repository.ts index ebbaf4aa44504..dfc92cc8b2d31 100644 --- a/packages/@aws-cdk/aws-ecr/test/test.repository.ts +++ b/packages/@aws-cdk/aws-ecr/test/test.repository.ts @@ -266,7 +266,7 @@ export = { const repo = new ecr.Repository(stack, 'Repo'); // WHEN - repo.addToResourcePolicy(new iam.PolicyStatement().addAction('*')); + repo.addToResourcePolicy(new iam.PolicyStatement({ actions: ['*'] })); // THEN expect(stack).to(haveResource('AWS::ECR::Repository', { diff --git a/packages/@aws-cdk/aws-ecs/lib/cluster.ts b/packages/@aws-cdk/aws-ecs/lib/cluster.ts index cf9812bbe887a..6297269a0f94a 100644 --- a/packages/@aws-cdk/aws-ecs/lib/cluster.ts +++ b/packages/@aws-cdk/aws-ecs/lib/cluster.ts @@ -153,18 +153,21 @@ export class Cluster extends Resource implements ICluster { // ECS instances must be able to do these things // Source: https://docs.aws.amazon.com/AmazonECS/latest/developerguide/instance_IAM_role.html - autoScalingGroup.addToRolePolicy(new iam.PolicyStatement().addActions( - "ecs:CreateCluster", - "ecs:DeregisterContainerInstance", - "ecs:DiscoverPollEndpoint", - "ecs:Poll", - "ecs:RegisterContainerInstance", - "ecs:StartTelemetrySession", - "ecs:Submit*", - "ecr:GetAuthorizationToken", - "logs:CreateLogStream", - "logs:PutLogEvents" - ).addAllResources()); + autoScalingGroup.addToRolePolicy(new iam.PolicyStatement({ + actions: [ + "ecs:CreateCluster", + "ecs:DeregisterContainerInstance", + "ecs:DiscoverPollEndpoint", + "ecs:Poll", + "ecs:RegisterContainerInstance", + "ecs:StartTelemetrySession", + "ecs:Submit*", + "ecr:GetAuthorizationToken", + "logs:CreateLogStream", + "logs:PutLogEvents" + ], + resources: ['*'] + })); // 0 disables, otherwise forward to underlying implementation which picks the sane default if (options.taskDrainTimeSeconds !== 0) { diff --git a/packages/@aws-cdk/aws-ecs/lib/drain-hook/instance-drain-hook.ts b/packages/@aws-cdk/aws-ecs/lib/drain-hook/instance-drain-hook.ts index 87fd362a0eccb..a065b774dbad2 100644 --- a/packages/@aws-cdk/aws-ecs/lib/drain-hook/instance-drain-hook.ts +++ b/packages/@aws-cdk/aws-ecs/lib/drain-hook/instance-drain-hook.ts @@ -71,27 +71,30 @@ export class InstanceDrainHook extends cdk.Construct { // FIXME: These should probably be restricted usefully in some way, but I don't exactly // know how. - fn.addToRolePolicy(new iam.PolicyStatement() - .addActions( + fn.addToRolePolicy(new iam.PolicyStatement({ + actions: [ 'autoscaling:CompleteLifecycleAction', 'ec2:DescribeInstances', 'ec2:DescribeInstanceAttribute', 'ec2:DescribeInstanceStatus', 'ec2:DescribeHosts', - ) - .addAllResources()); + ], + resources: ['*'] + })); // FIXME: These should be restricted to the ECS cluster probably, but I don't exactly // know how. - fn.addToRolePolicy(new iam.PolicyStatement() - .addActions( + fn.addToRolePolicy(new iam.PolicyStatement({ + actions: [ 'ecs:ListContainerInstances', 'ecs:SubmitContainerStateChange', 'ecs:SubmitTaskStateChange', 'ecs:DescribeContainerInstances', 'ecs:UpdateContainerInstancesState', 'ecs:ListTasks', - 'ecs:DescribeTasks') - .addAllResources()); + 'ecs:DescribeTasks' + ], + resources: ['*'] + })); } } diff --git a/packages/@aws-cdk/aws-events-targets/lib/codebuild.ts b/packages/@aws-cdk/aws-events-targets/lib/codebuild.ts index be60e79d1db03..9ad4022e36350 100644 --- a/packages/@aws-cdk/aws-events-targets/lib/codebuild.ts +++ b/packages/@aws-cdk/aws-events-targets/lib/codebuild.ts @@ -17,10 +17,10 @@ export class CodeBuildProject implements events.IRuleTarget { return { id: this.project.node.id, arn: this.project.projectArn, - role: singletonEventRole(this.project, [new iam.PolicyStatement() - .addAction('codebuild:StartBuild') - .addResource(this.project.projectArn) - ]), + role: singletonEventRole(this.project, [new iam.PolicyStatement({ + actions: ['codebuild:StartBuild'], + resources: [this.project.projectArn], + })]), }; } } diff --git a/packages/@aws-cdk/aws-events-targets/lib/codepipeline.ts b/packages/@aws-cdk/aws-events-targets/lib/codepipeline.ts index 10df105a3987b..f2f7099c94a88 100644 --- a/packages/@aws-cdk/aws-events-targets/lib/codepipeline.ts +++ b/packages/@aws-cdk/aws-events-targets/lib/codepipeline.ts @@ -14,9 +14,10 @@ export class CodePipeline implements events.IRuleTarget { return { id: this.pipeline.node.id, arn: this.pipeline.pipelineArn, - role: singletonEventRole(this.pipeline, [new iam.PolicyStatement() - .addResource(this.pipeline.pipelineArn) - .addAction('codepipeline:StartPipelineExecution')]) + role: singletonEventRole(this.pipeline, [new iam.PolicyStatement({ + resources: [this.pipeline.pipelineArn], + actions: ['codepipeline:StartPipelineExecution'], + })]) }; } } diff --git a/packages/@aws-cdk/aws-events-targets/lib/ecs-task.ts b/packages/@aws-cdk/aws-events-targets/lib/ecs-task.ts index b2e8205725698..5f1b064334c9d 100644 --- a/packages/@aws-cdk/aws-events-targets/lib/ecs-task.ts +++ b/packages/@aws-cdk/aws-events-targets/lib/ecs-task.ts @@ -78,25 +78,29 @@ export class EcsTask implements events.IRuleTarget { * Allows using tasks as target of CloudWatch events */ public bind(rule: events.IRule): events.RuleTargetConfig { - const policyStatements = [new iam.PolicyStatement() - .addAction('ecs:RunTask') - .addResource(this.taskDefinition.taskDefinitionArn) - .addCondition('ArnEquals', { "ecs:cluster": this.cluster.clusterArn }) - ]; + const policyStatements = [new iam.PolicyStatement({ + actions: ['ecs:RunTask'], + resources: [this.taskDefinition.taskDefinitionArn], + conditions: { + ArnEquals: { "ecs:cluster": this.cluster.clusterArn } + } + })]; // If it so happens that a Task Execution Role was created for the TaskDefinition, // then the CloudWatch Events Role must have permissions to pass it (otherwise it doesn't). if (this.taskDefinition.executionRole !== undefined) { - policyStatements.push(new iam.PolicyStatement() - .addAction('iam:PassRole') - .addResource(this.taskDefinition.executionRole.roleArn)); + policyStatements.push(new iam.PolicyStatement({ + actions: ['iam:PassRole'], + resources: [this.taskDefinition.executionRole.roleArn], + })); } // For Fargate task we need permission to pass the task role. if (this.taskDefinition.isFargateCompatible) { - policyStatements.push(new iam.PolicyStatement() - .addAction('iam:PassRole') - .addResource(this.taskDefinition.taskRole.roleArn)); + policyStatements.push(new iam.PolicyStatement({ + actions: ['iam:PassRole'], + resources: [this.taskDefinition.taskRole.roleArn] + })); } const id = this.taskDefinition.node.id + '-on-' + this.cluster.node.id; @@ -147,12 +151,14 @@ export class EcsTask implements events.IRuleTarget { physicalResourceId: id, }, policyStatements: [ // Cannot use automatic policy statements because we need iam:PassRole - new iam.PolicyStatement() - .addAction('events:PutTargets') - .addResource(rule.ruleArn), - new iam.PolicyStatement() - .addAction('iam:PassRole') - .addResource(role.roleArn) + new iam.PolicyStatement({ + actions: ['events:PutTargets'], + resources: [rule.ruleArn], + }), + new iam.PolicyStatement({ + actions: ['iam:PassRole'], + resources: [role.roleArn], + }) ] }); } diff --git a/packages/@aws-cdk/aws-events-targets/lib/state-machine.ts b/packages/@aws-cdk/aws-events-targets/lib/state-machine.ts index 9e79811ff1ea5..2cd83b10d256e 100644 --- a/packages/@aws-cdk/aws-events-targets/lib/state-machine.ts +++ b/packages/@aws-cdk/aws-events-targets/lib/state-machine.ts @@ -31,10 +31,10 @@ export class SfnStateMachine implements events.IRuleTarget { return { id: this.machine.node.id, arn: this.machine.stateMachineArn, - role: singletonEventRole(this.machine, [new iam.PolicyStatement() - .addAction('states:StartExecution') - .addResource(this.machine.stateMachineArn) - ]), + role: singletonEventRole(this.machine, [new iam.PolicyStatement({ + actions: ['states:StartExecution'], + resources: [this.machine.stateMachineArn] + })]), input: this.props.input }; } diff --git a/packages/@aws-cdk/aws-iam/lib/grant.ts b/packages/@aws-cdk/aws-iam/lib/grant.ts index 4208ee8df9297..5b6b35feddef1 100644 --- a/packages/@aws-cdk/aws-iam/lib/grant.ts +++ b/packages/@aws-cdk/aws-iam/lib/grant.ts @@ -106,10 +106,11 @@ export class Grant { if (result.success) { return result; } - const statement = new PolicyStatement() - .addActions(...options.actions) - .addResources(...(options.resourceSelfArns || options.resourceArns)) - .addPrincipal(options.grantee!.grantPrincipal); + const statement = new PolicyStatement({ + actions: options.actions, + resources: (options.resourceSelfArns || options.resourceArns), + principals: [options.grantee!.grantPrincipal] + }); options.resource.addToResourcePolicy(statement); @@ -123,9 +124,10 @@ export class Grant { * the permissions to a present principal is not an error. */ public static addToPrincipal(options: GrantOnPrincipalOptions): Grant { - const statement = new PolicyStatement() - .addActions(...options.actions) - .addResources(...options.resourceArns); + const statement = new PolicyStatement({ + actions: options.actions, + resources: options.resourceArns + }); const addedToPrincipal = options.grantee.grantPrincipal.addToPolicy(statement); @@ -147,10 +149,11 @@ export class Grant { scope: options.resource, }); - const statement = new PolicyStatement() - .addActions(...options.actions) - .addResources(...(options.resourceSelfArns || options.resourceArns)) - .addPrincipal(options.grantee!.grantPrincipal); + const statement = new PolicyStatement({ + actions: options.actions, + resources: (options.resourceSelfArns || options.resourceArns), + principals: [options.grantee!.grantPrincipal] + }); options.resource.addToResourcePolicy(statement); diff --git a/packages/@aws-cdk/aws-iam/lib/policy-document.ts b/packages/@aws-cdk/aws-iam/lib/policy-document.ts index fd0bafe92cc80..642a13c08488c 100644 --- a/packages/@aws-cdk/aws-iam/lib/policy-document.ts +++ b/packages/@aws-cdk/aws-iam/lib/policy-document.ts @@ -1,6 +1,6 @@ import cdk = require('@aws-cdk/cdk'); import { IPostProcessor } from '@aws-cdk/cdk'; -import { IPolicyStatement } from './policy-statement'; +import { PolicyStatement } from './policy-statement'; /** * Properties for a new PolicyDocument @@ -12,17 +12,26 @@ export interface PolicyDocumentProps { * @default false */ readonly assignSids?: boolean; + + /** + * Initial statements to add to the policy document + * + * @default - No statements + */ + readonly statements?: PolicyStatement[]; } /** * A PolicyDocument is a collection of statements */ export class PolicyDocument implements cdk.IResolvable { - private readonly statements = new Array(); + private readonly statements = new Array(); private readonly autoAssignSids: boolean; constructor(props: PolicyDocumentProps = {}) { this.autoAssignSids = !!props.assignSids; + + this.addStatements(...props.statements || []); } public resolve(context: cdk.IResolveContext): any { @@ -47,7 +56,7 @@ export class PolicyDocument implements cdk.IResolvable { * * @param statement the statement to add. */ - public addStatements(...statement: IPolicyStatement[]) { + public addStatements(...statement: PolicyStatement[]) { this.statements.push(...statement); } diff --git a/packages/@aws-cdk/aws-iam/lib/policy-statement.ts b/packages/@aws-cdk/aws-iam/lib/policy-statement.ts index ec5bb844a80c8..7960a047738a6 100644 --- a/packages/@aws-cdk/aws-iam/lib/policy-statement.ts +++ b/packages/@aws-cdk/aws-iam/lib/policy-statement.ts @@ -3,58 +3,38 @@ import { AccountPrincipal, AccountRootPrincipal, Anyone, ArnPrincipal, Canonical FederatedPrincipal, IPrincipal, ServicePrincipal, ServicePrincipalOpts } from './principals'; import { mergePrincipal } from './util'; -/** - * An abstract PolicyStatement - */ -export interface IPolicyStatement { - /** - * Render the policy statement to JSON - */ - toStatementJson(): any; -} - /** * Represents a statement in an IAM policy document. */ -export class PolicyStatement implements IPolicyStatement { - public static fromAttributes(attrs: PolicyStatementAttributes): IPolicyStatement { - const st = new PolicyStatement(); - st.addActions(...attrs.actions || []); - (attrs.principals || []).forEach(p => st.addPrincipal(p)); - st.addResources(...attrs.resourceArns || []); - if (attrs.conditions !== undefined) { - st.addConditions(attrs.conditions); - } - return st; - } - +export class PolicyStatement { /** * Statement ID for this statement */ public sid?: string; + public effect: Effect; private action = new Array(); private principal: { [key: string]: any[] } = {}; private resource = new Array(); private condition: { [key: string]: any } = { }; - private effect?: PolicyStatementEffect; - constructor(effect: PolicyStatementEffect = PolicyStatementEffect.Allow) { - this.effect = effect; + constructor(props: PolicyStatementProps = {}) { + this.effect = Effect.Allow; + + this.addActions(...props.actions || []); + this.addPrincipals(...props.principals || []); + this.addResources(...props.resources || []); + if (props.conditions !== undefined) { + this.addConditions(props.conditions); + } } // // Actions // - public addAction(action: string): PolicyStatement { - this.action.push(action); - return this; - } - - public addActions(...actions: string[]): PolicyStatement { - actions.forEach(action => this.addAction(action)); - return this; + public addActions(...actions: string[]) { + this.action.push(...actions); } // @@ -68,19 +48,20 @@ export class PolicyStatement implements IPolicyStatement { return Object.keys(this.principal).length > 0; } - public addPrincipal(principal: IPrincipal): this { - const fragment = principal.policyFragment; - mergePrincipal(this.principal, fragment.principalJson); - this.addConditions(fragment.conditions); - return this; + public addPrincipals(...principals: IPrincipal[]) { + for (const principal of principals) { + const fragment = principal.policyFragment; + mergePrincipal(this.principal, fragment.principalJson); + this.addConditions(fragment.conditions); + } } - public addAwsAccountPrincipal(accountId: string): this { - return this.addPrincipal(new AccountPrincipal(accountId)); + public addAwsAccountPrincipal(accountId: string) { + this.addPrincipals(new AccountPrincipal(accountId)); } - public addArnPrincipal(arn: string): this { - return this.addPrincipal(new ArnPrincipal(arn)); + public addArnPrincipal(arn: string) { + this.addPrincipals(new ArnPrincipal(arn)); } /** @@ -89,45 +70,39 @@ export class PolicyStatement implements IPolicyStatement { * @param service the service name for which a service principal is requested (e.g: `s3.amazonaws.com`). * @param opts options for adding the service principal (such as specifying a principal in a different region) */ - public addServicePrincipal(service: string, opts?: ServicePrincipalOpts): this { - return this.addPrincipal(new ServicePrincipal(service, opts)); + public addServicePrincipal(service: string, opts?: ServicePrincipalOpts) { + this.addPrincipals(new ServicePrincipal(service, opts)); } - public addFederatedPrincipal(federated: any, conditions: {[key: string]: any}): this { - return this.addPrincipal(new FederatedPrincipal(federated, conditions)); + public addFederatedPrincipal(federated: any, conditions: {[key: string]: any}) { + this.addPrincipals(new FederatedPrincipal(federated, conditions)); } - public addAccountRootPrincipal(): this { - return this.addPrincipal(new AccountRootPrincipal()); + public addAccountRootPrincipal() { + this.addPrincipals(new AccountRootPrincipal()); } - public addCanonicalUserPrincipal(canonicalUserId: string): this { - return this.addPrincipal(new CanonicalUserPrincipal(canonicalUserId)); + public addCanonicalUserPrincipal(canonicalUserId: string) { + this.addPrincipals(new CanonicalUserPrincipal(canonicalUserId)); } - public addAnyPrincipal(): this { - return this.addPrincipal(new Anyone()); + public addAnyPrincipal() { + this.addPrincipals(new Anyone()); } // // Resources // - public addResource(arn: string): PolicyStatement { - this.resource.push(arn); - return this; + public addResources(...arns: string[]) { + this.resource.push(...arns); } /** * Adds a ``"*"`` resource to this statement. */ - public addAllResources(): PolicyStatement { - return this.addResource('*'); - } - - public addResources(...arns: string[]): PolicyStatement { - arns.forEach(r => this.addResource(r)); - return this; + public addAllResources() { + this.addResources('*'); } /** @@ -137,34 +112,6 @@ export class PolicyStatement implements IPolicyStatement { return this.resource && this.resource.length > 0; } - /** - * @deprecated Use `statement.sid = value` - */ - public describe(sid: string): PolicyStatement { - this.sid = sid; - return this; - } - - // - // Effect - // - - /** - * Sets the permission effect to allow access to resources. - */ - public allow(): PolicyStatement { - this.effect = PolicyStatementEffect.Allow; - return this; - } - - /** - * Sets the permission effect to deny access to resources. - */ - public deny(): PolicyStatement { - this.effect = PolicyStatementEffect.Deny; - return this; - } - // // Condition // @@ -172,32 +119,24 @@ export class PolicyStatement implements IPolicyStatement { /** * Add a condition to the Policy */ - public addCondition(key: string, value: any): PolicyStatement { + public addCondition(key: string, value: any) { this.condition[key] = value; - return this; } /** * Add multiple conditions to the Policy */ - public addConditions(conditions: {[key: string]: any}): PolicyStatement { + public addConditions(conditions: {[key: string]: any}) { Object.keys(conditions).map(key => { this.addCondition(key, conditions[key]); }); - return this; } /** - * Add a condition to the Policy. - * - * @deprecated For backwards compatibility. Use addCondition() instead. + * Add a condition that limits to a given account */ - public setCondition(key: string, value: any): PolicyStatement { - return this.addCondition(key, value); - } - - public limitToAccount(accountId: string): PolicyStatement { - return this.addCondition('StringEquals', { 'sts:ExternalId': accountId }); + public addAccountCondition(accountId: string) { + this.addCondition('StringEquals', { 'sts:ExternalId': accountId }); } public toStatementJson(): any { @@ -274,7 +213,7 @@ export class PolicyStatement implements IPolicyStatement { } } -export enum PolicyStatementEffect { +export enum Effect { Allow = 'Allow', Deny = 'Deny', } @@ -282,32 +221,32 @@ export enum PolicyStatementEffect { /** * Interface for creating a policy statement */ -export interface PolicyStatementAttributes { +export interface PolicyStatementProps { /** * List of actions to add to the statement * * @default - no actions */ - actions?: string[]; + readonly actions?: string[]; /** * List of principals to add to the statement * * @default - no principals */ - principals?: IPrincipal[]; + readonly principals?: IPrincipal[]; /** * Resource ARNs to add to the statement * * @default - no principals */ - resourceArns?: string[]; + readonly resources?: string[]; /** * Conditions to add to the statement * * @default - no condition */ - conditions?: {[key: string]: any}; + readonly conditions?: {[key: string]: any}; } \ No newline at end of file diff --git a/packages/@aws-cdk/aws-iam/lib/role.ts b/packages/@aws-cdk/aws-iam/lib/role.ts index c9304050c8194..22a8949df321a 100644 --- a/packages/@aws-cdk/aws-iam/lib/role.ts +++ b/packages/@aws-cdk/aws-iam/lib/role.ts @@ -320,9 +320,8 @@ export interface IRole extends IIdentity { function createAssumeRolePolicy(principal: IPrincipal, externalId?: string) { const statement = new PolicyStatement(); - statement - .addPrincipal(principal) - .addAction(principal.assumeRoleAction); + statement.addPrincipals(principal); + statement.addActions(principal.assumeRoleAction); if (externalId !== undefined) { statement.addCondition('StringEquals', { 'sts:ExternalId': externalId }); diff --git a/packages/@aws-cdk/aws-iam/test/example.role.lit.ts b/packages/@aws-cdk/aws-iam/test/example.role.lit.ts index 1edbc98df4398..16435effd71f5 100644 --- a/packages/@aws-cdk/aws-iam/test/example.role.lit.ts +++ b/packages/@aws-cdk/aws-iam/test/example.role.lit.ts @@ -10,9 +10,9 @@ export class ExampleConstruct extends cdk.Construct { assumedBy: new ServicePrincipal('sns.amazonaws.com') }); - role.addToPolicy(new PolicyStatement() - .addAllResources() - .addAction('lambda:InvokeFunction')); + role.addToPolicy(new PolicyStatement({ + resources: ['*'], + actions: ['lambda:InvokeFunction'] })); /// !hide } } diff --git a/packages/@aws-cdk/aws-iam/test/integ.policy.ts b/packages/@aws-cdk/aws-iam/test/integ.policy.ts index 0cb682080ac08..193643b3e166a 100644 --- a/packages/@aws-cdk/aws-iam/test/integ.policy.ts +++ b/packages/@aws-cdk/aws-iam/test/integ.policy.ts @@ -9,11 +9,11 @@ const stack = new Stack(app, 'aws-cdk-iam-policy'); const user = new User(stack, 'MyUser'); const policy = new Policy(stack, 'HelloPolicy', { policyName: 'Default' }); -policy.addStatements(new PolicyStatement().addResource('*').addAction('sqs:SendMessage')); +policy.addStatements(new PolicyStatement({ resources: ['*'], actions: ['sqs:SendMessage'] })); policy.attachToUser(user); const policy2 = new Policy(stack, 'GoodbyePolicy'); -policy2.addStatements(new PolicyStatement().addResource('*').addAction('lambda:InvokeFunction')); +policy2.addStatements(new PolicyStatement({ resources: ['*'], actions: ['lambda:InvokeFunction'] })); policy2.attachToUser(user); app.synth(); diff --git a/packages/@aws-cdk/aws-iam/test/integ.role.ts b/packages/@aws-cdk/aws-iam/test/integ.role.ts index cf47c170ac02b..b18b429db597f 100644 --- a/packages/@aws-cdk/aws-iam/test/integ.role.ts +++ b/packages/@aws-cdk/aws-iam/test/integ.role.ts @@ -9,10 +9,10 @@ const role = new Role(stack, 'TestRole', { assumedBy: new ServicePrincipal('sqs.amazonaws.com') }); -role.addToPolicy(new PolicyStatement().addResource('*').addAction('sqs:SendMessage')); +role.addToPolicy(new PolicyStatement({ resources: ['*'], actions: ['sqs:SendMessage'] })); const policy = new Policy(stack, 'HelloPolicy', { policyName: 'Default' }); -policy.addStatements(new PolicyStatement().addAction('ec2:*').addResource('*')); +policy.addStatements(new PolicyStatement({ actions: ['ec2:*'], resources: ['*'] })); policy.attachToRole(role); // Role with an external ID diff --git a/packages/@aws-cdk/aws-iam/test/integ.users-and-groups.ts b/packages/@aws-cdk/aws-iam/test/integ.users-and-groups.ts index e99c30c3b1eb8..b332ed31c215e 100644 --- a/packages/@aws-cdk/aws-iam/test/integ.users-and-groups.ts +++ b/packages/@aws-cdk/aws-iam/test/integ.users-and-groups.ts @@ -16,6 +16,9 @@ for (let i = 0; i < 5; ++i) { const policy = new Policy(stack, 'MyPolicy'); policy.attachToGroup(g1); -policy.addStatements(new PolicyStatement().addResource(g2.groupArn).addAction('iam:*')); +policy.addStatements(new PolicyStatement({ + resources: [g2.groupArn], + actions: ['iam:*'] +})); app.synth(); diff --git a/packages/@aws-cdk/aws-iam/test/test.policy-document.ts b/packages/@aws-cdk/aws-iam/test/test.policy-document.ts index 2c3f60b6f8484..d2f3965a9eeb3 100644 --- a/packages/@aws-cdk/aws-iam/test/test.policy-document.ts +++ b/packages/@aws-cdk/aws-iam/test/test.policy-document.ts @@ -1,6 +1,6 @@ import { Lazy, Stack, Token } from '@aws-cdk/cdk'; import { Test } from 'nodeunit'; -import { Anyone, AnyPrincipal, CanonicalUserPrincipal, IPrincipal, PolicyDocument, PolicyStatement } from '../lib'; +import { Anyone, AnyPrincipal, CanonicalUserPrincipal, Effect, IPrincipal, PolicyDocument, PolicyStatement } from '../lib'; import { ArnPrincipal, CompositePrincipal, FederatedPrincipal, PrincipalPolicyFragment, ServicePrincipal } from '../lib'; export = { @@ -8,14 +8,14 @@ export = { const stack = new Stack(); const p = new PolicyStatement(); - p.addAction('sqs:SendMessage'); + p.addActions('sqs:SendMessage'); p.addActions('dynamodb:CreateTable', 'dynamodb:DeleteTable'); - p.addResource('myQueue'); - p.addResource('yourQueue'); + p.addResources('myQueue'); + p.addResources('yourQueue'); p.addAllResources(); p.addAwsAccountPrincipal(`my${Token.asString({ account: 'account' })}name`); - p.limitToAccount('12221121221'); + p.addAccountCondition('12221121221'); test.deepEqual(stack.resolve(p.toStatementJson()), { Action: [ 'sqs:SendMessage', @@ -41,11 +41,11 @@ export = { const stack = new Stack(); const doc = new PolicyDocument(); const p1 = new PolicyStatement(); - p1.addAction('sqs:SendMessage'); - p1.addResource('*'); + p1.addActions('sqs:SendMessage'); + p1.addResources('*'); const p2 = new PolicyStatement(); - p2.deny(); + p2.effect = Effect.Deny; p2.addActions('cloudformation:CreateStack'); doc.addStatements(p1); @@ -62,7 +62,10 @@ export = { 'Permission allows specifying multiple actions upon construction'(test: Test) { const stack = new Stack(); - const perm = new PolicyStatement().addResource('MyResource').addActions('Action1', 'Action2', 'Action3'); + const perm = new PolicyStatement(); + perm.addResources('MyResource'); + perm.addActions('Action1', 'Action2', 'Action3'); + test.deepEqual(stack.resolve(perm.toStatementJson()), { Effect: 'Allow', Action: [ 'Action1', 'Action2', 'Action3' ], @@ -81,7 +84,7 @@ export = { const stack = new Stack(); const p = new PolicyStatement(); const canoncialUser = "averysuperduperlongstringfor"; - p.addPrincipal(new CanonicalUserPrincipal(canoncialUser)); + p.addPrincipals(new CanonicalUserPrincipal(canoncialUser)); test.deepEqual(stack.resolve(p.toStatementJson()), { Effect: "Allow", Principal: { @@ -158,7 +161,7 @@ export = { 'true if there is one resource'(test: Test) { test.equal( - new PolicyStatement().addResource('one-resource').hasResource, + new PolicyStatement({ resources: ['one-resource'] }).hasResource, true, 'hasResource is true when there is one resource'); test.done(); @@ -166,8 +169,8 @@ export = { 'true for multiple resources'(test: Test) { const p = new PolicyStatement(); - p.addResource('r1'); - p.addResource('r2'); + p.addResources('r1'); + p.addResources('r2'); test.equal(p.hasResource, true, 'hasResource is true when there are multiple resource'); test.done(); }, @@ -190,9 +193,9 @@ export = { 'statementCount returns the number of statement in the policy document'(test: Test) { const p = new PolicyDocument(); test.equal(p.statementCount, 0); - p.addStatements(new PolicyStatement().addAction('action1')); + p.addStatements(new PolicyStatement({ actions: ['action1'] })); test.equal(p.statementCount, 1); - p.addStatements(new PolicyStatement().addAction('action2')); + p.addStatements(new PolicyStatement({ actions: ['action2'] })); test.equal(p.statementCount, 2); test.done(); }, @@ -202,7 +205,7 @@ export = { const stack = new Stack(); const p = new PolicyDocument(); - p.addStatements(new PolicyStatement().addPrincipal(new Anyone())); + p.addStatements(new PolicyStatement({ principals: [new Anyone()] })); test.deepEqual(stack.resolve(p), { Statement: [ @@ -217,7 +220,7 @@ export = { const stack = new Stack(); const p = new PolicyDocument(); - p.addStatements(new PolicyStatement().addPrincipal(new AnyPrincipal())); + p.addStatements(new PolicyStatement({ principals: [new AnyPrincipal()] })); test.deepEqual(stack.resolve(p), { Statement: [ @@ -232,7 +235,9 @@ export = { const stack = new Stack(); const p = new PolicyDocument(); - p.addStatements(new PolicyStatement().addAnyPrincipal()); + const s = new PolicyStatement(); + s.addAnyPrincipal(); + p.addStatements(s); test.deepEqual(stack.resolve(p), { Statement: [ @@ -247,9 +252,9 @@ export = { 'addResources() will not break a list-encoded Token'(test: Test) { const stack = new Stack(); - const statement = new PolicyStatement() - .addActions(...Lazy.listValue({ produce: () => ['a', 'b', 'c'] })) - .addResources(...Lazy.listValue({ produce: () => ['x', 'y', 'z'] })); + const statement = new PolicyStatement(); + statement.addActions(...Lazy.listValue({ produce: () => ['a', 'b', 'c'] })); + statement.addResources(...Lazy.listValue({ produce: () => ['x', 'y', 'z'] })); test.deepEqual(stack.resolve(statement.toStatementJson()), { Effect: 'Allow', @@ -264,8 +269,14 @@ export = { const stack = new Stack(); const p = new PolicyDocument(); - p.addStatements(new PolicyStatement().addCanonicalUserPrincipal('cannonical-user-1')); - p.addStatements(new PolicyStatement().addPrincipal(new CanonicalUserPrincipal('cannonical-user-2'))); + const s1 = new PolicyStatement(); + s1.addCanonicalUserPrincipal('cannonical-user-1'); + + const s2 = new PolicyStatement(); + s2.addPrincipals(new CanonicalUserPrincipal('cannonical-user-2')); + + p.addStatements(s1); + p.addStatements(s2); test.deepEqual(stack.resolve(p), { Statement: [ @@ -286,8 +297,9 @@ export = { policyFragment: new PrincipalPolicyFragment({ AWS: ['foo', 'bar'] }), addToPolicy() { return false; } }; - const s = new PolicyStatement().addAccountRootPrincipal() - .addPrincipal(arrayPrincipal); + const s = new PolicyStatement(); + s.addAccountRootPrincipal(); + s.addPrincipals(arrayPrincipal); test.deepEqual(stack.resolve(s.toStatementJson()), { Effect: 'Allow', Principal: { @@ -303,11 +315,11 @@ export = { // https://github.com/awslabs/aws-cdk/issues/1201 'policy statements with multiple principal types can be created using multiple addPrincipal calls'(test: Test) { const stack = new Stack(); - const s = new PolicyStatement() - .addArnPrincipal('349494949494') - .addServicePrincipal('test.service') - .addResource('resource') - .addAction('action'); + const s = new PolicyStatement(); + s.addArnPrincipal('349494949494'); + s.addServicePrincipal('test.service'); + s.addResources('resource'); + s.addActions('action'); test.deepEqual(stack.resolve(s.toStatementJson()), { Action: 'action', @@ -322,9 +334,9 @@ export = { 'Service principals': { 'regional service principals resolve appropriately'(test: Test) { const stack = new Stack(undefined, undefined, { env: { region: 'cn-north-1' } }); - const s = new PolicyStatement() - .addAction('test:Action') - .addServicePrincipal('codedeploy.amazonaws.com'); + const s = new PolicyStatement(); + s.addActions('test:Action'); + s.addServicePrincipal('codedeploy.amazonaws.com'); test.deepEqual(stack.resolve(s.toStatementJson()), { Effect: 'Allow', @@ -337,9 +349,9 @@ export = { 'regional service principals resolve appropriately (with user-set region)'(test: Test) { const stack = new Stack(undefined, undefined, { env: { region: 'cn-northeast-1' } }); - const s = new PolicyStatement() - .addAction('test:Action') - .addServicePrincipal('codedeploy.amazonaws.com', { region: 'cn-north-1' }); + const s = new PolicyStatement(); + s.addActions('test:Action'); + s.addServicePrincipal('codedeploy.amazonaws.com', { region: 'cn-north-1' }); test.deepEqual(stack.resolve(s.toStatementJson()), { Effect: 'Allow', @@ -352,9 +364,9 @@ export = { 'obscure service principals resolve to the user-provided value'(test: Test) { const stack = new Stack(undefined, undefined, { env: { region: 'cn-north-1' } }); - const s = new PolicyStatement() - .addAction('test:Action') - .addServicePrincipal('test.service-principal.dev'); + const s = new PolicyStatement(); + s.addActions('test:Action'); + s.addServicePrincipal('test.service-principal.dev'); test.deepEqual(stack.resolve(s.toStatementJson()), { Effect: 'Allow', @@ -371,7 +383,8 @@ export = { 'with a single principal'(test: Test) { const stack = new Stack(); const p = new CompositePrincipal(new ArnPrincipal('i:am:an:arn')); - const statement = new PolicyStatement().addPrincipal(p); + const statement = new PolicyStatement(); + statement.addPrincipals(p); test.deepEqual(stack.resolve(statement.toStatementJson()), { Effect: 'Allow', Principal: { AWS: 'i:am:an:arn' } }); test.done(); }, @@ -397,7 +410,8 @@ export = { new ServicePrincipal('another.service') ); - const statement = new PolicyStatement().addPrincipal(p); + const statement = new PolicyStatement(); + statement.addPrincipals(p); // add via policy statement statement.addArnPrincipal('aws-principal-3'); @@ -435,11 +449,11 @@ export = { const stack = new Stack(); const p = new PolicyDocument(); - const statement = new PolicyStatement() - .addResources('resource1', 'resource2') - .addActions('action1', 'action2') - .addServicePrincipal('service') - .addConditions({ + const statement = new PolicyStatement(); + statement.addResources('resource1', 'resource2'); + statement.addActions('action1', 'action2'); + statement.addServicePrincipal('service'); + statement.addConditions({ a: { b: 'c' }, @@ -463,12 +477,13 @@ export = { const stack = new Stack(); const p = new PolicyDocument(); - const statement1 = new PolicyStatement() - .addResource(Lazy.stringValue({ produce: () => 'resource' })) - .addAction(Lazy.stringValue({ produce: () => 'action' })); - const statement2 = new PolicyStatement() - .addResource(Lazy.stringValue({ produce: () => 'resource' })) - .addAction(Lazy.stringValue({ produce: () => 'action' })); + const statement1 = new PolicyStatement(); + statement1.addResources(Lazy.stringValue({ produce: () => 'resource' })); + statement1.addActions(Lazy.stringValue({ produce: () => 'action' })); + + const statement2 = new PolicyStatement(); + statement2.addResources(Lazy.stringValue({ produce: () => 'resource' })); + statement2.addActions(Lazy.stringValue({ produce: () => 'action' })); // WHEN p.addStatements(statement1); @@ -488,11 +503,11 @@ export = { }); // WHEN - doc.addStatements(new PolicyStatement().addAction('action1').addResource('resource1')); - doc.addStatements(new PolicyStatement().addAction('action1').addResource('resource1')); - doc.addStatements(new PolicyStatement().addAction('action1').addResource('resource1')); - doc.addStatements(new PolicyStatement().addAction('action1').addResource('resource1')); - doc.addStatements(new PolicyStatement().addAction('action2').addResource('resource2')); + doc.addStatements(new PolicyStatement({ actions: ['action1'], resources: ['resource1']})); + doc.addStatements(new PolicyStatement({ actions: ['action1'], resources: ['resource1']})); + doc.addStatements(new PolicyStatement({ actions: ['action1'], resources: ['resource1']})); + doc.addStatements(new PolicyStatement({ actions: ['action1'], resources: ['resource1']})); + doc.addStatements(new PolicyStatement({ actions: ['action2'], resources: ['resource2']})); // THEN const stack = new Stack(); @@ -506,20 +521,22 @@ export = { test.done(); }, - 'fromAttributes is equivalent'(test: Test) { + 'constructor args are equivalent to mutating in-place'(test: Test) { const stack = new Stack(); + const s = new PolicyStatement(); + s.addActions('action1', 'action2'); + s.addAllResources(); + s.addArnPrincipal('arn'); + s.addCondition('key', { equals: 'value' }); + const doc1 = new PolicyDocument(); - doc1.addStatements(new PolicyStatement() - .addActions('action1', 'action2') - .addAllResources() - .addArnPrincipal('arn') - .addCondition('key', { equals: 'value' })); + doc1.addStatements(s); const doc2 = new PolicyDocument(); - doc2.addStatements(PolicyStatement.fromAttributes({ + doc2.addStatements(new PolicyStatement({ actions: ['action1', 'action2'], - resourceArns: ['*'], + resources: ['*'], principals: [new ArnPrincipal('arn')], conditions: { key: { equals: 'value' } diff --git a/packages/@aws-cdk/aws-iam/test/test.policy.ts b/packages/@aws-cdk/aws-iam/test/test.policy.ts index e0cc671ec877f..fd2530c6ab835 100644 --- a/packages/@aws-cdk/aws-iam/test/test.policy.ts +++ b/packages/@aws-cdk/aws-iam/test/test.policy.ts @@ -19,8 +19,8 @@ export = { const stack = new Stack(app, 'MyStack'); const policy = new Policy(stack, 'MyPolicy', { policyName: 'MyPolicyName' }); - policy.addStatements(new PolicyStatement().addResource('*').addAction('sqs:SendMessage')); - policy.addStatements(new PolicyStatement().addResource('arn').addAction('sns:Subscribe')); + policy.addStatements(new PolicyStatement({ resources: ['*'], actions: ['sqs:SendMessage'] })); + policy.addStatements(new PolicyStatement({ resources: ['arn'], actions: ['sns:Subscribe'] })); const group = new Group(stack, 'MyGroup'); group.attachInlinePolicy(policy); @@ -45,8 +45,8 @@ export = { const stack = new Stack(app, 'MyStack'); const policy = new Policy(stack, 'MyPolicy'); - policy.addStatements(new PolicyStatement().addResource('*').addAction('sqs:SendMessage')); - policy.addStatements(new PolicyStatement().addResource('arn').addAction('sns:Subscribe')); + policy.addStatements(new PolicyStatement({ resources: ['*'], actions: ['sqs:SendMessage'] })); + policy.addStatements(new PolicyStatement({ resources: ['arn'], actions: ['sns:Subscribe'] })); const user = new User(stack, 'MyUser'); user.attachInlinePolicy(policy); @@ -82,7 +82,7 @@ export = { users: [ user1 ], groups: [ group1 ], roles: [ role1 ], - statements: [ new PolicyStatement().addResource('*').addAction('dynamodb:PutItem') ], + statements: [ new PolicyStatement({ resources: ['*'], actions: ['dynamodb:PutItem'] }) ], }); expect(stack).toMatch({ Resources: @@ -116,7 +116,7 @@ export = { const app = new App(); const stack = new Stack(app, 'MyStack'); const p = new Policy(stack, 'MyPolicy'); - p.addStatements(new PolicyStatement().addAction('*').addResource('*')); + p.addStatements(new PolicyStatement({ actions: ['*'], resources: ['*'] })); const user = new User(stack, 'MyUser'); p.attachToUser(user); @@ -148,7 +148,7 @@ export = { p.attachToUser(new User(stack, 'User2')); p.attachToGroup(new Group(stack, 'Group1')); p.attachToRole(new Role(stack, 'Role1', { assumedBy: new ServicePrincipal('test.service') })); - p.addStatements(new PolicyStatement().addResource('*').addAction('dynamodb:GetItem')); + p.addStatements(new PolicyStatement({ resources: ['*'], actions: ['dynamodb:GetItem'] })); expect(stack).toMatch({ Resources: { MyTestPolicy316BDB50: @@ -190,7 +190,7 @@ export = { group.attachInlinePolicy(policy); role.attachInlinePolicy(policy); - policy.addStatements(new PolicyStatement().addResource('*').addAction('*')); + policy.addStatements(new PolicyStatement({ resources: ['*'], actions: ['*'] })); expect(stack).toMatch({ Resources: { MyPolicy39D66CF6: diff --git a/packages/@aws-cdk/aws-iam/test/test.role.ts b/packages/@aws-cdk/aws-iam/test/test.role.ts index 154e1d79492ab..e992376b47e48 100644 --- a/packages/@aws-cdk/aws-iam/test/test.role.ts +++ b/packages/@aws-cdk/aws-iam/test/test.role.ts @@ -89,7 +89,7 @@ export = { // add a policy to the role const after = new Stack(); const afterRole = new Role(after, 'MyRole', { assumedBy: new ServicePrincipal('sns.amazonaws.com') }); - afterRole.addToPolicy(new PolicyStatement().addResource('myresource').addAction('myaction')); + afterRole.addToPolicy(new PolicyStatement({ resources: ['myresource'], actions: ['myaction'] })); expect(after).to(haveResource('AWS::IAM::Policy', { PolicyDocument: { Statement: [ diff --git a/packages/@aws-cdk/aws-kms/lib/key.ts b/packages/@aws-cdk/aws-kms/lib/key.ts index a3711bbc564bc..8deba0e077386 100644 --- a/packages/@aws-cdk/aws-kms/lib/key.ts +++ b/packages/@aws-cdk/aws-kms/lib/key.ts @@ -250,9 +250,10 @@ export class Key extends KeyBase { "kms:CancelKeyDeletion" ]; - this.addToResourcePolicy(new PolicyStatement() - .addAllResources() - .addActions(...actions) - .addAccountRootPrincipal()); + this.addToResourcePolicy(new PolicyStatement({ + resources: ['*'], + actions, + principals: [new iam.AccountRootPrincipal()] + })); } } diff --git a/packages/@aws-cdk/aws-kms/test/integ.key.ts b/packages/@aws-cdk/aws-kms/test/integ.key.ts index c212903dfefa1..da5d9ad15c035 100644 --- a/packages/@aws-cdk/aws-kms/test/integ.key.ts +++ b/packages/@aws-cdk/aws-kms/test/integ.key.ts @@ -1,4 +1,5 @@ import { PolicyStatement } from '@aws-cdk/aws-iam'; +import iam = require('@aws-cdk/aws-iam'); import { App, Stack } from '@aws-cdk/cdk'; import { Key } from '../lib'; @@ -8,10 +9,11 @@ const stack = new Stack(app, `aws-cdk-kms-1`); const key = new Key(stack, 'MyKey', { retain: false }); -key.addToResourcePolicy(new PolicyStatement() - .addAllResources() - .addAction('kms:encrypt') - .addArnPrincipal(stack.accountId)); +key.addToResourcePolicy(new PolicyStatement({ + resources: ['*'], + actions: ['kms:encrypt'], + principals: [new iam.ArnPrincipal(stack.accountId)], +})); key.addAlias('alias/bar'); diff --git a/packages/@aws-cdk/aws-kms/test/test.key.ts b/packages/@aws-cdk/aws-kms/test/test.key.ts index b8cef36985346..137207670e887 100644 --- a/packages/@aws-cdk/aws-kms/test/test.key.ts +++ b/packages/@aws-cdk/aws-kms/test/test.key.ts @@ -79,7 +79,7 @@ export = { const stack = new Stack(app, 'Test'); const key = new Key(stack, 'MyKey'); - const p = new PolicyStatement().addAllResources().addAction('kms:encrypt'); + const p = new PolicyStatement({ resources: ['*'], actions: ['kms:encrypt'] }); p.addArnPrincipal('arn'); key.addToResourcePolicy(p); @@ -153,7 +153,7 @@ export = { enableKeyRotation: true, enabled: false, }); - const p = new PolicyStatement().addAllResources().addAction('kms:encrypt'); + const p = new PolicyStatement({ resources: ['*'], actions: ['kms:encrypt'] }); p.addArnPrincipal('arn'); key.addToResourcePolicy(p); @@ -398,7 +398,7 @@ export = { const key = Key.fromKeyArn(stack, 'Imported', 'foo/bar'); - key.addToResourcePolicy(new PolicyStatement().addAllResources().addAction('*')); + key.addToResourcePolicy(new PolicyStatement({ resources: ['*'], actions: ['*'] })); test.done(); }, @@ -410,7 +410,7 @@ export = { const key = Key.fromKeyArn(stack, 'Imported', 'foo/bar'); test.throws(() => - key.addToResourcePolicy(new PolicyStatement().addAllResources().addAction('*'), /* allowNoOp */ false), + key.addToResourcePolicy(new PolicyStatement({ resources: ['*'], actions: ['*'] }), /* allowNoOp */ false), 'Unable to add statement to IAM resource policy for KMS key: "foo/bar"'); test.done(); diff --git a/packages/@aws-cdk/aws-kms/test/test.via-service-principal.ts b/packages/@aws-cdk/aws-kms/test/test.via-service-principal.ts index 85bdd0a6110a9..7d3cc0daab133 100644 --- a/packages/@aws-cdk/aws-kms/test/test.via-service-principal.ts +++ b/packages/@aws-cdk/aws-kms/test/test.via-service-principal.ts @@ -9,10 +9,11 @@ export = { const stack = new cdk.Stack(); // WHEN - const statement = new iam.PolicyStatement() - .addAction('abc:call') - .addPrincipal(new kms.ViaServicePrincipal('bla.amazonaws.com')) - .addResource('*'); + const statement = new iam.PolicyStatement({ + actions: ['abc:call'], + principals: [new kms.ViaServicePrincipal('bla.amazonaws.com')], + resources: ['*'] + }); // THEN test.deepEqual(stack.resolve(statement), { @@ -31,10 +32,11 @@ export = { const stack = new cdk.Stack(); // WHEN - const statement = new iam.PolicyStatement() - .addAction('abc:call') - .addPrincipal(new kms.ViaServicePrincipal('bla.amazonaws.com', new iam.OrganizationPrincipal('o-1234'))) - .addResource('*'); + const statement = new iam.PolicyStatement({ + actions: ['abc:call'], + principals: [new kms.ViaServicePrincipal('bla.amazonaws.com', new iam.OrganizationPrincipal('o-1234'))], + resources: ['*'] + }); // THEN test.deepEqual(stack.resolve(statement), { diff --git a/packages/@aws-cdk/aws-lambda/lib/function.ts b/packages/@aws-cdk/aws-lambda/lib/function.ts index 5329ad923ba0d..ad269634bca1a 100644 --- a/packages/@aws-cdk/aws-lambda/lib/function.ts +++ b/packages/@aws-cdk/aws-lambda/lib/function.ts @@ -612,9 +612,10 @@ export class Function extends FunctionBase { retentionPeriodSec: 1209600 }); - this.addToRolePolicy(new iam.PolicyStatement() - .addAction('sqs:SendMessage') - .addResource(deadLetterQueue.queueArn)); + this.addToRolePolicy(new iam.PolicyStatement({ + actions: ['sqs:SendMessage'], + resources: [deadLetterQueue.queueArn] + })); return { targetArn: deadLetterQueue.queueArn @@ -626,9 +627,10 @@ export class Function extends FunctionBase { return undefined; } - this.addToRolePolicy(new iam.PolicyStatement() - .addActions('xray:PutTraceSegments', 'xray:PutTelemetryRecords') - .addAllResources()); + this.addToRolePolicy(new iam.PolicyStatement({ + actions: ['xray:PutTraceSegments', 'xray:PutTelemetryRecords'], + resources: ['*'] + })); return { mode: Tracing[props.tracing] diff --git a/packages/@aws-cdk/aws-lambda/lib/log-retention.ts b/packages/@aws-cdk/aws-lambda/lib/log-retention.ts index 1196f04d38012..6c4bc5ced5bf4 100644 --- a/packages/@aws-cdk/aws-lambda/lib/log-retention.ts +++ b/packages/@aws-cdk/aws-lambda/lib/log-retention.ts @@ -39,14 +39,14 @@ export class LogRetention extends cdk.Construct { lambdaPurpose: 'LogRetention', }); - provider.addToRolePolicy( // Duplicate statements will be deduplicated by `PolicyDocument` - new iam.PolicyStatement() - .addActions('logs:PutRetentionPolicy', 'logs:DeleteRetentionPolicy') - // We need '*' here because we will also put a retention policy on - // the log group of the provider function. Referencing it's name - // creates a CF circular dependency. - .addAllResources() - ); + // Duplicate statements will be deduplicated by `PolicyDocument` + provider.addToRolePolicy(new iam.PolicyStatement({ + actions: ['logs:PutRetentionPolicy', 'logs:DeleteRetentionPolicy'], + // We need '*' here because we will also put a retention policy on + // the log group of the provider function. Referencing it's name + // creates a CF circular dependency. + resources: ['*'], + })); // Need to use a CfnResource here to prevent lerna dependency cycles // @aws-cdk/aws-cloudformation -> @aws-cdk/aws-lambda -> @aws-cdk/aws-cloudformation diff --git a/packages/@aws-cdk/aws-lambda/test/integ.lambda.ts b/packages/@aws-cdk/aws-lambda/test/integ.lambda.ts index 4a0830b6ddaf8..f3c754880942a 100644 --- a/packages/@aws-cdk/aws-lambda/test/integ.lambda.ts +++ b/packages/@aws-cdk/aws-lambda/test/integ.lambda.ts @@ -12,7 +12,10 @@ const fn = new lambda.Function(stack, 'MyLambda', { runtime: lambda.Runtime.NodeJS810, }); -fn.addToRolePolicy(new iam.PolicyStatement().addAllResources().addAction('*')); +fn.addToRolePolicy(new iam.PolicyStatement({ + resources: ['*'], + actions: ['*'] +})); const version = fn.addVersion('1'); diff --git a/packages/@aws-cdk/aws-lambda/test/test.lambda.ts b/packages/@aws-cdk/aws-lambda/test/test.lambda.ts index f491ff4eb101c..a724f4ab344f0 100644 --- a/packages/@aws-cdk/aws-lambda/test/test.lambda.ts +++ b/packages/@aws-cdk/aws-lambda/test/test.lambda.ts @@ -51,7 +51,7 @@ export = { code: new lambda.InlineCode('foo'), handler: 'index.handler', runtime: lambda.Runtime.NodeJS810, - initialPolicy: [new iam.PolicyStatement().addAction("*").addAllResources()] + initialPolicy: [new iam.PolicyStatement({ actions: ["*"], resources: ['*'] })], }); expect(stack).toMatch({ Resources: { MyLambdaServiceRole4539ECB6: @@ -205,7 +205,7 @@ export = { const role = new iam.Role(stack, 'SomeRole', { assumedBy: new iam.ServicePrincipal('lambda.amazonaws.com'), }); - role.addToPolicy(new iam.PolicyStatement().addAction('confirm:itsthesame')); + role.addToPolicy(new iam.PolicyStatement({ actions: ['confirm:itsthesame'] })); // WHEN const fn = new lambda.Function(stack, 'Function', { @@ -214,11 +214,11 @@ export = { handler: 'index.test', role, initialPolicy: [ - new iam.PolicyStatement().addAction('inline:inline') + new iam.PolicyStatement({ actions: ['inline:inline'] }) ] }); - fn.addToRolePolicy(new iam.PolicyStatement().addAction('explicit:explicit')); + fn.addToRolePolicy(new iam.PolicyStatement({ actions: ['explicit:explicit'] })); // THEN expect(stack).to(haveResource('AWS::IAM::Policy', { diff --git a/packages/@aws-cdk/aws-logs/test/test.destination.ts b/packages/@aws-cdk/aws-logs/test/test.destination.ts index 61d2ab1ee8402..9d42a6f3852c4 100644 --- a/packages/@aws-cdk/aws-logs/test/test.destination.ts +++ b/packages/@aws-cdk/aws-logs/test/test.destination.ts @@ -43,8 +43,9 @@ export = { }); // WHEN - dest.addToPolicy(new iam.PolicyStatement() - .addAction('logs:TalkToMe')); + dest.addToPolicy(new iam.PolicyStatement({ + actions: ['logs:TalkToMe'] + })); // THEN expect(stack).to(haveResource('AWS::Logs::Destination', (props: any) => { diff --git a/packages/@aws-cdk/aws-rds/lib/instance.ts b/packages/@aws-cdk/aws-rds/lib/instance.ts index 3ae8dd578f85e..4ef3196dac473 100644 --- a/packages/@aws-cdk/aws-rds/lib/instance.ts +++ b/packages/@aws-cdk/aws-rds/lib/instance.ts @@ -487,13 +487,7 @@ abstract class DatabaseInstanceNew extends DatabaseInstanceBase implements IData if (props.monitoringInterval) { monitoringRole = new iam.Role(this, 'MonitoringRole', { assumedBy: new iam.ServicePrincipal('monitoring.rds.amazonaws.com'), - managedPolicyArns: [Stack.of(this).formatArn({ - service: 'iam', - region: '', - account: 'aws', - resource: 'policy', - resourceName: 'service-role/AmazonRDSEnhancedMonitoringRole' - })] + managedPolicies: [iam.ManagedPolicy.fromAwsManagedPolicyName('service-role/AmazonRDSEnhancedMonitoringRole')], }); } diff --git a/packages/@aws-cdk/aws-s3-notifications/lib/sns.ts b/packages/@aws-cdk/aws-s3-notifications/lib/sns.ts index 15dd25da5e465..81a5bce07234b 100644 --- a/packages/@aws-cdk/aws-s3-notifications/lib/sns.ts +++ b/packages/@aws-cdk/aws-s3-notifications/lib/sns.ts @@ -11,11 +11,14 @@ export class SnsDestination implements s3.IBucketNotificationDestination { } public bind(_scope: Construct, bucket: s3.IBucket): s3.BucketNotificationDestinationConfig { - this.topic.addToResourcePolicy(new iam.PolicyStatement() - .addServicePrincipal('s3.amazonaws.com') - .addAction('sns:Publish') - .addResource(this.topic.topicArn) - .addCondition('ArnLike', { "aws:SourceArn": bucket.bucketArn })); + this.topic.addToResourcePolicy(new iam.PolicyStatement({ + principals: [new iam.ServicePrincipal('s3.amazonaws.com')], + actions: ['sns:Publish'], + resources: [this.topic.topicArn], + conditions: { + ArnLike: { "aws:SourceArn": bucket.bucketArn } + } + })); return { arn: this.topic.topicArn, diff --git a/packages/@aws-cdk/aws-s3-notifications/lib/sqs.ts b/packages/@aws-cdk/aws-s3-notifications/lib/sqs.ts index a3cbed991526c..19de8e6ab7db7 100644 --- a/packages/@aws-cdk/aws-s3-notifications/lib/sqs.ts +++ b/packages/@aws-cdk/aws-s3-notifications/lib/sqs.ts @@ -24,11 +24,11 @@ export class SqsDestination implements s3.IBucketNotificationDestination { // if this queue is encrypted, we need to allow S3 to read messages since that's how // it verifies that the notification destination configuration is valid. if (this.queue.encryptionMasterKey) { - this.queue.encryptionMasterKey.addToResourcePolicy(new iam.PolicyStatement() - .addServicePrincipal('s3.amazonaws.com') - .addAction('kms:GenerateDataKey*') - .addAction('kms:Decrypt') - .addAllResources(), /* allowNoOp */ false); + this.queue.encryptionMasterKey.addToResourcePolicy(new iam.PolicyStatement({ + principals: [new iam.ServicePrincipal('s3.amazonaws.com')], + actions: ['kms:GenerateDataKey*', 'kms:Decrypt'], + resources: ['*'], + }), /* allowNoOp */ false); } return { diff --git a/packages/@aws-cdk/aws-s3/lib/notifications-resource/notifications-resource-handler.ts b/packages/@aws-cdk/aws-s3/lib/notifications-resource/notifications-resource-handler.ts index 76e570d53c4d5..ac543e91dcdb4 100644 --- a/packages/@aws-cdk/aws-s3/lib/notifications-resource/notifications-resource-handler.ts +++ b/packages/@aws-cdk/aws-s3/lib/notifications-resource/notifications-resource-handler.ts @@ -56,9 +56,10 @@ export class NotificationsResourceHandler extends cdk.Construct { }); // handler allows to put bucket notification on s3 buckets. - role.addToPolicy(new iam.PolicyStatement() - .addAction('s3:PutBucketNotification') - .addAllResources()); + role.addToPolicy(new iam.PolicyStatement({ + actions: ['s3:PutBucketNotification'], + resources: ['*'] + })); const resourceType = 'AWS::Lambda::Function'; class InLineLambda extends cdk.CfnResource { diff --git a/packages/@aws-cdk/aws-s3/test/test.bucket.ts b/packages/@aws-cdk/aws-s3/test/test.bucket.ts index 1972c9e617174..5025e39ffbab7 100644 --- a/packages/@aws-cdk/aws-s3/test/test.bucket.ts +++ b/packages/@aws-cdk/aws-s3/test/test.bucket.ts @@ -429,7 +429,7 @@ export = { const stack = new cdk.Stack(); const bucket = new s3.Bucket(stack, 'MyBucket', { encryption: s3.BucketEncryption.Unencrypted }); - bucket.addToResourcePolicy(new iam.PolicyStatement().addResource('foo').addAction('bar')); + bucket.addToResourcePolicy(new iam.PolicyStatement({ resources: ['foo'], actions: [ 'bar' ]})); expect(stack).toMatch({ "Resources": { @@ -466,7 +466,7 @@ export = { const bucket = new s3.Bucket(stack, 'MyBucket', { encryption: s3.BucketEncryption.Unencrypted }); - const x = new iam.PolicyStatement().addResource(bucket.bucketArn).addAction('s3:ListBucket'); + const x = new iam.PolicyStatement({ resources: [bucket.bucketArn], actions: ['s3:ListBucket'] }); test.deepEqual(stack.resolve(x), { Action: 's3:ListBucket', @@ -482,7 +482,7 @@ export = { const bucket = new s3.Bucket(stack, 'MyBucket', { encryption: s3.BucketEncryption.Unencrypted }); - const p = new iam.PolicyStatement().addResource(bucket.arnForObjects('hello/world')).addAction('s3:GetObject'); + const p = new iam.PolicyStatement({ resources: [bucket.arnForObjects('hello/world')], actions: ['s3:GetObject'] }); test.deepEqual(stack.resolve(p), { Action: 's3:GetObject', @@ -508,7 +508,7 @@ export = { const team = new iam.Group(stack, 'MyTeam'); const resource = bucket.arnForObjects(`home/${team.groupName}/${user.userName}/*`); - const p = new iam.PolicyStatement().addResource(resource).addAction('s3:GetObject'); + const p = new iam.PolicyStatement({ resources: [resource], actions: ['s3:GetObject'] }); test.deepEqual(stack.resolve(p), { Action: 's3:GetObject', @@ -557,9 +557,9 @@ export = { const bucket = s3.Bucket.fromBucketAttributes(stack, 'ImportedBucket', { bucketArn }); // this is a no-op since the bucket is external - bucket.addToResourcePolicy(new iam.PolicyStatement().addResource('foo').addAction('bar')); + bucket.addToResourcePolicy(new iam.PolicyStatement({ resources: ['foo'], actions: ['bar']})); - const p = new iam.PolicyStatement().addResource(bucket.bucketArn).addAction('s3:ListBucket'); + const p = new iam.PolicyStatement({ resources: [bucket.bucketArn], actions: ['s3:ListBucket'] }); // it is possible to obtain a permission statement for a ref test.deepEqual(stack.resolve(p), { @@ -578,7 +578,7 @@ export = { 'import can also be used to import arbitrary ARNs'(test: Test) { const stack = new cdk.Stack(); const bucket = s3.Bucket.fromBucketAttributes(stack, 'ImportedBucket', { bucketArn: 'arn:aws:s3:::my-bucket' }); - bucket.addToResourcePolicy(new iam.PolicyStatement().addAllResources().addAction('*')); + bucket.addToResourcePolicy(new iam.PolicyStatement({ resources: ['*'], actions: ['*'] })); // at this point we technically didn't create any resources in the consuming stack. expect(stack).toMatch({}); @@ -586,9 +586,9 @@ export = { // but now we can reference the bucket // you can even use the bucket name, which will be extracted from the arn provided. const user = new iam.User(stack, 'MyUser'); - user.addToPolicy(new iam.PolicyStatement() - .addResource(bucket.arnForObjects(`my/folder/${bucket.bucketName}`)) - .addAction('s3:*')); + user.addToPolicy(new iam.PolicyStatement({ + resources: [bucket.arnForObjects(`my/folder/${bucket.bucketName}`)], + actions: ['s3:*'] })); expect(stack).toMatch({ "Resources": { diff --git a/packages/@aws-cdk/aws-ses/lib/receipt-rule-action.ts b/packages/@aws-cdk/aws-ses/lib/receipt-rule-action.ts index e84f2eb64216d..d004a729a51a4 100644 --- a/packages/@aws-cdk/aws-ses/lib/receipt-rule-action.ts +++ b/packages/@aws-cdk/aws-ses/lib/receipt-rule-action.ts @@ -339,24 +339,27 @@ export class ReceiptRuleS3Action implements IReceiptRuleAction { // See https://docs.aws.amazon.com/ses/latest/DeveloperGuide/receiving-email-permissions.html#receiving-email-permissions-s3 const keyPattern = this.props.objectKeyPrefix || ''; - const s3Statement = new iam.PolicyStatement() - .addAction('s3:PutObject') - .addServicePrincipal('ses.amazonaws.com') - .addResource(this.props.bucket.arnForObjects(`${keyPattern}*`)) - .addCondition('StringEquals', { - 'aws:Referer': cdk.Aws.accountId - }); + const s3Statement = new iam.PolicyStatement({ + actions: ['s3:PutObject'], + principals: [new iam.ServicePrincipal('ses.amazonaws.com')], + resources: [this.props.bucket.arnForObjects(`${keyPattern}*`)], + conditions: { + StringEquals: { + 'aws:Referer': cdk.Aws.accountId + } + } + }); this.props.bucket.addToResourcePolicy(s3Statement); // Allow SES to use KMS master key // See https://docs.aws.amazon.com/ses/latest/DeveloperGuide/receiving-email-permissions.html#receiving-email-permissions-kms if (this.props.kmsKey && !/alias\/aws\/ses$/.test(this.props.kmsKey.keyArn)) { - const kmsStatement = new iam.PolicyStatement() - .addActions('km:Encrypt', 'kms:GenerateDataKey') - .addServicePrincipal('ses.amazonaws.com') - .addAllResources() - .addConditions({ + const kmsStatement = new iam.PolicyStatement({ + actions: ['km:Encrypt', 'kms:GenerateDataKey'], + principals: [ new iam.ServicePrincipal('ses.amazonaws.com')], + resources: ['*'], + conditions: { Null: { 'kms:EncryptionContext:aws:ses:rule-name': 'false', 'kms:EncryptionContext:aws:ses:message-id': 'false' @@ -364,7 +367,8 @@ export class ReceiptRuleS3Action implements IReceiptRuleAction { StringEquals: { 'kms:EncryptionContext:aws:ses:source-account': cdk.Aws.accountId } - }); + } + }); this.props.kmsKey.addToResourcePolicy(kmsStatement); } diff --git a/packages/@aws-cdk/aws-sns-subscriptions/lib/sqs.ts b/packages/@aws-cdk/aws-sns-subscriptions/lib/sqs.ts index 2e427276d4fd5..5a3e92569a171 100644 --- a/packages/@aws-cdk/aws-sns-subscriptions/lib/sqs.ts +++ b/packages/@aws-cdk/aws-sns-subscriptions/lib/sqs.ts @@ -47,10 +47,13 @@ export class SqsSubscription implements sns.ITopicSubscription { // add a statement to the queue resource policy which allows this topic // to send messages to the queue. - this.queue.addToResourcePolicy(new iam.PolicyStatement() - .addResource(this.queue.queueArn) - .addAction('sqs:SendMessage') - .addServicePrincipal('sns.amazonaws.com') - .setCondition('ArnEquals', { 'aws:SourceArn': topic.topicArn })); + this.queue.addToResourcePolicy(new iam.PolicyStatement({ + resources: [this.queue.queueArn], + actions: ['sqs:SendMessage'], + principals: [new iam.ServicePrincipal('sns.amazonaws.com')], + conditions: { + ArnEquals: { 'aws:SourceArn': topic.topicArn } + } + })); } } diff --git a/packages/@aws-cdk/aws-sns/test/test.sns.ts b/packages/@aws-cdk/aws-sns/test/test.sns.ts index 4447c45d5d2b9..1ba0eb9365373 100644 --- a/packages/@aws-cdk/aws-sns/test/test.sns.ts +++ b/packages/@aws-cdk/aws-sns/test/test.sns.ts @@ -97,10 +97,11 @@ export = { const topic = new sns.Topic(stack, 'Topic'); // WHEN - topic.addToResourcePolicy(new iam.PolicyStatement() - .addAllResources() - .addActions('sns:*') - .addPrincipal(new iam.ArnPrincipal('arn'))); + topic.addToResourcePolicy(new iam.PolicyStatement({ + resources: ['*'], + actions: ['sns:*'], + principals: [new iam.ArnPrincipal('arn')] + })); // THEN expect(stack).to(haveResource('AWS::SNS::TopicPolicy', { @@ -150,8 +151,8 @@ export = { const topic = new sns.Topic(stack, 'MyTopic'); - topic.addToResourcePolicy(new iam.PolicyStatement().addAction('statement0')); - topic.addToResourcePolicy(new iam.PolicyStatement().addAction('statement1')); + topic.addToResourcePolicy(new iam.PolicyStatement({ actions: ['statement0'] })); + topic.addToResourcePolicy(new iam.PolicyStatement({ actions: ['statement1'] })); expect(stack).toMatch({ "Resources": { diff --git a/packages/@aws-cdk/aws-sqs/test/test.sqs.ts b/packages/@aws-cdk/aws-sqs/test/test.sqs.ts index 18c22edfcba05..35d22487a40f8 100644 --- a/packages/@aws-cdk/aws-sqs/test/test.sqs.ts +++ b/packages/@aws-cdk/aws-sqs/test/test.sqs.ts @@ -56,7 +56,12 @@ export = { 'addToPolicy will automatically create a policy for this queue'(test: Test) { const stack = new Stack(); const queue = new sqs.Queue(stack, 'MyQueue'); - queue.addToResourcePolicy(new iam.PolicyStatement().addAllResources().addActions('sqs:*').addPrincipal(new iam.ArnPrincipal('arn'))); + queue.addToResourcePolicy(new iam.PolicyStatement({ + resources: ['*'], + actions: ['sqs:*'], + principals: [new iam.ArnPrincipal('arn')] + })); + expect(stack).toMatch({ "Resources": { "MyQueueE6CA6235": { diff --git a/packages/@aws-cdk/aws-stepfunctions-tasks/lib/invoke-function.ts b/packages/@aws-cdk/aws-stepfunctions-tasks/lib/invoke-function.ts index 17bfe9e5c2cff..67d42fdf79537 100644 --- a/packages/@aws-cdk/aws-stepfunctions-tasks/lib/invoke-function.ts +++ b/packages/@aws-cdk/aws-stepfunctions-tasks/lib/invoke-function.ts @@ -15,10 +15,10 @@ export class InvokeFunction implements sfn.IStepFunctionsTask { public bind(_task: sfn.Task): sfn.StepFunctionsTaskConfig { return { resourceArn: this.lambdaFunction.functionArn, - policyStatements: [new iam.PolicyStatement() - .addResource(this.lambdaFunction.functionArn) - .addActions("lambda:InvokeFunction") - ], + policyStatements: [new iam.PolicyStatement({ + resources: [this.lambdaFunction.functionArn], + actions: ["lambda:InvokeFunction"], + })], metricPrefixSingular: 'LambdaFunction', metricPrefixPlural: 'LambdaFunctions', metricDimensions: { LambdaFunctionArn: this.lambdaFunction.functionArn }, diff --git a/packages/@aws-cdk/aws-stepfunctions-tasks/lib/publish-to-topic.ts b/packages/@aws-cdk/aws-stepfunctions-tasks/lib/publish-to-topic.ts index 91c4305257c95..87f2a254af44a 100644 --- a/packages/@aws-cdk/aws-stepfunctions-tasks/lib/publish-to-topic.ts +++ b/packages/@aws-cdk/aws-stepfunctions-tasks/lib/publish-to-topic.ts @@ -42,10 +42,10 @@ export class PublishToTopic implements sfn.IStepFunctionsTask { public bind(_task: sfn.Task): sfn.StepFunctionsTaskConfig { return { resourceArn: 'arn:aws:states:::sns:publish', - policyStatements: [new iam.PolicyStatement() - .addAction('sns:Publish') - .addResource(this.topic.topicArn) - ], + policyStatements: [new iam.PolicyStatement({ + actions: ['sns:Publish'], + resources: [this.topic.topicArn] + })], parameters: { TopicArn: this.topic.topicArn, ...sfn.FieldUtils.renderObject({ diff --git a/packages/@aws-cdk/aws-stepfunctions-tasks/lib/run-ecs-task-base.ts b/packages/@aws-cdk/aws-stepfunctions-tasks/lib/run-ecs-task-base.ts index c05cabd30f5bf..b51a08e08ff31 100644 --- a/packages/@aws-cdk/aws-stepfunctions-tasks/lib/run-ecs-task-base.ts +++ b/packages/@aws-cdk/aws-stepfunctions-tasks/lib/run-ecs-task-base.ts @@ -122,25 +122,29 @@ export class EcsRunTaskBase implements ec2.IConnectable, sfn.IStepFunctionsTask // https://docs.aws.amazon.com/step-functions/latest/dg/ecs-iam.html const policyStatements = [ - new iam.PolicyStatement() - .addAction('ecs:RunTask') - .addResource(this.props.taskDefinition.taskDefinitionArn), - new iam.PolicyStatement() - .addActions('ecs:StopTask', 'ecs:DescribeTasks') - .addAllResources(), - new iam.PolicyStatement() - .addAction('iam:PassRole') - .addResources(...cdk.Lazy.listValue({ produce: () => this.taskExecutionRoles().map(r => r.roleArn) })) + new iam.PolicyStatement({ + actions: ['ecs:RunTask'], + resources: [this.props.taskDefinition.taskDefinitionArn], + }), + new iam.PolicyStatement({ + actions: ['ecs:StopTask', 'ecs:DescribeTasks'], + resources: ['*'], + }), + new iam.PolicyStatement({ + actions: ['iam:PassRole'], + resources: cdk.Lazy.listValue({ produce: () => this.taskExecutionRoles().map(r => r.roleArn) }) + }), ]; if (this.sync) { - policyStatements.push(new iam.PolicyStatement() - .addActions("events:PutTargets", "events:PutRule", "events:DescribeRule") - .addResource(stack.formatArn({ + policyStatements.push(new iam.PolicyStatement({ + actions: ["events:PutTargets", "events:PutRule", "events:DescribeRule"], + resources: [stack.formatArn({ service: 'events', resource: 'rule', resourceName: 'StepFunctionsGetEventsForECSTaskRule' - }))); + })] + })); } return policyStatements; diff --git a/packages/@aws-cdk/aws-stepfunctions-tasks/lib/send-to-queue.ts b/packages/@aws-cdk/aws-stepfunctions-tasks/lib/send-to-queue.ts index ce10c26947cfc..4a764b4ba0b74 100644 --- a/packages/@aws-cdk/aws-stepfunctions-tasks/lib/send-to-queue.ts +++ b/packages/@aws-cdk/aws-stepfunctions-tasks/lib/send-to-queue.ts @@ -51,10 +51,10 @@ export class SendToQueue implements sfn.IStepFunctionsTask { public bind(_task: sfn.Task): sfn.StepFunctionsTaskConfig { return { resourceArn: 'arn:aws:states:::sqs:sendMessage', - policyStatements: [new iam.PolicyStatement() - .addAction('sqs:SendMessage') - .addResource(this.queue.queueArn) - ], + policyStatements: [new iam.PolicyStatement({ + actions: ['sqs:SendMessage'], + resources: [this.queue.queueArn] + })], parameters: { QueueUrl: this.queue.queueUrl, ...sfn.FieldUtils.renderObject({ diff --git a/packages/@aws-cdk/aws-stepfunctions/test/test.state-machine-resources.ts b/packages/@aws-cdk/aws-stepfunctions/test/test.state-machine-resources.ts index 718937305faa8..fa6ee71fb6786 100644 --- a/packages/@aws-cdk/aws-stepfunctions/test/test.state-machine-resources.ts +++ b/packages/@aws-cdk/aws-stepfunctions/test/test.state-machine-resources.ts @@ -12,10 +12,10 @@ export = { task: { bind: () => ({ resourceArn: 'resource', - policyStatements: [new iam.PolicyStatement() - .addAction('resource:Everything') - .addResource('resource') - ], + policyStatements: [new iam.PolicyStatement({ + actions: ['resource:Everything'], + resources: ['resource'] + })], }) } }); @@ -50,9 +50,10 @@ export = { bind: () => ({ resourceArn: 'resource', policyStatements: [ - new iam.PolicyStatement() - .addAction('resource:Everything') - .addResource('resource') + new iam.PolicyStatement({ + actions: ['resource:Everything'], + resources: ['resource'] + }) ] }) } From 83b7493f1d2a2fc9787d3a4090661ad39e41290d Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Thu, 13 Jun 2019 20:26:33 +0200 Subject: [PATCH 05/12] Test fixx0r4g3 --- .../aws-kms/test/test.via-service-principal.ts | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/packages/@aws-cdk/aws-kms/test/test.via-service-principal.ts b/packages/@aws-cdk/aws-kms/test/test.via-service-principal.ts index 7d3cc0daab133..d1e1714e83b0b 100644 --- a/packages/@aws-cdk/aws-kms/test/test.via-service-principal.ts +++ b/packages/@aws-cdk/aws-kms/test/test.via-service-principal.ts @@ -1,13 +1,9 @@ import iam = require('@aws-cdk/aws-iam'); -import cdk = require('@aws-cdk/cdk'); import { Test } from 'nodeunit'; import kms = require('../lib'); export = { 'Via service, any principal'(test: Test) { - // GIVEN - const stack = new cdk.Stack(); - // WHEN const statement = new iam.PolicyStatement({ actions: ['abc:call'], @@ -16,7 +12,7 @@ export = { }); // THEN - test.deepEqual(stack.resolve(statement), { + test.deepEqual(statement.toStatementJson(), { Action: 'abc:call', Condition: { StringEquals: { 'kms:ViaService': 'bla.amazonaws.com' } }, Effect: 'Allow', @@ -28,9 +24,6 @@ export = { }, 'Via service, principal with conditions'(test: Test) { - // GIVEN - const stack = new cdk.Stack(); - // WHEN const statement = new iam.PolicyStatement({ actions: ['abc:call'], @@ -39,7 +32,7 @@ export = { }); // THEN - test.deepEqual(stack.resolve(statement), { + test.deepEqual(statement.toStatementJson(), { Action: 'abc:call', Condition: { StringEquals: { From f1fa730d6d60b44ce13e169f88afc0e893d1c095 Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Thu, 13 Jun 2019 20:48:52 +0200 Subject: [PATCH 06/12] Fixes --- packages/@aws-cdk/aws-iam/lib/policy-statement.ts | 14 ++++++++++++-- packages/@aws-cdk/aws-kms/test/integ.key.ts | 2 +- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/packages/@aws-cdk/aws-iam/lib/policy-statement.ts b/packages/@aws-cdk/aws-iam/lib/policy-statement.ts index 7960a047738a6..376d3c37fa6e5 100644 --- a/packages/@aws-cdk/aws-iam/lib/policy-statement.ts +++ b/packages/@aws-cdk/aws-iam/lib/policy-statement.ts @@ -140,14 +140,14 @@ export class PolicyStatement { } public toStatementJson(): any { - return { + return noUndef({ Action: _norm(this.action), Condition: _norm(this.condition), Effect: _norm(this.effect), Principal: _normPrincipal(this.principal), Resource: _norm(this.resource), Sid: _norm(this.sid), - }; + }); function _norm(values: any) { @@ -249,4 +249,14 @@ export interface PolicyStatementProps { * @default - no condition */ readonly conditions?: {[key: string]: any}; +} + +function noUndef(x: any): any { + const ret: any = {}; + for (const [key, value] of Object.entries(x)) { + if (value !== undefined) { + ret[key] = value; + } + } + return ret; } \ No newline at end of file diff --git a/packages/@aws-cdk/aws-kms/test/integ.key.ts b/packages/@aws-cdk/aws-kms/test/integ.key.ts index 37116db675181..973207603b52a 100644 --- a/packages/@aws-cdk/aws-kms/test/integ.key.ts +++ b/packages/@aws-cdk/aws-kms/test/integ.key.ts @@ -12,7 +12,7 @@ const key = new Key(stack, 'MyKey', { retain: false }); key.addToResourcePolicy(new PolicyStatement({ resources: ['*'], actions: ['kms:encrypt'], - principals: [new iam.AccountPrincipal(stack.account)] + principals: [new iam.ArnPrincipal(stack.account)] })); key.addAlias('alias/bar'); From d7c0ae15b3b826e9ca41ad4ef20aed10167a2bf6 Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Thu, 13 Jun 2019 20:53:43 +0200 Subject: [PATCH 07/12] yaya --- packages/@aws-cdk/aws-s3/test/test.bucket.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/@aws-cdk/aws-s3/test/test.bucket.ts b/packages/@aws-cdk/aws-s3/test/test.bucket.ts index 5025e39ffbab7..ffcfc7a633268 100644 --- a/packages/@aws-cdk/aws-s3/test/test.bucket.ts +++ b/packages/@aws-cdk/aws-s3/test/test.bucket.ts @@ -468,7 +468,7 @@ export = { const x = new iam.PolicyStatement({ resources: [bucket.bucketArn], actions: ['s3:ListBucket'] }); - test.deepEqual(stack.resolve(x), { + test.deepEqual(stack.resolve(x.toStatementJson()), { Action: 's3:ListBucket', Effect: 'Allow', Resource: { 'Fn::GetAtt': ['MyBucketF68F3FF0', 'Arn'] } @@ -484,7 +484,7 @@ export = { const p = new iam.PolicyStatement({ resources: [bucket.arnForObjects('hello/world')], actions: ['s3:GetObject'] }); - test.deepEqual(stack.resolve(p), { + test.deepEqual(stack.resolve(p.toStatementJson()), { Action: 's3:GetObject', Effect: 'Allow', Resource: { @@ -510,7 +510,7 @@ export = { const resource = bucket.arnForObjects(`home/${team.groupName}/${user.userName}/*`); const p = new iam.PolicyStatement({ resources: [resource], actions: ['s3:GetObject'] }); - test.deepEqual(stack.resolve(p), { + test.deepEqual(stack.resolve(p.toStatementJson()), { Action: 's3:GetObject', Effect: 'Allow', Resource: { @@ -562,7 +562,7 @@ export = { const p = new iam.PolicyStatement({ resources: [bucket.bucketArn], actions: ['s3:ListBucket'] }); // it is possible to obtain a permission statement for a ref - test.deepEqual(stack.resolve(p), { + test.deepEqual(p.toStatementJson(), { Action: 's3:ListBucket', Effect: 'Allow', Resource: 'arn:aws:s3:::my-bucket' From c3bbf12f752c1cdbc499b4181bbea83c1a86c540 Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Thu, 13 Jun 2019 22:56:00 +0200 Subject: [PATCH 08/12] Reduce coverage requirements --- packages/@aws-cdk/aws-route53/package.json | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/packages/@aws-cdk/aws-route53/package.json b/packages/@aws-cdk/aws-route53/package.json index 9450dcaf111ae..b6f1d95b3077c 100644 --- a/packages/@aws-cdk/aws-route53/package.json +++ b/packages/@aws-cdk/aws-route53/package.json @@ -92,5 +92,10 @@ "from-attributes:fromPublicHostedZoneAttributes" ] }, - "stability": "experimental" -} \ No newline at end of file + "stability": "experimental", + "nyc": { + "statements": 60, + "lines": 60, + "branches": 60 + } +} From 604909ae90aba431067b65fbe12b1118c206b65c Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Mon, 17 Jun 2019 11:55:19 +0200 Subject: [PATCH 09/12] Update tests --- .../integ.deployment-group.expected.json | 15 +++++- .../test/lambda/test.deployment-group.ts | 26 +++++++++- .../integ.deployment-group.expected.json | 15 +++++- .../aws-stepfunctions-tasks/package-lock.json | 47 ------------------- 4 files changed, 50 insertions(+), 53 deletions(-) diff --git a/packages/@aws-cdk/aws-codedeploy/test/lambda/integ.deployment-group.expected.json b/packages/@aws-cdk/aws-codedeploy/test/lambda/integ.deployment-group.expected.json index d1b1f415cd098..bf2574797575c 100644 --- a/packages/@aws-cdk/aws-codedeploy/test/lambda/integ.deployment-group.expected.json +++ b/packages/@aws-cdk/aws-codedeploy/test/lambda/integ.deployment-group.expected.json @@ -493,7 +493,18 @@ "Version": "2012-10-17" }, "ManagedPolicyArns": [ - "arn:aws:iam::aws:policy/service-role/AWSCodeDeployRoleForLambda" + { + "Fn::Join": [ + "", + [ + "arn:", + { + "Ref": "AWS::Partition" + }, + ":iam::aws:policy/service-role/AWSCodeDeployRoleForLambda" + ] + ] + } ] } }, @@ -608,4 +619,4 @@ "Description": "Artifact hash for asset \"aws-cdk-codedeploy-lambda/PostHook/Code\"" } } -} \ No newline at end of file +} diff --git a/packages/@aws-cdk/aws-codedeploy/test/lambda/test.deployment-group.ts b/packages/@aws-cdk/aws-codedeploy/test/lambda/test.deployment-group.ts index fb4a61c53ac73..9911dfd4193ca 100644 --- a/packages/@aws-cdk/aws-codedeploy/test/lambda/test.deployment-group.ts +++ b/packages/@aws-cdk/aws-codedeploy/test/lambda/test.deployment-group.ts @@ -95,7 +95,18 @@ export = { }], Version: "2012-10-17" }, - ManagedPolicyArns: ['arn:aws:iam::aws:policy/service-role/AWSCodeDeployRoleForLambda'] + ManagedPolicyArns: [ + { + "Fn::Join": [ + "", + [ + "arn:", + { Ref: "AWS::Partition" }, + ':iam::aws:policy/service-role/AWSCodeDeployRoleForLambda' + ] + ] + } + ] })); test.done(); @@ -143,7 +154,18 @@ export = { }], Version: "2012-10-17" }, - ManagedPolicyArns: ['arn:aws:iam::aws:policy/service-role/AWSCodeDeployRoleForLambda'] + ManagedPolicyArns: [ + { + "Fn::Join": [ + "", + [ + "arn:", + { Ref: "AWS::Partition" }, + ':iam::aws:policy/service-role/AWSCodeDeployRoleForLambda' + ] + ] + } + ] })); test.done(); diff --git a/packages/@aws-cdk/aws-codedeploy/test/server/integ.deployment-group.expected.json b/packages/@aws-cdk/aws-codedeploy/test/server/integ.deployment-group.expected.json index 74cf1cf212580..807f10d06b551 100644 --- a/packages/@aws-cdk/aws-codedeploy/test/server/integ.deployment-group.expected.json +++ b/packages/@aws-cdk/aws-codedeploy/test/server/integ.deployment-group.expected.json @@ -796,7 +796,18 @@ "Version": "2012-10-17" }, "ManagedPolicyArns": [ - "arn:aws:iam::aws:policy/service-role/AWSCodeDeployRole" + { + "Fn::Join": [ + "", + [ + "arn:", + { + "Ref": "AWS::Partition" + }, + ":iam::aws:policy/service-role/AWSCodeDeployRole" + ] + ] + } ] } }, @@ -843,4 +854,4 @@ } } } -} \ No newline at end of file +} diff --git a/packages/@aws-cdk/aws-stepfunctions-tasks/package-lock.json b/packages/@aws-cdk/aws-stepfunctions-tasks/package-lock.json index 9fe62e9ec1ff3..2aad8cca5e64d 100644 --- a/packages/@aws-cdk/aws-stepfunctions-tasks/package-lock.json +++ b/packages/@aws-cdk/aws-stepfunctions-tasks/package-lock.json @@ -4,53 +4,6 @@ "lockfileVersion": 1, "requires": true, "dependencies": { - "@aws-cdk/aws-kms": { - "version": "0.33.0", - "resolved": "https://registry.npmjs.org/@aws-cdk/aws-kms/-/aws-kms-0.33.0.tgz", - "integrity": "sha512-Yj1i/kqcpLu4LMIfIrk8F1Znereh7kL05+j7Ho0gy+HjwMbODIxB0BcyTiJ/5CjHkJPsR8Tl2GPyerZ6OGk/Dw==", - "requires": { - "@aws-cdk/aws-iam": "^0.33.0", - "@aws-cdk/cdk": "^0.33.0" - }, - "dependencies": { - "@aws-cdk/aws-iam": { - "version": "0.33.0", - "resolved": "https://registry.npmjs.org/@aws-cdk/aws-iam/-/aws-iam-0.33.0.tgz", - "integrity": "sha512-d6HVkScJlG3a0rwWO0LgmZCTndze1c2cpoIezJINZ+sXPyMQiWWyFQDVTDC3LxPPUalG9t42gr2139d2zbfX6w==", - "requires": { - "@aws-cdk/cdk": "^0.33.0", - "@aws-cdk/region-info": "^0.33.0" - } - }, - "@aws-cdk/cdk": { - "version": "0.33.0", - "resolved": "https://registry.npmjs.org/@aws-cdk/cdk/-/cdk-0.33.0.tgz", - "integrity": "sha512-ARfTC6ZTg1r2FOWntYo4kZ3S/Fju2vAagQavll56BJ3EPCxfYbPnIAWu3oFiSzg/4XQ345tbAZP1GSVZsF4RJw==", - "requires": { - "@aws-cdk/cx-api": "^0.33.0" - } - } - } - }, - "@aws-cdk/cx-api": { - "version": "0.33.0", - "resolved": "https://registry.npmjs.org/@aws-cdk/cx-api/-/cx-api-0.33.0.tgz", - "integrity": "sha512-PvPO1quhrezUyYtyi3kEq4CHjmg5TccWQrU4khmTrP9bmb7sNKCmR7ish1VHcA2FBaNjtAj0PgdA+2/+Q+Pzrw==", - "requires": { - "semver": "^6.0.0" - }, - "dependencies": { - "semver": { - "version": "6.1.0", - "bundled": true - } - } - }, - "@aws-cdk/region-info": { - "version": "0.33.0", - "resolved": "https://registry.npmjs.org/@aws-cdk/region-info/-/region-info-0.33.0.tgz", - "integrity": "sha512-Sy0gXDqzGNuOYAF7edd5rlY3iChVSfjaaZ+bONyClF7gulkYv4jehYkQ1ShATl8XsVRedtCOwSU+mDo/tu8npA==" - }, "@babel/code-frame": { "version": "7.0.0", "resolved": "https://registry.npmjs.org/@babel/code-frame/-/code-frame-7.0.0.tgz", From 213ceea313727887874463e5647787c1bbd328dd Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Mon, 17 Jun 2019 12:00:26 +0200 Subject: [PATCH 10/12] Update sagemaker --- .../lib/sagemaker-train-task.ts | 54 +++++++++++-------- .../lib/sagemaker-transform-task.ts | 52 ++++++++++-------- .../test/sagemaker-training-job.test.ts | 8 +-- .../test/sagemaker-transform-job.test.ts | 4 +- 4 files changed, 66 insertions(+), 52 deletions(-) diff --git a/packages/@aws-cdk/aws-stepfunctions-tasks/lib/sagemaker-train-task.ts b/packages/@aws-cdk/aws-stepfunctions-tasks/lib/sagemaker-train-task.ts index 9d173384cdf0e..6059174c5eb75 100644 --- a/packages/@aws-cdk/aws-stepfunctions-tasks/lib/sagemaker-train-task.ts +++ b/packages/@aws-cdk/aws-stepfunctions-tasks/lib/sagemaker-train-task.ts @@ -122,8 +122,8 @@ export class SagemakerTrainTask implements ec2.IConnectable, sfn.IStepFunctionsT // set the sagemaker role or create new one this.role = props.role || new iam.Role(scope, 'SagemakerRole', { assumedBy: new iam.ServicePrincipal('sagemaker.amazonaws.com'), - managedPolicyArns: [ - new iam.AwsManagedPolicy('AmazonSageMakerFullAccess', scope).policyArn + managedPolicies: [ + iam.ManagedPolicy.fromAwsManagedPolicyName('AmazonSageMakerFullAccess') ] }); @@ -254,30 +254,38 @@ export class SagemakerTrainTask implements ec2.IConnectable, sfn.IStepFunctionsT // https://docs.aws.amazon.com/step-functions/latest/dg/sagemaker-iam.html const policyStatements = [ - new iam.PolicyStatement() - .addActions('sagemaker:CreateTrainingJob', 'sagemaker:DescribeTrainingJob', 'sagemaker:StopTrainingJob') - .addResource(stack.formatArn({ - service: 'sagemaker', - resource: 'training-job', - resourceName: '*' - })), - new iam.PolicyStatement() - .addAction('sagemaker:ListTags') - .addAllResources(), - new iam.PolicyStatement() - .addAction('iam:PassRole') - .addResources(this.role.roleArn) - .addCondition('StringEquals', { "iam:PassedToService": "sagemaker.amazonaws.com" }) + new iam.PolicyStatement({ + actions: ['sagemaker:CreateTrainingJob', 'sagemaker:DescribeTrainingJob', 'sagemaker:StopTrainingJob'], + resources: [ + stack.formatArn({ + service: 'sagemaker', + resource: 'training-job', + resourceName: '*' + }) + ], + }), + new iam.PolicyStatement({ + actions: ['sagemaker:ListTags'], + resources: ['*'] + }), + new iam.PolicyStatement({ + actions: ['iam:PassRole'], + resources: [this.role.roleArn], + conditions: { + StringEquals: { "iam:PassedToService": "sagemaker.amazonaws.com" } + } + }) ]; if (this.props.synchronous) { - policyStatements.push(new iam.PolicyStatement() - .addActions("events:PutTargets", "events:PutRule", "events:DescribeRule") - .addResource(stack.formatArn({ - service: 'events', - resource: 'rule', - resourceName: 'StepFunctionsGetEventsForSageMakerTrainingJobsRule' - }))); + policyStatements.push(new iam.PolicyStatement({ + actions: ["events:PutTargets", "events:PutRule", "events:DescribeRule"], + resources: [stack.formatArn({ + service: 'events', + resource: 'rule', + resourceName: 'StepFunctionsGetEventsForSageMakerTrainingJobsRule' + })] + })); } return policyStatements; diff --git a/packages/@aws-cdk/aws-stepfunctions-tasks/lib/sagemaker-transform-task.ts b/packages/@aws-cdk/aws-stepfunctions-tasks/lib/sagemaker-transform-task.ts index bfd365f88a293..9d485f5bfadcb 100644 --- a/packages/@aws-cdk/aws-stepfunctions-tasks/lib/sagemaker-transform-task.ts +++ b/packages/@aws-cdk/aws-stepfunctions-tasks/lib/sagemaker-transform-task.ts @@ -99,8 +99,8 @@ export class SagemakerTransformTask implements sfn.IStepFunctionsTask { // set the sagemaker role or create new one this.role = props.role || new iam.Role(scope, 'SagemakerRole', { assumedBy: new iam.ServicePrincipal('sagemaker.amazonaws.com'), - managedPolicyArns: [ - new iam.AwsManagedPolicy('AmazonSageMakerFullAccess', scope).policyArn + managedPolicies: [ + iam.ManagedPolicy.fromAwsManagedPolicyName('AmazonSageMakerFullAccess') ] }); @@ -195,30 +195,36 @@ export class SagemakerTransformTask implements sfn.IStepFunctionsTask { // https://docs.aws.amazon.com/step-functions/latest/dg/sagemaker-iam.html const policyStatements = [ - new iam.PolicyStatement() - .addActions('sagemaker:CreateTransformJob', 'sagemaker:DescribeTransformJob', 'sagemaker:StopTransformJob') - .addResource(stack.formatArn({ - service: 'sagemaker', - resource: 'transform-job', - resourceName: '*' - })), - new iam.PolicyStatement() - .addAction('sagemaker:ListTags') - .addAllResources(), - new iam.PolicyStatement() - .addAction('iam:PassRole') - .addResources(this.role.roleArn) - .addCondition('StringEquals', { "iam:PassedToService": "sagemaker.amazonaws.com" }) + new iam.PolicyStatement({ + actions: ['sagemaker:CreateTransformJob', 'sagemaker:DescribeTransformJob', 'sagemaker:StopTransformJob'], + resources: [stack.formatArn({ + service: 'sagemaker', + resource: 'transform-job', + resourceName: '*' + })] + }), + new iam.PolicyStatement({ + actions: ['sagemaker:ListTags'], + resources: ['*'], + }), + new iam.PolicyStatement({ + actions: ['iam:PassRole'], + resources: [this.role.roleArn], + conditions: { + StringEquals: { "iam:PassedToService": "sagemaker.amazonaws.com" } + } + }) ]; if (this.props.synchronous) { - policyStatements.push(new iam.PolicyStatement() - .addActions("events:PutTargets", "events:PutRule", "events:DescribeRule") - .addResource(stack.formatArn({ - service: 'events', - resource: 'rule', - resourceName: 'StepFunctionsGetEventsForSageMakerTransformJobsRule' - }))); + policyStatements.push(new iam.PolicyStatement({ + actions: ["events:PutTargets", "events:PutRule", "events:DescribeRule"], + resources: [stack.formatArn({ + service: 'events', + resource: 'rule', + resourceName: 'StepFunctionsGetEventsForSageMakerTransformJobsRule' + }) ] + })); } return policyStatements; diff --git a/packages/@aws-cdk/aws-stepfunctions-tasks/test/sagemaker-training-job.test.ts b/packages/@aws-cdk/aws-stepfunctions-tasks/test/sagemaker-training-job.test.ts index dd8de65a04552..66575a68dfc04 100644 --- a/packages/@aws-cdk/aws-stepfunctions-tasks/test/sagemaker-training-job.test.ts +++ b/packages/@aws-cdk/aws-stepfunctions-tasks/test/sagemaker-training-job.test.ts @@ -82,8 +82,8 @@ test('create complex training job', () => { const role = new iam.Role(stack, 'Role', { assumedBy: new iam.ServicePrincipal('sagemaker.amazonaws.com'), - managedPolicyArns: [ - new iam.AwsManagedPolicy('AmazonSageMakerFullAccess', stack).policyArn + managedPolicies: [ + iam.ManagedPolicy.fromAwsManagedPolicyName('AmazonSageMakerFullAccess') ], }); @@ -228,8 +228,8 @@ test('pass param to training job', () => { // WHEN const role = new iam.Role(stack, 'Role', { assumedBy: new iam.ServicePrincipal('sagemaker.amazonaws.com'), - managedPolicyArns: [ - new iam.AwsManagedPolicy('AmazonSageMakerFullAccess', stack).policyArn + managedPolicies: [ + iam.ManagedPolicy.fromAwsManagedPolicyName('AmazonSageMakerFullAccess'), ], }); diff --git a/packages/@aws-cdk/aws-stepfunctions-tasks/test/sagemaker-transform-job.test.ts b/packages/@aws-cdk/aws-stepfunctions-tasks/test/sagemaker-transform-job.test.ts index e6a25b0f490dc..f2e3aef6b55c5 100644 --- a/packages/@aws-cdk/aws-stepfunctions-tasks/test/sagemaker-transform-job.test.ts +++ b/packages/@aws-cdk/aws-stepfunctions-tasks/test/sagemaker-transform-job.test.ts @@ -15,8 +15,8 @@ beforeEach(() => { stack = new cdk.Stack(); role = new iam.Role(stack, 'Role', { assumedBy: new iam.ServicePrincipal('sagemaker.amazonaws.com'), - managedPolicyArns: [ - new iam.AwsManagedPolicy('AmazonSageMakerFullAccess', stack).policyArn + managedPolicies: [ + iam.ManagedPolicy.fromAwsManagedPolicyName('AmazonSageMakerFullAccess') ], }); }); From 703a98c893818918bd79d8a74a1cf924a1a60942 Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Mon, 17 Jun 2019 13:16:02 +0200 Subject: [PATCH 11/12] More tests --- .../test/cloudformation/test.pipeline-actions.ts | 6 +++--- .../test/integ.pipeline-code-deploy.expected.json | 15 +++++++++++++-- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/packages/@aws-cdk/aws-codepipeline-actions/test/cloudformation/test.pipeline-actions.ts b/packages/@aws-cdk/aws-codepipeline-actions/test/cloudformation/test.pipeline-actions.ts index 96ed8f3c19d93..a83f47cab910a 100644 --- a/packages/@aws-cdk/aws-codepipeline-actions/test/cloudformation/test.pipeline-actions.ts +++ b/packages/@aws-cdk/aws-codepipeline-actions/test/cloudformation/test.pipeline-actions.ts @@ -72,7 +72,7 @@ export = nodeunit.testCase({ }); test.deepEqual( - stack.resolve(pipelineRole.statements), + stack.resolve(pipelineRole.statements.map(s => s.toStatementJson())), [ { Action: 'iam:PassRole', @@ -153,7 +153,7 @@ export = nodeunit.testCase({ }); test.deepEqual( - stack.resolve(pipelineRole.statements), + stack.resolve(pipelineRole.statements.map(s => s.toStatementJson())), [ { Action: 'cloudformation:ExecuteChangeSet', @@ -267,7 +267,7 @@ function _assertPermissionGranted(test: nodeunit.Test, statements: iam.PolicySta const conditionStr = conditions ? ` with condition(s) ${JSON.stringify(resolve(conditions))}` : ''; - const resolvedStatements = resolve(statements); + const resolvedStatements = resolve(statements.map(s => s.toStatementJson())); const statementsStr = JSON.stringify(resolvedStatements, null, 2); test.ok(_grantsPermission(resolvedStatements, action, resource, conditions), `Expected to find a statement granting ${action} on ${JSON.stringify(resolve(resource))}${conditionStr}, found:\n${statementsStr}`); diff --git a/packages/@aws-cdk/aws-codepipeline-actions/test/integ.pipeline-code-deploy.expected.json b/packages/@aws-cdk/aws-codepipeline-actions/test/integ.pipeline-code-deploy.expected.json index 50db4d0869478..0a3e241126f71 100644 --- a/packages/@aws-cdk/aws-codepipeline-actions/test/integ.pipeline-code-deploy.expected.json +++ b/packages/@aws-cdk/aws-codepipeline-actions/test/integ.pipeline-code-deploy.expected.json @@ -46,7 +46,18 @@ "Version": "2012-10-17" }, "ManagedPolicyArns": [ - "arn:aws:iam::aws:policy/service-role/AWSCodeDeployRole" + { + "Fn::Join": [ + "", + [ + "arn:", + { + "Ref": "AWS::Partition" + }, + ":iam::aws:policy/service-role/AWSCodeDeployRole" + ] + ] + } ] } }, @@ -359,4 +370,4 @@ ] } } -} \ No newline at end of file +} From bf9acf0a77709a55ff5cbd3d639a195a2a89fd76 Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Mon, 17 Jun 2019 13:41:32 +0200 Subject: [PATCH 12/12] Fixup new changes on master --- packages/@aws-cdk/aws-iam/lib/role.ts | 2 +- packages/@aws-cdk/aws-iam/test/test.role.ts | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/@aws-cdk/aws-iam/lib/role.ts b/packages/@aws-cdk/aws-iam/lib/role.ts index 1162a3f4c6a2d..cdd70d551e563 100644 --- a/packages/@aws-cdk/aws-iam/lib/role.ts +++ b/packages/@aws-cdk/aws-iam/lib/role.ts @@ -126,7 +126,7 @@ export class Role extends Resource implements IRole { this.defaultPolicy = new Policy(this, 'Policy'); this.attachInlinePolicy(this.defaultPolicy); } - this.defaultPolicy.addStatement(statement); + this.defaultPolicy.addStatements(statement); return true; } diff --git a/packages/@aws-cdk/aws-iam/test/test.role.ts b/packages/@aws-cdk/aws-iam/test/test.role.ts index 3ed7fc70dde13..93d423b653200 100644 --- a/packages/@aws-cdk/aws-iam/test/test.role.ts +++ b/packages/@aws-cdk/aws-iam/test/test.role.ts @@ -268,9 +268,10 @@ export = { const importedRole = Role.fromRoleArn(stack, 'ImportedRole', 'arn:aws:iam::123456789012:role/MyRole'); // WHEN - importedRole.addToPolicy(new PolicyStatement() - .addAction('s3:*') - .addResource('xyz')); + importedRole.addToPolicy(new PolicyStatement({ + actions: ['s3:*'], + resources: ['xyz'] + })); // THEN expect(stack).to(haveResource('AWS::IAM::Policy', {