Skip to content

Commit

Permalink
fix(aws-lambda): CloudWatch event rule permissions (#558)
Browse files Browse the repository at this point in the history
Lambda permissions granted when it was added as an event rule target
did not include "SourceArn" as required. This allowed any event rule
to trigger the function, and also did not show as a trigger in the AWS
Lambda console.

Added a integration test to verify.

BREAKING CHANGE

To fix this, we needed to modify `IEventRuleTarget` to pass the ARN of
the rule and a unique ID to the registered target in order to allow it
to specify the Source ARN. This required fixing all existing event rule
targets (which, so far would return a role to be assumed by CWE, so the
source ARN was not required).

Fixes #555
  • Loading branch information
Elad Ben-Israel committed Aug 14, 2018
1 parent 0efe25b commit 327f5e1
Show file tree
Hide file tree
Showing 11 changed files with 235 additions and 34 deletions.
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-codebuild/lib/project.ts
Expand Up @@ -254,7 +254,7 @@ export abstract class ProjectRef extends cdk.Construct implements events.IEventR
/**
* Allows using build projects as event rule targets.
*/
public get eventRuleTarget(): events.EventRuleTargetProps {
public asEventRuleTarget(_ruleArn: events.RuleArn, _ruleId: string): events.EventRuleTargetProps {
if (!this.eventsRole) {
this.eventsRole = new iam.Role(this, 'EventsRole', {
assumedBy: new cdk.ServicePrincipal('events.amazonaws.com')
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-codepipeline/lib/pipeline.ts
Expand Up @@ -148,7 +148,7 @@ export class Pipeline extends cdk.Construct implements events.IEventRuleTarget {
* rule.addTarget(pipeline);
*
*/
public get eventRuleTarget(): events.EventRuleTargetProps {
public asEventRuleTarget(_ruleArn: events.RuleArn, _ruleId: string): events.EventRuleTargetProps {
// the first time the event rule target is retrieved, we define an IAM
// role assumable by the CloudWatch events service which is allowed to
// start the execution of this pipeline. no need to define more than one
Expand Down
8 changes: 5 additions & 3 deletions packages/@aws-cdk/aws-events/lib/rule.ts
Expand Up @@ -100,13 +100,15 @@ export class EventRule extends EventRuleRef {
public addTarget(target?: IEventRuleTarget, inputOptions?: TargetInputTemplate) {
if (!target) { return; }

const targetProps = target.asEventRuleTarget(this.ruleArn, this.uniqueId);

// check if a target with this ID already exists
if (this.targets.find(t => t.id === target.eventRuleTarget.id)) {
throw new Error('Duplicate event rule target with ID: ' + target.eventRuleTarget.id);
if (this.targets.find(t => t.id === targetProps.id)) {
throw new Error('Duplicate event rule target with ID: ' + targetProps.id);
}

this.targets.push({
...target.eventRuleTarget,
...targetProps,
inputTransformer: renderTransformer(),
});

Expand Down
7 changes: 5 additions & 2 deletions packages/@aws-cdk/aws-events/lib/target.ts
@@ -1,6 +1,6 @@
import iam = require('@aws-cdk/aws-iam');
import cdk = require('@aws-cdk/cdk');
import { cloudformation } from './events.generated';
import { cloudformation, RuleArn } from './events.generated';

export interface EventRuleTargetProps {
/**
Expand Down Expand Up @@ -50,6 +50,9 @@ export interface IEventRuleTarget {
/**
* Returns the rule target specification.
* NOTE: Do not use the various `inputXxx` options. They can be set in a call to `addTarget`.
*
* @param ruleArn The ARN of the CloudWatch Event Rule that would trigger this target.
* @param ruleUniqueId A unique ID for this rule. Can be used to implement idempotency.
*/
readonly eventRuleTarget: EventRuleTargetProps;
asEventRuleTarget(ruleArn: RuleArn, ruleUniqueId: string): EventRuleTargetProps;
}
48 changes: 39 additions & 9 deletions packages/@aws-cdk/aws-events/test/test.rule.ts
@@ -1,8 +1,9 @@
import { expect } from '@aws-cdk/assert';
import iam = require('@aws-cdk/aws-iam');
import cdk = require('@aws-cdk/cdk');
import { resolve } from '@aws-cdk/cdk';
import { Test } from 'nodeunit';
import { IEventRuleTarget } from '../lib';
import { IEventRuleTarget, RuleArn } from '../lib';
import { EventRule } from '../lib/rule';

// tslint:disable:object-literal-key-quotes
Expand Down Expand Up @@ -137,19 +138,19 @@ export = {
'targets can be added via props or addTarget with input transformer'(test: Test) {
const stack = new cdk.Stack();
const t1: IEventRuleTarget = {
eventRuleTarget: {
asEventRuleTarget: () => ({
id: 'T1',
arn: new cdk.Arn('ARN1'),
kinesisParameters: { partitionKeyPath: 'partitionKeyPath' }
}
})
};

const t2: IEventRuleTarget = {
eventRuleTarget: {
asEventRuleTarget: () => ({
id: 'T2',
arn: new cdk.Arn('ARN2'),
roleArn: new iam.RoleArn('IAM-ROLE-ARN')
}
})
};

const rule = new EventRule(stack, 'EventRule', {
Expand Down Expand Up @@ -201,12 +202,14 @@ export = {
'input template can contain tokens'(test: Test) {
const stack = new cdk.Stack();
const t1: IEventRuleTarget = {
eventRuleTarget: { id: 'T1', arn: new cdk.Arn('ARN1'), kinesisParameters: { partitionKeyPath: 'partitionKeyPath' } }
asEventRuleTarget: () => ({
id: 'T1', arn: new cdk.Arn('ARN1'), kinesisParameters: { partitionKeyPath: 'partitionKeyPath' }
})
};

const t2: IEventRuleTarget = { eventRuleTarget: { id: 'T2', arn: new cdk.Arn('ARN2'), roleArn: new iam.RoleArn('IAM-ROLE-ARN') } };
const t3: IEventRuleTarget = { eventRuleTarget: { id: 'T3', arn: new cdk.Arn('ARN3') } };
const t4: IEventRuleTarget = { eventRuleTarget: { id: 'T4', arn: new cdk.Arn('ARN4') } };
const t2: IEventRuleTarget = { asEventRuleTarget: () => ({ id: 'T2', arn: new cdk.Arn('ARN2'), roleArn: new iam.RoleArn('IAM-ROLE-ARN') }) };
const t3: IEventRuleTarget = { asEventRuleTarget: () => ({ id: 'T3', arn: new cdk.Arn('ARN3') }) };
const t4: IEventRuleTarget = { asEventRuleTarget: () => ({ id: 'T4', arn: new cdk.Arn('ARN4') }) };

const rule = new EventRule(stack, 'EventRule');

Expand Down Expand Up @@ -310,6 +313,33 @@ export = {
}
});

test.done();
},

'asEventRuleTarget can use the ruleArn and a uniqueId of the rule'(test: Test) {
const stack = new cdk.Stack();

let receivedRuleArn = new RuleArn('FAIL');
let receivedRuleId = 'FAIL';

const t1: IEventRuleTarget = {
asEventRuleTarget: (ruleArn: RuleArn, ruleId: string) => {
receivedRuleArn = ruleArn;
receivedRuleId = ruleId;

return {
id: 'T1',
arn: new cdk.Arn('ARN1'),
kinesisParameters: { partitionKeyPath: 'partitionKeyPath' }
};
}
};

const rule = new EventRule(stack, 'EventRule');
rule.addTarget(t1);

test.deepEqual(resolve(receivedRuleArn), resolve(rule.ruleArn));
test.deepEqual(receivedRuleId, rule.uniqueId);
test.done();
}
};
18 changes: 6 additions & 12 deletions packages/@aws-cdk/aws-lambda/lib/lambda-ref.ts
Expand Up @@ -131,12 +131,6 @@ export abstract class FunctionRef extends cdk.Construct implements events.IEvent
*/
protected abstract readonly canCreatePermissions: boolean;

/**
* Indicates if the resource policy that allows CloudWatch events to publish
* notifications to this lambda have been added.
*/
private eventRuleTargetPolicyAdded = false;

/**
* Indicates if the policy that allows CloudWatch logs to publish to this lambda has been added.
*/
Expand Down Expand Up @@ -177,14 +171,14 @@ export abstract class FunctionRef extends cdk.Construct implements events.IEvent
* Returns a RuleTarget that can be used to trigger this Lambda as a
* result from a CloudWatch event.
*/
public get eventRuleTarget(): events.EventRuleTargetProps {
if (!this.eventRuleTargetPolicyAdded) {
this.addPermission('InvokedByCloudWatch', {
public asEventRuleTarget(ruleArn: events.RuleArn, ruleId: string): events.EventRuleTargetProps {
const permissionId = `AllowEventRule${ruleId}`;
if (!this.tryFindChild(permissionId)) {
this.addPermission(permissionId, {
action: 'lambda:InvokeFunction',
principal: new cdk.ServicePrincipal('events.amazonaws.com')
principal: new cdk.ServicePrincipal('events.amazonaws.com'),
sourceArn: ruleArn
});

this.eventRuleTargetPolicyAdded = true;
}

return {
Expand Down
132 changes: 132 additions & 0 deletions packages/@aws-cdk/aws-lambda/test/integ.events.expected.json
@@ -0,0 +1,132 @@
{
"Resources": {
"MyFuncServiceRole54065130": {
"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"
]
]
}
]
}
},
"MyFunc8A243A2C": {
"Type": "AWS::Lambda::Function",
"Properties": {
"Code": {
"ZipFile": "exports.handler = function handler(event, _context, callback) {\n console.log(JSON.stringify(event, undefined, 2));\n return callback();\n}"
},
"Handler": "index.handler",
"Role": {
"Fn::GetAtt": [
"MyFuncServiceRole54065130",
"Arn"
]
},
"Runtime": "nodejs6.10"
},
"DependsOn": [
"MyFuncServiceRole54065130"
]
},
"MyFuncAllowEventRulelambdaeventsTimer0E6AB6D8E3B334A3": {
"Type": "AWS::Lambda::Permission",
"Properties": {
"Action": "lambda:InvokeFunction",
"FunctionName": {
"Ref": "MyFunc8A243A2C"
},
"Principal": "events.amazonaws.com",
"SourceArn": {
"Fn::GetAtt": [
"TimerBF6F831F",
"Arn"
]
}
}
},
"MyFuncAllowEventRulelambdaeventsTimer27F866A1E0669C645": {
"Type": "AWS::Lambda::Permission",
"Properties": {
"Action": "lambda:InvokeFunction",
"FunctionName": {
"Ref": "MyFunc8A243A2C"
},
"Principal": "events.amazonaws.com",
"SourceArn": {
"Fn::GetAtt": [
"Timer2B6F162E9",
"Arn"
]
}
}
},
"TimerBF6F831F": {
"Type": "AWS::Events::Rule",
"Properties": {
"ScheduleExpression": "rate(1 minute)",
"State": "ENABLED",
"Targets": [
{
"Arn": {
"Fn::GetAtt": [
"MyFunc8A243A2C",
"Arn"
]
},
"Id": "MyFunc"
}
]
}
},
"Timer2B6F162E9": {
"Type": "AWS::Events::Rule",
"Properties": {
"ScheduleExpression": "rate(2 minutes)",
"State": "ENABLED",
"Targets": [
{
"Arn": {
"Fn::GetAtt": [
"MyFunc8A243A2C",
"Arn"
]
},
"Id": "MyFunc"
}
]
}
}
}
}
27 changes: 27 additions & 0 deletions packages/@aws-cdk/aws-lambda/test/integ.events.ts
@@ -0,0 +1,27 @@
import events = require('@aws-cdk/aws-events');
import cdk = require('@aws-cdk/cdk');
import lambda = require('../lib');

const app = new cdk.App(process.argv);

const stack = new cdk.Stack(app, 'lambda-events');

const fn = new lambda.Function(stack, 'MyFunc', {
runtime: lambda.Runtime.NodeJS610,
handler: 'index.handler',
code: lambda.Code.inline(`exports.handler = ${handler.toString()}`)
});

const timer = new events.EventRule(stack, 'Timer', { scheduleExpression: 'rate(1 minute)' });
timer.addTarget(fn);

const timer2 = new events.EventRule(stack, 'Timer2', { scheduleExpression: 'rate(2 minutes)' });
timer2.addTarget(fn);

process.stdout.write(app.run());

// tslint:disable:no-console
function handler(event: any, _context: any, callback: any) {
console.log(JSON.stringify(event, undefined, 2));
return callback();
}
Expand Up @@ -123,4 +123,4 @@
]
}
}
}
}
19 changes: 15 additions & 4 deletions packages/@aws-cdk/aws-lambda/test/test.lambda.ts
@@ -1,4 +1,4 @@
import { expect, haveResource } from '@aws-cdk/assert';
import { countResources, expect, haveResource } from '@aws-cdk/assert';
import events = require('@aws-cdk/aws-events');
import iam = require('@aws-cdk/aws-iam');
import cdk = require('@aws-cdk/cdk');
Expand Down Expand Up @@ -255,20 +255,31 @@ export = {
// GIVEN
const stack = new cdk.Stack();
const fn = newTestLambda(stack);
const rule = new events.EventRule(stack, 'Rule');
const rule1 = new events.EventRule(stack, 'Rule');
const rule2 = new events.EventRule(stack, 'Rule2');

// WHEN
rule.addTarget(fn);
rule1.addTarget(fn);
rule2.addTarget(fn);

// THEN
const lambdaId = "MyLambdaCCE802FB";

expect(stack).to(haveResource('AWS::Lambda::Permission', {
"Action": "lambda:InvokeFunction",
"FunctionName": { "Ref": lambdaId },
"Principal": "events.amazonaws.com"
"Principal": "events.amazonaws.com",
"SourceArn": { "Fn::GetAtt": [ "Rule4C995B7F", "Arn" ] }
}));

expect(stack).to(haveResource('AWS::Lambda::Permission', {
"Action": "lambda:InvokeFunction",
"FunctionName": { "Ref": "MyLambdaCCE802FB" },
"Principal": "events.amazonaws.com",
"SourceArn": { "Fn::GetAtt": [ "Rule270732244", "Arn" ] }
}));

expect(stack).to(countResources('AWS::Events::Rule', 2));
expect(stack).to(haveResource('AWS::Events::Rule', {
"Targets": [
{
Expand Down

0 comments on commit 327f5e1

Please sign in to comment.