Skip to content

Commit

Permalink
stage_executor: reset platform in systemcontext for stages
Browse files Browse the repository at this point in the history
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 <flouthoc.git@gmail.com>
  • Loading branch information
flouthoc committed Feb 18, 2025
1 parent 7fea494 commit f5d79f8
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 16 deletions.
3 changes: 3 additions & 0 deletions imagebuildah/executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
35 changes: 19 additions & 16 deletions imagebuildah/stage_executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -571,8 +572,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,
}
Expand Down Expand Up @@ -827,7 +828,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,
Expand Down Expand Up @@ -952,19 +953,21 @@ 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)
builderSystemContext := s.systemContext
// 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 != "" || (stage.Position != 0 && builderSystemContext.ArchitectureChoice == "" && builderSystemContext.VariantChoice == "" && builderSystemContext.OSChoice == "") {
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 != "" {
builderSystemContext.ArchitectureChoice = imageArch
builderSystemContext.VariantChoice = imageVariant
}
if os != "" {
builderSystemContext.OSChoice = os
if imageOS != "" {
builderSystemContext.OSChoice = imageOS
}
}

Expand Down Expand Up @@ -2041,7 +2044,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
}
Expand Down Expand Up @@ -2154,7 +2157,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,
Expand Down Expand Up @@ -2192,7 +2195,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,
Expand Down Expand Up @@ -2414,7 +2417,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,
Expand Down
12 changes: 12 additions & 0 deletions tests/bud.bats
Original file line number Diff line number Diff line change
Expand Up @@ -6486,6 +6486,18 @@ _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
}

# * 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
Expand Down
7 changes: 7 additions & 0 deletions tests/bud/multiarch/Containerfile.reset-platform
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit f5d79f8

Please sign in to comment.