diff --git a/README.md b/README.md index a90d997..0dcd7c9 100644 --- a/README.md +++ b/README.md @@ -66,14 +66,33 @@ $ go run main.go approvals/kir_test.TestKind.Job.input.yaml | xargs docker scout ## How `kir` treats each document -A manifest stream usually mixes workloads with other objects. `kir` handles each by kind: +A manifest stream usually mixes workloads with other objects. `kir` handles each by what it contains, not by its kind: | Document | Result | | --- | --- | -| A workload — `Pod`, `Deployment`, `DaemonSet`, `ReplicaSet`, `StatefulSet`, `Job`, `CronJob` | its images are printed to stdout | -| A valid object with no images — `Service`, `ConfigMap`, `Secret`, … | skipped silently (exit 0) | +| Anything containing a `PodSpec` — `Pod`, `Deployment`, …, `CronJob`, and custom resources like an Argo `Rollout` | its images are printed to stdout | +| A valid object with no `PodSpec` — `Service`, `ConfigMap`, `Secret`, … | skipped silently (exit 0) | | Malformed or unreadable input | reported on stderr, non-zero exit | | A workload whose image value isn't a valid image reference | that image is reported on stderr with a non-zero exit; the document's other images are still printed | -| An unrecognized custom resource (CRD) | skipped for now — see [#75](https://github.com/MPV/kir/issues/75) | +| A resource whose images aren't in a `PodSpec` — an Argo `Workflow` | describe it with `--config` | So stdout carries only images and stderr stays quiet for normal input. See [ADR 0007](docs/adr/0007-document-classification.md) for the rationale. + +There is no list of supported kinds. A document yields images if it holds something shaped like a `PodSpec` — matched by decoding it against the Kubernetes API types — so a custom resource that embeds one works without `kir` knowing anything about it. + +### Teaching `kir` about a resource it can't infer + +Some resources keep images somewhere that isn't a `PodSpec`, so there is no shape to recognise. An Argo `Workflow` is the common case: a list of templates, each holding a container, a script, or neither. Describe those with `--config`, using [JMESPath](https://jmespath.org): + +```yaml +# workflows.yaml +resources: + - kind: Workflow + containers: ["spec.templates[*].[container, script][]"] +``` + +```shell +$ kir --config workflows.yaml manifests/ +``` + +An entry wins for its kind, so this also *corrects* `kir` where inference gets something wrong — an entry with no expressions silences a kind entirely. Everything not described this way is still inferred, so most manifests need no configuration at all. The built-in entries in [`k8s/resources.yaml`](k8s/resources.yaml) are only a shortcut for the common kinds: delete them and `kir` finds the same images, just more slowly. diff --git a/approvals/kir_test.TestCLI.Usage.stderr.approved.txt b/approvals/kir_test.TestCLI.Usage.stderr.approved.txt index f4c597f..e78f7b8 100644 --- a/approvals/kir_test.TestCLI.Usage.stderr.approved.txt +++ b/approvals/kir_test.TestCLI.Usage.stderr.approved.txt @@ -1 +1 @@ -Usage: kir [ ...] | kir - | kir --version +Usage: kir [--config ] [ ...] | kir - | kir --version diff --git a/approvals/kir_test.TestCustomResource.Inferred.exitcode.approved.txt b/approvals/kir_test.TestCustomResource.Inferred.exitcode.approved.txt new file mode 100644 index 0000000..573541a --- /dev/null +++ b/approvals/kir_test.TestCustomResource.Inferred.exitcode.approved.txt @@ -0,0 +1 @@ +0 diff --git a/approvals/kir_test.TestCustomResource.Inferred.stderr.approved.txt b/approvals/kir_test.TestCustomResource.Inferred.stderr.approved.txt new file mode 100644 index 0000000..e69de29 diff --git a/approvals/kir_test.TestCustomResource.Inferred.stdout.approved.txt b/approvals/kir_test.TestCustomResource.Inferred.stdout.approved.txt new file mode 100644 index 0000000..0815858 --- /dev/null +++ b/approvals/kir_test.TestCustomResource.Inferred.stdout.approved.txt @@ -0,0 +1,2 @@ +my-registry/app:1.4.2 +busybox:1.36 diff --git a/approvals/kir_test.TestCustomResource.Workflow.config.yaml b/approvals/kir_test.TestCustomResource.Workflow.config.yaml new file mode 100644 index 0000000..9d4ec66 --- /dev/null +++ b/approvals/kir_test.TestCustomResource.Workflow.config.yaml @@ -0,0 +1,7 @@ +# An Argo Workflow keeps images in a list of templates, and each template holds +# either a container, a script, or neither (a dag, a suspend). One expression +# covers it: select both shapes from every template, flatten, and the templates +# that have neither drop out. +resources: + - kind: Workflow + containers: ["spec.templates[*].[container, script][]"] diff --git a/approvals/kir_test.TestCustomResource.Workflow.input.yaml b/approvals/kir_test.TestCustomResource.Workflow.input.yaml new file mode 100644 index 0000000..93a0dd7 --- /dev/null +++ b/approvals/kir_test.TestCustomResource.Workflow.input.yaml @@ -0,0 +1,23 @@ +apiVersion: argoproj.io/v1alpha1 +kind: Workflow +metadata: + name: pipeline +spec: + entrypoint: main + templates: + - name: main + dag: + tasks: + - name: build + template: build + - name: build + container: + image: builder:1.2.0 + command: [make] + - name: report + script: + image: python:3.12 + source: | + print("done") + - name: approve + suspend: {} diff --git a/approvals/kir_test.TestCustomResource.WorkflowConfigured.exitcode.approved.txt b/approvals/kir_test.TestCustomResource.WorkflowConfigured.exitcode.approved.txt new file mode 100644 index 0000000..573541a --- /dev/null +++ b/approvals/kir_test.TestCustomResource.WorkflowConfigured.exitcode.approved.txt @@ -0,0 +1 @@ +0 diff --git a/approvals/kir_test.TestCustomResource.WorkflowConfigured.stderr.approved.txt b/approvals/kir_test.TestCustomResource.WorkflowConfigured.stderr.approved.txt new file mode 100644 index 0000000..e69de29 diff --git a/approvals/kir_test.TestCustomResource.WorkflowConfigured.stdout.approved.txt b/approvals/kir_test.TestCustomResource.WorkflowConfigured.stdout.approved.txt new file mode 100644 index 0000000..95aac7e --- /dev/null +++ b/approvals/kir_test.TestCustomResource.WorkflowConfigured.stdout.approved.txt @@ -0,0 +1,2 @@ +builder:1.2.0 +python:3.12 diff --git a/approvals/kir_test.TestCustomResource.WorkflowUndescribed.exitcode.approved.txt b/approvals/kir_test.TestCustomResource.WorkflowUndescribed.exitcode.approved.txt new file mode 100644 index 0000000..573541a --- /dev/null +++ b/approvals/kir_test.TestCustomResource.WorkflowUndescribed.exitcode.approved.txt @@ -0,0 +1 @@ +0 diff --git a/approvals/kir_test.TestCustomResource.WorkflowUndescribed.stderr.approved.txt b/approvals/kir_test.TestCustomResource.WorkflowUndescribed.stderr.approved.txt new file mode 100644 index 0000000..e69de29 diff --git a/approvals/kir_test.TestCustomResource.WorkflowUndescribed.stdout.approved.txt b/approvals/kir_test.TestCustomResource.WorkflowUndescribed.stdout.approved.txt new file mode 100644 index 0000000..e69de29 diff --git a/approvals/kir_test.TestCustomResource.input.yaml b/approvals/kir_test.TestCustomResource.input.yaml new file mode 100644 index 0000000..0942193 --- /dev/null +++ b/approvals/kir_test.TestCustomResource.input.yaml @@ -0,0 +1,18 @@ +apiVersion: argoproj.io/v1alpha1 +kind: Rollout +metadata: + name: my-rollout +spec: + replicas: 3 + strategy: + canary: + steps: + - setWeight: 20 + template: + spec: + containers: + - name: app + image: my-registry/app:1.4.2 + initContainers: + - name: setup + image: busybox:1.36 diff --git a/approvals/kir_test.TestFailure.BadYAML.stderr.approved.txt b/approvals/kir_test.TestFailure.BadYAML.stderr.approved.txt index 49591ae..d43cd99 100644 --- a/approvals/kir_test.TestFailure.BadYAML.stderr.approved.txt +++ b/approvals/kir_test.TestFailure.BadYAML.stderr.approved.txt @@ -1 +1 @@ -error: yaml: line 9: did not find expected ',' or ']' +error: error converting YAML to JSON: yaml: line 9: did not find expected ',' or ']' diff --git a/approvals/kir_test.TestFailure.PartialStream.stderr.approved.txt b/approvals/kir_test.TestFailure.PartialStream.stderr.approved.txt index 49591ae..d43cd99 100644 --- a/approvals/kir_test.TestFailure.PartialStream.stderr.approved.txt +++ b/approvals/kir_test.TestFailure.PartialStream.stderr.approved.txt @@ -1 +1 @@ -error: yaml: line 9: did not find expected ',' or ']' +error: error converting YAML to JSON: yaml: line 9: did not find expected ',' or ']' diff --git a/approvals/kir_test.TestKind.PodTemplate.exitcode.approved.txt b/approvals/kir_test.TestKind.PodTemplate.exitcode.approved.txt new file mode 100644 index 0000000..573541a --- /dev/null +++ b/approvals/kir_test.TestKind.PodTemplate.exitcode.approved.txt @@ -0,0 +1 @@ +0 diff --git a/approvals/kir_test.TestKind.PodTemplate.input.yaml b/approvals/kir_test.TestKind.PodTemplate.input.yaml new file mode 100644 index 0000000..09b29a7 --- /dev/null +++ b/approvals/kir_test.TestKind.PodTemplate.input.yaml @@ -0,0 +1,9 @@ +apiVersion: v1 +kind: PodTemplate +metadata: + name: tmpl +template: + spec: + containers: + - name: worker + image: worker:3.1 diff --git a/approvals/kir_test.TestKind.PodTemplate.stderr.approved.txt b/approvals/kir_test.TestKind.PodTemplate.stderr.approved.txt new file mode 100644 index 0000000..e69de29 diff --git a/approvals/kir_test.TestKind.PodTemplate.stdout.approved.txt b/approvals/kir_test.TestKind.PodTemplate.stdout.approved.txt new file mode 100644 index 0000000..94a335b --- /dev/null +++ b/approvals/kir_test.TestKind.PodTemplate.stdout.approved.txt @@ -0,0 +1 @@ +worker:3.1 diff --git a/approvals/kir_test.TestKind.ReplicationController.exitcode.approved.txt b/approvals/kir_test.TestKind.ReplicationController.exitcode.approved.txt new file mode 100644 index 0000000..573541a --- /dev/null +++ b/approvals/kir_test.TestKind.ReplicationController.exitcode.approved.txt @@ -0,0 +1 @@ +0 diff --git a/approvals/kir_test.TestKind.ReplicationController.input.yaml b/approvals/kir_test.TestKind.ReplicationController.input.yaml new file mode 100644 index 0000000..b24833e --- /dev/null +++ b/approvals/kir_test.TestKind.ReplicationController.input.yaml @@ -0,0 +1,11 @@ +apiVersion: v1 +kind: ReplicationController +metadata: + name: legacy +spec: + replicas: 2 + template: + spec: + containers: + - name: web + image: nginx:1.27 diff --git a/approvals/kir_test.TestKind.ReplicationController.stderr.approved.txt b/approvals/kir_test.TestKind.ReplicationController.stderr.approved.txt new file mode 100644 index 0000000..e69de29 diff --git a/approvals/kir_test.TestKind.ReplicationController.stdout.approved.txt b/approvals/kir_test.TestKind.ReplicationController.stdout.approved.txt new file mode 100644 index 0000000..20a9c2f --- /dev/null +++ b/approvals/kir_test.TestKind.ReplicationController.stdout.approved.txt @@ -0,0 +1 @@ +nginx:1.27 diff --git a/approvals/kir_test.TestSkipsNonWorkloads.Lookalike.exitcode.approved.txt b/approvals/kir_test.TestSkipsNonWorkloads.Lookalike.exitcode.approved.txt new file mode 100644 index 0000000..573541a --- /dev/null +++ b/approvals/kir_test.TestSkipsNonWorkloads.Lookalike.exitcode.approved.txt @@ -0,0 +1 @@ +0 diff --git a/approvals/kir_test.TestSkipsNonWorkloads.Lookalike.input.yaml b/approvals/kir_test.TestSkipsNonWorkloads.Lookalike.input.yaml new file mode 100644 index 0000000..f66e324 --- /dev/null +++ b/approvals/kir_test.TestSkipsNonWorkloads.Lookalike.input.yaml @@ -0,0 +1,9 @@ +apiVersion: logistics.example.com/v1 +kind: ShippingManifest +metadata: + name: not-a-pod +spec: + containers: + - name: cargo-hold-1 + capacity: 40ft + image: photo-of-container.jpg diff --git a/approvals/kir_test.TestSkipsNonWorkloads.Lookalike.stderr.approved.txt b/approvals/kir_test.TestSkipsNonWorkloads.Lookalike.stderr.approved.txt new file mode 100644 index 0000000..e69de29 diff --git a/approvals/kir_test.TestSkipsNonWorkloads.Lookalike.stdout.approved.txt b/approvals/kir_test.TestSkipsNonWorkloads.Lookalike.stdout.approved.txt new file mode 100644 index 0000000..e69de29 diff --git a/approvals/kir_test.go b/approvals/kir_test.go index 04effb3..6aa7b8a 100644 --- a/approvals/kir_test.go +++ b/approvals/kir_test.go @@ -42,7 +42,9 @@ func newlineTerminated(s string) string { } func TestKind(t *testing.T) { - kinds := []string{"Pod", "CronJob", "DaemonSet", "Deployment", "Job", "ReplicaSet", "StatefulSet"} + // PodTemplate and ReplicationController are built-in kinds carrying a + // PodSpec that the previous fixed kind list omitted. + kinds := []string{"Pod", "CronJob", "DaemonSet", "Deployment", "Job", "PodTemplate", "ReplicaSet", "ReplicationController", "StatefulSet"} for _, kind := range kinds { t.Run(kind, func(t *testing.T) { @@ -51,10 +53,40 @@ func TestKind(t *testing.T) { } } -// A non-workload kind (Service) is skipped: no images, no error, exit 0. +// A non-workload document is skipped: no images, no error, exit 0. Service is +// a built-in without a PodSpec; Lookalike is a custom resource with a field +// named containers holding something that is not a container, which pins the +// cost of matching on shape — a name alone must not be enough to match. func TestSkipsNonWorkloads(t *testing.T) { - t.Run("Service", func(t *testing.T) { - verify(t, []string{"kir_test.TestSkipsNonWorkloads.Service.input.yaml"}, nil) + for _, name := range []string{"Service", "Lookalike"} { + t.Run(name, func(t *testing.T) { + verify(t, []string{"kir_test.TestSkipsNonWorkloads." + name + ".input.yaml"}, nil) + }) + } +} + +// The two halves of how kir reads a custom resource. +// +// Rollout is inferred: it embeds a PodSpec, so the walk finds its images with +// no configuration and nothing in kir naming that kind. Workflow has to be +// described: its images sit in bare containers across a list of templates, +// which is not a PodSpec and so has no shape to match — the case configuration +// exists for. Both are pinned, because the pair is the whole argument for +// having both mechanisms. +func TestCustomResource(t *testing.T) { + t.Run("Inferred", func(t *testing.T) { + verify(t, []string{"kir_test.TestCustomResource.input.yaml"}, nil) + }) + + t.Run("WorkflowUndescribed", func(t *testing.T) { + verify(t, []string{"kir_test.TestCustomResource.Workflow.input.yaml"}, nil) + }) + + t.Run("WorkflowConfigured", func(t *testing.T) { + verify(t, []string{ + "--config", "kir_test.TestCustomResource.Workflow.config.yaml", + "kir_test.TestCustomResource.Workflow.input.yaml", + }, nil) }) } diff --git a/cmd/cmd.go b/cmd/cmd.go index 3dea33e..2f3af0b 100644 --- a/cmd/cmd.go +++ b/cmd/cmd.go @@ -4,10 +4,12 @@ import ( "fmt" "io" "log" + "os" "strings" "github.com/mpv/kir/fileutil" "github.com/mpv/kir/imageref" + "github.com/mpv/kir/k8s" "github.com/mpv/kir/processor" ) @@ -34,7 +36,13 @@ func Run(args []string, stdin io.Reader, stdout, stderr io.Writer) int { logger := log.New(stderr, "", 0) if len(args) == 0 { - logger.Print("Usage: kir [ ...] | kir - | kir --version") + logger.Print("Usage: kir [--config ] [ ...] | kir - | kir --version") + return 1 + } + + args, config, err := configFlag(args) + if err != nil { + logger.Printf("error: %v", err) return 1 } @@ -44,7 +52,7 @@ func Run(args []string, stdin io.Reader, stdout, stderr io.Writer) int { if stdin == nil { stdin = strings.NewReader("") } - images, err := processor.ProcessStdin(stdin) + images, err := processor.ProcessStdin(config, stdin) failures := logErrors(logger, err) failures += printImages(stdout, logger, "stdin", images) if failures > 0 { @@ -64,7 +72,7 @@ func Run(args []string, stdin io.Reader, stdout, stderr io.Writer) int { } failures := 0 for _, filePath := range files { - images, err := processor.ProcessFile(filePath) + images, err := processor.ProcessFile(config, filePath) // Not `continue`: a file that failed on one document may still have // yielded images from the others, and dropping them would defeat the // point of reporting the failure. @@ -77,6 +85,30 @@ func Run(args []string, stdin io.Reader, stdout, stderr io.Writer) int { return 0 } +// configFlag consumes a leading `--config ` and returns the +// remaining arguments alongside the configuration to use. The file is merged +// over the built-in one, so a resource can be described — or a built-in +// corrected — without rebuilding kir. +func configFlag(args []string) ([]string, *k8s.Config, error) { + config := k8s.DefaultConfig() + if len(args) == 0 || args[0] != "--config" { + return args, config, nil + } + if len(args) < 2 { + return nil, nil, fmt.Errorf("--config requires a file") + } + + data, err := os.ReadFile(args[1]) + if err != nil { + return nil, nil, fmt.Errorf("error reading config: %v", err) + } + extra, err := k8s.LoadConfig(data) + if err != nil { + return nil, nil, err + } + return args[2:], config.Merge(extra), nil +} + // logErrors writes one "error:" line per failure and returns how many it wrote. // // A single input can fail on more than one document, and ProcessReader packs diff --git a/docs/adr/0009-podspec-discovery.md b/docs/adr/0009-podspec-discovery.md new file mode 100644 index 0000000..b6aae69 --- /dev/null +++ b/docs/adr/0009-podspec-discovery.md @@ -0,0 +1,80 @@ +# 9. Infer images structurally, with configured overrides + +- Status: **proposed** — a fifth candidate for #26, combining the approaches in #82 and #84; supersedes [0001](0001-typed-kubernetes-decoding.md) if accepted +- Date: 2026-08-09 + +## Context + +[ADR 0001](0001-typed-kubernetes-decoding.md) decodes each document with the +typed client-go scheme and reads the PodSpec through a type switch over seven +hardcoded kinds. #26 asks for images to be *found* rather than looked up, so +custom resources work too. + +Four candidates were raised and measured (#81–#84). Two of them are the ones +that matter, and each fails where the other succeeds: + +- **Structural inference** (#82) recognises anything shaped like a PodSpec, with + no configuration — but a resource holding *bare containers* has no PodSpec + shape to match, so an Argo `Workflow` is invisible to it, with no way for a + user to say otherwise. +- **Configured expressions** (#84) reach anything a user can write an expression + for, exactly — but every Argo, Knative or in-house CRD stays invisible until + somebody writes that file, including resources whose shape already speaks for + itself. + +## Decision + +Do 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 by the structural walk. The two never both contribute to one document, +so an image cannot be reported twice. + +The built-in `resources.yaml` keeps entries for the built-in kinds, but **as an +accelerator, not as knowledge**. Deleting every entry changes no answer, only +speed — `TestBuiltInConfigIsRedundant` pins exactly that, comparing each +built-in kind's configured result against its inferred one. Without that test +`resources.yaml` would quietly become the hardcoded kind list #26 set out to +remove. + +## Consequences + +The union of both reaches, and one capability neither has alone: + +- Built-in workloads, and custom resources embedding a PodSpec (Argo `Rollout`), + need **no configuration** — inferred. +- Resources holding bare containers (Argo `Workflow`) are reachable, which + inference alone cannot do. +- An entry with **no expressions silences a kind**, so a user can overrule the + walk when it reads something wrongly. Inference alone cannot be told to + ignore; configuration alone has nothing to ignore. + +It is also **cheaper than inference alone on typical input**: with the built-ins +configured, ordinary manifests take the exact lookup and the walk never runs. +Over 1000 Deployments, 107 ms — against 171 ms for inference alone (#82), and +near configuration alone's 92 ms (#84). Only kinds nobody has described pay for +the walk, which is the reverse of the usual cost of combining two mechanisms. + +The costs: + +- **Two mechanisms** to document and reason about, where each of #82 and #84 has + one. The precedence rule is the whole of the extra contract, but it is a + contract. +- `k8s.io/api` (the inference schema) *and* `go-jmespath` (the expression + engine): 12.6 MB and 78 `go.sum` lines. Larger than configuration alone + (4.1 MB, 42) and barely above inference alone (12.4 MB, 72); still less than + half of today's 27.2 MB. +- Precision is uneven by design. The walk validates shape against the Kubernetes + types and rejects lookalikes; a configured expression is taken at its word, + and nothing checks that what it selects is a container. ADR 0008's reference + validation blunts this — since 0.4.4 a selected value that is not a reportable + image reference is named on stderr with a non-zero exit rather than printed — + so a mis-aimed expression fails loudly instead of silently. What survives is + an expression selecting something that merely *looks* like a reference. + +This also makes the "seen but not detected" warning planned in #75 both rare and +actionable for the first time: a document that is neither configured nor yields +anything from the walk is precisely the case worth reporting, and `--config` is +the remedy to point the user at. Under inference alone the warning has no +remedy; under configuration alone nearly every custom resource trips it. diff --git a/docs/adr/README.md b/docs/adr/README.md index 6dd9e53..b67e8b6 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -18,3 +18,4 @@ date the decision was actually made. | [0006](0006-conventional-commits-and-releases.md) | Automate releases from Conventional Commits | 2026-08-06 | | [0007](0007-document-classification.md) | How kir classifies each document (workload / image-less / unprocessable) | 2026-08-08 | | [0008](0008-best-effort-processing-and-exit-codes.md) | Best-effort processing; failures surface via the exit code | 2026-08-09 | +| [0009](0009-podspec-discovery.md) | Infer images structurally, with configured overrides (proposed — #26) | 2026-08-09 | diff --git a/go.mod b/go.mod index bf15914..ca05ac6 100644 --- a/go.mod +++ b/go.mod @@ -5,9 +5,10 @@ go 1.26.0 require ( github.com/approvals/go-approval-tests v1.14.0 github.com/distribution/reference v0.6.0 + github.com/jmespath/go-jmespath v0.4.0 k8s.io/api v0.36.3 k8s.io/apimachinery v0.36.3 - k8s.io/client-go v0.36.3 + sigs.k8s.io/yaml v1.6.0 ) require ( @@ -29,5 +30,4 @@ require ( sigs.k8s.io/json v0.0.0-20250730193827-2d320260d730 // indirect sigs.k8s.io/randfill v1.0.0 // indirect sigs.k8s.io/structured-merge-diff/v6 v6.3.3 // indirect - sigs.k8s.io/yaml v1.6.0 // indirect ) diff --git a/go.sum b/go.sum index c7f6c0f..fcc1662 100644 --- a/go.sum +++ b/go.sum @@ -14,6 +14,10 @@ github.com/go-logr/logr v1.4.3/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ4 github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8= github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX3N/iU= github.com/google/gofuzz v1.0.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg= +github.com/jmespath/go-jmespath v0.4.0 h1:BEgLn5cpjn8UN1mAw4NjwDrS35OdebyEtFe+9YPoQUg= +github.com/jmespath/go-jmespath v0.4.0/go.mod h1:T8mJZnbsbmF+m6zOOFylbeCJqk5+pHWvzYPziyZiYoo= +github.com/jmespath/go-jmespath/internal/testify v1.5.1 h1:shLQSRRSCCPj3f2gpwzGwWFoC7ycTf1rcQZHOlsJ6N8= +github.com/jmespath/go-jmespath/internal/testify v1.5.1/go.mod h1:L3OGu8Wl2/fWfCI6z80xFu9LTZmf1ZRjMHUOPmWr69U= github.com/json-iterator/go v1.1.12 h1:PV8peI4a0ysnczrg+LtxykD8LfKY9ML6u2jnxaEnrnM= github.com/json-iterator/go v1.1.12/go.mod h1:e30LSqwooZae/UwlEbR2852Gd8hjQvJoHmT4TnhNGBo= github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE= @@ -54,14 +58,14 @@ gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntN gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EVd6muEfDQjcINNoR0C8j2r3qZ4Q= gopkg.in/inf.v0 v0.9.1 h1:73M5CoZyi3ZLMOyDlQh031Cx6N9NDJ2Vvfl76EDAgDc= gopkg.in/inf.v0 v0.9.1/go.mod h1:cWUDdTG/fYaXco+Dcufb5Vnc6Gp2YChqWtbxRZE0mXw= +gopkg.in/yaml.v2 v2.2.8 h1:obN1ZagJSUGI0Ek/LBmuj4SNLPfIny3KsKFopxRdj10= +gopkg.in/yaml.v2 v2.2.8/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= k8s.io/api v0.36.3 h1:NxB+05W2UGqXWFXcLO0RB5cnqnUPP5v5sVlaOH0Iz4w= k8s.io/api v0.36.3/go.mod h1:JzLQKqRHC5+I8RVj/lS3lCg0mg6nWI9Fo/Sk3ElxHzg= k8s.io/apimachinery v0.36.3 h1:PkzMRBRG8joFD8EhCuQAtNPvJlxb82FwplP26HIzvAM= k8s.io/apimachinery v0.36.3/go.mod h1:cTSjBWgPe/6CQyBKzY/hDIRWCQQQeK0mfLbml0UYFHE= -k8s.io/client-go v0.36.3 h1:M4JdVzXxYcZk4fGpfDdYnxSwhLKWCFoQsHW6t+z8Hfg= -k8s.io/client-go v0.36.3/go.mod h1:gcPwr0c87vjjG6HB6pWEqOeuYVoXSsREjzux2j6GF30= k8s.io/klog/v2 v2.140.0 h1:Tf+J3AH7xnUzZyVVXhTgGhEKnFqye14aadWv7bzXdzc= k8s.io/klog/v2 v2.140.0/go.mod h1:o+/RWfJ6PwpnFn7OyAG3QnO47BFsymfEfrz6XyYSSp0= k8s.io/kube-openapi v0.0.0-20260317180543-43fb72c5454a h1:xCeOEAOoGYl2jnJoHkC3hkbPJgdATINPMAxaynU2Ovg= diff --git a/k8s/config.go b/k8s/config.go new file mode 100644 index 0000000..eb66600 --- /dev/null +++ b/k8s/config.go @@ -0,0 +1,227 @@ +package k8s + +import ( + _ "embed" + "fmt" + + "github.com/jmespath/go-jmespath" + "sigs.k8s.io/yaml" +) + +//go:embed resources.yaml +var defaultResources string + +// Resource says where one kind keeps its images, as JMESPath expressions. +type Resource struct { + Kind string `json:"kind"` + // PodSpecs select PodSpec-shaped nodes, whose container fields are read. + PodSpecs []string `json:"podSpecs,omitempty"` + // Containers select containers directly, for resources that hold bare + // containers rather than a PodSpec — which the walk cannot recognise, + // there being no PodSpec shape to match. + Containers []string `json:"containers,omitempty"` + // Documents select whole objects, each processed in its own right. + Documents []string `json:"documents,omitempty"` +} + +// Config maps kinds to where they keep their images. A kind with no entry is +// not unsupported — it is inferred by the structural walk instead. +type Config struct { + Resources []Resource `json:"resources"` + + byKind map[string]expressions +} + +// expressions holds one resource's compiled queries. +type expressions struct { + podSpecs []*jmespath.JMESPath + containers []*jmespath.JMESPath + documents []*jmespath.JMESPath +} + +// LoadConfig parses a resource configuration and compiles its expressions. A +// malformed expression is an error here rather than one that silently matches +// nothing later. +func LoadConfig(data []byte) (*Config, error) { + var config Config + if err := yaml.UnmarshalStrict(data, &config); err != nil { + return nil, fmt.Errorf("parsing config: %w", err) + } + if err := config.compile(); err != nil { + return nil, err + } + return &config, nil +} + +// DefaultConfig returns the built-in configuration. +func DefaultConfig() *Config { + config, err := LoadConfig([]byte(defaultResources)) + if err != nil { + // The embedded config is loaded in tests; a failure here is a bug. + panic(fmt.Sprintf("embedded config does not load: %v", err)) + } + return config +} + +// Merge returns a copy of c with other's entries applied over it. An entry for +// a kind already configured replaces it, so a user can correct a built-in as +// well as add a resource. +func (c *Config) Merge(other *Config) *Config { + merged := &Config{Resources: append(append([]Resource{}, c.Resources...), other.Resources...)} + // Both halves compiled when they were loaded, so this cannot fail. + if err := merged.compile(); err != nil { + panic(fmt.Sprintf("merging already-compiled configs: %v", err)) + } + return merged +} + +func (c *Config) compile() error { + c.byKind = make(map[string]expressions, len(c.Resources)) + for _, resource := range c.Resources { + var compiled expressions + var err error + if compiled.podSpecs, err = compileAll(resource.Kind, "podSpecs", resource.PodSpecs); err != nil { + return err + } + if compiled.containers, err = compileAll(resource.Kind, "containers", resource.Containers); err != nil { + return err + } + if compiled.documents, err = compileAll(resource.Kind, "documents", resource.Documents); err != nil { + return err + } + c.byKind[resource.Kind] = compiled + } + return nil +} + +func compileAll(kind, field string, exprs []string) ([]*jmespath.JMESPath, error) { + compiled := make([]*jmespath.JMESPath, 0, len(exprs)) + for _, expr := range exprs { + parsed, err := jmespath.Compile(expr) + if err != nil { + return nil, fmt.Errorf("%s.%s: %q: %w", kind, field, expr, err) + } + compiled = append(compiled, parsed) + } + return compiled, nil +} + +// Kinds returns the configured kinds, for diagnostics. +func (c *Config) Kinds() []string { + kinds := make([]string, 0, len(c.Resources)) + for _, resource := range c.Resources { + kinds = append(kinds, resource.Kind) + } + return kinds +} + +// FindImages returns the images of doc, a manifest decoded into plain Go +// values. +// +// An entry for the document's kind decides on its own: its expressions are +// followed and the walk is not consulted. Everything else is inferred +// structurally. So configuration is never needed for a document whose shape +// speaks for itself, and always available for one whose does not — a resource +// holding bare containers, or one the walk reads wrongly, which an entry with +// no expressions silences outright. +// +// The two never both contribute to the same document, so an image cannot be +// reported twice. +func (c *Config) FindImages(doc map[string]any) []string { + if images, configured := c.lookup(doc); configured { + return images + } + return infer(doc) +} + +// lookup applies the configured expressions for doc's kind, reporting whether +// the kind was configured at all — which is what makes an entry with no +// expressions mean "this kind has no images", rather than "fall back". +func (c *Config) lookup(doc map[string]any) ([]string, bool) { + kind, _ := doc["kind"].(string) + resource, ok := c.byKind[kind] + if !ok { + return nil, false + } + + var images []string + for _, expr := range resource.podSpecs { + for _, node := range search(expr, doc) { + images = append(images, configuredPodSpecImages(node)...) + } + } + for _, expr := range resource.containers { + for _, node := range search(expr, doc) { + images = append(images, containerImages(node)...) + } + } + // A nested document goes back through FindImages, so an item of a + // configured List is itself either configured or inferred. + for _, expr := range resource.documents { + for _, node := range search(expr, doc) { + if nested, ok := node.(map[string]any); ok { + images = append(images, c.FindImages(nested)...) + } + } + } + return images, true +} + +// search runs one expression and returns the nodes it selected. +// +// A JMESPath projection yields one entry per input element, null where the +// field is absent — `spec.templates[*].container` over templates that hold no +// container, say — so nulls are dropped rather than counted as matches. +func search(expr *jmespath.JMESPath, doc any) []any { + result, err := expr.Search(doc) + if err != nil || result == nil { + return nil + } + + list, ok := result.([]any) + if !ok { + return []any{result} + } + + nodes := make([]any, 0, len(list)) + for _, node := range list { + if node != nil { + nodes = append(nodes, node) + } + } + return nodes +} + +// configuredPodSpecImages reads the images out of a node the configuration has +// declared to be a PodSpec. Unlike the walk, nothing here checks that claim: +// an explicit entry is taken at its word. +func configuredPodSpecImages(node any) []string { + podSpec, ok := node.(map[string]any) + if !ok { + return nil + } + + var images []string + for _, field := range containerFields { + images = append(images, containerImages(podSpec[field])...) + } + return images +} + +// containerImages reads the image of a container, or of every container in a +// list of them. +func containerImages(node any) []string { + switch n := node.(type) { + case []any: + var images []string + for _, item := range n { + images = append(images, containerImages(item)...) + } + return images + case map[string]any: + if image, ok := n["image"].(string); ok && image != "" { + return []string{image} + } + } + return nil +} diff --git a/k8s/config_test.go b/k8s/config_test.go new file mode 100644 index 0000000..533d348 --- /dev/null +++ b/k8s/config_test.go @@ -0,0 +1,217 @@ +package k8s + +import ( + "slices" + "strings" + "testing" + + "sigs.k8s.io/yaml" +) + +// decodeDoc turns a manifest into a whole document, which is what Config reads. +// The walk's own helper yields `any`, since it descends into fragments too. +func decodeDoc(t *testing.T, manifest string) map[string]any { + t.Helper() + var doc map[string]any + if err := yaml.Unmarshal([]byte(manifest), &doc); err != nil { + t.Fatalf("decoding fixture: %v", err) + } + return doc +} + +func TestDefaultConfigLoads(t *testing.T) { + if len(DefaultConfig().Kinds()) == 0 { + t.Fatal("embedded config describes no kinds") + } +} + +// The built-in entries are an accelerator, not knowledge: they save the walk +// from re-deriving where a Deployment keeps its PodSpec, but they are not the +// reason kir understands one. Deleting them all must change no answer. +// +// This is what stops resources.yaml from quietly becoming the hardcoded kind +// list that #26 set out to remove. +func TestBuiltInConfigIsRedundant(t *testing.T) { + manifests := map[string]string{ + "Pod": ` +kind: Pod +spec: + containers: [{name: app, image: app:1}] +`, + "Deployment": ` +kind: Deployment +spec: + template: + spec: + containers: [{name: app, image: app:2}] +`, + "CronJob": ` +kind: CronJob +spec: + jobTemplate: + spec: + template: + spec: + containers: [{name: app, image: app:3}] +`, + "PodTemplate": ` +kind: PodTemplate +template: + spec: + containers: [{name: app, image: app:4}] +`, + "ReplicationController": ` +kind: ReplicationController +spec: + template: + spec: + containers: [{name: app, image: app:5}] +`, + "List": ` +kind: List +items: +- kind: Pod + spec: + containers: [{name: a, image: a:1}] +- kind: Deployment + spec: + template: + spec: + containers: [{name: b, image: b:1}] +`, + } + + configured := DefaultConfig() + empty, err := LoadConfig([]byte("resources: []\n")) + if err != nil { + t.Fatalf("LoadConfig() error = %v", err) + } + + for kind, manifest := range manifests { + t.Run(kind, func(t *testing.T) { + doc := decodeDoc(t, manifest) + viaConfig := configured.FindImages(doc) + viaWalk := empty.FindImages(doc) + if len(viaWalk) == 0 { + t.Fatalf("walk alone found nothing for %s", kind) + } + if !slices.Equal(viaConfig, viaWalk) { + t.Errorf("configured = %v, inferred = %v — they must agree", viaConfig, viaWalk) + } + }) + } +} + +// A configured kind is looked up and never also inferred, so an image cannot be +// reported twice for describing something kir would have found anyway. +func TestConfiguredKindIsNotAlsoInferred(t *testing.T) { + rollout := decodeDoc(t, ` +kind: Rollout +spec: + template: + spec: + containers: [{name: app, image: app:1.4.2}] +`) + + inferred := DefaultConfig().FindImages(rollout) + if want := []string{"app:1.4.2"}; !slices.Equal(inferred, want) { + t.Fatalf("inferred = %v, want %v", inferred, want) + } + + extra, err := LoadConfig([]byte("resources:\n - kind: Rollout\n podSpecs: [spec.template.spec]\n")) + if err != nil { + t.Fatalf("LoadConfig() error = %v", err) + } + if got := DefaultConfig().Merge(extra).FindImages(rollout); !slices.Equal(got, inferred) { + t.Errorf("configured = %v, inferred = %v — describing a kind must not duplicate it", got, inferred) + } +} + +// An entry with no expressions means "this kind has no images", which is how a +// user overrules the walk. Neither mechanism can do this alone: the walk cannot +// be told to ignore something, and configuration alone has nothing to ignore. +func TestEmptyEntrySilencesAKind(t *testing.T) { + rollout := decodeDoc(t, ` +kind: Rollout +spec: + template: + spec: + containers: [{name: app, image: app:1.4.2}] +`) + + silence, err := LoadConfig([]byte("resources:\n - kind: Rollout\n podSpecs: []\n")) + if err != nil { + t.Fatalf("LoadConfig() error = %v", err) + } + if got := DefaultConfig().Merge(silence).FindImages(rollout); len(got) != 0 { + t.Errorf("FindImages() = %v, want no images", got) + } +} + +// The reach configuration adds: a resource holding bare containers, which has +// no PodSpec shape for the walk to match. +func TestConfigReachesBareContainers(t *testing.T) { + workflow := decodeDoc(t, ` +kind: Workflow +spec: + templates: + - name: build + container: {image: builder:1} + - name: report + script: {image: python:3.12} + - name: fanout + dag: {tasks: [{name: a}]} +`) + + if got := DefaultConfig().FindImages(workflow); len(got) != 0 { + t.Fatalf("undescribed Workflow = %v, want no images — the walk cannot see bare containers", got) + } + + extra, err := LoadConfig([]byte("resources:\n - kind: Workflow\n containers: [\"spec.templates[*].[container, script][]\"]\n")) + if err != nil { + t.Fatalf("LoadConfig() error = %v", err) + } + + want := []string{"builder:1", "python:3.12"} + if got := DefaultConfig().Merge(extra).FindImages(workflow); !slices.Equal(got, want) { + t.Errorf("FindImages() = %v, want %v", got, want) + } +} + +// Merging replaces a kind's expressions, so a built-in entry can be corrected +// and not merely extended. +func TestMergeReplacesExpressions(t *testing.T) { + pod := decodeDoc(t, ` +kind: Pod +elsewhere: + containers: [{name: app, image: app:1}] +`) + + extra, err := LoadConfig([]byte("resources:\n - kind: Pod\n podSpecs: [elsewhere]\n")) + if err != nil { + t.Fatalf("LoadConfig() error = %v", err) + } + + want := []string{"app:1"} + if got := DefaultConfig().Merge(extra).FindImages(pod); !slices.Equal(got, want) { + t.Errorf("FindImages() = %v, want %v", got, want) + } +} + +func TestLoadConfigRejectsUnknownFields(t *testing.T) { + if _, err := LoadConfig([]byte("resources:\n - kind: Pod\n podSpec: [spec]\n")); err == nil { + t.Error("LoadConfig() accepted an unknown field, want an error") + } +} + +// Expressions are compiled when the config loads, so a typo is an error naming +// the offending kind and field rather than one that silently matches nothing. +func TestLoadConfigRejectsBadExpression(t *testing.T) { + _, err := LoadConfig([]byte("resources:\n - kind: Pod\n podSpecs: [\"spec[\"]\n")) + if err == nil { + t.Fatal("LoadConfig() accepted a malformed expression, want an error") + } + if !strings.Contains(err.Error(), "Pod.podSpecs") { + t.Errorf("error = %q, want it to name the offending kind and field", err) + } +} diff --git a/k8s/k8s.go b/k8s/k8s.go index 1e88e2f..c777481 100644 --- a/k8s/k8s.go +++ b/k8s/k8s.go @@ -1,54 +1,121 @@ +// Package k8s finds container images in Kubernetes manifests that have been +// decoded into plain Go values. +// +// Images are found two ways. A structural walk infers them, matching nodes +// against the Kubernetes API types, which needs no configuration and reaches +// custom resources. A configuration (resources.yaml, plus anything the user +// supplies) states where a given kind keeps its images, which is exact and can +// reach what the walk cannot see. Config.FindImages puts them together: an +// entry wins for its kind, everything else is inferred. package k8s import ( - "fmt" + "encoding/json" + "maps" + "slices" - appsv1 "k8s.io/api/apps/v1" - batchv1 "k8s.io/api/batch/v1" corev1 "k8s.io/api/core/v1" + "sigs.k8s.io/yaml" ) -// GetPodSpec extracts the PodSpec from a Kubernetes object -func GetPodSpec(obj any) (*corev1.PodSpec, error) { - switch resource := obj.(type) { - case *corev1.Pod: - return &resource.Spec, nil - case *appsv1.Deployment: - return &resource.Spec.Template.Spec, nil - case *appsv1.DaemonSet: - return &resource.Spec.Template.Spec, nil - case *appsv1.ReplicaSet: - return &resource.Spec.Template.Spec, nil - case *appsv1.StatefulSet: - return &resource.Spec.Template.Spec, nil - case *batchv1.Job: - return &resource.Spec.Template.Spec, nil - case *batchv1.CronJob: - return &resource.Spec.JobTemplate.Spec.Template.Spec, nil - default: - return nil, fmt.Errorf("object does not have a PodSpec") +// containerFields are the PodSpec fields that carry images, listed in the order +// kir reports them. +var containerFields = []string{"containers", "initContainers", "ephemeralContainers"} + +// maxDepth bounds the walk. Real manifests nest a handful of levels; the limit +// only guards against absurd input. +const maxDepth = 100 + +// infer returns the images of every PodSpec-shaped node reachable from doc, a +// manifest decoded into plain Go values (maps, slices, scalars). +// +// Nothing here knows what a Deployment is. The walk descends until it meets a +// node shaped like a PodSpec, which is why a custom resource that embeds one is +// understood on the same footing as a built-in workload — and why a List needs +// no special case, its items being just more nodes. +func infer(doc any) []string { + var images []string + find(doc, &images, 0) + return images +} + +func find(node any, images *[]string, depth int) { + if depth > maxDepth { + return + } + + switch n := node.(type) { + case map[string]any: + if found, ok := podSpecImages(n); ok { + *images = append(*images, found...) + return // a PodSpec does not contain another PodSpec + } + // Sorted, so output order depends on the manifest rather than on Go's + // randomised map iteration. + for _, key := range slices.Sorted(maps.Keys(n)) { + find(n[key], images, depth+1) + } + case []any: + for _, item := range n { + find(item, images, depth+1) + } } } -func GetContainerImages(containers []corev1.Container) []string { +// podSpecImages reports whether node is PodSpec-shaped, and if so its images. +// +// The test is not "has a field called containers" but "does that field decode +// into the real corev1 type, rejecting unknown fields". The Kubernetes Go types +// are the schema, so a custom resource embedding a genuine PodSpec matches, +// while a lookalike — a field named containers holding something else — does +// not. +func podSpecImages(node map[string]any) ([]string, bool) { var images []string - for _, container := range containers { - images = append(images, container.Image) + matched := false + + for _, field := range containerFields { + value, ok := node[field] + if !ok { + continue + } + containers, err := decodeContainers(field, value) + if err != nil || len(containers) == 0 { + continue + } + matched = true + for _, container := range containers { + if container.Image != "" { + images = append(images, container.Image) + } + } } - return images + + return images, matched } -func GetContainersFromObject(obj any) ([]corev1.Container, error) { - podSpec, err := GetPodSpec(obj) +// decodeContainers strictly decodes one container list into its corev1 type. +// An error means "not that type", which is the signal the walk needs. +func decodeContainers(field string, value any) ([]corev1.Container, error) { + data, err := json.Marshal(value) if err != nil { return nil, err } + if field == "ephemeralContainers" { + var ephemeral []corev1.EphemeralContainer + if err := yaml.UnmarshalStrict(data, &ephemeral); err != nil { + return nil, err + } + containers := make([]corev1.Container, 0, len(ephemeral)) + for _, ec := range ephemeral { + containers = append(containers, corev1.Container(ec.EphemeralContainerCommon)) + } + return containers, nil + } + var containers []corev1.Container - containers = append(containers, podSpec.Containers...) - containers = append(containers, podSpec.InitContainers...) - for _, ec := range podSpec.EphemeralContainers { - containers = append(containers, corev1.Container(ec.EphemeralContainerCommon)) + if err := yaml.UnmarshalStrict(data, &containers); err != nil { + return nil, err } return containers, nil } diff --git a/k8s/k8s_test.go b/k8s/k8s_test.go index a7c5422..1e8267a 100644 --- a/k8s/k8s_test.go +++ b/k8s/k8s_test.go @@ -1,275 +1,249 @@ package k8s import ( + "slices" "testing" - appsv1 "k8s.io/api/apps/v1" - batchv1 "k8s.io/api/batch/v1" - corev1 "k8s.io/api/core/v1" + "sigs.k8s.io/yaml" ) -// Test that GetPodSpec works for the kinds that have a PodSpec: -func TestGetPodSpec(t *testing.T) { - tests := []struct { - name string - obj any - wantErr bool - }{ - {"Pod", &corev1.Pod{}, false}, - {"Deployment", &appsv1.Deployment{}, false}, - {"DaemonSet", &appsv1.DaemonSet{}, false}, - {"ReplicaSet", &appsv1.ReplicaSet{}, false}, - {"StatefulSet", &appsv1.StatefulSet{}, false}, - {"Job", &batchv1.Job{}, false}, - {"CronJob", &batchv1.CronJob{}, false}, - {"Invalid", "invalid", true}, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - _, err := GetPodSpec(tt.obj) - if (err != nil) != tt.wantErr { - t.Errorf("GetPodSpec() error = %v, wantErr %v", err, tt.wantErr) - } - }) +// decode turns a manifest fragment into the plain Go values FindImages walks. +func decode(t *testing.T, manifest string) any { + t.Helper() + var doc any + if err := yaml.Unmarshal([]byte(manifest), &doc); err != nil { + t.Fatalf("decoding fixture: %v", err) } + return doc } -// Test that GetPodSpec returns the correct PodSpec: -func TestGetPodSpecPod(t *testing.T) { - pod := &corev1.Pod{ - Spec: corev1.PodSpec{ - Containers: []corev1.Container{ - { - Name: "test-container", - Image: "test-image", - }, - }, - }, - } - - spec, err := GetPodSpec(pod) - if err != nil { - t.Fatalf("GetPodSpec() error = %v", err) - } - - if len(spec.Containers) != 1 { - t.Fatalf("expected 1 container, got %d", len(spec.Containers)) - } - - if spec.Containers[0].Image != "test-image" { - t.Errorf("expected image %q, got %q", "test-image", spec.Containers[0].Image) - } -} - -// Test that GetPodSpec returns the correct PodSpec for all supported kinds: -func TestGetPodSpecSupported(t *testing.T) { - commonPodSpec := corev1.PodSpec{ - Containers: []corev1.Container{ - { - Name: "test-container", - Image: "test-image", - }, - }, - } - +// The kinds kir has always supported are found without being named: each is +// just a PodSpec at a different depth. +func TestFindImagesWorkloadKinds(t *testing.T) { tests := []struct { - name string - obj any + name string + manifest string + want []string }{ - {"Deployment", &appsv1.Deployment{ - Spec: appsv1.DeploymentSpec{ - Template: corev1.PodTemplateSpec{ - Spec: commonPodSpec, - }, - }, - }}, - {"DaemonSet", &appsv1.DaemonSet{ - Spec: appsv1.DaemonSetSpec{ - Template: corev1.PodTemplateSpec{ - Spec: commonPodSpec, - }, - }, - }}, - {"ReplicaSet", &appsv1.ReplicaSet{ - Spec: appsv1.ReplicaSetSpec{ - Template: corev1.PodTemplateSpec{ - Spec: commonPodSpec, - }, - }, - }}, - {"StatefulSet", &appsv1.StatefulSet{ - Spec: appsv1.StatefulSetSpec{ - Template: corev1.PodTemplateSpec{ - Spec: commonPodSpec, - }, - }, - }}, - {"Job", &batchv1.Job{ - Spec: batchv1.JobSpec{ - Template: corev1.PodTemplateSpec{ - Spec: commonPodSpec, - }, - }, - }}, - {"CronJob", &batchv1.CronJob{ - Spec: batchv1.CronJobSpec{ - JobTemplate: batchv1.JobTemplateSpec{ - Spec: batchv1.JobSpec{ - Template: corev1.PodTemplateSpec{ - Spec: commonPodSpec, - }, - }, - }, - }, - }}, + { + name: "Pod (PodSpec directly on spec)", + manifest: ` +kind: Pod +spec: + containers: + - name: app + image: app:1 +`, + want: []string{"app:1"}, + }, + { + name: "Deployment (PodSpec under a template)", + manifest: ` +kind: Deployment +spec: + template: + spec: + containers: + - name: app + image: app:2 +`, + want: []string{"app:2"}, + }, + { + name: "CronJob (PodSpec four levels down)", + manifest: ` +kind: CronJob +spec: + jobTemplate: + spec: + template: + spec: + containers: + - name: app + image: app:3 +`, + want: []string{"app:3"}, + }, + { + name: "List (every item contributes)", + manifest: ` +kind: List +items: +- kind: Pod + spec: + containers: + - name: a + image: a:1 +- kind: Service + spec: + ports: + - port: 80 +- kind: Pod + spec: + containers: + - name: b + image: b:1 +`, + want: []string{"a:1", "b:1"}, + }, + { + name: "all three container fields, in report order", + manifest: ` +kind: Pod +spec: + containers: + - name: app + image: app:1 + initContainers: + - name: init + image: init:1 + ephemeralContainers: + - name: debugger + image: debug:1 + targetContainerName: app +`, + want: []string{"app:1", "init:1", "debug:1"}, + }, + { + name: "no PodSpec anywhere", + manifest: ` +kind: Service +spec: + ports: + - port: 80 +`, + want: nil, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - spec, err := GetPodSpec(tt.obj) - if err != nil { - t.Fatalf("GetPodSpec() error = %v", err) - } - - if len(spec.Containers) != 1 { - t.Fatalf("expected 1 container, got %d", len(spec.Containers)) - } - - if spec.Containers[0].Image != "test-image" { - t.Errorf("expected image %q, got %q", "test-image", spec.Containers[0].Image) + got := infer(decode(t, tt.manifest)) + if !slices.Equal(got, tt.want) { + t.Errorf("infer() = %v, want %v", got, tt.want) } }) } } -// Test that GetPodSpec fails for an object that does not have a PodSpec: -func TestGetPodSpecInvalid(t *testing.T) { - _, err := GetPodSpec("invalid") - - // Assert correct error message ()"object does not have a PodSpec"): - if err == nil || err.Error() != "object does not have a PodSpec" { - t.Fatalf("GetPodSpec() error = %v, want %q", err, "object does not have a PodSpec") +// The point of structural discovery: a custom resource the Kubernetes scheme +// cannot decode is understood, because its PodSpec is a PodSpec. +func TestFindImagesCustomResource(t *testing.T) { + rollout := ` +apiVersion: argoproj.io/v1alpha1 +kind: Rollout +spec: + strategy: + canary: + steps: + - setWeight: 20 + template: + spec: + containers: + - name: app + image: app:1.4.2 +` + + want := []string{"app:1.4.2"} + if got := infer(decode(t, rollout)); !slices.Equal(got, want) { + t.Errorf("infer() = %v, want %v", got, want) } } -func TestGetContainerImages(t *testing.T) { - containers := []corev1.Container{ - {Name: "container1", Image: "image1"}, - {Name: "container2", Image: "image2"}, - } - - expected := []string{"image1", "image2"} - images := GetContainerImages(containers) - - if len(images) != len(expected) { - t.Fatalf("expected %d images, got %d", len(expected), len(images)) - } - - for i, img := range images { - if img != expected[i] { - t.Errorf("expected image %q, got %q", expected[i], img) - } - } -} - -func TestGetContainersFromObject(t *testing.T) { +// The counterweight: matching on shape must not mean matching on a field name. +// Decoding into corev1.Container is what separates a PodSpec from a lookalike. +func TestFindImagesRejectsLookalikes(t *testing.T) { tests := []struct { - name string - obj any - want []corev1.Container - wantErr bool + name string + manifest string }{ { - name: "Pod", - obj: &corev1.Pod{ - Spec: corev1.PodSpec{ - Containers: []corev1.Container{ - {Name: "container1", Image: "image1"}, - }, - InitContainers: []corev1.Container{ - {Name: "init-container1", Image: "init-image1"}, - }, - }, - }, - want: []corev1.Container{ - {Name: "container1", Image: "image1"}, - {Name: "init-container1", Image: "init-image1"}, - }, - wantErr: false, + name: "containers of another kind entirely", + manifest: ` +kind: ShippingManifest +spec: + containers: + - name: cargo-hold-1 + capacity: 40ft + image: photo-of-container.jpg +`, }, { - name: "Deployment", - obj: &appsv1.Deployment{ - Spec: appsv1.DeploymentSpec{ - Template: corev1.PodTemplateSpec{ - Spec: corev1.PodSpec{ - Containers: []corev1.Container{ - {Name: "container1", Image: "image1"}, - }, - InitContainers: []corev1.Container{ - {Name: "init-container1", Image: "init-image1"}, - }, - }, - }, - }, - }, - want: []corev1.Container{ - {Name: "container1", Image: "image1"}, - {Name: "init-container1", Image: "init-image1"}, - }, - wantErr: false, + name: "containers holding strings", + manifest: ` +kind: Warehouse +spec: + containers: + - CONTAINER-A + - CONTAINER-B +`, }, { - name: "Pod with ephemeral container", - obj: &corev1.Pod{ - Spec: corev1.PodSpec{ - Containers: []corev1.Container{ - {Name: "container1", Image: "image1"}, - }, - InitContainers: []corev1.Container{ - {Name: "init-container1", Image: "init-image1"}, - }, - EphemeralContainers: []corev1.EphemeralContainer{ - { - EphemeralContainerCommon: corev1.EphemeralContainerCommon{ - Name: "debugger", - Image: "ephemeral-image1", - }, - }, - }, - }, - }, - want: []corev1.Container{ - {Name: "container1", Image: "image1"}, - {Name: "init-container1", Image: "init-image1"}, - {Name: "debugger", Image: "ephemeral-image1"}, - }, - wantErr: false, + name: "a manifest embedded as a string", + manifest: ` +kind: ConfigMap +data: + pod.yaml: | + kind: Pod + spec: + containers: + - name: inner + image: inner:1 +`, }, { - name: "Invalid", - obj: "invalid", - want: nil, - wantErr: true, + name: "container status, which reports images but is not a PodSpec", + manifest: ` +kind: Pod +status: + containerStatuses: + - name: app + image: app:1 + imageID: docker-pullable://app@sha256:abc +`, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := GetContainersFromObject(tt.obj) - if (err != nil) != tt.wantErr { - t.Errorf("GetContainersFromObject() error = %v, wantErr %v", err, tt.wantErr) - return - } - if len(got) != len(tt.want) { - t.Errorf("expected %d containers, got %d", len(tt.want), len(got)) - } - for i, container := range got { - if container.Name != tt.want[i].Name || container.Image != tt.want[i].Image { - t.Errorf("expected container %v, got %v", tt.want[i], container) - } + if got := infer(decode(t, tt.manifest)); len(got) != 0 { + t.Errorf("infer() = %v, want no images", got) } }) } } + +// Go randomises map iteration, so the walk sorts keys. Without that the golden +// files would flake whenever a document holds more than one PodSpec. +func TestFindImagesOrderIsStable(t *testing.T) { + manifest := ` +kind: List +items: +- kind: Pod + spec: + containers: + - name: a + image: a:1 +- kind: Pod + spec: + containers: + - name: b + image: b:1 +- kind: Pod + spec: + containers: + - name: c + image: c:1 +` + + doc := decode(t, manifest) + want := infer(doc) + if len(want) != 3 { + t.Fatalf("expected 3 images, got %d", len(want)) + } + for range 50 { + if got := infer(doc); !slices.Equal(got, want) { + t.Fatalf("infer() = %v, want %v — order is not stable", got, want) + } + } +} diff --git a/k8s/resources.yaml b/k8s/resources.yaml new file mode 100644 index 0000000..2ed29c3 --- /dev/null +++ b/k8s/resources.yaml @@ -0,0 +1,48 @@ +# Where each kind keeps its images. +# +# This is the whole of kir's knowledge about Kubernetes: a kind, and JMESPath +# expressions locating the images inside it. Nothing is inferred — a kind absent +# from this file yields no images. +# +# podSpecs select PodSpec-shaped nodes; their containers, initContainers +# and ephemeralContainers are read. +# containers select containers directly, for resources holding bare +# containers rather than a PodSpec. +# documents select whole objects, each processed in its own right, which is +# how a List is unwrapped. +# +# Expressions are compiled when the file loads, so a malformed one is an error +# rather than a path that silently matches nothing. +# +# Add your own with --config; entries are merged over these, so naming a kind +# listed here replaces its expressions. +resources: + - kind: Pod + podSpecs: [spec] + + - kind: PodTemplate + podSpecs: [template.spec] + + - kind: ReplicationController + podSpecs: [spec.template.spec] + + - kind: Deployment + podSpecs: [spec.template.spec] + + - kind: DaemonSet + podSpecs: [spec.template.spec] + + - kind: ReplicaSet + podSpecs: [spec.template.spec] + + - kind: StatefulSet + podSpecs: [spec.template.spec] + + - kind: Job + podSpecs: [spec.template.spec] + + - kind: CronJob + podSpecs: [spec.jobTemplate.spec.template.spec] + + - kind: List + documents: ["items[*]"] diff --git a/processor/processor.go b/processor/processor.go index 9533a33..d8e9cd1 100644 --- a/processor/processor.go +++ b/processor/processor.go @@ -5,21 +5,22 @@ import ( "io" "os" + "github.com/mpv/kir/k8s" "github.com/mpv/kir/yamlparser" ) // ProcessStdin reads a (possibly multi-document) manifest stream from r and // returns its images. Taking an io.Reader keeps it testable and lets the CLI // inject stdin. -func ProcessStdin(r io.Reader) ([]string, error) { - return yamlparser.ProcessReader(r) +func ProcessStdin(config *k8s.Config, r io.Reader) ([]string, error) { + return yamlparser.ProcessReader(config, r) } -func ProcessFile(filePath string) ([]string, error) { +func ProcessFile(config *k8s.Config, filePath string) ([]string, error) { file, err := os.Open(filePath) if err != nil { return nil, fmt.Errorf("error reading file: %v", err) } defer file.Close() - return yamlparser.ProcessReader(file) + return yamlparser.ProcessReader(config, file) } diff --git a/processor/processor_test.go b/processor/processor_test.go index 6a77e26..e9fb359 100644 --- a/processor/processor_test.go +++ b/processor/processor_test.go @@ -3,6 +3,8 @@ package processor import ( "os" "testing" + + "github.com/mpv/kir/k8s" ) func TestProcessFile(t *testing.T) { @@ -28,7 +30,7 @@ spec: image: another-image `) - images, err := ProcessFile(dir + "/test.yaml") + images, err := ProcessFile(k8s.DefaultConfig(), dir+"/test.yaml") if err != nil { t.Fatalf("ProcessFile() error = %v", err) } diff --git a/yamlparser/processreader_test.go b/yamlparser/processreader_test.go index 52e58e8..85ca6f3 100644 --- a/yamlparser/processreader_test.go +++ b/yamlparser/processreader_test.go @@ -3,6 +3,8 @@ package yamlparser import ( "strings" "testing" + + "github.com/mpv/kir/k8s" ) // ProcessReader must collect images from every document in a stream and split @@ -51,7 +53,7 @@ func TestProcessReader(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := ProcessReader(strings.NewReader(tt.data)) + got, err := ProcessReader(k8s.DefaultConfig(), strings.NewReader(tt.data)) if err != nil { t.Fatalf("ProcessReader() error = %v", err) } diff --git a/yamlparser/yamlparser.go b/yamlparser/yamlparser.go index 91a060e..8edffc3 100644 --- a/yamlparser/yamlparser.go +++ b/yamlparser/yamlparser.go @@ -5,21 +5,14 @@ import ( "errors" "fmt" "io" - "slices" "github.com/mpv/kir/k8s" - corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/runtime/serializer" utilyaml "k8s.io/apimachinery/pkg/util/yaml" - "k8s.io/client-go/kubernetes/scheme" + "sigs.k8s.io/yaml" ) -var supportedKinds = []string{"Pod", "Deployment", "DaemonSet", "ReplicaSet", "StatefulSet", "Job", "CronJob"} - // ProcessReader reads a (possibly multi-document) YAML stream and returns the -// container images of every supported workload it contains. Documents are +// container images of every workload it contains, using config for the kinds it describes and structural inference for the rest. Documents are // separated using the Kubernetes YAML reader, which correctly handles leading // and trailing "---" separators, separators followed by trailing whitespace, // CRLF line endings, and a final document without a trailing newline. @@ -30,7 +23,7 @@ var supportedKinds = []string{"Pod", "Deployment", "DaemonSet", "ReplicaSet", "S // image it finds and surface failures through the exit code; that has to hold // within a stream as well as between inputs, or one unparseable document in a // cluster dump costs every image around it. -func ProcessReader(r io.Reader) ([]string, error) { +func ProcessReader(config *k8s.Config, r io.Reader) ([]string, error) { var images []string var errs []error reader := utilyaml.NewYAMLReader(bufio.NewReader(r)) @@ -45,7 +38,7 @@ func ProcessReader(r io.Reader) ([]string, error) { errs = append(errs, fmt.Errorf("error reading YAML document: %v", err)) break } - imgs, err := ProcessData(doc) + imgs, err := ProcessData(config, doc) if err != nil { errs = append(errs, err) continue @@ -55,72 +48,31 @@ func ProcessReader(r io.Reader) ([]string, error) { return images, errors.Join(errs...) } -func ProcessData(data []byte) ([]string, error) { - // Decode the YAML file into a Kubernetes object - decode := serializer.NewCodecFactory(scheme.Scheme).UniversalDeserializer().Decode - obj, gvk, err := decode(data, nil, nil) - if err != nil { - // Kinds that aren't registered in the scheme (CRDs and other custom - // resources) are skipped rather than failing the whole stream. Some of - // them may embed a PodSpec we could inspect; surfacing those ("seen but - // not detected") is tracked in #75. For now they are skipped silently, - // like any other non-workload document — see - // docs/adr/0007-document-classification.md. - if runtime.IsNotRegisteredError(err) { - return nil, nil - } +// ProcessData returns the images in a single manifest document. +// +// The document is decoded into plain Go values rather than into a typed +// Kubernetes object. Whether its images are looked up or inferred is +// k8s.Config.FindImages's business; either way no list of supported kinds is +// consulted, and a custom resource the Kubernetes scheme has never heard of is +// read on the same terms as a Deployment. +func ProcessData(config *k8s.Config, data []byte) ([]string, error) { + var doc map[string]any + if err := yaml.Unmarshal(data, &doc); err != nil { return nil, err } - var images []string - - // Check if the object has containers - if containers, err := k8s.GetContainersFromObject(obj); err == nil { - images = append(images, k8s.GetContainerImages(containers)...) - return images, nil + // Everything kir accepts is a Kubernetes object, and every Kubernetes + // object has a kind. Requiring it keeps kir pointed at manifests instead of + // mining arbitrary YAML (a Helm values.yaml, say) for anything image-like, + // and it keeps an empty or malformed document an error rather than a silent + // no-op — the unprocessable tier of + // docs/adr/0007-document-classification.md, which ADR 0008 surfaces as a + // non-zero exit. + if _, ok := doc["kind"]; !ok { + return nil, fmt.Errorf("Object 'Kind' is missing in %q", data) } - // Handle List type separately - if gvk.Kind == "List" { - list, ok := obj.(*corev1.List) - if !ok { - return nil, fmt.Errorf("not a List") - } - for _, item := range list.Items { - var unstructuredObj unstructured.Unstructured - if err := unstructuredObj.UnmarshalJSON(item.Raw); err != nil { - return nil, fmt.Errorf("error unmarshaling item: %v", err) - } - imgs, err := processUnstructured(unstructuredObj) - if err != nil { - return nil, fmt.Errorf("error processing unstructured item: %v", err) - } - images = append(images, imgs...) - } - return images, nil - } - - // Any other kind (Service, ConfigMap, ...) is a valid object with no images - // to report, not an error; skip it silently so a single non-workload - // document does not discard images from the rest of the stream. See - // docs/adr/0007-document-classification.md. - return nil, nil -} - -func processUnstructured(item unstructured.Unstructured) ([]string, error) { - itemData, err := item.MarshalJSON() - if err != nil { - return nil, fmt.Errorf("error marshaling item: %v", err) - } - gvk := item.GroupVersionKind() - if slices.Contains(supportedKinds, gvk.Kind) { - images, err := ProcessData(itemData) - if err != nil { - return nil, fmt.Errorf("error processing data: %v", err) - } - return images, nil - } - // Non-workload items inside a List are skipped, mirroring how top-level - // non-workload documents are handled. - return nil, nil + // A document with nothing to report — a Service, a ConfigMap — is not an + // error. See ADR 0007. + return config.FindImages(doc), nil } diff --git a/yamlparser/yamlparser_test.go b/yamlparser/yamlparser_test.go index 53ff24d..099ca6d 100644 --- a/yamlparser/yamlparser_test.go +++ b/yamlparser/yamlparser_test.go @@ -5,6 +5,8 @@ import ( "slices" "strings" "testing" + + "github.com/mpv/kir/k8s" ) func TestProcessData(t *testing.T) { @@ -19,7 +21,7 @@ spec: image: test-image ` - images, err := ProcessData([]byte(data)) + images, err := ProcessData(k8s.DefaultConfig(), []byte(data)) if err != nil { t.Fatalf("ProcessData() error = %v", err) } @@ -55,7 +57,7 @@ spec: targetContainerName: test-container ` - images, err := ProcessData([]byte(data)) + images, err := ProcessData(k8s.DefaultConfig(), []byte(data)) if err != nil { t.Fatalf("ProcessData() error = %v", err) } @@ -83,7 +85,7 @@ func TestProcessReaderKeepsImagesAroundABadDocument(t *testing.T) { "apiVersion: v1\nkind: Pod\nspec:\n containers:\n - {name: c, image: after-the-break}\n", }, "---\n") - images, err := ProcessReader(strings.NewReader(stream)) + images, err := ProcessReader(k8s.DefaultConfig(), strings.NewReader(stream)) if err == nil { t.Error("ProcessReader() error = nil, want the bad document reported") @@ -100,7 +102,7 @@ func TestProcessReaderReportsEveryBadDocument(t *testing.T) { bad := "apiVersion: v1\nkind: Pod\nspec:\n containers:\n - {name: c, image: nginx, ports: [8080}\n" stream := strings.Join([]string{bad, bad}, "---\n") - images, err := ProcessReader(strings.NewReader(stream)) + images, err := ProcessReader(k8s.DefaultConfig(), strings.NewReader(stream)) if len(images) != 0 { t.Errorf("ProcessReader() images = %v, want none", images)