Author SHA1 Message Date
clawbot c196c1a25e Make snapshot remove fail for an unknown snapshot ID (closes #293)
check / check (push) Waiting to run
snapshot remove never checked that the ID existed: it deleted nothing,
reported the snapshot removed from the local database and from the
destination store, and exited 0, under --json, --dry-run and
--local-only too. It now looks the ID up in the local index and, only
when that has no row and --local-only is not set, on the destination
store; finding neither is an error. The command reports a removal from
the local database only when the local index had the row, and
remote_removed is true only when metadata was actually deleted.

Judgement call: under --local-only the destination store is not
consulted, so the snapshot must be in the local index.

Model: opus-5-5
2026-10-13 02:40:12 +00:00
clawbot 07c957f84b Bind the local index to the s3.* destination too (closes #298)
check / check (push) Waiting to run
The local index was bound to its destination only through storage_url.
With the s3.* form nothing was checked, so after s3.bucket or s3.prefix
changed the next backup treated every chunk as already stored, uploaded
nothing, and completed a snapshot that could not be restored.

Without storage_url, the binding now records the destination the s3.*
fields name, written as an s3:// URL from s3.endpoint, s3.bucket and
s3.prefix, and a change is refused with the same error and advice. The
advice line now names the s3.* fields as well as storage_url.

Judgement call: the fields are compared as written, so an equivalent
spelling (a trailing slash on the prefix, a scheme on the endpoint) is
also refused, as it already is for storage_url.

Model: opus-5-5
2026-10-13 03:48:36 +02:00
11 changed files with 311 additions and 144 deletions
+8 -8
View File
@@ -349,9 +349,11 @@ the backup destination store. Blobs are NOT touched — deleting blobs
requires reading every remaining remote manifest (the destination store
may hold snapshots this host doesn't know about), which is what
`vaultik prune` does. On success the command prints the exact `vaultik
prune` invocation to run as a follow-up. Local row cleanup (files,
chunks, blobs the snapshot was the last referrer for) runs
automatically. If the destination store is unreachable, the local-DB
prune` invocation to run as a follow-up. An ID that names no snapshot in
the local index or on the destination store is an error; with
`--local-only`, the snapshot must be in the local index. Local row
cleanup (files, chunks, blobs the snapshot was the last referrer for)
runs automatically. If the destination store is unreachable, the local-DB
removal still completes and a warning is emitted; run `vaultik snapshot
remove <snapshot-id>` again once the store is reachable to remove the
snapshot's metadata from it (`vaultik prune` does not). To wipe everything
@@ -412,7 +414,8 @@ destination store.
entirely. Remote storage is unaffected; the next backup will do a full
scan and re-deduplicate against existing remote blobs, and the local
index will re-bind to the currently configured storage destination.
Use this after changing `storage_url` to a different destination.
Use this after changing `storage_url`, or `s3.endpoint`, `s3.bucket` or
`s3.prefix`, to a different destination.
* `--force`: Skip confirmation prompt
---
@@ -426,10 +429,7 @@ object store. Credentials are read from `s3.access_key_id` and
`s3.secret_access_key` in the config file.
**Local filesystem** (`file:///path/to/backup`): Stores blobs and metadata on
a local or mounted filesystem. Useful for testing or backing up to a NAS. The
path must be absolute; a relative one is refused when the config loads.
Percent-escapes in the path are decoded, so `my%20stick` and `my stick` name
the same directory.
a local or mounted filesystem. Useful for testing or backing up to a NAS.
**Rclone** (`rclone://remote/path`): Uses rclone's 70+ supported cloud
providers. Requires rclone to be configured separately (`rclone config`).
+18 -10
View File
@@ -22,16 +22,24 @@ the tag exists and is exercised; what is left is merging `next` to
# Completed Steps
- 2026-10-13: Refused a `file://` `storage_url` whose path is relative
([issue #299](https://git.eeqj.de/sneak/vaultik/issues/299)). The text
after `file://` was used as written, so `file://dest` resolved against
the working directory: a backup run from another directory wrote to a
different destination, still exited 0, and left a snapshot that could
not be restored. Such a URL now fails when the config loads, with an
error asking for an absolute path. Percent-escapes in the path are now
decoded, so `my%20stick` names the directory `my stick` instead of
creating one named `my%20stick`; a path written with a literal space
works as before.
- 2026-10-13: Made `snapshot remove` fail for a snapshot that does not
exist ([issue #293](https://git.eeqj.de/sneak/vaultik/issues/293)).
The command never checked the ID: it reported the snapshot removed
from the local database and from the destination store and exited 0,
under `--json`, `--dry-run` and `--local-only` too. An ID that names
no snapshot in the local index or on the destination store is now an
error; with `--local-only`, the snapshot must be in the local index.
The command now reports a removal from either side only when one
happened, and `remote_removed` is true only when metadata was deleted.
- 2026-10-12: Bound the local index to the destination an `s3.*` config
names ([issue #298](https://git.eeqj.de/sneak/vaultik/issues/298)).
Only `storage_url` was checked, so after `s3.bucket` or `s3.prefix`
changed, the next backup treated every chunk as already stored,
uploaded nothing, and completed a snapshot that could not be restored.
Without `storage_url`, the local index is now bound to `s3.endpoint`,
`s3.bucket` and `s3.prefix`, and a change is refused as a changed
`storage_url` is.
- 2026-10-12: Rejected a retention duration longer than about 292 years
([issue #289](https://git.eeqj.de/sneak/vaultik/issues/289)). The
+5 -1
View File
@@ -255,8 +255,12 @@ manifest (the destination store may hold snapshots this host doesn't
know about), which is what 'vaultik prune' does. On success the command
prints the exact 'vaultik prune' invocation to run as a follow-up.
An ID that names no snapshot in the local index or on the destination
store is an error.
Use --local-only to skip the remote half (e.g. when you want to forget a
snapshot locally without touching the destination store).
snapshot locally without touching the destination store); the snapshot
must then be in the local index.
If the remote is unreachable, the local-database removal still completes
and a warning is emitted; run 'vaultik snapshot remove <snapshot-id>' again
+2 -12
View File
@@ -61,9 +61,6 @@ var (
errBadS3PartSize = errors.New("s3.part_size must be between 5MiB and 5GiB")
errBadStorageScheme = errors.New(
"storage_url must start with s3://, file://, or rclone://")
errFileURLNotAbsolute = errors.New(
"a file:// storage_url needs an absolute path, " +
"as in file:///path/to/backup")
errStorageNotConfigured = errors.New(
"storage not configured; set storage_url or provide s3.endpoint + " +
"s3.bucket + credentials")
@@ -464,18 +461,11 @@ func (c *Config) validateStorage() error {
// validateStorageURL validates URL-based storage configuration. File and
// rclone URLs need no credentials; S3 URLs require the legacy s3.*
// credential fields. A file URL must name an absolute path: a relative one
// would resolve against the working directory, so the destination would
// move with it.
// credential fields.
func (c *Config) validateStorageURL() error {
switch {
case strings.HasPrefix(c.StorageURL, "file://"):
// storage.ParseStorageURL applies the same rule, but config
// cannot import storage, which imports config.
if !strings.HasPrefix(c.StorageURL, "file:///") {
return errFileURLNotAbsolute
}
// File storage doesn't need S3 credentials
return nil
case strings.HasPrefix(c.StorageURL, "rclone://"):
// Rclone storage uses rclone's own config
-29
View File
@@ -337,35 +337,6 @@ func TestLoadS3PartSize(t *testing.T) {
})
}
// TestLoadRejectsRelativeFileURL checks that a file:// storage_url with a
// relative path fails at load. Accepted, it would resolve against the
// working directory, so a backup run from another directory would write
// to a different destination and still succeed.
func TestLoadRejectsRelativeFileURL(t *testing.T) {
t.Parallel()
const relativeConfig = "snapshots:\n" +
" test:\n" +
" paths: [/tmp/vaultik-test-source]\n" +
"storage_url: file://dest\n"
path := filepath.Join(t.TempDir(), "config.yml")
err := os.WriteFile(path, []byte(relativeConfig), 0o600)
if err != nil {
t.Fatalf("write config: %v", err)
}
_, err = Load(path)
if !errors.Is(err, errFileURLNotAbsolute) {
t.Fatalf("Load() error = %v, want errFileURLNotAbsolute", err)
}
if !strings.Contains(err.Error(), "absolute path") {
t.Errorf("error %q does not say to use an absolute path", err.Error())
}
}
// TestValidateAgeRecipients checks that recipients are parsed at config load
// (a bad entry fails immediately, not mid-backup) and that no invalid entry —
// least of all a pasted secret key — is echoed in the error. An empty list
+12 -26
View File
@@ -17,10 +17,8 @@ const (
// Sentinel errors for storage URL parsing.
var (
ErrEmptyStorageURL = errors.New("storage URL is empty")
ErrEmptyFilePath = errors.New("file URL path is empty")
ErrRelativeFilePath = errors.New(
"file URL needs an absolute path, as in file:///path/to/backup")
ErrEmptyStorageURL = errors.New("storage URL is empty")
ErrEmptyFilePath = errors.New("file URL path is empty")
ErrMissingBucket = errors.New("s3 URL missing bucket name")
ErrMissingRemote = errors.New("rclone URL missing remote name")
ErrUnsupportedScheme = errors.New(
@@ -53,8 +51,17 @@ func ParseStorageURL(rawURL string) (*URL, error) {
return nil, ErrEmptyStorageURL
}
// Handle file:// URLs
if after, ok := strings.CutPrefix(rawURL, "file://"); ok {
return parseFileURL(after)
path := after
if path == "" {
return nil, ErrEmptyFilePath
}
return &URL{
Scheme: schemeFile,
Prefix: path,
}, nil
}
if strings.HasPrefix(rawURL, "s3://") {
@@ -68,27 +75,6 @@ func ParseStorageURL(rawURL string) (*URL, error) {
return nil, ErrUnsupportedScheme
}
// parseFileURL parses the path that follows file://. The path must be
// absolute: a relative one would resolve against the working directory,
// so the destination would move with it. Percent-escapes are decoded, so
// my%20stick names the directory "my stick"; a literal space is kept.
func parseFileURL(rawPath string) (*URL, error) {
if rawPath == "" {
return nil, ErrEmptyFilePath
}
if !strings.HasPrefix(rawPath, "/") {
return nil, ErrRelativeFilePath
}
path, err := url.PathUnescape(rawPath)
if err != nil {
return nil, fmt.Errorf("invalid file URL path: %w", err)
}
return &URL{Scheme: schemeFile, Prefix: path}, nil
}
// parseS3URL parses an s3://bucket/prefix URL. It rejects credentials in
// the userinfo and any query parameter other than endpoint, region and
// ssl, so a credential-bearing URL is never stored or echoed.
+2 -16
View File
@@ -14,10 +14,7 @@ import (
func TestParseStorageURLValid(t *testing.T) {
t.Parallel()
const (
bucket = "mybucket"
fileScheme = "file"
)
const bucket = "mybucket"
cases := []struct {
name string
@@ -27,17 +24,7 @@ func TestParseStorageURLValid(t *testing.T) {
{
name: "file absolute path",
raw: "file:///var/backups/vaultik",
want: &storage.URL{Scheme: fileScheme, Prefix: "/var/backups/vaultik"},
},
{
name: "file percent-escaped path is decoded",
raw: "file:///Volumes/my%20stick/backup",
want: &storage.URL{Scheme: fileScheme, Prefix: "/Volumes/my stick/backup"},
},
{
name: "file path with a literal space",
raw: "file:///Volumes/my stick/backup",
want: &storage.URL{Scheme: fileScheme, Prefix: "/Volumes/my stick/backup"},
want: &storage.URL{Scheme: "file", Prefix: "/var/backups/vaultik"},
},
{
name: "s3 bucket and prefix, ssl defaults on",
@@ -103,7 +90,6 @@ func TestParseStorageURLErrors(t *testing.T) {
}{
{"empty url", "", storage.ErrEmptyStorageURL},
{"file empty path", "file://", storage.ErrEmptyFilePath},
{"file relative path", "file://dest", storage.ErrRelativeFilePath},
{"s3 missing bucket", "s3://", storage.ErrMissingBucket},
{"s3 missing bucket with path", "s3:///justprefix", storage.ErrMissingBucket},
{"rclone missing remote", "rclone://", storage.ErrMissingRemote},
+115
View File
@@ -3,17 +3,21 @@ package vaultik_test
import (
"bytes"
"context"
"database/sql"
"io"
"strings"
"sync"
"testing"
"time"
"github.com/klauspost/compress/zstd"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"sneak.berlin/go/vaultik/internal/database"
"sneak.berlin/go/vaultik/internal/log"
"sneak.berlin/go/vaultik/internal/snapshot"
"sneak.berlin/go/vaultik/internal/storage"
"sneak.berlin/go/vaultik/internal/types"
"sneak.berlin/go/vaultik/internal/vaultik"
)
@@ -214,6 +218,37 @@ func addBlob(t *testing.T, store *testStorer, hash string) {
require.NoError(t, err)
}
// newLocalIndex returns an in-memory local index holding a completed
// snapshot row for each of ids.
func newLocalIndex(t *testing.T, ids ...string) *database.Repositories {
t.Helper()
ctx := context.Background()
db, err := database.New(ctx, ":memory:")
require.NoError(t, err)
t.Cleanup(func() { _ = db.Close() })
repos := database.NewRepositories(db)
for _, id := range ids {
completedAt := time.Now().UTC()
snap := &database.Snapshot{
ID: types.SnapshotID(id),
Hostname: snapHostname,
StartedAt: completedAt,
CompletedAt: &completedAt,
}
err = repos.WithTx(ctx, func(ctx context.Context, tx *sql.Tx) error {
return repos.Snapshots.Create(ctx, tx, snap)
})
require.NoError(t, err)
}
return repos
}
// ============================================================================
// Unit Tests for RemoveSnapshot
// ============================================================================
@@ -233,6 +268,7 @@ func TestRemoveSnapshot_LocalOnly_PreservesRemote(t *testing.T) {
addBlob(t, store, blobA)
tv := vaultik.NewForTesting(store)
tv.Repositories = newLocalIndex(t, "snapshot-001")
opts := &vaultik.RemoveOptions{Force: true, LocalOnly: true}
result, err := tv.RemoveSnapshot("snapshot-001", opts)
@@ -268,6 +304,7 @@ func TestRemoveSnapshot_DefaultRemovesMetadataNotBlobs(t *testing.T) {
addBlob(t, store, blobShared)
tv := vaultik.NewForTesting(store)
tv.Repositories = newLocalIndex(t, "snapshot-001", "snapshot-002")
opts := &vaultik.RemoveOptions{Force: true}
result, err := tv.RemoveSnapshot("snapshot-001", opts)
@@ -316,6 +353,84 @@ func TestRemoveSnapshot_DryRun(t *testing.T) {
assert.Contains(t, tv.Stdout.String(), "[Dry run - no changes made]")
}
// TestRemoveSnapshot_UnknownIDIsError checks that an ID naming no
// snapshot in the local index or on the destination store is an error
// under every option, and that nothing is reported as removed.
func TestRemoveSnapshot_UnknownIDIsError(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
for name, opts := range map[string]vaultik.RemoveOptions{
"force": {Force: true},
"json": {JSON: true},
"dry-run": {Force: true, DryRun: true},
"local-only": {Force: true, LocalOnly: true},
} {
t.Run(name, func(t *testing.T) {
t.Parallel()
store := newTestStorer()
addManifest(t, store, "snapshot-001", nil)
tv := vaultik.NewForTesting(store)
tv.Repositories = newLocalIndex(t, "snapshot-001")
result, err := tv.RemoveSnapshot("snapshot-999", &opts)
require.ErrorContains(t, err, "snapshot not found")
assert.False(t, result.RemoteRemoved)
assert.Empty(t, tv.Stdout.String())
assert.True(t, store.hasKey(remoteKeyPath("snapshot-001")))
})
}
}
// TestRemoveSnapshot_NoMetadataOnDestination checks that removing a
// snapshot whose metadata is not on the destination store does not
// report metadata removed from it.
func TestRemoveSnapshot_NoMetadataOnDestination(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
tv := vaultik.NewForTesting(newTestStorer())
tv.Repositories = newLocalIndex(t, "snapshot-001")
result, err := tv.RemoveSnapshot("snapshot-001",
&vaultik.RemoveOptions{Force: true})
require.NoError(t, err)
assert.False(t, result.RemoteRemoved)
out := tv.Stdout.String()
assert.Contains(t, out, "Removed snapshot 'snapshot-001' from local database")
assert.NotContains(t, out, "Removed snapshot metadata from remote storage")
}
// TestRemoveSnapshot_NotInLocalIndex checks that removing a snapshot
// only the destination store holds removes its metadata there without
// reporting a removal from the local database.
func TestRemoveSnapshot_NotInLocalIndex(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
store := newTestStorer()
addManifest(t, store, "snapshot-001", nil)
tv := vaultik.NewForTesting(store)
tv.Repositories = newLocalIndex(t)
result, err := tv.RemoveSnapshot("snapshot-001",
&vaultik.RemoveOptions{Force: true})
require.NoError(t, err)
assert.True(t, result.RemoteRemoved)
assert.False(t, store.hasKey(remoteKeyPath("snapshot-001")))
out := tv.Stdout.String()
assert.NotContains(t, out, "from local database")
assert.Contains(t, out, "Removed snapshot metadata from remote storage")
}
func TestRemoveAllSnapshots_RequiresForce(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
+91 -32
View File
@@ -23,6 +23,7 @@ var (
errNoSnapshotsInConfig = errors.New("no snapshots configured")
errSnapshotVerifyFailed = errors.New("verification failed")
errRemoveAllNeedsForce = errors.New("--all requires --force")
errSnapshotNotFound = errors.New("snapshot not found")
errInvalidTableName = errors.New("invalid table name")
errNoAgeRecipients = errors.New(
"creating a snapshot needs at least one public key in " +
@@ -643,7 +644,7 @@ func (v *Vaultik) confirmAndExecutePurge(
"snapshot_id", snapshotID, "error", err)
}
err = v.deleteRemoteSnapshotByKey(snapshot.RemoteSnapshotKey(snapshotID))
_, err = v.deleteRemoteSnapshotByKey(snapshot.RemoteSnapshotKey(snapshotID))
if err != nil {
return fmt.Errorf("deleting snapshot %s from remote: %w", snapshotID, err)
}
@@ -1153,6 +1154,11 @@ func (v *Vaultik) RemoveSnapshot(
return result, err
}
inLocalIndex, err := v.findSnapshotToRemove(snapshotID, opts)
if err != nil {
return result, err
}
if opts.DryRun {
return v.removeSnapshotDryRun(result, snapshotID, opts)
}
@@ -1161,11 +1167,13 @@ func (v *Vaultik) RemoveSnapshot(
return result, nil
}
log.Info("Removing snapshot from local database", "snapshot_id", snapshotID)
if inLocalIndex {
log.Info("Removing snapshot from local database", "snapshot_id", snapshotID)
err = v.deleteSnapshotFromLocalDB(snapshotID)
if err != nil {
return result, fmt.Errorf("removing from local database: %w", err)
err = v.deleteSnapshotFromLocalDB(snapshotID)
if err != nil {
return result, fmt.Errorf("removing from local database: %w", err)
}
}
if !opts.LocalOnly {
@@ -1184,9 +1192,11 @@ func (v *Vaultik) RemoveSnapshot(
return result, v.outputRemoveJSON(result)
}
v.stdoutf("Removed snapshot '%s' from local database\n", snapshotID)
if inLocalIndex {
v.stdoutf("Removed snapshot '%s' from local database\n", snapshotID)
}
if !opts.LocalOnly && result.RemoteRemoved {
if result.RemoteRemoved {
v.printlnStdout("Removed snapshot metadata from remote storage")
v.stdoutf("\nNote: The removed snapshot's blobs remain on the remote. "+
"Run '%s' to delete any blobs no longer referenced by any "+
@@ -1196,6 +1206,45 @@ func (v *Vaultik) RemoveSnapshot(
return result, nil
}
// findSnapshotToRemove reports whether the local index has a row for
// snapshotID, and returns errSnapshotNotFound when there is nothing to
// remove: no row and, unless LocalOnly is set, no metadata on the
// destination store either. The destination store is consulted only
// when the local index has no row, so a snapshot in the local index can
// still be removed from it while the destination store is unreachable.
func (v *Vaultik) findSnapshotToRemove(
snapshotID string, opts *RemoveOptions,
) (bool, error) {
if v.Repositories != nil {
snap, err := v.Repositories.Snapshots.GetByID(v.ctx, snapshotID)
if err != nil {
return false, fmt.Errorf("looking up snapshot in local index: %w", err)
}
if snap != nil {
return true, nil
}
}
if opts.LocalOnly {
return false, fmt.Errorf("%w in the local index: %s",
errSnapshotNotFound, snapshotID)
}
objects, err := v.remoteSnapshotObjects(snapshot.RemoteSnapshotKey(snapshotID))
if err != nil {
return false, fmt.Errorf("looking up snapshot on destination store: %w", err)
}
if len(objects) == 0 {
return false, fmt.Errorf(
"%w in the local index or on the destination store: %s",
errSnapshotNotFound, snapshotID)
}
return false, nil
}
// removeSnapshotDryRun reports what RemoveSnapshot would do without
// making changes.
func (v *Vaultik) removeSnapshotDryRun(
@@ -1255,14 +1304,14 @@ func (v *Vaultik) confirmRemoveSnapshot(snapshotID string, opts *RemoveOptions)
// didn't finish and to run `vaultik snapshot remove` for the snapshot
// again once the destination store is reachable (`vaultik prune` never
// removes snapshot metadata). Returns true when the remote removal
// succeeded.
// succeeded and deleted something.
func (v *Vaultik) removeSnapshotRemote(snapshotID string, opts *RemoveOptions) bool {
log.Info("Removing snapshot metadata from remote storage",
"snapshot_id", snapshotID)
remoteKey := snapshot.RemoteSnapshotKey(snapshotID)
err := v.deleteRemoteSnapshotByKey(remoteKey)
deleted, err := v.deleteRemoteSnapshotByKey(remoteKey)
if err != nil {
log.Warn("Could not remove snapshot metadata from remote storage; "+
"run '"+snapshotRemoveCommandHint+"' with the snapshot's ID "+
@@ -1280,7 +1329,7 @@ func (v *Vaultik) removeSnapshotRemote(snapshotID string, opts *RemoveOptions) b
return false
}
return true
return deleted > 0
}
// RemoveAllSnapshots removes every snapshot known to the local
@@ -1572,7 +1621,7 @@ func (v *Vaultik) removeAllLocalSnapshots(
}
if !opts.LocalOnly {
err = v.deleteRemoteSnapshotByKey(
_, err = v.deleteRemoteSnapshotByKey(
snapshot.RemoteSnapshotKey(snapshotID))
if err != nil {
log.Warn("Failed to remove snapshot metadata from remote",
@@ -1598,7 +1647,7 @@ func (v *Vaultik) removeAllOrphanRemotes(
for _, key := range orphanRemoteKeys {
log.Info("Removing orphan remote snapshot", "remote_key", key)
err := v.deleteRemoteSnapshotByKey(key)
_, err := v.deleteRemoteSnapshotByKey(key)
if err != nil {
log.Warn("Failed to remove orphan from remote",
"remote_key", key, "error", err)
@@ -1645,34 +1694,44 @@ func (v *Vaultik) deleteSnapshotFromLocalDB(snapshotID string) error {
}
// deleteRemoteSnapshotByKey removes everything under
// metadata/<remoteKey>/ on the destination store. The argument is a
// remote key (double-SHA256 derivation), not a human snapshot ID;
// callers that have a human ID must hash via snapshot.RemoteSnapshotKey
// first.
func (v *Vaultik) deleteRemoteSnapshotByKey(remoteKey string) error {
prefix := fmt.Sprintf("metadata/%s/", remoteKey)
objectCh := v.Storage.ListStream(v.ctx, prefix)
var objectsToDelete []string
for object := range objectCh {
if object.Err != nil {
return fmt.Errorf("listing objects: %w", object.Err)
}
objectsToDelete = append(objectsToDelete, object.Key)
// metadata/<remoteKey>/ on the destination store and returns how many
// objects it deleted. The argument is a remote key (double-SHA256
// derivation), not a human snapshot ID; callers that have a human ID
// must hash via snapshot.RemoteSnapshotKey first.
func (v *Vaultik) deleteRemoteSnapshotByKey(remoteKey string) (int, error) {
objectsToDelete, err := v.remoteSnapshotObjects(remoteKey)
if err != nil {
return 0, err
}
for _, key := range objectsToDelete {
err := v.Storage.Delete(v.ctx, key)
for i, key := range objectsToDelete {
err = v.Storage.Delete(v.ctx, key)
if err != nil {
return fmt.Errorf("removing %s: %w", key, err)
return i, fmt.Errorf("removing %s: %w", key, err)
}
log.Debug("Deleted remote object", "key", key)
}
return nil
return len(objectsToDelete), nil
}
// remoteSnapshotObjects lists the keys of every object under
// metadata/<remoteKey>/ on the destination store.
func (v *Vaultik) remoteSnapshotObjects(remoteKey string) ([]string, error) {
prefix := fmt.Sprintf("metadata/%s/", remoteKey)
var keys []string
for object := range v.Storage.ListStream(v.ctx, prefix) {
if object.Err != nil {
return nil, fmt.Errorf("listing objects: %w", object.Err)
}
keys = append(keys, object.Key)
}
return keys, nil
}
// outputRemoveJSON outputs the removal result as JSON
+10 -10
View File
@@ -9,7 +9,7 @@ import (
)
// errStorageBindingMismatch is returned when the local index database is
// bound to a different destination than the configured storage_url.
// bound to a different destination than the configured one.
var errStorageBindingMismatch = errors.New(
"local index is bound to a different backup destination")
@@ -24,8 +24,9 @@ var errStorageBindingMismatch = errors.New(
//
// Behaviour:
// - On first use (empty stored value), record the configured
// storage_url and log the binding.
// - When the stored value matches the configured storage_url, do
// destination (storage_url, or the s3.* endpoint, bucket and
// prefix) and log the binding.
// - When the stored value matches the configured destination, do
// nothing and return nil.
// - When the two differ, refuse with an error that tells the user
// how to recover (revert the config, or run `vaultik database
@@ -46,12 +47,10 @@ func (v *Vaultik) EnsureStorageBinding() error {
configured := v.Config.StorageURL
if configured == "" {
// Some legacy configs still use the split s3.* keys instead of
// storage_url. Falling back to a synthetic URL for those would
// hide the fact that the binding is loose. Instead, treat
// unset as "nothing to bind against" — the check is
// necessarily best-effort for those configs.
return nil
// The s3.* form: bind on its endpoint, bucket and prefix,
// written the way storage_url would name the same destination.
s3 := v.Config.S3
configured = "s3://" + s3.Bucket + "/" + s3.Prefix + "?endpoint=" + s3.Endpoint
}
stored, err := v.Repositories.LocalMeta.Get(v.ctx, database.LocalMetaKeyStorageURL)
@@ -96,7 +95,8 @@ func buildBindingMismatchMessage(stored, configured string) string {
"future snapshots referencing blobs that don't exist at the new destination.\n" +
"\n" +
"To proceed, either:\n" +
" - revert storage_url in your config to the bound destination, or\n" +
" - revert storage_url (or the s3.* endpoint, bucket and prefix) in your\n" +
" config to the bound destination, or\n" +
" - run 'vaultik database delete' to discard the local index and rebuild it\n" +
" from a fresh full backup against the new destination"
}
+48
View File
@@ -76,3 +76,51 @@ func TestEnsureStorageBinding_MismatchRefuses(t *testing.T) {
assert.Contains(t, err.Error(), "file:///mnt/backups/new")
assert.Contains(t, err.Error(), "vaultik database delete")
}
// A config with no storage_url names its destination with s3.endpoint,
// s3.bucket and s3.prefix; a change to any of them must be refused just
// as a changed storage_url is.
func TestEnsureStorageBinding_S3FieldsChangeRefuses(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
bound := config.S3Config{
Endpoint: "https://s3.example.com",
Bucket: "bucket-a",
Prefix: "backups",
}
tests := []struct {
name string
changed config.S3Config
}{
{"bucket", config.S3Config{
Endpoint: bound.Endpoint, Bucket: "bucket-b", Prefix: bound.Prefix,
}},
{"prefix", config.S3Config{
Endpoint: bound.Endpoint, Bucket: bound.Bucket, Prefix: "other",
}},
{"endpoint", config.S3Config{
Endpoint: "https://s3.other.com", Bucket: bound.Bucket, Prefix: bound.Prefix,
}},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
tv, _ := buildBindTestVaultik(t, "")
tv.Config.S3 = bound
require.NoError(t, tv.EnsureStorageBinding(), "first use binds")
require.NoError(t, tv.EnsureStorageBinding(), "same destination passes")
tv.Config.S3 = tt.changed
err := tv.EnsureStorageBinding()
require.Error(t, err)
assert.Contains(t, err.Error(),
"local index is bound to a different backup destination")
assert.Contains(t, err.Error(), "vaultik database delete")
})
}
}