diff --git a/mutate/compress.go b/mutate/compress.go index 740968c17..d8a2427ec 100644 --- a/mutate/compress.go +++ b/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 +} diff --git a/mutate/compress_test.go b/mutate/compress_test.go index 6cfd47ffe..3eda48dd9 100644 --- a/mutate/compress_test.go +++ b/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) { diff --git a/oci/layer/generate.go b/oci/layer/generate.go index 1f1ab0c09..ffd562473 100644 --- a/oci/layer/generate.go +++ b/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 } diff --git a/oci/layer/tar_generate.go b/oci/layer/tar_generate.go index 76b35ca37..63b6b6d38 100644 --- a/oci/layer/tar_generate.go +++ b/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 diff --git a/oci/layer/utils.go b/oci/layer/utils.go index f22122f78..8d8e1574f 100644 --- a/oci/layer/utils.go +++ b/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 } diff --git a/pkg/system/copy.go b/pkg/system/copy.go index 4d4d4ace6..219331a78 100644 --- a/pkg/system/copy.go +++ b/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 diff --git a/test/helpers.bash b/test/helpers.bash index fd981ce77..9fd73a6e1 100644 --- a/test/helpers.bash +++ b/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 $? } diff --git a/test/insert.bats b/test/insert.bats index 1fa1b4ca1..38206524f 100644 --- a/test/insert.bats +++ b/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"