Add CORS support to python and golang runtimes

[vallard]

similar to issue #250 it would be nice to have cors support for python or golang. For python, if I could at least have my function return a bottle.Response that would work but the runtime doesn't seem to allow it. Here is my code:

def list(event, context):
  return bottle.Response("foo", 200, {"Access-Control-Allow-Origin" : "*"})

[andresmgot]

Hi @vallard, that'd be great. Right now we don't have the resources to work on that, would you mind giving it a try? We are open to pull requests :)

[imishravmw]

@andresmgot This fix does not seems to best approach for solving CORS. As sometime a user may not want to enable CORS or one may wants to enable CORS only for a specific domain.

[imishravmw]

Better way to fix CORS support to enable or disable CORS in Ingress resource when Kubeless creates Ingress from HttpTrigger. @andresmgot , do you think this is proper approach. As while creating http trigger, we can give options for enabling cors and domain like below.
kubeless trigger http create get-python --function-name get-python --path hello --cors true --cors-domain "*.subdomain.com"
We will like to work on this, which will add CORS support for all runtimes.

[vallard]

We would require those flags be something we could put in a serverless config file as well. The default node implementation just enables it by default. The ingress approach though seems pretty good. There are also issues I have as the default golang only returns a string when we’d like to also return JSON or different status codes other than 200. That is probably for another issue though.

…

On Oct 29, 2018, at 7:10 AM, imishravmw ***@***.***> wrote: Better way to fix CORS support to enable or disable CORS in Ingress resource when Kubeless creates Ingress from HttpTrigger. @andresmgot , do you think this is proper approach. As while creating http trigger, we can give options for enabling cors and domain like below. kubeless trigger http create get-python --function-name get-python --path hello --cors true --cors-domain "*.subdomain.com" We will like to work on this, which will add CORS support for all runtimes. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[andresmgot]

I am not sure how that would work at the ingress level (I am not sure if you can configure that in an ingress object). In the function, if that is a feature you need, I would just define an env var for that purpose. For example ACCESS_CONTROL_ALLOW_ORIGIN, ACCESS_CONTROL_ALLOW_METHODS... In the function, if those variables are not set, we can set the default to "allow all". That way it wouldn't be necessary to add a new flag.

[imishravmw]

We would require those flags be something we could put in a serverless config file as well. The default node implementation just enables it by default. The ingress approach though seems pretty good. There are also issues I have as the default golang only returns a string when we’d like to also return JSON or different status codes other than 200. That is probably for another issue though.
…

yes certainly, we need to make changes to serverless to provide that, one draw back in server less is it only provides cors "true" or "false", it should also support cors domain when cors is true.

[imishravmw]

I am not sure how that would work at the ingress level (I am not sure if you can configure that in an ingress object). In the function, if that is a feature you need, I would just define an env var for that purpose. For example ACCESS_CONTROL_ALLOW_ORIGIN, ACCESS_CONTROL_ALLOW_METHODS... In the function, if those variables are not set, we can set the default to "allow all". That way it wouldn't be necessary to add a new flag.

Kubernetes nginx Ingress does support CORS annotations, please refer nginx-ingress-doc.
Adding env to function has a drawback, that I might create two HTTP trigger, with one I want CORS, but with another I don't want. That will not be solved by doing only at function level. as CORS is handled by Ingress controller. But certainly we could something like, if cors is defined at function level, that gets applied to all triggers by default and if cors is not set at function level, then one can define at a individual trigger level.

[vallard]

Do you have any target for when you are going to put this PR in? We are getting along fine with our custom images at present but I could submit the changes as suggested by Andres tomorrow for golang and python.

…

On Tue, Oct 30, 2018 at 2:52 AM imishravmw ***@***.***> wrote: I am not sure how that would work at the ingress level (I am not sure if you can configure that in an ingress object). In the function, if that is a feature you need, I would just define an env var for that purpose. For example ACCESS_CONTROL_ALLOW_ORIGIN, ACCESS_CONTROL_ALLOW_METHODS... In the function, if those variables are not set, we can set the default to "allow all". That way it wouldn't be necessary to add a new flag. Kubbernetes nginx Ingress does support CORS annotations, please refer nginx-ingress-doc <https://kubernetes.github.io/ingress-nginx/user-guide/nginx-configuration/annotations/> . Adding env to function has a drawback, that I might create two HTTP trigger, with one I want CORS, but with another I don't want. That will not be solved by doing only at function level. as CORS is handled by Ingress controller. But certainly we could something like, if cors is defined at function level, that gets applied to all triggers by default and if cors is not set at function level, then one can define at a individual trigger level. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#934 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAKVTWhWLebm4FVIaJDK9nfxygo1RzfNks5uqCFCgaJpZM4X7DTQ> .

[andresmgot]

@vallard you can go ahead with your changes since it would be useful to handle CORS in the function anyway. As far as I understood we can do it in both places, the function and the Ingress controller. @imishravmw can follow up with further PRs if needed. I would not include the env vars for the moment, just the accept-all policy that we have right now.

[imishravmw]

@andresmgot I will be raising PR for CORS at ingress level , so it will be available for all runtimes by same solution. I am just waiting for some approvals at my org before raising PR.

[imishravmw]

I have got two PR ,one in http-trigger repo vmware-archive/http-trigger#10 , which needs to get first done to pass another PR which is in kubeless repo #1008 ,

second PR is failing CI checks because it depends upon first PR.
Even first PR vmware-archive/http-trigger#10 is failing with circleci errors, but not sure why ,can someone help for this. @andresmgot

[andresmgot]

@imishravmw this repo is now using the latest release of the http-triggger with your changes. You can update the PR in this repository.

[imishravmw]

@imishravmw this repo is now using the latest release of the http-triggger with your changes. You can update the PR in this repository.

I could not understand, what you want me to do by saying "update the PR"
There is related issue #819 , both of these could be closed.

[andresmgot]

I meant that at that point you could update #1008 (that is now closed). Thanks for your contribution!