Merge pull request #37 from erezrokah/feat/s3_request_parameters

Feature: allow passing requestParameters to s3 integration

Author: theburningmonk

README.md

@@ -176,6 +176,27 @@ Sample request after deploying.
 curl https://xxxxxx.execute-api.us-east-1.amazonaws.com/dev/s3 -d '{"message": "testtest"}' -H 'Content-Type:application/json'
 ```
 
+#### Customizing request parameters
+
+Similar to the [SQS](#sqs) support, you can customize the default request parameters `serverless.yml` like so:
+
+```yml
+custom:
+  apiGatewayServiceProxies:
+    - s3:
+        path: /s3
+        method: post
+        action: PutObject
+        bucket:
+          Ref: S3Bucket
+        cors: true
+
+        requestParameters:
+          # if requestParameters has a 'integration.request.path.object' property you should remove the key setting
+          'integration.request.path.object': 'context.requestId'
+          "integration.request.header.cache-control": "'public, max-age=31536000, immutable'"
+```
+
 ### SNS
 
 Sample syntax for SNS proxy in `serverless.yml`.

lib/apiGateway/schema.js

@@ -157,7 +157,15 @@ const proxiesSchemas = {
         .valid('GetObject', 'PutObject', 'DeleteObject')
         .required(),
       bucket: stringOrRef.required(),
-      key: key.required()
+      // don't accept a key when requestParameters has a 'integration.request.path.object' property
+      key: Joi.when('requestParameters', {
+        is: requestParameters
+          .keys({ 'integration.request.path.object': Joi.string().required() })
+          .required(),
+        then: Joi.forbidden(),
+        otherwise: key.required()
+      }),
+      requestParameters
     })
   }),
   sns: Joi.object({

lib/apiGateway/validate.test.js

@@ -581,7 +581,6 @@ describe('#validateServiceProxies()', () => {
 
     const proxiesToTest = [
       { proxy: 'kinesis', props: { streamName: 'streamName' } },
-      { proxy: 's3', props: { bucket: 'bucket', action: 'PutObject', key: 'myKey' } },
       { proxy: 'sns', props: { topicName: 'topicName' } }
     ]
     proxiesToTest.forEach(({ proxy, props }) => {
@@ -896,11 +895,11 @@ describe('#validateServiceProxies()', () => {
       )
     }
 
-    const shouldSucceed = (key, value) => {
+    const shouldSucceed = (key, value, ...missing) => {
       serverlessApigatewayServiceProxy.serverless.service.custom = {
         apiGatewayServiceProxies: [
           {
-            s3: getProxy(key, value)
+            s3: getProxy(key, value, ...missing)
           }
         ]
       }
@@ -1015,6 +1014,46 @@ describe('#validateServiceProxies()', () => {
         { param: 'myKey' }
       )
     })
+
+    it('should throw if requestParameters is not a string to string mapping', () => {
+      shouldError(
+        'child "s3" fails because [child "requestParameters" fails because [child "key2" fails because ["key2" must be a string]]]',
+        'requestParameters',
+        { key1: 'value', key2: [] }
+      )
+    })
+
+    it('should not throw if requestParameters is a string to string mapping', () => {
+      shouldSucceed('requestParameters', { key1: 'value1', key2: 'value2' })
+    })
+
+    it('should throw if requestParameters has "integration.request.path.object" and key is defined', () => {
+      shouldError(
+        'child "s3" fails because [child "key" fails because ["key" is not allowed]]',
+        'requestParameters',
+        {
+          'integration.request.path.object': 'context.requestId',
+          'integration.request.header.cache-control': "'public, max-age=31536000, immutable'"
+        }
+      )
+    })
+
+    it('should not throw if requestParameters has "integration.request.path.object" and key is not defined', () => {
+      shouldSucceed(
+        'requestParameters',
+        {
+          'integration.request.path.object': 'context.requestId',
+          'integration.request.header.cache-control': "'public, max-age=31536000, immutable'"
+        },
+        'key'
+      )
+    })
+
+    it(`should not throw if requestParameters doesn't have "integration.request.path.object" and key is defined`, () => {
+      shouldSucceed('requestParameters', {
+        'integration.request.header.cache-control': "'public, max-age=31536000, immutable'"
+      })
+    })
   })
 
   describe('sns', () => {

lib/package/s3/compileMethodsToS3.js

@@ -122,13 +122,20 @@ module.exports = {
   getS3MethodIntegration(http) {
     const bucket = http.bucket
     const httpMethod = this.getIntegrationHttpMethod(http)
-    const objectRequestParam = this.getObjectRequestParameter(http)
-    const requestParams = _.merge(this.getIntegrationRequestParameters(http), {
-      'integration.request.path.object': objectRequestParam,
+
+    let requestParams = _.merge(this.getIntegrationRequestParameters(http), {
       'integration.request.path.bucket': {
         'Fn::Sub': ["'${bucket}'", { bucket }]
       }
     })
+
+    if (_.has(http, 'key')) {
+      const objectRequestParam = this.getObjectRequestParameter(http)
+      requestParams = _.merge(requestParams, {
+        'integration.request.path.object': objectRequestParam
+      })
+    }
+
     const responseParams = this.getIntegrationResponseParameters(http)
 
     const integration = {
@@ -141,7 +148,7 @@ module.exports = {
         'Fn::Sub': ['arn:aws:apigateway:${AWS::Region}:s3:path/{bucket}/{object}', {}]
       },
       PassthroughBehavior: 'WHEN_NO_MATCH',
-      RequestParameters: requestParams
+      RequestParameters: _.merge(requestParams, http.requestParameters)
     }
 
     const integrationResponse = {

lib/package/s3/compileMethodsToS3.test.js

@@ -567,4 +567,61 @@ describe('#compileMethodsToS3()', () => {
 
     expect(serverless.service.provider.compiledCloudFormationTemplate.Resources).to.be.empty
   })
+
+  const testRequestParameters = (requestParametersOverride) => {
+    const http = {
+      path: 's3',
+      method: 'post',
+      bucket: {
+        Ref: 'MyBucket'
+      },
+      action: 'PutObject',
+      auth: { authorizationType: 'NONE' },
+      requestParameters: requestParametersOverride
+    }
+
+    const requestParams = {
+      'method.request.header.Content-Type': true
+    }
+
+    const intRequestParams = {
+      'integration.request.path.bucket': {
+        'Fn::Sub': [
+          "'${bucket}'",
+          {
+            bucket: {
+              Ref: 'MyBucket'
+            }
+          }
+        ]
+      },
+      'integration.request.header.x-amz-acl': "'authenticated-read'",
+      'integration.request.header.Content-Type': 'method.request.header.Content-Type'
+    }
+
+    const responseParams = {
+      'method.response.header.Content-Type': true,
+      'method.response.header.Content-Length': true
+    }
+
+    const intResponseParams = {
+      'method.response.header.Content-Type': 'integration.response.header.Content-Type',
+      'method.response.header.Content-Length': 'integration.response.header.Content-Length'
+    }
+
+    testSingleProxy({
+      http,
+      logicalId: 'ApiGatewayMethods3Post',
+      method: 'POST',
+      intMethod: 'PUT',
+      requestParams: requestParams,
+      intRequestParams: _.merge(intRequestParams, requestParametersOverride),
+      responseParams,
+      intResponseParams
+    })
+  }
+
+  it('should add custom request parameters mapping', () => {
+    testRequestParameters({ 'integration.request.path.object': 'context.requestId' })
+  })
 })