Merge lp:~chipaca/snappy/atomic-follow-symlinks into lp:~snappy-dev/snappy/snappy-moved-to-github

[John Lenton]

Make atomic follow symlinks (so you don't have to write to /etc/writable).

[Michael Vogt (mvo)]

Thanks!

review: Approve

[Snappy Tarmac (snappydevtarmac)]

The attempt to merge lp:~chipaca/snappy/atomic-follow-symlinks into lp:snappy failed. Below is the output from the failed tests.

Checking docs
Checking formatting
Installing godeps
Install golint
Obtaining dependencies
update github.com/gorilla/mux failed; trying to fetch newer version
update github.com/mvo5/goconfigparser failed; trying to fetch newer version
github.com/gorilla/mux now at ee1815431e497d3850809578c93ab6705f1a19f7
update github.com/cheggaaa/pb failed; trying to fetch newer version
github.com/mvo5/goconfigparser now at 26426272dda20cc76aa1fa44286dc743d2972fe8
update github.com/mvo5/uboot-go failed; trying to fetch newer version
github.com/cheggaaa/pb now at e8c7cc515bfde3e267957a3b110080ceed51354e
update gopkg.in/check.v1 failed; trying to fetch newer version
github.com/mvo5/uboot-go now at 361f6ebcbb54f389d15dc9faefa000e996ba3e37
update github.com/gosexy/gettext failed; trying to fetch newer version
gopkg.in/check.v1 now at 64131543e7896d5bcc6bd5a76287eb75ea96c673
update github.com/peterh/liner failed; trying to fetch newer version
github.com/gosexy/gettext now at 98b7b91596d20b96909e6b60d57411547dd9959c
update gopkg.in/tomb.v2 failed; trying to fetch newer version
github.com/peterh/liner now at 1bb0d1c1a25ed393d8feb09bab039b2b1b1fbced
update code.google.com/p/go.crypto failed; trying to fetch newer version
gopkg.in/tomb.v2 now at 14b3d72120e8d10ea6e6b7f87f7175734b1faab8
update github.com/blakesmith/ar failed; trying to fetch newer version
code.google.com/p/go.crypto now at 69e2a90ed92d03812364aeb947b7068dc42e561e
update github.com/coreos/go-systemd failed; trying to fetch newer version
github.com/blakesmith/ar now at c9a977dd0cc1392b023382c7bfa5a22af8d3b730
update github.com/gorilla/context failed; trying to fetch newer version
github.com/coreos/go-systemd now at f743bc15d6bddd23662280b4ad20f7c874cdd5ad
update github.com/jessevdk/go-flags failed; trying to fetch newer version
github.com/gorilla/context now at 1c83b3eabd45b6d76072b66b746c20815fb2872d
update golang.org/x/crypto failed; trying to fetch newer version
github.com/jessevdk/go-flags now at 1acbbaff2f347c412a0c7884873bd72cc9c1f5b4
update gopkg.in/yaml.v2 failed; trying to fetch newer version
golang.org/x/crypto now at 60052bd85f2d91293457e8811b0cf26b773de469
gopkg.in/yaml.v2 now at 49c95bdc21843256fb6c4e0d370a05f24a0bf213
Building
Running tests from /tmp/tmp.YRgRWPKpsg/src/launchpad.net/snappy
? launchpad.net/snappy/cmd/snapd [no test files]
=== RUN Test
OK: 12 passed
--- PASS: Test (0.04 seconds)
PASS
coverage: 21.2% of statements
ok launchpad.net/snappy/cmd/snappy 0.051s coverage: 21.2% of statements
=== RUN Test
OK: 53 passed
--- PASS: Test (0.35 seconds)
PASS
coverage: 91.7% of statements
ok launchpad.net/snappy/coreconfig 0.352s coverage: 91.7% of statements
=== RUN Test
OK: 39 passed
--- PASS: Test (0.16 seconds)
PASS
coverage: 69.8% of statements
ok launchpad.net/snappy/daemon 0.172s coverage: 69.8% of statements
? launchpad.net/snappy/dirs [no test files]
=== RUN Test
OK: 64 passed
--- PASS: Test (0.36 seconds)
PASS
coverage: 79.2% of statements
ok launchpad.net/snappy/helpers 0.371s coverage: 79.2% of statements
=== RUN Test
OK: 0 passed, 2 ski...

[lp:~chipaca/snappy/atomic-follow-symlinks]

799. By John Lenton on 2015-10-22

make the follow symlink behaviour depend on a flag. Oh, add a flag.

[Michael Vogt (mvo)]

Thanks!

review: Approve

=== modified file 'coreconfig/config.go'
--- coreconfig/config.go	2015-10-22 10:44:15 +0000
+++ coreconfig/config.go	2015-10-22 12:45:46 +0000
@@ -39,12 +39,12 @@
 
 const (
 	tzPathEnvironment string = "UBUNTU_CORE_CONFIG_TZ_FILE"
-	tzPathDefault     string = "/etc/writable/timezone"
+	tzPathDefault     string = "/etc/timezone"
 )
 
 var (
 	tzZoneInfoPath   = "/usr/share/zoneinfo"
-	tzZoneInfoTarget = "/etc/writable/localtime"
+	tzZoneInfoTarget = "/etc/localtime"
 )
 
 const (
@@ -58,8 +58,8 @@
 	modulesPath         = "/etc/modules-load.d/ubuntu-core.conf"
 	interfacesRoot      = "/etc/network/interfaces.d/"
 	pppRoot             = "/etc/ppp/"
-	watchdogConfigPath  = "/etc/writable/watchdog.conf"
-	watchdogStartupPath = "/etc/writable/default/watchdog"
+	watchdogConfigPath  = "/etc/watchdog.conf"
+	watchdogStartupPath = "/etc/default/watchdog"
 )
 
 var (
@@ -311,7 +311,7 @@
 		return err
 	}
 
-	return helpers.AtomicWriteFile(tzFile(), []byte(timezone), 0644)
+	return helpers.AtomicWriteFile(tzFile(), []byte(timezone), 0644, helpers.AtomicWriteFollow)
 }
 
 func getPassthrough(rootDir string) (pc []passthroughConfig, err error) {
@@ -340,7 +340,7 @@
 			os.Remove(path)
 			continue
 		}
