Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ A manifest stream usually mixes workloads with other objects. `kir` handles each
| 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 |
| 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) |

So stdout carries only images and stderr stays quiet for normal input. See [ADR 0007](docs/adr/0007-document-classification.md) for the rationale.
18 changes: 15 additions & 3 deletions cmd/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"strings"

"github.com/mpv/kir/fileutil"
"github.com/mpv/kir/imageref"
"github.com/mpv/kir/processor"
)

Expand Down Expand Up @@ -45,7 +46,7 @@ func Run(args []string, stdin io.Reader, stdout, stderr io.Writer) int {
}
images, err := processor.ProcessStdin(stdin)
failures := logErrors(logger, err)
printImages(stdout, images)
failures += printImages(stdout, logger, "stdin", images)
if failures > 0 {
return 1
}
Expand All @@ -68,7 +69,7 @@ func Run(args []string, stdin io.Reader, stdout, stderr io.Writer) int {
// yielded images from the others, and dropping them would defeat the
// point of reporting the failure.
failures += logErrors(logger, err)
printImages(stdout, images)
failures += printImages(stdout, logger, filePath, images)
}
if failures > 0 {
return 1
Expand Down Expand Up @@ -96,8 +97,19 @@ func logErrors(logger *log.Logger, err error) int {
return 1
}

func printImages(w io.Writer, images []string) {
// printImages writes every reportable image to w, reports the rest to logger,
// and returns how many it rejected. An unreportable reference gets the same
// treatment as a malformed document (ADR 0008): named on stderr, counted
// against the exit code, and not allowed to discard the images beside it.
func printImages(w io.Writer, logger *log.Logger, source string, images []string) int {
rejected := 0
for _, image := range images {
if err := imageref.Validate(image); err != nil {
logger.Printf("error: %s: %v", source, err)
rejected++
continue
}
fmt.Fprintln(w, image)
}
return rejected
}
36 changes: 36 additions & 0 deletions cmd/cmd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,42 @@ func TestRunFileFailure(t *testing.T) {
}
}

// Which values are unreportable is imageref's business; this pins the CLI
// contract around them. The hostile bytes are written here rather than in an
// approvals fixture, which tooling would normalise (see AGENTS.md).
const hostileImageManifest = `
apiVersion: v1
kind: Pod
metadata:
name: hostile
spec:
containers:
- name: reportable
image: registry.k8s.io/nginx-slim:0.8
- name: forges-a-second-entry
image: "evil\nsecond-line"
- name: repaints-the-terminal
image: "nginx:1.0\x1b[2K\rregistry.io/trusted:safe"
`

func TestRunRejectsUnreportableImages(t *testing.T) {
var stdout, stderr bytes.Buffer
code := Run([]string{"-"}, strings.NewReader(hostileImageManifest), &stdout, &stderr)

if code != 1 {
t.Errorf("exit code = %d, want 1", code)
}
if got, want := stdout.String(), "registry.k8s.io/nginx-slim:0.8\n"; got != want {
t.Errorf("stdout = %q, want only the reportable image %q", got, want)
}
if got, want := strings.Count(stderr.String(), "error:"), 2; got != want {
t.Errorf("stderr reported %d errors, want %d:\n%s", got, want, stderr.String())
}
if strings.ContainsRune(stderr.String(), '\x1b') {
t.Errorf("stderr contains a raw escape byte, want it quoted:\n%q", stderr.String())
}
}

func TestRunNoArgs(t *testing.T) {
var stdout, stderr bytes.Buffer
code := Run(nil, nil, &stdout, &stderr)
Expand Down
22 changes: 20 additions & 2 deletions docs/adr/0007-document-classification.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,14 @@
- 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:
occasional malformed document. Each falls into one of these 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 (ADR 0008) |
| Workload with an unreportable image | an image value that is not a valid reference | its other images | `error: …` | non-zero |

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
Expand All @@ -19,7 +20,24 @@ stdout to images, keeps stderr quiet for normal input, and keeps the exit code
trustworthy (an earlier version erred on every Service, which — once failures
became non-zero, ADR 0008 — would make `kir manifests/*` exit non-zero).

Unregistered kinds (CRDs) are a fourth case, today handled like image-less —
The last tier classifies an image *value*, because stdout is a contract as much
as a report: one image per line, normally fed straight into another program's
arguments. A line break in a value forges an extra entry in that list, escape
sequences can make a terminal show a registry the scanner was never given, and a
leading dash arrives at the scanner as an option. Each is refused on its own,
leaving the document's other images reported.

Validity is the canonical parser's verdict
([`distribution/reference`](https://github.com/distribution/reference)), not a
hand-written rule set — the load-bearing choice here. kir accepts exactly what a
registry client would, so anything it refuses was never a pullable image, which
is what makes refusing safe; and it refuses nothing a registry would serve, so it
cannot drop a real image. Hand-written rules failed both ways: an earlier draft
of this let `nginx:`, `{{.Values.image}}` and `$IMAGE` through as images.
Normalisation is not borrowed, only validation — kir reports what the manifest
said, so `nginx` stays `nginx`, not `docker.io/library/nginx`.

Unregistered kinds (CRDs) are a further 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
Expand Down
2 changes: 2 additions & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ go 1.26.0

require (
github.com/approvals/go-approval-tests v1.14.0
github.com/distribution/reference v0.6.0
k8s.io/api v0.36.3
k8s.io/apimachinery v0.36.3
k8s.io/client-go v0.36.3
Expand All @@ -16,6 +17,7 @@ require (
github.com/kr/text v0.2.0 // indirect
github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
github.com/modern-go/reflect2 v1.0.3-0.20250322232337-35a7c28c31ee // indirect
github.com/opencontainers/go-digest v1.0.0 // indirect
github.com/x448/float16 v0.8.4 // indirect
go.yaml.in/yaml/v2 v2.4.3 // indirect
golang.org/x/net v0.49.0 // indirect
Expand Down
4 changes: 4 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSs
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1VwoXQT9A3Wy9MM3WgvqSxFWenqJduM=
github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/distribution/reference v0.6.0 h1:0IXCQ5g4/QMHHkarYzh5l+u8T3t73zM5QvfrDyIgxBk=
github.com/distribution/reference v0.6.0/go.mod h1:BbU0aIcezP1/5jX/8MP0YiH4SdvB5Y4f/wlDRiLyi3E=
github.com/fxamacker/cbor/v2 v2.9.0 h1:NpKPmjDBgUfBms6tr6JZkTHtfFGcMKsw3eGcmD/sapM=
github.com/fxamacker/cbor/v2 v2.9.0/go.mod h1:vM4b+DJCtHn+zz7h3FFp/hDAI9WNWCsZj23V5ytsSxQ=
github.com/go-logr/logr v1.4.3 h1:CjnDlHq8ikf6E492q6eKboGOC0T8CDaOvkHCIg8idEI=
Expand All @@ -24,6 +26,8 @@ github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd/go.mod h1:6dJ
github.com/modern-go/reflect2 v1.0.2/go.mod h1:yWuevngMOJpCy52FWWMvUC8ws7m/LJsjYzDa0/r8luk=
github.com/modern-go/reflect2 v1.0.3-0.20250322232337-35a7c28c31ee h1:W5t00kpgFdJifH4BDsTlE89Zl93FEloxaWZfGcifgq8=
github.com/modern-go/reflect2 v1.0.3-0.20250322232337-35a7c28c31ee/go.mod h1:yWuevngMOJpCy52FWWMvUC8ws7m/LJsjYzDa0/r8luk=
github.com/opencontainers/go-digest v1.0.0 h1:apOUWs51W5PlhuyGyz9FCeeBIOUDA/6nW8Oi/yOhh5U=
github.com/opencontainers/go-digest v1.0.0/go.mod h1:0JzlMkj0TRzQZfJkVvzbP0HBR3IKzErnv2BNG4W4MAM=
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 h1:Jamvg5psRIccs7FGNTlIRMkT8wgtp5eCXdBlqhYGL6U=
github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
Expand Down
31 changes: 31 additions & 0 deletions imageref/imageref.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// Package imageref decides whether a container image reference is safe for kir
// to report. See docs/adr/0007-document-classification.md for why kir refuses
// some values outright.
package imageref

import (
// go-digest resolves sha256 only when that hash is linked in, and rejects
// every digest-pinned reference without it. kir links it anyway today via
// client-go, so this changes nothing now; it keeps imageref standing on its
// own for when #26's candidates drop client-go. No test here can pin it —
// the test binary links the hash regardless.
_ "crypto/sha256"
"fmt"
"strconv"

"github.com/distribution/reference"
)

// Validate returns an error describing why image cannot be reported, or nil if
// it can. The canonical parser decides, so kir accepts exactly what a registry
// client would. The parsed value is discarded: kir reports what the manifest
// said, so "nginx" stays "nginx" rather than becoming "docker.io/library/nginx".
func Validate(image string) error {
if _, err := reference.ParseNormalizedNamed(image); err != nil {
// Quoted, not interpolated: the parser echoes the offending value back,
// and an escape sequence in it must not reach the terminal this is
// reported on.
return fmt.Errorf("invalid image reference %q: %s", image, strconv.Quote(err.Error()))
}
return nil
}
67 changes: 67 additions & 0 deletions imageref/imageref_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
package imageref

import (
"strings"
"testing"
)

// One case per way a reference can be unreportable, rather than one per hostile
// byte — the parser treats them all the same, so byte variants would pin
// nothing extra. Hostile bytes live here and not in an approvals fixture: a
// checked-in .yaml would have its escapes and trailing whitespace normalised
// (see AGENTS.md).
func TestValidateRejects(t *testing.T) {
for name, image := range map[string]string{
"empty": "",
"newline forges an entry": "evil\nsecond-line",
"escape sequence spoofs": "nginx:1.0\x1b[2K\rregistry.io/trusted:safe",
"whitespace splits args": "a b c",
"leading dash is a flag": "--platform=linux/amd64",
"empty tag": "nginx:",
"unrendered helm template": "{{.Values.image}}",
"unexpanded variable": "$IMAGE",
} {
t.Run(name, func(t *testing.T) {
if err := Validate(image); err == nil {
t.Errorf("Validate(%q) = nil, want an error", image)
}
})
}
}

// Over-rejection is the failure that matters: a dropped image is
// indistinguishable from a manifest that had none. Every shape a registry could
// serve must pass.
func TestValidateAccepts(t *testing.T) {
for _, image := range []string{
"nginx",
"nginx:1.28",
"registry.k8s.io/nginx-slim:0.8",
"kiwigrid/k8s-sidecar",
"localhost:5000/kir:0.4.3",
"quay.io/org/sub/repo:v1.2.3-rc.1",
"registry.example.com:5000/team/app:2026-08-10_build.7",
// Digest-pinned references are what break when go-digest cannot resolve
// sha256; see the crypto/sha256 import in imageref.go.
"ghcr.io/mpv/kir@sha256:66b7bf84cfc7c1b2d4a7d9848114270ce6049a04d5dab67d767d8ab5c0b3412a",
"nginx:1.0@sha256:76944c9752702d324e06e2a9fa791c38f2b654ee4100c85327729eb3377a4284",
} {
t.Run(image, func(t *testing.T) {
if err := Validate(image); err != nil {
t.Errorf("Validate(%q) = %v, want nil", image, err)
}
})
}
}

// The parser echoes the offending value back in its message, so reporting a
// rejection must not pass those bytes through to a terminal.
func TestValidateErrorEscapesTheValue(t *testing.T) {
err := Validate("nginx:1.0\x1b[2K\rregistry.io/trusted:safe")
if err == nil {
t.Fatal("Validate() = nil, want an error")
}
if strings.ContainsAny(err.Error(), "\x1b\r\n") {
t.Errorf("error message carries raw control bytes: %q", err.Error())
}
}