Conversation
…caps
Introduce DistHTTPLimits to bound inbound request bodies (server-side)
and response bodies (client-side) on the dist HTTP transport, preventing
unbounded memory consumption from oversized or malicious payloads.
- Add DistHTTPLimits struct with tunable BodyLimit, ResponseLimit,
timeouts, and concurrency; zero fields inherit package defaults
- Add WithDistHTTPLimits option for DistMemory and WithMgmt{IdleTimeout,
BodyLimit,Concurrency} options for ManagementHTTPServer
- Fix ManagementHTTPServer fiber app construction order so user-supplied
options are applied before the app is built
- Cap client-side response bodies via http.MaxBytesReader; replace
streaming JSON decoders with read-then-unmarshal to surface
MaxBytesError instead of masking it as unexpected EOF
- Extract drainBody helper to deduplicate drain+close across all
transport methods
- Add tests: server rejects oversized body (413), client rejects
oversized response (MaxBytesError), default limits sanity check
- Fix TestDistMemoryHeartbeatLiveness race by polling NodesRemoved
metric after membership transition settles
…wnWithContext
Eliminate the goroutine-leak risk in both ManagementHTTPServer and
distHTTPServer where the old `go func() { ch <- s.app.Shutdown() }()`
pattern returned to the caller via the ctx-done branch but left the
inner goroutine running until fiber drained organically.
- Replace the manual goroutine+select in Shutdown/stop with
fiber's ShutdownWithContext, which closes listeners, waits for
in-flight requests, and force-closes on ctx deadline.
- Capture the server-lifecycle context in ManagementHTTPServer.ctx and
distHTTPServer.ctx at Start/start time so handlers (TriggerEviction,
Clear, applySet, applyRemove) use a stable context rather than the
pooled, short-lived fiber.Ctx that races with happy-eyeballs dial
goroutines spawned by http.Client.Do.
- Refactor duplicate route registrations into shared handler methods
(handleSet, handleGet, handleRemove) mounted on both legacy
(/internal/cache/*) and canonical (/internal/*) paths.
- Remove now-unused sentinel.ErrMgmtHTTPShutdownTimeout.
- Fix bench target to use ./... instead of . to pick up all sub-packages.
- Add TestDistHTTPServer_StopRespectsCanceledContext to guard against
regression: verifies Stop returns sub-second with an already-canceled
context.
… cancellation Introduces three security and reliability improvements to the distributed HTTP transport layer: 1. Bearer-token authentication (DistHTTPAuth): constant-time token validation on the server, automatic request signing on the auto-created HTTP client. ServerVerify and ClientSign escape hatches support JWT, HMAC, and mTLS-derived identity. Applied to all dist endpoints including /health. 2. TLS support via DistHTTPLimits.TLSConfig: wraps TCP listeners with tls.NewListener and configures the auto-created client with a matching *tls.Config. Forces HTTP/1.1 via ALPN to avoid h2/fasthttp mismatch; resolver advertises https:// when TLS is configured. 3. Deterministic lifecycle context cancellation: DistMemory and ManagementHTTPServer derive a lifeCtx/lifeCancel pair from the constructor context. Stop() cancels lifeCtx before channel tear-down, so in-flight handlers and background goroutines observe Done() independently of the (usually non-canceling) constructor context. Additional changes: - LastServeError() surfaces background serve-goroutine failures instead of silently swallowing them - LifecycleContext() accessor and WithDistHTTPAuth() option added to DistMemory; ErrUnauthorized sentinel added - makePeerURLResolver extracted and shared between tryStartHTTP and EnableHTTPForTest - Integration tests: dist_http_auth_test.go, dist_http_tls_test.go, dist_http_lifecycle_test.go
Contributor
There was a problem hiding this comment.
Pull request overview
This PR modernizes the distributed HTTP and management-server layers by adding configurable transport limits, optional auth/TLS support, and explicit lifecycle-context cancellation so in-flight work can observe shutdown. It also adds end-to-end tests around those behaviors and removes several generated benchmark/lint/race artifacts from the repository.
Changes:
- Add dist HTTP transport/server configuration for body/response limits, TLS, bearer/custom auth, and shared peer URL resolution.
- Introduce lifecycle contexts canceled by
Stop/Shutdownfor DistMemory and management HTTP handlers/background work. - Add focused tests for auth, TLS, limits, and lifecycle behavior; remove checked-in benchmark/race/lint output files.
Reviewed changes
Copilot reviewed 21 out of 24 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/hypercache_distmemory_heartbeat_test.go | Stabilizes heartbeat metric assertion with an extra polling helper. |
| tests/dist_http_tls_test.go | Adds TLS handshake and replication integration tests for dist HTTP. |
| tests/dist_http_limits_test.go | Adds tests for HTTP body/response limits and shutdown behavior. |
| tests/dist_http_lifecycle_test.go | Adds lifecycle-context cancellation test for DistMemory.Stop. |
| tests/dist_http_auth_test.go | Adds auth enforcement, signing, and custom verifier tests. |
| race-step2.log | Removes generated race-test output artifact. |
| race-baseline-v2.log | Removes generated race baseline artifact. |
| pkg/stats/histogramcollector_test.go | Simplifies nolint comments in histogram tests. |
| pkg/backend/dist_memory_test_helpers.go | Updates test helper HTTP startup to use new limits/auth-aware constructors. |
| pkg/backend/dist_memory.go | Adds HTTP limits/auth config, lifecycle context support, and shared peer URL resolver. |
| pkg/backend/dist_http_transport.go | Adds TLS/auth-aware HTTP transport and bounded response-body handling. |
| pkg/backend/dist_http_server.go | Adds auth/TLS support, request limits, lifecycle context usage, and graceful shutdown changes. |
| management_http.go | Adds configurable server limits plus lifecycle-context and shutdown improvements. |
| lint-baseline-v2.txt | Removes generated lint baseline artifact. |
| internal/sentinel/sentinel.go | Replaces old management shutdown sentinel with unauthorized sentinel. |
| cspell.config.yaml | Updates spelling dictionary for new terminology in code/tests. |
| bench-step3.txt | Removes generated benchmark output artifact. |
| bench-step2.txt | Removes generated benchmark output artifact. |
| bench-step1.txt | Removes generated benchmark output artifact. |
| bench-step1-unit.txt | Removes generated unit benchmark output artifact. |
| bench-phase1.txt | Removes generated benchmark output artifact. |
| bench-baseline.txt | Removes generated benchmark baseline artifact. |
| bench-baseline-v2.txt | Removes generated benchmark baseline artifact. |
| Makefile | Changes benchmark target scope from current package to recursive packages. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+95
to
+99
| // net/http docs. | ||
| tlsConf := limits.TLSConfig.Clone() | ||
| if len(tlsConf.NextProtos) == 0 { | ||
| tlsConf.NextProtos = []string{"http/1.1"} | ||
| } |
| return func(fctx fiber.Ctx) error { | ||
| err := s.auth.verify(fctx) | ||
| if err != nil { | ||
| return fctx.Status(fiber.StatusUnauthorized).JSON(fiber.Map{constants.ErrorLabel: err.Error()}) |
Comment on lines
+141
to
+152
| // LastServeError returns the last error captured from the background | ||
| // serve goroutine. Returns nil when the server shut down cleanly. | ||
| func (s *ManagementHTTPServer) LastServeError() error { | ||
| if s == nil { | ||
| return nil | ||
| } | ||
|
|
||
| if errp := s.serveErr.Load(); errp != nil { | ||
| return *errp | ||
| } | ||
|
|
||
| return nil |
| # bench runs the benchmark tests in the benchmark subpackage of the tests package. | ||
| bench: | ||
| cd tests/benchmark && go test -bench=. -benchmem -benchtime=4s . -timeout 30m | ||
| cd tests/benchmark && go test -bench=. -benchmem -benchtime=4s ./... -timeout 30m |
| } | ||
|
|
||
| if a.Token == "" { | ||
| return nil |
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
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.
No description provided.