diff --git a/README.md b/README.md index 425d569..70f5eb4 100644 --- a/README.md +++ b/README.md @@ -200,8 +200,10 @@ local index or the destination store. `config`, `database delete`, ### stdout and stderr Log output — everything from `--verbose` and `--debug`, and every -warning and error the logger emits — goes to **stderr**. stdout carries -the output you asked for: tables, and the documents produced by `--json`. +warning and error the logger emits — goes to **stderr**, and so does the +startup banner. stdout carries the output you asked for: tables, the +documents produced by `--json`, `config get` values, and completion +scripts. This means `vaultik snapshot list --verbose > out.txt` captures the listing and leaves the diagnostics on your terminal. To capture both, @@ -645,8 +647,8 @@ Work planned after 1.0. Loosely ordered by priority. ## output style Every command's user-facing output is governed by `internal/ui`, in one -of two ways. Color is enabled when stdout is a TTY and the `NO_COLOR` -environment variable is unset (https://no-color.org/). +of two ways. Color is enabled when the stream written to is a TTY and +the `NO_COLOR` environment variable is unset (https://no-color.org/). * **Status, progress, warnings, and errors** go through the `internal/ui` message methods below: marker-prefixed, colored on a TTY, and — except @@ -656,24 +658,26 @@ environment variable is unset (https://no-color.org/). `config init`, `config set`, and `database delete`. * **The data a command exists to produce** is written plain, with no marker and no color, because a marker would corrupt a table or a - parsed document. This covers the `version`, `info`, and `remote info` - reports, the `snapshot list` table, `config get` values, and every - `--json` document. `--quiet` silences the human reports and tables - (`version`, `info`, `remote info`, `snapshot list`) but never the - machine-consumed `config get` value or the `--json` documents, which a - script depends on. The `database delete` confirmation prompt is also - written this way and always shown: it is an interactive exchange the - operator must see. + parsed document. This covers the `version`, `info`, `remote info` and + `snapshot verify` reports, the `snapshot list` table, `config get` + values, and every `--json` document. `--quiet` silences the human + reports and tables (`version`, `info`, `remote info`, `snapshot + verify`, `snapshot list`) but never the machine-consumed `config get` + value or the `--json` documents, which a script depends on. The + `database delete` confirmation prompt is also written this way and + always shown: it is an interactive exchange the operator must see. -`internal/ui` writes to stdout; it is the output the user asked for. -Structured log records are a different thing and go through -`internal/log`, which writes to stderr (see "stdout and stderr" above). +`internal/ui` writes to stdout; it is the output the user asked for. The +exceptions are the startup banner and the error a failed command ends +with, which go to stderr. Structured log records are a different thing +and go through `internal/log`, which writes to stderr (see "stdout and +stderr" above). Message classes: | Class | Marker | Alignment | Use for | |-------|--------|-----------|---------| -| Banner | none | column 0 | The startup line printed once per invocation | +| Banner | none | column 0 | The startup line printed once per invocation, on stderr | | Begin | `》` (white) | column 0 | An operation is about to start (present-continuous verb) | | Complete | `》` (green) | column 0 | An operation just finished (past-tense verb) | | Info | `》` (white) | column 0 | Neutral status update | diff --git a/TODO.md b/TODO.md index b5307d3..16c4f22 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-06: Made command output follow the README's stdout and stderr + rules ([issue #224](https://git.eeqj.de/sneak/vaultik/issues/224)). The + startup banner went to stdout, so a `completion` script or a + `config get` value started with it; the banner now goes to stderr. A + failing `remote info`, `prune` or `snapshot remove` under `--json` + printed nothing on either stream, and now reports its error on stderr. + `snapshot verify --quiet` printed its whole report; it now prints + none, and a failure still reaches stderr with the same exit status. + - 2026-10-06: Made a restore path argument select only that path and what is beneath it ([issue #223](https://git.eeqj.de/sneak/vaultik/issues/223)). The diff --git a/internal/cli/app.go b/internal/cli/app.go index dc305b3..76ba625 100644 --- a/internal/cli/app.go +++ b/internal/cli/app.go @@ -200,10 +200,10 @@ func RunApp(ctx context.Context, app *fx.App) error { } // errReported marks a failure the operation has already shown the user -// (and deliberately withheld under --json). Entry turns it into a -// non-zero exit status without printing anything further, so the error -// line is not doubled. It flows up from RunOperation through cobra to -// Entry. +// (or, under `snapshot verify --json`, put in its document). Entry +// turns it into a non-zero exit status without printing anything +// further, so the error line is not doubled. It flows up from +// RunOperation through cobra to Entry. var errReported = errors.New("operation failed") // RunOperation runs op against the Vaultik instance inside the fx app @@ -220,10 +220,10 @@ var errReported = errors.New("operation failed") // interrupt OnStop cancels op and waits for the goroutine to return, so // op's cleanup (removing decrypted scratch files) runs before the // process exits; the wait is bounded by shutdownTimeout. report is -// called with a non-canceled failure so the caller can log it (and -// suppress it under --json) before it becomes errReported. A context -// cancellation is the interrupt path, not a failure: it is neither -// reported nor counted as one. +// called with a non-canceled failure so the caller can show it to the +// user before it becomes errReported. A context cancellation is the +// interrupt path, not a failure: it is neither reported nor counted as +// one. func RunOperation( ctx context.Context, opts AppOptions, op func(v *vaultik.Vaultik) error, report func(err error), @@ -293,13 +293,12 @@ func RunOperation( // runVaultikApp runs the standard single-operation command lifecycle // shared by the snapshot list/purge/remove and remote nuke subcommands: // resolve the config, then run op against the Vaultik instance through -// RunOperation, reporting a failure prefixed with failMsg (suppressed -// while suppressErrors is true, e.g. under --json). mode says whether the -// command takes the PID lock. jsonOutput marks a command whose stdout is a -// JSON document: it quiets the UI but, unlike Quiet, leaves the stderr log -// level alone. +// RunOperation, reporting a failure prefixed with failMsg on stderr. mode +// says whether the command takes the PID lock. jsonOutput marks a command +// whose stdout is a JSON document: it quiets the UI but, unlike Quiet, +// leaves the stderr log level alone. func runVaultikApp( - cmd *cobra.Command, mode lockMode, jsonOutput, suppressErrors bool, + cmd *cobra.Command, mode lockMode, jsonOutput bool, failMsg string, op func(v *vaultik.Vaultik) error, ) error { configPath, err := ResolveConfigPath() @@ -319,10 +318,6 @@ func runVaultikApp( }, Mode: mode, }, op, func(err error) { - if suppressErrors { - return - } - log.Error(failMsg, "error", err) ReportErrorf("%s: %v", failMsg, err) }) diff --git a/internal/cli/entry.go b/internal/cli/entry.go index 917e6e7..5736c40 100644 --- a/internal/cli/entry.go +++ b/internal/cli/entry.go @@ -16,16 +16,18 @@ import ( const shortCommitLen = 12 // Entry is the main entry point for the CLI application. -// It prints the startup banner to stdout (unless a banner-suppressing +// It prints the startup banner to stderr (unless a banner-suppressing // flag is present in os.Args — see bannerSuppressedInArgs), executes the // root cobra command, and routes any returned error through the // ui.Writer so the user sees a properly formatted "🛑 ERROR:" line. +// The banner goes to stderr because stdout carries only the output the +// user asked for, such as a completion script or a `config get` value. // // It returns the process exit code (0 on success, 1 on error) rather // than calling os.Exit, so that main's deferred profile writers run // before the process ends. See run in cmd/vaultik/main.go. func Entry() int { - emitStartupBanner(os.Args[1:], os.Stdout) + emitStartupBanner(os.Args[1:], os.Stderr) rootCmd := NewRootCommand() rootCmd.SilenceErrors = true @@ -33,8 +35,9 @@ func Entry() int { err := rootCmd.Execute() if err != nil { // An operation that ran inside the fx app has already reported - // its own failure (and suppressed it under --json); errReported - // says so. Printing it again here would double the error line. + // its own failure (`snapshot verify --json` puts it in the + // document instead); errReported says so. Printing it again + // here would double the error line. // Every other error — bad arguments, a config that would not // load — reaches Entry unreported, so it is shown here. if !errors.Is(err, errReported) { @@ -49,9 +52,8 @@ func Entry() int { // emitStartupBanner writes the startup banner to w unless args (the // argument vector with the program name already stripped) contains a -// flag that suppresses it. Split out of Entry so that the decision — the -// only thing standing between a --json invocation and a parseable -// stdout — is reachable from a test without running the whole CLI. +// flag that suppresses it. Split out of Entry so that the decision is +// reachable from a test without running the whole CLI. func emitStartupBanner(args []string, w io.Writer) { if bannerSuppressedInArgs(args) { return @@ -86,8 +88,7 @@ func ReportErrorf(format string, args ...any) { // --json is a subcommand flag rather than a persistent one, but so is // --cron (it exists only on `snapshot create`), so this adds no new // class of imprecision. The only cost of a false positive is a missing -// decorative banner; the cost of a false negative is a corrupt document -// on stdout, so the scan errs deliberately in that direction. +// decorative banner. func bannerSuppressedInArgs(args []string) bool { for _, a := range args { if a == "--" { diff --git a/internal/cli/entry_banner_test.go b/internal/cli/entry_banner_test.go index df7321b..3aa7f20 100644 --- a/internal/cli/entry_banner_test.go +++ b/internal/cli/entry_banner_test.go @@ -41,18 +41,9 @@ const ( someSnapshotID = "host_2026-01-01T00:00:00Z" ) -// placeholderJSONDocument stands in for whatever document a --json -// command writes to stdout. `snapshot list --json` with no snapshots -// prints exactly this; the other --json commands print an object rather -// than an array, but this test is not about their shape. It is about -// what is on stdout *before* them, which is the same for all of them -// because Entry prints the banner before cobra has parsed anything and -// therefore before it can know which command is running. -const placeholderJSONDocument = "[]\n" - // jsonArgumentVectors are the argument vectors of every --json // invocation the CLI accepts, with the program name stripped exactly as -// Entry strips it. Each one must leave stdout untouched by the banner. +// Entry strips it. Each one must suppress the banner. // //nolint:gochecknoglobals // read-only test fixture shared by two tests var jsonArgumentVectors = map[string][]string{ @@ -74,39 +65,23 @@ var jsonArgumentVectors = map[string][]string{ }, } -// TestJSONInvocationStdoutIsExactlyOneDocument is the CLI-layer -// regression guard for issue #106: `vaultik snapshot list --json | jq` -// must work with no other flags. -// -// internal/vaultik's TestListSnapshots_JSONStdoutIsOnlyTheDocument -// guards the same contract one layer down, but it calls the library -// function directly and so cannot see Entry, which is where the -// contamination was: the startup banner is written to stdout before -// cobra parses anything, and the suppression scan did not know about -// --json. The two banner lines and the blank line landed ahead of the -// document and `jq` refused the result. -// -// The document is a constant here because this test is about the -// argument vectors, one per --json command; the one that runs a real -// command end to end is TestEntryJSONStdoutIsExactlyOneDocument below. -func TestJSONInvocationStdoutIsExactlyOneDocument(t *testing.T) { +// TestJSONInvocationSuppressesBanner checks that every --json +// invocation suppresses the startup banner, as the README says --json +// does along with --quiet and --cron. The scan is over the raw argument +// vector, so each position and spelling of --json is listed. +func TestJSONInvocationSuppressesBanner(t *testing.T) { t.Parallel() for name, argv := range jsonArgumentVectors { t.Run(name, func(t *testing.T) { t.Parallel() - var stdout bytes.Buffer + var banner bytes.Buffer - emitStartupBanner(argv, &stdout) + emitStartupBanner(argv, &banner) - require.Empty(t, stdout.String(), - "nothing may reach stdout ahead of a --json document") - - _, err := stdout.WriteString(placeholderJSONDocument) - require.NoError(t, err) - - requireExactlyOneJSONDocument(t, stdout.String()) + assert.Empty(t, banner.String(), + "--json suppresses the banner") }) } } @@ -127,11 +102,11 @@ func TestBannerStillPrintedWithoutSuppressingFlag(t *testing.T) { t.Run(name, func(t *testing.T) { t.Parallel() - var stdout bytes.Buffer + var banner bytes.Buffer - emitStartupBanner(argv, &stdout) + emitStartupBanner(argv, &banner) - assert.Contains(t, stdout.String(), "starting up at", + assert.Contains(t, banner.String(), "starting up at", "the banner belongs on invocations that did not opt out") }) } @@ -247,9 +222,7 @@ func TestEntryJSONStdoutIsExactlyOneDocument(t *testing.T) { // captureProcessStdout redirects the process's own stdout to a pipe for // the duration of fn and returns what was written to it. The redirection // has to be at the file-descriptor level rather than through an injected -// writer, because the banner and the JSON encoder reach os.Stdout -// independently and the point of the test is that both land in the same -// place. +// writer, because the commands Entry runs reach os.Stdout directly. // // Not parallel-safe: os.Stdout is process-global. func captureProcessStdout(t *testing.T, fn func()) string { diff --git a/internal/cli/entry_status_test.go b/internal/cli/entry_status_test.go index 9042bce..2bd7bd6 100644 --- a/internal/cli/entry_status_test.go +++ b/internal/cli/entry_status_test.go @@ -15,8 +15,8 @@ import ( // run, so a failing command must come back with a non-zero code rather // than ending the process here. // -// Stdout is captured only to keep the banner and command output off the -// test log; the assertion is on the returned code. +// Stdout and stderr are captured only to keep the banner and command +// output off the test log; the assertion is on the returned code. // //nolint:paralleltest // replaces os.Args and rootFlags func TestEntryReturnsStatusCode(t *testing.T) { @@ -50,7 +50,7 @@ func TestEntryReturnsStatusCode(t *testing.T) { var code int - _ = captureProcessStdout(t, func() { code = Entry() }) + _, _ = captureProcessStdoutAndStderr(t, func() { code = Entry() }) assert.Equal(t, testCase.want, code) }) diff --git a/internal/cli/entry_stdout_stderr_test.go b/internal/cli/entry_stdout_stderr_test.go new file mode 100644 index 0000000..43a6e42 --- /dev/null +++ b/internal/cli/entry_stdout_stderr_test.go @@ -0,0 +1,152 @@ +package cli //nolint:testpackage // shares hermeticConfig and the capture helpers + +import ( + "context" + "fmt" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/adrg/xdg" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "sneak.berlin/go/vaultik/internal/database" +) + +// TestEntryCompletionStdoutIsTheScript runs `vaultik completion bash`, +// whose stdout the README tells the user to source. The script has to +// start on the first line. +// +//nolint:paralleltest // replaces os.Args, os.Stdout and os.Stderr +func TestEntryCompletionStdoutIsTheScript(t *testing.T) { + code, stdout, _ := runEntry(t, "completion", "bash") + + require.Equal(t, 0, code) + + firstLine, _, _ := strings.Cut(stdout, "\n") + assert.True(t, strings.HasPrefix(firstLine, "# bash completion"), + "the first line of stdout must be the script's, got %q", firstLine) +} + +// TestEntryConfigGetStdoutIsTheValue runs `vaultik config get`, whose +// stdout a script reads as the value and nothing else. +// +//nolint:paralleltest // replaces os.Args, os.Stdout and os.Stderr +func TestEntryConfigGetStdoutIsTheValue(t *testing.T) { + configPath := filepath.Join(t.TempDir(), "config.yml") + require.NoError(t, os.WriteFile(configPath, + []byte("hostname: test-host\n"), configFileMode)) + + code, stdout, _ := runEntry(t, + flagConfig, configPath, "config", "get", "hostname") + + require.Equal(t, 0, code) + assert.Equal(t, "test-host\n", stdout) +} + +// TestEntryJSONFailureIsReportedOnStderr runs each --json command that +// writes no document when it fails, against a destination it cannot +// use. The error must reach stderr, and stdout must stay empty. +// +//nolint:paralleltest // replaces os.Args, os.Stdout, os.Stderr and the xdg globals +func TestEntryJSONFailureIsReportedOnStderr(t *testing.T) { + for _, testCase := range []struct { + name string + args []string + wantOnStderr string + }{ + { + name: "remote info", + args: []string{cmdRemote, cmdInfo, flagJSON}, + wantOnStderr: "Failed to get remote info", + }, + { + name: "prune", + args: []string{cmdPrune, flagJSON}, + wantOnStderr: "Prune failed", + }, + { + name: "snapshot remove", + args: []string{cmdSnapshot, cmdRemove, someSnapshotID, flagJSON}, + wantOnStderr: "Failed to remove snapshot", + }, + } { + t.Run(testCase.name, func(t *testing.T) { + configPath := writeUnusableDestinationConfig(t) + + code, stdout, stderr := runEntry(t, + append([]string{flagConfig, configPath}, testCase.args...)...) + + assert.Equal(t, 1, code) + assert.Empty(t, stdout, + "a failed --json command has no document to write") + assert.Contains(t, stderr, testCase.wantOnStderr, + "the failure must be reported on stderr") + }) + } +} + +// writeUnusableDestinationConfig builds a config whose destination +// directory does not exist, which fails `remote info`, and whose local +// index is bound to another destination, which fails `prune` and +// `snapshot remove` (a missing destination alone only makes `snapshot +// remove` warn). Returns the config path. +func writeUnusableDestinationConfig(t *testing.T) string { + t.Helper() + + dir := t.TempDir() + configPath := filepath.Join(dir, "config.yml") + indexPath := filepath.Join(dir, "index.sqlite") + + contents := fmt.Sprintf(hermeticConfig, + filepath.Join(dir, "source"), + filepath.Join(dir, "missing-store"), + indexPath) + + require.NoError(t, + os.WriteFile(configPath, []byte(contents), configFileMode)) + + // The PID lock lives under xdg.DataHome, which xdg resolves at + // package init; point it at the temp dir so the test neither + // touches nor collides with the real one. + t.Setenv("XDG_DATA_HOME", filepath.Join(dir, "data")) + xdg.Reload() + t.Cleanup(xdg.Reload) + + ctx := context.Background() + + db, err := database.New(ctx, indexPath) + require.NoError(t, err) + + defer func() { require.NoError(t, db.Close()) }() + + require.NoError(t, database.NewRepositories(db).LocalMeta.Set(ctx, + database.LocalMetaKeyStorageURL, "file://"+filepath.Join(dir, "other"))) + + return configPath +} + +// runEntry runs Entry with args after the program name and returns its +// exit code and what it wrote to stdout and stderr. +// +// Not parallel-safe: it replaces os.Args, os.Stdout and os.Stderr. +func runEntry(t *testing.T, args ...string) (int, string, string) { + t.Helper() + + previousArgs := os.Args + + t.Cleanup(func() { + os.Args = previousArgs + rootFlags = RootFlags{} + }) + + os.Args = append([]string{programName}, args...) + + var code int + + stdout, stderr := captureProcessStdoutAndStderr(t, + func() { code = Entry() }) + + return code, stdout, stderr +} diff --git a/internal/cli/prune.go b/internal/cli/prune.go index b3b6040..ef3cef6 100644 --- a/internal/cli/prune.go +++ b/internal/cli/prune.go @@ -48,10 +48,6 @@ work (e.g. after a crashed backup or to reclaim storage).`, }, func(v *vaultik.Vaultik) error { return v.Prune(opts) }, func(err error) { - if opts.JSON { - return - } - log.Error("Prune operation failed", "error", err) ReportErrorf("Prune failed: %v", err) }) diff --git a/internal/cli/remote.go b/internal/cli/remote.go index e561298..dd72e9f 100644 --- a/internal/cli/remote.go +++ b/internal/cli/remote.go @@ -45,7 +45,7 @@ This is destructive and irreversible. Requires --force.`, return errNukeNeedsForce } - return runVaultikApp(cmd, mutating, false, false, "Remote nuke failed", + return runVaultikApp(cmd, mutating, false, "Remote nuke failed", func(v *vaultik.Vaultik) error { return v.NukeRemote(true) }) @@ -92,10 +92,6 @@ func newRemoteInfoCommand() *cobra.Command { }, func(v *vaultik.Vaultik) error { return v.RemoteInfo(jsonOutput) }, func(err error) { - if jsonOutput { - return - } - log.Error("Failed to get remote info", "error", err) ReportErrorf("Failed to get remote info: %v", err) }) diff --git a/internal/cli/snapshot.go b/internal/cli/snapshot.go index cee6594..4a0e095 100644 --- a/internal/cli/snapshot.go +++ b/internal/cli/snapshot.go @@ -126,7 +126,7 @@ func newSnapshotListCommand() *cobra.Command { Long: "Lists all snapshots with their ID, timestamp, and compressed size", Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, _ []string) error { - return runVaultikApp(cmd, readOnly, false, false, + return runVaultikApp(cmd, readOnly, false, "Failed to list snapshots", func(v *vaultik.Vaultik) error { return v.ListSnapshots(jsonOutput) @@ -162,7 +162,7 @@ restrict the operation to specific snapshot names.`, return errPurgeCriteriaBoth } - return runVaultikApp(cmd, mutating, false, false, + return runVaultikApp(cmd, mutating, false, "Failed to purge snapshots", func(v *vaultik.Vaultik) error { return v.PurgeSnapshotsWithOptions(opts) @@ -265,7 +265,7 @@ To wipe the entire destination store and start over, use 'vaultik remote nuke --force' — it is the single supported entry point for that.`, Args: requireSnapshotIDArg, RunE: func(cmd *cobra.Command, args []string) error { - return runVaultikApp(cmd, mutating, opts.JSON, opts.JSON, + return runVaultikApp(cmd, mutating, opts.JSON, "Failed to remove snapshot", func(v *vaultik.Vaultik) error { _, err := v.RemoveSnapshot(args[0], opts) diff --git a/internal/vaultik/snapshot.go b/internal/vaultik/snapshot.go index cb61ba5..02dbb51 100644 --- a/internal/vaultik/snapshot.go +++ b/internal/vaultik/snapshot.go @@ -727,7 +727,7 @@ func (v *Vaultik) VerifySnapshotWithOptions( result.BlobCount = manifest.BlobCount result.TotalSize = manifest.TotalCompressedSize - if !opts.JSON { + if !opts.JSON && !v.UI.Quiet() { v.stdoutf("Snapshot information:\n") v.stdoutf(" Blob count: %d\n", manifest.BlobCount) v.stdoutf(" Total size: %s\n", ubytes(manifest.TotalCompressedSize)) @@ -782,7 +782,7 @@ func (v *Vaultik) printVerifyHeader(snapshotID string, opts *VerifyOptions) { snapshotTime = t } - if !opts.JSON { + if !opts.JSON && !v.UI.Quiet() { v.stdoutf("Verifying snapshot %s\n", snapshotID) if !snapshotTime.IsZero() { @@ -822,7 +822,7 @@ func (v *Vaultik) verifyManifestBlobs( stat, err := v.Storage.Stat(v.ctx, blobPath) switch { case err != nil: - if !opts.JSON { + if !opts.JSON && !v.UI.Quiet() { v.stdoutf(" Missing: %s (%s)\n", blob.Hash, ubytes(blob.CompressedSize)) } @@ -830,7 +830,7 @@ func (v *Vaultik) verifyManifestBlobs( missing++ missingSize += blob.CompressedSize case stat.Size != blob.CompressedSize: - if !opts.JSON { + if !opts.JSON && !v.UI.Quiet() { v.stdoutf(" Wrong size: %s (store has %s, manifest lists %s)\n", blob.Hash, ubytes(stat.Size), ubytes(blob.CompressedSize)) } @@ -862,6 +862,22 @@ func (v *Vaultik) formatVerifyResult( return v.outputVerifyJSON(result) } + // Under --quiet a failure is still returned, and the cli layer + // prints it on stderr. + if !v.UI.Quiet() { + v.printVerifySummary(result, failure) + } + + if failure != "" { + return fmt.Errorf("%w: %s", errSnapshotVerifyFailed, failure) + } + + return nil +} + +// printVerifySummary prints the counts and the status line that end the +// human-readable shallow verify report. failure is empty when it passed. +func (v *Vaultik) printVerifySummary(result *VerifyResult, failure string) { v.stdoutf("\nVerification complete:\n") v.stdoutf(" Present with listed size: %d blobs\n", result.Verified) @@ -883,14 +899,12 @@ func (v *Vaultik) formatVerifyResult( if failure != "" { v.stdoutf("FAILED - %s\n", failure) - return fmt.Errorf("%w: %s", errSnapshotVerifyFailed, failure) + return } // Report only what was actually checked: presence and size, not contents. v.stdoutf("OK - all %d blobs listed in the manifest are present with the "+ "listed size; contents not checked (use --deep)\n", result.Verified) - - return nil } // shallowVerifyFailure returns a human-readable description of everything diff --git a/internal/vaultik/verify.go b/internal/vaultik/verify.go index cf43c62..070c47b 100644 --- a/internal/vaultik/verify.go +++ b/internal/vaultik/verify.go @@ -103,7 +103,7 @@ func (v *Vaultik) RunDeepVerify(snapshotID string, opts *VerifyOptions) error { log.Info("Starting snapshot verification", "snapshot_id", snapshotID, "mode", "deep") - if !opts.JSON { + if !opts.JSON && !v.UI.Quiet() { v.stdoutf("Deep verification of snapshot: %s\n\n", snapshotID) } @@ -143,10 +143,13 @@ func (v *Vaultik) RunDeepVerify(snapshotID string, opts *VerifyOptions) error { log.Info("✓ Verification completed successfully", "snapshot_id", snapshotID, "mode", "deep", "blobs_verified", len(dbBlobs)) - v.stdoutf("\n✓ Verification completed successfully\n") - v.stdoutf(" Snapshot: %s\n", snapshotID) - v.stdoutf(" Blobs verified: %d\n", len(dbBlobs)) - v.stdoutf(" Total size: %s\n", ubytes(totalSize)) + + if !v.UI.Quiet() { + v.stdoutf("\n✓ Verification completed successfully\n") + v.stdoutf(" Snapshot: %s\n", snapshotID) + v.stdoutf(" Blobs verified: %d\n", len(dbBlobs)) + v.stdoutf(" Total size: %s\n", ubytes(totalSize)) + } return nil } @@ -170,7 +173,7 @@ func (v *Vaultik) loadVerificationData( // remote manifests; see its doc comment. log.Info("Downloading manifest", "remote_key", remoteKey) - if !opts.JSON { + if !opts.JSON && !v.UI.Quiet() { v.stdoutf("Downloading manifest...\n") } @@ -185,7 +188,7 @@ func (v *Vaultik) loadVerificationData( "manifest_blob_count", manifest.BlobCount, "manifest_total_size", ubytes(manifest.TotalCompressedSize)) - if !opts.JSON { + if !opts.JSON && !v.UI.Quiet() { v.stdoutf("Manifest loaded: %d blobs (%s)\n", manifest.BlobCount, ubytes(manifest.TotalCompressedSize)) v.stdoutf("Downloading and decrypting database...\n") @@ -215,7 +218,7 @@ func (v *Vaultik) loadVerificationData( "db_blob_count", len(dbBlobs), "db_total_size", ubytes(dbTotalSize)) - if !opts.JSON { + if !opts.JSON && !v.UI.Quiet() { v.stdoutf("Database loaded: %d blobs (%s)\n", len(dbBlobs), ubytes(dbTotalSize)) } @@ -273,7 +276,7 @@ func (v *Vaultik) runVerificationSteps( totalSize int64, identities []age.Identity, ) error { - if !opts.JSON { + if !opts.JSON && !v.UI.Quiet() { v.stdoutf("Verifying manifest against database...\n") } @@ -282,7 +285,7 @@ func (v *Vaultik) runVerificationSteps( return v.deepVerifyFailure(result, opts, err.Error(), err) } - if !opts.JSON { + if !opts.JSON && !v.UI.Quiet() { v.stdoutf("Manifest verified.\n") v.stdoutf("Checking blob existence in remote storage...\n") } @@ -292,7 +295,7 @@ func (v *Vaultik) runVerificationSteps( return v.deepVerifyFailure(result, opts, err.Error(), err) } - if !opts.JSON { + if !opts.JSON && !v.UI.Quiet() { v.stdoutf("All blobs exist.\n") v.stdoutf("Downloading and verifying blob contents (%d blobs, %s)...\n", len(dbBlobs), ubytes(totalSize)) @@ -748,7 +751,7 @@ func (v *Vaultik) performDeepVerificationFromDB( "eta", eta.Round(time.Second), ) - if !opts.JSON { + if !opts.JSON && !v.UI.Quiet() { v.stdoutf(" Verified %d/%d blobs (%d remaining) - %s/%s - elapsed %s, eta %s\n", i+1, len(blobs), remaining, ubytes(bytesProcessed), diff --git a/internal/vaultik/verify_quiet_test.go b/internal/vaultik/verify_quiet_test.go new file mode 100644 index 0000000..b4350db --- /dev/null +++ b/internal/vaultik/verify_quiet_test.go @@ -0,0 +1,102 @@ +package vaultik_test + +import ( + "bytes" + "context" + "io" + "os" + "path/filepath" + "testing" + + "github.com/spf13/afero" + "github.com/stretchr/testify/require" + "sneak.berlin/go/vaultik/internal/log" + "sneak.berlin/go/vaultik/internal/snapshot" + "sneak.berlin/go/vaultik/internal/ui" + "sneak.berlin/go/vaultik/internal/vaultik" +) + +// TestVerify_QuietSuppressesReport is the --quiet contract for +// `snapshot verify`: neither shallow nor deep verify writes its report, +// a failed verify still returns its error (which the cli layer prints +// on stderr), and the --json document still emits. +func TestVerify_QuietSuppressesReport(t *testing.T) { + log.Initialize(log.Config{}) + t.Parallel() + + fs := afero.NewOsFs() + tempDir := t.TempDir() + + dataDir := filepath.Join(tempDir, "source") + storeDir := filepath.Join(tempDir, "remote") + dbPath := filepath.Join(tempDir, "index.sqlite") + + chunkSize := int64(32 * 1024) + maxBlobSize := int64(128 * 1024) + + require.NoError(t, fs.MkdirAll(dataDir, 0o755)) + require.NoError(t, afero.WriteFile(fs, + filepath.Join(dataDir, "data.bin"), + bytesPattern("quiet-", int(maxBlobSize*2)), 0o644)) + + ctx := context.Background() + + cfg, storer, snapshotID := runFileStorageBackup( + ctx, t, fs, dataDir, storeDir, dbPath, chunkSize, maxBlobSize) + + // The UI writes to the same buffer as Stdout, as both write to the + // process's stdout in production. + var stdout bytes.Buffer + + newQuietVerifier := func() *vaultik.Vaultik { + v := &vaultik.Vaultik{ + Config: cfg, + Storage: storer, + Fs: fs, + Stdout: &stdout, + Stderr: io.Discard, + UI: ui.NewWithColor(&stdout, false), + } + v.SetContext(ctx) + v.UI.SetQuiet(true) + + return v + } + + require.NoError(t, newQuietVerifier().VerifySnapshotWithOptions( + snapshotID, &vaultik.VerifyOptions{})) + require.Empty(t, stdout.String(), + "shallow verify must write no report under --quiet") + + require.NoError(t, newQuietVerifier().VerifySnapshotWithOptions( + snapshotID, &vaultik.VerifyOptions{Deep: true})) + require.Empty(t, stdout.String(), + "deep verify must write no report under --quiet") + + require.NoError(t, newQuietVerifier().VerifySnapshotWithOptions( + snapshotID, &vaultik.VerifyOptions{JSON: true})) + require.Equal(t, "ok", decodeVerifyResult(t, stdout.Bytes()).Status, + "the --json document must still emit under --quiet") + + // A snapshot without its encrypted database fails shallow verify. A + // failed report also lists each missing blob and each blob of the + // wrong size, so remove one blob and grow another. + require.NoError(t, os.Remove(filepath.Join(storeDir, "metadata", + snapshot.RemoteSnapshotKey(snapshotID), "db.zst.age"))) + + blobFiles, err := filepath.Glob( + filepath.Join(storeDir, "blobs", "*", "*", "*")) + require.NoError(t, err) + require.GreaterOrEqual(t, len(blobFiles), 2, + "the snapshot must span two blobs, one to remove and one to grow") + require.NoError(t, os.Remove(blobFiles[0])) + growOneBlob(t, fs, filepath.Join(storeDir, "blobs")) + + stdout.Reset() + + require.Error(t, newQuietVerifier().VerifySnapshotWithOptions( + snapshotID, &vaultik.VerifyOptions{}), + "--quiet must not change the outcome of a failed verify") + require.Empty(t, stdout.String(), + "a failed verify must write no report under --quiet") +}