From af6f0957bbfeeda31353d75f87b860b361d114a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=F0=9F=8C=88=20=F0=9F=92=BB=20=20Miley?= Date: Sun, 3 Jun 2018 17:11:43 -0700 Subject: [PATCH 1/3] Allow lambdas to tak additional permissions as part of thier properties! In the case of custom resources (as they stand right now), we need to be able to add permissions to the lambda when creating it. This also fixes a minor partition bug - where we had hard coded `arn:aws:whatever` (we should open an issue about this gloablly, it could also apply to service principals that vary per partition ) --- packages/aws-cdk-lambda/lib/lambda.ts | 13 +++- .../aws-cdk-lambda/test/inline.expected.json | 2 +- .../test/integ.inline.expected.json | 2 +- .../test/integ.lambda.expected.json | 2 +- packages/aws-cdk-lambda/test/test.lambda.ts | 64 +++++++++++++++++-- 5 files changed, 73 insertions(+), 10 deletions(-) diff --git a/packages/aws-cdk-lambda/lib/lambda.ts b/packages/aws-cdk-lambda/lib/lambda.ts index 82b44da0722f6..7a11c60b0de66 100644 --- a/packages/aws-cdk-lambda/lib/lambda.ts +++ b/packages/aws-cdk-lambda/lib/lambda.ts @@ -1,4 +1,4 @@ -import { Construct, ServicePrincipal, Token } from 'aws-cdk'; +import { AwsPartition, Construct, FnConcat, PolicyStatement, ServicePrincipal, Token } from 'aws-cdk'; import { Role } from 'aws-cdk-iam'; import { lambda } from 'aws-cdk-resources'; import { LambdaCode } from './code'; @@ -70,6 +70,12 @@ export interface LambdaProps { * @default The default value is 128 MB */ memorySize?: number; + + /** + * Additional permissions to add to the created Lambda Role. + * You can also call addToRolePolicy to the created lambda. + */ + additionalPermissions?: PolicyStatement[]; } /** @@ -113,8 +119,11 @@ export class Lambda extends LambdaRef { this.role = new Role(this, 'ServiceRole', { assumedBy: new ServicePrincipal('lambda.amazonaws.com'), - managedPolicyArns: [ 'arn:aws:iam::aws:policy/service-role/AWSLambdaBasicExecutionRole' ], + managedPolicyArns: [ new FnConcat('arn:', new AwsPartition(), ':iam::aws:policy/service-role/AWSLambdaBasicExecutionRole')], }); + if (props.additionalPermissions && props.additionalPermissions.length > 0) { + props.additionalPermissions.forEach(permission => this.role!.addToPolicy(permission)); + } const resource = new lambda.FunctionResource(this, 'Resource', { functionName: props.functionName, diff --git a/packages/aws-cdk-lambda/test/inline.expected.json b/packages/aws-cdk-lambda/test/inline.expected.json index 3e60be5e3c3e4..3c2bcb12c340f 100644 --- a/packages/aws-cdk-lambda/test/inline.expected.json +++ b/packages/aws-cdk-lambda/test/inline.expected.json @@ -74,7 +74,7 @@ "Version": "2012-10-17" }, "ManagedPolicyArns": [ - "arn:aws:iam::aws:policy/service-role/AWSLambdaBasicExecutionRole" + {"Fn::Join" : ["", ["arn:", {"Ref": "AWS::Partition"}, ":iam::aws:policy/service-role/AWSLambdaBasicExecutionRole"]]} ] } }, diff --git a/packages/aws-cdk-lambda/test/integ.inline.expected.json b/packages/aws-cdk-lambda/test/integ.inline.expected.json index 48cc0a7744ee1..c923a0a94f44f 100644 --- a/packages/aws-cdk-lambda/test/integ.inline.expected.json +++ b/packages/aws-cdk-lambda/test/integ.inline.expected.json @@ -74,7 +74,7 @@ "Version": "2012-10-17" }, "ManagedPolicyArns": [ - "arn:aws:iam::aws:policy/service-role/AWSLambdaBasicExecutionRole" + {"Fn::Join" : ["", ["arn:", {"Ref": "AWS::Partition"}, ":iam::aws:policy/service-role/AWSLambdaBasicExecutionRole"]]} ] } }, diff --git a/packages/aws-cdk-lambda/test/integ.lambda.expected.json b/packages/aws-cdk-lambda/test/integ.lambda.expected.json index 60b88677e40e9..12114b38c688a 100644 --- a/packages/aws-cdk-lambda/test/integ.lambda.expected.json +++ b/packages/aws-cdk-lambda/test/integ.lambda.expected.json @@ -16,7 +16,7 @@ "Version": "2012-10-17" }, "ManagedPolicyArns": [ - "arn:aws:iam::aws:policy/service-role/AWSLambdaBasicExecutionRole" + {"Fn::Join" : ["", ["arn:", {"Ref": "AWS::Partition"}, ":iam::aws:policy/service-role/AWSLambdaBasicExecutionRole"]]} ] } }, diff --git a/packages/aws-cdk-lambda/test/test.lambda.ts b/packages/aws-cdk-lambda/test/test.lambda.ts index 933a5721443b7..8fcc4e7f66138 100644 --- a/packages/aws-cdk-lambda/test/test.lambda.ts +++ b/packages/aws-cdk-lambda/test/test.lambda.ts @@ -1,4 +1,4 @@ -import { AccountPrincipal, Arn, ArnPrincipal, AwsAccountId, Construct, ServicePrincipal, Stack } from 'aws-cdk'; +import { AccountPrincipal, Arn, ArnPrincipal, AwsAccountId, Construct, PolicyStatement, ServicePrincipal, Stack } from 'aws-cdk'; import { expect } from 'aws-cdk-assert'; import { Test } from 'nodeunit'; import { Lambda, LambdaInlineCode, LambdaRuntime } from '../lib'; @@ -26,7 +26,8 @@ export = { Principal: { Service: 'lambda.amazonaws.com' } } ], Version: '2012-10-17' }, ManagedPolicyArns: - [ 'arn:aws:iam::aws:policy/service-role/AWSLambdaBasicExecutionRole' ] } }, + [{'Fn::Join': ['', ['arn:', {Ref: 'AWS::Partition'}, ':iam::aws:policy/service-role/AWSLambdaBasicExecutionRole']]}], + }}, MyLambdaCCE802FB: { Type: 'AWS::Lambda::Function', Properties: @@ -38,6 +39,60 @@ export = { test.done(); }, + 'adds policy permissions'(test: Test) { + const stack = new Stack(); + new Lambda(stack, 'MyLambda', { + code: new LambdaInlineCode('foo'), + handler: 'index.handler', + runtime: LambdaRuntime.NodeJS610, + additionalPermissions: [new PolicyStatement().addAction("*").addResource("*")] + }); + expect(stack).toMatch({ Resources: + { MyLambdaServiceRole4539ECB6: + { Type: 'AWS::IAM::Role', + Properties: + { AssumeRolePolicyDocument: + { Statement: + [ { Action: 'sts:AssumeRole', + Effect: 'Allow', + Principal: { Service: 'lambda.amazonaws.com' } } ], + Version: '2012-10-17' }, + ManagedPolicyArns: + [{'Fn::Join': ['', ['arn:', {Ref: 'AWS::Partition'}, ':iam::aws:policy/service-role/AWSLambdaBasicExecutionRole']]}] + }}, + MyLambdaServiceRoleDefaultPolicy5BBC6F68: { + Type: "AWS::IAM::Policy", + Properties: { + PolicyDocument: { + Statement: [ + { + Action: "*", + Effect: "Allow", + Resource: "*" + } + ], + Version: "2012-10-17" + }, + PolicyName: "MyLambdaServiceRoleDefaultPolicy5BBC6F68", + Roles: [ + { + Ref: "MyLambdaServiceRole4539ECB6" + } + ] + } + }, + MyLambdaCCE802FB: + { Type: 'AWS::Lambda::Function', + Properties: + { Code: { ZipFile: 'foo' }, + Handler: 'index.handler', + Role: { 'Fn::GetAtt': [ 'MyLambdaServiceRole4539ECB6', 'Arn' ] }, + Runtime: 'nodejs6.10' }, + DependsOn: [ 'MyLambdaServiceRole4539ECB6', 'MyLambdaServiceRoleDefaultPolicy5BBC6F68' ] } } } ); + test.done(); + + }, + 'fails if inline code is used for an invalid runtime'(test: Test) { const stack = new Stack(); test.throws(() => new Lambda(stack, 'MyLambda', { @@ -77,9 +132,8 @@ export = { ], "Version": "2012-10-17" }, - "ManagedPolicyArns": [ - "arn:aws:iam::aws:policy/service-role/AWSLambdaBasicExecutionRole" - ] + "ManagedPolicyArns": + [{'Fn::Join': ['', ['arn:', {Ref: 'AWS::Partition'}, ':iam::aws:policy/service-role/AWSLambdaBasicExecutionRole']]}], } }, "MyLambdaCCE802FB": { From 29d47ed7e691a32efac297b7e69a05326f45d63c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=F0=9F=8C=88=20=F0=9F=92=BB=20=20Miley?= Date: Mon, 4 Jun 2018 13:45:21 -0700 Subject: [PATCH 2/3] PR feedback, updates to the tests in response, renamed property as specifcied. --- packages/aws-cdk-lambda/lib/lambda.ts | 23 +++++++++++++------ .../aws-cdk-lambda/test/inline.expected.json | 3 ++- .../test/integ.inline.expected.json | 2 +- .../test/integ.lambda.expected.json | 2 +- packages/aws-cdk-lambda/test/test.lambda.ts | 12 ++++++---- 5 files changed, 28 insertions(+), 14 deletions(-) diff --git a/packages/aws-cdk-lambda/lib/lambda.ts b/packages/aws-cdk-lambda/lib/lambda.ts index 7a11c60b0de66..2fc9fb6ccbe57 100644 --- a/packages/aws-cdk-lambda/lib/lambda.ts +++ b/packages/aws-cdk-lambda/lib/lambda.ts @@ -1,4 +1,4 @@ -import { AwsPartition, Construct, FnConcat, PolicyStatement, ServicePrincipal, Token } from 'aws-cdk'; +import { Arn, Construct, PolicyStatement, ServicePrincipal, Token } from 'aws-cdk'; import { Role } from 'aws-cdk-iam'; import { lambda } from 'aws-cdk-resources'; import { LambdaCode } from './code'; @@ -72,10 +72,11 @@ export interface LambdaProps { memorySize?: number; /** - * Additional permissions to add to the created Lambda Role. - * You can also call addToRolePolicy to the created lambda. + * Initial policy statements to add to the created Lambda Role. + * + * You can call `addToRolePolicy` to the created lambda to add statements post creation. */ - additionalPermissions?: PolicyStatement[]; + initialPolicyStatements?: PolicyStatement[]; } /** @@ -119,10 +120,18 @@ export class Lambda extends LambdaRef { this.role = new Role(this, 'ServiceRole', { assumedBy: new ServicePrincipal('lambda.amazonaws.com'), - managedPolicyArns: [ new FnConcat('arn:', new AwsPartition(), ':iam::aws:policy/service-role/AWSLambdaBasicExecutionRole')], + // the arn is in the form of - arn:aws:iam::aws:policy/service-role/AWSLambdaBasicExecutionRole + managedPolicyArns: [ Arn.fromComponents({ + service: "iam", + region: "", // no region for managed policy + account: "aws", // the account for a managed policy is 'aws' + resource: "policy", + resourceName: "service-role/AWSLambdaBasicExecutionRole", + })], }); - if (props.additionalPermissions && props.additionalPermissions.length > 0) { - props.additionalPermissions.forEach(permission => this.role!.addToPolicy(permission)); + + for (const statement of (props.initialPolicyStatements || [])) { + this.role.addToPolicy(statement); } const resource = new lambda.FunctionResource(this, 'Resource', { diff --git a/packages/aws-cdk-lambda/test/inline.expected.json b/packages/aws-cdk-lambda/test/inline.expected.json index 3c2bcb12c340f..326a26384fa91 100644 --- a/packages/aws-cdk-lambda/test/inline.expected.json +++ b/packages/aws-cdk-lambda/test/inline.expected.json @@ -74,7 +74,8 @@ "Version": "2012-10-17" }, "ManagedPolicyArns": [ - {"Fn::Join" : ["", ["arn:", {"Ref": "AWS::Partition"}, ":iam::aws:policy/service-role/AWSLambdaBasicExecutionRole"]]} + {"Fn::Join": ["", ["arn", ":", {"Ref": "AWS::Partition"}, ":", "iam", ":", "", ":", "aws", ":", "policy", "/", "service-role/AWSLambdaBasicExecutionRole"]]} + ] } }, diff --git a/packages/aws-cdk-lambda/test/integ.inline.expected.json b/packages/aws-cdk-lambda/test/integ.inline.expected.json index c923a0a94f44f..3653c0d5e7ad0 100644 --- a/packages/aws-cdk-lambda/test/integ.inline.expected.json +++ b/packages/aws-cdk-lambda/test/integ.inline.expected.json @@ -74,7 +74,7 @@ "Version": "2012-10-17" }, "ManagedPolicyArns": [ - {"Fn::Join" : ["", ["arn:", {"Ref": "AWS::Partition"}, ":iam::aws:policy/service-role/AWSLambdaBasicExecutionRole"]]} + {"Fn::Join": ["", ["arn", ":", {"Ref": "AWS::Partition"}, ":", "iam", ":", "", ":", "aws", ":", "policy", "/", "service-role/AWSLambdaBasicExecutionRole"]]} ] } }, diff --git a/packages/aws-cdk-lambda/test/integ.lambda.expected.json b/packages/aws-cdk-lambda/test/integ.lambda.expected.json index 12114b38c688a..b1410c9c7e248 100644 --- a/packages/aws-cdk-lambda/test/integ.lambda.expected.json +++ b/packages/aws-cdk-lambda/test/integ.lambda.expected.json @@ -16,7 +16,7 @@ "Version": "2012-10-17" }, "ManagedPolicyArns": [ - {"Fn::Join" : ["", ["arn:", {"Ref": "AWS::Partition"}, ":iam::aws:policy/service-role/AWSLambdaBasicExecutionRole"]]} + {"Fn::Join": ["", ["arn", ":", {"Ref": "AWS::Partition"}, ":", "iam", ":", "", ":", "aws", ":", "policy", "/", "service-role/AWSLambdaBasicExecutionRole"]]} ] } }, diff --git a/packages/aws-cdk-lambda/test/test.lambda.ts b/packages/aws-cdk-lambda/test/test.lambda.ts index 8fcc4e7f66138..65018331ad7e8 100644 --- a/packages/aws-cdk-lambda/test/test.lambda.ts +++ b/packages/aws-cdk-lambda/test/test.lambda.ts @@ -26,7 +26,9 @@ export = { Principal: { Service: 'lambda.amazonaws.com' } } ], Version: '2012-10-17' }, ManagedPolicyArns: - [{'Fn::Join': ['', ['arn:', {Ref: 'AWS::Partition'}, ':iam::aws:policy/service-role/AWSLambdaBasicExecutionRole']]}], + // arn:aws:iam::aws:policy/service-role/AWSLambdaBasicExecutionRole + // tslint:disable-next-line:max-line-length + [{'Fn::Join': ['', ['arn', ':', {Ref: 'AWS::Partition'}, ':', 'iam', ':', '', ':', 'aws', ':', 'policy', '/', 'service-role/AWSLambdaBasicExecutionRole']]}], }}, MyLambdaCCE802FB: { Type: 'AWS::Lambda::Function', @@ -45,7 +47,7 @@ export = { code: new LambdaInlineCode('foo'), handler: 'index.handler', runtime: LambdaRuntime.NodeJS610, - additionalPermissions: [new PolicyStatement().addAction("*").addResource("*")] + initialPolicyStatements: [new PolicyStatement().addAction("*").addResource("*")] }); expect(stack).toMatch({ Resources: { MyLambdaServiceRole4539ECB6: @@ -58,7 +60,8 @@ export = { Principal: { Service: 'lambda.amazonaws.com' } } ], Version: '2012-10-17' }, ManagedPolicyArns: - [{'Fn::Join': ['', ['arn:', {Ref: 'AWS::Partition'}, ':iam::aws:policy/service-role/AWSLambdaBasicExecutionRole']]}] + // tslint:disable-next-line:max-line-length + [{'Fn::Join': ['', ['arn', ':', {Ref: 'AWS::Partition'}, ':', 'iam', ':', '', ':', 'aws', ':', 'policy', '/', 'service-role/AWSLambdaBasicExecutionRole']]}], }}, MyLambdaServiceRoleDefaultPolicy5BBC6F68: { Type: "AWS::IAM::Policy", @@ -133,7 +136,8 @@ export = { "Version": "2012-10-17" }, "ManagedPolicyArns": - [{'Fn::Join': ['', ['arn:', {Ref: 'AWS::Partition'}, ':iam::aws:policy/service-role/AWSLambdaBasicExecutionRole']]}], + // tslint:disable-next-line:max-line-length + [{'Fn::Join': ['', ['arn', ':', {Ref: 'AWS::Partition'}, ':', 'iam', ':', '', ':', 'aws', ':', 'policy', '/', 'service-role/AWSLambdaBasicExecutionRole']]}], } }, "MyLambdaCCE802FB": { From 3bb6a24a482dfa201d68e4e85561269f33d73b6c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=F0=9F=8C=88=20=F0=9F=92=BB=20=20Miley?= Date: Mon, 4 Jun 2018 14:00:05 -0700 Subject: [PATCH 3/3] Renamed initialPolicyStatements to initalPolicy --- packages/aws-cdk-lambda/lib/lambda.ts | 4 ++-- packages/aws-cdk-lambda/test/test.lambda.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/aws-cdk-lambda/lib/lambda.ts b/packages/aws-cdk-lambda/lib/lambda.ts index 2fc9fb6ccbe57..84922d12d28c3 100644 --- a/packages/aws-cdk-lambda/lib/lambda.ts +++ b/packages/aws-cdk-lambda/lib/lambda.ts @@ -76,7 +76,7 @@ export interface LambdaProps { * * You can call `addToRolePolicy` to the created lambda to add statements post creation. */ - initialPolicyStatements?: PolicyStatement[]; + initialPolicy?: PolicyStatement[]; } /** @@ -130,7 +130,7 @@ export class Lambda extends LambdaRef { })], }); - for (const statement of (props.initialPolicyStatements || [])) { + for (const statement of (props.initialPolicy || [])) { this.role.addToPolicy(statement); } diff --git a/packages/aws-cdk-lambda/test/test.lambda.ts b/packages/aws-cdk-lambda/test/test.lambda.ts index 65018331ad7e8..aaf09a60fd3ab 100644 --- a/packages/aws-cdk-lambda/test/test.lambda.ts +++ b/packages/aws-cdk-lambda/test/test.lambda.ts @@ -47,7 +47,7 @@ export = { code: new LambdaInlineCode('foo'), handler: 'index.handler', runtime: LambdaRuntime.NodeJS610, - initialPolicyStatements: [new PolicyStatement().addAction("*").addResource("*")] + initialPolicy: [new PolicyStatement().addAction("*").addResource("*")] }); expect(stack).toMatch({ Resources: { MyLambdaServiceRole4539ECB6: