From 32b98a8b0c85b3af656db8293b37a7551726515b Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Fri, 13 Dec 2019 19:46:56 +0100 Subject: [PATCH] refactor(elbv2): introduce ListenerCertificate (#5405) Finally model certificates in a consistent way, using an `IListenerCertificate` interface (it has to be an interface to be able to maintain backwards compatibility with the erroneously introduced `INetworkListenerCertificateProps` interface) and an implemention of it called `ListenerCertificate`. `ListenerCertificate` can currently be created from an ACM certificate, and in the future should also be creatable from an IAM certificate. Make it the same for ALB and NLBs. Fixes #5330. --- .../application-load-balanced-service-base.ts | 4 +- .../alb/application-listener-certificate.ts | 26 ++++++++++++- .../lib/alb/application-listener.ts | 39 +++++++++++++++---- .../aws-elasticloadbalancingv2/lib/index.ts | 1 + .../lib/nlb/network-listener.ts | 13 ++++--- .../lib/shared/listener-certificate.ts | 39 +++++++++++++++++++ .../aws-elasticloadbalancingv2/package.json | 3 +- .../test/alb/test.listener.ts | 26 +++++++++++++ .../test/nlb/test.listener.ts | 3 +- 9 files changed, 135 insertions(+), 19 deletions(-) create mode 100644 packages/@aws-cdk/aws-elasticloadbalancingv2/lib/shared/listener-certificate.ts diff --git a/packages/@aws-cdk/aws-ecs-patterns/lib/base/application-load-balanced-service-base.ts b/packages/@aws-cdk/aws-ecs-patterns/lib/base/application-load-balanced-service-base.ts index d3e37d2bc250e..359dec8e71949 100644 --- a/packages/@aws-cdk/aws-ecs-patterns/lib/base/application-load-balanced-service-base.ts +++ b/packages/@aws-cdk/aws-ecs-patterns/lib/base/application-load-balanced-service-base.ts @@ -1,7 +1,7 @@ import { DnsValidatedCertificate, ICertificate } from '@aws-cdk/aws-certificatemanager'; import { IVpc } from '@aws-cdk/aws-ec2'; import { AwsLogDriver, BaseService, CloudMapOptions, Cluster, ContainerImage, ICluster, LogDriver, PropagatedTagSource, Secret } from '@aws-cdk/aws-ecs'; -import { ApplicationListener, ApplicationLoadBalancer, ApplicationProtocol, ApplicationTargetGroup } from '@aws-cdk/aws-elasticloadbalancingv2'; +import { ApplicationListener, ApplicationLoadBalancer, ApplicationProtocol, ApplicationTargetGroup, ListenerCertificate } from '@aws-cdk/aws-elasticloadbalancingv2'; import { IRole } from '@aws-cdk/aws-iam'; import { AddressRecordTarget, ARecord, IHostedZone } from '@aws-cdk/aws-route53'; import { LoadBalancerTarget } from '@aws-cdk/aws-route53-targets'; @@ -330,7 +330,7 @@ export abstract class ApplicationLoadBalancedServiceBase extends cdk.Construct { } } if (this.certificate !== undefined) { - this.listener.addCertificateArns('Arns', [this.certificate.certificateArn]); + this.listener.addCertificates('Arns', [ListenerCertificate.fromCertificateManager(this.certificate)]); } let domainName = this.loadBalancer.loadBalancerDnsName; diff --git a/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/alb/application-listener-certificate.ts b/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/alb/application-listener-certificate.ts index 7c63722634fa0..e416fd715b700 100644 --- a/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/alb/application-listener-certificate.ts +++ b/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/alb/application-listener-certificate.ts @@ -1,5 +1,6 @@ import cdk = require('@aws-cdk/core'); import { CfnListenerCertificate } from '../elasticloadbalancingv2.generated'; +import { IListenerCertificate } from '../shared/listener-certificate'; import { IApplicationListener } from './application-listener'; /** @@ -15,8 +16,20 @@ export interface ApplicationListenerCertificateProps { * ARNs of certificates to attach * * Duplicates are not allowed. + * + * @deprecated Use `certificates` instead. + * @default - One of 'certificates' and 'certificateArns' is required. */ - readonly certificateArns: string[]; + readonly certificateArns?: string[]; + + /** + * Certificates to attach + * + * Duplicates are not allowed. + * + * @default - One of 'certificates' and 'certificateArns' is required. + */ + readonly certificates?: IListenerCertificate[]; } /** @@ -26,9 +39,18 @@ export class ApplicationListenerCertificate extends cdk.Construct { constructor(scope: cdk.Construct, id: string, props: ApplicationListenerCertificateProps) { super(scope, id); + if (!props.certificateArns && !props.certificates) { + throw new Error(`At least one of 'certificateArns' or 'certificates' is required`); + } + + const certificates = [ + ...(props.certificates || []).map(c => ({ certificateArn: c.certificateArn })), + ...(props.certificateArns || []).map(certificateArn => ({ certificateArn })), + ]; + new CfnListenerCertificate(this, 'Resource', { listenerArn: props.listener.listenerArn, - certificates: props.certificateArns.map(certificateArn => ({ certificateArn })), + certificates, }); } } 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 a3ae6e4086f47..b5d5a0fd842c4 100644 --- a/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/alb/application-listener.ts +++ b/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/alb/application-listener.ts @@ -3,6 +3,7 @@ import { Construct, Duration, IResource, Lazy, Resource } from '@aws-cdk/core'; import { BaseListener } from '../shared/base-listener'; import { HealthCheck } from '../shared/base-target-group'; import { ApplicationProtocol, SslPolicy } from '../shared/enums'; +import { IListenerCertificate, ListenerCertificate } from '../shared/listener-certificate'; import { determineProtocolAndPort } from '../shared/util'; import { ApplicationListenerCertificate } from './application-listener-certificate'; import { ApplicationListenerRule, FixedResponse, RedirectResponse, validateFixedResponse, validateRedirectResponse } from './application-listener-rule'; @@ -31,9 +32,17 @@ export interface BaseApplicationListenerProps { * The certificates to use on this listener * * @default - No certificates. + * @deprecated Use the `certificates` property instead */ readonly certificateArns?: string[]; + /** + * Certificate list of ACM cert ARNs + * + * @default - No certificates. + */ + readonly certificates?: IListenerCertificate[]; + /** * The security policy that defines which ciphers and protocols are supported. * @@ -115,7 +124,7 @@ export class ApplicationListener extends BaseListener implements IApplicationLis super(scope, id, { loadBalancerArn: props.loadBalancer.loadBalancerArn, - certificates: Lazy.anyValue({ produce: () => this.certificateArns.map(certificateArn => ({ certificateArn })) }, { omitEmptyArray: true}), + certificates: Lazy.anyValue({ produce: () => this.certificateArns.map(certificateArn => ({ certificateArn })) }, { omitEmptyArray: true }), protocol, port, sslPolicy: props.sslPolicy, @@ -129,6 +138,9 @@ export class ApplicationListener extends BaseListener implements IApplicationLis if (props.certificateArns && props.certificateArns.length > 0) { this.addCertificateArns("ListenerCertificate", props.certificateArns); } + if (props.certificates && props.certificates.length > 0) { + this.addCertificates("DefaultCertificates", props.certificates); + } // This listener edits the securitygroup of the load balancer, // but adds its own default port. @@ -150,19 +162,32 @@ export class ApplicationListener extends BaseListener implements IApplicationLis * After the first certificate, this creates ApplicationListenerCertificates * resources since cloudformation requires the certificates array on the * listener resource to have a length of 1. + * + * @deprecated Use `addCertificates` instead. */ public addCertificateArns(id: string, arns: string[]): void { - const additionalCertArns = [...arns]; + this.addCertificates(id, arns.map(ListenerCertificate.fromArn)); + } + + /** + * Add one or more certificates to this listener. + * + * After the first certificate, this creates ApplicationListenerCertificates + * 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 && additionalCertArns.length > 0) { - const first = additionalCertArns.splice(0, 1)[0]; - this.certificateArns.push(first); + if (this.certificateArns.length === 0 && additionalCerts.length > 0) { + const first = additionalCerts.splice(0, 1)[0]; + this.certificateArns.push(first.certificateArn); } - if (additionalCertArns.length > 0) { + if (additionalCerts.length > 0) { new ApplicationListenerCertificate(this, id, { listener: this, - certificateArns: additionalCertArns + certificates: additionalCerts }); } } diff --git a/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/index.ts b/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/index.ts index 1c20fc773424d..898031490fb37 100644 --- a/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/index.ts +++ b/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/index.ts @@ -16,3 +16,4 @@ export * from './shared/base-load-balancer'; export * from './shared/base-target-group'; export * from './shared/enums'; export * from './shared/load-balancer-targets'; +export * from './shared/listener-certificate'; 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 8da066d6555ad..2aa71dd644749 100644 --- a/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/nlb/network-listener.ts +++ b/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/nlb/network-listener.ts @@ -2,6 +2,7 @@ import { Construct, Duration, IResource, Resource } from '@aws-cdk/core'; import { BaseListener } from '../shared/base-listener'; import { HealthCheck } from '../shared/base-target-group'; import { Protocol, SslPolicy } from '../shared/enums'; +import { IListenerCertificate } from '../shared/listener-certificate'; import { INetworkLoadBalancer } from './network-load-balancer'; import { INetworkLoadBalancerTarget, INetworkTargetGroup, NetworkTargetGroup } from './network-target-group'; @@ -33,7 +34,7 @@ export interface BaseNetworkListenerProps { * * @default - No certificates. */ - readonly certificates?: INetworkListenerCertificateProps[]; + readonly certificates?: IListenerCertificate[]; /** * SSL Policy @@ -45,12 +46,12 @@ export interface BaseNetworkListenerProps { /** * Properties for adding a certificate to a listener + * + * This interface exists for backwards compatibility. + * + * @deprecated Use IListenerCertificate instead */ -export interface INetworkListenerCertificateProps { - /** - * Certificate ARN from ACM - */ - readonly certificateArn: string +export interface INetworkListenerCertificateProps extends IListenerCertificate { } /** diff --git a/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/shared/listener-certificate.ts b/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/shared/listener-certificate.ts new file mode 100644 index 0000000000000..5e3f2202040f0 --- /dev/null +++ b/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/shared/listener-certificate.ts @@ -0,0 +1,39 @@ +import acm = require('@aws-cdk/aws-certificatemanager'); + +/** + * A certificate source for an ELBv2 listener + */ +export interface IListenerCertificate { + /** + * The ARN of the certificate to use + */ + readonly certificateArn: string; +} + +/** + * A certificate source for an ELBv2 listener + */ +export class ListenerCertificate implements IListenerCertificate { + /** + * Use an ACM certificate as a listener certificate + */ + public static fromCertificateManager(acmCertificate: acm.ICertificate) { + return new ListenerCertificate(acmCertificate.certificateArn); + } + + /** + * Use any certificate, identified by its ARN, as a listener certificate + */ + public static fromArn(certificateArn: string) { + return new ListenerCertificate(certificateArn); + } + + /** + * The ARN of the certificate to use + */ + public readonly certificateArn: string; + + protected constructor(certificateArn: string) { + this.certificateArn = certificateArn; + } +} \ No newline at end of file diff --git a/packages/@aws-cdk/aws-elasticloadbalancingv2/package.json b/packages/@aws-cdk/aws-elasticloadbalancingv2/package.json index 3944eb30eb525..e6e54752d25d9 100644 --- a/packages/@aws-cdk/aws-elasticloadbalancingv2/package.json +++ b/packages/@aws-cdk/aws-elasticloadbalancingv2/package.json @@ -92,6 +92,7 @@ }, "awslint": { "exclude": [ + "no-unused-type:@aws-cdk/aws-elasticloadbalancingv2.INetworkListenerCertificateProps", "construct-ctor:@aws-cdk/aws-elasticloadbalancingv2.BaseListener..params[2]", "construct-ctor:@aws-cdk/aws-elasticloadbalancingv2.BaseLoadBalancer.", "construct-ctor:@aws-cdk/aws-elasticloadbalancingv2.TargetGroupBase.", @@ -121,4 +122,4 @@ ] }, "stability": "stable" -} \ No newline at end of file +} diff --git a/packages/@aws-cdk/aws-elasticloadbalancingv2/test/alb/test.listener.ts b/packages/@aws-cdk/aws-elasticloadbalancingv2/test/alb/test.listener.ts index b16d2283b6233..8a0e440f834db 100644 --- a/packages/@aws-cdk/aws-elasticloadbalancingv2/test/alb/test.listener.ts +++ b/packages/@aws-cdk/aws-elasticloadbalancingv2/test/alb/test.listener.ts @@ -5,6 +5,7 @@ import cdk = require('@aws-cdk/core'); import { ConstructNode, Duration } from '@aws-cdk/core'; import { Test } from 'nodeunit'; import elbv2 = require('../../lib'); +import { ListenerCertificate } from '../../lib'; import { FakeSelfRegisteringTarget } from '../helpers'; export = { @@ -859,6 +860,31 @@ export = { test.done(); }, + 'Can use certificate wrapper class'(test: Test) { + // 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, + certificates: [ListenerCertificate.fromArn('cert1'), ListenerCertificate.fromArn('cert2')], + defaultTargetGroups: [new elbv2.ApplicationTargetGroup(stack, 'Group', { vpc, port: 80 })] + }); + + // THEN + expect(stack).to(haveResource('AWS::ElasticLoadBalancingV2::Listener', { + Protocol: 'HTTPS' + })); + + expect(stack).to(haveResource('AWS::ElasticLoadBalancingV2::ListenerCertificate', { + Certificates: [{ CertificateArn: 'cert2' }], + })); + + test.done(); + }, + 'Can add additional certificates via addCertficateArns to application listener'(test: Test) { // GIVEN const stack = new cdk.Stack(); diff --git a/packages/@aws-cdk/aws-elasticloadbalancingv2/test/nlb/test.listener.ts b/packages/@aws-cdk/aws-elasticloadbalancingv2/test/nlb/test.listener.ts index ef1189ecb3663..ccd58ff5596f5 100644 --- a/packages/@aws-cdk/aws-elasticloadbalancingv2/test/nlb/test.listener.ts +++ b/packages/@aws-cdk/aws-elasticloadbalancingv2/test/nlb/test.listener.ts @@ -4,6 +4,7 @@ import ec2 = require('@aws-cdk/aws-ec2'); import cdk = require('@aws-cdk/core'); import { Test } from 'nodeunit'; import elbv2 = require('../../lib'); +import { ListenerCertificate } from '../../lib'; import { FakeSelfRegisteringTarget } from '../helpers'; export = { @@ -155,7 +156,7 @@ export = { lb.addListener('Listener', { port: 443, protocol: elbv2.Protocol.TLS, - certificates: [ { certificateArn: cert.certificateArn } ], + certificates: [ ListenerCertificate.fromCertificateManager(cert) ], sslPolicy: elbv2.SslPolicy.TLS12, defaultTargetGroups: [new elbv2.NetworkTargetGroup(stack, 'Group', { vpc, port: 80 })] });