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

[aws-lambda] Add "Topics" property to EventSourceMappingOptions #10138

Closed
1 of 2 tasks
dscpinheiro opened this issue Sep 2, 2020 · 7 comments · Fixed by #10445
Closed
1 of 2 tasks

[aws-lambda] Add "Topics" property to EventSourceMappingOptions #10138

dscpinheiro opened this issue Sep 2, 2020 · 7 comments · Fixed by #10445
Assignees
Labels
@aws-cdk/aws-lambda Related to AWS Lambda effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. in-progress This issue is being actively worked on. p2

Comments

@dscpinheiro
Copy link
Contributor

Lambda recently added support for MSK as an event source (https://aws.amazon.com/about-aws/whats-new/2020/08/aws-lambda-now-supports-amazon-managed-streaming-for-apache-kafka-as-an-event-source/), and there's now a "Topics" property on the CloudFormation resource definition (https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-lambda-eventsourcemapping.html#cfn-lambda-eventsourcemapping-topics).

Use Case

Using Lambda simplifies reading data from Apache Kafka, as other options usually require customers to worry about infrastructure management.

Proposed Solution

Include topics as an optional parameter of EventSourceMappingOptions interface:

export interface EventSourceMappingOptions {
    /**
     * existing properties
     */

    /**
     * The name of the Kafka topic.
     * 
     * Valid range:
     * * Maximum value of 1 
     */
    readonly topics?: string[];
}

It'd then be used as:

lambdaFn.addEventSourceMapping('MSKTrigger', {
    eventSourceArn: 'arn:aws:kafka:us-east-1:123456789012:cluster/kafka-cluster/random-id',
    startingPosition: lambda.StartingPosition.LATEST,
    topics: ['my-topic']
});

Other

In the future, there could even be a MSKEventSource in the aws-lambda-event-sources package, but just adding the topics property should allow customers to get started.

  • 👋 I may be able to implement this feature request
  • ⚠️ This feature might incur a breaking change

This is a 🚀 Feature Request

@dscpinheiro dscpinheiro added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Sep 2, 2020
@github-actions github-actions bot added the @aws-cdk/aws-lambda Related to AWS Lambda label Sep 2, 2020
@dscpinheiro
Copy link
Contributor Author

So it looks like CfnEventSourceMapping does not contain topics yet, but it's included in this PR (#9979).

@nija-at Is there an estimate when that PR will be merged?

@nija-at
Copy link
Contributor

nija-at commented Sep 17, 2020

There is currently no higher level construct support for MSK in the CDK - https://github.com/aws/aws-cdk/tree/master/packages/%40aws-cdk/aws-msk

It will make sense for this to be added once we have some basic L2 support.

@nija-at nija-at added @aws-cdk/aws-lambda-event-sources effort/small Small work item – less than a day of effort p2 and removed @aws-cdk/aws-lambda Related to AWS Lambda needs-triage This issue or PR still needs to be triaged. labels Sep 17, 2020
@dscpinheiro
Copy link
Contributor Author

dscpinheiro commented Sep 17, 2020

@nija-at This might be a dumb question, but wouldn't it still make sense to still add the topics property to the interface? The EventSourceMappingOptions interface would then match completely with the AWS::Lambda::EventSourceMapping resource type.

Edit: I just noticed that the cfnspec has been merged, so I can create a PR for this change.

@nija-at
Copy link
Contributor

nija-at commented Sep 17, 2020

Yes, you are right. This can absolutely be done.

Usually, this is accompanied by an integration class in aws-lambda-event-sources package. This will have to wait until the higher level construct for MSK is in place.

@dscpinheiro
Copy link
Contributor Author

If I created the PR just adding the topics property, would that be rejected? My concern is that if a customer is already using the CDK to manage their MSK clusters, and now wants to add an integration with Lambda, that's just not possible (and waiting for a higher level construct might make them take a different approach).

I did find a PR for a L2 construct for MSK, but it hasn't been reviewed yet (#9908).

@nija-at
Copy link
Contributor

nija-at commented Sep 18, 2020

If I created the PR just adding the topics property, would that be rejected?

Definitely not. Feel free to post one.

@dscpinheiro
Copy link
Contributor Author

@nija-at PR was created: #10445

@SomayaB SomayaB added the in-progress This issue is being actively worked on. label Sep 21, 2020
@mergify mergify bot closed this as completed in #10445 Sep 22, 2020
mergify bot pushed a commit that referenced this issue Sep 22, 2020
Lambda recently added support for MSK as an event source (https://aws.amazon.com/about-aws/whats-new/2020/08/aws-lambda-now-supports-amazon-managed-streaming-for-apache-kafka-as-an-event-source/), and there's now a "Topics" property on the CloudFormation resource definition (https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-lambda-eventsourcemapping.html#cfn-lambda-eventsourcemapping-topics).

Closes #10138


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-lambda Related to AWS Lambda effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. in-progress This issue is being actively worked on. p2
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants