image/manifest: Split unpackLayerEntry into its own function

To help address: $ make lint checking lint image/manifest.go:140::warning: cyclomatic complexity 39 of function unpackLayer() is high (> 35) (gocyclo) ... Signed-off-by: W. Trevor King <wking@tremily.us>
opencontainers · Oct 21, 2016 · bb40ed1 · bb40ed1
1 parent 6055f4d
commit bb40ed1
Showing 1 changed file with 100 additions and 85 deletions.
diff --git a/image/manifest.go b/image/manifest.go
@@ -160,116 +160,131 @@ loop:
 			return errors.Wrapf(err, "error advancing tar stream")
 		}
 
-		hdr.Name = filepath.Clean(hdr.Name)
-		if !strings.HasSuffix(hdr.Name, string(os.PathSeparator)) {
-			// Not the root directory, ensure that the parent directory exists
-			parent := filepath.Dir(hdr.Name)
-			parentPath := filepath.Join(dest, parent)
-			if _, err2 := os.Lstat(parentPath); err2 != nil && os.IsNotExist(err2) {
-				if err3 := os.MkdirAll(parentPath, 0755); err3 != nil {
-					return err3
-				}
-			}
-		}
-		path := filepath.Join(dest, hdr.Name)
-		if entries[path] {
-			return fmt.Errorf("duplicate entry for %s", path)
-		}
-		entries[path] = true
-		rel, err := filepath.Rel(dest, path)
+		var whiteout bool
+		whiteout, err = unpackLayerEntry(dest, hdr, tr, &entries)
 		if err != nil {
 			return err
 		}
