carlos / platform Public

s3compat: one home for S3-compatible quirks; fix CAS on Ceph RGW (#92)

Hetzner Object Storage compares If-Match against its UNQUOTED internal
etag, so the quoted entity-tag form S3 returns — and that the AWS SDK
faithfully round-trips — never matches. Measured against the carlos
bucket, a single writer lost compare-and-swap 20 times out of 20:

  PUT If-None-Match: *  (absent)        -> OK
  PUT If-None-Match: *  (exists)        -> 412   correctly rejected
  PUT If-Match: "<correct etag>"        -> 412   <-- the bug
  PUT If-Match:  <correct etag>         -> OK
  PUT If-Match:  <wrong etag, unquoted> -> 412   correctly rejected
  DELETE If-Match: "<etag>"             -> OK    (quoted is fine here)

The fifth line is what makes the fix safe rather than reckless: the
condition IS evaluated, only the quoting form differs, so stripping the
quotes repairs compare-and-swap instead of silently disabling it. RFC 9110
entity-tags are quoted, so AWS is right and RGW is wrong; CARLOS still has
to interoperate.

Because create-only writes work, a fresh deployment looks healthy and only
REPLACES fail — which is every CAS in CARLOS: release pointers and
manifests, the ledger head, appconfig generations, provision records,
status snapshots, metrics rollups. On the private CARLOS box this showed up
as the box reporting itself UNREACHABLE on its own status page, ~68 CAS
conflicts every three hours.

It also defeats hibernation deterministically, which is worse than the
known spurious-403 wart. litestream's leaser renews at TTL/3 with
PUT If-Match, so a woken instance lost its lease at the FIRST renewal and
the activator stood it back down — every five minutes, not occasionally.
Confirmed by driving lss3.Leaser directly: Acquire OK (If-None-Match:*),
Renew FAILS ("lease not held"), Release OK.

New internal/s3compat owns both halves of "this store is not AWS":

- the universal settings (endpoint, path-style, when-required checksums),
  which release.NewS3Store and internal/lease had each duplicated;
- a QUIRKS TABLE, so adding a provider is one entry naming what it matches,
  what it changes, and the measurement that justifies it.

The If-Match fix is an SDK middleware in the BUILD step — after
serialization writes the header, before Finalize signs it. If-Match is a
signed header, so a later rewrite would produce SignatureDoesNotMatch
rather than a fix; a test asserts if-match is still in SignedHeaders.

Scoped deliberately:
- PutObject only. Conditional DELETE accepts the quoted form on the same
  store (it is how the lease releases today, and it works); a working path
  is not worth touching on an untested guess.
- Matched by hostname to the provider actually measured. CARLOS_S3_QUIRKS
  force-enables a quirk by name, so a new store can be tried without a
  rebuild.
- s3compat.Options() is a NO-OP without CARLOS_S3_ENDPOINT, so the
  flagship's stock-AWS path is untouched — including registering no
  middleware at all. release.NewS3Store keeps applying its checksum
  settings unconditionally, as it always has, rather than quietly changing
  behaviour on the one deployment that cannot be tested from here.

This is also the only way the fix reaches litestream: its leaser takes a
ready-made client via SetClient and has no provider handling of its own.
Its replica client needs nothing here (LTX writes are unconditional).
Worth reporting upstream separately.

Verified against the real Hetzner bucket, quirk firing on hostname alone:
- CAS replaces: 20/20 conflicts before -> 0/20 after, data landed.
- Leaser: Acquire + 3 consecutive renewals + Release all clean.

Tests: the RGW fake reproduces the exact wire behaviour, with the quirk-off
subtest asserting the failure so the quirk-on pass cannot be vacuous, plus
a stale-etag test proving CAS still rejects a genuine conflict.

go build/vet/test ./... clean, 21 packages; gofmt clean. go.mod: credentials
promoted indirect -> direct (a test imports it), same version, go.sum unchanged.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
James Adam 6a52ae09c2be7ade8a76e4cae5ce24ac938e8ab0 parent f300101
6 files changed, +610 −30
  • go.mod +1 −1
  • internal/lease/endpoint.go +12 −20
  • internal/release/store.go +12 −9
  • internal/release/store_rgw_test.go +169 −0
  • internal/s3compat/s3compat.go +181 −0
  • internal/s3compat/s3compat_test.go +235 −0
diff --git a/go.mod b/go.mod
index 6d1969096..2c15bd17c 100644
--- a/go.mod
+++ b/go.mod
@@ -5,6 +5,7 @@ go 1.26
require (
github.com/aws/aws-sdk-go-v2 v1.42.1
github.com/aws/aws-sdk-go-v2/config v1.32.6
+ github.com/aws/aws-sdk-go-v2/credentials v1.19.6
github.com/aws/aws-sdk-go-v2/service/route53 v1.64.1
github.com/aws/aws-sdk-go-v2/service/s3 v1.105.0
github.com/aws/smithy-go v1.27.3
@@ -16,7 +17,6 @@ require (
require (
github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream v1.7.14 // indirect
- github.com/aws/aws-sdk-go-v2/credentials v1.19.6 // indirect
github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.18.16 // indirect
github.com/aws/aws-sdk-go-v2/feature/s3/manager v1.20.18 // indirect
github.com/aws/aws-sdk-go-v2/internal/configsources v1.4.30 // indirect
diff --git a/internal/lease/endpoint.go b/internal/lease/endpoint.go
index b78a8addc..c945f0dd1 100644
--- a/internal/lease/endpoint.go
+++ b/internal/lease/endpoint.go
@@ -17,20 +17,18 @@ package lease
import (
"context"
"fmt"
- "os"
- "strings"
- "github.com/aws/aws-sdk-go-v2/aws"
awsconfig "github.com/aws/aws-sdk-go-v2/config"
awss3 "github.com/aws/aws-sdk-go-v2/service/s3"
lss3 "github.com/benbjohnson/litestream/s3"
+
+ "github.com/carlosframework/platform/internal/s3compat"
)
// s3Endpoint reports the CARLOS_S3_ENDPOINT override, or "" for stock AWS.
-// Read per call rather than cached at init so a test (and a process that
-// re-execs its config) sees the value it just set.
+// Delegated to s3compat so there is exactly one reader of that variable.
func s3Endpoint() string {
- return strings.TrimSpace(os.Getenv("CARLOS_S3_ENDPOINT"))
+ return s3compat.Endpoint()
}
// resolveRegion decides the region every client in this package signs for, in
@@ -88,12 +86,13 @@ func regionFor(explicit, chainResolved string) string {
// override has to be applied right here or the lock lands on AWS while the
// data it guards lands on the compatible store.
//
-// An endpoint implies path-style addressing and when-required checksums, for
-// the reason release.NewS3Store gives: default-on flexible checksums wrap
-// bodies in aws-chunked framing that some compatibles reject. Both are applied
-// ONLY under an override, matching what litestream's own replica client does
-// (replica_client.go Init: the checksum knobs are gated on Endpoint != ""), so
-// the pure-AWS path stays byte-identical to before this file existed.
+// The override, path-style addressing, when-required checksums and every
+// per-provider quirk come from internal/s3compat, which is a no-op on stock
+// AWS — so the flagship's path stays byte-identical while this client picks up
+// the unquoted-If-Match fix that litestream's leaser needs to renew a lease on
+// Ceph RGW. Passing that client to lss3.Leaser.SetClient is the ONLY way the
+// fix reaches litestream, which has no provider handling in its leaser at all.
+//
// It returns the region it resolved alongside the client, so acquireTTL can
// carry that exact value onto the Held for Replicate's replica client rather
// than letting litestream substitute a different default of its own.
@@ -107,14 +106,7 @@ func newLeaserClient(ctx context.Context, region string) (*awss3.Client, string,
return nil, "", fmt.Errorf("load aws config: %w", err)
}
cfg.Region = regionFor(region, cfg.Region)
- return awss3.NewFromConfig(cfg, func(o *awss3.Options) {
- if ep := s3Endpoint(); ep != "" {
- o.BaseEndpoint = &ep
- o.UsePathStyle = true
- o.RequestChecksumCalculation = aws.RequestChecksumCalculationWhenRequired
- o.ResponseChecksumValidation = aws.ResponseChecksumValidationWhenRequired
- }
- }), cfg.Region, nil
+ return awss3.NewFromConfig(cfg, s3compat.Options()), cfg.Region, nil
}
// newReplicaClient builds litestream's replica client for the LTX files at
diff --git a/internal/release/store.go b/internal/release/store.go
index f8b5cbd7d..f672d74e4 100644
--- a/internal/release/store.go
+++ b/internal/release/store.go
@@ -24,6 +24,8 @@ import (
awsconfig "github.com/aws/aws-sdk-go-v2/config"
awss3 "github.com/aws/aws-sdk-go-v2/service/s3"
"github.com/aws/smithy-go"
+
+ "github.com/carlosframework/platform/internal/s3compat"
)
// ErrCASConflict is a conditional Put that lost: the object already existed
@@ -229,20 +231,21 @@ type S3Store struct {
}
// NewS3Store builds a store on the standard AWS credential/region chain.
-// CARLOS_S3_ENDPOINT (optional) points at an S3-compatible (R2, MinIO) and
-// implies path-style addressing; checksums go when-required because
-// default-on flexible checksums wrap bodies in aws-chunked framing some
-// compatibles reject (Eleven s3blobstore.go's R2 lesson).
+// Everything that makes a non-AWS store work — CARLOS_S3_ENDPOINT, path-style
+// addressing, when-required checksums, and any per-provider quirk — now lives
+// in internal/s3compat, so this store and the hibernation lease cannot drift
+// apart against the same bucket. See that package for the quirk table.
func NewS3Store(ctx context.Context, bucket string) (*S3Store, error) {
cfg, err := awsconfig.LoadDefaultConfig(ctx)
if err != nil {
return nil, fmt.Errorf("s3store: load aws config: %w", err)
}
- client := awss3.NewFromConfig(cfg, func(o *awss3.Options) {
- if ep := strings.TrimSpace(os.Getenv("CARLOS_S3_ENDPOINT")); ep != "" {
- o.BaseEndpoint = &ep
- o.UsePathStyle = true
- }
+ client := awss3.NewFromConfig(cfg, s3compat.Options(), func(o *awss3.Options) {
+ // s3compat sets these too, but only under a custom endpoint. This
+ // store has always set them on EVERY client including stock AWS, and
+ // the flagship runs on stock AWS — so the unconditional form stays
+ // here rather than becoming a silent behaviour change on the one
+ // deployment that cannot be tested from a compatible store.
o.RequestChecksumCalculation = aws.RequestChecksumCalculationWhenRequired
o.ResponseChecksumValidation = aws.ResponseChecksumValidationWhenRequired
})
diff --git a/internal/release/store_rgw_test.go b/internal/release/store_rgw_test.go
new file mode 100644
index 000000000..78ef98533
--- /dev/null
+++ b/internal/release/store_rgw_test.go
@@ -0,0 +1,169 @@
+package release
+
+import (
+ "context"
+ "errors"
+ "fmt"
+ "io"
+ "net/http"
+ "net/http/httptest"
+ "strings"
+ "testing"
+)
+
+// rgwFake is a fake S3 that reproduces the one behaviour measured against
+// Hetzner Object Storage (Ceph RGW) on 2026-08-07: If-Match on PUT is compared
+// against the UNQUOTED etag, so the quoted entity-tag form S3 itself returns
+// never matches and every compare-and-swap replace 412s.
+//
+// It is deliberately strict in both directions — a wrong etag is rejected in
+// either form — so a fix that merely stopped sending the condition would fail
+// this test rather than pass it.
+type rgwFake struct {
+ etag string // current etag, unquoted
+ body string
+}
+
+func newRGWFake(t *testing.T) (*rgwFake, string) {
+ t.Helper()
+ f := &rgwFake{}
+ srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ precondFailed := func() {
+ w.Header().Set("Content-Type", "application/xml")
+ w.WriteHeader(http.StatusPreconditionFailed)
+ _, _ = w.Write([]byte(`<?xml version="1.0" encoding="UTF-8"?>` +
+ `<Error><Code>PreconditionFailed</Code><Message>etag mismatch</Message></Error>`))
+ }
+ switch r.Method {
+ case http.MethodGet:
+ if f.etag == "" {
+ w.Header().Set("Content-Type", "application/xml")
+ w.WriteHeader(http.StatusNotFound)
+ _, _ = w.Write([]byte(`<?xml version="1.0" encoding="UTF-8"?>` +
+ `<Error><Code>NoSuchKey</Code><Message>absent</Message></Error>`))
+ return
+ }
+ w.Header().Set("ETag", `"`+f.etag+`"`) // quoted, exactly as S3 does
+ _, _ = w.Write([]byte(f.body))
+ case http.MethodPut:
+ raw, _ := io.ReadAll(r.Body)
+ switch {
+ case r.Header.Get("If-None-Match") == "*":
+ if f.etag != "" {
+ precondFailed() // create-only against an existing object
+ return
+ }
+ case r.Header.Get("If-Match") != "":
+ got := r.Header.Get("If-Match")
+ // THE QUIRK: RGW compares against the unquoted etag, so a
+ // correctly-quoted If-Match never matches.
+ if got != f.etag {
+ precondFailed()
+ return
+ }
+ }
+ f.body = string(raw)
+ f.etag = fmt.Sprintf("%08x", len(f.body)*2654435761)
+ w.Header().Set("ETag", `"`+f.etag+`"`)
+ w.WriteHeader(http.StatusOK)
+ default:
+ w.WriteHeader(http.StatusMethodNotAllowed)
+ }
+ }))
+ t.Cleanup(srv.Close)
+ return f, srv.URL
+}
+
+func rgwEnv(t *testing.T, url string) {
+ t.Helper()
+ t.Setenv("AWS_ACCESS_KEY_ID", "k")
+ t.Setenv("AWS_SECRET_ACCESS_KEY", "s")
+ t.Setenv("AWS_SESSION_TOKEN", "")
+ t.Setenv("AWS_REGION", "nbg1")
+ t.Setenv("AWS_EC2_METADATA_DISABLED", "true")
+ t.Setenv("CARLOS_S3_ENDPOINT", url)
+}
+
+// TestS3Store_CASReplaceAgainstRGW is the regression test for the
+// compare-and-swap failure that made the private CARLOS box report itself
+// UNREACHABLE: a single writer lost CAS 20/20 times against Hetzner because
+// every replace sent a quoted If-Match.
+//
+// The subtests are a matched pair — the quirk off case must FAIL, or the fix
+// is not what is making the on case pass.
+func TestS3Store_CASReplaceAgainstRGW(t *testing.T) {
+ ctx := context.Background()
+
+ replaceOnce := func(t *testing.T) error {
+ t.Helper()
+ _, url := newRGWFake(t)
+ rgwEnv(t, url)
+ s, err := NewS3Store(ctx, "b")
+ if err != nil {
+ t.Fatalf("NewS3Store: %v", err)
+ }
+ // Create-only first: this path uses If-None-Match:* and works on RGW,
+ // which is exactly why a fresh deployment looks healthy.
+ if _, err := s.Put(ctx, "status/boxes/x.json", []byte(`{"n":0}`), ""); err != nil {
+ t.Fatalf("create-only Put should succeed even on RGW: %v", err)
+ }
+ _, etag, err := s.Get(ctx, "status/boxes/x.json")
+ if err != nil {
+ t.Fatalf("Get: %v", err)
+ }
+ if !strings.HasPrefix(etag, `"`) {
+ t.Fatalf("fake should hand back a quoted etag like S3 does, got %q", etag)
+ }
+ // The replace — the operation that fails in production.
+ _, err = s.Put(ctx, "status/boxes/x.json", []byte(`{"n":1}`), etag)
+ return err
+ }
+
+ t.Run("quirk off: reproduces the production failure", func(t *testing.T) {
+ t.Setenv("CARLOS_S3_QUIRKS", "")
+ err := replaceOnce(t)
+ if !errors.Is(err, ErrCASConflict) {
+ t.Fatalf("want ErrCASConflict without the quirk (this is the bug), got %v", err)
+ }
+ })
+
+ t.Run("quirk on: replace succeeds", func(t *testing.T) {
+ // The fake's host is 127.0.0.1, so match-by-hostname cannot fire;
+ // force by name, the same way an operator would on a new store.
+ t.Setenv("CARLOS_S3_QUIRKS", "unquoted-if-match")
+ if err := replaceOnce(t); err != nil {
+ t.Fatalf("replace should succeed with the unquoted-if-match quirk: %v", err)
+ }
+ })
+}
+
+// TestS3Store_CASStillRejectsAStaleEtag is the safety half. Stripping quotes
+// must FIX compare-and-swap, not remove it: a genuinely stale etag has to keep
+// losing, or every CAS in CARLOS silently becomes a blind overwrite.
+func TestS3Store_CASStillRejectsAStaleEtag(t *testing.T) {
+ ctx := context.Background()
+ _, url := newRGWFake(t)
+ rgwEnv(t, url)
+ t.Setenv("CARLOS_S3_QUIRKS", "unquoted-if-match")
+
+ s, err := NewS3Store(ctx, "b")
+ if err != nil {
+ t.Fatalf("NewS3Store: %v", err)
+ }
+ key := "status/boxes/y.json"
+ if _, err := s.Put(ctx, key, []byte(`{"n":0}`), ""); err != nil {
+ t.Fatalf("seed Put: %v", err)
+ }
+ _, stale, err := s.Get(ctx, key)
+ if err != nil {
+ t.Fatalf("Get: %v", err)
+ }
+ // Someone else moves the object on.
+ if _, err := s.Put(ctx, key, []byte(`{"n":1,"other":true}`), stale); err != nil {
+ t.Fatalf("first replace: %v", err)
+ }
+ // Our now-stale etag must lose.
+ if _, err := s.Put(ctx, key, []byte(`{"n":2}`), stale); !errors.Is(err, ErrCASConflict) {
+ t.Fatalf("a stale etag must still lose CAS, got %v — the quirk has disabled compare-and-swap", err)
+ }
+}
diff --git a/internal/s3compat/s3compat.go b/internal/s3compat/s3compat.go
new file mode 100644
index 000000000..68995019f
--- /dev/null
+++ b/internal/s3compat/s3compat.go
@@ -0,0 +1,181 @@
+// Package s3compat is the one place CARLOS records how a given S3-compatible
+// store differs from AWS S3, and the only place that knows CARLOS_S3_ENDPOINT.
+//
+// Two kinds of adjustment live here. The first is universal — anything that is
+// not AWS needs a base endpoint, path-style addressing and when-required
+// checksums (default-on flexible checksums wrap bodies in aws-chunked framing
+// some compatibles reject). Those were already duplicated in
+// release.NewS3Store and internal/lease; Options folds them into one.
+//
+// The second is per-provider QUIRKS: places a store answers differently enough
+// that correct S3 code breaks against it. Those go in the quirks table below,
+// each entry naming the provider it matches, what it changes, and — the part
+// that matters six months later — the measurement that justifies it. Adding a
+// provider is one entry; nothing else in the codebase changes.
+//
+// Deliberately NOT covered: litestream's replica client, which builds its own
+// S3 client we cannot decorate. It needs none of the quirks here — it writes
+// LTX files with unconditional PUTs — and its own provider auto-detection
+// already handles the universal settings when an Endpoint is set. The leaser
+// is different: it takes a ready-made client (lss3.Leaser.SetClient), which is
+// exactly how the If-Match quirk below reaches it.
+package s3compat
+
+import (
+ "context"
+ "net/url"
+ "os"
+ "strings"
+
+ "github.com/aws/aws-sdk-go-v2/aws"
+ awsmiddleware "github.com/aws/aws-sdk-go-v2/aws/middleware"
+ awss3 "github.com/aws/aws-sdk-go-v2/service/s3"
+ "github.com/aws/smithy-go/middleware"
+ smithyhttp "github.com/aws/smithy-go/transport/http"
+)
+
+// Endpoint reports the CARLOS_S3_ENDPOINT override, or "" for stock AWS. Read
+// per call rather than cached at init so a test — and a process that re-reads
+// its config — sees the value it just set.
+func Endpoint() string {
+ return strings.TrimSpace(os.Getenv("CARLOS_S3_ENDPOINT"))
+}
+
+// EndpointHost is the bare host of the override ("" for stock AWS), which is
+// what quirk matchers key on. A bare host with no scheme still parses: the
+// value is normalised to a URL first, so both "https://x.example" and
+// "x.example" match the same way.
+func EndpointHost() string {
+ ep := Endpoint()
+ if ep == "" {
+ return ""
+ }
+ if !strings.Contains(ep, "://") {
+ ep = "https://" + ep
+ }
+ u, err := url.Parse(ep)
+ if err != nil {
+ return ""
+ }
+ return u.Hostname()
+}
+
+// Options returns the S3 client option every CARLOS S3 client should apply.
+// With no override it is a NO-OP — the flagship's pure-AWS path must stay
+// byte-identical, so nothing here may touch a client that talks to real S3.
+func Options() func(*awss3.Options) {
+ return func(o *awss3.Options) {
+ ep := Endpoint()
+ if ep == "" {
+ return
+ }
+ o.BaseEndpoint = &ep
+ o.UsePathStyle = true
+ o.RequestChecksumCalculation = aws.RequestChecksumCalculationWhenRequired
+ o.ResponseChecksumValidation = aws.ResponseChecksumValidationWhenRequired
+ for _, q := range Quirks {
+ if q.enabled(EndpointHost()) {
+ q.Apply(o)
+ }
+ }
+ }
+}
+
+// Quirk is one provider's documented deviation from S3 and the fix for it.
+type Quirk struct {
+ // Name is the stable identifier, also the token CARLOS_S3_QUIRKS uses to
+ // force the quirk on for a store whose hostname Match does not know.
+ Name string
+ // Reason is why this exists — ideally the measurement, not a guess.
+ Reason string
+ // Match reports whether the quirk applies to an endpoint host. A nil Match
+ // means "every non-AWS store", which should be rare: prefer naming the
+ // providers actually measured, so an unmeasured store fails loudly in its
+ // own way rather than silently inheriting someone else's workaround.
+ Match func(host string) bool
+ // Apply adjusts the client. Wire-level fixes register SDK middleware here.
+ Apply func(*awss3.Options)
+}
+
+func (q Quirk) enabled(host string) bool {
+ for _, forced := range strings.Split(os.Getenv("CARLOS_S3_QUIRKS"), ",") {
+ if strings.TrimSpace(forced) == q.Name {
+ return true
+ }
+ }
+ if q.Match == nil {
+ return true
+ }
+ return q.Match(host)
+}
+
+// Quirks is the table. Add a provider by adding an entry.
+var Quirks = []Quirk{unquotedIfMatch}
+
+// isHetzner matches Hetzner Object Storage (Ceph RGW), e.g.
+// nbg1.your-objectstorage.com. Same suffix litestream's own
+// IsHetznerEndpoint uses.
+func isHetzner(host string) bool {
+ return strings.HasSuffix(host, ".your-objectstorage.com")
+}
+
+// unquotedIfMatch is the fix for the compare-and-swap failure measured against
+// Hetzner Object Storage on 2026-08-07.
+//
+// RGW compares If-Match against its UNQUOTED internal etag, so the quoted
+// entity-tag form that S3 returns — and that the AWS SDK faithfully round-trips
+// — never matches. Measured against the carlos bucket:
+//
+// PUT If-Match: "<correct etag>" -> 412 (quoted, as AWS returns it)
+// PUT If-Match: <correct etag> -> OK (same etag, unquoted)
+// PUT If-Match: <wrong etag> -> 412 (still correctly rejected)
+// PUT If-None-Match: * (exists) -> 412 (correctly rejected)
+//
+// The third line is what makes this safe: the condition IS evaluated, only the
+// quoting form differs, so stripping the quotes fixes compare-and-swap rather
+// than disabling it. RFC 9110 entity-tags are quoted, so AWS is right and RGW
+// is wrong — but CARLOS still has to interoperate.
+//
+// Scoped to PutObject on purpose. Conditional DELETE accepts the quoted form
+// on the same store (it is how the lease releases today, and it works), and a
+// working path is not worth touching on the strength of an untested guess.
+//
+// What it unblocks: every CAS replace in CARLOS — release pointers and
+// manifests, the ledger head, appconfig generations, provision records, status
+// snapshots, metrics rollups — plus litestream's lease heartbeat, which renews
+// with PUT If-Match every TTL/3 and so lost the lease at the first renewal,
+// standing a woken instance back down five minutes later, every time.
+var unquotedIfMatch = Quirk{
+ Name: "unquoted-if-match",
+ Reason: "Ceph RGW (Hetzner) compares If-Match against the unquoted etag; the quoted form always 412s",
+ Match: isHetzner,
+ Apply: func(o *awss3.Options) {
+ o.APIOptions = append(o.APIOptions, unquoteIfMatchMiddleware)
+ },
+}
+
+// unquoteIfMatchMiddleware strips the surrounding quotes from a PutObject's
+// If-Match header.
+//
+// It runs in the BUILD step, which is after serialization has written the
+// header and before the Finalize step signs the request — the only window
+// where the change is both possible and covered by the signature. Rewriting
+// after signing would produce a SignatureDoesNotMatch instead of a fix.
+func unquoteIfMatchMiddleware(stack *middleware.Stack) error {
+ return stack.Build.Add(middleware.BuildMiddlewareFunc("carlosUnquoteIfMatch",
+ func(ctx context.Context, in middleware.BuildInput, next middleware.BuildHandler) (
+ middleware.BuildOutput, middleware.Metadata, error,
+ ) {
+ if awsmiddleware.GetOperationName(ctx) == "PutObject" {
+ if req, ok := in.Request.(*smithyhttp.Request); ok {
+ if v := req.Header.Get("If-Match"); v != "" {
+ if u := strings.Trim(v, `"`); u != v {
+ req.Header.Set("If-Match", u)
+ }
+ }
+ }
+ }
+ return next.HandleBuild(ctx, in)
+ },
+ ), middleware.After)
+}
diff --git a/internal/s3compat/s3compat_test.go b/internal/s3compat/s3compat_test.go
new file mode 100644
index 000000000..00268072c
--- /dev/null
+++ b/internal/s3compat/s3compat_test.go
@@ -0,0 +1,235 @@
+package s3compat
+
+import (
+ "context"
+ "net/http"
+ "net/http/httptest"
+ "strings"
+ "sync"
+ "testing"
+
+ "github.com/aws/aws-sdk-go-v2/aws"
+ "github.com/aws/aws-sdk-go-v2/credentials"
+ awss3 "github.com/aws/aws-sdk-go-v2/service/s3"
+)
+
+// recorder is a fake S3 that records the conditional headers it was sent.
+type recorder struct {
+ mu sync.Mutex
+ seen map[string]string // "METHOD If-Match" -> value
+ auth map[string]string // "METHOD" -> Authorization
+}
+
+func newRecorder(t *testing.T) (*recorder, string) {
+ t.Helper()
+ r := &recorder{seen: map[string]string{}, auth: map[string]string{}}
+ srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
+ r.mu.Lock()
+ r.seen[req.Method+" If-Match"] = req.Header.Get("If-Match")
+ r.auth[req.Method] = req.Header.Get("Authorization")
+ r.mu.Unlock()
+ w.Header().Set("ETag", `"deadbeef"`)
+ w.WriteHeader(http.StatusOK)
+ }))
+ t.Cleanup(srv.Close)
+ return r, srv.URL
+}
+
+func (r *recorder) get(k string) string {
+ r.mu.Lock()
+ defer r.mu.Unlock()
+ return r.seen[k]
+}
+
+func (r *recorder) authFor(m string) string {
+ r.mu.Lock()
+ defer r.mu.Unlock()
+ return r.auth[m]
+}
+
+func testClient(t *testing.T) *awss3.Client {
+ t.Helper()
+ cfg := aws.Config{
+ Region: "nbg1",
+ Credentials: credentials.NewStaticCredentialsProvider("k", "s", ""),
+ }
+ return awss3.NewFromConfig(cfg, Options())
+}
+
+// TestUnquotedIfMatch_RewritesPutOnly is the core of the fix: a PutObject's
+// If-Match must reach the wire unquoted (what Ceph RGW compares against),
+// while a DeleteObject's must be left exactly as the SDK wrote it — the quoted
+// form is what works for conditional DELETE on the same store, and a working
+// path is not worth touching.
+func TestUnquotedIfMatch_RewritesPutOnly(t *testing.T) {
+ rec, url := newRecorder(t)
+ t.Setenv("CARLOS_S3_ENDPOINT", url)
+ // The httptest host is 127.0.0.1, not a Hetzner hostname, so force the
+ // quirk on by name — which also exercises the CARLOS_S3_QUIRKS escape
+ // hatch that lets an operator try a fix against an unrecognised store.
+ t.Setenv("CARLOS_S3_QUIRKS", "unquoted-if-match")
+
+ c := testClient(t)
+ ctx := context.Background()
+ quoted := `"deadbeef"`
+
+ if _, err := c.PutObject(ctx, &awss3.PutObjectInput{
+ Bucket: aws.String("b"), Key: aws.String("k"),
+ Body: strings.NewReader("x"), IfMatch: aws.String(quoted),
+ }); err != nil {
+ t.Fatalf("PutObject: %v", err)
+ }
+ if got := rec.get("PUT If-Match"); got != "deadbeef" {
+ t.Errorf("PUT If-Match = %q, want %q (quotes stripped)", got, "deadbeef")
+ }
+
+ if _, err := c.DeleteObject(ctx, &awss3.DeleteObjectInput{
+ Bucket: aws.String("b"), Key: aws.String("k"), IfMatch: aws.String(quoted),
+ }); err != nil {
+ t.Fatalf("DeleteObject: %v", err)
+ }
+ if got := rec.get("DELETE If-Match"); got != quoted {
+ t.Errorf("DELETE If-Match = %q, want %q (left untouched)", got, quoted)
+ }
+}
+
+// TestUnquotedIfMatch_RewriteIsSigned guards the subtlety that makes this a
+// Build-step middleware rather than a Finalize one: If-Match is a SIGNED
+// header, so a rewrite after signing would produce SignatureDoesNotMatch
+// instead of a fix. If the signer did not see if-match, it is not in
+// SignedHeaders — which is observable from the Authorization header.
+func TestUnquotedIfMatch_RewriteIsSigned(t *testing.T) {
+ rec, url := newRecorder(t)
+ t.Setenv("CARLOS_S3_ENDPOINT", url)
+ t.Setenv("CARLOS_S3_QUIRKS", "unquoted-if-match")
+
+ c := testClient(t)
+ if _, err := c.PutObject(context.Background(), &awss3.PutObjectInput{
+ Bucket: aws.String("b"), Key: aws.String("k"),
+ Body: strings.NewReader("x"), IfMatch: aws.String(`"deadbeef"`),
+ }); err != nil {
+ t.Fatalf("PutObject: %v", err)
+ }
+ auth := rec.authFor("PUT")
+ if !strings.Contains(auth, "SignedHeaders=") {
+ t.Fatalf("no SignedHeaders in Authorization: %q", auth)
+ }
+ if !strings.Contains(auth, "if-match") {
+ t.Errorf("if-match missing from SignedHeaders — the rewrite is outside the signature: %q", auth)
+ }
+}
+
+// TestOptions_NoOpWithoutEndpoint is the flagship guard: on stock AWS this
+// package must change nothing at all, including registering no middleware.
+func TestOptions_NoOpWithoutEndpoint(t *testing.T) {
+ t.Setenv("CARLOS_S3_ENDPOINT", "")
+ t.Setenv("CARLOS_S3_QUIRKS", "")
+
+ var o awss3.Options
+ Options()(&o)
+
+ if o.BaseEndpoint != nil {
+ t.Errorf("BaseEndpoint = %q, want nil on stock AWS", *o.BaseEndpoint)
+ }
+ if o.UsePathStyle {
+ t.Error("UsePathStyle = true, want false on stock AWS")
+ }
+ if len(o.APIOptions) != 0 {
+ t.Errorf("APIOptions = %d, want 0 — no quirk middleware on stock AWS", len(o.APIOptions))
+ }
+ if o.RequestChecksumCalculation != 0 {
+ t.Errorf("RequestChecksumCalculation = %v, want the SDK default on stock AWS", o.RequestChecksumCalculation)
+ }
+}
+
+// TestOptions_UniversalSettingsUnderEndpoint pins what every compatible store
+// gets regardless of provider.
+func TestOptions_UniversalSettingsUnderEndpoint(t *testing.T) {
+ t.Setenv("CARLOS_S3_ENDPOINT", "https://nbg1.your-objectstorage.com")
+ t.Setenv("CARLOS_S3_QUIRKS", "")
+
+ var o awss3.Options
+ Options()(&o)
+
+ if o.BaseEndpoint == nil || *o.BaseEndpoint != "https://nbg1.your-objectstorage.com" {
+ t.Errorf("BaseEndpoint = %v, want the override", o.BaseEndpoint)
+ }
+ if !o.UsePathStyle {
+ t.Error("UsePathStyle = false, want true under an override")
+ }
+ if o.RequestChecksumCalculation != aws.RequestChecksumCalculationWhenRequired {
+ t.Error("RequestChecksumCalculation should be when-required under an override")
+ }
+ if len(o.APIOptions) != 1 {
+ t.Errorf("APIOptions = %d, want 1 (the Hetzner If-Match quirk matched by host)", len(o.APIOptions))
+ }
+}
+
+func TestEndpointHost(t *testing.T) {
+ for _, tt := range []struct{ ep, want string }{
+ {"https://nbg1.your-objectstorage.com", "nbg1.your-objectstorage.com"},
+ {"nbg1.your-objectstorage.com", "nbg1.your-objectstorage.com"}, // no scheme
+ {"http://localhost:9000", "localhost"},
+ {"", ""},
+ } {
+ t.Setenv("CARLOS_S3_ENDPOINT", tt.ep)
+ if got := EndpointHost(); got != tt.want {
+ t.Errorf("EndpointHost() with %q = %q, want %q", tt.ep, got, tt.want)
+ }
+ }
+}
+
+// TestQuirkMatching keeps the table honest: a quirk measured on one provider
+// must not leak onto stores nobody has tested it against.
+func TestQuirkMatching(t *testing.T) {
+ t.Setenv("CARLOS_S3_QUIRKS", "")
+ for _, tt := range []struct {
+ host string
+ want bool
+ }{
+ {"nbg1.your-objectstorage.com", true},
+ {"fsn1.your-objectstorage.com", true},
+ {"s3.eu-west-1.amazonaws.com", false},
+ {"minio.internal", false},
+ {"evil-your-objectstorage.com", false}, // suffix match needs the dot
+ } {
+ if got := unquotedIfMatch.enabled(tt.host); got != tt.want {
+ t.Errorf("unquoted-if-match enabled for %q = %v, want %v", tt.host, got, tt.want)
+ }
+ }
+}
+
+// TestQuirkForceEnable covers the escape hatch: trying a known fix against an
+// unrecognised store should not need a rebuild.
+func TestQuirkForceEnable(t *testing.T) {
+ t.Setenv("CARLOS_S3_QUIRKS", "unquoted-if-match")
+ if !unquotedIfMatch.enabled("some.unknown.store") {
+ t.Error("CARLOS_S3_QUIRKS did not force the quirk on for an unmatched host")
+ }
+ t.Setenv("CARLOS_S3_QUIRKS", " other-quirk , unquoted-if-match ")
+ if !unquotedIfMatch.enabled("some.unknown.store") {
+ t.Error("CARLOS_S3_QUIRKS should tolerate spaces and a list")
+ }
+ t.Setenv("CARLOS_S3_QUIRKS", "something-else")
+ if unquotedIfMatch.enabled("some.unknown.store") {
+ t.Error("an unrelated CARLOS_S3_QUIRKS entry must not enable this quirk")
+ }
+}
+
+// TestQuirkNamesAreUnique — the table is looked up by name from an env var, so
+// duplicates would make CARLOS_S3_QUIRKS ambiguous.
+func TestQuirkNamesAreUnique(t *testing.T) {
+ seen := map[string]bool{}
+ for _, q := range Quirks {
+ if q.Name == "" {
+ t.Error("a quirk has no Name; CARLOS_S3_QUIRKS could not address it")
+ }
+ if q.Reason == "" {
+ t.Errorf("quirk %q has no Reason — the table exists to record WHY", q.Name)
+ }
+ if seen[q.Name] {
+ t.Errorf("duplicate quirk name %q", q.Name)
+ }
+ seen[q.Name] = true
+ }
+}