From 719b660462c99f1e204fc492d49ed1b1b41a4ae6 Mon Sep 17 00:00:00 2001 From: Aditya Rajan Date: Mon, 18 Oct 2021 11:21:51 +0530 Subject: [PATCH] buildkit: add from field to bind and cache mounts so images can be used as source Following commit adds buildkit like support for `from` field to `--mount=type=bind` and `--mount=type=cache` so images and stage can be used as mount source. Usage looks like ```dockerfile RUN --mount=type=bind,source=.,from=,target=/path ls /path ``` and ```dockerfile RUN --mount=type=cache,from=,target=/path ls /path ``` Signed-off-by: Aditya Rajan --- cmd/buildah/run.go | 9 +- docs/Containerfile.5.md | 6 +- docs/buildah-run.1.md | 6 +- imagebuildah/stage_executor.go | 60 +++ internal/parse/parse.go | 411 ++++++++++++++++++ internal/types.go | 11 + internal/util/util.go | 24 + pkg/parse/parse.go | 301 +------------ run.go | 12 + run_linux.go | 75 +++- tests/bud.bats | 173 ++++++++ .../buildkit-mount-from/Dockerfilebindfrom | 3 + .../Dockerfilebindfromrelative | 4 + .../Dockerfilebindfromwithemptyfrom | 4 + .../Dockerfilebindfromwithoutsource | 4 + .../Dockerfilebindfromwriteable | 3 + .../Dockerfilebuildkitbase | 2 + .../Dockerfilebuildkitbaserelative | 3 + .../buildkit-mount-from/Dockerfilecachefrom | 8 + .../Dockerfilecachefromimage | 5 + .../Dockerfilecachemultiplefrom | 10 + .../Dockerfilemultistagefrom | 6 + .../Dockerfilemultistagefromcache | 11 + .../buildkit-mount-from/Dockermultistagefrom | 6 + tests/bud/buildkit-mount-from/hello | 1 + tests/bud/buildkit-mount-from/hello2 | 1 + tests/run.bats | 39 +- 27 files changed, 896 insertions(+), 302 deletions(-) create mode 100644 internal/parse/parse.go create mode 100644 internal/types.go create mode 100644 internal/util/util.go create mode 100644 tests/bud/buildkit-mount-from/Dockerfilebindfrom create mode 100644 tests/bud/buildkit-mount-from/Dockerfilebindfromrelative create mode 100644 tests/bud/buildkit-mount-from/Dockerfilebindfromwithemptyfrom create mode 100644 tests/bud/buildkit-mount-from/Dockerfilebindfromwithoutsource create mode 100644 tests/bud/buildkit-mount-from/Dockerfilebindfromwriteable create mode 100644 tests/bud/buildkit-mount-from/Dockerfilebuildkitbase create mode 100644 tests/bud/buildkit-mount-from/Dockerfilebuildkitbaserelative create mode 100644 tests/bud/buildkit-mount-from/Dockerfilecachefrom create mode 100644 tests/bud/buildkit-mount-from/Dockerfilecachefromimage create mode 100644 tests/bud/buildkit-mount-from/Dockerfilecachemultiplefrom create mode 100644 tests/bud/buildkit-mount-from/Dockerfilemultistagefrom create mode 100644 tests/bud/buildkit-mount-from/Dockerfilemultistagefromcache create mode 100644 tests/bud/buildkit-mount-from/Dockermultistagefrom create mode 100644 tests/bud/buildkit-mount-from/hello create mode 100644 tests/bud/buildkit-mount-from/hello2 diff --git a/cmd/buildah/run.go b/cmd/buildah/run.go index e0f60d609c..bf53e2f5d5 100644 --- a/cmd/buildah/run.go +++ b/cmd/buildah/run.go @@ -149,13 +149,20 @@ func runCmd(c *cobra.Command, args []string, iopts runInputOptions) error { } } - mounts, err := parse.GetVolumes(iopts.volumes, iopts.mounts, iopts.contextDir) + systemContext, err := parse.SystemContextFromOptions(c) + if err != nil { + return errors.Wrapf(err, "error building system context") + } + mounts, mountedImages, err := parse.GetVolumes(systemContext, store, iopts.volumes, iopts.mounts, iopts.contextDir) if err != nil { return err } options.Mounts = mounts + // Run() will automatically clean them up. + options.ExternalImageMounts = mountedImages runerr := builder.Run(args, options) + if runerr != nil { logrus.Debugf("error running %v in container %q: %v", args, builder.Container, runerr) } diff --git a/docs/Containerfile.5.md b/docs/Containerfile.5.md index de728c2496..842d25d3f8 100644 --- a/docs/Containerfile.5.md +++ b/docs/Containerfile.5.md @@ -114,7 +114,7 @@ Current supported mount TYPES are bind, cache, secret and tmpfs. Common Options: - · src, source: mount source spec for bind and volume. Mandatory for bind. + · src, source: mount source spec for bind and volume. Mandatory for bind. If `from` is specified, `src` is the subpath in the `from` field. · dst, destination, target: mount destination spec. @@ -126,6 +126,8 @@ Current supported mount TYPES are bind, cache, secret and tmpfs. . bind-nonrecursive: do not setup a recursive bind mount. By default it is recursive. + · from: stage or image name for the root of the source. Defaults to the build context. + Options specific to tmpfs: · tmpfs-size: Size of the tmpfs mount in bytes. Unlimited by default in Linux. @@ -146,6 +148,8 @@ Current supported mount TYPES are bind, cache, secret and tmpfs. · gid: gid for cache directory. + · from: stage name for the root of the source. Defaults to host cache directory. + **RUN Secrets** diff --git a/docs/buildah-run.1.md b/docs/buildah-run.1.md index 22164006e9..47d5038058 100644 --- a/docs/buildah-run.1.md +++ b/docs/buildah-run.1.md @@ -122,7 +122,7 @@ Current supported mount TYPES are bind, cache, secret and tmpfs. [[1]](#Foo Common Options: - · src, source: mount source spec for bind and volume. Mandatory for bind. + · src, source: mount source spec for bind and volume. Mandatory for bind. If `from` is specified, `src` is the subpath in the `from` field. · dst, destination, target: mount destination spec. @@ -134,6 +134,8 @@ Current supported mount TYPES are bind, cache, secret and tmpfs. [[1]](#Foo . bind-nonrecursive: do not setup a recursive bind mount. By default it is recursive. + · from: stage or image name for the root of the source. Defaults to the build context. + Options specific to tmpfs: · tmpfs-size: Size of the tmpfs mount in bytes. Unlimited by default in Linux. @@ -158,6 +160,8 @@ Current supported mount TYPES are bind, cache, secret and tmpfs. [[1]](#Foo · gid: gid for cache directory. + · from: stage name for the root of the source. Defaults to host cache directory. + **--network**, **--net**=*mode* Sets the configuration for the network namespace for the container. diff --git a/imagebuildah/stage_executor.go b/imagebuildah/stage_executor.go index 33abc3edc3..a768148225 100644 --- a/imagebuildah/stage_executor.go +++ b/imagebuildah/stage_executor.go @@ -15,6 +15,7 @@ import ( "github.com/containers/buildah/copier" "github.com/containers/buildah/define" buildahdocker "github.com/containers/buildah/docker" + "github.com/containers/buildah/internal" "github.com/containers/buildah/pkg/rusage" "github.com/containers/buildah/util" cp "github.com/containers/image/v5/copy" @@ -413,10 +414,67 @@ func (s *StageExecutor) Copy(excludes []string, copies ...imagebuilder.Copy) err return nil } +// Returns a map of StageName/ImageName:internal.StageMountDetails for RunOpts if any --mount with from is provided +// Stage can automatically cleanup this mounts when a stage is removed +// check if RUN contains `--mount` with `from`. If yes pre-mount images or stages from executor for Run. +// stages mounted here will we used be Run(). +func (s *StageExecutor) runStageMountPoints(mountList []string) (map[string]internal.StageMountDetails, error) { + stageMountPoints := make(map[string]internal.StageMountDetails) + for _, flag := range mountList { + if strings.Contains(flag, "from") { + arr := strings.SplitN(flag, ",", 2) + if len(arr) < 2 { + return nil, errors.Errorf("Invalid --mount command: %s", flag) + } + tokens := strings.Split(arr[1], ",") + for _, val := range tokens { + kv := strings.SplitN(val, "=", 2) + switch kv[0] { + case "from": + if len(kv) == 1 { + return nil, errors.Errorf("unable to resolve argument for `from=`: bad argument") + } + if kv[1] == "" { + return nil, errors.Errorf("unable to resolve argument for `from=`: from points to an empty value") + } + from, fromErr := imagebuilder.ProcessWord(kv[1], s.stage.Builder.Arguments()) + if fromErr != nil { + return nil, errors.Wrapf(fromErr, "unable to resolve argument %q", kv[1]) + } + // If the source's name corresponds to the + // result of an earlier stage, wait for that + // stage to finish being built. + if isStage, err := s.executor.waitForStage(s.ctx, from, s.stages[:s.index]); isStage && err != nil { + return nil, err + } + if otherStage, ok := s.executor.stages[from]; ok && otherStage.index < s.index { + stageMountPoints[from] = internal.StageMountDetails{IsStage: true, MountPoint: otherStage.mountPoint} + break + } else { + mountPoint, err := s.getImageRootfs(s.ctx, from) + if err != nil { + return nil, errors.Errorf("%s from=%s: no stage or image found with that name", flag, from) + } + stageMountPoints[from] = internal.StageMountDetails{IsStage: false, MountPoint: mountPoint} + break + } + default: + continue + } + } + } + } + return stageMountPoints, nil +} + // Run executes a RUN instruction using the stage's current working container // as a root directory. func (s *StageExecutor) Run(run imagebuilder.Run, config docker.Config) error { logrus.Debugf("RUN %#v, %#v", run, config) + stageMountPoints, err := s.runStageMountPoints(run.Mounts) + if err != nil { + return err + } if s.builder == nil { return errors.Errorf("no build container available") } @@ -451,6 +509,8 @@ func (s *StageExecutor) Run(run imagebuilder.Run, config docker.Config) error { Secrets: s.executor.secrets, SSHSources: s.executor.sshsources, RunMounts: run.Mounts, + StageMountPoints: stageMountPoints, + SystemContext: s.executor.systemContext, } if config.NetworkDisabled { options.ConfigureNetwork = buildah.NetworkDisabled diff --git a/internal/parse/parse.go b/internal/parse/parse.go new file mode 100644 index 0000000000..38a612718f --- /dev/null +++ b/internal/parse/parse.go @@ -0,0 +1,411 @@ +package parse + +import ( + "context" + "fmt" + "os" + "path/filepath" + "strconv" + "strings" + + "github.com/containers/buildah/internal" + internalUtil "github.com/containers/buildah/internal/util" + "github.com/containers/common/pkg/parse" + "github.com/containers/image/v5/types" + "github.com/containers/storage" + "github.com/containers/storage/pkg/idtools" + specs "github.com/opencontainers/runtime-spec/specs-go" + selinux "github.com/opencontainers/selinux/go-selinux" + "github.com/pkg/errors" +) + +const ( + // TypeBind is the type for mounting host dir + TypeBind = "bind" + // TypeTmpfs is the type for mounting tmpfs + TypeTmpfs = "tmpfs" + // TypeCache is the type for mounting a common persistent cache from host + TypeCache = "cache" + // mount=type=cache must create a persistent directory on host so its available for all consecutive builds. + // Lifecycle of following directory will be inherited from how host machine treats temporary directory + BuildahCacheDir = "buildah-cache" +) + +var ( + errBadMntOption = errors.New("invalid mount option") + errBadOptionArg = errors.New("must provide an argument for option") + errBadVolDest = errors.New("must set volume destination") + errBadVolSrc = errors.New("must set volume source") +) + +// GetBindMount parses a single bind mount entry from the --mount flag. +// Returns specifiedMount and a string which contains name of image that we mounted otherwise its empty. +// Caller is expected to perform unmount of any mounted images +func GetBindMount(ctx *types.SystemContext, args []string, contextDir string, store storage.Store, imageMountLabel string, additionalMountPoints map[string]internal.StageMountDetails) (specs.Mount, string, error) { + newMount := specs.Mount{ + Type: TypeBind, + } + + mountReadability := false + setDest := false + bindNonRecursive := false + fromImage := "" + + for _, val := range args { + kv := strings.SplitN(val, "=", 2) + switch kv[0] { + case "bind-nonrecursive": + newMount.Options = append(newMount.Options, "bind") + bindNonRecursive = true + case "ro", "nosuid", "nodev", "noexec": + // TODO: detect duplication of these options. + // (Is this necessary?) + newMount.Options = append(newMount.Options, kv[0]) + mountReadability = true + case "rw", "readwrite": + newMount.Options = append(newMount.Options, "rw") + mountReadability = true + case "readonly": + // Alias for "ro" + newMount.Options = append(newMount.Options, "ro") + mountReadability = true + case "shared", "rshared", "private", "rprivate", "slave", "rslave", "Z", "z", "U": + newMount.Options = append(newMount.Options, kv[0]) + case "from": + if len(kv) == 1 { + return newMount, "", errors.Wrapf(errBadOptionArg, kv[0]) + } + fromImage = kv[1] + case "bind-propagation": + if len(kv) == 1 { + return newMount, "", errors.Wrapf(errBadOptionArg, kv[0]) + } + newMount.Options = append(newMount.Options, kv[1]) + case "src", "source": + if len(kv) == 1 { + return newMount, "", errors.Wrapf(errBadOptionArg, kv[0]) + } + newMount.Source = kv[1] + case "target", "dst", "destination": + if len(kv) == 1 { + return newMount, "", errors.Wrapf(errBadOptionArg, kv[0]) + } + if err := parse.ValidateVolumeCtrDir(kv[1]); err != nil { + return newMount, "", err + } + newMount.Destination = kv[1] + setDest = true + case "consistency": + // Option for OS X only, has no meaning on other platforms + // and can thus be safely ignored. + // See also the handling of the equivalent "delegated" and "cached" in ValidateVolumeOpts + default: + return newMount, "", errors.Wrapf(errBadMntOption, kv[0]) + } + } + + // default mount readability is always readonly + if !mountReadability { + newMount.Options = append(newMount.Options, "ro") + } + + // Following variable ensures that we return imagename only if we did additional mount + isImageMounted := false + if fromImage != "" { + mountPoint := "" + //TODO: remove this selinux check when comment is resolved. https://github.com/containers/buildah/pull/3590#issuecomment-956349109 + if additionalMountPoints != nil && (selinux.EnforceMode() != 1) { + if val, ok := additionalMountPoints[fromImage]; ok { + mountPoint = val.MountPoint + } + } + // if mountPoint of image was not found in additionalMap + // or additionalMap was nil, try mounting image + if mountPoint == "" { + image, err := internalUtil.LookupImage(ctx, store, fromImage) + if err != nil { + return newMount, "", err + } + + mountPoint, err = image.Mount(context.Background(), nil, imageMountLabel) + if err != nil { + return newMount, "", err + } + isImageMounted = true + } + contextDir = mountPoint + } + + // buildkit parity: default bind option must be `rbind` + // unless specified + if !bindNonRecursive { + newMount.Options = append(newMount.Options, "rbind") + } + + if !setDest { + return newMount, fromImage, errBadVolDest + } + + // buildkit parity: support absolute path for sources from current build context + if contextDir != "" { + // path should be /contextDir/specified path + newMount.Source = filepath.Join(contextDir, filepath.Clean(string(filepath.Separator)+newMount.Source)) + } else { + // looks like its coming from `build run --mount=type=bind` allow using absolute path + // error out if no source is set + if newMount.Source == "" { + return newMount, "", errBadVolSrc + } + if err := parse.ValidateVolumeHostDir(newMount.Source); err != nil { + return newMount, "", err + } + } + + opts, err := parse.ValidateVolumeOpts(newMount.Options) + if err != nil { + return newMount, fromImage, err + } + newMount.Options = opts + + if !isImageMounted { + // we don't want any cleanups if image was not mounted explicitly + // so dont return anything + fromImage = "" + } + + return newMount, fromImage, nil +} + +// GetCacheMount parses a single cache mount entry from the --mount flag. +func GetCacheMount(args []string, store storage.Store, imageMountLabel string, additionalMountPoints map[string]internal.StageMountDetails) (specs.Mount, error) { + var err error + var mode uint64 + var ( + setDest bool + setShared bool + setReadOnly bool + ) + fromStage := "" + newMount := specs.Mount{ + Type: TypeBind, + } + // if id is set a new subdirectory with `id` will be created under /host-temp/buildah-build-cache/id + id := "" + //buidkit parity: cache directory defaults to 755 + mode = 0o755 + //buidkit parity: cache directory defaults to uid 0 if not specified + uid := 0 + //buidkit parity: cache directory defaults to gid 0 if not specified + gid := 0 + + for _, val := range args { + kv := strings.SplitN(val, "=", 2) + switch kv[0] { + case "nosuid", "nodev", "noexec": + // TODO: detect duplication of these options. + // (Is this necessary?) + newMount.Options = append(newMount.Options, kv[0]) + case "rw", "readwrite": + newMount.Options = append(newMount.Options, "rw") + case "readonly", "ro": + // Alias for "ro" + newMount.Options = append(newMount.Options, "ro") + setReadOnly = true + case "shared", "rshared", "private", "rprivate", "slave", "rslave", "Z", "z", "U": + newMount.Options = append(newMount.Options, kv[0]) + setShared = true + case "bind-propagation": + if len(kv) == 1 { + return newMount, errors.Wrapf(errBadOptionArg, kv[0]) + } + newMount.Options = append(newMount.Options, kv[1]) + case "id": + if len(kv) == 1 { + return newMount, errors.Wrapf(errBadOptionArg, kv[0]) + } + id = kv[1] + case "from": + if len(kv) == 1 { + return newMount, errors.Wrapf(errBadOptionArg, kv[0]) + } + fromStage = kv[1] + case "target", "dst", "destination": + if len(kv) == 1 { + return newMount, errors.Wrapf(errBadOptionArg, kv[0]) + } + if err := parse.ValidateVolumeCtrDir(kv[1]); err != nil { + return newMount, err + } + newMount.Destination = kv[1] + setDest = true + case "src", "source": + if len(kv) == 1 { + return newMount, errors.Wrapf(errBadOptionArg, kv[0]) + } + newMount.Source = kv[1] + case "mode": + if len(kv) == 1 { + return newMount, errors.Wrapf(errBadOptionArg, kv[0]) + } + mode, err = strconv.ParseUint(kv[1], 8, 32) + if err != nil { + return newMount, errors.Wrapf(err, "Unable to parse cache mode") + } + case "uid": + if len(kv) == 1 { + return newMount, errors.Wrapf(errBadOptionArg, kv[0]) + } + uid, err = strconv.Atoi(kv[1]) + if err != nil { + return newMount, errors.Wrapf(err, "Unable to parse cache uid") + } + case "gid": + if len(kv) == 1 { + return newMount, errors.Wrapf(errBadOptionArg, kv[0]) + } + gid, err = strconv.Atoi(kv[1]) + if err != nil { + return newMount, errors.Wrapf(err, "Unable to parse cache gid") + } + default: + return newMount, errors.Wrapf(errBadMntOption, kv[0]) + } + } + + if !setDest { + return newMount, errBadVolDest + } + + if fromStage != "" { + // do not create cache on host + // instead use read-only mounted stage as cache + mountPoint := "" + //TODO: remove this selinux check when comment is resolved. https://github.com/containers/buildah/pull/3590#issuecomment-956349109 + if additionalMountPoints != nil && (selinux.EnforceMode() != 1) { + if val, ok := additionalMountPoints[fromStage]; ok { + if val.IsStage { + mountPoint = val.MountPoint + } + } + } + // Cache does not supports using image so if not stage found + // return with error + if mountPoint == "" { + return newMount, fmt.Errorf("no stage found with name %s", fromStage) + } + // path should be /contextDir/specified path + newMount.Source = filepath.Join(mountPoint, filepath.Clean(string(filepath.Separator)+newMount.Source)) + } else { + // we need to create cache on host if no image is being used + + // since type is cache and cache can be reused by consecutive builds + // create a common cache directory, which persists on hosts within temp lifecycle + // add subdirectory if specified + + // cache parent directory + cacheParent := filepath.Join(getTempDir(), BuildahCacheDir) + // create cache on host if not present + err = os.MkdirAll(cacheParent, os.FileMode(0755)) + if err != nil { + return newMount, errors.Wrapf(err, "Unable to create build cache directory") + } + + if id != "" { + newMount.Source = filepath.Join(cacheParent, filepath.Clean(id)) + } else { + newMount.Source = filepath.Join(cacheParent, filepath.Clean(newMount.Destination)) + } + idPair := idtools.IDPair{ + UID: uid, + GID: gid, + } + //buildkit parity: change uid and gid if specificed otheriwise keep `0` + err = idtools.MkdirAllAndChownNew(newMount.Source, os.FileMode(mode), idPair) + if err != nil { + return newMount, errors.Wrapf(err, "Unable to change uid,gid of cache directory") + } + } + + // buildkit parity: default sharing should be shared + // unless specified + if !setShared { + newMount.Options = append(newMount.Options, "shared") + } + + // buildkit parity: cache must writable unless `ro` or `readonly` is configured explicitly + if !setReadOnly { + newMount.Options = append(newMount.Options, "rw") + } + + newMount.Options = append(newMount.Options, "bind") + + opts, err := parse.ValidateVolumeOpts(newMount.Options) + if err != nil { + return newMount, err + } + newMount.Options = opts + + return newMount, nil +} + +// GetTmpfsMount parses a single tmpfs mount entry from the --mount flag +func GetTmpfsMount(args []string) (specs.Mount, error) { + newMount := specs.Mount{ + Type: TypeTmpfs, + Source: TypeTmpfs, + } + + setDest := false + + for _, val := range args { + kv := strings.SplitN(val, "=", 2) + switch kv[0] { + case "ro", "nosuid", "nodev", "noexec": + newMount.Options = append(newMount.Options, kv[0]) + case "readonly": + // Alias for "ro" + newMount.Options = append(newMount.Options, "ro") + case "tmpcopyup": + //the path that is shadowed by the tmpfs mount is recursively copied up to the tmpfs itself. + newMount.Options = append(newMount.Options, kv[0]) + case "tmpfs-mode": + if len(kv) == 1 { + return newMount, errors.Wrapf(errBadOptionArg, kv[0]) + } + newMount.Options = append(newMount.Options, fmt.Sprintf("mode=%s", kv[1])) + case "tmpfs-size": + if len(kv) == 1 { + return newMount, errors.Wrapf(errBadOptionArg, kv[0]) + } + newMount.Options = append(newMount.Options, fmt.Sprintf("size=%s", kv[1])) + case "src", "source": + return newMount, errors.Errorf("source is not supported with tmpfs mounts") + case "target", "dst", "destination": + if len(kv) == 1 { + return newMount, errors.Wrapf(errBadOptionArg, kv[0]) + } + if err := parse.ValidateVolumeCtrDir(kv[1]); err != nil { + return newMount, err + } + newMount.Destination = kv[1] + setDest = true + default: + return newMount, errors.Wrapf(errBadMntOption, kv[0]) + } + } + + if !setDest { + return newMount, errBadVolDest + } + + return newMount, nil +} + +/* This is internal function and could be changed at any time */ +/* for external usage please refer to buildah/pkg/parse.GetTempDir() */ +func getTempDir() string { + if tmpdir, ok := os.LookupEnv("TMPDIR"); ok { + return tmpdir + } + return "/var/tmp" +} diff --git a/internal/types.go b/internal/types.go new file mode 100644 index 0000000000..8ddff99fb7 --- /dev/null +++ b/internal/types.go @@ -0,0 +1,11 @@ +package internal + +// Types is internal packages are suspected to change with releases avoid using these outside of buildah + +// StageMountDetails holds the Stage/Image mountpoint returned by StageExecutor +// StageExecutor has ability to mount stages/images in current context and +// automatically clean them up. +type StageMountDetails struct { + IsStage bool // tells if mountpoint returned from stage executor is stage or image + MountPoint string // mountpoint of stage/image +} diff --git a/internal/util/util.go b/internal/util/util.go new file mode 100644 index 0000000000..cce5081674 --- /dev/null +++ b/internal/util/util.go @@ -0,0 +1,24 @@ +package util + +import ( + "github.com/containers/common/libimage" + "github.com/containers/image/v5/types" + "github.com/containers/storage" +) + +// LookupImage returns *Image to corresponding imagename or id +func LookupImage(ctx *types.SystemContext, store storage.Store, image string) (*libimage.Image, error) { + systemContext := ctx + if systemContext == nil { + systemContext = &types.SystemContext{} + } + runtime, err := libimage.RuntimeFromStore(store, &libimage.RuntimeOptions{SystemContext: systemContext}) + if err != nil { + return nil, err + } + localImage, _, err := runtime.LookupImage(image, nil) + if err != nil { + return nil, err + } + return localImage, nil +} diff --git a/pkg/parse/parse.go b/pkg/parse/parse.go index 5c8b56fefd..dc90d441ed 100644 --- a/pkg/parse/parse.go +++ b/pkg/parse/parse.go @@ -15,9 +15,11 @@ import ( "github.com/containerd/containerd/platforms" "github.com/containers/buildah/define" + internalParse "github.com/containers/buildah/internal/parse" "github.com/containers/buildah/pkg/sshagent" "github.com/containers/common/pkg/parse" "github.com/containers/image/v5/types" + "github.com/containers/storage" "github.com/containers/storage/pkg/idtools" "github.com/containers/storage/pkg/unshare" units "github.com/docker/go-units" @@ -46,10 +48,7 @@ const ( ) var ( - errBadMntOption = errors.Errorf("invalid mount option") errDuplicateDest = errors.Errorf("duplicate mount destination") - optionArgError = errors.Errorf("must provide an argument for option") - noDestError = errors.Errorf("must set volume destination") ) // CommonBuildOptions parses the build options from the bud cli @@ -296,18 +295,18 @@ func getVolumeMounts(volumes []string) (map[string]specs.Mount, error) { } // GetVolumes gets the volumes from --volume and --mount -func GetVolumes(volumes []string, mounts []string, contextDir string) ([]specs.Mount, error) { - unifiedMounts, err := getMounts(mounts, contextDir) +func GetVolumes(ctx *types.SystemContext, store storage.Store, volumes []string, mounts []string, contextDir string) ([]specs.Mount, []string, error) { + unifiedMounts, mountedImages, err := getMounts(ctx, store, mounts, contextDir) if err != nil { - return nil, err + return nil, mountedImages, err } volumeMounts, err := getVolumeMounts(volumes) if err != nil { - return nil, err + return nil, mountedImages, err } for dest, mount := range volumeMounts { if _, ok := unifiedMounts[dest]; ok { - return nil, errors.Wrapf(errDuplicateDest, dest) + return nil, mountedImages, errors.Wrapf(errDuplicateDest, dest) } unifiedMounts[dest] = mount } @@ -316,15 +315,16 @@ func GetVolumes(volumes []string, mounts []string, contextDir string) ([]specs.M for _, mount := range unifiedMounts { finalMounts = append(finalMounts, mount) } - return finalMounts, nil + return finalMounts, mountedImages, nil } // getMounts takes user-provided input from the --mount flag and creates OCI // spec mounts. // buildah run --mount type=bind,src=/etc/resolv.conf,target=/etc/resolv.conf ... // buildah run --mount type=tmpfs,target=/dev/shm ... -func getMounts(mounts []string, contextDir string) (map[string]specs.Mount, error) { +func getMounts(ctx *types.SystemContext, store storage.Store, mounts []string, contextDir string) (map[string]specs.Mount, []string, error) { finalMounts := make(map[string]specs.Mount) + mountedImages := make([]string, 0) errInvalidSyntax := errors.Errorf("incorrect mount format: should be --mount type=,[src=,]target=[,options]") @@ -334,304 +334,51 @@ func getMounts(mounts []string, contextDir string) (map[string]specs.Mount, erro for _, mount := range mounts { arr := strings.SplitN(mount, ",", 2) if len(arr) < 2 { - return nil, errors.Wrapf(errInvalidSyntax, "%q", mount) + return nil, mountedImages, errors.Wrapf(errInvalidSyntax, "%q", mount) } kv := strings.Split(arr[0], "=") // TODO: type is not explicitly required in Docker. // If not specified, it defaults to "volume". if len(kv) != 2 || kv[0] != "type" { - return nil, errors.Wrapf(errInvalidSyntax, "%q", mount) + return nil, mountedImages, errors.Wrapf(errInvalidSyntax, "%q", mount) } tokens := strings.Split(arr[1], ",") switch kv[1] { case TypeBind: - mount, err := GetBindMount(tokens, contextDir) + mount, image, err := internalParse.GetBindMount(ctx, tokens, contextDir, store, "", nil) if err != nil { - return nil, err + return nil, mountedImages, err } if _, ok := finalMounts[mount.Destination]; ok { - return nil, errors.Wrapf(errDuplicateDest, mount.Destination) + return nil, mountedImages, errors.Wrapf(errDuplicateDest, mount.Destination) } finalMounts[mount.Destination] = mount + mountedImages = append(mountedImages, image) case TypeCache: - mount, err := GetCacheMount(tokens) + mount, err := internalParse.GetCacheMount(tokens, store, "", nil) if err != nil { - return nil, err + return nil, mountedImages, err } if _, ok := finalMounts[mount.Destination]; ok { - return nil, errors.Wrapf(errDuplicateDest, mount.Destination) + return nil, mountedImages, errors.Wrapf(errDuplicateDest, mount.Destination) } finalMounts[mount.Destination] = mount case TypeTmpfs: - mount, err := GetTmpfsMount(tokens) + mount, err := internalParse.GetTmpfsMount(tokens) if err != nil { - return nil, err + return nil, mountedImages, err } if _, ok := finalMounts[mount.Destination]; ok { - return nil, errors.Wrapf(errDuplicateDest, mount.Destination) + return nil, mountedImages, errors.Wrapf(errDuplicateDest, mount.Destination) } finalMounts[mount.Destination] = mount default: - return nil, errors.Errorf("invalid filesystem type %q", kv[1]) + return nil, mountedImages, errors.Errorf("invalid filesystem type %q", kv[1]) } } - return finalMounts, nil -} - -// GetBindMount parses a single bind mount entry from the --mount flag. -func GetBindMount(args []string, contextDir string) (specs.Mount, error) { - newMount := specs.Mount{ - Type: TypeBind, - } - - setDest := false - bindNonRecursive := false - - for _, val := range args { - kv := strings.SplitN(val, "=", 2) - switch kv[0] { - case "bind-nonrecursive": - newMount.Options = append(newMount.Options, "bind") - bindNonRecursive = true - case "ro", "nosuid", "nodev", "noexec": - // TODO: detect duplication of these options. - // (Is this necessary?) - newMount.Options = append(newMount.Options, kv[0]) - case "rw", "readwrite": - newMount.Options = append(newMount.Options, "rw") - case "readonly": - // Alias for "ro" - newMount.Options = append(newMount.Options, "ro") - case "shared", "rshared", "private", "rprivate", "slave", "rslave", "Z", "z", "U": - newMount.Options = append(newMount.Options, kv[0]) - case "bind-propagation": - if len(kv) == 1 { - return newMount, errors.Wrapf(optionArgError, kv[0]) - } - newMount.Options = append(newMount.Options, kv[1]) - case "src", "source": - if len(kv) == 1 { - return newMount, errors.Wrapf(optionArgError, kv[0]) - } - if err := parse.ValidateVolumeHostDir(kv[1]); err != nil { - return newMount, err - } - newMount.Source = kv[1] - case "target", "dst", "destination": - if len(kv) == 1 { - return newMount, errors.Wrapf(optionArgError, kv[0]) - } - if err := parse.ValidateVolumeCtrDir(kv[1]); err != nil { - return newMount, err - } - newMount.Destination = kv[1] - setDest = true - case "consistency": - // Option for OS X only, has no meaning on other platforms - // and can thus be safely ignored. - // See also the handling of the equivalent "delegated" and "cached" in ValidateVolumeOpts - default: - return newMount, errors.Wrapf(errBadMntOption, kv[0]) - } - } - - // buildkit parity: default bind option must be `rbind` - // unless specified - if !bindNonRecursive { - newMount.Options = append(newMount.Options, "rbind") - } - - if !setDest { - return newMount, noDestError - } - - // buildkit parity: support absolute path for sources from current build context - if strings.HasPrefix(newMount.Source, ".") || newMount.Source == "" || !filepath.IsAbs(newMount.Source) { - // path should be /contextDir/specified path - newMount.Source = filepath.Join(contextDir, filepath.Clean(string(filepath.Separator)+newMount.Source)) - } - - opts, err := parse.ValidateVolumeOpts(newMount.Options) - if err != nil { - return newMount, err - } - newMount.Options = opts - - return newMount, nil -} - -// GetCacheMount parses a single cache mount entry from the --mount flag. -func GetCacheMount(args []string) (specs.Mount, error) { - var err error - var ( - setDest bool - setShared bool - setReadOnly bool - ) - newMount := specs.Mount{ - Type: TypeBind, - } - // if id is set a new subdirectory with `id` will be created under /host-temp/buildah-build-cache/id - id := "" - //buidkit parity: cache directory defaults to 755 - mode := 0755 - //buidkit parity: cache directory defaults to uid 0 if not specified - uid := 0 - //buidkit parity: cache directory defaults to gid 0 if not specified - gid := 0 - - for _, val := range args { - kv := strings.SplitN(val, "=", 2) - switch kv[0] { - case "nosuid", "nodev", "noexec": - // TODO: detect duplication of these options. - // (Is this necessary?) - newMount.Options = append(newMount.Options, kv[0]) - case "rw", "readwrite": - newMount.Options = append(newMount.Options, "rw") - case "readonly", "ro": - // Alias for "ro" - newMount.Options = append(newMount.Options, "ro") - setReadOnly = true - case "shared", "rshared", "private", "rprivate", "slave", "rslave", "Z", "z", "U": - newMount.Options = append(newMount.Options, kv[0]) - setShared = true - case "bind-propagation": - if len(kv) == 1 { - return newMount, errors.Wrapf(optionArgError, kv[0]) - } - newMount.Options = append(newMount.Options, kv[1]) - case "id": - id = kv[1] - case "target", "dst", "destination": - if len(kv) == 1 { - return newMount, errors.Wrapf(optionArgError, kv[0]) - } - if err := parse.ValidateVolumeCtrDir(kv[1]); err != nil { - return newMount, err - } - newMount.Destination = kv[1] - setDest = true - case "mode": - mode, err = strconv.Atoi(kv[1]) - if err != nil { - return newMount, errors.Wrapf(err, "Unable to parse cache mode") - } - case "uid": - uid, err = strconv.Atoi(kv[1]) - if err != nil { - return newMount, errors.Wrapf(err, "Unable to parse cache uid") - } - case "gid": - gid, err = strconv.Atoi(kv[1]) - if err != nil { - return newMount, errors.Wrapf(err, "Unable to parse cache gid") - } - default: - return newMount, errors.Wrapf(errBadMntOption, kv[0]) - } - } - - if !setDest { - return newMount, noDestError - } - - // since type is cache and cache can be resused by consecutive builds - // create a common cache directory, which should persists on hosts within temp lifecycle - // add subdirectory if specified - if id != "" { - newMount.Source = filepath.Join(GetTempDir(), BuildahCacheDir, id) - } else { - newMount.Source = filepath.Join(GetTempDir(), BuildahCacheDir) - } - // create cache on host if not present - err = os.MkdirAll(newMount.Source, os.FileMode(mode)) - if err != nil { - return newMount, errors.Wrapf(err, "Unable to create build cache directory") - } - //buidkit parity: change uid and gid if specificed otheriwise keep `0` - err = os.Chown(newMount.Source, uid, gid) - if err != nil { - return newMount, errors.Wrapf(err, "Unable to change uid,gid of cache directory") - } - - // buildkit parity: default sharing should be shared - // unless specified - if !setShared { - newMount.Options = append(newMount.Options, "shared") - } - - // buildkit parity: cache must writable unless `ro` or `readonly` is configured explicitly - if !setReadOnly { - newMount.Options = append(newMount.Options, "rw") - } - - // buildkit parity: default bind option for cache must be `rbind` - // since we are actually looking for arbitrary content under cache directory - newMount.Options = append(newMount.Options, "rbind") - - opts, err := parse.ValidateVolumeOpts(newMount.Options) - if err != nil { - return newMount, err - } - newMount.Options = opts - - return newMount, nil -} - -// GetTmpfsMount parses a single tmpfs mount entry from the --mount flag -func GetTmpfsMount(args []string) (specs.Mount, error) { - newMount := specs.Mount{ - Type: TypeTmpfs, - Source: TypeTmpfs, - } - - setDest := false - - for _, val := range args { - kv := strings.SplitN(val, "=", 2) - switch kv[0] { - case "ro", "nosuid", "nodev", "noexec": - newMount.Options = append(newMount.Options, kv[0]) - case "readonly": - // Alias for "ro" - newMount.Options = append(newMount.Options, "ro") - case "tmpcopyup": - //the path that is shadowed by the tmpfs mount is recursively copied up to the tmpfs itself. - newMount.Options = append(newMount.Options, kv[0]) - case "tmpfs-mode": - if len(kv) == 1 { - return newMount, errors.Wrapf(optionArgError, kv[0]) - } - newMount.Options = append(newMount.Options, fmt.Sprintf("mode=%s", kv[1])) - case "tmpfs-size": - if len(kv) == 1 { - return newMount, errors.Wrapf(optionArgError, kv[0]) - } - newMount.Options = append(newMount.Options, fmt.Sprintf("size=%s", kv[1])) - case "src", "source": - return newMount, errors.Errorf("source is not supported with tmpfs mounts") - case "target", "dst", "destination": - if len(kv) == 1 { - return newMount, errors.Wrapf(optionArgError, kv[0]) - } - if err := parse.ValidateVolumeCtrDir(kv[1]); err != nil { - return newMount, err - } - newMount.Destination = kv[1] - setDest = true - default: - return newMount, errors.Wrapf(errBadMntOption, kv[0]) - } - } - - if !setDest { - return newMount, noDestError - } - - return newMount, nil + return finalMounts, mountedImages, nil } // ValidateVolumeHostDir validates a volume mount's source directory diff --git a/run.go b/run.go index 8264273e10..fedeb6dc52 100644 --- a/run.go +++ b/run.go @@ -5,7 +5,9 @@ import ( "io" "github.com/containers/buildah/define" + "github.com/containers/buildah/internal" "github.com/containers/buildah/pkg/sshagent" + "github.com/containers/image/v5/types" "github.com/opencontainers/runtime-spec/specs-go" "github.com/sirupsen/logrus" ) @@ -147,6 +149,14 @@ type RunOptions struct { // RunMounts are mounts for this run. RunMounts for this run // will not show up in subsequent runs. RunMounts []string + // Map of stages and container mountpoint if any from stage executor + StageMountPoints map[string]internal.StageMountDetails + // External Image mounts to be cleaned up. + // Buildah run --mount could mount image before RUN calls, RUN could cleanup + // them up as well + ExternalImageMounts []string + // System context of current build + SystemContext *types.SystemContext } // RunMountArtifacts are the artifacts created when using a run mount. @@ -155,6 +165,8 @@ type runMountArtifacts struct { RunMountTargets []string // TmpFiles are artifacts that need to be removed outside the container TmpFiles []string + // Any external images which were mounted inside container + MountedImages []string // Agents are the ssh agents started Agents []*sshagent.AgentServer // SSHAuthSock is the path to the ssh auth sock inside the container diff --git a/run_linux.go b/run_linux.go index 0b94b3b1fe..a70c7dc396 100644 --- a/run_linux.go +++ b/run_linux.go @@ -24,6 +24,9 @@ import ( "github.com/containers/buildah/chroot" "github.com/containers/buildah/copier" "github.com/containers/buildah/define" + "github.com/containers/buildah/internal" + internalParse "github.com/containers/buildah/internal/parse" + internalUtil "github.com/containers/buildah/internal/util" "github.com/containers/buildah/pkg/overlay" "github.com/containers/buildah/pkg/parse" "github.com/containers/buildah/pkg/sshagent" @@ -35,12 +38,14 @@ import ( "github.com/containers/common/pkg/chown" "github.com/containers/common/pkg/config" "github.com/containers/common/pkg/subscriptions" + imagetypes "github.com/containers/image/v5/types" "github.com/containers/storage" "github.com/containers/storage/pkg/idtools" "github.com/containers/storage/pkg/ioutils" "github.com/containers/storage/pkg/reexec" "github.com/containers/storage/pkg/stringid" "github.com/containers/storage/pkg/unshare" + storagetypes "github.com/containers/storage/types" "github.com/docker/go-units" "github.com/docker/libnetwork/resolvconf" "github.com/docker/libnetwork/types" @@ -250,7 +255,7 @@ rootless=%d bindFiles["/run/.containerenv"] = containerenvPath } - runArtifacts, err := b.setupMounts(mountPoint, spec, path, options.Mounts, bindFiles, volumes, b.CommonBuildOpts.Volumes, b.CommonBuildOpts.ShmSize, namespaceOptions, options.Secrets, options.SSHSources, options.RunMounts, options.ContextDir) + runArtifacts, err := b.setupMounts(options.SystemContext, mountPoint, spec, path, options.Mounts, bindFiles, volumes, b.CommonBuildOpts.Volumes, b.CommonBuildOpts.ShmSize, namespaceOptions, options.Secrets, options.SSHSources, options.RunMounts, options.ContextDir, options.StageMountPoints) if err != nil { return errors.Wrapf(err, "error resolving mountpoints for container %q", b.ContainerID) } @@ -259,9 +264,16 @@ rootless=%d spec.Process.Env = append(spec.Process.Env, sshenv) } + // following run was called from `buildah run` + // and some images were mounted for this run + // add them to cleanup artifacts + if len(options.ExternalImageMounts) > 0 { + runArtifacts.MountedImages = append(runArtifacts.MountedImages, options.ExternalImageMounts...) + } + defer func() { - if err := cleanupRunMounts(mountPoint, runArtifacts); err != nil { - options.Logger.Errorf("unabe to cleanup run mounts %v", err) + if err := b.cleanupRunMounts(options.SystemContext, mountPoint, runArtifacts); err != nil { + options.Logger.Errorf("unable to cleanup run mounts %v", err) } }() @@ -416,7 +428,7 @@ func runSetupBuiltinVolumes(mountLabel, mountPoint, containerDir string, builtin return mounts, nil } -func (b *Builder) setupMounts(mountPoint string, spec *specs.Spec, bundlePath string, optionMounts []specs.Mount, bindFiles map[string]string, builtinVolumes, volumeMounts []string, shmSize string, namespaceOptions define.NamespaceOptions, secrets map[string]define.Secret, sshSources map[string]*sshagent.Source, runFileMounts []string, contextDir string) (*runMountArtifacts, error) { +func (b *Builder) setupMounts(context *imagetypes.SystemContext, mountPoint string, spec *specs.Spec, bundlePath string, optionMounts []specs.Mount, bindFiles map[string]string, builtinVolumes, volumeMounts []string, shmSize string, namespaceOptions define.NamespaceOptions, secrets map[string]define.Secret, sshSources map[string]*sshagent.Source, runFileMounts []string, contextDir string, stageMountPoints map[string]internal.StageMountDetails) (*runMountArtifacts, error) { // Start building a new list of mounts. var mounts []specs.Mount haveMount := func(destination string) bool { @@ -531,7 +543,7 @@ func (b *Builder) setupMounts(mountPoint string, spec *specs.Spec, bundlePath st // Get the list of mounts that are just for this Run() call. // TODO: acui: de-spaghettify run mounts - runMounts, mountArtifacts, err := b.runSetupRunMounts(runFileMounts, secrets, sshSources, cdir, contextDir, spec.Linux.UIDMappings, spec.Linux.GIDMappings, int(rootUID), int(rootGID), int(processUID), int(processGID)) + runMounts, mountArtifacts, err := b.runSetupRunMounts(context, runFileMounts, secrets, stageMountPoints, sshSources, cdir, contextDir, spec.Linux.UIDMappings, spec.Linux.GIDMappings, int(rootUID), int(rootGID), int(processUID), int(processGID)) if err != nil { return nil, err } @@ -2407,9 +2419,10 @@ func init() { } // runSetupRunMounts sets up mounts that exist only in this RUN, not in subsequent runs -func (b *Builder) runSetupRunMounts(mounts []string, secrets map[string]define.Secret, sshSources map[string]*sshagent.Source, containerWorkingDir string, contextDir string, uidmap []spec.LinuxIDMapping, gidmap []spec.LinuxIDMapping, rootUID int, rootGID int, processUID int, processGID int) ([]spec.Mount, *runMountArtifacts, error) { - mountTargets := make([]string, 0, len(mounts)) +func (b *Builder) runSetupRunMounts(context *imagetypes.SystemContext, mounts []string, secrets map[string]define.Secret, stageMountPoints map[string]internal.StageMountDetails, sshSources map[string]*sshagent.Source, containerWorkingDir string, contextDir string, uidmap []spec.LinuxIDMapping, gidmap []spec.LinuxIDMapping, rootUID int, rootGID int, processUID int, processGID int) ([]spec.Mount, *runMountArtifacts, error) { + mountTargets := make([]string, 0, 10) tmpFiles := make([]string, 0, len(mounts)) + mountImages := make([]string, 0, 10) finalMounts := make([]specs.Mount, 0, len(mounts)) agents := make([]*sshagent.AgentServer, 0, len(mounts)) sshCount := 0 @@ -2455,12 +2468,16 @@ func (b *Builder) runSetupRunMounts(mounts []string, secrets map[string]define.S sshCount++ } case "bind": - mount, err := b.getBindMount(tokens, contextDir, rootUID, rootGID, processUID, processGID) + mount, image, err := b.getBindMount(context, tokens, contextDir, rootUID, rootGID, processUID, processGID, stageMountPoints) if err != nil { return nil, nil, err } finalMounts = append(finalMounts, *mount) mountTargets = append(mountTargets, mount.Destination) + // only perform cleanup if image was mounted ignore everything else + if image != "" { + mountImages = append(mountImages, image) + } case "tmpfs": mount, err := b.getTmpfsMount(tokens, rootUID, rootGID, processUID, processGID) if err != nil { @@ -2469,7 +2486,7 @@ func (b *Builder) runSetupRunMounts(mounts []string, secrets map[string]define.S finalMounts = append(finalMounts, *mount) mountTargets = append(mountTargets, mount.Destination) case "cache": - mount, err := b.getCacheMount(tokens, rootUID, rootGID, processUID, processGID) + mount, err := b.getCacheMount(tokens, rootUID, rootGID, processUID, processGID, stageMountPoints) if err != nil { return nil, nil, err } @@ -2483,31 +2500,32 @@ func (b *Builder) runSetupRunMounts(mounts []string, secrets map[string]define.S RunMountTargets: mountTargets, TmpFiles: tmpFiles, Agents: agents, + MountedImages: mountImages, SSHAuthSock: defaultSSHSock, } return finalMounts, artifacts, nil } -func (b *Builder) getBindMount(tokens []string, contextDir string, rootUID, rootGID, processUID, processGID int) (*spec.Mount, error) { +func (b *Builder) getBindMount(context *imagetypes.SystemContext, tokens []string, contextDir string, rootUID, rootGID, processUID, processGID int, stageMountPoints map[string]internal.StageMountDetails) (*spec.Mount, string, error) { if contextDir == "" { - return nil, errors.New("Context Directory for current run invocation is not configured") + return nil, "", errors.New("Context Directory for current run invocation is not configured") } var optionMounts []specs.Mount - mount, err := parse.GetBindMount(tokens, contextDir) + mount, image, err := internalParse.GetBindMount(context, tokens, contextDir, b.store, b.MountLabel, stageMountPoints) if err != nil { - return nil, err + return nil, image, err } optionMounts = append(optionMounts, mount) volumes, err := b.runSetupVolumeMounts(b.MountLabel, nil, optionMounts, rootUID, rootGID, processUID, processGID) if err != nil { - return nil, err + return nil, image, err } - return &volumes[0], nil + return &volumes[0], image, nil } func (b *Builder) getTmpfsMount(tokens []string, rootUID, rootGID, processUID, processGID int) (*spec.Mount, error) { var optionMounts []specs.Mount - mount, err := parse.GetTmpfsMount(tokens) + mount, err := internalParse.GetTmpfsMount(tokens) if err != nil { return nil, err } @@ -2519,9 +2537,9 @@ func (b *Builder) getTmpfsMount(tokens []string, rootUID, rootGID, processUID, p return &volumes[0], nil } -func (b *Builder) getCacheMount(tokens []string, rootUID, rootGID, processUID, processGID int) (*spec.Mount, error) { +func (b *Builder) getCacheMount(tokens []string, rootUID, rootGID, processUID, processGID int, stageMountPoints map[string]internal.StageMountDetails) (*spec.Mount, error) { var optionMounts []specs.Mount - mount, err := parse.GetCacheMount(tokens) + mount, err := internalParse.GetCacheMount(tokens, b.store, b.MountLabel, stageMountPoints) if err != nil { return nil, err } @@ -2767,7 +2785,7 @@ func (b *Builder) getSSHMount(tokens []string, count int, sshsources map[string] } // cleanupRunMounts cleans up run mounts so they only appear in this run. -func cleanupRunMounts(mountpoint string, artifacts *runMountArtifacts) error { +func (b *Builder) cleanupRunMounts(context *imagetypes.SystemContext, mountpoint string, artifacts *runMountArtifacts) error { for _, agent := range artifacts.Agents { err := agent.Shutdown() if err != nil { @@ -2775,6 +2793,25 @@ func cleanupRunMounts(mountpoint string, artifacts *runMountArtifacts) error { } } + //cleanup any mounted images for this run + for _, image := range artifacts.MountedImages { + if image != "" { + // if flow hits here some image was mounted for this run + i, err := internalUtil.LookupImage(context, b.store, image) + if err == nil { + // silently try to unmount and do nothing + // if image is being used by something else + _ = i.Unmount(false) + } + if errors.Cause(err) == storagetypes.ErrImageUnknown { + // Ignore only if ErrImageUnknown + // Reason: Image is already unmounted do nothing + continue + } + return err + } + } + opts := copier.RemoveOptions{ All: true, } diff --git a/tests/bud.bats b/tests/bud.bats index 3dedc5eae0..12d95f539f 100644 --- a/tests/bud.bats +++ b/tests/bud.bats @@ -3743,3 +3743,176 @@ _EOF die "Unexpected bogus environment." fi } + +@test "bud-with-mount-bind-from-like-buildkit" { + skip_if_no_runtime + skip_if_in_container + mkdir ${TESTDIR}/bud + cp -R ${TESTSDIR}/bud/buildkit-mount-from ${TESTDIR}/bud/buildkit-mount-from + # build base image which we will use as our `from` + run_buildah build -t buildkitbase --signature-policy ${TESTSDIR}/policy.json -f ${TESTDIR}/bud/buildkit-mount-from/Dockerfilebuildkitbase ${TESTDIR}/bud/buildkit-mount-from/ + # try reading something from another image in a different build + run_buildah build -t testbud --signature-policy ${TESTSDIR}/policy.json -f ${TESTDIR}/bud/buildkit-mount-from/Dockerfilebindfrom + expect_output --substring "hello" + run_buildah rmi -f buildkitbase + run_buildah rmi -f testbud +} + +@test "bud-with-writeable-mount-bind-from-like-buildkit" { + if which selinuxenabled > /dev/null 2> /dev/null ; then + if selinuxenabled ; then + #TODO: Once pending commit from https://github.com/containers/buildah/pull/3590 is merged + #See comment: https://github.com/containers/buildah/pull/3590#issuecomment-956349109 + skip "skip if selinux enabled, since stages have different selinux label" + fi + fi + skip_if_no_runtime + skip_if_in_container + mkdir ${TESTDIR}/bud + cp -R ${TESTSDIR}/bud/buildkit-mount-from ${TESTDIR}/bud/buildkit-mount-from + # build base image which we will use as our `from` + run_buildah build -t buildkitbase --signature-policy ${TESTSDIR}/policy.json -f ${TESTDIR}/bud/buildkit-mount-from/Dockerfilebuildkitbase ${TESTDIR}/bud/buildkit-mount-from/ + # try reading something from another image in a different build + run_buildah build -t testbud --signature-policy ${TESTSDIR}/policy.json -f ${TESTDIR}/bud/buildkit-mount-from/Dockerfilebindfromwriteable + expect_output --substring "world" + run_buildah rmi -f buildkitbase + run_buildah rmi -f testbud +} + +@test "bud-with-mount-bind-from-without-source-like-buildkit" { + skip_if_no_runtime + skip_if_in_container + mkdir ${TESTDIR}/bud + cp -R ${TESTSDIR}/bud/buildkit-mount-from ${TESTDIR}/bud/buildkit-mount-from + # build base image which we will use as our `from` + run_buildah build -t buildkitbase --signature-policy ${TESTSDIR}/policy.json -f ${TESTDIR}/bud/buildkit-mount-from/Dockerfilebuildkitbase ${TESTDIR}/bud/buildkit-mount-from/ + # try reading something from another image in a different build + run_buildah build -t testbud --signature-policy ${TESTSDIR}/policy.json -f ${TESTDIR}/bud/buildkit-mount-from/Dockerfilebindfromwithoutsource + expect_output --substring "hello" + run_buildah rmi -f buildkitbase + run_buildah rmi -f testbud +} + +@test "bud-with-mount-bind-from-with-empty-from-like-buildkit" { + skip_if_no_runtime + skip_if_in_container + mkdir ${TESTDIR}/bud + cp -R ${TESTSDIR}/bud/buildkit-mount-from ${TESTDIR}/bud/buildkit-mount-from + # build base image which we will use as our `from` + run_buildah build -t buildkitbase --signature-policy ${TESTSDIR}/policy.json -f ${TESTDIR}/bud/buildkit-mount-from/Dockerfilebuildkitbase ${TESTDIR}/bud/buildkit-mount-from/ + # try reading something from image in a different build + run_buildah 125 build -t testbud --signature-policy ${TESTSDIR}/policy.json -f ${TESTDIR}/bud/buildkit-mount-from/Dockerfilebindfromwithemptyfrom + expect_output --substring "points to an empty value" + run_buildah rmi -f buildkitbase +} + +@test "bud-with-mount-cache-from-like-buildkit" { + if which selinuxenabled > /dev/null 2> /dev/null ; then + if selinuxenabled ; then + #TODO: Once pending commit from https://github.com/containers/buildah/pull/3590 is merged + #See comment: https://github.com/containers/buildah/pull/3590#issuecomment-956349109 + skip "skip if selinux enabled, since stages have different selinux label" + fi + fi + skip_if_no_runtime + skip_if_in_container + mkdir ${TESTDIR}/bud + cp -R ${TESTSDIR}/bud/buildkit-mount-from ${TESTDIR}/bud/buildkit-mount-from + # try reading something from persistant cache in a different build + run_buildah build -t testbud --signature-policy ${TESTSDIR}/policy.json -f ${TESTDIR}/bud/buildkit-mount-from/Dockerfilecachefrom ${TESTDIR}/bud/buildkit-mount-from/ + expect_output --substring "hello" + run_buildah rmi -f testbud +} + +# following test must fail +@test "bud-with-mount-cache-image-from-like-buildkit" { + if which selinuxenabled > /dev/null 2> /dev/null ; then + if selinuxenabled ; then + #TODO: Once pending commit from https://github.com/containers/buildah/pull/3590 is merged + #See comment: https://github.com/containers/buildah/pull/3590#issuecomment-956349109 + skip "skip if selinux enabled, since stages have different selinux label" + fi + fi + skip_if_no_runtime + skip_if_in_container + mkdir ${TESTDIR}/bud + cp -R ${TESTSDIR}/bud/buildkit-mount-from ${TESTDIR}/bud/buildkit-mount-from + # build base image which we will use as our `from` + run_buildah build -t buildkitbase --signature-policy ${TESTSDIR}/policy.json -f ${TESTDIR}/bud/buildkit-mount-from/Dockerfilebuildkitbase ${TESTDIR}/bud/buildkit-mount-from/ + # try reading something from persistant cache in a different build + run_buildah 125 build -t testbud --signature-policy ${TESTSDIR}/policy.json -f ${TESTDIR}/bud/buildkit-mount-from/Dockerfilecachefromimage + expect_output --substring "hello" + run_buildah rmi -f buildkitbase +} + +@test "bud-with-mount-cache-multiple-from-like-buildkit" { + if which selinuxenabled > /dev/null 2> /dev/null ; then + if selinuxenabled ; then + #TODO: Once pending commit from https://github.com/containers/buildah/pull/3590 is merged + #See comment: https://github.com/containers/buildah/pull/3590#issuecomment-956349109 + skip "skip if selinux enabled, since stages have different selinux label" + fi + fi + skip_if_no_runtime + skip_if_in_container + mkdir ${TESTDIR}/bud + cp -R ${TESTSDIR}/bud/buildkit-mount-from ${TESTDIR}/bud/buildkit-mount-from + # try reading something from persistant cache in a different build + run_buildah build -t testbud --signature-policy ${TESTSDIR}/policy.json -f ${TESTDIR}/bud/buildkit-mount-from/Dockerfilecachemultiplefrom ${TESTDIR}/bud/buildkit-mount-from/ + expect_output --substring "hello" + expect_output --substring "hello2" + run_buildah rmi -f testbud +} + +@test "bud-with-mount-bind-from-relative-like-buildkit" { + skip_if_no_runtime + skip_if_in_container + mkdir ${TESTDIR}/bud + cp -R ${TESTSDIR}/bud/buildkit-mount-from ${TESTDIR}/bud/buildkit-mount-from + # build base image which we will use as our `from` + run_buildah build -t buildkitbaserelative --signature-policy ${TESTSDIR}/policy.json -f ${TESTDIR}/bud/buildkit-mount-from/Dockerfilebuildkitbaserelative ${TESTDIR}/bud/buildkit-mount-from/ + # try reading something from image in a different build + run_buildah build -t testbud --signature-policy ${TESTSDIR}/policy.json -f ${TESTDIR}/bud/buildkit-mount-from/Dockerfilebindfromrelative + expect_output --substring "hello" + run_buildah rmi -f buildkitbaserelative + run_buildah rmi -f testbud +} + +@test "bud-with-mount-bind-from-multistage-relative-like-buildkit" { + if which selinuxenabled > /dev/null 2> /dev/null ; then + if selinuxenabled ; then + #TODO: Once pending commit from https://github.com/containers/buildah/pull/3590 is merged + #See comment: https://github.com/containers/buildah/pull/3590#issuecomment-956349109 + skip "skip if selinux enabled, since stages have different selinux label" + fi + fi + + mkdir ${TESTDIR}/bud + cp -R ${TESTSDIR}/bud/buildkit-mount-from ${TESTDIR}/bud/buildkit-mount-from + skip_if_no_runtime + skip_if_in_container + # build base image which we will use as our `from` + run_buildah build -t testbud --signature-policy ${TESTSDIR}/policy.json -f ${TESTDIR}/bud/buildkit-mount-from/Dockerfilemultistagefrom ${TESTDIR}/bud/buildkit-mount-from/ + expect_output --substring "hello" + run_buildah rmi -f testbud +} + +@test "bud-with-mount-bind-from-cache-multistage-relative-like-buildkit" { + if which selinuxenabled > /dev/null 2> /dev/null ; then + if selinuxenabled ; then + #TODO: Once pending commit from https://github.com/containers/buildah/pull/3590 is merged + #See comment: https://github.com/containers/buildah/pull/3590#issuecomment-956349109 + skip "skip if selinux enabled, since stages have different selinux label" + fi + fi + + skip_if_no_runtime + skip_if_in_container + mkdir ${TESTDIR}/bud + cp -R ${TESTSDIR}/bud/buildkit-mount-from ${TESTDIR}/bud/buildkit-mount-from + # build base image which we will use as our `from` + run_buildah build -t testbud --signature-policy ${TESTSDIR}/policy.json -f ${TESTDIR}/bud/buildkit-mount-from/Dockerfilemultistagefromcache ${TESTDIR}/bud/buildkit-mount-from/ + expect_output --substring "hello" + expect_output --substring "hello2" + run_buildah rmi -f testbud +} diff --git a/tests/bud/buildkit-mount-from/Dockerfilebindfrom b/tests/bud/buildkit-mount-from/Dockerfilebindfrom new file mode 100644 index 0000000000..e81ee4e8da --- /dev/null +++ b/tests/bud/buildkit-mount-from/Dockerfilebindfrom @@ -0,0 +1,3 @@ +FROM alpine +# use from= as mount source +RUN --mount=type=bind,source=.,from=buildkitbase,target=/test cat /test/hello diff --git a/tests/bud/buildkit-mount-from/Dockerfilebindfromrelative b/tests/bud/buildkit-mount-from/Dockerfilebindfromrelative new file mode 100644 index 0000000000..c421a0cc47 --- /dev/null +++ b/tests/bud/buildkit-mount-from/Dockerfilebindfromrelative @@ -0,0 +1,4 @@ +FROM alpine +RUN mkdir /test +# use from= as mount source +RUN --mount=type=bind,source=subdir,from=buildkitbaserelative,target=/test cat /test/hello diff --git a/tests/bud/buildkit-mount-from/Dockerfilebindfromwithemptyfrom b/tests/bud/buildkit-mount-from/Dockerfilebindfromwithemptyfrom new file mode 100644 index 0000000000..5fc867f85a --- /dev/null +++ b/tests/bud/buildkit-mount-from/Dockerfilebindfromwithemptyfrom @@ -0,0 +1,4 @@ +FROM alpine +RUN mkdir /test +# use from= as mount source +RUN --mount=type=bind,source=.,from=,target=/test cat /test/hello diff --git a/tests/bud/buildkit-mount-from/Dockerfilebindfromwithoutsource b/tests/bud/buildkit-mount-from/Dockerfilebindfromwithoutsource new file mode 100644 index 0000000000..3dee4e27b3 --- /dev/null +++ b/tests/bud/buildkit-mount-from/Dockerfilebindfromwithoutsource @@ -0,0 +1,4 @@ +FROM alpine +RUN mkdir /test +# use from= as mount source +RUN --mount=type=bind,from=buildkitbase,target=/test cat /test/hello diff --git a/tests/bud/buildkit-mount-from/Dockerfilebindfromwriteable b/tests/bud/buildkit-mount-from/Dockerfilebindfromwriteable new file mode 100644 index 0000000000..1f4cc31b3e --- /dev/null +++ b/tests/bud/buildkit-mount-from/Dockerfilebindfromwriteable @@ -0,0 +1,3 @@ +FROM alpine +# use from= as mount source +RUN --mount=type=bind,source=.,from=buildkitbase,target=/test,rw echo "world" > /test/hello && cat /test/hello diff --git a/tests/bud/buildkit-mount-from/Dockerfilebuildkitbase b/tests/bud/buildkit-mount-from/Dockerfilebuildkitbase new file mode 100644 index 0000000000..079440f1bc --- /dev/null +++ b/tests/bud/buildkit-mount-from/Dockerfilebuildkitbase @@ -0,0 +1,2 @@ +FROM scratch +COPY hello . diff --git a/tests/bud/buildkit-mount-from/Dockerfilebuildkitbaserelative b/tests/bud/buildkit-mount-from/Dockerfilebuildkitbaserelative new file mode 100644 index 0000000000..a5c242920f --- /dev/null +++ b/tests/bud/buildkit-mount-from/Dockerfilebuildkitbaserelative @@ -0,0 +1,3 @@ +FROM alpine +RUN mkdir subdir +COPY hello /subdir diff --git a/tests/bud/buildkit-mount-from/Dockerfilecachefrom b/tests/bud/buildkit-mount-from/Dockerfilecachefrom new file mode 100644 index 0000000000..c8b843a435 --- /dev/null +++ b/tests/bud/buildkit-mount-from/Dockerfilecachefrom @@ -0,0 +1,8 @@ +FROM alpine as builder +RUN mkdir subdir +COPY hello . + +FROM alpine as second +RUN mkdir /test +# use another stage as cache source +RUN --mount=type=cache,from=builder,target=/test cat /test/hello diff --git a/tests/bud/buildkit-mount-from/Dockerfilecachefromimage b/tests/bud/buildkit-mount-from/Dockerfilecachefromimage new file mode 100644 index 0000000000..8965fa6808 --- /dev/null +++ b/tests/bud/buildkit-mount-from/Dockerfilecachefromimage @@ -0,0 +1,5 @@ +FROM alpine +RUN mkdir /test +# use another image as cache source +# following should fail as cache does not supports mounting image +RUN --mount=type=cache,from=buildkitbase,target=/test cat /test/hello diff --git a/tests/bud/buildkit-mount-from/Dockerfilecachemultiplefrom b/tests/bud/buildkit-mount-from/Dockerfilecachemultiplefrom new file mode 100644 index 0000000000..93aba35230 --- /dev/null +++ b/tests/bud/buildkit-mount-from/Dockerfilecachemultiplefrom @@ -0,0 +1,10 @@ +FROM alpine as builder +COPY hello . + +FROM alpine as builder2 +COPY hello2 . + +FROM alpine +RUN mkdir /test +# use other stages as cache source +RUN --mount=type=cache,from=builder,target=/test --mount=type=cache,from=builder2,target=/test2 cat /test2/hello2 && cat /test/hello diff --git a/tests/bud/buildkit-mount-from/Dockerfilemultistagefrom b/tests/bud/buildkit-mount-from/Dockerfilemultistagefrom new file mode 100644 index 0000000000..470aa03939 --- /dev/null +++ b/tests/bud/buildkit-mount-from/Dockerfilemultistagefrom @@ -0,0 +1,6 @@ +FROM alpine as builder +RUN mkdir subdir +COPY hello ./subdir/ + +FROM alpine as second +RUN --mount=type=bind,source=/subdir,from=builder,target=/test cat /test/hello diff --git a/tests/bud/buildkit-mount-from/Dockerfilemultistagefromcache b/tests/bud/buildkit-mount-from/Dockerfilemultistagefromcache new file mode 100644 index 0000000000..26f656b3b1 --- /dev/null +++ b/tests/bud/buildkit-mount-from/Dockerfilemultistagefromcache @@ -0,0 +1,11 @@ +FROM alpine as builder +RUN mkdir subdir +RUN mkdir subdir/subdir2 +COPY hello ./subdir/ +COPY hello2 ./subdir/subdir2/ + +FROM alpine as second +RUN --mount=type=cache,id=1,source=/subdir,from=builder,target=/test cat /test/hello + +FROM alpine as third +RUN --mount=type=cache,id=2,source=/subdir/subdir2,from=builder,target=/test cat /test/hello2 diff --git a/tests/bud/buildkit-mount-from/Dockermultistagefrom b/tests/bud/buildkit-mount-from/Dockermultistagefrom new file mode 100644 index 0000000000..470aa03939 --- /dev/null +++ b/tests/bud/buildkit-mount-from/Dockermultistagefrom @@ -0,0 +1,6 @@ +FROM alpine as builder +RUN mkdir subdir +COPY hello ./subdir/ + +FROM alpine as second +RUN --mount=type=bind,source=/subdir,from=builder,target=/test cat /test/hello diff --git a/tests/bud/buildkit-mount-from/hello b/tests/bud/buildkit-mount-from/hello new file mode 100644 index 0000000000..ce01362503 --- /dev/null +++ b/tests/bud/buildkit-mount-from/hello @@ -0,0 +1 @@ +hello diff --git a/tests/bud/buildkit-mount-from/hello2 b/tests/bud/buildkit-mount-from/hello2 new file mode 100644 index 0000000000..14be0d41c6 --- /dev/null +++ b/tests/bud/buildkit-mount-from/hello2 @@ -0,0 +1 @@ +hello2 diff --git a/tests/run.bats b/tests/run.bats index a959e2b3b8..624eeb9b0c 100644 --- a/tests/run.bats +++ b/tests/run.bats @@ -337,13 +337,46 @@ function configure_and_check_user() { mkdir -p ${TESTDIR}/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=bind,src=${TESTDIR}/was:empty,dst=/var/not-empty${zflag:+,${zflag}} $cid touch /var/not-empty/testfile + run_buildah run --mount type=bind,src=${TESTDIR}/was:empty,dst=/var/not-empty,rw${zflag:+,${zflag}} $cid touch /var/not-empty/testfile # 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=${TESTDIR}/was:empty,dst=/var/not-empty,ro${zflag:+,${zflag}} $cid touch /var/not-empty/testfile # Even if the parent directory doesn't exist yet, this should succeed. - run_buildah run --mount type=bind,src=${TESTDIR}/was:empty,dst=/var/multi-level/subdirectory $cid touch /var/multi-level/subdirectory/testfile + run_buildah run --mount type=bind,src=${TESTDIR}/was:empty,dst=/var/multi-level/subdirectory,rw $cid touch /var/multi-level/subdirectory/testfile # And check the same for file volumes. - run_buildah run --mount type=bind,src=${TESTDIR}/was:empty/testfile,dst=/var/different-multi-level/subdirectory/testfile $cid touch /var/different-multi-level/subdirectory/testfile + run_buildah run --mount type=bind,src=${TESTDIR}/was:empty/testfile,dst=/var/different-multi-level/subdirectory/testfile,rw $cid touch /var/different-multi-level/subdirectory/testfile +} + +@test "run --mount=type=bind with from like buildkit" { + skip_if_no_runtime + zflag= + if which selinuxenabled > /dev/null 2> /dev/null ; then + if selinuxenabled ; then + skip "skip if selinux enabled, since stages have different selinux label" + fi + fi + run_buildah build -t buildkitbase --signature-policy ${TESTSDIR}/policy.json -f ${TESTSDIR}/bud/buildkit-mount-from/Dockerfilebuildkitbase ${TESTSDIR}/bud/buildkit-mount-from/ + _prefetch alpine + run_buildah from --quiet --pull=false --signature-policy ${TESTSDIR}/policy.json alpine + cid=$output + run_buildah run --mount type=bind,source=.,from=buildkitbase,target=/test,z $cid cat /test/hello + expect_output --substring "hello" + run_buildah rmi -f buildkitbase +} + +@test "run --mount=type=cache like buildkit" { + skip_if_no_runtime + zflag= + if which selinuxenabled > /dev/null 2> /dev/null ; then + if selinuxenabled ; then + skip "skip if selinux enabled, since stages have different selinux label" + fi + fi + _prefetch alpine + run_buildah from --quiet --pull=false --signature-policy ${TESTSDIR}/policy.json alpine + cid=$output + run_buildah run --mount type=cache,target=/test,z $cid sh -c 'echo "hello" > /test/hello && cat /test/hello' + run_buildah run --mount type=cache,target=/test,z $cid cat /test/hello + expect_output --substring "hello" } @test "run symlinks" {