serving icon indicating copy to clipboard operation
serving copied to clipboard

Enable CORS configuration

Open zhaojizhuang opened this issue 5 years ago • 27 comments

In what area(s)?

/area networking

Describe the feature

We should enable Cross-Origin Resource Sharing (CORS) configuration in ksvc’s annotations

Reconciler reconcile CORS info from ksvc (annotations) to virtualservice: ksvc(annotations) -> route(annotations) -> kingress( Entity spec) -> virtualservice

we can config by doing that:

  • Configurations set in config-domain for default
  • Configuration set in ksvc or route for each service

Fields included listed as follows, like istio Corspolicy

corsPolicy:
      allowOrigins:
      - exact: https://example.com
      allowMethods:
      - POST
      - GET
      allowCredentials: false
      allowHeaders:
      - X-Foo-Bar
      maxAge: "24h"

zhaojizhuang avatar Nov 05 '20 09:11 zhaojizhuang

kind/feature

zhaojizhuang avatar Nov 05 '20 09:11 zhaojizhuang

/area networking

zhaojizhuang avatar Nov 05 '20 09:11 zhaojizhuang

@markusthoemmes @vagababov have a look

zhaojizhuang avatar Nov 05 '20 09:11 zhaojizhuang

/assign @tcnghia @nak3 @ZhiminXiang

vagababov avatar Nov 05 '20 17:11 vagababov

I am wondering that this kind of custom configuration should be supported by each KIngress's (net-istio, net-contour, etc...) as their plugin :thinking: But I don't have any good conclusion yet.

nak3 avatar Nov 06 '20 02:11 nak3

I am wondering that this kind of custom configuration should be supported by each KIngress's (net-istio, net-contour, etc...) as their plugin 🤔 But I don't have any good conclusion yet.

@nak3 Do you mean the global configuration in KIngress's controller (net-istio, net-contour, etc...)? In that way it can't be configured separately for each service . And I think we can configure the global Configuration in configmap config-domain and separately configurations in earch ksvc。What do you think about this?

zhaojizhuang avatar Nov 06 '20 07:11 zhaojizhuang

@tcnghia @ZhiminXiang What do you think about this?

zhaojizhuang avatar Nov 06 '20 07:11 zhaojizhuang

I am not so positive to introduce the complex annotations. CORS policy will need 5 or 6 new annotation settings... There isn't any smart or simple way?

And it would be a minor problem but if you support it as global configuration, all Kingress must support it in the same way. For example, Istio can support exact,prefix,regex for allowOrigins. But ambassador seems not have the regex but just wildcard * only - https://www.getambassador.io/docs/latest/topics/using/cors/#the-cors-attribute

nak3 avatar Nov 06 '20 10:11 nak3

I am not so positive to introduce the complex annotations. CORS policy will need 5 or 6 new annotation settings... There isn't any smart or simple way?

And it would be a minor problem but if you support it as global configuration, all Kingress must support it in the same way. For example, Istio can support exact,prefix,regex for allowOrigins. But ambassador seems not have the regex but just wildcard * only - https://www.getambassador.io/docs/latest/topics/using/cors/#the-cors-attribute

To avoid introducing the complex annotations, we can support just CORS enable setting, at least. Such as networking.kantive.dev/enable-cors: "true" . And all Kingress support CORS in their default way. like

allow-methods: Default: GET, PUT, POST, DELETE, PATCH, OPTIONS
allow-headers: Default DNT,X-CustomHeader,Keep-Alive,User-Agent,X-Requested-With,If-Modified-Since,Cache-Control,Content-Type,Authorization
allow-origin: Default *
allow-credentials: Default true
max-age: 1728000

zhaojizhuang avatar Nov 06 '20 15:11 zhaojizhuang

Can't the service itself send the respective CORS headers? :thinking:

markusthoemmes avatar Nov 06 '20 15:11 markusthoemmes

Can't the service itself send the respective CORS headers? 🤔

@markusthoemmes yes,we can. But is it much better if we separate service code and Cors configuration? Follow the 12-factor APP

zhaojizhuang avatar Nov 06 '20 15:11 zhaojizhuang

I'm not quite sure how this would relate to 12-factor, can you point out which rule you see violated if the service itself returns the respective CORS headers?

If it can be solved as "easily" as saying: just return the CORS headers you need from your app, I'd vote -1 on including it into our Ingress APIs, seeing as we kinda want to keep them as flat and of little surface as we can. Anything we add is more customized bits we need to make sure interact well with the entire ecosystem.

markusthoemmes avatar Nov 06 '20 15:11 markusthoemmes

