Make an S3 listing the server cuts short an error (closes #297)
check / check (push) Waiting to run
check / check (push) Waiting to run
The SDK paginator takes a listing page marked truncated but carrying no continuation token for the last page and stops without an error. A server answering V2 listings V1-style sends such pages, so ListObjects and ListObjectsStream returned the first page as the whole listing, and prune treated the blobs of the snapshots on later pages as unreferenced. Both now fetch each page through nextListPage, which returns ErrNoContinuationToken for such a page; prune, snapshot list and remote nuke already handle a listing error. The test serves a fixed reply from a stub server, since gofakes3 always sends the token. Model: opus-5-5
This commit is contained in:
@@ -4,6 +4,7 @@ import (
|
||||
"bytes"
|
||||
"context"
|
||||
"errors"
|
||||
"io"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"slices"
|
||||
@@ -22,13 +23,16 @@ import (
|
||||
// s3TestBucket is the bucket created for each in-process S3 server.
|
||||
const s3TestBucket = "test-bucket"
|
||||
|
||||
// Credentials for the tests that build a storer from a config.Config. The
|
||||
// in-process S3 server accepts any.
|
||||
// Credentials for the tests' S3 clients. The test servers accept any.
|
||||
const (
|
||||
s3TestAccessKeyID = "key"
|
||||
s3TestSecretAccessKey = "secret"
|
||||
)
|
||||
|
||||
// s3TestRegion is the region of the S3 clients built here with
|
||||
// s3.NewClient. The test servers ignore it.
|
||||
const s3TestRegion = "us-east-1"
|
||||
|
||||
// newS3Storer builds an s3:// backend backed by a fresh in-process
|
||||
// S3 server. It reuses the same in-memory S3 harness (gofakes3 + s3mem
|
||||
// over httptest) that internal/s3 and the not-found regression test use,
|
||||
@@ -55,7 +59,7 @@ func newS3Storer(t *testing.T) storage.Storer {
|
||||
Bucket: s3TestBucket,
|
||||
AccessKeyID: "test",
|
||||
SecretAccessKey: "test",
|
||||
Region: "us-east-1",
|
||||
Region: s3TestRegion,
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("new client: %v", err)
|
||||
@@ -93,6 +97,68 @@ func TestS3StorerMissingKeyMapsToErrNotFound(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestS3ListingCutShortIsAnError checks that List and ListStream fail on a
|
||||
// listing page marked truncated that carries no continuation token, as a
|
||||
// server answering V2 listings V1-style sends it. Taken as the last page,
|
||||
// it would hide every later key, and prune would treat the blobs of the
|
||||
// snapshots listed there as unreferenced.
|
||||
func TestS3ListingCutShortIsAnError(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
const (
|
||||
prefix = "metadata/"
|
||||
truncatedPage = `<?xml version="1.0" encoding="UTF-8"?>
|
||||
<ListBucketResult xmlns="http://s3.amazonaws.com/doc/2006-03-01/">
|
||||
<Name>test-bucket</Name>
|
||||
<Prefix>metadata/</Prefix>
|
||||
<KeyCount>1</KeyCount>
|
||||
<MaxKeys>1</MaxKeys>
|
||||
<IsTruncated>true</IsTruncated>
|
||||
<Contents>
|
||||
<Key>metadata/snap/manifest.json.zst</Key>
|
||||
<Size>8</Size>
|
||||
</Contents>
|
||||
</ListBucketResult>`
|
||||
)
|
||||
|
||||
srv := httptest.NewServer(http.HandlerFunc(
|
||||
func(w http.ResponseWriter, _ *http.Request) {
|
||||
_, _ = io.WriteString(w, truncatedPage)
|
||||
}))
|
||||
t.Cleanup(srv.Close)
|
||||
|
||||
client, err := s3.NewClient(context.Background(), s3.Config{
|
||||
Endpoint: srv.URL,
|
||||
Bucket: s3TestBucket,
|
||||
AccessKeyID: s3TestAccessKeyID,
|
||||
SecretAccessKey: s3TestSecretAccessKey,
|
||||
Region: s3TestRegion,
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("new client: %v", err)
|
||||
}
|
||||
|
||||
storer := storage.NewS3Storer(client)
|
||||
ctx := context.Background()
|
||||
|
||||
keys, err := storer.List(ctx, prefix)
|
||||
if !errors.Is(err, s3.ErrNoContinuationToken) {
|
||||
t.Errorf("List = %q, %v; want ErrNoContinuationToken", keys, err)
|
||||
}
|
||||
|
||||
var streamErr error
|
||||
|
||||
for obj := range storer.ListStream(ctx, prefix) {
|
||||
if obj.Err != nil {
|
||||
streamErr = obj.Err
|
||||
}
|
||||
}
|
||||
|
||||
if !errors.Is(streamErr, s3.ErrNoContinuationToken) {
|
||||
t.Errorf("ListStream error = %v, want ErrNoContinuationToken", streamErr)
|
||||
}
|
||||
}
|
||||
|
||||
// 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
|
||||
|
||||
Reference in New Issue
Block a user