-
Notifications
You must be signed in to change notification settings - Fork 112
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
Add support for Connect-Protocol-Version header #416
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
timostamm
approved these changes
Dec 8, 2022
pkwarren
reviewed
Dec 8, 2022
buildbreaker
approved these changes
Dec 8, 2022
pkwarren
approved these changes
Dec 8, 2022
renovate bot
referenced
this pull request
in open-feature/flagd
Jan 12, 2023
[](https://renovatebot.com) This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [github.com/bufbuild/connect-go](https://togithub.com/bufbuild/connect-go) | require | minor | `v1.3.1` -> `v1.4.1` | --- ### Release Notes <details> <summary>bufbuild/connect-go</summary> ### [`v1.4.1`](https://togithub.com/bufbuild/connect-go/releases/tag/v1.4.1) [Compare Source](https://togithub.com/bufbuild/connect-go/compare/v1.4.0...v1.4.1) <!-- Release notes generated using configuration in .github/release.yml at main --> #### What's Changed ##### Bugfixes - Ensure server reflection can always access protobuf descriptors by [@​joshcarp](https://togithub.com/joshcarp) in [https://github.com/bufbuild/connect-go/pull/418](https://togithub.com/bufbuild/connect-go/pull/418) - Don't clobber custom User-Agents by [@​akshayjshah](https://togithub.com/akshayjshah) in [https://github.com/bufbuild/connect-go/pull/423](https://togithub.com/bufbuild/connect-go/pull/423) **Full Changelog**: bufbuild/connect-go@v1.4.0...v1.4.1 ### [`v1.4.0`](https://togithub.com/bufbuild/connect-go/releases/tag/v1.4.0) [Compare Source](https://togithub.com/bufbuild/connect-go/compare/v1.3.2...v1.4.0) <!-- Release notes generated using configuration in .github/release.yml at main --> #### What's Changed ##### Enhancements - Improve panic message on invalid handler returns by [@​akshayjshah](https://togithub.com/akshayjshah) in [https://github.com/bufbuild/connect-go/pull/415](https://togithub.com/bufbuild/connect-go/pull/415) - Add support for Connect-Protocol-Version header by [@​akshayjshah](https://togithub.com/akshayjshah) in [https://github.com/bufbuild/connect-go/pull/416](https://togithub.com/bufbuild/connect-go/pull/416) #### Migration Notes As a consequence of [https://github.com/bufbuild/connect-go/pull/416](https://togithub.com/bufbuild/connect-go/pull/416), `connect-go` servers exposed to web browsers may need to amend their CORS configuration to add `Connect-Protocol-Version` to `Access-Control-Allow-Headers`. The [pull request description](https://togithub.com/bufbuild/connect-go/pull/416) explains the motivation for this additional header. **Full Changelog**: bufbuild/connect-go@v1.3.2...v1.4.0 ### [`v1.3.2`](https://togithub.com/bufbuild/connect-go/releases/tag/v1.3.2) [Compare Source](https://togithub.com/bufbuild/connect-go/compare/v1.3.1...v1.3.2) <!-- Release notes generated using configuration in .github/release.yml at main --> #### What's Changed ##### Bugfixes - Send gRPC error metadata only as HTTP trailers by [@​akshayjshah](https://togithub.com/akshayjshah) in [https://github.com/bufbuild/connect-go/pull/410](https://togithub.com/bufbuild/connect-go/pull/410) **Full Changelog**: bufbuild/connect-go@v1.3.1...v1.3.2 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://app.renovatebot.com/dashboard#github/open-feature/flagd). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNC45Ny42IiwidXBkYXRlZEluVmVyIjoiMzQuOTcuNiJ9--> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
akshayjshah
added a commit
that referenced
this pull request
Jul 26, 2023
Currently, it's difficult for proxies, net/http middleware, and other "in-between" code to distinguish unary Connect RPC requests from other HTTP traffic (especially with JSON payloads, which use the common `application/json` Content-Type). To work around this, any in-between code today must check whether the HTTP path matches a known RPC method, which requires intimate knowledge of the service schema. This isn't great. To work around this limitation, we've added an optional header to the specification for Connect RPC requests: `Connect-Protocol-Version`. Generated clients always send this header, so the vast majority of traffic should include it. Servers _may_ require that requests include this header, which helps proxies and net/http middleware identify _every_ Connect request. This lets in-between code function more reliably: for example, it could help a metrics-collecting reverse proxy produce higher-fidelity statistics, since it wouldn't miss even the handful of RPCs made with ad-hoc cURL commands. However, it makes ad-hoc debugging with cURL or fetch slightly more laborious. This PR makes clients using the Connect protocol send the `Connect-Protocol-Version` header, and it allows servers to opt into strict validation by using the `WithRequireConnectProtocolHeader` option. It makes no changes to the behavior of the gRPC or gRPC-Web protocols. Note that servers exposing Connect RPCs to web browsers may need to update their CORS configuration to allow the `Connect-Protocol-Version` header.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently, it's difficult for proxies, net/http middleware, and other
"in-between" code to distinguish unary Connect RPC requests from other HTTP
traffic (especially with JSON payloads, which use the common
application/json
Content-Type). To work around this, any in-between code today must check
whether the HTTP path matches a known RPC method, which requires intimate
knowledge of the service schema. This isn't great.
To work around this limitation, we've added an optional header to the
specification for Connect RPC requests:
Connect-Protocol-Version
. Generatedclients always send this header, so the vast majority of traffic should include
it. Servers may require that requests include this header, which helps
proxies and net/http middleware identify every Connect request. This lets
in-between code function more reliably: for example, it could help a
metrics-collecting reverse proxy produce higher-fidelity statistics, since it
wouldn't miss even the handful of RPCs made with ad-hoc cURL commands. However,
it makes ad-hoc debugging with cURL or fetch slightly more laborious.
This PR makes clients using the Connect protocol send the
Connect-Protocol-Version
header, and it allows servers to opt into strictvalidation by using the
WithRequireConnectProtocolHeader
option. It makes nochanges to the behavior of the gRPC or gRPC-Web protocols.
Note that servers exposing Connect RPCs to web browsers may need to update
their CORS configuration to allow the
Connect-Protocol-Version
header.