Compare commits

..
Author SHA1 Message Date
sneak c0c79cd0a2 Make prune and remote nuke fail when a delete fails (closes #290)
check / check (push) Waiting to run
prune, and the prune step of snapshot create --prune, counted a blob
it could not delete and still exited 0. It now prints its summary or
--json document and then returns an error, which replaces the
"Failed to delete N blob(s)" line.

remote nuke only warned when a snapshot's metadata could not be
deleted, advised vaultik prune, which never removes metadata, and then
reported the store empty. It now stops with an error, does not report
the store empty, and says to run vaultik remote nuke --force again. It
checks that the local index belongs to the configured destination
before deleting anything, and that error carries no such advice. A
successful nuke no longer advises prune.

Model: opus-5-5
2026-10-13 05:44:12 +00:00
27 changed files with 319 additions and 612 deletions
+3 -24
View File
@@ -5,16 +5,7 @@
FROM golangci/golangci-lint:v2.14.0@sha256:ad862ba6b3798cbe0fd9fd7408d498fd74fbd2623a92406b2fd3898faf0bf98f AS lint FROM golangci/golangci-lint:v2.14.0@sha256:ad862ba6b3798cbe0fd9fd7408d498fd74fbd2623a92406b2fd3898faf0bf98f AS lint
WORKDIR /src WORKDIR /src
COPY go.mod go.sum ./ COPY go.mod go.sum ./
# Without the time limit a stalled connection to the module proxy holds RUN go mod download
# this step, and every build on the host sharing it, until the kernel
# drops the connection, which can take an hour. The download normally
# takes under half a minute.
RUN for attempt in 1 2 3; do \
timeout 180 go mod download && exit 0; \
echo "go mod download attempt $attempt failed or ran over 180s" >&2; \
done; \
echo "module download failed: giving up after 3 attempts" >&2; \
exit 1
COPY . . COPY . .
# `golangci-lint run` silently ignores an unknown top-level key in # `golangci-lint run` silently ignores an unknown top-level key in
# .golangci.yml, such as a misspelt `linters:`; `config verify` fails on it. # .golangci.yml, such as a misspelt `linters:`; `config verify` fails on it.
@@ -27,13 +18,7 @@ RUN golangci-lint run --config .golangci.yml ./...
FROM golang:1.26.1@sha256:cd78d88e00afadbedd272f977d375a6247455f3a4b1178f8ae8bbcb201743a8a AS test FROM golang:1.26.1@sha256:cd78d88e00afadbedd272f977d375a6247455f3a4b1178f8ae8bbcb201743a8a AS test
WORKDIR /src WORKDIR /src
COPY go.mod go.sum ./ COPY go.mod go.sum ./
# Limited and retried as in the lint phase above. RUN go mod download
RUN for attempt in 1 2 3; do \
timeout 180 go mod download && exit 0; \
echo "go mod download attempt $attempt failed or ran over 180s" >&2; \
done; \
echo "module download failed: giving up after 3 attempts" >&2; \
exit 1
COPY . . COPY . .
RUN go test -timeout 90s -race -cover ./... || \ RUN go test -timeout 90s -race -cover ./... || \
{ echo "--- Rerunning with -v for details ---"; \ { echo "--- Rerunning with -v for details ---"; \
@@ -51,13 +36,7 @@ RUN apk add --no-cache git
RUN git config --system --add safe.directory /src RUN git config --system --add safe.directory /src
WORKDIR /src WORKDIR /src
COPY go.mod go.sum ./ COPY go.mod go.sum ./
# Limited and retried as in the lint phase above. RUN go mod download
RUN for attempt in 1 2 3; do \
timeout 180 go mod download && exit 0; \
echo "go mod download attempt $attempt failed or ran over 180s" >&2; \
done; \
echo "module download failed: giving up after 3 attempts" >&2; \
exit 1
COPY . . COPY . .
# The VERSION build arg when one is given, otherwise # The VERSION build arg when one is given, otherwise
+9 -13
View File
@@ -286,9 +286,7 @@ needed.
remove orphaned blobs from remote storage. By default keeps only the latest remove orphaned blobs from remote storage. By default keeps only the latest
snapshot per name; use `--keep-newer-than` for a rolling window. snapshot per name; use `--keep-newer-than` for a rolling window.
* `--keep-newer-than <duration>`: With `--prune`, keep snapshots newer than * `--keep-newer-than <duration>`: With `--prune`, keep snapshots newer than
this duration instead of only the latest (e.g. `4w`, `30d`, `6mo`, `1y`). this duration instead of only the latest (e.g. `4w`, `30d`, `6mo`, `1y`)
Without `--prune`, or with a value that is not a valid duration, the
command fails before the backup starts.
**`snapshot list`**: Show every snapshot known to this host — the union **`snapshot list`**: Show every snapshot known to this host — the union
of the local index and the backup destination store — with timestamps of the local index and the backup destination store — with timestamps
@@ -386,10 +384,12 @@ manifest on the destination store to compute the still-referenced blob
set and delete any blob not in that set. Step (3) reads all remote set and delete any blob not in that set. Step (3) reads all remote
manifests — network cost scales with the number of snapshots. `snapshot manifests — network cost scales with the number of snapshots. `snapshot
create --prune` runs the same cleanup automatically; this is the create --prune` runs the same cleanup automatically; this is the
manual entry point for the same work. manual entry point for the same work. If a blob cannot be deleted, the
command (and `snapshot create --prune`) exits non-zero.
* `--force`: Skip confirmation prompt * `--force`: Skip confirmation prompt
* `--json`: Output stats as JSON. Also skips the confirmation prompt, as * `--json`: Output stats as JSON. Also skips the confirmation prompt, as
`--force` does. `--force` does. A blob that cannot be deleted is counted in
`blobs_failed`, and the document is printed before the non-zero exit.
**`info`**: Display system configuration, storage settings, encryption **`info`**: Display system configuration, storage settings, encryption
recipients, and local database statistics. recipients, and local database statistics.
@@ -403,15 +403,14 @@ orphaned blob figures are reported as unknown; `--json` gives them as
`null`, lists the remote key of each unreadable manifest in `null`, lists the remote key of each unreadable manifest in
`unreadable_manifests` and counts the manifests under skipped names in `unreadable_manifests` and counts the manifests under skipped names in
`skipped_manifest_count`. An unreadable manifest also leaves its `skipped_manifest_count`. An unreadable manifest also leaves its
snapshot's blob count and blob size unknown, `null` in `--json`. A file snapshot's blob count and blob size unknown, `null` in `--json`.
under `blobs/` whose name is not a blob hash is not counted as a blob,
since `prune` never deletes it.
* `--json`: Output as JSON * `--json`: Output as JSON
**`remote nuke`**: Delete every snapshot's metadata and every blob from the **`remote nuke`**: Delete every snapshot's metadata and every blob from the
backup destination store, leaving the bucket prefix empty. Destructive and backup destination store, leaving the bucket prefix empty. Destructive and
irreversible. This is the single supported way to wipe the entire irreversible. This is the single supported way to wipe the entire
destination store. destination store. If anything cannot be deleted, the command exits
non-zero; running it again deletes what is left.
* `--force`: Required to confirm destruction. * `--force`: Required to confirm destruction.
**`database delete`**: Delete the local SQLite state database file **`database delete`**: Delete the local SQLite state database file
@@ -433,10 +432,7 @@ object store. Credentials are read from `s3.access_key_id` and
`s3.secret_access_key` in the config file. `s3.secret_access_key` in the config file.
**Local filesystem** (`file:///path/to/backup`): Stores blobs and metadata on **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 a local or mounted filesystem. Useful for testing or backing up to a NAS.
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.
**Rclone** (`rclone://remote/path`): Uses rclone's 70+ supported cloud **Rclone** (`rclone://remote/path`): Uses rclone's 70+ supported cloud
providers. Requires rclone to be configured separately (`rclone config`). providers. Requires rclone to be configured separately (`rclone config`).
+10 -7
View File
@@ -22,13 +22,16 @@ the tag exists and is exercised; what is left is merging `next` to
# Completed Steps # Completed Steps
- 2026-10-13: Stopped `remote info` counting a file under `blobs/` that - 2026-10-13: Made `prune` and `remote nuke` exit non-zero when a delete
is not a blob as an orphaned blob fails ([issue #290](https://git.eeqj.de/sneak/vaultik/issues/290)).
([issue #295](https://git.eeqj.de/sneak/vaultik/issues/295)). A stray `prune`, and the prune step of `snapshot create --prune`, counted a
file such as `.DS_Store` was reported as an orphaned blob and the blob it could not delete and still exited 0; it now prints its summary
report advised running `prune`, which skips such a file and deleted or `--json` document and then fails. `remote nuke` warned about a
nothing. `remote info` now takes its blob list from the same listing snapshot's metadata it could not delete, said to run `vaultik prune`,
`prune` uses, in the table and in `--json`. which never removes metadata, and then reported the store empty. It
now fails without that report and says to run `vaultik remote nuke
--force` again, and a successful nuke no longer advises `prune` for
blobs it deletes itself.
- 2026-10-13: Made `snapshot remove` fail for a snapshot that does not - 2026-10-13: Made `snapshot remove` fail for a snapshot that does not
exist ([issue #293](https://git.eeqj.de/sneak/vaultik/issues/293)). exist ([issue #293](https://git.eeqj.de/sneak/vaultik/issues/293)).
-45
View File
@@ -1,45 +0,0 @@
package main_test
import (
"strings"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
// TestDockerfileBoundsEveryModuleDownload fails unless every
// `go mod download` in the Dockerfile runs under a time limit, is tried
// a fixed number of times, and then fails the build with a message that
// names the module download (issue #351). Like the version tests, this
// parses the Dockerfile; that a stalled download really is cut off and
// retried was checked by hand.
func TestDockerfileBoundsEveryModuleDownload(t *testing.T) {
t.Parallel()
var downloads []string
for _, instruction := range instructions(t, productDockerfile) {
if strings.Contains(instruction, "go mod download") {
downloads = append(downloads, instruction)
}
}
require.NotEmpty(t, downloads, "%s must download modules",
productDockerfile)
for _, download := range downloads {
assert.Contains(t, download, "timeout 180 go mod download",
"%s: a module download must have a time limit: %s",
productDockerfile, download)
assert.Contains(t, download, "for attempt in 1 2 3",
"%s: a module download must be retried: %s",
productDockerfile, download)
assert.Contains(t, download, "module download failed",
"%s: a module download that never succeeds must say so: %s",
productDockerfile, download)
assert.True(t, strings.HasSuffix(download, "exit 1"),
"%s: a module download that never succeeds must fail the"+
" build: %s", productDockerfile, download)
}
}
-11
View File
@@ -245,17 +245,6 @@ func (p *Packer) Flush(ctx context.Context) error {
return nil return nil
} }
// DiscardCurrentBlob closes and removes the temp file of the blob being
// packed, if any, and drops that blob. A caller that stops before Flush
// must call it, or the temp file is left behind. Thread-safe.
func (p *Packer) DiscardCurrentBlob() {
p.mu.Lock()
defer p.mu.Unlock()
p.cleanupTempFile()
p.currentBlob = nil
}
// FinalizeBlob finalizes the current blob being assembled. // FinalizeBlob finalizes the current blob being assembled.
// This compresses the accumulated chunks, encrypts the result, and computes // This compresses the accumulated chunks, encrypts the result, and computes
// the content-addressed hash. The finalized blob is either passed to the // the content-addressed hash. The finalized blob is either passed to the
-6
View File
@@ -251,12 +251,6 @@ func NewConfigCommand() *cobra.Command {
Use: "config", Use: "config",
Short: "Manage the configuration file", Short: "Manage the configuration file",
Long: "Commands for creating, editing, and querying the vaultik config file.", Long: "Commands for creating, editing, and querying the vaultik config file.",
// cobra checks Args only on a command that has a Run: without
// one, a mistyped subcommand printed help and exited 0.
Args: cobra.NoArgs,
Run: func(cmd *cobra.Command, _ []string) {
_ = cmd.Help()
},
} }
cmd.AddCommand(newConfigInitCommand()) cmd.AddCommand(newConfigInitCommand())
-6
View File
@@ -15,12 +15,6 @@ func NewDatabaseCommand() *cobra.Command {
Use: "database", Use: "database",
Short: "Manage the local state database", Short: "Manage the local state database",
Long: `Commands for managing the local SQLite state database.`, Long: `Commands for managing the local SQLite state database.`,
// cobra checks Args only on a command that has a Run: without
// one, a mistyped subcommand printed help and exited 0.
Args: cobra.NoArgs,
Run: func(cmd *cobra.Command, _ []string) {
_ = cmd.Help()
},
} }
cmd.AddCommand( cmd.AddCommand(
@@ -1,108 +0,0 @@
package cli //nolint:testpackage // shares runEntry and the hermetic config helpers
import (
"os"
"path/filepath"
"strings"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
// Flags of `snapshot create` used to build argument vectors below.
const (
flagPrune = "--prune"
flagKeepNewerThan = "--keep-newer-than"
)
// TestEntrySnapshotCreateRejectsKeepNewerThanBeforeBackup checks that a
// --keep-newer-than the post-backup prune could not use stops `snapshot
// create` before the backup: the command fails and the destination
// store stays empty. It used to be read only by the prune, after the
// snapshot was uploaded, and without --prune it was ignored.
//
//nolint:paralleltest // replaces os.Args, os.Stdout, os.Stderr and the xdg globals
func TestEntrySnapshotCreateRejectsKeepNewerThanBeforeBackup(t *testing.T) {
for _, testCase := range []struct {
name string
args []string
}{
{
name: "decimal in a day count",
args: []string{flagPrune, flagKeepNewerThan, "1.5d"},
},
{
name: "unknown unit",
args: []string{flagPrune, flagKeepNewerThan, "4x"},
},
{
name: "without --prune",
args: []string{flagKeepNewerThan, "4w"},
},
} {
t.Run(testCase.name, func(t *testing.T) {
configPath, storeDir := writeBackupConfig(t)
code, _, _ := runEntry(t, append(
[]string{flagConfig, configPath, cmdSnapshot, cmdCreate},
testCase.args...)...)
assert.Equal(t, 1, code)
stored, err := os.ReadDir(storeDir)
require.NoError(t, err)
assert.Empty(t, stored, "nothing may be uploaded")
})
}
}
// TestEntrySnapshotCreateReportsPostBackupPruneFailure makes the
// post-backup prune fail after a good backup, by planting a manifest
// that cannot be decoded, and checks that the failure is reported as the
// prune's and not as a failed snapshot creation.
//
//nolint:paralleltest // replaces os.Args, os.Stdout, os.Stderr and the xdg globals
func TestEntrySnapshotCreateReportsPostBackupPruneFailure(t *testing.T) {
configPath, storeDir := writeBackupConfig(t)
// A remote key is 64 lowercase hex characters. The prune reads every
// manifest on the store and fails on one it cannot decode.
plantedDir := filepath.Join(storeDir, "metadata", strings.Repeat("a", 64))
require.NoError(t, os.MkdirAll(plantedDir, 0o750))
require.NoError(t, os.WriteFile(
filepath.Join(plantedDir, "manifest.json.zst"),
[]byte("not a valid manifest"), 0o600))
code, _, stderr := runEntry(t,
flagConfig, configPath, cmdSnapshot, cmdCreate, flagPrune)
assert.Equal(t, 1, code)
assert.Contains(t, stderr, "Backup completed, but post-backup prune failed")
assert.NotContains(t, stderr, "Snapshot creation failed")
snapshots, err := os.ReadDir(filepath.Join(storeDir, "metadata"))
require.NoError(t, err)
assert.Len(t, snapshots, 2,
"the new snapshot is on the store beside the planted one")
}
// writeBackupConfig builds a hermetic config with one small file in its
// source directory and an empty destination store directory. Returns the
// config path and the store directory.
func writeBackupConfig(t *testing.T) (string, string) {
t.Helper()
configPath := writeHermeticPruneConfig(t, false)
// writeHermeticPruneConfig puts the source and store directories
// beside the config file.
dir := filepath.Dir(configPath)
sourceDir := filepath.Join(dir, "source")
require.NoError(t, os.Mkdir(sourceDir, 0o750))
require.NoError(t, os.WriteFile(
filepath.Join(sourceDir, "file.txt"), []byte("hello"), 0o600))
return configPath, filepath.Join(dir, "store")
}
@@ -1,43 +0,0 @@
package cli //nolint:testpackage // shares runEntry and the command-word constants
import (
"testing"
"github.com/stretchr/testify/assert"
)
// TestEntryGroupRejectsUnknownSubcommand runs each group command with a
// mistyped subcommand, as a crontab line might carry one. It must fail
// with exit 1 and name the word, as `vaultik bogus` does at the top
// level, instead of printing the group's help and exiting 0 having done
// nothing. The group name alone still prints its help.
//
//nolint:paralleltest // replaces os.Args, os.Stdout and os.Stderr
func TestEntryGroupRejectsUnknownSubcommand(t *testing.T) {
for _, testCase := range []struct {
group string
mistyped string
}{
{group: cmdSnapshot, mistyped: "craete"},
{group: "config", mistyped: "inti"},
{group: cmdRemote, mistyped: "ifno"},
{group: "database", mistyped: "delte"},
{group: "completion", mistyped: "nosuchshell"},
} {
t.Run(testCase.group, func(t *testing.T) {
code, stdout, stderr := runEntry(t,
testCase.group, testCase.mistyped)
assert.Equal(t, 1, code)
assert.Empty(t, stdout)
assert.Contains(t, stderr,
`unknown command "`+testCase.mistyped+`"`)
code, stdout, _ = runEntry(t, testCase.group)
assert.Equal(t, 0, code)
assert.Contains(t, stdout,
programName+" "+testCase.group+" [command]")
})
}
}
-6
View File
@@ -18,12 +18,6 @@ func NewRemoteCommand() *cobra.Command {
Use: "remote", Use: "remote",
Short: "Remote storage management commands", Short: "Remote storage management commands",
Long: "Commands for inspecting and managing remote storage", Long: "Commands for inspecting and managing remote storage",
// cobra checks Args only on a command that has a Run: without
// one, a mistyped subcommand printed help and exited 0.
Args: cobra.NoArgs,
Run: func(cmd *cobra.Command, _ []string) {
_ = cmd.Help()
},
} }
// Add subcommands // Add subcommands
-15
View File
@@ -74,21 +74,6 @@ on the source system.`,
NewDatabaseCommand(), NewDatabaseCommand(),
) )
// cobra adds the completion command only when Execute runs, and with
// no Run, so it skipped its own cobra.NoArgs and a mistyped shell name
// printed help and exited 0. Adding it here lets it have a Run. Its
// scripts go to os.Stdout as it is now: a later SetOut does not
// redirect them.
cmd.InitDefaultCompletionCmd()
for _, sub := range cmd.Commands() {
if sub.Name() == "completion" {
sub.Run = func(completion *cobra.Command, _ []string) {
_ = completion.Help()
}
}
}
return cmd return cmd
} }
-18
View File
@@ -41,12 +41,6 @@ func NewSnapshotCommand() *cobra.Command {
Use: "snapshot", Use: "snapshot",
Short: "Snapshot management commands", Short: "Snapshot management commands",
Long: "Commands for creating, listing, and managing snapshots", Long: "Commands for creating, listing, and managing snapshots",
// cobra checks Args only on a command that has a Run: without
// one, a mistyped subcommand printed help and exited 0.
Args: cobra.NoArgs,
Run: func(cmd *cobra.Command, _ []string) {
_ = cmd.Help()
},
} }
// Add subcommands // Add subcommands
@@ -77,11 +71,6 @@ otherwise from the platform config directory (~/.config/vaultik/config.yml
on Linux), then /etc/vaultik/config.yml.`, on Linux), then /etc/vaultik/config.yml.`,
Args: cobra.ArbitraryArgs, Args: cobra.ArbitraryArgs,
RunE: func(cmd *cobra.Command, args []string) error { RunE: func(cmd *cobra.Command, args []string) error {
err := opts.Validate()
if err != nil {
return err
}
// Pass snapshot names from args // Pass snapshot names from args
opts.Snapshots = args opts.Snapshots = args
// --skip-errors is a global flag on the root command. // --skip-errors is a global flag on the root command.
@@ -108,13 +97,6 @@ on Linux), then /etc/vaultik/config.yml.`,
}, func(v *vaultik.Vaultik) error { }, func(v *vaultik.Vaultik) error {
return v.CreateSnapshot(opts) return v.CreateSnapshot(opts)
}, func(err error) { }, func(err error) {
if errors.Is(err, vaultik.ErrPostBackupPruneFailed) {
log.Error("Post-backup prune failed", "error", err)
ReportErrorf("Backup completed, but %v", err)
return
}
log.Error("Snapshot creation failed", "error", err) log.Error("Snapshot creation failed", "error", err)
ReportErrorf("Snapshot creation failed: %v", err) ReportErrorf("Snapshot creation failed: %v", err)
}) })
+2 -12
View File
@@ -61,9 +61,6 @@ var (
errBadS3PartSize = errors.New("s3.part_size must be between 5MiB and 5GiB") errBadS3PartSize = errors.New("s3.part_size must be between 5MiB and 5GiB")
errBadStorageScheme = errors.New( errBadStorageScheme = errors.New(
"storage_url must start with s3://, file://, or rclone://") "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( errStorageNotConfigured = errors.New(
"storage not configured; set storage_url or provide s3.endpoint + " + "storage not configured; set storage_url or provide s3.endpoint + " +
"s3.bucket + credentials") "s3.bucket + credentials")
@@ -464,18 +461,11 @@ func (c *Config) validateStorage() error {
// validateStorageURL validates URL-based storage configuration. File and // validateStorageURL validates URL-based storage configuration. File and
// rclone URLs need no credentials; S3 URLs require the legacy s3.* // rclone URLs need no credentials; S3 URLs require the legacy s3.*
// credential fields. A file URL must name an absolute path: a relative one // credential fields.
// would resolve against the working directory, so the destination would
// move with it.
func (c *Config) validateStorageURL() error { func (c *Config) validateStorageURL() error {
switch { switch {
case strings.HasPrefix(c.StorageURL, "file://"): case strings.HasPrefix(c.StorageURL, "file://"):
// storage.ParseStorageURL applies the same rule, but config // File storage doesn't need S3 credentials
// cannot import storage, which imports config.
if !strings.HasPrefix(c.StorageURL, "file:///") {
return errFileURLNotAbsolute
}
return nil return nil
case strings.HasPrefix(c.StorageURL, "rclone://"): case strings.HasPrefix(c.StorageURL, "rclone://"):
// Rclone storage uses rclone's own config // 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 // TestValidateAgeRecipients checks that recipients are parsed at config load
// (a bad entry fails immediately, not mid-backup) and that no invalid entry — // (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 // least of all a pasted secret key — is echoed in the error. An empty list
-47
View File
@@ -531,50 +531,3 @@ func TestExcludePatterns_UnanchoredPatternFile(t *testing.T) {
require.Equal(t, 1, result.FilesScanned, require.Equal(t, 1, result.FilesScanned,
"Unanchored pattern for file should match anywhere") "Unanchored pattern for file should match anywhere")
} }
func TestExcludePatterns_DoubleStarDirectoryAtEveryDepth(t *testing.T) {
t.Parallel()
// As in gitignore, a leading **/ also matches directly under the source dir
fs := setupAnchoredTestFS(t)
scanner, repos, cleanup := createTestScanner(t, fs, []string{"**/projectname"})
defer cleanup()
ctx := context.Background()
createSnapshotRecord(ctx, t, repos, "test-snapshot")
result, err := scanner.Scan(ctx, "/backup", "test-snapshot")
require.NoError(t, err)
// **/projectname should exclude BOTH:
// - /backup/projectname/file.txt
// - /backup/otherproject/projectname/file.txt
// Total files: 4, excluded: 2
require.Equal(t, 2, result.FilesScanned,
"**/ directory pattern should match at the top level and nested")
}
func TestExcludePatterns_DoubleStarFileAtEveryDepth(t *testing.T) {
t.Parallel()
// As in gitignore, a leading **/ also matches directly under the source dir
fs := setupAnchoredTestFS(t)
scanner, repos, cleanup := createTestScanner(t, fs, []string{"**/*.txt"})
defer cleanup()
ctx := context.Background()
createSnapshotRecord(ctx, t, repos, "test-snapshot")
result, err := scanner.Scan(ctx, "/backup", "test-snapshot")
require.NoError(t, err)
// **/*.txt should exclude ALL .txt files:
// - /backup/file.txt
// - /backup/projectname/file.txt
// - /backup/otherproject/projectname/file.txt
// Total files: 4, excluded: 3
require.Equal(t, 1, result.FilesScanned,
"**/ file pattern should match at the top level and nested")
}
-13
View File
@@ -1295,11 +1295,6 @@ func (s *Scanner) batchAddFilesToSnapshot(
func (s *Scanner) processPhase( func (s *Scanner) processPhase(
ctx context.Context, filesToProcess []*FileToProcess, result *ScanResult, ctx context.Context, filesToProcess []*FileToProcess, result *ScanResult,
) error { ) error {
// On an error or cancellation the blob being packed is never
// finalized, so its temp file must be removed here. After a
// successful flush there is no such blob and this does nothing.
defer s.packer.DiscardCurrentBlob()
// Calculate total bytes to process // Calculate total bytes to process
var totalBytes int64 var totalBytes int64
for _, f := range filesToProcess { for _, f := range filesToProcess {
@@ -2018,14 +2013,6 @@ func compileExcludePatterns(patterns []string) []compiledPattern {
// Remove trailing slash if present (directory indicator) // Remove trailing slash if present (directory indicator)
pattern = strings.TrimSuffix(pattern, "/") pattern = strings.TrimSuffix(pattern, "/")
// A leading "**/" demands a "/" before the name and so misses an
// entry directly under the source path. An unanchored pattern
// already matches at every depth, so drop the prefix: as in
// gitignore, "**/x" then also matches a top-level "x".
if !anchored {
pattern = strings.TrimPrefix(pattern, "**/")
}
// Compile the glob pattern // Compile the glob pattern
// For patterns without path separators, we need to match them as components // For patterns without path separators, we need to match them as components
// e.g., ".git" should match ".git" anywhere in the path // e.g., ".git" should match ".git" anywhere in the path
-57
View File
@@ -381,60 +381,3 @@ func TestScannerCancelStopsSkipErrorsRun(t *testing.T) {
}) })
} }
} }
// TestScannerRemovesBlobTempFileWhenRunStops checks that a run stopped
// part-way, by a file it cannot read or by a cancellation, removes the
// temp file of the blob it was packing.
func TestScannerRemovesBlobTempFileWhenRunStops(t *testing.T) {
t.Parallel()
// a.txt is backed up first and starts a blob; the run stops at b.txt.
const target = "/source/b.txt"
tests := []struct {
name string
newFs func(cancel context.CancelFunc) afero.Fs
}{
{
name: "file cannot be read",
newFs: func(context.CancelFunc) afero.Fs {
return &readFailFs{Fs: afero.NewMemMapFs(), target: target}
},
},
{
name: "cancelled",
newFs: func(cancel context.CancelFunc) afero.Fs {
return &cancelOnOpenFs{
Fs: afero.NewMemMapFs(), target: target, cancel: cancel,
}
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
fs := tt.newFs(cancel)
writeSkipErrorTestFile(t, fs, "/source/a.txt", "first file content")
writeSkipErrorTestFile(t, fs, target, "second file content")
_, err := runSkipErrorScan(ctx, t, fs, "/source", false, nil)
if err == nil {
t.Fatal("expected the scan to stop with an error, got nil")
}
left, err := afero.Glob(fs, filepath.Join(os.TempDir(), "vaultik-blob-*"))
if err != nil {
t.Fatalf("listing blob temp files: %v", err)
}
if len(left) != 0 {
t.Fatalf("expected no blob temp file left, got %v", left)
}
})
}
}
+23 -9
View File
@@ -1,15 +1,16 @@
// Package faultstore provides a storage.Storer wrapper that injects // Package faultstore provides a storage.Storer wrapper that injects
// faults on demand, so tests can reproduce the failure modes a real // faults on demand, so tests can reproduce the failure modes a real
// backend exhibits: an upload that fails partway, a backend that reports // backend exhibits: an upload that fails partway, a backend that reports
// success while storing nothing, and reads that return corrupt or // success while storing nothing, reads that return corrupt or truncated
// truncated bytes. It is the seam called for by the fault-injection // bytes, and a delete the backend refuses. It is the seam called for by
// tests (sneak/vaultik issue 72) and is meant to be reused by future // the fault-injection tests (sneak/vaultik issue 72) and is meant to be
// tests rather than re-implemented per case. // reused by future tests rather than re-implemented per case.
// //
// The wrapper delegates every method to the inner Storer. Two hooks // The wrapper delegates every method to the inner Storer. Three hooks
// change that: OnPut decides the fate of each write, and OnGet decides // change that: OnPut decides the fate of each write, OnGet decides how
// how each read's bytes are returned. Both are keyed by the object key, // each read's bytes are returned, and OnDelete decides whether each
// so a test can fault only blobs, only metadata, or a single object. // delete fails. All are keyed by the object key, so a test can fault
// only blobs, only metadata, or a single object.
package faultstore package faultstore
import ( import (
@@ -25,6 +26,10 @@ import (
// ErrInjectedUpload is returned by a Put the OnPut hook chose to fail. // ErrInjectedUpload is returned by a Put the OnPut hook chose to fail.
var ErrInjectedUpload = errors.New("faultstore: injected upload failure") var ErrInjectedUpload = errors.New("faultstore: injected upload failure")
// ErrInjectedDelete is returned by a Delete the OnDelete hook chose to
// fail.
var ErrInjectedDelete = errors.New("faultstore: injected delete failure")
// PutAction is the disposition OnPut assigns to a write. // PutAction is the disposition OnPut assigns to a write.
type PutAction int type PutAction int
@@ -66,6 +71,11 @@ type Storer struct {
// OnGet, when set, is consulted for every Get with the object key // OnGet, when set, is consulted for every Get with the object key
// and damages the returned bytes accordingly. // and damages the returned bytes accordingly.
OnGet func(key string) GetFault OnGet func(key string) GetFault
// OnDelete, when set, is consulted before every Delete with the
// object key. Returning true fails the delete and leaves the object
// in place.
OnDelete func(key string) bool
} }
// New wraps inner. inner must be non-nil. // New wraps inner. inner must be non-nil.
@@ -154,8 +164,12 @@ func (f *Storer) Stat(ctx context.Context, key string) (*storage.ObjectInfo, err
return f.inner.Stat(ctx, key) return f.inner.Stat(ctx, key)
} }
// Delete delegates unchanged. // Delete removes the object unless OnDelete fails the delete.
func (f *Storer) Delete(ctx context.Context, key string) error { func (f *Storer) Delete(ctx context.Context, key string) error {
if f.OnDelete != nil && f.OnDelete(key) {
return fmt.Errorf("%w for %q", ErrInjectedDelete, key)
}
return f.inner.Delete(ctx, key) return f.inner.Delete(ctx, key)
} }
+12 -26
View File
@@ -17,10 +17,8 @@ const (
// Sentinel errors for storage URL parsing. // Sentinel errors for storage URL parsing.
var ( var (
ErrEmptyStorageURL = errors.New("storage URL is empty") ErrEmptyStorageURL = errors.New("storage URL is empty")
ErrEmptyFilePath = errors.New("file URL path 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")
ErrMissingBucket = errors.New("s3 URL missing bucket name") ErrMissingBucket = errors.New("s3 URL missing bucket name")
ErrMissingRemote = errors.New("rclone URL missing remote name") ErrMissingRemote = errors.New("rclone URL missing remote name")
ErrUnsupportedScheme = errors.New( ErrUnsupportedScheme = errors.New(
@@ -53,8 +51,17 @@ func ParseStorageURL(rawURL string) (*URL, error) {
return nil, ErrEmptyStorageURL return nil, ErrEmptyStorageURL
} }
// Handle file:// URLs
if after, ok := strings.CutPrefix(rawURL, "file://"); ok { 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://") { if strings.HasPrefix(rawURL, "s3://") {
@@ -68,27 +75,6 @@ func ParseStorageURL(rawURL string) (*URL, error) {
return nil, ErrUnsupportedScheme 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 // parseS3URL parses an s3://bucket/prefix URL. It rejects credentials in
// the userinfo and any query parameter other than endpoint, region and // the userinfo and any query parameter other than endpoint, region and
// ssl, so a credential-bearing URL is never stored or echoed. // 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) { func TestParseStorageURLValid(t *testing.T) {
t.Parallel() t.Parallel()
const ( const bucket = "mybucket"
bucket = "mybucket"
fileScheme = "file"
)
cases := []struct { cases := []struct {
name string name string
@@ -27,17 +24,7 @@ func TestParseStorageURLValid(t *testing.T) {
{ {
name: "file absolute path", name: "file absolute path",
raw: "file:///var/backups/vaultik", raw: "file:///var/backups/vaultik",
want: &storage.URL{Scheme: fileScheme, Prefix: "/var/backups/vaultik"}, want: &storage.URL{Scheme: "file", 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"},
}, },
{ {
name: "s3 bucket and prefix, ssl defaults on", name: "s3 bucket and prefix, ssl defaults on",
@@ -103,7 +90,6 @@ func TestParseStorageURLErrors(t *testing.T) {
}{ }{
{"empty url", "", storage.ErrEmptyStorageURL}, {"empty url", "", storage.ErrEmptyStorageURL},
{"file empty path", "file://", storage.ErrEmptyFilePath}, {"file empty path", "file://", storage.ErrEmptyFilePath},
{"file relative path", "file://dest", storage.ErrRelativeFilePath},
{"s3 missing bucket", "s3://", storage.ErrMissingBucket}, {"s3 missing bucket", "s3://", storage.ErrMissingBucket},
{"s3 missing bucket with path", "s3:///justprefix", storage.ErrMissingBucket}, {"s3 missing bucket with path", "s3:///justprefix", storage.ErrMissingBucket},
{"rclone missing remote", "rclone://", storage.ErrMissingRemote}, {"rclone missing remote", "rclone://", storage.ErrMissingRemote},
+19 -8
View File
@@ -167,6 +167,10 @@ const (
// metadataKeyParts is the minimum "/"-separated segment count of a // metadataKeyParts is the minimum "/"-separated segment count of a
// metadata object key (metadata/<snapshot-id>/<filename>). // metadata object key (metadata/<snapshot-id>/<filename>).
metadataKeyParts = 3 metadataKeyParts = 3
// blobKeyParts is the minimum "/"-separated segment count of a blob
// object key (blobs/<aa>/<bb>/<hash>).
blobKeyParts = 4
) )
// SnapshotMetadataInfo contains information about a single snapshot's metadata // SnapshotMetadataInfo contains information about a single snapshot's metadata
@@ -450,16 +454,23 @@ func (v *Vaultik) scanRemoteBlobStorage(
v.stdoutf("Scanning blobs...\n") v.stdoutf("Scanning blobs...\n")
} }
// Use prune's listing, so a file under blobs/ that prune would never blobCh := v.Storage.ListStream(v.ctx, "blobs/")
// delete is not counted as a blob, and so not as an orphaned one. allBlobs := make(map[string]int64)
allBlobs, err := v.listAllRemoteBlobs()
if err != nil {
return err
}
for _, size := range allBlobs { for obj := range blobCh {
if obj.Err != nil {
return fmt.Errorf("listing blobs: %w", obj.Err)
}
parts := strings.Split(obj.Key, "/")
if len(parts) < blobKeyParts {
continue
}
hash := parts[3]
allBlobs[hash] = obj.Size
result.TotalBlobCount++ result.TotalBlobCount++
result.TotalBlobSize += size result.TotalBlobSize += obj.Size
} }
// A blob named only by a manifest that could not be read, or by one // A blob named only by a manifest that could not be read, or by one
+81 -1
View File
@@ -5,12 +5,14 @@ import (
"io/fs" "io/fs"
"os" "os"
"path/filepath" "path/filepath"
"strings"
"testing" "testing"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
"sneak.berlin/go/vaultik/internal/log" "sneak.berlin/go/vaultik/internal/log"
"sneak.berlin/go/vaultik/internal/snapshot" "sneak.berlin/go/vaultik/internal/snapshot"
"sneak.berlin/go/vaultik/internal/storage/faultstore"
) )
// TestNukeRemoteLeavesNoFiles checks that remote nuke leaves no file // TestNukeRemoteLeavesNoFiles checks that remote nuke leaves no file
@@ -23,7 +25,7 @@ func TestNukeRemoteLeavesNoFiles(t *testing.T) {
ctx := context.Background() ctx := context.Background()
storeDir := filepath.Join(t.TempDir(), "store") storeDir := filepath.Join(t.TempDir(), "store")
v, repos, _ := backUpToFileDestination(ctx, t, storeDir) v, repos, out := backUpToFileDestination(ctx, t, storeDir)
snapshots, err := repos.Snapshots.ListRecent(ctx, listRecentTestLimit) snapshots, err := repos.Snapshots.ListRecent(ctx, listRecentTestLimit)
require.NoError(t, err) require.NoError(t, err)
@@ -61,4 +63,82 @@ func TestNukeRemoteLeavesNoFiles(t *testing.T) {
}) })
require.NoError(t, err) require.NoError(t, err)
assert.Empty(t, files) assert.Empty(t, files)
// Nuke deletes the blobs itself, so advice to run prune is wrong.
assert.Contains(t, out.String(), "Backup destination store is now empty.")
assert.NotContains(t, out.String(), "vaultik prune")
}
// TestNukeRemoteFailsWhenADeleteFails checks that remote nuke returns an
// error when a snapshot's metadata or a blob cannot be deleted, does not
// report the store empty, and advises running nuke again: prune deletes
// no metadata.
func TestNukeRemoteFailsWhenADeleteFails(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
for _, refusedPrefix := range []string{"metadata/", "blobs/"} {
t.Run(refusedPrefix, func(t *testing.T) {
t.Parallel()
ctx := context.Background()
storeDir := filepath.Join(t.TempDir(), "store")
v, _, out := backUpToFileDestination(ctx, t, storeDir)
store := faultstore.New(v.Storage)
store.OnDelete = func(key string) bool {
return strings.HasPrefix(key, refusedPrefix)
}
v.Storage = store
require.Error(t, v.NukeRemote(true))
assert.NotContains(t, out.String(), "now empty")
assert.Contains(t, out.String(), "vaultik remote nuke --force")
assert.NotContains(t, out.String(), "vaultik prune")
})
}
}
// TestNukeRemoteRefusesAnotherDestinationWithoutRetryAdvice checks that
// remote nuke, when the local index belongs to a different destination
// than the configured one, fails without advising to run nuke again:
// that run would fail the same way.
func TestNukeRemoteRefusesAnotherDestinationWithoutRetryAdvice(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
ctx := context.Background()
storeDir := filepath.Join(t.TempDir(), "store")
v, _, out := backUpToFileDestination(ctx, t, storeDir)
v.Config.StorageURL = "file://" + filepath.Join(t.TempDir(), "elsewhere")
err := v.NukeRemote(true)
require.ErrorContains(t, err, "bound to a different backup destination")
assert.NotContains(t, out.String(), "vaultik remote nuke --force")
}
// TestNukeRemoteFailsWhenALocalSnapshotCannotBeRemoved checks that a
// snapshot remote nuke cannot remove from the local index fails the
// run: nuke then leaves that snapshot's metadata on the destination
// store.
func TestNukeRemoteFailsWhenALocalSnapshotCannotBeRemoved(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
ctx := context.Background()
storeDir := filepath.Join(t.TempDir(), "store")
v, _, out := backUpToFileDestination(ctx, t, storeDir)
_, err := v.DB.Conn().ExecContext(ctx, `
CREATE TRIGGER refuse_snapshot_delete BEFORE DELETE ON snapshots
BEGIN SELECT RAISE(ABORT, 'simulated index error'); END`)
require.NoError(t, err)
require.Error(t, v.NukeRemote(true))
assert.NotContains(t, out.String(), "now empty")
assert.Contains(t, out.String(), "vaultik remote nuke --force")
} }
+39 -11
View File
@@ -19,14 +19,18 @@ type PruneOptions struct {
var errNukeRequiresForce = errors.New( var errNukeRequiresForce = errors.New(
"nuke requires --force (this deletes ALL remote snapshots and blobs)") "nuke requires --force (this deletes ALL remote snapshots and blobs)")
// errBlobDeleteFailed reports unreferenced blobs prune could not delete.
var errBlobDeleteFailed = errors.New("blob(s) could not be deleted")
// nukeCommandHint is the command suggested when a remote nuke could not
// delete everything: running it again deletes what is left, which
// `vaultik prune` does not do for snapshot metadata.
const nukeCommandHint = "vaultik remote nuke --force"
// metadataDirName is the top-level remote directory holding snapshot // metadataDirName is the top-level remote directory holding snapshot
// metadata. // metadata.
const metadataDirName = "metadata" const metadataDirName = "metadata"
// blobKeyParts is the "/"-separated segment count of a blob object key
// (blobs/<aa>/<bb>/<hash>).
const blobKeyParts = 4
// NukeRemote deletes every snapshot's metadata and every blob from remote // NukeRemote deletes every snapshot's metadata and every blob from remote
// storage, along with any object an upload cut off part-way left under a // storage, along with any object an upload cut off part-way left under a
// temporary `.partial` name. After this returns successfully the bucket // temporary `.partial` name. After this returns successfully the bucket
@@ -39,6 +43,29 @@ func (v *Vaultik) NukeRemote(force bool) error {
return errNukeRequiresForce return errNukeRequiresForce
} }
// Checked before anything is deleted: running nuke again would fail
// the same way, so the advice below must not follow this error.
err := v.EnsureStorageBinding()
if err != nil {
return err
}
err = v.emptyDestinationStore()
if err != nil {
v.UI.Warningf("Run '%s' again to delete what is left on the "+
"backup destination store.", nukeCommandHint)
return err
}
v.UI.Completef("Backup destination store is now empty.")
return nil
}
// emptyDestinationStore does the deleting for NukeRemote and stops at
// the first step that fails.
func (v *Vaultik) emptyDestinationStore() error {
v.UI.Beginf("Removing all snapshot metadata from backup destination store.") v.UI.Beginf("Removing all snapshot metadata from backup destination store.")
_, err := v.RemoveAllSnapshots(&RemoveOptions{Force: true}) _, err := v.RemoveAllSnapshots(&RemoveOptions{Force: true})
@@ -62,8 +89,6 @@ func (v *Vaultik) NukeRemote(force bool) error {
} }
} }
v.UI.Completef("Backup destination store is now empty.")
return nil return nil
} }
@@ -166,14 +191,17 @@ func (v *Vaultik) PruneBlobs(opts *PruneOptions) error {
v.deleteUnreferencedBlobs(unreferencedBlobs, allBlobs, result) v.deleteUnreferencedBlobs(unreferencedBlobs, allBlobs, result)
if opts.JSON { if opts.JSON {
return v.outputPruneBlobsJSON(result) err = v.outputPruneBlobsJSON(result)
if err != nil {
return err
}
} else {
v.stdoutf("\nDeleted %d blob(s) totaling %s\n",
result.BlobsDeleted, ubytes(result.BytesFreed))
} }
v.stdoutf("\nDeleted %d blob(s) totaling %s\n",
result.BlobsDeleted, ubytes(result.BytesFreed))
if result.BlobsFailed > 0 { if result.BlobsFailed > 0 {
v.stdoutf("Failed to delete %d blob(s)\n", result.BlobsFailed) return fmt.Errorf("%d %w", result.BlobsFailed, errBlobDeleteFailed)
} }
return nil return nil
@@ -0,0 +1,98 @@
package vaultik_test
import (
"context"
"encoding/json"
"os"
"path/filepath"
"strings"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"sneak.berlin/go/vaultik/internal/log"
"sneak.berlin/go/vaultik/internal/storage/faultstore"
"sneak.berlin/go/vaultik/internal/vaultik"
)
// refuseBlobDeletes is an OnDelete hook that fails every blob delete.
func refuseBlobDeletes(key string) bool {
return strings.HasPrefix(key, "blobs/")
}
// TestPruneBlobs_FailedDeleteReturnsError checks that prune returns an
// error, so the command exits non-zero, when an unreferenced blob cannot
// be deleted.
func TestPruneBlobs_FailedDeleteReturnsError(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
env := newListEnv(t)
// The store holds no manifest, so nothing references this blob.
addBlob(t, env.store.testStorer, testBlobHashA)
store := faultstore.New(env.store)
store.OnDelete = refuseBlobDeletes
env.v.Storage = store
err := env.v.PruneBlobs(&vaultik.PruneOptions{Force: true})
require.ErrorContains(t, err, "1 blob(s) could not be deleted")
}
// TestPruneBlobs_JSONFailedDeleteReturnsError checks that prune --json
// prints its document and then returns an error when an unreferenced
// blob cannot be deleted.
func TestPruneBlobs_JSONFailedDeleteReturnsError(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
env := newListEnv(t)
addBlob(t, env.store.testStorer, testBlobHashA)
store := faultstore.New(env.store)
store.OnDelete = refuseBlobDeletes
env.v.Storage = store
err := env.v.PruneBlobs(&vaultik.PruneOptions{JSON: true})
require.Error(t, err, "prune must fail when a blob cannot be deleted")
var result vaultik.PruneBlobsResult
require.NoError(t, json.Unmarshal(env.stdout.Bytes(), &result),
"stdout must hold the JSON document, got:\n%s", env.stdout.String())
assert.Equal(t, 1, result.BlobsFailed)
assert.Equal(t, 0, result.BlobsDeleted)
}
// TestCreateSnapshotPrune_FailedBlobDeleteReturnsError checks that the
// prune step of snapshot create --prune fails the run when an
// unreferenced blob cannot be deleted.
func TestCreateSnapshotPrune_FailedBlobDeleteReturnsError(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
ctx := context.Background()
storeDir := filepath.Join(t.TempDir(), "store")
v, _, _ := backUpToFileDestination(ctx, t, storeDir)
// No snapshot references this blob, so the prune step deletes it.
orphan := filepath.Join(storeDir, "blobs", testBlobHashA[:2],
testBlobHashA[2:4], testBlobHashA)
require.NoError(t, os.MkdirAll(filepath.Dir(orphan), 0o750))
require.NoError(t, os.WriteFile(orphan, []byte("orphan"), 0o600))
store := faultstore.New(v.Storage)
store.OnDelete = refuseBlobDeletes
v.Storage = store
err := v.CreateSnapshot(&vaultik.SnapshotCreateOptions{
Cron: true,
Prune: true,
})
require.ErrorContains(t, err, "1 blob(s) could not be deleted")
assert.FileExists(t, orphan)
}
-33
View File
@@ -185,39 +185,6 @@ func TestRemoteInfo_SkipsNonConformingMetadataName(t *testing.T) {
assert.NotContains(t, doc, "unreadable_manifests") assert.NotContains(t, doc, "unreadable_manifests")
} }
// TestRemoteInfo_DoesNotCountNonBlobFileUnderBlobs checks that a file
// under blobs/ whose name is not a blob hash is not counted as a blob.
// prune never deletes such a file, so counting it as an orphaned blob
// would advise a prune that removes nothing.
func TestRemoteInfo_DoesNotCountNonBlobFileUnderBlobs(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
env := newListEnv(t)
env.addRemote(t, listRemoteID, time.Date(2026, 3, 2, 0, 0, 0, 0, time.UTC))
addBlob(t, env.store.testStorer, testBlobHashA)
require.NoError(t, env.store.Put(context.Background(),
"blobs/"+testBlobHashA[:2]+"/"+testBlobHashA[2:4]+"/.DS_Store",
bytes.NewReader([]byte("junk"))))
require.NoError(t, env.v.RemoteInfo(false))
text := env.stdout.String()
assert.Contains(t, text, "Total blobs on remote: 1 (")
assert.Contains(t, text, "Orphaned (unreferenced): 0 (")
assert.NotContains(t, text, "vaultik prune")
env.stdout.Reset()
require.NoError(t, env.v.RemoteInfo(true))
var doc map[string]any
require.NoError(t, json.Unmarshal(env.stdout.Bytes(), &doc))
assert.InDelta(t, 1, doc["total_blob_count"], 0)
assert.InDelta(t, 0, doc["orphaned_blob_count"], 0)
assert.InDelta(t, 0, doc["orphaned_blob_size"], 0)
}
// TestRemoteInfo_DirectoryWithoutManifestLeavesOrphansKnown checks that // TestRemoteInfo_DirectoryWithoutManifestLeavesOrphansKnown checks that
// a directory under metadata/ holding no manifest.json.zst, such as one // a directory under metadata/ holding no manifest.json.zst, such as one
// left by a backup interrupted before its manifest upload, leaves the // left by a backup interrupted before its manifest upload, leaves the
+2 -1
View File
@@ -476,7 +476,8 @@ func TestRemoveAllSnapshots_WithForce(t *testing.T) {
out := tv.Stdout.String() out := tv.Stdout.String()
assert.Contains(t, out, "Removed 2 snapshot(s)") assert.Contains(t, out, "Removed 2 snapshot(s)")
assert.Contains(t, out, "Removed snapshot metadata from remote storage") assert.Contains(t, out, "Removed snapshot metadata from remote storage")
assert.Contains(t, out, "vaultik prune") // Remote nuke, the only caller, deletes the blobs next itself.
assert.NotContains(t, out, "vaultik prune")
} }
func TestRemoveAllSnapshots_DryRun(t *testing.T) { func TestRemoveAllSnapshots_DryRun(t *testing.T) {
+19 -47
View File
@@ -24,18 +24,13 @@ var (
errSnapshotVerifyFailed = errors.New("verification failed") errSnapshotVerifyFailed = errors.New("verification failed")
errRemoveAllNeedsForce = errors.New("--all requires --force") errRemoveAllNeedsForce = errors.New("--all requires --force")
errSnapshotNotFound = errors.New("snapshot not found") errSnapshotNotFound = errors.New("snapshot not found")
errSnapshotRemoveFailed = errors.New("snapshot(s) could not be removed")
errInvalidTableName = errors.New("invalid table name") errInvalidTableName = errors.New("invalid table name")
errNoAgeRecipients = errors.New( errNoAgeRecipients = errors.New(
"creating a snapshot needs at least one public key in " + "creating a snapshot needs at least one public key in " +
"age_recipients (generate a keypair with: age-keygen)") "age_recipients (generate a keypair with: age-keygen)")
errKeepNewerThanNeedsPrune = errors.New("--keep-newer-than requires --prune")
) )
// ErrPostBackupPruneFailed wraps a failure of the post-backup prune,
// which runs only after every snapshot was created and uploaded, so the
// caller can report it apart from a failed backup.
var ErrPostBackupPruneFailed = errors.New("post-backup prune failed")
// listRecentLimit caps how many snapshot rows are fetched from the // listRecentLimit caps how many snapshot rows are fetched from the
// local index for listing and purge operations. // local index for listing and purge operations.
const listRecentLimit = 10000 const listRecentLimit = 10000
@@ -51,26 +46,6 @@ type SnapshotCreateOptions struct {
Snapshots []string // Optional list of snapshot names to process (empty = all) Snapshots []string // Optional list of snapshot names to process (empty = all)
} }
// Validate fails when KeepNewerThan is set without Prune or is not a
// valid duration. Only the post-backup prune reads KeepNewerThan, after
// the backup, so the caller checks it before starting one.
func (opts *SnapshotCreateOptions) Validate() error {
if opts.KeepNewerThan == "" {
return nil
}
if !opts.Prune {
return errKeepNewerThanNeedsPrune
}
_, err := parseDuration(opts.KeepNewerThan)
if err != nil {
return fmt.Errorf("--keep-newer-than: %w", err)
}
return nil
}
// CreateSnapshot executes the snapshot creation operation // CreateSnapshot executes the snapshot creation operation
func (v *Vaultik) CreateSnapshot(opts *SnapshotCreateOptions) error { func (v *Vaultik) CreateSnapshot(opts *SnapshotCreateOptions) error {
// config.Load accepts an empty list, since listing, verifying and // config.Load accepts an empty list, since listing, verifying and
@@ -149,7 +124,7 @@ func (v *Vaultik) CreateSnapshot(opts *SnapshotCreateOptions) error {
if opts.Prune { if opts.Prune {
err = v.runPostBackupPrune(snapshotNames, opts.KeepNewerThan) err = v.runPostBackupPrune(snapshotNames, opts.KeepNewerThan)
if err != nil { if err != nil {
return fmt.Errorf("%w: %w", ErrPostBackupPruneFailed, err) return fmt.Errorf("post-backup prune: %w", err)
} }
} }
@@ -1539,8 +1514,8 @@ func (v *Vaultik) handleRemoveAllDryRun(
// executeRemoveAll deletes every local snapshot and, unless LocalOnly // executeRemoveAll deletes every local snapshot and, unless LocalOnly
// is set, every corresponding remote metadata directory plus any // is set, every corresponding remote metadata directory plus any
// orphan remote keys. Blobs are not touched; the printed prune-command // orphan remote keys. Blobs are not touched. It returns an error when
// hint is the next step. // any snapshot could not be removed.
func (v *Vaultik) executeRemoveAll( func (v *Vaultik) executeRemoveAll(
localSnaps, orphanRemoteKeys []string, opts *RemoveOptions, localSnaps, orphanRemoteKeys []string, opts *RemoveOptions,
) (*RemoveResult, error) { ) (*RemoveResult, error) {
@@ -1554,18 +1529,11 @@ func (v *Vaultik) executeRemoveAll(
result := &RemoveResult{} result := &RemoveResult{}
remoteErrors := v.removeAllLocalSnapshots(localSnaps, opts, result) failures := v.removeAllLocalSnapshots(localSnaps, opts, result)
if !opts.LocalOnly { if !opts.LocalOnly {
remoteErrors += v.removeAllOrphanRemotes(orphanRemoteKeys, result) failures += v.removeAllOrphanRemotes(orphanRemoteKeys, result)
result.RemoteRemoved = failures == 0
if remoteErrors == 0 {
result.RemoteRemoved = true
} else if v.UI != nil {
v.UI.Warningf("Some remote metadata deletions failed. Run '%s' "+
"once the remote is healthy to clean up unreferenced blobs.",
pruneCommandHint)
}
} }
if v.SnapshotManager != nil { if v.SnapshotManager != nil {
@@ -1575,6 +1543,10 @@ func (v *Vaultik) executeRemoveAll(
} }
} }
if failures > 0 {
return result, fmt.Errorf("%d %w", failures, errSnapshotRemoveFailed)
}
if opts.JSON { if opts.JSON {
return result, v.outputRemoveJSON(result) return result, v.outputRemoveJSON(result)
} }
@@ -1583,9 +1555,6 @@ func (v *Vaultik) executeRemoveAll(
if !opts.LocalOnly && result.RemoteRemoved { if !opts.LocalOnly && result.RemoteRemoved {
v.printlnStdout("Removed snapshot metadata from remote storage") v.printlnStdout("Removed snapshot metadata from remote storage")
v.stdoutf("\nNote: Removed snapshots' blobs remain on the remote. "+
"Run '%s' to delete any blobs no longer referenced by any "+
"remaining remote snapshot.\n", pruneCommandHint)
} }
return result, nil return result, nil
@@ -1628,12 +1597,13 @@ func (v *Vaultik) deleteIncompleteSnapshot(snapshotIDStr string, id any) bool {
} }
// removeAllLocalSnapshots removes each known local snapshot (and, unless // removeAllLocalSnapshots removes each known local snapshot (and, unless
// --local-only, its remote metadata), returning the number of remote // --local-only, its remote metadata), returning the number of snapshots
// deletion failures. // it could not remove. A snapshot left in the local index keeps its
// remote metadata too, since it is not an orphan remote key.
func (v *Vaultik) removeAllLocalSnapshots( func (v *Vaultik) removeAllLocalSnapshots(
localSnaps []string, opts *RemoveOptions, result *RemoveResult, localSnaps []string, opts *RemoveOptions, result *RemoveResult,
) int { ) int {
remoteErrors := 0 failures := 0
for _, snapshotID := range localSnaps { for _, snapshotID := range localSnaps {
log.Info("Removing snapshot", "snapshot_id", snapshotID) log.Info("Removing snapshot", "snapshot_id", snapshotID)
@@ -1643,6 +1613,8 @@ func (v *Vaultik) removeAllLocalSnapshots(
log.Error("Failed to remove from local database", log.Error("Failed to remove from local database",
"snapshot_id", snapshotID, "error", err) "snapshot_id", snapshotID, "error", err)
failures++
continue continue
} }
@@ -1653,14 +1625,14 @@ func (v *Vaultik) removeAllLocalSnapshots(
log.Warn("Failed to remove snapshot metadata from remote", log.Warn("Failed to remove snapshot metadata from remote",
"snapshot_id", snapshotID, "error", err) "snapshot_id", snapshotID, "error", err)
remoteErrors++ failures++
} }
} }
result.SnapshotsRemoved = append(result.SnapshotsRemoved, snapshotID) result.SnapshotsRemoved = append(result.SnapshotsRemoved, snapshotID)
} }
return remoteErrors return failures
} }
// removeAllOrphanRemotes deletes remote snapshot metadata that has no // removeAllOrphanRemotes deletes remote snapshot metadata that has no