refactor(elbv2): introduce ListenerCertificate (aws#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 aws#5330.
ed-at-work · Dec 17, 2019 · 34ad4c0 · 34ad4c0
1 parent 8a25894
commit 34ad4c0
Show file tree

Hide file tree

Showing 9 changed files with 135 additions and 19 deletions.
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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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;
+  }
+}
diff --git 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.<initializer>.params[2]",
       "construct-ctor:@aws-cdk/aws-elasticloadbalancingv2.BaseLoadBalancer.<initializer>",
       "construct-ctor:@aws-cdk/aws-elasticloadbalancingv2.TargetGroupBase.<initializer>",
@@ -121,4 +122,4 @@
     ]
   },
   "stability": "stable"
-}
+}
diff --git 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
@@ -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 })]
     });