Skip to content

Commit 25c0bcd

Browse files
committed
feat make proxy request read timeout configurable
Signed-off-by: Sanskarzz <sanskar.gur@gmail.com>
1 parent 7f15a63 commit 25c0bcd

25 files changed

Lines changed: 601 additions & 234 deletions

cmd/thv-operator/api/v1beta1/mcpserver_types.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -415,6 +415,15 @@ type MCPServerSpec struct {
415415
// Requires Redis session storage to be configured for distributed rate limiting.
416416
// +optional
417417
RateLimiting *ratelimittypes.RateLimitConfig `json:"rateLimiting,omitempty"`
418+
419+
// ProxyReadTimeout bounds how long the proxy spends reading a full request
420+
// (headers + body), mitigating slow-upload connection exhaustion. Applies to
421+
// all transports. Defaults to 30s if not specified. Example: "1m".
422+
// +kubebuilder:validation:Type=string
423+
// +kubebuilder:validation:Format=duration
424+
// +kubebuilder:validation:XValidation:rule="duration(self) >= duration('0s')",message="proxyReadTimeout must be non-negative"
425+
// +optional
426+
ProxyReadTimeout *metav1.Duration `json:"proxyReadTimeout,omitempty"`
418427
}
419428

420429
// ResourceOverrides defines overrides for annotations and labels on created resources

cmd/thv-operator/api/v1beta1/zz_generated.deepcopy.go

Lines changed: 5 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

cmd/thv-operator/controllers/mcpserver_runconfig.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -277,6 +277,11 @@ func (r *MCPServerReconciler) createRunConfigFromMCPServer(m *mcpv1beta1.MCPServ
277277
options = append(options, runner.WithRateLimitConfig(m.Namespace, m.Spec.RateLimiting))
278278
}
279279

280+
// Add proxy HTTP server read timeout if specified
281+
if m.Spec.ProxyReadTimeout != nil {
282+
options = append(options, runner.WithProxyReadTimeout(m.Spec.ProxyReadTimeout.Duration))
283+
}
284+
280285
// Use the RunConfigBuilder for operator context with full builder pattern
281286
runConfig, err := runner.NewOperatorRunConfigBuilder(
282287
context.Background(),

cmd/thv-operator/controllers/mcpserver_runconfig_test.go

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"fmt"
1010
"reflect"
1111
"testing"
12+
"time"
1213

1314
"github.com/stretchr/testify/assert"
1415
"github.com/stretchr/testify/require"
@@ -61,6 +62,36 @@ func TestCreateRunConfigFromMCPServer(t *testing.T) {
6162
assert.Equal(t, 8080, config.Port)
6263
},
6364
},
65+
{
66+
name: "nil proxy read timeout leaves the RunConfig value empty",
67+
mcpServer: v1beta1test.NewMCPServer("nil-timeout-server", "test-ns"),
68+
//nolint:thelper // We want to see the error at the specific line
69+
expected: func(t *testing.T, config *runner.RunConfig) {
70+
assert.Empty(t, config.ProxyReadTimeout)
71+
},
72+
},
73+
{
74+
name: "zero proxy read timeout uses the proxy default",
75+
mcpServer: v1beta1test.NewMCPServer("zero-timeout-server", "test-ns",
76+
v1beta1test.Mutate(func(m *mcpv1beta1.MCPServer) {
77+
m.Spec.ProxyReadTimeout = &metav1.Duration{}
78+
})),
79+
//nolint:thelper // We want to see the error at the specific line
80+
expected: func(t *testing.T, config *runner.RunConfig) {
81+
assert.Empty(t, config.ProxyReadTimeout)
82+
},
83+
},
84+
{
85+
name: "positive proxy read timeout is translated",
86+
mcpServer: v1beta1test.NewMCPServer("positive-timeout-server", "test-ns",
87+
v1beta1test.Mutate(func(m *mcpv1beta1.MCPServer) {
88+
m.Spec.ProxyReadTimeout = &metav1.Duration{Duration: time.Minute}
89+
})),
90+
//nolint:thelper // We want to see the error at the specific line
91+
expected: func(t *testing.T, config *runner.RunConfig) {
92+
assert.Equal(t, "1m0s", config.ProxyReadTimeout)
93+
},
94+
},
6495
{
6596
name: "with environment variables",
6697
mcpServer: v1beta1test.NewMCPServer("env-server", "test-ns",

cmd/thv-operator/test-integration/mcp-server/mcpserver_sessionstorage_cel_test.go

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
package controllers
55

66
import (
7+
"time"
8+
79
. "github.com/onsi/ginkgo/v2"
810
. "github.com/onsi/gomega"
911
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -117,4 +119,25 @@ var _ = Describe("CEL Validation for SessionStorageConfig on MCPServer",
117119
Expect(err).To(HaveOccurred())
118120
})
119121
})
122+
123+
Context("proxyReadTimeout field", func() {
124+
DescribeTable("should accept non-negative values",
125+
func(name string, timeout *metav1.Duration) {
126+
server := newMinimalMCPServer(name, nil)
127+
server.Spec.ProxyReadTimeout = timeout
128+
err := k8sClient.Create(ctx, server)
129+
Expect(err).NotTo(HaveOccurred())
130+
},
131+
Entry("when omitted", "mcp-proxy-read-timeout-omitted", nil),
132+
Entry("when zero", "mcp-proxy-read-timeout-zero", &metav1.Duration{}),
133+
Entry("when positive", "mcp-proxy-read-timeout-positive", &metav1.Duration{Duration: 45 * time.Second}),
134+
)
135+
136+
It("should reject a negative value", func() {
137+
server := newMinimalMCPServer("mcp-proxy-read-timeout-negative", nil)
138+
server.Spec.ProxyReadTimeout = &metav1.Duration{Duration: -time.Second}
139+
err := k8sClient.Create(ctx, server)
140+
Expect(err).To(MatchError(ContainSubstring("proxyReadTimeout must be non-negative")))
141+
})
142+
})
120143
})

