stage_executor: reset platform in systemcontext for stages

Every stage now has its own copy of systemcontext.

On processing of every stage platform spec in systemcontext must be
correctly reset.

Closes: #5968

Signed-off-by: flouthoc <[email protected]>

Author: flouthoc

imagebuildah/executor.go

@@ -372,9 +372,12 @@ func newExecutor(logger *logrus.Logger, logPrefix string, store storage.Store, o
 // startStage creates a new stage executor that will be referenced whenever a
 // COPY or ADD statement uses a --from=NAME flag.
 func (b *Executor) startStage(ctx context.Context, stage *imagebuilder.Stage, stages imagebuilder.Stages, output string) *StageExecutor {
+	// create a copy of systemContext for each stage executor.
+	systemContext := *b.systemContext
 	stageExec := &StageExecutor{
 		ctx:             ctx,
 		executor:        b,
+		systemContext:   &systemContext,
 		log:             b.log,
 		index:           stage.Position,
 		stages:          stages,

imagebuildah/stage_executor.go

@@ -59,6 +59,7 @@ import (
 // name to the image that it produces.
 type StageExecutor struct {
 	ctx                   context.Context
+	systemContext         *types.SystemContext
 	executor              *Executor
 	log                   func(format string, args ...interface{})
 	index                 int
@@ -584,8 +585,8 @@ func (s *StageExecutor) performCopy(excludes []string, copies ...imagebuilder.Co
 			// The values for these next two fields are ultimately
 			// based on command line flags with names that sound
 			// much more generic.
-			CertPath:              s.executor.systemContext.DockerCertPath,
-			InsecureSkipTLSVerify: s.executor.systemContext.DockerInsecureSkipTLSVerify,
+			CertPath:              s.systemContext.DockerCertPath,
+			InsecureSkipTLSVerify: s.systemContext.DockerInsecureSkipTLSVerify,
 			MaxRetries:            s.executor.maxPullPushRetries,
 			RetryDelay:            s.executor.retryPullPushDelay,
 			Parents:               copy.Parents,
@@ -841,7 +842,7 @@ func (s *StageExecutor) Run(run imagebuilder.Run, config docker.Config) error {
 		Stderr:               s.executor.err,
 		Stdin:                stdin,
 		Stdout:               s.executor.out,
-		SystemContext:        s.executor.systemContext,
+		SystemContext:        s.systemContext,
 		Terminal:             buildah.WithoutTerminal,
 		User:                 config.User,
 		WorkingDir:           config.WorkingDir,
@@ -966,19 +967,20 @@ func (s *StageExecutor) prepare(ctx context.Context, from string, initializeIBCo
 		}
 	}
 
-	builderSystemContext := s.executor.systemContext
-	// get platform string from stage
-	if stage.Builder.Platform != "" {
-		os, arch, variant, err := parse.Platform(stage.Builder.Platform)
+	// In a multi-stage build where `FROM --platform=<>` was used then we must
+	// reset context for new stages so that new stages don't inherit unexpected
+	// `--platform` from prior stages.
+	if stage.Builder.Platform != "" || (len(s.stages) > 1 && ((s.systemContext.ArchitectureChoice == "" && s.systemContext.VariantChoice == "" && s.systemContext.OSChoice == "") || (s.executor.skipUnusedStages == types.OptionalBoolFalse))) {
+		imageOS, imageArch, imageVariant, err := parse.Platform(stage.Builder.Platform)
 		if err != nil {
 			return nil, fmt.Errorf("unable to parse platform %q: %w", stage.Builder.Platform, err)
 		}
-		if arch != "" || variant != "" {
-			builderSystemContext.ArchitectureChoice = arch
-			builderSystemContext.VariantChoice = variant
+		if imageArch != "" || imageVariant != "" {
+			s.systemContext.ArchitectureChoice = imageArch
+			s.systemContext.VariantChoice = imageVariant
 		}
-		if os != "" {
-			builderSystemContext.OSChoice = os
+		if imageOS != "" {
+			s.systemContext.OSChoice = imageOS
 		}
 	}
 
@@ -992,7 +994,7 @@ func (s *StageExecutor) prepare(ctx context.Context, from string, initializeIBCo
 		BlobDirectory:         s.executor.blobDirectory,
 		SignaturePolicyPath:   s.executor.signaturePolicyPath,
 		ReportWriter:          s.executor.reportWriter,
-		SystemContext:         builderSystemContext,
+		SystemContext:         s.systemContext,
 		Isolation:             s.executor.isolation,
 		NamespaceOptions:      s.executor.namespaceOptions,
 		ConfigureNetwork:      s.executor.configureNetwork,
@@ -2058,7 +2060,7 @@ func (s *StageExecutor) tagExistingImage(ctx context.Context, cacheID, output st
 		return "", nil, err
 	}
 
-	policyContext, err := util.GetPolicyContext(s.executor.systemContext)
+	policyContext, err := util.GetPolicyContext(s.systemContext)
 	if err != nil {
 		return "", nil, err
 	}
@@ -2171,7 +2173,7 @@ func (s *StageExecutor) pushCache(ctx context.Context, src, cacheKey string) err
 			Compression:         s.executor.compression,
 			SignaturePolicyPath: s.executor.signaturePolicyPath,
 			Store:               s.executor.store,
-			SystemContext:       s.executor.systemContext,
+			SystemContext:       s.systemContext,
 			BlobDirectory:       s.executor.blobDirectory,
 			SignBy:              s.executor.signBy,
 			MaxRetries:          s.executor.maxPullPushRetries,
@@ -2209,7 +2211,7 @@ func (s *StageExecutor) pullCache(ctx context.Context, cacheKey string) (referen
 		options := buildah.PullOptions{
 			SignaturePolicyPath: s.executor.signaturePolicyPath,
 			Store:               s.executor.store,
-			SystemContext:       s.executor.systemContext,
+			SystemContext:       s.systemContext,
 			BlobDirectory:       s.executor.blobDirectory,
 			MaxRetries:          s.executor.maxPullPushRetries,
 			RetryDelay:          s.executor.retryPullPushDelay,
@@ -2431,7 +2433,7 @@ func (s *StageExecutor) commit(ctx context.Context, createdBy string, emptyLayer
 		SignaturePolicyPath:   s.executor.signaturePolicyPath,
 		ReportWriter:          writer,
 		PreferredManifestType: s.executor.outputFormat,
-		SystemContext:         s.executor.systemContext,
+		SystemContext:         s.systemContext,
 		Squash:                squash,
 		OmitHistory:           s.executor.commonBuildOptions.OmitHistory,
 		EmptyLayer:            emptyLayer,

tests/bud.bats

@@ -6573,6 +6573,21 @@ _EOF
   done
 }
 
+@test "build must reset platform for stages if needed" {
+  run_buildah info --format '{{.host.arch}}'
+  myarch="$output"
+  otherarch="arm64"
+  # just make sure that other arch is not equivalent to host arch
+  if [[ "$otherarch" == "$myarch" ]]; then
+    otherarch="amd64"
+  fi
+  run_buildah build $WITH_POLICY_JSON --build-arg FOREIGNARCH=$otherarch -f $BUDFILES/multiarch/Containerfile.reset-platform $BUDFILES/multiarch
+  run_buildah build $WITH_POLICY_JSON --build-arg TARGETPLATFORM=linux/$myarch --build-arg FOREIGNARCH=$otherarch -f $BUDFILES/multiarch/Containerfile.reset-platform $BUDFILES/multiarch
+  # Following line here attempts to cover edge case discussed here https://github.com/containers/buildah/pull/5971/files#r1970277844 but it's hard to reproduce this in a certain
+  # way since it can happen only when unused stage is picked in a particular order, following line ensures if this thing breaks then we luckily catch it in our CI.
+  run_buildah build $WITH_POLICY_JSON --jobs=2 --skip-unused-stages=true --build-arg FOREIGNARCH=$otherarch -f $BUDFILES/multiarch/Containerfile.unused-stage $BUDFILES/multiarch
+}
+
 # * Performs multi-stage build with label1=value1 and verifies
 # * Relabels build with label1=value2 and verifies
 # * Rebuild with label1=value1 and makes sure everything is used from cache

tests/bud/multiarch/Containerfile.reset-platform

@@ -0,0 +1,7 @@
+ARG FOREIGNARCH
+FROM --platform=linux/$FOREIGNARCH busybox AS foreign
+FROM busybox
+COPY --from=foreign /bin/busybox /bin/busybox.foreign
+RUN arch
+RUN ls -l /bin/busybox /bin/busybox.foreign
+RUN ! cmp /bin/busybox /bin/busybox.foreign

tests/bud/multiarch/Containerfile.unused-stage

@@ -0,0 +1,3 @@
+FROM busybox
+RUN arch
+FROM --platform=linux/$FOREIGNARCH busybox AS foreign