From 05b1e64e81a461364808bf1959c9ff74c34aa2d5 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 6 Aug 2026 21:04:02 +0000 Subject: [PATCH] feat: skip non-workload documents instead of aborting the stream MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Any document whose kind has no PodSpec (a Service, ConfigMap, etc.) made ProcessData return an error, which aborted the whole file and discarded images already collected from earlier documents. Before: $ kir deployment-and-service.yaml error: error processing document: unsupported kind Service # exit 0, and the Deployment's image is lost After: $ kir deployment-and-service.yaml nginx:1.27 # the Service is skipped; surrounding workloads are preserved Treat non-workload kinds — and kinds not registered in the scheme (CRDs and other custom resources) — as skippable: no images, no error. In the golden suite the former TestError becomes TestSkipsNonWorkloads (Service now yields empty stderr) and TestMixed covers a workload + Service + workload file. Document the decision so the stderr/exit contract is explicit: a new docs/adr/0007-document-classification.md records the three-tier policy (workload / known image-less / unprocessable) and the planned "seen but not detected" warning for unrecognized CRDs (#75); the README summarizes it for users; and the yamlparser comments point back to the ADR. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01Pc6NAURAqjU4LYJx93tgSC --- README.md | 13 +++++++++ ...test.TestError.Service.stderr.approved.txt | 1 - ... kir_test.TestMixed.exitcode.approved.txt} | 0 approvals/kir_test.TestMixed.input.yaml | 27 +++++++++++++++++++ ...=> kir_test.TestMixed.stderr.approved.txt} | 0 .../kir_test.TestMixed.stdout.approved.txt | 2 ++ ...NonWorkloads.Service.exitcode.approved.txt | 1 + ....TestSkipsNonWorkloads.Service.input.yaml} | 0 ...psNonWorkloads.Service.stderr.approved.txt | 0 ...psNonWorkloads.Service.stdout.approved.txt | 0 approvals/kir_test.go | 11 ++++++-- docs/adr/0007-document-classification.md | 26 ++++++++++++++++++ docs/adr/README.md | 1 + yamlparser/yamlparser.go | 20 ++++++++++++-- 14 files changed, 97 insertions(+), 5 deletions(-) delete mode 100644 approvals/kir_test.TestError.Service.stderr.approved.txt rename approvals/{kir_test.TestError.Service.exitcode.approved.txt => kir_test.TestMixed.exitcode.approved.txt} (100%) create mode 100644 approvals/kir_test.TestMixed.input.yaml rename approvals/{kir_test.TestError.Service.stdout.approved.txt => kir_test.TestMixed.stderr.approved.txt} (100%) create mode 100644 approvals/kir_test.TestMixed.stdout.approved.txt create mode 100644 approvals/kir_test.TestSkipsNonWorkloads.Service.exitcode.approved.txt rename approvals/{kir_test.TestError.Service.input.yaml => kir_test.TestSkipsNonWorkloads.Service.input.yaml} (100%) create mode 100644 approvals/kir_test.TestSkipsNonWorkloads.Service.stderr.approved.txt create mode 100644 approvals/kir_test.TestSkipsNonWorkloads.Service.stdout.approved.txt create mode 100644 docs/adr/0007-document-classification.md diff --git a/README.md b/README.md index c43a7f1..b964068 100644 --- a/README.md +++ b/README.md @@ -63,3 +63,16 @@ $ go run main.go approvals/kir_test.TestKind.Job.input.yaml | xargs snyk contain # Docker Scout $ go run main.go approvals/kir_test.TestKind.Job.input.yaml | xargs docker scout cves ``` + +## How `kir` treats each document + +A manifest stream usually mixes workloads with other objects. `kir` handles each by 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) | +| Malformed or unreadable input | reported on stderr, non-zero exit | +| An unrecognized custom resource (CRD) | skipped for now — see [#75](https://github.com/MPV/kir/issues/75) | + +So stdout carries only images and stderr stays quiet for normal input. See [ADR 0007](docs/adr/0007-document-classification.md) for the rationale. diff --git a/approvals/kir_test.TestError.Service.stderr.approved.txt b/approvals/kir_test.TestError.Service.stderr.approved.txt deleted file mode 100644 index bf94249..0000000 --- a/approvals/kir_test.TestError.Service.stderr.approved.txt +++ /dev/null @@ -1 +0,0 @@ -error: error processing document: unsupported kind Service diff --git a/approvals/kir_test.TestError.Service.exitcode.approved.txt b/approvals/kir_test.TestMixed.exitcode.approved.txt similarity index 100% rename from approvals/kir_test.TestError.Service.exitcode.approved.txt rename to approvals/kir_test.TestMixed.exitcode.approved.txt diff --git a/approvals/kir_test.TestMixed.input.yaml b/approvals/kir_test.TestMixed.input.yaml new file mode 100644 index 0000000..4993a65 --- /dev/null +++ b/approvals/kir_test.TestMixed.input.yaml @@ -0,0 +1,27 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: web +spec: + template: + spec: + containers: + - image: nginx:1.27 +--- +apiVersion: v1 +kind: Service +metadata: + name: web +spec: + ports: + - port: 80 +--- +apiVersion: batch/v1 +kind: Job +metadata: + name: migrate +spec: + template: + spec: + containers: + - image: migrate:v2 diff --git a/approvals/kir_test.TestError.Service.stdout.approved.txt b/approvals/kir_test.TestMixed.stderr.approved.txt similarity index 100% rename from approvals/kir_test.TestError.Service.stdout.approved.txt rename to approvals/kir_test.TestMixed.stderr.approved.txt diff --git a/approvals/kir_test.TestMixed.stdout.approved.txt b/approvals/kir_test.TestMixed.stdout.approved.txt new file mode 100644 index 0000000..f3e29f1 --- /dev/null +++ b/approvals/kir_test.TestMixed.stdout.approved.txt @@ -0,0 +1,2 @@ +nginx:1.27 +migrate:v2 diff --git a/approvals/kir_test.TestSkipsNonWorkloads.Service.exitcode.approved.txt b/approvals/kir_test.TestSkipsNonWorkloads.Service.exitcode.approved.txt new file mode 100644 index 0000000..c227083 --- /dev/null +++ b/approvals/kir_test.TestSkipsNonWorkloads.Service.exitcode.approved.txt @@ -0,0 +1 @@ +0 \ No newline at end of file diff --git a/approvals/kir_test.TestError.Service.input.yaml b/approvals/kir_test.TestSkipsNonWorkloads.Service.input.yaml similarity index 100% rename from approvals/kir_test.TestError.Service.input.yaml rename to approvals/kir_test.TestSkipsNonWorkloads.Service.input.yaml diff --git a/approvals/kir_test.TestSkipsNonWorkloads.Service.stderr.approved.txt b/approvals/kir_test.TestSkipsNonWorkloads.Service.stderr.approved.txt new file mode 100644 index 0000000..e69de29 diff --git a/approvals/kir_test.TestSkipsNonWorkloads.Service.stdout.approved.txt b/approvals/kir_test.TestSkipsNonWorkloads.Service.stdout.approved.txt new file mode 100644 index 0000000..e69de29 diff --git a/approvals/kir_test.go b/approvals/kir_test.go index bc5cc5d..d8142c8 100644 --- a/approvals/kir_test.go +++ b/approvals/kir_test.go @@ -38,9 +38,10 @@ func TestKind(t *testing.T) { } } -func TestError(t *testing.T) { +// A non-workload kind (Service) is skipped: no images, no error, exit 0. +func TestSkipsNonWorkloads(t *testing.T) { t.Run("Service", func(t *testing.T) { - verify(t, []string{"kir_test.TestError.Service.input.yaml"}, nil) + verify(t, []string{"kir_test.TestSkipsNonWorkloads.Service.input.yaml"}, nil) }) } @@ -48,6 +49,12 @@ func TestMultiple(t *testing.T) { verify(t, []string{"kir_test.TestMultiple.input.yaml"}, nil) } +// A file mixing supported workloads with a non-workload document yields the +// workloads' images; the non-workload is skipped without discarding the rest. +func TestMixed(t *testing.T) { + verify(t, []string{"kir_test.TestMixed.input.yaml"}, nil) +} + // TestCLI covers behaviour that only exists at the CLI boundary — stdin wiring, // argument resolution, and no-args usage — which no file-argument scenario // above reaches. diff --git a/docs/adr/0007-document-classification.md b/docs/adr/0007-document-classification.md new file mode 100644 index 0000000..9a6a867 --- /dev/null +++ b/docs/adr/0007-document-classification.md @@ -0,0 +1,26 @@ +# 7. How kir classifies each document + +- Status: accepted +- Date: 2026-08-08 + +A manifest stream mixes workloads, image-less objects, custom resources, and the +occasional malformed document. Each falls into one of three tiers: + +| Tier | Examples | stdout | stderr | exit | +|---|---|---|---|---| +| Workload (has a PodSpec) | Pod, Deployment, …, CronJob | images | — | 0 | +| Known, image-less | Service, ConfigMap, Secret, … | — | — | 0 | +| Unprocessable | malformed YAML, unreadable file | — | `error: …` | non-zero (#55) | + +The load-bearing choice: a valid image-less document is **not** an error and +**not** a warning — it's expected input with nothing to report, so it's skipped +silently. Only unprocessable input hits stderr and the exit code. That keeps +stdout to images, keeps stderr quiet for normal input, and keeps the exit code +trustworthy (an earlier version erred on every Service, which would make +`kir manifests/*` exit non-zero under #55). + +Unregistered kinds (CRDs) are a fourth case, today handled like image-less — +skipped silently. But a CRD may embed a PodSpec (Argo Rollouts, Knative, …), so +skipping it silently can drop images (the #49 failure mode). Planned (#75): a +`warning:` on stderr, exit 0 — "seen but not detected" — distinct from the silent +known-image-less tier. Updated when that lands. diff --git a/docs/adr/README.md b/docs/adr/README.md index a3b2dd3..a5c7ddf 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -16,3 +16,4 @@ date the decision was actually made. | [0004](0004-approval-testing.md) | Pin behaviour with golden (approval) tests | 2025-03-18 | | [0005](0005-run-entry-point-seam.md) | Expose the CLI as an in-process `Run(args, stdin, stdout, stderr) int` | 2026-08-02 | | [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 | diff --git a/yamlparser/yamlparser.go b/yamlparser/yamlparser.go index b25dd53..e06a3ec 100644 --- a/yamlparser/yamlparser.go +++ b/yamlparser/yamlparser.go @@ -7,6 +7,7 @@ import ( "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" "k8s.io/client-go/kubernetes/scheme" ) @@ -18,6 +19,15 @@ func ProcessData(data []byte) ([]string, error) { 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 + } return nil, err } @@ -49,7 +59,11 @@ func ProcessData(data []byte) ([]string, error) { return images, nil } - return nil, fmt.Errorf("unsupported kind %s", gvk.Kind) + // 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) { @@ -65,5 +79,7 @@ func processUnstructured(item unstructured.Unstructured) ([]string, error) { } return images, nil } - return nil, fmt.Errorf("error: unsupported kind %s in List", gvk.Kind) + // Non-workload items inside a List are skipped, mirroring how top-level + // non-workload documents are handled. + return nil, nil }