Skip to content

Commit 5502064

Browse files
committed
feat make proxy request-body limit configurable
Signed-off-by: Sanskarzz <sanskar.gur@gmail.com>
1 parent fd1e7b5 commit 5502064

25 files changed

Lines changed: 368 additions & 1 deletion

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

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

425434
// 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
@@ -121,6 +121,9 @@ type RunFlags struct {
121121
// SessionTTL is the session inactivity timeout. Zero uses the transport default.
122122
SessionTTL time.Duration
123123

124+
// ProxyReadTimeout bounds reading a full request on the proxy. Zero uses the default.
125+
ProxyReadTimeout time.Duration
126+
124127
// Network mode
125128
Network string
126129

@@ -310,6 +313,8 @@ func AddRunFlags(cmd *cobra.Command, config *RunFlags) {
310313
"Use for MCP servers implementing streamable-HTTP stateless mode.")
311314
cmd.Flags().DurationVar(&config.SessionTTL, "session-ttl", 0,
312315
"Session inactivity timeout (e.g., 30m, 2h); zero uses the default (2h)")
316+
cmd.Flags().DurationVar(&config.ProxyReadTimeout, "proxy-read-timeout", 0,
317+
"Maximum time to read a full request on the proxy (e.g., 30s, 1m); zero uses the default (30s)")
313318
cmd.Flags().StringVar(&config.EndpointPrefix, "endpoint-prefix", "",
314319
"Path prefix to prepend to SSE endpoint URLs (e.g., /playwright)")
315320
cmd.Flags().StringVar(&config.Network, "network", "",
@@ -735,6 +740,7 @@ func buildRunnerConfig(
735740
runner.WithStrictProtocolValidation(runFlags.StrictProtocolValidation),
736741
runner.WithStateless(runFlags.Stateless),
737742
runner.WithSessionTTL(runFlags.SessionTTL),
743+
runner.WithProxyReadTimeout(runFlags.ProxyReadTimeout),
738744
runner.WithEndpointPrefix(runFlags.EndpointPrefix),
739745
runner.WithNetworkMode(runFlags.Network),
740746
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.
@@ -2317,6 +2327,16 @@ spec:
23172327
maximum: 65535
23182328
minimum: 1
23192329
type: integer
2330+
proxyReadTimeout:
2331+
description: |-
2332+
ProxyReadTimeout bounds how long the proxy spends reading a full request
2333+
(headers + body), mitigating slow-upload connection exhaustion. Applies to
2334+
all transports. Defaults to 30s if not specified. Example: "1m".
2335+
format: duration
2336+
type: string
2337+
x-kubernetes-validations:
2338+
- message: proxyReadTimeout must be non-negative
2339+
rule: duration(self) >= duration('0s')
23202340
rateLimiting:
23212341
description: |-
23222342
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.
@@ -2320,6 +2330,16 @@ spec:
23202330
maximum: 65535
23212331
minimum: 1
23222332
type: integer
2333+
proxyReadTimeout:
2334+
description: |-
2335+
ProxyReadTimeout bounds how long the proxy spends reading a full request
2336+
(headers + body), mitigating slow-upload connection exhaustion. Applies to
2337+
all transports. Defaults to 30s if not specified. Example: "1m".
2338+
format: duration
2339+
type: string
2340+
x-kubernetes-validations:
2341+
- message: proxyReadTimeout must be non-negative
2342+
rule: duration(self) >= duration('0s')
23232343
rateLimiting:
23242344
description: |-
23252345
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
@@ -368,6 +368,19 @@ thv run my-slow-server
368368

369369
**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.
370370

371+
### Proxy Request Read Timeout (All Transports)
372+
373+
Every proxy HTTP server limits reading a complete inbound request, including its body, to 30 seconds by default.
374+
This prevents a slow or stalled upload from holding a connection open indefinitely.
375+
Operators can override the limit per workload with `thv run --proxy-read-timeout` or the MCPServer `spec.proxyReadTimeout` field.
376+
RunConfig stores the same setting as `proxy_read_timeout`, using a Go duration string such as `45s` or `2m`.
377+
378+
Omitting the setting or specifying zero retains the 30-second default; it never disables the timeout.
379+
The read timeout does not limit response streaming, so long-lived SSE responses remain unaffected.
380+
381+
This setting is distinct from `TOOLHIVE_PROXY_REQUEST_TIMEOUT` above: the read timeout bounds the client-to-proxy HTTP upload,
382+
while the stdio proxy request timeout bounds how long an MCP request waits for its correlated server response.
383+
371384
### Health Check Tuning Parameters
372385

373386
**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)