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 command goes on. Not found
still stops it. The documents now say so, and that such a host does
not notice an emptied destination, so vaultik database delete must be
run on it after the destination is emptied. The fault-injecting test
store gains a Stat hook for the test.

Model: opus-5-5

ARCHITECTURE.md

@@ -382,15 +382,19 @@ bucket/
         └── manifest.json.zst        # Blob list (for pruning/verification)
 ```
 
-A destination without `vaultik-destination.txt` is treated as not present
-while the local index holds snapshots stored there (`checkDestinationMarker`
-in `internal/vaultik/destination_marker.go`): `snapshot create` refuses to
-write to it, `prune`, `snapshot purge` and `remote nuke` fail, and
-`snapshot list` and `snapshot remove` behave as for a destination that cannot
-be listed. This keeps those commands from taking an unplugged volume's empty
-mount point for a destination whose snapshots are all gone. The `file://`
-backend never creates its destination directory, so a write to a missing one
-fails.
+A destination that answers a look-up of `vaultik-destination.txt` with not
+found is treated as not present while the local index holds snapshots stored
+there (`checkDestinationMarker` in `internal/vaultik/destination_marker.go`):
+`snapshot create` refuses to write to it, `prune`, `snapshot purge` and
+`remote nuke` fail, and `snapshot list` and `snapshot remove` behave as for a
+destination that cannot be listed. This keeps those commands from taking an
+unplugged volume's empty mount point for a destination whose snapshots are
+all gone. A destination that refuses the look-up, as S3 does to credentials
+that allow only PUT, is not checked, so such a host does not notice an
+emptied destination; see
+[docs/REPOSTRUCTURE.md](docs/REPOSTRUCTURE.md#marker-file-vaultik-destinationtxt).
+The `file://` backend never creates its destination directory, so a write to a
+missing one fails.
 
 The `{remote-key}` directory name is a one-way double SHA-256 hash of the human
 snapshot ID, so the human ID (hostname, snapshot name, timestamp) is never

README.md

@@ -332,7 +332,9 @@ value where a list belongs or a word where a number belongs), is refused.
   command fails before the backup starts.
 * Fails before it writes anything when a `file://` destination directory
   does not exist, or when the local index holds snapshots and the
