From ebd9774b3e269cefd577fc64c79b22f02e4f63f7 Mon Sep 17 00:00:00 2001 From: ehsanmsb Date: Thu, 29 Oct 2026 11:30:05 +0330 Subject: [PATCH 1/3] fix: bound storage response time and proxy idle time --- .github/workflows/verify.yml | 2 + README.md | 2 + charts/minigate/README.md | 5 +++ charts/minigate/templates/deployment.yaml | 2 +- charts/minigate/templates/httpproxy.yaml | 3 ++ charts/minigate/values.yaml | 4 ++ cmd/minigate/main.go | 6 ++- docs/deployment.md | 4 ++ internal/gateway/gateway.go | 28 +++++++++--- internal/gateway/gateway_test.go | 53 +++++++++++++++++++++-- 10 files changed, 98 insertions(+), 11 deletions(-) diff --git a/.github/workflows/verify.yml b/.github/workflows/verify.yml index 6c53a32..3fb2998 100644 --- a/.github/workflows/verify.yml +++ b/.github/workflows/verify.yml @@ -30,6 +30,8 @@ jobs: grep -q '^kind: Route$' /tmp/minigate-route.yaml helm template minigate charts/minigate --set existingSecret=minigate-config --set httpProxy.enabled=true --set httpProxy.fqdn=files.example.com --set httpProxy.tlsSecretName=files-tls > /tmp/minigate-httpproxy.yaml grep -q '^kind: HTTPProxy$' /tmp/minigate-httpproxy.yaml + grep -q 'response: "infinity"' /tmp/minigate-httpproxy.yaml + grep -q 'idle: "60s"' /tmp/minigate-httpproxy.yaml - if: github.event_name == 'pull_request' run: docker build -t minigate:ci . diff --git a/README.md b/README.md index 67ce98d..d272ec2 100644 --- a/README.md +++ b/README.md @@ -46,6 +46,8 @@ Prometheus metrics are served separately at `127.0.0.1:9090/metrics` by default. The service accepts `GET /{name}/{object-key}` and `GET /health`. Other methods return 405. A missing or inaccessible object returns 404; storage failures return 502. S3 can return `AccessDenied` for a missing object when credentials lack `ListBucket`, so MiniGate maps that error to 404. `Range` requests are forwarded to storage for partial downloads. S3 content metadata is forwarded in the response. +S3 response headers must arrive within 15 seconds per attempt. The entire object response, including retries and streaming, is limited to 5 minutes by default; set `-s3-timeout` to change that limit. A timeout before response headers returns 504. If streaming stalls after headers were sent, the connection ends and the failure is logged because its HTTP status cannot be changed. + Logs are JSON on standard output. `MINIGATE_LOG_LEVEL` accepts `DEBUG`, `INFO`, `WARN`, or `ERROR` (default `WARN`). Unexpected storage and server failures are logged by default. Set `INFO` to also see access denials and invalid ranges, or `DEBUG` for one compact record per object request, including successful reads and expected 404s. Health probes are not logged. Routine request logs omit object keys and credentials. HTTP routing uses [chi](https://github.com/go-chi/chi); storage access uses the official AWS SDK for Go v2. The HTTP server itself uses Go's standard `net/http` package. diff --git a/charts/minigate/README.md b/charts/minigate/README.md index 8557b79..8f674a0 100644 --- a/charts/minigate/README.md +++ b/charts/minigate/README.md @@ -34,6 +34,7 @@ To let Kubernetes adjust replica count, enable `autoscaling.enabled` and set `mi | `image.tag` | `""` | Uses the chart's `appVersion` when empty | | `imagePullSecrets` | `[]` | Credentials for a private registry | | `logLevel` | `WARN` | Application log level | +| `s3Timeout` | `5m` | Maximum time for one S3 response, including body streaming | | `service.port` | `80` | Internal HTTP Service port | | `service.metricsPort` | `9090` | Internal Prometheus Service port | | `ingress.enabled` | `false` | Expose file URLs through an Ingress | @@ -43,6 +44,8 @@ To let Kubernetes adjust replica count, enable `autoscaling.enabled` and set `mi | `httpProxy.enabled` | `false` | Create a Contour HTTPProxy | | `httpProxy.fqdn` | `""` | Required hostname when HTTPProxy is enabled | | `httpProxy.tlsSecretName` | `""` | Optional TLS Secret for HTTPProxy | +| `httpProxy.timeoutPolicy.response` | `infinity` | Permit downloads longer than Envoy's 15-second default | +| `httpProxy.timeoutPolicy.idle` | `60s` | Close a stream with no traffic for 60 seconds | | `serviceMonitor.enabled` | `false` | Create a Prometheus Operator ServiceMonitor | | `autoscaling.enabled` | `false` | Create a CPU-based HPA | | `podDisruptionBudget.enabled` | `false` | Create a PDB with `maxUnavailable: 1` | @@ -55,3 +58,5 @@ The chart also supports `nameOverride`, `fullnameOverride`, `podAnnotations`, `p Ingress sends traffic only to the file-serving port. Metrics stay on the internal Service. Enable ServiceMonitor only when its CRD is installed. The chart does not create or store S3 credentials. For OpenShift, see the [deployment guide](../../docs/deployment.md#openshift). HTTPProxy requires Contour and its `projectcontour.io/v1` CRD; it is not an OpenShift built-in. Enable only the exposure template your cluster uses. Route and HTTPProxy also send traffic only to the file-serving Service port. + +The app limits each S3 response to `s3Timeout`. For HTTPProxy, `response: infinity` avoids cutting off a valid large download after 15 seconds, while `idle: 60s` closes stalled streams. Adjust both values for your file sizes and network behavior. diff --git a/charts/minigate/templates/deployment.yaml b/charts/minigate/templates/deployment.yaml index 4bd4e0c..b4faeb1 100644 --- a/charts/minigate/templates/deployment.yaml +++ b/charts/minigate/templates/deployment.yaml @@ -36,7 +36,7 @@ spec: - name: minigate image: "{{ .Values.image.repository }}:{{ .Values.image.tag | default .Chart.AppVersion }}" imagePullPolicy: {{ .Values.image.pullPolicy }} - args: ["-config", "/etc/minigate/config.yaml", "-metrics-addr", ":9090"] + args: ["-config", "/etc/minigate/config.yaml", "-metrics-addr", ":9090", "-s3-timeout", {{ .Values.s3Timeout | quote }}] env: - name: MINIGATE_LOG_LEVEL value: {{ .Values.logLevel | quote }} diff --git a/charts/minigate/templates/httpproxy.yaml b/charts/minigate/templates/httpproxy.yaml index 736366e..18577f0 100644 --- a/charts/minigate/templates/httpproxy.yaml +++ b/charts/minigate/templates/httpproxy.yaml @@ -18,6 +18,9 @@ spec: routes: - conditions: - prefix: / + timeoutPolicy: + response: {{ .Values.httpProxy.timeoutPolicy.response | quote }} + idle: {{ .Values.httpProxy.timeoutPolicy.idle | quote }} services: - name: {{ include "minigate.fullname" . }} port: {{ .Values.service.port }} diff --git a/charts/minigate/values.yaml b/charts/minigate/values.yaml index b2dd8f4..0c65710 100644 --- a/charts/minigate/values.yaml +++ b/charts/minigate/values.yaml @@ -12,6 +12,7 @@ imagePullSecrets: [] # Create this Secret separately with a config.yaml key containing S3 credentials. existingSecret: "" logLevel: WARN +s3Timeout: 5m # Maximum time to receive one S3 object, including its body. serviceAccount: create: false @@ -56,6 +57,9 @@ httpProxy: enabled: false fqdn: "" tlsSecretName: "" # Optional TLS Secret in the release namespace. + timeoutPolicy: + response: infinity # Do not cut off long downloads after Envoy's 15s default. + idle: 60s # Close a stream with no traffic for this long. resources: requests: diff --git a/cmd/minigate/main.go b/cmd/minigate/main.go index 9b0ab71..d383684 100644 --- a/cmd/minigate/main.go +++ b/cmd/minigate/main.go @@ -24,7 +24,11 @@ func main() { configPath := flag.String("config", "config.yaml", "path to config file") addr := flag.String("addr", ":8080", "http listen address") metricsAddr := flag.String("metrics-addr", "127.0.0.1:9090", "metrics listen address") + s3Timeout := flag.Duration("s3-timeout", 5*time.Minute, "maximum time for one object response, including download") flag.Parse() + if *s3Timeout <= 0 { + log.Fatal("s3-timeout must be positive") + } level := slog.LevelWarn if value := os.Getenv("MINIGATE_LOG_LEVEL"); value != "" { if err := level.UnmarshalText([]byte(value)); err != nil { @@ -38,7 +42,7 @@ func main() { slog.Error("load config", "error", err) os.Exit(1) } - appHandler, metricsHandler := instrument(gateway.New(cfg), len(cfg.Buckets)) + appHandler, metricsHandler := instrument(gateway.New(cfg, *s3Timeout), len(cfg.Buckets)) server := &http.Server{ Addr: *addr, Handler: appHandler, diff --git a/docs/deployment.md b/docs/deployment.md index bfe8ddf..08df235 100644 --- a/docs/deployment.md +++ b/docs/deployment.md @@ -17,6 +17,8 @@ The chart requests `100m` CPU and `64Mi` memory and limits each container to `50 The chart defaults to `logLevel: WARN`, so routine requests do not fill the logs. Use `--set logLevel=INFO` to include access denials and invalid ranges, or `--set logLevel=DEBUG` to record every object request. Health probes stay silent. +The application waits at most 15 seconds per attempt for S3 response headers and 5 minutes in total for each object response, including streaming. Set `s3Timeout` in chart values if files need longer. A timeout before response headers returns 504; a stalled stream after headers is terminated and logged. + For a public hostname, configure your ingress controller and TLS Secret, then enable the optional Ingress: ```sh @@ -63,3 +65,5 @@ helm upgrade --install minigate ./charts/minigate \ ``` The `files-tls` Secret must contain a certificate for `files.example.com`. Leave `httpProxy.tlsSecretName` empty only if HTTP is intentionally served behind another TLS edge. Enable only the exposure template used by your cluster; Ingress, Route, and HTTPProxy are all disabled by default. + +When HTTPProxy is enabled, its route uses `timeoutPolicy.response: infinity` and `timeoutPolicy.idle: 60s`. The response setting prevents Contour's 15-second total response default from cutting off large downloads. The idle setting closes a stream that stops transferring data. The application-level `s3Timeout` still sets the overall transfer limit. diff --git a/internal/gateway/gateway.go b/internal/gateway/gateway.go index ed76267..536f299 100644 --- a/internal/gateway/gateway.go +++ b/internal/gateway/gateway.go @@ -5,12 +5,15 @@ import ( "errors" "io" "log/slog" + "net" "net/http" "net/url" "strconv" "strings" + "time" "github.com/aws/aws-sdk-go-v2/aws" + awshttp "github.com/aws/aws-sdk-go-v2/aws/transport/http" "github.com/aws/aws-sdk-go-v2/credentials" "github.com/aws/aws-sdk-go-v2/service/s3" "github.com/aws/smithy-go" @@ -30,16 +33,21 @@ type route struct { } type Handler struct { - routes map[string]route - router http.Handler + routes map[string]route + router http.Handler + timeout time.Duration } -func New(cfg *config.Config) *Handler { +func New(cfg *config.Config, timeout time.Duration) *Handler { routes := make(map[string]route, len(cfg.Buckets)) + httpClient := awshttp.NewBuildableClient().WithTransportOptions(func(t *http.Transport) { + t.ResponseHeaderTimeout = min(timeout, 15*time.Second) + }) for _, b := range cfg.Buckets { awsCfg := aws.Config{ Region: b.Region, Credentials: aws.NewCredentialsCache(credentials.NewStaticCredentialsProvider(b.AccessKey, b.SecretKey, "")), + HTTPClient: httpClient, } client := s3.NewFromConfig(awsCfg, func(o *s3.Options) { o.BaseEndpoint = aws.String(b.Endpoint) @@ -47,7 +55,7 @@ func New(cfg *config.Config) *Handler { }) routes[b.Name] = route{bucket: b.Bucket, allowedPaths: b.AllowedPaths, client: client} } - h := &Handler{routes: routes} + h := &Handler{routes: routes, timeout: timeout} r := chi.NewRouter() r.Use(middleware.SetHeader("Access-Control-Allow-Origin", "*")) r.Use(middleware.Recoverer) @@ -104,7 +112,9 @@ func (h *Handler) getObject(w http.ResponseWriter, r *http.Request) { if value := r.Header.Get("Range"); strings.HasPrefix(value, "bytes=") && r.Header.Get("If-Range") == "" { input.Range = aws.String(value) } - obj, err := selected.client.GetObject(r.Context(), input) + ctx, cancel := context.WithTimeout(r.Context(), h.timeout) + defer cancel() + obj, err := selected.client.GetObject(ctx, input) if err != nil { var apiErr smithy.APIError if errors.As(err, &apiErr) { @@ -124,6 +134,12 @@ func (h *Handler) getObject(w http.ResponseWriter, r *http.Request) { return } } + var netErr net.Error + if errors.Is(err, context.DeadlineExceeded) || (errors.As(err, &netErr) && netErr.Timeout()) { + slog.Error("request", "status", http.StatusGatewayTimeout, "reason", "storage_timeout", "route", name, "error", err) + http.Error(w, "storage timeout", http.StatusGatewayTimeout) + return + } slog.Error("request", "status", http.StatusBadGateway, "reason", "storage_error", "route", name, "error", err) http.Error(w, "storage unavailable", http.StatusBadGateway) return @@ -157,6 +173,8 @@ func (h *Handler) getObject(w http.ResponseWriter, r *http.Request) { if err != nil { if r.Context().Err() != nil { slog.Debug("request", "status", status, "reason", "client_disconnected", "route", name, "bytes", bytes) + } else if ctx.Err() == context.DeadlineExceeded { + slog.Error("request", "status", status, "reason", "stream_timeout", "route", name, "bytes", bytes) } else { slog.Error("request", "status", status, "reason", "stream_error", "route", name, "bytes", bytes, "error", err) } diff --git a/internal/gateway/gateway_test.go b/internal/gateway/gateway_test.go index 6539206..e3f1c8d 100644 --- a/internal/gateway/gateway_test.go +++ b/internal/gateway/gateway_test.go @@ -11,6 +11,7 @@ import ( "net/http/httptest" "strings" "testing" + "time" "github.com/aws/aws-sdk-go-v2/aws" "github.com/aws/aws-sdk-go-v2/service/s3" @@ -35,7 +36,7 @@ func testHandler(f *fakeStorage) *Handler { h := New(&config.Config{Buckets: []config.Bucket{{ Name: "avatars", Bucket: "private-images", Endpoint: "https://storage.example.com", Region: "us-east-1", AccessKey: "access", SecretKey: "secret", AllowedPaths: []string{"public"}, - }}}) + }}}, 5*time.Minute) r := h.routes["avatars"] r.client = f h.routes["avatars"] = r @@ -96,7 +97,7 @@ func TestAllowedPathsExposeOnlySelectedDirectories(t *testing.T) { Name: "avatars", Bucket: "private-images", Endpoint: "https://storage.example.com", Region: "us-east-1", AccessKey: "access", SecretKey: "secret", AllowedPaths: []string{"photos/public", "docs/shared"}, - }}}) + }}}, 5*time.Minute) r := h.routes["avatars"] r.client = f h.routes["avatars"] = r @@ -252,7 +253,7 @@ func TestConfiguredEndpointUsesPathStyle(t *testing.T) { h := New(&config.Config{Buckets: []config.Bucket{{ Name: "public", Bucket: "private-files", Endpoint: upstream.URL, Region: "us-east-1", AccessKey: "access", SecretKey: "secret", - }}}) + }}}, 5*time.Minute) w := httptest.NewRecorder() h.ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/public/folder/file.txt", nil)) if w.Code != http.StatusOK || w.Body.String() != "file" || upstreamPath != "/private-files/folder/file.txt" { @@ -268,10 +269,54 @@ func TestGenericStorage404ReturnsNotFound(t *testing.T) { h := New(&config.Config{Buckets: []config.Bucket{{ Name: "public", Bucket: "private-files", Endpoint: upstream.URL, Region: "us-east-1", AccessKey: "access", SecretKey: "secret", - }}}) + }}}, 5*time.Minute) w := httptest.NewRecorder() h.ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/public/missing.txt", nil)) if w.Code != http.StatusNotFound { t.Errorf("status=%d want=404", w.Code) } } + +func TestStorageTimeout(t *testing.T) { + for _, tc := range []struct { + name string + upstream http.HandlerFunc + wantStatus int + wantBody string + }{ + { + name: "before headers", + upstream: func(w http.ResponseWriter, r *http.Request) { + time.Sleep(time.Second) + _, _ = io.WriteString(w, "late") + }, + wantStatus: http.StatusGatewayTimeout, + }, + { + name: "during body", + upstream: func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Length", "2") + _, _ = io.WriteString(w, "a") + w.(http.Flusher).Flush() + time.Sleep(time.Second) + _, _ = io.WriteString(w, "b") + }, + wantStatus: http.StatusOK, + wantBody: "a", + }, + } { + t.Run(tc.name, func(t *testing.T) { + upstream := httptest.NewServer(tc.upstream) + defer upstream.Close() + h := New(&config.Config{Buckets: []config.Bucket{{ + Name: "public", Bucket: "private-files", Endpoint: upstream.URL, + Region: "us-east-1", AccessKey: "access", SecretKey: "secret", + }}}, 250*time.Millisecond) + w := httptest.NewRecorder() + h.ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/public/file.txt", nil)) + if w.Code != tc.wantStatus || (tc.wantBody != "" && w.Body.String() != tc.wantBody) { + t.Fatalf("status=%d body=%q", w.Code, w.Body.String()) + } + }) + } +} From 8cde3408c6b9f8dc7f1fdbde582e29c586b33838 Mon Sep 17 00:00:00 2001 From: ehsanmsb Date: Thu, 29 Oct 2026 12:07:27 +0330 Subject: [PATCH 2/3] fix: shorten object delivery timeouts for frontend use --- .github/workflows/verify.yml | 3 +-- README.md | 2 +- charts/minigate/README.md | 7 +++---- charts/minigate/templates/httpproxy.yaml | 1 - charts/minigate/values.yaml | 5 ++--- cmd/minigate/main.go | 2 +- docs/deployment.md | 4 ++-- internal/gateway/gateway.go | 2 +- internal/gateway/gateway_test.go | 8 ++++---- 9 files changed, 15 insertions(+), 19 deletions(-) diff --git a/.github/workflows/verify.yml b/.github/workflows/verify.yml index 3fb2998..281249f 100644 --- a/.github/workflows/verify.yml +++ b/.github/workflows/verify.yml @@ -30,8 +30,7 @@ jobs: grep -q '^kind: Route$' /tmp/minigate-route.yaml helm template minigate charts/minigate --set existingSecret=minigate-config --set httpProxy.enabled=true --set httpProxy.fqdn=files.example.com --set httpProxy.tlsSecretName=files-tls > /tmp/minigate-httpproxy.yaml grep -q '^kind: HTTPProxy$' /tmp/minigate-httpproxy.yaml - grep -q 'response: "infinity"' /tmp/minigate-httpproxy.yaml - grep -q 'idle: "60s"' /tmp/minigate-httpproxy.yaml + grep -q 'response: "20s"' /tmp/minigate-httpproxy.yaml - if: github.event_name == 'pull_request' run: docker build -t minigate:ci . diff --git a/README.md b/README.md index d272ec2..6ccc8a6 100644 --- a/README.md +++ b/README.md @@ -46,7 +46,7 @@ Prometheus metrics are served separately at `127.0.0.1:9090/metrics` by default. The service accepts `GET /{name}/{object-key}` and `GET /health`. Other methods return 405. A missing or inaccessible object returns 404; storage failures return 502. S3 can return `AccessDenied` for a missing object when credentials lack `ListBucket`, so MiniGate maps that error to 404. `Range` requests are forwarded to storage for partial downloads. S3 content metadata is forwarded in the response. -S3 response headers must arrive within 15 seconds per attempt. The entire object response, including retries and streaming, is limited to 5 minutes by default; set `-s3-timeout` to change that limit. A timeout before response headers returns 504. If streaming stalls after headers were sent, the connection ends and the failure is logged because its HTTP status cannot be changed. +S3 response headers must arrive within 5 seconds per attempt. The entire object response, including retries and streaming, is limited to 15 seconds by default; set `-s3-timeout` to change that limit. A timeout before response headers returns 504. If streaming stalls after headers were sent, the connection ends and the failure is logged because its HTTP status cannot be changed. Logs are JSON on standard output. `MINIGATE_LOG_LEVEL` accepts `DEBUG`, `INFO`, `WARN`, or `ERROR` (default `WARN`). Unexpected storage and server failures are logged by default. Set `INFO` to also see access denials and invalid ranges, or `DEBUG` for one compact record per object request, including successful reads and expected 404s. Health probes are not logged. Routine request logs omit object keys and credentials. diff --git a/charts/minigate/README.md b/charts/minigate/README.md index 8f674a0..050cb7b 100644 --- a/charts/minigate/README.md +++ b/charts/minigate/README.md @@ -34,7 +34,7 @@ To let Kubernetes adjust replica count, enable `autoscaling.enabled` and set `mi | `image.tag` | `""` | Uses the chart's `appVersion` when empty | | `imagePullSecrets` | `[]` | Credentials for a private registry | | `logLevel` | `WARN` | Application log level | -| `s3Timeout` | `5m` | Maximum time for one S3 response, including body streaming | +| `s3Timeout` | `15s` | Maximum time for one S3 response, including body streaming | | `service.port` | `80` | Internal HTTP Service port | | `service.metricsPort` | `9090` | Internal Prometheus Service port | | `ingress.enabled` | `false` | Expose file URLs through an Ingress | @@ -44,8 +44,7 @@ To let Kubernetes adjust replica count, enable `autoscaling.enabled` and set `mi | `httpProxy.enabled` | `false` | Create a Contour HTTPProxy | | `httpProxy.fqdn` | `""` | Required hostname when HTTPProxy is enabled | | `httpProxy.tlsSecretName` | `""` | Optional TLS Secret for HTTPProxy | -| `httpProxy.timeoutPolicy.response` | `infinity` | Permit downloads longer than Envoy's 15-second default | -| `httpProxy.timeoutPolicy.idle` | `60s` | Close a stream with no traffic for 60 seconds | +| `httpProxy.timeoutPolicy.response` | `20s` | Total response limit, slightly longer than the app's default | | `serviceMonitor.enabled` | `false` | Create a Prometheus Operator ServiceMonitor | | `autoscaling.enabled` | `false` | Create a CPU-based HPA | | `podDisruptionBudget.enabled` | `false` | Create a PDB with `maxUnavailable: 1` | @@ -59,4 +58,4 @@ Ingress sends traffic only to the file-serving port. Metrics stay on the interna For OpenShift, see the [deployment guide](../../docs/deployment.md#openshift). HTTPProxy requires Contour and its `projectcontour.io/v1` CRD; it is not an OpenShift built-in. Enable only the exposure template your cluster uses. Route and HTTPProxy also send traffic only to the file-serving Service port. -The app limits each S3 response to `s3Timeout`. For HTTPProxy, `response: infinity` avoids cutting off a valid large download after 15 seconds, while `idle: 60s` closes stalled streams. Adjust both values for your file sizes and network behavior. +The app limits each S3 response to `s3Timeout`. For HTTPProxy, the 20-second response timeout gives the app time to return its own 504 after 15 seconds. If you increase `s3Timeout`, increase `httpProxy.timeoutPolicy.response` too. diff --git a/charts/minigate/templates/httpproxy.yaml b/charts/minigate/templates/httpproxy.yaml index 18577f0..3db1e7b 100644 --- a/charts/minigate/templates/httpproxy.yaml +++ b/charts/minigate/templates/httpproxy.yaml @@ -20,7 +20,6 @@ spec: - prefix: / timeoutPolicy: response: {{ .Values.httpProxy.timeoutPolicy.response | quote }} - idle: {{ .Values.httpProxy.timeoutPolicy.idle | quote }} services: - name: {{ include "minigate.fullname" . }} port: {{ .Values.service.port }} diff --git a/charts/minigate/values.yaml b/charts/minigate/values.yaml index 0c65710..ca6edf7 100644 --- a/charts/minigate/values.yaml +++ b/charts/minigate/values.yaml @@ -12,7 +12,7 @@ imagePullSecrets: [] # Create this Secret separately with a config.yaml key containing S3 credentials. existingSecret: "" logLevel: WARN -s3Timeout: 5m # Maximum time to receive one S3 object, including its body. +s3Timeout: 15s # Maximum time to receive one S3 object, including its body. serviceAccount: create: false @@ -58,8 +58,7 @@ httpProxy: fqdn: "" tlsSecretName: "" # Optional TLS Secret in the release namespace. timeoutPolicy: - response: infinity # Do not cut off long downloads after Envoy's 15s default. - idle: 60s # Close a stream with no traffic for this long. + response: 20s # Slightly longer than the app's 15s limit, so it can return 504. resources: requests: diff --git a/cmd/minigate/main.go b/cmd/minigate/main.go index d383684..be95feb 100644 --- a/cmd/minigate/main.go +++ b/cmd/minigate/main.go @@ -24,7 +24,7 @@ func main() { configPath := flag.String("config", "config.yaml", "path to config file") addr := flag.String("addr", ":8080", "http listen address") metricsAddr := flag.String("metrics-addr", "127.0.0.1:9090", "metrics listen address") - s3Timeout := flag.Duration("s3-timeout", 5*time.Minute, "maximum time for one object response, including download") + s3Timeout := flag.Duration("s3-timeout", 15*time.Second, "maximum time for one object response, including download") flag.Parse() if *s3Timeout <= 0 { log.Fatal("s3-timeout must be positive") diff --git a/docs/deployment.md b/docs/deployment.md index 08df235..7bbe300 100644 --- a/docs/deployment.md +++ b/docs/deployment.md @@ -17,7 +17,7 @@ The chart requests `100m` CPU and `64Mi` memory and limits each container to `50 The chart defaults to `logLevel: WARN`, so routine requests do not fill the logs. Use `--set logLevel=INFO` to include access denials and invalid ranges, or `--set logLevel=DEBUG` to record every object request. Health probes stay silent. -The application waits at most 15 seconds per attempt for S3 response headers and 5 minutes in total for each object response, including streaming. Set `s3Timeout` in chart values if files need longer. A timeout before response headers returns 504; a stalled stream after headers is terminated and logged. +The application waits at most 5 seconds per attempt for S3 response headers and 15 seconds in total for each object response, including streaming. Set `s3Timeout` in chart values if needed. A timeout before response headers returns 504; a stalled stream after headers is terminated and logged. For a public hostname, configure your ingress controller and TLS Secret, then enable the optional Ingress: @@ -66,4 +66,4 @@ helm upgrade --install minigate ./charts/minigate \ The `files-tls` Secret must contain a certificate for `files.example.com`. Leave `httpProxy.tlsSecretName` empty only if HTTP is intentionally served behind another TLS edge. Enable only the exposure template used by your cluster; Ingress, Route, and HTTPProxy are all disabled by default. -When HTTPProxy is enabled, its route uses `timeoutPolicy.response: infinity` and `timeoutPolicy.idle: 60s`. The response setting prevents Contour's 15-second total response default from cutting off large downloads. The idle setting closes a stream that stops transferring data. The application-level `s3Timeout` still sets the overall transfer limit. +When HTTPProxy is enabled, its route uses `timeoutPolicy.response: 20s`, slightly longer than the application's 15-second `s3Timeout`. This gives the app time to return 504 on a storage timeout. If you increase `s3Timeout`, increase the HTTPProxy response timeout as well. diff --git a/internal/gateway/gateway.go b/internal/gateway/gateway.go index 536f299..4bb7874 100644 --- a/internal/gateway/gateway.go +++ b/internal/gateway/gateway.go @@ -41,7 +41,7 @@ type Handler struct { func New(cfg *config.Config, timeout time.Duration) *Handler { routes := make(map[string]route, len(cfg.Buckets)) httpClient := awshttp.NewBuildableClient().WithTransportOptions(func(t *http.Transport) { - t.ResponseHeaderTimeout = min(timeout, 15*time.Second) + t.ResponseHeaderTimeout = min(timeout, 5*time.Second) }) for _, b := range cfg.Buckets { awsCfg := aws.Config{ diff --git a/internal/gateway/gateway_test.go b/internal/gateway/gateway_test.go index e3f1c8d..304dee7 100644 --- a/internal/gateway/gateway_test.go +++ b/internal/gateway/gateway_test.go @@ -36,7 +36,7 @@ func testHandler(f *fakeStorage) *Handler { h := New(&config.Config{Buckets: []config.Bucket{{ Name: "avatars", Bucket: "private-images", Endpoint: "https://storage.example.com", Region: "us-east-1", AccessKey: "access", SecretKey: "secret", AllowedPaths: []string{"public"}, - }}}, 5*time.Minute) + }}}, 15*time.Second) r := h.routes["avatars"] r.client = f h.routes["avatars"] = r @@ -97,7 +97,7 @@ func TestAllowedPathsExposeOnlySelectedDirectories(t *testing.T) { Name: "avatars", Bucket: "private-images", Endpoint: "https://storage.example.com", Region: "us-east-1", AccessKey: "access", SecretKey: "secret", AllowedPaths: []string{"photos/public", "docs/shared"}, - }}}, 5*time.Minute) + }}}, 15*time.Second) r := h.routes["avatars"] r.client = f h.routes["avatars"] = r @@ -253,7 +253,7 @@ func TestConfiguredEndpointUsesPathStyle(t *testing.T) { h := New(&config.Config{Buckets: []config.Bucket{{ Name: "public", Bucket: "private-files", Endpoint: upstream.URL, Region: "us-east-1", AccessKey: "access", SecretKey: "secret", - }}}, 5*time.Minute) + }}}, 15*time.Second) w := httptest.NewRecorder() h.ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/public/folder/file.txt", nil)) if w.Code != http.StatusOK || w.Body.String() != "file" || upstreamPath != "/private-files/folder/file.txt" { @@ -269,7 +269,7 @@ func TestGenericStorage404ReturnsNotFound(t *testing.T) { h := New(&config.Config{Buckets: []config.Bucket{{ Name: "public", Bucket: "private-files", Endpoint: upstream.URL, Region: "us-east-1", AccessKey: "access", SecretKey: "secret", - }}}, 5*time.Minute) + }}}, 15*time.Second) w := httptest.NewRecorder() h.ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/public/missing.txt", nil)) if w.Code != http.StatusNotFound { From 55faab81202ce20e36cd7794d86a47264d7915aa Mon Sep 17 00:00:00 2001 From: ehsanmsb Date: Thu, 29 Oct 2026 12:15:08 +0330 Subject: [PATCH 3/3] fix: allow video transfers while bounding storage waits --- .github/workflows/verify.yml | 3 ++- README.md | 2 +- charts/minigate/README.md | 7 ++++--- charts/minigate/templates/httpproxy.yaml | 1 + charts/minigate/values.yaml | 5 +++-- cmd/minigate/main.go | 2 +- docs/deployment.md | 4 ++-- internal/gateway/gateway_test.go | 8 ++++---- 8 files changed, 18 insertions(+), 14 deletions(-) diff --git a/.github/workflows/verify.yml b/.github/workflows/verify.yml index 281249f..3fb2998 100644 --- a/.github/workflows/verify.yml +++ b/.github/workflows/verify.yml @@ -30,7 +30,8 @@ jobs: grep -q '^kind: Route$' /tmp/minigate-route.yaml helm template minigate charts/minigate --set existingSecret=minigate-config --set httpProxy.enabled=true --set httpProxy.fqdn=files.example.com --set httpProxy.tlsSecretName=files-tls > /tmp/minigate-httpproxy.yaml grep -q '^kind: HTTPProxy$' /tmp/minigate-httpproxy.yaml - grep -q 'response: "20s"' /tmp/minigate-httpproxy.yaml + grep -q 'response: "infinity"' /tmp/minigate-httpproxy.yaml + grep -q 'idle: "60s"' /tmp/minigate-httpproxy.yaml - if: github.event_name == 'pull_request' run: docker build -t minigate:ci . diff --git a/README.md b/README.md index 6ccc8a6..1e253fc 100644 --- a/README.md +++ b/README.md @@ -46,7 +46,7 @@ Prometheus metrics are served separately at `127.0.0.1:9090/metrics` by default. The service accepts `GET /{name}/{object-key}` and `GET /health`. Other methods return 405. A missing or inaccessible object returns 404; storage failures return 502. S3 can return `AccessDenied` for a missing object when credentials lack `ListBucket`, so MiniGate maps that error to 404. `Range` requests are forwarded to storage for partial downloads. S3 content metadata is forwarded in the response. -S3 response headers must arrive within 5 seconds per attempt. The entire object response, including retries and streaming, is limited to 15 seconds by default; set `-s3-timeout` to change that limit. A timeout before response headers returns 504. If streaming stalls after headers were sent, the connection ends and the failure is logged because its HTTP status cannot be changed. +S3 response headers must arrive within 5 seconds per attempt. The entire object transfer, including retries and streaming, is limited to 10 minutes by default; set `-s3-timeout` to change that limit. This is a transfer ceiling, not a 10-minute wait before the first byte. Video players can use `Range` requests to fetch parts of a file. A timeout before response headers returns 504. If streaming stalls after headers were sent, the connection ends and the failure is logged because its HTTP status cannot be changed. Logs are JSON on standard output. `MINIGATE_LOG_LEVEL` accepts `DEBUG`, `INFO`, `WARN`, or `ERROR` (default `WARN`). Unexpected storage and server failures are logged by default. Set `INFO` to also see access denials and invalid ranges, or `DEBUG` for one compact record per object request, including successful reads and expected 404s. Health probes are not logged. Routine request logs omit object keys and credentials. diff --git a/charts/minigate/README.md b/charts/minigate/README.md index 050cb7b..693bfb3 100644 --- a/charts/minigate/README.md +++ b/charts/minigate/README.md @@ -34,7 +34,7 @@ To let Kubernetes adjust replica count, enable `autoscaling.enabled` and set `mi | `image.tag` | `""` | Uses the chart's `appVersion` when empty | | `imagePullSecrets` | `[]` | Credentials for a private registry | | `logLevel` | `WARN` | Application log level | -| `s3Timeout` | `15s` | Maximum time for one S3 response, including body streaming | +| `s3Timeout` | `10m` | Maximum transfer time for one object request | | `service.port` | `80` | Internal HTTP Service port | | `service.metricsPort` | `9090` | Internal Prometheus Service port | | `ingress.enabled` | `false` | Expose file URLs through an Ingress | @@ -44,7 +44,8 @@ To let Kubernetes adjust replica count, enable `autoscaling.enabled` and set `mi | `httpProxy.enabled` | `false` | Create a Contour HTTPProxy | | `httpProxy.fqdn` | `""` | Required hostname when HTTPProxy is enabled | | `httpProxy.tlsSecretName` | `""` | Optional TLS Secret for HTTPProxy | -| `httpProxy.timeoutPolicy.response` | `20s` | Total response limit, slightly longer than the app's default | +| `httpProxy.timeoutPolicy.response` | `infinity` | Let the app control total transfer time for videos | +| `httpProxy.timeoutPolicy.idle` | `60s` | Close streams with no traffic for 60 seconds | | `serviceMonitor.enabled` | `false` | Create a Prometheus Operator ServiceMonitor | | `autoscaling.enabled` | `false` | Create a CPU-based HPA | | `podDisruptionBudget.enabled` | `false` | Create a PDB with `maxUnavailable: 1` | @@ -58,4 +59,4 @@ Ingress sends traffic only to the file-serving port. Metrics stay on the interna For OpenShift, see the [deployment guide](../../docs/deployment.md#openshift). HTTPProxy requires Contour and its `projectcontour.io/v1` CRD; it is not an OpenShift built-in. Enable only the exposure template your cluster uses. Route and HTTPProxy also send traffic only to the file-serving Service port. -The app limits each S3 response to `s3Timeout`. For HTTPProxy, the 20-second response timeout gives the app time to return its own 504 after 15 seconds. If you increase `s3Timeout`, increase `httpProxy.timeoutPolicy.response` too. +The app limits each S3 response to `s3Timeout`, including active streaming. This does not delay the first byte; S3 response headers have a separate 5-second timeout per attempt. HTTPProxy allows long responses but closes streams idle for 60 seconds. Video players can use `Range` requests for partial downloads. diff --git a/charts/minigate/templates/httpproxy.yaml b/charts/minigate/templates/httpproxy.yaml index 3db1e7b..18577f0 100644 --- a/charts/minigate/templates/httpproxy.yaml +++ b/charts/minigate/templates/httpproxy.yaml @@ -20,6 +20,7 @@ spec: - prefix: / timeoutPolicy: response: {{ .Values.httpProxy.timeoutPolicy.response | quote }} + idle: {{ .Values.httpProxy.timeoutPolicy.idle | quote }} services: - name: {{ include "minigate.fullname" . }} port: {{ .Values.service.port }} diff --git a/charts/minigate/values.yaml b/charts/minigate/values.yaml index ca6edf7..76a2e45 100644 --- a/charts/minigate/values.yaml +++ b/charts/minigate/values.yaml @@ -12,7 +12,7 @@ imagePullSecrets: [] # Create this Secret separately with a config.yaml key containing S3 credentials. existingSecret: "" logLevel: WARN -s3Timeout: 15s # Maximum time to receive one S3 object, including its body. +s3Timeout: 10m # Maximum transfer time per object request; not time to first byte. serviceAccount: create: false @@ -58,7 +58,8 @@ httpProxy: fqdn: "" tlsSecretName: "" # Optional TLS Secret in the release namespace. timeoutPolicy: - response: 20s # Slightly longer than the app's 15s limit, so it can return 504. + response: infinity # Let the app control total transfer time for videos. + idle: 60s # Close streams that stop transferring data. resources: requests: diff --git a/cmd/minigate/main.go b/cmd/minigate/main.go index be95feb..8aa06ec 100644 --- a/cmd/minigate/main.go +++ b/cmd/minigate/main.go @@ -24,7 +24,7 @@ func main() { configPath := flag.String("config", "config.yaml", "path to config file") addr := flag.String("addr", ":8080", "http listen address") metricsAddr := flag.String("metrics-addr", "127.0.0.1:9090", "metrics listen address") - s3Timeout := flag.Duration("s3-timeout", 15*time.Second, "maximum time for one object response, including download") + s3Timeout := flag.Duration("s3-timeout", 10*time.Minute, "maximum time for one object response, including download") flag.Parse() if *s3Timeout <= 0 { log.Fatal("s3-timeout must be positive") diff --git a/docs/deployment.md b/docs/deployment.md index 7bbe300..c819ef7 100644 --- a/docs/deployment.md +++ b/docs/deployment.md @@ -17,7 +17,7 @@ The chart requests `100m` CPU and `64Mi` memory and limits each container to `50 The chart defaults to `logLevel: WARN`, so routine requests do not fill the logs. Use `--set logLevel=INFO` to include access denials and invalid ranges, or `--set logLevel=DEBUG` to record every object request. Health probes stay silent. -The application waits at most 5 seconds per attempt for S3 response headers and 15 seconds in total for each object response, including streaming. Set `s3Timeout` in chart values if needed. A timeout before response headers returns 504; a stalled stream after headers is terminated and logged. +The application waits at most 5 seconds per attempt for S3 response headers and allows up to 10 minutes for the complete object transfer. The 10-minute limit is for transfer, not for waiting before playback starts. Set `s3Timeout` in chart values if needed. A timeout before response headers returns 504; a stalled stream after headers is terminated and logged. For a public hostname, configure your ingress controller and TLS Secret, then enable the optional Ingress: @@ -66,4 +66,4 @@ helm upgrade --install minigate ./charts/minigate \ The `files-tls` Secret must contain a certificate for `files.example.com`. Leave `httpProxy.tlsSecretName` empty only if HTTP is intentionally served behind another TLS edge. Enable only the exposure template used by your cluster; Ingress, Route, and HTTPProxy are all disabled by default. -When HTTPProxy is enabled, its route uses `timeoutPolicy.response: 20s`, slightly longer than the application's 15-second `s3Timeout`. This gives the app time to return 504 on a storage timeout. If you increase `s3Timeout`, increase the HTTPProxy response timeout as well. +When HTTPProxy is enabled, its route uses `timeoutPolicy.response: infinity` so a video transfer is not cut off by Contour's 15-second total response default. `timeoutPolicy.idle: 60s` closes a connection that stops transferring data. The application's `s3Timeout` still caps each complete object request. diff --git a/internal/gateway/gateway_test.go b/internal/gateway/gateway_test.go index 304dee7..62c2cbe 100644 --- a/internal/gateway/gateway_test.go +++ b/internal/gateway/gateway_test.go @@ -36,7 +36,7 @@ func testHandler(f *fakeStorage) *Handler { h := New(&config.Config{Buckets: []config.Bucket{{ Name: "avatars", Bucket: "private-images", Endpoint: "https://storage.example.com", Region: "us-east-1", AccessKey: "access", SecretKey: "secret", AllowedPaths: []string{"public"}, - }}}, 15*time.Second) + }}}, 10*time.Minute) r := h.routes["avatars"] r.client = f h.routes["avatars"] = r @@ -97,7 +97,7 @@ func TestAllowedPathsExposeOnlySelectedDirectories(t *testing.T) { Name: "avatars", Bucket: "private-images", Endpoint: "https://storage.example.com", Region: "us-east-1", AccessKey: "access", SecretKey: "secret", AllowedPaths: []string{"photos/public", "docs/shared"}, - }}}, 15*time.Second) + }}}, 10*time.Minute) r := h.routes["avatars"] r.client = f h.routes["avatars"] = r @@ -253,7 +253,7 @@ func TestConfiguredEndpointUsesPathStyle(t *testing.T) { h := New(&config.Config{Buckets: []config.Bucket{{ Name: "public", Bucket: "private-files", Endpoint: upstream.URL, Region: "us-east-1", AccessKey: "access", SecretKey: "secret", - }}}, 15*time.Second) + }}}, 10*time.Minute) w := httptest.NewRecorder() h.ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/public/folder/file.txt", nil)) if w.Code != http.StatusOK || w.Body.String() != "file" || upstreamPath != "/private-files/folder/file.txt" { @@ -269,7 +269,7 @@ func TestGenericStorage404ReturnsNotFound(t *testing.T) { h := New(&config.Config{Buckets: []config.Bucket{{ Name: "public", Bucket: "private-files", Endpoint: upstream.URL, Region: "us-east-1", AccessKey: "access", SecretKey: "secret", - }}}, 15*time.Second) + }}}, 10*time.Minute) w := httptest.NewRecorder() h.ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/public/missing.txt", nil)) if w.Code != http.StatusNotFound {