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 7a9c37ce..c0ed76df 100644 --- a/pkg/api/client.go +++ b/pkg/api/client.go @@ -168,6 +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 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 87481bf9..c62502e4 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{ @@ -226,31 +228,47 @@ 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), } + // 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 (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 + // 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{ 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 ead9883f..f2874ef6 100644 --- a/pkg/clouds/pulumi/gcp/cloudsql_proxy_test.go +++ b/pkg/clouds/pulumi/gcp/cloudsql_proxy_test.go @@ -139,3 +139,30 @@ 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_ProbesTolerantToStarvation(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) + + 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 d32a8bfb..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,7 +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} { + reverse_proxy http://${service}.${namespace}.svc.cluster.local:${port} {${requestBuffers} header_down Server nginx ${addHeaders} import handle_server_error ${extraHelpers} @@ -716,7 +724,7 @@ ${proto}://${domain} { } else if args.Prefix != "" { caddyfileEntryTemplate = ` handle_path /${prefix}* {${additionalProxyConfig} - reverse_proxy http://${service}.${namespace}.svc.cluster.local:${port} { + reverse_proxy http://${service}.${namespace}.svc.cluster.local:${port} {${requestBuffers} header_down Server nginx ${addHeaders} import handle_server_error ${extraHelpers} @@ -762,17 +770,46 @@ ${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 (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, } 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..7ef2adff 100644 --- a/pkg/clouds/pulumi/kubernetes/simple_container_test.go +++ b/pkg/clouds/pulumi/kubernetes/simple_container_test.go @@ -760,3 +760,110 @@ 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 #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(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_RequestBuffersDefaultWithLbConfigSet(t *testing.T) { + RegisterTestingT(t) + + // The dominant consumer shape: lbConfig present (extraHelpers), size unset. + args := createBasicTestArgs() + args.LbConfig = &api.SimpleContainerLBConfig{ + ExtraHelpers: []string{"lb_retries 2"}, + } + + entry := caddyfileEntryFor(t, args) + + Expect(entry).To(MatchRegexp(`(?m)^\s+request_buffers 1048576$`)) +} + +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: "4MiB", + } + + entry := caddyfileEntryFor(t, args) + + 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")) +}