Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cmd/snap/cmd_run.go
Original file line number Diff line number Diff line change
Expand Up @@ -1679,7 +1679,7 @@ func (x *cmdRun) runSnapConfine(info *snap.Info, runner runnable, beforeExec fun
return err
}

snapenv.ExtendEnvForRun(env, info, runner.Component(), opts)
snapenv.ExtendEnvForRun(env, info, runner.App(), runner.Component(), opts)

if len(xauthPath) > 0 {
// Environment is not nil here because it comes from
Expand Down
32 changes: 29 additions & 3 deletions snap/snapenv/snapenv.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,19 +49,25 @@ var userCurrent = user.Current
//
// It ensures all SNAP_* override any pre-existing environment
// variables.
func ExtendEnvForRun(env osutil.Environment, info *snap.Info, component *snap.ComponentInfo, opts *dirs.SnapDirOptions) {
func ExtendEnvForRun(env osutil.Environment, info *snap.Info, app *snap.AppInfo, component *snap.ComponentInfo, opts *dirs.SnapDirOptions) {
// Set various SNAP_ environment variables as well as some non-SNAP variables,
// depending on snap confinement mode. Note that this does not include environment
// set by snap-exec.
for k, v := range snapEnv(info, component, opts) {
for k, v := range snapEnv(info, app, component, opts) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does it make sense to have similar information for hooks?

type HookInfo struct {
	Snap *Info

	// Component will be nil if the hook is not a component hook.
	Component *Component

	Name  string
	Plugs map[string]*PlugInfo
	Slots map[string]*SlotInfo

	Environment  strutil.OrderedMap
	CommandChain []string

	Explicit bool
}

looks like we could have SNAP_HOOK=<hook-name>

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, look like they could be useful too, although if an hook is running I assume it know that it knows what its own name?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in theory the same applies to apps 😄

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well... Not much, in the sense that while hooks are designed to be inside a snap, an application can be embedded into anyof snap

env[k] = v
}
}

func snapEnv(info *snap.Info, component *snap.ComponentInfo, opts *dirs.SnapDirOptions) osutil.Environment {
func snapEnv(info *snap.Info, app *snap.AppInfo, component *snap.ComponentInfo, opts *dirs.SnapDirOptions) osutil.Environment {
// Environment variables with basic properties of a snap.
env := basicEnv(info)

if app != nil {
for k, v := range appEnv(info, app) {
env[k] = v
}
}

if component != nil {
for k, v := range componentEnv(info, component) {
env[k] = v
Expand Down Expand Up @@ -137,6 +143,26 @@ func basicEnv(info *snap.Info) osutil.Environment {
logger.Noticef("cannot determine existence of save data directory for snap %q: %v",
info.InstanceName(), err)
}

return env
}

// appEnv returns the app-level environment variables for a snap.
func appEnv(info *snap.Info, app *snap.AppInfo) osutil.Environment {
env := osutil.Environment{
"SNAP_APP_NAME": app.Name,
Comment thread
zyga marked this conversation as resolved.
}

if app.CommonID != "" {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is all very welcome. We must document it somewhere.

env["SNAP_APP_COMMON_ID"] = app.CommonID
}
if df := app.DesktopFile(); df != "" && osutil.FileExists(df) {
env["SNAP_APP_DESKTOP_FILE"] = df
}
if app.BusName != "" {
env["SNAP_APP_BUS_NAME"] = app.BusName
}

return env
}

Expand Down
98 changes: 91 additions & 7 deletions snap/snapenv/snapenv_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,35 @@ var mockSnapInfo = &snap.Info{
Revision: snap.R(17),
},
}
var mockSnapInfoWithDesktopFile = func() *snap.Info {
mi := *mockSnapInfo
mi.Plugs = map[string]*snap.PlugInfo{
"desktop": {
Snap: &mi,
Interface: "desktop",
Name: "desktop",
Attrs: map[string]any{
"desktop-file-ids": []any{"io.snapcraft.foo.bar.desktop"},
},
},
}
return &mi
}()
var mockAppInfo = &snap.AppInfo{
Snap: mockSnapInfo,
Name: "bar",
CommonID: "io.snapcraft.foo.bar",
BusName: "io.snapcraft.foo.bar.bus",
}
var mockAppInfoWithDesktopFile = func() *snap.AppInfo {
mai := *mockAppInfo
mai.Snap = mockSnapInfoWithDesktopFile
return &mai
}()
var mockAppInfoMinimal = &snap.AppInfo{
Snap: mockSnapInfo,
Name: "bar",
}
var mockComponentInfo = &snap.ComponentInfo{
Component: naming.ComponentRef{
SnapName: "foo",
Expand Down Expand Up @@ -94,6 +123,10 @@ var mockClassicSnapInfo = &snap.Info{
func (s *HTestSuite) SetUpTest(c *C) {
s.BaseTest.SetUpTest(c)
s.BaseTest.AddCleanup(snap.MockSanitizePlugsSlots(func(snapInfo *snap.Info) {}))

defaultDesktopFilesDir := dirs.SnapDesktopFilesDir
dirs.SnapDesktopFilesDir = c.MkDir()
s.BaseTest.AddCleanup(func() { dirs.SnapDesktopFilesDir = defaultDesktopFilesDir })
}

func (s *HTestSuite) TearDownTest(c *C) {
Expand Down Expand Up @@ -160,6 +193,47 @@ func (ts *HTestSuite) TestBasicWithSources(c *C) {
})
}

func (ts *HTestSuite) TestAppEnvironment(c *C) {
env := appEnv(mockSnapInfo, mockAppInfo)
c.Assert(env, DeepEquals, osutil.Environment{
"SNAP_APP_NAME": "bar",
"SNAP_APP_COMMON_ID": "io.snapcraft.foo.bar",
"SNAP_APP_BUS_NAME": "io.snapcraft.foo.bar.bus",
Comment thread
3v1n0 marked this conversation as resolved.
})
}

func (ts *HTestSuite) testAppDesktopFileEnvironment(c *C, appInfo *snap.AppInfo, desktopFileName string) {
desktopFilePath := filepath.Join(dirs.SnapDesktopFilesDir, desktopFileName)
desktopFile := fmt.Sprintf(`[Desktop Entry]
Exec=%s
X-SnapInstanceName=%s
X-SnapAppName=%s
`, appInfo.Command, appInfo.Snap.InstanceName(), appInfo.Name)
c.Assert(os.WriteFile(desktopFilePath, []byte(desktopFile), 0644), IsNil)

c.Assert(appInfo.DesktopFile(), Equals, desktopFilePath)

env := appEnv(appInfo.Snap, appInfo)
c.Assert(env, DeepEquals, osutil.Environment{
"SNAP_APP_NAME": appInfo.Name,
"SNAP_APP_COMMON_ID": appInfo.CommonID,
"SNAP_APP_BUS_NAME": appInfo.BusName,
"SNAP_APP_DESKTOP_FILE": appInfo.DesktopFile(),
})
}

func (ts *HTestSuite) TestAppWithFallbackDesktopFileIDEnvironment(c *C) {
ts.testAppDesktopFileEnvironment(c, mockAppInfo, "foo_bar.desktop")
}

func (ts *HTestSuite) TestAppWithDesktopFileIDEnvironment(c *C) {
ts.testAppDesktopFileEnvironment(c, mockAppInfoWithDesktopFile, "io.snapcraft.foo.bar.desktop")
}

func (ts *HTestSuite) TestAppWithDesktopFileIDUsingFallbackDesktopFileEnvironment(c *C) {
ts.testAppDesktopFileEnvironment(c, mockAppInfoWithDesktopFile, "foo_bar.desktop")
}

func (ts *HTestSuite) TestSaveDataEnvironmentNotPresent(c *C) {
dirs.SetRootDir(c.MkDir())
ts.AddCleanup(func() { dirs.SetRootDir("") })
Expand Down Expand Up @@ -236,7 +310,7 @@ func (s *HTestSuite) TestSnapRunSnapExecEnv(c *C) {
os.Setenv("HOME", "")
}

env := snapEnv(info, nil, nil)
env := snapEnv(info, mockAppInfo, nil, nil)
c.Assert(env, DeepEquals, osutil.Environment{
"SNAP": fmt.Sprintf("%s/snapname/42", dirs.CoreSnapMountDir),
"SNAP_COMMON": "/var/snap/snapname/common",
Expand All @@ -256,6 +330,9 @@ func (s *HTestSuite) TestSnapRunSnapExecEnv(c *C) {
"SNAP_REAL_HOME": usr.HomeDir,
"SNAP_UID": fmt.Sprint(sys.Getuid()),
"SNAP_EUID": fmt.Sprint(sys.Geteuid()),
"SNAP_APP_COMMON_ID": "io.snapcraft.foo.bar",
"SNAP_APP_BUS_NAME": "io.snapcraft.foo.bar.bus",
"SNAP_APP_NAME": "bar",
})
}
}
Expand All @@ -279,7 +356,7 @@ func (s *HTestSuite) TestParallelInstallSnapRunSnapExecEnv(c *C) {
os.Setenv("HOME", "")
}

env := snapEnv(info, nil, nil)
env := snapEnv(info, mockAppInfoMinimal, nil, nil)
c.Check(env, DeepEquals, osutil.Environment{
// Those are mapped to snap-specific directories by
// mount namespace setup
Expand All @@ -303,6 +380,7 @@ func (s *HTestSuite) TestParallelInstallSnapRunSnapExecEnv(c *C) {
"SNAP_REAL_HOME": usr.HomeDir,
"SNAP_UID": fmt.Sprint(sys.Getuid()),
"SNAP_EUID": fmt.Sprint(sys.Geteuid()),
"SNAP_APP_NAME": "bar",
})
}
}
Expand Down Expand Up @@ -355,23 +433,29 @@ func (ts *HTestSuite) TestParallelInstallUserForClassicConfinement(c *C) {
func (s *HTestSuite) TestExtendEnvForRunForNonClassic(c *C) {
env := osutil.Environment{"TMPDIR": "/var/tmp"}

ExtendEnvForRun(env, mockSnapInfo, nil, nil)
ExtendEnvForRun(env, mockSnapInfo, mockAppInfo, nil, nil)

c.Assert(env["SNAP_NAME"], Equals, "foo")
c.Assert(env["SNAP_COMMON"], Equals, "/var/snap/foo/common")
c.Assert(env["SNAP_DATA"], Equals, "/var/snap/foo/17")
c.Assert(env["SNAP_APP_NAME"], Equals, "bar")
c.Assert(env["SNAP_APP_COMMON_ID"], Equals, "io.snapcraft.foo.bar")
c.Assert(env["SNAP_APP_BUS_NAME"], Equals, "io.snapcraft.foo.bar.bus")

c.Assert(env["TMPDIR"], Equals, "/var/tmp")
}

func (s *HTestSuite) TestExtendEnvForRunForClassic(c *C) {
env := osutil.Environment{"TMPDIR": "/var/tmp"}

ExtendEnvForRun(env, mockClassicSnapInfo, nil, nil)
ExtendEnvForRun(env, mockClassicSnapInfo, mockAppInfoMinimal, nil, nil)

c.Assert(env["SNAP_NAME"], Equals, "foo")
c.Assert(env["SNAP_COMMON"], Equals, "/var/snap/foo/common")
c.Assert(env["SNAP_DATA"], Equals, "/var/snap/foo/17")
c.Assert(env["SNAP_APP_NAME"], Equals, "bar")
c.Assert(func() bool { _, ok := env["SNAP_APP_COMMON_ID"]; return ok }(), Equals, false)
c.Assert(func() bool { _, ok := env["SNAP_APP_BUS_NAME"]; return ok }(), Equals, false)

c.Assert(env["TMPDIR"], Equals, "/var/tmp")
}
Expand All @@ -392,15 +476,15 @@ func checkEnvWithComp(c *C, env osutil.Environment, compVersion string) {
func (s *HTestSuite) TestExtendEnvForRunWithComponent(c *C) {
env := osutil.Environment{"TMPDIR": "/var/tmp"}

ExtendEnvForRun(env, mockSnapInfo, mockComponentInfo, nil)
ExtendEnvForRun(env, mockSnapInfo, nil, mockComponentInfo, nil)
compVersion := "1.1"
checkEnvWithComp(c, env, compVersion)
}

func (s *HTestSuite) TestExtendEnvForRunWithComponentNoVersion(c *C) {
env := osutil.Environment{"TMPDIR": "/var/tmp"}

ExtendEnvForRun(env, mockSnapInfo, mockComponentInfoNoVersion, nil)
ExtendEnvForRun(env, mockSnapInfo, nil, mockComponentInfoNoVersion, nil)
// Same as snap in this case
compVersion := "1.0"
checkEnvWithComp(c, env, compVersion)
Expand All @@ -425,7 +509,7 @@ func (s *HTestSuite) TestHiddenDirEnv(c *C) {
{dir: dirs.HiddenSnapDataHomeDir, opts: &dirs.SnapDirOptions{HiddenSnapDataDir: true}},
{dir: dirs.HiddenSnapDataHomeDir, opts: &dirs.SnapDirOptions{HiddenSnapDataDir: true, MigratedToExposedHome: true}}} {
env := osutil.Environment{}
ExtendEnvForRun(env, mockSnapInfo, nil, t.opts)
ExtendEnvForRun(env, mockSnapInfo, mockAppInfo, nil, t.opts)

c.Check(env["SNAP_USER_COMMON"], Equals, filepath.Join(testDir, t.dir, mockSnapInfo.SuggestedName, "common"))
c.Check(env["SNAP_USER_DATA"], DeepEquals, filepath.Join(testDir, t.dir, mockSnapInfo.SuggestedName, mockSnapInfo.Revision.String()))
Expand Down
26 changes: 22 additions & 4 deletions tests/main/snap-env/task.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ prepare: |
snap set system experimental.parallel-instances=true
fi
"$TESTSTOOLS"/snaps-state install-local-as test-snapd-tools "$NAME"
"$TESTSTOOLS"/snaps-state install-local test-snapd-desktop-file-ids

restore: |
if [[ "$SPREAD_VARIANT" == "parallel" ]]; then
Expand Down Expand Up @@ -57,6 +58,7 @@ execute: |
MATCH "^SNAP_CONTEXT=$CTX" < snap-vars.txt
# parallel-installs: $SNAP_NAME is always _the_ snap name
MATCH '^SNAP_NAME=test-snapd-tools$' < snap-vars.txt
MATCH '^SNAP_APP_NAME=env$' < snap-vars.txt

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be nice to test a snap which has all the new variables set.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eh... Could you hep with that? Not being able to run the spread tests myself makes it a bit annoying to do...

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can run them with run-spread garden:... just install image-garden snap.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test-snapd-desktop-file-ids should do the trick, but needs a rebase on master. I'll poke at this and see what I can do.

# parallel-install: name of a particular instance
MATCH "^SNAP_INSTANCE_NAME=$NAME$" < snap-vars.txt
# parallel-installs: empty if none is set
Expand All @@ -67,11 +69,11 @@ execute: |
# if on UC20+, then we should see an additional variable (SNAP_SAVE_DATA)
if [[ "$SPREAD_SYSTEM" == ubuntu-core-2* ]]; then
MATCH "^SNAP_SAVE_DATA=/var/lib/snapd/save/snap/$NAME$" < snap-vars.txt
# 18 variables are expected on ubuntu-core
test "$(wc -l < snap-vars.txt)" -eq 18 || { cat snap-vars.txt; exit 1; }
# 19 variables are expected on ubuntu-core
test "$(wc -l < snap-vars.txt)" -eq 19 || { cat snap-vars.txt; exit 1; }
else
# 17 variables are expected on non ubuntu-core
test "$(wc -l < snap-vars.txt)" -eq 17 || { cat snap-vars.txt; exit 1; }
# 18 variables are expected on non ubuntu-core
test "$(wc -l < snap-vars.txt)" -eq 18 || { cat snap-vars.txt; exit 1; }
fi

echo "Ensure that XDG environment variables are what we expect"
Expand Down Expand Up @@ -99,3 +101,19 @@ execute: |
# parallel-installs: $HOME is set to instance specific path
MATCH "^HOME=/root/snap/$NAME/x1$" < misc-vars.txt
test "$(wc -l < misc-vars.txt)" -eq 4 || { cat misc-vars.txt; exit 1; }

echo "Collect SNAP environment variables for snap with desktop metadata"
echo env | snap run --shell test-snapd-desktop-file-ids.cmd | grep -E '^SNAP_' | sort > snap-vars-desktop.txt
echo "Ensure that SNAP environment variables are what we expect"
MATCH '^SNAP_NAME=test-snapd-desktop-file-ids$' < snap-vars-desktop.txt
MATCH '^SNAP_APP_NAME=cmd$' < snap-vars-desktop.txt
MATCH '^SNAP_APP_COMMON_ID=org.example.Foo$' < snap-vars-desktop.txt
MATCH '^SNAP_APP_DESKTOP_FILE=/var/lib/snapd/desktop/applications/org.example.Foo.desktop$' < snap-vars-desktop.txt

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAICT there are no existing spread tests which check the bus name, except a few use python scripts, which seems a stretch to use here. But we can at least check the other new desktop metadata.

# if on UC20+, then we should see an additional variable (SNAP_SAVE_DATA)
if [[ "$SPREAD_SYSTEM" == ubuntu-core-2* ]]; then
# 21 variables are expected on ubuntu-core
test "$(wc -l < snap-vars-desktop.txt)" -eq 21 || { cat snap-vars-desktop.txt; exit 1; }
else
# 20 variables are expected on non ubuntu-core
test "$(wc -l < snap-vars-desktop.txt)" -eq 20 || { cat snap-vars-desktop.txt; exit 1; }
fi
Loading