Map s3 not-found to storage.ErrNotFound in Get and Stat (closes #129) #142
@@ -25,6 +25,15 @@ release" is exactly the contradiction
|
|||||||
|
|
||||||
# Completed Steps
|
# 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
|
- 2026-09-21: Fixed `verify --deep` reporting healthy snapshots as
|
||||||
corrupt. Its final blob-integrity check hashed the encrypted
|
corrupt. Its final blob-integrity check hashed the encrypted
|
||||||
downloaded bytes with a single SHA256 and compared that to the blob
|
downloaded bytes with a single SHA256 and compared that to the blob
|
||||||
|
|||||||
+13
-5
@@ -219,11 +219,7 @@ func (c *Client) HeadObject(ctx context.Context, key string) (bool, error) {
|
|||||||
Key: aws.String(fullKey),
|
Key: aws.String(fullKey),
|
||||||
})
|
})
|
||||||
if err != nil {
|
if err != nil {
|
||||||
var (
|
if IsNotFound(err) {
|
||||||
notFound *s3types.NotFound
|
|
||||||
noSuchKey *s3types.NoSuchKey
|
|
||||||
)
|
|
||||||
if errors.As(err, ¬Found) || errors.As(err, &noSuchKey) {
|
|
||||||
return false, nil
|
return false, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -233,6 +229,18 @@ func (c *Client) HeadObject(ctx context.Context, key string) (bool, error) {
|
|||||||
return true, nil
|
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.
|
// ObjectInfo contains information about an S3 object.
|
||||||
// It is used by ListObjectsStream to return object metadata
|
// It is used by ListObjectsStream to return object metadata
|
||||||
// along with any errors encountered during listing.
|
// along with any errors encountered during listing.
|
||||||
|
|||||||
+16
-1
@@ -38,14 +38,29 @@ func (s *S3Storer) PutWithProgress(
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Get retrieves data from the specified key.
|
// 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) {
|
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.
|
// 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) {
|
func (s *S3Storer) Stat(ctx context.Context, key string) (*ObjectInfo, error) {
|
||||||
info, err := s.client.StatObject(ctx, key)
|
info, err := s.client.StatObject(ctx, key)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
|
if s3.IsNotFound(err) {
|
||||||
|
return nil, fmt.Errorf("stat %q: %w", key, ErrNotFound)
|
||||||
|
}
|
||||||
|
|
||||||
return nil, err
|
return nil, err
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -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)
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user