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
+  `<bucket>/<prefix>/blobs/...` layout. The `s3.prefix` config setting goes
+  through the same client and gets the same join.
+
 - 2026-10-06: Made restore apply owners, modes and times in an order
   that keeps them
   ([issue #219](https://git.eeqj.de/sneak/vaultik/issues/219)). A

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
 }

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,109 @@ 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. Both List and ListStream are
+// checked: ListStream is what every snapshot listing goes through.
+func TestS3URLPrefixKeyLayout(t *testing.T) {
+	t.Parallel()
+
+	const (
+		blobKey      = "blobs/aa/bb/aabbccdd"
+		listPrefix   = "metadata/"
+		manifestKey  = listPrefix + "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, listPrefix)
+			if err != nil {
+				t.Fatalf("List: %v", err)
+			}
+
+			if !slices.Equal(keys, []string{manifestKey}) {
+				t.Errorf("List(%q) = %q, want [%q]", listPrefix, keys, manifestKey)
+			}
+
+			streamed := listStreamKeys(t, storer, listPrefix)
+			if !slices.Equal(streamed, []string{manifestKey}) {
+				t.Errorf("ListStream(%q) = %q, want [%q]", listPrefix, streamed, manifestKey)
+			}
+		})
+	}
+}
+
+// listStreamKeys returns the keys ListStream yields under a prefix, and
+// fails the test on a listing error.
+func listStreamKeys(t *testing.T, s storage.Storer, prefix string) []string {
+	t.Helper()
+
+	var keys []string
+
+	for obj := range s.ListStream(context.Background(), prefix) {
+		if obj.Err != nil {
+			t.Fatalf("ListStream %q: %v", prefix, obj.Err)
+		}
+
+		keys = append(keys, obj.Key)
+	}
+
+	return keys
+}