From 512b39475b7ca56b6398bd0208ee112e8880f720 Mon Sep 17 00:00:00 2001 From: Sascha Grunert Date: Thu, 27 Apr 2023 08:41:32 +0200 Subject: [PATCH] Update c/common and avoid setting umask We can now use the new API for creating files and directories without setting the umask to allow parallel usage of those methods. This patch also bumps c/common for that. [NO NEW TESTS NEEDED] Signed-off-by: Sascha Grunert --- go.mod | 2 +- go.sum | 4 +- libpod/container_internal_common.go | 5 +- .../common/pkg/subscriptions/subscriptions.go | 33 ++++++----- .../containers/common/pkg/umask/umask.go | 58 +++++++++++++++++++ .../containers/common/version/version.go | 2 +- vendor/modules.txt | 2 +- 7 files changed, 82 insertions(+), 24 deletions(-) create mode 100644 vendor/github.com/containers/common/pkg/umask/umask.go diff --git a/go.mod b/go.mod index ff9d2d212c..34e176ad9d 100644 --- a/go.mod +++ b/go.mod @@ -13,7 +13,7 @@ require ( github.com/containernetworking/cni v1.1.2 github.com/containernetworking/plugins v1.2.0 github.com/containers/buildah v1.30.0 - github.com/containers/common v0.52.1-0.20230424070932-46c446398f30 + github.com/containers/common v0.53.0 github.com/containers/conmon v2.0.20+incompatible github.com/containers/image/v5 v5.25.0 github.com/containers/libhvee v0.0.5 diff --git a/go.sum b/go.sum index b5c034c058..c9cfca7705 100644 --- a/go.sum +++ b/go.sum @@ -239,8 +239,8 @@ github.com/containernetworking/plugins v1.2.0 h1:SWgg3dQG1yzUo4d9iD8cwSVh1VqI+bP github.com/containernetworking/plugins v1.2.0/go.mod h1:/VjX4uHecW5vVimFa1wkG4s+r/s9qIfPdqlLF4TW8c4= github.com/containers/buildah v1.30.0 h1:mdp2COGKFFEZNEGP8VZ5ITuUFVNPFoH+iK2sSesNfTA= github.com/containers/buildah v1.30.0/go.mod h1:lyMLZIevpAa6zSzjRl7z4lFJMCMQLFjfo56YIefaB/U= -github.com/containers/common v0.52.1-0.20230424070932-46c446398f30 h1:2HUDH+YRnT214PTxLvnyawyDl4iMmtu0pxCN63F2jG4= -github.com/containers/common v0.52.1-0.20230424070932-46c446398f30/go.mod h1:pABPxJwlTE8oYk9/2BW0e0mumkuhJHIPsABHTGRXN3w= +github.com/containers/common v0.53.0 h1:Ax814cLeX5VXSnkKUdxz762g+27fJj1st4UvKoXmkKs= +github.com/containers/common v0.53.0/go.mod h1:pABPxJwlTE8oYk9/2BW0e0mumkuhJHIPsABHTGRXN3w= github.com/containers/conmon v2.0.20+incompatible h1:YbCVSFSCqFjjVwHTPINGdMX1F6JXHGTUje2ZYobNrkg= github.com/containers/conmon v2.0.20+incompatible/go.mod h1:hgwZ2mtuDrppv78a/cOBNiCm6O0UMWGx1mu7P00nu5I= github.com/containers/image/v5 v5.25.0 h1:TJ0unmalbU+scd0i3Txap2wjGsAnv06MSCwgn6bsizk= diff --git a/libpod/container_internal_common.go b/libpod/container_internal_common.go index 8e24374e1a..1e16a93feb 100644 --- a/libpod/container_internal_common.go +++ b/libpod/container_internal_common.go @@ -2745,10 +2745,7 @@ func (c *Container) createSecretMountDir() error { src := filepath.Join(c.state.RunDir, "/run/secrets") _, err := os.Stat(src) if os.IsNotExist(err) { - oldUmask := umask.Set(0) - defer umask.Set(oldUmask) - - if err := os.MkdirAll(src, 0755); err != nil { + if err := umask.MkdirAllIgnoreUmask(src, os.FileMode(0o755)); err != nil { return err } if err := label.Relabel(src, c.config.MountLabel, false); err != nil { diff --git a/vendor/github.com/containers/common/pkg/subscriptions/subscriptions.go b/vendor/github.com/containers/common/pkg/subscriptions/subscriptions.go index cdece0a1cb..b751f48772 100644 --- a/vendor/github.com/containers/common/pkg/subscriptions/subscriptions.go +++ b/vendor/github.com/containers/common/pkg/subscriptions/subscriptions.go @@ -27,9 +27,10 @@ var ( UserOverrideMountsFile = filepath.Join(os.Getenv("HOME"), ".config/containers/mounts.conf") ) -// subscriptionData stores the name of the file and the content read from it +// subscriptionData stores the relative name of the file and the content read from it type subscriptionData struct { - name string + // relPath is the relative path to the file + relPath string data []byte mode os.FileMode dirMode os.FileMode @@ -37,11 +38,16 @@ type subscriptionData struct { // saveTo saves subscription data to given directory func (s subscriptionData) saveTo(dir string) error { - path := filepath.Join(dir, s.name) - if err := os.MkdirAll(filepath.Dir(path), s.dirMode); err != nil { - return err + // We need to join the path here and create all parent directories, only + // creating dir is not good enough as relPath could also contain directories. + path := filepath.Join(dir, s.relPath) + if err := umask.MkdirAllIgnoreUmask(filepath.Dir(path), s.dirMode); err != nil { + return fmt.Errorf("create subscription directory: %w", err) } - return os.WriteFile(path, s.data, s.mode) + if err := umask.WriteFileIgnoreUmask(path, s.data, s.mode); err != nil { + return fmt.Errorf("write subscription data: %w", err) + } + return nil } func readAll(root, prefix string, parentMode os.FileMode) ([]subscriptionData, error) { @@ -94,7 +100,7 @@ func readFileOrDir(root, name string, parentMode os.FileMode) ([]subscriptionDat return nil, err } return []subscriptionData{{ - name: name, + relPath: name, data: bytes, mode: s.Mode(), dirMode: parentMode, @@ -242,13 +248,9 @@ func addSubscriptionsFromMountsFile(filePath, mountLabel, containerRunDir string return nil, err } - // Don't let the umask have any influence on the file and directory creation - oldUmask := umask.Set(0) - defer umask.Set(oldUmask) - switch mode := fileInfo.Mode(); { case mode.IsDir(): - if err = os.MkdirAll(ctrDirOrFileOnHost, mode.Perm()); err != nil { + if err = umask.MkdirAllIgnoreUmask(ctrDirOrFileOnHost, mode.Perm()); err != nil { return nil, fmt.Errorf("making container directory: %w", err) } data, err := getHostSubscriptionData(hostDirOrFile, mode.Perm()) @@ -266,10 +268,11 @@ func addSubscriptionsFromMountsFile(filePath, mountLabel, containerRunDir string return nil, err } for _, s := range data { - if err := os.MkdirAll(filepath.Dir(ctrDirOrFileOnHost), s.dirMode); err != nil { - return nil, err + dir := filepath.Dir(ctrDirOrFileOnHost) + if err := umask.MkdirAllIgnoreUmask(dir, s.dirMode); err != nil { + return nil, fmt.Errorf("create container dir: %w", err) } - if err := os.WriteFile(ctrDirOrFileOnHost, s.data, s.mode); err != nil { + if err := umask.WriteFileIgnoreUmask(ctrDirOrFileOnHost, s.data, s.mode); err != nil { return nil, fmt.Errorf("saving data to container filesystem: %w", err) } } diff --git a/vendor/github.com/containers/common/pkg/umask/umask.go b/vendor/github.com/containers/common/pkg/umask/umask.go new file mode 100644 index 0000000000..93f1d2b3c0 --- /dev/null +++ b/vendor/github.com/containers/common/pkg/umask/umask.go @@ -0,0 +1,58 @@ +package umask + +import ( + "fmt" + "os" + "path/filepath" +) + +// MkdirAllIgnoreUmask creates a directory by ignoring the currently set umask. +func MkdirAllIgnoreUmask(dir string, mode os.FileMode) error { + parent := dir + dirs := []string{} + + // Find all parent directories which would have been created by MkdirAll + for { + if _, err := os.Stat(parent); err == nil { + break + } else if !os.IsNotExist(err) { + return fmt.Errorf("cannot stat %s: %w", dir, err) + } + + dirs = append(dirs, parent) + newParent := filepath.Dir(parent) + + // Only possible if the root paths are not existing, which would be odd + if parent == newParent { + break + } + + parent = newParent + } + + if err := os.MkdirAll(dir, mode); err != nil { + return fmt.Errorf("create directory %s: %w", dir, err) + } + + for _, d := range dirs { + if err := os.Chmod(d, mode); err != nil { + return fmt.Errorf("chmod directory %s: %w", d, err) + } + } + + return nil +} + +// WriteFileIgnoreUmask write the provided data to the path by ignoring the +// currently set umask. +func WriteFileIgnoreUmask(path string, data []byte, mode os.FileMode) error { + if err := os.WriteFile(path, data, mode); err != nil { + return fmt.Errorf("write file: %w", err) + } + + if err := os.Chmod(path, mode); err != nil { + return fmt.Errorf("chmod file: %w", err) + } + + return nil +} diff --git a/vendor/github.com/containers/common/version/version.go b/vendor/github.com/containers/common/version/version.go index ddf2f15fb2..8bd54871c0 100644 --- a/vendor/github.com/containers/common/version/version.go +++ b/vendor/github.com/containers/common/version/version.go @@ -1,4 +1,4 @@ package version // Version is the version of the build. -const Version = "0.53.0-dev" +const Version = "0.53.0" diff --git a/vendor/modules.txt b/vendor/modules.txt index b0f83d304b..518b4ff8f8 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -125,7 +125,7 @@ github.com/containers/buildah/pkg/rusage github.com/containers/buildah/pkg/sshagent github.com/containers/buildah/pkg/util github.com/containers/buildah/util -# github.com/containers/common v0.52.1-0.20230424070932-46c446398f30 +# github.com/containers/common v0.53.0 ## explicit; go 1.18 github.com/containers/common/libimage github.com/containers/common/libimage/define