diff --git a/client/go.mod b/client/go.mod index 2680e8028..be5f8acdf 100644 --- a/client/go.mod +++ b/client/go.mod @@ -20,7 +20,6 @@ require ( github.com/mattn/go-isatty v0.0.21 github.com/neticdk/go-stdlib v1.0.1 github.com/planetscale/vtprotobuf v0.6.1-0.20250313105119-ba97887b0a25 - github.com/sergi/go-diff v1.4.0 github.com/siderolabs/gen v0.8.6 github.com/siderolabs/go-api-signature v0.3.12 github.com/siderolabs/go-kubeconfig v0.1.2 diff --git a/client/go.sum b/client/go.sum index 0a91a0312..b87bfa8f2 100644 --- a/client/go.sum +++ b/client/go.sum @@ -145,11 +145,8 @@ github.com/jxskiss/base62 v1.1.0 h1:A5zbF8v8WXx2xixnAKD2w+abC+sIzYJX+nxmhA6HWFw= github.com/jxskiss/base62 v1.1.0/go.mod h1:HhWAlUXvxKThfOlZbcuFzsqwtF5TcqS9ru3y5GfjWAc= github.com/klauspost/compress v1.18.5 h1:/h1gH5Ce+VWNLSWqPzOVn6XBO+vJbCNGvjoaGBFW2IE= github.com/klauspost/compress v1.18.5/go.mod h1:cwPg85FWrGar70rWktvGQj8/hthj3wpl0PGDogxkrSQ= -github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo= github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE= github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk= -github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= -github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= github.com/liggitt/tabwriter v0.0.0-20181228230101-89fcab3d43de h1:9TO3cAIGXtEhnIaL+V+BEER86oLrvS+kWobKpbJuye0= @@ -360,14 +357,12 @@ google.golang.org/grpc v1.80.0/go.mod h1:ho/dLnxwi3EDJA4Zghp7k2Ec1+c2jqup0bFkw07 google.golang.org/protobuf v1.36.12-0.20260120151049-f2248ac996af h1:+5/Sw3GsDNlEmu7TfklWKPdQ0Ykja5VEmq2i817+jbI= google.golang.org/protobuf v1.36.12-0.20260120151049-f2248ac996af/go.mod h1:HTf+CrKn2C3g5S8VImy6tdcUvCska2kB7j23XfzDpco= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= -gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk= gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EVd6muEfDQjcINNoR0C8j2r3qZ4Q= gopkg.in/evanphx/json-patch.v4 v4.13.0 h1:czT3CmqEaQ1aanPc5SdlgQrrEIb8w/wwCvWWnfEbYzo= gopkg.in/evanphx/json-patch.v4 v4.13.0/go.mod h1:p8EYWUEYMpynmqDbY58zCKCFZw8pRWMG4EsWvDvM72M= gopkg.in/inf.v0 v0.9.1 h1:73M5CoZyi3ZLMOyDlQh031Cx6N9NDJ2Vvfl76EDAgDc= gopkg.in/inf.v0 v0.9.1/go.mod h1:cWUDdTG/fYaXco+Dcufb5Vnc6Gp2YChqWtbxRZE0mXw= -gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.4.0 h1:D8xgwECY7CYvx+Y2n4sBz93Jn9JRvxdiyyo8CTfuKaY= gopkg.in/yaml.v2 v2.4.0/go.mod h1:RDklbk79AGWmwhnvt/jBztapEOGDOx6ZbXqjP6csGnQ= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/client/pkg/execdiff/execdiff.go b/client/pkg/execdiff/execdiff.go new file mode 100644 index 000000000..b0eb8dede --- /dev/null +++ b/client/pkg/execdiff/execdiff.go @@ -0,0 +1,272 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/. + +// Package execdiff provides diff rendering for dry-run operations, +// supporting both built-in colorized output and external diff tools +// via the OMNI_EXTERNAL_DIFF environment variable. +package execdiff + +import ( + "errors" + "fmt" + "io" + "os" + "os/exec" + "path/filepath" + "strings" + + "github.com/fatih/color" + + "github.com/siderolabs/omni/client/pkg/diff" +) + +// EnvExternalDiff is the environment variable that specifies an external diff program. +// +// When set, the diff tool is invoked with two directory paths containing the +// old and new resource YAML files. The value may include arguments, e.g. +// "colordiff -N -u". +// +// Exit codes: 0 = no differences, 1 = differences found, >1 = error. +const EnvExternalDiff = "OMNI_EXTERNAL_DIFF" + +// ErrDifferencesFound is a sentinel error returned by callers of Differ.Flush() +// when they want to signal that differences were found. Commands should wrap +// their result with this sentinel so that the top-level CLI can translate it +// into the documented exit status (1 = differences found). +var ErrDifferencesFound = errors.New("differences found") + +type entry struct { + label string + filename string + oldYAML []byte + newYAML []byte +} + +// Differ accumulates resource diffs and renders them either using a built-in +// colorized unified diff or by invoking an external diff program. +type Differ struct { + w io.Writer + extCmd string + entries []entry + hasDiff bool +} + +// New creates a Differ that writes to w. If OMNI_EXTERNAL_DIFF is set, +// diffs are queued and rendered by the external tool on Flush. +func New(w io.Writer) *Differ { + return &Differ{ + w: w, + extCmd: os.Getenv(EnvExternalDiff), + } +} + +// IsExternal returns true if an external diff tool is configured. +func (d *Differ) IsExternal() bool { + return d.extCmd != "" +} + +// SanitizeFilename returns a safe filename derived from the given parts. +// +// Path separators and traversal sequences are replaced with underscores so +// that joining the result with a temp directory base path cannot escape the +// directory. Empty / dot-only parts are replaced with an underscore. +func SanitizeFilename(parts ...string) string { + out := make([]string, 0, len(parts)) + + for _, p := range parts { + out = append(out, sanitizeFilenamePart(p)) + } + + return strings.Join(out, "-") +} + +func sanitizeFilenamePart(part string) string { + sanitized := strings.NewReplacer( + "/", "_", + "\\", "_", + "..", "__", + ":", "_", + "\x00", "_", + ).Replace(part) + + switch sanitized { + case "", ".", "..": + return "_" + default: + return sanitized + } +} + +// AddDiff records a diff between two versions of a resource. +// +// For creates, oldYAML should be nil. For deletes, newYAML should be nil. +// label is a human-readable description (e.g. "MachineSet(cluster1)"). +// filename is used as the YAML file name in temp directories for external mode; +// callers must provide a sanitized filename (see SanitizeFilename) - filenames +// that contain path separators or traversal sequences are rejected. +// +// In built-in mode, the diff is rendered immediately to the writer. +// In external mode, the entry is queued for Flush. +func (d *Differ) AddDiff(label, filename string, oldYAML, newYAML []byte) error { + if d.IsExternal() { + if err := validateFilename(filename); err != nil { + return err + } + + d.entries = append(d.entries, entry{ + label: label, + filename: filename, + oldYAML: oldYAML, + newYAML: newYAML, + }) + + return nil + } + + return d.renderBuiltin(label, oldYAML, newYAML) +} + +// validateFilename ensures the filename is safe to join with a temp directory +// base path: no path separators, no traversal components, not absolute, not empty. +func validateFilename(name string) error { + if name == "" { + return errors.New("empty filename") + } + + if filepath.IsAbs(name) { + return fmt.Errorf("absolute path not allowed: %q", name) + } + + if name != filepath.Base(name) { + return fmt.Errorf("filename must not contain path separators: %q", name) + } + + // filepath.Base("..") returns "..", so this also blocks parent-traversal. + if name == "." || name == ".." { + return fmt.Errorf("invalid filename: %q", name) + } + + return nil +} + +// Flush invokes the external diff tool with temp directories containing +// the queued entries. Returns true if differences were found. +// +// In built-in mode this is a no-op and returns hasDiff accumulated from AddDiff calls. +func (d *Differ) Flush() (bool, error) { + if !d.IsExternal() { + return d.hasDiff, nil + } + + if len(d.entries) == 0 { + return false, nil + } + + liveDir, err := os.MkdirTemp("", "omni-diff-LIVE-") + if err != nil { + return false, fmt.Errorf("failed to create temp dir: %w", err) + } + + defer os.RemoveAll(liveDir) //nolint:errcheck + + mergedDir, err := os.MkdirTemp("", "omni-diff-MERGED-") + if err != nil { + return false, fmt.Errorf("failed to create temp dir: %w", err) + } + + defer os.RemoveAll(mergedDir) //nolint:errcheck + + for _, e := range d.entries { + // Filenames were validated in AddDiff, but defend in depth: re-validate + // here to avoid any path escape if the slice was mutated. + if err := validateFilename(e.filename); err != nil { + return false, err + } + + if e.oldYAML != nil { + if err := os.WriteFile(filepath.Join(liveDir, e.filename), e.oldYAML, 0o600); err != nil { + return false, fmt.Errorf("failed to write old YAML for %s: %w", e.label, err) + } + } + + if e.newYAML != nil { + if err := os.WriteFile(filepath.Join(mergedDir, e.filename), e.newYAML, 0o600); err != nil { + return false, fmt.Errorf("failed to write new YAML for %s: %w", e.label, err) + } + } + } + + return d.runExternal(liveDir, mergedDir) +} + +func (d *Differ) renderBuiltin(label string, oldYAML, newYAML []byte) error { + diffStr, err := diff.Compute(oldYAML, newYAML) + if err != nil { + return err + } + + // Strip the library's generic header; we print our own with the label. + diffStr, _ = strings.CutPrefix(diffStr, "--- a\n+++ b\n") + + if diffStr == "" { + return nil + } + + d.hasDiff = true + + bold := color.New(color.Bold) + bold.Fprintf(d.w, "--- %s\n", label) //nolint:errcheck + bold.Fprintf(d.w, "+++ %s\n", label) //nolint:errcheck + + cyan := color.New(color.FgCyan) + red := color.New(color.FgRed) + green := color.New(color.FgGreen) + + for line := range strings.SplitSeq(diffStr, "\n") { + switch { + case strings.HasPrefix(line, "@@"): + cyan.Fprintln(d.w, line) //nolint:errcheck + case strings.HasPrefix(line, "-"): + red.Fprintln(d.w, line) //nolint:errcheck + case strings.HasPrefix(line, "+"): + green.Fprintln(d.w, line) //nolint:errcheck + case line == "": + // skip trailing empty line + default: + fmt.Fprintln(d.w, line) //nolint:errcheck + } + } + + return nil +} + +func (d *Differ) runExternal(liveDir, mergedDir string) (bool, error) { + parts := strings.Fields(d.extCmd) + if len(parts) == 0 { + return false, errors.New("OMNI_EXTERNAL_DIFF is set but empty after parsing") + } + + name := parts[0] + args := append(parts[1:], liveDir, mergedDir) + + cmd := exec.Command(name, args...) + cmd.Stdout = d.w + cmd.Stderr = d.w + + err := cmd.Run() + if err == nil { + return false, nil + } + + var exitErr *exec.ExitError + if errors.As(err, &exitErr) { + if exitErr.ExitCode() == 1 { + return true, nil + } + + return false, fmt.Errorf("external diff exited with code %d", exitErr.ExitCode()) + } + + return false, fmt.Errorf("failed to run external diff %q: %w", name, err) +} diff --git a/client/pkg/execdiff/execdiff_test.go b/client/pkg/execdiff/execdiff_test.go new file mode 100644 index 000000000..c54ba2210 --- /dev/null +++ b/client/pkg/execdiff/execdiff_test.go @@ -0,0 +1,271 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/. + +package execdiff_test + +import ( + "bytes" + "os" + "path/filepath" + "runtime" + "strings" + "testing" + + "github.com/fatih/color" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/siderolabs/omni/client/pkg/execdiff" +) + +func init() { + // Disable colorization to keep golden-string comparisons stable. + color.NoColor = true +} + +func TestSanitizeFilename(t *testing.T) { + t.Parallel() + + for _, tc := range []struct { + name string + parts []string + want string + }{ + {"simple", []string{"Cluster", "prod"}, "Cluster-prod"}, + {"slash in id", []string{"Config", "prod/access-policies"}, "Config-prod_access-policies"}, + {"backslash in id", []string{"Config", `prod\bad`}, "Config-prod_bad"}, + {"parent traversal", []string{"Config", ".."}, "Config-__"}, + {"nested traversal", []string{"Config", "a/../b"}, "Config-a____b"}, + {"empty part", []string{"Config", ""}, "Config-_"}, + {"null byte", []string{"Config", "a\x00b"}, "Config-a_b"}, + {"colon", []string{"Config", "ns:name"}, "Config-ns_name"}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + assert.Equal(t, tc.want, execdiff.SanitizeFilename(tc.parts...)) + }) + } +} + +func TestBuiltinNoDiff(t *testing.T) { + t.Parallel() + + var buf bytes.Buffer + + d := execdiff.New(&buf) + + yaml := []byte("a: 1\nb: 2\n") + require.NoError(t, d.AddDiff("R(one)", "r-one.yaml", yaml, yaml)) + + hasDiff, err := d.Flush() + require.NoError(t, err) + assert.False(t, hasDiff) + assert.Empty(t, buf.String()) +} + +func TestBuiltinDiffRendered(t *testing.T) { + t.Parallel() + + var buf bytes.Buffer + + d := execdiff.New(&buf) + + oldYAML := []byte("a: 1\nb: 2\n") + newYAML := []byte("a: 1\nb: 3\n") + + require.NoError(t, d.AddDiff("R(one)", "r-one.yaml", oldYAML, newYAML)) + + hasDiff, err := d.Flush() + require.NoError(t, err) + assert.True(t, hasDiff) + + got := buf.String() + assert.Contains(t, got, "--- R(one)") + assert.Contains(t, got, "+++ R(one)") + assert.Contains(t, got, "-b: 2") + assert.Contains(t, got, "+b: 3") + assert.NotContains(t, got, "--- a\n+++ b\n", "generic diff header should be stripped") +} + +func TestBuiltinCreateAndDelete(t *testing.T) { + t.Parallel() + + var buf bytes.Buffer + + d := execdiff.New(&buf) + + require.NoError(t, d.AddDiff("R(new)", "r-new.yaml", nil, []byte("a: 1\n"))) + require.NoError(t, d.AddDiff("R(old)", "r-old.yaml", []byte("a: 1\n"), nil)) + + hasDiff, err := d.Flush() + require.NoError(t, err) + assert.True(t, hasDiff) + + got := buf.String() + assert.Contains(t, got, "--- R(new)") + assert.Contains(t, got, "+a: 1") + assert.Contains(t, got, "--- R(old)") + assert.Contains(t, got, "-a: 1") +} + +func TestAddDiffRejectsUnsafeFilenamesInExternalMode(t *testing.T) { + t.Setenv(execdiff.EnvExternalDiff, "diff -u") + + d := execdiff.New(&bytes.Buffer{}) + + for _, unsafe := range []string{ + "", + "../escape.yaml", + "sub/dir.yaml", + ".", + "..", + } { + t.Run(unsafe, func(t *testing.T) { + err := d.AddDiff("R", unsafe, []byte("a: 1"), []byte("a: 2")) + assert.Error(t, err, "expected rejection for %q", unsafe) + }) + } +} + +func TestAddDiffRejectsAbsolutePathsInExternalMode(t *testing.T) { + t.Setenv(execdiff.EnvExternalDiff, "diff -u") + + d := execdiff.New(&bytes.Buffer{}) + + abs := "/tmp/escape.yaml" + if runtime.GOOS == "windows" { + abs = `C:\tmp\escape.yaml` + } + + err := d.AddDiff("R", abs, []byte("a: 1"), []byte("a: 2")) + assert.Error(t, err) +} + +// TestExternalDiffExitCodes verifies the exit-code contract with a stub script +// that we generate at runtime: 0 => no diff, 1 => diff found, 2 => error. +func TestExternalDiffExitCodes(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("shell stub not supported on windows") + } + + stub := writeShellStub(t) + + for _, tc := range []struct { + name string + exit string + wantErr bool + want bool + }{ + {"no diff", "0", false, false}, + {"diff found", "1", false, true}, + {"error", "2", true, false}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Setenv(execdiff.EnvExternalDiff, stub+" "+tc.exit) + + var buf bytes.Buffer + + d := execdiff.New(&buf) + require.NoError(t, d.AddDiff("R(x)", "r-x.yaml", []byte("a: 1\n"), []byte("a: 2\n"))) + + hasDiff, err := d.Flush() + if tc.wantErr { + assert.Error(t, err) + + return + } + + require.NoError(t, err) + assert.Equal(t, tc.want, hasDiff) + }) + } +} + +func TestExternalDiffReceivesFiles(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("shell stub not supported on windows") + } + + stub := writeRecordingStub(t) + recordFile := stub + ".out" + + t.Setenv(execdiff.EnvExternalDiff, stub+" "+recordFile) + + var buf bytes.Buffer + + d := execdiff.New(&buf) + require.NoError(t, d.AddDiff("R(x)", "r-x.yaml", []byte("old-content"), []byte("new-content"))) + + // Exit 1 => diff found. runExternal translates that to hasDiff=true with no error. + hasDiff, err := d.Flush() + require.NoError(t, err) + assert.True(t, hasDiff) + + rec, readErr := os.ReadFile(recordFile) + require.NoError(t, readErr) + + recStr := string(rec) + assert.Contains(t, recStr, "old-content") + assert.Contains(t, recStr, "new-content") +} + +func TestExternalModeRejectsEmptyCommand(t *testing.T) { + t.Setenv(execdiff.EnvExternalDiff, " ") + + d := execdiff.New(&bytes.Buffer{}) + require.NoError(t, d.AddDiff("R(x)", "r-x.yaml", []byte("a"), []byte("b"))) + + _, err := d.Flush() + assert.Error(t, err) +} + +func TestFlushNoEntriesExternalMode(t *testing.T) { + t.Setenv(execdiff.EnvExternalDiff, "false-command") + + d := execdiff.New(&bytes.Buffer{}) + + hasDiff, err := d.Flush() + require.NoError(t, err) + assert.False(t, hasDiff) +} + +// writeShellStub writes a shell script that exits with the status code +// passed as its final argument. External diff receives live+merged dirs as +// its last two args; the test passes an extra exit-code arg before those, +// so $1 is the desired exit code. +func writeShellStub(t *testing.T) string { + t.Helper() + + dir := t.TempDir() + path := filepath.Join(dir, "stub.sh") + + body := "#!/bin/sh\nexit \"$1\"\n" + require.NoError(t, os.WriteFile(path, []byte(body), 0o755)) + + return path +} + +// writeRecordingStub writes a shell script that concatenates the contents of +// both temp dirs into the file passed as $1, then exits 1 (diff found). +func writeRecordingStub(t *testing.T) string { + t.Helper() + + dir := t.TempDir() + path := filepath.Join(dir, "record.sh") + + body := strings.Join([]string{ + "#!/bin/sh", + "out=\"$1\"", + "shift", + "live=\"$1\"", + "merged=\"$2\"", + "{ cat \"$live\"/* 2>/dev/null; echo ---; cat \"$merged\"/* 2>/dev/null; } > \"$out\"", + "exit 1", + }, "\n") + "\n" + + require.NoError(t, os.WriteFile(path, []byte(body), 0o755)) + + return path +} diff --git a/client/pkg/omnictl/apply.go b/client/pkg/omnictl/apply.go index 6a1d91101..7cecd6cc3 100644 --- a/client/pkg/omnictl/apply.go +++ b/client/pkg/omnictl/apply.go @@ -18,12 +18,14 @@ import ( "github.com/cosi-project/runtime/pkg/resource" "github.com/cosi-project/runtime/pkg/resource/protobuf" "github.com/cosi-project/runtime/pkg/state" - "github.com/sergi/go-diff/diffmatchpatch" + "github.com/fatih/color" "github.com/siderolabs/gen/ensure" "github.com/spf13/cobra" "go.yaml.in/yaml/v4" "github.com/siderolabs/omni/client/pkg/client" + "github.com/siderolabs/omni/client/pkg/diff" + "github.com/siderolabs/omni/client/pkg/execdiff" "github.com/siderolabs/omni/client/pkg/omnictl/internal/access" ) @@ -38,17 +40,52 @@ var applyCmd = &cobra.Command{ Short: "Create or update resource using YAML file or directory as an input", Long: `Create or update resources using YAML file(s) as input. - If a file is specified, only that file will be processed. - If a directory is specified, all YAML files (*.yaml, *.yml) in the directory - and its subdirectories will be processed recursively. Each file is processed - independently, similar to kubectl behavior.`, +If a file is specified, only that file will be processed. +If a directory is specified, all YAML files (*.yaml, *.yml) in the directory +and its subdirectories will be processed recursively. Each file is processed +independently, similar to kubectl behavior. + +When --dry-run is used, ` + execdiff.EnvExternalDiff + ` environment variable can be +used to select an external diff command. Users can use external commands with +params too, example: ` + execdiff.EnvExternalDiff + `="colordiff -N -u" + +By default, the built-in colorized unified diff is used. + +Exit status: 0 No differences were found. 1 Differences were found. >1 An error occurred.`, Args: cobra.NoArgs, - RunE: func(*cobra.Command, []string) error { + RunE: func(cmd *cobra.Command, _ []string) error { if applyCmdFlags.options.dryRun { applyCmdFlags.options.verbose = true + applyCmdFlags.options.differ = execdiff.New(os.Stdout) + } + + runErr := access.WithClient(applyConfigFiles) + + var hasDiff bool + + if applyCmdFlags.options.differ != nil { + flushed, flushErr := applyCmdFlags.options.differ.Flush() + hasDiff = flushed + + if flushErr != nil { + return errors.Join(runErr, flushErr) + } + } + + if runErr != nil { + return runErr } - return access.WithClient(applyConfigFiles) + if hasDiff { + // Don't surface ErrDifferencesFound as a user-facing error; it + // only carries the exit-code contract. + cmd.SilenceErrors = true + cmd.SilenceUsage = true + + return execdiff.ErrDifferencesFound + } + + return nil }, } @@ -167,18 +204,37 @@ func applyConfigFromBytes(ctx context.Context, client *client.Client, yamlRaw [] } type options struct { + differ *execdiff.Differ dryRun bool verbose bool } +func resourceLabel(res resource.Resource) string { + return fmt.Sprintf("%s(%s)", res.Metadata().Type(), res.Metadata().ID()) +} + +func resourceFilename(res resource.Resource) string { + return execdiff.SanitizeFilename(res.Metadata().Type(), res.Metadata().ID()) + ".yaml" +} + func createResource(ctx context.Context, st state.State, res resource.Resource, opts options) error { if opts.verbose { - out, err := marshalResource(res) + newYAML, err := marshalResource(res) if err != nil { return err } - fmt.Printf("Creating resource '%s'\n\n%s\n\n", res.Metadata().ID(), out) + if opts.differ != nil { + if err := opts.differ.AddDiff(resourceLabel(res), resourceFilename(res), nil, newYAML); err != nil { + return err + } + } else { + fmt.Printf("Creating resource '%s'\n", res.Metadata().ID()) + + if err := printUnifiedDiff(os.Stdout, resourceLabel(res), nil, newYAML); err != nil { + return err + } + } } if opts.dryRun { @@ -194,20 +250,27 @@ func createResource(ctx context.Context, st state.State, res resource.Resource, func updateResource(ctx context.Context, st state.State, got resource.Resource, res resource.Resource, opts options) error { if opts.verbose { - outGot, err := marshalResource(got) + oldYAML, err := marshalResource(got) if err != nil { return err } - outRes, err := marshalResource(res) + newYAML, err := marshalResource(res) if err != nil { return err } - dmp := diffmatchpatch.New() - diffs := dmp.DiffMain(outGot, outRes, false) + if opts.differ != nil { + if err := opts.differ.AddDiff(resourceLabel(res), resourceFilename(res), oldYAML, newYAML); err != nil { + return err + } + } else { + fmt.Printf("Updating resource '%s'\n", res.Metadata().ID()) - fmt.Printf("Updating resource '%s'\n\n%s\n\n", res.Metadata().ID(), dmp.DiffPrettyText(diffs)) + if err := printUnifiedDiff(os.Stdout, resourceLabel(res), oldYAML, newYAML); err != nil { + return err + } + } } if opts.dryRun { @@ -223,18 +286,59 @@ func updateResource(ctx context.Context, st state.State, got resource.Resource, return nil } -func marshalResource(res resource.Resource) (string, error) { +// printUnifiedDiff renders a colorized unified diff, matching the --dry-run +// differ output so verbose apply is consistent whether or not dry-run is set. +func printUnifiedDiff(w io.Writer, label string, oldYAML, newYAML []byte) error { + diffStr, err := diff.Compute(oldYAML, newYAML) + if err != nil { + return err + } + + diffStr, _ = strings.CutPrefix(diffStr, "--- a\n+++ b\n") + + if diffStr == "" { + return nil + } + + bold := color.New(color.Bold) + bold.Fprintf(w, "--- %s\n", label) //nolint:errcheck + bold.Fprintf(w, "+++ %s\n", label) //nolint:errcheck + + cyan := color.New(color.FgCyan) + red := color.New(color.FgRed) + green := color.New(color.FgGreen) + + for line := range strings.SplitSeq(diffStr, "\n") { + switch { + case strings.HasPrefix(line, "@@"): + cyan.Fprintln(w, line) //nolint:errcheck + case strings.HasPrefix(line, "-"): + red.Fprintln(w, line) //nolint:errcheck + case strings.HasPrefix(line, "+"): + green.Fprintln(w, line) //nolint:errcheck + case line == "": + default: + fmt.Fprintln(w, line) //nolint:errcheck + } + } + + fmt.Fprintln(w) //nolint:errcheck + + return nil +} + +func marshalResource(res resource.Resource) ([]byte, error) { yamlRes, err := resource.MarshalYAML(res) if err != nil { - return "", fmt.Errorf("failed to marshal resource '%s' '%s': %w", res.Metadata().ID(), res.Metadata().Type(), err) + return nil, fmt.Errorf("failed to marshal resource '%s' '%s': %w", res.Metadata().ID(), res.Metadata().Type(), err) } out, err := yaml.Marshal(yamlRes) if err != nil { - return "", fmt.Errorf("failed to marshal resource '%s' '%s': %w", res.Metadata().ID(), res.Metadata().Type(), err) + return nil, fmt.Errorf("failed to marshal resource '%s' '%s': %w", res.Metadata().ID(), res.Metadata().Type(), err) } - return string(out), nil + return out, nil } func init() { diff --git a/client/pkg/omnictl/cluster/template/diff.go b/client/pkg/omnictl/cluster/template/diff.go index 146142f5d..32d25928b 100644 --- a/client/pkg/omnictl/cluster/template/diff.go +++ b/client/pkg/omnictl/cluster/template/diff.go @@ -6,28 +6,60 @@ package template import ( "context" + "errors" "os" "github.com/spf13/cobra" "github.com/siderolabs/omni/client/pkg/client" + "github.com/siderolabs/omni/client/pkg/execdiff" "github.com/siderolabs/omni/client/pkg/omnictl/internal/access" "github.com/siderolabs/omni/client/pkg/template/operations" ) // diffCmd represents the template diff command. var diffCmd = &cobra.Command{ - Use: "diff", - Short: "Show diff in resources if the template is synced.", - Long: `Query existing resources for the cluster and compare them with the resources generated from the template. This command requires API access.`, + Use: "diff", + Short: "Show diff in resources if the template is synced.", + Long: `Query existing resources for the cluster and compare them with the resources generated from the template. This command requires API access. + +` + execdiff.EnvExternalDiff + ` environment variable can be used to select an +external diff command. Users can use external commands with params too, +example: ` + execdiff.EnvExternalDiff + `="colordiff -N -u" + +By default, the built-in colorized unified diff is used. + +Exit status: 0 No differences were found. 1 Differences were found. >1 An error occurred.`, Example: "", Args: cobra.NoArgs, - RunE: func(*cobra.Command, []string) error { - return access.WithClient(diff) + RunE: func(cmd *cobra.Command, _ []string) error { + differ := execdiff.New(os.Stdout) + + runErr := access.WithClient(func(ctx context.Context, c *client.Client, _ access.ServerInfo) error { + return diff(ctx, c, differ) + }) + + flushed, flushErr := differ.Flush() + if flushErr != nil { + return errors.Join(runErr, flushErr) + } + + if runErr != nil { + return runErr + } + + if flushed { + cmd.SilenceErrors = true + cmd.SilenceUsage = true + + return execdiff.ErrDifferencesFound + } + + return nil }, } -func diff(ctx context.Context, client *client.Client, _ access.ServerInfo) error { +func diff(ctx context.Context, client *client.Client, differ *execdiff.Differ) error { f, err := os.Open(cmdFlags.TemplatePath) if err != nil { return err @@ -35,7 +67,7 @@ func diff(ctx context.Context, client *client.Client, _ access.ServerInfo) error defer f.Close() //nolint:errcheck - return operations.DiffTemplate(ctx, f, os.Stdout, client.Omni().State(), resolvedRoot) + return operations.DiffTemplate(ctx, f, os.Stdout, client.Omni().State(), differ, resolvedRoot) } func init() { diff --git a/client/pkg/omnictl/cluster/template/sync.go b/client/pkg/omnictl/cluster/template/sync.go index 7ba5bbf4e..2fcb5d975 100644 --- a/client/pkg/omnictl/cluster/template/sync.go +++ b/client/pkg/omnictl/cluster/template/sync.go @@ -6,6 +6,7 @@ package template import ( "context" + "errors" "fmt" "io/fs" "os" @@ -15,6 +16,7 @@ import ( "github.com/spf13/cobra" "github.com/siderolabs/omni/client/pkg/client" + "github.com/siderolabs/omni/client/pkg/execdiff" "github.com/siderolabs/omni/client/pkg/omnictl/internal/access" "github.com/siderolabs/omni/client/pkg/template/operations" ) @@ -32,11 +34,49 @@ var syncCmd = &cobra.Command{ If a file is specified, only that file will be processed. If a directory is specified, all YAML files (*.yaml, *.yml) in the directory and its subdirectories will be processed recursively. Each template file is - processed independently, allowing management of multiple clusters.`, + processed independently, allowing management of multiple clusters. + + When --dry-run is used, ` + execdiff.EnvExternalDiff + ` environment variable can be + used to select an external diff command. Users can use external commands with + params too, example: ` + execdiff.EnvExternalDiff + `="colordiff -N -u" + + By default, the built-in colorized unified diff is used. + + Exit status: 0 No differences were found. 1 Differences were found. >1 An error occurred.`, Example: "", Args: cobra.NoArgs, - RunE: func(*cobra.Command, []string) error { - return access.WithClient(syncTemplateFiles) + RunE: func(cmd *cobra.Command, _ []string) error { + if syncCmdFlags.options.DryRun { + syncCmdFlags.options.Differ = execdiff.New(os.Stdout) + } + + runErr := access.WithClient(syncTemplateFiles) + + var hasDiff bool + + if syncCmdFlags.options.Differ != nil { + flushed, flushErr := syncCmdFlags.options.Differ.Flush() + hasDiff = flushed + + if flushErr != nil { + return errors.Join(runErr, flushErr) + } + } + + if runErr != nil { + return runErr + } + + if hasDiff { + // Don't surface ErrDifferencesFound as a user-facing error; it + // only carries the exit-code contract. + cmd.SilenceErrors = true + cmd.SilenceUsage = true + + return execdiff.ErrDifferencesFound + } + + return nil }, } diff --git a/client/pkg/template/operations/diff.go b/client/pkg/template/operations/diff.go index f8f2c737c..5d90f145b 100644 --- a/client/pkg/template/operations/diff.go +++ b/client/pkg/template/operations/diff.go @@ -12,12 +12,16 @@ import ( "github.com/cosi-project/runtime/pkg/state" + "github.com/siderolabs/omni/client/pkg/execdiff" "github.com/siderolabs/omni/client/pkg/template" - "github.com/siderolabs/omni/client/pkg/template/operations/internal/utils" ) // DiffTemplate outputs the diff between template resources and existing resources. -func DiffTemplate(ctx context.Context, templateReader io.Reader, output io.Writer, st state.State, root *os.Root) error { +// +// When differ is non-nil, it is used to render diffs (built-in colorized output +// or via an external diff tool when OMNI_EXTERNAL_DIFF is set). When differ is +// nil, the legacy direct-write rendering is used. +func DiffTemplate(ctx context.Context, templateReader io.Reader, output io.Writer, st state.State, differ *execdiff.Differ, root *os.Root) error { tmpl, err := template.Load(templateReader, template.WithRoot(root)) if err != nil { return fmt.Errorf("error loading template: %w", err) @@ -33,20 +37,20 @@ func DiffTemplate(ctx context.Context, templateReader io.Reader, output io.Write } for _, p := range syncResult.Update { - if err = utils.RenderDiff(output, p.Old, p.New); err != nil { + if err = renderDiff(output, differ, p.Old, p.New); err != nil { return err } } for _, r := range syncResult.Create { - if err = utils.RenderDiff(output, nil, r); err != nil { + if err = renderDiff(output, differ, nil, r); err != nil { return err } } for _, phase := range syncResult.Destroy { for _, r := range phase { - if err = utils.RenderDiff(output, r, nil); err != nil { + if err = renderDiff(output, differ, r, nil); err != nil { return err } } diff --git a/client/pkg/template/operations/sync.go b/client/pkg/template/operations/sync.go index e62919c71..b269729ca 100644 --- a/client/pkg/template/operations/sync.go +++ b/client/pkg/template/operations/sync.go @@ -14,6 +14,7 @@ import ( "github.com/cosi-project/runtime/pkg/state" "github.com/fatih/color" + "github.com/siderolabs/omni/client/pkg/execdiff" "github.com/siderolabs/omni/client/pkg/omni/resources" "github.com/siderolabs/omni/client/pkg/template" "github.com/siderolabs/omni/client/pkg/template/operations/internal/utils" @@ -21,6 +22,9 @@ import ( // SyncOptions contains options for SyncTemplate. type SyncOptions struct { + // Differ, when set, handles diff rendering (built-in or external tool). + Differ *execdiff.Differ + // DryRun indicates that no changes should be made to the cluster. DryRun bool @@ -69,7 +73,7 @@ func sync(ctx context.Context, syncResult *template.SyncResult, out io.Writer, s yellow.Fprintf(out, "* creating%s %s\n", dryRun, boldFunc(utils.Describe(r))) //nolint:errcheck if syncOptions.Verbose { - if err := utils.RenderDiff(out, nil, r); err != nil { + if err := renderDiff(out, syncOptions.Differ, nil, r); err != nil { return err } } @@ -87,7 +91,7 @@ func sync(ctx context.Context, syncResult *template.SyncResult, out io.Writer, s yellow.Fprintf(out, "* updating%s %s\n", dryRun, boldFunc(utils.Describe(p.New))) //nolint:errcheck if syncOptions.Verbose { - if err := utils.RenderDiff(os.Stdout, p.Old, p.New); err != nil { + if err := renderDiff(out, syncOptions.Differ, p.Old, p.New); err != nil { return err } } @@ -146,7 +150,7 @@ func syncDeleteResources(ctx context.Context, toDelete []resource.Resource, out yellow.Fprintf(out, "* tearing down%s %s\n", dryRun, boldFunc(utils.Describe(r))) //nolint:errcheck if syncOptions.Verbose { - if err := utils.RenderDiff(os.Stdout, r, nil); err != nil { + if err := renderDiff(out, syncOptions.Differ, r, nil); err != nil { return err } } @@ -199,3 +203,41 @@ func syncDeleteResources(ctx context.Context, toDelete []resource.Resource, out return nil } + +// renderDiff renders a diff between two resources. When a Differ is set, it +// delegates to the Differ (which may queue the diff for an external tool). +// Otherwise it falls back to the built-in colorized RenderDiff. +func renderDiff(w io.Writer, differ *execdiff.Differ, oldR, newR resource.Resource) error { + if differ == nil { + return utils.RenderDiff(w, oldR, newR) + } + + var ( + oldYAML, newYAML []byte + err error + ) + + if oldR != nil { + oldYAML, err = utils.MarshalResource(oldR) + if err != nil { + return err + } + } + + if newR != nil { + newYAML, err = utils.MarshalResource(newR) + if err != nil { + return err + } + } + + r := oldR + if r == nil { + r = newR + } + + label := utils.Describe(r) + filename := execdiff.SanitizeFilename(r.Metadata().Type(), r.Metadata().ID()) + ".yaml" + + return differ.AddDiff(label, filename, oldYAML, newYAML) +} diff --git a/cmd/omnictl/main.go b/cmd/omnictl/main.go index 1df8d6536..46e7907a1 100644 --- a/cmd/omnictl/main.go +++ b/cmd/omnictl/main.go @@ -7,8 +7,10 @@ package main import ( + "errors" "os" + "github.com/siderolabs/omni/client/pkg/execdiff" "github.com/siderolabs/omni/client/pkg/omnictl" "github.com/siderolabs/omni/client/pkg/version" internalversion "github.com/siderolabs/omni/internal/version" @@ -23,6 +25,15 @@ func main() { omnictl.RootCmd.Version = version.String() if err := omnictl.RootCmd.Execute(); err != nil { - os.Exit(1) + // Differences-found is a signaling error (dry-run showed a diff), + // not a runtime failure. Map it to exit 1 silently so the documented + // contract holds: 0 = no diff, 1 = diff found, >1 = error. + // Cobra prints all other errors itself (including flag-parse errors), + // so main does not print again to avoid duplicate "Error:" lines. + if errors.Is(err, execdiff.ErrDifferencesFound) { + os.Exit(1) + } + + os.Exit(2) } }