feat: add mount setting for app#60
Conversation
There was a problem hiding this comment.
Pull request overview
Tip
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.
This PR adds support for configuring host bind mounts for ONCE applications, exposing the setting via both the TUI settings UI and the CLI, and wiring the mounts into the container deploy configuration.
Changes:
- Add a new Mounts settings section in the UI with a dedicated mounts form (including scrolling + dynamic row creation).
- Extend
docker.ApplicationSettingsto persistmountsand validate mount constraints (absolute paths, no duplicate targets, no conflicts with built-in volume mounts). - Add CLI support for repeated
--mount SOURCE:TARGETflags and apply them to app settings; include docker settings marshal/equality/validation tests for mounts.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/ui/settings_menu.go | Adds “Mounts” to the settings menu. |
| internal/ui/settings_form_mounts.go | New mounts editor form (two-column source/target, scrolling, dynamic row append). |
| internal/ui/settings.go | Routes the new Mounts section; validates settings before deploy on submit. |
| internal/ui/app.go | Adds SettingsSectionMounts constant. |
| internal/docker/application_settings.go | Adds MountSetting, Mounts field, validation, and equality checks. |
| internal/docker/application_settings_test.go | Adds tests for mounts marshal/omit/equality/validation. |
| internal/docker/application.go | Defines mount validation errors; applies bind mounts during container mount construction. |
| internal/command/settings_flags.go | Adds --mount flag parsing and applies mounts to settings updates/creation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if !filepath.IsAbs(m.Target) { | ||
| return ErrMountTargetRelative | ||
| } | ||
| if reserved[m.Target] { | ||
| return ErrMountTargetReserved |
There was a problem hiding this comment.
ValidateMounts checks reserved/duplicate targets using the raw m.Target string. That can be bypassed with equivalent paths like /storage/, /storage/., or /storage/../storage, which would slip past the reserved/seen maps but still resolve to the same mount point. Consider normalizing (e.g., filepath.Clean (or path.Clean for container paths) + possibly trimming trailing slashes) before the abs/reserved/duplicate checks, and perform comparisons on the normalized value.
| func NewSettingsFormMounts(settings docker.ApplicationSettings) SettingsFormMounts { | ||
| var items []FormItem | ||
|
|
||
| for _, m := range settings.Mounts { | ||
| items = append(items, newMountSourceItem(m.Source), newMountTargetItem(m.Target)) |
There was a problem hiding this comment.
This adds a new settings form with non-trivial behavior (dynamic row append on rebuild, mount extraction on submit, scrolling/focus handling), but there are no corresponding UI tests. Other settings forms in this package are covered by internal/ui/settings_form_test.go; adding similar tests for mounts would help prevent regressions (e.g., verifying a new blank row appears after entering a source, and that submit produces the expected SettingsSectionSubmitMsg mounts slice).
Hi, I noticed that ONCE doesn't support setting bind mounts for apps, but for some use cases. I would like to be able to mount a file or directory on the host into a container, for instance, for self-hosted media apps (music, video and photos). This PR adds this ability to app setting.