From 052d3496e249aa0a6ffa40aa1ab125cdfd23538a Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Mon, 4 Apr 2022 13:17:50 +0200 Subject: [PATCH] snap-exec: fix detection if `cups` interface is connected * snap-exec: fix detection if `cups` interface is connected The cups interface needs a way to set the environment variable `CUPS_SERVER` if the `cups` interface is connected. However we have no mechanism for setting environment vars based on interface connections currently. To workaround this, the cups work added a workaround in `snap-exec` that tried to detect if `cups` is connected and when it is set the environment. Unfortunately the code in there was too simplistic because it just checked if the directory `/var/cups` exists. However ths dir now always exists because it's needed as the mount point. This commit fixes the detection by checking if `/var/cups` is a bind mount. This is checked by looking at the `stat()` data and the `dev_t` field in there. If they differ it means the bind mount exists and the only thing that creates this bind mount is the `cups` `MountConnectedPlug()` code. This is not great but it fixes the spread failure we see in the `cups-control` test (which is a real bug) and should be good enough until we have a proper interface backend that can set environment variables. * tests: re-enable interfaces-cups-control test For unclear reasons the error message on unplug changes when I run this on my 20.04 system so I updated the tests - it most likely because the test-snapd-cups-control-consumer snap moved from core16 to core20 a week ago but that is not 100% confirmed. * snap-exec: fix unit test for CUPS_SERVER workaround --- cmd/snap-exec/export_test.go | 12 ++++++++++++ cmd/snap-exec/main.go | 10 +++++++--- cmd/snap-exec/main_test.go | 19 ++++++++++++++----- tests/main/interfaces-cups-control/task.yaml | 2 +- 4 files changed, 34 insertions(+), 9 deletions(-) diff --git a/cmd/snap-exec/export_test.go b/cmd/snap-exec/export_test.go index 8dc27cd30e..b187587c9d 100644 --- a/cmd/snap-exec/export_test.go +++ b/cmd/snap-exec/export_test.go @@ -19,6 +19,12 @@ package main +import ( + "syscall" + + "github.com/snapcore/snapd/testutil" +) + var ( ExpandEnvCmdArgs = expandEnvCmdArgs FindCommand = findCommand @@ -58,3 +64,9 @@ func MockOsReadlink(f func(string) (string, error)) func() { osReadlink = realOsReadlink } } + +func MockSyscallStat(f func(string, *syscall.Stat_t) (err error)) func() { + r := testutil.Backup(&syscallStat) + syscallStat = f + return r +} diff --git a/cmd/snap-exec/main.go b/cmd/snap-exec/main.go index adedfba13a..75e37a5b25 100644 --- a/cmd/snap-exec/main.go +++ b/cmd/snap-exec/main.go @@ -36,6 +36,7 @@ import ( // for the tests var syscallExec = syscall.Exec +var syscallStat = syscall.Stat var osReadlink = os.Readlink // commandline args @@ -199,10 +200,13 @@ func execApp(snapApp, revision, command string, args []string) error { // variables to plugging snap apps, but this is a lot simpler as a // work-around // we currently only handle the CUPS_SERVER environment variable, setting it - // to /var/cups/ if that directory exists - it should not exist anywhere + // to /var/cups/ if that dir is a bind-mount - it should not be one // except in a strictly confined snap where we setup the bind mount from the - // source cups slot snap to the plugging snap - if exists, _, _ := osutil.DirExists(dirs.GlobalRootDir + "/var/cups"); exists { + // source cups slot snap to the plugging snap. + var stVar, stVarCups syscall.Stat_t + err1 := syscallStat(dirs.GlobalRootDir+"/var/", &stVar) + err2 := syscallStat(dirs.GlobalRootDir+"/var/cups/", &stVarCups) + if err1 == nil && err2 == nil && stVar.Dev != stVarCups.Dev { env["CUPS_SERVER"] = "/var/cups/cups.sock" } diff --git a/cmd/snap-exec/main_test.go b/cmd/snap-exec/main_test.go index 40055cb977..16c2659a71 100644 --- a/cmd/snap-exec/main_test.go +++ b/cmd/snap-exec/main_test.go @@ -25,6 +25,8 @@ import ( "os" "os/exec" "path/filepath" + "strings" + "syscall" "testing" . "gopkg.in/check.v1" @@ -193,14 +195,21 @@ func (s *snapExecSuite) TestSnapExecAppIntegrationCupsServerWorkaround(c *C) { Revision: snap.R("42"), }) - // mock the /var/cups dir so that we observe CUPS_SERVER set - err := os.MkdirAll(filepath.Join(dir, "/var/cups"), 0755) - c.Assert(err, IsNil) + // mock the /var/cups dir is a bind-mount + restore := snapExec.MockSyscallStat(func(p string, st *syscall.Stat_t) error { + if strings.HasSuffix(p, "/var/cups/") { + st.Dev = 2 + } else { + st.Dev = 1 + } + return nil + }) + defer restore() execArgv0 := "" execArgs := []string{} execEnv := []string{} - restore := snapExec.MockSyscallExec(func(argv0 string, argv []string, env []string) error { + restore = snapExec.MockSyscallExec(func(argv0 string, argv []string, env []string) error { execArgv0 = argv0 execArgs = argv execEnv = env @@ -209,7 +218,7 @@ func (s *snapExecSuite) TestSnapExecAppIntegrationCupsServerWorkaround(c *C) { defer restore() // launch and verify its run the right way - err = snapExec.ExecApp("snapname.app", "42", "stop", []string{"arg1", "arg2"}) + err := snapExec.ExecApp("snapname.app", "42", "stop", []string{"arg1", "arg2"}) c.Assert(err, IsNil) c.Check(execArgv0, Equals, fmt.Sprintf("%s/snapname/42/stop-app", dirs.SnapMountDir)) c.Check(execArgs, DeepEquals, []string{execArgv0, "arg1", "arg2"}) diff --git a/tests/main/interfaces-cups-control/task.yaml b/tests/main/interfaces-cups-control/task.yaml index 174ed5f769..e4ef5a2550 100644 --- a/tests/main/interfaces-cups-control/task.yaml +++ b/tests/main/interfaces-cups-control/task.yaml @@ -72,4 +72,4 @@ execute: | echo "Expected error with plug disconnected" exit 1 fi - MATCH "scheduler not responding" < print.error + MATCH "(scheduler not responding|No default destination)" < print.error