fix: normalize Additional Host Path Mounts textarea to a comma list#740
Conversation
The field's help text says "one per line", but the multiline value was rendered into a single-quoted YAML scalar, which YAML-folds the newline into a space. runtime-api then split only on commas/newlines, so the space-joined value became one malformed token and was skipped — no mounts. Normalize newlines to a comma list at render time (the proven splitList/trim/join idiom used for the model textareas), so the env var is always a clean single-line comma list. Works with the currently deployed runtime-api too, no image rebuild required.
|
✅ Review complete. This review was performed through OpenHands Cloud Automation. You can log in and view the conversation here. |
all-hands-bot
left a comment
There was a problem hiding this comment.
🟢 Good taste — Minimal, correct fix following the exact idiom already established in this file for other textarea fields.
What the change does: Replaces a naive ConfigOption call with the same splitList/trim/join normalization pattern already used for anthropic_models, azure_deployments, custom_models, and other textarea fields in this file. This prevents KOTS's single-quoted YAML scalar folding from space-joining multi-line entries before runtime-api parses them — a real customer-impacting bug.
Edge case correctness:
- Empty field →
""(empty list join = empty string, identical to before) ✅ - Single path →
"path"✅ - Multiple paths →
"path1,path2"✅ - Windows
\r\nline endings → handled viareplace "\r" "\n"✅ - Empty / whitespace-only lines → filtered by
trim+if $e✅
The inline comment on line 315 is warranted — the template is opaque and the YAML-folding root cause is non-obvious to anyone who hasn't encountered it. It's a "workaround for external bug" comment in the best sense.
[RISK ASSESSMENT]
- [Overall PR]
⚠️ Risk Assessment: 🟢 LOW
replicated/-only change; no chart templates touched; no version bump needed. The comma-separated output is already accepted by the currently deployed runtime-api. Follows a proven pattern used for every other textarea field in the same file. Fully backward-compatible.
VERDICT:
✅ Worth merging: Correct fix, minimal scope, consistent with existing patterns in the file.
KEY INSIGHT:
The fix is exactly right because it reuses the same normalization idiom already proven for all other textarea fields in this config — consistency here is a feature, not laziness.
This review was generated by an AI agent (OpenHands) on behalf of the user through OpenHands Automation. View conversation
The branch-protection ruleset requires the 9 path-filtered publish-charts(...) checks, which never run on replicated/-only PRs, leaving them permanently BLOCKED. Touch a standalone chart to fire the chart workflow so those checks report. (Also a true doc note.)
Problem
The KOTS Additional Host Path Mounts field (
sandbox_additional_host_paths, help text "one per line") is rendered into a single-quoted inline YAML scalar:A multi-line value inside a single-quoted scalar gets YAML-folded — the newline between entries becomes a single space. runtime-api then split only on commas/newlines, so a two-line value arrived as one space-joined token, was read as a single malformed entry, and was silently skipped → zero hostPath mounts for the sandbox pods. A customer hit this verbatim (their support bundle showed
ADDITIONAL_HOST_PATHS = "/proj/a:/proj/a /proj/b:/proj/b"and the runtime-api skip log).Fix
Normalize the textarea to a clean single-line comma list at render time, using the same
splitList "\n" | trim | joinidiom already used for every model textarea in this file (e.g.anthropic_modelsat line 31/262,azure_deployments,custom_models, …):This feeds the shared
charts/runtime-api/templates/_env.yamlhelper, so the runtime-api Deployment and the warm-runtimes/cleanup CronJobs all get the clean value. Because it emits commas (not a folded space), it works with the currently shipped runtime-api image — a Cloud-only release fixes affected installs without a runtime-api rebuild.Relationship to runtime-api#617
Belt-and-suspenders with OpenHands/runtime-api#617, which makes the parser also split on whitespace. Either alone fixes it; together the value is clean at the source and the parser is tolerant of any separator (so already-deployed space-folded configs auto-heal on upgrade).
Validation
replicated/change only — no chart template touched, so no chart version bump. Template braces/quotes balanced; mirrors the proven model-textarea idiom. The end-to-end behavior (space → 0 mounts; comma list → mounts present + readable) was verified live on an embedded-cluster install via runtime-api#617's reproduction.