Skip to content

Commit

Permalink
fix(lambda): unlock use case for cross-account functions w/ preconfig…
Browse files Browse the repository at this point in the history
…ured permissions (#18979)

There have been a few reports of regressions after #18228 was merged in v1.139.0: #18781, #18967.

AFAIK, #18228 fixed a bug in our environment configuring logic for `fromFunctionAttributes()`, so the environment is now being properly passed to the `functionBase` that CDK creates. The bug was being relied on by CDK customers for a particular use case: calling `grantInvoke()` on imported cross-account lambdas with pre-configured permissions. In general, `grantInvoke()` modifies the trust policy of the resource if it is not from the same environment. In the case of cross-account lambdas, this is impossible, and results in a synth-time error. CDK users know this and have modified the lambda permissions in question to have the correct permissions. These users do not need permissions added to the cross-account lambda, so it is not an error.

To support this use case, I'm introducing `skipPermissions` as a property for `fromFunctionAttributes()`. When set, we skip this synth-time check all together. Beware, users who set this property are now on their own for the permissions of their cross-account lambda.

```ts
const fn = lambda.Function.fromFunctionAttributes(stack, 'Function', {
  functionArn: 'arn:aws:lambda:us-east-1:123456789012:function:MyFn',
  skipPermissions: true,
});
```

Closes #18781. 

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
kaizencc committed Feb 16, 2022
1 parent 45279e3 commit 023108a
Show file tree
Hide file tree
Showing 4 changed files with 76 additions and 6 deletions.
29 changes: 29 additions & 0 deletions packages/@aws-cdk/aws-lambda/README.md
Expand Up @@ -480,6 +480,35 @@ fn.addEventSource(new eventsources.S3EventSource(bucket, {

See the documentation for the __@aws-cdk/aws-lambda-event-sources__ module for more details.

## Imported Lambdas

When referencing an imported lambda in the CDK, use `fromFunctionArn()` for most use cases:

```ts
const fn = lambda.Function.fromFunctionArn(
this,
'Function',
'arn:aws:lambda:us-east-1:123456789012:function:MyFn',
);
```

The `fromFunctionAttributes()` API is available for more specific use cases:

```ts
const fn = lambda.Function.fromFunctionAttributes(this, 'Function', {
functionArn: 'arn:aws:lambda:us-east-1:123456789012:function:MyFn',
// The following are optional properties for specific use cases and should be used with caution:

// Use Case: imported function is in the same account as the stack. This tells the CDK that it
// can modify the function's permissions.
sameEnvironment: true,

// Use Case: imported function is in a different account and user commits to ensuring that the
// imported function has the correct permissions outside the CDK.
skipPermissions: true,
});
```

## Lambda with DLQ

A dead-letter queue can be automatically created for a Lambda function by
Expand Down
30 changes: 27 additions & 3 deletions packages/@aws-cdk/aws-lambda/lib/function-base.ts
Expand Up @@ -180,6 +180,20 @@ export interface FunctionAttributes {
*/
readonly sameEnvironment?: boolean;

/**
* Setting this property informs the CDK that the imported function ALREADY HAS the necessary permissions
* for what you are trying to do. When not configured, the CDK attempts to auto-determine whether or not
* additional permissions are necessary on the function when grant APIs are used. If the CDK tried to add
* permissions on an imported lambda, it will fail.
*
* Set this property *ONLY IF* you are committing to manage the imported function's permissions outside of
* CDK. You are acknowledging that your CDK code alone will have insufficient permissions to access the
* imported function.
*
* @default false
*/
readonly skipPermissions?: boolean;

/**
* The architecture of this Lambda Function (this is an optional attribute and defaults to X86_64).
* @default - Architecture.X86_64
Expand Down Expand Up @@ -228,6 +242,15 @@ export abstract class FunctionBase extends Resource implements IFunction, ec2.IC
*/
protected abstract readonly canCreatePermissions: boolean;

/**
* Whether the user decides to skip adding permissions.
* The only use case is for cross-account, imported lambdas
* where the user commits to modifying the permisssions
* on the imported lambda outside CDK.
* @internal
*/
protected readonly _skipPermissions?: boolean;

/**
* Actual connections object for this Lambda
*
Expand Down Expand Up @@ -342,9 +365,10 @@ export abstract class FunctionBase extends Resource implements IFunction, ec2.IC
});

const permissionNode = this._functionNode().tryFindChild(identifier);
if (!permissionNode) {
throw new Error('Cannot modify permission to lambda function. Function is either imported or $LATEST version. '
+ 'If the function is imported from the same account use `fromFunctionAttributes()` API with the `sameEnvironment` flag.');
if (!permissionNode && !this._skipPermissions) {
throw new Error('Cannot modify permission to lambda function. Function is either imported or $LATEST version.\n'
+ 'If the function is imported from the same account use `fromFunctionAttributes()` API with the `sameEnvironment` flag.\n'
+ 'If the function is imported from a different account and already has the correct permissions use `fromFunctionAttributes()` API with the `skipPermissions` flag.');
}
return { statementAdded: true, policyDependable: permissionNode };
},
Expand Down
1 change: 1 addition & 0 deletions packages/@aws-cdk/aws-lambda/lib/function.ts
Expand Up @@ -453,6 +453,7 @@ export class Function extends FunctionBase {
public readonly architecture = attrs.architecture ?? Architecture.X86_64;

protected readonly canCreatePermissions = attrs.sameEnvironment ?? this._isStackAccount();
protected readonly _skipPermissions = attrs.skipPermissions ?? false;

constructor(s: Construct, i: string) {
super(s, i, {
Expand Down
22 changes: 19 additions & 3 deletions packages/@aws-cdk/aws-lambda/test/function.test.ts
Expand Up @@ -845,7 +845,6 @@ describe('function', () => {
});

describe('grantInvoke', () => {

test('adds iam:InvokeFunction', () => {
// GIVEN
const stack = new cdk.Stack();
Expand Down Expand Up @@ -1091,8 +1090,25 @@ describe('function', () => {
const fn = lambda.Function.fromFunctionArn(stack, 'Function', 'arn:aws:lambda:us-east-1:123456789012:function:MyFn');

// THEN
expect(() => { fn.grantInvoke(new iam.ServicePrincipal('elasticloadbalancing.amazonaws.com')); })
.toThrow(/Cannot modify permission to lambda function/);
expect(() => {
fn.grantInvoke(new iam.ServicePrincipal('elasticloadbalancing.amazonaws.com'));
}).toThrow(/Cannot modify permission to lambda function/);
});

test('on an imported function (different account & w/ skipPermissions', () => {
// GIVEN
const stack = new cdk.Stack(undefined, undefined, {
env: { account: '111111111111' }, // Different account
});
const fn = lambda.Function.fromFunctionAttributes(stack, 'Function', {
functionArn: 'arn:aws:lambda:us-east-1:123456789012:function:MyFn',
skipPermissions: true,
});

// THEN
expect(() => {
fn.grantInvoke(new iam.ServicePrincipal('elasticloadbalancing.amazonaws.com'));
}).not.toThrow();
});
});

Expand Down

0 comments on commit 023108a

Please sign in to comment.