From 1a30272c8bd99a919bde695b5b1b1f5cb458cb64 Mon Sep 17 00:00:00 2001 From: OksanaH <34384274+OksanaH@users.noreply.github.com> Date: Mon, 12 Apr 2021 12:39:22 +0100 Subject: [PATCH] feat(elasticloadbalancing): rename 'sslCertificateId' property of LB listener to 'sslCertificateArn'; deprecate sslCertificateId property (#13766) The property `sslCertificateId` of the LoadBalancer listener actually means sslCertificateArn. So as suggested in #9303, I have deprecated `sslCertificateId` and replaced it by `sslCertificateArn` to better reflect its actual meaning. fixes #9303 --- .../lib/load-balancer.ts | 16 ++- .../test/loadbalancer.test.ts | 103 +++++++++++++++--- 2 files changed, 102 insertions(+), 17 deletions(-) diff --git a/packages/@aws-cdk/aws-elasticloadbalancing/lib/load-balancer.ts b/packages/@aws-cdk/aws-elasticloadbalancing/lib/load-balancer.ts index eaed2daee58d2..d60d91e0a9a51 100644 --- a/packages/@aws-cdk/aws-elasticloadbalancing/lib/load-balancer.ts +++ b/packages/@aws-cdk/aws-elasticloadbalancing/lib/load-balancer.ts @@ -187,10 +187,18 @@ export interface LoadBalancerListener { readonly policyNames?: string[]; /** - * ID of SSL certificate + * the ARN of the SSL certificate + * @deprecated - use sslCertificateArn instead */ readonly sslCertificateId?: string; + /** + * the ARN of the SSL certificate + * + * @default - none + */ + readonly sslCertificateArn?: string; + /** * Allow connections to the load balancer from the given set of connection peers * @@ -264,8 +272,12 @@ export class LoadBalancer extends Resource implements IConnectable { * @returns A ListenerPort object that controls connections to the listener port */ public addListener(listener: LoadBalancerListener): ListenerPort { + if (listener.sslCertificateArn && listener.sslCertificateId) { + throw new Error('"sslCertificateId" is deprecated, please use "sslCertificateArn" only.'); + } const protocol = ifUndefinedLazy(listener.externalProtocol, () => wellKnownProtocol(listener.externalPort)); const instancePort = listener.internalPort || listener.externalPort; + const sslCertificateArn = listener.sslCertificateArn || listener.sslCertificateId; const instanceProtocol = ifUndefined(listener.internalProtocol, ifUndefined(tryWellKnownProtocol(instancePort), isHttpProtocol(protocol) ? LoadBalancingProtocol.HTTP : LoadBalancingProtocol.TCP)); @@ -275,7 +287,7 @@ export class LoadBalancer extends Resource implements IConnectable { protocol, instancePort: instancePort.toString(), instanceProtocol, - sslCertificateId: listener.sslCertificateId, + sslCertificateId: sslCertificateArn, policyNames: listener.policyNames, }); diff --git a/packages/@aws-cdk/aws-elasticloadbalancing/test/loadbalancer.test.ts b/packages/@aws-cdk/aws-elasticloadbalancing/test/loadbalancer.test.ts index 60bf1ee3632bb..9cac87e057e87 100644 --- a/packages/@aws-cdk/aws-elasticloadbalancing/test/loadbalancer.test.ts +++ b/packages/@aws-cdk/aws-elasticloadbalancing/test/loadbalancer.test.ts @@ -1,4 +1,4 @@ -import { expect, haveResource } from '@aws-cdk/assert-internal'; +import '@aws-cdk/assert-internal/jest'; import { Connections, Peer, SubnetType, Vpc } from '@aws-cdk/aws-ec2'; import { Duration, Stack } from '@aws-cdk/core'; import { ILoadBalancerTarget, LoadBalancer, LoadBalancingProtocol } from '../lib'; @@ -18,14 +18,14 @@ describe('tests', () => { internalPort: 8080, }); - expect(stack).to(haveResource('AWS::ElasticLoadBalancing::LoadBalancer', { + expect(stack).toHaveResource('AWS::ElasticLoadBalancing::LoadBalancer', { Listeners: [{ InstancePort: '8080', InstanceProtocol: 'http', LoadBalancerPort: '8080', Protocol: 'http', }], - })); + }); }); test('add a health check', () => { @@ -45,7 +45,7 @@ describe('tests', () => { }); // THEN - expect(stack).to(haveResource('AWS::ElasticLoadBalancing::LoadBalancer', { + expect(stack).toHaveResource('AWS::ElasticLoadBalancing::LoadBalancer', { HealthCheck: { HealthyThreshold: '2', Interval: '60', @@ -53,7 +53,7 @@ describe('tests', () => { Timeout: '5', UnhealthyThreshold: '5', }, - })); + }); }); test('add a listener and load balancing target', () => { @@ -75,7 +75,7 @@ describe('tests', () => { elb.addTarget(new FakeTarget()); // THEN: at the very least it added a security group rule for the backend - expect(stack).to(haveResource('AWS::EC2::SecurityGroup', { + expect(stack).toHaveResource('AWS::EC2::SecurityGroup', { SecurityGroupEgress: [ { Description: 'Port 8080 LB to fleet', @@ -85,7 +85,7 @@ describe('tests', () => { ToPort: 8080, }, ], - })); + }); }); test('enable cross zone load balancing', () => { @@ -100,9 +100,9 @@ describe('tests', () => { }); // THEN - expect(stack).to(haveResource('AWS::ElasticLoadBalancing::LoadBalancer', { + expect(stack).toHaveResource('AWS::ElasticLoadBalancing::LoadBalancer', { CrossZone: true, - })); + }); }); test('disable cross zone load balancing', () => { @@ -117,9 +117,9 @@ describe('tests', () => { }); // THEN - expect(stack).to(haveResource('AWS::ElasticLoadBalancing::LoadBalancer', { + expect(stack).toHaveResource('AWS::ElasticLoadBalancing::LoadBalancer', { CrossZone: false, - })); + }); }); test('cross zone load balancing enabled by default', () => { @@ -133,9 +133,9 @@ describe('tests', () => { }); // THEN - expect(stack).to(haveResource('AWS::ElasticLoadBalancing::LoadBalancer', { + expect(stack).toHaveResource('AWS::ElasticLoadBalancing::LoadBalancer', { CrossZone: true, - })); + }); }); test('use specified subnet', () => { @@ -170,11 +170,84 @@ describe('tests', () => { }); // THEN - expect(stack).to(haveResource('AWS::ElasticLoadBalancing::LoadBalancer', { + expect(stack).toHaveResource('AWS::ElasticLoadBalancing::LoadBalancer', { Subnets: vpc.selectSubnets({ subnetGroupName: 'private1', }).subnetIds.map((subnetId: string) => stack.resolve(subnetId)), - })); + }); + }); + + test('does not fail when deprecated property sslCertificateId is used', () => { + // GIVEN + const sslCertificateArn = 'arn:aws:acm:us-east-1:12345:test/12345'; + const stack = new Stack(); + const vpc = new Vpc(stack, 'VCP'); + + // WHEN + const lb = new LoadBalancer(stack, 'LB', { vpc }); + + lb.addListener({ + externalPort: 80, + internalPort: 8080, + sslCertificateId: sslCertificateArn, + }); + + // THEN + expect(stack).toHaveResource('AWS::ElasticLoadBalancing::LoadBalancer', { + Listeners: [{ + InstancePort: '8080', + InstanceProtocol: 'http', + LoadBalancerPort: '80', + Protocol: 'http', + SSLCertificateId: sslCertificateArn, + }], + }); + }); + + test('does not fail when sslCertificateArn is used', () => { + // GIVEN + const sslCertificateArn = 'arn:aws:acm:us-east-1:12345:test/12345'; + const stack = new Stack(); + const vpc = new Vpc(stack, 'VCP'); + + // WHEN + const lb = new LoadBalancer(stack, 'LB', { vpc }); + + lb.addListener({ + externalPort: 80, + internalPort: 8080, + sslCertificateArn: sslCertificateArn, + }); + + // THEN + expect(stack).toHaveResource('AWS::ElasticLoadBalancing::LoadBalancer', { + Listeners: [{ + InstancePort: '8080', + InstanceProtocol: 'http', + LoadBalancerPort: '80', + Protocol: 'http', + SSLCertificateId: sslCertificateArn, + }], + }); + }); + + test('throws error when both sslCertificateId and sslCertificateArn are used', () => { + // GIVEN + const sslCertificateArn = 'arn:aws:acm:us-east-1:12345:test/12345'; + const stack = new Stack(); + const vpc = new Vpc(stack, 'VCP'); + + // WHEN + const lb = new LoadBalancer(stack, 'LB', { vpc }); + + // THEN + expect(() => + lb.addListener({ + externalPort: 80, + internalPort: 8080, + sslCertificateArn: sslCertificateArn, + sslCertificateId: sslCertificateArn, + })).toThrow(/"sslCertificateId" is deprecated, please use "sslCertificateArn" only./); }); });