From 4de092c092d6d43ca99ba35a27070b1de934636b Mon Sep 17 00:00:00 2001 From: Alex Casalboni Date: Sun, 7 Jan 2018 16:37:23 +0100 Subject: [PATCH 1/5] not setting 'Suspended' acceleration by default (permission would be required!) --- .../aws/package/lib/generateCoreTemplate.js | 16 ++++++++-------- .../aws/package/lib/generateCoreTemplate.test.js | 5 ----- 2 files changed, 8 insertions(+), 13 deletions(-) diff --git a/lib/plugins/aws/package/lib/generateCoreTemplate.js b/lib/plugins/aws/package/lib/generateCoreTemplate.js index 09a20d4dd..233c1a1eb 100644 --- a/lib/plugins/aws/package/lib/generateCoreTemplate.js +++ b/lib/plugins/aws/package/lib/generateCoreTemplate.js @@ -45,15 +45,15 @@ module.exports = { }); } - this.serverless.service.provider.compiledCloudFormationTemplate - .Resources.ServerlessDeploymentBucket.Properties = { - AccelerateConfiguration: { - AccelerationStatus: - isS3TransferAccelerationEnabled ? 'Enabled' : 'Suspended', - }, - }; - if (isS3TransferAccelerationEnabled) { + // enable acceleration via CloudFormation + this.serverless.service.provider.compiledCloudFormationTemplate + .Resources.ServerlessDeploymentBucket.Properties = { + AccelerateConfiguration: { + AccelerationStatus: 'Enabled', + }, + }; + // keep track of acceleration status via CloudFormation Output this.serverless.service.provider.compiledCloudFormationTemplate .Outputs.ServerlessDeploymentBucketAccelerated = { Value: true }; } diff --git a/lib/plugins/aws/package/lib/generateCoreTemplate.test.js b/lib/plugins/aws/package/lib/generateCoreTemplate.test.js index a089d5369..77e72afc7 100644 --- a/lib/plugins/aws/package/lib/generateCoreTemplate.test.js +++ b/lib/plugins/aws/package/lib/generateCoreTemplate.test.js @@ -80,11 +80,6 @@ describe('#generateCoreTemplate()', () => { .Resources.ServerlessDeploymentBucket ).to.be.deep.equal({ Type: 'AWS::S3::Bucket', - Properties: { - AccelerateConfiguration: { - AccelerationStatus: 'Suspended', - }, - }, }); }) ); From a9b82e538cc29063f6c0ba9a944627af54fdd127 Mon Sep 17 00:00:00 2001 From: Alex Casalboni Date: Sun, 7 Jan 2018 17:29:54 +0100 Subject: [PATCH 2/5] Added new `no-aws-s3-accelerate` argument to explicitly disable S3 Transfer Acceleration (updated doc too) --- docs/providers/aws/cli-reference/deploy.md | 3 ++- .../aws/package/lib/generateCoreTemplate.js | 16 ++++++++++++++++ lib/plugins/aws/provider/awsProvider.js | 4 ++++ 3 files changed, 22 insertions(+), 1 deletion(-) diff --git a/docs/providers/aws/cli-reference/deploy.md b/docs/providers/aws/cli-reference/deploy.md index 66204c690..8bdeced09 100644 --- a/docs/providers/aws/cli-reference/deploy.md +++ b/docs/providers/aws/cli-reference/deploy.md @@ -25,7 +25,8 @@ serverless deploy - `--verbose` or `-v` Shows all stack events during deployment, and display any Stack Output. - `--function` or `-f` Invoke `deploy function` (see above). Convenience shortcut - cannot be used with `--package`. - `--conceal` Hides secrets from the output (e.g. API Gateway key values). -- `--aws-s3-accelerate` Enables S3 Transfer Acceleration making uploading artifacts much faster. You can read more about it [here](http://docs.aws.amazon.com/AmazonS3/latest/dev/transfer-acceleration.html). **Note: When using Transfer Acceleration, additional data transfer charges may apply** +- `--aws-s3-accelerate` Enables S3 Transfer Acceleration making uploading artifacts much faster. You can read more about it [here](http://docs.aws.amazon.com/AmazonS3/latest/dev/transfer-acceleration.html). It requires additional `s3:PutAccelerateConfiguration` permissions. **Note: When using Transfer Acceleration, additional data transfer charges may apply.** +- `--no-aws-s3-accelerate` Explicitly disables S3 Transfer Acceleration). It also requires additional `s3:PutAccelerateConfiguration` permissions. ## Artifacts diff --git a/lib/plugins/aws/package/lib/generateCoreTemplate.js b/lib/plugins/aws/package/lib/generateCoreTemplate.js index 233c1a1eb..bd751279a 100644 --- a/lib/plugins/aws/package/lib/generateCoreTemplate.js +++ b/lib/plugins/aws/package/lib/generateCoreTemplate.js @@ -25,6 +25,14 @@ module.exports = { const bucketName = this.serverless.service.provider.deploymentBucket; const isS3TransferAccelerationEnabled = this.provider.isS3TransferAccelerationEnabled(); + const isS3TransferAccelerationDisabled = this.provider.isS3TransferAccelerationDisabled(); + + if (isS3TransferAccelerationEnabled && isS3TransferAccelerationDisabled) { + const errorMessage = [ + 'You cannot enable and disable S3 Transfer Acceleration at the same time', + ].join(''); + throw new this.serverless.classes.Error(errorMessage); + } if (bucketName) { return BbPromise.bind(this) @@ -56,6 +64,14 @@ module.exports = { // keep track of acceleration status via CloudFormation Output this.serverless.service.provider.compiledCloudFormationTemplate .Outputs.ServerlessDeploymentBucketAccelerated = { Value: true }; + } else if (isS3TransferAccelerationDisabled) { + // explicitly disable acceleration via CloudFormation + this.serverless.service.provider.compiledCloudFormationTemplate + .Resources.ServerlessDeploymentBucket.Properties = { + AccelerateConfiguration: { + AccelerationStatus: 'Suspended', + }, + }; } const coreTemplateFileName = this.provider.naming.getCoreTemplateFileName(); diff --git a/lib/plugins/aws/provider/awsProvider.js b/lib/plugins/aws/provider/awsProvider.js index 4c79f14be..bcc78e583 100644 --- a/lib/plugins/aws/provider/awsProvider.js +++ b/lib/plugins/aws/provider/awsProvider.js @@ -317,6 +317,10 @@ class AwsProvider { return !!this.options['aws-s3-accelerate']; } + isS3TransferAccelerationDisabled() { + return !!this.options['no-aws-s3-accelerate']; + } + disableTransferAccelerationForCurrentDeploy() { delete this.options['aws-s3-accelerate']; } From 1aee4b82404cc04cb47ff7cdd624a53ffd3af7fb Mon Sep 17 00:00:00 2001 From: Alex Casalboni Date: Sun, 7 Jan 2018 17:30:18 +0100 Subject: [PATCH 3/5] updated tests (100% file coverage) --- .../package/lib/generateCoreTemplate.test.js | 71 +++++++++++++++++-- 1 file changed, 67 insertions(+), 4 deletions(-) diff --git a/lib/plugins/aws/package/lib/generateCoreTemplate.test.js b/lib/plugins/aws/package/lib/generateCoreTemplate.test.js index 77e72afc7..63fbdc02a 100644 --- a/lib/plugins/aws/package/lib/generateCoreTemplate.test.js +++ b/lib/plugins/aws/package/lib/generateCoreTemplate.test.js @@ -61,14 +61,45 @@ describe('#generateCoreTemplate()', () => { return expect(awsPlugin.generateCoreTemplate()).to.be.fulfilled .then(() => { + const template = awsPlugin.serverless.service.provider.compiledCloudFormationTemplate; expect( - awsPlugin.serverless.service.provider.compiledCloudFormationTemplate - .Outputs.ServerlessDeploymentBucketName.Value + template.Outputs.ServerlessDeploymentBucketName.Value ).to.equal(bucketName); // eslint-disable-next-line no-unused-expressions expect( - awsPlugin.serverless.service.provider.compiledCloudFormationTemplate - .Resources.ServerlessDeploymentBucket + template.Resources.ServerlessDeploymentBucket + ).to.not.exist; + }); + }); + + it('should use a custom bucket if specified, even with S3 transfer acceleration', () => { + const bucketName = 'com.serverless.deploys'; + + awsPlugin.serverless.service.provider.deploymentBucket = bucketName; + awsPlugin.provider.options['aws-s3-accelerate'] = true; + + const coreCloudFormationTemplate = awsPlugin.serverless.utils.readFileSync( + path.join( + __dirname, + 'core-cloudformation-template.json' + ) + ); + awsPlugin.serverless.service.provider + .compiledCloudFormationTemplate = coreCloudFormationTemplate; + + return expect(awsPlugin.generateCoreTemplate()).to.be.fulfilled + .then(() => { + const template = awsPlugin.serverless.service.provider.compiledCloudFormationTemplate; + expect( + template.Outputs.ServerlessDeploymentBucketName.Value + ).to.equal(bucketName); + // eslint-disable-next-line no-unused-expressions + expect( + template.Resources.ServerlessDeploymentBucket + ).to.not.exist; + // eslint-disable-next-line no-unused-expressions + expect( + template.Outputs.ServerlessDeploymentBucketAccelerated ).to.not.exist; }); }); @@ -109,4 +140,36 @@ describe('#generateCoreTemplate()', () => { expect(template.Outputs.ServerlessDeploymentBucketAccelerated.Value).to.equal(true); }); }); + + it('should explode if transfer acceleration is both enabled and disabled', () => { + sinon.stub(awsPlugin.provider, 'request').resolves(); + sinon.stub(serverless.utils, 'writeFileSync').resolves(); + serverless.config.servicePath = './'; + awsPlugin.provider.options['no-aws-s3-accelerate'] = true; + + return awsPlugin.generateCoreTemplate() + .then(() => { + const template = serverless.service.provider.coreCloudFormationTemplate; + expect(template.Resources.ServerlessDeploymentBucket).to.be.deep.equal({ + Type: 'AWS::S3::Bucket', + Properties: { + AccelerateConfiguration: { + AccelerationStatus: 'Suspended', + }, + }, + }); + }); + }); + + it('should explode if transfer acceleration is both enabled and disabled', () => { + sinon.stub(awsPlugin.provider, 'request').resolves(); + sinon.stub(serverless.utils, 'writeFileSync').resolves(); + serverless.config.servicePath = './'; + awsPlugin.provider.options['aws-s3-accelerate'] = true; + awsPlugin.provider.options['no-aws-s3-accelerate'] = true; + + return expect( + () => awsPlugin.generateCoreTemplate() + ).to.throw(/at the same time/); + }); }); From c7200181255815191295ad15cb416ae83d2edffd Mon Sep 17 00:00:00 2001 From: Alex Casalboni Date: Sun, 7 Jan 2018 17:38:43 +0100 Subject: [PATCH 4/5] test typo --- lib/plugins/aws/package/lib/generateCoreTemplate.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/plugins/aws/package/lib/generateCoreTemplate.test.js b/lib/plugins/aws/package/lib/generateCoreTemplate.test.js index 63fbdc02a..58b4b7dc3 100644 --- a/lib/plugins/aws/package/lib/generateCoreTemplate.test.js +++ b/lib/plugins/aws/package/lib/generateCoreTemplate.test.js @@ -141,7 +141,7 @@ describe('#generateCoreTemplate()', () => { }); }); - it('should explode if transfer acceleration is both enabled and disabled', () => { + it('should explicitly disable S3 Transfer Acceleration, if requested', () => { sinon.stub(awsPlugin.provider, 'request').resolves(); sinon.stub(serverless.utils, 'writeFileSync').resolves(); serverless.config.servicePath = './'; From 407b0f06e6f466fb029ea4c619589a65e1c23401 Mon Sep 17 00:00:00 2001 From: Alex Casalboni Date: Mon, 8 Jan 2018 15:03:41 +0100 Subject: [PATCH 5/5] BbPromise.reject instead of throwing error --- lib/plugins/aws/package/lib/generateCoreTemplate.js | 2 +- lib/plugins/aws/package/lib/generateCoreTemplate.test.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/plugins/aws/package/lib/generateCoreTemplate.js b/lib/plugins/aws/package/lib/generateCoreTemplate.js index bd751279a..fae3c2000 100644 --- a/lib/plugins/aws/package/lib/generateCoreTemplate.js +++ b/lib/plugins/aws/package/lib/generateCoreTemplate.js @@ -31,7 +31,7 @@ module.exports = { const errorMessage = [ 'You cannot enable and disable S3 Transfer Acceleration at the same time', ].join(''); - throw new this.serverless.classes.Error(errorMessage); + return BbPromise.reject(new this.serverless.classes.Error(errorMessage)); } if (bucketName) { diff --git a/lib/plugins/aws/package/lib/generateCoreTemplate.test.js b/lib/plugins/aws/package/lib/generateCoreTemplate.test.js index 58b4b7dc3..e4cb5eba2 100644 --- a/lib/plugins/aws/package/lib/generateCoreTemplate.test.js +++ b/lib/plugins/aws/package/lib/generateCoreTemplate.test.js @@ -169,7 +169,7 @@ describe('#generateCoreTemplate()', () => { awsPlugin.provider.options['no-aws-s3-accelerate'] = true; return expect( - () => awsPlugin.generateCoreTemplate() - ).to.throw(/at the same time/); + awsPlugin.generateCoreTemplate() + ).to.be.rejectedWith(serverless.classes.Error, /at the same time/); }); });