CNTRLPLANE-3380: Merge https://github.com/kubernetes-sigs/apiserver-network-proxy:master (588e629) into main#109
Merged
openshift-merge-bot[bot] merged 58 commits intoMay 20, 2026
Conversation
Signed-off-by: Imran Pochi <imranpochi@microsoft.com>
Introduce a `sync.Pool` to reuse read buffers in the `ServeHTTP` handler for the http-connect mode. Previously, a new 32KB buffer was allocated for every hijacked connection, causing high GC pressure in high-throughput scenarios. By pooling and reusing these buffers, we significantly reduce the number of memory allocations, which improves CPU usage and lowers request latency by decreasing the impact of garbage collection. Signed-off-by: Imran Pochi <imranpochi@microsoft.com>
Signed-off-by: Imran Pochi <imranpochi@microsoft.com>
Bumps [golangci/golangci-lint-action](https://github.com/golangci/golangci-lint-action) from 8 to 9. - [Release notes](https://github.com/golangci/golangci-lint-action/releases) - [Commits](golangci/golangci-lint-action@v8...v9) --- updated-dependencies: - dependency-name: golangci/golangci-lint-action dependency-version: '9' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>
Signed-off-by: Imran Pochi <imranpochi@microsoft.com>
if leaseController is enabled and graceful shutdown is enabled as well, without this lease controller will terminate while the servers would continue to shutdown gracefully. Potentially causing a situtaion where the leases will be deleted while the existing connections are still served upto shutdown point. Signed-off-by: Imran Pochi <imranpochi@microsoft.com>
Bumps [actions/checkout](https://github.com/actions/checkout) from 5 to 6. - [Release notes](https://github.com/actions/checkout/releases) - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md) - [Commits](actions/checkout@v5...v6) --- updated-dependencies: - dependency-name: actions/checkout dependency-version: '6' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>
Signed-off-by: Imran Pochi <imranpochi@microsoft.com>
Replace 'intialized' with 'initialized' in log message
Bumps [actions/upload-artifact](https://github.com/actions/upload-artifact) from 4 to 6. - [Release notes](https://github.com/actions/upload-artifact/releases) - [Commits](actions/upload-artifact@v4...v6) --- updated-dependencies: - dependency-name: actions/upload-artifact dependency-version: '6' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [actions/download-artifact](https://github.com/actions/download-artifact) from 5 to 7. - [Release notes](https://github.com/actions/download-artifact/releases) - [Commits](actions/download-artifact@v5...v7) --- updated-dependencies: - dependency-name: actions/download-artifact dependency-version: '7' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>
…t/github_actions/actions/upload-artifact-6 chore(deps): bump actions/upload-artifact from 4 to 6
…for-http-connect http-connect: reduce memory allocations
…t/github_actions/golangci/golangci-lint-action-9 chore(deps): bump golangci/golangci-lint-action from 8 to 9
…t/github_actions/actions/checkout-6 chore(deps): bump actions/checkout from 5 to 6
…t/github_actions/actions/download-artifact-7 chore(deps): bump actions/download-artifact from 5 to 7
…rmination-for-server Graceful termination for server
…ajor fix: add missing ProtoMajor in response
Signed-off-by: Imran Pochi <imranpochi@microsoft.com>
This commit adds a fallback in the case where all the agents in system are draining. Rather than drop the request with error, we fallback to the existing behavior i.e continue to the send the request to the agent even if its draining. As for the agent side issue, if the agent has sent the DRAIN signal to the server, ideally it should stop doing the syncOnce with the server. This mistakes the server the agent is back ready. Signed-off-by: Imran Pochi <imranpochi@microsoft.com>
Signed-off-by: Imran Pochi <imranpochi@microsoft.com>
Bumps [actions/download-artifact](https://github.com/actions/download-artifact) from 7 to 8. - [Release notes](https://github.com/actions/download-artifact/releases) - [Commits](actions/download-artifact@v7...v8) --- updated-dependencies: - dependency-name: actions/download-artifact dependency-version: '8' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [actions/upload-artifact](https://github.com/actions/upload-artifact) from 6 to 7. - [Release notes](https://github.com/actions/upload-artifact/releases) - [Commits](actions/upload-artifact@v6...v7) --- updated-dependencies: - dependency-name: actions/upload-artifact dependency-version: '7' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>
…erver-agent-draining implement server agent draining
…t/github_actions/actions/upload-artifact-7 chore(deps): bump actions/upload-artifact from 6 to 7
…t/github_actions/actions/download-artifact-8 chore(deps): bump actions/download-artifact from 7 to 8
prompt: Please upgrade the go.mod in this directory to match https://github.com/kubernetes/kubernetes/blob/release-1.33/go.mod Added agent.md to help correct ML based upgrades.
Adding full sha to git commands.
Updated konnectivity-client dependencies
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR introduces graceful shutdown timeout support for the proxy server, updates Go to 1.25.0 with dependency refreshes, hardens GitHub Actions workflows through commit SHA pinning, upgrades Kubernetes test versions, and updates the konnectivity-client module independently. ChangesGraceful Shutdown, Build Infrastructure, and CI/CD Updates
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 11 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (11 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
openshift-ci
Bot
added
the
approved
bryan-cox
force-pushed
the
rebase-bot-main
branch
from
ca86bde to
1836689
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmd/server/app/server.go`:
- Around line 191-210: The frontendStop functions currently only perform
graceful shutdown (e.g., grpc.Server.GracefulStop() and http.Server.Shutdown())
which can block indefinitely; update every frontend shutdown implementation
referenced by frontendStop (apply to both UDS and mTLS frontend branches) to
enforce a hard stop after the graceful timeout: run the graceful shutdown on a
context with the configured timeout, and if it doesn't complete, escalate to the
forced stop path (call grpc.Server.Stop() for gRPC frontends and
http.Server.Close() or close the listener for HTTP frontends) so shutdown always
completes even when GracefulShutdownTimeout==0 or the graceful path stalls;
ensure these forced-stop calls are invoked alongside existing teardown for
p.agentServer, p.adminServer, p.healthServer and leaseController logic.
In `@konnectivity-client/agent.md`:
- Around line 1-6: The guidance text is overly broad and self-contradictory;
replace the final sentence "Please do not make changes in
apiserver-network-proxy and anywhere else in apiserver-network-proxy." with a
scoped restriction such as "Do not make changes to the
apiserver-network-proxy/konnectivity-client subtree without prior approval." and
tighten lines 1–4 to state that dependencies must remain compatible with the
oldest supported Kubernetes release (include the kubernetes.io/releases link)
and that the primary consumer is the kubernetes-apiserver; ensure the wording
clearly bounds the maintenance scope to the konnectivity-client subtree and
removes the blanket ban on the whole repo.
In `@konnectivity-client/go.mod`:
- Line 11: Update the grpc module in go.mod from google.golang.org/grpc v1.68.1
to v1.79.3 (or later) to remediate the authorization bypass; edit the module
line referencing google.golang.org/grpc to the new version and then run `go mod
tidy` / `go get` to refresh module locks and update go.sum so the dependency
graph and checksums are consistent with v1.79.3+.
In `@Makefile`:
- Around line 87-90: The TEST_LIST derivation is filtering out most packages by
relying on bracketed output from "go list -test"; update TEST_LIST to mirror the
fast-test approach: use "go list ./..." (instead of "go list -test ./... | egrep
' \\[.*\\]' | cut -d' ' -f1") and then apply the same e2e exclusion (grep -v -e
"/e2e$$" -e "/e2e/.*") so TEST_LIST includes all non-e2e packages; keep the
existing echo and go test invocation that uses $(TEST_LIST).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 329a6efa-dd60-4f77-b4ea-d3389c75bfd0
⛔ Files ignored due to path filters (269)
go.sumis excluded by!**/*.sumkonnectivity-client/go.sumis excluded by!**/*.sumvendor/github.com/go-logr/logr/.golangci.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/moby/spdystream/NOTICEis excluded by!**/vendor/**,!vendor/**vendor/github.com/moby/spdystream/connection.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/moby/spdystream/spdy/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/moby/spdystream/spdy/PATENTSis excluded by!**/vendor/**,!vendor/**vendor/github.com/moby/spdystream/spdy/dictionary.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/moby/spdystream/spdy/options.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/moby/spdystream/spdy/read.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/moby/spdystream/spdy/types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/moby/spdystream/spdy/write.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/stretchr/testify/assert/assertion_compare.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/stretchr/testify/assert/assertion_format.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/stretchr/testify/assert/assertion_forward.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/stretchr/testify/assert/assertion_order.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/stretchr/testify/assert/assertions.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/stretchr/testify/assert/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/stretchr/testify/assert/http_assertions.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/stretchr/testify/assert/yaml/yaml_custom.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/stretchr/testify/assert/yaml/yaml_default.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/stretchr/testify/assert/yaml/yaml_fail.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/attribute/encoder.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/attribute/filter.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/attribute/hash.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/attribute/internal/attribute.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/attribute/internal/xxhash/xxhash.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/attribute/iterator.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/attribute/key.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/attribute/kv.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/attribute/rawhelpers.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/attribute/set.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/attribute/type_string.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/attribute/value.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/codes/codes.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/internal/gen.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/internal/rawhelpers.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/semconv/v1.26.0/README.mdis excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/semconv/v1.26.0/attribute_group.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/semconv/v1.26.0/metric.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/semconv/v1.39.0/MIGRATION.mdis excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/semconv/v1.39.0/README.mdis excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/semconv/v1.39.0/attribute_group.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/semconv/v1.39.0/doc.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/semconv/v1.39.0/error_type.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/semconv/v1.39.0/exception.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/semconv/v1.39.0/schema.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/trace/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/trace/auto.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/trace/config.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/trace/hex.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/trace/internal/telemetry/attr.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/trace/internal/telemetry/id.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/trace/internal/telemetry/span.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/trace/internal/telemetry/status.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/trace/internal/telemetry/traces.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/trace/internal/telemetry/value.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/trace/noop.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/trace/span.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/trace/trace.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/trace/tracestate.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/html/escape.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/html/iter.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/html/node.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/html/nodetype_string.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/html/parse.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/html/render.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/http2/client_priority_go126.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/http2/client_priority_go127.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/http2/config.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/http2/config_go124.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/http2/config_go125.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/http2/config_go126.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/http2/config_pre_go124.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/http2/frame.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/http2/gotrack.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/http2/hpack/tables.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/http2/http2.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/http2/server.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/http2/timer.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/http2/transport.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/http2/writesched.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/http2/writesched_priority_rfc7540.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/http2/writesched_priority_rfc9218.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/http2/writesched_random.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/http2/writesched_roundrobin.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/internal/httpcommon/request.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/internal/httpsfv/httpsfv.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/internal/socks/socks.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/trace/events.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/websocket/hybi.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/oauth2/deviceauth.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/oauth2/oauth2.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/oauth2/pkce.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/oauth2/token.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/oauth2/transport.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/plan9/pwd_go15_plan9.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/plan9/pwd_plan9.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/plan9/syscall_plan9.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/affinity_linux.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/fdset.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/ifreq_linux.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/ioctl_signed.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/ioctl_unsigned.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/mkall.shis excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/mkerrors.shis excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/syscall_darwin.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/syscall_linux.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/syscall_netbsd.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/syscall_solaris.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/syscall_unix.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zerrors_linux.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zerrors_linux_386.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zerrors_linux_amd64.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zerrors_linux_arm.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zerrors_linux_arm64.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zerrors_linux_loong64.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zerrors_linux_mips.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zerrors_linux_mips64.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zerrors_linux_mips64le.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zerrors_linux_mipsle.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zerrors_linux_ppc.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zerrors_linux_ppc64.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zerrors_linux_ppc64le.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zerrors_linux_riscv64.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zerrors_linux_s390x.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zerrors_linux_sparc64.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zsyscall_linux.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zsyscall_solaris_amd64.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zsysnum_linux_386.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zsysnum_linux_amd64.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zsysnum_linux_arm.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zsysnum_linux_arm64.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zsysnum_linux_loong64.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zsysnum_linux_mips.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zsysnum_linux_mips64.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zsysnum_linux_mips64le.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zsysnum_linux_mipsle.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zsysnum_linux_ppc.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zsysnum_linux_ppc64.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zsysnum_linux_ppc64le.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zsysnum_linux_riscv64.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zsysnum_linux_s390x.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/zsysnum_linux_sparc64.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/ztypes_linux.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/ztypes_linux_386.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/ztypes_linux_amd64.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/ztypes_linux_arm.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/ztypes_linux_arm64.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/ztypes_linux_loong64.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/ztypes_linux_mips.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/ztypes_linux_mips64.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/ztypes_linux_mips64le.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/ztypes_linux_mipsle.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/ztypes_linux_ppc.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/ztypes_linux_ppc64.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/ztypes_linux_ppc64le.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/ztypes_linux_riscv64.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/ztypes_linux_s390x.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/ztypes_linux_sparc64.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/ztypes_netbsd_arm.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/windows/aliases.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/windows/dll_windows.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/windows/security_windows.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/windows/syscall_windows.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/windows/types_windows.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/windows/zsyscall_windows.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/term/term_windows.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/term/terminal.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/secure/bidirule/bidirule.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/secure/bidirule/bidirule10.0.0.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/secure/bidirule/bidirule9.0.0.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/unicode/bidi/core.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/unicode/bidi/tables10.0.0.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/unicode/bidi/tables11.0.0.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/unicode/bidi/tables12.0.0.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/unicode/bidi/tables13.0.0.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/unicode/bidi/tables15.0.0.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/unicode/bidi/tables17.0.0.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/unicode/bidi/tables9.0.0.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/unicode/norm/forminfo.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/unicode/norm/tables10.0.0.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/unicode/norm/tables11.0.0.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/unicode/norm/tables12.0.0.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/unicode/norm/tables15.0.0.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/unicode/norm/tables17.0.0.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/unicode/norm/tables9.0.0.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/genproto/googleapis/rpc/status/status.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/CONTRIBUTING.mdis excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/MAINTAINERS.mdis excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/README.mdis excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/attributes/attributes.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/balancer/balancer.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/balancer/base/balancer.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/balancer/endpointsharding/endpointsharding.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/balancer/pickfirst/internal/internal.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/balancer/pickfirst/pickfirst.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/balancer/pickfirst/pickfirstleaf/pickfirstleaf.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/balancer/roundrobin/roundrobin.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/balancer/subconn.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/balancer_wrapper.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/binarylog/grpc_binarylog_v1/binarylog.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/clientconn.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/credentials/credentials.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/credentials/insecure/insecure.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/credentials/tls.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/dialoptions.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/encoding/encoding.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/encoding/internal/internal.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/encoding/proto/proto.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/experimental/stats/metricregistry.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/experimental/stats/metrics.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/interceptor.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/internal/balancer/gracefulswitch/gracefulswitch.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/internal/balancer/weight/weight.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/internal/buffer/unbounded.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/internal/channelz/trace.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/internal/credentials/credentials.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/internal/envconfig/envconfig.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/internal/envconfig/xds.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/internal/experimental.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/internal/grpcsync/callback_serializer.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/internal/grpcsync/event.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/internal/idle/idle.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/internal/internal.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/internal/mem/buffer_pool.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/internal/resolver/delegatingresolver/delegatingresolver.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/internal/resolver/dns/dns_resolver.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/internal/stats/metrics_recorder_list.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/internal/stats/stats.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/internal/status/status.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/internal/transport/client_stream.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/internal/transport/controlbuf.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/internal/transport/defaults.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/internal/transport/flowcontrol.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/internal/transport/handler_server.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/internal/transport/http2_client.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/internal/transport/http2_server.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/internal/transport/http_util.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/internal/transport/server_stream.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/internal/transport/transport.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/mem/buffer_pool.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/mem/buffer_slice.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/mem/buffers.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/picker_wrapper.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/preloader.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/resolver/map.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/resolver/resolver.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/resolver_wrapper.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/rpc_util.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/server.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/stats/handlers.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/stats/stats.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/stream.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/version.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/encoding/protowire/wire.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/internal/editiondefaults/editions_defaults.binpbis excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/internal/encoding/tag/tag.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/internal/encoding/text/decode.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/internal/filedesc/desc.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/internal/filedesc/desc_init.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/internal/filedesc/desc_lazy.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/internal/filedesc/editions.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/internal/filedesc/presence.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/internal/genid/api_gen.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/internal/genid/descriptor_gen.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/internal/impl/codec_map.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/internal/impl/codec_message_opaque.gois excluded by!**/vendor/**,!vendor/**
📒 Files selected for processing (31)
.github/dependabot.yml.github/workflows/e2e.yaml.github/workflows/golangci-lint.yamlMakefilecloudbuild.yamlcmd/server/app/options/options.gocmd/server/app/options/options_test.gocmd/server/app/server.gocmd/test-client/main.gogo.modkonnectivity-client/agent.mdkonnectivity-client/go.modpkg/agent/clientset.gopkg/agent/metrics/metrics.gopkg/server/backend_manager.gopkg/server/backend_manager_test.gopkg/server/metrics/metrics.gopkg/server/server.gopkg/server/tunnel.gopkg/testing/leases.gopkg/testing/metrics/metrics.gopkg/util/certificates.gopkg/util/flags.gopkg/util/handlers.gopkg/util/leases.gopkg/util/leases_test.gopkg/util/net.gopkg/util/net_test.gopkg/util/url.gopkg/util/util.gopkg/util/util_test.go
Comment on lines
+191
to
+210
| // If graceful shutdown timeout is 0, use the old behavior (immediate shutdown) | ||
| if o.GracefulShutdownTimeout == 0 { | ||
| if frontendStop != nil { | ||
| if err := frontendStop(context.Background()); err != nil { | ||
| klog.ErrorS(err, "failed to stop frontend server") | ||
| } | ||
| } | ||
| if p.agentServer != nil { | ||
| p.agentServer.Stop() | ||
| } | ||
| if p.adminServer != nil { | ||
| p.adminServer.Close() | ||
| } | ||
| if p.healthServer != nil { | ||
| p.healthServer.Close() | ||
| } | ||
| if leaseController != nil { | ||
| leaseController.Stop() | ||
| } | ||
| return nil |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Go, does grpc.Server.GracefulStop()honor a context or timeout, and whennet/http.Server.Shutdown(ctx)returns becausectxexpired, must the caller invokeClose() to force termination?
💡 Result:
In Go, grpc.Server.GracefulStop does not accept a context or timeout [1][2]. It blocks indefinitely until all pending RPCs have completed [1][3]. Because it does not honor a timeout, it is a standard and recommended practice to implement a "safety net" by running GracefulStop in a goroutine and using a select statement with a timer or context to trigger a forceful s.Stop if the graceful shutdown takes too long [3][4][5][6]. Regarding net/http.Server.Shutdown(ctx), when the provided context expires, the method returns immediately with the context's error (e.g., context.DeadlineExceeded) [7]. It does not automatically force-close active connections when the context expires [7]. Consequently, if you want to ensure the server terminates after the timeout, you must explicitly handle the cleanup, typically by calling Close to force the termination of any remaining connections [7][8].
Citations:
- 1: https://pkg.go.dev/google.golang.org/grpc/examples/features/gracefulstop
- 2: https://grpc.io/docs/guides/server-graceful-stop/
- 3: https://github.com/grpc/grpc-go/blob/49e224f832c21df2cd3fe833189c8a50244f0eee/examples/features/gracefulstop/server/main.go
- 4: https://gist.github.com/skwair/c67d328e1040f638a07bf841be94fd11
- 5: Health Server Graceful Shutdown grpc/grpc-go#8776
- 6: https://stackoverflow.com/questions/58480775/how-to-close-all-grpc-server-streams-using-gracefulstop
- 7: https://dev.to/gabrielanhaia/gos-nethttp-servershutdown-the-three-stage-drain-you-probably-skip-1dgl
- 8: proposal: net/http: context based graceful server shutdown golang/go#52805
Add forced-stop paths to all frontend shutdown implementations.
frontendStop is graceful-only in every implementation. With the default GracefulShutdownTimeout=0, the zero-timeout branch calls it with context.Background(), allowing active frontend connections to block shutdown indefinitely. In the timed path, the timeout never escalates the frontend to a hard stop, despite the caller's assumption. grpc.Server.GracefulStop() blocks indefinitely without context awareness, and net/http.Server.Shutdown() doesn't force-close connections after timeout expiry—both require explicit escalation paths.
Implementation pattern
For gRPC (UDS and mTLS):
- stop = func(_ context.Context) error {
- grpcServer.GracefulStop()
- return nil
- }
+ stop = func(shutdownCtx context.Context) error {
+ done := make(chan struct{})
+ go func() {
+ grpcServer.GracefulStop()
+ close(done)
+ }()
+ select {
+ case <-done:
+ return nil
+ case <-shutdownCtx.Done():
+ grpcServer.Stop()
+ <-done
+ return shutdownCtx.Err()
+ }
+ }For HTTP (UDS and mTLS):
- stop = func(shutdownCtx context.Context) error {
- return server.Shutdown(shutdownCtx)
- }
+ stop = func(shutdownCtx context.Context) error {
+ if err := server.Shutdown(shutdownCtx); err != nil {
+ if shutdownCtx.Err() != nil {
+ _ = server.Close()
+ }
+ return err
+ }
+ return nil
+ }Apply to both UDS and mTLS frontend branches (lines 282-295, 369-383, 467-483).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cmd/server/app/server.go` around lines 191 - 210, The frontendStop functions
currently only perform graceful shutdown (e.g., grpc.Server.GracefulStop() and
http.Server.Shutdown()) which can block indefinitely; update every frontend
shutdown implementation referenced by frontendStop (apply to both UDS and mTLS
frontend branches) to enforce a hard stop after the graceful timeout: run the
graceful shutdown on a context with the configured timeout, and if it doesn't
complete, escalate to the forced stop path (call grpc.Server.Stop() for gRPC
frontends and http.Server.Close() or close the listener for HTTP frontends) so
shutdown always completes even when GracefulShutdownTimeout==0 or the graceful
path stalls; ensure these forced-stop calls are invoked alongside existing
teardown for p.agentServer, p.adminServer, p.healthServer and leaseController
logic.
Comment on lines
+1
to
+6
| This is the client library meant to be imported by users apiserver-network-proxy. | ||
| The primary user of this library is the kubernetes-apiserver from https://github.com/kubernetes/kubernetes. | ||
| The dependencies for this library should be kept in line with the old supported version of Kubernetes. | ||
| The oldest supported version of kubernetes can be found at https://kubernetes.io/releases/ | ||
|
|
||
| Please do not make changes in apiserver-network-proxy/konnectivity-client and anywhere else in apiserver-network-proxy. |
There was a problem hiding this comment.
The maintenance guidance is overbroad and contradictory.
Line 6 says not to change konnectivity-client and anywhere else in apiserver-network-proxy, which effectively forbids all repo changes. Please narrow this to the intended scope (likely this subtree only) and tighten wording in Lines 1-4.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@konnectivity-client/agent.md` around lines 1 - 6, The guidance text is overly
broad and self-contradictory; replace the final sentence "Please do not make
changes in apiserver-network-proxy and anywhere else in
apiserver-network-proxy." with a scoped restriction such as "Do not make changes
to the apiserver-network-proxy/konnectivity-client subtree without prior
approval." and tighten lines 1–4 to state that dependencies must remain
compatible with the oldest supported Kubernetes release (include the
kubernetes.io/releases link) and that the primary consumer is the
kubernetes-apiserver; ensure the wording clearly bounds the maintenance scope to
the konnectivity-client subtree and removes the blanket ban on the whole repo.
| github.com/prometheus/client_golang v1.22.0 | ||
| go.uber.org/goleak v1.3.0 | ||
| golang.org/x/net v0.38.0 // indirect | ||
| google.golang.org/grpc v1.68.1 |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What is the first patched version of google.golang.org/grpc for GO-2026-4762 (GHSA-p77j-4mvh-x3m3)?
💡 Result:
The first patched version of google.golang.org/grpc for the vulnerability GO-2026-4762 (also tracked as GHSA-p77j-4mvh-x3m3 and CVE-2026-33186) is v1.79.3 [1][2][3]. Users are advised to upgrade to this version or newer to address the authorization bypass issue caused by missing leading slashes in the:path pseudo-header [2][4].
Citations:
- 1: https://pkg.go.dev/vuln/GO-2026-4762
- 2: https://test.osv.dev/vulnerability/GHSA-p77j-4mvh-x3m3
- 3: https://test.osv.dev/vulnerability/GO-2026-4762
- 4: https://cve.circl.lu/vuln/ghsa-p77j-4mvh-x3m3
Upgrade gRPC to v1.79.3 or later immediately.
Line 11 references google.golang.org/grpc v1.68.1, which is vulnerable to a critical authorization bypass (GO-2026-4762 / GHSA-p77j-4mvh-x3m3) caused by missing leading slashes in the:path pseudo-header. Bump to v1.79.3 or newer and refresh module locks.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@konnectivity-client/go.mod` at line 11, Update the grpc module in go.mod from
google.golang.org/grpc v1.68.1 to v1.79.3 (or later) to remediate the
authorization bypass; edit the module line referencing google.golang.org/grpc to
the new version and then run `go mod tidy` / `go get` to refresh module locks
and update go.sum so the dependency graph and checksums are consistent with
v1.79.3+.
Comment on lines
+87
to
+90
| $(eval TEST_LIST := $(shell go list -test ./... | egrep " \[.*\]" | cut -d' ' -f1 | grep -v -e "/e2e$$" -e "/e2e/.*")) | ||
| echo "Running tests on $(TEST_LIST)" | ||
| $(info GOTOOLCHAIN is $(GOTOOLCHAIN)) | ||
| go test -v -mod=vendor -race -covermode=atomic -coverprofile=konnectivity.out $(TEST_LIST) && go tool cover -html=konnectivity.out -o=konnectivity.html |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What is the default output format of go list -test ./...? Does it include bracketed entries like pkg [pkg.test]without using-deps?
💡 Result:
The default output format of the go list command is the package import path, which is equivalent to using the format flag -f '{{.ImportPath}}' [1][2][3]. When you use the -test flag with go list, the command reports not only the named packages but also their associated test binaries [1]. The output will include these test binaries, which are identified by the import path of the package followed by a.test suffix (e.g., math/rand.test) [1]. Regarding your question about bracketed entries like pkg [pkg.test]: Yes, go list -test will include these entries even without the -deps flag [1]. When building a test, the Go toolchain may need to recompile certain dependencies specifically for that test binary [1]. In these cases, the reported import path of the recompiled package is followed by a space and the name of the test binary in brackets, such as math/rand [math/rand.test] [1]. This behavior is intended to convey exactly how test binaries are constructed [1].
Citations:
- 1: https://go.dev/cmd/go/internal/list%40go1.25.1
- 2: https://stackoverflow.com/questions/69783479/what-does-go-list-do-in-gos-toolset
- 3: https://dave.cheney.net/2014/09/14/go-list-your-swiss-army-knife
🏁 Script executed:
# First, let's examine the Makefile at the specified lines
fd -type f -name "Makefile" | head -5Repository: openshift/apiserver-network-proxy
Length of output: 248
🏁 Script executed:
# Once we have the Makefile path, let's read the relevant section
cat -n Makefile | sed -n '80,100p'Repository: openshift/apiserver-network-proxy
Length of output: 1289
🏁 Script executed:
# Let's also check if there are any comments or related code that might explain the intent
cat -n Makefile | sed -n '75,105p'Repository: openshift/apiserver-network-proxy
Length of output: 1928
TEST_LIST derivation is filtering out most packages, reducing CI test coverage.
Line 87 uses go list -test ./... with an egrep " \[.*\]" filter that matches only packages with bracket notation (e.g., pkg [pkg.test]). However, go list -test produces bracketed output only for packages that require recompilation specifically for testing—most packages output without brackets and are excluded by this filter. This leaves $(TEST_LIST) with only a small subset of packages, causing Line 90 to skip coverage for the majority of the codebase.
The fast-test target (line 81) uses the simpler and correct approach: go list ./... with just e2e filtering. Apply the same pattern here.
Proposed fix
- $(eval TEST_LIST := $(shell go list -test ./... | egrep " \[.*\]" | cut -d' ' -f1 | grep -v -e "/e2e$$" -e "/e2e/.*"))
+ $(eval TEST_LIST := $(shell go list ./... | grep -v -e "/e2e$$" -e "/e2e/.*"))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| $(eval TEST_LIST := $(shell go list -test ./... | egrep " \[.*\]" | cut -d' ' -f1 | grep -v -e "/e2e$$" -e "/e2e/.*")) | |
| echo "Running tests on $(TEST_LIST)" | |
| $(info GOTOOLCHAIN is $(GOTOOLCHAIN)) | |
| go test -v -mod=vendor -race -covermode=atomic -coverprofile=konnectivity.out $(TEST_LIST) && go tool cover -html=konnectivity.out -o=konnectivity.html | |
| $(eval TEST_LIST := $(shell go list ./... | grep -v -e "/e2e$$" -e "/e2e/.*")) | |
| echo "Running tests on $(TEST_LIST)" | |
| $(info GOTOOLCHAIN is $(GOTOOLCHAIN)) | |
| go test -v -mod=vendor -race -covermode=atomic -coverprofile=konnectivity.out $(TEST_LIST) && go tool cover -html=konnectivity.out -o=konnectivity.html |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Makefile` around lines 87 - 90, The TEST_LIST derivation is filtering out
most packages by relying on bracketed output from "go list -test"; update
TEST_LIST to mirror the fast-test approach: use "go list ./..." (instead of "go
list -test ./... | egrep ' \\[.*\\]' | cut -d' ' -f1") and then apply the same
e2e exclusion (grep -v -e "/e2e$$" -e "/e2e/.*") so TEST_LIST includes all
non-e2e packages; keep the existing echo and go test invocation that uses
$(TEST_LIST).
bryan-cox
force-pushed
the
rebase-bot-main
branch
2 times, most recently
from
009a1c2 to
0012e91
Compare
Member
Author
|
/test e2e-hypershift |
bryan-cox
force-pushed
the
rebase-bot-main
branch
from
9f3c64f to
f900d38
Compare
|
@bryan-cox: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions 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-sigs/prow repository. I understand the commands that are listed here. |
Member
Author
|
/retest |
Member
Author
|
/test commitchecker |
|
@bryan-cox: The specified target(s) for The following commands are available to trigger optional jobs: Use DetailsIn response to this:
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-sigs/prow repository. |
bryan-cox
changed the title
openshift-ci-robot
added
the
jira/valid-reference
|
@bryan-cox: This pull request references CNTRLPLANE-3380 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
jparrill
reviewed
May 19, 2026
| } | ||
| } | ||
|
|
||
| func TestDestHostBackendManager_Backend_DrainingFallback(t *testing.T) { |
There was a problem hiding this comment.
This test expects DestHostBackendManager.Backend() to filter out draining backends, but pkg/server/desthost_backend_manager.go wasn't modified in this PR. Line 85 still does dibm.backends[destHost][dibm.random.Intn(len(bes))] with no IsDraining() check.
Upstream master does have this logic (iterates backends, skips draining ones, falls back if all are draining). Looks like it got dropped during the rebase. Without this change, this test will be flaky or fail outright.
| go func() { | ||
| defer wg.Done() | ||
| klog.V(1).Infoln("Gracefully stopping agent server...") | ||
| p.agentServer.GracefulStop() |
There was a problem hiding this comment.
The immediate shutdown path (line 198) checks if p.agentServer != nil before calling Stop(), but the graceful path here doesn't. It's unlikely to be nil at this point, but the inconsistency is suspicious — if you guard it in one place, guard it in both.
| if p.healthServer != nil { | ||
| p.healthServer.Close() | ||
| } | ||
| // frontend server's force-stop is handled by its StopFunc |
There was a problem hiding this comment.
This comment says "frontend server's force-stop is handled by its StopFunc", but the gRPC frontend's StopFunc is grpcServer.GracefulStop() which blocks indefinitely and doesn't accept a context. If the frontend's graceful stop hangs, there's no way to force it. Admin/health get Close(), the agent server gets Stop() — but the gRPC frontend has no equivalent. Consider calling grpcServer.Stop() here as well in the timeout path.
| // Test 2: Non-draining backends are returned | ||
| b, err := p.Backend(context.Background()) | ||
| if err != nil { | ||
| t.Errorf("unexpected error: %v", err) |
There was a problem hiding this comment.
If err != nil, b is likely nil and the next line (b.IsDraining()) panics. Use t.Fatalf instead of t.Errorf so the test stops on error rather than crashing with a nil pointer dereference. Same pattern repeats at several other assertions in these tests.
| @@ -999,6 +1000,8 @@ func (s *ProxyServer) serveRecvBackend(backend *Backend, agentID string, recvCh | |||
|
|
|||
| case client.PacketType_DRAIN: | |||
| klog.V(2).InfoS("agent is draining", "agentID", agentID) | |||
| backend.SetDraining() | |||
| klog.V(2).InfoS("marked backend as draining, will not route new requests to this agent", "agentID", agentID) | |||
There was a problem hiding this comment.
Two log lines at the same verbosity level for the same event. The first one (line above) was already there, this one is new. I'd consolidate into one after SetDraining().
| // bufferPool is a pool of byte slices used for reading data from hijacked connections. | ||
| // This reduces memory allocations and GC pressure by reusing buffers across connections. | ||
| var bufferPool = sync.Pool{ | ||
| New: func() interface{} { |
There was a problem hiding this comment.
nit: with go 1.25.0 in go.mod, any is the idiomatic form. interface{} works the same but it's the legacy style.
| ) | ||
| go runpprof.Do(context.Background(), labels, func(context.Context) { grpcServer.Serve(lis) }) | ||
| stop = grpcServer.GracefulStop | ||
| stop = func(_ context.Context) error { |
There was a problem hiding this comment.
nit: the StopFunc contract is func(context.Context) error, which implies the context controls the deadline. But here it's ignored because GracefulStop() doesn't accept a context. Not a bug (the timeout fallback in Run() compensates for the agent server), but a brief inline comment explaining why the context is ignored would help readers.
| steps: | ||
| - name: Check out code | ||
| uses: actions/checkout@v5 | ||
| uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd #v6.0.2 |
There was a problem hiding this comment.
Why are there GH Workflow changes in this PR? Is this intended?
| - "konnectivity-client/**" | ||
| schedule: | ||
| interval: weekly | ||
| interval: daily |
There was a problem hiding this comment.
Why change the schedule from weekly to daily?
|
Alright after talking in slack we agree that this PR contains:
Having that, let's move on. |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bryan-cox, jparrill The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
8 participants
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.
Summary by CodeRabbit
New Features
--graceful-shutdown-timeoutCLI flag to configure graceful shutdown behavior for the proxy.Dependencies