Skip to content

docs: refresh design doc to current architecture #348

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jcogilvie wants to merge 1 commit into
base: main
Choose a base branch
from
Open

docs: refresh design doc to current architecture #348

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 60 additions & 0 deletions CLAUDE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -381,6 +381,66 @@ Keep a small set of ANSI tests (~5-7) to smoke test visual output formatting.
- Always strive to diagnose and address root causes, not symptoms
- Empty strings, nil maps, missing fields must all be handled correctly

## Keeping Documentation in Sync

The design doc at `design/design-doc-cli-diff.md` is hand-maintained and drifts quickly if it isn't updated alongside the
code. Before completing a change, check whether any of these triggers apply and update the corresponding section.

**Triggers and where to update**:

| Code change | Update |
|--------------------------------------------------------------------------------|----------------------------------------------------------------------------|
| Interface in `cmd/diff/diffprocessor/` (`DiffProcessor`, `CompDiffProcessor`, `DiffCalculator`, `ResourceManager`, `SchemaValidator`, `FunctionProvider`) — added/removed/renamed methods or changed signatures | `§6` of the design doc (the relevant interface block); `diff-processor-architecture.mermaid` and `package-overview-fixed.mermaid` |
| New or removed Kubernetes/Crossplane client in `cmd/diff/client/` | `§5.2.4`, `§6.9`; `client-architecture.mermaid`, `layered-architecture.mermaid`, `conceptual-layers.mermaid` |
| Renderer changes (`cmd/diff/renderer/`): new `OutputFormat`, new structured-output type, change to error-emission contract | `§6.8`; `diff-rendering-architecture.mermaid` |
| New CLI flag, renamed flag, removed flag, or new subcommand | `§8.1` examples; if it's a `ProcessorConfig` field too, `§6.1.2` |
| Project layout: new top-level package under `cmd/diff/`, removed file/dir | `§12.1` directory tree |
| Changes to the iterative render loop, eventual-state behaviour, or how `RequiredResources`/`RequiredSchemas` are consumed | `§9.5.6` (esp. `§9.5.6.2`) |
| Behavioural change to a domain component without a signature change (e.g. switching the schema-validation backend, moving defaulting in/out of validation, changing what errors propagate) | The relevant `§6` narrative paragraph; if it touches integration with upstream Crossplane libs, the matching `§9.5.x` block too |
| Workflow change: nested-XR handling, two-phase diff split, cleanup semantics, claim handling | `§7`; `diff-call-sequence.mermaid` |
| New integration-test category (e.g. a new structured-assertion pattern, a new claim edge case) | `§4` test-case list |
| A future-work item ships, or a new follow-up is identified | `§10` future enhancements |

**Diagrams**: the `*.mermaid` sources under `design/design-doc-cli-diff/` are the source of truth. After editing a
mermaid file, regenerate its `.svg` so the rendered doc matches. Prefer the official mermaid-cli Docker image for
portability across machines and CI:

```bash
cd design/design-doc-cli-diff
for f in *.mermaid; do
docker run --rm -u "$(id -u):$(id -g)" -v "$PWD:/data" minlag/mermaid-cli \
-i "/data/$f" -o "/data/${f%.mermaid}.svg"
done
Comment on lines +405 to +413
```

**README and E2E docs**: `README.md` documents user-facing CLI behaviour and output formats; update it for any change a
user would notice in their workflow — flag/subcommand changes, exit-code changes, structured-output schema changes
(new fields under `errors[]`, `validationFailures[]`, `impactAnalysis[]`, etc.), and human-readable rendering changes
that meaningfully alter what hits stdout/stderr. `test/e2e/README.md` covers E2E structure; update it when adding a new
test category, changing expectation-file conventions, or adding a new helper to the assertion harness.

**When in doubt**, prefer updating the doc in the same PR as the code change. A single PR that ships drift is much
cheaper than retroactively auditing several PRs' worth of changes (as happened in the doc-refresh on 2026-06-15).

## Git, Commits, Issues, and Pull Requests

A few project-wide conventions apply to every change you submit to this repository:

- **Sign every commit (DCO)**. This repo enforces the Developer Certificate of Origin. Always commit with `git commit -s`
so a `Signed-off-by:` trailer is added; PRs without DCO sign-off on every commit will be blocked from merging. If you
need to fix a missing sign-off after the fact, use `git commit --amend -s --no-edit` (or `git rebase --signoff` for a
range of commits).
- **Open PRs as drafts**. Always use `gh pr create --draft …` (or pass `draft: true` via the API). The maintainer can
mark the PR ready for review once they've assessed it; opening as a draft prevents premature reviewer notifications
and lets CI run before review starts.
- **Follow the repository's issue and PR templates**. The PR body must follow `.github/PULL_REQUEST_TEMPLATE.md` —
including the `### Description of your changes` heading, the `Fixes #` line (with a real issue number, or omit the
line entirely if there is none), and the "I have:" checklist. **Every checklist item must be either `[x]` checked or
struck through with `~~…~~`** — leaving items as `[ ]` is not acceptable. If a checklist item legitimately doesn't
apply (e.g. no e2e tests for a docs-only change), strike it through rather than skipping it. Issues filed via `gh
issue create` must use the matching template under `.github/ISSUE_TEMPLATE/` (`bug_report.md` or
`feature_request.md`); pass it via `--template bug_report.md` and fill in every section.

## Related Documentation

- [Design Document](design/design-doc-cli-diff.md): Comprehensive technical design and architecture
Expand Down
91 changes: 90 additions & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -513,7 +513,96 @@ The structured output includes:
- **Full resource details**: apiVersion, kind, name, namespace
- **Diff content**: old/new values for modifications, full spec for additions/removals
- **Impact analysis** (comp only): which XRs are affected by composition changes and their status
- **Errors**: A top-level `errors` array with entries of the form `{ "resourceID": "...", "message": "..." }`, plus per-XR `error` fields in `impactAnalysis` for composition diffs
- **Errors**: A top-level `errors` array of `OutputError` objects (see [Validation Errors](#validation-errors) below for the schema and an example), plus per-XR `error` fields in `impactAnalysis` for composition diffs

### Validation Errors

When schema validation fails on the input XR or any rendered composed resource, `crossplane-diff` exits with code 2 and reports the failure in both human-readable and machine-readable form.

**Human-readable output** (`crossplane-diff xr invalid-xr.yaml`):

Per Unix convention, errors go to stderr and diff content goes to stdout. When validation is the only failure, stdout is empty and the structured failure detail appears on stderr, prefixed by an `ERROR: <resourceID>:` marker:

```
# stderr
ERROR: XNopResource/invalid-schema-xr: ns.diff.example.org/v1alpha1/XNopResource default/invalid-schema-xr:
spec.coolField: Invalid value: "number": ... [schema]
ns.nop.example.org/v1alpha1/XDownstreamResource default/invalid-schema-xr:
spec.forProvider.configData: Invalid value: "boolean": ... [schema]

# stdout
(empty)
```

Each per-resource block has the shape `<apiVersion>/<Kind> [<namespace>/]<name>:` followed by indented `<message> [<type>]` lines, where `<type>` is one of `[schema]`, `[cel]`, `[unknownField]`, or `[defaulting]`. A bad value is appended as `(got <value>)` when it isn't already substring-present in the message. When some inputs in a batched run succeed and others fail validation, the successful diffs appear on stdout and the failing inputs' `ERROR:` blocks appear on stderr.

**Machine-readable output** (`crossplane-diff xr invalid-xr.yaml --output json`):

```json
{
"summary": { "added": 0, "modified": 0, "removed": 0 },
"changes": [],
"errors": [
{
"resourceID": "XNopResource/invalid-schema-xr",
"message": "ns.diff.example.org/v1alpha1/XNopResource default/invalid-schema-xr:\n spec.coolField: Invalid value: \"number\": ... [schema]\nns.nop.example.org/v1alpha1/XDownstreamResource default/invalid-schema-xr:\n spec.forProvider.configData: Invalid value: \"boolean\": ... [schema]",
"validationFailures": [
{
"apiVersion": "ns.diff.example.org/v1alpha1",
"kind": "XNopResource",
"name": "invalid-schema-xr",
"namespace": "default",
"status": "invalid",
"errors": [
{
"type": "schema",
"field": "spec.coolField",
"message": "spec.coolField: Invalid value: \"number\": ...",
"value": "number"
}
]
},
{
"apiVersion": "ns.nop.example.org/v1alpha1",
"kind": "XDownstreamResource",
"name": "invalid-schema-xr",
"namespace": "default",
"status": "invalid",
"errors": [
{
"type": "schema",
"field": "spec.forProvider.configData",
"message": "spec.forProvider.configData: Invalid value: \"boolean\": ...",
"value": "boolean"
}
]
}
]
}
]
}
```

The `OutputError` schema:

| Field | Type | Description |
|-------|------|-------------|
| `resourceID` | string | Identifies which user-supplied input the diff was processing (one entry per batched run). Format: `<Kind>/<name>`. |
| `message` | string | Human-readable error string — the same text written to stderr. |
| `validationFailures` | `[]ResourceValidationFailure`, optional | Structured per-resource breakdown. Set only for schema-validation failures; `nil` for tool, IO, render, and scope-check errors. |

`ResourceValidationFailure` carries `apiVersion`, `kind`, `name`, `namespace`, `status` (one of `"invalid"` or `"missingSchema"` — `"valid"` rows are filtered out), and `errors`, a list of `FieldValidationError` records:

| Field | Type | Description |
|-------|------|-------------|
| `type` | string | `"schema"`, `"cel"`, `"unknownField"`, or `"defaulting"`. |
| `field` | string, optional | JSONPath of the offending field, when locatable. |
| `message` | string | Validator-emitted human-readable description; for k8s-derived schema errors this typically already embeds the field path and bad value. |
| `value` | any, optional | The offending value as the validator saw it. Type-preserved (string, number, bool, struct). |

`resourceID` and `validationFailures` are intentionally complementary: `resourceID` anchors the failure to one user-supplied input, while `validationFailures` enumerates every resource (the input itself plus any composed resource) that failed validation under that input. They overlap on `kind`+`name` when the input itself is among the failing resources — that's deliberate, so consumers iterating `validationFailures` never miss an XR-level rejection.

This same structure is used for `comp` output (under each composition's `errors` array) — see the composition diff JSON example above for placement.

## Exit Codes

Expand Down
Loading
Loading