-		if err := helpers.AtomicWriteFile(path, []byte(c.Content), 0644); err != nil {
+		if err := helpers.AtomicWriteFile(path, []byte(c.Content), 0644, helpers.AtomicWriteFollow); err != nil {
 			return err
 		}
 	}
@@ -376,7 +376,7 @@
 
 // setModprobe sets the specified modprobe config
 var setModprobe = func(modprobe string) error {
-	return helpers.AtomicWriteFile(modprobePath, []byte(modprobe), 0644)
+	return helpers.AtomicWriteFile(modprobePath, []byte(modprobe), 0644, helpers.AtomicWriteFollow)
 }
 
 func getModules() ([]string, error) {
@@ -463,7 +463,7 @@
 		buf.WriteByte('\n')
 	}
 
-	return helpers.AtomicWriteFile(modulesPath, buf.Bytes(), 0644)
+	return helpers.AtomicWriteFile(modulesPath, buf.Bytes(), 0644, helpers.AtomicWriteFollow)
 }
 
 // getWatchdog returns the current watchdog config
@@ -489,11 +489,11 @@
 
 // setWatchdog sets the specified watchdog config
 var setWatchdog = func(wf *watchdogConfig) error {
-	if err := helpers.AtomicWriteFile(watchdogStartupPath, []byte(wf.Startup), 0644); err != nil {
+	if err := helpers.AtomicWriteFile(watchdogStartupPath, []byte(wf.Startup), 0644, helpers.AtomicWriteFollow); err != nil {
 		return err
 	}
 
-	return helpers.AtomicWriteFile(watchdogConfigPath, []byte(wf.Config), 0644)
+	return helpers.AtomicWriteFile(watchdogConfigPath, []byte(wf.Config), 0644, helpers.AtomicWriteFollow)
 }
 
 // for testing purposes
@@ -568,5 +568,5 @@
 		return err
 	}
 
-	return helpers.AtomicWriteFile(hostnamePath, hostnameB, 0644)
+	return helpers.AtomicWriteFile(hostnamePath, hostnameB, 0644, helpers.AtomicWriteFollow)
 }

=== modified file 'helpers/helpers.go'
--- helpers/helpers.go	2015-10-15 07:32:34 +0000
+++ helpers/helpers.go	2015-10-22 12:45:46 +0000
@@ -292,12 +292,29 @@
 	return string(bs)
 }
 
