diff --git a/cmd/snap-update-ns/change.go b/cmd/snap-update-ns/change.go
index f8777871c58..e1a334ffd7f 100644
--- a/cmd/snap-update-ns/change.go
+++ b/cmd/snap-update-ns/change.go
@@ -65,6 +65,10 @@ type Change struct {
Action Action
}
+func (c *Change) GoString() string {
+ return fmt.Sprintf("%#+v", *c)
+}
+
// String formats mount change to a human-readable line.
func (c Change) String() string {
return fmt.Sprintf("%s (%s)", c.Action, c.Entry)
diff --git a/cmd/snap-update-ns/change_tricky_test.go b/cmd/snap-update-ns/change_tricky_test.go
new file mode 100644
index 00000000000..c6b791da82d
--- /dev/null
+++ b/cmd/snap-update-ns/change_tricky_test.go
@@ -0,0 +1,246 @@
+// -*- Mode: Go; indent-tabs-mode: t -*-
+
+/*
+ * Copyright (C) 2024 Canonical Ltd
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see .
+ *
+ */
+
+package main_test
+
+import (
+ . "gopkg.in/check.v1"
+
+ update "github.com/snapcore/snapd/cmd/snap-update-ns"
+ "github.com/snapcore/snapd/osutil"
+)
+
+func (s *changeSuite) TestContentLayout1InitiallyConnected(c *C) {
+ // NOTE: This doesn't measure what is going on during construction. It
+ // merely measures what is constructed is stable and that it does not cause
+ // further changes to be created.
+ current, err := osutil.LoadMountProfile("testdata/content-layout-1-initially-connected.current.fstab")
+ c.Assert(err, IsNil)
+ desired, err := osutil.LoadMountProfile("testdata/content-layout-1-initially-connected.desired.fstab")
+ c.Assert(err, IsNil)
+ changes := update.NeededChanges(current, desired)
+ showCurrentDesiredAndChanges(c, current, desired, changes)
+
+ // The change plan is to do nothing.
+ // Note that the order of entries is back to front.
+ //
+ // At this time, the mount namespace is correct:
+ // zyga@wyzima:/run/snapd/ns$ sudo nsenter -mtest-snapd-content-layout.mnt
+ // root@wyzima:/# ls -l /usr/share/secureboot/potato
+ // total 1
+ // -rw-rw-r-- 1 root root 22 Aug 30 09:36 canary.txt
+ // drwxrwxr-x 2 root root 32 Aug 30 09:36 meta
+ // root@wyzima:/# ls -l /snap/test-snapd-content-layout/
+ // current/ x1/ x2/
+ // root@wyzima:/# ls -l /snap/test-snapd-content-layout/x2/attached-content/
+ // total 1
+ // -rw-rw-r-- 1 root root 22 Aug 30 09:36 canary.txt
+ // drwxrwxr-x 2 root root 32 Aug 30 09:36 meta
+ c.Assert(changes, DeepEquals, []*update.Change{
+ {Action: "keep", Entry: current.Entries[4]},
+ {Action: "keep", Entry: current.Entries[3]},
+ {Action: "keep", Entry: current.Entries[2]},
+ {Action: "keep", Entry: current.Entries[1]},
+ {Action: "keep", Entry: current.Entries[0]},
+ })
+}
+
+func (s *changeSuite) TestContentLayout2InitiallyConnectedThenDisconnected(c *C) {
+ current, err := osutil.LoadMountProfile("testdata/content-layout-1-initially-connected.current.fstab")
+ c.Assert(err, IsNil)
+ desired, err := osutil.LoadMountProfile("testdata/content-layout-2-after-disconnect.desired.fstab")
+ c.Assert(err, IsNil)
+ changes := update.NeededChanges(current, desired)
+ showCurrentDesiredAndChanges(c, current, desired, changes)
+
+ // The change plan is to do detach the content entry.
+ //
+ // Detached entries are first isolated from mount propagation. So the bug
+ // here is that the mount entry propagated to the layout during initial
+ // construction sticks around and is not updated. This is a bug.
+ // This is tracked as https://warthogs.atlassian.net/browse/SNAPDENG-31645
+ //
+ // zyga@wyzima:/run/snapd/ns$ sudo nsenter -mtest-snapd-content-layout.mnt
+ // root@wyzima:/# ls -l /snap/test-snapd-content-layout/x2/attached-content/
+ // total 1
+ // -rw-rw-r-- 1 root root 46 Aug 30 09:36 hidden
+ // root@wyzima:/# ls -l /usr/share/secureboot/potato
+ // total 1
+ // -rw-rw-r-- 1 root root 22 Aug 30 09:36 canary.txt
+ // drwxrwxr-x 2 root root 32 Aug 30 09:36 meta
+ //
+ // Note that the order of entries is back to front. There is another bug
+ // here, although it is not visible from the change plan alone. The reverse
+ // order of mount entries listed here is actually stored as the new current
+ // mount profile. This is tracked as
+ // https://warthogs.atlassian.net/browse/SNAPDENG-31644
+ c.Assert(changes, DeepEquals, []*update.Change{
+ {Action: "keep", Entry: current.Entries[4]},
+ {Action: "keep", Entry: current.Entries[3]},
+ {Action: "keep", Entry: current.Entries[2]},
+ {Action: "unmount", Entry: withDetachOption(current.Entries[1])},
+ {Action: "keep", Entry: current.Entries[0]},
+ })
+
+ // The actual entry for clarity.
+ c.Assert(changes[3].Entry, DeepEquals, osutil.MountEntry{
+ Name: "/snap/test-snapd-content/x1",
+ Dir: "/snap/test-snapd-content-layout/x2/attached-content",
+ Type: "none",
+ Options: []string{"bind", "ro", "x-snapd.detach"},
+ })
+}
+
+func (s *changeSuite) TestContentLayout3InitiallyConnectedThenDisconnectedAndReconnected(c *C) {
+ current, err := osutil.LoadMountProfile("testdata/content-layout-2-after-disconnect.current.fstab")
+ c.Assert(err, IsNil)
+ desired, err := osutil.LoadMountProfile("testdata/content-layout-3-after-reconnect.desired.fstab")
+ c.Assert(err, IsNil)
+ changes := update.NeededChanges(current, desired)
+ showCurrentDesiredAndChanges(c, current, desired, changes)
+
+ // In theory we should get back to the initial state but the reality is
+ // much more complicated. The change looks good on paper but the
+ // propagation that is not taken into account makes the actual mount
+ // namespace incorrect. The content connection is new and correct but the layout
+ // is still the same and was not propagated.
+ //
+ // zyga@wyzima:/run/snapd/ns$ sudo nsenter -mtest-snapd-content-layout.mnt
+ // root@wyzima:/# ls -l /usr/share/secureboot/potato
+ // total 1
+ // -rw-rw-r-- 1 root root 22 Aug 30 09:36 canary.txt
+ // drwxrwxr-x 2 root root 32 Aug 30 09:36 meta
+ // root@wyzima:/# ls -l /snap/test-snapd-content-layout/x2/attached-content/
+ // total 1
+ // -rw-rw-r-- 1 root root 22 Aug 30 09:36 canary.txt
+ // drwxrwxr-x 2 root root 32 Aug 30 09:36 meta
+ //
+ // Yes, but:
+ //
+ // root@wyzima:/# cat /proc/self/mountinfo | grep attached
+ // 212 945 7:12 / /snap/test-snapd-content-layout/x2/attached-content ro,relatime master:34 - squashfs /dev/loop12 ro,errors=continue,threads=single
+ //
+ // root@wyzima:/# cat /proc/self/mountinfo | grep potato
+ // 572 598 7:12 / /usr/share/secureboot/potato ro,relatime master:34 - squashfs /dev/loop12 ro,errors=continue,threads=single
+ c.Assert(changes, DeepEquals, []*update.Change{
+ {Action: "keep", Entry: current.Entries[3]},
+ {Action: "keep", Entry: current.Entries[2]},
+ {Action: "keep", Entry: current.Entries[1]},
+ {Action: "keep", Entry: current.Entries[0]},
+ {Action: "mount", Entry: desired.Entries[1]},
+ })
+
+ // The actual entry for clarity.
+ c.Assert(changes[4].Entry, DeepEquals, osutil.MountEntry{
+ Name: "/snap/test-snapd-content/x1",
+ Dir: "/snap/test-snapd-content-layout/x2/attached-content",
+ Type: "none",
+ Options: []string{"bind", "ro"},
+ })
+}
+
+func (s *changeSuite) TestContentLayout4InitiallyDisconnectedThenConnected(c *C) {
+ current, err := osutil.LoadMountProfile("testdata/content-layout-4-initially-disconnected-then-connected.before.current.fstab")
+ c.Assert(err, IsNil)
+ desired, err := osutil.LoadMountProfile("testdata/content-layout-4-initially-disconnected-then-connected.desired.fstab")
+ c.Assert(err, IsNil)
+ changes := update.NeededChanges(current, desired)
+ showCurrentDesiredAndChanges(c, current, desired, changes)
+
+ c.Assert(changes, DeepEquals, []*update.Change{
+ {Action: "keep", Entry: current.Entries[3]},
+ {Action: "keep", Entry: current.Entries[2]},
+ {Action: "keep", Entry: current.Entries[1]},
+ {Action: "keep", Entry: current.Entries[0]},
+ {Action: "mount", Entry: desired.Entries[1]},
+ })
+
+ // The actual entry for clarity.
+ c.Assert(changes[4].Entry, DeepEquals, osutil.MountEntry{
+ Name: "/snap/test-snapd-content/x1",
+ Dir: "/snap/test-snapd-content-layout/x2/attached-content",
+ Type: "none",
+ Options: []string{"bind", "ro"},
+ })
+}
+
+func (s *changeSuite) TestContentLayout5InitiallyConnectedThenContentRefreshed(c *C) {
+ current, err := osutil.LoadMountProfile("testdata/content-layout-5-initially-connected-then-content-refreshed.before.current.fstab")
+ c.Assert(err, IsNil)
+ desired, err := osutil.LoadMountProfile("testdata/content-layout-5-initially-connected-then-content-refreshed.desired.fstab")
+ c.Assert(err, IsNil)
+ changes := update.NeededChanges(current, desired)
+ showCurrentDesiredAndChanges(c, current, desired, changes)
+
+ // This test shows similar behavior to -2- test - the layout stays propagated.
+ c.Assert(changes, DeepEquals, []*update.Change{
+ {Action: "keep", Entry: current.Entries[4]},
+ {Action: "keep", Entry: current.Entries[3]},
+ {Action: "keep", Entry: current.Entries[2]},
+ {Action: "unmount", Entry: withDetachOption(current.Entries[1])},
+ {Action: "keep", Entry: current.Entries[0]},
+ {Action: "mount", Entry: desired.Entries[1]},
+ })
+}
+
+func (s *changeSuite) TestContentLayout6InitiallyConnectedThenAppRefreshed(c *C) {
+ current, err := osutil.LoadMountProfile("testdata/content-layout-6-initially-connected-then-app-refreshed.before.current.fstab")
+ c.Assert(err, IsNil)
+ desired, err := osutil.LoadMountProfile("testdata/content-layout-6-initially-connected-then-app-refreshed.desired.fstab")
+ c.Assert(err, IsNil)
+ changes := update.NeededChanges(current, desired)
+ showCurrentDesiredAndChanges(c, current, desired, changes)
+
+ // In this case, because the attached content is mounted to $SNAP (and not $SNAP_COMMON), the path changes
+ // and both the layout and content are re-made.
+ c.Assert(changes, DeepEquals, []*update.Change{
+ {Action: "unmount", Entry: withDetachOption(current.Entries[4])},
+ {Action: "keep", Entry: current.Entries[3]},
+ {Action: "keep", Entry: current.Entries[2]},
+ {Action: "unmount", Entry: withDetachOption(current.Entries[1])},
+ {Action: "keep", Entry: current.Entries[0]},
+ // It is interesting to note that we first mount the content to $SNAP/attached-content
+ // and only then construct the layout from $SNAP/attached-content to /usr/share/secureboot/potato.
+ // This is correct but the ordering is fraigle.
+ {Action: "mount", Entry: desired.Entries[1]},
+ {Action: "mount", Entry: desired.Entries[0]},
+ })
+}
+
+// withDetachOption returns a copy of the given mount entry with the x-snapd.detach option.
+func withDetachOption(e osutil.MountEntry) osutil.MountEntry {
+ e.Options = append([]string{}, e.Options...)
+ e.Options = append(e.Options, "x-snapd.detach")
+ return e
+}
+
+func showCurrentDesiredAndChanges(c *C, current, desired *osutil.MountProfile, changes []*update.Change) {
+ c.Logf("Number of current entires: %d", len(current.Entries))
+ for _, entry := range current.Entries {
+ c.Logf("- current : %v", entry)
+ }
+ c.Logf("Number of desired entires: %d", len(desired.Entries))
+ for _, entry := range desired.Entries {
+ c.Logf("- desired: %v", entry)
+ }
+ c.Logf("Number of changes: %d", len(changes))
+ for _, change := range changes {
+ c.Logf("- change: %v", change)
+ }
+}
diff --git a/cmd/snap-update-ns/testdata/Makefile b/cmd/snap-update-ns/testdata/Makefile
new file mode 100644
index 00000000000..277360d68b5
--- /dev/null
+++ b/cmd/snap-update-ns/testdata/Makefile
@@ -0,0 +1,127 @@
+# This makefiles uses grouped-target feature and relies on it for correctness.
+ifeq (,$(findstring grouped-target,$(.FEATURES)))
+$(error You need make with the grouped-taget feature to build this dataset)
+endif
+
+fstab_files = content-layout-1-initially-connected.current.fstab \
+ content-layout-1-initially-connected.desired.fstab \
+ content-layout-2-after-disconnect.desired.fstab \
+ content-layout-2-after-disconnect.current.fstab \
+ content-layout-3-after-reconnect.desired.fstab \
+ content-layout-3-after-reconnect.current.fstab \
+ content-layout-4-initially-disconnected-then-connected.before.current.fstab \
+ content-layout-4-initially-disconnected-then-connected.desired.fstab \
+ content-layout-4-initially-disconnected-then-connected.current.fstab \
+ content-layout-5-initially-connected-then-content-refreshed.before.current.fstab \
+ content-layout-5-initially-connected-then-content-refreshed.desired.fstab \
+ content-layout-5-initially-connected-then-content-refreshed.current.fstab \
+ content-layout-6-initially-connected-then-app-refreshed.before.current.fstab \
+ content-layout-6-initially-connected-then-app-refreshed.desired.fstab \
+ content-layout-6-initially-connected-then-app-refreshed.current.fstab
+
+# None of the fstab files can be built in parallel as the depend on global system state.
+.NOTPARALLEL: $(fstab_files)
+.PHONY: all
+all: $(fstab_files)
+
+content-layout-1-initially-connected.desired.fstab content-layout-1-initially-connected.current.fstab &: test-snapd-content-layout_a_all.snap test-snapd-content_a_all.snap
+ sudo snap remove --purge test-snapd-content-layout
+ sudo snap remove --purge test-snapd-content
+ sudo snap install --dangerous $(word 1,$^)
+ sudo snap install --dangerous $(word 1,$^) # Reinstall to get another revision for better clarity of the data.
+ sudo snap install --dangerous $(word 2,$^)
+ sudo snap connect test-snapd-content-layout:just-content test-snapd-content:just-content
+ snap run test-snapd-content-layout.sh -c true
+ cp /var/lib/snapd/mount/snap.test-snapd-content-layout.fstab content-layout-1-initially-connected.desired.fstab
+ cp /run/snapd/ns/snap.test-snapd-content-layout.fstab content-layout-1-initially-connected.current.fstab
+ sudo snap remove --purge test-snapd-content-layout
+ sudo snap remove --purge test-snapd-content
+ patch = 0; i-- {
+ change := changes[i]
+ if change.Action == Keep {
+ profile.Entries = append(profile.Entries, change.Entry)
}
}
- return upCtx.SaveCurrentProfile(¤tAfter)
+ for _, change := range changes {
+ if change.Action == Mount {
+ profile.Entries = append(profile.Entries, change.Entry)
+ }
+ }
+
+ return profile
}
diff --git a/cmd/snap-update-ns/update_test.go b/cmd/snap-update-ns/update_test.go
index f231bb08464..a68db153193 100644
--- a/cmd/snap-update-ns/update_test.go
+++ b/cmd/snap-update-ns/update_test.go
@@ -363,14 +363,70 @@ func (s *updateSuite) TestKeepSyntheticMountsLP2043993(c *C) {
c.Assert(update.ExecuteMountProfileUpdate(upCtx), IsNil)
c.Assert(saved.Entries, HasLen, 2)
- // TODO: the change of order is a bit unexpected, but that is a larger issue
- // synth mount kept because it is needed by a desired mount
- c.Check(saved.Entries[1].Type, Equals, "tmpfs")
- c.Check(saved.Entries[1].Name, Equals, "tmpfs")
- c.Check(saved.Entries[1].Dir, Equals, filepath.Join(baseSourceDir, "rofs"))
- c.Check(saved.Entries[1].XSnapdSynthetic(), Equals, true)
- c.Check(saved.Entries[1].XSnapdNeededBy(), Equals, "test-id")
- c.Check(saved.Entries[0], DeepEquals, desiredMountEntry)
+ c.Check(saved.Entries[0].Type, Equals, "tmpfs")
+ c.Check(saved.Entries[0].Name, Equals, "tmpfs")
+ c.Check(saved.Entries[0].Dir, Equals, filepath.Join(baseSourceDir, "rofs"))
+ c.Check(saved.Entries[0].XSnapdSynthetic(), Equals, true)
+ c.Check(saved.Entries[0].XSnapdNeededBy(), Equals, "test-id")
+ c.Check(saved.Entries[1], DeepEquals, desiredMountEntry)
+}
+
+func (s *updateSuite) TestCurrentProfileFromChangesMade(c *C) {
+ // Changes are computed back-to-front, starting from the last entry in
+ // the mount profile.
+ changes := []*update.Change{
+ {Action: update.Mount, Entry: osutil.MountEntry{Dir: "/dir-1"}},
+ {Action: update.Mount, Entry: osutil.MountEntry{Dir: "/dir-2"}},
+ {Action: update.Mount, Entry: osutil.MountEntry{Dir: "/dir-3"}},
+ }
+ profile := update.CurrentProfileFromChangesMade(changes)
+ c.Check(profile, DeepEquals, osutil.MountProfile{Entries: []osutil.MountEntry{
+ {Dir: "/dir-1"},
+ {Dir: "/dir-2"},
+ {Dir: "/dir-3"},
+ }})
+
+ // When we keep a number of entries we are not still processing them
+ // back-to-front but the order of actual changes is front-to-back.
+ changes = []*update.Change{
+ {Action: update.Keep, Entry: osutil.MountEntry{Dir: "/dir-3"}},
+ {Action: update.Keep, Entry: osutil.MountEntry{Dir: "/dir-2"}},
+ {Action: update.Keep, Entry: osutil.MountEntry{Dir: "/dir-1"}},
+ }
+ profile = update.CurrentProfileFromChangesMade(changes)
+ c.Check(profile, DeepEquals, osutil.MountProfile{Entries: []osutil.MountEntry{
+ {Dir: "/dir-1"},
+ {Dir: "/dir-2"},
+ {Dir: "/dir-3"},
+ }})
+
+ // When we unmount things they just don't appear in the resulting profile.
+ changes = []*update.Change{
+ {Action: update.Unmount, Entry: osutil.MountEntry{Dir: "/dir-1"}},
+ {Action: update.Unmount, Entry: osutil.MountEntry{Dir: "/dir-2"}},
+ {Action: update.Unmount, Entry: osutil.MountEntry{Dir: "/dir-3"}},
+ }
+ profile = update.CurrentProfileFromChangesMade(changes)
+ c.Check(profile, DeepEquals, osutil.MountProfile{})
+
+ // When we have a mixture of changes unmount entries are removed, keep
+ // entries are retained first, back to front and then mount entries are
+ // retained in the order in which they were executed.
+ changes = []*update.Change{
+ {Action: update.Unmount, Entry: osutil.MountEntry{Dir: "/old-2"}},
+ {Action: update.Unmount, Entry: osutil.MountEntry{Dir: "/old-1"}},
+ {Action: update.Keep, Entry: osutil.MountEntry{Dir: "/same-2"}},
+ {Action: update.Keep, Entry: osutil.MountEntry{Dir: "/same-1"}},
+ {Action: update.Mount, Entry: osutil.MountEntry{Dir: "/new-1"}},
+ {Action: update.Mount, Entry: osutil.MountEntry{Dir: "/new-2"}},
+ }
+ profile = update.CurrentProfileFromChangesMade(changes)
+ c.Check(profile, DeepEquals, osutil.MountProfile{Entries: []osutil.MountEntry{
+ {Dir: "/same-1"},
+ {Dir: "/same-2"},
+ {Dir: "/new-1"},
+ {Dir: "/new-2"},
+ }})
}
// testProfileUpdateContext implements MountProfileUpdateContext and is suitable for testing.
diff --git a/osutil/mountentry.go b/osutil/mountentry.go
index 39bb858cb49..b7ce7e8be34 100644
--- a/osutil/mountentry.go
+++ b/osutil/mountentry.go
@@ -40,9 +40,9 @@ import (
// int mnt_passno; /* pass number on parallel fsck */
// };
type MountEntry struct {
- Name string
- Dir string
- Type string
+ Name string // Device name, bind source, pseudo name
+ Dir string // Directory name, bind target (including file)
+ Type string // File system type
Options []string
DumpFrequency int