From 07c957f84bf38145fefa50d934e7b871e88840cc Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Tue, 13 Oct 2026 03:48:36 +0200 Subject: [PATCH] Bind the local index to the s3.* destination too (closes #298) 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 --- README.md | 3 +- TODO.md | 9 +++++ internal/vaultik/storage_bind.go | 20 +++++------ internal/vaultik/storage_bind_test.go | 48 +++++++++++++++++++++++++++ 4 files changed, 69 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 5c58a8f..1523495 100644 --- a/README.md +++ b/README.md @@ -412,7 +412,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 --- diff --git a/TODO.md b/TODO.md index 018c80c..61fdbc6 100644 --- a/TODO.md +++ b/TODO.md @@ -22,6 +22,15 @@ the tag exists and is exercised; what is left is merging `next` to # Completed Steps +- 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 number given to `--older-than` or `--keep-newer-than` was multiplied diff --git a/internal/vaultik/storage_bind.go b/internal/vaultik/storage_bind.go index 2570d48..d9fe081 100644 --- a/internal/vaultik/storage_bind.go +++ b/internal/vaultik/storage_bind.go @@ -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" } diff --git a/internal/vaultik/storage_bind_test.go b/internal/vaultik/storage_bind_test.go index a2f8272..69d97b6 100644 --- a/internal/vaultik/storage_bind_test.go +++ b/internal/vaultik/storage_bind_test.go @@ -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") + }) + } +}