-		info := hdr.FileInfo()
-		if strings.HasPrefix(rel, ".."+string(os.PathSeparator)) {
-			return fmt.Errorf("%q is outside of %q", hdr.Name, dest)
+		if whiteout {
+			continue loop
 		}
 
-		if strings.HasPrefix(info.Name(), ".wh.") {
-			path = strings.Replace(path, ".wh.", "", 1)
+		// Directory mtimes must be handled at the end to avoid further
+		// file creation in them to modify the directory mtime
+		if hdr.Typeflag == tar.TypeDir {
+			dirs = append(dirs, hdr)
+		}
+	}
+	for _, hdr := range dirs {
+		path := filepath.Join(dest, hdr.Name)
 
-			if err := os.RemoveAll(path); err != nil {
-				return errors.Wrap(err, "unable to delete whiteout path")
+		finfo := hdr.FileInfo()
+		// I believe the old version was using time.Now().UTC() to overcome an
+		// invalid error from chtimes.....but here we lose hdr.AccessTime like this...
+		if err := os.Chtimes(path, time.Now().UTC(), finfo.ModTime()); err != nil {
+			return errors.Wrap(err, "error changing time")
+		}
+	}
+	return nil
+}
+
+// unpackLayerEntry unpacks a single entry from a layer.
+func unpackLayerEntry(dest string, header *tar.Header, reader io.Reader, entries *map[string]bool) (whiteout bool, err error) {
+	header.Name = filepath.Clean(header.Name)
+	if !strings.HasSuffix(header.Name, string(os.PathSeparator)) {
+		// Not the root directory, ensure that the parent directory exists
+		parent := filepath.Dir(header.Name)
+		parentPath := filepath.Join(dest, parent)
+		if _, err2 := os.Lstat(parentPath); err2 != nil && os.IsNotExist(err2) {
+			if err3 := os.MkdirAll(parentPath, 0755); err3 != nil {
+				return false, err3
 			}
+		}
+	}
+	path := filepath.Join(dest, header.Name)
+	if (*entries)[path] {
+		return false, fmt.Errorf("duplicate entry for %s", path)
+	}
+	(*entries)[path] = true
+	rel, err := filepath.Rel(dest, path)
+	if err != nil {
+		return false, err
+	}
+	info := header.FileInfo()
+	if strings.HasPrefix(rel, ".."+string(os.PathSeparator)) {
+		return false, fmt.Errorf("%q is outside of %q", header.Name, dest)
+	}
 
-			continue loop
+	if strings.HasPrefix(info.Name(), ".wh.") {
+		path = strings.Replace(path, ".wh.", "", 1)
+
+		if err = os.RemoveAll(path); err != nil {
+			return true, errors.Wrap(err, "unable to delete whiteout path")
 		}
 
-		if hdr.Typeflag != tar.TypeDir {
-			err = os.RemoveAll(path)
-			if err != nil && !os.IsNotExist(err) {
-				return err
-			}
+		return true, nil
+	}
+
+	if header.Typeflag != tar.TypeDir {
+		err = os.RemoveAll(path)
+		if err != nil && !os.IsNotExist(err) {
+			return false, err
 		}
+	}
 
-		switch hdr.Typeflag {
-		case tar.TypeDir:
-			fi, err := os.Lstat(path)
+	switch header.Typeflag {
+	case tar.TypeDir:
+		fi, err := os.Lstat(path)
+		if err != nil && !os.IsNotExist(err) {
+			return false, err
+		}
+		if os.IsNotExist(err) || !fi.IsDir() {
+			err = os.RemoveAll(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
-				}
+				return false, err
 			}
-
-		case tar.TypeReg, tar.TypeRegA:
-			f, err := os.OpenFile(path, os.O_CREATE|os.O_WRONLY, info.Mode())
+			err = os.MkdirAll(path, info.Mode())
 			if err != nil {
-				return errors.Wrap(err, "unable to open file")
+				return false, err
 			}
+		}
 
-			if _, err := io.Copy(f, tr); err != nil {
-				f.Close()
-				return errors.Wrap(err, "unable to copy")
-			}
-			f.Close()
+	case tar.TypeReg, tar.TypeRegA:
+		f, err := os.OpenFile(path, os.O_CREATE|os.O_WRONLY, info.Mode())
+		if err != nil {
+			return false, errors.Wrap(err, "unable to open file")
+		}
 
-		case tar.TypeLink:
-			target := filepath.Join(dest, hdr.Linkname)
+		if _, err := io.Copy(f, reader); err != nil {
+			f.Close()
+			return false, errors.Wrap(err, "unable to copy")
+		}
+		f.Close()
 
-			if !strings.HasPrefix(target, dest) {
-				return fmt.Errorf("invalid hardlink %q -> %q", target, hdr.Linkname)
-			}
+	case tar.TypeLink:
+		target := filepath.Join(dest, header.Linkname)
 
-			if err := os.Link(target, path); err != nil {
-				return err
-			}
+		if !strings.HasPrefix(target, dest) {
+			return false, fmt.Errorf("invalid hardlink %q -> %q", target, header.Linkname)
+		}
 
-		case tar.TypeSymlink:
-			target := filepath.Join(filepath.Dir(path), hdr.Linkname)
+		if err := os.Link(target, path); err != nil {
+			return false, err
+		}
 
-			if !strings.HasPrefix(target, dest) {
-				return fmt.Errorf("invalid symlink %q -> %q", path, hdr.Linkname)
-			}
+	case tar.TypeSymlink:
+		target := filepath.Join(filepath.Dir(path), header.Linkname)
 
-			if err := os.Symlink(hdr.Linkname, path); err != nil {
-				return err
-			}
-		case tar.TypeXGlobalHeader:
-			return nil
+		if !strings.HasPrefix(target, dest) {
+			return false, fmt.Errorf("invalid symlink %q -> %q", path, header.Linkname)
 		}
-		// Directory mtimes must be handled at the end to avoid further
-		// file creation in them to modify the directory mtime
-		if hdr.Typeflag == tar.TypeDir {
-			dirs = append(dirs, hdr)
-		}
-	}
-	for _, hdr := range dirs {
-		path := filepath.Join(dest, hdr.Name)
 
-		finfo := hdr.FileInfo()
-		// I believe the old version was using time.Now().UTC() to overcome an
-		// invalid error from chtimes.....but here we lose hdr.AccessTime like this...
-		if err := os.Chtimes(path, time.Now().UTC(), finfo.ModTime()); err != nil {
-			return errors.Wrap(err, "error changing time")
+		if err := os.Symlink(header.Linkname, path); err != nil {
+			return false, err
 		}
+	case tar.TypeXGlobalHeader:
+		return false, nil
 	}
-	return nil
+
+	return false, nil
 }