From 1f205c0aec5ea9e983d61a64e7ce871ae416bebd Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Tue, 18 Oct 2016 02:16:46 -0700 Subject: [PATCH 1/2] image/manifest: Recursively remove pre-existing entries when unpacking Implementing the logic that is in-flight with [1], but using recursive removal [2]. GNU tar has a --recursive-unlink option that's not enabled by default, with the motivation being something like "folks would be mad if we blew away a full tree and replaced it with a broken symlink" [3]. That makes sense for working filesystems, but we're building the rootfs from scratch here so losing information is not a concern. This commit always uses recursive removal to get that old thing off the filesystem (whatever it takes ;). The exception to the removal is if both the tar entry and existing path occupant are directories. In this case we want to use GNU tar's default --overwrite-dir behavior, but unpackLayer's metadata handling is currently very weak so I've left it at "don't delete the old directory". The reworked directory case also fixes a minor bug from 44210d05 (cmd/oci-image-tool: fix unpacking..., 2016-07-22, #177) where the: if fi, err := os.Lstat(path); !(err == nil && fi.IsDir()) { block would not error out if the Lstat failed for a reason besides the acceptable IsNotExist. Instead, it would attempt to call MkdirAll, which would probably fail for the same reason that Lstat failed (e.g. ENOTDIR). But it's better to handle the Lstat errors directly. [1]: https://github.com/opencontainers/image-spec/pull/317 [2]: https://github.com/opencontainers/image-spec/pull/317/files#r79214718 [3]: https://www.gnu.org/software/tar/manual/html_node/Dealing-with-Old-Files.html Signed-off-by: W. Trevor King --- image/manifest.go | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/image/manifest.go b/image/manifest.go index 8834c1e5f2f0..144bd4f62219 100644 --- a/src/import/image/manifest.go +++ b/src/import/image/manifest.go @@ -253,11 +253,27 @@ loop: continue loop } + if hdr.Typeflag != tar.TypeDir { + err = os.RemoveAll(path) + if err != nil && !os.IsNotExist(err) { + return err + } + } + switch hdr.Typeflag { case tar.TypeDir: - if fi, err := os.Lstat(path); !(err == nil && fi.IsDir()) { - if err2 := os.MkdirAll(path, info.Mode()); err2 != nil { - return errors.Wrap(err2, "error creating directory") + fi, err := os.Lstat(path) + if err != nil && !os.IsNotExist(err) { + return err + } + if os.IsNotExist(err) || !fi.IsDir() { + err = os.RemoveAll(path) + if err != nil && !os.IsNotExist(err) { + return err + } + err = os.MkdirAll(path, info.Mode()) + if err != nil { + return err } } -- 2.4.0.53.g8440f74