diff --git a/internal/storage/faultstore/faultstore.go b/internal/storage/faultstore/faultstore.go index bdc4dc3..10afb4f 100644 --- a/internal/storage/faultstore/faultstore.go +++ b/internal/storage/faultstore/faultstore.go @@ -2,15 +2,17 @@ // faults on demand, so tests can reproduce the failure modes a real // backend exhibits: an upload that fails partway, a backend that reports // success while storing nothing, reads that return corrupt or truncated -// bytes, and a delete the backend refuses. It is the seam called for by -// the fault-injection tests (sneak/vaultik issue 72) and is meant to be -// reused by future tests rather than re-implemented per case. +// bytes, and a delete or a stat the backend refuses. It is the seam +// called for by the fault-injection tests (sneak/vaultik issue 72) and +// is meant to be reused by future tests rather than re-implemented per +// case. // -// The wrapper delegates every method to the inner Storer. Three hooks +// The wrapper delegates every method to the inner Storer. Four hooks // change that: OnPut decides the fate of each write, OnGet decides how -// each read's bytes are returned, and OnDelete decides whether each -// delete fails. All are keyed by the object key, so a test can fault -// only blobs, only metadata, or a single object. +// each read's bytes are returned, OnDelete decides whether each delete +// fails, and OnStat whether each stat fails. All are keyed by the object +// key, so a test can fault only blobs, only metadata, or a single +// object. package faultstore import ( @@ -30,6 +32,11 @@ var ErrInjectedUpload = errors.New("faultstore: injected upload failure") // fail. var ErrInjectedDelete = errors.New("faultstore: injected delete failure") +// ErrInjectedStat is returned by a Stat the OnStat hook chose to fail. +// It is not storage.ErrNotFound: the backend refused the request, as S3 +// refuses a HEAD to credentials that allow only PUT. +var ErrInjectedStat = errors.New("faultstore: injected stat failure") + // PutAction is the disposition OnPut assigns to a write. type PutAction int @@ -76,6 +83,10 @@ type Storer struct { // object key. Returning true fails the delete and leaves the object // in place. OnDelete func(key string) bool + + // OnStat, when set, is consulted before every Stat with the object + // key. Returning true fails the stat with ErrInjectedStat. + OnStat func(key string) bool } // New wraps inner. inner must be non-nil. @@ -159,8 +170,12 @@ func damage(fault GetFault, data []byte) []byte { return out } -// Stat delegates unchanged. +// Stat returns the object's info unless OnStat fails the stat. func (f *Storer) Stat(ctx context.Context, key string) (*storage.ObjectInfo, error) { + if f.OnStat != nil && f.OnStat(key) { + return nil, fmt.Errorf("%w for %q", ErrInjectedStat, key) + } + return f.inner.Stat(ctx, key) } diff --git a/internal/vaultik/destination_marker.go b/internal/vaultik/destination_marker.go index 7ce437d..8a42189 100644 --- a/internal/vaultik/destination_marker.go +++ b/internal/vaultik/destination_marker.go @@ -5,6 +5,7 @@ import ( "fmt" "strings" + "sneak.berlin/go/vaultik/internal/log" "sneak.berlin/go/vaultik/internal/storage" ) @@ -30,6 +31,14 @@ var errDestinationNotPresent = errors.New("backup destination store is not prese // first backup or on another machine, there is nothing to check; nor // when the local index is bound to a different destination than the // configured one, since its snapshots are stored there. +// +// A look-up of the marker that fails for any other reason than not +// found is logged and otherwise ignored, because the check could not be +// made: S3 refuses it to credentials that allow only PUT, which is all +// a backup needs. The check exists for a file:// volume's empty mount +// point, which always answers not found, and a destination that cannot +// be reached fails on the next request made to it, such as the backup's +// PUT of the marker. func (v *Vaultik) checkDestinationMarker() error { ids, err := v.localSnapshotIDs() if err != nil { @@ -63,8 +72,11 @@ func (v *Vaultik) checkDestinationMarker() error { destinationMarkerKey) } + // Info, not a warning: --cron must stay silent on a successful + // backup from a host whose credentials allow only PUT. if err != nil { - return fmt.Errorf("checking for %s: %w", destinationMarkerKey, err) + log.Info("Could not check the backup destination store for its marker", + "key", destinationMarkerKey, "error", err) } return nil diff --git a/internal/vaultik/destination_marker_test.go b/internal/vaultik/destination_marker_test.go index 08cb35b..5605bcc 100644 --- a/internal/vaultik/destination_marker_test.go +++ b/internal/vaultik/destination_marker_test.go @@ -13,6 +13,7 @@ import ( "sneak.berlin/go/vaultik/internal/database" "sneak.berlin/go/vaultik/internal/log" "sneak.berlin/go/vaultik/internal/storage" + "sneak.berlin/go/vaultik/internal/storage/faultstore" "sneak.berlin/go/vaultik/internal/vaultik" ) @@ -93,6 +94,39 @@ func TestBackupRefusesDestinationWithoutMarker(t *testing.T) { assertUntouched(ctx, t, repos, storeDir) } +// TestBackupGoesOnWhenMarkerCannotBeChecked checks that a backup goes on +// and writes the marker when looking it up fails with an error other +// than not found, as S3 answers credentials that allow only PUT +// (https://git.eeqj.de/sneak/vaultik/issues/389), and that a marker +// that is not found still stops it. +func TestBackupGoesOnWhenMarkerCannotBeChecked(t *testing.T) { + log.Initialize(log.Config{}) + t.Parallel() + + ctx := context.Background() + storeDir := t.TempDir() + v, repos, _ := backUpToFileDestination(ctx, t, storeDir) + + // Removed so the test sees the backup write it again: with the + // look-up failing, the backup cannot tell that it is gone. + markerPath := filepath.Join(storeDir, markerFile) + require.NoError(t, os.Remove(markerPath)) + + store := faultstore.New(v.Storage) + store.OnStat = func(key string) bool { return key == markerFile } + v.Storage = store + + require.NoError(t, backUp(v, "second")) + localSnapshotID(ctx, t, repos, "second") + assert.FileExists(t, markerPath) + + require.NoError(t, os.Remove(markerPath)) + + store.OnStat = nil + + require.ErrorContains(t, backUp(v, "third"), markerFile) +} + // TestBackupChecksMarkerBeforeDroppingIncompleteSnapshots checks that // snapshot create writes nothing when the local index holds only an // incomplete snapshot and the destination has no marker. The backup drops