docs: fix default.json examples that stop the daemon from starting - #988
Conversation
|
@emrcbrn is attempting to deploy a commit to the NetBird GmbH Team on Vercel. A member of the Team first needs to authorize it. |
|
Warning Review limit reachedNext included review available in 9 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe documentation changes URL configuration fields to JSON objects and documents startup validation. Docker and Kubernetes instructions now provide writable state storage for daemon file updates and profile creation. ChangesConfiguration bootstrap
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Other Suggested reviewers: Merge Risk: ⚪ Minimal · up to The updated examples use the client’s required URL format and provide writable daemon state, so the documented Docker and Kubernetes startup paths are ready to merge. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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. A rabbit copies config with care Comment |
Every `default.json` example on the bootstrap page had at least one value the client cannot parse, and a bad value there is fatal: the daemon exits rather than falling back to defaults. - `ManagementURL` and `AdminURL` were shown as strings. `Config.ManagementURL` is a `*url.URL`, so the daemon dies with `cannot unmarshal string into Go struct field Config.ManagementURL of type url.URL`. Reproduced on 0.69.0, 0.73.0, 0.78.2 and 0.79.0-rc.1. - The Docker example bind-mounted the single file. The client rewrites `default.json` on first start via temp-file-plus-rename, and a rename cannot replace a bind-mounted file, so the daemon dies with `device or resource busy`. This happens with and without `:ro`. Mount the directory instead. - The Kubernetes example mounted the ConfigMap at the file path with `subPath`, which is the same read-only single-file mount. Seed a writable `emptyDir` from the ConfigMap with an init container instead. Verified in a systemd container against the real client: the corrected `default.json` starts cleanly, keeps the templated values, and regenerates `PrivateKey`. The fields left untouched (`IFaceBlackList` as an array, the empty `PrivateKey`, the platform path table, `status --check` values) were checked and are correct. The Kubernetes manifest is the one change not run end to end: no cluster was available. Both failure modes it avoids were reproduced directly with equivalent mounts, and the pattern it uses is the verified-working one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
113d9d2 to
5224ddb
Compare
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
2 participants
What
Every
default.jsonexample on Bootstrap peers via config file contains at least one value the client cannot parse. A bad value there is fatal: the daemon exits rather than falling back to defaults, so a reader following this page gets a peer that never starts.Found while investigating a community thread where a user bootstraps peers with Ansible and templates these files by hand.
The three defects
1.
ManagementURL/AdminURLshown as strings.Config.ManagementURLis a*url.URL, which marshals as an object. There is no customUnmarshalJSONin the package.Reproduced on 0.69.0, 0.73.0, 0.78.2 and 0.79.0-rc.1, so this is not a regression; the example has never worked.
2. The Docker example bind-mounts the single file. The client rewrites
default.jsonon first start (private key, SSH key) using temp-file-plus-rename, and a rename cannot replace a bind-mounted file:This fails with and without
:ro, so removing:rois not the fix. Mounting the directory is.3. The Kubernetes example mounts the ConfigMap at the file path via
subPath— the same read-only single-file mount. Replaced with an init container that seeds a writableemptyDir.Verification
Run against the real client in a systemd container, one scenario per case:
:rodevice or resource busy):roread-only file system, onactive_profile.json)The corrected
default.jsonstarts cleanly, keeps the templated values, and regeneratesPrivateKey(44 chars), confirming the page's existing claim about leaving it empty.Fields I checked and deliberately left alone, because they are correct:
IFaceBlackListas an array, the emptyPrivateKey, the platform path table (darwinandlinuxboth resolve to/var/lib/netbird), and thestatus --check live|ready|startupvalues.One limitation
The Kubernetes manifest is the only change not run end to end — no cluster was available in this environment. Both failure modes it avoids were reproduced directly with the equivalent Docker mounts, and the pattern it adopts is the one verified working, but the manifest itself deserves a look from someone with a cluster to hand.
🤖 Generated with Claude Code
Summary by CodeRabbit
{Scheme, Host}objects for management and administration URLs.