feat: machine readable comp diff by jcogilvie · Pull Request #270 · crossplane-contrib/crossplane-diff

jcogilvie · 2026-03-27T04:07:45Z

Description of your changes

Add machine-readable output for comp diff.

I have:

Read and followed Crossplane's contribution process.
Run earthly -P +reviewable to ensure this PR is ready for review.
Added or updated unit tests.
Added or updated e2e tests.
Documented this change as needed.
~~- [ ] Followed the API promotion workflow if this PR introduces, removes, or promotes an API.~~

Need help with this checklist? See the cheat sheet.

Signed-off-by: Jonathan Ogilvie <jonathan.ogilvie@sumologic.com>

Copilot

Pull request overview

Adds machine-readable (JSON/YAML) output for crossplane-diff comp so composition diffs and their downstream XR impacts can be consumed programmatically (e.g., CI/CD).

Changes:

Introduces internal + JSON/YAML output types for composition diff results, including XR impact status and downstream changes.
Adds human-readable and structured composition diff renderers, plus unit tests covering JSON/YAML schema and formatting helpers.
Refactors the composition diff processor to build a structured result and delegate rendering via a new renderer factory based on --output.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
cmd/diff/renderer/structured_renderer.go	Adds internal comp-diff result model + JSON output schema types and conversion helpers.
cmd/diff/renderer/comp_diff_renderer.go	New human/structured comp diff renderers; structured renderer emits JSON/YAML.
cmd/diff/renderer/comp_diff_renderer_test.go	New tests for structured comp diff output + formatting helpers.
cmd/diff/diffprocessor/processor_config.go	Adds `CompDiffRenderer` factory and wires it to `--output` selection.
cmd/diff/diffprocessor/comp_processor.go	Refactors comp diff processing to build `CompDiffOutput` and render via renderer.
cmd/diff/diffprocessor/comp_processor_test.go	Updates tests to use constructor wiring and new method signatures.
README.md	Documents `--output json

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Signed-off-by: Jonathan Ogilvie <jonathan.ogilvie@sumologic.com>

Copilot

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 6 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-27T19:48:54Z

+// OutputError represents an error in structured output.
+// Used consistently by both XR diff and comp diff for machine-readable error handling.
+type OutputError struct {
+	ResourceID string `json:"resourceID,omitempty" yaml:"resourceId,omitempty"`


OutputError.ResourceID uses json:"resourceID" but yaml:"resourceId", and the repo docs (CLAUDE.md) describe resourceId. This makes structured output inconsistent across JSON/YAML and likely breaks consumers expecting a stable field name. Use a single canonical key (e.g., resourceId) for both JSON and YAML tags, and keep it consistent with the documented schema.

Suggested change

ResourceID string `json:"resourceID,omitempty" yaml:"resourceId,omitempty"`

ResourceID string `json:"resourceId,omitempty" yaml:"resourceId,omitempty"`

Copilot · 2026-03-27T19:48:54Z

+type xrImpactJSON struct {
+	corev1.ObjectReference `json:",inline"`
+
+	Status            XRStatus           `json:"status"                      yaml:"status"`
+	Error             string             `json:"error,omitempty"             yaml:"error,omitempty"`
+	DownstreamChanges *DownstreamChanges `json:"downstreamChanges,omitempty" yaml:"downstreamChanges,omitempty"`


corev1.ObjectReference is embedded with the struct tag json:",inline", but encoding/json does not support inline and the presence of a json tag prevents anonymous-field promotion. This will likely serialize as a nested ObjectReference:{...} field instead of top-level apiVersion/kind/name/namespace, conflicting with the README example and intended machine-readable schema. Embed corev1.ObjectReference without a json tag (and, if needed, use yaml:",inline" for YAML) so identity fields are actually inlined.

Copilot · 2026-03-27T19:48:55Z

+			validate: func(t *testing.T, result string) {
+				t.Helper()
+
+				var parsed compDiffJSONOutput
+				if err := json.Unmarshal([]byte(result), &parsed); err != nil {
+					t.Fatalf("Failed to parse JSON: %v", err)
+				}
+
+				if len(parsed.Compositions) != 1 {
+					t.Fatalf("Expected 1 composition, got %d", len(parsed.Compositions))
+				}
+
+				if parsed.Compositions[0].Name != "test-comp" {
+					t.Errorf("Expected name 'test-comp', got '%s'", parsed.Compositions[0].Name)
+				}
+
+				if parsed.Compositions[0].AffectedResources.Total != 2 {
+					t.Errorf("Expected total 2, got %d", parsed.Compositions[0].AffectedResources.Total)
+				}
+				// Verify compositionChanges is present in JSON
+				if parsed.Compositions[0].CompositionChanges == nil {
+					t.Error("Expected compositionChanges to be present")
+				}
+			},


The structured comp diff tests don't currently assert the JSON/YAML shape of the embedded XR identity fields (apiVersion/kind/name/namespace). Adding an assertion that these fields are top-level (and not nested under an ObjectReference key) would catch schema regressions like incorrect struct tags on the embedded reference.

Signed-off-by: Jonathan Ogilvie <jonathan.ogilvie@sumologic.com>

Copilot

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 8 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-27T21:19:30Z

+					t.Error("Expected Manual update policy message")
+				}
+			},
+		},


DefaultCompDiffRenderer now has an error-bearing path via CompositionDiff.Error (set by the processor), but the unit tests here don't cover how the renderer should behave when a composition failed to process. Adding a test case with CompositionDiff{Error: ...} would help prevent regressions and ensure error output is user-friendly.

Suggested change

},

},

"CompositionProcessingError": {

output: &CompDiffOutput{

Compositions: []CompositionDiff{{

Name: "error-comp",

Error: errors.New("failed to process composition"),

}},

},

colorize: false,

validate: func(t *testing.T, result string) {

t.Helper()

if !strings.Contains(result, "error-comp") {

t.Error("Expected output to contain the composition name for error case")

}

if !strings.Contains(result, "failed to process composition") {

t.Error("Expected output to include the composition error message")

}

},

},

Copilot · 2026-03-27T21:19:31Z


+	// Write errors at the end (for human-readable output)
+	for _, e := range errs {
+		errMsg := fmt.Sprintf("ERROR: %s: %s", e.ResourceID, e.Message)


DefaultDiffRenderer prints errors as ERROR: %s: %s unconditionally. Since OutputError.ResourceID is optional, this can yield ERROR: : <message> when ResourceID is empty. Consider omitting the extra colon/prefix when ResourceID isn't set (or using a different format for global errors).

Suggested change

errMsg := fmt.Sprintf("ERROR: %s: %s", e.ResourceID, e.Message)

var errMsg string

if e.ResourceID != "" {

errMsg = fmt.Sprintf("ERROR: %s: %s", e.ResourceID, e.Message)

} else {

errMsg = fmt.Sprintf("ERROR: %s", e.Message)

}

Copilot · 2026-03-27T21:19:31Z

+				comps := rawParsed["compositions"].([]any)
+				comp := comps[0].(map[string]any)
+				impacts := comp["impactAnalysis"].([]any)
+				impact := impacts[0].(map[string]any)
+


This test relies on unchecked type assertions (e.g., rawParsed["compositions"].([]any)), which will panic if the JSON shape changes and can obscure the actual failure. Consider using comma-ok assertions and failing the test with a descriptive message instead of panicking.

Suggested change

comps := rawParsed["compositions"].([]any)

comp := comps[0].(map[string]any)

impacts := comp["impactAnalysis"].([]any)

impact := impacts[0].(map[string]any)

rawComps, ok := rawParsed["compositions"]

if !ok {

t.Fatalf("Expected 'compositions' key in JSON output, but it was missing")

}

comps, ok := rawComps.([]any)

if !ok {

t.Fatalf("Expected 'compositions' to be a JSON array, got %T", rawComps)

}

if len(comps) == 0 {

t.Fatalf("Expected at least one composition in 'compositions', but got none")

}

comp, ok := comps[0].(map[string]any)

if !ok {

t.Fatalf("Expected first element of 'compositions' to be an object, got %T", comps[0])

}

rawImpacts, ok := comp["impactAnalysis"]

if !ok {

t.Fatalf("Expected 'impactAnalysis' key in first composition, but it was missing")

}

impacts, ok := rawImpacts.([]any)

if !ok {

t.Fatalf("Expected 'impactAnalysis' to be a JSON array, got %T", rawImpacts)

}

if len(impacts) == 0 {

t.Fatalf("Expected at least one impact in 'impactAnalysis', but got none")

}

impact, ok := impacts[0].(map[string]any)

if !ok {

t.Fatalf("Expected first element of 'impactAnalysis' to be an object, got %T", impacts[0])

}

Signed-off-by: Jonathan Ogilvie <jonathan.ogilvie@sumologic.com>

Copilot

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Signed-off-by: Jonathan Ogilvie <jonathan.ogilvie@sumologic.com>

Copilot

Pull request overview

Copilot reviewed 19 out of 19 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Signed-off-by: Jonathan Ogilvie <jonathan.ogilvie@sumologic.com>

Copilot

Pull request overview

Copilot reviewed 19 out of 19 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

jcogilvie added 3 commits March 27, 2026 00:07

feat: machine readable comp diff

07f8dc7

Signed-off-by: Jonathan Ogilvie <jonathan.ogilvie@sumologic.com>

fix: cleanup data flow for comp diff rendering

b0817ef

Signed-off-by: Jonathan Ogilvie <jonathan.ogilvie@sumologic.com>

doc: update readme for machine readable diff output

2573a49

Signed-off-by: Jonathan Ogilvie <jonathan.ogilvie@sumologic.com>

jcogilvie marked this pull request as ready for review March 27, 2026 18:11

Copilot AI review requested due to automatic review settings March 27, 2026 18:11

Copilot started reviewing on behalf of jcogilvie March 27, 2026 18:11 View session

Copilot AI reviewed Mar 27, 2026

View reviewed changes

Comment thread cmd/diff/renderer/comp_diff_renderer.go

Comment thread cmd/diff/renderer/comp_diff_renderer.go

Comment thread cmd/diff/diffprocessor/comp_processor.go

jcogilvie added 2 commits March 27, 2026 15:28

fix: cleanup error handling for diff rendering, both xr and comp

b4f0f7b

Signed-off-by: Jonathan Ogilvie <jonathan.ogilvie@sumologic.com>

fix: cleanup error handling for diff rendering, both xr and comp

ae2190f

Signed-off-by: Jonathan Ogilvie <jonathan.ogilvie@sumologic.com>

Copilot AI review requested due to automatic review settings March 27, 2026 19:42

Copilot started reviewing on behalf of jcogilvie March 27, 2026 19:43 View session

Copilot AI reviewed Mar 27, 2026

View reviewed changes

fix: cleanup error handling for diff rendering, both xr and comp

27209ca

Signed-off-by: Jonathan Ogilvie <jonathan.ogilvie@sumologic.com>

jcogilvie requested a review from Copilot March 27, 2026 21:12

Copilot started reviewing on behalf of jcogilvie March 27, 2026 21:13 View session

Copilot AI reviewed Mar 27, 2026

View reviewed changes

jcogilvie added 2 commits March 27, 2026 18:22

fix: review feedback

456f7fa

Signed-off-by: Jonathan Ogilvie <jonathan.ogilvie@sumologic.com>

fix: add more test coverage

efba395

Signed-off-by: Jonathan Ogilvie <jonathan.ogilvie@sumologic.com>

Copilot AI review requested due to automatic review settings March 31, 2026 15:50

Copilot started reviewing on behalf of jcogilvie March 31, 2026 17:27 View session

Copilot AI reviewed Mar 31, 2026

View reviewed changes

jcogilvie added 2 commits March 31, 2026 15:13

fix: avoid ns collisions in keys

c20f33c

Signed-off-by: Jonathan Ogilvie <jonathan.ogilvie@sumologic.com>

fix: remove unneeded comments

02140ee

Signed-off-by: Jonathan Ogilvie <jonathan.ogilvie@sumologic.com>

Copilot AI review requested due to automatic review settings March 31, 2026 19:38

Copilot started reviewing on behalf of jcogilvie March 31, 2026 19:39 View session

Copilot AI reviewed Mar 31, 2026

View reviewed changes

jcogilvie added 2 commits March 31, 2026 18:38

fix: share test data between yaml and json

d752bb0

Signed-off-by: Jonathan Ogilvie <jonathan.ogilvie@sumologic.com>

fix: remove redundant test assertion

1a37a06

Signed-off-by: Jonathan Ogilvie <jonathan.ogilvie@sumologic.com>

Copilot AI review requested due to automatic review settings March 31, 2026 22:56

Copilot started reviewing on behalf of jcogilvie March 31, 2026 22:57 View session

Copilot AI reviewed Mar 31, 2026

View reviewed changes

jcogilvie merged commit d10e74c into main Apr 1, 2026
15 checks passed

jcogilvie deleted the machine-readable-comp-diff branch April 1, 2026 14:50

	ResourceID string `json:"resourceID,omitempty" yaml:"resourceId,omitempty"`
	ResourceID string `json:"resourceId,omitempty" yaml:"resourceId,omitempty"`

-		},
+		},
+		"CompositionProcessingError": {
+			output: &CompDiffOutput{
+				Compositions: []CompositionDiff{{
+					Name:  "error-comp",
+					Error: errors.New("failed to process composition"),
+				}},
+			},
+			colorize: false,
+			validate: func(t *testing.T, result string) {
+				t.Helper()
+				if !strings.Contains(result, "error-comp") {
+					t.Error("Expected output to contain the composition name for error case")
+				}
+				if !strings.Contains(result, "failed to process composition") {
+					t.Error("Expected output to include the composition error message")
+				}
+			},
+		},

-		errMsg := fmt.Sprintf("ERROR: %s: %s", e.ResourceID, e.Message)
+		var errMsg string
+		if e.ResourceID != "" {
+			errMsg = fmt.Sprintf("ERROR: %s: %s", e.ResourceID, e.Message)
+		} else {
+			errMsg = fmt.Sprintf("ERROR: %s", e.Message)
+		}

-				comps := rawParsed["compositions"].([]any)
-				comp := comps[0].(map[string]any)
-				impacts := comp["impactAnalysis"].([]any)
-				impact := impacts[0].(map[string]any)
+				rawComps, ok := rawParsed["compositions"]
+				if !ok {
+					t.Fatalf("Expected 'compositions' key in JSON output, but it was missing")
+				}
+				comps, ok := rawComps.([]any)
+				if !ok {
+					t.Fatalf("Expected 'compositions' to be a JSON array, got %T", rawComps)
+				}
+				if len(comps) == 0 {
+					t.Fatalf("Expected at least one composition in 'compositions', but got none")
+				}
+				comp, ok := comps[0].(map[string]any)
+				if !ok {
+					t.Fatalf("Expected first element of 'compositions' to be an object, got %T", comps[0])
+				}
+				rawImpacts, ok := comp["impactAnalysis"]
+				if !ok {
+					t.Fatalf("Expected 'impactAnalysis' key in first composition, but it was missing")
+				}
+				impacts, ok := rawImpacts.([]any)
+				if !ok {
+					t.Fatalf("Expected 'impactAnalysis' to be a JSON array, got %T", rawImpacts)
+				}
+				if len(impacts) == 0 {
+					t.Fatalf("Expected at least one impact in 'impactAnalysis', but got none")
+				}
+				impact, ok := impacts[0].(map[string]any)
+				if !ok {
+					t.Fatalf("Expected first element of 'impactAnalysis' to be an object, got %T", impacts[0])
+				}

Conversation

jcogilvie commented Mar 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of your changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jcogilvie commented Mar 27, 2026 •

edited

Loading