Go on with a backup when the destination marker cannot be checked (closes #389)

A source host whose credentials allow only PUT, which the documents
say is enough to back up, failed every backup after its first: the
marker check looks the marker up with an S3 HEAD, which those
credentials are refused, and any error other than not found was
fatal.

Such an error now means the check could not be made: it is logged at
info level, so --cron stays silent, and the backup goes on and writes
the marker. Not found still stops the backup; the check exists for a
file:// volume's empty mount point, which always answers not found.
The fault-injecting test store gains a Stat hook for the test.

Model: opus-5-5

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
+// bytes, and a delete or a stat the backend refuses. It is the seam
-// the fault-injection tests (sneak/vaultik issue 72) and is meant to be
+// called for by the fault-injection tests (sneak/vaultik issue 72) and
-// reused by future tests rather than re-implemented per case.
+// 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
+// each read's bytes are returned, OnDelete decides whether each delete
-// delete fails. All are keyed by the object key, so a test can fault
+// fails, and OnStat whether each stat fails. All are keyed by the object
-// only blobs, only metadata, or a single 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)
 }
 

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

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