fix(manifest): parse JSON pod manifests instead of silently no-opping (issue #74) - #93
Merged
Merged
Conversation
Parse's YAML per-document loop silently skipped (continue) a document that parsed to an empty root map, producing a zero-value ParseResult with no error. splitDocuments already strips whitespace/comment-only chunks before a document reaches this point, so a real-content document that yields zero fields is always a parser defect or an unsupported top-level shape (e.g. a bare list instead of a map) -- never a legitimate no-op. Same defect class as issue #66 (docs/devlog.md, 2026-07-09): every silent-default in the input path eventually admits something dangerous. Closes the YAML-side half of the standing lesson; the JSON path (issue #74) already rejects an empty/kind-less document.
Apply Pod now accepts application/json in addition to application/yaml (issue #74).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
POST /api/v1/podswithContent-Type: application/jsonreturned201 {"pods":null}and created nothing (issue #74: POST /api/v1/pods silently no-ops on a JSON body), because the endpoint always fed the raw body to a hand-rolled, YAML-only, line-based parser regardless of content type. This makes JSON manifests actually parse (JSON is documented as an accepted body format), and makes any manifest document that resolves to nothing a400, never a silent201.Changes
internal/manifest/parse.go:Parsenow sniffs the body's first non-whitespace byte; a{or[routes the whole body throughencoding/jsoninstead of the YAML line-walker (JSON has no---multi-document convention, so a JSON body is always exactly one document, or an array of documents). The existing kind-dispatch switch (Pod/Job/Deployment/StatefulSet/CronJob) is factored out into a sharedparseDocumenthelper so both paths use identical logic — no changes were needed anywhere else in the manifest package, sinceencoding/json's native decode (map[string]interface{},[]interface{},string,float64,bool) already satisfies every existing map-walking helper (getString's fallback already doesfmt.Sprintf("%v", v)for non-string values, matching what call sites already expect from bare YAML scalars).internal/manifest/parse.go: the YAML per-document loop used to silentlycontinuepast a document that parsed to an empty root map, returning a zero-valueParseResultwith no error. SincesplitDocumentsalready strips whitespace/comment-only chunks before a document reaches this point, a real-content document that yields zero fields is always a parser defect or an unsupported top-level shape (e.g. a bare list instead of a map) — never a legitimate no-op. This now returns an error instead. Same defect class as issue Flow-style YAML maps (resources: { cpu: "1", memory: 512Mi }) are silently dropped — pod admitted with zero requests #66 (docs/devlog.md's 2026-07-09 entry, "flow-style YAML maps silently dropped"): "every silent-default in the input path eventually admitted something dangerous. Any new parser branch must reject what it does not understand." This closes the YAML-side half of that standing lesson; the JSON path enforces the same rule for an empty/kind-less JSON document.internal/manifest/parse_test.go:TestParse_ValidJSONPod(a pretty-printed JSON Pod manifest parses identically to the YAML equivalent),TestParse_EmptyResultDocumentIsError(an empty JSON{}document errors),TestParse_YAMLEmptyResultDocumentIsError(a bare top-level YAML list, which has no map, now errors instead of silently vanishing).internal/api/pods_mutate_test.go:TestApplyPod_JSON(a real HTTPPOSTwith a JSON body creates the pod and it's findable in the store — the exact symptom from the issue, now green) andTestApplyPod_MalformedJSON(an unparseable JSON body gets400, never201).README.md: the "Apply Pod" section now documentsContent-Type: application/jsonas an accepted alternative to YAML, with a matchingcurlexample, and states the empty/no-kinddocument is a400..gitignore: ignore local.kazi/and.mcp.jsontooling artifacts (unrelated housekeeping from the authoring session, kept as its own commit).Out of scope, deliberately untouched:
internal/manifest/yaml.go's hand-rolled YAML parser internals (JSON is routed around it, not into it), andinternal/api/pods_mutate.go(the handler already content-sniffs by always callingmanifest.Parse; it never readContent-Typeand still doesn't — the fix lives entirely in the parser).Testing
Run from a clean worktree at this branch's tip:
go build ./...— passes.go vet ./...— passes.staticcheck ./...— passes.go test ./... -race -timeout 120s -count=1— passes, all 13 packages, ~40s wall clock (internal/watcheris the slow package at ~37s; everything else is 1-4s).go test ./internal/manifest/... -race -vandgo test ./internal/api/... -race -v— the new tests observed red before each fix and green after (verified by hand for the YAML backstop commit; verified via kazi's t0/t-final predicate vector for the JSON-routing commits).Not yet done (intentionally out of scope for this PR — the coordinator handles it centrally after merge): a live
curlagainst the DGX host reproducing the exact issue #74 repro steps. Recipe for that:Linked issues
Part of #74 (POST /api/v1/pods with a JSON body returns 201
{"pods":null}and creates nothing) — not closing it here; the coordinator closes it after live-verifying on the DGX per the recipe above.