From 011b2f456e3eef6ef30be5c242fdc1bfc4f37d73 Mon Sep 17 00:00:00 2001 From: Tim Trinidad Date: Tue, 16 Jul 2019 09:58:58 -0700 Subject: [PATCH] use alb ID instead of priority number --- lib/plugins/aws/lib/naming.js | 12 ++--- lib/plugins/aws/lib/naming.test.js | 12 ++--- .../compile/events/alb/lib/listenerRules.js | 2 +- .../events/alb/lib/listenerRules.test.js | 6 ++- .../compile/events/alb/lib/targetGroups.js | 8 ++-- .../events/alb/lib/targetGroups.test.js | 14 +++--- .../compile/events/alb/lib/validate.js | 20 ++++++++ .../compile/events/alb/lib/validate.test.js | 46 +++++++++++++++++++ 8 files changed, 95 insertions(+), 25 deletions(-) diff --git a/lib/plugins/aws/lib/naming.js b/lib/plugins/aws/lib/naming.js index 82280d884..db263aae2 100644 --- a/lib/plugins/aws/lib/naming.js +++ b/lib/plugins/aws/lib/naming.js @@ -374,18 +374,18 @@ module.exports = { }, // ALB - getAlbTargetGroupLogicalId(functionName, priority) { - return `${this.getNormalizedFunctionName(functionName)}AlbTargetGroup${priority}`; + getAlbTargetGroupLogicalId(functionName, albId) { + return `${this.getNormalizedFunctionName(functionName)}AlbTargetGroup${albId}`; }, - getAlbTargetGroupNameTagValue(functionName, priority) { + getAlbTargetGroupNameTagValue(functionName, albId) { return `${ this.provider.serverless.service.service - }-${functionName}-${priority}-${this.provider.getStage()}`; + }-${functionName}-${albId}-${this.provider.getStage()}`; }, - getAlbTargetGroupName(functionName, priority) { + getAlbTargetGroupName(functionName, albId) { return crypto .createHash('md5') - .update(this.getAlbTargetGroupNameTagValue(functionName, priority)) + .update(this.getAlbTargetGroupNameTagValue(functionName, albId)) .digest('hex'); }, getAlbListenerRuleLogicalId(functionName, idx) { diff --git a/lib/plugins/aws/lib/naming.test.js b/lib/plugins/aws/lib/naming.test.js index e5dc77794..0d5521444 100644 --- a/lib/plugins/aws/lib/naming.test.js +++ b/lib/plugins/aws/lib/naming.test.js @@ -674,8 +674,8 @@ describe('#naming()', () => { describe('#getAlbTargetGroupLogicalId()', () => { it('should normalize the function name', () => { - expect(sdk.naming.getAlbTargetGroupLogicalId('functionName', 1)).to.equal( - 'FunctionNameAlbTargetGroup1' + expect(sdk.naming.getAlbTargetGroupLogicalId('functionName', 'abc123')).to.equal( + 'FunctionNameAlbTargetGroupabc123' ); }); }); @@ -691,8 +691,8 @@ describe('#naming()', () => { describe('#getAlbTargetGroupName()', () => { it('should return a unique identifier based on the service name, function name and stage', () => { serverless.service.service = 'myService'; - expect(sdk.naming.getAlbTargetGroupName('functionName', 1)).to.equal( - '4e188e517d5406742f43905b64d911a9' + expect(sdk.naming.getAlbTargetGroupName('functionName', 'abc123')).to.equal( + '61615b0fb1c56b80657106894392e27f' ); }); }); @@ -700,8 +700,8 @@ describe('#naming()', () => { describe('#getAlbTargetGroupNameTagValue()', () => { it('should return the composition of service name, function name and stage', () => { serverless.service.service = 'myService'; - expect(sdk.naming.getAlbTargetGroupNameTagValue('functionName', 1)).to.equal( - `${serverless.service.service}-functionName-1-${sdk.naming.provider.getStage()}` + expect(sdk.naming.getAlbTargetGroupNameTagValue('functionName', 'abc123')).to.equal( + `${serverless.service.service}-functionName-abc123-${sdk.naming.provider.getStage()}` ); }); }); diff --git a/lib/plugins/aws/package/compile/events/alb/lib/listenerRules.js b/lib/plugins/aws/package/compile/events/alb/lib/listenerRules.js index bda865b99..3f5fdc780 100644 --- a/lib/plugins/aws/package/compile/events/alb/lib/listenerRules.js +++ b/lib/plugins/aws/package/compile/events/alb/lib/listenerRules.js @@ -9,7 +9,7 @@ module.exports = { ); const targetGroupLogicalId = this.provider.naming.getAlbTargetGroupLogicalId( event.functionName, - event.priority + event.albId ); const Conditions = [ diff --git a/lib/plugins/aws/package/compile/events/alb/lib/listenerRules.test.js b/lib/plugins/aws/package/compile/events/alb/lib/listenerRules.test.js index cf2dc8ab5..b65e67eb0 100644 --- a/lib/plugins/aws/package/compile/events/alb/lib/listenerRules.test.js +++ b/lib/plugins/aws/package/compile/events/alb/lib/listenerRules.test.js @@ -22,6 +22,7 @@ describe('#compileListenerRules()', () => { events: [ { functionName: 'first', + albId: '50dc6c495c0c9188', listenerArn: 'arn:aws:elasticloadbalancing:' + 'us-east-1:123456789012:listener/app/my-load-balancer/' + @@ -34,6 +35,7 @@ describe('#compileListenerRules()', () => { }, { functionName: 'second', + albId: '50dc6c495c0c9188', listenerArn: 'arn:aws:elasticloadbalancing:' + 'us-east-1:123456789012:listener/app/my-load-balancer/' + @@ -57,7 +59,7 @@ describe('#compileListenerRules()', () => { Actions: [ { TargetGroupArn: { - Ref: 'FirstAlbTargetGroup1', + Ref: 'FirstAlbTargetGroup50dc6c495c0c9188', }, Type: 'forward', }, @@ -85,7 +87,7 @@ describe('#compileListenerRules()', () => { Actions: [ { TargetGroupArn: { - Ref: 'SecondAlbTargetGroup2', + Ref: 'SecondAlbTargetGroup50dc6c495c0c9188', }, Type: 'forward', }, diff --git a/lib/plugins/aws/package/compile/events/alb/lib/targetGroups.js b/lib/plugins/aws/package/compile/events/alb/lib/targetGroups.js index 8eab58d35..16ae779f4 100644 --- a/lib/plugins/aws/package/compile/events/alb/lib/targetGroups.js +++ b/lib/plugins/aws/package/compile/events/alb/lib/targetGroups.js @@ -3,11 +3,11 @@ module.exports = { compileTargetGroups() { this.validated.events.forEach(event => { - const { functionName, priority } = event; + const { functionName, albId } = event; const targetGroupLogicalId = this.provider.naming.getAlbTargetGroupLogicalId( functionName, - priority + albId ); const lambdaLogicalId = this.provider.naming.getLambdaLogicalId(functionName); const lambdaPermissionLogicalId = this.provider.naming.getLambdaAlbPermissionLogicalId( @@ -26,11 +26,11 @@ module.exports = { }, }, ], - Name: this.provider.naming.getAlbTargetGroupName(functionName, priority), + Name: this.provider.naming.getAlbTargetGroupName(functionName, albId), Tags: [ { Key: 'Name', - Value: this.provider.naming.getAlbTargetGroupNameTagValue(functionName, priority), + Value: this.provider.naming.getAlbTargetGroupNameTagValue(functionName, albId), }, ], }, diff --git a/lib/plugins/aws/package/compile/events/alb/lib/targetGroups.test.js b/lib/plugins/aws/package/compile/events/alb/lib/targetGroups.test.js index d60205017..3d2235b69 100644 --- a/lib/plugins/aws/package/compile/events/alb/lib/targetGroups.test.js +++ b/lib/plugins/aws/package/compile/events/alb/lib/targetGroups.test.js @@ -22,6 +22,7 @@ describe('#compileTargetGroups()', () => { events: [ { functionName: 'first', + albId: '50dc6c495c0c9188', listenerArn: 'arn:aws:elasticloadbalancing:' + 'us-east-1:123456789012:listener/app/my-load-balancer/' + @@ -34,6 +35,7 @@ describe('#compileTargetGroups()', () => { }, { functionName: 'second', + albId: '50dc6c495c0c9188', listenerArn: 'arn:aws:elasticloadbalancing:' + 'us-east-1:123456789012:listener/app/my-load-balancer/' + @@ -51,10 +53,10 @@ describe('#compileTargetGroups()', () => { const resources = awsCompileAlbEvents.serverless.service.provider.compiledCloudFormationTemplate.Resources; - expect(resources.FirstAlbTargetGroup1).to.deep.equal({ + expect(resources.FirstAlbTargetGroup50dc6c495c0c9188).to.deep.equal({ Type: 'AWS::ElasticLoadBalancingV2::TargetGroup', Properties: { - Name: 'f84548ee49366c89ada86c9655ea00b5', + Name: '879129784b3012595bceeaa4a76fc7bc', TargetType: 'lambda', Targets: [ { @@ -66,16 +68,16 @@ describe('#compileTargetGroups()', () => { Tags: [ { Key: 'Name', - Value: 'some-service-first-1-dev', + Value: 'some-service-first-50dc6c495c0c9188-dev', }, ], }, DependsOn: ['FirstLambdaPermissionAlb'], }); - expect(resources.SecondAlbTargetGroup2).to.deep.equal({ + expect(resources.SecondAlbTargetGroup50dc6c495c0c9188).to.deep.equal({ Type: 'AWS::ElasticLoadBalancingV2::TargetGroup', Properties: { - Name: '6dd7b94b30032ee093c50743d117b64d', + Name: '2107a18b6db85bd904d38cb2bdf5af5c', TargetType: 'lambda', Targets: [ { @@ -87,7 +89,7 @@ describe('#compileTargetGroups()', () => { Tags: [ { Key: 'Name', - Value: 'some-service-second-2-dev', + Value: 'some-service-second-50dc6c495c0c9188-dev', }, ], }, diff --git a/lib/plugins/aws/package/compile/events/alb/lib/validate.js b/lib/plugins/aws/package/compile/events/alb/lib/validate.js index e38dc9f70..1b58bb6e3 100644 --- a/lib/plugins/aws/package/compile/events/alb/lib/validate.js +++ b/lib/plugins/aws/package/compile/events/alb/lib/validate.js @@ -5,6 +5,8 @@ const _ = require('lodash'); // eslint-disable-next-line max-len const CIDR_IPV6_PATTERN = /^s*((([0-9A-Fa-f]{1,4}:){7}([0-9A-Fa-f]{1,4}|:))|(([0-9A-Fa-f]{1,4}:){6}(:[0-9A-Fa-f]{1,4}|((25[0-5]|2[0-4]d|1dd|[1-9]?d)(.(25[0-5]|2[0-4]d|1dd|[1-9]?d)){3})|:))|(([0-9A-Fa-f]{1,4}:){5}(((:[0-9A-Fa-f]{1,4}){1,2})|:((25[0-5]|2[0-4]d|1dd|[1-9]?d)(.(25[0-5]|2[0-4]d|1dd|[1-9]?d)){3})|:))|(([0-9A-Fa-f]{1,4}:){4}(((:[0-9A-Fa-f]{1,4}){1,3})|((:[0-9A-Fa-f]{1,4})?:((25[0-5]|2[0-4]d|1dd|[1-9]?d)(.(25[0-5]|2[0-4]d|1dd|[1-9]?d)){3}))|:))|(([0-9A-Fa-f]{1,4}:){3}(((:[0-9A-Fa-f]{1,4}){1,4})|((:[0-9A-Fa-f]{1,4}){0,2}:((25[0-5]|2[0-4]d|1dd|[1-9]?d)(.(25[0-5]|2[0-4]d|1dd|[1-9]?d)){3}))|:))|(([0-9A-Fa-f]{1,4}:){2}(((:[0-9A-Fa-f]{1,4}){1,5})|((:[0-9A-Fa-f]{1,4}){0,3}:((25[0-5]|2[0-4]d|1dd|[1-9]?d)(.(25[0-5]|2[0-4]d|1dd|[1-9]?d)){3}))|:))|(([0-9A-Fa-f]{1,4}:){1}(((:[0-9A-Fa-f]{1,4}){1,6})|((:[0-9A-Fa-f]{1,4}){0,4}:((25[0-5]|2[0-4]d|1dd|[1-9]?d)(.(25[0-5]|2[0-4]d|1dd|[1-9]?d)){3}))|:))|(:(((:[0-9A-Fa-f]{1,4}){1,7})|((:[0-9A-Fa-f]{1,4}){0,5}:((25[0-5]|2[0-4]d|1dd|[1-9]?d)(.(25[0-5]|2[0-4]d|1dd|[1-9]?d)){3}))|:)))(%.+)?s*(\/([0-9]|[1-9][0-9]|1[0-1][0-9]|12[0-8]))$/; const CIDR_IPV4_PATTERN = /^([0-9]{1,3}\.){3}[0-9]{1,3}(\/([0-9]|[1-2][0-9]|3[0-2]))$/; +// see https://docs.aws.amazon.com/general/latest/gr/aws-arns-and-namespaces.html#arn-syntax-elb-application +const ALB_LISTENER_PATTERN = /^arn:aws:elasticloadbalancing:.+:listener\/app\/[\w-]+\/([\w-]+)\/[\w-]+$/; module.exports = { validate() { @@ -15,6 +17,7 @@ module.exports = { if (_.has(event, 'alb')) { if (_.isObject(event.alb)) { const albObj = { + albId: this.validateListenerArnAndExtractAlbId(event, functionName), listenerArn: event.alb.listenerArn, priority: event.alb.priority, conditions: { @@ -50,6 +53,23 @@ module.exports = { }; }, + validateListenerArnAndExtractAlbId(event, functionName) { + const listenerArn = event.alb.listenerArn; + if (!listenerArn) { + throw new this.serverless.classes.Error( + `listenerArn is missing in function "${functionName}".` + ); + } + const matches = listenerArn.match(ALB_LISTENER_PATTERN); + if (!matches) { + throw new this.serverless.classes.Error( + `Invalid ALB listenerArn in function "${functionName}".` + ); + } + + return matches[1]; + }, + validateHeaderCondition(event, functionName) { const messageTitle = `Invalid ALB event "header" condition in function "${functionName}".`; if ( diff --git a/lib/plugins/aws/package/compile/events/alb/lib/validate.test.js b/lib/plugins/aws/package/compile/events/alb/lib/validate.test.js index a5b69e3f5..d77813514 100644 --- a/lib/plugins/aws/package/compile/events/alb/lib/validate.test.js +++ b/lib/plugins/aws/package/compile/events/alb/lib/validate.test.js @@ -1,5 +1,6 @@ 'use strict'; +const _ = require('lodash'); const expect = require('chai').expect; const AwsCompileAlbEvents = require('../index'); const Serverless = require('../../../../../../../Serverless'); @@ -65,6 +66,7 @@ describe('#validate()', () => { expect(validated.events).to.deep.equal([ { functionName: 'first', + albId: '50dc6c495c0c9188', listenerArn: 'arn:aws:elasticloadbalancing:' + 'us-east-1:123456789012:listener/app/my-load-balancer/' + @@ -79,6 +81,7 @@ describe('#validate()', () => { }, { functionName: 'second', + albId: '50dc6c495c0c9188', listenerArn: 'arn:aws:elasticloadbalancing:' + 'us-east-1:123456789012:listener/app/my-load-balancer/' + @@ -167,6 +170,49 @@ describe('#validate()', () => { expect(() => awsCompileAlbEvents.validate()).to.throw(Error); }); + describe('#validateListenerArnAndExtractAlbId()', () => { + it('returns the alb ID when given a valid listener ARN', () => { + const event = { + alb: { + listenerArn: + 'arn:aws:elasticloadbalancing:us-east-1:123456789012:listener/app/my-load-balancer/50dc6c495c0c9188/f2f7dc8efc522ab2', + }, + }; + expect( + awsCompileAlbEvents.validateListenerArnAndExtractAlbId(event, 'functionname') + ).to.be.equal('50dc6c495c0c9188'); + }); + + it('throws an error if the listener ARN is missing', () => { + const listenerArns = [undefined, null, false, '']; + _.forEach(listenerArns, listenerArn => { + const event = { alb: { listenerArn } }; + expect(() => + awsCompileAlbEvents.validateListenerArnAndExtractAlbId(event, 'functionname') + ).to.throw('listenerArn is missing in function "functionname".'); + }); + }); + + it('throws an error if the listener ARN is invalid', () => { + const listenerArns = [ + // ALB listener rule (not a listener) + 'arn:aws:elasticloadbalancing:us-east-1:123456789012:listener-rule/app/my-load-balancer/50dc6c495c0c9188/f2f7dc8efc522ab2/9683b2d02a6cabee', + // ELB + 'arn:aws:elasticloadbalancing:us-east-1:123456789012:listener/net/my-load-balancer/50dc6c495c0c9188/f2f7dc8efc522ab2', + // Non ec2 ARN + 'arn:aws:iam::123456789012:server-certificate/division_abc/subdivision_xyz/ProdServerCert', + // something completely different + 'foo', + ]; + _.forEach(listenerArns, listenerArn => { + const event = { alb: { listenerArn } }; + expect(() => + awsCompileAlbEvents.validateListenerArnAndExtractAlbId(event, 'functionname') + ).to.throw('Invalid ALB listenerArn in function "functionname".'); + }); + }); + }); + describe('#validateIpCondition()', () => { it('should throw if ip is not a valid ipv6 or ipv4 cidr block', () => { const event = { alb: { conditions: { ip: 'fe80:0000:0000:0000:0204:61ff:fe9d:f156/' } } };