Skip to content

Commit

Permalink
fix(aws-lambda-event-sources): unsupported properties for SelfManaged…
Browse files Browse the repository at this point in the history
…KafkaEventSource and ManagedKafkaEventSource (#17965)

This PR fixes a bug in the CDK where some `kafkaEventSource` properties are actually unsupported. These properties exist only for kinesis and dynamodb streams. The existing KafkaEventSourceProps Interface erroneously extends an interface that includes kinesis and dynamodb specific properties. This PR separates these properties into a `Base` interface with shared stream properties for all 3, as well as an interface for `kinesis` and `dynamodb` specific properties. 

Unit testing unavailable because the scope of the PR is to remove properties. It is enough to ensure that current tests still succeed.

We are allowing the breaking changes specified in  `allowed-breaking-changes.txt` because they never worked in the first place.

Fixes #17934.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
zehsor committed Jan 7, 2022
1 parent 4fb0309 commit 5ddaef4
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 30 deletions.
23 changes: 23 additions & 0 deletions allowed-breaking-changes.txt
Expand Up @@ -93,3 +93,26 @@ incompatible-argument:@aws-cdk/aws-autoscaling-hooktargets.FunctionHook.bind
incompatible-argument:@aws-cdk/aws-autoscaling-hooktargets.QueueHook.bind
incompatible-argument:@aws-cdk/aws-autoscaling-hooktargets.TopicHook.bind
incompatible-argument:@aws-cdk/aws-autoscaling.ILifecycleHookTarget.bind

# removed properties from kafka eventsources as they are not supported
removed:@aws-cdk/aws-lambda-event-sources.KafkaEventSourceProps.bisectBatchOnError
removed:@aws-cdk/aws-lambda-event-sources.KafkaEventSourceProps.maxRecordAge
removed:@aws-cdk/aws-lambda-event-sources.KafkaEventSourceProps.parallelizationFactor
removed:@aws-cdk/aws-lambda-event-sources.KafkaEventSourceProps.reportBatchItemFailures
removed:@aws-cdk/aws-lambda-event-sources.KafkaEventSourceProps.retryAttempts
removed:@aws-cdk/aws-lambda-event-sources.KafkaEventSourceProps.tumblingWindow
removed:@aws-cdk/aws-lambda-event-sources.ManagedKafkaEventSourceProps.bisectBatchOnError
removed:@aws-cdk/aws-lambda-event-sources.ManagedKafkaEventSourceProps.maxRecordAge
removed:@aws-cdk/aws-lambda-event-sources.ManagedKafkaEventSourceProps.parallelizationFactor
removed:@aws-cdk/aws-lambda-event-sources.ManagedKafkaEventSourceProps.reportBatchItemFailures
removed:@aws-cdk/aws-lambda-event-sources.ManagedKafkaEventSourceProps.retryAttempts
removed:@aws-cdk/aws-lambda-event-sources.ManagedKafkaEventSourceProps.tumblingWindow
removed:@aws-cdk/aws-lambda-event-sources.SelfManagedKafkaEventSourceProps.bisectBatchOnError
removed:@aws-cdk/aws-lambda-event-sources.SelfManagedKafkaEventSourceProps.maxRecordAge
removed:@aws-cdk/aws-lambda-event-sources.SelfManagedKafkaEventSourceProps.parallelizationFactor
removed:@aws-cdk/aws-lambda-event-sources.SelfManagedKafkaEventSourceProps.reportBatchItemFailures
removed:@aws-cdk/aws-lambda-event-sources.SelfManagedKafkaEventSourceProps.retryAttempts
removed:@aws-cdk/aws-lambda-event-sources.SelfManagedKafkaEventSourceProps.tumblingWindow
base-types:@aws-cdk/aws-lambda-event-sources.KafkaEventSourceProps
base-types:@aws-cdk/aws-lambda-event-sources.ManagedKafkaEventSourceProps
base-types:@aws-cdk/aws-lambda-event-sources.SelfManagedKafkaEventSourceProps
4 changes: 2 additions & 2 deletions packages/@aws-cdk/aws-lambda-event-sources/lib/kafka.ts
Expand Up @@ -4,7 +4,7 @@ import * as iam from '@aws-cdk/aws-iam';
import * as lambda from '@aws-cdk/aws-lambda';
import * as secretsmanager from '@aws-cdk/aws-secretsmanager';
import { Stack, Names } from '@aws-cdk/core';
import { StreamEventSource, StreamEventSourceProps } from './stream';
import { StreamEventSource, BaseStreamEventSourceProps } from './stream';

// keep this import separate from other imports to reduce chance for merge conflicts with v2-main
// eslint-disable-next-line no-duplicate-imports, import/order
Expand All @@ -13,7 +13,7 @@ import { Construct } from '@aws-cdk/core';
/**
* Properties for a Kafka event source
*/
export interface KafkaEventSourceProps extends StreamEventSourceProps {
export interface KafkaEventSourceProps extends BaseStreamEventSourceProps{
/**
* The Kafka topic to subscribe to
*/
Expand Down
62 changes: 34 additions & 28 deletions packages/@aws-cdk/aws-lambda-event-sources/lib/stream.ts
Expand Up @@ -5,7 +5,7 @@ import { Duration } from '@aws-cdk/core';
* The set of properties for event sources that follow the streaming model,
* such as, Dynamo, Kinesis and Kafka.
*/
export interface StreamEventSourceProps {
export interface BaseStreamEventSourceProps{
/**
* The largest number of records that AWS Lambda will retrieve from your event
* source at the time of invoking your function. Your function receives an
Expand All @@ -15,25 +15,51 @@ export interface StreamEventSourceProps {
* * Minimum value of 1
* * Maximum value of:
* * 1000 for {@link DynamoEventSource}
* * 10000 for {@link KinesisEventSource}
* * 10000 for {@link KinesisEventSource}, {@link ManagedKafkaEventSource} and {@link SelfManagedKafkaEventSource}
*
* @default 100
*/
readonly batchSize?: number;

/**
* If the function returns an error, split the batch in two and retry.
* An Amazon SQS queue or Amazon SNS topic destination for discarded records.
*
* @default false
* @default discarded records are ignored
*/
readonly bisectBatchOnError?: boolean;
readonly onFailure?: lambda.IEventSourceDlq;

/**
* An Amazon SQS queue or Amazon SNS topic destination for discarded records.
* Where to begin consuming the stream.
*/
readonly startingPosition: lambda.StartingPosition;

/**
* The maximum amount of time to gather records before invoking the function.
* Maximum of Duration.minutes(5)
*
* @default discarded records are ignored
* @default Duration.seconds(0)
*/
readonly onFailure?: lambda.IEventSourceDlq;
readonly maxBatchingWindow?: Duration;

/**
* If the stream event source mapping should be enabled.
*
* @default true
*/
readonly enabled?: boolean;
}

/**
* The set of properties for event sources that follow the streaming model,
* such as, Dynamo, Kinesis.
*/
export interface StreamEventSourceProps extends BaseStreamEventSourceProps {
/**
* If the function returns an error, split the batch in two and retry.
*
* @default false
*/
readonly bisectBatchOnError?: boolean;

/**
* The maximum age of a record that Lambda sends to a function for processing.
Expand Down Expand Up @@ -65,11 +91,6 @@ export interface StreamEventSourceProps {
*/
readonly parallelizationFactor?: number;

/**
* Where to begin consuming the stream.
*/
readonly startingPosition: lambda.StartingPosition;

/**
* Allow functions to return partially successful responses for a batch of records.
*
Expand All @@ -79,28 +100,13 @@ export interface StreamEventSourceProps {
*/
readonly reportBatchItemFailures?: boolean;

/**
* The maximum amount of time to gather records before invoking the function.
* Maximum of Duration.minutes(5)
*
* @default Duration.seconds(0)
*/
readonly maxBatchingWindow?: Duration;

/**
* The size of the tumbling windows to group records sent to DynamoDB or Kinesis
* Valid Range: 0 - 15 minutes
*
* @default - None
*/
readonly tumblingWindow?: Duration;

/**
* If the stream event source mapping should be enabled.
*
* @default true
*/
readonly enabled?: boolean;
}

/**
Expand Down

0 comments on commit 5ddaef4

Please sign in to comment.