Enable CORS configuration

[zhaojizhuang]

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]

kind/feature

[zhaojizhuang]

/area networking

[zhaojizhuang]

@markusthoemmes @vagababov have a look

[vagababov]

/assign @tcnghia @nak3 @ZhiminXiang

[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.

[zhaojizhuang]

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]

@tcnghia @ZhiminXiang What do you think about this？

[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

[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

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

[markusthoemmes]

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

[zhaojizhuang]

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

[markusthoemmes]

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.

[zhaojizhuang]

@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

[ZhiminXiang]

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]

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 🤔

[zhaojizhuang]

@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.

[ZhiminXiang]

@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.

[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.

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

[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.

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).

[github-actions]

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.

[kahirokunn]

Ingress V2 is the Gateway API?

[nak3]

Ingress V2 is the Gateway API?

Yes, exactly.

For memo myself: Gateway API is also still under discussion - kubernetes-sigs/gateway-api#1767

[github-actions]

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.

[kahirokunn]

keep

[github-actions]

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.

[kahirokunn]

keep

[whatnick]

keep