fix: report manifests that fail to decode instead of dropping them si… - #1236
fix: report manifests that fail to decode instead of dropping them si…#1236Eljees wants to merge 2 commits into
Conversation
…lently parseObjects() falls back to an unstructured object whenever the typed decoder fails. The fallback exists for custom resources whose Go type KubeLinter does not know, but it also swallowed a manifest of a registered kind that simply is invalid: the workload became an unstructured object, extract.PodTemplateSpec() no longer saw it, and every pod-spec based check silently stopped applying to it. The user was left with an unrelated diagnostic about a different object. Restrict the fallback to kinds that are not registered in the scheme (runtime.IsNotRegisteredError), so a broken Deployment surfaces as an invalid object while custom resources keep parsing as before. The invalid objects were then still hidden behind --verbose in the lint command; that gate arrived with a mechanical code move and is dropped, so the load failure and its cause are printed by default. Fixes stackrox#591 Fixes stackrox#669 Signed-off-by: Eljees <3.14hell@gmail.com>
📝 WalkthroughSummary by CodeRabbit
WalkthroughKubernetes parsing now returns errors for invalid registered objects instead of masking them with unstructured decoding. Blank YAML documents are skipped. KubeLinter reports failed object loads on stderr without requiring ChangesInvalid object handling
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1236 +/- ##
===========================================
- Coverage 62.36% 31.43% -30.93%
===========================================
Files 197 239 +42
Lines 4854 6569 +1715
===========================================
- Hits 3027 2065 -962
- Misses 1439 4327 +2888
+ Partials 388 177 -211
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Printing load failures by default surfaced a pre-existing false positive: a document that holds nothing but comments is not a manifest, but the typed decoder still fails on it with "Object 'Kind' is missing" and it was recorded as an invalid object. Files that separate sections with "--- # some comment" therefore produced a warning per separator, and --error-on-invalid-resource failed on them. The repository's own tests/checks/env-var-value-from.yml is such a file, which is what turned the e2e bats run red. Treat a document whose every line is blank, a comment, or a bare document marker the same way as an empty one: skip it before decoding, so it is neither a valid object nor an invalid one. Signed-off-by: Eljees <3.14hell@gmail.com>
|
The first CI run was red on That turned out to be a real false positive rather than a test problem: a document that holds nothing but comments is not a manifest, yet it was already being recorded as an invalid object — and already failed Fixed in 30ed792: a document whose every line is blank, a comment, or a bare document marker is now skipped before decoding, the same way an empty one is. Added a regression test for it; no existing test or fixture was adjusted to accommodate the change. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkg/lintcontext/parse_yaml.go (1)
71-77: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winHandle unregistered custom resources inside
v1.Listitems.
parseObjectsfalls back only when the top-level decode fails. Thev1.Listbranch then decodes each Raw item withd.Decode(item.Raw, ...)and returns immediately on error, so an unregistered custom resource in a list item makes the whole list invalid. Reuse the top-level fallback behavior for list items and add a list regression test.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/lintcontext/parse_yaml.go` around lines 71 - 77, Update the v1.List item-decoding flow in parseObjects to apply the same runtime.IsNotRegisteredError fallback used for top-level decoding: preserve registered-type errors, but decode unregistered item resources as unstructured and continue processing the remaining items. Add a regression test covering an unregistered custom resource nested in a v1.List.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/lintcontext/parse_yaml.go`:
- Around line 187-208: Update isBlankDocument to recognize YAML end markers
followed by optional whitespace and a comment, such as "... # comment", as
blank-document lines while preserving the existing handling of bare markers. Add
a regression test covering a document with a commented explicit end marker and
verify loadObjectFromYAMLReader does not pass it to object parsing or report it
as invalid.
---
Outside diff comments:
In `@pkg/lintcontext/parse_yaml.go`:
- Around line 71-77: Update the v1.List item-decoding flow in parseObjects to
apply the same runtime.IsNotRegisteredError fallback used for top-level
decoding: preserve registered-type errors, but decode unregistered item
resources as unstructured and continue processing the remaining items. Add a
regression test covering an unregistered custom resource nested in a v1.List.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 5e9b4fc6-7e39-4353-8997-67cc9597790f
📒 Files selected for processing (2)
pkg/lintcontext/parse_yaml.gopkg/lintcontext/parse_yaml_invalid_object_test.go
| // isBlankDocument reports whether a YAML document carries no content at all: every | ||
| // line is blank, a comment, or a bare document marker. Files that separate sections | ||
| // with "--- # some comment" produce such documents, and there is nothing in them to | ||
| // decode - reporting them as unreadable objects would be a false positive. | ||
| func isBlankDocument(doc []byte) bool { | ||
| for _, line := range bytes.Split(doc, []byte("\n")) { | ||
| line = bytes.TrimSpace(line) | ||
| if len(line) == 0 || line[0] == '#' || bytes.Equal(line, []byte("---")) || bytes.Equal(line, []byte("...")) { | ||
| continue | ||
| } | ||
| return false | ||
| } | ||
| return true | ||
| } | ||
|
|
||
| func (l *lintContextImpl) loadObjectFromYAMLReader(filePath string, r *yaml.YAMLReader) error { | ||
| doc, err := r.Read() | ||
| if err != nil { | ||
| return err | ||
| } | ||
| doc = bytes.TrimSpace(doc) | ||
| if len(doc) == 0 { | ||
| if len(doc) == 0 || isBlankDocument(doc) { |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate parse_yaml.go =="
fd -a 'parse_yaml.go' . || true
echo "== relevant file section =="
if [ -f pkg/lintcontext/parse_yaml.go ]; then
wc -l pkg/lintcontext/parse_yaml.go
sed -n '150,230p' pkg/lintcontext/parse_yaml.go | cat -n
fi
echo "== app/use of isBlankDocument/loadObjectFromYAMLReader && parseObjects =="
rg -n "isBlankDocument|loadObjectFromYAMLReader|parseObjects|\\.Read\\(" pkg/lintcontext -S || true
echo "== dependency versions mentioning apimachinery yaml =="
rg -n "apimachinery|k8s\\.io/apimachinery|yaml.*decoder|YAMLReader" -S --glob '!vendor/**' .
echo "== go.mod/go.sum relevant =="
if [ -f go.mod ]; then sed -n '1,220p' go.mod; fi
rg "k8s.io/apimachinery" go.sum go.mod || trueRepository: stackrox/kube-linter
Length of output: 24697
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== fetch k8s.io/apimachinery v0.36.3 yaml decoder relevant section =="
curl -fsSL https://raw.githubusercontent.com/kubernetes/apimachinery/v0.36.3/pkg/util/yaml/decoder.go | nl -ba | sed -n '1,260p'
echo "== deterministic blank-document probe matching current code =="
python3 - <<'PY'
import subprocess, textwrap, re
src = None
try:
from pathlib import Path
p = Path("pkg/lintcontext/parse_yaml.go")
if p.exists():
src = p.read_text()
except Exception as e:
print(f"read_source_error {e}")
def is_blank_document(doc):
for line in doc.split("\n"):
line = line.strip()
if len(line) == 0 or line[0] == '#' or line == "---" or line == "...":
continue
return False
return True
samples = [
("empty\n", True),
("# comment\n", True),
("---\n", True),
("--- # comment\n", True),
("...\n", True),
("... # comment\n", False),
("... \t \n", True),
("... \t comment\n", False),
]
for s, expected in samples:
print(repr(s), is_blank_document(s), expected)
# structural/context check: does original predicate only pass exact marker?
if src:
for marker in ['bytes.Equal(line, []byte("---"))', 'bytes.Equal(line, []byte("..."))']:
print(f"contains {marker}:", marker in src)
print("trailing-comment handling likely none if exact Equal used")
PYRepository: stackrox/kube-linter
Length of output: 342
🏁 Script executed:
#!/bin/bash
set -euo pipefail
TMP="$(mktemp -d)"
cd "$TMP"
echo "== fetch and inspect k8s.io/apimachinery v0.36.3 decoder.go relevant sections =="
url="https://raw.githubusercontent.com/kubernetes/apimachinery/v0.36.3/pkg/util/yaml/decoder.go"
if command -v curl >/dev/null 2>&1; then
curl -fsSL "$url" > decoder.go
elif command -v wget >/dev/null 2>&1; then
wget -q -O decoder.go "$url"
else
echo "no curl or wget"
exit 0
fi
if [ -f decoder.go ]; then
wc -l decoder.go
sed -n '1,230p' decoder.go | awk '{printf "%6d\t%s\n", NR, $0}'
echo "== occurrences of document markers in decoder.go =="
grep -n '---\|\.\.\.\\|Document|delimiter|YAMLReader|Read' decoder.go || true
fi
echo "== deterministic blank-document probe matching current predicate =="
python3 - <<'PY'
def is_blank_document(doc):
for line in doc.split("\n"):
line = line.strip()
if len(line) == 0 or line[0] == '#' or line == "---" or line == "...":
continue
return False
return True
samples = [
("", True),
("# comment\n", True),
("--- # comment\n# another\n", True),
("... # comment\n", False),
("... \t \n", True),
("...\ncore: apiVersion: v1\n", False),
]
for s, expected in samples:
print("input", repr(s), "isBlankDocument", is_blank_document(s), "expected", expected)
PYRepository: stackrox/kube-linter
Length of output: 8771
🏁 Script executed:
#!/bin/bash
set -euo pipefail
TMP="$(mktemp -d)"
cd "$TMP"
url="https://raw.githubusercontent.com/kubernetes/apimachinery/v0.36.3/pkg/util/yaml/decoder.go"
( command -v curl >/dev/null 2>&1 && curl -fsSL "$url" > decoder.go ) || \
{ command -v wget >/dev/null 2>&1 && wget -q -O decoder.go "$url"; } || true
if [ -f decoder.go ]; then
echo "== lines 207-280 =="
awk '{printf "%6d\t%s\n", NR, $0}' decoder.go | sed -n '207,280p'
echo "== read() lines 280-360 =="
awk '{printf "%6d\t%s\n", NR, $0}' decoder.go | sed -n '280,360p'
echo "== exact marker strings =="
grep -n '---\|'\\.\\.\\.' decoder.go || true
fiRepository: stackrox/kube-linter
Length of output: 219
Handle comments after explicit end markers.
isBlankDocument only accepts the exact ... line. A document such as ... # comment is treated as content and passed to parseObjects, which can report a false invalid object. Accept a document marker followed by optional whitespace and an optional comment, plus a regression test.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pkg/lintcontext/parse_yaml.go` around lines 187 - 208, Update isBlankDocument
to recognize YAML end markers followed by optional whitespace and a comment,
such as "... # comment", as blank-document lines while preserving the existing
handling of bare markers. Add a regression test covering a document with a
commented explicit end marker and verify loadObjectFromYAMLReader does not pass
it to object parsing or report it as invalid.
Source: MCP tools
|
The red check here is Codecov's chosen base is It fails the same way on every open PR here that has a codecov context - #1189, #1197, #1206, #1216 - and passes on none of them. The pattern fits coverage uploads having stopped landing on |
1 participant
…lently
parseObjects() falls back to an unstructured object whenever the typed decoder fails. The fallback exists for custom resources whose Go type KubeLinter does not know, but it also swallowed a manifest of a registered kind that simply is invalid: the workload became an unstructured object, extract.PodTemplateSpec() no longer saw it, and every pod-spec based check silently stopped applying to it. The user was left with an unrelated diagnostic about a different object.
Restrict the fallback to kinds that are not registered in the scheme (runtime.IsNotRegisteredError), so a broken Deployment surfaces as an invalid object while custom resources keep parsing as before.
The invalid objects were then still hidden behind --verbose in the lint command; that gate arrived with a mechanical code move and is dropped, so the load failure and its cause are printed by default.
Fixes #591
Fixes #669