(feat) early snap support#1238
Conversation
The OpenShell snap cannot chown VM rootfs files to the default 10001:10001 sandbox identity because snap confinement only permits ownership changes to the snap_daemon shared user and group. Add SNAP_SANDBOX mode for the VM driver so snap builds map the in-VM sandbox user and group to 584788:584788, rewrite existing /etc/passwd and /etc/group sandbox entries, normalize /sandbox ownership after extraction, and salt the prepared rootfs cache by the effective sandbox UID/GID. Skip symlinks during /sandbox ownership normalization so the snap-sandboxed gateway does not need CAP_DAC_OVERRIDE for symlink ownership changes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
This is a stepping stone towards a full snapcraft based build, by allowing us to reuse the existing mise-based build-system to construct the binaries and merely assemble the snap with "snap pack". When building the package and installing locally please connect both kvm, system-observe and ssh-public-keys (this grants access to connecting over ssh). Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Document how to build and use a locally-built snap package of openshell, as well as how it works internally. Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
|
Thank you for your submission! We ask that you sign our Developer Certificate of Origin before we can accept your contribution. You can sign the DCO by adding a comment below using this text: I have read the DCO document and I hereby sign the DCO. You can retrigger this bot by commenting recheck in this Pull Request. Posted by the DCO Assistant Lite bot. |
|
I will put another set of eyes on this from a snapping perspective |
|
With this I can get a working VM sandbox on Ubuntu 26.04. My next steps are to propose a full snapcraft.yaml and then a full publishing pipeline. |
|
I have read the DCO document and I hereby sign the DCO. |
|
recheck |
| path = "src/main.rs" | ||
|
|
||
| [features] | ||
| SNAP_SANDBOX = [] |
There was a problem hiding this comment.
Cargo feature names would normally be lowercase, so snap_sandbox or snap-sandbox.
There was a problem hiding this comment.
ack, I'll update this in the next round
| fs::symlink_metadata(&link) | ||
| .expect("stat symlink") | ||
| .file_type() | ||
| .is_symlink() |
There was a problem hiding this comment.
Looks like this is checking that the symlink is still a symlink. I imagine we want to validate the owner/group of the file that the symlink references, to make sure the chown did not affect it?
| let mut lines = Vec::new(); | ||
| for existing in contents.lines() { | ||
| if exists(existing) { | ||
| if !replaced { |
There was a problem hiding this comment.
Nit: this is going out of our way to only replace the first matching line. If there's not a specific reason for doing this, it could be simpler to just replace all matching lines. Alternatively, if we're only expecting a single match, we could give an error if a second match is found. I think this function is only used to search for a specific user or group in passwd, group, shadow, and gshadow, in which case it would be a misconfiguration for the same user or group to appear twice in the same file?
3 participants
Summary
This pull request adds early support for snap packaging so that openshell can be built in CI and eventually distributed from the snap store.
Related Issue
Changes
Testing
mise run pre-commitpassesChecklist