From 1a8b94cc4f85df5aa2eea10bec4fbebc386e361b Mon Sep 17 00:00:00 2001 From: David Tanner Date: Thu, 22 Sep 2016 15:55:52 -0600 Subject: [PATCH] Upping code coverage. Found some unused code when looking for throttling errors, added functionality to get code from SDK errors. Added tests to cover functionality. Fixed prefix to be serverless/{serviceName}/{stage} --- lib/classes/Error.js | 3 +- lib/plugins/aws/deploy/lib/cleanupS3Bucket.js | 2 +- lib/plugins/aws/deploy/lib/createStack.js | 8 +- .../lib/generateArtifactDirectoryName.js | 2 +- .../aws/deploy/tests/cleanupS3Bucket.js | 99 +++++---- lib/plugins/aws/index.js | 2 +- lib/plugins/aws/tests/index.js | 199 +++++++++++++++--- 7 files changed, 238 insertions(+), 77 deletions(-) diff --git a/lib/classes/Error.js b/lib/classes/Error.js index abbda299b..e754e4d21 100644 --- a/lib/classes/Error.js +++ b/lib/classes/Error.js @@ -2,10 +2,11 @@ const chalk = require('chalk'); module.exports.SError = class ServerlessError extends Error { - constructor(message) { + constructor(message, statusCode) { super(message); this.name = this.constructor.name; this.message = message; + this.statusCode = statusCode; Error.captureStackTrace(this, this.constructor); } }; diff --git a/lib/plugins/aws/deploy/lib/cleanupS3Bucket.js b/lib/plugins/aws/deploy/lib/cleanupS3Bucket.js index 73c62a293..ad92b5a37 100644 --- a/lib/plugins/aws/deploy/lib/cleanupS3Bucket.js +++ b/lib/plugins/aws/deploy/lib/cleanupS3Bucket.js @@ -21,7 +21,7 @@ module.exports = { if (result.Contents.length) { let directories = []; const regex = new RegExp( - `serverless/${serviceStage}/(.+\-.+\-.+\-.+)` + `serverless/${serviceStage}/(.+-.+-.+-.+)` ); // get the unique directory names diff --git a/lib/plugins/aws/deploy/lib/createStack.js b/lib/plugins/aws/deploy/lib/createStack.js index 8ac3cbba3..308306098 100644 --- a/lib/plugins/aws/deploy/lib/createStack.js +++ b/lib/plugins/aws/deploy/lib/createStack.js @@ -7,10 +7,8 @@ module.exports = { create() { this.serverless.cli.log('Creating Stack...'); const stackName = `${this.serverless.service.service}-${this.options.stage}`; - if (!this.serverless.service.provider.compiledCloudFormationTemplate) { - this.serverless.service.provider.compiledCloudFormationTemplate = - this.loadCoreCloudFormationTemplate(); - } + const coreCloudFormationTemplate = this.loadCoreCloudFormationTemplate(); + const params = { StackName: stackName, OnFailure: 'DELETE', @@ -18,7 +16,7 @@ module.exports = { 'CAPABILITY_IAM', ], Parameters: [], - TemplateBody: JSON.stringify(this.serverless.service.provider.compiledCloudFormationTemplate), + TemplateBody: JSON.stringify(coreCloudFormationTemplate), Tags: [{ Key: 'STAGE', Value: this.options.stage, diff --git a/lib/plugins/aws/deploy/lib/generateArtifactDirectoryName.js b/lib/plugins/aws/deploy/lib/generateArtifactDirectoryName.js index 84bdb7057..5656ead86 100644 --- a/lib/plugins/aws/deploy/lib/generateArtifactDirectoryName.js +++ b/lib/plugins/aws/deploy/lib/generateArtifactDirectoryName.js @@ -5,7 +5,7 @@ const BbPromise = require('bluebird'); module.exports = { generateArtifactDirectoryName() { const date = new Date(); - const serviceStage = `${this.serverless.service.service}-${this.options.stage}`; + const serviceStage = `${this.serverless.service.service}/${this.options.stage}`; const dateString = `${date.getTime().toString()}-${date.toISOString()}`; this.serverless.service.package .artifactDirectoryName = `serverless/${serviceStage}/${dateString}`; diff --git a/lib/plugins/aws/deploy/tests/cleanupS3Bucket.js b/lib/plugins/aws/deploy/tests/cleanupS3Bucket.js index b9a8f1c0a..61097ea46 100644 --- a/lib/plugins/aws/deploy/tests/cleanupS3Bucket.js +++ b/lib/plugins/aws/deploy/tests/cleanupS3Bucket.js @@ -9,13 +9,16 @@ const Serverless = require('../../../../Serverless'); describe('cleanupS3Bucket', () => { let serverless; let awsDeploy; + let s3Key; beforeEach(() => { serverless = new Serverless(); + serverless.service.service = 'cleanupS3Bucket'; const options = { stage: 'dev', region: 'us-east-1', }; + s3Key = `serverless/${serverless.service.service}/${options.stage}`; awsDeploy = new AwsDeploy(serverless, options); awsDeploy.bucketName = 'deployment-bucket'; awsDeploy.serverless.cli = new serverless.classes.CLI(); @@ -35,6 +38,7 @@ describe('cleanupS3Bucket', () => { expect(listObjectsStub.args[0][0]).to.be.equal('S3'); expect(listObjectsStub.args[0][1]).to.be.equal('listObjectsV2'); expect(listObjectsStub.args[0][2].Bucket).to.be.equal(awsDeploy.bucketName); + expect(listObjectsStub.args[0][2].Prefix).to.be.equal(`${s3Key}`); expect(listObjectsStub.calledWith(awsDeploy.options.stage, awsDeploy.options.region)); awsDeploy.sdk.request.restore(); }); @@ -43,18 +47,18 @@ describe('cleanupS3Bucket', () => { it('should return all to be removed service objects (except the last 4)', () => { const serviceObjects = { Contents: [ - { Key: '151224711231-2016-08-18T15:42:00/artifact.zip' }, - { Key: '151224711231-2016-08-18T15:42:00/cloudformation.json' }, - { Key: '141264711231-2016-08-18T15:42:00/artifact.zip' }, - { Key: '141264711231-2016-08-18T15:42:00/cloudformation.json' }, - { Key: '141321321541-2016-08-18T11:23:02/artifact.zip' }, - { Key: '141321321541-2016-08-18T11:23:02/cloudformation.json' }, - { Key: '142003031341-2016-08-18T12:46:04/artifact.zip' }, - { Key: '142003031341-2016-08-18T12:46:04/cloudformation.json' }, - { Key: '113304333331-2016-08-18T13:40:06/artifact.zip' }, - { Key: '113304333331-2016-08-18T13:40:06/cloudformation.json' }, - { Key: '903940390431-2016-08-18T23:42:08/artifact.zip' }, - { Key: '903940390431-2016-08-18T23:42:08/cloudformation.json' }, + { Key: `${s3Key}/151224711231-2016-08-18T15:42:00/artifact.zip` }, + { Key: `${s3Key}/151224711231-2016-08-18T15:42:00/cloudformation.json` }, + { Key: `${s3Key}/141264711231-2016-08-18T15:42:00/artifact.zip` }, + { Key: `${s3Key}/141264711231-2016-08-18T15:42:00/cloudformation.json` }, + { Key: `${s3Key}/141321321541-2016-08-18T11:23:02/artifact.zip` }, + { Key: `${s3Key}/141321321541-2016-08-18T11:23:02/cloudformation.json` }, + { Key: `${s3Key}/142003031341-2016-08-18T12:46:04/artifact.zip` }, + { Key: `${s3Key}/142003031341-2016-08-18T12:46:04/cloudformation.json` }, + { Key: `${s3Key}/113304333331-2016-08-18T13:40:06/artifact.zip` }, + { Key: `${s3Key}/113304333331-2016-08-18T13:40:06/cloudformation.json` }, + { Key: `${s3Key}/903940390431-2016-08-18T23:42:08/artifact.zip` }, + { Key: `${s3Key}/903940390431-2016-08-18T23:42:08/cloudformation.json` }, ], }; @@ -63,25 +67,42 @@ describe('cleanupS3Bucket', () => { return awsDeploy.getObjectsToRemove().then((objectsToRemove) => { expect(objectsToRemove).to.not - .include({ Key: '141321321541-2016-08-18T11:23:02/artifact.zip' }); + .include( + { Key: `${s3Key}${s3Key}/141321321541-2016-08-18T11:23:02/artifact.zip` }); + expect(objectsToRemove).to.not - .include({ Key: '141321321541-2016-08-18T11:23:02/cloudformation.json' }); + .include( + { Key: `${s3Key}${s3Key}/141321321541-2016-08-18T11:23:02/cloudformation.json` }); + expect(objectsToRemove).to.not - .include({ Key: '142003031341-2016-08-18T12:46:04/artifact.zip' }); + .include( + { Key: `${s3Key}${s3Key}/142003031341-2016-08-18T12:46:04/artifact.zip` }); + expect(objectsToRemove).to.not - .include({ Key: '142003031341-2016-08-18T12:46:04/cloudformation.json' }); + .include( + { Key: `${s3Key}${s3Key}/142003031341-2016-08-18T12:46:04/cloudformation.json` }); + expect(objectsToRemove).to.not - .include({ Key: '151224711231-2016-08-18T15:42:00/artifact.zip' }); + .include( + { Key: `${s3Key}${s3Key}/151224711231-2016-08-18T15:42:00/artifact.zip` }); + expect(objectsToRemove).to.not - .include({ Key: '151224711231-2016-08-18T15:42:00/cloudformation.json' }); + .include( + { Key: `${s3Key}${s3Key}/151224711231-2016-08-18T15:42:00/cloudformation.json` }); + expect(objectsToRemove).to.not - .include({ Key: '903940390431-2016-08-18T23:42:08/artifact.zip' }); + .include( + { Key: `${s3Key}${s3Key}/903940390431-2016-08-18T23:42:08/artifact.zip` }); + expect(objectsToRemove).to.not - .include({ Key: '903940390431-2016-08-18T23:42:08/cloudformation.json' }); + .include( + { Key: `${s3Key}${s3Key}/903940390431-2016-08-18T23:42:08/cloudformation.json` }); + expect(listObjectsStub.calledOnce).to.be.equal(true); expect(listObjectsStub.args[0][0]).to.be.equal('S3'); expect(listObjectsStub.args[0][1]).to.be.equal('listObjectsV2'); expect(listObjectsStub.args[0][2].Bucket).to.be.equal(awsDeploy.bucketName); + expect(listObjectsStub.args[0][2].Prefix).to.be.equal(`${s3Key}`); expect(listObjectsStub.calledWith(awsDeploy.options.stage, awsDeploy.options.region)); awsDeploy.sdk.request.restore(); }); @@ -90,12 +111,12 @@ describe('cleanupS3Bucket', () => { it('should return an empty array if there are less than 4 directories available', () => { const serviceObjects = { Contents: [ - { Key: '151224711231-2016-08-18T15:42:00/artifact.zip' }, - { Key: '151224711231-2016-08-18T15:42:00/cloudformation.json' }, - { Key: '141264711231-2016-08-18T15:42:00/artifact.zip' }, - { Key: '141264711231-2016-08-18T15:42:00/cloudformation.json' }, - { Key: '141321321541-2016-08-18T11:23:02/artifact.zip' }, - { Key: '141321321541-2016-08-18T11:23:02/cloudformation.json' }, + { Key: `${s3Key}151224711231-2016-08-18T15:42:00/artifact.zip` }, + { Key: `${s3Key}151224711231-2016-08-18T15:42:00/cloudformation.json` }, + { Key: `${s3Key}141264711231-2016-08-18T15:42:00/artifact.zip` }, + { Key: `${s3Key}141264711231-2016-08-18T15:42:00/cloudformation.json` }, + { Key: `${s3Key}141321321541-2016-08-18T11:23:02/artifact.zip` }, + { Key: `${s3Key}141321321541-2016-08-18T11:23:02/cloudformation.json` }, ], }; @@ -108,6 +129,7 @@ describe('cleanupS3Bucket', () => { expect(listObjectsStub.args[0][0]).to.be.equal('S3'); expect(listObjectsStub.args[0][1]).to.be.equal('listObjectsV2'); expect(listObjectsStub.args[0][2].Bucket).to.be.equal(awsDeploy.bucketName); + expect(listObjectsStub.args[0][2].Prefix).to.be.equal(`${s3Key}`); expect(listObjectsStub.calledWith(awsDeploy.options.stage, awsDeploy.options.region)); awsDeploy.sdk.request.restore(); }); @@ -116,14 +138,14 @@ describe('cleanupS3Bucket', () => { it('should resolve if there are exactly 4 directories available', () => { const serviceObjects = { Contents: [ - { Key: '151224711231-2016-08-18T15:42:00/artifact.zip' }, - { Key: '151224711231-2016-08-18T15:42:00/cloudformation.json' }, - { Key: '141264711231-2016-08-18T15:42:00/artifact.zip' }, - { Key: '141264711231-2016-08-18T15:42:00/cloudformation.json' }, - { Key: '141321321541-2016-08-18T11:23:02/artifact.zip' }, - { Key: '141321321541-2016-08-18T11:23:02/cloudformation.json' }, - { Key: '142003031341-2016-08-18T12:46:04/artifact.zip' }, - { Key: '142003031341-2016-08-18T12:46:04/cloudformation.json' }, + { Key: `${s3Key}151224711231-2016-08-18T15:42:00/artifact.zip` }, + { Key: `${s3Key}151224711231-2016-08-18T15:42:00/cloudformation.json` }, + { Key: `${s3Key}141264711231-2016-08-18T15:42:00/artifact.zip` }, + { Key: `${s3Key}141264711231-2016-08-18T15:42:00/cloudformation.json` }, + { Key: `${s3Key}141321321541-2016-08-18T11:23:02/artifact.zip` }, + { Key: `${s3Key}141321321541-2016-08-18T11:23:02/cloudformation.json` }, + { Key: `${s3Key}142003031341-2016-08-18T12:46:04/artifact.zip` }, + { Key: `${s3Key}142003031341-2016-08-18T12:46:04/cloudformation.json` }, ], }; @@ -136,6 +158,7 @@ describe('cleanupS3Bucket', () => { expect(listObjectsStub.args[0][0]).to.be.equal('S3'); expect(listObjectsStub.args[0][1]).to.be.equal('listObjectsV2'); expect(listObjectsStub.args[0][2].Bucket).to.be.equal(awsDeploy.bucketName); + expect(listObjectsStub.args[0][2].Prefix).to.be.equal(`${s3Key}`); expect(listObjectsStub.calledWith(awsDeploy.options.stage, awsDeploy.options.region)); awsDeploy.sdk.request.restore(); }); @@ -159,10 +182,10 @@ describe('cleanupS3Bucket', () => { it('should remove all old service files from the S3 bucket if available', () => { const objectsToRemove = [ - { Key: '113304333331-2016-08-18T13:40:06/artifact.zip' }, - { Key: '113304333331-2016-08-18T13:40:06/cloudformation.json' }, - { Key: '141264711231-2016-08-18T15:42:00/artifact.zip' }, - { Key: '141264711231-2016-08-18T15:42:00/cloudformation.json' }, + { Key: `${s3Key}113304333331-2016-08-18T13:40:06/artifact.zip` }, + { Key: `${s3Key}113304333331-2016-08-18T13:40:06/cloudformation.json` }, + { Key: `${s3Key}141264711231-2016-08-18T15:42:00/artifact.zip` }, + { Key: `${s3Key}141264711231-2016-08-18T15:42:00/cloudformation.json` }, ]; return awsDeploy.removeObjects(objectsToRemove).then(() => { diff --git a/lib/plugins/aws/index.js b/lib/plugins/aws/index.js index 720e6a292..0ad4c2b74 100644 --- a/lib/plugins/aws/index.js +++ b/lib/plugins/aws/index.js @@ -69,7 +69,7 @@ class SDK { ].join(''); err.message = errorMessage; } - reject(new this.serverless.classes.Error(err.message)); + reject(new this.serverless.classes.Error(err.message, err.statusCode)); } else { resolve(data); } diff --git a/lib/plugins/aws/tests/index.js b/lib/plugins/aws/tests/index.js index be422ae36..8bc1a7954 100644 --- a/lib/plugins/aws/tests/index.js +++ b/lib/plugins/aws/tests/index.js @@ -7,38 +7,43 @@ const Serverless = require('../../../Serverless'); const AwsSdk = require('../'); describe('AWS SDK', () => { + let awsSdk; + let serverless; + + beforeEach(() => { + serverless = new Serverless(); + const options = { + stage: 'dev', + region: 'us-east-1', + }; + awsSdk = new AwsSdk(serverless, options); + awsSdk.serverless.cli = new serverless.classes.CLI(); + }); + describe('#constructor()', () => { it('should set AWS instance', () => { - const serverless = new Serverless(); - const awsSdk = new AwsSdk(serverless); - expect(typeof awsSdk.sdk).to.not.equal('undefined'); }); it('should set Serverless instance', () => { - const serverless = new Serverless(); - const awsSdk = new AwsSdk(serverless); - expect(typeof awsSdk.serverless).to.not.equal('undefined'); }); it('should set AWS proxy', () => { - const serverless = new Serverless(); process.env.proxy = 'http://a.b.c.d:n'; - const awsSdk = new AwsSdk(serverless); + const newAwsSdk = new AwsSdk(serverless); - expect(typeof awsSdk.sdk.config.httpOptions.agent).to.not.equal('undefined'); + expect(typeof newAwsSdk.sdk.config.httpOptions.agent).to.not.equal('undefined'); // clear env delete process.env.proxy; }); it('should set AWS timeout', () => { - const serverless = new Serverless(); process.env.AWS_CLIENT_TIMEOUT = '120000'; - const awsSdk = new AwsSdk(serverless); + const newAwsSdk = new AwsSdk(serverless); - expect(typeof awsSdk.sdk.config.httpOptions.timeout).to.not.equal('undefined'); + expect(typeof newAwsSdk.sdk.config.httpOptions.timeout).to.not.equal('undefined'); // clear env delete process.env.AWS_CLIENT_TIMEOUT; @@ -59,12 +64,10 @@ describe('AWS SDK', () => { }; } } - const serverless = new Serverless(); - const awsSdk = new AwsSdk(serverless); awsSdk.sdk = { S3: FakeS3, }; - serverless.service.environment = { + awsSdk.serverless.service.environment = { vars: {}, stages: { dev: { @@ -75,42 +78,127 @@ describe('AWS SDK', () => { }, }, }; - serverless.service.environment.stages.dev.regions['us-east-1'] = { - vars: {}, - }; + return awsSdk.request('S3', 'putObject', {}, 'dev', 'us-east-1').then(data => { expect(data.called).to.equal(true); }); }); + + it('should retry if error code is 429', function (done) { + this.timeout(10000); + let first = true; + const error = { + statusCode: 429, + message: 'Testing retry', + }; + class FakeS3 { + constructor(credentials) { + this.credentials = credentials; + } + + error() { + return { + send(cb) { + if (first) { + cb(error); + } else { + cb(undefined, {}); + } + first = false; + }, + }; + } + } + awsSdk.sdk = { + S3: FakeS3, + }; + awsSdk.request('S3', 'error', {}, 'dev', 'us-east-1') + .then(data => { + // eslint-disable-next-line no-unused-expressions + expect(data).to.exist; + // eslint-disable-next-line no-unused-expressions + expect(first).to.be.false; + done(); + }) + .catch(done); + }); + + it('should reject errors', (done) => { + const error = { + statusCode: 500, + message: 'Some error message', + }; + class FakeS3 { + constructor(credentials) { + this.credentials = credentials; + } + + error() { + return { + send(cb) { + cb(error); + }, + }; + } + } + awsSdk.sdk = { + S3: FakeS3, + }; + awsSdk.request('S3', 'error', {}, 'dev', 'us-east-1') + .then(() => done('Should not succeed')) + .catch(() => done()); + }); + + it('should return ref to docs for missing credentials', (done) => { + const error = { + statusCode: 403, + message: 'Missing credentials in config', + }; + class FakeS3 { + constructor(credentials) { + this.credentials = credentials; + } + + error() { + return { + send(cb) { + cb(error); + }, + }; + } + } + awsSdk.sdk = { + S3: FakeS3, + }; + awsSdk.request('S3', 'error', {}, 'dev', 'us-east-1') + .then(() => done('Should not succeed')) + .catch((err) => { + expect(err.message).to.contain('https://git.io/viZAC'); + done(); + }) + .catch(done); + }); }); describe('#getCredentials()', () => { it('should set region for credentials', () => { - const serverless = new Serverless(); - const awsSdk = new AwsSdk(serverless); const credentials = awsSdk.getCredentials('testregion'); expect(credentials.region).to.equal('testregion'); }); it('should get credentials from provider', () => { - const serverless = new Serverless(); - const awsSdk = new AwsSdk(serverless); serverless.service.provider.profile = 'notDefault'; const credentials = awsSdk.getCredentials(); expect(credentials.credentials.profile).to.equal('notDefault'); }); it('should not set credentials if empty profile is set', () => { - const serverless = new Serverless(); - const awsSdk = new AwsSdk(serverless); serverless.service.provider.profile = ''; const credentials = awsSdk.getCredentials('testregion'); expect(credentials).to.eql({ region: 'testregion' }); }); it('should not set credentials if profile is not set', () => { - const serverless = new Serverless(); - const awsSdk = new AwsSdk(serverless); serverless.service.provider.profile = undefined; const credentials = awsSdk.getCredentials('testregion'); expect(credentials).to.eql({ region: 'testregion' }); @@ -119,8 +207,6 @@ describe('AWS SDK', () => { describe('#getServerlessDeploymentBucketName', () => { it('should return the name of the serverless deployment bucket', () => { - const serverless = new Serverless(); - const awsSdk = new AwsSdk(serverless); const options = { stage: 'dev', region: 'us-east-1', @@ -148,13 +234,66 @@ describe('AWS SDK', () => { awsSdk.request.restore(); }); }); + it('should validate the region for the given S3 bucket', (done) => { + const stage = 'test'; + const region = 'us-east-1'; + + class FakeS3 { + constructor(credentials) { + this.credentials = credentials; + } + + getBucketLocation() { + return { + send(cb) { + cb(undefined, { LocationConstraint: region }); + }, + }; + } + } + awsSdk.sdk = { + S3: FakeS3, + }; + awsSdk.serverless.service.provider.bucketName = 'com.serverless.deploys'; + awsSdk.getServerlessDeploymentBucketName(stage, region) + .then(() => done()) + .catch(done); + }); + + it('should reject an S3 bucket in the wrong region', (done) => { + const stage = 'test'; + const region = 'us-east-1'; + + class FakeS3 { + constructor(credentials) { + this.credentials = credentials; + } + + getBucketLocation() { + return { + send(cb) { + cb(undefined, { LocationConstraint: 'us-west-1' }); + }, + }; + } + } + awsSdk.sdk = { + S3: FakeS3, + }; + awsSdk.serverless.service.provider.bucketName = 'com.serverless.deploys'; + awsSdk.getServerlessDeploymentBucketName(stage, region) + .then(done) + .catch((err) => { + expect(err.message).to.contain('not in the same region'); + done(); + }) + .catch(done); + }); }); describe('#getStackName', () => { it('should return the stack name', () => { - const serverless = new Serverless(); serverless.service.service = 'myservice'; - const awsSdk = new AwsSdk(serverless); expect(awsSdk.getStackName('dev')).to.equal('myservice-dev'); });