diff --git a/TODO.md b/TODO.md index b3011b5..01faf4f 100644 --- a/TODO.md +++ b/TODO.md @@ -25,6 +25,15 @@ release" is exactly the contradiction # Completed Steps +- 2026-09-21: Made the s3 storage backend report a missing object as + `storage.ErrNotFound`, like the `file` and `rclone` backends and as the + `Storer` interface documents. `S3Storer.Get` and `Stat` returned the raw + AWS SDK error, so `errors.Is(err, storage.ErrNotFound)` was false on s3 + and callers branched differently per backend. Added a small `s3.IsNotFound` + helper (reused by `HeadObject`) and a test that a missing key maps to + `ErrNotFound` + ([issue #129](https://git.eeqj.de/sneak/vaultik/issues/129)). + - 2026-09-21: Fixed `verify --deep` reporting healthy snapshots as corrupt. Its final blob-integrity check hashed the encrypted downloaded bytes with a single SHA256 and compared that to the blob diff --git a/internal/s3/client.go b/internal/s3/client.go index f043ba3..b54d3f4 100644 --- a/internal/s3/client.go +++ b/internal/s3/client.go @@ -219,11 +219,7 @@ func (c *Client) HeadObject(ctx context.Context, key string) (bool, error) { Key: aws.String(fullKey), }) if err != nil { - var ( - notFound *s3types.NotFound - noSuchKey *s3types.NoSuchKey - ) - if errors.As(err, ¬Found) || errors.As(err, &noSuchKey) { + if IsNotFound(err) { return false, nil } @@ -233,6 +229,18 @@ func (c *Client) HeadObject(ctx context.Context, key string) (bool, error) { return true, nil } +// IsNotFound reports whether err indicates that an object does not exist. +// Head and Get requests surface a missing object as different SDK types, +// so both are checked here. +func IsNotFound(err error) bool { + var ( + notFound *s3types.NotFound + noSuchKey *s3types.NoSuchKey + ) + + return errors.As(err, ¬Found) || errors.As(err, &noSuchKey) +} + // ObjectInfo contains information about an S3 object. // It is used by ListObjectsStream to return object metadata // along with any errors encountered during listing. diff --git a/internal/storage/s3.go b/internal/storage/s3.go index 1f4f7c5..648e697 100644 --- a/internal/storage/s3.go +++ b/internal/storage/s3.go @@ -38,14 +38,29 @@ func (s *S3Storer) PutWithProgress( } // Get retrieves data from the specified key. +// Returns ErrNotFound if the object does not exist. func (s *S3Storer) Get(ctx context.Context, key string) (io.ReadCloser, error) { - return s.client.GetObject(ctx, key) + rc, err := s.client.GetObject(ctx, key) + if err != nil { + if s3.IsNotFound(err) { + return nil, fmt.Errorf("get %q: %w", key, ErrNotFound) + } + + return nil, err + } + + return rc, nil } // Stat returns metadata about an object without retrieving its contents. +// Returns ErrNotFound if the object does not exist. func (s *S3Storer) Stat(ctx context.Context, key string) (*ObjectInfo, error) { info, err := s.client.StatObject(ctx, key) if err != nil { + if s3.IsNotFound(err) { + return nil, fmt.Errorf("stat %q: %w", key, ErrNotFound) + } + return nil, err } diff --git a/internal/storage/s3_test.go b/internal/storage/s3_test.go new file mode 100644 index 0000000..3554a43 --- /dev/null +++ b/internal/storage/s3_test.go @@ -0,0 +1,59 @@ +package storage_test + +import ( + "context" + "errors" + "net/http/httptest" + "testing" + + "github.com/johannesboyne/gofakes3" + "github.com/johannesboyne/gofakes3/backend/s3mem" + + "sneak.berlin/go/vaultik/internal/s3" + "sneak.berlin/go/vaultik/internal/storage" +) + +// TestS3StorerMissingKeyMapsToErrNotFound verifies that the s3 backend reports +// a missing object as storage.ErrNotFound, matching the file and rclone +// backends and the Storer contract. Without the mapping, Get and Stat leak the +// raw SDK error and errors.Is(err, storage.ErrNotFound) is false. +// +//nolint:paralleltest // shares an in-process S3 server via t.Cleanup +func TestS3StorerMissingKeyMapsToErrNotFound(t *testing.T) { + const bucket = "test-bucket" + + backend := s3mem.New() + + err := backend.CreateBucket(bucket) + if err != nil { + t.Fatalf("create bucket: %v", err) + } + + srv := httptest.NewServer(gofakes3.New(backend).Server()) + t.Cleanup(srv.Close) + + ctx := context.Background() + + client, err := s3.NewClient(ctx, s3.Config{ + Endpoint: srv.URL, + Bucket: bucket, + AccessKeyID: "test", + SecretAccessKey: "test", + Region: "us-east-1", + }) + if err != nil { + t.Fatalf("new client: %v", err) + } + + storer := storage.NewS3Storer(client) + + _, err = storer.Get(ctx, "does-not-exist") + if !errors.Is(err, storage.ErrNotFound) { + t.Errorf("Get on missing key: got %v, want ErrNotFound", err) + } + + _, err = storer.Stat(ctx, "does-not-exist") + if !errors.Is(err, storage.ErrNotFound) { + t.Errorf("Stat on missing key: got %v, want ErrNotFound", err) + } +}