/snap-confine

Make snap mount directory configurable #119

Merged
merged 3 commits into from Aug 26, 2016

Conversation

Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
6 participants
@zyga @chipaca @mvo5 @Conan-Kudo @tyhicks @jdstrand
@zyga
Collaborator

zyga commented Aug 26, 2016

This patch adds the configuration option --with-snap-mount-dir that
can change the default /snap directory to another location. The code
has been tweaked to respect the new macro SNAP_MOUNT_DIR as well as
to treat /snap bind mount specially.

Signed-off-by: Zygmunt Krynicki zygmunt.krynicki@canonical.com

zyga added some commits Aug 26, 2016

@zyga
Make snap mount directory configurable 
This patch adds the configuration option --with-snap-mount-dir that
can change the default /snap directory to another location. The code
has been tweaked to respect the new macro SNAP_MOUNT_DIR as well as
to treat /snap bind mount specially.

Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
5e38b01
@zyga
Merge branch 'master' of github.com:snapcore/snap-confine into snap-m… 
…ount-dir
b9e7f80
@chipaca
Member

chipaca commented Aug 26, 2016

👍

that's a surprisingly small number of places we use that
src/mount-support.c
void setup_snappy_os_mounts()
{
debug("%s", __func__);
char rootfs_dir[MAX_BUF] = { 0 };
- // Create a temporary directory that will become the root directory of this
@zyga

zyga Aug 26, 2016

Collaborator

I actually wonder why indent chose to change this line

@mvo5
Contributor

mvo5 commented Aug 26, 2016

👍
@Conan-Kudo

Conan-Kudo commented Aug 26, 2016

👍
@zyga
Respect SNAP_MOUNT_DIR in apparmor profile 
This patch changes the apparmor profile to respect the SNAP_MOUNT_DIR
configuration option. It was tested with the default value as testing
other values is difficult right now.

Some of the profile refers to /snap in a genuine way as it runs after
the call to pivot_root. Ideally this would be better separated but I
would like to make this change as small as it can for now.

Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
c77ce3d
src/snap-confine.apparmor.in
@@ -94,19 +94,21 @@
# for running snaps on classic
mount options=(rw rslave) -> /,
@zyga

zyga Aug 26, 2016

Collaborator

FYI: this is split because in the past the regular expression /{tmp/snap.rootfs_*,}snap/ covered /snap which is no longer a constant.

src/snap-confine.apparmor.in
- /{tmp/snap.rootfs_*,}snap/ r,
- /{tmp/snap.rootfs_*,}snap/** r,
+ /tmp/snap.rootfs_*/snap/ r,
+ /tmp/snap.rootfs_*/snap/** r,
@jdstrand

jdstrand Aug 26, 2016

Contributor

I'm not sure how this change is related to this PR, but the change is fine.

+1 on all changes.

@zyga

zyga Aug 26, 2016

Collaborator

Like this: https://github.com/snapcore/snap-confine/pull/119/files#r76422977

@tyhicks
Collaborator

tyhicks commented Aug 26, 2016

Code changes look good to me and I see that @jdstrand acked the policy changes. Thanks!

@zyga zyga merged commit 7786446 into master Aug 26, 2016
1 check passed

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@zyga zyga deleted the snap-mount-dir branch Sep 5, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment