-
Notifications
You must be signed in to change notification settings - Fork 8.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature Request: multi-origin support for cors-allow-origin #5496
Comments
As it currently stands, if you attempt to specify a comma-separated list of allowed origins in the This may give the false impression that the list is actually being obeyed, since all listed origins will work... but so will all other origins, since it's ending up as |
Having this feature would be great, we are also struggling to do it. As a workaround, we are using configuration snippet with custom nginx config which is working but still an ugly solution. |
Here's my ugly configuration snippet to work around: apiVersion: networking.k8s.io/v1beta1
kind: Ingress
metadata:
name: foo
annotations:
kubernetes.io/ingress.class: "nginx"
cert-manager.io/cluster-issuer: "letsencrypt-prod"
nginx.ingress.kubernetes.io/configuration-snippet: |
if ($http_origin ~* "^https?://((?:www\.exactmatch\.com)|(?:.*\.regexmatch\.com))$") {
set $cors "true";
}
if ($request_method = 'OPTIONS') {
set $cors "${cors}options";
}
if ($cors = "true") {
add_header 'Access-Control-Allow-Origin' "$http_origin" always;
add_header 'Access-Control-Allow-Methods' 'GET, PUT, POST, DELETE, PATCH, OPTIONS' always;
add_header 'Access-Control-Allow-Headers' 'DNT,X-CustomHeader,Keep-Alive,User-Agent,X-Requested-With,If-Modified-Since,Cache-Control,Content-Type,Authorization' always;
add_header 'Access-Control-Expose-Headers' 'Content-Length,Content-Range' always;
}
if ($cors = "trueoptions") {
add_header 'Access-Control-Allow-Origin' "$http_origin";
add_header 'Access-Control-Allow-Methods' 'GET, PUT, POST, DELETE, PATCH, OPTIONS';
add_header 'Access-Control-Allow-Headers' 'DNT,X-CustomHeader,Keep-Alive,User-Agent,X-Requested-With,If-Modified-Since,Cache-Control,Content-Type,Authorization';
add_header 'Access-Control-Expose-Headers' 'Content-Length,Content-Range';
add_header 'Access-Control-Max-Age' 1728000;
add_header 'Content-Type' 'text/plain charset=UTF-8';
add_header 'Content-Length' 0;
return 204;
} |
What is the expected behavior in this case if the $http_origin is not on whitelist ? |
No headers added, which means no CORS policy so all CORS requests blocked by the browser. |
Hi all apiVersion: extensions/v1beta1
kind: Ingress
metadata:
annotations:
kubernetes.io/ingress.class: nginx
nginx.ingress.kubernetes.io/rewrite-target: /
nginx.ingress.kubernetes.io/cors-allow-credentials: "true"
nginx.ingress.kubernetes.io/enable-access-log: "true"
nginx.ingress.kubernetes.io/enable-rewrite-log: "true"
nginx.ingress.kubernetes.io/proxy-body-size: 100m
ingress.kubernetes.io/force-ssl-redirect: "true"
nginx.ingress.kubernetes.io/configuration-snippet: |
if ($http_origin ~* "^https?://((?:ui|uia)\.stand01\.example\.com)$") {
set $cors "true";
}
if ($cors = "true") {
add_header 'Access-Control-Allow-Origin' "$http_origin";
add_header 'Access-Control-Allow-Methods' 'GET, PUT, POST, DELETE, PATCH, OPTIONS';
add_header 'Access-Control-Allow-Headers' 'DNT,X-CustomHeader,Keep-Alive,User-Agent,X-Requested-With,If-Modified-Since,Cache-Control,Content-Type,Authorization';
add_header 'Access-Control-Expose-Headers' 'Content-Length,Content-Range';
}
name: ui-https-cors-ing
spec:
tls:
- hosts:
- ui.stand01.example.com
secretName: stand01-06-example-com-crt-fullchain
rules:
- host: ui.stand01.example.com
http:
paths:
- backend:
serviceName: ui-svc
servicePort: 8080
path: / |
This would be really good! Support for wildcard subdomains would be really nice:
because "https://app1.domain.com", "https://app2.domain.com", "https://app3.domain.com" all needing to call "https://api.domain.com" is a common need. |
The problem with the workaround:
is that in case the pod is down for some reason, these headers will not be included and the request will be blocked instead of getting 503. so you must add
|
This is a case: |
Hi, any news for CORS support for wildcard subdomains? |
Welcome to submit a new issue to track it. |
@tokers this is the issue "Feature Request: multi-origin support for cors-allow-origin". The op suggested comma-separated list, I suggested wildcard subdomains, or maybe both? I don't think any of us really care that much about those differences we just want a simple way to get multi-origin support w/o writing snippets. |
Ignore this message, I'm not sorry that I misunderstood some stuff there. |
Extend the cors-allow-origin to be a comma-separated string so that multiple origins can be supported is a good point, I will look through the current code base and judge whether this is feasible, if so, I will submit a PR to support it, or, I will explain the reason here. |
/assign |
See #7134 for details :) |
@matthewhartstonge @tokers can we consider to close this one? |
@iamNoah1 hmm, I think we're going to have to go with no... |
PR was reverted as the implementation is not right 😂 |
@tokers It has to resolve to a single origin or |
I think this needs to change:
|
This is a much needed implementation |
@tokers @matthewhartstonge @jfbrennan @constb @gtskaushik we accept and are happy for any contributions. |
Yep, we are also looking for this capability. |
Hi Guys, please suggest how to achieve this use case. We have Cloudflare, which proxies all the requests to our backend based on K8S Nginx ingress. We need to support multiple subdomains (subd01.mydomain.com, subd02.mydomain.com, .....) for our clients, and all should hit the same API backend (api.mydomain.com). |
/help |
@iamNoah1: Please ensure the request meets the requirements listed here. If this request no longer meets these requirements, the label can be removed In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
I have no idea if this is a good way to do it but it works. Let me know if there is something that can be done better. |
IMHO, below are the two solutions wd be perfect
|
Any news? |
@arvtiwar there are enterprises where that list would be HUGE. Being able to * is really important and it looks like the PR supports that |
* Add Initial support for multiple cors origins in nginx - bump cluster version for `make dev-env` - add buildOriginRegex function in nginx.tmpl - add e2e 4 e2e tests for cors.go - refers to feature request #5496 * add tests + use search to identify '*' origin * add tests + use search to identify '*' origin Signed-off-by: Christopher Larivière <[email protected]> * fix "should enable cors test" looking at improper values * Modify tests and add some logic for origin validation - add origin validation in cors ingress annotations - add extra tests to validate regex - properly escape regex using "QuoteMeta" - fix some copy/paste errors * add TrimSpace and length validation before adding a new origin * modify documentation for cors and remove dangling comment * add support for optional port mapping on origin * support single-level wildcard subdomains + tests * Remove automatic `*` fonctionality from incorrect origins - use []string instead of basic string to avoid reparsing in template.go - fix typo in docs - modify template to properly enable only if the whole block is enabled - modify cors parsing - test properly by validating that the value returned is the proper origin - update unit tests and annotation tests * Re-add `*` when no cors origins are supplied + fix tests - fix e2e tests to allow for `*` - re-add `*` to cors parsing if trimmed cors-allow-origin is empty (supplied but empty) and if it wasn't supplied at all. * remove unecessary logic for building cors origin + remove comments - add some edge cases in e2e tests - rework logic for building cors origin there was no need for logic in template.go for buildCorsOriginRegex if there is a `*` it ill be short-circuited by first if. if it's a wildcard domain or any domain (without a wildcard), it MUST match the main/cors.go regex format. if there's a star in a wildcard domain, it must be replaced with `[A-Za-z0-9]+` * add missing check in e2e tests
merged, but build isn't ready yet. |
/assign |
@larivierec just stumbled upon this issue. Any timeline for the release? :) |
you'll have to wait for the official release from the maintainer @rikatz Update: I'll close once version is released |
@larivierec: You can't close an active issue/PR unless you authored it or you are a collaborator. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/close |
@rikatz: Closing this issue. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Please share code with multi-origin support for cors-allow-origin |
You can now specify multiple, comma-separated domains in
|
* Add Initial support for multiple cors origins in nginx - bump cluster version for `make dev-env` - add buildOriginRegex function in nginx.tmpl - add e2e 4 e2e tests for cors.go - refers to feature request kubernetes#5496 * add tests + use search to identify '*' origin * add tests + use search to identify '*' origin Signed-off-by: Christopher Larivière <[email protected]> * fix "should enable cors test" looking at improper values * Modify tests and add some logic for origin validation - add origin validation in cors ingress annotations - add extra tests to validate regex - properly escape regex using "QuoteMeta" - fix some copy/paste errors * add TrimSpace and length validation before adding a new origin * modify documentation for cors and remove dangling comment * add support for optional port mapping on origin * support single-level wildcard subdomains + tests * Remove automatic `*` fonctionality from incorrect origins - use []string instead of basic string to avoid reparsing in template.go - fix typo in docs - modify template to properly enable only if the whole block is enabled - modify cors parsing - test properly by validating that the value returned is the proper origin - update unit tests and annotation tests * Re-add `*` when no cors origins are supplied + fix tests - fix e2e tests to allow for `*` - re-add `*` to cors parsing if trimmed cors-allow-origin is empty (supplied but empty) and if it wasn't supplied at all. * remove unecessary logic for building cors origin + remove comments - add some edge cases in e2e tests - rework logic for building cors origin there was no need for logic in template.go for buildCorsOriginRegex if there is a `*` it ill be short-circuited by first if. if it's a wildcard domain or any domain (without a wildcard), it MUST match the main/cors.go regex format. if there's a star in a wildcard domain, it must be replaced with `[A-Za-z0-9]+` * add missing check in e2e tests
thanks this feature worked great!!! |
Feature Request
In a world of a single API used by multiple SPAs .... 🌏
It would be really helpful to be able to specify a list of origins/domains/subdomains in the
nginx.ingress.kubernetes.io/cors-allow-origin
annotation. Especially for subdomains serving media:media1.site.com, media2.site.com, media3.site.com
Intriguingly, this case was put forward in the initial feature request, but looks like it got reduced to an MVP of a single domain? But since the initial merge the thread has continued with many code blocks of how-to config snippet hacks to enable the support of multiple domain handling. This of course gets very messy - especially when having to apply to each microservice's ingress definition.
Linked Issues
Initial Feature Request: #1171
Prior Art?
/kind feature
The text was updated successfully, but these errors were encountered: