From 2bf3bd84378ec39c5778ba96deff85f8bc7d582a Mon Sep 17 00:00:00 2001 From: Justin Plock Date: Tue, 4 Jun 2019 08:49:48 -0400 Subject: [PATCH] Cleanups --- README.md | 12 +++---- __tests__/index.test.js | 22 ++++++++++-- __tests__/subnet_groups.test.js | 60 +++++++++++++++++++++++---------- package-lock.json | 2 +- package.json | 2 +- src/constants.js | 6 ++++ src/index.js | 23 ++++++------- src/subnet_groups.js | 26 +++++--------- 8 files changed, 92 insertions(+), 61 deletions(-) diff --git a/README.md b/README.md index a5821850..c4701a67 100644 --- a/README.md +++ b/README.md @@ -41,7 +41,7 @@ If your Lambda functions need to [access the internet](https://docs.aws.amazon.c By default, `AWS::EC2::VPCEndpoint` "Gateway" endpoints for S3 and DynamoDB will be provisioned within each availability zone to provide internal access to these services (there is no additional charge for using Gateway Type VPC endpoints). You can selectively control which `AWS::EC2::VPCEndpoint` "Interface" endpoints are available within your VPC using the `services` configuration option below. Not all AWS services are available in every region, so the plugin will query AWS to validate the services you have selected and notify you if any changes are required (there is an additional charge for using Interface Type VPC endpoints). -If you specify more then one availability zone, this plugin will also provision the following database-related resources: +If you specify more then one availability zone, this plugin will also provision the following database-related resources (controlled using the `subnetGroups` plugin option): - `AWS::RDS::DBSubnetGroup` - `AWS::ElastiCache::SubnetGroup` @@ -99,13 +99,13 @@ custom: # Whether to create a NAT instance createNatInstance: false - # optionally specify AZs (defaults to auto-discover all availabile AZs) + # Optionally specify AZs (defaults to auto-discover all availabile AZs) zones: - us-east-1a - us-east-1b - us-east-1c - # by default, s3 and dynamodb endpoints will be available within the VPC + # By default, S3 and DynamoDB endpoints will be available within the VPC # see https://docs.aws.amazon.com/vpc/latest/userguide/vpc-endpoints.html # for a list of available service endpoints to provision within the VPC # (varies per region) @@ -113,10 +113,8 @@ custom: - kms - secretsmanager - # optional - # can pick one of subnet groups in (rds / redshift / elasticache / dax) - # By default, if not specified, all of the subnet groups will be created. - # ex) DAX is not available in ap-northeast-2 so I don't want to make DAX subnet group + # Optionally specify subnet groups to create. If not provided, subnet groups + # for RDS, Redshift, ElasticCache and DAX will be provisioned. subnetGroups: - rds ``` diff --git a/__tests__/index.test.js b/__tests__/index.test.js index 63dfbe4f..e99e6481 100644 --- a/__tests__/index.test.js +++ b/__tests__/index.test.js @@ -53,7 +53,8 @@ describe('ServerlessVpcPlugin', () => { it('should initialize with custom options', () => { const options = { - zones: ['us-west-2'], + zones: ['us-west-2a'], + services: [], }; plugin = new ServerlessVpcPlugin(serverless, options); expect(plugin.serverless).toBeInstanceOf(Serverless); @@ -72,6 +73,7 @@ describe('ServerlessVpcPlugin', () => { it('should require a bastion key name', async () => { serverless.service.custom.vpcConfig = { createBastionHost: true, + services: [], }; await expect(plugin.afterInitialize()).rejects.toThrow( @@ -83,7 +85,8 @@ describe('ServerlessVpcPlugin', () => { it('createNatGateway should be either boolean or a number', async () => { serverless.service.custom.vpcConfig = { createNatGateway: 'hello', - zones: ['us-east-1'], + zones: ['us-east-1a'], + services: [], }; await expect(plugin.afterInitialize()).rejects.toThrow( @@ -109,7 +112,7 @@ describe('ServerlessVpcPlugin', () => { AWS.mock('EC2', 'describeAvailabilityZones', mockCallback); serverless.service.custom.vpcConfig = { - services: [], // remove default s3 and dynamodb + services: [], }; const actual = await plugin.afterInitialize(); @@ -117,6 +120,19 @@ describe('ServerlessVpcPlugin', () => { expect(mockCallback).toHaveBeenCalled(); expect.assertions(3); }); + + it('rejects invalid subnet groups', async () => { + serverless.service.custom.vpcConfig = { + zones: ['us-east-1a', 'us-east-1b'], + subnetGroups: ['invalid'], + services: [], + }; + + await expect(plugin.afterInitialize()).rejects.toThrow( + 'WARNING: Invalid subnetGroups option. Valid options: rds, redshift, elasticache, dax', + ); + expect.assertions(1); + }); }); describe('#getZonesPerRegion', () => { diff --git a/__tests__/subnet_groups.test.js b/__tests__/subnet_groups.test.js index cecedee9..0eae2489 100644 --- a/__tests__/subnet_groups.test.js +++ b/__tests__/subnet_groups.test.js @@ -7,8 +7,6 @@ const { } = require('../src/subnet_groups'); describe('subnet_groups', () => { - let subnetGroupList = {}; - describe('#buildRDSSubnetGroup', () => { it('skips building an RDS subnet group with no zones', () => { const actual = buildRDSSubnetGroup(); @@ -38,7 +36,6 @@ describe('subnet_groups', () => { }, }, }; - subnetGroupList.rds = expected; const actual = buildRDSSubnetGroup(2); expect(actual).toEqual(expected); expect.assertions(1); @@ -103,7 +100,6 @@ describe('subnet_groups', () => { }, }, }; - subnetGroupList.elasticache = expected; const actual = buildElastiCacheSubnetGroup(2); expect(actual).toEqual(expected); expect.assertions(1); @@ -165,7 +161,6 @@ describe('subnet_groups', () => { }, }, }; - subnetGroupList.redshift = expected; const actual = buildRedshiftSubnetGroup(2); expect(actual).toEqual(expected); expect.assertions(1); @@ -227,7 +222,6 @@ describe('subnet_groups', () => { }, }, }; - subnetGroupList.dax = expected; const actual = buildDAXSubnetGroup(2); expect(actual).toEqual(expected); expect.assertions(1); @@ -264,22 +258,52 @@ describe('subnet_groups', () => { }); describe('#buildSubnetGroups', () => { - it('no subnetGroups option', () => { - const expected = Object.keys(subnetGroupList).reduce( - (acc, key) => Object.assign(acc, subnetGroupList[key]), - {}, - ); + it('skips building if no groups specified', () => { + const expected = {}; const actual = buildSubnetGroups(2, []); expect(actual).toEqual(expected); expect.assertions(1); }); - it('get specific subnetGroups option', () => { - const getSubnetGroupsOptions = ['rds', 'redshift']; - const expected = getSubnetGroupsOptions.reduce( - (acc, key) => Object.assign(acc, subnetGroupList[key]), - {}, - ); - const actual = buildSubnetGroups(2, getSubnetGroupsOptions); + + it('builds an RDS and Redshift subnet groups', () => { + const expected = { + RDSSubnetGroup: { + Properties: { + DBSubnetGroupDescription: { + Ref: 'AWS::StackName', + }, + DBSubnetGroupName: { + Ref: 'AWS::StackName', + }, + SubnetIds: [ + { + Ref: 'DBSubnet1', + }, + { + Ref: 'DBSubnet2', + }, + ], + }, + Type: 'AWS::RDS::DBSubnetGroup', + }, + RedshiftSubnetGroup: { + Properties: { + Description: { + Ref: 'AWS::StackName', + }, + SubnetIds: [ + { + Ref: 'DBSubnet1', + }, + { + Ref: 'DBSubnet2', + }, + ], + }, + Type: 'AWS::Redshift::ClusterSubnetGroup', + }, + }; + const actual = buildSubnetGroups(2, ['rds', 'redshift']); expect(actual).toEqual(expected); expect.assertions(1); }); diff --git a/package-lock.json b/package-lock.json index 459b942c..fdb96601 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "serverless-vpc-plugin", - "version": "0.7.0", + "version": "0.8.0", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/package.json b/package.json index 870bae5f..54c9259c 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "serverless-vpc-plugin", - "version": "0.7.0", + "version": "0.8.0", "engines": { "node": ">=8.0" }, diff --git a/src/constants.js b/src/constants.js index 70b587dc..675bcc81 100644 --- a/src/constants.js +++ b/src/constants.js @@ -19,9 +19,15 @@ const PUBLIC_SUBNET = 'Public'; */ const DB_SUBNET = 'DB'; +/** + * @type {Array} Valid subnet groups + */ +const VALID_SUBNET_GROUPS = ['rds', 'redshift', 'elasticache', 'dax']; + module.exports = { DEFAULT_VPC_EIP_LIMIT, APP_SUBNET, PUBLIC_SUBNET, DB_SUBNET, + VALID_SUBNET_GROUPS, }; diff --git a/src/index.js b/src/index.js index 31111e88..66283786 100644 --- a/src/index.js +++ b/src/index.js @@ -1,4 +1,4 @@ -const { DEFAULT_VPC_EIP_LIMIT, APP_SUBNET } = require('./constants'); +const { DEFAULT_VPC_EIP_LIMIT, APP_SUBNET, VALID_SUBNET_GROUPS } = require('./constants'); const { splitSubnets } = require('./subnets'); const { buildAvailabilityZones } = require('./az'); const { @@ -8,7 +8,7 @@ const { buildLambdaSecurityGroup, } = require('./vpc'); const { buildAppNetworkAcl, buildPublicNetworkAcl, buildDBNetworkAcl } = require('./nacl'); -const { buildSubnetGroups, validSubnetGroups } = require('./subnet_groups'); +const { buildSubnetGroups } = require('./subnet_groups'); const { buildEndpointServices, buildLambdaVPCEndpointSecurityGroup } = require('./vpce'); const { buildLogBucket, buildLogBucketPolicy, buildVpcFlowLogs } = require('./flow_logs'); const { buildBastion } = require('./bastion'); @@ -38,7 +38,7 @@ class ServerlessVpcPlugin { let createNatInstance = false; let createBastionHost = false; let bastionHostKeyName = null; - let subnetGroups = []; + let subnetGroups = VALID_SUBNET_GROUPS; const { vpcConfig } = this.serverless.service.custom; @@ -71,8 +71,8 @@ class ServerlessVpcPlugin { if (Array.isArray(vpcConfig.services)) { services = vpcConfig.services.map(s => s.trim().toLowerCase()); } - if (Array.isArray(vpcConfig.subnetGroups) && vpcConfig.subnetGroups.length > 0) { - ({ subnetGroups } = vpcConfig); + if (Array.isArray(vpcConfig.subnetGroups)) { + subnetGroups = vpcConfig.subnetGroups.map(s => s.trim().toLowerCase()); } if ('createDbSubnet' in vpcConfig && typeof vpcConfig.createDbSubnet === 'boolean') { @@ -226,14 +226,11 @@ class ServerlessVpcPlugin { if (numZones < 2) { this.serverless.cli.log('WARNING: less than 2 AZs; skipping subnet group provisioning'); } else { - for (let i = 0; i < subnetGroups.length; i += 1) { - const subnetGrp = subnetGroups[i]; - if (validSubnetGroups.indexOf(subnetGrp) === -1) { - throw new this.serverless.classes.Error( - `WARNING: '${subnetGrp}' is invalid subnetGroups option. - you should input among these options: ['rds', 'redshift', 'elasticache', 'dax']`, - ); - } + const invalidGroup = subnetGroups.some(group => !VALID_SUBNET_GROUPS.includes(group)); + if (invalidGroup) { + throw new this.serverless.classes.Error( + 'WARNING: Invalid subnetGroups option. Valid options: rds, redshift, elasticache, dax', + ); } Object.assign(resources, buildSubnetGroups(numZones, subnetGroups)); } diff --git a/src/subnet_groups.js b/src/subnet_groups.js index be6489a8..eb0a3a94 100644 --- a/src/subnet_groups.js +++ b/src/subnet_groups.js @@ -1,7 +1,5 @@ const { DB_SUBNET } = require('./constants'); -const validSubnetGroups = ['rds', 'redshift', 'elasticache', 'dax']; - /** * Build an RDSubnetGroup for a given number of zones * @@ -142,27 +140,20 @@ function buildSubnetGroups(numZones = 0, subnetGroups = []) { if (numZones < 2) { return {}; } - const subnetGroupList = { + if (!Array.isArray(subnetGroups) || subnetGroups.length < 1) { + return {}; + } + + const groupMapping = { rds: buildRDSSubnetGroup, redshift: buildRedshiftSubnetGroup, elasticache: buildElastiCacheSubnetGroup, dax: buildDAXSubnetGroup, }; - function assembleSubnetGrp(acc, service) { - const builtSubnetGroup = subnetGroupList[service.toLowerCase()](numZones); - return Object.assign(acc, builtSubnetGroup); - } - if (subnetGroups.length > 0) { - return subnetGroups.reduce(assembleSubnetGrp, {}); - } - return Object.assign( - {}, - buildRDSSubnetGroup(numZones), - buildRedshiftSubnetGroup(numZones), - buildElastiCacheSubnetGroup(numZones), - buildDAXSubnetGroup(numZones), - ); + return subnetGroups.reduce((acc, cur) => { + return Object.assign(acc, groupMapping[cur.toLowerCase()](numZones)); + }, {}); } module.exports = { @@ -171,5 +162,4 @@ module.exports = { buildRDSSubnetGroup, buildRedshiftSubnetGroup, buildSubnetGroups, - validSubnetGroups, };