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-autoscaling-hooktargets: "Topic" already exists (can only use FunctionHook once per stack) #29010

Open
adam-nielsen opened this issue Feb 7, 2024 · 1 comment
Assignees
Labels
@aws-cdk/aws-autoscaling-hooktargets bug This issue is a bug. effort/medium Medium work item – several days of effort p2

Comments

@adam-nielsen
Copy link

Describe the bug

If you try to use FunctionHook more than once in a stack, or you use it once but try to attach it to more than one lifecycle event, you get this error:

Error: A subscription with id "Topic" already exists under the scope mystack/lambdaAutoScaling

Expected Behavior

I expected all lifecycle events to be attached to the Lambda function hook without any errors.

Current Behavior

The stack fails to synth with an error saying the name Topic is used more than once.

Reproduction Steps

const lambdaAutoScaling = new NodejsFunction(this, 'lambdaAutoScaling', {});
const autoScalingGroup = new AutoScalingGroup(this, 'example', {});

autoScalingGroup.addLifecycleHook('lambda-onLaunch', {
  lifecycleTransition: LifecycleTransition.INSTANCE_LAUNCHING,
  notificationTarget: new FunctionHook(lambdaAutoScaling),
});

autoScalingGroup.addLifecycleHook('lambda-onTerminate', {
  lifecycleTransition: LifecycleTransition.INSTANCE_TERMINATING,
  notificationTarget: new FunctionHook(lambdaAutoScaling),
});

The problem still exists if you create only one FunctionHook and pass it to both addLifecycleHook() calls.

Possible Solution

The problem appears to be in lambda-hook.ts:28 where it creates an SNS topic with the hard-coded name Topic. So you can only create a single one or the name conflicts.

I guess using something like cdk.Names.nodeUniqueId() is needed here?

Additional Information/Context

I'm using a loop to create four hooks - launch and terminate, for AMD and ARM architectures. So even if the events were somehow combined into an array, I'd still have to call addLifecycleHook() once for each architecture's auto scaling group.

CDK CLI Version

2.126.0 (build fb74c41)

Framework Version

No response

Node.js Version

v21.6.0

OS

Arch Linux

Language

TypeScript

Language Version

TypeScript (5.3.3)

Other information

No response

@adam-nielsen adam-nielsen added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Feb 7, 2024
@pahud
Copy link
Contributor

pahud commented Feb 8, 2024

Thanks for pointing out the bug. Yes we should make it a unique ID. Are you interested to submit a PR for that?

@pahud pahud added p2 effort/medium Medium work item – several days of effort and removed needs-triage This issue or PR still needs to be triaged. labels Feb 8, 2024
@pahud pahud self-assigned this Jun 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-autoscaling-hooktargets bug This issue is a bug. effort/medium Medium work item – several days of effort p2
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants