Add support for CORS in APIG

[eahefnawy]

We need to have an easy way to set a certain endpoint to be cross origin friendly. To do that we'll need to to add the relevant CORS headers to response.parameters, which we currently don't set. This is relevant to supporting custom response templates (see Issue #1593 ). What I'm thinking is the following config for http event:

- http:
    path: user/create
    method: get
    cors: true # could be true by default maybe?

We can add even more options by inheriting logic from the serverless-cors-plugin. Although maybe we should first discuss allowing custom response templates first, since custom response templates might overwrite this cors settings

Thoughts...? cc/ @flomotlik @pmuens @ac360

Parent Issue:
#1408

[eahefnawy]

@patoncrispy suggests this structure in issue 1586:

functions: 
  user:
    handler: handler.user
    events:
      - http:
          method: post
          cors: true
custom: 
    cors:
        allow-origin: '*'
        allow-headers:
            - Content-Type
            - X-Amz-Date
            - Authorization
            - X-Api-Key
        allow-methods:
            - POST
plugins:
    - api-gateway-cors-plugin

This provide more options for users, but it's a bit more complex. Plus, it seems that this is a service wide config. @patoncrispy do you think this is necessary? Why not just enable CORS for specific endpdionts, and users can always user serverless variables or JSON-REF if they wanna stay DRY.

So I vote for the following:

- http:
    path: user/create
    method: get
    cors: true

This will enable cors with some default config, so that users don't have to define the headers and methods and so on. Not really sure about the exact default config (@patoncrispy suggestions are super welcomed here 😊 ). If the user want more flexibility they can always turn the cors property into an object rather than a boolean and be more specific. This boolean/string & object pattern is something we've been following in other events too. So the following would work too:

- http:
    path: user/create
    method: get
    cors: 
        allow-origin: '*'
        allow-headers:
            - Content-Type
            - X-Amz-Date
            - Authorization
            - X-Api-Key
        allow-methods:
            - POST

Thoughts...?

[mthenw]

I really like @eahefnawy approach. It's simple and follows the same pattern with simple/extended config. Less confusing as whole config is under cors key, not somewhere else.

[pmuens]

I would like to keep the event related config inside the event object (like @eahefnawy showed with the last example) so that it's near to each other and kind of "encapsulated" logic which is easy to find and follow along.

[patoncrispy]

Apologies, but your final example @eahefnawy had been my initial intent. :) I think allowing endpoint-specific config will be really important. As for defaults, following the AWS developer guide, I would suggest perhaps:

- http:
    path: user/create
    method: get
    cors: 
        allow-origin: '*'
        allow-headers:
            - Content-Type
            - X-Amz-Date
            - Authorization
            - X-Api-Key

In this instance, the Access-Control-Allow-Methods option is set to whatever HTTP endpoint the CORS configuration sits under (GET, in this example). This would be based on the default settings APIGW sets when you configure CORS via the console. The above example would be the same as writing:

- http:
    path: user/create
    method: get
    cors: true

[eahefnawy]

@patoncrispy that's perfect! 😊 Let's do that then. When someone has the following config:

- http:
    path: user/create
    method: get
    cors: true

In the background, this will translate into:

- http:
    path: user/create
    method: get
    cors: 
        allow-origin: '*'
        allow-headers:
            - Content-Type
            - X-Amz-Date
            - Authorization
            - X-Api-Key

and if the cors property is an object rather than a boolean, then whatever that user sets in there will be deployed (I'm guessing well need some validation in that case).

@flomotlik @pmuens any further thoughts?

[pmuens]

Looks good. But I still have both options to specify a simple true but also the more complex config object, right?

[eahefnawy]

yep

[flomotlik]

looks great

[patoncrispy]

Is it OK for me to get started on this then? :)

[eahefnawy]

yep @patoncrispy go ahead ... Thanks for pitching in! 😊 💃

[patoncrispy]

Woop! 😃 🎉 Thanks to @flomotlik for updating the CONTRIBUTING.md guide! Will hopefully get started tomorrow evening BST. Excited!

[flomotlik]

yeah sorry @patoncrispy totally get started on this one. Very much appreciate the help and I'll make sure to be more specific on when I think its good to go (something we already discussed internally we want to do better). Thanks for the reminder :)

[wedgybo]

👍 @patoncrispy If you need any help just shout as I'm coming across this issue now and would like to get it sorted before moving on if possible.

[patoncrispy]

@wedgybo I'm almost done! Should hopefully be able to get it done by the end of the weekend (damn full time job!). :)

[flomotlik]

@patoncrispy could you open a PR (even if its still not ready) so we can take a look?

[patoncrispy]

Sure! I'll do it tonight when I get home. Would be good to get feedback.

[patoncrispy]

As you'll see in the pull request, keen to get some advice on where to put the preflight configuration.

[pmuens]

Awesome! Thank you @patoncrispy. We'll look into it ASAP.

[patoncrispy]

A quick note just to say that during development I simplified the schema slightly to:

- http:
    path: user/create
    method: get
    cors: 
        origins: '*'
        headers:
            - Content-Type
            - X-Amz-Date
            - Authorization
            - X-Api-Key

[pmuens]

Done with #1703

[pmuens]

Finally done with #1820 🎉