diff --git a/TODO.md b/TODO.md index a0c35be..c177e39 100644 --- a/TODO.md +++ b/TODO.md @@ -22,6 +22,14 @@ the tag exists and is exercised; what is left is merging `next` to # Completed Steps +- 2026-10-06: Made `s3://bucket/prefix` and `s3://bucket/prefix/` the same + destination ([issue #222](https://git.eeqj.de/sneak/vaultik/issues/222)). + The S3 client put the prefix directly in front of each key, so a prefix + without a trailing slash stored `prefixblobs/...`. A non-empty prefix is + now joined to every key with one `/`, giving the README's + `//blobs/...` layout. The `s3.prefix` config setting goes + through the same client and gets the same join. + - 2026-10-06: Made `go.mod` what `go mod tidy` writes, so the pre-commit hook no longer stops every commit ([issue #246](https://git.eeqj.de/sneak/vaultik/issues/246)). A test diff --git a/internal/s3/client.go b/internal/s3/client.go index b54d3f4..43f8725 100644 --- a/internal/s3/client.go +++ b/internal/s3/client.go @@ -6,6 +6,7 @@ import ( "context" "errors" "io" + "strings" "sync/atomic" "github.com/aws/aws-sdk-go-v2/aws" @@ -30,6 +31,8 @@ type Client struct { // Config contains S3 client configuration. // All fields are required except Prefix, which defaults to an empty string. +// A non-empty Prefix is joined to every key with one "/", whether or not +// it ends with one. // The Endpoint field should include the protocol (http:// or https://). type Config struct { Endpoint string @@ -75,10 +78,17 @@ func NewClient(ctx context.Context, cfg Config) (*Client, error) { s3Client := s3.NewFromConfig(awsCfg, s3Opts) + // Every method below builds a key as prefix + key, so the prefix + // must carry its own trailing "/". + prefix := strings.TrimRight(cfg.Prefix, "/") + if prefix != "" { + prefix += "/" + } + return &Client{ s3Client: s3Client, bucket: cfg.Bucket, - prefix: cfg.Prefix, + prefix: prefix, endpoint: cfg.Endpoint, }, nil } diff --git a/internal/storage/s3_test.go b/internal/storage/s3_test.go index 966d5c6..95f86b7 100644 --- a/internal/storage/s3_test.go +++ b/internal/storage/s3_test.go @@ -4,11 +4,14 @@ import ( "context" "errors" "net/http/httptest" + "slices" + "strings" "testing" "github.com/johannesboyne/gofakes3" "github.com/johannesboyne/gofakes3/backend/s3mem" + "sneak.berlin/go/vaultik/internal/config" "sneak.berlin/go/vaultik/internal/s3" "sneak.berlin/go/vaultik/internal/storage" ) @@ -79,3 +82,84 @@ func TestS3StorerMissingKeyMapsToErrNotFound(t *testing.T) { t.Errorf("Stat on missing key: got %v, want ErrNotFound", err) } } + +// TestS3URLPrefixKeyLayout pins the bucket keys an s3:// URL reads and +// writes: the README's remote storage layout, with the prefix joined to +// each key by one "/". s3://b/p and s3://b/p/ must be the same +// destination, or a host that writes the URL the other way finds no +// snapshots. The listed object is put straight into the bucket, as +// another host would have written it. +func TestS3URLPrefixKeyLayout(t *testing.T) { + t.Parallel() + + const ( + blobKey = "blobs/aa/bb/aabbccdd" + manifestKey = "metadata/snap/manifest.json.zst" + manifestBody = "manifest" + ) + + cases := []struct { + urlPath string // URL path after the bucket name + keyPrefix string // what every key in the bucket must start with + }{ + {urlPath: "/p", keyPrefix: "p/"}, + {urlPath: "/p/", keyPrefix: "p/"}, + {urlPath: "", keyPrefix: ""}, + } + + for _, tc := range cases { + storageURL := "s3://" + s3TestBucket + tc.urlPath + + t.Run(storageURL, func(t *testing.T) { + t.Parallel() + + backend := s3mem.New() + + err := backend.CreateBucket(s3TestBucket) + if err != nil { + t.Fatalf("create bucket: %v", err) + } + + srv := httptest.NewServer(gofakes3.New(backend).Server()) + t.Cleanup(srv.Close) + + storer, err := storage.NewStorer(&config.Config{ + StorageURL: storageURL + "?endpoint=" + srv.URL, + S3: config.S3Config{ + AccessKeyID: "key", + SecretAccessKey: "secret", + }, + }) + if err != nil { + t.Fatalf("NewStorer: %v", err) + } + + ctx := context.Background() + + err = storer.Put(ctx, blobKey, strings.NewReader("blob")) + if err != nil { + t.Fatalf("Put: %v", err) + } + + _, err = backend.HeadObject(s3TestBucket, tc.keyPrefix+blobKey) + if err != nil { + t.Errorf("blob not stored at %q: %v", tc.keyPrefix+blobKey, err) + } + + _, err = backend.PutObject(s3TestBucket, tc.keyPrefix+manifestKey, + nil, strings.NewReader(manifestBody), int64(len(manifestBody))) + if err != nil { + t.Fatalf("seed manifest: %v", err) + } + + keys, err := storer.List(ctx, "metadata/") + if err != nil { + t.Fatalf("List: %v", err) + } + + if !slices.Equal(keys, []string{manifestKey}) { + t.Errorf("List(metadata/) = %q, want [%q]", keys, manifestKey) + } + }) + } +}