NGF: Add document for upstream http2#2021
Conversation
github-actions
Bot
added
documentation
|
This is a shorter guide which is technically covering a Kubernetes concept (Service's appProtocol), however I couldn't find a suitable document to include this information in. So I decided to write a standalone document for it. |
| ```nginx | ||
| location /coffee { | ||
| ... | ||
| proxy_http_version 2; | ||
| proxy_pass http://default_coffee_80; | ||
| ... | ||
| } | ||
| ``` | ||
|
|
There was a problem hiding this comment.
so the curl doesn't return the protocol? or a different response to identify a http2 connection?
There was a problem hiding this comment.
The upstream application in this example doesn't respond back with http2 responses/ "doesn't speak http2" so it won't really work with a curl request. I don't really feel the need to show a response with a working http2-available backend, so I felt that just showing the nginx conf would suffice.
If you feel like its necessary to show a curl request, basically checking if the nginx conf proxy_http_version 2; works, then we can consider that, but I don't feel the need.
There was a problem hiding this comment.
A user guide implies end-to-end coverage. If this doesn't do that, it should be scoped down to a section within a larger doc, this feels incomplete to me.
could this be part of data plane configuration section as part of how to?
There was a problem hiding this comment.
we could do a small section to enable HTTP2 on a service section
| ## Important Notes | ||
|
|
||
| - `kubernetes.io/h2c` is supported on HTTPRoutes and GRPCRoutes. It isn't supported on TLSRoutes. | ||
| - For NGINX to set `proxy_http_version 2` for a location, all valid backend references in the routing rule must have `appProtocol: kubernetes.io/h2c` set on their Service ports. If any valid backend doesn't use `kubernetes.io/h2c`, NGINX falls back to the default HTTP/1.1. |
There was a problem hiding this comment.
i think this could be in the troubleshooting section
There was a problem hiding this comment.
Would that be in a different file? I feel like this is relevant enough to this feature that I would want it all contained in this single file. I can put it in a different section in the file if you think that would be better
There was a problem hiding this comment.
no, just after other supported protocols section
| {{< call-out "note" >}} | ||
|
|
||
| Kubernetes is generally not recommended for hostile multi-tenant environments and NGINX is designed to treat upstreams as trusted. If you need a dataplane that doesn't treat upstreams as trusted, you may want to explore alternative solutions. | ||
|
|
||
| {{< /call-out >}} |
There was a problem hiding this comment.
added this note here for the request by the user @salonichf5, do you have any thoughts? Should this even go in our docs here or maybe should it just live in our github docs somewhere? I don't really feel the need of highlighting this statement, and am on the border of if its necessary at all and might do more harm than good.
There was a problem hiding this comment.
can we just remove this part "If you need a dataplane that doesn't treat upstreams as trusted, you may want to explore alternative solutions."
we shouldn't be recommending that I feel. The short callout is fine by me.
2 participants
Proposed changes
Add document on supporting http2 to upstream through the appProtocol field on a Service.
Checklist
Before sharing this pull request, I completed the following checklist:
Footnotes
Potentially sensitive information includes personally identify information (PII), authentication credentials, and live URLs. Refer to the style guide for guidance about placeholder content. ↩