Skip to content

Commit 028a3ac

Browse files
committed
feat(diff): add dash0 diff command
New experimental `dash0 diff -f <file|directory> [--since <ref>]` command previews what `apply` would do without ever creating, updating, or deleting anything: - Fetches each document's current state from Dash0 (per-kind, non-mutating) to accurately distinguish create from update, unlike `apply --dry-run`, which is local-only and can't tell the two apart. - All-or-nothing fetch gate: any document fetch failure (other than a plain "not found") aborts the whole plan before anything is printed. - `--since <ref>` reuses apply's identifier-diffing to preview deletions alongside creates/updates, never deleting anything regardless of confirmation. - Three-way exit code (0 clean, 1 differences pending, 2 error), modeled on `kubectl diff` — a deliberate, narrow exception to this CLI's uniform 0/1 convention. - Human-mode unified-diff report and agent-mode JSON output matching `apply --dry-run`'s {path, changes} shape, extended with a "create" op. Supporting changes: - Move apply --since's git-diffing plan (deletionPlan) into internal/git as the exported SincePlan/ComputeSincePlan, so diff can reuse it without depending on internal/apply. - Move apply's document validation (validateDocuments) and PrometheusRule CRD parsing into internal/asset as ValidateDocuments/ParsePrometheusRuleCRD, for the same reason. - Fix a real cobra Traverse bug in cmd/dash0/main.go: a boolean persistent flag preceding the subcommand (e.g. `dash0 --experimental diff ...`, the form used throughout this CLI's docs) was misresolved to the root command because Traverse's flag lookup doesn't see PersistentFlags() until cobra's own lazy merge runs during Execute. This silently broke diff's exit code (always falling back to 1 instead of 2 on a genuine error) for that invocation order. - Deprecate `apply --dry-run` in favor of `dash0 diff`, with a runtime stderr warning and updated help text. References #256.
1 parent 41d6e4f commit 028a3ac

28 files changed

Lines changed: 1992 additions & 444 deletions

.chloggen/feat_diff-command.yaml

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
change_type: new_component
2+
3+
component: diff
4+
5+
note: "Add the experimental `dash0 diff` command, which previews what `apply` would do (creates, updates, and — with `--since` — deletions) without ever mutating anything."
6+
7+
issues: [256]
8+
9+
subtext: |
10+
`diff` fetches each document's current state from Dash0 first, so it can accurately distinguish a create from
11+
an update — unlike `apply --dry-run`, which is local-only and cannot tell the two apart. It uses a three-way
12+
exit code (0 clean, 1 differences pending, 2 error), modeled on `kubectl diff`.
13+
`apply --dry-run` is now deprecated in favor of `dash0 diff` and prints a warning to stderr on every invocation.
14+
15+
change_logs: [user]

README.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -373,6 +373,12 @@ Validate without applying:
373373
dash0 apply -f assets.yaml --dry-run
374374
```
375375
376+
`--dry-run` is deprecated in favor of `dash0 diff` (experimental, requires `-X`), which fetches each asset's current state from Dash0 first to accurately distinguish a create from an update:
377+
378+
```bash
379+
dash0 -X diff -f assets.yaml
380+
```
381+
376382
Sync a directory to match its state as of a git ref, deleting assets removed since then (experimental, requires `-X`):
377383
378384
```bash

cmd/dash0/main.go

Lines changed: 59 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package main
22

