From e53efb2b0649e1927a1d9d05c904c132e526f02a Mon Sep 17 00:00:00 2001 From: Andrei Alecu Date: Mon, 1 Mar 2021 20:02:18 +0200 Subject: [PATCH 1/3] fix(elasticloadbalancingv2): should allow more than 2 certificates --- .../lib/alb/application-listener.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 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 7897f5582eb0a..d06a4cbb4f121 100644 --- a/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/alb/application-listener.ts +++ b/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/alb/application-listener.ts @@ -219,10 +219,12 @@ export class ApplicationListener extends BaseListener implements IApplicationLis this.certificateArns.push(first.certificateArn); } - if (additionalCerts.length > 0) { + // Only one certificate can be specified per resource, even though + // `certificates` is of type Array + for (const cert of additionalCerts) { new ApplicationListenerCertificate(this, id, { listener: this, - certificates: additionalCerts, + certificates: [cert], }); } } From 29854b8f5c5235c6d21be25f661f927769822d19 Mon Sep 17 00:00:00 2001 From: Andrei Alecu Date: Tue, 2 Mar 2021 12:24:21 +0200 Subject: [PATCH 2/3] fix and add tests --- .../lib/alb/application-listener.ts | 14 +++++--- .../test/alb/listener.test.ts | 35 +++++++++++++++++++ 2 files changed, 45 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 b00155222333a..ae6caf4682868 100644 --- a/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/alb/application-listener.ts +++ b/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/alb/application-listener.ts @@ -263,12 +263,18 @@ export class ApplicationListener extends BaseListener implements IApplicationLis this.certificateArns.push(first.certificateArn); } - // Only one certificate can be specified per resource, even though + // Only one certificate can be specified per resource, even though // `certificates` is of type Array - for (const cert of additionalCerts) { - new ApplicationListenerCertificate(this, id, { + for (let i = 0; i < additionalCerts.length; i++) { + let certId = id; + // ids should look like: `id`, `id2`, `id3` (for backwards-compatibility) + if (i > 0) { + certId += i + 1; + } + + new ApplicationListenerCertificate(this, certId, { listener: this, - certificates: [cert], + 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 3e7b639cb1a8f..923f27fd21ae8 100644 --- a/packages/@aws-cdk/aws-elasticloadbalancingv2/test/alb/listener.test.ts +++ b/packages/@aws-cdk/aws-elasticloadbalancingv2/test/alb/listener.test.ts @@ -143,6 +143,41 @@ describe('tests', () => { }); }); + test('HTTPS listener can add more than two certificates', () => { + // GIVEN + const stack = new cdk.Stack(); + const vpc = new ec2.Vpc(stack, 'Stack'); + const lb = new elbv2.ApplicationLoadBalancer(stack, 'LB', { vpc }); + + // WHEN + lb.addListener('Listener', { + port: 443, + defaultTargetGroups: [ + new elbv2.ApplicationTargetGroup(stack, 'Group', { vpc, port: 80 }), + ], + certificates: [ + elbv2.ListenerCertificate.fromArn('cert1'), + elbv2.ListenerCertificate.fromArn('cert2'), + elbv2.ListenerCertificate.fromArn('cert3'), + ], + }); + + // THEN + expect(stack).toHaveResource('AWS::ElasticLoadBalancingV2::Listener', { + Certificates: [ + { CertificateArn: 'cert1' }, + ], + }); + + expect(stack).toHaveResource('AWS::ElasticLoadBalancingV2::ListenerCertificate', { + Certificates: [{ CertificateArn: 'cert2' }], + }); + + expect(stack).toHaveResource('AWS::ElasticLoadBalancingV2::ListenerCertificate', { + Certificates: [{ CertificateArn: 'cert3' }], + }); + }); + test('Can configure targetType on TargetGroups', () => { // GIVEN const stack = new cdk.Stack(); From 5d4058dccbbd3d6d6145f35e20e17816a8ad24d7 Mon Sep 17 00:00:00 2001 From: Nick Lynch Date: Tue, 2 Mar 2021 11:26:56 +0000 Subject: [PATCH 3/3] Minor tweaks --- .../lib/alb/application-listener.ts | 6 +----- .../test/alb/listener.test.ts | 10 ++++++---- 2 files changed, 7 insertions(+), 9 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 ae6caf4682868..b72151f81f2f8 100644 --- a/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/alb/application-listener.ts +++ b/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/alb/application-listener.ts @@ -266,12 +266,8 @@ 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++) { - let certId = id; // ids should look like: `id`, `id2`, `id3` (for backwards-compatibility) - if (i > 0) { - certId += i + 1; - } - + const certId = (i > 0) ? `${id}${i + 1}` : id; new ApplicationListenerCertificate(this, certId, { 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 923f27fd21ae8..b6e379a17e463 100644 --- a/packages/@aws-cdk/aws-elasticloadbalancingv2/test/alb/listener.test.ts +++ b/packages/@aws-cdk/aws-elasticloadbalancingv2/test/alb/listener.test.ts @@ -150,7 +150,7 @@ describe('tests', () => { const lb = new elbv2.ApplicationLoadBalancer(stack, 'LB', { vpc }); // WHEN - lb.addListener('Listener', { + const listener = lb.addListener('Listener', { port: 443, defaultTargetGroups: [ new elbv2.ApplicationTargetGroup(stack, 'Group', { vpc, port: 80 }), @@ -162,11 +162,13 @@ describe('tests', () => { ], }); + expect(listener.node.tryFindChild('DefaultCertificates')).toBeDefined(); + expect(listener.node.tryFindChild('DefaultCertificates2')).toBeDefined(); + expect(listener.node.tryFindChild('DefaultCertificates3')).not.toBeDefined(); + // THEN expect(stack).toHaveResource('AWS::ElasticLoadBalancingV2::Listener', { - Certificates: [ - { CertificateArn: 'cert1' }, - ], + Certificates: [{ CertificateArn: 'cert1' }], }); expect(stack).toHaveResource('AWS::ElasticLoadBalancingV2::ListenerCertificate', {