From 7d184f90aeee357227b74c4c3b3d51c9165e1586 Mon Sep 17 00:00:00 2001 From: Niranjan Jayakar Date: Tue, 14 Jul 2020 17:29:08 +0100 Subject: [PATCH] fix(lambda-event-sources): use of CfnParameter for maxBatchSize, retryAttempts & parallelizationFactor fails Code was not accounting for tokens in its validation checks. Added support for tokens. --- .../aws-lambda-event-sources/lib/kinesis.ts | 9 +++-- .../test/test.kinesis.ts | 15 +++++++++ .../aws-lambda/lib/event-source-mapping.ts | 17 ++++++---- .../test/test.event-source-mapping.ts | 33 +++++++++++++++++++ 4 files changed, 65 insertions(+), 9 deletions(-) diff --git a/packages/@aws-cdk/aws-lambda-event-sources/lib/kinesis.ts b/packages/@aws-cdk/aws-lambda-event-sources/lib/kinesis.ts index 2e78487075fbd..1a2de4c390155 100644 --- a/packages/@aws-cdk/aws-lambda-event-sources/lib/kinesis.ts +++ b/packages/@aws-cdk/aws-lambda-event-sources/lib/kinesis.ts @@ -1,5 +1,6 @@ import * as kinesis from '@aws-cdk/aws-kinesis'; import * as lambda from '@aws-cdk/aws-lambda'; +import * as cdk from '@aws-cdk/core'; import {StreamEventSource, StreamEventSourceProps} from './stream'; export interface KinesisEventSourceProps extends StreamEventSourceProps { @@ -14,9 +15,11 @@ export class KinesisEventSource extends StreamEventSource { constructor(readonly stream: kinesis.IStream, props: KinesisEventSourceProps) { super(props); - if (this.props.batchSize !== undefined && (this.props.batchSize < 1 || this.props.batchSize > 10000)) { - throw new Error(`Maximum batch size must be between 1 and 10000 inclusive (given ${this.props.batchSize})`); - } + this.props.batchSize !== undefined && cdk.withResolved(this.props.batchSize, batchSize => { + if (batchSize < 1 || batchSize > 10000) { + throw new Error(`Maximum batch size must be between 1 and 10000 inclusive (given ${this.props.batchSize})`); + } + }); } public bind(target: lambda.IFunction) { diff --git a/packages/@aws-cdk/aws-lambda-event-sources/test/test.kinesis.ts b/packages/@aws-cdk/aws-lambda-event-sources/test/test.kinesis.ts index 9ee1a1d9c25e0..6fea04f3d9089 100644 --- a/packages/@aws-cdk/aws-lambda-event-sources/test/test.kinesis.ts +++ b/packages/@aws-cdk/aws-lambda-event-sources/test/test.kinesis.ts @@ -136,6 +136,21 @@ export = { test.done(); }, + 'accepts if batch size is a token'(test: Test) { + // GIVEN + const stack = new cdk.Stack(); + const fn = new TestFunction(stack, 'Fn'); + const stream = new kinesis.Stream(stack, 'S'); + + // WHEN + fn.addEventSource(new sources.KinesisEventSource(stream, { + batchSize: cdk.Lazy.numberValue({ produce: () => 10 }), + startingPosition: lambda.StartingPosition.LATEST, + })); + + test.done(); + }, + 'specific maxBatchingWindow'(test: Test) { // GIVEN const stack = new cdk.Stack(); diff --git a/packages/@aws-cdk/aws-lambda/lib/event-source-mapping.ts b/packages/@aws-cdk/aws-lambda/lib/event-source-mapping.ts index 556b0cc732af1..6fc56a2e1d325 100644 --- a/packages/@aws-cdk/aws-lambda/lib/event-source-mapping.ts +++ b/packages/@aws-cdk/aws-lambda/lib/event-source-mapping.ts @@ -152,13 +152,18 @@ export class EventSourceMapping extends cdk.Resource implements IEventSourceMapp throw new Error('maxRecordAge must be between 60 seconds and 7 days inclusive'); } - if (props.retryAttempts && (props.retryAttempts < 0 || props.retryAttempts > 10000)) { - throw new Error(`retryAttempts must be between 0 and 10000 inclusive, got ${props.retryAttempts}`); - } + props.retryAttempts !== undefined && cdk.withResolved(props.retryAttempts, (attempts) => { + if (attempts < 0 || attempts > 10000) { + throw new Error(`retryAttempts must be between 0 and 10000 inclusive, got ${attempts}`); + } + }); + + props.parallelizationFactor !== undefined && cdk.withResolved(props.parallelizationFactor, (factor) => { + if (factor < 1 || factor > 10) { + throw new Error(`parallelizationFactor must be between 1 and 10 inclusive, got ${factor}`); + } + }); - if ((props.parallelizationFactor || props.parallelizationFactor === 0) && (props.parallelizationFactor < 1 || props.parallelizationFactor > 10)) { - throw new Error(`parallelizationFactor must be between 1 and 10 inclusive, got ${props.parallelizationFactor}`); - } let destinationConfig; diff --git a/packages/@aws-cdk/aws-lambda/test/test.event-source-mapping.ts b/packages/@aws-cdk/aws-lambda/test/test.event-source-mapping.ts index 942c7fae8b337..4de8108b609d2 100644 --- a/packages/@aws-cdk/aws-lambda/test/test.event-source-mapping.ts +++ b/packages/@aws-cdk/aws-lambda/test/test.event-source-mapping.ts @@ -103,6 +103,22 @@ export = { test.done(); }, + 'accepts if retryAttempts is a token'(test: Test) { + const stack = new cdk.Stack(); + const fn = new Function(stack, 'fn', { + handler: 'index.handler', + code: Code.fromInline('exports.handler = ${handler.toString()}'), + runtime: Runtime.NODEJS_10_X, + }); + + new EventSourceMapping(stack, 'test', { + target: fn, + eventSourceArn: '', + retryAttempts: cdk.Lazy.numberValue({ produce: () => 100 }), + }); + + test.done(); + }, 'throws if parallelizationFactor is below 1'(test: Test) { const stack = new cdk.Stack(); const fn = new Function(stack, 'fn', { @@ -144,6 +160,23 @@ export = { test.done(); }, + 'accepts if parallelizationFactor is a token'(test: Test) { + const stack = new cdk.Stack(); + const fn = new Function(stack, 'fn', { + handler: 'index.handler', + code: Code.fromInline('exports.handler = ${handler.toString()}'), + runtime: Runtime.NODEJS_10_X, + }); + + new EventSourceMapping(stack, 'test', { + target: fn, + eventSourceArn: '', + parallelizationFactor: cdk.Lazy.numberValue({ produce: () => 20 }), + }); + + test.done(); + }, + 'import event source mapping'(test: Test) { const stack = new cdk.Stack(undefined, undefined, { stackName: 'test-stack' }); const imported = EventSourceMapping.fromEventSourceMappingId(stack, 'imported', '14e0db71-5d35-4eb5-b481-8945cf9d10c2');