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.

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

Allow serving http2 over port 80 with a configmap setting or annotation #10429

Open
stabai opened this issue Sep 20, 2023 · 7 comments · May be fixed by #10918
Open

Allow serving http2 over port 80 with a configmap setting or annotation #10429

stabai opened this issue Sep 20, 2023 · 7 comments · May be fixed by #10918
Labels
kind/feature Categorizes issue or PR as related to a new feature. needs-priority needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one.

Comments

@stabai
Copy link

stabai commented Sep 20, 2023

Problem

Using the normal config, http2 traffic (such as gRPC) is only ever served over https contexts. This causes issues with local development, where using a self-signed cert is possible, but will require extra work and also generate lots of security warnings.

See this StackOverflow question for more details.

Proposed solution

We enable gRPC traffic using an annotation:

nginx.ingress.kubernetes.io/backend-protocol: "GRPC"

It would be great if there were another annotation that allowed serving http2 traffic over port 80. For example:

annotations:
  nginx.ingress.kubernetes.io/http2-insecure: true

Alternatively, the configmap might be a more appropriate place for this:

data:
  http2-insecure: true

If neither of these are feasible, it would at least be good to document this fact in the gRPC example page along with any potential workarounds.

@stabai stabai added the kind/feature Categorizes issue or PR as related to a new feature. label Sep 20, 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 Sep 20, 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

Yes, it would help to be able to server HTTP2 over port 80.

But on one hand the implementation of breaking changes like this is not a great direction to take for any ingress-controller. Maybe you can research if any other ingress-controller has this feature. That info will be added clarity.

Secondly, I don't think there are resources to even design HTTP2 and HTTP1 over plaintext port 80. Not even sure how gRPC is practical on over plaintext or insecure with port 80 because I am not a developer.

@stabai
Copy link
Author

stabai commented Sep 20, 2023

NGINX is the only place I've seen this requirement of using TLS with gRPC (which is why it was so surprising to me). Every other place I've used gRPC before works just fine over both plaintext and secure transport (just like http 1.1 does).

Also, while I agree that making this a breaking change would be really bad, I don't see how adding a new annotation or configmap that allowed changing the behavior would be a breaking change? Is it just not possible for those things to alter the NGINX config in this way? Would a custom template be the right place for a user to alter this behavior?

@longwuyuan
Copy link
Contributor

I will defer to other folks on that. Thanks.
@rikatz @tao12345666333

@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 lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. and removed lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. labels Oct 22, 2023
@rsafonseca rsafonseca linked a pull request Jan 26, 2024 that will close this issue
5 tasks
@rikatz
Copy link
Contributor

rikatz commented Jan 26, 2024

Hi, just a headsup. on nginx v1.25 the http2 handler changes places, so it will be enabled by server{} directive and not on the listen directive. It should solve this

@jetersen
Copy link
Contributor

@rikatz now that v1.25 is out. How would this be supported? What config do we need to change?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. needs-priority needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one.
Projects
Development

Successfully merging a pull request may close this issue.

5 participants