From 0c72dd456031c8647dff2e1c67fbb6f3ec178e85 Mon Sep 17 00:00:00 2001 From: Dmitrii Creed Date: Fri, 17 Jul 2026 17:42:11 +0300 Subject: [PATCH 1/2] Buffer proxied request bodies; harden cloudsql-proxy readiness reverse_proxy render: add request_buffers (default 1MiB, per-stack lbConfig.requestBufferSize, "0" disables). Chunked request bodies reach WSGI upstreams without Content-Length; Django (ticket #28668) reads an empty body on such requests - webhook 500s, HMAC signature failures over the empty body, lost CSRF tokens. Bodies above the buffer keep streaming as before. cloudsql-proxy sidecar: readiness timeout 3s->10s, CPU request 50m->100m. At 50m the health server starves under node CPU pressure; 3s probe timeouts flap pods out of Service endpoints (sidecar readiness gates pod readiness per KEP-753 - the old comment claimed otherwise and is fixed too). Combined with node consolidation this can briefly leave a Service with zero ready pods. Signed-off-by: Dmitrii Creed --- pkg/api/client.go | 6 +++ pkg/clouds/pulumi/gcp/cloudsql_proxy.go | 22 +++++++--- pkg/clouds/pulumi/gcp/cloudsql_proxy_test.go | 17 +++++++ .../pulumi/kubernetes/simple_container.go | 12 +++++ .../kubernetes/simple_container_test.go | 44 +++++++++++++++++++ 5 files changed, 95 insertions(+), 6 deletions(-) diff --git a/pkg/api/client.go b/pkg/api/client.go index 7a9c37ce..13d73525 100644 --- a/pkg/api/client.go +++ b/pkg/api/client.go @@ -168,6 +168,12 @@ type SimpleContainerLBConfig struct { // what's accepted at each scope; mis-placing a directive yields a // Caddyfile parse error at reload time. SiteExtraHelpers []string `json:"siteExtraHelpers" yaml:"siteExtraHelpers"` + // RequestBufferSize sets `request_buffers` on the generated reverse_proxy + // block. Bodies up to this size are buffered so the upstream receives + // Content-Length instead of Transfer-Encoding: chunked — WSGI frameworks + // (Django #28668) read an empty body on chunked requests. Bodies larger + // than the buffer keep streaming as before. Default "1MiB"; "0" disables. + RequestBufferSize *string `json:"requestBufferSize" yaml:"requestBufferSize"` } type StackConfigCompose struct { diff --git a/pkg/clouds/pulumi/gcp/cloudsql_proxy.go b/pkg/clouds/pulumi/gcp/cloudsql_proxy.go index 87481bf9..84d77b2c 100644 --- a/pkg/clouds/pulumi/gcp/cloudsql_proxy.go +++ b/pkg/clouds/pulumi/gcp/cloudsql_proxy.go @@ -198,7 +198,9 @@ func cloudsqlProxyContainerArgs(secretName, project, region, instanceName string }, Requests: sdk.StringMap{ "memory": sdk.String("200Mi"), - "cpu": sdk.String("50m"), + // 100m (was 50m): at 50m the proxy's health server starves under + // node CPU pressure and readiness probes time out. + "cpu": sdk.String("100m"), }, }, VolumeMounts: v1.VolumeMountArray{ @@ -230,19 +232,27 @@ func cloudsqlProxyContainerArgs(secretName, project, region, instanceName string TimeoutSeconds: sdk.IntPtr(3), FailureThreshold: sdk.IntPtr(30), } + // Sidecar readiness GATES POD READINESS (KEP-753): three consecutive + // failures drop the whole pod from Service endpoints. timeout 10s (was 3s): + // with a 50m-CPU-request sidecar the health server starves under node + // pressure and 3s timeouts flap pods out of rotation; combined with + // Autopilot node consolidation this can leave a Service with zero ready + // pods. + // /readiness stays (unlike /liveness it verifies instance connectivity + // and connection capacity; /liveness returns 200 whenever the health + // server responds at all). container.ReadinessProbe = &v1.ProbeArgs{ HttpGet: v1.HTTPGetActionArgs{ Path: sdk.String("/readiness"), Port: sdk.String("csql-hc"), }, PeriodSeconds: sdk.IntPtr(10), - TimeoutSeconds: sdk.IntPtr(3), + TimeoutSeconds: sdk.IntPtr(10), FailureThreshold: sdk.IntPtr(3), } - // On a native sidecar a failing readiness probe neither restarts the container nor - // gates pod readiness — only liveness recovers a proxy that passed startup and then - // hung (deadlock / pool exhaustion / partial-OOM), where the process stays alive but - // app DB calls to localhost:5432 fail. /liveness is already served by --health-check. + // Liveness recovers a proxy that passed startup and then hung (deadlock / + // pool exhaustion / partial-OOM), where the process stays alive but app DB + // calls to localhost:5432 fail. /liveness is already served by --health-check. // kubelet defers liveness until the startup probe succeeds, so no InitialDelay is needed. container.LivenessProbe = &v1.ProbeArgs{ HttpGet: v1.HTTPGetActionArgs{ diff --git a/pkg/clouds/pulumi/gcp/cloudsql_proxy_test.go b/pkg/clouds/pulumi/gcp/cloudsql_proxy_test.go index ead9883f..3bb4c8da 100644 --- a/pkg/clouds/pulumi/gcp/cloudsql_proxy_test.go +++ b/pkg/clouds/pulumi/gcp/cloudsql_proxy_test.go @@ -139,3 +139,20 @@ func TestAttachCloudsqlProxyAsNativeSidecar_LandsInInitContainers(t *testing.T) assert.Empty(t, kubeArgs.SidecarOutputs, "proxy must NOT land in regular containers") assert.Len(t, kubeArgs.VolumeOutputs, 1, "credential secret volume must ride along") } + +// 3s readiness timeouts on a 50m-CPU sidecar can flap pods out of Service +// endpoints during Autopilot node consolidation (sidecar readiness +// gates pod readiness, KEP-753). Generous timeout + bigger CPU request keep +// transient starvation from dropping the whole pod out of rotation. +func TestCloudsqlProxyContainerArgs_ReadinessTolerantToStarvation(t *testing.T) { + c := cloudsqlProxyContainerArgs("creds", "proj", "reg", "inst", 0) + + rp := c.ReadinessProbe.(*v1.ProbeArgs) + assert.Equal(t, sdk.IntPtr(10), rp.TimeoutSeconds) + assert.Equal(t, sdk.IntPtr(10), rp.PeriodSeconds) + assert.Equal(t, sdk.IntPtr(3), rp.FailureThreshold) + + res := c.Resources.(*v1.ResourceRequirementsArgs) + req := res.Requests.(sdk.StringMap) + assert.Equal(t, sdk.String("100m"), req["cpu"]) +} diff --git a/pkg/clouds/pulumi/kubernetes/simple_container.go b/pkg/clouds/pulumi/kubernetes/simple_container.go index d32a8bfb..290aaccc 100644 --- a/pkg/clouds/pulumi/kubernetes/simple_container.go +++ b/pkg/clouds/pulumi/kubernetes/simple_container.go @@ -706,6 +706,7 @@ func NewSimpleContainer(ctx *sdk.Context, args *SimpleContainerArgs, opts ...sdk caddyfileEntryTemplate = ` ${proto}://${domain} { reverse_proxy http://${service}.${namespace}.svc.cluster.local:${port} { + request_buffers ${requestBuffers} header_down Server nginx ${addHeaders} import handle_server_error ${extraHelpers} @@ -717,6 +718,7 @@ ${proto}://${domain} { caddyfileEntryTemplate = ` handle_path /${prefix}* {${additionalProxyConfig} reverse_proxy http://${service}.${namespace}.svc.cluster.local:${port} { + request_buffers ${requestBuffers} header_down Server nginx ${addHeaders} import handle_server_error ${extraHelpers} @@ -762,6 +764,15 @@ ${proto}://${domain} { if helpers := lo.FromPtr(args.LbConfig).SiteExtraHelpers; len(helpers) > 0 { siteExtraHelpersStr = "\n " + strings.Join(helpers, "\n ") } + // Buffer request bodies so upstreams receive Content-Length instead of + // Transfer-Encoding: chunked — WSGI apps (Django #28668) read an empty + // body on chunked requests: webhook 500s, HMAC signature failures over + // the empty body, lost CSRF tokens. 1MiB covers webhook/API/form bodies; + // larger bodies keep streaming exactly as before. + requestBuffersStr := "1MiB" + if v := lo.FromPtr(lo.FromPtr(args.LbConfig).RequestBufferSize); v != "" { + requestBuffersStr = v + } placeholdersMap := placeholders.MapData{ "proto": lo.If(lo.FromPtr(args.LbConfig).Https, "https").Else("http"), "domain": args.Domain, @@ -773,6 +784,7 @@ ${proto}://${domain} { "extraHelpers": strings.Join(lo.FromPtr(args.LbConfig).ExtraHelpers, "\n "), "imports": strings.Join(imports, "\n "), "siteExtraHelpers": siteExtraHelpersStr, + "requestBuffers": requestBuffersStr, } if args.ProxyKeepPrefix { placeholdersMap["additionalProxyConfig"] = fmt.Sprintf("\n rewrite * /%s{uri}", args.Prefix) diff --git a/pkg/clouds/pulumi/kubernetes/simple_container_test.go b/pkg/clouds/pulumi/kubernetes/simple_container_test.go index 7212b645..5932d28c 100644 --- a/pkg/clouds/pulumi/kubernetes/simple_container_test.go +++ b/pkg/clouds/pulumi/kubernetes/simple_container_test.go @@ -760,3 +760,47 @@ func TestNewSimpleContainer_MinimalConfiguration(t *testing.T) { Expect(err).ToNot(HaveOccurred(), "Test should complete without errors") } + +// request_buffers Rendering Tests +// +// Regression coverage: without request buffering +// Caddy forwards chunked request bodies as-is, and WSGI upstreams (Django, +// django ticket #28668) read an empty request.body — webhook 500s, HMAC +// signature failures over the empty body, "lost" CSRF tokens. The rendered +// reverse_proxy block must always carry request_buffers so bodies up to the +// buffer size reach upstreams with Content-Length. + +func TestCaddyfileEntry_RequestBuffersDefault(t *testing.T) { + RegisterTestingT(t) + + entry := caddyfileEntryFor(t, createBasicTestArgs()) + + Expect(entry).To(ContainSubstring("request_buffers 1MiB")) +} + +func TestCaddyfileEntry_RequestBuffersOverride(t *testing.T) { + RegisterTestingT(t) + + args := createBasicTestArgs() + args.LbConfig = &api.SimpleContainerLBConfig{ + RequestBufferSize: lo.ToPtr("4MiB"), + } + + entry := caddyfileEntryFor(t, args) + + Expect(entry).To(ContainSubstring("request_buffers 4MiB")) + Expect(entry).ToNot(ContainSubstring("request_buffers 1MiB")) +} + +func TestCaddyfileEntry_RequestBuffersDisabled(t *testing.T) { + RegisterTestingT(t) + + args := createBasicTestArgs() + args.LbConfig = &api.SimpleContainerLBConfig{ + RequestBufferSize: lo.ToPtr("0"), + } + + entry := caddyfileEntryFor(t, args) + + Expect(entry).To(ContainSubstring("request_buffers 0")) +} From 12278830bbde49989ab8ecb4346ff1bc5ac9a443 Mon Sep 17 00:00:00 2001 From: Dmitrii Creed Date: Sat, 18 Jul 2026 07:55:04 +0300 Subject: [PATCH 2/2] Address review: validate requestBufferSize, liveness parity, schema regen - lbConfig.requestBufferSize is now parsed (humanize), capped at 16MiB, and re-rendered as a plain byte count - no user-controlled text reaches the shared Caddyfile (a malformed value previously produced a parse-fatal aggregated config detonating at the next Caddy restart, taking down every tenant). Invalid values fail only the offending stack's deploy. Field is now a plain string with omitempty (sibling idiom; keeps it out of the JSON schema's required list). - "0" now OMITS the request_buffers directive entirely (placeholder carries the whole line) - real escape hatch for edges pinned to Caddy < 2.6.0. A render-time warning fires when extraHelpers carries a duplicate request_buffers (Caddy last-wins would silently override the typed knob). - Liveness probe timeout 3s->10s to match readiness: /liveness is served by the same starvable health server; a 3s timeout would RESTART the sidecar (severing live DB connections) under the exact pressure the readiness change tolerates. Startup stays tight on purpose (documented). - Regenerated docs/schemas (requestBufferSize in stackconfigcompose). - Tests per panel findings: prefix-template coverage (mutation-verified to fail on regression), line-anchored+placement assertions instead of substrings, invalid/cap/zero/default-with-lbConfig cases, all probe numerics pinned. go-humanize promoted to direct dependency. Signed-off-by: Dmitrii Creed --- docs/schemas/core/stackconfigcompose.json | 3 + go.mod | 2 +- pkg/api/client.go | 20 ++-- pkg/clouds/pulumi/gcp/cloudsql_proxy.go | 14 ++- pkg/clouds/pulumi/gcp/cloudsql_proxy_test.go | 12 ++- .../pulumi/kubernetes/simple_container.go | 49 +++++++--- .../kubernetes/simple_container_test.go | 91 ++++++++++++++++--- 7 files changed, 154 insertions(+), 37 deletions(-) diff --git a/docs/schemas/core/stackconfigcompose.json b/docs/schemas/core/stackconfigcompose.json index 8f72a855..f1a2f702 100644 --- a/docs/schemas/core/stackconfigcompose.json +++ b/docs/schemas/core/stackconfigcompose.json @@ -350,6 +350,9 @@ "https": { "type": "boolean" }, + "requestBufferSize": { + "type": "string" + }, "siteExtraHelpers": { "items": { "type": "string" diff --git a/go.mod b/go.mod index 265e1f57..c44a7e12 100644 --- a/go.mod +++ b/go.mod @@ -21,6 +21,7 @@ require ( github.com/containerd/platforms v0.2.1 github.com/disgoorg/disgo v0.19.6 github.com/disgoorg/snowflake/v2 v2.0.3 + github.com/dustin/go-humanize v1.0.1 github.com/fatih/color v1.19.0 github.com/go-delve/delve v1.26.3 github.com/go-git/go-billy/v5 v5.9.0 @@ -189,7 +190,6 @@ require ( github.com/dlclark/regexp2 v1.11.0 // indirect github.com/docker/go-connections v0.7.0 // indirect github.com/docker/go-units v0.5.0 // indirect - github.com/dustin/go-humanize v1.0.1 // indirect github.com/edsrzf/mmap-go v1.1.0 // indirect github.com/emicklei/go-restful/v3 v3.13.0 // indirect github.com/emirpasic/gods v1.18.1 // indirect diff --git a/pkg/api/client.go b/pkg/api/client.go index 13d73525..c0ed76df 100644 --- a/pkg/api/client.go +++ b/pkg/api/client.go @@ -168,12 +168,20 @@ type SimpleContainerLBConfig struct { // what's accepted at each scope; mis-placing a directive yields a // Caddyfile parse error at reload time. SiteExtraHelpers []string `json:"siteExtraHelpers" yaml:"siteExtraHelpers"` - // RequestBufferSize sets `request_buffers` on the generated reverse_proxy - // block. Bodies up to this size are buffered so the upstream receives - // Content-Length instead of Transfer-Encoding: chunked — WSGI frameworks - // (Django #28668) read an empty body on chunked requests. Bodies larger - // than the buffer keep streaming as before. Default "1MiB"; "0" disables. - RequestBufferSize *string `json:"requestBufferSize" yaml:"requestBufferSize"` + // RequestBufferSize controls the `request_buffers` directive on the + // generated reverse_proxy block. Bodies up to this size are buffered so + // the upstream receives Content-Length instead of Transfer-Encoding: + // chunked — WSGI frameworks (Django #28668) read an empty body on chunked + // requests. The value is parsed as a byte size (humanize: "512KB", + // "4MiB"), validated (max 16MiB — the buffer is held in edge-proxy memory + // per in-flight request), and rendered as a plain byte count, so no + // user-controlled text reaches the shared Caddyfile. "0" omits the + // directive entirely (also the escape hatch for edges running Caddy + // < 2.6.0, which predates request_buffers). Unset/empty = default 1MiB. + // Chunked bodies LARGER than the buffer still reach the upstream chunked + // and remain subject to Django #28668 — raise the size for endpoints + // receiving large chunked uploads. + RequestBufferSize string `json:"requestBufferSize,omitempty" yaml:"requestBufferSize,omitempty"` } type StackConfigCompose struct { diff --git a/pkg/clouds/pulumi/gcp/cloudsql_proxy.go b/pkg/clouds/pulumi/gcp/cloudsql_proxy.go index 84d77b2c..c62502e4 100644 --- a/pkg/clouds/pulumi/gcp/cloudsql_proxy.go +++ b/pkg/clouds/pulumi/gcp/cloudsql_proxy.go @@ -228,6 +228,8 @@ func cloudsqlProxyContainerArgs(secretName, project, region, instanceName string Path: sdk.String("/startup"), Port: sdk.String("csql-hc"), }, + // Startup keeps the tight 3s timeout deliberately: nothing is connected + // yet, and the 30-failure budget already absorbs slow cold starts. PeriodSeconds: sdk.IntPtr(2), TimeoutSeconds: sdk.IntPtr(3), FailureThreshold: sdk.IntPtr(30), @@ -235,7 +237,8 @@ func cloudsqlProxyContainerArgs(secretName, project, region, instanceName string // Sidecar readiness GATES POD READINESS (KEP-753): three consecutive // failures drop the whole pod from Service endpoints. timeout 10s (was 3s): // with a 50m-CPU-request sidecar the health server starves under node - // pressure and 3s timeouts flap pods out of rotation; combined with + // pressure (observed at the previous 50m request) and 3s timeouts flap + // pods out of rotation; combined with // Autopilot node consolidation this can leave a Service with zero ready // pods. // /readiness stays (unlike /liveness it verifies instance connectivity @@ -259,8 +262,13 @@ func cloudsqlProxyContainerArgs(secretName, project, region, instanceName string Path: sdk.String("/liveness"), Port: sdk.String("csql-hc"), }, - PeriodSeconds: sdk.IntPtr(10), - TimeoutSeconds: sdk.IntPtr(3), + PeriodSeconds: sdk.IntPtr(10), + // Same 10s budget as readiness: /liveness is served by the same health + // server, and under the exact starvation the readiness change tolerates, + // a 3s liveness timeout would RESTART the sidecar (dropping every live + // DB connection) — strictly worse than an endpoint drop. A genuinely + // hung process still fails a 10s timeout, so deadlock recovery holds. + TimeoutSeconds: sdk.IntPtr(10), FailureThreshold: sdk.IntPtr(3), } return container diff --git a/pkg/clouds/pulumi/gcp/cloudsql_proxy_test.go b/pkg/clouds/pulumi/gcp/cloudsql_proxy_test.go index 3bb4c8da..f2874ef6 100644 --- a/pkg/clouds/pulumi/gcp/cloudsql_proxy_test.go +++ b/pkg/clouds/pulumi/gcp/cloudsql_proxy_test.go @@ -144,7 +144,7 @@ func TestAttachCloudsqlProxyAsNativeSidecar_LandsInInitContainers(t *testing.T) // endpoints during Autopilot node consolidation (sidecar readiness // gates pod readiness, KEP-753). Generous timeout + bigger CPU request keep // transient starvation from dropping the whole pod out of rotation. -func TestCloudsqlProxyContainerArgs_ReadinessTolerantToStarvation(t *testing.T) { +func TestCloudsqlProxyContainerArgs_ProbesTolerantToStarvation(t *testing.T) { c := cloudsqlProxyContainerArgs("creds", "proj", "reg", "inst", 0) rp := c.ReadinessProbe.(*v1.ProbeArgs) @@ -152,6 +152,16 @@ func TestCloudsqlProxyContainerArgs_ReadinessTolerantToStarvation(t *testing.T) assert.Equal(t, sdk.IntPtr(10), rp.PeriodSeconds) assert.Equal(t, sdk.IntPtr(3), rp.FailureThreshold) + lp := c.LivenessProbe.(*v1.ProbeArgs) + assert.Equal(t, sdk.IntPtr(10), lp.TimeoutSeconds, "liveness shares the starved health server; 3s would restart the sidecar under the exact pressure readiness now tolerates") + assert.Equal(t, sdk.IntPtr(10), lp.PeriodSeconds) + assert.Equal(t, sdk.IntPtr(3), lp.FailureThreshold) + + sp := c.StartupProbe.(*v1.ProbeArgs) + assert.Equal(t, sdk.IntPtr(2), sp.PeriodSeconds) + assert.Equal(t, sdk.IntPtr(3), sp.TimeoutSeconds, "startup stays tight deliberately: nothing is connected yet") + assert.Equal(t, sdk.IntPtr(30), sp.FailureThreshold) + res := c.Resources.(*v1.ResourceRequirementsArgs) req := res.Requests.(sdk.StringMap) assert.Equal(t, sdk.String("100m"), req["cpu"]) diff --git a/pkg/clouds/pulumi/kubernetes/simple_container.go b/pkg/clouds/pulumi/kubernetes/simple_container.go index 290aaccc..877ba446 100644 --- a/pkg/clouds/pulumi/kubernetes/simple_container.go +++ b/pkg/clouds/pulumi/kubernetes/simple_container.go @@ -12,6 +12,7 @@ import ( "strconv" "strings" + "github.com/dustin/go-humanize" "github.com/pkg/errors" "github.com/samber/lo" @@ -38,6 +39,13 @@ var Caddyconfig embed.FS const ( AppTypeSimpleContainer = "simple-container" + // defaultRequestBufferBytes is the default reverse_proxy request_buffers + // size (see api.SimpleContainerLBConfig.RequestBufferSize for the contract); + // maxRequestBufferBytes caps per-stack overrides because the buffer is held + // in shared-edge memory per in-flight request. + defaultRequestBufferBytes = 1 << 20 // 1MiB + maxRequestBufferBytes = 16 << 20 // 16MiB + AnnotationCaddyfileEntry = "simple-container.com/caddyfile-entry" AnnotationParentStack = "simple-container.com/parent-stack" AnnotationDomain = "simple-container.com/domain" @@ -705,8 +713,7 @@ func NewSimpleContainer(ctx *sdk.Context, args *SimpleContainerArgs, opts ...sdk // e.g. a catch-all `respond` directive emitted earlier. caddyfileEntryTemplate = ` ${proto}://${domain} { - reverse_proxy http://${service}.${namespace}.svc.cluster.local:${port} { - request_buffers ${requestBuffers} + reverse_proxy http://${service}.${namespace}.svc.cluster.local:${port} {${requestBuffers} header_down Server nginx ${addHeaders} import handle_server_error ${extraHelpers} @@ -717,8 +724,7 @@ ${proto}://${domain} { } else if args.Prefix != "" { caddyfileEntryTemplate = ` handle_path /${prefix}* {${additionalProxyConfig} - reverse_proxy http://${service}.${namespace}.svc.cluster.local:${port} { - request_buffers ${requestBuffers} + reverse_proxy http://${service}.${namespace}.svc.cluster.local:${port} {${requestBuffers} header_down Server nginx ${addHeaders} import handle_server_error ${extraHelpers} @@ -766,22 +772,41 @@ ${proto}://${domain} { } // Buffer request bodies so upstreams receive Content-Length instead of // Transfer-Encoding: chunked — WSGI apps (Django #28668) read an empty - // body on chunked requests: webhook 500s, HMAC signature failures over - // the empty body, lost CSRF tokens. 1MiB covers webhook/API/form bodies; - // larger bodies keep streaming exactly as before. - requestBuffersStr := "1MiB" - if v := lo.FromPtr(lo.FromPtr(args.LbConfig).RequestBufferSize); v != "" { - requestBuffersStr = v + // body on chunked requests (see api.SimpleContainerLBConfig.RequestBufferSize + // for the full contract). The placeholder carries the WHOLE directive + // line: user input is parsed and re-rendered as a byte count so nothing + // user-controlled reaches the shared Caddyfile, and a parsed 0 omits + // the line entirely (escape hatch for pre-2.6.0 Caddy images). + lbc := lo.FromPtr(args.LbConfig) + requestBufferBytes := uint64(defaultRequestBufferBytes) + if v := lbc.RequestBufferSize; v != "" { + n, err := humanize.ParseBytes(v) + if err != nil { + return nil, errors.Wrapf(err, "invalid lbConfig.requestBufferSize %q", v) + } + if n > maxRequestBufferBytes { + return nil, errors.Errorf("lbConfig.requestBufferSize %q exceeds the %s cap (buffered in shared edge memory per in-flight request)", v, humanize.IBytes(maxRequestBufferBytes)) + } + requestBufferBytes = n + } + requestBuffersStr := "" + if requestBufferBytes > 0 { + requestBuffersStr = fmt.Sprintf("\n request_buffers %d", requestBufferBytes) + } + for _, h := range lbc.ExtraHelpers { + if strings.Contains(h, "request_buffers") { + args.Log.Warn(ctx.Context(), "lbConfig.extraHelpers contains a request_buffers directive; it overrides requestBufferSize (Caddy last-wins) — drop the workaround and use lbConfig.requestBufferSize") + } } placeholdersMap := placeholders.MapData{ - "proto": lo.If(lo.FromPtr(args.LbConfig).Https, "https").Else("http"), + "proto": lo.If(lbc.Https, "https").Else("http"), "domain": args.Domain, "prefix": args.Prefix, "service": sanitizedService, "namespace": sanitizedNamespace, "port": strconv.Itoa(lo.FromPtr(mainPort)), "addHeaders": addHeadersStr, - "extraHelpers": strings.Join(lo.FromPtr(args.LbConfig).ExtraHelpers, "\n "), + "extraHelpers": strings.Join(lbc.ExtraHelpers, "\n "), "imports": strings.Join(imports, "\n "), "siteExtraHelpers": siteExtraHelpersStr, "requestBuffers": requestBuffersStr, diff --git a/pkg/clouds/pulumi/kubernetes/simple_container_test.go b/pkg/clouds/pulumi/kubernetes/simple_container_test.go index 5932d28c..7ef2adff 100644 --- a/pkg/clouds/pulumi/kubernetes/simple_container_test.go +++ b/pkg/clouds/pulumi/kubernetes/simple_container_test.go @@ -763,44 +763,107 @@ func TestNewSimpleContainer_MinimalConfiguration(t *testing.T) { // request_buffers Rendering Tests // -// Regression coverage: without request buffering -// Caddy forwards chunked request bodies as-is, and WSGI upstreams (Django, -// django ticket #28668) read an empty request.body — webhook 500s, HMAC -// signature failures over the empty body, "lost" CSRF tokens. The rendered -// reverse_proxy block must always carry request_buffers so bodies up to the -// buffer size reach upstreams with Content-Length. +// Regression coverage: without request buffering Caddy forwards chunked +// request bodies as-is and WSGI upstreams (Django #28668) read an empty +// request.body. The rendered reverse_proxy block must carry request_buffers +// (as a pure byte count — user input must never reach the shared Caddyfile +// verbatim), and "0" must omit the directive entirely. func TestCaddyfileEntry_RequestBuffersDefault(t *testing.T) { RegisterTestingT(t) entry := caddyfileEntryFor(t, createBasicTestArgs()) - Expect(entry).To(ContainSubstring("request_buffers 1MiB")) + Expect(entry).To(MatchRegexp(`(?m)reverse_proxy [^{]+\{\n\s+request_buffers 1048576$`), + "default 1MiB must render as a byte count on the first line inside reverse_proxy, got:\n%s", entry) } -func TestCaddyfileEntry_RequestBuffersOverride(t *testing.T) { +func TestCaddyfileEntry_RequestBuffersDefaultWithLbConfigSet(t *testing.T) { RegisterTestingT(t) + // The dominant consumer shape: lbConfig present (extraHelpers), size unset. args := createBasicTestArgs() args.LbConfig = &api.SimpleContainerLBConfig{ - RequestBufferSize: lo.ToPtr("4MiB"), + ExtraHelpers: []string{"lb_retries 2"}, } entry := caddyfileEntryFor(t, args) - Expect(entry).To(ContainSubstring("request_buffers 4MiB")) - Expect(entry).ToNot(ContainSubstring("request_buffers 1MiB")) + Expect(entry).To(MatchRegexp(`(?m)^\s+request_buffers 1048576$`)) } -func TestCaddyfileEntry_RequestBuffersDisabled(t *testing.T) { +func TestCaddyfileEntry_RequestBuffersOnPrefixTemplate(t *testing.T) { + RegisterTestingT(t) + + args := createBasicTestArgs() + args.Domain = "" + args.Prefix = "api" + + entry := caddyfileEntryFor(t, args) + + Expect(entry).To(MatchRegexp(`(?m)reverse_proxy [^{]+\{\n\s+request_buffers 1048576$`), + "the handle_path template variant must buffer too, got:\n%s", entry) +} + +func TestCaddyfileEntry_RequestBuffersOverrideNormalized(t *testing.T) { RegisterTestingT(t) args := createBasicTestArgs() args.LbConfig = &api.SimpleContainerLBConfig{ - RequestBufferSize: lo.ToPtr("0"), + RequestBufferSize: "4MiB", } entry := caddyfileEntryFor(t, args) - Expect(entry).To(ContainSubstring("request_buffers 0")) + Expect(entry).To(MatchRegexp(`(?m)^\s+request_buffers 4194304$`)) + Expect(entry).ToNot(ContainSubstring("request_buffers 1048576")) + Expect(entry).ToNot(ContainSubstring("4MiB"), "user text must not reach the Caddyfile") +} + +func TestCaddyfileEntry_RequestBuffersZeroOmitsDirective(t *testing.T) { + RegisterTestingT(t) + + args := createBasicTestArgs() + args.LbConfig = &api.SimpleContainerLBConfig{ + RequestBufferSize: "0", + } + + entry := caddyfileEntryFor(t, args) + + Expect(entry).ToNot(ContainSubstring("request_buffers"), + "\"0\" is the escape hatch for pre-2.6.0 Caddy: the directive must be absent, got:\n%s", entry) +} + +// caddyfileEntryErrFor mirrors caddyfileEntryFor for the error path. +func caddyfileEntryErrFor(t *testing.T, args *SimpleContainerArgs) error { + t.Helper() + mocks := NewSimpleContainerMocks() + return pulumi.RunErr(func(ctx *pulumi.Context) error { + _, err := NewSimpleContainer(ctx, args) + return err + }, pulumi.WithMocks("project", "stack", mocks)) +} + +func TestCaddyfileEntry_RequestBuffersInvalidValueFailsDeploy(t *testing.T) { + RegisterTestingT(t) + + for _, bad := range []string{"10potatoes", "1MiB\nrespond 403", "-1"} { + args := createBasicTestArgs() + args.LbConfig = &api.SimpleContainerLBConfig{RequestBufferSize: bad} + err := caddyfileEntryErrFor(t, args) + Expect(err).To(HaveOccurred(), "value %q must fail the offending stack's deploy, not reach the shared Caddyfile", bad) + Expect(err.Error()).To(ContainSubstring("requestBufferSize")) + } +} + +func TestCaddyfileEntry_RequestBuffersCapEnforced(t *testing.T) { + RegisterTestingT(t) + + args := createBasicTestArgs() + args.LbConfig = &api.SimpleContainerLBConfig{RequestBufferSize: "32MiB"} + + err := caddyfileEntryErrFor(t, args) + + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("cap")) }