fix: make s3 key, requestParameters mutually exclusive when applicable

Author: erezrokah

README.md

@@ -189,12 +189,12 @@ custom:
         action: PutObject
         bucket:
           Ref: S3Bucket
-        key: static-key.json
         cors: true
 
         requestParameters:
-          # use auto generated object key (will override the static key)
+          # 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

lib/apiGateway/schema.js

@@ -157,7 +157,14 @@ 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
     })
   }),

lib/apiGateway/validate.test.js

@@ -895,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)
           }
         ]
       }
@@ -1026,6 +1026,34 @@ describe('#validateServiceProxies()', () => {
     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 = {

lib/package/s3/compileMethodsToS3.test.js

@@ -576,7 +576,6 @@ describe('#compileMethodsToS3()', () => {
         Ref: 'MyBucket'
       },
       action: 'PutObject',
-      key: 'static',
       auth: { authorizationType: 'NONE' },
       requestParameters: requestParametersOverride
     }
@@ -586,7 +585,6 @@ describe('#compileMethodsToS3()', () => {
     }
 
     const intRequestParams = {
-      'integration.request.path.object': 'method.request.path.key',
       'integration.request.path.bucket': {
         'Fn::Sub': [
           "'${bucket}'",