From 934e7e0e4c126c0585cf0671ab07c5724d71e85b Mon Sep 17 00:00:00 2001 From: Nick Lynch Date: Tue, 9 Mar 2021 12:42:49 +0000 Subject: [PATCH] fix(elasticloadbalancingv2): upgrade to v1.92.0 drops certificates on ALB if more than 2 certificates exist Support for multiple certificates attached to a single ALB listener was originally implemented by putting all certificates in an array on a single `ListenerCertificate` resource. The docs state that only one certificate may be specified, although multiple certificates do appear to work initially. Initial resource creation of a `ListenerCertificate` with multiple certificates appears to succeed, but subsequent updates to this resource (to either add or remove certificates) yields undefined and undesireable behavior. The fix in #13332 attempted to fix this by creating a new `ListenerCertificate` per certificate, and -- at my direction -- maintained partial backwards compatibility by keeping the original ID for the first `ListenerCertificate` resource. However, this has the effect of triggering an update to the existing resource, which does not appear to work correctly. By forcing a logical ID change for all `ListenerCertificate` resources, we can force all existing resources to be deleted, and new resources created. This avoids doing any updates on any `ListenerCertificate` resources with an array of certificates, which appears to side-step the undefined behavior. fixes #13437 --- .../lib/alb/application-listener.ts | 4 +--- .../aws-elasticloadbalancingv2/test/alb/listener.test.ts | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/alb/application-listener.ts b/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/alb/application-listener.ts index b72151f81f2f8..3a13b8a3490a1 100644 --- a/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/alb/application-listener.ts +++ b/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/alb/application-listener.ts @@ -266,9 +266,7 @@ export class ApplicationListener extends BaseListener implements IApplicationLis // Only one certificate can be specified per resource, even though // `certificates` is of type Array for (let i = 0; i < additionalCerts.length; i++) { - // ids should look like: `id`, `id2`, `id3` (for backwards-compatibility) - const certId = (i > 0) ? `${id}${i + 1}` : id; - new ApplicationListenerCertificate(this, certId, { + new ApplicationListenerCertificate(this, `${id}${i + 1}`, { listener: this, certificates: [additionalCerts[i]], }); diff --git a/packages/@aws-cdk/aws-elasticloadbalancingv2/test/alb/listener.test.ts b/packages/@aws-cdk/aws-elasticloadbalancingv2/test/alb/listener.test.ts index b6e379a17e463..e29e5a7837895 100644 --- a/packages/@aws-cdk/aws-elasticloadbalancingv2/test/alb/listener.test.ts +++ b/packages/@aws-cdk/aws-elasticloadbalancingv2/test/alb/listener.test.ts @@ -162,7 +162,7 @@ describe('tests', () => { ], }); - expect(listener.node.tryFindChild('DefaultCertificates')).toBeDefined(); + expect(listener.node.tryFindChild('DefaultCertificates1')).toBeDefined(); expect(listener.node.tryFindChild('DefaultCertificates2')).toBeDefined(); expect(listener.node.tryFindChild('DefaultCertificates3')).not.toBeDefined();