aws / aws-cdk

The AWS Cloud Development Kit is a framework for defining cloud infrastructure in code
https://aws.amazon.com/cdk
Apache License 2.0
11.52k stars 3.86k forks source link

(SelfManagedKafkaEventSource): cannot update existing trigger #26416

Closed pwmcintyre-shell closed 6 months ago

pwmcintyre-shell commented 1 year ago

Describe the bug

When using Lambda + SelfManagedKafkaEventSource, I'm having trouble updating the stack as it attempts to deploy a new trigger, instead of updating the existing one.

(Forgive me, i'm new to CDK and am unsure how logical ID's work)

I have an array of topics i want to subscribe to and have first deployed with just 1 array, then 2, but on second deploy it fails (see below)

eg. imagine the topics array as:

const topics: string[] = ['rtl-lifecycle.dev.public.accounts']
const topics: string[] = ['rtl-lifecycle.dev.public.accounts', 'foo']

how are logical ID's created and why does it create a new one?

Expected Behavior

update existing trigger

Current Behavior

on first deploy it creates a trigger with logical ID:

sinklambdakafkamongodbsinkKafkaEventSource562c7a1bc77115e40ea235bd00b5c841rtllifecycledevpublicaccountsAE0242C8

but on second deploy it attempts to create a new trigger with logical ID:

sinklambdakafkamongodbsinkKafkaEventSource87ddffe9edd288b483137fc5b55b1890rtllifecycledevpublicaccountsAA7C406B

which fails with:

Resource handler returned message: "An event source mapping with event source ("lkc-pgk3ry-4n0xz.ap-southeast-2.aws.glb.confluent.cloud:9092"), function ("arn:aws:lambda:ap-southeast-2:716053528758:function:data-backbone-databases-d-sinklambdakafkamongodbsi-rWdusb0jXAYV"), topics ("rtl-lifecycle.dev.public.accounts") already exists. Please update or delete the existing mapping with UUID 23bfd1d1-9772-4a0d-96c5-1c4e7ab3790b (Service: Lambda, Status Code: 409, Request ID: b72d431a-1dad-42b9-9578-7d6cf8b32bfa)" (RequestToken: 91584dc5-ebef-7ce4-a80d-b6b1a6be3058, HandlerErrorCode: AlreadyExists)

Reproduction Steps

import { ISecurityGroup, IVpc } from 'aws-cdk-lib/aws-ec2'
import { Function, StartingPosition } from 'aws-cdk-lib/aws-lambda'
import { AuthenticationMethod, SelfManagedKafkaEventSource } from 'aws-cdk-lib/aws-lambda-event-sources'
import { ISecret } from 'aws-cdk-lib/aws-secretsmanager'
import { Construct } from 'constructs'

export interface KafkaSinkLambdaProps {
    readonly consumerGroupId: string
    readonly kafkaSecret: ISecret
    readonly topics: string[]
    readonly securityGroup: ISecurityGroup
    readonly vpc: IVpc
}

export class KafkaSinkLambda extends Construct {
    constructor(scope: Construct, id: string, props: KafkaSinkLambdaProps) {
        super(scope, id)

        // lambda
        const lambda = new Function( ... )

        // triggers
        // add Self-managed Kafka Event Source Mapping for each Kafka topic
        const bootstrapServers = props.kafkaSecret.secretValueFromJson('bootstrap_endpoint').toString().split(',')
        props.topics.forEach((topic: string) => {
            lambda.addEventSource(new SelfManagedKafkaEventSource({
                bootstrapServers,
                topic,
                startingPosition: StartingPosition.TRIM_HORIZON,
                consumerGroupId: `${ props.consumerGroupId }:${ topic }`,
                authenticationMethod: AuthenticationMethod.BASIC_AUTH,
                secret: props.kafkaSecret,
                vpc: props.vpc,
                vpcSubnets: {
                    subnets: props.vpc.privateSubnets,
                },
                securityGroup: props.securityGroup,
            }))
        })
    }
}

Possible Solution

No response

Additional Information/Context

No response

CDK CLI Version

2.78.0 (build 8e95c37)

Framework Version

No response

Node.js Version

v18.15.0

OS

Windows WSL + Ubuntu 20

Language

Typescript

Language Version

TypeScript 5

Other information

No response

indrora commented 1 year ago

I don't see a reason why this should cause a logical ID to change like this.

If you create the lambda event sources and then add them to the lambda function, does this behavior replicate?

pwmcintyre-shell commented 1 year ago

If you create the lambda event sources and then add them to the lambda function, does this behavior replicate?

do you mean like this?

        props.topics.forEach((topic: string) => {
            const a = new SelfManagedKafkaEventSource({ ... })
            lambda.addEventSource( a )
        })

this is actually how my code is, i just simplified it for brevity for this post ... do you think it makes a difference? i could try both

pwmcintyre-shell commented 1 year ago

i think this is related https://github.com/aws/serverless-application-model/issues/1320#issuecomment-1340354731

i will need to do more investigation to see if this is CDK or CFN issue

indrora commented 1 year ago

That does seem to point at the possibility of it being a service/cloudformation related issue.

indrora commented 1 year ago

This also appears to affect Terraform. The workaround is using the AWS SDK/CLI to remove the "broken" handler and then re-apply.

zabrip commented 1 year ago

Topics field cannot be updated: https://docs.aws.amazon.com/lambda/latest/dg/API_UpdateEventSourceMapping.html#API_UpdateEventSourceMapping_RequestSyntax

Going from one topic to another should work as long as no other trigger is using the same combination of event source + function + topic. This requires a new trigger to be created.


Edit: I misread the code, I see now that it is adding two separate triggers. In this case the first trigger should not be updated and the second trigger should be added, so there does appear to be an issue.

Did the bootstrap servers, function, or topic for the first trigger change between deployments?

tim-finnigan commented 6 months ago

Checking in - are there any updates on this issue, or have you tried testing in a more recent version?

github-actions[bot] commented 6 months ago

This issue has not received a response in a while. If you want to keep this issue open, please leave a comment below and auto-close will be canceled.