interfaces,overlord: support unversioned data #1095

Merged
merged 8 commits into from Apr 30, 2016
@@ -233,13 +233,15 @@ var defaultTemplate = []byte(`
# Writable home area for this version.
owner @{HOME}/snap/@{SNAP_NAME}/@{SNAP_REVISION}/** wl,
+ owner @{HOME}/snap/@{SNAP_NAME}/common/** wl,
# Read-only system area for other versions
/var/snap/@{SNAP_NAME}/ r,
/var/snap/@{SNAP_NAME}/** mrkix,
# Writable system area only for this version
/var/snap/@{SNAP_NAME}/@{SNAP_REVISION}/** wl,
+ /var/snap/@{SNAP_NAME}/common/** wl,
@jdstrand

jdstrand Apr 29, 2016

Contributor

FYI, these changes look fine security policy-wise.

# The ubuntu-core-launcher creates an app-specific private restricted /tmp
# and will fail to launch the app if something goes wrong. As such, we can
@@ -42,6 +42,7 @@ type managerBackend interface {
UnlinkSnap(info *snap.Info, meter progress.Meter) error
RemoveSnapFiles(s snap.PlaceInfo, meter progress.Meter) error
RemoveSnapData(info *snap.Info) error
+ RemoveSnapCommonData(info *snap.Info) error
// testing helpers
Candidate(sideInfo *snap.SideInfo)
@@ -119,3 +120,7 @@ func (b *defaultBackend) RemoveSnapFiles(s snap.PlaceInfo, meter progress.Meter)
func (b *defaultBackend) RemoveSnapData(info *snap.Info) error {
return snappy.RemoveSnapData(info)
}
+
+func (b *defaultBackend) RemoveSnapCommonData(info *snap.Info) error {
+ return snappy.RemoveSnapCommonData(info)
+}
@@ -206,6 +206,14 @@ func (f *fakeSnappyBackend) RemoveSnapData(info *snap.Info) error {
return nil
}
+func (f *fakeSnappyBackend) RemoveSnapCommonData(info *snap.Info) error {
+ f.ops = append(f.ops, fakeOp{
+ op: "remove-snap-common-data",
+ name: info.MountDir(),
+ })
+ return nil
+}
+
func (f *fakeSnappyBackend) Candidate(sideInfo *snap.SideInfo) {
var sinfo snap.SideInfo
if sideInfo != nil {
@@ -276,7 +276,7 @@ func (m *SnapManager) doUnlinkSnap(t *state.Task, _ *tomb.Tomb) error {
func (m *SnapManager) doClearSnapData(t *state.Task, _ *tomb.Tomb) error {
t.State().Lock()
- ss, err := TaskSnapSetup(t)
+ ss, snapst, err := snapSetupAndState(t)
t.State().Unlock()
if err != nil {
return err
@@ -289,7 +289,18 @@ func (m *SnapManager) doClearSnapData(t *state.Task, _ *tomb.Tomb) error {
return err
}
- return m.backend.RemoveSnapData(info)
+ if err = m.backend.RemoveSnapData(info); err != nil {
+ return err
+ }
+
+ // Only remove data common between versions if this is the last version
+ if len(snapst.Sequence) == 1 {
+ if err = m.backend.RemoveSnapCommonData(info); err != nil {
+ return err
+ }
+ }
+
+ return nil
}
func (m *SnapManager) doDiscardSnap(t *state.Task, _ *tomb.Tomb) error {
@@ -927,7 +927,7 @@ func (s *snapmgrTestSuite) TestRemoveIntegration(c *C) {
s.settle()
s.state.Lock()
- c.Assert(s.fakeBackend.ops, HasLen, 6)
+ c.Assert(s.fakeBackend.ops, HasLen, 7)
expected := []fakeOp{
fakeOp{
op: "can-remove",
@@ -947,6 +947,10 @@ func (s *snapmgrTestSuite) TestRemoveIntegration(c *C) {
op: "remove-snap-data",
name: "/snap/some-snap/7",
},
+ fakeOp{
+ op: "remove-snap-common-data",
+ name: "/snap/some-snap/7",
+ },
fakeOp{
op: "remove-snap-files",
name: "/snap/some-snap/7",
@@ -1018,7 +1022,7 @@ func (s *snapmgrTestSuite) TestRemoveWithManyRevisionsIntegration(c *C) {
s.settle()
s.state.Lock()
- c.Assert(s.fakeBackend.ops, HasLen, 10)
+ c.Assert(s.fakeBackend.ops, HasLen, 11)
expected := []fakeOp{
{
op: "can-remove",
@@ -1054,6 +1058,10 @@ func (s *snapmgrTestSuite) TestRemoveWithManyRevisionsIntegration(c *C) {
op: "remove-snap-data",
name: "/snap/some-snap/5",
},
+ {
+ op: "remove-snap-common-data",
+ name: "/snap/some-snap/5",
+ },
{
op: "remove-snap-files",
name: "/snap/some-snap/5",
View
@@ -45,8 +45,14 @@ type PlaceInfo interface {
// DataDir returns the data directory of the snap.
DataDir() string
+ // CommonDataDir returns the data directory common across revisions of the snap.
+ CommonDataDir() string
+
// DataHomeDir returns the per user data directory of the snap.
DataHomeDir() string
+
+ // CommonDataHomeDir returns the per user data directory common across revisions of the snap.
+ CommonDataHomeDir() string
}
// MinimalPlaceInfo returns a PlaceInfo with just the location information for a snap of the given name and revision.
@@ -157,11 +163,21 @@ func (s *Info) DataDir() string {
return filepath.Join(dirs.SnapDataDir, s.Name(), s.strRevno())
}
+// CommonDataDir returns the data directory common across revisions of the snap.
+func (s *Info) CommonDataDir() string {
+ return filepath.Join(dirs.SnapDataDir, s.Name(), "common")
+}
+
// DataHomeDir returns the per user data directory of the snap.
func (s *Info) DataHomeDir() string {
return filepath.Join(dirs.SnapDataHomeGlob, s.Name(), s.strRevno())
@zyga

zyga Apr 29, 2016

Contributor

FYI: note the glob remark on CommonDataHomeDir below

}
+// CommonDataHomeDir returns the per user data directory common across revisions of the snap.
+func (s *Info) CommonDataHomeDir() string {
+ return filepath.Join(dirs.SnapDataHomeGlob, s.Name(), "common")
+}
+
// sanity check that Info is a PlacInfo
var _ PlaceInfo = (*Info)(nil)
View
@@ -36,14 +36,14 @@ const expectedWrapper = `#!/bin/sh
set -e
# snap info
-export SNAP="/snap/pastebinit/44/"
-export SNAP_DATA="/var/snap/pastebinit/44/"
+export SNAP="/snap/pastebinit/44"
+export SNAP_DATA="/var/snap/pastebinit/44"
export SNAP_NAME="pastebinit"
export SNAP_VERSION="1.4.0.0.1"
export SNAP_REVISION="44"
export SNAP_ARCH="%[1]s"
export SNAP_LIBRARY_PATH="/var/lib/snapd/lib/gl:"
-export SNAP_USER_DATA="$HOME/snap/pastebinit/44/"
+export SNAP_USER_DATA="$HOME/snap/pastebinit/44"
if [ ! -d "$SNAP_USER_DATA" ]; then
mkdir -p "$SNAP_USER_DATA"
@@ -57,7 +57,7 @@ ubuntu-core-launcher snap.pastebinit.pastebinit snap.pastebinit.pastebinit /snap
`
func (s *SnapTestSuite) TestSnappyGenerateSnapBinaryWrapper(c *C) {
- pkgPath := "/snap/pastebinit/44/"
+ pkgPath := "/snap/pastebinit/44"
info := &snap.Info{}
info.SuggestedName = "pastebinit"
info.Version = "1.4.0.0.1"
@@ -76,7 +76,7 @@ func (s *SnapTestSuite) TestSnappyGenerateSnapBinaryWrapper(c *C) {
}
func (s *SnapTestSuite) TestSnappyGenerateSnapBinaryWrapperIllegalChars(c *C) {
- pkgPath := "/snap/pastebinit/44/"
+ pkgPath := "/snap/pastebinit/44"
info := &snap.Info{}
info.SuggestedName = "pastebinit"
info.Version = "1.4.0.0.1"
@@ -91,12 +91,12 @@ func (s *SnapTestSuite) TestSnappyGenerateSnapBinaryWrapperIllegalChars(c *C) {
func (s *SnapTestSuite) TestSnappyBinPathForBinaryNoExec(c *C) {
binary := &snap.AppInfo{Name: "pastebinit", Command: "bin/pastebinit"}
- pkgPath := "/snap/pastebinit/44/"
+ pkgPath := "/snap/pastebinit/44"
c.Assert(binPathForBinary(pkgPath, binary), Equals, "/snap/pastebinit/44/bin/pastebinit")
}
func (s *SnapTestSuite) TestSnappyBinPathForBinaryWithExec(c *C) {
binary := &snap.AppInfo{Name: "pastebinit", Command: "bin/random-pastebin"}
- pkgPath := "/snap/pastebinit/44/"
+ pkgPath := "/snap/pastebinit/44"
c.Assert(binPathForBinary(pkgPath, binary), Equals, "/snap/pastebinit/44/bin/random-pastebin")
}
View
@@ -124,7 +124,16 @@ func (s *SnapTestSuite) TestClickInstallGCSimple(c *C) {
// gc should no longer leave one more data than app
globs, err = filepath.Glob(filepath.Join(dirs.SnapDataDir, "foo", "*"))
c.Check(err, IsNil)
- c.Check(globs, HasLen, 2+1) // +1 for "current"
+ c.Check(globs, HasLen, 2+1+1) // +1 for "current", +1 for common
+
+ // ensure common data is actually present, and it isn't the old version
+ commonFound := false
+ for _, glob := range globs {
+ if filepath.Base(glob) == "common" {
+ commonFound = true
+ }
+ }
+ c.Check(commonFound, Equals, true)
}
// check that if flags does not include DoInstallGC, no gc is done
@@ -137,7 +146,16 @@ func (s *SnapTestSuite) TestClickInstallGCSuppressed(c *C) {
globs, err = filepath.Glob(filepath.Join(dirs.SnapDataDir, "foo", "*"))
c.Check(err, IsNil)
- c.Check(globs, HasLen, 3+1) // +1 for "current"
+ c.Check(globs, HasLen, 3+1+1) // +1 for "current", +1 for common
+
+ // ensure common data is actually present
+ commonFound := false
+ for _, glob := range globs {
+ if filepath.Base(glob) == "common" {
+ commonFound = true
+ }
+ }
+ c.Check(commonFound, Equals, true)
}
func (s *SnapTestSuite) TestInstallAppTwiceFails(c *C) {
View
@@ -178,13 +178,17 @@ func UndoSetupSnap(s snap.PlaceInfo, meter progress.Meter) {
}
func CopyData(newSnap, oldSnap *snap.Info, flags InstallFlags, meter progress.Meter) error {
- dataDir := newSnap.DataDir()
-
// deal with the old data or
// otherwise just create a empty data dir
+ // Make sure the common data directory exists, even if this isn't a new
+ // install.
+ if err := os.MkdirAll(newSnap.CommonDataDir(), 0755); err != nil {
+ return err
+ }
+
if oldSnap == nil {
- return os.MkdirAll(dataDir, 0755)
+ return os.MkdirAll(newSnap.DataDir(), 0755)
}
return copySnapData(oldSnap, newSnap)
View
@@ -73,7 +73,7 @@ func (s *SnapTestSuite) TestLocalSnapInstall(c *C) string {
c.Check(snapEntries, DeepEquals, []string{"0", "current"})
snapDataEntries := listDir(c, filepath.Join(dirs.SnapDataDir, fooComposedName))
- c.Check(snapDataEntries, DeepEquals, []string{"0", "current"})
+ c.Check(snapDataEntries, DeepEquals, []string{"0", "common", "current"})
return snapPath
}
@@ -98,7 +98,7 @@ func (s *SnapTestSuite) TestLocalSnapInstallWithBlessedMetadata(c *C) {
c.Check(snapEntries, DeepEquals, []string{"40", "current"})
snapDataEntries := listDir(c, filepath.Join(dirs.SnapDataDir, fooComposedName))
- c.Check(snapDataEntries, DeepEquals, []string{"40", "current"})
+ c.Check(snapDataEntries, DeepEquals, []string{"40", "common", "current"})
}
func (s *SnapTestSuite) TestLocalSnapInstallWithBlessedMetadataOverridingName(c *C) {
@@ -401,6 +401,9 @@ func (s *SnapTestSuite) TestCopyData(c *C) {
homeData := filepath.Join(homeDir, appDir, "10")
err := os.MkdirAll(homeData, 0755)
c.Assert(err, IsNil)
+ homeCommonData := filepath.Join(homeDir, appDir, "common")
+ err = os.MkdirAll(homeCommonData, 0755)
+ c.Assert(err, IsNil)
snapYamlContent := `name: foo
`
@@ -412,8 +415,13 @@ func (s *SnapTestSuite) TestCopyData(c *C) {
canaryDataFile := filepath.Join(dirs.SnapDataDir, appDir, "10", "canary.txt")
err = ioutil.WriteFile(canaryDataFile, canaryData, 0644)
c.Assert(err, IsNil)
+ canaryDataFile = filepath.Join(dirs.SnapDataDir, appDir, "common", "canary.common")
+ err = ioutil.WriteFile(canaryDataFile, canaryData, 0644)
+ c.Assert(err, IsNil)
err = ioutil.WriteFile(filepath.Join(homeData, "canary.home"), canaryData, 0644)
c.Assert(err, IsNil)
+ err = ioutil.WriteFile(filepath.Join(homeCommonData, "canary.common_home"), canaryData, 0644)
+ c.Assert(err, IsNil)
snapPath = makeTestSnapPackage(c, snapYamlContent+"version: 2.0")
_, err = (&Overlord{}).InstallWithSideInfo(snapPath, fooSI20, AllowUnauthenticated, nil)
@@ -423,8 +431,20 @@ func (s *SnapTestSuite) TestCopyData(c *C) {
c.Assert(err, IsNil)
c.Assert(content, DeepEquals, canaryData)
- newHomeDataCanaryFile := filepath.Join(homeDir, appDir, "20", "canary.home")
- content, err = ioutil.ReadFile(newHomeDataCanaryFile)
+ // ensure common data file is still there (even though it didn't get copied)
+ newCanaryDataFile = filepath.Join(dirs.SnapDataDir, appDir, "common", "canary.common")
+ content, err = ioutil.ReadFile(newCanaryDataFile)
+ c.Assert(err, IsNil)
+ c.Assert(content, DeepEquals, canaryData)
+
+ newCanaryDataFile = filepath.Join(homeDir, appDir, "20", "canary.home")
+ content, err = ioutil.ReadFile(newCanaryDataFile)
+ c.Assert(err, IsNil)
+ c.Assert(content, DeepEquals, canaryData)
+
+ // ensure home common data file is still there (even though it didn't get copied)
+ newCanaryDataFile = filepath.Join(homeDir, appDir, "common", "canary.common_home")
+ content, err = ioutil.ReadFile(newCanaryDataFile)
c.Assert(err, IsNil)
c.Assert(content, DeepEquals, canaryData)
}
@@ -445,15 +465,21 @@ func (s *SnapTestSuite) TestCopyDataNoUserHomes(c *C) {
canaryDataFile := filepath.Join(snap.DataDir(), "canary.txt")
err = ioutil.WriteFile(canaryDataFile, []byte(""), 0644)
c.Assert(err, IsNil)
+ canaryDataFile = filepath.Join(snap.CommonDataDir(), "canary.common")
+ err = ioutil.WriteFile(canaryDataFile, []byte(""), 0644)
+ c.Assert(err, IsNil)
snapPath = makeTestSnapPackage(c, snapYamlContent+"version: 2.0")
snap2, err := (&Overlord{}).InstallWithSideInfo(snapPath, fooSI20, AllowUnauthenticated, nil)
c.Assert(err, IsNil)
_, err = os.Stat(filepath.Join(snap2.DataDir(), "canary.txt"))
c.Assert(err, IsNil)
+ _, err = os.Stat(filepath.Join(snap2.CommonDataDir(), "canary.common"))
+ c.Assert(err, IsNil)
// sanity atm
c.Check(snap.DataDir(), Not(Equals), snap2.DataDir())
+ c.Check(snap.CommonDataDir(), Equals, snap2.CommonDataDir())
}
func (s *SnapTestSuite) TestSnappyHandleBinariesOnUpgrade(c *C) {
@@ -466,7 +492,7 @@ apps:
_, err := (&Overlord{}).InstallWithSideInfo(snapPath, fooSI10, AllowUnauthenticated, nil)
c.Assert(err, IsNil)
- // ensure that the binary wrapper file go generated with the right
+ // ensure that the binary wrapper file got generated with the right
// path
oldSnapBin := filepath.Join(dirs.SnapSnapsDir[len(dirs.GlobalRootDir):], "foo", "10", "bin", "bar")
binaryWrapper := filepath.Join(dirs.SnapBinariesDir, "foo.bar")
Oops, something went wrong.