merge branch 'packing-add-id-mappings'

Closes: cyphar/umoci#32
Signed-off-by: Aleksa Sarai <asarai@suse.com>

cmd/umoci/repack.go

@@ -30,6 +30,7 @@ import (
 	"github.com/cyphar/umoci/image/cas"
 	"github.com/cyphar/umoci/image/generator"
 	"github.com/cyphar/umoci/image/layer"
+	"github.com/cyphar/umoci/pkg/idtools"
 	"github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/urfave/cli"
 	"github.com/vbatts/go-mtree"
@@ -68,6 +69,14 @@ manifest and configuration information uses the new diff atop the old manifest.`
 			Name:  "tag",
 			Usage: "tag name for repacked image",
 		},
+		cli.StringSliceFlag{
+			Name:  "uid-map",
+			Usage: "specifies a uid mapping to use when repacking",
+		},
+		cli.StringSliceFlag{
+			Name:  "gid-map",
+			Usage: "specifies a gid mapping to use when repacking",
+		},
 	},
 
 	Action: repack,
@@ -88,6 +97,27 @@ func repack(ctx *cli.Context) error {
 		return fmt.Errorf("reference name cannot be empty")
 	}
 
+	// Parse map options.
+	mapOptions := layer.MapOptions{}
+	for _, uidmap := range ctx.StringSlice("uid-map") {
+		idMap, err := idtools.ParseMapping(uidmap)
+		if err != nil {
+			return fmt.Errorf("failure parsing --uid-map %s: %s", uidmap, err)
+		}
+		mapOptions.UIDMappings = append(mapOptions.UIDMappings, idMap)
+	}
+	for _, gidmap := range ctx.StringSlice("gid-map") {
+		idMap, err := idtools.ParseMapping(gidmap)
+		if err != nil {
+			return fmt.Errorf("failure parsing --gid-map %s: %s", gidmap, err)
+		}
+		mapOptions.GIDMappings = append(mapOptions.GIDMappings, idMap)
+	}
+	logrus.WithFields(logrus.Fields{
+		"map.uid": mapOptions.UIDMappings,
+		"map.gid": mapOptions.GIDMappings,
+	}).Infof("parsed mappings")
+
 	// Get a reference to the CAS.
 	engine, err := cas.Open(imagePath)
 	if err != nil {
@@ -143,7 +173,7 @@ func repack(ctx *cli.Context) error {
 		"ndiff": len(diffs),
 	}).Debugf("umoci: checked mtree spec")
 
-	reader, err := layer.GenerateLayer(fullRootfsPath, diffs)
+	reader, err := layer.GenerateLayer(fullRootfsPath, diffs, &mapOptions)
 	if err != nil {
 		return err
 	}
@@ -169,6 +199,7 @@ func repack(ctx *cli.Context) error {
 	go func() {
 		_, err := io.Copy(gzw, hashReader)
 		if err != nil {
+			logrus.Warnf("failed when copying to gzip: %s", err)
 			pipeWriter.CloseWithError(err)
 			return
 		}

cmd/umoci/unpack.go

@@ -26,6 +26,7 @@ import (
 	"github.com/Sirupsen/logrus"
 	"github.com/cyphar/umoci/image/cas"
 	"github.com/cyphar/umoci/image/layer"
+	"github.com/cyphar/umoci/pkg/idtools"
 	"github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/urfave/cli"
 	"github.com/vbatts/go-mtree"
@@ -59,6 +60,14 @@ creation with umoci-repack(1).`,
 			Name:  "bundle",
 			Usage: "destination bundle path",
 		},
+		cli.StringSliceFlag{
+			Name:  "uid-map",
+			Usage: "specifies a uid mapping to use when repacking",
+		},
+		cli.StringSliceFlag{
+			Name:  "gid-map",
+			Usage: "specifies a gid mapping to use when repacking",
+		},
 	},
 
 	Action: unpack,