cmd/thv/app/run_flags.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,9 @@ type RunFlags struct {
112112
// SessionTTL is the session inactivity timeout. Zero uses the transport default.
113113
SessionTTL time.Duration
114114

115+
// ProxyReadTimeout bounds reading a full request on the proxy. Zero uses the default.
116+
ProxyReadTimeout time.Duration
117+
115118
// Network mode
116119
Network string
117120

@@ -295,6 +298,8 @@ func AddRunFlags(cmd *cobra.Command, config *RunFlags) {
295298
"Use for MCP servers implementing streamable-HTTP stateless mode.")
296299
cmd.Flags().DurationVar(&config.SessionTTL, "session-ttl", 0,
297300
"Session inactivity timeout (e.g., 30m, 2h); zero uses the default (2h)")
301+
cmd.Flags().DurationVar(&config.ProxyReadTimeout, "proxy-read-timeout", 0,
302+
"Maximum time to read a full request on the proxy (e.g., 30s, 1m); zero uses the default (30s)")
298303
cmd.Flags().StringVar(&config.EndpointPrefix, "endpoint-prefix", "",
299304
"Path prefix to prepend to SSE endpoint URLs (e.g., /playwright)")
300305
cmd.Flags().StringVar(&config.Network, "network", "",
@@ -707,6 +712,7 @@ func buildRunnerConfig(
707712
runner.WithStrictProtocolValidation(runFlags.StrictProtocolValidation),
708713
runner.WithStateless(runFlags.Stateless),
709714
runner.WithSessionTTL(runFlags.SessionTTL),
715+
runner.WithProxyReadTimeout(runFlags.ProxyReadTimeout),
710716
runner.WithEndpointPrefix(runFlags.EndpointPrefix),
711717
runner.WithNetworkMode(runFlags.Network),
712718
runner.WithK8sPodPatch(runFlags.K8sPodPatch),

deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpservers.yaml

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -390,6 +390,16 @@ spec:
390390
maximum: 65535
391391
minimum: 1
392392
type: integer
393+
proxyReadTimeout:
394+
description: |-
395+
ProxyReadTimeout bounds how long the proxy spends reading a full request
396+
(headers + body), mitigating slow-upload connection exhaustion. Applies to
397+
all transports. Defaults to 30s if not specified. Example: "1m".
398+
format: duration
399+
type: string
400+
x-kubernetes-validations:
401+
- message: proxyReadTimeout must be non-negative
402+
rule: duration(self) >= duration('0s')
393403
rateLimiting:
394404
description: |-
395405
RateLimiting defines rate limiting configuration for the MCP server.
@@ -1334,6 +1344,16 @@ spec:
13341344
maximum: 65535
13351345
minimum: 1
13361346
type: integer
1347+
proxyReadTimeout:
1348+
description: |-
1349+
ProxyReadTimeout bounds how long the proxy spends reading a full request
1350+
(headers + body), mitigating slow-upload connection exhaustion. Applies to
1351+
all transports. Defaults to 30s if not specified. Example: "1m".
1352+
format: duration
1353+
type: string
1354+
x-kubernetes-validations:
1355+
- message: proxyReadTimeout must be non-negative
1356+
rule: duration(self) >= duration('0s')
13371357
rateLimiting:
13381358
description: |-
13391359
RateLimiting defines rate limiting configuration for the MCP server.

deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpservers.yaml

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -393,6 +393,16 @@ spec:
393393
maximum: 65535
394394
minimum: 1
395395
type: integer
396+
proxyReadTimeout:
397+
description: |-
398+
ProxyReadTimeout bounds how long the proxy spends reading a full request
399+
(headers + body), mitigating slow-upload connection exhaustion. Applies to
400+
all transports. Defaults to 30s if not specified. Example: "1m".
401+
format: duration
402+
type: string
403+
x-kubernetes-validations:
404+
- message: proxyReadTimeout must be non-negative
405+
rule: duration(self) >= duration('0s')
396406
rateLimiting:
397407
description: |-
398408
RateLimiting defines rate limiting configuration for the MCP server.
@@ -1337,6 +1347,16 @@ spec:
13371347
maximum: 65535
13381348
minimum: 1
13391349
type: integer
1350+
proxyReadTimeout:
1351+
description: |-
1352+
ProxyReadTimeout bounds how long the proxy spends reading a full request
1353+
(headers + body), mitigating slow-upload connection exhaustion. Applies to
1354+
all transports. Defaults to 30s if not specified. Example: "1m".
1355+
format: duration
1356+
type: string
1357+
x-kubernetes-validations:
1358+
- message: proxyReadTimeout must be non-negative
1359+
rule: duration(self) >= duration('0s')
13401360
rateLimiting:
13411361
description: |-
13421362
RateLimiting defines rate limiting configuration for the MCP server.

docs/arch/03-transport-architecture.md

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -355,6 +355,19 @@ thv run my-slow-server
355355

356356
**Note:** This timeout only affects the streamable HTTP proxy used with stdio transport. The transparent proxy used by SSE and streamable-http transports (where the container runs its own HTTP server) does not impose a request timeout.
357357

358+
### Proxy Request Read Timeout (All Transports)
359+
360+
Every proxy HTTP server limits reading a complete inbound request, including its body, to 30 seconds by default.
361+
This prevents a slow or stalled upload from holding a connection open indefinitely.
362+
Operators can override the limit per workload with `thv run --proxy-read-timeout` or the MCPServer `spec.proxyReadTimeout` field.
363+
RunConfig stores the same setting as `proxy_read_timeout`, using a Go duration string such as `45s` or `2m`.
364+
365+
Omitting the setting or specifying zero retains the 30-second default; it never disables the timeout.
366+
The read timeout does not limit response streaming, so long-lived SSE responses remain unaffected.
367+
368+
This setting is distinct from `TOOLHIVE_PROXY_REQUEST_TIMEOUT` above: the read timeout bounds the client-to-proxy HTTP upload,
369+
while the stdio proxy request timeout bounds how long an MCP request waits for its correlated server response.
370+
358371
### Health Check Tuning Parameters
359372

360373
**Implementation**: `pkg/transport/proxy/transparent/transparent_proxy.go`

docs/arch/05-runconfig-and-permissions.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,8 @@ thv run uvx://mcp-server \
124124
"transport": "stdio",
125125
"host": "127.0.0.1",
126126
"port": 8080,
127-
"proxy_mode": "streamable-http"
127+
"proxy_mode": "streamable-http",
128+
"proxy_read_timeout": "45s"
128129
}
129130
```
130131

@@ -146,6 +147,7 @@ thv run uvx://mcp-server \
146147
- `target_port`: Container port (SSE/Streamable only)
147148
- `target_host`: Container host (default: `127.0.0.1`)
148149
- `proxy_mode`: For stdio: `sse` or `streamable-http`
150+
- `proxy_read_timeout`: Maximum time to read a complete client request, as a Go duration string; omitted or zero uses the secure 30-second default
149151

150152
**Implementation**: `pkg/runner/config.go`
151153

0 commit comments

Comments
 (0)