fix(settings): refuse boot on a nested directory that serves no tenant - #711
Conversation
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (10)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. 📜 Recent review details🧰 Additional context used📓 Path-based instructions (5)Source excerpt: In a Docker / Podman / Kubernetes deployment, **`data_dir` must resolve to a host-backed volume**.📄 CodeRabbit inference engine (docs/src/content/docs/deployment.md) Files:
Source excerpt: | Change | Files to update | | ------ | --------------- | | Change architecture / add a package | `docs/src/content/docs/architecture.md`, `AGENTS.md` |📄 CodeRabbit inference engine (AGENTS.md) Files:
Source excerpt: **wire.go** — one `wire*` function per component, each handed the settings registry whole and deriving the per-call getters the internal packages take (`DLQFor`, `DedupeFor`, `GapWindow`, …) and registering its `AfterAdopt`...📄 CodeRabbit inference engine (docs/src/content/docs/architecture.md) Files:
See [AGENTS.md](../AGENTS.md) for project conventions, architecture notes, and AI agent instructions.📄 CodeRabbit inference engine (.github/copilot-instructions.md) Files:
Source excerpt: **Docs prose**: never hard-wrap Markdown — one paragraph is one line.📄 CodeRabbit inference engine (CONTRIBUTING.md) Files:
🪛 LanguageTooldocs/src/content/docs/architecture.md[style] ~223-~223: This word has been used in one of the immediately preceding sentences. Using a synonym could make your text more interesting to read, unless the repetition is intentional. (EN_REPEATEDWORDS_WHOLE) CHANGELOG.md[typographical] ~98-~98: Consider using an em dash in dialogues and enumerations. (DASH_RULE) 🔇 Additional comments (10)
📝 SummarySummary by CodeRabbit
WalkthroughNested settings roots with no serving tenant now fail at boot. Reloads can apply the same empty tree, remove all tenants, and keep the process running. Flat-directory behavior and validation behavior remain unchanged. ChangesNested settings tenant availability
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix · Severity of issue fixed: Medium Suggested reviewers: Merge Risk: ⚪ Minimal · up to Nested settings roots that serve no tenant now refuse to boot, while reloads and validation behave as before. No merge-blocking risk was identified. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The change tightens startup validation without broadening tenant access. Instances with no valid tenant configuration now refuse startup, while existing reload and recovery behavior remains intact. No introduced security weakness was identified in the inspected paths, but verification was limited to the affected lifecycle and tenant-routing controls. Retained concerns Security review detailsSecurity Blast Radius
Security Findings and Attack Paths
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
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 |
github-actions
Bot
added
documentation
|
📚 Docs preview is live → https://9ee4f67b-wavehouse-docs.wave-rf.workers.dev |
Code Coverage OverviewLanguages: Go GoThe overall line coverage in commit 7da05f8 in the Show a line coverage summary of the most impacted files.
Updated |
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
1 participant
Summary
A
settings.dirholding folders but none of the four files is read as nested. When every folder was rejected, or none was named by a tenant id (a fresh volume whose only entry islost+found, a parent directory mounted by mistake),settings.Openstill returned a registry and the server came up serving no tenant, answering every tenant routeunknown tenant. Before #598 that root refused boot.Boot now refuses a nested root that would serve no tenant, with the findings plus one naming the rule, through the same path a flat invalid root takes, so the error still points at
wavehouse validateandwavehouse bootstrap. The rule is boot's alone: a whole-directory reload that finds every folder gone or broken still drops every tenant and keeps running (#611), andwavehouse validateis unchanged. A root with one valid folder beside a rejected or stray one still boots and serves that tenant, and an empty or missing root still refuses as the four files, missing.Test plan
settings.Openrefuses a root whose every folder is rejected, and a root whose only folder islost+found, each with the findingssettings.Openon one valid folder beside a broken one and a stray one opens and serves the valid tenantwavehouse validateexit codesmake cipasses locally, integration and e2e includedRelated Issues
Closes #599
Part of #583