Skip to content

feat(podspec): find PodSpecs structurally, validated by the Go types (option B) - #82

Open
MPV wants to merge 3 commits into
masterfrom
claude/podspec-b-structural-matching
Open

feat(podspec): find PodSpecs structurally, validated by the Go types (option B)#82
MPV wants to merge 3 commits into
masterfrom
claude/podspec-b-structural-matching

Conversation

@MPV

@MPV MPV commented Aug 9, 2026

Copy link
Copy Markdown
Owner

Option B of the candidates for #26, raised side by side so they can be compared. Scope feat(podspec) finds the set — #81 #82 #83 #84 #106. Only one should be merged.

See also #106, which combines this PR with #84 — inference here, configured expressions for what inference cannot see. It reaches strictly more than this PR and, because configured kinds skip the walk, runs faster than it.

Rebased onto 2abd515 (0.4.3) — Kubernetes v0.36.3, Go 1.26.

master A · reflection B · structural ← this C · CUE D · config E · B+D
CR embedding a PodSpec (Argo Rollout) ✓ auto ✓ auto ✓ via config ✓ auto
CR with bare containers (Argo Workflow) ✓ via config ✓ via config
Can silence a wrong reading n/a
Rejects a containers lookalike ✓ strict decode
1000 documents 144 ms 135 ms 171 ms 3058 ms 92 ms 107 ms
Binary 27.2 MB 27.2 MB 12.4 MB 25.4 MB 4.1 MB 12.6 MB
go.sum lines 74 74 72 109 42 78
Code generation make schema
New CLI surface --schema --config --config

One interleaved best-of-7 run; every variant emits the same 2000 images. Absolute timings drift with machine load between runs — the ratios are the stable quantity.

Approach

Stop decoding into typed Kubernetes objects. Decode each document into plain Go values, walk it, and test each node for PodSpec shape.

The shape test is the load-bearing part, and it is deliberately not a field-name heuristic: a candidate's containers / initContainers / ephemeralContainers are decoded into the real corev1 types with unknown fields rejected. The Kubernetes Go types are the schema, so a node matches when Kubernetes itself would call it a PodSpec.

What it buys

Custom resources work, which is the half of #26 option A cannot reach:

$ kir approvals/kir_test.TestCustomResource.input.yaml   # an Argo Rollout
my-registry/app:1.4.2
busybox:1.36

A checked-in fixture with a golden, so the example can't drift. List stops being a special case — its items are just more nodes — taking the kind allow-list and the unstructured item handling with it. ReplicationController and PodTemplate come along for free.

Dropping typed decoding drops k8s.io/client-go entirely: 27.2 MB → 12.4 MB. k8s.io/api stays, as the schema.

What it costs

  • Precision rests on the strict decode. TestSkipsNonWorkloads.Lookalike pins it: a resource with a containers field holding non-containers yields nothing. Worth reviewing that fixture specifically — it is the guard against this approach going wrong.
  • A CR holding bare containers is missed, since there is no PodSpec shape to match, and a user has no way to say otherwise. An Argo Workflow is the common case. This is the gap feat(podspec): infer images structurally, with configured overrides (option E) #106 closes.
  • Bound to the vendored k8s.io/api: a container field newer than it fails the strict decode. In practice the binding is loose — this rebase carried Kubernetes v0.32.3 → v0.36.3, four minor versions, and moved no goldens and needed no code change.
  • 144 ms → 171 ms over 1000 documents.

Notes for review

  • The walk sorts map keys — without that, Go's randomised map iteration would make output order flake whenever a document holds more than one PodSpec. TestFindImagesOrderIsStable guards it.
  • k8s/k8s_test.go is rewritten because the type switch it tested is gone. The replacement corpus is shared verbatim with option C, so the two engines are compared on identical cases.
  • One golden changes: TestFailure.BadYAML's stderr gains an error converting YAML to JSON: prefix — same class, same exit 1, different parser wrapping the message. Shared with C, D and E.