@markusthoemmes III Config: https://12factor.net/config.
May be we have another way. See https://kubernetes.github.io/ingress-nginx/user-guide/nginx-configuration/annotations/#enable-cors . Ingress APIs need not be changed. Let Kingress's plugins (net-istio, net-contour, etc...) support Cors in there way: for example, net-istio watching kingress's cors annotations,like istio.networking.kantive.dev/allow-origin: * ... and do some thing. net-contour watching kingress's cors annotations,like net-contour.networking.kantive.dev/allow-origin: * ... and so on. The plugins maintains their own annotations

zhaojizhuang avatar Nov 06 '20 16:11 zhaojizhuang

  1. I agree with @nak3 that introducing CORS into Kingress spec may not be a good idea considering that different implementations may or may not support the full set of CORS. We may want to start with net-istio.

  2. The flat annotations sounds like a good starting point to me considering this pattern has been used in k8s ingress.

ZhiminXiang avatar Nov 10 '20 04:11 ZhiminXiang

Thought about this again. How should this feature align with Ingress V2 https://kubernetes-sigs.github.io/service-apis/concepts/? The context here is that we plan to use Ingress V2 as the intermediate ingress resources for the long term. (You can consider Ingrerss V2 will be a replacement to current Kingress). Because of this, it requires the new Knative networking features align with Ingress V2.

A open question here for Knative networking is: how we can make Knative networking extensible for the features that are not supported by Ingress V2? Currently we can pass annotations to Kingress, and let contributors customize Kingress implementation based on the annotations. But with Ingress V2, seems like contributors have to work with the specific Ingress V2 solution (e.g. Istio) to support those customized features 🤔

ZhiminXiang avatar Nov 16 '20 06:11 ZhiminXiang

@ZhiminXiang Why we can't paas annotations to Ingress V2(Such as HTTPRoute's annotations)? we can config CORS on Kingress now or Ingress v2 (HTTPRoute)when ingress v2 supported. Both are ok.

zhaojizhuang avatar Nov 16 '20 15:11 zhaojizhuang

@zhaojizhuang it works with the current infra as currently we have `net-*) controller that can consume the annotations within Kingress, and then translate to the corresponding networking solutions (e.g. Istio, contour, kong, etc).

In the ingress V2 world, we will get rid of net-* controller assuming that Ingress V2 is the standard API that different networking solutions will naturally support. The new workflow will be Route->Ingress V2-> Istio,contour,kong,etc,. Because we will not have the net-* controller layer to watch Ingress V2 and do the translation, it completely rely on if the specific networking solution (e.g. Istio, contour, Kong) will consume the annotations and implement the features defined with the annotations.

ZhiminXiang avatar Nov 17 '20 00:11 ZhiminXiang

@ZhiminXiang In another word, It is the provider(e.g. Istio, contour, Kong networking solution)‘s own to to implement or not implement this feature. Right? If that case, we do not need to do it in knative.

And I want to know, when will we depricate net-* controller.

zhaojizhuang avatar Nov 17 '20 02:11 zhaojizhuang

@ZhiminXiang In another word, It is the provider(e.g. Istio, contour, Kong networking solution)‘s own to to implement or not implement this feature. Right? If that case, we do not need to do it in knative.

Yes exactly.

And I want to know, when will we depricate net-* controller.

We don't have the specific timeline yet as the Ingress V2 is in an early stage, and the implementations from the providers are being worked in progress. A very rough estimate could be Q3/Q4 next year (hopefully).

ZhiminXiang avatar Nov 18 '20 21:11 ZhiminXiang

This issue is stale because it has been open for 90 days with no activity. It will automatically close after 30 more days of inactivity. Reopen the issue with /reopen. Mark the issue as fresh by adding the comment /remove-lifecycle stale.

github-actions[bot] avatar Feb 17 '21 01:02 github-actions[bot]

Ingress V2 is the Gateway API?

kahirokunn avatar Jan 06 '23 09:01 kahirokunn

Ingress V2 is the Gateway API?

Yes, exactly.

For memo myself: Gateway API is also still under discussion - https://github.com/kubernetes-sigs/gateway-api/issues/1767

nak3 avatar May 06 '23 05:05 nak3

This issue is stale because it has been open for 90 days with no activity. It will automatically close after 30 more days of inactivity. Reopen the issue with /reopen. Mark the issue as fresh by adding the comment /remove-lifecycle stale.

github-actions[bot] avatar Aug 06 '23 01:08 github-actions[bot]

keep

kahirokunn avatar Aug 06 '23 01:08 kahirokunn

This issue is stale because it has been open for 90 days with no activity. It will automatically close after 30 more days of inactivity. Reopen the issue with /reopen. Mark the issue as fresh by adding the comment /remove-lifecycle stale.

github-actions[bot] avatar Nov 06 '23 01:11 github-actions[bot]

keep

kahirokunn avatar Nov 06 '23 02:11 kahirokunn

keep

whatnick avatar Feb 28 '24 04:02 whatnick