serverless-cors-plugin icon indicating copy to clipboard operation
serverless-cors-plugin copied to clipboard

Deploy OPTIONS without `-a`

Open doapp-ryanp opened this issue 8 years ago • 13 comments

How do you just deploy the OPTIONS method for one endpoint? I was hoping when I deployed my GET method this plugin would detect that no OPTIONS method was present for endpoint, and it would automatically create the OPTIONS method.

I have added the following to s-project.json

"custom": {
  "cors": {
      "allowOrigin": "*",
      "allowHeaders": [
        "Content-Type",
        "X-Amz-Date",
        "Authorization",
        "X-Api-Key"
      ],
      "allowCredentials": true,
      "maxAge": 3600
    }
...

I have also tried adding to s-function.json.

the OPTIONS does not show up under serverless dash deploy and it gives me an error when I try to run serverless endpoint deploy "discover/wx/{version}/latlng~OPTIONS" saying method not found.

Lastly I added an OPTIONS to my s-function.json endpoints. I was able to deploy the method, however looking at --debug I don't see your plugin getting invoked and the integration request is not using a MOCK

any ideas whats going on here? serverless v0.5.1. Thanks

doapp-ryanp avatar Apr 04 '16 21:04 doapp-ryanp

Hi @doapp-ryanp! There is similar issue here: https://github.com/joostfarla/serverless-cors-plugin/issues/6

To reopen this discussion, I'll repeat one of my comments:

This is done intentionally because multiple functions with different HTTP methods can share the same path. The question I was struggling with was: If one of x functions with the same path is being deployed, should it deploy the preflight endpoint for all methods?

What do you think?

joostfarla avatar Apr 05 '16 05:04 joostfarla

Thanks for the quick response.

I've read https://github.com/joostfarla/serverless-cors-plugin/issues/6 and I'm not quite sure I understand the issue (it is early in the am tho ;)

An endpoint can have one or more method (where methods are GET,POST,PUT, OPTIONS etc). So OPTIONS are at the same level under and endpoint as other methods.

Here is how I would attack the problem. Disclaimer I'm not sure if the serverless plugin system allows hooks into these extension points...

  1. If I define custom.cors in my s-function.json I would expect all endpoints in the s-function.json (except for OPTIONS methods) to create the CORS OPTIONS method in APIG when I deploy the endpoint.

If multiple functions have endpoints with the same path that is ok - deploying the OPTIONS method under that endpoint again will not hurt anything. You could write logic to check to see if an OPTIONS method already exists for said endpoint, but might not be worth it (because someone may be updating their OPTIONS config via your custom.cors attrs (which are nice btw). 2. If I define custom.cors in my s-project.json anytime I do sls endpoint deploy I would expect your plugin to create an OPTIONS method in APIG.

To summarize: 2 functions can define the same endpoints.path and even the same function can have 2 endpoints with the same path. But because OPTIONS is just a method under the endpoint AND because options uses a MOCK integration there is no harm in always creating the OPTIONS method in APIG.

I rarely want to deploy all endpoints. I (and I think most users) only want to deploy a specific endpoint(s) they are working on and not touch any other endpoints. For ex: If I already have a users~GET and I'm add a users~DELETE I do not want to re-deploy users~GET just to get my OPTIONS method created for users~DELETE. If something is working in production, I don't to touch it...

Does that make sense or am I missing something?

doapp-ryanp avatar Apr 05 '16 14:04 doapp-ryanp

I agree with @doapp-ryanp that if it is possible (and I might not understand all the edge cases) that I would like to be able to only deploy the OPTIONS endpoint defined in the function for which the endpoint exists.

duckworth avatar Apr 05 '16 17:04 duckworth

So I think we found a more straightforward way to handle CORS with just using serverless templates. Using the strategy below eliminates need for this plugin and npm module, and it allows tighter integration with other serverless tools (like it shows up in the dash) as well as more fine grain control.

s-templates.json:

"apiDefault200Response": {
    "statusCode": "200",
    "responseParameters": {
      "method.response.header.Access-Control-Allow-Origin": "'*'"
    },
    "responseModels": {},
    "responseTemplates": {
      "application/json": ""
    }
  },
  "api400ErrorMatchResponse": {
    "statusCode": "400",
    "selectionPattern": "MlnError.*|Task timed out.*",
    "responseTemplates": {
      "application/json": ""
    },
    "responseParameters": {
      "method.response.header.Access-Control-Allow-Origin": "'*'"
    }
  },
  "apiCorsOptionsResponse": {
    "default": {
      "statusCode": "200",
      "responseParameters": {
        "method.response.header.Access-Control-Allow-Headers": "'Content-Type,X-Amz-Date,Authorization,X-Api-Key'",
        "method.response.header.Access-Control-Allow-Methods": "'GET,OPTIONS,HEAD,DELETE,PATCH,POST,PUT'",
        "method.response.header.Access-Control-Allow-Origin": "'*'",
        "method.response.header.Access-Control-Max-Age": "'3600'"
      },
      "responseModels": {},
      "responseTemplates": {
        "application/json": ""
      }
    }
  },
  "apiCorsRequestTemplate": {
    "application/json": {
      "statusCode": 200
    }
  }

s-function.json

...
"endpoints": [
    {
      "path": "discover/wx/{version}/latlng",
      "method": "GET",
      "type": "AWS",
      "authorizationType": "none",
      "authorizerFunction": false,
      "apiKeyRequired": false,
      "requestParameters": {
        "integration.request.querystring.lat": "method.request.querystring.lat",
        "integration.request.querystring.lng": "method.request.querystring.lng"
      },
      "requestTemplates": "$${apiRequestTemplate}",
      "responses": {
        "400": "$${api400ErrorMatchResponse}",
        "default": "$${apiDefault200Response}"
      }
    },
    {
      "path": "discover/wx/{version}/latlng",
      "method": "OPTIONS",
      "type": "MOCK",
      "requestTemplates": "$${apiCorsRequestTemplate}",
      "responses": "$${apiCorsOptionsResponse}"
    }
  ],
...

As you can see the OPTIONS method is super straight forward. If you want your endpoint to have OPTIONS just add the path and 4 attributes.

The only semi-tricky thing is the other methods (GET in this example) must include "method.response.header.Access-Control-Allow-Origin": "'*'" in their responseParameters. You can see we put this in our response templates so they get auto-inherited in all endpoints that wish to use them.

Lastly, if you need Access-Control-Allow-Credentials this must also be defined in your OPTIONS and other methods. It is just not pictured here.

This method doesn't dynamically set the Access-Control-Allow-Methods based on the other methods under your endpoint, however having methods in the Access-Control-Allow-Methods that you do not implement does not have a negative impact.

doapp-ryanp avatar Apr 05 '16 19:04 doapp-ryanp

@doapp-ryanp Thank you sooooo much. I got confused by the existence of this plugin - but really love your solution. No plugin and fine grain control at the place where it belongs.

I suggest the Readme of this Plugin should point your solution out in HUGE letters ! It is too easy to fall into the trap of using to many unnecessary plugins.

BerndWessels avatar Apr 28 '16 01:04 BerndWessels

Of course you're free to implement CORS manually using templates, but I do think the plugin has added value:

  • You don't need advanced CORS knowledge to implement it 100% the right way
  • The plugin is tested and maintained; bugfixes and improvements will automatically be applied
  • It automatically keeps the method.response.header.Access-Control-Allow-Methods in sync
  • Soon, the plugin will be capable of dynamically returning headers for authenticated requests, which is something you otherwise would have to do by hand.

The problem however is the current plugin architecture of the SLS framework. It's missing essential extension points to achieve a tighter integration with the framework. What we need are more strategic extension points, like for example a postProjectLoad action/hook. Any change done here would have the desirable effect on all the CLI tools, like dash or any custom built action.

One other issue is that the plugin handles merging of the templates at a very late stage. In my opinion, this should be done instantly on project loading. That is the issue which is causing the unwanted warnings on deployment.

I will make an effort to solve the mentioned issues with the core team. Furthermore, I don't think having small convenience plugins is a bad thing, we're building Node.js apps, right? ;)

To get back to the original question, I do want to implement partial deployments. (thanx @doapp-ryanp for your thoughts!). Will work on it asap.

joostfarla avatar Apr 28 '16 07:04 joostfarla

@doapp-ryanp how do you avoid your OPTIONS endpoints from executing the handler of the GET/POST/etc endpoint?

arieljake avatar May 13 '16 22:05 arieljake

@arieljake I back them with "type": "MOCK" APIG implementations. Check out my example from April 5 above.

doapp-ryanp avatar May 16 '16 13:05 doapp-ryanp

Just created two issues (https://github.com/serverless/serverless/issues/1133 & https://github.com/serverless/serverless/issues/1134) which will potentially solve the mentioned issues and make the Serverless framework more extensible.

joostfarla avatar May 18 '16 13:05 joostfarla

Just as a side note: It looks like serverless will be moving to a complete CloudFormation based solution without _meta folders and all these complications. That would mean that @doapp-ryanp way of managing CORS is definitely the more desirable way since it is a native part of CloudFormation without the need of another artificial process.

BerndWessels avatar May 18 '16 19:05 BerndWessels

@BerndWessels I disagree! If the framework would offer extension points on project loading, this would still be included in the CF template and offer lots of convenience! See https://github.com/serverless/serverless/issues/1133

joostfarla avatar May 18 '16 19:05 joostfarla

@joostfarla I appreciate your work a lot.

I just come from a different angle. In big enterprise companies with huge amounts of developers and processed around access rights , Git and PRs it becomes very difficult.

Not every developer will be allowed to change resources, use plugins and so on. Everything will be restricted and has to follow processes. There are further limitations on CI processes and limitations.

That is why the pure CloudFormation based approach seems much more flexible to put these complicated restrictions on top.

For smaller companies with only a handful of developers who all have full access to all resources the convenience that plugins offer is absolutely fine.

BerndWessels avatar May 18 '16 20:05 BerndWessels

@BerndWessels I understand your position, but that will probably count for all plugins then ;)

joostfarla avatar May 19 '16 18:05 joostfarla