Ignore error when xattr data is not available

mutate/compress.go

@@ -22,8 +22,15 @@ type Compressor interface {
 	// indicate what compression type is used, e.g. "gzip", or "" for no
 	// compression.
 	MediaTypeSuffix() string
+
+	// WithOpt applies an option and can be chained.
+	WithOpt(CompressorOpt) Compressor
 }
 
+// CompressorOpt is a compressor option which can be used to configure a
+// compressor.
+type CompressorOpt interface{}
+
 type noopCompressor struct{}
 
 func (nc noopCompressor) Compress(r io.Reader) (io.ReadCloser, error) {
@@ -37,16 +44,24 @@ func (nc noopCompressor) MediaTypeSuffix() string {
 // NoopCompressor provides no compression.
 var NoopCompressor Compressor = noopCompressor{}
 
+func (nc noopCompressor) WithOpt(CompressorOpt) Compressor {
+	return nc
+}
+
 // GzipCompressor provides gzip compression.
-var GzipCompressor Compressor = gzipCompressor{}
+var GzipCompressor Compressor = gzipCompressor{blockSize: 256 << 10}
+
+type GzipBlockSize int
 
-type gzipCompressor struct{}
+type gzipCompressor struct {
+	blockSize int
+}
 
 func (gz gzipCompressor) Compress(reader io.Reader) (io.ReadCloser, error) {
 	pipeReader, pipeWriter := io.Pipe()
 
 	gzw := gzip.NewWriter(pipeWriter)
-	if err := gzw.SetConcurrency(256<<10, 2*runtime.NumCPU()); err != nil {
+	if err := gzw.SetConcurrency(gz.blockSize, 2*runtime.NumCPU()); err != nil {
 		return nil, errors.Wrapf(err, "set concurrency level to %v blocks", 2*runtime.NumCPU())
 	}
 	go func() {
@@ -76,6 +91,15 @@ func (gz gzipCompressor) MediaTypeSuffix() string {
 	return "gzip"
 }
 
+func (gz gzipCompressor) WithOpt(opt CompressorOpt) Compressor {
+	switch val := opt.(type) {
+	case GzipBlockSize:
+		gz.blockSize = int(val)
+	}
+
+	return gz
+}
+
 // ZstdCompressor provides zstd compression.
 var ZstdCompressor Compressor = zstdCompressor{}
 
@@ -114,3 +138,7 @@ func (zs zstdCompressor) Compress(reader io.Reader) (io.ReadCloser, error) {
 func (zs zstdCompressor) MediaTypeSuffix() string {
 	return "zstd"
 }
+
+func (zs zstdCompressor) WithOpt(CompressorOpt) Compressor {
+	return zs
+}

mutate/compress_test.go

@@ -46,6 +46,22 @@ func TestGzipCompressor(t *testing.T) {
 	assert.NoError(err)
 
 	assert.Equal(string(content), fact)
+
+	// with options
+	buf = bytes.NewBufferString(fact)
+	c = GzipCompressor.WithOpt(GzipBlockSize(256 << 12))
+
+	r, err = c.Compress(buf)
+	assert.NoError(err)
+	assert.Equal(c.MediaTypeSuffix(), "gzip")
+
+	r, err = gzip.NewReader(r)
+	assert.NoError(err)
+
+	content, err = ioutil.ReadAll(r)
+	assert.NoError(err)
+
+	assert.Equal(string(content), fact)
 }
 
 func TestZstdCompressor(t *testing.T) {

oci/layer/generate.go

@@ -54,7 +54,9 @@ func GenerateLayer(path string, deltas []mtree.InodeDelta, opt *RepackOptions) (
 	go func() (Err error) {
 		// Close with the returned error.
 		defer func() {
-			log.Warnf("could not generate layer: %v", Err)
+			if Err != nil {
+				log.Warnf("could not generate layer: %v", Err)
+			}
 			// #nosec G104
 			_ = writer.CloseWithError(errors.Wrap(Err, "generate layer"))
 		}()
@@ -86,7 +88,7 @@ func GenerateLayer(path string, deltas []mtree.InodeDelta, opt *RepackOptions) (
 						return errors.Wrapf(err, "couldn't determine overlay whiteout for %s", fullPath)
 					}
 
-					whiteout, err := isOverlayWhiteout(fi)
+					whiteout, err := isOverlayWhiteout(fi, fullPath, tg.fsEval)
 					if err != nil {
 						return err
 					}
@@ -135,13 +137,21 @@ func GenerateInsertLayer(root string, target string, opaque bool, opt *RepackOpt
 
 	go func() (Err error) {
 		defer func() {
-			log.Warnf("could not generate insert layer: %v", Err)
+			if Err != nil {
+				log.Warnf("could not generate insert layer: %v", Err)
+			}
 			// #nosec G104
 			_ = writer.CloseWithError(errors.Wrap(Err, "generate insert layer"))
 		}()
 
 		tg := newTarGenerator(writer, packOptions.MapOptions)
 
+		defer func() {
+			if err := tg.tw.Close(); err != nil {
+				log.Warnf("generate insert layer: could not close tar.Writer: %s", err)
+			}
+		}()
+
 		if opaque {
 			if err := tg.AddOpaqueWhiteout(target); err != nil {
 				return err
@@ -156,7 +166,7 @@ func GenerateInsertLayer(root string, target string, opaque bool, opt *RepackOpt
 			}
 
 			pathInTar := path.Join(target, curPath[len(root):])
-			whiteout, err := isOverlayWhiteout(info)
+			whiteout, err := isOverlayWhiteout(info, curPath, tg.fsEval)
 			if err != nil {
 				return err
 			}

oci/layer/tar_generate.go

@@ -219,10 +219,14 @@ func (tg *tarGenerator) AddFile(name, path string) error {
 		//       (we'd need to use libcap to parse it).
 		value, err := tg.fsEval.Lgetxattr(path, name)
 		if err != nil {
-			// XXX: I'm not sure if we're unprivileged whether Lgetxattr can
-			//      fail with EPERM. If it can, we should ignore it (like when
-			//      we try to clear xattrs).
-			return errors.Wrapf(err, "get xattr: %s", name)
+			v := errors.Cause(err)
+			log.Debugf("failure reading xattr from list on %q: %q", name, v)
+			if v != unix.EOPNOTSUPP && v != unix.ENODATA && v != unix.EPERM {
+				// XXX: I'm not sure if we're unprivileged whether Lgetxattr can
+				//      fail with EPERM. If it can, we should ignore it (like when
+				//      we try to clear xattrs).
+				return errors.Wrapf(err, "get xattr: %s", name)
+			}
 		}
 		// https://golang.org/issues/20698 -- We don't just error out here
 		// because it's not _really_ a fatal error. Currently it's unclear

oci/layer/utils.go

@@ -25,6 +25,7 @@ import (
 
 	"github.com/apex/log"
 	rspec "github.com/opencontainers/runtime-spec/specs-go"
+	"github.com/opencontainers/umoci/pkg/fseval"
 	"github.com/opencontainers/umoci/pkg/idtools"
 	"github.com/pkg/errors"
 	rootlesscontainers "github.com/rootless-containers/proto/go-proto"
@@ -230,7 +231,7 @@ func InnerErrno(err error) error {
 
 // isOverlayWhiteout returns true if the FileInfo represents an overlayfs style
 // whiteout (i.e. mknod c 0 0) and false otherwise.
-func isOverlayWhiteout(info os.FileInfo) (bool, error) {
+func isOverlayWhiteout(info os.FileInfo, fullPath string, fsEval fseval.FsEval) (bool, error) {
 	var major, minor uint32
 	switch stat := info.Sys().(type) {
 	case *unix.Stat_t:
@@ -243,6 +244,25 @@ func isOverlayWhiteout(info os.FileInfo) (bool, error) {
 		return false, errors.Errorf("[internal error] unknown stat info type %T", info.Sys())
 	}
 
-	return major == 0 && minor == 0 &&
-		info.Mode()&os.ModeCharDevice != 0, nil
+	if major == 0 && minor == 0 &&
+		info.Mode()&os.ModeCharDevice != 0 {
+		return true, nil
+	}
+
+	// also evaluate xattrs which may have opaque value set
+	attr, err := fsEval.Lgetxattr(fullPath, "user.overlay.opaque")
+	if err != nil {
+		v := errors.Cause(err)
+		if !errors.Is(err, os.ErrNotExist) && v != unix.EOPNOTSUPP && v != unix.ENODATA && v != unix.EPERM && v != unix.EACCES {
+			return false, errors.Errorf("[internal error] unknown stat info type %T", info.Sys())
+		}
+
+		return false, nil
+	}
+
+	if string(attr) == "y" {
+		return true, nil
+	}
+
+	return false, nil
 }

pkg/system/copy.go

@@ -29,7 +29,7 @@ import (
 func Copy(dst io.Writer, src io.Reader) (int64, error) {
 	// Make a buffer so io.Copy doesn't make one for each iteration.
 	var buf []byte
-	size := 32 * 1024
+	size := 256 << 12 // 1 MiB
 	if lr, ok := src.(*io.LimitedReader); ok && lr.N < int64(size) {
 		if lr.N < 1 {
 			size = 1

test/helpers.bash

@@ -99,7 +99,21 @@ function requires() {
 }
 
 function image-verify() {
-	oci-image-tool validate --type "imageLayout" "$@"
+	local ocidir="$@"
+	# test that each generated targz file is valid according to gnutar:
+	for f in $(ls $ocidir/blobs/sha256/); do
+		file $ocidir/blobs/sha256/$f | grep "gzip" || {
+			continue
+		}
+		zcat $ocidir/blobs/sha256/$f | tar tvf - >/dev/null || {
+			rc=$?
+			file $ocidir/blobs/sha256/$f
+			echo "error untarring $f: $rc"
+			return $rc
+		}
+		echo $f: valid tar archive
+	done
+	oci-image-tool validate --type "imageLayout" "$ocidir"
 	return $?
 }
 

test/insert.bats

@@ -45,6 +45,7 @@ function teardown() {
 	touch "${INSERTDIR}/test/a"
 	touch "${INSERTDIR}/test/b"
 	chmod +x "${INSERTDIR}/test/b"
+	echo "foo" > "${INSERTDIR}/test/smallfile"
 
 	# Make sure rootless mode works.
 	mkdir -p "${INSERTDIR}/some/path"
@@ -68,6 +69,10 @@ function teardown() {
 	[ "$status" -eq 0 ]
 	image-verify "${IMAGE}"
 
+	umoci insert --image "${IMAGE}:${TAG}" "${INSERTDIR}/test/smallfile" /tester/smallfile
+	[ "$status" -eq 0 ]
+	image-verify "${IMAGE}"
+
 	# Unpack after the inserts.
 	new_bundle_rootfs
 	umoci unpack --image "${IMAGE}:${TAG}-new" "$BUNDLE"