dbus: ensure io.snapcraft.Launcher.service is created on re-exec #4034

Merged
merged 5 commits into from Oct 13, 2017

Conversation

Projects
None yet
5 participants
Collaborator

mvo5 commented Oct 12, 2017

For snap userd to work we need the file:
/usr/share/dbus-1/services/io.snapcraft.Launcher.service

It is part of the deb/rpm package. However if snapd is using
re-exec we need to ensure this file is created by the dbus
backend.

See https://forum.snapcraft.io/t/snapd-2-28-xdg-open-doesnt-work-for-electron-applications/2447/5

dbus: ensure io.snapcraft.Launcher.service is created on re-exec
For `snap userd` to work we need the file:
/usr/share/dbus-1/services/io.snapcraft.Launcher.service

It is part of the deb/rpm package. However if snapd is using
re-exec we need to ensure this file is created by the dbus
backend.

@mvo5 mvo5 added this to the 2.28 milestone Oct 12, 2017

Seems fine; two questions.

interfaces/dbus/backend.go
+// `snap us erd` instance on re-exec
+func setupDbusServiceForUserd(snapInfo *snap.Info) error {
+ coreRoot := snapInfo.MountDir()
+ dst := "/usr/share/dbus-1/services/io.snapcraft.Launcher.service"
@chipaca

chipaca Oct 12, 2017

Member

is this path ok cross-distro?

@mvo5

mvo5 Oct 12, 2017

Collaborator

Yeah, that should be ok.

@@ -57,6 +71,13 @@ func (b *Backend) Setup(snapInfo *snap.Info, opts interfaces.ConfinementOptions,
return fmt.Errorf("cannot obtain dbus specification for snap %q: %s", snapName, err)
}
+ // core on classic is special
+ if snapName == "core" && release.OnClassic && !release.ReleaseInfo.ForceDevMode() {
@chipaca

chipaca Oct 12, 2017

Member

do you really mean snapName == "core" and not snapInfo.Type == snap.TypeCore?

@mvo5

mvo5 Oct 12, 2017

Collaborator

Hm, hm, a good question. I would say "core" is ok here because only on "core" we know that this parth exists.

@zyga

zyga Oct 12, 2017

Contributor

Why is the ForceDevMode check there? I think this is "are we supporting reexec" not "force devmode"

Looks good, just two questions.

interfaces/dbus/backend.go
@@ -46,6 +49,17 @@ func (b *Backend) Name() interfaces.SecuritySystem {
return "dbus"
}
+// setupDbusServiceForUserd will setup the service file for the new
+// `snap us erd` instance on re-exec
@stolowski

stolowski Oct 12, 2017

Contributor

A typo ("snap us erd")?

interfaces/dbus/backend.go
+ // core on classic is special
+ if snapName == "core" && release.OnClassic && !release.ReleaseInfo.ForceDevMode() {
+ if err := setupDbusServiceForUserd(snapInfo); err != nil {
+ logger.Noticef("cannot create host snap-confine apparmor configuration: %s", err)
@stolowski

stolowski Oct 12, 2017

Contributor

Hmm, this error message looks off.

Thanks!

codecov-io commented Oct 12, 2017

Codecov Report

Merging #4034 into master will decrease coverage by 0.02%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4034      +/-   ##
==========================================
- Coverage   75.88%   75.85%   -0.03%     
==========================================
  Files         431      431              
  Lines       36915    36926      +11     
==========================================
- Hits        28014    28012       -2     
- Misses       6948     6960      +12     
- Partials     1953     1954       +1
Impacted Files Coverage Δ
interfaces/dbus/backend.go 61.64% <0%> (-10.94%) ⬇️
overlord/ifacestate/helpers.go 62.33% <0%> (-0.67%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6810f48...2ff8733. Read the comment docs.

mvo5 added some commits Oct 12, 2017

@mvo5 mvo5 merged commit e338a29 into snapcore:master Oct 13, 2017

1 of 7 checks passed

artful-amd64 autopkgtest running
Details
artful-i386 autopkgtest running
Details
xenial-amd64 autopkgtest running
Details
xenial-i386 autopkgtest running
Details
xenial-ppc64el autopkgtest running
Details
zesty-amd64 autopkgtest running
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment