From 35ee02e16edfd2c64944ec2e3a52bca89eb9ef0c Mon Sep 17 00:00:00 2001 From: Michele Mancioppi Date: Fri, 6 Mar 2026 05:36:21 +0100 Subject: [PATCH] feat: add diff output to apply and update commands --- .chloggen/diff-output.yaml | 26 ++ README.md | 2 + docs/commands.md | 34 +- internal/apply/apply.go | 19 +- internal/apply/integration_test.go | 12 +- internal/asset/checkrule.go | 23 +- internal/asset/dashboard.go | 33 +- internal/asset/diff.go | 133 +++++++ internal/asset/diff_test.go | 336 ++++++++++++++++++ internal/asset/import.go | 2 + internal/asset/syntheticcheck.go | 34 +- internal/asset/view.go | 36 +- internal/checkrules/update.go | 24 +- internal/dashboards/update.go | 33 +- .../dashboards/update_integration_test.go | 18 +- internal/output/formatter_test.go | 1 + internal/syntheticchecks/update.go | 24 +- internal/views/update.go | 24 +- 18 files changed, 711 insertions(+), 103 deletions(-) create mode 100644 .chloggen/diff-output.yaml create mode 100644 internal/asset/diff.go create mode 100644 internal/asset/diff_test.go diff --git a/.chloggen/diff-output.yaml b/.chloggen/diff-output.yaml new file mode 100644 index 0000000..387f4e9 --- /dev/null +++ b/.chloggen/diff-output.yaml @@ -0,0 +1,26 @@ +# Use this changelog template to create an entry for release notes. + +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: enhancement + +# The name of the component, or a single word describing the area of concern (e.g. dashboards, config, apply) +component: output + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: The `update` and `apply` commands now show a unified diff of changes + +# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists. +issues: [66] + +# (Optional) One or more lines of additional information to render under the primary note. +# These lines will be padded with 2 spaces and then inserted directly into the document. +# Use pipe (|) for multiline entries. +subtext: + +# If your change doesn't affect end users or the exported elements of any package, +# you should instead start your pull request title with "chore" or use the "Skip Changelog" label. +# Optional: The change log or logs in which this entry should be included. +# e.g. '[user]' or '[user, api]' +# Include 'user' if the change is relevant to end users. +# Default: '[user]' +change_logs: [] diff --git a/README.md b/README.md index f84ae8e..b3ed0f8 100644 --- a/README.md +++ b/README.md @@ -343,6 +343,8 @@ The `list` and `get` commands for assets support multiple output formats via `-o - **`yaml`**: Full asset data in YAML format - **`csv`**: Comma-separated values with the same columns as `wide`, suitable for piping and automation +The `update` and `apply` commands show a unified diff of changes. + The `logs query`, `spans query`, and `traces get` commands support a different set of formats via `-o`: - **`table`** (default): Columnar output (`logs query` shows timestamp, severity, and body; `spans query` shows timestamp, duration, name, status, service, and trace ID; `traces get` shows a hierarchical span tree) diff --git a/docs/commands.md b/docs/commands.md index 18fddc6..d39b0f2 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -288,21 +288,42 @@ Aliases: `add` Update an existing asset from a YAML or JSON file. If the ID argument is omitted, the ID is extracted from the file content. +The output shows a unified diff of what changed. +The `--dry-run` flag shows the diff without applying the update. ```bash -dash0 dashboards update [id] -f [--dry-run] [-o ] +dash0 dashboards update [id] -f [--dry-run] ``` Examples: ```bash -# Update with explicit ID +# Update a dashboard from a file $ dash0 dashboards update -f dashboard.yaml -Dashboard "My Dashboard" updated successfully +--- Dashboard (before) ++++ Dashboard (after) +@@ -2,7 +2,7 @@ + spec: + display: +- name: Old Dashboard Name ++ name: New Dashboard Name -# Update using the ID from the file +# Preview changes without applying +$ dash0 dashboards update -f dashboard.yaml --dry-run +--- Dashboard (before) ++++ Dashboard (after) +@@ -2,7 +2,7 @@ + spec: + display: +- name: Old Dashboard Name ++ name: New Dashboard Name +``` + +When nothing changed: + +```bash $ dash0 dashboards update -f dashboard.yaml -Dashboard "My Dashboard" updated successfully +Dashboard "My Dashboard": no changes ``` ### `delete` @@ -350,6 +371,9 @@ dash0 apply -f [--dry-run] | `--file` | `-f` | Path to a YAML/JSON file, a directory, or `-` for stdin | | `--dry-run` | | Validate without applying | +For assets that are updated, a unified diff of the changes is shown. +Assets that are created show the standard creation message. + When a directory is specified, all `.yaml` and `.yml` files are discovered recursively. Hidden files and directories (starting with `.`) are skipped. All documents are validated before any are applied. diff --git a/internal/apply/apply.go b/internal/apply/apply.go index 35f886b..311d015 100644 --- a/internal/apply/apply.go +++ b/internal/apply/apply.go @@ -122,6 +122,8 @@ type applyResult struct { name string id string action applyAction + before any // asset state before update (nil for creates) + after any // asset state after update/create } func runApply(ctx context.Context, flags *applyFlags) error { @@ -191,7 +193,12 @@ func runApply(ctx context.Context, flags *applyFlags) error { displayKind := asset.KindDisplayName(r.kind) label := formatNameAndId(r.name, r.id) applied = append(applied, fmt.Sprintf("%s %s", displayKind, label)) - if fromDirectory { + + if r.action == actionUpdated && r.before != nil { + if err := asset.PrintDiff(os.Stdout, displayKind, r.name, r.before, r.after); err != nil { + return err + } + } else if fromDirectory { fmt.Printf("%s: %s %s %s\n", doc.filePath, displayKind, label, r.action) } else { fmt.Printf("%s %s %s\n", displayKind, label, r.action) @@ -550,7 +557,7 @@ func applyDocument(ctx context.Context, apiClient dash0api.Client, doc assetDocu AssetName: asset.ExtractDashboardDisplayName(&dashboard), }) } - return []applyResult{{kind: doc.kind, name: result.Name, id: result.ID, action: applyAction(result.Action)}}, nil + return []applyResult{{kind: doc.kind, name: result.Name, id: result.ID, action: applyAction(result.Action), before: result.Before, after: result.After}}, nil case "checkrule": var rule dash0api.PrometheusAlertRule @@ -564,7 +571,7 @@ func applyDocument(ctx context.Context, apiClient dash0api.Client, doc assetDocu AssetName: rule.Name, }) } - return []applyResult{{kind: doc.kind, name: result.Name, id: result.ID, action: applyAction(result.Action)}}, nil + return []applyResult{{kind: doc.kind, name: result.Name, id: result.ID, action: applyAction(result.Action), before: result.Before, after: result.After}}, nil case "prometheusrule": var promRule asset.PrometheusRule @@ -599,7 +606,7 @@ func applyDocument(ctx context.Context, apiClient dash0api.Client, doc assetDocu AssetName: check.Metadata.Name, }) } - return []applyResult{{kind: doc.kind, name: result.Name, id: result.ID, action: applyAction(result.Action)}}, nil + return []applyResult{{kind: doc.kind, name: result.Name, id: result.ID, action: applyAction(result.Action), before: result.Before, after: result.After}}, nil case "view": var view dash0api.ViewDefinition @@ -613,7 +620,7 @@ func applyDocument(ctx context.Context, apiClient dash0api.Client, doc assetDocu AssetName: view.Metadata.Name, }) } - return []applyResult{{kind: doc.kind, name: result.Name, id: result.ID, action: applyAction(result.Action)}}, nil + return []applyResult{{kind: doc.kind, name: result.Name, id: result.ID, action: applyAction(result.Action), before: result.Before, after: result.After}}, nil default: return nil, fmt.Errorf("unsupported kind: %s", doc.kind) @@ -633,6 +640,8 @@ func applyPrometheusRule(ctx context.Context, apiClient dash0api.Client, promRul name: r.Name, id: r.ID, action: applyAction(r.Action), + before: r.Before, + after: r.After, }) } diff --git a/internal/apply/integration_test.go b/internal/apply/integration_test.go index 4fa7bf5..4586030 100644 --- a/internal/apply/integration_test.go +++ b/internal/apply/integration_test.go @@ -97,8 +97,9 @@ expression: up == 0 }) require.NoError(t, cmdErr) + // Output is a diff; since GET and Import return the same fixture, expect "no changes" assert.Contains(t, output, "Check rule") - assert.Contains(t, output, "updated") + assert.Contains(t, output, "no changes") // Verify the import request body importReq := findImportRequest(server.Requests(), apiPathImportCheckRule) @@ -189,8 +190,9 @@ spec: }) require.NoError(t, cmdErr) + // Output is a diff; since GET and Import return the same fixture, expect "no changes" assert.Contains(t, output, "Dashboard") - assert.Contains(t, output, "updated") + assert.Contains(t, output, "no changes") // Verify the import request body has server-generated fields stripped importReq := findImportRequest(server.Requests(), apiPathImportDashboard) @@ -930,8 +932,9 @@ spec: }) require.NoError(t, cmdErr) + // Output is a diff; since GET and Import return the same fixture, expect "no changes" assert.Contains(t, output, "View") - assert.Contains(t, output, "updated") + assert.Contains(t, output, "no changes") // Verify the import request body has server-generated fields stripped importReq := findImportRequest(server.Requests(), apiPathImportView) @@ -989,8 +992,9 @@ spec: }) require.NoError(t, cmdErr) + // Output is a diff; since GET and Import return the same fixture, expect "no changes" assert.Contains(t, output, "Synthetic check") - assert.Contains(t, output, "updated") + assert.Contains(t, output, "no changes") // Verify the import request body has server-generated fields stripped importReq := findImportRequest(server.Requests(), apiPathImportSyntheticCheck) diff --git a/internal/asset/checkrule.go b/internal/asset/checkrule.go index 134239d..0ed4c0b 100644 --- a/internal/asset/checkrule.go +++ b/internal/asset/checkrule.go @@ -6,22 +6,29 @@ import ( dash0api "github.com/dash0hq/dash0-api-client-go" ) +// StripCheckRuleServerFields removes server-generated fields from a check rule +// definition. Used by both Import (to avoid sending rejected fields to the API) +// and diff rendering (to suppress noise). +func StripCheckRuleServerFields(r *dash0api.PrometheusAlertRule) { + r.Dataset = nil + if r.Labels != nil { + delete(*r.Labels, "dash0.com/origin") + } +} + // ImportCheckRule checks existence by rule ID, strips the ID when the asset // is not found, and calls the import API. func ImportCheckRule(ctx context.Context, apiClient dash0api.Client, rule *dash0api.PrometheusAlertRule, dataset *string) (ImportResult, error) { - // Strip server-managed fields — the import API manages these and rejects - // requests that include them (e.g. dataset). - rule.Dataset = nil - if rule.Labels != nil { - delete(*rule.Labels, "dash0.com/origin") - } + StripCheckRuleServerFields(rule) // Check if check rule exists action := ActionCreated + var before any if rule.Id != nil && *rule.Id != "" { - _, err := apiClient.GetCheckRule(ctx, *rule.Id, dataset) + existing, err := apiClient.GetCheckRule(ctx, *rule.Id, dataset) if err == nil { action = ActionUpdated + before = existing } else { // Asset not found — strip ID so the API creates a fresh asset. rule.Id = nil @@ -37,7 +44,7 @@ func ImportCheckRule(ctx context.Context, apiClient dash0api.Client, rule *dash0 if result.Id != nil { id = *result.Id } - return ImportResult{Name: result.Name, ID: id, Action: action}, nil + return ImportResult{Name: result.Name, ID: id, Action: action, Before: before, After: result}, nil } // ExtractCheckRuleID extracts the ID from a check rule definition. diff --git a/internal/asset/dashboard.go b/internal/asset/dashboard.go index bdfd22d..8a41796 100644 --- a/internal/asset/dashboard.go +++ b/internal/asset/dashboard.go @@ -7,35 +7,38 @@ import ( "github.com/google/uuid" ) +// StripDashboardServerFields removes server-generated metadata fields from a +// dashboard definition. Used by both Import (to avoid sending stale values to +// the API) and diff rendering (to suppress noise like timestamps and version). +func StripDashboardServerFields(d *dash0api.DashboardDefinition) { + d.Metadata.Annotations = nil + d.Metadata.CreatedAt = nil + d.Metadata.UpdatedAt = nil + d.Metadata.Version = nil + if d.Metadata.Dash0Extensions != nil { + d.Metadata.Dash0Extensions.Dataset = nil + } +} + // ImportDashboard checks existence by dash0Extensions.id, // strips server-generated fields when the asset is not found, and calls the // import API. The import API uses dash0Extensions.id as the upsert key; when // the input has no id, a fresh UUID is generated so re-applying the exported // YAML updates the dashboard instead of creating a duplicate. func ImportDashboard(ctx context.Context, apiClient dash0api.Client, dashboard *dash0api.DashboardDefinition, dataset *string) (ImportResult, error) { - // Use dash0Extensions.id for the existence check — this is the upsert key - // used by the import API. Note: metadata.name for dashboards is the display - // name, not a UUID, so it cannot be used for lookups. - - // Strip server-generated metadata fields — the import API manages these - // and rejects requests that include stale values (e.g. an outdated version). - dashboard.Metadata.Annotations = nil - dashboard.Metadata.CreatedAt = nil - dashboard.Metadata.UpdatedAt = nil - dashboard.Metadata.Version = nil - if dashboard.Metadata.Dash0Extensions != nil { - dashboard.Metadata.Dash0Extensions.Dataset = nil - } + StripDashboardServerFields(dashboard) action := ActionCreated + var before any id := "" if dashboard.Metadata.Dash0Extensions != nil && dashboard.Metadata.Dash0Extensions.Id != nil { id = *dashboard.Metadata.Dash0Extensions.Id } if id != "" { - _, err := apiClient.GetDashboard(ctx, id, dataset) + existing, err := apiClient.GetDashboard(ctx, id, dataset) if err == nil { action = ActionUpdated + before = existing } else { // Asset not found — strip the ID so the API creates a fresh asset // instead of colliding with a soft-deleted record. @@ -64,7 +67,7 @@ func ImportDashboard(ctx context.Context, apiClient dash0api.Client, dashboard * if name == "" { name = result.Metadata.Name } - return ImportResult{Name: name, ID: id, Action: action}, nil + return ImportResult{Name: name, ID: id, Action: action, Before: before, After: result}, nil } // ExtractDashboardID extracts the ID from a dashboard definition. diff --git a/internal/asset/diff.go b/internal/asset/diff.go new file mode 100644 index 0000000..76c7544 --- /dev/null +++ b/internal/asset/diff.go @@ -0,0 +1,133 @@ +package asset + +import ( + "fmt" + "io" + "strings" + + dash0api "github.com/dash0hq/dash0-api-client-go" + "github.com/fatih/color" + "github.com/pmezard/go-difflib/difflib" + sigsyaml "sigs.k8s.io/yaml" +) + +// marshalForDiff deep-copies a typed asset, strips server-generated fields via +// the per-type Strip*ServerFields functions, and marshals the result to YAML. +func marshalForDiff(asset any) (string, error) { + jsonBytes, err := sigsyaml.Marshal(asset) + if err != nil { + return "", fmt.Errorf("failed to marshal asset: %w", err) + } + + var stripped any + switch asset.(type) { + case *dash0api.DashboardDefinition: + var d dash0api.DashboardDefinition + if err := sigsyaml.Unmarshal(jsonBytes, &d); err != nil { + return "", fmt.Errorf("failed to unmarshal dashboard: %w", err) + } + StripDashboardServerFields(&d) + stripped = &d + case *dash0api.PrometheusAlertRule: + var r dash0api.PrometheusAlertRule + if err := sigsyaml.Unmarshal(jsonBytes, &r); err != nil { + return "", fmt.Errorf("failed to unmarshal check rule: %w", err) + } + StripCheckRuleServerFields(&r) + stripped = &r + case *dash0api.ViewDefinition: + var v dash0api.ViewDefinition + if err := sigsyaml.Unmarshal(jsonBytes, &v); err != nil { + return "", fmt.Errorf("failed to unmarshal view: %w", err) + } + StripViewServerFields(&v) + stripped = &v + case *dash0api.SyntheticCheckDefinition: + var c dash0api.SyntheticCheckDefinition + if err := sigsyaml.Unmarshal(jsonBytes, &c); err != nil { + return "", fmt.Errorf("failed to unmarshal synthetic check: %w", err) + } + StripSyntheticCheckServerFields(&c) + stripped = &c + default: + stripped = asset + } + + out, err := sigsyaml.Marshal(stripped) + if err != nil { + return "", fmt.Errorf("failed to marshal asset for diff: %w", err) + } + return string(out), nil +} + +// PrintDiff computes a unified diff between the before and after states of an +// asset and writes it to w. If there are no changes, a "no changes" message is +// printed instead. +func PrintDiff(w io.Writer, displayKind, name string, before, after any) error { + beforeYAML, err := marshalForDiff(before) + if err != nil { + return fmt.Errorf("failed to marshal before state: %w", err) + } + + afterYAML, err := marshalForDiff(after) + if err != nil { + return fmt.Errorf("failed to marshal after state: %w", err) + } + + diff := difflib.UnifiedDiff{ + A: difflib.SplitLines(beforeYAML), + B: difflib.SplitLines(afterYAML), + FromFile: fmt.Sprintf("%s (before)", displayKind), + ToFile: fmt.Sprintf("%s (after)", displayKind), + Context: 3, + } + + text, err := difflib.GetUnifiedDiffString(diff) + if err != nil { + return fmt.Errorf("failed to compute diff: %w", err) + } + + if text == "" { + fmt.Fprintf(w, "%s %q: no changes\n", displayKind, name) + return nil + } + + if color.NoColor { + _, err := io.WriteString(w, text) + return err + } + + return writeColorizedDiff(w, text) +} + +var ( + colorRed = color.New(color.FgRed) + colorGreen = color.New(color.FgGreen) + colorCyan = color.New(color.FgCyan) + colorBold = color.New(color.Bold) +) + +func writeColorizedDiff(w io.Writer, text string) error { + for _, line := range strings.Split(text, "\n") { + if line == "" { + continue + } + var err error + switch { + case strings.HasPrefix(line, "---"), strings.HasPrefix(line, "+++"): + _, err = colorBold.Fprintln(w, line) + case strings.HasPrefix(line, "@@"): + _, err = colorCyan.Fprintln(w, line) + case strings.HasPrefix(line, "-"): + _, err = colorRed.Fprintln(w, line) + case strings.HasPrefix(line, "+"): + _, err = colorGreen.Fprintln(w, line) + default: + _, err = fmt.Fprintln(w, line) + } + if err != nil { + return err + } + } + return nil +} diff --git a/internal/asset/diff_test.go b/internal/asset/diff_test.go new file mode 100644 index 0000000..fb07e76 --- /dev/null +++ b/internal/asset/diff_test.go @@ -0,0 +1,336 @@ +package asset + +import ( + "bytes" + "testing" + "time" + + dash0api "github.com/dash0hq/dash0-api-client-go" + "github.com/fatih/color" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestPrintDiff_NoChanges(t *testing.T) { + color.NoColor = true + defer func() { color.NoColor = false }() + + dashboard := &dash0api.DashboardDefinition{ + Kind: "Dashboard", + Metadata: dash0api.DashboardMetadata{ + Name: "test", + }, + Spec: map[string]interface{}{ + "display": map[string]interface{}{"name": "My Dashboard"}, + }, + } + same := &dash0api.DashboardDefinition{ + Kind: "Dashboard", + Metadata: dash0api.DashboardMetadata{ + Name: "test", + }, + Spec: map[string]interface{}{ + "display": map[string]interface{}{"name": "My Dashboard"}, + }, + } + + var buf bytes.Buffer + err := PrintDiff(&buf, "Dashboard", "My Dashboard", dashboard, same) + require.NoError(t, err) + assert.Contains(t, buf.String(), `Dashboard "My Dashboard": no changes`) +} + +func TestPrintDiff_WithChanges(t *testing.T) { + color.NoColor = true + defer func() { color.NoColor = false }() + + before := &dash0api.DashboardDefinition{ + Kind: "Dashboard", + Metadata: dash0api.DashboardMetadata{ + Name: "test", + }, + Spec: map[string]interface{}{ + "display": map[string]interface{}{"name": "Old Name"}, + }, + } + after := &dash0api.DashboardDefinition{ + Kind: "Dashboard", + Metadata: dash0api.DashboardMetadata{ + Name: "test", + }, + Spec: map[string]interface{}{ + "display": map[string]interface{}{"name": "New Name"}, + }, + } + + var buf bytes.Buffer + err := PrintDiff(&buf, "Dashboard", "New Name", before, after) + require.NoError(t, err) + + output := buf.String() + assert.Contains(t, output, "--- Dashboard (before)") + assert.Contains(t, output, "+++ Dashboard (after)") + assert.Contains(t, output, "@@") + assert.Contains(t, output, "- name: Old Name") + assert.Contains(t, output, "+ name: New Name") +} + +func TestPrintDiff_StripsServerFields_Dashboard(t *testing.T) { + color.NoColor = true + defer func() { color.NoColor = false }() + + t1 := time.Date(2024, 1, 1, 0, 0, 0, 0, time.UTC) + t2 := time.Date(2024, 1, 2, 0, 0, 0, 0, time.UTC) + v1 := int64(1) + v2 := int64(2) + ds1 := dash0api.Dataset("default") + ds2 := dash0api.Dataset("other") + id := "abc" + + before := &dash0api.DashboardDefinition{ + Kind: "Dashboard", + Metadata: dash0api.DashboardMetadata{ + Name: "test", + CreatedAt: &t1, + UpdatedAt: &t1, + Version: &v1, + Dash0Extensions: &dash0api.DashboardMetadataExtensions{ + Dataset: &ds1, + Id: &id, + }, + }, + Spec: map[string]interface{}{"display": map[string]interface{}{"name": "Dashboard"}}, + } + after := &dash0api.DashboardDefinition{ + Kind: "Dashboard", + Metadata: dash0api.DashboardMetadata{ + Name: "test", + CreatedAt: &t1, + UpdatedAt: &t2, + Version: &v2, + Dash0Extensions: &dash0api.DashboardMetadataExtensions{ + Dataset: &ds2, + Id: &id, + }, + }, + Spec: map[string]interface{}{"display": map[string]interface{}{"name": "Dashboard"}}, + } + + var buf bytes.Buffer + err := PrintDiff(&buf, "Dashboard", "Dashboard", before, after) + require.NoError(t, err) + assert.Contains(t, buf.String(), `Dashboard "Dashboard": no changes`) +} + +func TestPrintDiff_StripsServerFields_CheckRule(t *testing.T) { + color.NoColor = true + defer func() { color.NoColor = false }() + + ds1 := dash0api.Dataset("default") + ds2 := dash0api.Dataset("other") + + before := &dash0api.PrometheusAlertRule{ + Name: "High Error Rate", + Expression: "rate(errors[5m]) > 0.1", + Dataset: &ds1, + Labels: &map[string]string{"dash0.com/origin": "cli"}, + } + after := &dash0api.PrometheusAlertRule{ + Name: "High Error Rate", + Expression: "rate(errors[5m]) > 0.1", + Dataset: &ds2, + Labels: &map[string]string{"dash0.com/origin": "api"}, + } + + var buf bytes.Buffer + err := PrintDiff(&buf, "Check rule", "High Error Rate", before, after) + require.NoError(t, err) + assert.Contains(t, buf.String(), `Check rule "High Error Rate": no changes`) +} + +func TestPrintDiff_StripsServerFields_View(t *testing.T) { + color.NoColor = true + defer func() { color.NoColor = false }() + + v1 := "1" + v2 := "2" + id := "view-id" + + before := &dash0api.ViewDefinition{ + Kind: "View", + Metadata: dash0api.ViewMetadata{ + Name: "Error Logs", + Labels: &dash0api.ViewLabels{ + Dash0Comid: &id, + Dash0Comversion: &v1, + }, + }, + Spec: dash0api.ViewSpec{ + Display: dash0api.ViewDisplay{}, + }, + } + after := &dash0api.ViewDefinition{ + Kind: "View", + Metadata: dash0api.ViewMetadata{ + Name: "Error Logs", + Labels: &dash0api.ViewLabels{ + Dash0Comid: &id, + Dash0Comversion: &v2, + }, + }, + Spec: dash0api.ViewSpec{ + Display: dash0api.ViewDisplay{}, + }, + } + + var buf bytes.Buffer + err := PrintDiff(&buf, "View", "Error Logs", before, after) + require.NoError(t, err) + assert.Contains(t, buf.String(), `View "Error Logs": no changes`) +} + +func TestPrintDiff_ColorOutput(t *testing.T) { + color.NoColor = false + + before := &dash0api.DashboardDefinition{ + Kind: "Dashboard", + Metadata: dash0api.DashboardMetadata{Name: "test"}, + Spec: map[string]interface{}{"display": map[string]interface{}{"name": "old"}}, + } + after := &dash0api.DashboardDefinition{ + Kind: "Dashboard", + Metadata: dash0api.DashboardMetadata{Name: "test"}, + Spec: map[string]interface{}{"display": map[string]interface{}{"name": "new"}}, + } + + var buf bytes.Buffer + err := PrintDiff(&buf, "Dashboard", "Test", before, after) + require.NoError(t, err) + + output := buf.String() + // Color output includes ANSI escape codes + assert.Contains(t, output, "\033[") +} + +func TestStripDashboardServerFields(t *testing.T) { + ts := time.Now() + version := int64(3) + ds := dash0api.Dataset("default") + id := "keep-id" + + d := &dash0api.DashboardDefinition{ + Kind: "Dashboard", + Metadata: dash0api.DashboardMetadata{ + Name: "keep", + CreatedAt: &ts, + UpdatedAt: &ts, + Version: &version, + Annotations: &dash0api.DashboardAnnotations{}, + Dash0Extensions: &dash0api.DashboardMetadataExtensions{ + Id: &id, + Dataset: &ds, + }, + }, + } + + StripDashboardServerFields(d) + + assert.Equal(t, "keep", d.Metadata.Name) + assert.Nil(t, d.Metadata.CreatedAt) + assert.Nil(t, d.Metadata.UpdatedAt) + assert.Nil(t, d.Metadata.Version) + assert.Nil(t, d.Metadata.Annotations) + assert.Equal(t, &id, d.Metadata.Dash0Extensions.Id) + assert.Nil(t, d.Metadata.Dash0Extensions.Dataset) +} + +func TestStripCheckRuleServerFields(t *testing.T) { + ds := dash0api.Dataset("default") + id := "keep-id" + + r := &dash0api.PrometheusAlertRule{ + Name: "keep", + Expression: "up == 0", + Id: &id, + Dataset: &ds, + Labels: &map[string]string{"dash0.com/origin": "remove", "severity": "keep"}, + } + + StripCheckRuleServerFields(r) + + assert.Equal(t, "keep", r.Name) + assert.Equal(t, &id, r.Id) + assert.Nil(t, r.Dataset) + assert.NotContains(t, *r.Labels, "dash0.com/origin") + assert.Equal(t, "keep", (*r.Labels)["severity"]) +} + +func TestStripViewServerFields(t *testing.T) { + id := "keep-id" + version := "3" + dataset := "default" + origin := "cli" + + v := &dash0api.ViewDefinition{ + Kind: "View", + Metadata: dash0api.ViewMetadata{ + Name: "keep", + Annotations: &dash0api.ViewAnnotations{}, + Labels: &dash0api.ViewLabels{ + Dash0Comid: &id, + Dash0Comversion: &version, + Dash0Comdataset: &dataset, + Dash0Comorigin: &origin, + }, + }, + Spec: dash0api.ViewSpec{ + Display: dash0api.ViewDisplay{}, + }, + } + + StripViewServerFields(v) + + assert.Equal(t, "keep", v.Metadata.Name) + assert.Nil(t, v.Metadata.Annotations) + assert.Equal(t, &id, v.Metadata.Labels.Dash0Comid) + assert.Nil(t, v.Metadata.Labels.Dash0Comversion) + assert.Nil(t, v.Metadata.Labels.Dash0Comdataset) + assert.Nil(t, v.Metadata.Labels.Dash0Comorigin) + assert.Nil(t, v.Metadata.Labels.Dash0Comsource) +} + +func TestStripSyntheticCheckServerFields(t *testing.T) { + id := "keep-id" + version := "3" + dataset := "default" + origin := "cli" + custom := map[string]string{"env": "prod"} + + c := &dash0api.SyntheticCheckDefinition{ + Kind: "SyntheticCheck", + Metadata: dash0api.SyntheticCheckMetadata{ + Name: "keep", + Annotations: &dash0api.SyntheticCheckAnnotations{}, + Labels: &dash0api.SyntheticCheckLabels{ + Dash0Comid: &id, + Dash0Comversion: &version, + Dash0Comdataset: &dataset, + Dash0Comorigin: &origin, + Custom: &custom, + }, + }, + Spec: dash0api.SyntheticCheckSpec{ + Enabled: true, + }, + } + + StripSyntheticCheckServerFields(c) + + assert.Equal(t, "keep", c.Metadata.Name) + assert.Nil(t, c.Metadata.Annotations) + assert.Equal(t, &id, c.Metadata.Labels.Dash0Comid) + assert.Nil(t, c.Metadata.Labels.Dash0Comversion) + assert.Nil(t, c.Metadata.Labels.Dash0Comdataset) + assert.Nil(t, c.Metadata.Labels.Dash0Comorigin) + assert.Nil(t, c.Metadata.Labels.Custom) +} diff --git a/internal/asset/import.go b/internal/asset/import.go index 7af865a..b75f12e 100644 --- a/internal/asset/import.go +++ b/internal/asset/import.go @@ -13,4 +13,6 @@ type ImportResult struct { Name string ID string Action ImportAction + Before any // asset state before update (nil for creates) + After any // asset state after update/create } diff --git a/internal/asset/syntheticcheck.go b/internal/asset/syntheticcheck.go index a6ad4a6..c02ccb8 100644 --- a/internal/asset/syntheticcheck.go +++ b/internal/asset/syntheticcheck.go @@ -6,28 +6,34 @@ import ( dash0api "github.com/dash0hq/dash0-api-client-go" ) +// StripSyntheticCheckServerFields removes server-generated fields from a +// synthetic check definition. Used by both Import (to avoid sending rejected +// fields to the API) and diff rendering (to suppress noise). +func StripSyntheticCheckServerFields(c *dash0api.SyntheticCheckDefinition) { + c.Metadata.Annotations = nil + if c.Metadata.Labels == nil { + c.Metadata.Labels = &dash0api.SyntheticCheckLabels{} + } + c.Metadata.Labels.Dash0Comversion = nil + c.Metadata.Labels.Custom = nil + c.Metadata.Labels.Dash0Comdataset = nil + c.Metadata.Labels.Dash0Comorigin = nil + c.Spec.Permissions = nil +} + // ImportSyntheticCheck checks existence by Dash0Comid, strips server-generated // fields, and calls the import API. func ImportSyntheticCheck(ctx context.Context, apiClient dash0api.Client, check *dash0api.SyntheticCheckDefinition, dataset *string) (ImportResult, error) { - // Strip server-generated metadata fields — the import API manages these. - check.Metadata.Annotations = nil - if check.Metadata.Labels == nil { - check.Metadata.Labels = &dash0api.SyntheticCheckLabels{} - } - check.Metadata.Labels.Dash0Comversion = nil - check.Metadata.Labels.Custom = nil - check.Metadata.Labels.Dash0Comdataset = nil - check.Spec.Permissions = nil - // Strip origin — the import API manages it server-side and rejects any - // value submitted in the request body. - check.Metadata.Labels.Dash0Comorigin = nil + StripSyntheticCheckServerFields(check) // Check if synthetic check exists using its ID action := ActionCreated + var before any if check.Metadata.Labels.Dash0Comid != nil && *check.Metadata.Labels.Dash0Comid != "" { - _, err := apiClient.GetSyntheticCheck(ctx, *check.Metadata.Labels.Dash0Comid, dataset) + existing, err := apiClient.GetSyntheticCheck(ctx, *check.Metadata.Labels.Dash0Comid, dataset) if err == nil { action = ActionUpdated + before = existing } else { // Asset not found — strip the ID so the API creates a fresh asset. check.Metadata.Labels.Dash0Comid = nil @@ -43,7 +49,7 @@ func ImportSyntheticCheck(ctx context.Context, apiClient dash0api.Client, check if result.Metadata.Labels != nil && result.Metadata.Labels.Dash0Comid != nil { id = *result.Metadata.Labels.Dash0Comid } - return ImportResult{Name: ExtractSyntheticCheckName(result), ID: id, Action: action}, nil + return ImportResult{Name: ExtractSyntheticCheckName(result), ID: id, Action: action, Before: before, After: result}, nil } // ExtractSyntheticCheckID extracts the ID from a synthetic check definition. diff --git a/internal/asset/view.go b/internal/asset/view.go index 6e741d0..9502491 100644 --- a/internal/asset/view.go +++ b/internal/asset/view.go @@ -6,30 +6,34 @@ import ( dash0api "github.com/dash0hq/dash0-api-client-go" ) +// StripViewServerFields removes server-generated fields from a view definition. +// Used by both Import (to avoid sending rejected fields to the API) and diff +// rendering (to suppress noise). +func StripViewServerFields(v *dash0api.ViewDefinition) { + v.Metadata.Annotations = nil + if v.Metadata.Labels == nil { + v.Metadata.Labels = &dash0api.ViewLabels{} + } + v.Metadata.Labels.Dash0Comversion = nil + v.Metadata.Labels.Dash0Comsource = nil + v.Metadata.Labels.Dash0Comdataset = nil + v.Metadata.Labels.Dash0Comorigin = nil + v.Spec.Permissions = nil +} + // ImportView checks existence by Dash0Comid, strips server-generated fields, // and calls the import API. func ImportView(ctx context.Context, apiClient dash0api.Client, view *dash0api.ViewDefinition, dataset *string) (ImportResult, error) { - // Strip server-generated metadata and user-specific fields — the import - // API manages these and rejects requests that include them. - view.Metadata.Annotations = nil - if view.Metadata.Labels == nil { - view.Metadata.Labels = &dash0api.ViewLabels{} - } - view.Metadata.Labels.Dash0Comversion = nil - view.Metadata.Labels.Dash0Comsource = nil - view.Metadata.Labels.Dash0Comdataset = nil - view.Spec.Permissions = nil - - // Strip origin — the import API manages it server-side and rejects any - // value submitted in the request body. - view.Metadata.Labels.Dash0Comorigin = nil + StripViewServerFields(view) // Check if view exists using its ID action := ActionCreated + var before any if view.Metadata.Labels.Dash0Comid != nil && *view.Metadata.Labels.Dash0Comid != "" { - _, err := apiClient.GetView(ctx, *view.Metadata.Labels.Dash0Comid, dataset) + existing, err := apiClient.GetView(ctx, *view.Metadata.Labels.Dash0Comid, dataset) if err == nil { action = ActionUpdated + before = existing } else { // Asset not found — strip the ID so the API creates a fresh asset. view.Metadata.Labels.Dash0Comid = nil @@ -45,7 +49,7 @@ func ImportView(ctx context.Context, apiClient dash0api.Client, view *dash0api.V if result.Metadata.Labels != nil && result.Metadata.Labels.Dash0Comid != nil { id = *result.Metadata.Labels.Dash0Comid } - return ImportResult{Name: ExtractViewName(result), ID: id, Action: action}, nil + return ImportResult{Name: ExtractViewName(result), ID: id, Action: action, Before: before, After: result}, nil } // ExtractViewID extracts the ID from a view definition. diff --git a/internal/checkrules/update.go b/internal/checkrules/update.go index d672119..d5cb57d 100644 --- a/internal/checkrules/update.go +++ b/internal/checkrules/update.go @@ -61,17 +61,26 @@ func runUpdate(ctx context.Context, args []string, flags *asset.FileInputFlags) } } - if flags.DryRun { - fmt.Println("Dry run: check rule definition is valid") - return nil - } - apiClient, err := client.NewClientFromContext(ctx, flags.ApiUrl, flags.AuthToken) if err != nil { return err } - result, err := apiClient.UpdateCheckRule(ctx, id, &rule, client.ResolveDataset(ctx, flags.Dataset)) + dataset := client.ResolveDataset(ctx, flags.Dataset) + + before, err := apiClient.GetCheckRule(ctx, id, dataset) + if err != nil { + return client.HandleAPIError(err, client.ErrorContext{ + AssetType: "check rule", + AssetID: id, + }) + } + + if flags.DryRun { + return asset.PrintDiff(os.Stdout, "Check rule", rule.Name, before, &rule) + } + + result, err := apiClient.UpdateCheckRule(ctx, id, &rule, dataset) if err != nil { return client.HandleAPIError(err, client.ErrorContext{ AssetType: "check rule", @@ -80,6 +89,5 @@ func runUpdate(ctx context.Context, args []string, flags *asset.FileInputFlags) }) } - fmt.Printf("Check rule %q updated successfully\n", result.Name) - return nil + return asset.PrintDiff(os.Stdout, "Check rule", result.Name, before, result) } diff --git a/internal/dashboards/update.go b/internal/dashboards/update.go index 7236c69..e5ad302 100644 --- a/internal/dashboards/update.go +++ b/internal/dashboards/update.go @@ -61,17 +61,31 @@ func runUpdate(ctx context.Context, args []string, flags *asset.FileInputFlags) } } - if flags.DryRun { - fmt.Println("Dry run: dashboard definition is valid") - return nil - } - apiClient, err := client.NewClientFromContext(ctx, flags.ApiUrl, flags.AuthToken) if err != nil { return err } - result, err := apiClient.UpdateDashboard(ctx, id, &dashboard, client.ResolveDataset(ctx, flags.Dataset)) + dataset := client.ResolveDataset(ctx, flags.Dataset) + + // Fetch the current state before updating so we can show what changed. + before, err := apiClient.GetDashboard(ctx, id, dataset) + if err != nil { + return client.HandleAPIError(err, client.ErrorContext{ + AssetType: "dashboard", + AssetID: id, + }) + } + + if flags.DryRun { + displayName := asset.ExtractDashboardDisplayName(&dashboard) + if displayName == "" { + displayName = dashboard.Metadata.Name + } + return asset.PrintDiff(os.Stdout, "Dashboard", displayName, before, &dashboard) + } + + result, err := apiClient.UpdateDashboard(ctx, id, &dashboard, dataset) if err != nil { return client.HandleAPIError(err, client.ErrorContext{ AssetType: "dashboard", @@ -80,6 +94,9 @@ func runUpdate(ctx context.Context, args []string, flags *asset.FileInputFlags) }) } - fmt.Printf("Dashboard %q updated successfully\n", result.Metadata.Name) - return nil + displayName := asset.ExtractDashboardDisplayName(result) + if displayName == "" { + displayName = result.Metadata.Name + } + return asset.PrintDiff(os.Stdout, "Dashboard", displayName, before, result) } diff --git a/internal/dashboards/update_integration_test.go b/internal/dashboards/update_integration_test.go index ecc351b..eb1ba36 100644 --- a/internal/dashboards/update_integration_test.go +++ b/internal/dashboards/update_integration_test.go @@ -28,6 +28,11 @@ spec: require.NoError(t, err) server := testutil.NewMockServer(t, testutil.FixturesDir()) + server.On(http.MethodGet, "/api/dashboards/"+testDashboardID, testutil.MockResponse{ + StatusCode: http.StatusOK, + BodyFile: fixtureGetSuccess, + Validator: testutil.RequireHeaders, + }) server.On(http.MethodPut, "/api/dashboards/"+testDashboardID, testutil.MockResponse{ StatusCode: http.StatusOK, BodyFile: fixtureGetSuccess, @@ -43,8 +48,8 @@ spec: }) require.NoError(t, cmdErr) - assert.Contains(t, output, "Dashboard") - assert.Contains(t, output, "updated") + // Output is a diff; since GET and PUT return the same fixture, expect "no changes" + assert.Contains(t, output, "no changes") } func TestUpdateDashboard_WithIDFromFile(t *testing.T) { @@ -65,6 +70,11 @@ spec: require.NoError(t, err) server := testutil.NewMockServer(t, testutil.FixturesDir()) + server.On(http.MethodGet, "/api/dashboards/"+dashboardID, testutil.MockResponse{ + StatusCode: http.StatusOK, + BodyFile: fixtureGetSuccess, + Validator: testutil.RequireHeaders, + }) server.On(http.MethodPut, "/api/dashboards/"+dashboardID, testutil.MockResponse{ StatusCode: http.StatusOK, BodyFile: fixtureGetSuccess, @@ -80,8 +90,8 @@ spec: }) require.NoError(t, cmdErr) - assert.Contains(t, output, "Dashboard") - assert.Contains(t, output, "updated") + // Output is a diff; since GET and PUT return the same fixture, expect "no changes" + assert.Contains(t, output, "no changes") } func TestUpdateDashboard_IDMismatch(t *testing.T) { diff --git a/internal/output/formatter_test.go b/internal/output/formatter_test.go index 31ceef3..bfacc5e 100644 --- a/internal/output/formatter_test.go +++ b/internal/output/formatter_test.go @@ -25,6 +25,7 @@ func TestParseFormat(t *testing.T) { {"wide", FormatWide, false}, {"csv", FormatCSV, false}, {"CSV", FormatCSV, false}, + {"diff", "", true}, {"invalid", "", true}, } diff --git a/internal/syntheticchecks/update.go b/internal/syntheticchecks/update.go index 6ef59c1..f6b2a31 100644 --- a/internal/syntheticchecks/update.go +++ b/internal/syntheticchecks/update.go @@ -61,17 +61,26 @@ func runUpdate(ctx context.Context, args []string, flags *asset.FileInputFlags) } } - if flags.DryRun { - fmt.Println("Dry run: synthetic check definition is valid") - return nil - } - apiClient, err := client.NewClientFromContext(ctx, flags.ApiUrl, flags.AuthToken) if err != nil { return err } - result, err := apiClient.UpdateSyntheticCheck(ctx, id, &check, client.ResolveDataset(ctx, flags.Dataset)) + dataset := client.ResolveDataset(ctx, flags.Dataset) + + before, err := apiClient.GetSyntheticCheck(ctx, id, dataset) + if err != nil { + return client.HandleAPIError(err, client.ErrorContext{ + AssetType: "synthetic check", + AssetID: id, + }) + } + + if flags.DryRun { + return asset.PrintDiff(os.Stdout, "Synthetic check", check.Metadata.Name, before, &check) + } + + result, err := apiClient.UpdateSyntheticCheck(ctx, id, &check, dataset) if err != nil { return client.HandleAPIError(err, client.ErrorContext{ AssetType: "synthetic check", @@ -80,6 +89,5 @@ func runUpdate(ctx context.Context, args []string, flags *asset.FileInputFlags) }) } - fmt.Printf("Synthetic check %q updated successfully\n", asset.ExtractSyntheticCheckName(result)) - return nil + return asset.PrintDiff(os.Stdout, "Synthetic check", asset.ExtractSyntheticCheckName(result), before, result) } diff --git a/internal/views/update.go b/internal/views/update.go index 69cb0c3..70908c0 100644 --- a/internal/views/update.go +++ b/internal/views/update.go @@ -61,17 +61,26 @@ func runUpdate(ctx context.Context, args []string, flags *asset.FileInputFlags) } } - if flags.DryRun { - fmt.Println("Dry run: view definition is valid") - return nil - } - apiClient, err := client.NewClientFromContext(ctx, flags.ApiUrl, flags.AuthToken) if err != nil { return err } - result, err := apiClient.UpdateView(ctx, id, &view, client.ResolveDataset(ctx, flags.Dataset)) + dataset := client.ResolveDataset(ctx, flags.Dataset) + + before, err := apiClient.GetView(ctx, id, dataset) + if err != nil { + return client.HandleAPIError(err, client.ErrorContext{ + AssetType: "view", + AssetID: id, + }) + } + + if flags.DryRun { + return asset.PrintDiff(os.Stdout, "View", view.Metadata.Name, before, &view) + } + + result, err := apiClient.UpdateView(ctx, id, &view, dataset) if err != nil { return client.HandleAPIError(err, client.ErrorContext{ AssetType: "view", @@ -80,6 +89,5 @@ func runUpdate(ctx context.Context, args []string, flags *asset.FileInputFlags) }) } - fmt.Printf("View %q updated successfully\n", asset.ExtractViewName(result)) - return nil + return asset.PrintDiff(os.Stdout, "View", asset.ExtractViewName(result), before, result) }