diff --git a/CHANGELOG.md b/CHANGELOG.md index e779d8f..40ee3c7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,12 @@ Two version streams move independently: ## [Unreleased] +### Added + +- Configurable timeout for upstream `GetObject` and `PutObject` operations via + `S3PROXY_S3_OPERATION_TIMEOUT`; defaults to 120 seconds and accepts values up + to 30 minutes. + ### Chart - **`chart/1.9.3`** — Dashboard usability + Go runtime panels. diff --git a/README.md b/README.md index 8efcbe4..d370645 100644 --- a/README.md +++ b/README.md @@ -89,6 +89,7 @@ All runtime configuration is sourced from environment variables. The Helm chart | `AWS_SECRET_ACCESS_KEY` | yes | — | Idem. | | `S3PROXY_THROTTLING_REQUESTSMAX` | no | `0` (off) | Cap on **concurrent in-flight requests** (not RPS). Excess requests are rejected. | | `S3PROXY_PUTBODY_MAX` | no | `268435456` (256 MiB) | Per-request PutObject body size ceiling, in bytes. Up to `5368709120` (5 GiB, the S3 hard cap). | +| `S3PROXY_S3_OPERATION_TIMEOUT` | no | `120` (2 min) | Upper bound, in seconds, on a single upstream GetObject/PutObject call. Raise this for large objects on slow links. Up to `1800` (30 min). | | `S3PROXY_DEKTAG_NAME` | no | `isec` | S3 object-metadata key used to store the encrypted DEK. | | `S3PROXY_DEKTAG_KEKVER` | no | `-kek-ver` | S3 object-metadata key recording which KEK derivation version wrapped the DEK. | | `S3PROXY_INSECURE` | no | unset | Set to `1` to use plain HTTP (not HTTPS) when talking to upstream. **Dev / e2e only.** Emits a loud warning at startup. | diff --git a/s3proxy/internal/config/config.go b/s3proxy/internal/config/config.go index 7c41a7d..6d7d5a5 100644 --- a/s3proxy/internal/config/config.go +++ b/s3proxy/internal/config/config.go @@ -11,6 +11,7 @@ import ( "errors" "fmt" "strings" + "time" "github.com/knadh/koanf/providers/env" "github.com/knadh/koanf/v2" @@ -95,6 +96,21 @@ func (c *Config) MaxPutBodySize() int64 { return v } +// S3OperationTimeout returns the configured upper bound on a single S3 +// GetObject/PutObject call. Falls back to DefaultS3OperationTimeout when +// S3PROXY_S3_OPERATION_TIMEOUT (seconds) is unset, zero, or out of range. +func (c *Config) S3OperationTimeout() time.Duration { + const key = "s3proxy.s3.operation.timeout" + if !c.k.Exists(key) { + return DefaultS3OperationTimeout + } + v := time.Duration(c.k.Int64(key)) * time.Second + if v <= 0 || v > MaxS3OperationTimeout { + return DefaultS3OperationTimeout + } + return v +} + // DecryptionFallback reports whether GetObject should retry decryption with an // all-zero KEK when the configured KEK fails (S3PROXY_DECRYPTION_FALLBACK=1). // Intended for one-shot migrations away from objects written without an @@ -155,5 +171,8 @@ func GetMaxPutBodySize() int64 { return Default().MaxPutBodySize() } // GetInsecure returns whether upstream S3 traffic should use http:// from the default config. func GetInsecure() bool { return Default().Insecure() } +// GetS3OperationTimeout returns the S3 operation timeout from the default config. +func GetS3OperationTimeout() time.Duration { return Default().S3OperationTimeout() } + // GetDecryptionFallback returns the decryption-fallback flag from the default config. func GetDecryptionFallback() bool { return Default().DecryptionFallback() } diff --git a/s3proxy/internal/config/validation.go b/s3proxy/internal/config/validation.go index 48ea508..edb56a9 100644 --- a/s3proxy/internal/config/validation.go +++ b/s3proxy/internal/config/validation.go @@ -4,6 +4,7 @@ import ( "errors" "fmt" "regexp" + "time" ) // Validate asserts that the required s3proxy configuration is present and well-formed. @@ -93,6 +94,15 @@ const MaxObjectSize = 5 * 1024 * 1024 * 1024 // streaming encryption path is introduced. const DefaultMaxPutBodySize = 256 * 1024 * 1024 +// DefaultS3OperationTimeout is the default upper bound on a single S3 +// GetObject/PutObject call. Large objects on slow links can need more time; +// operators can raise this via S3PROXY_S3_OPERATION_TIMEOUT (seconds). +const DefaultS3OperationTimeout = 2 * time.Minute + +// MaxS3OperationTimeout bounds how high S3PROXY_S3_OPERATION_TIMEOUT can be set, +// so a misconfiguration cannot leave requests hanging indefinitely. +const MaxS3OperationTimeout = 30 * time.Minute + // ValidateContentLength validates the content length of a request against the S3 hard cap. func ValidateContentLength(contentLength int64) error { if contentLength < 0 { diff --git a/s3proxy/internal/config/validation_test.go b/s3proxy/internal/config/validation_test.go index 422c732..90a381a 100644 --- a/s3proxy/internal/config/validation_test.go +++ b/s3proxy/internal/config/validation_test.go @@ -9,7 +9,9 @@ package config import ( "strings" "testing" + "time" + "github.com/knadh/koanf/v2" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -104,3 +106,30 @@ func TestValidatePutBodySize(t *testing.T) { require.Error(t, err) assert.Contains(t, err.Error(), "exceeds PutObject body cap") } + +func TestS3OperationTimeout(t *testing.T) { + tests := map[string]struct { + value string + want time.Duration + }{ + "valid": {value: "300", want: 5 * time.Minute}, + "at maximum": {value: "1800", want: MaxS3OperationTimeout}, + "zero": {value: "0", want: DefaultS3OperationTimeout}, + "negative": {value: "-1", want: DefaultS3OperationTimeout}, + "over maximum": {value: "1801", want: DefaultS3OperationTimeout}, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + t.Setenv("S3PROXY_S3_OPERATION_TIMEOUT", tc.value) + cfg, err := Load() + require.NoError(t, err) + assert.Equal(t, tc.want, cfg.S3OperationTimeout()) + }) + } + + t.Run("unset", func(t *testing.T) { + cfg := &Config{k: koanf.New(".")} + assert.Equal(t, DefaultS3OperationTimeout, cfg.S3OperationTimeout()) + }) +} diff --git a/s3proxy/internal/router/object.go b/s3proxy/internal/router/object.go index d7be9d6..f0141fc 100644 --- a/s3proxy/internal/router/object.go +++ b/s3proxy/internal/router/object.go @@ -45,12 +45,6 @@ func releaseLargeBuffer(buf *[]byte) { } } -// s3OperationTimeout bounds the total duration of an individual S3 GetObject/PutObject call. -// We detach from the request context (context.WithoutCancel) so a client disconnect does not -// abort a partially-uploaded PutObject, but we still cap overall work to protect against -// hung upstreams producing zombie requests. -const s3OperationTimeout = 2 * time.Minute - // object bundles data to implement http.Handler methods that use data from incoming requests. type object struct { keks cryptoutil.KEKProvider @@ -84,7 +78,7 @@ func (o object) get(w http.ResponseWriter, r *http.Request) { // Detach from the request cancellation to avoid aborting an S3 operation when // the client disconnects mid-flight, but cap the total duration so a hung // upstream cannot produce a zombie request. - ctx, cancel := context.WithTimeout(context.WithoutCancel(r.Context()), s3OperationTimeout) + ctx, cancel := context.WithTimeout(context.WithoutCancel(r.Context()), config.GetS3OperationTimeout()) defer cancel() output, err := o.client.GetObject(ctx, o.bucket, o.key, o.versionID, o.sseCustomerAlgorithm, o.sseCustomerKey, o.sseCustomerKeyMD5) @@ -211,7 +205,7 @@ func (o object) put(w http.ResponseWriter, r *http.Request) { o.metadata[config.GetDekTagName()] = hex.EncodeToString(encryptedDEK) o.metadata[config.GetKEKVersionTagName()] = kekVersion - ctx, cancel := context.WithTimeout(context.WithoutCancel(r.Context()), s3OperationTimeout) + ctx, cancel := context.WithTimeout(context.WithoutCancel(r.Context()), config.GetS3OperationTimeout()) defer cancel() output, err := o.client.PutObject(ctx, o.bucket, o.key, o.tags, o.contentType, o.objectLockLegalHoldStatus, o.objectLockMode, o.sseCustomerAlgorithm, o.sseCustomerKey, o.sseCustomerKeyMD5, o.objectLockRetainUntilDate, o.metadata, ciphertext)