Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(lambda): unable to add permissions to imported lambda functions #8828

Merged
merged 31 commits into from
Sep 21, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
21ae4fe
prelim changes just to put out
BryanPan342 Jul 1, 2020
ed3590b
add integ test for import permission adding
BryanPan342 Jul 29, 2020
16d4dc4
bug fix
BryanPan342 Jul 29, 2020
ae17f66
prelim edits
BryanPan342 Aug 6, 2020
de5e4d7
Merge branch 'master' into lambda-permissions
BryanPan342 Aug 17, 2020
f3c77e3
refactor(appsync): graphQLApi to graphqlApi for better snakecasing
BryanPan342 Aug 31, 2020
51edc01
Revert "refactor(appsync): graphQLApi to graphqlApi for better snakec…
BryanPan342 Aug 31, 2020
884e185
Merge remote-tracking branch 'upstream/master'
BryanPan342 Aug 31, 2020
9ad9d36
Merge remote-tracking branch 'upstream/master'
BryanPan342 Sep 1, 2020
d9ebafb
Merge remote-tracking branch 'upstream/master'
BryanPan342 Sep 1, 2020
64db87e
Merge remote-tracking branch 'upstream/master'
BryanPan342 Sep 1, 2020
b3622b0
Merge remote-tracking branch 'upstream/master'
BryanPan342 Sep 1, 2020
4cdd244
Merge remote-tracking branch 'upstream/master'
BryanPan342 Sep 2, 2020
6608704
Merge remote-tracking branch 'upstream/master'
BryanPan342 Sep 3, 2020
f85ce81
Merge remote-tracking branch 'upstream/master'
BryanPan342 Sep 4, 2020
b8d5ba6
update to check if account and functionArn is resolved
BryanPan342 Sep 4, 2020
c48ca51
remove unnecessary code
BryanPan342 Sep 5, 2020
e8cfc0c
Merge remote-tracking branch 'upstream/master'
BryanPan342 Sep 8, 2020
55b73c2
Merge branch 'master' into lambda-permissions
BryanPan342 Sep 8, 2020
1423bc6
Merge remote-tracking branch 'upstream/master'
BryanPan342 Sep 8, 2020
2ea72ed
Merge remote-tracking branch 'upstream/master'
BryanPan342 Sep 8, 2020
322d9a1
Merge remote-tracking branch 'upstream/master'
BryanPan342 Sep 9, 2020
0f1b683
Merge remote-tracking branch 'upstream/master'
BryanPan342 Sep 10, 2020
d1f4956
Merge remote-tracking branch 'upstream/master'
BryanPan342 Sep 11, 2020
056df67
Merge remote-tracking branch 'upstream/master'
BryanPan342 Sep 11, 2020
b564177
Merge remote-tracking branch 'upstream/master'
BryanPan342 Sep 11, 2020
c745df5
Merge remote-tracking branch 'upstream/master'
Sep 15, 2020
0bec4e1
Merge remote-tracking branch 'upstream/master'
BryanPan342 Sep 16, 2020
b87e18b
Merge branch 'lambda-permissions' of https://github.com/bryanpan342/a…
BryanPan342 Sep 16, 2020
5d73d57
address comments
BryanPan342 Sep 16, 2020
b809d51
Merge branch 'master' into lambda-permissions
mergify[bot] Sep 21, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-lambda/lib/alias.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ export class Alias extends QualifiedFunctionBase implements IAlias {
public readonly grantPrincipal = attrs.aliasVersion.grantPrincipal;
public readonly role = attrs.aliasVersion.role;

protected readonly canCreatePermissions = false;
protected readonly canCreatePermissions = this._isStackAccount();
protected readonly qualifier = attrs.aliasName;
}
return new Imported(scope, id);
Expand Down
28 changes: 25 additions & 3 deletions packages/@aws-cdk/aws-lambda/lib/function-base.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import * as cloudwatch from '@aws-cdk/aws-cloudwatch';
import * as ec2 from '@aws-cdk/aws-ec2';
import * as iam from '@aws-cdk/aws-iam';
import { ConstructNode, IResource, Resource } from '@aws-cdk/core';
import { ConstructNode, IResource, Resource, Token } from '@aws-cdk/core';
import { AliasOptions } from './alias';
import { EventInvokeConfig, EventInvokeConfigOptions } from './event-invoke-config';
import { IEventSource } from './event-source';
Expand Down Expand Up @@ -182,7 +182,8 @@ export abstract class FunctionBase extends Resource implements IFunction {
/**
* Whether the addPermission() call adds any permissions
*
* True for new Lambdas, false for imported Lambdas (they might live in different accounts).
* True for new Lambdas, false for version $LATEST and imported Lambdas
* from different accounts.
*/
protected abstract readonly canCreatePermissions: boolean;

Expand Down Expand Up @@ -346,6 +347,27 @@ export abstract class FunctionBase extends Resource implements IFunction {
return this.node;
}

/**
* Given the function arn, check if the account id matches this account
*
* Function ARNs look like this:
*
* arn:aws:lambda:region:account-id:function:function-name
*
* ..which means that in order to extract the `account-id` component from the ARN, we can
* split the ARN using ":" and select the component in index 4.
*
* @returns true if account id of function matches this account
*
* @internal
*/
protected _isStackAccount(): boolean {
if (Token.isUnresolved(this.stack.account) || Token.isUnresolved(this.functionArn)) {
return false;
}
return this.stack.parseArn(this.functionArn).account === this.stack.account;
}

/**
* Translate IPrincipal to something we can pass to AWS::Lambda::Permissions
*
Expand Down Expand Up @@ -431,7 +453,7 @@ class LatestVersion extends FunctionBase implements IVersion {
public readonly version = '$LATEST';
public readonly permissionsNode = this.node;

protected readonly canCreatePermissions = true;
protected readonly canCreatePermissions = false;

constructor(lambda: FunctionBase) {
super(lambda, '$LATEST');
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-lambda/lib/function.ts
Original file line number Diff line number Diff line change
Expand Up @@ -382,7 +382,7 @@ export class Function extends FunctionBase {
public readonly role = role;
public readonly permissionsNode = this.node;

protected readonly canCreatePermissions = false;
protected readonly canCreatePermissions = this._isStackAccount();

constructor(s: Construct, i: string) {
super(s, i);
Expand Down
4 changes: 2 additions & 2 deletions packages/@aws-cdk/aws-lambda/lib/lambda-version.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ export class Version extends QualifiedFunctionBase implements IVersion {
public readonly role = lambda.role;

protected readonly qualifier = version;
protected readonly canCreatePermissions = false;
protected readonly canCreatePermissions = this._isStackAccount();

public addAlias(name: string, opts: AliasOptions = { }): Alias {
return addAlias(this, this, name, opts);
Expand All @@ -154,7 +154,7 @@ export class Version extends QualifiedFunctionBase implements IVersion {
public readonly role = attrs.lambda.role;

protected readonly qualifier = attrs.version;
protected readonly canCreatePermissions = false;
protected readonly canCreatePermissions = this._isStackAccount();

public addAlias(name: string, opts: AliasOptions = { }): Alias {
return addAlias(this, this, name, opts);
Expand Down
53 changes: 53 additions & 0 deletions packages/@aws-cdk/aws-lambda/test/test.lambda.ts
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,59 @@ export = {
// THEN
test.deepEqual(imported.functionArn, 'arn:aws:lambda:us-east-1:123456789012:function:ProcessKinesisRecords');
test.deepEqual(imported.functionName, 'ProcessKinesisRecords');
expect(stack2).notTo(haveResource('AWS::Lambda::Permission'));
test.done();
},

'imported Function w/ resolved account and function arn can addPermissions'(test: Test) {
nija-at marked this conversation as resolved.
Show resolved Hide resolved
// GIVEN
const app = new cdk.App();
const stack = new cdk.Stack(app, 'Imports', {
env: { account: '123456789012', region: 'us-east-1' },
});
const stack2 = new cdk.Stack(app, 'imported', {
env: { account: '123456789012', region: 'us-east-1' },
});
new lambda.Function(stack, 'BaseFunction', {
code: new lambda.InlineCode('foo'),
handler: 'index.handler',
runtime: lambda.Runtime.NODEJS_10_X,
});

// WHEN
const iFunc = lambda.Function.fromFunctionAttributes(stack2, 'iFunc', {
functionArn: 'arn:aws:lambda:us-east-1:123456789012:function:BaseFunction',
});
iFunc.addPermission('iFunc', {
principal: new iam.ServicePrincipal('cloudformation.amazonaws.com'),
});

// THEN
expect(stack2).to(haveResource('AWS::Lambda::Permission'));
test.done();
},

'imported Function w/o account cannot addPermissions'(test: Test) {
// GIVEN
const app = new cdk.App();
const stack = new cdk.Stack(app, 'Base');
const importedStack = new cdk.Stack(app, 'Imported');
new lambda.Function(stack, 'BaseFunction', {
code: new lambda.InlineCode('foo'),
handler: 'index.handler',
runtime: lambda.Runtime.NODEJS_10_X,
});

// WHEN
const iFunc = lambda.Function.fromFunctionAttributes(importedStack, 'iFunc', {
functionArn: 'arn:aws:lambda:us-east-1:123456789012:function:BaseFunction',
});
iFunc.addPermission('iFunc', {
principal: new iam.ServicePrincipal('cloudformation.amazonaws.com'),
});

// THEN
expect(importedStack).notTo(haveResource('AWS::Lambda::Permission'));
test.done();
},

Expand Down