From dae5df2cef60f1a4780f942845e9fae28e513382 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 23 Jul 2026 06:32:50 +0000 Subject: [PATCH] fix: split YAML documents with the Kubernetes YAML reader Document splitting used bytes.Split on the literal "\n---\n", which drops every document after the first when the separator isn't exactly that: a separator with trailing whitespace ("--- ") or CRLF line endings both glue the documents together. Before (two Pods separated by "---" with trailing spaces): $ kir pods.yaml img-a:1 # img-b:2 from the second document is silently dropped After: $ kir pods.yaml img-a:1 img-b:2 Replace it with apimachinery's YAMLReader via a new yamlparser.ProcessReader that streams documents from an io.Reader. Both the file and stdin paths read through it, so their splitting behaves identically and matches how Kubernetes itself parses manifests. Returning ProcessData's error directly also drops the redundant "error processing document:" wrapper (e.g. malformed YAML now reads "error: " instead of "error: error processing document: "); the TestFailure/BadYAML golden is updated to match. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01Pc6NAURAqjU4LYJx93tgSC --- ...st.TestFailure.BadYAML.stderr.approved.txt | 2 +- processor/processor.go | 34 ++-------- yamlparser/processreader_test.go | 68 +++++++++++++++++++ yamlparser/yamlparser.go | 28 ++++++++ 4 files changed, 104 insertions(+), 28 deletions(-) create mode 100644 yamlparser/processreader_test.go diff --git a/approvals/kir_test.TestFailure.BadYAML.stderr.approved.txt b/approvals/kir_test.TestFailure.BadYAML.stderr.approved.txt index 9390bd3..49591ae 100644 --- a/approvals/kir_test.TestFailure.BadYAML.stderr.approved.txt +++ b/approvals/kir_test.TestFailure.BadYAML.stderr.approved.txt @@ -1 +1 @@ -error: error processing document: yaml: line 9: did not find expected ',' or ']' +error: yaml: line 9: did not find expected ',' or ']' diff --git a/processor/processor.go b/processor/processor.go index 5d83e95..9533a33 100644 --- a/processor/processor.go +++ b/processor/processor.go @@ -1,7 +1,6 @@ package processor import ( - "bytes" "fmt" "io" "os" @@ -9,37 +8,18 @@ import ( "github.com/mpv/kir/yamlparser" ) -// ProcessStdin reads a manifest stream from r and returns its images. Taking an -// io.Reader (rather than reading os.Stdin directly) keeps it testable and lets -// the CLI inject stdin. +// 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) { - data, err := io.ReadAll(r) - if err != nil { - return nil, fmt.Errorf("error reading stdin: %v", err) - } - return processDocuments(data) + return yamlparser.ProcessReader(r) } func ProcessFile(filePath string) ([]string, error) { - data, err := os.ReadFile(filePath) + file, err := os.Open(filePath) if err != nil { return nil, fmt.Errorf("error reading file: %v", err) } - return processDocuments(data) -} - -// processDocuments splits a (possibly multi-document) YAML stream and collects -// the images from every document. Both the file and stdin paths go through it -// so they handle multi-document input identically. -func processDocuments(data []byte) ([]string, error) { - var images []string - docs := bytes.Split(data, []byte("\n---\n")) - for _, doc := range docs { - imgs, err := yamlparser.ProcessData(doc) - if err != nil { - return nil, fmt.Errorf("error processing document: %v", err) - } - images = append(images, imgs...) - } - return images, nil + defer file.Close() + return yamlparser.ProcessReader(file) } diff --git a/yamlparser/processreader_test.go b/yamlparser/processreader_test.go new file mode 100644 index 0000000..52e58e8 --- /dev/null +++ b/yamlparser/processreader_test.go @@ -0,0 +1,68 @@ +package yamlparser + +import ( + "strings" + "testing" +) + +// ProcessReader must collect images from every document in a stream and split +// documents robustly. A naive bytes.Split on "\n---\n" drops every document +// after the first when the separator isn't exactly that literal — a separator +// with trailing whitespace, or CRLF line endings. The leading-separator and +// no-trailing-newline cases are general correctness checks: a plain bytes.Split +// happens to handle those, but the YAML reader must too. +func TestProcessReader(t *testing.T) { + pod := func(name, image string) string { + return "apiVersion: v1\nkind: Pod\nmetadata:\n name: " + name + + "\nspec:\n containers:\n - name: c\n image: " + image + "\n" + } + + tests := []struct { + name string + data string + want []string + }{ + { + name: "multiple documents", + data: pod("one", "image-one") + "---\n" + pod("two", "image-two"), + want: []string{"image-one", "image-two"}, + }, + { + name: "leading separator", + data: "---\n" + pod("one", "image-one"), + want: []string{"image-one"}, + }, + { + name: "separator with trailing whitespace", + data: pod("one", "image-one") + "--- \n" + pod("two", "image-two"), + want: []string{"image-one", "image-two"}, + }, + { + name: "crlf line endings", + data: strings.ReplaceAll(pod("one", "image-one")+"---\n"+pod("two", "image-two"), "\n", "\r\n"), + want: []string{"image-one", "image-two"}, + }, + { + name: "no trailing newline", + data: strings.TrimRight(pod("one", "image-one"), "\n"), + want: []string{"image-one"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := ProcessReader(strings.NewReader(tt.data)) + if err != nil { + t.Fatalf("ProcessReader() error = %v", err) + } + if len(got) != len(tt.want) { + t.Fatalf("expected %d images, got %d: %v", len(tt.want), len(got), got) + } + for i, img := range got { + if img != tt.want[i] { + t.Errorf("image %d: expected %q, got %q", i, tt.want[i], img) + } + } + }) + } +} diff --git a/yamlparser/yamlparser.go b/yamlparser/yamlparser.go index e06a3ec..8405ad2 100644 --- a/yamlparser/yamlparser.go +++ b/yamlparser/yamlparser.go @@ -1,7 +1,9 @@ package yamlparser import ( + "bufio" "fmt" + "io" "slices" "github.com/mpv/kir/k8s" @@ -9,11 +11,37 @@ import ( "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" ) 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 +// 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. +func ProcessReader(r io.Reader) ([]string, error) { + var images []string + reader := utilyaml.NewYAMLReader(bufio.NewReader(r)) + for { + doc, err := reader.Read() + if err == io.EOF { + break + } + if err != nil { + return nil, fmt.Errorf("error reading YAML document: %v", err) + } + imgs, err := ProcessData(doc) + if err != nil { + return nil, err + } + images = append(images, imgs...) + } + return images, nil +} + func ProcessData(data []byte) ([]string, error) { // Decode the YAML file into a Kubernetes object decode := serializer.NewCodecFactory(scheme.Scheme).UniversalDeserializer().Decode