*: various whiteout improvements

CHANGELOG.md

@@ -12,6 +12,20 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - Added `umoci unpack --keep-dirlinks` (in the same vein as rsync's flag with
   the same name) which allows layers that contain entries which have a symlink
   as a path component. openSUSE/umoci#246
+- `umoci insert` now supports whiteouts in two significant ways. You can use
+  `--whiteout` to "insert" a deletion of a given path, while you can use
+  `--opaque` to replace a directory by adding an opaque whiteout (the default
+  behaviour causes the old and new directories to be merged).
+  openSUSE/umoci#257
+
+## Fixed
+- Docker has changed how they handle whiteouts for non-existent files. The
+  specification is loose on this (and in umoci we've always been liberal with
+  whiteout generation -- to avoid cases where someone was confused we didn't
+  have a whiteout for every entry). But now that they have deviated from the
+  spec, in the interest of playing nice, we can just follow their new
+  restriction (even though it is not supported by the spec). This also makes
+  our layers *slightly* smaller. openSUSE/umoci#254
 
 ## [0.4.1] - 2018-08-16
 ### Added

cmd/umoci/insert.go

@@ -36,45 +36,84 @@ import (
 
 var insertCommand = uxRemap(uxHistory(cli.Command{
 	Name:  "insert",
-	Usage: "insert a file into an OCI image",
-	ArgsUsage: `--image <image-path>[:<tag>] <file> <path>
+	Usage: "insert content into an OCI image",
+	ArgsUsage: `--image <image-path>[:<tag>] [--opaque] <source> <target>
+                                  --image <image-path>[:<tag>] [--whiteout] <target>
 
-Where "<image-path>" is the path to the OCI image, "<tag>" is the name of the
-tag that the content wil be inserted into (if not specified, defaults to
-"latest"), "<file>" is the file or folder to insert, and "<path>" is the full
-name of the path to that the file should be inserted at. Insert is
-automatically recursive if the source is a directory.
+Where "<image-path>" is the path to the OCI image, and "<tag>" is the name of
+the tag that the content wil be inserted into (if not specified, defaults to
+"latest").
 
-For example:
+The path at "<source>" is added to the image with the given "<target>" name.
+If "--whiteout" is specified, rather than inserting content into the image, a
+removal entry for "<target>" is inserted instead.
+
+If "--opaque" is specified then any paths below "<target>" (assuming it is a
+directory) from previous layers will no longer be present. Only the contents
+inserted by this command will be visible. This can be used to replace an entire
+directory, while the default behaviour merges the old contents with the new.
+
+Note that this command works by creating a new layer, so this should not be
+used to remove (or replace) secrets from an already-built image. See
+umoci-config(1) and --config.volume for how to achieve this correctly.
+
+Some examples:
 	umoci insert --image oci:foo mybinary /usr/bin/mybinary
 	umoci insert --image oci:foo myconfigdir /etc/myconfigdir
+	umoci insert --image oci:foo --opaque myoptdir /opt
+	umoci insert --image oci:foo --whiteout /some/old/dir
 `,
 
 	Category: "image",
 
 	Action: insert,
 
+	Flags: []cli.Flag{
+		cli.BoolFlag{
+			Name:  "whiteout",
+			Usage: "insert a 'removal entry' for the given path",
+		},
+		cli.BoolFlag{
+			Name:  "opaque",
+			Usage: "mask any previous entries in the target directory",
+		},
+	},
+
 	Before: func(ctx *cli.Context) error {
-		if ctx.NArg() != 2 {
-			return errors.Errorf("invalid number of positional arguments: expected <file> and <path>")
+		// This command is quite weird because we need to support two different
+		// positional-argument numbers. Awesome.
+		numArgs := 2
+		if ctx.IsSet("whiteout") {
+			numArgs = 1
+		}
+		if ctx.NArg() != numArgs {
+			return errors.Errorf("invalid number of positional arguments: expected %d", numArgs)
 		}
-		if ctx.Args()[0] == "" {
-			return errors.Errorf("<file> cannot be empty")
+		for idx, args := range ctx.Args() {
+			if args == "" {
+				return errors.Errorf("invalid positional argument %d: arguments cannot be empty", idx)
+			}
 		}
-		ctx.App.Metadata["insertFile"] = ctx.Args()[0]
 
-		if ctx.Args()[1] == "" {
-			return errors.Errorf("<path> cannot be empty")
+		// Figure out the arguments.
+		var sourcePath, targetPath string
+		targetPath = ctx.Args()[0]
+		if !ctx.IsSet("whiteout") {
+			sourcePath = targetPath
+			targetPath = ctx.Args()[1]
 		}
-		ctx.App.Metadata["insertPath"] = ctx.Args()[1]
 
+		ctx.App.Metadata["--source-path"] = sourcePath
+		ctx.App.Metadata["--target-path"] = targetPath
 		return nil
 	},
 }))
 
 func insert(ctx *cli.Context) error {
 	imagePath := ctx.App.Metadata["--image-path"].(string)
 	tagName := ctx.App.Metadata["--image-tag"].(string)
+	sourcePath := ctx.App.Metadata["--source-path"].(string)
+	targetPath := ctx.App.Metadata["--target-path"].(string)
 
 	// Get a reference to the CAS.
 	engine, err := dir.Open(imagePath)
@@ -102,9 +141,6 @@ func insert(ctx *cli.Context) error {
 		return errors.Wrap(err, "create mutator for base image")
 	}
 
-	insertFile := ctx.App.Metadata["insertFile"].(string)
-	insertPath := ctx.App.Metadata["insertPath"].(string)
-
 	var meta umoci.Meta
 	meta.Version = umoci.MetaVersion
 
@@ -114,7 +150,7 @@ func insert(ctx *cli.Context) error {
 		return err
 	}
 
-	reader := layer.GenerateInsertLayer(insertFile, insertPath, &meta.MapOptions)
+	reader := layer.GenerateInsertLayer(sourcePath, targetPath, ctx.IsSet("opaque"), &meta.MapOptions)
 	defer reader.Close()
 
 	created := time.Now()

cmd/umoci/repack.go

@@ -184,7 +184,9 @@ func repack(ctx *cli.Context) error {
 			maskedPaths = append(maskedPaths, v)
 		}
 	}
-	diffs = mtreefilter.FilterDeltas(diffs, mtreefilter.MaskFilter(maskedPaths))
+	diffs = mtreefilter.FilterDeltas(diffs,
+		mtreefilter.MaskFilter(maskedPaths),
+		mtreefilter.SimplifyFilter(diffs))
 
 	reader, err := layer.GenerateLayer(fullRootfsPath, diffs, &meta.MapOptions)
 	if err != nil {

doc/man/umoci-insert.1.md

@@ -1,32 +1,46 @@
-% umoci-insert(1) # umoci insert - insert a file into an OCI image
+% umoci-insert(1) # umoci insert - insert content into an OCI image
 % Aleksa Sarai
 % SEPTEMBER 2018
 # NAME
-umoci insert - insert a file into an OCI image
+umoci insert - insert content into an OCI image
 
 # SYNOPSIS
 **umoci insert**
 **--image**=*image*[:*tag*]
+[**--opaque**]
 [**--rootless**]
 [**--uid-map**=*value*]
 [**--uid-map**=*value*]
 [**--history.comment**=*comment*]
 [**--history.created_by**=*created_by*]
 [**--history.author**=*author*]
 [**--history-created**=*date*]
-*file*
-*path*
+*source*
+*target*
 
-# DESCRIPTION
-Creates a new OCI image layout. The new OCI image does not contain any new
-references or blobs, but those can be created through the use of
-**umoci-new**(1), **umoci-tag**(1), **umoci-repack**(1) and other similar
-commands.
+**umoci insert**
+[options]
+**--whiteout**
+*target*
 
-Inserts *file* into the OCI image given by **--image** (overwriting it),
-creating a new layer containing just the contents of *file* at *path*. *file*
-can be a file or a directory to insert (in the latter case the directory is
-always recursed), and *path* is the full path where *file* will be inserted.
+
+# DESCRIPTION
+In the first form, insert the contents of *source* into the OCI image given by
+**--image** (**overwriting it** -- this is the only **umoci**(1) command which
+currently does this). This is done by creating a new layer containing just the
+contents of *source* with a name of *target*. *source* can be either a file or
+directory, and in the latter case it will be recursed. If **--opaque** is
+specified then any paths below *target* in the previous image layers (assuming
+*target* is a directory) will be removed.
+
+In the second form, inserts a "deletion entry" into the OCI image for *target*
+inside the image. This is done by inserting a layer containing just a whiteout
+entry for the given path.
+
+Note that this command works by creating a new layer, so this should not be
+used to remove (or replace) secrets from an already-built image. See
+**umoci-config**(1) and **--config.volume** for how to achieve this correctly
+by not creating image layers with secrets in the first place.
 
 # OPTIONS
 The global options are defined in **umoci**(1).
@@ -37,6 +51,17 @@ The global options are defined in **umoci**(1).
   must be a valid tag in the image. If *tag* is not provided it defaults to
   "latest".
 
+**--opaque**
+  (Assuming *target* is a directory.) Add an opaque whiteout entry for *target*
+  so that any child path of *target* in previous layers is masked by the new
+  entry for *target*, which will just contain the contents of *source*. This
+  allows for the complete replacement of a directory, as opposed to the merging
+  of directory entries.
+
+**--whiteout**
+  Add a deletion entry for *target*, so that it is not present in future
+  extractions of the image.
+
 **--rootless**
   Enable rootless insertion support. This allows for **umoci-insert**(1) to be
   used as an unprivileged user. Use of this flag implies **--uid-map=0:$(id
@@ -85,5 +110,14 @@ directories are merged (with the newer layer taking precedence).
 % umoci insert --image oci:foo myconfigdir /etc/myconfigdir
 ```
 
+And in these examples we delete `/usr/bin/mybinary` and replace the entirety of
+`/etc` with `myetcdir` (such that none of the old `/etc` entries will be
+present on **umoci-unpack**(1)).
+
+```
+% umoci insert --image oci:foo --whiteout /usr/bin/mybinary
+% umoci insert --image oci:foo --opaque myetcdir /etc
+```
+
 # SEE ALSO
 **umoci**(1), **umoci-repack**(1), **umoci-raw-add-layer**(1)

oci/layer/generate.go

@@ -101,9 +101,10 @@ func GenerateLayer(path string, deltas []mtree.InodeDelta, opt *MapOptions) (io.
 	return reader, nil
 }
 
-// GenerateInsertLayer generates a completely new layer from the root path to
-// be inserted into the image at "target".
-func GenerateInsertLayer(root string, target string, opt *MapOptions) io.ReadCloser {
+// GenerateInsertLayer generates a completely new layer from "root"to be
+// inserted into the image at "target". If "root" is an empty string then the
+// "target" will be removed via a whiteout.
+func GenerateInsertLayer(root string, target string, opaque bool, opt *MapOptions) io.ReadCloser {
 	root = CleanPath(root)
 
 	var mapOptions MapOptions
@@ -113,29 +114,29 @@ func GenerateInsertLayer(root string, target string, opt *MapOptions) io.ReadClo
 
 	reader, writer := io.Pipe()
 
-	go func() {
-		var err error
-
+	go func() (Err error) {
 		defer func() {
-			writer.CloseWithError(errors.Wrap(err, "generate layer"))
+			writer.CloseWithError(errors.Wrap(Err, "generate layer"))
 		}()
 
 		tg := newTarGenerator(writer, mapOptions)
 
-		err = unpriv.Walk(root, func(curPath string, info os.FileInfo, err error) error {
+		if opaque {
+			if err := tg.AddOpaqueWhiteout(target); err != nil {
+				return err
+			}
+		}
+		if root == "" {
+			return tg.AddWhiteout(target)
+		}
+		return unpriv.Walk(root, func(curPath string, info os.FileInfo, err error) error {
 			if err != nil {
 				return err
 			}
 
 			pathInTar := path.Join(target, curPath[len(root):])
 			return tg.AddFile(pathInTar, curPath)
 		})
-		if err != nil {
-			return
-		}
-
-		err = tg.tw.Close()
 	}()
-
 	return reader
 }

oci/layer/tar_generate.go

@@ -23,7 +23,6 @@ import (
 	"os"
 	"path/filepath"
 	"strings"
-	"time"
 
 	"github.com/apex/log"
 	"github.com/openSUSE/umoci/pkg/fseval"
@@ -262,34 +261,41 @@ const whPrefix = ".wh."
 // siblings in a directory are to be dropped in the "lower" layer.
 const whOpaque = whPrefix + whPrefix + ".opq"
 
-// AddWhiteout adds a whiteout file for the given name inside the tar archive.
-// It's not recommended to add a file with AddFile and then white it out.
-func (tg *tarGenerator) AddWhiteout(name string) error {
+// addWhiteout adds a whiteout file for the given name inside the tar archive.
+// It's not recommended to add a file with AddFile and then white it out. If
+// you specify opaque, then the whiteout created is an opaque whiteout *for the
+// directory path* given.
+func (tg *tarGenerator) addWhiteout(name string, opaque bool) error {
 	name, err := normalise(name, false)
 	if err != nil {
 		return errors.Wrap(err, "normalise path")
 	}
 
-	// Create the explicit whiteout for the file.
-	dir, file := filepath.Split(name)
-	whiteout := filepath.Join(dir, whPrefix+file)
-	timestamp := time.Now()
-
 	// Disallow having a whiteout of a whiteout, purely for our own sanity.
+	dir, file := filepath.Split(name)
 	if strings.HasPrefix(file, whPrefix) {
 		return errors.Errorf("invalid path has whiteout prefix %q: %s", whPrefix, name)
 	}
 
-	// Add a dummy header for the whiteout file.
-	if err := tg.tw.WriteHeader(&tar.Header{
-		Name:       whiteout,
-		Size:       0,
-		ModTime:    timestamp,
-		AccessTime: timestamp,
-		ChangeTime: timestamp,
-	}); err != nil {
-		return errors.Wrap(err, "write whiteout header")
+	// Figure out the whiteout name.
+	whiteout := filepath.Join(dir, whPrefix+file)
+	if opaque {
+		whiteout = filepath.Join(name, whOpaque)
 	}
 
-	return nil
+	// Add a dummy header for the whiteout file.
+	return errors.Wrap(tg.tw.WriteHeader(&tar.Header{
+		Name: whiteout,
+		Size: 0,
+	}), "write whiteout header")
+}
+
+// AddWhiteout creates a whiteout for the provided path.
+func (tg *tarGenerator) AddWhiteout(name string) error {
+	return tg.addWhiteout(name, false)
+}
+
+// AddOpaqueWhiteout creates a whiteout for the provided path.
+func (tg *tarGenerator) AddOpaqueWhiteout(name string) error {
+	return tg.addWhiteout(name, true)
 }