Join the S3 prefix to every key with one slash (closes #222)
check / check (push) Waiting to run
check / check (push) Waiting to run
The S3 client built each key as prefix + key, and the URL parser keeps the prefix as written, so s3://bucket/p stored p + "blobs/..." with no slash while s3://bucket/p/ stored p/blobs/.... A recovery host that wrote the URL the other way found no snapshots. NewClient now strips trailing slashes from the prefix and adds one back when anything is left, giving the README layout for both URL forms; an empty prefix stays at the bucket root. The s3.prefix config setting goes through the same client and gets the same join. A new test writes and lists through each URL shape against an in-process S3 server and checks the keys in the bucket. Model: opus-5-5
This commit is contained in:
@@ -22,6 +22,14 @@ the tag exists and is exercised; what is left is merging `next` to
|
|||||||
|
|
||||||
# Completed Steps
|
# 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
|
||||||
|
`<bucket>/<prefix>/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
|
- 2026-10-06: Made `go.mod` what `go mod tidy` writes, so the
|
||||||
pre-commit hook no longer stops every commit
|
pre-commit hook no longer stops every commit
|
||||||
([issue #246](https://git.eeqj.de/sneak/vaultik/issues/246)). A test
|
([issue #246](https://git.eeqj.de/sneak/vaultik/issues/246)). A test
|
||||||
|
|||||||
+11
-1
@@ -6,6 +6,7 @@ import (
|
|||||||
"context"
|
"context"
|
||||||
"errors"
|
"errors"
|
||||||
"io"
|
"io"
|
||||||
|
"strings"
|
||||||
"sync/atomic"
|
"sync/atomic"
|
||||||
|
|
||||||
"github.com/aws/aws-sdk-go-v2/aws"
|
"github.com/aws/aws-sdk-go-v2/aws"
|
||||||
@@ -30,6 +31,8 @@ type Client struct {
|
|||||||
|
|
||||||
// Config contains S3 client configuration.
|
// Config contains S3 client configuration.
|
||||||
// All fields are required except Prefix, which defaults to an empty string.
|
// 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://).
|
// The Endpoint field should include the protocol (http:// or https://).
|
||||||
type Config struct {
|
type Config struct {
|
||||||
Endpoint string
|
Endpoint string
|
||||||
@@ -75,10 +78,17 @@ func NewClient(ctx context.Context, cfg Config) (*Client, error) {
|
|||||||
|
|
||||||
s3Client := s3.NewFromConfig(awsCfg, s3Opts)
|
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{
|
return &Client{
|
||||||
s3Client: s3Client,
|
s3Client: s3Client,
|
||||||
bucket: cfg.Bucket,
|
bucket: cfg.Bucket,
|
||||||
prefix: cfg.Prefix,
|
prefix: prefix,
|
||||||
endpoint: cfg.Endpoint,
|
endpoint: cfg.Endpoint,
|
||||||
}, nil
|
}, nil
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -4,11 +4,14 @@ import (
|
|||||||
"context"
|
"context"
|
||||||
"errors"
|
"errors"
|
||||||
"net/http/httptest"
|
"net/http/httptest"
|
||||||
|
"slices"
|
||||||
|
"strings"
|
||||||
"testing"
|
"testing"
|
||||||
|
|
||||||
"github.com/johannesboyne/gofakes3"
|
"github.com/johannesboyne/gofakes3"
|
||||||
"github.com/johannesboyne/gofakes3/backend/s3mem"
|
"github.com/johannesboyne/gofakes3/backend/s3mem"
|
||||||
|
|
||||||
|
"sneak.berlin/go/vaultik/internal/config"
|
||||||
"sneak.berlin/go/vaultik/internal/s3"
|
"sneak.berlin/go/vaultik/internal/s3"
|
||||||
"sneak.berlin/go/vaultik/internal/storage"
|
"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)
|
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)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user