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(iot): unable to add the same lambda function to two TopicRule Actions #17521

Merged
merged 7 commits into from
Nov 18, 2021
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import * as iam from '@aws-cdk/aws-iam';
import * as iot from '@aws-cdk/aws-iot';
import * as lambda from '@aws-cdk/aws-lambda';
import { Names } from '@aws-cdk/core';

/**
* The action to invoke an AWS Lambda function, passing in an MQTT message.
Expand All @@ -12,7 +13,7 @@ export class LambdaFunctionAction implements iot.IAction {
constructor(private readonly func: lambda.IFunction) {}

bind(topicRule: iot.ITopicRule): iot.ActionConfig {
this.func.addPermission('invokedByAwsIotRule', {
this.func.addPermission(`${Names.nodeUniqueId(topicRule.node)}:Permission`, {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would make the "Permission" part more module-specific. Something like:

Suggested change
this.func.addPermission(`${Names.nodeUniqueId(topicRule.node)}:Permission`, {
this.func.addPermission(`${Names.nodeUniqueId(topicRule.node)}:IotLambdaFunctionAction`, {

action: 'lambda:InvokeFunction',
principal: new iam.ServicePrincipal('iot.amazonaws.com'),
sourceAccount: topicRule.env.account,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
"MyFunctionServiceRole3C357FF2"
]
},
"MyFunctioninvokedByAwsIotRule5581F304": {
"MyFunctionteststackTopicRule1CB8242FPermission62920382": {
"Type": "AWS::Lambda::Permission",
"Properties": {
"Action": "lambda:InvokeFunction",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,3 +55,26 @@ test('create a topic rule with lambda action and a lambda permission to be invok
},
});
});

test('When two topic rules have the same action, it should not throw a error', () => {
// GIVEN
const stack = new cdk.Stack();
const func = new lambda.Function(stack, 'MyFunction', {
runtime: lambda.Runtime.NODEJS_14_X,
handler: 'index.handler',
code: lambda.Code.fromInline('console.log("foo")'),
});
const action = new actions.LambdaFunctionAction(func);

// WHEN
new iot.TopicRule(stack, 'MyTopicRule1', {
sql: iot.IotSql.fromStringAsVer20160323("SELECT topic(2) as device_id FROM 'device/+/data'"),
actions: [action],
});

// THEN
expect(() => new iot.TopicRule(stack, 'MyTopicRule2', {
sql: iot.IotSql.fromStringAsVer20160323("SELECT topic(2) as device_id FROM 'device/+/data'"),
actions: [action],
})).not.toThrow();
yamatatsu marked this conversation as resolved.
Show resolved Hide resolved
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about we actually assert that two different permissions are created, instead of just asserting the lack of an exception being thrown?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's good!