Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
Prefer the "core" snap is one is available. #161
Conversation
| + * (legacy). The mount point dependes on build-time configuration and may | ||
| + * differ from distribution to distribution. | ||
| + **/ | ||
| +static const char *sc_get_outer_core_mount_point() |
jdstrand
reviewed
Sep 29, 2016
The access() checks add a TOCTOU but this is not a security concern because the code calling sc_get_inner_core_mount_point and sc_get_outer_core_mount_point fails closed if the mount point is removed after the existence check but before the mount and mimic.
+1 on code and policy (though please address minor nitpick).
Please add tests before committing.
| @@ -107,7 +107,7 @@ | ||
| # mount calls to setup the pivot_root based chroot with the core snap as | ||
| # the root filesystem. | ||
| - mount options=(rw bind) @SNAP_MOUNT_DIR@/ubuntu-core/*/ -> /tmp/snap.rootfs_*/, | ||
| + mount options=(rw bind) @SNAP_MOUNT_DIR@/{ubuntu-,}core/*/ -> /tmp/snap.rootfs_*/, |
jdstrand
Sep 29, 2016
Contributor
For consistency with other rules with alternation, can we use the equivalent {,ubuntu-} here and elsewhere instead?
zyga
added some commits
Sep 30, 2016
zyga
merged commit 82da3ac
into
master
Sep 30, 2016
1 check passed
| +summary: The snap named 'core' is preferred to the snap 'ubuntu-core' | ||
| +prepare: | | ||
| + snap install --devmode snapd-hacker-toolbelt | ||
| + snap install core |
jdstrand
Sep 30, 2016
Contributor
Wouldn't snap install ubuntu-core ; snap install core ; snap install --devmode snapd-hacker-toolbelt be better? At some point, snap install --devmode snapd-hacker-toolbelt will pull 'core' instead of 'ubuntu-core' and then this test wouldn't be testing anything any more.
zyga
Sep 30, 2016
Collaborator
The "ubuntu-core" snap is installed early in spread-prepare.sh. Even if installing snapd-hacker-toolbelt would pull in core we still install it separately. I think this is correct.
zyga commentedSep 29, 2016
This patch allows the core snap to be named just "core" (instead of
ubuntu-core). The apparmor profile is updated to allow both directories
to be used.
Fixes: https://bugs.launchpad.net/snap-confine/+bug/1628612
Signed-off-by: Zygmunt Krynicki zygmunt.krynicki@canonical.com