From c510173829a934b666973d47d182779a8f8ef757 Mon Sep 17 00:00:00 2001 From: Nalin Dahyabhai Date: Thu, 17 Apr 2025 14:17:34 -0400 Subject: [PATCH 01/10] CI: expand the in-a-container-tests filesystem combinations Expand our task that runs the integration tests inside of a container from running buildah-using-vfs-inside-podman-using-vfs and buildah-using-overlay-inside-podman-using-overlay to also test vfs over overlay, and overlay over vfs. Signed-off-by: Nalin Dahyabhai --- .cirrus.yml | 13 +++++++++++-- contrib/cirrus/lib.sh | 4 ++-- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/.cirrus.yml b/.cirrus.yml index d3f3c65de84..02b34817768 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -294,7 +294,7 @@ integration_rootless_task: <<: *standardlogs in_podman_task: - name: "Containerized Integration" + name: "Containerized Integration w/ $NESTED_STORAGE_DRIVER over $STORAGE_DRIVER" alias: in_podman skip: *not_build_docs depends_on: *smoke_vendor @@ -307,7 +307,16 @@ in_podman_task: # This is key, cause the scripts to re-execute themselves inside a container. IN_PODMAN: 'true' BUILDAH_ISOLATION: 'chroot' - STORAGE_DRIVER: 'vfs' + matrix: + - env: + STORAGE_DRIVER: 'vfs' + - env: + STORAGE_DRIVER: 'overlay' + matrix: + - env: + NESTED_STORAGE_DRIVER: 'vfs' + - env: + NESTED_STORAGE_DRIVER: 'overlay' # Separate scripts for separate outputs, makes debugging easier. setup_script: '${SCRIPT_BASE}/setup.sh |& ${_TIMESTAMP}' diff --git a/contrib/cirrus/lib.sh b/contrib/cirrus/lib.sh index a0f5818759d..1a7da836a01 100755 --- a/contrib/cirrus/lib.sh +++ b/contrib/cirrus/lib.sh @@ -198,13 +198,13 @@ in_podman() { --cgroupns=host \ "${envargs[@]}" \ -e BUILDAH_ISOLATION \ - -e STORAGE_DRIVER \ + -e STORAGE_DRIVER"${NESTED_STORAGE_DRIVER:+=${NESTED_STORAGE_DRIVER}}" \ -e "IN_PODMAN=false" \ -e "CONTAINER=podman" \ -e "CGROUP_MANAGER=cgroupfs" \ -v "$HOME/auth:$HOME/auth:ro" \ -v /sys/fs/cgroup:/sys/fs/cgroup:rw \ - -v /dev/fuse:/dev/fuse:rw \ + --device /dev/fuse \ -v "$GOSRC:$GOSRC:z" \ --workdir "$GOSRC" \ "$@" From 22d770e26e0db7d7578613fafc15503012e717b0 Mon Sep 17 00:00:00 2001 From: Nalin Dahyabhai Date: Thu, 13 Mar 2025 13:23:34 -0400 Subject: [PATCH 02/10] RUN: only use an overlay for --mount=type=bind,rw on overlay Only use an overlay for --mounts of type=bind,rw, where changes should be discarded, if the storage driver is overlay. Otherwise, make a temporary copy and use a bind mount to the copy. Signed-off-by: Nalin Dahyabhai --- internal/volumes/volumes.go | 118 ++++++++++++++++++++++-- pkg/overlay/overlay.go | 12 +-- tests/bud/buildkit-mount/Containerfile5 | 1 + 3 files changed, 118 insertions(+), 13 deletions(-) diff --git a/internal/volumes/volumes.go b/internal/volumes/volumes.go index 941d6f9539c..22d9e097013 100644 --- a/internal/volumes/volumes.go +++ b/internal/volumes/volumes.go @@ -4,12 +4,14 @@ import ( "context" "errors" "fmt" + "io" "os" "path" "path/filepath" "slices" "strconv" "strings" + "sync" "github.com/containers/buildah/copier" "github.com/containers/buildah/define" @@ -77,6 +79,11 @@ func mountIsReadWrite(m specs.Mount) bool { return rw } +// convertToOverlay accepts a specs.Mount that specifies a bind mount, and +// returns a replacement for it that instead mounts an overlay at the desired +// location, using the original source location as a "lower", so that writes to +// it will be redirected to it to a temporary location which can later be +// discarded, leaving the original location unmodified. func convertToOverlay(m specs.Mount, store storage.Store, mountLabel, tmpDir string, uid, gid int) (specs.Mount, string, error) { overlayDir, err := overlay.TempDir(tmpDir, uid, gid) if err != nil { @@ -93,7 +100,7 @@ func convertToOverlay(m specs.Mount, store storage.Store, mountLabel, tmpDir str // do the normal thing of mounting this directory as a lower with a temporary upper mountThisInstead, err = overlay.MountWithOptions(overlayDir, m.Source, m.Destination, &options) if err != nil { - return specs.Mount{}, "", fmt.Errorf("setting up overlay of %q: %w", m.Source, err) + return specs.Mount{}, "", fmt.Errorf("setting up overlay of directory %q: %w", m.Source, err) } } else { // mount the parent directory as the lower with a temporary upper, and return a @@ -103,7 +110,7 @@ func convertToOverlay(m specs.Mount, store storage.Store, mountLabel, tmpDir str destination := m.Destination mountedOverlay, err := overlay.MountWithOptions(overlayDir, sourceDir, destination, &options) if err != nil { - return specs.Mount{}, "", fmt.Errorf("setting up overlay of %q: %w", sourceDir, err) + return specs.Mount{}, "", fmt.Errorf("setting up overlay of directory %q containing %q: %w", sourceDir, sourceBase, err) } if mountedOverlay.Type != define.TypeBind { if err2 := overlay.RemoveTemp(overlayDir); err2 != nil { @@ -118,6 +125,91 @@ func convertToOverlay(m specs.Mount, store storage.Store, mountLabel, tmpDir str return mountThisInstead, overlayDir, nil } +// makeTempCopy accepts a specs.Mount that specifies a bind mount, and returns +// a replacement for it that instead bind mounts a temporary copy of the +// original source location, so that writes to it will succeed without actually +// modifying the original copy. +func makeTempCopy(m specs.Mount, tmpDir string, uid, gid int) (specs.Mount, string, error) { + succeeded := false + overlayDir, err := overlay.TempDir(tmpDir, uid, gid) + if err != nil { + return specs.Mount{}, "", fmt.Errorf("setting up temporary copy of %q: %w", m.Destination, err) + } + defer func() { + if !succeeded { + if err := overlay.RemoveTemp(overlayDir); err != nil { + logrus.Warnf("%v", err) + } + } + }() + mergedDir := filepath.Join(overlayDir, "merge") + fileInfo, err := os.Stat(m.Source) + if err != nil { + return specs.Mount{}, "", fmt.Errorf("setting up temporary copy of %q: %w", m.Source, err) + } + copiedDir := filepath.Join(mergedDir, "copied") + if fileInfo.IsDir() { + if err = os.Mkdir(copiedDir, fileInfo.Mode()); err != nil { + return specs.Mount{}, "", fmt.Errorf("creating top-level directory for copy of %q: %w", m.Source, err) + } + } else { + if err = os.Mkdir(copiedDir, 0o755); err != nil { + return specs.Mount{}, "", fmt.Errorf("creating parent directory for copy of %q: %w", m.Source, err) + } + } + if err = os.Chown(copiedDir, uid, gid); err != nil { + return specs.Mount{}, "", fmt.Errorf("setting ownership of %q: %w", m.Source, err) + } + forceOwner := idtools.IDPair{UID: uid, GID: gid} + getOptions := copier.GetOptions{ + ChownDirs: &forceOwner, + ChownFiles: &forceOwner, + } + var wg sync.WaitGroup + var putErr, getErr error + copyReader, copyWriter := io.Pipe() + wg.Add(1) + go func() { + defer wg.Done() + putErr = copier.Put(copiedDir, copiedDir, copier.PutOptions{}, copyReader) + copyReader.Close() + }() + mountThisInstead := m + mountThisInstead.Options = append(slices.Clone(define.BindOptions), "z") + wg.Add(1) + if fileInfo.IsDir() { + // copy the directory to the temporary directory + go func() { + defer wg.Done() + getErr = copier.Get(m.Source, m.Source, getOptions, []string{"."}, copyWriter) + copyWriter.Close() + }() + // point the if-everything-works mount at the temporary driectory + mountThisInstead.Type = define.TypeBind + mountThisInstead.Source = copiedDir + mountThisInstead.Destination = m.Destination + } else { + // copy just the one thing to the temporary directory + sourceDir := filepath.Dir(m.Source) + sourceBase := filepath.Base(m.Source) + go func() { + defer wg.Done() + getErr = copier.Get(sourceDir, sourceDir, getOptions, []string{sourceBase}, copyWriter) + copyWriter.Close() + }() + // point the if-everything-works mount at the new copy of the item + mountThisInstead.Type = define.TypeBind + mountThisInstead.Source = filepath.Join(copiedDir, sourceBase) + mountThisInstead.Destination = m.Destination + } + wg.Wait() + if getErr != nil || putErr != nil { + return specs.Mount{}, "", fmt.Errorf("creating a temporary copy: %w", errors.Join(getErr, putErr)) + } + succeeded = true + return mountThisInstead, overlayDir, nil +} + // FIXME: this code needs to be merged with pkg/parse/parse.go ValidateVolumeOpts // // GetBindMount parses a single bind mount entry from the --mount flag. @@ -329,8 +421,15 @@ func GetBindMount(sys *types.SystemContext, args []string, contextDir string, st overlayDir := "" if mountedImage != "" || mountIsReadWrite(newMount) { - if newMount, overlayDir, err = convertToOverlay(newMount, store, mountLabel, tmpDir, 0, 0); err != nil { - return newMount, "", "", "", err + switch store.GraphDriverName() { + case "overlay": + if newMount, overlayDir, err = convertToOverlay(newMount, store, mountLabel, tmpDir, 0, 0); err != nil { + return newMount, "", "", "", err + } + default: + if newMount, overlayDir, err = makeTempCopy(newMount, tmpDir, 0, 0); err != nil { + return newMount, "", "", "", err + } } } @@ -662,8 +761,15 @@ func GetCacheMount(sys *types.SystemContext, args []string, store storage.Store, overlayDir := "" if needToOverlay { - if newMount, overlayDir, err = convertToOverlay(newMount, store, mountLabel, tmpDir, 0, 0); err != nil { - return newMount, "", "", "", nil, err + switch store.GraphDriverName() { + case "overlay": + if newMount, overlayDir, err = convertToOverlay(newMount, store, mountLabel, tmpDir, 0, 0); err != nil { + return newMount, "", "", "", nil, err + } + default: + if newMount, overlayDir, err = makeTempCopy(newMount, tmpDir, 0, 0); err != nil { + return newMount, "", "", "", nil, err + } } } diff --git a/pkg/overlay/overlay.go b/pkg/overlay/overlay.go index 5bdc10775d6..ea66eb7408e 100644 --- a/pkg/overlay/overlay.go +++ b/pkg/overlay/overlay.go @@ -57,8 +57,8 @@ type Options struct { // TempDir generates a uniquely-named directory under ${containerDir}/overlay // which can be used as a parent directory for the upper and working -// directories for an overlay mount, creates "upper" and "work" directories -// beneath it, and then returns the path of the new directory. +// directories for an overlay mount, creates "upper", "work", and "merge" +// directories beneath it, and then returns the path of the new directory. func TempDir(containerDir string, rootUID, rootGID int) (string, error) { contentDir := filepath.Join(containerDir, "overlay") if err := idtools.MkdirAllAs(contentDir, 0o700, rootUID, rootGID); err != nil { @@ -126,12 +126,10 @@ func findMountProgram(graphOptions []string) string { } for _, i := range graphOptions { - s := strings.SplitN(i, "=", 2) - if len(s) != 2 { + key, val, ok := strings.Cut(i, "=") + if !ok { continue } - key := s[0] - val := s[1] if _, has := mountMap[key]; has { return val } @@ -146,7 +144,7 @@ func mountWithMountProgram(mountProgram, overlayOptions, mergeDir string) error cmd := exec.Command(mountProgram, "-o", overlayOptions, mergeDir) if err := cmd.Run(); err != nil { - return fmt.Errorf("exec %s: %w", mountProgram, err) + return fmt.Errorf("exec %s -o %q %q: %w", mountProgram, overlayOptions, mergeDir, err) } return nil } diff --git a/tests/bud/buildkit-mount/Containerfile5 b/tests/bud/buildkit-mount/Containerfile5 index 9e31936072f..bbd2fd8865b 100644 --- a/tests/bud/buildkit-mount/Containerfile5 +++ b/tests/bud/buildkit-mount/Containerfile5 @@ -3,3 +3,4 @@ ARG INPUTPATH_1=subdir RUN mkdir /test # use option z if selinux is enabled RUN --mount=type=bind,source=${INPUTPATH_1:?}/,target=/test,z cat /test/input_file +RUN --mount=type=bind,source=${INPUTPATH_1:?}/,target=/test,z --mount=type=bind,source=${INPUTPATH_1:?}/,target=/test2,rw,z cmp /test/input_file /test2/input_file From c2f530c4ed5b4c577beaf179dc11ffe3cd2f2d9e Mon Sep 17 00:00:00 2001 From: Nalin Dahyabhai Date: Wed, 12 Mar 2025 16:32:20 -0400 Subject: [PATCH 03/10] pkg/overlay: fall back to trying fuse-overlayfs If we're forcing a mount using overlay, but we fail, and our chosen upper directory is on overlay, try using fuse-overlayfs. Signed-off-by: Nalin Dahyabhai --- pkg/overlay/overlay_linux.go | 29 ++++++++++++++++++++++++++--- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/pkg/overlay/overlay_linux.go b/pkg/overlay/overlay_linux.go index 8b84d463584..e07a8c8f30d 100644 --- a/pkg/overlay/overlay_linux.go +++ b/pkg/overlay/overlay_linux.go @@ -1,8 +1,10 @@ package overlay import ( + "errors" "fmt" "os" + "os/exec" "path/filepath" "strings" "syscall" @@ -10,6 +12,7 @@ import ( "github.com/containers/storage/pkg/unshare" "github.com/opencontainers/runtime-spec/specs-go" "github.com/opencontainers/selinux/go-selinux/label" + "golang.org/x/sys/unix" ) // MountWithOptions creates ${contentDir}/merge, where ${contentDir} was @@ -83,11 +86,10 @@ func MountWithOptions(contentDir, source, dest string, opts *Options) (mount spe return mount, nil } + // A mount_program is not specified: fallback to try mounting native overlay. if unshare.IsRootless() { - // If a mount_program is not specified, fallback to try mounting native overlay. overlayOptions = fmt.Sprintf("%s,userxattr", overlayOptions) } - mount.Source = mergeDir mount.Destination = dest mount.Type = "overlay" @@ -95,7 +97,28 @@ func MountWithOptions(contentDir, source, dest string, opts *Options) (mount spe if opts.ForceMount { if err := mountNatively(overlayOptions, mergeDir); err != nil { - return mount, err + if opts.ReadOnly { + // We don't expect any kinds of error that we can respond to. + return mount, err + } + if errors.Is(err, syscall.EINVAL) { + // We couldn't do that with the kernel's built-in overlay + // filesystem; check if it was because the upper we made is + // already on an overlay filesystem, and if so, make a + // last desperate effort to find fuse-overlayfs, which doesn't + // mind us doing stuff like this. + upperDir := filepath.Join(contentDir, "upper") + var fs syscall.Statfs_t + if err2 := syscall.Statfs(upperDir, &fs); err2 == nil && fs.Type == unix.OVERLAYFS_SUPER_MAGIC { + if path, err3 := exec.LookPath("fuse-overlayfs"); err3 == nil { + mountProgram = path + err = mountWithMountProgram(mountProgram, overlayOptions, mergeDir) + } + } + } + if err != nil { + return mount, err + } } mount.Source = mergeDir From dad67d7bca65a1015fa2e3458a1765321f42e1a2 Mon Sep 17 00:00:00 2001 From: Nalin Dahyabhai Date: Thu, 24 Apr 2025 15:45:07 -0400 Subject: [PATCH 04/10] Builder.runSetupVolumeMounts(): force overlay mounts Force the mount of the overlay in runSetupVolumeMounts() so that we can take advantage of logic that tries to use fuse-overlayfs if we happen to be on top of overlay, which would otherwise fail. Signed-off-by: Nalin Dahyabhai --- run_linux.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/run_linux.go b/run_linux.go index 9a55cba3b7e..850d331a021 100644 --- a/run_linux.go +++ b/run_linux.go @@ -1171,6 +1171,8 @@ func (b *Builder) runSetupVolumeMounts(mountLabel string, volumeMounts []string, UpperDirOptionFragment: upperDir, WorkDirOptionFragment: workDir, GraphOpts: slices.Clone(b.store.GraphOptions()), + ForceMount: true, + MountLabel: b.MountLabel, } overlayMount, err := overlay.MountWithOptions(contentDir, host, container, &overlayOpts) From b12d92da120056a020f50662810c39e58a42fbeb Mon Sep 17 00:00:00 2001 From: Nalin Dahyabhai Date: Thu, 17 Apr 2025 15:48:21 -0400 Subject: [PATCH 05/10] imagebuildah.StageExecutor.volumeCacheSaveOverlay(): force mount Force the mount of the overlay in volumeCacheSaveOverlay(), and clean it up in volumeCacheRestoreOverlay(), so that we can take advantage of logic that tries to use fuse-overlayfs if we happen to be on top of overlay, which would otherwise fail. Signed-off-by: Nalin Dahyabhai --- imagebuildah/stage_executor.go | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/imagebuildah/stage_executor.go b/imagebuildah/stage_executor.go index 89bc4707fcd..ecda24111f4 100644 --- a/imagebuildah/stage_executor.go +++ b/imagebuildah/stage_executor.go @@ -21,6 +21,7 @@ import ( "github.com/containers/buildah/internal" "github.com/containers/buildah/internal/tmpdir" internalUtil "github.com/containers/buildah/internal/util" + "github.com/containers/buildah/pkg/overlay" "github.com/containers/buildah/pkg/parse" "github.com/containers/buildah/pkg/rusage" "github.com/containers/buildah/util" @@ -313,12 +314,24 @@ func (s *StageExecutor) volumeCacheRestoreVFS() (err error) { // using it as a lower for an overlay mount in the same location, and then // discarding the upper. func (s *StageExecutor) volumeCacheSaveOverlay() (mounts []specs.Mount, err error) { + containerDir, err := s.executor.store.ContainerDirectory(s.builder.ContainerID) + if err != nil { + return nil, fmt.Errorf("unable to locate temporary directory for container") + } + volumeCacheDir := filepath.Join(containerDir, "volume-cache") + options := overlay.Options{ + MountLabel: s.builder.MountLabel, + ForceMount: true, + } for cachedPath := range s.volumeCache { volumePath := filepath.Join(s.mountPoint, cachedPath) - mount := specs.Mount{ - Source: volumePath, - Destination: cachedPath, - Options: []string{"O", "private"}, + tmpdir, err := overlay.TempDir(volumeCacheDir, 0, 0) + if err != nil { + return nil, fmt.Errorf("unable to create temporary directory for cache for volumes") + } + mount, err := overlay.MountWithOptions(tmpdir, volumePath, cachedPath, &options) + if err != nil { + return nil, fmt.Errorf("unable to create temporary directory for cache for volumes") } mounts = append(mounts, mount) } @@ -327,7 +340,12 @@ func (s *StageExecutor) volumeCacheSaveOverlay() (mounts []specs.Mount, err erro // Reset the contents of each of the executor's list of volumes. func (s *StageExecutor) volumeCacheRestoreOverlay() error { - return nil + containerDir, err := s.executor.store.ContainerDirectory(s.builder.ContainerID) + if err != nil { + return fmt.Errorf("unable to locate temporary directory for container") + } + volumeCacheDir := filepath.Join(containerDir, "volume-cache") + return overlay.CleanupContent(volumeCacheDir) } // Save the contents of each of the executor's list of volumes for which we From 7c5f9b99c90f91f904ac97f63b8b33580f1b1269 Mon Sep 17 00:00:00 2001 From: Nalin Dahyabhai Date: Thu, 17 Apr 2025 14:13:02 -0400 Subject: [PATCH 06/10] run.bats(run overlay --volume with custom upper and workdir): drop ,z Drop the conditional ,z that we were trying to add when testing the -v flag with the "O" option, the combination of which is not allowed by github.com/containers/common/pkg/parse.ValidateVolumeOpts(). Signed-off-by: Nalin Dahyabhai --- tests/run.bats | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/tests/run.bats b/tests/run.bats index 289ccb1223a..fc9e7f2860e 100644 --- a/tests/run.bats +++ b/tests/run.bats @@ -337,12 +337,6 @@ function configure_and_check_user() { @test "run overlay --volume with custom upper and workdir" { skip_if_no_runtime - zflag= - if which selinuxenabled > /dev/null 2> /dev/null ; then - if selinuxenabled ; then - zflag=z - fi - fi ${OCI} --version _prefetch alpine run_buildah from --quiet --pull=false $WITH_POLICY_JSON alpine @@ -351,16 +345,16 @@ function configure_and_check_user() { mkdir -p ${TEST_SCRATCH_DIR}/workdir mkdir -p ${TEST_SCRATCH_DIR}/lower - echo 'hello' >> ${TEST_SCRATCH_DIR}/lower/hello + echo 'hello' > ${TEST_SCRATCH_DIR}/lower/hello # As a baseline, this should succeed. - run_buildah run -v ${TEST_SCRATCH_DIR}/lower:/test:O,upperdir=${TEST_SCRATCH_DIR}/upperdir,workdir=${TEST_SCRATCH_DIR}/workdir${zflag:+:${zflag}} $cid cat /test/hello + run_buildah run -v ${TEST_SCRATCH_DIR}/lower:/test:O,upperdir=${TEST_SCRATCH_DIR}/upperdir,workdir=${TEST_SCRATCH_DIR}/workdir $cid cat /test/hello expect_output "hello" - run_buildah run -v ${TEST_SCRATCH_DIR}/lower:/test:O,upperdir=${TEST_SCRATCH_DIR}/upperdir,workdir=${TEST_SCRATCH_DIR}/workdir${zflag:+:${zflag}} $cid sh -c 'echo "world" > /test/world' + run_buildah run -v ${TEST_SCRATCH_DIR}/lower:/test:O,upperdir=${TEST_SCRATCH_DIR}/upperdir,workdir=${TEST_SCRATCH_DIR}/workdir $cid sh -c 'echo "world" > /test/world' - #upper dir should persist content + # upper dir should persist content result="$(cat ${TEST_SCRATCH_DIR}/upperdir/world)" - test "$result" == "world" + assert "$result" == "world" } @test "run --volume with U flag" { From b2b6d04d66ecaed74357ae65d9dade3cf969e123 Mon Sep 17 00:00:00 2001 From: Nalin Dahyabhai Date: Fri, 25 Apr 2025 10:26:31 -0400 Subject: [PATCH 07/10] run.bats(root fs only mounted once): also accept rw by itself When grepping the mounts list for the root mount, also accept the entry if "rw" is the only flag. Signed-off-by: Nalin Dahyabhai --- tests/run.bats | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/run.bats b/tests/run.bats index fc9e7f2860e..203fae2884c 100644 --- a/tests/run.bats +++ b/tests/run.bats @@ -411,7 +411,7 @@ function configure_and_check_user() { zflag= if which selinuxenabled > /dev/null 2> /dev/null ; then if selinuxenabled ; then - zflag=z + zflag=,z fi fi ${OCI} --version @@ -420,23 +420,23 @@ function configure_and_check_user() { cid=$output mkdir -p ${TEST_SCRATCH_DIR}/was:empty # As a baseline, this should succeed. - run_buildah run --mount type=tmpfs,dst=/var/tmpfs-not-empty $cid touch /var/tmpfs-not-empty/testfile + run_buildah run --mount type=tmpfs,dst=/var/tmpfs-not-empty $cid touch /var/tmpfs-not-empty/testfile # This should succeed, but the writes should effectively be discarded - run_buildah run --mount type=bind,src=${TEST_SCRATCH_DIR}/was:empty,dst=/var/not-empty,rw${zflag:+,${zflag}} $cid touch /var/not-empty/testfile + run_buildah run --mount type=bind,src=${TEST_SCRATCH_DIR}/was:empty,dst=/var/not-empty,rw${zflag} $cid touch /var/not-empty/testfile if test -r ${TEST_SCRATCH_DIR}/was:empty/testfile ; then die write to mounted type=bind was not discarded, ${TEST_SCRATCH_DIR}/was:empty/testfile exists fi # If we're parsing the options at all, this should be read-only, so it should fail. - run_buildah 1 run --mount type=bind,src=${TEST_SCRATCH_DIR}/was:empty,dst=/var/not-empty,ro${zflag:+,${zflag}} $cid touch /var/not-empty/testfile + run_buildah 1 run --mount type=bind,src=${TEST_SCRATCH_DIR}/was:empty,dst=/var/not-empty,ro${zflag} $cid touch /var/not-empty/testfile # Even if the parent directory doesn't exist yet, this should succeed, but again the write should be discarded. - run_buildah run --mount type=bind,src=${TEST_SCRATCH_DIR}/was:empty,dst=/var/multi-level/subdirectory,rw${zflag:+,${zflag}} $cid touch /var/multi-level/subdirectory/testfile + run_buildah run --mount type=bind,src=${TEST_SCRATCH_DIR}/was:empty,dst=/var/multi-level/subdirectory,rw${zflag} $cid touch /var/multi-level/subdirectory/testfile if test -r ${TEST_SCRATCH_DIR}/was:empty/testfile ; then die write to mounted type=bind was not discarded, ${TEST_SCRATCH_DIR}/was:empty/testfile exists fi # And check the same for file volumes, which make life harder because the kernel's overlay # filesystem really only wants to be dealing with directories. : > ${TEST_SCRATCH_DIR}/was:empty/testfile - run_buildah run --mount type=bind,src=${TEST_SCRATCH_DIR}/was:empty/testfile,dst=/var/different-multi-level/subdirectory/testfile,rw${zflag:+,${zflag}} $cid sh -c 'echo wrote > /var/different-multi-level/subdirectory/testfile' + run_buildah run --mount type=bind,src=${TEST_SCRATCH_DIR}/was:empty/testfile,dst=/var/different-multi-level/subdirectory/testfile,rw${zflag} $cid sh -c 'echo wrote > /var/different-multi-level/subdirectory/testfile' if test -s ${TEST_SCRATCH_DIR}/was:empty/testfile ; then die write to mounted type=bind was not discarded, ${TEST_SCRATCH_DIR}/was:empty/testfile was written to fi @@ -1021,7 +1021,7 @@ _EOF echo "$output" > ${TEST_SCRATCH_DIR}/mountinfo1 echo "# mountinfo unfiltered:" cat ${TEST_SCRATCH_DIR}/mountinfo1 - grep ' / rw,' ${TEST_SCRATCH_DIR}/mountinfo1 > ${TEST_SCRATCH_DIR}/mountinfo2 + grep ' / rw[ ,]' ${TEST_SCRATCH_DIR}/mountinfo1 > ${TEST_SCRATCH_DIR}/mountinfo2 echo "# mountinfo grepped:" cat ${TEST_SCRATCH_DIR}/mountinfo2 wc -l < ${TEST_SCRATCH_DIR}/mountinfo2 > ${TEST_SCRATCH_DIR}/mountinfo3 From 60e2359646798c2e94d658f48f995b4cb7727ce9 Mon Sep 17 00:00:00 2001 From: Nalin Dahyabhai Date: Fri, 25 Apr 2025 16:54:06 -0400 Subject: [PATCH 08/10] run.bats(run --mount): skip if we're on overlay If we're on overlayfs, we can't use the directory name that we'd like to use as part of the test and expect things to succeed, even if we try to work around it using fuse-overlayfs. Signed-off-by: Nalin Dahyabhai --- tests/run.bats | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/run.bats b/tests/run.bats index 203fae2884c..8890863d000 100644 --- a/tests/run.bats +++ b/tests/run.bats @@ -419,6 +419,12 @@ function configure_and_check_user() { run_buildah from --quiet --pull=false $WITH_POLICY_JSON alpine cid=$output mkdir -p ${TEST_SCRATCH_DIR}/was:empty + if test $(stat -f -c %T ${TEST_SCRATCH_DIR}/was:empty) = overlayfs; then + # we'll try to use fuse-overlayfs, which at least through 1.13 + # can't accept ":" in layer locations, escaped or not, so bail + # now instead of breaking the whole thing + skip unable to test read-write bind from an overlay location that includes colon characters + fi # As a baseline, this should succeed. run_buildah run --mount type=tmpfs,dst=/var/tmpfs-not-empty $cid touch /var/tmpfs-not-empty/testfile # This should succeed, but the writes should effectively be discarded From 7574fe200899f3cbf01ebb3340307c2073b66303 Mon Sep 17 00:00:00 2001 From: Nalin Dahyabhai Date: Mon, 28 Apr 2025 10:05:49 -0400 Subject: [PATCH 09/10] overlay.bats(overlay path contains colon): skip if we're on overlay If we're on overlayfs, we can't use the directory name that we'd like to use as part of the test and expect things to succeed, even if we try to work around it using fuse-overlayfs. Signed-off-by: Nalin Dahyabhai --- tests/overlay.bats | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/overlay.bats b/tests/overlay.bats index ab2066bbe55..9faa2e3cac5 100644 --- a/tests/overlay.bats +++ b/tests/overlay.bats @@ -74,6 +74,13 @@ load helpers mkdir ${TEST_SCRATCH_DIR}/a:lower touch ${TEST_SCRATCH_DIR}/a:lower/foo + if test $(stat -f -c %T ${TEST_SCRATCH_DIR}/a:lower) = overlayfs; then + # we'll try to use fuse-overlayfs, which at least through 1.13 + # can't accept ":" in layer locations, escaped or not, so bail + # now instead of breaking the whole thing + skip unable to test bind from an overlay location that includes colon characters + fi + # This should succeed. # Add double backslash, because shell will escape. run_buildah from --quiet -v ${TEST_SCRATCH_DIR}/a\\:lower:/a\\:lower:O --quiet $WITH_POLICY_JSON $image From 2ed484546c63c252bc5efbb1eaee0b7a91a37e5c Mon Sep 17 00:00:00 2001 From: Nalin Dahyabhai Date: Thu, 24 Apr 2025 16:57:08 -0400 Subject: [PATCH 10/10] chroot.bats(chroot with overlay root): ensure we can overlay When setting up a chroot using overlay, if the intended upper directory is already on an overlayfs, mount a tmpfs onto it so that we can finish setting up the chroot that's an overlayfs that we'll actually test in, and copy the base image into the storage that it'll use. Signed-off-by: Nalin Dahyabhai --- tests/chroot.bats | 37 +++++++++++++++++++++++++++++++++---- 1 file changed, 33 insertions(+), 4 deletions(-) diff --git a/tests/chroot.bats b/tests/chroot.bats index 652ea3b0081..f6a54585e35 100644 --- a/tests/chroot.bats +++ b/tests/chroot.bats @@ -124,11 +124,24 @@ load helpers cp -v ${TEST_SOURCES}/containers.conf ${TEST_SCRATCH_DIR}/containers.conf chmod ugo+r ${TEST_SCRATCH_DIR}/containers.conf mkdir -p ${TEST_SCRATCH_DIR}/chroot + ${COPY_BINARY} containers-storage:[${STORAGE_DRIVER}@${TEST_SCRATCH_DIR}/root+${TEST_SCRATCH_DIR}/runroot]docker.io/library/busybox:latest dir:${TEST_SCRATCH_DIR}/base-image chown -R 1:1 ${TEST_SCRATCH_DIR}/root ${TEST_SCRATCH_DIR}/runroot ${TEST_SCRATCH_DIR}/chroot + if test ${STORAGE_DRIVER} = overlay ; then + if test -x /usr/bin/fuse-overlayfs ; then + local storage_opts="overlay.mount_program=/usr/bin/fuse-overlayfs" + else + skip trying to use overlay on top of overlay, but fuse-overlayfs is not present + fi + fi + # a script that runs inside of a new mount namespace and mounts the current + # rootfs as the "lower" for an overlay, then pivots into it cat > ${TEST_SCRATCH_DIR}/script1 <<- EOF PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin${PATH:+:$PATH} set -e set -x + if test \$(stat -f -c %T "${TEST_SCRATCH_DIR}/chroot") = overlayfs ; then + mount -t tmpfs -o size=16M none ${TEST_SCRATCH_DIR}/chroot + fi mkdir -p ${TEST_SCRATCH_DIR}/chroot/workdir mkdir -p ${TEST_SCRATCH_DIR}/chroot/upperdir mkdir -p ${TEST_SCRATCH_DIR}/chroot/merged @@ -152,21 +165,34 @@ load helpers if test -d /var/tmp; then mount --bind /var/tmp ${TEST_SCRATCH_DIR}/chroot/merged/var/tmp fi + mkdir -p ${TEST_SCRATCH_DIR}/chroot/merged/run + mount -t tmpfs -o size=1024k none ${TEST_SCRATCH_DIR}/chroot/merged/run + chmod 755 ${TEST_SCRATCH_DIR}/chroot/merged/run + mkdir -p ${TEST_SCRATCH_DIR}/chroot/merged/run/containers/storage + chmod 755 ${TEST_SCRATCH_DIR}/chroot/merged/run/containers/storage + mkdir -p ${TEST_SCRATCH_DIR}/chroot/merged/var/lib/containers/storage + chmod 755 ${TEST_SCRATCH_DIR}/chroot/merged/var/lib/containers/storage + chown -R 1:1 ${TEST_SCRATCH_DIR}/chroot/merged/run ${TEST_SCRATCH_DIR}/chroot/merged/var/lib/containers mount --bind ${TEST_SCRATCH_DIR} ${TEST_SCRATCH_DIR}/chroot/merged/${TEST_SCRATCH_DIR} mkdir -p ${TEST_SCRATCH_DIR}/chroot/merged/usr/local/bin + chmod 755 ${TEST_SCRATCH_DIR}/chroot/merged/usr/local/bin touch ${TEST_SCRATCH_DIR}/chroot/merged/usr/local/bin/buildah mount --bind ${BUILDAH_BINARY:-$TEST_SOURCES/../bin/buildah} ${TEST_SCRATCH_DIR}/chroot/merged/usr/local/bin/buildah cd ${TEST_SCRATCH_DIR}/chroot/merged + ${COPY_BINARY} --root ${TEST_SCRATCH_DIR}/root --runroot ${TEST_SCRATCH_DIR}/runroot --storage-driver ${STORAGE_DRIVER} ${storage_opts:+--storage-opt ${storage_opts}} dir:${TEST_SCRATCH_DIR}/base-image dir:${TEST_SCRATCH_DIR}/chroot/merged/base-image pivot_root . tmp mount --make-rslave tmp umount -f -l tmp - mount -o remount,ro --make-rshared / + mount -o remount --make-rshared / grep ' / / ' /proc/self/mountinfo # unshare from util-linux 2.39 also accepts INNER:OUTER:SIZE for --map-users # and --map-groups, but fedora 37's is too old, so the older OUTER,INNER,SIZE # (using commas instead of colons as field separators) will have to do - unshare --setuid 0 --setgid 0 --map-users=1,0,1024 --map-groups=1,0,1024 -UinCfpm bash ${TEST_SCRATCH_DIR}/script2 + unshare --setuid 0 --setgid 0 --map-users=1,0,1024 --map-users=1025,65534,2 --map-groups=1,0,1024 --map-groups=1025,65534,2 -UinCfpm bash ${TEST_SCRATCH_DIR}/script2 EOF + # a script that runs inside of a new user namespace with an unprivileged ID + # mapped to root, which is expected to be able to run, with the proper + # configuration options, on top of that overlay filesystem cat > ${TEST_SCRATCH_DIR}/script2 <<- EOF set -e set -x @@ -175,8 +201,11 @@ EOF cat /proc/self/uid_map cat /proc/self/gid_map mount --make-shared / - /usr/local/bin/buildah ${BUILDAH_REGISTRY_OPTS} ${ROOTDIR_OPTS} from --name ctrid --pull=never --quiet docker.io/library/busybox - /usr/local/bin/buildah ${BUILDAH_REGISTRY_OPTS} ${ROOTDIR_OPTS} run --isolation=chroot ctrid pwd + /usr/local/bin/buildah ${BUILDAH_REGISTRY_OPTS} --root /var/lib/containers/storage --runroot /run/containers/storage --storage-driver ${STORAGE_DRIVER} ${storage_opts:+--storage-opt ${storage_opts}} pull dir:/base-image + baseID=\$(jq -r .config.digest /base-image/manifest.json) + /usr/local/bin/buildah ${BUILDAH_REGISTRY_OPTS} --root /var/lib/containers/storage --runroot /run/containers/storage --storage-driver ${STORAGE_DRIVER} ${storage_opts:+--storage-opt ${storage_opts}} tag \${baseID} docker.io/library/busybox + /usr/local/bin/buildah ${BUILDAH_REGISTRY_OPTS} --root /var/lib/containers/storage --runroot /run/containers/storage --storage-driver ${STORAGE_DRIVER} ${storage_opts:+--storage-opt ${storage_opts}} from --name ctrid --pull=never --quiet docker.io/library/busybox + /usr/local/bin/buildah ${BUILDAH_REGISTRY_OPTS} --root /var/lib/containers/storage --runroot /run/containers/storage --storage-driver ${STORAGE_DRIVER} ${storage_opts:+--storage-opt ${storage_opts}} run --isolation=chroot ctrid pwd EOF chmod +x ${TEST_SCRATCH_DIR} chmod +rx ${TEST_SCRATCH_DIR}/script1 ${TEST_SCRATCH_DIR}/script2