From e8142e944ac5fae9948e5c010fe475806b83c94b Mon Sep 17 00:00:00 2001 From: Cory Hall <43035978+corymhall@users.noreply.github.com> Date: Thu, 31 Mar 2022 15:30:45 -0400 Subject: [PATCH] fix(elbv2): unable to add multiple certificates to NLB (#19289) This PR does a couple of things to update the NetworkListener to be on par with ApplicationListener. 1. Add a NetworkListenerCertificate construct that allows you to associate multiple certificates with a listener. 2. Add a `addCertificates` method to `NetworkListener` similar to the same method on the `ApplicationListener`. This is needed because even though the `certificates` property on a `Listener`is an array, it expects only one certificate. To add more than one you have to create an `AWS::ElasticLoadBalancingV2::ListenerCertificate`. This functionality was added to `ApplicationListner` via #13490. fixes #8918, #15328 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../lib/nlb/network-listener-certificate.ts | 43 +++++++++++++ .../lib/nlb/network-listener.ts | 37 ++++++++++- .../test/nlb/listener.test.ts | 61 +++++++++++++++++++ 3 files changed, 139 insertions(+), 2 deletions(-) create mode 100644 packages/@aws-cdk/aws-elasticloadbalancingv2/lib/nlb/network-listener-certificate.ts diff --git a/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/nlb/network-listener-certificate.ts b/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/nlb/network-listener-certificate.ts new file mode 100644 index 0000000000000..da8175962e49e --- /dev/null +++ b/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/nlb/network-listener-certificate.ts @@ -0,0 +1,43 @@ +import { Construct } from 'constructs'; +import { CfnListenerCertificate } from '../elasticloadbalancingv2.generated'; +import { IListenerCertificate } from '../shared/listener-certificate'; +import { INetworkListener } from './network-listener'; + +// keep this import separate from other imports to reduce chance for merge conflicts with v2-main +// eslint-disable-next-line no-duplicate-imports, import/order +import { Construct as CoreConstruct } from '@aws-cdk/core'; + +/** + * Properties for adding a set of certificates to a listener + */ +export interface NetworkListenerCertificateProps { + /** + * The listener to attach the rule to + */ + readonly listener: INetworkListener; + + /** + * Certificates to attach + * + * Duplicates are not allowed. + */ + readonly certificates: IListenerCertificate[]; +} + +/** + * Add certificates to a listener + */ +export class NetworkListenerCertificate extends CoreConstruct { + constructor(scope: Construct, id: string, props: NetworkListenerCertificateProps) { + super(scope, id); + + const certificates = [ + ...(props.certificates || []).map(c => ({ certificateArn: c.certificateArn })), + ]; + + new CfnListenerCertificate(this, 'Resource', { + listenerArn: props.listener.listenerArn, + certificates, + }); + } +} diff --git a/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/nlb/network-listener.ts b/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/nlb/network-listener.ts index d48dbafc8202a..df043d06282b7 100644 --- a/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/nlb/network-listener.ts +++ b/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/nlb/network-listener.ts @@ -1,5 +1,5 @@ import * as cxschema from '@aws-cdk/cloud-assembly-schema'; -import { Duration, IResource, Resource } from '@aws-cdk/core'; +import { Duration, IResource, Resource, Lazy } from '@aws-cdk/core'; import { Construct } from 'constructs'; import { BaseListener, BaseListenerLookupOptions } from '../shared/base-listener'; import { HealthCheck } from '../shared/base-target-group'; @@ -7,6 +7,7 @@ import { AlpnPolicy, Protocol, SslPolicy } from '../shared/enums'; import { IListenerCertificate } from '../shared/listener-certificate'; import { validateNetworkProtocol } from '../shared/util'; import { NetworkListenerAction } from './network-listener-action'; +import { NetworkListenerCertificate } from './network-listener-certificate'; import { INetworkLoadBalancer } from './network-load-balancer'; import { INetworkLoadBalancerTarget, INetworkTargetGroup, NetworkTargetGroup } from './network-target-group'; @@ -160,6 +161,11 @@ export class NetworkListener extends BaseListener implements INetworkListener { */ public readonly loadBalancer: INetworkLoadBalancer; + /** + * ARNs of certificates added to this listener + */ + private readonly certificateArns: string[]; + /** * the protocol of the listener */ @@ -188,13 +194,17 @@ export class NetworkListener extends BaseListener implements INetworkListener { protocol: proto, port: props.port, sslPolicy: props.sslPolicy, - certificates: props.certificates, + certificates: Lazy.any({ produce: () => this.certificateArns.map(certificateArn => ({ certificateArn })) }, { omitEmptyArray: true }), alpnPolicy: props.alpnPolicy ? [props.alpnPolicy] : undefined, }); + this.certificateArns = []; this.loadBalancer = props.loadBalancer; this.protocol = proto; + if (certs.length > 0) { + this.addCertificates('DefaultCertificates', certs); + } if (props.defaultAction && props.defaultTargetGroups) { throw new Error('Specify at most one of \'defaultAction\' and \'defaultTargetGroups\''); } @@ -208,6 +218,29 @@ export class NetworkListener extends BaseListener implements INetworkListener { } } + /** + * Add one or more certificates to this listener. + * + * After the first certificate, this creates NetworkListenerCertificates + * resources since cloudformation requires the certificates array on the + * listener resource to have a length of 1. + */ + public addCertificates(id: string, certificates: IListenerCertificate[]): void { + const additionalCerts = [...certificates]; + if (this.certificateArns.length === 0 && additionalCerts.length > 0) { + const first = additionalCerts.splice(0, 1)[0]; + this.certificateArns.push(first.certificateArn); + } + // Only one certificate can be specified per resource, even though + // `certificates` is of type Array + for (let i = 0; i < additionalCerts.length; i++) { + new NetworkListenerCertificate(this, `${id}${i + 1}`, { + listener: this, + certificates: [additionalCerts[i]], + }); + } + } + /** * Load balance incoming requests to the given target groups. * diff --git a/packages/@aws-cdk/aws-elasticloadbalancingv2/test/nlb/listener.test.ts b/packages/@aws-cdk/aws-elasticloadbalancingv2/test/nlb/listener.test.ts index b92df26e76bb7..d5343c559d623 100644 --- a/packages/@aws-cdk/aws-elasticloadbalancingv2/test/nlb/listener.test.ts +++ b/packages/@aws-cdk/aws-elasticloadbalancingv2/test/nlb/listener.test.ts @@ -416,6 +416,62 @@ describe('tests', () => { })).toThrow(/Protocol must be TLS when certificates have been specified/); }); + test('Can pass multiple certificates to network listener constructor', () => { + // GIVEN + const stack = new cdk.Stack(); + const vpc = new ec2.Vpc(stack, 'Stack'); + const lb = new elbv2.NetworkLoadBalancer(stack, 'LB', { vpc }); + + // WHEN + lb.addListener('Listener', { + port: 443, + certificates: [ + importedCertificate(stack, 'cert1'), + importedCertificate(stack, 'cert2'), + ], + defaultTargetGroups: [new elbv2.NetworkTargetGroup(stack, 'Group', { vpc, port: 80 })], + }); + + // THEN + Template.fromStack(stack).hasResourceProperties('AWS::ElasticLoadBalancingV2::Listener', { + Protocol: 'TLS', + }); + + Template.fromStack(stack).hasResourceProperties('AWS::ElasticLoadBalancingV2::ListenerCertificate', { + Certificates: [{ CertificateArn: 'cert2' }], + }); + }); + + test('Can add multiple certificates to network listener after construction', () => { + // GIVEN + const stack = new cdk.Stack(); + const vpc = new ec2.Vpc(stack, 'Stack'); + const lb = new elbv2.NetworkLoadBalancer(stack, 'LB', { vpc }); + + // WHEN + const listener = lb.addListener('Listener', { + port: 443, + certificates: [ + importedCertificate(stack, 'cert1'), + ], + defaultTargetGroups: [new elbv2.NetworkTargetGroup(stack, 'Group', { vpc, port: 80 })], + }); + + listener.addCertificates('extra', [ + importedCertificate(stack, 'cert2'), + ]); + + + // THEN + Template.fromStack(stack).hasResourceProperties('AWS::ElasticLoadBalancingV2::Listener', { + Protocol: 'TLS', + }); + + Template.fromStack(stack).hasResourceProperties('AWS::ElasticLoadBalancingV2::ListenerCertificate', { + Certificates: [{ CertificateArn: 'cert2' }], + }); + }); + test('not allowed to specify defaultTargetGroups and defaultAction together', () => { // GIVEN const stack = new cdk.Stack(); @@ -462,3 +518,8 @@ class ResourceWithLBDependency extends cdk.CfnResource { this.node.addDependency(targetGroup.loadBalancerAttached); } } + +function importedCertificate(stack: cdk.Stack, + certificateArn = 'arn:aws:certificatemanager:123456789012:testregion:certificate/fd0b8392-3c0e-4704-81b6-8edf8612c852') { + return acm.Certificate.fromCertificateArn(stack, certificateArn, certificateArn); +}