grpc: fix plan9 build by moving errno matching to a !plan9 file - #9255
Conversation
|
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #9255 +/- ##
=======================================
Coverage 83.23% 83.23%
=======================================
Files 421 421
Lines 34086 34107 +21
=======================================
+ Hits 28370 28390 +20
+ Misses 4281 4274 -7
- Partials 1435 1443 +8
🚀 New features to boost your workflow:
|
|
Thanks for fixing this @Yusufihsangorgel - this is stopping us upgrading rclone to 1.82.1 to pick up the security fix. |
|
Thanks Nick, glad it unblocks the rclone upgrade. The change passes all the test and build checks; the only red is the Validate PR label gate, which is waiting on a grpc-go maintainer to add a |
There was a problem hiding this comment.
Nit: Can we name this file clientconn_disconnect_reason.go and similarly for the file below. Thanks.
There was a problem hiding this comment.
We should use the _noplan9.go suffix to remain consistent with other OS-specific code in the repository. Note that, practically speaking, _noOS.go suffixes have no effect on compilation.
|
@mbissa For second set of eyes We would also need this to be cherry-picked before we push the 1.83 release. |
There was a problem hiding this comment.
We should use the _noplan9.go suffix to remain consistent with other OS-specific code in the repository. Note that, practically speaking, _noOS.go suffixes have no effect on compilation.
|
Done. Renamed both files to |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors the error labeling logic in clientconn.go by extracting platform-specific error handling into separate files (clientconn_disconnect_reason_noplan9.go and clientconn_disconnect_reason_plan9.go). This addresses the absence of syscall.Errno on the plan9 platform. The feedback suggests adding an explicit //go:build plan9 build tag to the plan9-specific file for consistency and compatibility.
arjan-bal
left a comment
There was a problem hiding this comment.
LGTM, thanks for the contribution!
…#9255) Fixes grpc#9253 `disconnectErrorString` (added for gRFC A94 in grpc#8973) references `syscall.Errno`, `syscall.ECONNRESET` and `syscall.ECONNABORTED`, none of which exist on plan9, so `GOOS=plan9 go build` fails on every release since v1.81.0. This moves the error-to-label classification into `disconnectErrorLabel` with two implementations: a `!plan9` file with the existing errno matching, unchanged in behavior, and a plan9 file that keeps the portable classifications (`subchannel shutdown`, `connection timed out`, `unknown`). Only plan9 is excluded so the errno granularity on js/wasm and wasip1, where `syscall.Errno` exists, stays as it is today. Verified `GOOS=plan9 GOARCH=amd64 go build ./...` fails on master and passes with this change, and `go build` plus the root package tests still pass on linux and darwin. RELEASE NOTES: * grpc: fix compilation on plan9, broken since v1.81.0.
…(#83) This PR contains the following updates: | Package | Change | [Age](https://docs.renovatebot.com/merge-confidence/) | [Confidence](https://docs.renovatebot.com/merge-confidence/) | |---|---|---|---| | [google.golang.org/grpc](https://github.com/grpc/grpc-go) | `v1.82.0` → `v1.83.1` |  |  | --- ### Release Notes <details> <summary>grpc/grpc-go (google.golang.org/grpc)</summary> ### [`v1.83.1`](https://github.com/grpc/grpc-go/releases/tag/v1.83.1): Release 1.83.1 [Compare Source](grpc/grpc-go@v1.83.0...v1.83.1) ### Security - xds/rbac: Fix a bug where nested `Principal` or `Permission` rules with `:scheme` or `grpc-` prefixed header matchers were not rejected, which could cause DENY rules to fail open. ([#​9258](grpc/grpc-go#9258)) - Special Thanks: [@​nvxbug](https://github.com/nvxbug) - xds/rbac: Fix a bug where the `host` header matcher was not being replaced with `:authority` in nested `Principal` or `Permission` rules. ([#​9258](grpc/grpc-go#9258)) - Special Thanks: [@​nvxbug](https://github.com/nvxbug) - xds/rbac: Fix a bug where a header matcher whose name was not lowercase, such as `X-Role`, matched no header, which could cause DENY rules to fail open. ([#​9332](grpc/grpc-go#9332)) - Special Thanks: [@​alimony](https://github.com/alimony) - xds/rbac: Fix a bug where a `:scheme` or `grpc-` prefixed header matcher was accepted when its name was not lowercase. ([#​9332](grpc/grpc-go#9332)) - Special Thanks: [@​alimony](https://github.com/alimony) - xds/rbac: Fix a bug where a `Host` header matcher was not replaced with `:authority`. ([#​9332](grpc/grpc-go#9332)) - Special Thanks: [@​alimony](https://github.com/alimony) ### Performance - transport: Restrict memory overhead of buffering small data frames. ([#​9331](grpc/grpc-go#9331)) ### [`v1.83.0`](https://github.com/grpc/grpc-go/releases/tag/v1.83.0): Release 1.83.0 [Compare Source](grpc/grpc-go@v1.82.1...v1.83.0) ### Security - server: Stop reading from connections when flooded by HTTP/2 frames to mitigate resource exhaustion. The default value for this limit is 100 frames, excluding DATA and HEADERS, and may be changed by setting environment variable `GRPC_GO_EXPERIMENTAL_CONTROL_BUFFER_THROTTLE_LIMIT`. - xds/rbac: Support `Metadata` and `RequestedServerName` permissions matcher fields. If present in a DENY rule, previously these would be ignored and fail-open. - xds/rbac: Fix panic when parsing unsupported fields in `NotRule`/`NotId` permissions. - xds/rbac: Support the deprecated `source_ip` principal identifier by treating it as equivalent to `direct_remote_ip`. - xds: Fix panic when parsing route header matchers configured with empty `exact_match`, `prefix_match`, or `suffix_match` strings. ([#​9223](grpc/grpc-go#9223)) ### New Features - xds/googlec2p: Enable DirectPath over Interconnect support for on-premises clients via the `force-xds` target URI query parameter. ([#​9133](grpc/grpc-go#9133)) - xds: Enable xDS configuration to control which fields get propagated from ORCA backend metric reports to LRS load reports. ([#​9145](grpc/grpc-go#9145)) - authz: Add `OnPolicyUpdate` callback to `FileWatcherOptions` to notify when an authz policy is loaded or updated. ([#​9142](grpc/grpc-go#9142)) - Special Thanks: [@​hnefatl](https://github.com/hnefatl) - xds: Add support for the GCP Authentication HTTP Filter, which automatically fetches and attaches GCP Service Account Identity JWT tokens to outgoing RPCs. - This feature can be enabled by setting environment variable `GRPC_EXPERIMENTAL_XDS_GCP_AUTHENTICATION_FILTER=true`. ([#​9119](grpc/grpc-go#9119)) - xds: Add support for xDS-based HTTP CONNECT proxies. - This feature can be enabled by setting environment variable `GRPC_EXPERIMENTAL_XDS_HTTP_CONNECT=true`. ([#​9151](grpc/grpc-go#9151)) - xds: Add support for `contains_match` in route header matchers. ([#​9223](grpc/grpc-go#9223)) ### Bug Fixes - credentials/alts: Fix panic when processing malformed frames by validating that the message frame length exceeds the message type field size. ([#​9197](grpc/grpc-go#9197)) - grpc: Fix compilation on Plan 9 targets (`GOOS=plan9`), broken since v1.81.0. ([#​9255](grpc/grpc-go#9255)) - Special Thanks: [@​Yusufihsangorgel](https://github.com/Yusufihsangorgel) ### [`v1.82.1`](https://github.com/grpc/grpc-go/releases/tag/v1.82.1): Release 1.82.1 [Compare Source](grpc/grpc-go@v1.82.0...v1.82.1) ### Security - server: Stop reading from the connection when flooded by HTTP/2 frames. The default value for this limit is 100 frames, excluding DATA and HEADERS, and may be changed by setting environment variable `GRPC_GO_EXPERIMENTAL_CONTROL_BUFFER_THROTTLE_LIMIT`. - xds/rbac: Support `Metadata` and `RequestedServerName` permissions matcher fields. If present in a DENY rule, previously these would be ignored and fail-open. - xds/rbac: Fix panic when parsing unsupported fields in `NotRule`/`NotId` permissions. - xds/rbac: Support the deprecated `source_ip` principal identifier by treating it as equivalent to `direct_remote_ip`. </details> --- ### Configuration 📅 **Schedule**: (in timezone Europe/Paris) - Branch creation - At any time (no schedule defined) - Automerge - At any time (no schedule defined) 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **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 CLI](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My4xMDEuMSIsInVwZGF0ZWRJblZlciI6IjQ0LjMxLjAiLCJ0YXJnZXRCcmFuY2giOiJtYWluIiwibGFiZWxzIjpbInR5cGUvbWlub3IiXX0=--> Reviewed-on: https://git.erwanleboucher.dev/eleboucher/runner-k8s-plugin/pulls/83
Fixes #9253
disconnectErrorString(added for gRFC A94 in #8973) referencessyscall.Errno,syscall.ECONNRESETandsyscall.ECONNABORTED, none of which exist on plan9, soGOOS=plan9 go buildfails on every release since v1.81.0.This moves the error-to-label classification into
disconnectErrorLabelwith two implementations: a!plan9file with the existing errno matching, unchanged in behavior, and a plan9 file that keeps the portable classifications (subchannel shutdown,connection timed out,unknown). Only plan9 is excluded so the errno granularity on js/wasm and wasip1, wheresyscall.Errnoexists, stays as it is today.Verified
GOOS=plan9 GOARCH=amd64 go build ./...fails on master and passes with this change, andgo buildplus the root package tests still pass on linux and darwin.RELEASE NOTES: