blob: 5594f9762036c8fb7a96dc346035b8ae47cf2356 [file] [log] [blame]
Brad Bishopd7bf8c12018-02-25 22:55:05 -05001From 1f205c0aec5ea9e983d61a64e7ce871ae416bebd Mon Sep 17 00:00:00 2001
2From: "W. Trevor King" <wking@tremily.us>
3Date: Tue, 18 Oct 2016 02:16:46 -0700
4Subject: [PATCH 1/2] image/manifest: Recursively remove pre-existing entries
5 when unpacking
6
7Implementing the logic that is in-flight with [1], but using recursive
8removal [2]. GNU tar has a --recursive-unlink option that's not
9enabled by default, with the motivation being something like "folks
10would be mad if we blew away a full tree and replaced it with a broken
11symlink" [3]. That makes sense for working filesystems, but we're
12building the rootfs from scratch here so losing information is not a
13concern. This commit always uses recursive removal to get that old
14thing off the filesystem (whatever it takes ;).
15
16The exception to the removal is if both the tar entry and existing
17path occupant are directories. In this case we want to use GNU tar's
18default --overwrite-dir behavior, but unpackLayer's metadata handling
19is currently very weak so I've left it at "don't delete the old
20directory".
21
22The reworked directory case also fixes a minor bug from 44210d05
23(cmd/oci-image-tool: fix unpacking..., 2016-07-22, #177) where the:
24
25 if fi, err := os.Lstat(path); !(err == nil && fi.IsDir()) {
26
27block would not error out if the Lstat failed for a reason besides the
28acceptable IsNotExist. Instead, it would attempt to call MkdirAll,
29which would probably fail for the same reason that Lstat failed
30(e.g. ENOTDIR). But it's better to handle the Lstat errors directly.
31
32[1]: https://github.com/opencontainers/image-spec/pull/317
33[2]: https://github.com/opencontainers/image-spec/pull/317/files#r79214718
34[3]: https://www.gnu.org/software/tar/manual/html_node/Dealing-with-Old-Files.html
35
36Signed-off-by: W. Trevor King <wking@tremily.us>
37---
38 image/manifest.go | 22 +++++++++++++++++++---
39 1 file changed, 19 insertions(+), 3 deletions(-)
40
41diff --git a/image/manifest.go b/image/manifest.go
42index 8834c1e5f2f0..144bd4f62219 100644
43--- a/src/import/image/manifest.go
44+++ b/src/import/image/manifest.go
45@@ -253,11 +253,27 @@ loop:
46 continue loop
47 }
48
49+ if hdr.Typeflag != tar.TypeDir {
50+ err = os.RemoveAll(path)
51+ if err != nil && !os.IsNotExist(err) {
52+ return err
53+ }
54+ }
55+
56 switch hdr.Typeflag {
57 case tar.TypeDir:
58- if fi, err := os.Lstat(path); !(err == nil && fi.IsDir()) {
59- if err2 := os.MkdirAll(path, info.Mode()); err2 != nil {
60- return errors.Wrap(err2, "error creating directory")
61+ fi, err := os.Lstat(path)
62+ if err != nil && !os.IsNotExist(err) {
63+ return err
64+ }
65+ if os.IsNotExist(err) || !fi.IsDir() {
66+ err = os.RemoveAll(path)
67+ if err != nil && !os.IsNotExist(err) {
68+ return err
69+ }
70+ err = os.MkdirAll(path, info.Mode())
71+ if err != nil {
72+ return err
73 }
74 }
75
76--
772.4.0.53.g8440f74
78