GoogleContainerTools / GoogleContainerTools/skaffold
skaffold filter drops the trailing newline from the last value in a ConfigMap (regression in v2.6.0)
- Dominant language
- Go
- Stars
- 15.9k
- Forks
- 1.7k
- Avg merge
- 3d 9h
- Merged PRs (30d)
- 10
Description
### Expected behaviour
`skaffold filter` should not change the *content* of the manifests it passes through. A ConfigMap value that ends with a newline should still end with a newline afterwards.
### Actual behaviour
The value under the **alphabetically last key** of a ConfigMap's `data` map loses its trailing newline. Values under earlier keys are unaffected.
```
$ helm template t ./chart | grep 'zzz:'
zzz: |
$ helm template t ./chart | skaffold filter | grep 'zzz:'
zzz: |-
```
The block chomping indicator changes from `|` to `|-`, and the parsed string really does differ — `'x\n# EOF\n'` in, `'x\n# EOF'` out. This is not a formatting nit: for any file format with a required terminator, the file becomes invalid.
### Reproduction
```sh
mkdir -p /tmp/nl/chart/templates && cd /tmp/nl
printf 'apiVersion: skaffold/v4beta11\nkind: Config\nmetadata:\n name: t\n' > skaffold.yaml
printf 'apiVersion: v2\nname: t\nversion: 0.1.0\n' > chart/Chart.yaml
printf 'apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: t\ndata:\n zzz: |\n x\n # EOF\n' > chart/templates/cm.yaml
helm template t ./chart | grep 'zzz:' # zzz: | <-- correct
helm template t ./chart | skaffold filter | grep 'zzz:' # zzz: |- <-- newline lost
```
The stub `skaffold.yaml` is only there because `filter` requires a config in the working directory; its contents are irrelevant — a bare config with no `deploy:` section reproduces it exactly as a full one does.
### The rule
The value appearing **last in the mapping, in document order** loses its trailing newline. Everything before it survives untouched. Key names are irrelevant.
Earlier notes here said "alphabetically last" — wrong, and it made it into the first version of #10159 (corrected by this edit). Every test had used key names whose alphabetical order matched their write order, so the two were indistinguishable. Setting them in opposition settles it:
| `data` mapping | doc-last | alpha-last | altered |
|---|---|---|---|
| `zzz_plain`, then `aaa_block: \|` | `aaa_block` | `zzz_plain` | **`aaa_block`** |
| `aaa_block: \|`, then `zzz_plain` | `zzz_plain` | `zzz_plain` | none |
| `zzz_block: \|`, then `aaa_block: \|` | `aaa_block` | `zzz_block` | **`aaa_block`** |
It follows document order every time.
| `data` map shape | result |
|---|---|
| block scalar first, plain scalar last | unaffected |
| plain scalar first, block scalar last | **last value altered** |
| two block scalars | **only the last altered** |
| `\|-` last (no trailing newline to begin with) | unaffected |
| `\|+` last (ends `\n\n`) | **altered** (`'a\n'` → `'a'`) |
| a second YAML document following it | still **altered** |
Worth flagging for anyone writing a test: a `data` map whose last key happens to hold a `|-` value reports a false pass on every version.
### Regression range
Bisected to a single release. v2.5.1 is clean; v2.6.0 and every release since is not.
| version | released | result |
|---|---|---|
| v2.0.17 | | pass |
| v2.2.0 | | pass |
| v2.4.1 | | pass |
| **v2.5.1** | 2023-06-12 | **pass — last good** |
| **v2.6.0** | 2023-06-27 | **fail — first bad** |
| v2.6.1, v2.6.2, v2.6.3 | | fail |
| v2.7.0, v2.7.1 | | fail |
| v2.11.1 | | fail |
| v2.14.2, v2.15.0, v2.16.1, v2.17.3, v2.18.3, v2.19.0 | | fail |
| v2.21.0, v2.22.1, v2.23.0, v2.24.0 | 2026-07-23 | fail |
v2.0.17's pass is not a pass-through: it re-serialises like the later versions do (output differs from input, redundant quotes dropped — `"1.7.0"` becomes `1.7.0`), it just preserves the trailing newline while doing so.
### Suspected cause
`v2.5.1...v2.6.0` changes no YAML dependency — `gopkg.in/yaml.v3 v3.0.1`, `sigs.k8s.io/yaml v1.3.0` and `sigs.k8s.io/kustomize/kyaml v0.10.17` are identical either side.
The change appears to be #8902 ("chore: port apply-setter krm function over to skaffold", `ce682a53`), which added an apply-setters pass to `runFilter` in `cmd/skaffold/app/cmd/filter.go`:
```go
+var ass applysetters.ApplySetters
+manifestOverrides := pkgutil.EnvSliceToMap(opts.ManifestsOverrides, "=")
+for k, v := range manifestOverrides {
+ ass.Setters = append(ass.Setters, applysetters.Setter{Name: k, Value: v})
+}
+manifestList, err = ass.Apply(ctx, manifestList)
+if err != nil {
+ return err
+}
```
`ApplySetters` is a `kio.Filter` built on `sigs.k8s.io/kustomize/kyaml`, so `Apply` parses and re-emits the entire manifest through a kyaml pipeline, and kyaml's serialiser normalises the block scalar.
The call is **unguarded**: it runs on every `skaffold filter` invocation, while `ass.Setters` is empty unless manifest overrides were supplied. So a deploy that passes no overrides still pays a full kyaml round-trip, and still loses the newline.
### Suggested fix
Skip the pass when there is nothing to apply, which restores v2.5.1 behaviour for the common case:
```go
if len(ass.Setters) > 0 {
manifestList, err = ass.Apply(ctx, manifestList)
if err != nil {
return err
}
}
```
That alone would not help users who *do* pass overrides, so preserving the value verbatim (or at least carrying over the input's chomping indicator) in the re-emit is the more complete fix.
### Why it matters in practice
Skaffold's Helm deployer runs `filter` as Helm's post-renderer, so this reaches anything deployed that way. Helm stores the post-rendered output, so the damaged value is what lands in the cluster.
The case that led us here: the `grafana/k8s-monitoring` chart mounts a `self-reporting-metric.prom` file into Grafana Alloy and scrapes it with the node_exporter textfile collector. That file is OpenMetrics, whose `# EOF` marker must be newline-terminated. With the newline stripped, Alloy logs this on every scrape interval, indefinitely:
```
level=error msg="failed to collect textfile data" component_path=/
component_id=prometheus.exporter.unix.kubernetes_monitoring_telemetry collector=textfile
file=self-reporting-metric.prom err="failed to parse textfile data from
\"/etc/alloy/self-reporting-metric.prom\": text format parsing error in line 12:
unexpected end of input stream"
```
It took a while to find because the stored release manifest differs from a plain `helm template` by exactly one byte, on one key.
### Helm is not involved
For completeness — the same value through Helm v4's postrenderer-plugin plumbing with a no-op plugin is preserved, so the transformation is skaffold's:
```sh
mkdir -p /tmp/nl/noop
printf 'name: "noopplug"\nversion: "0.1.0"\ntype: postrenderer/v1\napiVersion: v1\nruntime: subprocess\nruntimeConfig:\n platformCommand:\n - command: /usr/bin/cat\n' > /tmp/nl/noop/plugin.yaml
helm plugin install /tmp/nl/noop
helm template t ./chart --post-renderer noopplug | grep 'zzz:'
# zzz: | <-- preserved
```
### Information
- **skaffold version**: v2.24.0 (also reproduced on v2.6.0 through v2.23.0; see table)
- **helm version**: v4.2.3
- **operating system**: Ubuntu 26.04 LTS, x86_64
- **installed via**: released binaries from this repo
Contributor guide
Research direction
Reproduce the trailing-newline loss with the provided Helm manifest, then inspect cmd/skaffold/app/cmd/filter.go, especially runFilter and its apply-setters pass. Verify behavior with and without manifest overrides, and consider the existing applysetters.Apply path; done means filter preserves the final value's newline while still handling overrides correctly.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go, helm, kubernetes
- Domain
- cli, devops, tooling
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 78/100