aws-cdk icon indicating copy to clipboard operation
aws-cdk copied to clipboard

feat(ecs-patterns): allow passing a listener to ApplicationLoadBalancedFargateService pattern

Open hbjydev opened this issue 7 months ago • 4 comments

Reason for this change

We're attempting to write some CDK code that will put our load balancers into maintenance mode before rolling out a new image deployment, running migrations, etc.

In order to do that we need to create our listeners ahead of time due to the design of our app, but because this pattern tries to configure one for you, we can't implement it cleanly.

This PR resolves that problem.

Description of changes

This adds a new property for the ALB Fargate Service pattern, listener, which is an optional ApplicationListener, and does a ternary check to see if it's set. If it is, it uses that as the listener for the rest of the construct, and if not, it creates a new one, similarly to how this check is implemented for load balancers themselves.

Describe any new or updated permissions being added

None

Description of how you validated changes

I've tested this file having extracted it out of CDK source code and modifying a 'fork' of it in our codebase, which I then ran and verified does what I need it to.

Checklist


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

hbjydev avatar May 19 '25 12:05 hbjydev

The pull request linter fails with the following errors:

❌ Features must contain a change to an integration test file and the resulting snapshot.

Exemption Request - There are no integration tests for custom load balancers for this construct as far as I can see, so having one for listeners would be pointless.

hbjydev avatar May 19 '25 13:05 hbjydev

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
  • Commit ID: 2aab72b23a24910608170b73ba03fb0087e3a353
  • Result: FAILED
  • Build Logs (available for 30 days)

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

aws-cdk-automation avatar May 20 '25 10:05 aws-cdk-automation

While this may not be a widely used feature, allowing users to pass an existing listener makes sense as there's no easy workaround without introducing a new prop. This addition would support several valid use cases:

  1. Managing ALBs with multiple listeners (e.g., one for normal traffic, one for maintenance mode)
  2. Pre-configuring listeners with specific settings before service creation
  3. Reusing existing listeners instead of having the pattern create new ones

However, we should add input validation to ensure the provided listener belongs to the provided load balancer to prevent potential runtime issues. This will help users catch configuration errors early and provide clear guidance.

Consider adding some improvements before it can be merged:

  1. Add input validation to ensure the provided listener belongs to the provided load balancer
  2. Improve documentation to better explain the relationship between the load balancer and listener parameters
  3. Add examples in the README for common use cases, especially around maintenance mode scenarios

pahud avatar May 27 '25 16:05 pahud

Hi @hbjydev, just checking if this change is still needed. If you have figured out another way to fulfil your requirements, I will close this PR.

vishaalmehrishi avatar Jun 18 '25 08:06 vishaalmehrishi

Hi @hbjydev, just checking if this change is still needed. If you have figured out another way to fulfil your requirements, I will close this PR.

Hi, yeah, we ended up vendoring our changes temporarily. I'll look at merging in the feedback here in the next few days.

hbjydev avatar Jun 19 '25 14:06 hbjydev

This PR 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.

github-actions[bot] avatar Jul 07 '25 12:07 github-actions[bot]

Hi @vishaalmehrishi, We require the features in this Pull Request - can this be re-opened please?

davidjmemmett avatar Aug 04 '25 10:08 davidjmemmett