Skip to content
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.

Sign up for GitHub

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

Jump to bottom

cors-allow-headers does not allow wildcard #10501

Open
lebenitza opened this issue Oct 11, 2023 · 10 comments · May be fixed by #11655
Open

cors-allow-headers does not allow wildcard #10501

lebenitza opened this issue Oct 11, 2023 · 10 comments · May be fixed by #11655
Labels
lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. needs-kind Indicates a PR lacks a `kind/foo` label and requires one. needs-priority needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one.

Comments

@lebenitza
Copy link

What happened:

Tried to set wildcard value for nginx.ingress.kubernetes.io/cors-allow-headers.

What you expected to happen:

The value is allowed as per:
https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Access-Control-Allow-Headers
similar to:
https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Access-Control-Expose-Headers

Anything else we need to know:

Don't know why the allow-headers is treated differently:

		corsAllowHeadersAnnotation: {
			Validator: parser.ValidateRegex(parser.HeadersVariable, true),
			Scope:     parser.AnnotationScopeIngress,
			Risk:      parser.AnnotationRiskMedium,
			Documentation: `This annotation controls which headers are accepted.
			This is a multi-valued field, separated by ',' and accepts letters, numbers, _ and -`,
		},

from

		corsExposeHeadersAnnotation: {
			Validator: parser.ValidateRegex(corsExposeHeadersRegex, true),
			Scope:     parser.AnnotationScopeIngress,
			Risk:      parser.AnnotationRiskMedium,
			Documentation: `This annotation controls which headers are exposed to response.
			This is a multi-valued field, separated by ',' and accepts letters, numbers, _, - and *.`,
		},

See: /internal/ingress/annotations/cors/main.go
@lebenitza lebenitza added the kind/bug Categorizes issue or PR as related to a bug. label Oct 11, 2023
@k8s-ci-robot k8s-ci-robot added the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Oct 11, 2023
@k8s-ci-robot
Copy link
Contributor

This issue is currently awaiting triage.

If Ingress contributors determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

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.

@longwuyuan
Copy link
Contributor

I think you need to write a step-by-step instructions/guide that someone can copy/paste from a reproduce on a minikube/kind cluster. And also add the data like the requests you sent, the responses you got, the controller, the ingress, the logs of controller, etc etc etc.

It will help a reader to understand the issue. Currently there is no data here that is actionable. Like there is no ingress visible as output of kuebctl describe or the tcpdump showing the request and the responses.

/remove-kind bug

@k8s-ci-robot k8s-ci-robot added needs-kind Indicates a PR lacks a `kind/foo` label and requires one. and removed kind/bug Categorizes issue or PR as related to a bug. labels Oct 11, 2023
@lebenitza
Copy link
Author

@longwuyuan there is no need to do that. The code in the controller and actual documentation do not allow wildcard on allow-headers but it does on expose-headers. I clearly pointed the actual code where the problem is, the regex value and docs need to be similar to the bit of code on expose-headers. Don't know why we have to overcomplicate things...

@longwuyuan
Copy link
Contributor

Then its beyond my skills. Need to wait for an expert to comment.

@github-actions GitHub Actions
Copy link

This is stale, but we won't close it automatically, just bare in mind the maintainers may be busy with other tasks and will reach your issue ASAP. If you have any question or request to prioritize this, please reach #ingress-nginx-dev on Kubernetes Slack.

@github-actions github-actions bot added the lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. label Nov 13, 2023
@luarx
Copy link

luarx commented Mar 4, 2024 •
edited
Loading

This would be awesome if it could be implemented! 🙏

It is a standard way
https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Access-Control-Allow-Headers

@avodaqstephan
Copy link

avodaqstephan commented Jun 11, 2024 •
edited
Loading

nginx.ingress.kubernetes.io/cors-allow-headers not supporting a wildcard is weird and just created an issue on our end. The necessary code change was already pointed out. What is necessary @longwuyuan to get that shipped with the next release? :)

@longwuyuan
Copy link
Contributor

Which PR ?

@avodaqstephan
Copy link

There is none (yet). Just wanted to check if more than a PR is necessary due to your statement in the past.

Then its beyond my skills. Need to wait for an expert to comment.

I am by no means an expert on that topic but I could try to create a PR targeting that issue.

@croemmich croemmich linked a pull request Jul 18, 2024 that will close this issue
Open
10 tasks
@sagar-vapi
Copy link

Gentle nudge to merge in the PR if its ready. Would love to use a wildcard for cors-allow-headers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. needs-kind Indicates a PR lacks a `kind/foo` label and requires one. needs-priority needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one.
Projects
[SIG Network] Ingress NGINX
Status: No status
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Accept wildcard in nginx.ingress.kubernetes.io/cors-allow-headers annotation croemmich/ingress-nginx
6 participants
@longwuyuan @lebenitza @k8s-ci-robot @luarx @avodaqstephan @sagar-vapi