Skip to content
This repository has been archived by the owner on Nov 22, 2022. It is now read-only.

Commit

Permalink
fix(lambda-event-sources): cannot add sqs event source to an imported…
Browse files Browse the repository at this point in the history
… function (aws#21970)

If an SQS event sources is added to an imported function it will throw an error if the function is not imported with an IAM role.

This PR updates the logic to only attempt to add permissions to the principal if the role exists, otherwise it will add a warning indicating that permissions were not added.

fixes aws#12607


----

### All Submissions:

* [ ] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
corymhall authored and Kruspe committed Sep 13, 2022
1 parent 34e2828 commit a8b8197
Show file tree
Hide file tree
Showing 2 changed files with 118 additions and 2 deletions.
11 changes: 9 additions & 2 deletions packages/@aws-cdk/aws-lambda-event-sources/lib/sqs.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import * as lambda from '@aws-cdk/aws-lambda';
import * as sqs from '@aws-cdk/aws-sqs';
import { Duration, Names, Token } from '@aws-cdk/core';
import { Duration, Names, Token, Annotations } from '@aws-cdk/core';

export interface SqsEventSourceProps {
/**
Expand Down Expand Up @@ -84,7 +84,14 @@ export class SqsEventSource implements lambda.IEventSource {
});
this._eventSourceMappingId = eventSourceMapping.eventSourceMappingId;

this.queue.grantConsumeMessages(target);
// only grant access if the lambda function has an IAM role
// otherwise the IAM module will throw an error
if (target.role) {
this.queue.grantConsumeMessages(target);
} else {
Annotations.of(target).addWarning(`Function '${target.node.path}' was imported without an IAM role `+
`so it was not granted access to consume messages from '${this.queue.node.path}'`);
}
}

/**
Expand Down
109 changes: 109 additions & 0 deletions packages/@aws-cdk/aws-lambda-event-sources/test/sqs.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import { Template } from '@aws-cdk/assertions';
import * as iam from '@aws-cdk/aws-iam';
import * as lambda from '@aws-cdk/aws-lambda';
import * as sqs from '@aws-cdk/aws-sqs';
import * as cdk from '@aws-cdk/core';
import { App } from '@aws-cdk/core';
import * as sources from '../lib';
import { TestFunction } from './test-function';

Expand Down Expand Up @@ -282,8 +284,115 @@ describe('SQSEventSource', () => {
Template.fromStack(stack).hasResourceProperties('AWS::Lambda::EventSourceMapping', {
'FunctionResponseTypes': ['ReportBatchItemFailures'],
});
});

test('warning added if lambda function imported without role', () => {
const app = new App();
const stack = new cdk.Stack(app);
const fn = lambda.Function.fromFunctionName(stack, 'Handler', 'testFunction');
const q = new sqs.Queue(stack, 'Q');

// WHEN
fn.addEventSource(new sources.SqsEventSource(q));
const assembly = app.synth();

const messages = assembly.getStackArtifact(stack.artifactId).messages;

// THEN
expect(messages.length).toEqual(1);
expect(messages[0]).toMatchObject({
level: 'warning',
id: '/Default/Handler',
entry: {
data: expect.stringMatching(/Function 'Default\/Handler' was imported without an IAM role/),
},
});

// THEN
Template.fromStack(stack).resourceCountIs('AWS::Lambda::EventSourceMapping', 1);
Template.fromStack(stack).resourceCountIs('AWS::IAM::Policy', 0);
});

test('policy added to imported function role', () => {
// GIVEN
const stack = new cdk.Stack();
const fn = lambda.Function.fromFunctionAttributes(stack, 'Handler', {
functionArn: stack.formatArn({
service: 'lambda',
resource: 'function',
resourceName: 'testFunction',
}),
role: iam.Role.fromRoleName(stack, 'Role', 'testFunctionRole'),
});
const q = new sqs.Queue(stack, 'Q');

// WHEN
fn.addEventSource(new sources.SqsEventSource(q));

// THEN
Template.fromStack(stack).hasResourceProperties('AWS::IAM::Policy', {
'PolicyDocument': {
'Statement': [
{
'Action': [
'sqs:ReceiveMessage',
'sqs:ChangeMessageVisibility',
'sqs:GetQueueUrl',
'sqs:DeleteMessage',
'sqs:GetQueueAttributes',
],
'Effect': 'Allow',
'Resource': {
'Fn::GetAtt': [
'Q63C6E3AB',
'Arn',
],
},
},
],
'Version': '2012-10-17',
},
'Roles': ['testFunctionRole'],
});

Template.fromStack(stack).hasResourceProperties('AWS::Lambda::EventSourceMapping', {
'EventSourceArn': {
'Fn::GetAtt': [
'Q63C6E3AB',
'Arn',
],
},
'FunctionName': {
'Fn::Select': [
6,
{
'Fn::Split': [
':',
{
'Fn::Join': [
'',
[
'arn:',
{
'Ref': 'AWS::Partition',
},
':lambda:',
{
'Ref': 'AWS::Region',
},
':',
{
'Ref': 'AWS::AccountId',
},
':function/testFunction',
],
],
},
],
},
],
},
});
});

test('adding filter criteria', () => {
Expand Down

0 comments on commit a8b8197

Please sign in to comment.