Merge pull request containers#5975 from nalind/overlay-build-context

`buildah build`: use the same overlay for the context directory for the whole build

Author: openshift-merge-bot[bot]

imagebuildah/build.go

@@ -284,6 +284,12 @@ func BuildDockerfiles(ctx context.Context, store storage.Store, options define.B
 		}
 
 		builds.Go(func() error {
+			contextDirectory, processLabel, mountLabel, usingContextOverlay, cleanupOverlay, err := platformSetupContextDirectoryOverlay(store, &options)
+			if err != nil {
+				return fmt.Errorf("mounting an overlay over build context directory: %w", err)
+			}
+			defer cleanupOverlay()
+			platformOptions.ContextDirectory = contextDirectory
 			loggerPerPlatform := logger
 			if platformOptions.LogFile != "" && platformOptions.LogSplitByPlatform {
 				logFile := platformOptions.LogFile + "_" + platformOptions.OS + "_" + platformOptions.Architecture
@@ -302,7 +308,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)
@@ -413,7 +419,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)
@@ -454,7 +460,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/opencontainers/selinux/go-selinux/label"
+	"github.com/sirupsen/logrus"
+	"go.podman.io/storage"
+	"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"
+	"go.podman.io/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

@@ -73,6 +73,7 @@ type executor struct {
 	stages                         map[string]*stageExecutor
 	store                          storage.Store
 	contextDir                     string
+	contextDirWritesAreDiscarded   bool
 	pullPolicy                     define.PullPolicy
 	registry                       string
 	ignoreUnrecognizedInstructions bool
@@ -187,7 +188,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)
@@ -257,6 +258,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,
@@ -294,6 +296,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

@@ -20,6 +20,7 @@ import (
 	buildahdocker "github.com/containers/buildah/docker"
 	"github.com/containers/buildah/internal"
 	"github.com/containers/buildah/internal/metadata"
+	"github.com/containers/buildah/internal/sanitize"
 	"github.com/containers/buildah/internal/tmpdir"
 	internalUtil "github.com/containers/buildah/internal/util"
 	"github.com/containers/buildah/pkg/parse"
@@ -453,7 +454,7 @@ func (s *stageExecutor) performCopy(excludes []string, copies ...imagebuilder.Co
 			copy.Src = copySources
 		}
 
-		if len(copy.From) > 0 && len(copy.Files) == 0 {
+		if copy.From != "" && len(copy.Files) == 0 {
 			// If from has an argument within it, resolve it to its
 			// value.  Otherwise just return the value found.
 			from, fromErr := imagebuilder.ProcessWord(copy.From, s.stage.Builder.Arguments())
@@ -535,7 +536,8 @@ func (s *stageExecutor) performCopy(excludes []string, copies ...imagebuilder.Co
 				}
 				contextDir = mountPoint
 			}
-			// Original behaviour of buildah still stays true for COPY irrespective of additional context.
+			// With --from set, the content being copied isn't coming from the default
+			// build context directory, so we're not expected to force everything to 0:0
 			preserveOwnership = true
 			copyExcludes = excludes
 		} else {
@@ -618,9 +620,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, ",")
@@ -650,7 +657,7 @@ func (s *stageExecutor) runStageMountPoints(mountList []string) (map[string]inte
 						if additionalBuildContext.IsImage {
 							mountPoint, err := s.getImageRootfs(s.ctx, additionalBuildContext.Value)
 							if err != nil {
-								return nil, fmt.Errorf("%s from=%s: image found with that name", flag, from)
+								return nil, fmt.Errorf("%s from=%s: image not found with that name", flag, from)
 							}
 							// The `from` in stageMountPoints should point
 							// to `mountPoint` replaced from additional
@@ -932,6 +939,29 @@ func (s *stageExecutor) UnrecognizedInstruction(step *imagebuilder.Step) error {
 	return errors.New(err)
 }
 
+// sanitizeFrom limits which image names (with or without transport prefixes)
+// we'll accept.  For those it accepts which refer to filesystem objects, where
+// relative path names are evaluated relative to "contextDir", it will create a
+// copy of the original image, under "tmpdir", which contains no symbolic
+// links, and return either the original image reference or a reference to a
+// sanitized copy which should be used instead.
+func (s *stageExecutor) sanitizeFrom(from, tmpdir string) (newFrom string, err error) {
+	transportName, restOfImageName, maybeHasTransportName := strings.Cut(from, ":")
+	if !maybeHasTransportName || transports.Get(transportName) == nil {
+		if _, err = reference.ParseNormalizedNamed(from); err == nil {
+			// this is a normal-looking image-in-a-registry-or-named-in-storage name
+			return from, nil
+		}
+		if img, err := s.executor.store.Image(from); img != nil && err == nil {
+			// this is an image ID
+			return from, nil
+		}
+		return "", fmt.Errorf("parsing image name %q: %w", from, err)
+	}
+	// TODO: drop this part and just return an error... someday
+	return sanitize.ImageName(transportName, restOfImageName, s.executor.contextDir, tmpdir)
+}
+
 // prepare creates a working container based on the specified image, or if one
 // isn't specified, the first argument passed to the first FROM instruction we
 // can find in the stage's parsed tree.
@@ -948,6 +978,10 @@ func (s *stageExecutor) prepare(ctx context.Context, from string, initializeIBCo
 		}
 		from = base
 	}
+	sanitizedFrom, err := s.sanitizeFrom(from, tmpdir.GetTempDir())
+	if err != nil {
+		return nil, fmt.Errorf("invalid base image specification %q: %w", from, err)
+	}
 	displayFrom := from
 	if ib.Platform != "" {
 		displayFrom = "--platform=" + ib.Platform + " " + displayFrom
@@ -987,7 +1021,7 @@ func (s *stageExecutor) prepare(ctx context.Context, from string, initializeIBCo
 
 	builderOptions := buildah.BuilderOptions{
 		Args:                  ib.Args,
-		FromImage:             from,
+		FromImage:             sanitizedFrom,
 		GroupAdd:              s.executor.groupAdd,
 		PullPolicy:            pullPolicy,
 		ContainerSuffix:       s.executor.containerSuffix,
@@ -1025,16 +1059,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() {