Skip to content

feat(podspec): infer images structurally, with configured overrides (option E) - #106

Open
MPV wants to merge 2 commits into
masterfrom
claude/podspec-e-inferred-with-overrides
Open

feat(podspec): infer images structurally, with configured overrides (option E)#106
MPV wants to merge 2 commits into
masterfrom
claude/podspec-e-inferred-with-overrides

Conversation

@MPV

@MPV MPV commented Aug 11, 2026

Copy link
Copy Markdown
Owner

Option E for #26 — not a fifth alternative but a combination of #82 (structural inference) and #84 (configured expressions), which each fail exactly where the other succeeds. Scope feat(podspec) finds the set — #81 #82 #83 #84 #106.

master A · reflection B · structural C · CUE D · config E · both ← this
Custom resource embedding a PodSpec ✓ auto ✓ auto ✓ via config ✓ auto
Custom resource with bare containers ✓ via config ✓ via config
Built-in workloads with no config (config)
Can silence a wrong reading n/a
Rejects a containers lookalike
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

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

How it works

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 by the structural walk. The two never both contribute to one document, so an image cannot be reported twice (TestConfiguredKindIsNotAlsoInferred).

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

$ kir approvals/kir_test.TestCustomResource.Workflow.input.yaml   # bare containers: nothing to infer
$ kir --config approvals/kir_test.TestCustomResource.Workflow.config.yaml \
      approvals/kir_test.TestCustomResource.Workflow.input.yaml
builder:1.2.0
python:3.12

Both are checked-in fixtures with goldens (TestCustomResource/{Inferred,WorkflowUndescribed,WorkflowConfigured}), so the examples can't drift.

Why combine rather than pick

Three things fall out that no single option has:

  1. Zero config for the common case, an exact escape hatch for the rest. feat(podspec): find PodSpecs structurally, validated by the Go types (option B) #82 leaves Argo Workflows unreachable with no way for a user to help; feat(podspec): find images at configured per-kind JMESPath expressions (option D) #84 makes every Argo/Knative user write config for resources whose shape already speaks for itself.
  2. Suppression. An entry with no expressions silences a kind — podSpecs: []. Inference alone cannot be told to ignore something; configuration alone has nothing to ignore. This is the only option where a user can correct kir rather than only extend it (TestEmptyEntrySilencesAKind).
  3. It's cheaper than inference alone, which is the reverse of what combining usually costs. Configured kinds take the exact lookup and never walk: 107 ms vs feat(podspec): find PodSpecs structurally, validated by the Go types (option B) #82's 171 ms, close to feat(podspec): find images at configured per-kind JMESPath expressions (option D) #84's 92 ms. Only kinds nobody has described pay for the walk. I verified the mechanism by emptying the built-in config — it returns to inference-alone speed.

The test that keeps this honest

Shipping resources.yaml with the built-in kinds risks quietly reintroducing the hardcoded kind list #26 set out to remove. TestBuiltInConfigIsRedundant compares every built-in kind's configured result against its inferred result and requires they agree — so the file is an accelerator, not knowledge, and deleting it would change speed and nothing else.

It's load-bearing, not decorative — pointing one built-in entry at a wrong path fails it:

--- FAIL: TestBuiltInConfigIsRedundant/Deployment
    configured = [], inferred = [app:2] — they must agree

What it costs

A note on #75

This makes the planned "seen but not detected" warning both rare and actionable for the first time: a document that is neither configured nor yields anything from the walk is exactly the case worth warning about, and --config is the remedy to point at. Under #82 the warning has no remedy; under #84 nearly every custom resource trips it.

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

claude added 2 commits August 13, 2026 07:53
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 reports nonsense with nothing checking it. Since 0.4.4 that is
no longer quite true: 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. Verified against the binary:

  $ kir --config bad-expr.yaml rollout.yaml
  error: rollout.yaml: invalid image reference "not a valid image!!": ...
  # exit 1

What survives is an expression 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
@MPV
MPV force-pushed the claude/podspec-e-inferred-with-overrides branch from 5df80d4 to 0623814 Compare August 13, 2026 07:54
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