Skip to content

Commit 8e4ca14

Browse files
authored
fix(ecs-patterns): add missing redirectHTTP dependency on main listener in ApplicationLoadBalancedFargateService (#34510)
### Issue # (if applicable) Closes #34235 ### Reason for this change Fixes an issue with Cloudformation throwing `A listener already exists on this port for this load balancer` during deployments when switching from `redirectHTTP: false` to `redirectHTTP: true` on the `ApplicationLoadBalancedFargateService` construct. ### Description of changes There are 3 cases to handle: - For the default listener created by the construct and described in the reproduction stack in this issue. The bug is that when using the default listener port [here](https://github.com/aws/aws-cdk/blob/2bdc07e45f836a710bc049d43a2462806af1c75d/packages/aws-cdk-lib/aws-ecs-patterns/lib/base/application-load-balanced-service-base.ts#L508), we don’t wait for it to update to 443 before adding the redirect listener from 80 -> 443, causing a race condition that sometimes tries to add 2 listeners on the same port (80). Fixed by adding an explicit dependency. - The second case would be a user having the port 80 listener added via `loadBalancer.addListener` or expliclity via the `props.listenerPort`. We let the CFN error pass through. ### Describe any new or updated permissions being added None ### Description of how you validated changes Added unit tests and updated integ tests. I'm still trying to deploy the integ tests to my account but the issue is that I need to first have a route53 domain. ### Checklist - [X] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
2 parents 7490b92 + 9075d6f commit 8e4ca14

File tree

2 files changed

+5
-1
lines changed

2 files changed

+5
-1
lines changed

packages/@aws-cdk-testing/framework-integ/test/aws-ecs-patterns/test/fargate/integ.alb-fargate-service-https.js.snapshot/aws-ecs-integ-alb-fg-https.template.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -535,7 +535,8 @@
535535
},
536536
"Port": 80,
537537
"Protocol": "HTTP"
538-
}
538+
},
539+
"DependsOn": ["myServiceLBPublicListenerC78AE8A0","myServiceLBPublicListenerECSGroup17E9BBC1"]
539540
},
540541
"myServiceCertificate152F9DDA": {
541542
"Type": "AWS::CertificateManager::Certificate",

packages/aws-cdk-lib/aws-ecs-patterns/lib/base/application-load-balanced-service-base.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -541,6 +541,9 @@ export abstract class ApplicationLoadBalancedServiceBase extends Construct {
541541
permanent: true,
542542
}),
543543
});
544+
// Ensure the redirect listener is created after the main listener,
545+
// otherwise we run into a race condition that adds 2 listeners on port 80.
546+
this.redirectListener.node.addDependency(this.listener);
544547
}
545548

546549
let domainName = loadBalancer.loadBalancerDnsName;

0 commit comments

Comments
 (0)