Skip to content

Commit

Permalink
fix(elbv2): unable to add multiple certificates to NLB (#19289)
Browse files Browse the repository at this point in the history
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*
corymhall authored Mar 31, 2022

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
1 parent 88a7839 commit e8142e9
Showing 3 changed files with 139 additions and 2 deletions.
Original file line number Diff line number Diff line change
@@ -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,
});
}
}
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
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';
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.
*
Original file line number Diff line number Diff line change
@@ -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);
}

0 comments on commit e8142e9

Please sign in to comment.