Merge lp:~chipaca/snappy/atomic-follow-symlinks into lp:~snappy-dev/snappy/snappy-moved-to-github
- atomic-follow-symlinks
- Merge into snappy-moved-to-github
Status: | Merged |
---|---|
Approved by: | Michael Vogt |
Approved revision: | 799 |
Merged at revision: | 796 |
Proposed branch: | lp:~chipaca/snappy/atomic-follow-symlinks |
Merge into: | lp:~snappy-dev/snappy/snappy-moved-to-github |
Diff against target: |
377 lines (+140/-27) 10 files modified
coreconfig/config.go (+11/-11) helpers/helpers.go (+18/-1) helpers/helpers_test.go (+99/-3) partition/bootloader_uboot.go (+1/-1) partition/bootloader_uboot_test.go (+4/-4) partition/migrate_grub.go (+1/-1) snappy/auth.go (+1/-1) snappy/click.go (+1/-1) snappy/firstboot.go (+1/-1) snappy/hwaccess.go (+3/-3) |
To merge this branch: | bzr merge lp:~chipaca/snappy/atomic-follow-symlinks |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Michael Vogt (community) | Approve | ||
Review via email: mp+275310@code.launchpad.net |
Commit message
Make atomic follow symlinks (so you don't have to write to /etc/writable).
Description of the change
Snappy Tarmac (snappydevtarmac) wrote : | # |
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.
update github.
github.
update github.
github.
update github.
github.
update gopkg.in/check.v1 failed; trying to fetch newer version
github.
update github.
gopkg.in/check.v1 now at 64131543e7896d5
update github.
github.
update gopkg.in/tomb.v2 failed; trying to fetch newer version
github.
update code.google.
gopkg.in/tomb.v2 now at 14b3d72120e8d10
update github.
code.google.
update github.
github.
update github.
github.
update github.
github.
update golang.org/x/crypto failed; trying to fetch newer version
github.
update gopkg.in/yaml.v2 failed; trying to fetch newer version
golang.org/x/crypto now at 60052bd85f2d912
gopkg.in/yaml.v2 now at 49c95bdc2184325
Building
Running tests from /tmp/tmp.
? launchpad.
=== RUN Test
OK: 12 passed
--- PASS: Test (0.04 seconds)
PASS
coverage: 21.2% of statements
ok launchpad.
=== RUN Test
OK: 53 passed
--- PASS: Test (0.35 seconds)
PASS
coverage: 91.7% of statements
ok launchpad.
=== RUN Test
OK: 39 passed
--- PASS: Test (0.16 seconds)
PASS
coverage: 69.8% of statements
ok launchpad.
? launchpad.
=== RUN Test
OK: 64 passed
--- PASS: Test (0.36 seconds)
PASS
coverage: 79.2% of statements
ok launchpad.
=== RUN Test
OK: 0 passed, 2 ski...
- 799. By John Lenton
-
make the follow symlink behaviour depend on a flag. Oh, add a flag.
Preview Diff
1 | === modified file 'coreconfig/config.go' |
2 | --- coreconfig/config.go 2015-10-22 10:44:15 +0000 |
3 | +++ coreconfig/config.go 2015-10-22 12:45:46 +0000 |
4 | @@ -39,12 +39,12 @@ |
5 | |
6 | const ( |
7 | tzPathEnvironment string = "UBUNTU_CORE_CONFIG_TZ_FILE" |
8 | - tzPathDefault string = "/etc/writable/timezone" |
9 | + tzPathDefault string = "/etc/timezone" |
10 | ) |
11 | |
12 | var ( |
13 | tzZoneInfoPath = "/usr/share/zoneinfo" |
14 | - tzZoneInfoTarget = "/etc/writable/localtime" |
15 | + tzZoneInfoTarget = "/etc/localtime" |
16 | ) |
17 | |
18 | const ( |
19 | @@ -58,8 +58,8 @@ |
20 | modulesPath = "/etc/modules-load.d/ubuntu-core.conf" |
21 | interfacesRoot = "/etc/network/interfaces.d/" |
22 | pppRoot = "/etc/ppp/" |
23 | - watchdogConfigPath = "/etc/writable/watchdog.conf" |
24 | - watchdogStartupPath = "/etc/writable/default/watchdog" |
25 | + watchdogConfigPath = "/etc/watchdog.conf" |
26 | + watchdogStartupPath = "/etc/default/watchdog" |
27 | ) |
28 | |
29 | var ( |
30 | @@ -311,7 +311,7 @@ |
31 | return err |
32 | } |
33 | |
34 | - return helpers.AtomicWriteFile(tzFile(), []byte(timezone), 0644) |
35 | + return helpers.AtomicWriteFile(tzFile(), []byte(timezone), 0644, helpers.AtomicWriteFollow) |
36 | } |
37 | |
38 | func getPassthrough(rootDir string) (pc []passthroughConfig, err error) { |
39 | @@ -340,7 +340,7 @@ |
40 | os.Remove(path) |
41 | continue |
42 | } |
43 | - if err := helpers.AtomicWriteFile(path, []byte(c.Content), 0644); err != nil { |
44 | + if err := helpers.AtomicWriteFile(path, []byte(c.Content), 0644, helpers.AtomicWriteFollow); err != nil { |
45 | return err |
46 | } |
47 | } |
48 | @@ -376,7 +376,7 @@ |
49 | |
50 | // setModprobe sets the specified modprobe config |
51 | var setModprobe = func(modprobe string) error { |
52 | - return helpers.AtomicWriteFile(modprobePath, []byte(modprobe), 0644) |
53 | + return helpers.AtomicWriteFile(modprobePath, []byte(modprobe), 0644, helpers.AtomicWriteFollow) |
54 | } |
55 | |
56 | func getModules() ([]string, error) { |
57 | @@ -463,7 +463,7 @@ |
58 | buf.WriteByte('\n') |
59 | } |
60 | |
61 | - return helpers.AtomicWriteFile(modulesPath, buf.Bytes(), 0644) |
62 | + return helpers.AtomicWriteFile(modulesPath, buf.Bytes(), 0644, helpers.AtomicWriteFollow) |
63 | } |
64 | |
65 | // getWatchdog returns the current watchdog config |
66 | @@ -489,11 +489,11 @@ |
67 | |
68 | // setWatchdog sets the specified watchdog config |
69 | var setWatchdog = func(wf *watchdogConfig) error { |
70 | - if err := helpers.AtomicWriteFile(watchdogStartupPath, []byte(wf.Startup), 0644); err != nil { |
71 | + if err := helpers.AtomicWriteFile(watchdogStartupPath, []byte(wf.Startup), 0644, helpers.AtomicWriteFollow); err != nil { |
72 | return err |
73 | } |
74 | |
75 | - return helpers.AtomicWriteFile(watchdogConfigPath, []byte(wf.Config), 0644) |
76 | + return helpers.AtomicWriteFile(watchdogConfigPath, []byte(wf.Config), 0644, helpers.AtomicWriteFollow) |
77 | } |
78 | |
79 | // for testing purposes |
80 | @@ -568,5 +568,5 @@ |
81 | return err |
82 | } |
83 | |
84 | - return helpers.AtomicWriteFile(hostnamePath, hostnameB, 0644) |
85 | + return helpers.AtomicWriteFile(hostnamePath, hostnameB, 0644, helpers.AtomicWriteFollow) |
86 | } |
87 | |
88 | === modified file 'helpers/helpers.go' |
89 | --- helpers/helpers.go 2015-10-15 07:32:34 +0000 |
90 | +++ helpers/helpers.go 2015-10-22 12:45:46 +0000 |
91 | @@ -292,12 +292,29 @@ |
92 | return string(bs) |
93 | } |
94 | |
95 | +// AtomicWriteFlags are a bitfield of flags for AtomicWriteFile |
96 | +type AtomicWriteFlags uint |
97 | + |
98 | +const ( |
99 | + // AtomicWriteFollow makes AtomicWriteFile follows symlinks |
100 | + AtomicWriteFollow AtomicWriteFlags = 1 << iota |
101 | +) |
102 | + |
103 | // AtomicWriteFile updates the filename atomically and works otherwise |
104 | // like io/ioutil.WriteFile() |
105 | // |
106 | // Note that it won't follow symlinks and will replace existing symlinks |
107 | // with the real file |
108 | -func AtomicWriteFile(filename string, data []byte, perm os.FileMode) (err error) { |
109 | +func AtomicWriteFile(filename string, data []byte, perm os.FileMode, flags AtomicWriteFlags) (err error) { |
110 | + if flags&AtomicWriteFollow != 0 { |
111 | + if fn, err := os.Readlink(filename); err == nil || (fn != "" && os.IsNotExist(err)) { |
112 | + if filepath.IsAbs(fn) { |
113 | + filename = fn |
114 | + } else { |
115 | + filename = filepath.Join(filepath.Dir(filename), fn) |
116 | + } |
117 | + } |
118 | + } |
119 | tmp := filename + "." + MakeRandomString(12) |
120 | |
121 | // XXX: if go switches to use aio_fsync, we need to open the dir for writing |
122 | |
123 | === modified file 'helpers/helpers_test.go' |
124 | --- helpers/helpers_test.go 2015-10-19 10:38:22 +0000 |
125 | +++ helpers/helpers_test.go 2015-10-22 12:45:46 +0000 |
126 | @@ -241,7 +241,7 @@ |
127 | tmpdir := c.MkDir() |
128 | |
129 | p := filepath.Join(tmpdir, "foo") |
130 | - err := AtomicWriteFile(p, []byte("canary"), 0644) |
131 | + err := AtomicWriteFile(p, []byte("canary"), 0644, 0) |
132 | c.Assert(err, IsNil) |
133 | |
134 | content, err := ioutil.ReadFile(p) |
135 | @@ -258,7 +258,7 @@ |
136 | tmpdir := c.MkDir() |
137 | |
138 | p := filepath.Join(tmpdir, "foo") |
139 | - err := AtomicWriteFile(p, []byte(""), 0600) |
140 | + err := AtomicWriteFile(p, []byte(""), 0600, 0) |
141 | c.Assert(err, IsNil) |
142 | |
143 | st, err := os.Stat(p) |
144 | @@ -266,6 +266,102 @@ |
145 | c.Assert(st.Mode()&os.ModePerm, Equals, os.FileMode(0600)) |
146 | } |
147 | |
148 | +func (ts *HTestSuite) TestAtomicWriteFileOverwrite(c *C) { |
149 | + tmpdir := c.MkDir() |
150 | + p := filepath.Join(tmpdir, "foo") |
151 | + c.Assert(ioutil.WriteFile(p, []byte("hello"), 0644), IsNil) |
152 | + c.Assert(AtomicWriteFile(p, []byte("hi"), 0600, 0), IsNil) |
153 | + |
154 | + content, err := ioutil.ReadFile(p) |
155 | + c.Assert(err, IsNil) |
156 | + c.Assert(string(content), Equals, "hi") |
157 | +} |
158 | + |
159 | +func (ts *HTestSuite) TestAtomicWriteFileSymlinkNoFollow(c *C) { |
160 | + tmpdir := c.MkDir() |
161 | + rodir := filepath.Join(tmpdir, "ro") |
162 | + p := filepath.Join(rodir, "foo") |
163 | + s := filepath.Join(tmpdir, "foo") |
164 | + c.Assert(os.MkdirAll(rodir, 0755), IsNil) |
165 | + c.Assert(os.Symlink(s, p), IsNil) |
166 | + c.Assert(os.Chmod(rodir, 0500), IsNil) |
167 | + defer os.Chmod(rodir, 0700) |
168 | + |
169 | + err := AtomicWriteFile(p, []byte("hi"), 0600, 0) |
170 | + c.Assert(err, NotNil) |
171 | +} |
172 | + |
173 | +func (ts *HTestSuite) TestAtomicWriteFileAbsoluteSymlinks(c *C) { |
174 | + tmpdir := c.MkDir() |
175 | + rodir := filepath.Join(tmpdir, "ro") |
176 | + p := filepath.Join(rodir, "foo") |
177 | + s := filepath.Join(tmpdir, "foo") |
178 | + c.Assert(os.MkdirAll(rodir, 0755), IsNil) |
179 | + c.Assert(os.Symlink(s, p), IsNil) |
180 | + c.Assert(os.Chmod(rodir, 0500), IsNil) |
181 | + defer os.Chmod(rodir, 0700) |
182 | + |
183 | + err := AtomicWriteFile(p, []byte("hi"), 0600, AtomicWriteFollow) |
184 | + c.Assert(err, IsNil) |
185 | + |
186 | + content, err := ioutil.ReadFile(p) |
187 | + c.Assert(err, IsNil) |
188 | + c.Assert(string(content), Equals, "hi") |
189 | +} |
190 | + |
191 | +func (ts *HTestSuite) TestAtomicWriteFileOverwriteAbsoluteSymlink(c *C) { |
192 | + tmpdir := c.MkDir() |
193 | + rodir := filepath.Join(tmpdir, "ro") |
194 | + p := filepath.Join(rodir, "foo") |
195 | + s := filepath.Join(tmpdir, "foo") |
196 | + c.Assert(os.MkdirAll(rodir, 0755), IsNil) |
197 | + c.Assert(os.Symlink(s, p), IsNil) |
198 | + c.Assert(os.Chmod(rodir, 0500), IsNil) |
199 | + defer os.Chmod(rodir, 0700) |
200 | + |
201 | + c.Assert(ioutil.WriteFile(s, []byte("hello"), 0644), IsNil) |
202 | + c.Assert(AtomicWriteFile(p, []byte("hi"), 0600, AtomicWriteFollow), IsNil) |
203 | + |
204 | + content, err := ioutil.ReadFile(p) |
205 | + c.Assert(err, IsNil) |
206 | + c.Assert(string(content), Equals, "hi") |
207 | +} |
208 | + |
209 | +func (ts *HTestSuite) TestAtomicWriteFileRelativeSymlinks(c *C) { |
210 | + tmpdir := c.MkDir() |
211 | + rodir := filepath.Join(tmpdir, "ro") |
212 | + p := filepath.Join(rodir, "foo") |
213 | + c.Assert(os.MkdirAll(rodir, 0755), IsNil) |
214 | + c.Assert(os.Symlink("../foo", p), IsNil) |
215 | + c.Assert(os.Chmod(rodir, 0500), IsNil) |
216 | + defer os.Chmod(rodir, 0700) |
217 | + |
218 | + err := AtomicWriteFile(p, []byte("hi"), 0600, AtomicWriteFollow) |
219 | + c.Assert(err, IsNil) |
220 | + |
221 | + content, err := ioutil.ReadFile(p) |
222 | + c.Assert(err, IsNil) |
223 | + c.Assert(string(content), Equals, "hi") |
224 | +} |
225 | + |
226 | +func (ts *HTestSuite) TestAtomicWriteFileOverwriteRelativeSymlink(c *C) { |
227 | + tmpdir := c.MkDir() |
228 | + rodir := filepath.Join(tmpdir, "ro") |
229 | + p := filepath.Join(rodir, "foo") |
230 | + s := filepath.Join(tmpdir, "foo") |
231 | + c.Assert(os.MkdirAll(rodir, 0755), IsNil) |
232 | + c.Assert(os.Symlink("../foo", p), IsNil) |
233 | + c.Assert(os.Chmod(rodir, 0500), IsNil) |
234 | + defer os.Chmod(rodir, 0700) |
235 | + |
236 | + c.Assert(ioutil.WriteFile(s, []byte("hello"), 0644), IsNil) |
237 | + c.Assert(AtomicWriteFile(p, []byte("hi"), 0600, AtomicWriteFollow), IsNil) |
238 | + |
239 | + content, err := ioutil.ReadFile(p) |
240 | + c.Assert(err, IsNil) |
241 | + c.Assert(string(content), Equals, "hi") |
242 | +} |
243 | + |
244 | func (ts *HTestSuite) TestAtomicWriteFileNoOverwriteTmpExisting(c *C) { |
245 | tmpdir := c.MkDir() |
246 | realMakeRandomString := MakeRandomString |
247 | @@ -280,7 +376,7 @@ |
248 | err := ioutil.WriteFile(p+".4", []byte(""), 0644) |
249 | c.Assert(err, IsNil) |
250 | |
251 | - err = AtomicWriteFile(p, []byte(""), 0600) |
252 | + err = AtomicWriteFile(p, []byte(""), 0600, 0) |
253 | c.Assert(err, ErrorMatches, "open .*: file exists") |
254 | } |
255 | |
256 | |
257 | === modified file 'partition/bootloader_uboot.go' |
258 | --- partition/bootloader_uboot.go 2015-07-24 12:00:01 +0000 |
259 | +++ partition/bootloader_uboot.go 2015-10-22 12:45:46 +0000 |
260 | @@ -261,7 +261,7 @@ |
261 | } |
262 | |
263 | if updateNeeded { |
264 | - return atomicWriteFile(path, buf.Bytes(), 0644) |
265 | + return atomicWriteFile(path, buf.Bytes(), 0644, 0) |
266 | } |
267 | |
268 | return nil |
269 | |
270 | === modified file 'partition/bootloader_uboot_test.go' |
271 | --- partition/bootloader_uboot_test.go 2015-07-24 11:58:04 +0000 |
272 | +++ partition/bootloader_uboot_test.go 2015-10-22 12:45:46 +0000 |
273 | @@ -291,9 +291,9 @@ |
274 | s.makeFakeUbootEnv(c) |
275 | |
276 | atomiCall := false |
277 | - atomicWriteFile = func(a string, b []byte, c os.FileMode) error { |
278 | + atomicWriteFile = func(a string, b []byte, c os.FileMode, f helpers.AtomicWriteFlags) error { |
279 | atomiCall = true |
280 | - return helpers.AtomicWriteFile(a, b, c) |
281 | + return helpers.AtomicWriteFile(a, b, c, f) |
282 | } |
283 | |
284 | partition := New() |
285 | @@ -311,9 +311,9 @@ |
286 | c.Assert(ioutil.WriteFile(bootloaderUbootEnvFile, []byte(""), 0644), IsNil) |
287 | |
288 | atomiCall := false |
289 | - atomicWriteFile = func(a string, b []byte, c os.FileMode) error { |
290 | + atomicWriteFile = func(a string, b []byte, c os.FileMode, f helpers.AtomicWriteFlags) error { |
291 | atomiCall = true |
292 | - return helpers.AtomicWriteFile(a, b, c) |
293 | + return helpers.AtomicWriteFile(a, b, c, f) |
294 | } |
295 | |
296 | partition := New() |
297 | |
298 | === modified file 'partition/migrate_grub.go' |
299 | --- partition/migrate_grub.go 2015-09-16 11:19:03 +0000 |
300 | +++ partition/migrate_grub.go 2015-10-22 12:45:46 +0000 |
301 | @@ -149,5 +149,5 @@ |
302 | } |
303 | } |
304 | |
305 | - return helpers.AtomicWriteFile(bootloaderGrubConfigFile, []byte(newGrubConfig), 0644) |
306 | + return helpers.AtomicWriteFile(bootloaderGrubConfigFile, []byte(newGrubConfig), 0644, 0) |
307 | } |
308 | |
309 | === modified file 'snappy/auth.go' |
310 | --- snappy/auth.go 2015-06-04 22:04:09 +0000 |
311 | +++ snappy/auth.go 2015-10-22 12:45:46 +0000 |
312 | @@ -139,7 +139,7 @@ |
313 | return nil |
314 | } |
315 | |
316 | - return helpers.AtomicWriteFile(targetFile, []byte(outStr), 0600) |
317 | + return helpers.AtomicWriteFile(targetFile, []byte(outStr), 0600, 0) |
318 | } |
319 | |
320 | // ReadStoreToken reads a token previously write via WriteStoreToken |
321 | |
322 | === modified file 'snappy/click.go' |
323 | --- snappy/click.go 2015-10-15 07:32:34 +0000 |
324 | +++ snappy/click.go 2015-10-22 12:45:46 +0000 |
325 | @@ -758,7 +758,7 @@ |
326 | if err != nil { |
327 | return err |
328 | } |
329 | - if err := helpers.AtomicWriteFile(filepath.Join(clickMetaDir, cm.Name+".manifest"), []byte(outStr), 0644); err != nil { |
330 | + if err := helpers.AtomicWriteFile(filepath.Join(clickMetaDir, cm.Name+".manifest"), []byte(outStr), 0644, 0); err != nil { |
331 | return err |
332 | } |
333 | |
334 | |
335 | === modified file 'snappy/firstboot.go' |
336 | --- snappy/firstboot.go 2015-10-22 09:32:32 +0000 |
337 | +++ snappy/firstboot.go 2015-10-22 12:45:46 +0000 |
338 | @@ -135,7 +135,7 @@ |
339 | ethfile := filepath.Join(ethdir, eth) |
340 | data := fmt.Sprintf("allow-hotplug %[1]s\niface %[1]s inet dhcp\n", eth) |
341 | |
342 | - if err := helpers.AtomicWriteFile(ethfile, []byte(data), 0644); err != nil { |
343 | + if err := helpers.AtomicWriteFile(ethfile, []byte(data), 0644, 0); err != nil { |
344 | return err |
345 | } |
346 | |
347 | |
348 | === modified file 'snappy/hwaccess.go' |
349 | --- snappy/hwaccess.go 2015-09-29 07:07:13 +0000 |
350 | +++ snappy/hwaccess.go 2015-10-22 12:45:46 +0000 |
351 | @@ -91,7 +91,7 @@ |
352 | out = append(out, '\n') |
353 | |
354 | additionalFile := getHWAccessJSONFile(snapname) |
355 | - if err := helpers.AtomicWriteFile(additionalFile, out, 0640); err != nil { |
356 | + if err := helpers.AtomicWriteFile(additionalFile, out, 0640, 0); err != nil { |
357 | return err |
358 | } |
359 | |
360 | @@ -130,7 +130,7 @@ |
361 | // In both cases, updatedRules will have the right content. |
362 | updatedRules := append(rules, newRule...) |
363 | |
364 | - if err := helpers.AtomicWriteFile(udevRulesFile, updatedRules, 0644); nil != err { |
365 | + if err := helpers.AtomicWriteFile(udevRulesFile, updatedRules, 0644, 0); nil != err { |
366 | return err |
367 | } |
368 | |
369 | @@ -250,7 +250,7 @@ |
370 | out = out + rule + "\n" |
371 | } |
372 | |
373 | - if err := helpers.AtomicWriteFile(udevRulesFile, []byte(out), 0644); nil != err { |
374 | + if err := helpers.AtomicWriteFile(udevRulesFile, []byte(out), 0644, 0); nil != err { |
375 | return err |
376 | } |
377 | } else { |
Thanks!