+// AtomicWriteFlags are a bitfield of flags for AtomicWriteFile
+type AtomicWriteFlags uint
+
+const (
+	// AtomicWriteFollow makes AtomicWriteFile follows symlinks
+	AtomicWriteFollow AtomicWriteFlags = 1 << iota
+)
+
 // AtomicWriteFile updates the filename atomically and works otherwise
 // like io/ioutil.WriteFile()
 //
 // Note that it won't follow symlinks and will replace existing symlinks
 // with the real file
-func AtomicWriteFile(filename string, data []byte, perm os.FileMode) (err error) {
+func AtomicWriteFile(filename string, data []byte, perm os.FileMode, flags AtomicWriteFlags) (err error) {
+	if flags&AtomicWriteFollow != 0 {
+		if fn, err := os.Readlink(filename); err == nil || (fn != "" && os.IsNotExist(err)) {
+			if filepath.IsAbs(fn) {
+				filename = fn
+			} else {
+				filename = filepath.Join(filepath.Dir(filename), fn)
+			}
+		}
+	}
 	tmp := filename + "." + MakeRandomString(12)
 
 	// XXX: if go switches to use aio_fsync, we need to open the dir for writing

=== modified file 'helpers/helpers_test.go'
--- helpers/helpers_test.go	2015-10-19 10:38:22 +0000
+++ helpers/helpers_test.go	2015-10-22 12:45:46 +0000
@@ -241,7 +241,7 @@
 	tmpdir := c.MkDir()
 
 	p := filepath.Join(tmpdir, "foo")
-	err := AtomicWriteFile(p, []byte("canary"), 0644)
+	err := AtomicWriteFile(p, []byte("canary"), 0644, 0)
 	c.Assert(err, IsNil)
 
 	content, err := ioutil.ReadFile(p)
@@ -258,7 +258,7 @@
 	tmpdir := c.MkDir()
 
 	p := filepath.Join(tmpdir, "foo")
-	err := AtomicWriteFile(p, []byte(""), 0600)
+	err := AtomicWriteFile(p, []byte(""), 0600, 0)
 	c.Assert(err, IsNil)
 
 	st, err := os.Stat(p)
@@ -266,6 +266,102 @@
 	c.Assert(st.Mode()&os.ModePerm, Equals, os.FileMode(0600))
 }
 
+func (ts *HTestSuite) TestAtomicWriteFileOverwrite(c *C) {
+	tmpdir := c.MkDir()
+	p := filepath.Join(tmpdir, "foo")
+	c.Assert(ioutil.WriteFile(p, []byte("hello"), 0644), IsNil)
+	c.Assert(AtomicWriteFile(p, []byte("hi"), 0600, 0), IsNil)
+
+	content, err := ioutil.ReadFile(p)
+	c.Assert(err, IsNil)
+	c.Assert(string(content), Equals, "hi")
+}
+
+func (ts *HTestSuite) TestAtomicWriteFileSymlinkNoFollow(c *C) {
+	tmpdir := c.MkDir()
+	rodir := filepath.Join(tmpdir, "ro")
+	p := filepath.Join(rodir, "foo")
+	s := filepath.Join(tmpdir, "foo")
+	c.Assert(os.MkdirAll(rodir, 0755), IsNil)
+	c.Assert(os.Symlink(s, p), IsNil)
+	c.Assert(os.Chmod(rodir, 0500), IsNil)
+	defer os.Chmod(rodir, 0700)
+
+	err := AtomicWriteFile(p, []byte("hi"), 0600, 0)
+	c.Assert(err, NotNil)
+}
+
+func (ts *HTestSuite) TestAtomicWriteFileAbsoluteSymlinks(c *C) {
+	tmpdir := c.MkDir()
+	rodir := filepath.Join(tmpdir, "ro")
+	p := filepath.Join(rodir, "foo")
+	s := filepath.Join(tmpdir, "foo")
+	c.Assert(os.MkdirAll(rodir, 0755), IsNil)
+	c.Assert(os.Symlink(s, p), IsNil)
+	c.Assert(os.Chmod(rodir, 0500), IsNil)
+	defer os.Chmod(rodir, 0700)
+
+	err := AtomicWriteFile(p, []byte("hi"), 0600, AtomicWriteFollow)
+	c.Assert(err, IsNil)
+
+	content, err := ioutil.ReadFile(p)
+	c.Assert(err, IsNil)
+	c.Assert(string(content), Equals, "hi")
+}
+
+func (ts *HTestSuite) TestAtomicWriteFileOverwriteAbsoluteSymlink(c *C) {
+	tmpdir := c.MkDir()
+	rodir := filepath.Join(tmpdir, "ro")
+	p := filepath.Join(rodir, "foo")
+	s := filepath.Join(tmpdir, "foo")
+	c.Assert(os.MkdirAll(rodir, 0755), IsNil)
+	c.Assert(os.Symlink(s, p), IsNil)
+	c.Assert(os.Chmod(rodir, 0500), IsNil)
+	defer os.Chmod(rodir, 0700)
+
+	c.Assert(ioutil.WriteFile(s, []byte("hello"), 0644), IsNil)
+	c.Assert(AtomicWriteFile(p, []byte("hi"), 0600, AtomicWriteFollow), IsNil)
+
+	content, err := ioutil.ReadFile(p)
+	c.Assert(err, IsNil)
+	c.Assert(string(content), Equals, "hi")
+}
+
+func (ts *HTestSuite) TestAtomicWriteFileRelativeSymlinks(c *C) {
+	tmpdir := c.MkDir()
+	rodir := filepath.Join(tmpdir, "ro")
+	p := filepath.Join(rodir, "foo")
+	c.Assert(os.MkdirAll(rodir, 0755), IsNil)
+	c.Assert(os.Symlink("../foo", p), IsNil)
+	c.Assert(os.Chmod(rodir, 0500), IsNil)
+	defer os.Chmod(rodir, 0700)
+
+	err := AtomicWriteFile(p, []byte("hi"), 0600, AtomicWriteFollow)
+	c.Assert(err, IsNil)
+
+	content, err := ioutil.ReadFile(p)
+	c.Assert(err, IsNil)
+	c.Assert(string(content), Equals, "hi")
+}
+
+func (ts *HTestSuite) TestAtomicWriteFileOverwriteRelativeSymlink(c *C) {
+	tmpdir := c.MkDir()
+	rodir := filepath.Join(tmpdir, "ro")
+	p := filepath.Join(rodir, "foo")
+	s := filepath.Join(tmpdir, "foo")
+	c.Assert(os.MkdirAll(rodir, 0755), IsNil)
+	c.Assert(os.Symlink("../foo", p), IsNil)
+	c.Assert(os.Chmod(rodir, 0500), IsNil)
+	defer os.Chmod(rodir, 0700)
+
+	c.Assert(ioutil.WriteFile(s, []byte("hello"), 0644), IsNil)
+	c.Assert(AtomicWriteFile(p, []byte("hi"), 0600, AtomicWriteFollow), IsNil)
+
+	content, err := ioutil.ReadFile(p)
+	c.Assert(err, IsNil)
+	c.Assert(string(content), Equals, "hi")
+}
+
 func (ts *HTestSuite) TestAtomicWriteFileNoOverwriteTmpExisting(c *C) {
 	tmpdir := c.MkDir()
 	realMakeRandomString := MakeRandomString
@@ -280,7 +376,7 @@
 	err := ioutil.WriteFile(p+".4", []byte(""), 0644)
 	c.Assert(err, IsNil)
 
-	err = AtomicWriteFile(p, []byte(""), 0600)
+	err = AtomicWriteFile(p, []byte(""), 0600, 0)
 	c.Assert(err, ErrorMatches, "open .*: file exists")
 }
 

=== modified file 'partition/bootloader_uboot.go'
--- partition/bootloader_uboot.go	2015-07-24 12:00:01 +0000
+++ partition/bootloader_uboot.go	2015-10-22 12:45:46 +0000
@@ -261,7 +261,7 @@
 	}
 
 	if updateNeeded {
-		return atomicWriteFile(path, buf.Bytes(), 0644)
+		return atomicWriteFile(path, buf.Bytes(), 0644, 0)
 	}
 
 	return nil

=== modified file 'partition/bootloader_uboot_test.go'
--- partition/bootloader_uboot_test.go	2015-07-24 11:58:04 +0000
+++ partition/bootloader_uboot_test.go	2015-10-22 12:45:46 +0000
@@ -291,9 +291,9 @@
 	s.makeFakeUbootEnv(c)
 
 	atomiCall := false
-	atomicWriteFile = func(a string, b []byte, c os.FileMode) error {
+	atomicWriteFile = func(a string, b []byte, c os.FileMode, f helpers.AtomicWriteFlags) error {
 		atomiCall = true
-		return helpers.AtomicWriteFile(a, b, c)
+		return helpers.AtomicWriteFile(a, b, c, f)
 	}
 
 	partition := New()
@@ -311,9 +311,9 @@
 	c.Assert(ioutil.WriteFile(bootloaderUbootEnvFile, []byte(""), 0644), IsNil)
 
 	atomiCall := false
-	atomicWriteFile = func(a string, b []byte, c os.FileMode) error {
+	atomicWriteFile = func(a string, b []byte, c os.FileMode, f helpers.AtomicWriteFlags) error {
 		atomiCall = true
-		return helpers.AtomicWriteFile(a, b, c)
+		return helpers.AtomicWriteFile(a, b, c, f)
 	}
 
 	partition := New()

=== modified file 'partition/migrate_grub.go'
--- partition/migrate_grub.go	2015-09-16 11:19:03 +0000
+++ partition/migrate_grub.go	2015-10-22 12:45:46 +0000
@@ -149,5 +149,5 @@
 		}
 	}
 
-	return helpers.AtomicWriteFile(bootloaderGrubConfigFile, []byte(newGrubConfig), 0644)
+	return helpers.AtomicWriteFile(bootloaderGrubConfigFile, []byte(newGrubConfig), 0644, 0)
 }

=== modified file 'snappy/auth.go'
--- snappy/auth.go	2015-06-04 22:04:09 +0000
+++ snappy/auth.go	2015-10-22 12:45:46 +0000
@@ -139,7 +139,7 @@
 		return nil
 	}
 
-	return helpers.AtomicWriteFile(targetFile, []byte(outStr), 0600)
+	return helpers.AtomicWriteFile(targetFile, []byte(outStr), 0600, 0)
 }
 
 // ReadStoreToken reads a token previously write via WriteStoreToken

=== modified file 'snappy/click.go'
--- snappy/click.go	2015-10-15 07:32:34 +0000
+++ snappy/click.go	2015-10-22 12:45:46 +0000
@@ -758,7 +758,7 @@
 	if err != nil {
 		return err
 	}
-	if err := helpers.AtomicWriteFile(filepath.Join(clickMetaDir, cm.Name+".manifest"), []byte(outStr), 0644); err != nil {
+	if err := helpers.AtomicWriteFile(filepath.Join(clickMetaDir, cm.Name+".manifest"), []byte(outStr), 0644, 0); err != nil {
 		return err
 	}
 

=== modified file 'snappy/firstboot.go'
--- snappy/firstboot.go	2015-10-22 09:32:32 +0000
+++ snappy/firstboot.go	2015-10-22 12:45:46 +0000
@@ -135,7 +135,7 @@
 	ethfile := filepath.Join(ethdir, eth)
 	data := fmt.Sprintf("allow-hotplug %[1]s\niface %[1]s inet dhcp\n", eth)
 
-	if err := helpers.AtomicWriteFile(ethfile, []byte(data), 0644); err != nil {
+	if err := helpers.AtomicWriteFile(ethfile, []byte(data), 0644, 0); err != nil {
 		return err
 	}
 

=== modified file 'snappy/hwaccess.go'
--- snappy/hwaccess.go	2015-09-29 07:07:13 +0000
+++ snappy/hwaccess.go	2015-10-22 12:45:46 +0000
@@ -91,7 +91,7 @@
 	out = append(out, '\n')
 
 	additionalFile := getHWAccessJSONFile(snapname)
-	if err := helpers.AtomicWriteFile(additionalFile, out, 0640); err != nil {
+	if err := helpers.AtomicWriteFile(additionalFile, out, 0640, 0); err != nil {
 		return err
 	}
 
@@ -130,7 +130,7 @@
 	// In both cases, updatedRules will have the right content.
 	updatedRules := append(rules, newRule...)
 
-	if err := helpers.AtomicWriteFile(udevRulesFile, updatedRules, 0644); nil != err {
+	if err := helpers.AtomicWriteFile(udevRulesFile, updatedRules, 0644, 0); nil != err {
 		return err
 	}
 
@@ -250,7 +250,7 @@
 			out = out + rule + "\n"
 		}
 
-		if err := helpers.AtomicWriteFile(udevRulesFile, []byte(out), 0644); nil != err {
+		if err := helpers.AtomicWriteFile(udevRulesFile, []byte(out), 0644, 0); nil != err {
 			return err
 		}
 	} else {