Skip to content

Commit

Permalink
Merge pull request #3064 from vrothberg/backport-34ae47a22629-the-rea…
Browse files Browse the repository at this point in the history
…l-one

Backport 34ae47a the real one
  • Loading branch information
openshift-merge-robot authored Mar 8, 2021
2 parents ca3c296 + d1c9523 commit 89ff19c
Show file tree
Hide file tree
Showing 5 changed files with 195 additions and 18 deletions.
2 changes: 1 addition & 1 deletion buildah.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ const (
Package = "buildah"
// Version for the Package. Bump version in contrib/rpm/buildah.spec
// too.
Version = "1.19.7"
Version = "1.19.8"
// The value we use to identify what type of information, currently a
// serialized Builder structure, we are using as per-container state.
// This should only be changed when we make incompatible changes to
Expand Down
5 changes: 4 additions & 1 deletion contrib/rpm/buildah.spec
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@

Name: buildah
# Bump version in buildah.go too
Version: 1.19.7
Version: 1.19.8
Release: 1.git%{shortcommit}%{?dist}
Summary: A command line tool used to creating OCI Images
License: ASL 2.0
Expand Down Expand Up @@ -100,6 +100,9 @@ make DESTDIR=%{buildroot} PREFIX=%{_prefix} install install.completions
%{_datadir}/bash-completion/completions/*

%changelog
* Mon Mar 8, 2021 Valentin Rothberg <[email protected]> 1.19.8-1
- copier: support ignoring EPERMs

* Thu Mar 4, 2021 Dan Walsh <[email protected]> 1.19.7-1
- Set upperdir permissions based on source

Expand Down
42 changes: 35 additions & 7 deletions copier/copier.go
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,7 @@ type GetOptions struct {
KeepDirectoryNames bool // don't strip the top directory's basename from the paths of items in subdirectories
Rename map[string]string // rename items with the specified names, or under the specified names
NoDerefSymlinks bool // don't follow symlinks when globs match them
IgnoreUnreadable bool // ignore errors reading items, instead of returning an error
}

// Get produces an archive containing items that match the specified glob
Expand Down Expand Up @@ -1035,6 +1036,14 @@ func copierHandlerStat(req request, pm *fileutils.PatternMatcher) *response {
return &response{Stat: statResponse{Globs: stats}}
}

func errorIsPermission(err error) bool {
err = errors.Cause(err)
if err == nil {
return false
}
return os.IsPermission(err) || strings.Contains(err.Error(), "permission denied")
}

func copierHandlerGet(bulkWriter io.Writer, req request, pm *fileutils.PatternMatcher, idMappings *idtools.IDMappings) (*response, func() error, error) {
statRequest := req
statRequest.Request = requestStat
Expand Down Expand Up @@ -1111,6 +1120,12 @@ func copierHandlerGet(bulkWriter io.Writer, req request, pm *fileutils.PatternMa
options.ExpandArchives = false
walkfn := func(path string, info os.FileInfo, err error) error {
if err != nil {
if options.IgnoreUnreadable && errorIsPermission(err) {
if info != nil && info.IsDir() {
return filepath.SkipDir
}
return nil
}
return errors.Wrapf(err, "copier: get: error reading %q", path)
}
// compute the path of this item
Expand Down Expand Up @@ -1150,7 +1165,13 @@ func copierHandlerGet(bulkWriter io.Writer, req request, pm *fileutils.PatternMa
symlinkTarget = target
}
// add the item to the outgoing tar stream
return copierHandlerGetOne(info, symlinkTarget, rel, path, options, tw, hardlinkChecker, idMappings)
if err := copierHandlerGetOne(info, symlinkTarget, rel, path, options, tw, hardlinkChecker, idMappings); err != nil {
if req.GetOptions.IgnoreUnreadable && errorIsPermission(err) {
return nil
}
return err
}
return nil
}
// walk the directory tree, checking/adding items individually
if err := filepath.Walk(item, walkfn); err != nil {
Expand All @@ -1170,6 +1191,9 @@ func copierHandlerGet(bulkWriter io.Writer, req request, pm *fileutils.PatternMa
// dereferenced, be sure to use the name of the
// link.
if err := copierHandlerGetOne(info, "", filepath.Base(queue[i]), item, req.GetOptions, tw, hardlinkChecker, idMappings); err != nil {
if req.GetOptions.IgnoreUnreadable && errorIsPermission(err) {
continue
}
return errors.Wrapf(err, "copier: get: %q", queue[i])
}
itemsCopied++
Expand Down Expand Up @@ -1250,7 +1274,7 @@ func copierHandlerGetOne(srcfi os.FileInfo, symlinkTarget, name, contentPath str
if options.ExpandArchives && isArchivePath(contentPath) {
f, err := os.Open(contentPath)
if err != nil {
return errors.Wrapf(err, "error opening %s", contentPath)
return errors.Wrapf(err, "error opening file for reading archive contents")
}
defer f.Close()
rc, _, err := compression.AutoDecompress(f)
Expand Down Expand Up @@ -1321,17 +1345,21 @@ func copierHandlerGetOne(srcfi os.FileInfo, symlinkTarget, name, contentPath str
hdr.Mode = int64(*options.ChmodFiles)
}
}
var f *os.File
if hdr.Typeflag == tar.TypeReg {
// open the file first so that we don't write a header for it if we can't actually read it
f, err = os.Open(contentPath)
if err != nil {
return errors.Wrapf(err, "error opening file for adding its contents to archive")
}
defer f.Close()
}
// output the header
if err = tw.WriteHeader(hdr); err != nil {
return errors.Wrapf(err, "error writing header for %s (%s)", contentPath, hdr.Name)
}
if hdr.Typeflag == tar.TypeReg {
// output the content
f, err := os.Open(contentPath)
if err != nil {
return errors.Wrapf(err, "error opening %s", contentPath)
}
defer f.Close()
n, err := io.Copy(tw, f)
if err != nil {
return errors.Wrapf(err, "error copying %s", contentPath)
Expand Down
149 changes: 149 additions & 0 deletions copier/copier_linux_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
package copier

import (
"archive/tar"
"bytes"
"encoding/json"
"fmt"
"io"
"io/ioutil"
"os"
"path/filepath"
"testing"

"github.com/containers/storage/pkg/reexec"
"github.com/pkg/errors"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/syndtr/gocapability/capability"
)

func init() {
reexec.Register("get", getWrappedMain)
}

type getWrappedOptions struct {
Root, Directory string
GetOptions GetOptions
Globs []string
DropCaps []capability.Cap
}

func getWrapped(root string, directory string, getOptions GetOptions, globs []string, dropCaps []capability.Cap, bulkWriter io.Writer) error {
options := getWrappedOptions{
Root: root,
Directory: directory,
GetOptions: getOptions,
Globs: globs,
DropCaps: dropCaps,
}
encoded, err := json.Marshal(&options)
if err != nil {
return errors.Wrapf(err, "error marshalling options")
}
cmd := reexec.Command("get")
cmd.Env = append(cmd.Env, "OPTIONS="+string(encoded))
cmd.Stdout = bulkWriter
stderrBuf := bytes.Buffer{}
cmd.Stderr = &stderrBuf
err = cmd.Run()
if stderrBuf.Len() > 0 {
if err != nil {
return fmt.Errorf("%v: %s", err, stderrBuf.String())
}
return fmt.Errorf("%s", stderrBuf.String())
}
return err
}

func getWrappedMain() {
var options getWrappedOptions
if err := json.Unmarshal([]byte(os.Getenv("OPTIONS")), &options); err != nil {
fmt.Fprintf(os.Stderr, "%v", err)
os.Exit(1)
}
if len(options.DropCaps) > 0 {
caps, err := capability.NewPid(0)
if err != nil {
fmt.Fprintf(os.Stderr, "%v", err)
os.Exit(1)
}
for _, capType := range []capability.CapType{
capability.AMBIENT,
capability.BOUNDING,
capability.INHERITABLE,
capability.PERMITTED,
capability.EFFECTIVE,
} {
for _, cap := range options.DropCaps {
if caps.Get(capType, cap) {
caps.Unset(capType, cap)
}
}
if err := caps.Apply(capType); err != nil {
fmt.Fprintf(os.Stderr, "error dropping capability %+v: %v", options.DropCaps, err)
os.Exit(1)
}
}
}
if err := Get(options.Root, options.Directory, options.GetOptions, options.Globs, os.Stdout); err != nil {
fmt.Fprintf(os.Stderr, "%v", err)
os.Exit(1)
}
}

func TestGetPermissionErrorNoChroot(t *testing.T) {
couldChroot := canChroot
canChroot = false
testGetPermissionError(t)
canChroot = couldChroot
}

func TestGetPermissionErrorChroot(t *testing.T) {
if uid != 0 {
t.Skipf("chroot() requires root privileges, skipping")
}
couldChroot := canChroot
canChroot = true
testGetPermissionError(t)
canChroot = couldChroot
}

func testGetPermissionError(t *testing.T) {
dropCaps := []capability.Cap{capability.CAP_DAC_OVERRIDE, capability.CAP_DAC_READ_SEARCH}
tmp, err := ioutil.TempDir("", "")
require.NoError(t, err, "error creating temp directory")
err = os.Mkdir(filepath.Join(tmp, "unreadable-directory"), 0000)
require.NoError(t, err, "error creating an unreadable directory")
err = os.Mkdir(filepath.Join(tmp, "readable-directory"), 0755)
require.NoError(t, err, "error creating a readable directory")
err = os.Mkdir(filepath.Join(tmp, "readable-directory", "unreadable-subdirectory"), 0000)
require.NoError(t, err, "error creating an unreadable subdirectory")
err = ioutil.WriteFile(filepath.Join(tmp, "unreadable-file"), []byte("hi, i'm a file that you can't read"), 0000)
require.NoError(t, err, "error creating an unreadable file")
err = ioutil.WriteFile(filepath.Join(tmp, "readable-file"), []byte("hi, i'm also a file, and you can read me"), 0644)
require.NoError(t, err, "error creating a readable file")
err = ioutil.WriteFile(filepath.Join(tmp, "readable-directory", "unreadable-file"), []byte("hi, i'm also a file that you can't read"), 0000)
require.NoError(t, err, "error creating an unreadable file in a readable directory")
for _, ignore := range []bool{false, true} {
t.Run(fmt.Sprintf("ignore=%v", ignore), func(t *testing.T) {
var buf bytes.Buffer
err = getWrapped(tmp, tmp, GetOptions{IgnoreUnreadable: ignore}, []string{"."}, dropCaps, &buf)
if ignore {
assert.NoError(t, err, "expected no errors")
tr := tar.NewReader(&buf)
items := 0
_, err := tr.Next()
for err == nil {
items++
_, err = tr.Next()
}
assert.True(t, errors.Is(err, io.EOF), "expected EOF to finish read contents")
assert.Equalf(t, 2, items, "expected two readable items, got %d", items)
} else {
assert.Error(t, err, "expected an error")
assert.Truef(t, errorIsPermission(err), "expected the error (%v) to be a permission error", err)
}
})
}
}
15 changes: 6 additions & 9 deletions copier/copier_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -491,9 +491,8 @@ func testPut(t *testing.T) {
t.Skipf("test archive %q can only be tested with root privileges, skipping", testArchives[i].name)
}

tmp, err := ioutil.TempDir("", "copier-test-")
require.NoError(t, err, "error creating temporary directory")
defer os.RemoveAll(tmp)
tmp, err := ioutil.TempDir("", "")
require.NoError(t, err, "error creating temp directory")

archive := makeArchive(testArchives[i].headers, testArchives[i].contents)
err = Put(tmp, tmp, PutOptions{UIDMap: uidMap, GIDMap: gidMap, Rename: renames.renames}, archive)
Expand Down Expand Up @@ -532,9 +531,8 @@ func testPut(t *testing.T) {
{Name: "test", Typeflag: tar.TypeDir, Size: 0, Mode: 0755, ModTime: testDate},
{Name: "test", Typeflag: typeFlag, Size: 0, Mode: 0755, Linkname: "target", ModTime: testDate},
})
tmp, err := ioutil.TempDir("", "copier-test-")
require.NoError(t, err, "error creating temporary directory")
defer os.RemoveAll(tmp)
tmp, err := ioutil.TempDir("", "")
require.NoError(t, err, "error creating temp directory")
err = Put(tmp, tmp, PutOptions{UIDMap: uidMap, GIDMap: gidMap, NoOverwriteDirNonDir: !overwrite}, bytes.NewReader(archive))
if overwrite {
if unwrapError(err) != syscall.EPERM {
Expand All @@ -558,9 +556,8 @@ func testPut(t *testing.T) {
{Name: "link", Typeflag: tar.TypeLink, Size: 0, Mode: 0600, ModTime: testDate, Linkname: "test"},
{Name: "unrelated", Typeflag: tar.TypeReg, Size: 0, Mode: 0600, ModTime: testDate},
})
tmp, err := ioutil.TempDir("", "copier-test-")
require.NoError(t, err, "error creating temporary directory")
defer os.RemoveAll(tmp)
tmp, err := ioutil.TempDir("", "")
require.NoError(t, err, "error creating temp directory")
err = Put(tmp, tmp, PutOptions{UIDMap: uidMap, GIDMap: gidMap, IgnoreDevices: ignoreDevices}, bytes.NewReader(archive))
require.Nilf(t, err, "expected to extract content with typeflag %c without an error: %v", typeFlag, err)
fileList, err := enumerateFiles(tmp)
Expand Down

0 comments on commit 89ff19c

Please sign in to comment.