gofmt, go vet, go mod tidy no-op, go test -race ./... green. ADR 0009 records the decision as proposed, superseding ADR 0001 only if merged.

claude added 3 commits August 13, 2026 07:50
Option B of four candidate answers to #26, raised side by side for
comparison. Stops decoding into typed objects; finds the PodSpec by
shape instead.

Each document is decoded into plain Go values and walked. A node is a
PodSpec when its containers/initContainers/ephemeralContainers decode
into the real corev1 types with unknown fields rejected — the Kubernetes
Go types are the schema, so the test is "would Kubernetes call this a
PodSpec", not "is there a field named containers".

Custom resources therefore work, which is the half of #26 that option A
cannot reach:

  $ kir rollout.yaml     # an Argo Rollout; no kind named anywhere in kir
  my-registry/app:1.4.2
  busybox:1.36

List stops being a special case (its items are just more nodes), taking
the kind allow-list and the unstructured item handling with it. Dropping
typed decoding drops k8s.io/client-go entirely: 28.9 MB -> 12.8 MB.

The costs are real and covered by fixtures. Matching on shape could
match on name alone, so TestSkipsNonWorkloads.Lookalike pins that a
custom resource with a containers field holding non-containers yields
nothing. Walking every node costs 169ms -> 203ms over 1000 documents.
Precision is now tied to the vendored k8s.io/api. See ADR 0008.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013RYRsLAGHPcRuwhodXXmWP
The ADR carried figures taken before the rebase onto 0.4.2. Re-measured
on the rebased branch: 152ms -> 211ms over 1000 documents, and a 12.2 MB
binary against 27.6 MB.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013RYRsLAGHPcRuwhodXXmWP
The ADR carried figures from the 0.4.2 base. Re-measured on 0.4.3:
105ms -> 133ms over 1000 documents against a 105ms master, and a 12.4 MB
binary against 27.2 MB.

The bump is also evidence for the ADR claim it sits under: strict
decoding is bound to the vendored k8s.io/api, and four minor versions
moved no goldens and needed no code change. Say so.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013RYRsLAGHPcRuwhodXXmWP
@MPV
MPV force-pushed the claude/podspec-b-structural-matching branch from 8c44fd5 to eafaf7a Compare August 13, 2026 07:50
MPV pushed a commit that referenced this pull request Aug 13, 2026
A fifth candidate for #26, combining the two that matter. Structural
inference (#82) and configured expressions (#84) each fail exactly where
the other succeeds, so this runs both with configuration taking
precedence per kind.

Config.FindImages looks up the document's kind. An entry decides on its
own — its expressions are followed and the walk is not consulted.
Everything else is inferred. The two never both contribute to one
document, so an image cannot be reported twice.

The union reaches more than either alone:

  $ kir rollout.yaml          # inferred; no configuration, no kind named
  my-registry/app:1.4.2
  busybox:1.36
  $ kir workflow.yaml         # bare containers: no PodSpec shape to match
  $ kir --config workflows.yaml workflow.yaml
  builder:1.2.0
  python:3.12

And one thing neither can do alone: an entry with no expressions
silences a kind, so a user can overrule the walk where it reads
something wrongly. Inference cannot be told to ignore; configuration has
nothing to ignore.

It is also cheaper than inference alone on ordinary input, which is the
reverse of what combining two mechanisms usually costs: configured kinds
take the exact lookup and never walk, so 1000 Deployments run in 107ms
against inference alone's 171ms, near configuration alone's 92ms.

The built-in resources.yaml is an accelerator, not knowledge:
TestBuiltInConfigIsRedundant compares every built-in kind's configured
result against its inferred one, so deleting the file would change speed
and nothing else. Without that test it would quietly become the
hardcoded kind list #26 set out to remove — reverting one entry to a
wrong path fails it.

See ADR 0009.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013RYRsLAGHPcRuwhodXXmWP
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants