Merge pull request #4636 from alexcasalboni/issue4252-s3-transfer-acceleration-iam-breaking-change-fix

S3 Transfer Acceleration IAM breaking change fix

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
 

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('');
+      return BbPromise.reject(new this.serverless.classes.Error(errorMessage));
+    }
 
     if (bucketName) {
       return BbPromise.bind(this)
@@ -45,17 +53,25 @@ 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 };
+    } 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();

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;
       });
   });
@@ -80,11 +111,6 @@ describe('#generateCoreTemplate()', () => {
           .Resources.ServerlessDeploymentBucket
         ).to.be.deep.equal({
           Type: 'AWS::S3::Bucket',
-          Properties: {
-            AccelerateConfiguration: {
-              AccelerationStatus: 'Suspended',
-            },
-          },
         });
     })
   );
@@ -114,4 +140,36 @@ describe('#generateCoreTemplate()', () => {
         expect(template.Outputs.ServerlessDeploymentBucketAccelerated.Value).to.equal(true);
       });
   });
+
+  it('should explicitly disable S3 Transfer Acceleration, if requested', () => {
+    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.be.rejectedWith(serverless.classes.Error, /at the same time/);
+  });
 });

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'];
   }