feat(lambda-event-sources): add kafka consumerGroupId support

[RaphaelManke]

This PR adds the capability to specify the consumerGroupId in the event-source-mapping when connection to Kafka.
Adds the feature described in issue #21734

This features allows you to specify the consumerGroupId while connecting to a Kafka cluster. This feature was recently annouced https://aws.amazon.com/blogs/compute/using-custom-consumer-group-id-support-for-the-aws-lambda-event-sources-for-msk-and-self-managed-kafka/ and wasn't part of the cdk Kafka construct before.

Things done:

• Added missing attributes to class EventSourceMapping
  • amazonManagedKafkaEventSourceConfig
  • selfManagedKafkaEventSourceConfig
• Added validation for the consumerGroupId value based in CfnSpec
• Added a common api consumerGroupId for adding the consumerGroupId independant if selfManaged or awsManaged
• Updated existing integration test for SelfManagesKafkaConfig
  • The ManagedKafka Config is not integration tested because it requires a ManagedKafkaCluster which is not possible to deploy right now via cdk
• Added Tests for consumerGroupId validation
• Added Tests for template synth for SelfManagedKafka and AwsManagedKafka

---

All Submissions:

• Have you followed the guidelines in our Contributing guide?

Adding new Unconventional Dependencies:

• This PR adds new unconventional dependencies following the process described here

New Features

• Have you added the new feature to an integration test?
  • Did you use yarn integ to deploy the infrastructure and generate the snapshot (i.e. yarn integ without --dry-run)?

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

[gitpod-io]

[RaphaelManke]

@mrgrain You were reviewing a kafka related PR in the past, so maybe you are interested this time as well? 😃

[TheRealAmazonKendra]

Ultimately this resolves to a string so I'm not sure what the benefit of assigning the type this way.

[RaphaelManke]

Yes it is just a string, but my intention was to give someone that is looking at the code some guidance that the consumerGroupId is the same as the kafkaConsumerGroupId. And then it is easier to follow the path of the property to further documentation and usage. In this case the consumerGroupId is just a renaming of the property kafkaConsumerGroupId to have a shorter property name because we are aöready in the context of kafka properties.
But I get that this way of defining properties is a new style and therefore not optimal.
Please let me know if I should change it to or the property path reasoning has more value than just putting string here.

[TheRealAmazonKendra]

I appreciate trying to give the extra context but I'm not sure that writing it this way will play nice with JSII when translating it into the other languages we use. That, and consistency in style is something I tend to think is pretty important for those who come after and try to understand the code to make changes again. I would use the docstring to provide this context if you don't think it's clear enough.

I do, however, like that you did shorten the name for the reason you listed.

[TheRealAmazonKendra]

Let's figure out which one of these it should be mapped to and just set it there instead of providing both props.

[RaphaelManke]

I split it into two if statements to have less branching. Additionally i moved the validation to private function to have a cleaner view on the logic.

[TheRealAmazonKendra]

I like this much better. Thanks!

Pull request has been modified.

[TheRealAmazonKendra]

Please see my comments inline. This is almost perfect. Thanks for bearing with me on all the stylistic changes requested and my constant request of MOAR TESTS.

[TheRealAmazonKendra]

I appreciate trying to give the extra context but I'm not sure that writing it this way will play nice with JSII when translating it into the other languages we use. That, and consistency in style is something I tend to think is pretty important for those who come after and try to understand the code to make changes again. I would use the docstring to provide this context if you don't think it's clear enough.

I do, however, like that you did shorten the name for the reason you listed.

[TheRealAmazonKendra]

I like this much better. Thanks!

Pull request has been modified.

[mergify]

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

[aws-cdk-automation]

AWS CodeBuild CI Report

• CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
• Commit ID: 5ad6658
• Result: SUCCEEDED
• Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

[mergify]

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).