-  destination has no `vaultik-destination.txt` (see
+  destination answers that it has no `vaultik-destination.txt`. A
+  destination that refuses that look-up, as S3 does to credentials that
+  allow only PUT, is not checked (see
   [remote storage layout](#remote-storage-layout))
 
 **`snapshot list`**: Show every snapshot known to this host — the union
@@ -577,15 +579,22 @@ For an implementation-level view of the internals — the data model, the
 ```
 
 * `vaultik-destination.txt` holds one fixed line of text, with no host
-  name, path or time. A destination without it is treated as not
-  present while the local index holds snapshots stored there:
-  `snapshot create` refuses to write to it, `prune`, `snapshot purge`
-  and `remote nuke` fail, and `snapshot list` and `snapshot remove`
-  behave as for a destination that cannot be listed. This keeps an
-  unplugged volume's empty mount point from being taken for a
-  destination whose snapshots are all gone. Mount
+  name, path or time. While the local index holds snapshots stored
+  there, a destination that answers a look-up of it with not found is
+  treated as not present: `snapshot create` refuses to write to it,
+  `prune`, `snapshot purge` and `remote nuke` fail, and `snapshot list`
+  and `snapshot remove` behave as for a destination that cannot be
+  listed. This keeps an unplugged volume's empty mount point from being
+  taken for a destination whose snapshots are all gone. Mount
   the volume if it is removable; if the destination was emptied on
   purpose, run `vaultik database delete` and back up again.
+  When the store refuses the look-up, the check is skipped and the
+  command goes on. S3 refuses it on every run to credentials that
+  allow only PUT, so a host with such credentials does not notice a
+  destination emptied since its last backup, and its next backup would
+  leave out the data its local index records as already stored. After
+  the destination is emptied, run `vaultik database delete` on such a
+  host before it backs up again.
 * Blobs are two-level directory sharded using the first 4 hex chars of the blob hash
 * `db.zst.age` is a binary SQLite database (zstd compressed, age encrypted)
   containing all file metadata, chunk mappings, and relationships for the snapshot

docs/REPOSTRUCTURE.md

@@ -27,7 +27,9 @@ The metadata subdirectory is named with the **remote key**, a one-way hash of th
 
 ## Marker File (`vaultik-destination.txt`)
 
-Every backup writes this file at the root before it uploads anything. It holds one fixed line of plain text and names no host, path or time. A destination without it is treated as not present while the local index holds snapshots stored there: `snapshot create` refuses to write to it, and `prune`, `snapshot purge`, `snapshot remove`, `snapshot list` and `remote nuke` behave as for a destination that cannot be listed. An unplugged volume leaves an empty directory at its mount point, and without the marker that directory would read as a destination whose snapshots are all gone. `remote nuke` deletes the marker last.
+Every backup writes this file at the root before it uploads anything. It holds one fixed line of plain text and names no host, path or time. While the local index holds snapshots stored there, a destination that answers a look-up of the marker with not found is treated as not present: `snapshot create` refuses to write to it, and `prune`, `snapshot purge`, `snapshot remove`, `snapshot list` and `remote nuke` behave as for a destination that cannot be listed. An unplugged volume leaves an empty directory at its mount point, and without the marker that directory would read as a destination whose snapshots are all gone. `remote nuke` deletes the marker last.
+
+When the store refuses the look-up of the marker, the check is skipped and the command goes on. S3 refuses it on every run to credentials that allow only PUT, so a host with such credentials does not notice a destination emptied since its last backup, and its next backup would leave out the data its local index records as already stored. After the destination is emptied, run `vaultik database delete` on such a host before it backs up again.
 
 ## Blobs Directory (`blobs/`)
 

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)
 }
 

internal/vaultik/destination_marker.go

@@ -5,14 +5,16 @@ import (
 	"fmt"
 	"strings"
 
+	"sneak.berlin/go/vaultik/internal/log"
 	"sneak.berlin/go/vaultik/internal/storage"
 )
 
 // destinationMarkerKey is the object every backup writes at the root of
 // the backup destination store. While the local index holds a snapshot
-// stored there, a destination without it is treated as not present: an
-// unplugged volume leaves an empty directory at its mount point, which
-// would otherwise read as a store whose snapshots are all gone.
+// stored there, a destination that answers a look-up of it with not
+// found is treated as not present: an unplugged volume leaves an empty
+// directory at its mount point, which would otherwise read as a store
+// whose snapshots are all gone.
 const destinationMarkerKey = "vaultik-destination.txt"
 
 // destinationMarkerText is the whole content of the marker. It names no
@@ -20,8 +22,8 @@ const destinationMarkerKey = "vaultik-destination.txt"
 const destinationMarkerText = "This is a vaultik backup destination store.\n"
 
 // errDestinationNotPresent is returned when the local index holds a
-// snapshot stored on the backup destination store and the store has no
-// marker.
+// snapshot stored on the backup destination store and the store answers
+// that it has no marker.
 var errDestinationNotPresent = errors.New("backup destination store is not present")
 
 // checkDestinationMarker returns errDestinationNotPresent when the local
@@ -30,6 +32,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 +73,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

internal/vaultik/prune.go

@@ -58,8 +58,8 @@ func (v *Vaultik) NukeRemote(force bool) error {
 
 	// Checked before anything is deleted: running nuke again would fail
 	// the same way, so the advice below must not follow these errors.
-	// The listing also fails on a destination without the marker while
-	// the local index holds snapshots.
+	// The listing also fails on a destination that answers that it has
+	// no marker while the local index holds snapshots.
 	err := v.EnsureStorageBinding()
 	if err != nil {
 		return err