@@ -98,6 +107,27 @@ func unpack(ctx *cli.Context) error {
 		return fmt.Errorf("reference name cannot be empty")
 	}
 
+	// Parse map options.
+	mapOptions := layer.MapOptions{}
+	for _, uidmap := range ctx.StringSlice("uid-map") {
+		idMap, err := idtools.ParseMapping(uidmap)
+		if err != nil {
+			return fmt.Errorf("failure parsing --uid-map %s: %s", uidmap, err)
+		}
+		mapOptions.UIDMappings = append(mapOptions.UIDMappings, idMap)
+	}
+	for _, gidmap := range ctx.StringSlice("gid-map") {
+		idMap, err := idtools.ParseMapping(gidmap)
+		if err != nil {
+			return fmt.Errorf("failure parsing --gid-map %s: %s", gidmap, err)
+		}
+		mapOptions.GIDMappings = append(mapOptions.GIDMappings, idMap)
+	}
+	logrus.WithFields(logrus.Fields{
+		"map.uid": mapOptions.UIDMappings,
+		"map.gid": mapOptions.GIDMappings,
+	}).Infof("parsed mappings")
+
 	// Get a reference to the CAS.
 	engine, err := cas.Open(imagePath)
 	if err != nil {
@@ -153,7 +183,7 @@ func unpack(ctx *cli.Context) error {
 	//        extraction). I'm considering reimplementing it just so there are
 	//        competing implementations of this extraction functionality.
 	//           https://github.com/opencontainers/image-tools/issues/74
-	if err := layer.UnpackManifest(context.TODO(), engine, bundlePath, *manifest); err != nil {
+	if err := layer.UnpackManifest(context.TODO(), engine, bundlePath, *manifest, &mapOptions); err != nil {
 		return fmt.Errorf("failed to create runtime bundle: %q", err)
 	}
 

image/layer/generate.go

@@ -44,7 +44,12 @@ func (ids inodeDeltas) Swap(i, j int)      { ids[i], ids[j] = ids[j], ids[i] }
 // provided path (which should be the rootfs of the layer that was diffed). The
 // returned reader is for the *raw* tar data, it is the caller's responsibility
 // to gzip it.
-func GenerateLayer(path string, deltas []mtree.InodeDelta) (io.ReadCloser, error) {
+func GenerateLayer(path string, deltas []mtree.InodeDelta, opt *MapOptions) (io.ReadCloser, error) {
+	var mapOptions MapOptions
+	if opt != nil {
+		mapOptions = *opt
+	}
+
 	reader, writer := io.Pipe()
 
 	go func() {
@@ -53,7 +58,7 @@ func GenerateLayer(path string, deltas []mtree.InodeDelta) (io.ReadCloser, error
 		// We can't just dump all of the file contents into a tar file. We need
 		// to emulate a proper tar generator. Luckily there aren't that many
 		// things to emulate (and we can do them all in tar.go).
-		tg := newTarGenerator(writer)
+		tg := newTarGenerator(writer, mapOptions)
 		defer tg.tw.Close()
 
 		// Sort the delta paths.
@@ -73,7 +78,7 @@ func GenerateLayer(path string, deltas []mtree.InodeDelta) (io.ReadCloser, error
 			switch delta.Type() {
 			case mtree.Modified, mtree.Extra:
 				if err := tg.AddFile(name, fullPath); err != nil {
-					logrus.Warnf("could not add file: %s", err)
+					logrus.Warnf("could not add file %s: %s", name, err)
 					writer.CloseWithError(err)
 					return
 				}

image/layer/tar_extract.go

@@ -31,10 +31,23 @@ import (
 	"github.com/cyphar/umoci/third_party/symlink"
 )
 
-// applyMetadata applies the state described in tar.Header to the filesystem at
-// the given path. No sanity checking is done of the tar.Header's pathname or
-// other information.
-func applyMetadata(path string, hdr *tar.Header) error {
+// FIXME: Add tarExtractor.
+type tarExtractor struct {
+	// mapOptions is the set of mapping options to use when extracting filesystem layers.
+	mapOptions MapOptions
+}
+
+// newTarExtractor creates a new tarExtractor.
+func newTarExtractor(opt MapOptions) *tarExtractor {
+	return &tarExtractor{
+		mapOptions: opt,
+	}
+}
+
+// restoreMetadata applies the state described in tar.Header to the filesystem
+// at the given path. No sanity checking is done of the tar.Header's pathname
+// or other information. In addition, no mapping is done of the header.
+func restoreMetadata(path string, hdr *tar.Header) error {
 	// Some of the tar.Header fields don't match the OS API.
 	fi := hdr.FileInfo()
 
@@ -77,7 +90,10 @@ func applyMetadata(path string, hdr *tar.Header) error {
 	// Apply xattrs. In order to make sure that we *only* have the xattr set we
 	// want, we first clear the set of xattrs from the file then apply the ones
 	// set in the tar.Header.
-	// FIXME: This will almost certainly break horribly on RedHat.
+	// XXX: This will almost certainly break horribly on RedHat.
+	// FIXME: We really should not be clearing xattrs if the hdr has no Xattrs
+	//        entries (or something like that). Otherwise we're messing with
+	//        xattrs in rootless images.
 	if err := system.Lclearxattrs(path); err != nil {
 		return fmt.Errorf("apply metadata: %s: %s", path, err)
 	}
@@ -95,11 +111,26 @@ func applyMetadata(path string, hdr *tar.Header) error {
 	return nil
 }
 
+// applyMetadata applies the state described in tar.Header to the filesystem at
+// the given path, using the state of the tarExtractor to remap information
+// within the header. This should only be used with headers from a tar layer
+// (not from the filesystem). No sanity checking is done of the tar.Header's
+// pathname or other information.
+func (te *tarExtractor) applyMetadata(path string, hdr *tar.Header) error {
+	// Modify the header.
+	if err := unmapHeader(hdr, te.mapOptions); err != nil {
+		return err
+	}
+
+	// Restore it on the filesystme.
+	return restoreMetadata(path, hdr)
+}
+
 // unpackEntry extracts the given tar.Header to the provided root, ensuring
 // that the layer state is consistent with the layer state that produced the
 // tar archive being iterated over. This does handle whiteouts, so a tar.Header
 // that represents a whiteout will result in the path being removed.
-func unpackEntry(root string, hdr *tar.Header, r io.Reader) (Err error) {
+func (te *tarExtractor) unpackEntry(root string, hdr *tar.Header, r io.Reader) (Err error) {
 	// Make the paths safe.
 	hdr.Name = CleanPath(hdr.Name)
 	root = filepath.Clean(root)
@@ -140,11 +171,19 @@ func unpackEntry(root string, hdr *tar.Header, r io.Reader) (Err error) {
 			return err
 		}
 
+		// More faking to trick restoreMetadata to actually restore the directory.
+		dirHdr.Typeflag = tar.TypeDir
+		dirHdr.Linkname = ""
+
 		// Ensure that after everything we correctly re-apply the old metadata.
+		// We don't map this header because we're restoring files that already
+		// existed on the filesystem, not from a tar layer.
 		defer func() {
 			// Only overwrite the error if there wasn't one already.
-			if err := applyMetadata(dir, dirHdr); err != nil && Err == nil {
-				Err = err
+			if err := restoreMetadata(dir, dirHdr); err != nil {
+				if Err == nil {
+					Err = err
+				}
 			}
 		}()
 	}
@@ -313,7 +352,7 @@ func unpackEntry(root string, hdr *tar.Header, r io.Reader) (Err error) {
 	// apply metadata for hardlinks, because hardlinks don't have any separate
 	// metadata from their link (and the tar headers might not be filled).
 	if hdr.Typeflag != tar.TypeLink {
-		if err := applyMetadata(path, hdr); err != nil {
+		if err := te.applyMetadata(path, hdr); err != nil {
 			return err
 		}
 	}