imagebuildah: use a longer-lived overlay over the build context

Mount a read-write overlay directory over the build context directory to
restore the ability to use it as a covert cache of sorts during the
lifetime of the build, but in a way that still ensures that we don't
modify the real build context directory.

N.B.: builds where FROM in one stage referenced a relative path which
had been written to a bind-mounted default build context directory by an
earlier stage broke when we started making those bind mounts into
overlays to prevent/discard modifications to that directory, and while
this extends the lifetime of that overlay so that it's consistent
throughout the build, those relative path names are still going to point
to the wrong location.

Since we need to determine SELinux labeling before mounting the overlay,
go ahead and calculate the labels to use before creating the first
builder, and remove the logic that had whichever stage thought it was
the first one set them in its parent object for use by other stages, in
what was probably a racey way.

Signed-off-by: Nalin Dahyabhai <[email protected]>

Author: nalind

imagebuildah/build.go

@@ -204,6 +204,15 @@ func BuildDockerfiles(ctx context.Context, store storage.Store, options define.B
 		options.AdditionalBuildContexts = make(map[string]*define.AdditionalBuildContext)
 	}
 
+	contextDirectory, processLabel, mountLabel, usingContextOverlay, cleanupOverlay, err := platformSetupContextDirectoryOverlay(store, &options)
+	if err != nil {
+		return "", nil, fmt.Errorf("mounting an overlay over build context directory: %w", err)
+	}
+	defer cleanupOverlay()
+	if contextDirectory != "" {
+		options.ContextDirectory = contextDirectory
+	}
+
 	if len(options.Platforms) == 0 {
 		options.Platforms = append(options.Platforms, struct{ OS, Arch, Variant string }{
 			OS:   options.SystemContext.OSChoice,
@@ -278,7 +287,7 @@ func BuildDockerfiles(ctx context.Context, store storage.Store, options define.B
 				platformOptions.ReportWriter = reporter
 				platformOptions.Err = stderr
 			}
-			thisID, thisRef, err := buildDockerfilesOnce(ctx, store, loggerPerPlatform, logPrefix, platformOptions, paths, files)
+			thisID, thisRef, err := buildDockerfilesOnce(ctx, store, loggerPerPlatform, logPrefix, platformOptions, paths, files, processLabel, mountLabel, usingContextOverlay)
 			if err != nil {
 				if errorContext := strings.TrimSpace(logPrefix); errorContext != "" {
 					return fmt.Errorf("%s: %w", errorContext, err)
@@ -389,7 +398,7 @@ func BuildDockerfiles(ctx context.Context, store storage.Store, options define.B
 	return id, ref, nil
 }
 
-func buildDockerfilesOnce(ctx context.Context, store storage.Store, logger *logrus.Logger, logPrefix string, options define.BuildOptions, containerFiles []string, dockerfilecontents [][]byte) (string, reference.Canonical, error) {
+func buildDockerfilesOnce(ctx context.Context, store storage.Store, logger *logrus.Logger, logPrefix string, options define.BuildOptions, containerFiles []string, dockerfilecontents [][]byte, processLabel, mountLabel string, usingContextOverlay bool) (string, reference.Canonical, error) {
 	mainNode, err := imagebuilder.ParseDockerfile(bytes.NewReader(dockerfilecontents[0]))
 	if err != nil {
 		return "", nil, fmt.Errorf("parsing main Dockerfile: %s: %w", containerFiles[0], err)
@@ -441,7 +450,7 @@ func buildDockerfilesOnce(ctx context.Context, store storage.Store, logger *logr
 		mainNode.Children = append(mainNode.Children, additionalNode.Children...)
 	}
 
-	exec, err := newExecutor(logger, logPrefix, store, options, mainNode, containerFiles)
+	exec, err := newExecutor(logger, logPrefix, store, options, mainNode, containerFiles, processLabel, mountLabel, usingContextOverlay)
 	if err != nil {
 		return "", nil, fmt.Errorf("creating build executor: %w", err)
 	}

imagebuildah/build_linux.go

@@ -0,0 +1,86 @@
+package imagebuildah
+
+import (
+	"errors"
+	"fmt"
+	"io/fs"
+	"os"
+	"path/filepath"
+	"slices"
+
+	"github.com/containers/buildah/define"
+	"github.com/containers/buildah/internal/tmpdir"
+	"github.com/containers/buildah/pkg/overlay"
+	"github.com/containers/storage"
+	"github.com/opencontainers/selinux/go-selinux/label"
+	"github.com/sirupsen/logrus"
+	"golang.org/x/sys/unix"
+)
+
+// platformSetupContextDirectoryOverlay() sets up an overlay _over_ the build
+// context directory, and sorts out labeling.  Returns the location which
+// should be used as the default build context; the process label and mount
+// label for the build, if any; a boolean value that indicates whether we did,
+// in fact, mount an overlay; and a cleanup function which should be called
+// when the location is no longer needed (on success). Returned errors should
+// be treated as fatal.
+func platformSetupContextDirectoryOverlay(store storage.Store, options *define.BuildOptions) (string, string, string, bool, func(), error) {
+	var succeeded bool
+	var tmpDir, contentDir string
+	cleanup := func() {
+		if contentDir != "" {
+			if err := overlay.CleanupContent(tmpDir); err != nil {
+				logrus.Debugf("cleaning up overlay scaffolding for build context directory: %v", err)
+			}
+		}
+		if tmpDir != "" {
+			if err := os.Remove(tmpDir); err != nil && !errors.Is(err, fs.ErrNotExist) {
+				logrus.Debugf("removing should-be-empty temporary directory %q: %v", tmpDir, err)
+			}
+		}
+	}
+	defer func() {
+		if !succeeded {
+			cleanup()
+		}
+	}()
+	// double-check that the context directory location is an absolute path
+	contextDirectoryAbsolute, err := filepath.Abs(options.ContextDirectory)
+	if err != nil {
+		return "", "", "", false, nil, fmt.Errorf("determining absolute path of %q: %w", options.ContextDirectory, err)
+	}
+	var st unix.Stat_t
+	if err := unix.Stat(contextDirectoryAbsolute, &st); err != nil {
+		return "", "", "", false, nil, fmt.Errorf("checking ownership of %q: %w", options.ContextDirectory, err)
+	}
+	// figure out the labeling situation
+	processLabel, mountLabel, err := label.InitLabels(options.CommonBuildOpts.LabelOpts)
+	if err != nil {
+		return "", "", "", false, nil, err
+	}
+	// create a temporary directory
+	tmpDir, err = os.MkdirTemp(tmpdir.GetTempDir(), "buildah-context-")
+	if err != nil {
+		return "", "", "", false, nil, fmt.Errorf("creating temporary directory: %w", err)
+	}
+	// create the scaffolding for an overlay mount under it
+	contentDir, err = overlay.TempDir(tmpDir, 0, 0)
+	if err != nil {
+		return "", "", "", false, nil, fmt.Errorf("creating overlay scaffolding for build context directory: %w", err)
+	}
+	// mount an overlay that uses it as a lower
+	overlayOptions := overlay.Options{
+		GraphOpts:  slices.Clone(store.GraphOptions()),
+		ForceMount: true,
+		MountLabel: mountLabel,
+	}
+	targetDir := filepath.Join(contentDir, "target")
+	contextDirMountSpec, err := overlay.MountWithOptions(contentDir, contextDirectoryAbsolute, targetDir, &overlayOptions)
+	if err != nil {
+		return "", "", "", false, nil, fmt.Errorf("creating overlay scaffolding for build context directory: %w", err)
+	}
+	// going forward, pretend that the merged directory is the actual context directory
+	logrus.Debugf("mounted an overlay at %q over %q", contextDirMountSpec.Source, contextDirectoryAbsolute)
+	succeeded = true
+	return contextDirMountSpec.Source, processLabel, mountLabel, true, cleanup, nil
+}

imagebuildah/build_notlinux.go

@@ -0,0 +1,20 @@
+//go:build !linux
+
+package imagebuildah
+
+import (
+	"github.com/containers/buildah/define"
+	"github.com/containers/storage"
+)
+
+// platformSetupContextDirectoryOverlay() should set up an overlay _over_ the
+// build context directory, and sort out labeling.  Should return the location
+// which should be used as the default build context; the process label and
+// mount label for the build, if any; a boolean value that indicates whether we
+// did, in fact, mount an overlay; and a cleanup function which should be
+// called when the location is no longer needed (on success).  Returned errors
+// should be treated as fatal.
+// TODO: currenty a no-op on this platform.
+func platformSetupContextDirectoryOverlay(store storage.Store, options *define.BuildOptions) (string, string, string, bool, func(), error) {
+	return options.ContextDirectory, "", "", false, func() {}, nil
+}

imagebuildah/executor.go

@@ -69,6 +69,7 @@ type Executor struct {
 	stages                         map[string]*StageExecutor
 	store                          storage.Store
 	contextDir                     string
+	contextDirWritesAreDiscarded   bool
 	pullPolicy                     define.PullPolicy
 	registry                       string
 	ignoreUnrecognizedInstructions bool
@@ -173,7 +174,7 @@ type imageTypeAndHistoryAndDiffIDs struct {
 }
 
 // newExecutor creates a new instance of the imagebuilder.Executor interface.
-func newExecutor(logger *logrus.Logger, logPrefix string, store storage.Store, options define.BuildOptions, mainNode *parser.Node, containerFiles []string) (*Executor, error) {
+func newExecutor(logger *logrus.Logger, logPrefix string, store storage.Store, options define.BuildOptions, mainNode *parser.Node, containerFiles []string, processLabel, mountLabel string, contextWritesDiscarded bool) (*Executor, error) {
 	defaultContainerConfig, err := config.Default()
 	if err != nil {
 		return nil, fmt.Errorf("failed to get container config: %w", err)
@@ -238,6 +239,7 @@ func newExecutor(logger *logrus.Logger, logPrefix string, store storage.Store, o
 		stages:                                  make(map[string]*StageExecutor),
 		store:                                   store,
 		contextDir:                              options.ContextDirectory,
+		contextDirWritesAreDiscarded:            contextWritesDiscarded,
 		excludes:                                excludes,
 		groupAdd:                                options.GroupAdd,
 		ignoreFile:                              options.IgnoreFile,
@@ -273,6 +275,8 @@ func newExecutor(logger *logrus.Logger, logPrefix string, store storage.Store, o
 		squash:                                  options.Squash,
 		labels:                                  slices.Clone(options.Labels),
 		layerLabels:                             slices.Clone(options.LayerLabels),
+		processLabel:                            processLabel,
+		mountLabel:                              mountLabel,
 		annotations:                             slices.Clone(options.Annotations),
 		layers:                                  options.Layers,
 		noHostname:                              options.CommonBuildOpts.NoHostname,

imagebuildah/stage_executor.go

@@ -606,9 +606,14 @@ func (s *StageExecutor) performCopy(excludes []string, copies ...imagebuilder.Co
 }
 
 // Returns a map of StageName/ImageName:internal.StageMountDetails for the
-// items in the passed-in mounts list which include a "from=" value.
+// items in the passed-in mounts list which include a "from=" value.  The ""
+// key in the returned map corresponds to the default build context.
 func (s *StageExecutor) runStageMountPoints(mountList []string) (map[string]internal.StageMountDetails, error) {
 	stageMountPoints := make(map[string]internal.StageMountDetails)
+	stageMountPoints[""] = internal.StageMountDetails{
+		MountPoint:               s.executor.contextDir,
+		IsWritesDiscardedOverlay: s.executor.contextDirWritesAreDiscarded,
+	}
 	for _, flag := range mountList {
 		if strings.Contains(flag, "from") {
 			tokens := strings.Split(flag, ",")
@@ -1014,16 +1019,6 @@ func (s *StageExecutor) prepare(ctx context.Context, from string, initializeIBCo
 		return nil, fmt.Errorf("creating build container: %w", err)
 	}
 
-	// If executor's ProcessLabel and MountLabel is empty means this is the first stage
-	// Make sure we share first stage's ProcessLabel and MountLabel with all other subsequent stages
-	// Doing this will ensure and one stage in same build can mount another stage even if `selinux`
-	// is enabled.
-
-	if s.executor.mountLabel == "" && s.executor.processLabel == "" {
-		s.executor.mountLabel = builder.MountLabel
-		s.executor.processLabel = builder.ProcessLabel
-	}
-
 	if initializeIBConfig {
 		volumes := map[string]struct{}{}
 		for _, v := range builder.Volumes() {

internal/types.go

@@ -17,4 +17,5 @@ type StageMountDetails struct {
 	IsImage                  bool   // true if the mountpoint is an image's rootfs
 	IsAdditionalBuildContext bool   // true if the mountpoint is an additional build context
 	MountPoint               string // mountpoint of the stage or image's root directory or path of the additional build context
+	IsWritesDiscardedOverlay bool   // this is an overlay that discards writes
 }

internal/volumes/volumes.go

@@ -141,6 +141,7 @@ func GetBindMount(sys *types.SystemContext, args []string, contextDir string, st
 	setDest := ""
 	bindNonRecursive := false
 	fromWhere := ""
+	skipOverlay := false
 
 	for _, val := range args {
 		argName, argValue, hasArgValue := strings.Cut(val, "=")
@@ -248,6 +249,7 @@ func GetBindMount(sys *types.SystemContext, args []string, contextDir string, st
 		if additionalMountPoints != nil {
 			if val, ok := additionalMountPoints[fromWhere]; ok {
 				mountPoint = val.MountPoint
+				skipOverlay = val.IsWritesDiscardedOverlay
 			}
 		}
 		// if mountPoint of image was not found in additionalMap
@@ -273,6 +275,14 @@ func GetBindMount(sys *types.SystemContext, args []string, contextDir string, st
 			}()
 		}
 		contextDir = mountPoint
+	} else {
+		// special case an additional mount point for "" as shorthand for "preferred location of the default build context"
+		if additionalMountPoints != nil {
+			if val, ok := additionalMountPoints[""]; ok {
+				contextDir = val.MountPoint
+				skipOverlay = val.IsWritesDiscardedOverlay
+			}
+		}
 	}
 
 	// buildkit parity: default bind option must be `rbind`
@@ -328,7 +338,7 @@ func GetBindMount(sys *types.SystemContext, args []string, contextDir string, st
 	}
 
 	overlayDir := ""
-	if mountedImage != "" || mountIsReadWrite(newMount) {
+	if !skipOverlay && (mountedImage != "" || mountIsReadWrite(newMount)) {
 		if newMount, overlayDir, err = convertToOverlay(newMount, store, mountLabel, tmpDir, 0, 0); err != nil {
 			return newMount, "", "", "", err
 		}

pkg/cli/build.go

@@ -113,7 +113,10 @@ func GenBuildOptions(c *cobra.Command, inputArgs []string, iopts BuildOptions) (
 	if c.Flag("build-context").Changed {
 		for _, contextString := range iopts.BuildContext {
 			av := strings.SplitN(contextString, "=", 2)
-			if len(av) > 1 {
+			// the key should be non-empty: we use "" as internal
+			// shorthand for the default build context when there's
+			// an overlay mounted over it
+			if len(av) > 1 && av[0] != "" {
 				parseAdditionalBuildContext, err := parse.GetAdditionalBuildContext(av[1])
 				if err != nil {
 					return options, nil, nil, fmt.Errorf("while parsing additional build context: %w", err)

pkg/parse/parse.go

@@ -57,7 +57,12 @@ const (
 	BuildahCacheDir = "buildah-cache"
 )
 
-var errInvalidSecretSyntax = errors.New("incorrect secret flag format: should be --secret id=foo,src=bar[,env=ENV][,type=file|env]")
+var (
+	errInvalidSecretSyntax         = errors.New("incorrect secret flag format: should be --secret id=foo,src=bar[,env=ENV][,type=file|env]")
+	errInvalidBuildContextPathname = errors.New(`invalid build context path ""`)
+	errInvalidBuildContextImage    = errors.New(`invalid build context image name ""`)
+	errInvalidBuildContextURL      = errors.New(`invalid build context image URL ""`)
+)
 
 // RepoNamesToNamedReferences parse the raw string to Named reference
 func RepoNamesToNamedReferences(destList []string) ([]reference.Named, error) {
@@ -206,21 +211,39 @@ func CommonBuildOptionsFromFlagSet(flags *pflag.FlagSet, findFlagFunc func(name
 	return commonOpts, nil
 }
 
-// GetAdditionalBuildContext consumes raw string and returns parsed AdditionalBuildContext
+// GetAdditionalBuildContext consumes a raw string and returns a parsed
+// AdditionalBuildContext describing the build context.
 func GetAdditionalBuildContext(value string) (define.AdditionalBuildContext, error) {
+	if value == "" {
+		// reject empty values (filesystem paths?), because elsewhere we use an
+		// empty string as an internal nickname for the default build context
+		return define.AdditionalBuildContext{}, errInvalidBuildContextPathname
+	}
 	ret := define.AdditionalBuildContext{IsURL: false, IsImage: false, Value: value}
 	if strings.HasPrefix(value, "docker-image://") {
 		ret.IsImage = true
 		ret.Value = strings.TrimPrefix(value, "docker-image://")
+		if ret.Value == "" {
+			return define.AdditionalBuildContext{}, errInvalidBuildContextImage
+		}
 	} else if strings.HasPrefix(value, "container-image://") {
 		ret.IsImage = true
 		ret.Value = strings.TrimPrefix(value, "container-image://")
+		if ret.Value == "" {
+			return define.AdditionalBuildContext{}, errInvalidBuildContextImage
+		}
 	} else if strings.HasPrefix(value, "docker://") {
 		ret.IsImage = true
 		ret.Value = strings.TrimPrefix(value, "docker://")
+		if ret.Value == "" {
+			return define.AdditionalBuildContext{}, errInvalidBuildContextImage
+		}
 	} else if strings.HasPrefix(value, "http://") || strings.HasPrefix(value, "https://") {
 		ret.IsImage = false
 		ret.IsURL = true
+		if strings.TrimPrefix(ret.Value, "http://") == "" || strings.TrimPrefix(ret.Value, "https://") == "" {
+			return define.AdditionalBuildContext{}, errInvalidBuildContextURL
+		}
 	} else {
 		path, err := filepath.Abs(value)
 		if err != nil {

run.go

@@ -157,7 +157,9 @@ type RunOptions struct {
 	SSHSources map[string]*sshagent.Source `json:"-"`
 	// RunMounts are unparsed mounts to be added for this run
 	RunMounts []string
-	// Map of stages and container mountpoint if any from stage executor
+	// Map of already-mounted stages, images, and container mountpoints
+	// which entries in `RunMounts` might be referring to.  If a value for
+	// the "" key is present, it points to the context directory.
 	StageMountPoints map[string]internal.StageMountDetails
 	// IDs of mounted images to be unmounted before returning
 	// Deprecated: before 1.39, these images would not be consistently

tests/bud.bats

@@ -716,7 +716,7 @@ symlink(subdir)"
   _prefetch busybox
   run_buildah 125 build -t testbud3 $WITH_POLICY_JSON $BUDFILES/dockerignore3
   expect_output --substring 'building.*"COPY test1.txt /upload/test1.txt".*no such file or directory'
-  expect_output --substring $(realpath "$BUDFILES/dockerignore3/.dockerignore")
+  expect_output --substring 'filtered out using /[^ ]*/.dockerignore'
 }
 
 @test "bud with .dockerignore #4" {