/snapd

many: fix test cases to work with different DistroLibExecDir #3222

Merged
merged 23 commits into from Jun 19, 2017
Commits
Jump to file or symbol
Failed to load files and symbols.
+253 −184
Split
View
2 cmd/cmd.go
@@ -76,7 +76,7 @@ func distroSupportsReExec() bool {
// Ensure we do not use older version of snapd, look for info file and ignore
// version of core that do not yet have it.
func coreSupportsReExec(corePath string) bool {
- fullInfo := filepath.Join(corePath, "/usr/lib/snapd/info")
+ fullInfo := filepath.Join(corePath, filepath.Join(dirs.CoreLibExecDir, "info"))
if !osutil.FileExists(fullInfo) {
return false
}
View
18 cmd/snap/cmd_run.go
@@ -365,6 +365,15 @@ func migrateXauthority(info *snap.Info) (string, error) {
func runSnapConfine(info *snap.Info, securityTag, snapApp, command, hook string, args []string) error {
snapConfine := filepath.Join(dirs.DistroLibExecDir, "snap-confine")
+ // if we re-exec, we must run the snap-confine from the core snap
+ // as well, if they get out of sync, havoc will happen
+ if isReexeced() {
+ // run snap-confine from the core snap. that will work because
+ // snap-confine on the core snap is mostly statically linked
+ // (except libudev and libc)
+ snapConfine = filepath.Join(dirs.SnapMountDir, "core/current", dirs.CoreLibExecDir, "snap-confine")
+ }
+
if !osutil.FileExists(snapConfine) {
if hook != "" {
logger.Noticef("WARNING: skipping running hook %q of snap %q: missing snap-confine", hook, info.Name())
@@ -401,15 +410,6 @@ func runSnapConfine(info *snap.Info, securityTag, snapApp, command, hook string,
cmd = append(cmd, snapApp)
cmd = append(cmd, args...)
- // if we re-exec, we must run the snap-confine from the core snap
- // as well, if they get out of sync, havoc will happen
- if isReexeced() {
- // run snap-confine from the core snap. that will work because
- // snap-confine on the core snap is mostly statically linked
- // (except libudev and libc)
- cmd[0] = filepath.Join(dirs.SnapMountDir, "core/current", cmd[0])
- }
-
extraEnv := make(map[string]string)
if len(xauthPath) > 0 {
extraEnv["XAUTHORITY"] = xauthPath
View
46 cmd/snap/cmd_run_test.go
@@ -98,7 +98,7 @@ func (s *SnapSuite) TestSnapRunAppIntegration(c *check.C) {
// mock installed snap
dirs.SetRootDir(c.MkDir())
defer func() { dirs.SetRootDir("/") }()
- defer mockSnapConfine()()
+ defer mockSnapConfine(dirs.DistroLibExecDir)()
@mvo5

mvo5 Jun 7, 2017

Collaborator

Pardon my ignorance, but it looks like the argument for mockSnapConfine() is always dirs.DistroLibExecDir - do we really need this new argument in this case?

@morphis

morphis Jun 7, 2017

Contributor

There is exactly one case where it isn't. See https://github.com/snapcore/snapd/pull/3222/files/f78d7918f26f71239000599fd11db988cd605cc0#diff-78f744cbc3eb83652f8ed773dceac016R466

@mvo5

mvo5 Jun 7, 2017

Collaborator

Aha, silly me. I even found this in subsequent reading but then forgot to delete the comment. Sorry for the noise.
si := snaptest.MockSnap(c, string(mockYaml), string(mockContents), &snap.SideInfo{
Revision: snap.R("x2"),
@@ -126,7 +126,7 @@ func (s *SnapSuite) TestSnapRunAppIntegration(c *check.C) {
c.Check(execArgs, check.DeepEquals, []string{
filepath.Join(dirs.DistroLibExecDir, "snap-confine"),
"snap.snapname.app",
- filepath.Join(dirs.DistroLibExecDir, "snap-exec"),
+ filepath.Join(dirs.CoreLibExecDir, "snap-exec"),
"snapname.app", "--arg1", "arg2"})
c.Check(execEnv, testutil.Contains, "SNAP_REVISION=x2")
}
@@ -135,7 +135,7 @@ func (s *SnapSuite) TestSnapRunClassicAppIntegration(c *check.C) {
// mock installed snap
dirs.SetRootDir(c.MkDir())
defer func() { dirs.SetRootDir("/") }()
- defer mockSnapConfine()()
+ defer mockSnapConfine(dirs.DistroLibExecDir)()
si := snaptest.MockSnap(c, string(mockYaml)+"confinement: classic\n", string(mockContents), &snap.SideInfo{
Revision: snap.R("x2"),
@@ -163,7 +163,7 @@ func (s *SnapSuite) TestSnapRunClassicAppIntegration(c *check.C) {
c.Check(execArgs, check.DeepEquals, []string{
filepath.Join(dirs.DistroLibExecDir, "snap-confine"), "--classic",
"snap.snapname.app",
- filepath.Join(dirs.DistroLibExecDir, "snap-exec"),
+ filepath.Join(dirs.CoreLibExecDir, "snap-exec"),
"snapname.app", "--arg1", "arg2"})
c.Check(execEnv, testutil.Contains, "SNAP_REVISION=x2")
}
@@ -172,7 +172,7 @@ func (s *SnapSuite) TestSnapRunAppWithCommandIntegration(c *check.C) {
// mock installed snap
dirs.SetRootDir(c.MkDir())
defer func() { dirs.SetRootDir("/") }()
- defer mockSnapConfine()()
+ defer mockSnapConfine(dirs.DistroLibExecDir)()
si := snaptest.MockSnap(c, string(mockYaml), string(mockContents), &snap.SideInfo{
Revision: snap.R(42),
@@ -199,7 +199,7 @@ func (s *SnapSuite) TestSnapRunAppWithCommandIntegration(c *check.C) {
c.Check(execArgs, check.DeepEquals, []string{
filepath.Join(dirs.DistroLibExecDir, "snap-confine"),
"snap.snapname.app",
- filepath.Join(dirs.DistroLibExecDir, "snap-exec"),
+ filepath.Join(dirs.CoreLibExecDir, "snap-exec"),
"--command=my-command", "snapname.app", "arg1", "arg2"})
c.Check(execEnv, testutil.Contains, "SNAP_REVISION=42")
}
@@ -225,7 +225,7 @@ func (s *SnapSuite) TestSnapRunHookIntegration(c *check.C) {
// mock installed snap
dirs.SetRootDir(c.MkDir())
defer func() { dirs.SetRootDir("/") }()
- defer mockSnapConfine()()
+ defer mockSnapConfine(dirs.DistroLibExecDir)()
si := snaptest.MockSnap(c, string(mockYaml), string(mockContents), &snap.SideInfo{
Revision: snap.R(42),
@@ -252,7 +252,7 @@ func (s *SnapSuite) TestSnapRunHookIntegration(c *check.C) {
c.Check(execArgs, check.DeepEquals, []string{
filepath.Join(dirs.DistroLibExecDir, "snap-confine"),
"snap.snapname.hook.configure",
- filepath.Join(dirs.DistroLibExecDir, "snap-exec"),
+ filepath.Join(dirs.CoreLibExecDir, "snap-exec"),
"--hook=configure", "snapname"})
c.Check(execEnv, testutil.Contains, "SNAP_REVISION=42")
}
@@ -261,7 +261,7 @@ func (s *SnapSuite) TestSnapRunHookUnsetRevisionIntegration(c *check.C) {
// mock installed snap
dirs.SetRootDir(c.MkDir())
defer func() { dirs.SetRootDir("/") }()
- defer mockSnapConfine()()
+ defer mockSnapConfine(dirs.DistroLibExecDir)()
si := snaptest.MockSnap(c, string(mockYaml), string(mockContents), &snap.SideInfo{
Revision: snap.R(42),
@@ -288,7 +288,7 @@ func (s *SnapSuite) TestSnapRunHookUnsetRevisionIntegration(c *check.C) {
c.Check(execArgs, check.DeepEquals, []string{
filepath.Join(dirs.DistroLibExecDir, "snap-confine"),
"snap.snapname.hook.configure",
- filepath.Join(dirs.DistroLibExecDir, "snap-exec"),
+ filepath.Join(dirs.CoreLibExecDir, "snap-exec"),
"--hook=configure", "snapname"})
c.Check(execEnv, testutil.Contains, "SNAP_REVISION=42")
}
@@ -297,7 +297,7 @@ func (s *SnapSuite) TestSnapRunHookSpecificRevisionIntegration(c *check.C) {
// mock installed snap
dirs.SetRootDir(c.MkDir())
defer func() { dirs.SetRootDir("/") }()
- defer mockSnapConfine()()
+ defer mockSnapConfine(dirs.DistroLibExecDir)()
// Create both revisions 41 and 42
snaptest.MockSnap(c, string(mockYaml), string(mockContents), &snap.SideInfo{
@@ -326,7 +326,7 @@ func (s *SnapSuite) TestSnapRunHookSpecificRevisionIntegration(c *check.C) {
c.Check(execArgs, check.DeepEquals, []string{
filepath.Join(dirs.DistroLibExecDir, "snap-confine"),
"snap.snapname.hook.configure",
- filepath.Join(dirs.DistroLibExecDir, "snap-exec"),
+ filepath.Join(dirs.CoreLibExecDir, "snap-exec"),
"--hook=configure", "snapname"})
c.Check(execEnv, testutil.Contains, "SNAP_REVISION=41")
}
@@ -398,14 +398,14 @@ func (s *SnapSuite) TestSnapRunErorsForMissingApp(c *check.C) {
func (s *SnapSuite) TestSnapRunErorrForUnavailableApp(c *check.C) {
_, err := snaprun.Parser().ParseArgs([]string{"run", "not-there"})
- c.Assert(err, check.ErrorMatches, "cannot find current revision for snap not-there: readlink /snap/not-there/current: no such file or directory")
+ c.Assert(err, check.ErrorMatches, fmt.Sprintf("cannot find current revision for snap not-there: readlink %s/not-there/current: no such file or directory", dirs.SnapMountDir))
}
func (s *SnapSuite) TestSnapRunSaneEnvironmentHandling(c *check.C) {
// mock installed snap
dirs.SetRootDir(c.MkDir())
defer func() { dirs.SetRootDir("/") }()
- defer mockSnapConfine()()
+ defer mockSnapConfine(dirs.DistroLibExecDir)()
si := snaptest.MockSnap(c, string(mockYaml), string(mockContents), &snap.SideInfo{
Revision: snap.R(42),
@@ -451,8 +451,8 @@ func (s *SnapSuite) TestSnapRunIsReexeced(c *check.C) {
readlink string
expected bool
}{
- {filepath.Join(dirs.SnapMountDir, "/usr/lib/snapd/snapd"), true},
- {"/usr/lib/snapd/snapd", false},
+ {filepath.Join(dirs.SnapMountDir, dirs.CoreLibExecDir, "snapd"), true},
+ {filepath.Join(dirs.DistroLibExecDir, "snapd"), false},
} {
osReadlinkResult = t.readlink
c.Check(snaprun.IsReexeced(), check.Equals, t.expected)
@@ -463,7 +463,7 @@ func (s *SnapSuite) TestSnapRunAppIntegrationFromCore(c *check.C) {
// mock installed snap
dirs.SetRootDir(c.MkDir())
defer func() { dirs.SetRootDir("/") }()
- defer mockSnapConfine()()
+ defer mockSnapConfine(filepath.Join(dirs.SnapMountDir, "core", "current", dirs.CoreLibExecDir))()
si := snaptest.MockSnap(c, string(mockYaml), string(mockContents), &snap.SideInfo{
Revision: snap.R("x2"),
@@ -473,7 +473,7 @@ func (s *SnapSuite) TestSnapRunAppIntegrationFromCore(c *check.C) {
// pretend to be running from core
restorer := snaprun.MockOsReadlink(func(string) (string, error) {
- return filepath.Join(dirs.SnapMountDir, "/core/111//usr/bin/snap"), nil
+ return filepath.Join(dirs.SnapMountDir, "core/111/usr/bin/snap"), nil
})
defer restorer()
@@ -493,11 +493,11 @@ func (s *SnapSuite) TestSnapRunAppIntegrationFromCore(c *check.C) {
rest, err := snaprun.Parser().ParseArgs([]string{"run", "snapname.app", "--arg1", "arg2"})
c.Assert(err, check.IsNil)
c.Assert(rest, check.DeepEquals, []string{"snapname.app", "--arg1", "arg2"})
- c.Check(execArg0, check.Equals, filepath.Join(dirs.SnapMountDir, "/core/current", dirs.DistroLibExecDir, "snap-confine"))
+ c.Check(execArg0, check.Equals, filepath.Join(dirs.SnapMountDir, "/core/current", dirs.CoreLibExecDir, "snap-confine"))
c.Check(execArgs, check.DeepEquals, []string{
- filepath.Join(dirs.SnapMountDir, "/core/current", dirs.DistroLibExecDir, "snap-confine"),
+ filepath.Join(dirs.SnapMountDir, "/core/current", dirs.CoreLibExecDir, "snap-confine"),
"snap.snapname.app",
- filepath.Join(dirs.DistroLibExecDir, "snap-exec"),
+ filepath.Join(dirs.CoreLibExecDir, "snap-exec"),
"snapname.app", "--arg1", "arg2"})
c.Check(execEnv, testutil.Contains, "SNAP_REVISION=x2")
}
@@ -507,7 +507,7 @@ func (s *SnapSuite) TestSnapRunXauthorityMigration(c *check.C) {
// below /tmp which the Xauthority migration expects.
dirs.SetRootDir(c.MkDir())
defer func() { dirs.SetRootDir("/") }()
- defer mockSnapConfine()()
+ defer mockSnapConfine(dirs.DistroLibExecDir)()
u, err := user.Current()
c.Assert(err, check.IsNil)
@@ -553,7 +553,7 @@ func (s *SnapSuite) TestSnapRunXauthorityMigration(c *check.C) {
c.Check(execArgs, check.DeepEquals, []string{
filepath.Join(dirs.DistroLibExecDir, "snap-confine"),
"snap.snapname.app",
- filepath.Join(dirs.DistroLibExecDir, "snap-exec"),
+ filepath.Join(dirs.CoreLibExecDir, "snap-exec"),
"snapname.app"})
expectedXauthPath := filepath.Join(dirs.XdgRuntimeDirBase, u.Uid, ".Xauthority")
View
6 cmd/snap/main_test.go
@@ -152,9 +152,9 @@ func mockVersion(v string) (restore func()) {
return func() { cmd.Version = old }
}
-func mockSnapConfine() func() {
- snapConfine := filepath.Join(dirs.DistroLibExecDir, "snap-confine")
- if err := os.MkdirAll(dirs.DistroLibExecDir, 0755); err != nil {
+func mockSnapConfine(libExecDir string) func() {
+ snapConfine := filepath.Join(libExecDir, "snap-confine")
+ if err := os.MkdirAll(libExecDir, 0755); err != nil {
panic(err)
}
if err := ioutil.WriteFile(snapConfine, nil, 0644); err != nil {
View
16 dirs/dirs.go
@@ -32,7 +32,10 @@ import (
var (
GlobalRootDir string
- SnapMountDir string
+ SnapMountDir string
+
+ DistroLibExecDir string
+
SnapBlobDir string
SnapDataDir string
SnapDataHomeGlob string
@@ -73,9 +76,6 @@ var (
ClassicDir string
- DistroLibExecDir string
- CoreLibExecDir string
-
XdgRuntimeDirBase string
XdgRuntimeDirGlob string
@@ -84,6 +84,12 @@ var (
const (
defaultSnapMountDir = "/snap"
+
+ // These are directories which are static inside the core snap and
+ // can never be prefixed as they will be always absolute once we
+ // are in the snap confinement environment.
+ CoreLibExecDir = "/usr/lib/snapd"
+ CoreSnapMountDir = "/snap"
)
var (
@@ -183,8 +189,6 @@ func SetRootDir(rootdir string) {
DistroLibExecDir = filepath.Join(rootdir, "/usr/lib/snapd")
}
- CoreLibExecDir = filepath.Join(rootdir, "/usr/lib/snapd")
-
XdgRuntimeDirBase = filepath.Join(rootdir, "/run/user")
XdgRuntimeDirGlob = filepath.Join(rootdir, XdgRuntimeDirBase, "*/")
View
3 dirs/dirs_test.go
@@ -50,6 +50,9 @@ func (s *DirsTestSuite) TestStripRootDir(c *C) {
}
func (s *DirsTestSuite) TestClassicConfinementSupport(c *C) {
+ // Ensure that we have a distribution as base which supports classic confinement
+ reset := release.MockReleaseInfo(&release.OS{ID: "ubuntu"})
+ defer reset()
dirs.SetRootDir("/")
c.Assert(dirs.SupportsClassicConfinement(), Equals, true)
dirs.SetRootDir("/alt")
View
9 interfaces/builtin/content.go
@@ -25,6 +25,7 @@ import (
"path/filepath"
"strings"
+ "github.com/snapcore/snapd/dirs"
"github.com/snapcore/snapd/interfaces"
"github.com/snapcore/snapd/interfaces/apparmor"
"github.com/snapcore/snapd/interfaces/mount"
@@ -130,7 +131,11 @@ func (iface *contentInterface) path(slot *interfaces.Slot, name string) []string
// (this is the behavior that was used before the variables were supporter).
func resolveSpecialVariable(path string, snapInfo *snap.Info) string {
if strings.HasPrefix(path, "$SNAP/") || path == "$SNAP" {
- return strings.Replace(path, "$SNAP", snapInfo.MountDir(), 1)
+ // NOTE: We use dirs.CoreSnapMountDir here as the path used will be always
+ // inside the mount namespace snap-confine creates and there we will
+ // always have a /snap directory available regardless if the system
+ // we're running on supports this or not.
+ return strings.Replace(path, "$SNAP", filepath.Join(dirs.CoreSnapMountDir, snapInfo.Name(), snapInfo.Revision.String()), 1)
}
if strings.HasPrefix(path, "$SNAP_DATA/") || path == "$SNAP_DATA" {
return strings.Replace(path, "$SNAP_DATA", snapInfo.DataDir(), 1)
@@ -139,7 +144,7 @@ func resolveSpecialVariable(path string, snapInfo *snap.Info) string {
return strings.Replace(path, "$SNAP_COMMON", snapInfo.CommonDataDir(), 1)
}
// NOTE: assume $SNAP by default if nothing else is provided, for compatibility
- return filepath.Join(snapInfo.MountDir(), path)
+ return filepath.Join(filepath.Join(dirs.CoreSnapMountDir, snapInfo.Name(), snapInfo.Revision.String()), path)
}
func mountEntry(plug *interfaces.Plug, slot *interfaces.Slot, relSrc string, extraOptions ...string) mount.Entry {
View
24 interfaces/builtin/content_test.go
@@ -20,8 +20,12 @@
package builtin_test
import (
+ "fmt"
+ "path/filepath"
+
. "gopkg.in/check.v1"
+ "github.com/snapcore/snapd/dirs"
"github.com/snapcore/snapd/interfaces"
"github.com/snapcore/snapd/interfaces/apparmor"
"github.com/snapcore/snapd/interfaces/builtin"
@@ -196,11 +200,11 @@ apps:
func (s *ContentSuite) TestResolveSpecialVariable(c *C) {
info := snaptest.MockInfo(c, "name: name", &snap.SideInfo{Revision: snap.R(42)})
- c.Check(builtin.ResolveSpecialVariable("foo", info), Equals, "/snap/name/42/foo")
@zyga

zyga May 5, 2017

Contributor

Sorry for taking so long to verify this. This part is incorrect. The generated mount profile is only processed after pivot_root has happened so it must use the internal layout.

@morphis

morphis May 5, 2017 •
Edited 1 time

Contributor

How that? On Debian & Ubuntu we have /snap and on Fedora we have /var/lib/snapd/snap in and outside of the snap or do I miss anything?

@zyga

zyga May 5, 2017

Contributor

You are missing one thing. After pivot_root whatever the snap mount directory is it ends up being /snap. Only then do we process those mount directives so the hard-coded paths are actually always correct.

@morphis

morphis May 5, 2017

Contributor

Running the following

$ snap run --shell hello-world
bash-4.3$ echo $SNAP
/var/lib/snapd/snap/hello-world/27

Please also note that I didn't change builting.ResolveSpecialVariable. It is using dirs.BaseSnapMountDir already without this PR. See https://github.com/snapcore/snapd/blob/master/interfaces/builtin/content.go#L123

@zyga

zyga May 5, 2017

Contributor

As discussed on IRC there are a host of bugs here:

  • snap execution environment should look the same everywhere
  • $SNAP is always /snap/$SNAP_NAME/$SNAP_REVISION when seen from inside snap execution environment
  • various other things are wrong but we'll discuss and fix them in separate PRs
@morphis

morphis May 8, 2017

Contributor

The code is now changed to use /snap as static prefix.

@mvo5

mvo5 Jun 7, 2017

Collaborator

Silly question - if the code got changed to use /snap as static prefix, do we still need to change:

- "/snap/foo"
+ filepath.Join(dirs.CoreSnapMountDir, "foo")

I guess the answer is yes to make it more obvious when we mean "context-is-inside-core" and "context-is-outside"(?)

@morphis

morphis Jun 7, 2017

Contributor

I would say, we should do it this way, With that we always have a clear bind to the context and know what we're dealing with.
- c.Check(builtin.ResolveSpecialVariable("$SNAP/foo", info), Equals, "/snap/name/42/foo")
+ c.Check(builtin.ResolveSpecialVariable("foo", info), Equals, filepath.Join(dirs.CoreSnapMountDir, "name/42/foo"))
+ c.Check(builtin.ResolveSpecialVariable("$SNAP/foo", info), Equals, filepath.Join(dirs.CoreSnapMountDir, "name/42/foo"))
c.Check(builtin.ResolveSpecialVariable("$SNAP_DATA/foo", info), Equals, "/var/snap/name/42/foo")
c.Check(builtin.ResolveSpecialVariable("$SNAP_COMMON/foo", info), Equals, "/var/snap/name/common/foo")
- c.Check(builtin.ResolveSpecialVariable("$SNAP", info), Equals, "/snap/name/42")
+ c.Check(builtin.ResolveSpecialVariable("$SNAP", info), Equals, filepath.Join(dirs.CoreSnapMountDir, "name/42"))
c.Check(builtin.ResolveSpecialVariable("$SNAP_DATA", info), Equals, "/var/snap/name/42")
c.Check(builtin.ResolveSpecialVariable("$SNAP_COMMON", info), Equals, "/var/snap/name/common")
}
@@ -226,8 +230,8 @@ slots:
spec := &mount.Specification{}
c.Assert(spec.AddConnectedPlug(s.iface, plug, nil, slot, nil), IsNil)
expectedMnt := []mount.Entry{{
- Name: "/snap/producer/5/export",
- Dir: "/snap/consumer/7/import",
+ Name: filepath.Join(dirs.CoreSnapMountDir, "producer/5/export"),
+ Dir: filepath.Join(dirs.CoreSnapMountDir, "consumer/7/import"),
Options: []string{"bind", "ro"},
}}
c.Assert(spec.MountEntries(), DeepEquals, expectedMnt)
@@ -257,8 +261,8 @@ slots:
spec := &mount.Specification{}
c.Assert(spec.AddConnectedPlug(s.iface, plug, nil, slot, nil), IsNil)
expectedMnt := []mount.Entry{{
- Name: "/snap/producer/5/export",
- Dir: "/snap/consumer/7/import",
+ Name: filepath.Join(dirs.CoreSnapMountDir, "producer/5/export"),
+ Dir: filepath.Join(dirs.CoreSnapMountDir, "consumer/7/import"),
Options: []string{"bind", "ro"},
}}
c.Assert(spec.MountEntries(), DeepEquals, expectedMnt)
@@ -267,12 +271,12 @@ slots:
err := apparmorSpec.AddConnectedPlug(s.iface, plug, nil, slot, nil)
c.Assert(err, IsNil)
c.Assert(apparmorSpec.SecurityTags(), DeepEquals, []string{"snap.consumer.app"})
- expected := `
+ expected := fmt.Sprintf(`
# In addition to the bind mount, add any AppArmor rules so that
# snaps may directly access the slot implementation's files
# read-only.
-/snap/producer/5/export/** mrkix,
-`
+%s/producer/5/export/** mrkix,
+`, dirs.CoreSnapMountDir)
c.Assert(apparmorSpec.SnippetForTag("snap.consumer.app"), Equals, expected)
}
Oops, something went wrong.