feat(podspec): find images at configured per-kind JMESPath expressions (option D) - #84
Open
MPV wants to merge 5 commits into
Open
feat(podspec): find images at configured per-kind JMESPath expressions (option D)#84MPV wants to merge 5 commits into
MPV wants to merge 5 commits into
Conversation
This was referenced Aug 9, 2026
MPV
force-pushed
the
claude/podspec-d-configurable-paths
branch
from
August 9, 2026 20:35
1c499b0 to
1314ee4
Compare
This was referenced Aug 9, 2026
MPV
force-pushed
the
claude/podspec-d-configurable-paths
branch
from
August 10, 2026 06:46
ab472b2 to
4feab3f
Compare
This was referenced Aug 11, 2026
Option D of four candidate answers to #26, raised side by side for comparison. Keeps the lookup and moves it out of Go into configuration. k8s/resources.yaml, embedded, lists each kind and the paths at which it holds a PodSpec. A `documents` path names nodes to process as objects in their own right, so the List special case becomes two lines of config. --config merges a user's file over the built-in one, keyed by kind, so a custom resource can be added and a built-in corrected: $ kir rollout.yaml # nothing; kir has not been told $ kir --config rollouts.yaml rollout.yaml my-registry/app:1.4.2 busybox:1.36 Cheapest option by every mechanical measure, because the lookup never decides anything: 151ms -> 107ms over 1000 documents, a 3.9 MB binary against 28.9 MB, and go.sum down from 99 lines to 22 — dropping typed decoding drops k8s.io/client-go, and matching by path rather than by type drops k8s.io/api too, leaving sigs.k8s.io/yaml. Precision is exact by construction: a path matches or it does not. The cost is that it does not answer #26's second motivation on its own. Custom resources stay invisible until described, which TestCustomResource pins in both directions. This reads as the complement of structural discovery rather than its competitor. 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 -> 105ms over 1000 documents, a 3.7 MB binary against 27.6 MB, and 26 go.sum lines rather than 22 — master now splits documents with the Kubernetes YAML reader, so k8s.io/apimachinery comes back. Also say what resolves the paths, since it is a fair thing to ask: ~25 lines of Go, not JMESPath or JSONPath. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013RYRsLAGHPcRuwhodXXmWP
The hand-rolled resolver handled dot-separated paths with a [*] suffix, which covers every built-in kind but only navigates. Real custom resources need selection. An Argo Workflow holds a list of templates, each with a container, a script, or neither (a dag, a suspend). One expression covers it: $ kir --config workflow.yaml workflow.yaml builder:1.2.0 python:3.12 containers: ["spec.templates[*].[container, script][]"] Multi-select and flattening are not field navigation, and the projection drops the templates holding neither. TestCustomResource/Workflow pins it through the CLI; the k8s tests cover projection, multi-select and filter expressions directly. Since such resources hold bare containers rather than a PodSpec, a `containers` key joins `podSpecs` and `documents`. Expressions now compile when the config loads, so a typo is an error naming the kind and field rather than a path that silently matches nothing for the whole run — TestLoadConfigRejectsBadExpression. The built-in resources.yaml needed no edits: `spec.template.spec` means the same thing in both syntaxes. The cost is 13 go.sum lines, 0.2 MB of binary and 6ms per 1000 documents (105ms -> 111ms), and a larger surface — an expression can select something that is not a container, and nothing checks that claim. BREAKING CHANGE: locations in a --config file are JMESPath expressions. Plain field paths are unaffected; a literal key needing quoting is not. 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, where the dependency bumps made everything faster: 105ms -> 76ms over 1000 documents against a 105ms master, a 4.1 MB binary against 27.2 MB, and 42 go.sum lines rather than 74. JMESPath costs less than it looked: 75ms -> 76ms, not 6ms. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013RYRsLAGHPcRuwhodXXmWP
MPV
force-pushed
the
claude/podspec-d-configurable-paths
branch
from
August 13, 2026 07:53
4feab3f to
ddc8905
Compare
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
ADR 0009 said a configured expression selecting something that is not a container goes unchecked. Since 0.4.4 imageref.Validate rejects an unreportable value at output, so a mis-aimed expression is named on stderr with a non-zero exit rather than printed. What survives is one selecting something that merely looks like a reference. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013RYRsLAGHPcRuwhodXXmWP
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Option D 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.Rebased onto
2abd515(0.4.3) — Kubernetes v0.36.3, Go 1.26.masterRollout)Workflow)containerslookalikego.sumlinesmake schema--schema--config--configOne 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
Keep the lookup — the kinds and paths are not wrong, they are just in Go, which is why a new one needs a release. Move them into configuration, as JMESPath expressions.
k8s/resources.yaml(embedded) lists each kind and where it holds images:podSpecs,containers(for resources holding bare containers), anddocuments(how aListunwraps its items, so that special case is two lines of config).--configmerges a user's file over the built-in one, keyed by kind, so a custom resource can be added and a built-in corrected.Why JMESPath rather than plain field paths
An earlier revision used a ~25-line hand-rolled resolver. For navigation the two are indistinguishable — and the built-in
resources.yamlneeded no edits when the resolver was swapped.What field paths cannot express is selection. An Argo
Workflowholds a list of templates, each with a container, a script, or neither:Multi-select and flattening are not field navigation, and the projection drops templates holding neither. Only this option and #106 find those images — #82 and #83 look for PodSpec shape, and a Workflow template's
containeris a bare container.Also: expressions compile when the config loads, so a typo is an error naming the kind and field rather than one that silently matches nothing all run. Re-measured, the price of the dependency is 13
go.sumlines, 0.2 MB, ~1 ms per 1000 documents.What it buys overall
Cheapest by every mechanical measure, because the lookup never has to decide anything: 92 ms against
master's 144 ms, a 4.1 MB binary against 27.2 MB, and 42go.sumlines against 74. Precision is exact by construction, with no schema or generated code to keep in step with the Kubernetes API — the v0.32.3 → v0.36.3 bump touched nothing here.What it costs
It does not answer #26's second motivation on its own. A custom resource is invisible until somebody describes it, so every user of Argo, Knative, or an in-house CRD writes that file.
TestCustomResourcepins both halves deliberately —Undescribedyields nothing,Configuredyields images. That is the gap #106 closes by inferring the shapes that can be inferred.Notes for review
feat(podspec)!—--configlocations are JMESPath. Plain field paths are unaffected; a literal key that needs quoting is not.TestFailure.BadYAML's stderr gains anerror converting YAML to JSON:prefix — same class, same exit 1.gofmt,go vet,go mod tidyno-op,go test -race ./...green. ADR 0009 records the decision as proposed.