Brad Bishop | d7bf8c1 | 2018-02-25 22:55:05 -0500 | [diff] [blame] | 1 | From 1f205c0aec5ea9e983d61a64e7ce871ae416bebd Mon Sep 17 00:00:00 2001 |
| 2 | From: "W. Trevor King" <wking@tremily.us> |
| 3 | Date: Tue, 18 Oct 2016 02:16:46 -0700 |
| 4 | Subject: [PATCH 1/2] image/manifest: Recursively remove pre-existing entries |
| 5 | when unpacking |
| 6 | |
| 7 | Implementing the logic that is in-flight with [1], but using recursive |
| 8 | removal [2]. GNU tar has a --recursive-unlink option that's not |
| 9 | enabled by default, with the motivation being something like "folks |
| 10 | would be mad if we blew away a full tree and replaced it with a broken |
| 11 | symlink" [3]. That makes sense for working filesystems, but we're |
| 12 | building the rootfs from scratch here so losing information is not a |
| 13 | concern. This commit always uses recursive removal to get that old |
| 14 | thing off the filesystem (whatever it takes ;). |
| 15 | |
| 16 | The exception to the removal is if both the tar entry and existing |
| 17 | path occupant are directories. In this case we want to use GNU tar's |
| 18 | default --overwrite-dir behavior, but unpackLayer's metadata handling |
| 19 | is currently very weak so I've left it at "don't delete the old |
| 20 | directory". |
| 21 | |
| 22 | The 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 | |
| 27 | block would not error out if the Lstat failed for a reason besides the |
| 28 | acceptable IsNotExist. Instead, it would attempt to call MkdirAll, |
| 29 | which 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 | |
| 36 | Signed-off-by: W. Trevor King <wking@tremily.us> |
| 37 | --- |
| 38 | image/manifest.go | 22 +++++++++++++++++++--- |
| 39 | 1 file changed, 19 insertions(+), 3 deletions(-) |
| 40 | |
| 41 | diff --git a/image/manifest.go b/image/manifest.go |
| 42 | index 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 | -- |
| 77 | 2.4.0.53.g8440f74 |
| 78 | |