33
import (
44
"context"
5+
"errors"
56
"fmt"
67
"os"
78
"os/signal"
@@ -18,6 +19,7 @@ import (
1819
dashcolor "github.com/dash0hq/dash0-cli/internal/color"
1920
"github.com/dash0hq/dash0-cli/internal/config"
2021
"github.com/dash0hq/dash0-cli/internal/dashboards"
22+
"github.com/dash0hq/dash0-cli/internal/diff"
2123
"github.com/dash0hq/dash0-cli/internal/help"
2224
"github.com/dash0hq/dash0-cli/internal/logging"
2325
"github.com/dash0hq/dash0-cli/internal/login"
@@ -77,6 +79,7 @@ func init() {
7779
rootCmd.AddCommand(failedchecks.NewFailedChecksCmd())
7880
rootCmd.AddCommand(config.NewConfigCmd())
7981
rootCmd.AddCommand(dashboards.NewDashboardsCmd())
82+
rootCmd.AddCommand(diff.NewDiffCmd())
8083
rootCmd.AddCommand(logging.NewLogsCmd())
8184
rootCmd.AddCommand(login.NewLoginCmd())
8285
rootCmd.AddCommand(login.NewLogoutCmd())
@@ -298,8 +301,27 @@ func main() {
298301
ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM)
299302
defer stop()
300303

301-
// Determine which command will be executed (best-effort; Traverse may
302-
// return the root command when persistent flags like -X come first).
304+
// Command.Traverse decides whether a "--name value"-shaped argument is a
305+
// flag (consuming the next token) or a bareword by looking up the flag
306+
// in c.Flags() — which does NOT include persistent flags registered via
307+
// PersistentFlags() until cobra's own (unexported) mergePersistentFlags
308+
// runs, which normally happens lazily during Execute, i.e. after this
309+
// pre-flight Traverse call. Without this merge, a boolean persistent
310+
// flag preceding the subcommand (e.g. `dash0 --experimental diff ...`,
311+
// the form used throughout this CLI's own docs and examples) is
312+
// wrongly treated as expecting a value, which swallows the subcommand
313+
// name as that value and makes Traverse return the root command
314+
// instead of the real target. Replicating the merge here first (a
315+
// public, idempotent equivalent of cobra's own step) fixes that. See
316+
// TestTraverseTargetCommand in main_test.go for a regression test
317+
// against an isolated command tree (rootCmd itself is a package-level
318+
// singleton that Execute() mutates as a side effect, which would mask
319+
// this bug in a test that reused rootCmd after any earlier Execute call).
320+
rootCmd.Flags().AddFlagSet(rootCmd.PersistentFlags())
321+
322+
// Determine which command will be executed (best-effort; Traverse can
323+
// still fall back to the root command for shapes it doesn't model,
324+
// e.g. an unrecognized flag).
303325
targetCmd, _, _ := rootCmd.Traverse(os.Args[1:])
304326

305327
// Resolve agent mode before any output.
@@ -366,15 +388,43 @@ func main() {
366388
}
367389

368390
if err := rootCmd.ExecuteContext(ctx); err != nil {
369-
printError(err)
370-
// Show usage only for flag/argument errors, not for runtime errors.
371-
// Commands set SilenceUsage = true once past flag validation.
372-
if !agentmode.Enabled && targetCmd != nil && targetCmd.Name() != "dash0" && !targetCmd.SilenceUsage {
373-
fmt.Fprintln(os.Stderr)
374-
_ = targetCmd.Usage()
391+
cmdName := ""
392+
if targetCmd != nil {
393+
cmdName = targetCmd.Name()
375394
}
376-
os.Exit(1)
395+
396+
// A *diff.PendingDifferencesError is not a failure -- the diff
397+
// report was already printed to stdout/stderr by the time it's
398+
// returned, so it must never be rendered through the
399+
// "Error:"-prefixed path (and usage must not be printed either).
400+
if !errors.As(err, new(*diff.PendingDifferencesError)) {
401+
printError(err)
402+
// Show usage only for flag/argument errors, not for runtime errors.
403+
// Commands set SilenceUsage = true once past flag validation.
404+
if !agentmode.Enabled && targetCmd != nil && targetCmd.Name() != "dash0" && !targetCmd.SilenceUsage {
405+
fmt.Fprintln(os.Stderr)
406+
_ = targetCmd.Usage()
407+
}
408+
}
409+
os.Exit(exitCodeForError(cmdName, err))
410+
}
411+
}
412+
413+
// exitCodeForError determines the process exit code for a command whose
414+
// RunE returned a non-nil err. Every command exits 1 on error except dash0
415+
// diff, which uses a three-way exit code (0 clean, 1 differences pending, 2
416+
// genuine error) instead of this CLI's uniform 0/1 convention -- modeled on
417+
// `kubectl diff`, so a naive CI step doesn't fail on the routine "changes
418+
// pending" case, but still fails hard on a genuine error (bad --since ref,
419+
// API unreachable, and so on).
420+
func exitCodeForError(cmdName string, err error) int {
421+
if errors.As(err, new(*diff.PendingDifferencesError)) {
422+
return 1
423+
}
424+
if cmdName == "diff" {
425+
return 2
377426
}
427+
return 1
378428
}
379429

380430
// installJSONHelp replaces the default help function on cmd and all

cmd/dash0/main_test.go

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,16 @@ package main
33
import (
44
"bytes"
55
"errors"
6+
"fmt"
67
"io"
78
"os"
89
"path/filepath"
910
"testing"
1011

1112
"github.com/dash0hq/dash0-cli/internal/agentmode"
13+
"github.com/dash0hq/dash0-cli/internal/diff"
1214
"github.com/dash0hq/dash0-cli/internal/skill"
15+
"github.com/spf13/cobra"
1316
"github.com/stretchr/testify/assert"
1417
"github.com/stretchr/testify/require"
1518
)
@@ -57,6 +60,79 @@ func TestRootCommandExecution(t *testing.T) {
5760
}
5861
}
5962

63+
// newIsolatedRootForTraverseTest builds a fresh root+child command tree
64+
// shaped like the real dash0 root command (a boolean persistent flag plus
65+
// one subcommand with its own flags), so tests can exercise Traverse
66+
// without touching the package-level rootCmd singleton. rootCmd is mutated
67+
// as a side effect by cobra internals the first time anything calls
68+
// rootCmd.Execute() anywhere in the test binary (e.g.
69+
// TestRootCommandExecution) -- persistent flags get merged into its local
70+
// flag set lazily, which would silently fix the exact bug this test exists
71+
// to catch and make the regression test pass regardless of whether main()
72+
// still carries the fix.
73+
func newIsolatedRootForTraverseTest() (*cobra.Command, *cobra.Command) {
74+
root := &cobra.Command{Use: "dash0"}
75+
root.PersistentFlags().BoolP("experimental", "X", false, "Enable experimental features")
76+
child := &cobra.Command{Use: "diff"}
77+
child.Flags().StringP("file", "f", "", "")
78+
root.AddCommand(child)
79+
return root, child
80+
}
81+
82+
// TestTraverseTargetCommand is a regression test for a real cobra pitfall:
83+
// Command.Traverse decides whether a "--name" token expects a following
84+
// value by looking up the flag in c.Flags(), which does not include flags
85+
// registered via PersistentFlags() until cobra's own mergePersistentFlags
86+
// runs (normally during Execute, i.e. after Traverse). Without pre-merging
87+
// persistent flags into the root command's own flag set (main()'s fix,
88+
// right before its own Traverse call), a boolean persistent flag preceding
89+
// the subcommand -- e.g. `dash0 --experimental diff ...`, the invocation
90+
// form used throughout this CLI's own docs -- gets wrongly treated as
91+
// expecting a value, swallowing the subcommand name as that value and
92+
// making Traverse resolve to the root command instead of the real target.
93+
// This mattered concretely for `dash0 diff`'s three-way exit code: main()'s
94+
// exitCodeForError branches on the resolved command's name, so a
95+
// misresolved target silently fell back to exit 1 instead of exit 2 on a
96+
// genuine error.
97+
func TestTraverseTargetCommand(t *testing.T) {
98+
cases := []struct {
99+
name string
100+
args []string
101+
want string
102+
}{
103+
{"persistent bool flag before subcommand", []string{"--experimental", "diff", "-f", "x.yaml"}, "diff"},
104+
{"shorthand persistent bool flag before subcommand", []string{"-X", "diff", "-f", "x.yaml"}, "diff"},
105+
{"no persistent flag", []string{"diff", "-f", "x.yaml"}, "diff"},
106+
{"persistent flag after subcommand", []string{"diff", "--experimental", "-f", "x.yaml"}, "diff"},
107+
}
108+
for _, tc := range cases {
109+
t.Run(tc.name, func(t *testing.T) {
110+
root, _ := newIsolatedRootForTraverseTest()
111+
// The fix under test: without this, a fresh root command (never
112+
// Executed, so cobra's own lazy persistent-flag merge hasn't run
113+
// yet) reproduces the bug.
114+
root.Flags().AddFlagSet(root.PersistentFlags())
115+
116+
cmd, _, err := root.Traverse(tc.args)
117+
require.NoError(t, err)
118+
assert.Equal(t, tc.want, cmd.Name())
119+
})
120+
}
121+
}
122+
123+
// TestTraverseTargetCommand_ReproducesBugWithoutFix proves the fix is load-
124+
// bearing: the same isolated, never-Executed root command tree without the
125+
// AddFlagSet pre-merge misresolves a persistent bool flag preceding the
126+
// subcommand, confirming TestTraverseTargetCommand isn't passing for some
127+
// unrelated reason.
128+
func TestTraverseTargetCommand_ReproducesBugWithoutFix(t *testing.T) {
129+
root, _ := newIsolatedRootForTraverseTest()
130+
131+
cmd, _, err := root.Traverse([]string{"--experimental", "diff", "-f", "x.yaml"})
132+
require.NoError(t, err)
133+
assert.Equal(t, "dash0", cmd.Name(), "without the AddFlagSet pre-merge, Traverse should misresolve to the root command")
134+
}
135+
60136
// TestWithSkillHint covers the agent-mode error hint pointing at
61137
// `dash0 skill install`, added centrally in printError.
62138
func TestWithSkillHint(t *testing.T) {
@@ -256,3 +332,29 @@ func TestFlagValue(t *testing.T) {
256332
})
257333
}
258334
}
335+
336+
// TestExitCodeForError pins dash0 diff's three-way exit code (0 clean -- not
337+
// exercised here since exitCodeForError is only called when err != nil, 1
338+
// differences pending, 2 genuine error) against every other command's
339+
// uniform 1-on-any-error convention.
340+
func TestExitCodeForError(t *testing.T) {
341+
genericErr := errors.New("boom")
342+
pendingErr := &diff.PendingDifferencesError{Count: 3}
343+
344+
cases := []struct {
345+
name string
346+
cmdName string
347+
err error
348+
want int
349+
}{
350+
{"diff genuine error exits 2", "diff", genericErr, 2},
351+
{"diff pending differences exits 1", "diff", pendingErr, 1},
352+
{"diff pending differences wrapped still exits 1", "diff", fmt.Errorf("wrapped: %w", pendingErr), 1},
353+
{"non-diff command exits 1 on any error", "apply", genericErr, 1},
354+
}
355+
for _, tc := range cases {
356+
t.Run(tc.name, func(t *testing.T) {
357+
assert.Equal(t, tc.want, exitCodeForError(tc.cmdName, tc.err))
358+
})
359+
}
360+
}

0 commit comments

Comments
 (0)