diff --git a/imagebuildah/executor.go b/imagebuildah/executor.go index e3d49f034e0..2f284f341ab 100644 --- a/imagebuildah/executor.go +++ b/imagebuildah/executor.go @@ -71,7 +71,7 @@ type executor struct { cacheTTL time.Duration containerSuffix string logger *logrus.Logger - stages map[string]*stageExecutor + stages map[int]*stageExecutor // Maps from stage indexes to their stageExecutors, serialized by stagesLock. store storage.Store contextDir string contextDirWritesAreDiscarded bool @@ -115,17 +115,16 @@ type executor struct { layers bool saveStages bool stageLabels bool - stageImageIDs map[string]string // Tracks image IDs for each stage (by name and position) for label references + stageImageIDs map[int]string // Tracks image IDs for output of each stage (indexed by position) for label references. Serialized by stagesLock. noHostname bool noHosts bool useCache bool removeIntermediateCtrs bool forceRmIntermediateCtrs bool - imageMap map[string]string // Used to map images that we create to handle the AS construct. + imageMap map[int]string // Used to map from stage indexes to images that we create to be used in a later FROM...AS construct. Serialized by stagesLock. containerMap map[string]*buildah.Builder // Used to map from image names to only-created-for-the-rootfs containers. - baseMap map[string]struct{} // Holds the names of every base image, as given. - rootfsMap map[string]struct{} // Holds the names of every stage whose rootfs is referenced in a COPY or ADD instruction. - afterDependency map[string]string // Maps stage names to their --after dependency. + baseMap map[string]struct{} // Holds the set of names of every stage's base image, with ARGs resolved. + rootfsMap map[int]struct{} // Holds the set of indexes for every stage whose rootfs is referenced in a COPY or ADD instruction. blobDirectory string excludes []string groupAdd []string @@ -148,8 +147,8 @@ type executor struct { cachePushDestinationLookupReferenceFunc libimage.LookupReferenceFunc ociDecryptConfig *encconfig.DecryptConfig lastError error - terminatedStage map[string]error - stagesLock sync.Mutex + terminatedStage map[int]error // maps from stage indexes to error results, serialized by stagesLock + stagesLock sync.Mutex // serializes stages, stageImageIDs, imageMap, terminatedStage stagesSemaphore *semaphore.Weighted logRusage bool rusageLogFile io.Writer @@ -164,9 +163,9 @@ type executor struct { unsetEnvs []string unsetLabels []string unsetAnnotations []string - processLabel string // Shares processLabel of first stage container with containers of other stages in same build - mountLabel string // Shares mountLabel of first stage container with containers of other stages in same build - buildOutputs []string // Specifies instructions for any custom build output + processLabel string // processLabel to assign to all RUN instructions + mountLabel string // mountLabel to assign for all containers in all stages + buildOutputs []string // values for internal/output.GetBuildOutput() osVersion string osFeatures []string envs []string @@ -270,7 +269,7 @@ func newExecutor(logger *logrus.Logger, logPrefix string, store storage.Store, o cacheTTL: options.CacheTTL, containerSuffix: options.ContainerSuffix, logger: logger, - stages: make(map[string]*stageExecutor), + stages: make(map[int]*stageExecutor), store: store, contextDir: options.ContextDirectory, contextDirWritesAreDiscarded: contextWritesDiscarded, @@ -318,16 +317,16 @@ func newExecutor(logger *logrus.Logger, logPrefix string, store storage.Store, o layers: options.Layers, saveStages: options.SaveStages, stageLabels: options.StageLabels, - stageImageIDs: make(map[string]string), + stageImageIDs: make(map[int]string), noHostname: options.CommonBuildOpts.NoHostname, noHosts: options.CommonBuildOpts.NoHosts, useCache: !options.NoCache, removeIntermediateCtrs: options.RemoveIntermediateCtrs, forceRmIntermediateCtrs: options.ForceRmIntermediateCtrs, - imageMap: make(map[string]string), + imageMap: make(map[int]string), containerMap: make(map[string]*buildah.Builder), baseMap: make(map[string]struct{}), - rootfsMap: make(map[string]struct{}), + rootfsMap: make(map[int]struct{}), blobDirectory: options.BlobDirectory, unusedArgs: make(map[string]struct{}), capabilities: capabilities, @@ -343,7 +342,7 @@ func newExecutor(logger *logrus.Logger, logPrefix string, store storage.Store, o cachePushSourceLookupReferenceFunc: options.CachePushSourceLookupReferenceFunc, cachePushDestinationLookupReferenceFunc: options.CachePushDestinationLookupReferenceFunc, ociDecryptConfig: options.OciDecryptConfig, - terminatedStage: make(map[string]error), + terminatedStage: make(map[int]error), stagesSemaphore: options.JobSemaphore, logRusage: options.LogRusage, rusageLogFile: rusageLogFile, @@ -442,10 +441,7 @@ func (b *executor) startStage(ctx context.Context, stage *imagebuilder.Stage, st output: output, stage: stage, } - b.stages[stage.Name] = stageExec - if idx := strconv.Itoa(stage.Position); idx != stage.Name { - b.stages[idx] = stageExec - } + b.stages[stage.Position] = stageExec return stageExec } @@ -466,19 +462,37 @@ func (b *executor) resolveNameToImageRef(output string) (types.ImageReference, e return imageRef, err } -// waitForStage waits for an entry to be added to terminatedStage indicating -// that the specified stage has finished. If there is no stage defined by that -// name, then it will return (false, nil). If there is a stage defined by that -// name, it will return true along with any error it encounters. -func (b *executor) waitForStage(ctx context.Context, name string, stages imagebuilder.Stages) (bool, error) { - found := false - for _, otherStage := range stages { - if otherStage.Name == name || strconv.Itoa(otherStage.Position) == name { - found = true - break +// stageIndex locates a stage by a string which can be either its name or its +// position, returning the position and the corresponding stageExecutor if a +// match is found. If not, it returns -1 and nil. Acquires b.stagesLock, as +// we expect stages to be b.stages or a subslice of it. +func (b *executor) stageIndex(nameOrIndex string, stages imagebuilder.Stages) (int, *stageExecutor) { + b.stagesLock.Lock() + defer b.stagesLock.Unlock() + return b.stageIndexUnlocked(nameOrIndex, stages) +} + +// stageIndex locates a stage by a string which can be either its name or its +// position, returning the position and the corresponding stageExecutor if a +// match is found. If not, it returns -1 and nil. The caller should acquire +// b.stagesLock before calling this method. +func (b *executor) stageIndexUnlocked(nameOrIndex string, stages imagebuilder.Stages) (int, *stageExecutor) { + for _, otherStage := range slices.Backward(stages) { + if otherStage.Name == nameOrIndex || strconv.Itoa(otherStage.Position) == nameOrIndex { + return otherStage.Position, b.stages[otherStage.Position] } } - if !found { + return -1, nil +} + +// waitForStage waits for an entry to be added to terminatedStage indicating +// that the last stage with the specified name has finished. If there is no +// stage defined by that name, then it will return (false, -1, nil). If there +// is a stage defined by that name, it will return true along with the stage's +// index and any error it encounters. +func (b *executor) waitForStage(ctx context.Context, name string, stages imagebuilder.Stages) (bool, error) { + otherStageIndex, _ := b.stageIndex(name, stages) + if otherStageIndex == -1 { return false, nil } for { @@ -487,7 +501,7 @@ func (b *executor) waitForStage(ctx context.Context, name string, stages imagebu } b.stagesLock.Lock() - terminationError, terminated := b.terminatedStage[name] + terminationError, terminated := b.terminatedStage[otherStageIndex] b.stagesLock.Unlock() if terminationError != nil { @@ -546,7 +560,7 @@ func (b *executor) getImageTypeAndHistoryAndDiffIDs(ctx context.Context, imageID return oci.OS, oci.Architecture, manifestFormat, oci.History, oci.RootFS.DiffIDs, nil } -func (b *executor) buildStage(ctx context.Context, cleanupStages map[int]*stageExecutor, stages imagebuilder.Stages, stageIndex int) (imageID string, commitResults *buildah.CommitResults, onlyBaseImage bool, err error) { +func (b *executor) buildStage(ctx context.Context, cleanupStages map[int]*stageExecutor, stages imagebuilder.Stages, stageIndex int, afterDependency map[int]int) (imageID string, commitResults *buildah.CommitResults, onlyBaseImage bool, err error) { var prependInstructions, appendInstructions []string stage := stages[stageIndex] ib := stage.Builder @@ -554,10 +568,10 @@ func (b *executor) buildStage(ctx context.Context, cleanupStages map[int]*stageE // Wait for any --after deps before ib.From(node) which may try to access images // via local transports (like oci-archive:) populated by those deps. - if afterDep, ok := b.afterDependency[stage.Name]; ok { - logrus.Debugf("stage %d (%s): waiting for --after dependency %q", stageIndex, stage.Name, afterDep) - if isStage, err := b.waitForStage(ctx, afterDep, stages[:stageIndex]); isStage && err != nil { - return "", nil, false, fmt.Errorf("waiting for --after=%s: %w", afterDep, err) + if afterDep, ok := afterDependency[stage.Position]; ok { + logrus.Debugf("stage %d (%s): waiting for --after dependency stage %d", stageIndex, stage.Name, afterDep) + if isStage, err := b.waitForStage(ctx, strconv.Itoa(afterDep), stages[:stageIndex]); isStage && err != nil { + return "", nil, false, fmt.Errorf("waiting for --after=%d: %w", afterDep, err) } } @@ -610,7 +624,7 @@ func (b *executor) buildStage(ctx context.Context, cleanupStages map[int]*stageE // Skip if stage has no instructions if b.saveStages && b.stageLabels && len(stage.Node.Children) > 0 { // Wait for base stage if it references a previous stage - // This ensures stageImageIDs map is populated before buildStageLabelLine reads it + // This ensures that stageImageIDs has the base stage's output image's ID before buildStageLabelLine reads it if isStage, err := b.waitForStage(ctx, base, stages[:stageIndex]); isStage && err != nil { return "", nil, false, fmt.Errorf("waiting for base stage %s: %w", base, err) } @@ -675,13 +689,10 @@ func (b *executor) buildStage(ctx context.Context, cleanupStages map[int]*stageE return "", nil, onlyBaseImage, err } - // Store image ID for this stage so subsequent stages can reference it in labels + // Store image ID for this stage's result so subsequent stages can reference it in labels if imageID != "" { b.stagesLock.Lock() - if stage.Name != "" { - b.stageImageIDs[stage.Name] = imageID - } - b.stageImageIDs[strconv.Itoa(stageIndex)] = imageID + b.stageImageIDs[stageIndex] = imageID b.stagesLock.Unlock() } @@ -724,17 +735,12 @@ func (b *executor) buildStageLabelLine(stage *imagebuilder.Stage, base string, s // Check if base of the stage is another (previous) stage. // If yes, base is set as image ID of this stage. // If not original base name is set (pullspec). - for i, st := range stages { - if st.Name == base || strconv.Itoa(st.Position) == base { - b.stagesLock.Lock() - if imgID, ok := b.stageImageIDs[base]; ok { - base = imgID - } else if imgID, ok := b.stageImageIDs[strconv.Itoa(i)]; ok { - base = imgID - } - b.stagesLock.Unlock() - break + if otherStageIndex, _ := b.stageIndex(base, stages); otherStageIndex != -1 { + b.stagesLock.Lock() + if imgID, ok := b.stageImageIDs[otherStageIndex]; ok { + base = imgID } + b.stagesLock.Unlock() } labelLine += fmt.Sprintf(" %q=%q", "io.buildah.stage.base", base) return labelLine @@ -868,17 +874,25 @@ func (b *executor) Build(ctx context.Context, stages imagebuilder.Stages) (image } }() - // dependencyMap contains dependencyInfo for each stage, - // dependencyInfo is used later to mark if a particular - // stage is needed by target or not. + // dependencyMap contains dependencyInfo for each stage, indexed by + // both position and name (where name naturally points to the most + // recently processed stage with that name, if names are reused). + // dependencyInfo is used later to work out if a particular stage is + // needed by the target stage. dependencyMap := make(map[string]*stageDependencyInfo) - // Initialize afterDependency map to track --after= dependency per stage - b.afterDependency = make(map[string]string) + // Initialize afterDependency map to track --after= dependency per stage, mapping + // from a stage's index to the index of the one it's supposed to be built "after" + afterDependency := make(map[int]int) // Build maps of every named base image and every referenced stage root // filesystem. Individual stages can use them to determine whether or // not they can skip certain steps near the end of their stages. for stageIndex, stage := range stages { - dependencyMap[stage.Name] = &stageDependencyInfo{Name: stage.Name, Position: stage.Position} + currentStageInfo := &stageDependencyInfo{Name: stage.Name, Position: stage.Position} + idx := strconv.Itoa(stage.Position) + dependencyMap[idx] = currentStageInfo + if stage.Name != idx { + dependencyMap[stage.Name] = currentStageInfo + } stageLocalScopeArgs := make(map[string]string) node := stage.Node // first line for node != nil { // each line @@ -915,11 +929,9 @@ func (b *executor) Build(ctx context.Context, stages imagebuilder.Stages) (image logrus.Debugf("base for stage %d: %q resolves to %q", stageIndex, base, baseWithArg) // Check if selected base is not an additional // build context and if base is a valid stage - // add it to current stage's dependency tree. if _, ok := b.additionalBuildContexts[baseWithArg]; !ok { if _, ok := dependencyMap[baseWithArg]; ok { // update current stage's dependency info - currentStageInfo := dependencyMap[stage.Name] currentStageInfo.Needs = append(currentStageInfo.Needs, baseWithArg) } } @@ -931,7 +943,7 @@ func (b *executor) Build(ctx context.Context, stages imagebuilder.Stages) (image // only allow one --after flag per FROM for now; nothing necessarily // semantically wrong with multiple --after, but keeping it conservative until a // use case shows up - if _, exists := b.afterDependency[stage.Name]; exists { + if _, exists := afterDependency[stage.Position]; exists { return "", nil, fmt.Errorf("FROM --after=%s: only one --after flag is allowed per FROM instruction", after) } builtinArgs := argsMapToSlice(stage.Builder.BuiltinArgDefaults) @@ -945,30 +957,28 @@ func (b *executor) Build(ctx context.Context, stages imagebuilder.Stages) (image if err != nil { return "", nil, fmt.Errorf("while replacing arg variables with values for --after=%q: %w", after, err) } - // If --after= convert index to name - if index, err := strconv.Atoi(afterResolved); err == nil && index >= 0 && index < stageIndex { - afterResolved = stages[index].Name - } - if depInfo, ok := dependencyMap[afterResolved]; !ok { + depInfo, ok := dependencyMap[afterResolved] + if !ok { return "", nil, fmt.Errorf("FROM --after=%s: stage %q not found", after, afterResolved) - } else if depInfo.Position >= stageIndex { + } + if depInfo.Position >= stageIndex { return "", nil, fmt.Errorf("FROM --after=%s: cannot depend on later stage %q", after, afterResolved) } - // Mark the stage as a dep so we actually build it - currentStageInfo := dependencyMap[stage.Name] - currentStageInfo.Needs = append(currentStageInfo.Needs, afterResolved) - // And mark it on the stage executor itself so it knows to wait before even pulling - b.afterDependency[stage.Name] = afterResolved - logrus.Debugf("stage %d: explicit dependency on %q via --after", stageIndex, afterResolved) + // Mark the stage as a dep so we don't skip + // building it + currentStageInfo.Needs = append(currentStageInfo.Needs, strconv.Itoa(depInfo.Position)) + // And mark it for the stage executor itself + // so it knows to wait before even starting + afterDependency[stage.Position] = depInfo.Position + logrus.Debugf("stage %d: explicit dependency on %q(%d) via --after", stageIndex, afterResolved, depInfo.Position) } } case "ADD", "COPY": for _, flag := range child.Flags { // flags for this instruction - if after, ok := strings.CutPrefix(flag, "--from="); ok { + if copyFrom, ok := strings.CutPrefix(flag, "--from="); ok { // Populate dependency tree and check // if following ADD or COPY needs any other // stage. - stageName := after builtinArgs := argsMapToSlice(stage.Builder.BuiltinArgDefaults) headingArgs := argsMapToSlice(stage.Builder.HeadingArgs) userArgs := argsMapToSlice(stage.Builder.Args) @@ -976,27 +986,28 @@ func (b *executor) Build(ctx context.Context, stages imagebuilder.Stages) (image // ProcessWord uses first match; put highest priority first so // --build-arg overrides stage ARG overrides header ARG overrides builtin. userArgs = slices.Concat(userArgs, localScopeArgs, headingArgs, builtinArgs) - baseWithArg, err := imagebuilder.ProcessWord(stageName, userArgs) + copyFromWithArg, err := imagebuilder.ProcessWord(copyFrom, userArgs) if err != nil { - return "", nil, fmt.Errorf("while replacing arg variables with values for format %q: %w", stageName, err) - } - if baseWithArg != "" { - b.rootfsMap[baseWithArg] = struct{}{} + return "", nil, fmt.Errorf("while replacing arg variables with values for format %q: %w", copyFrom, err) } - logrus.Debugf("stage %d name: %q resolves to %q", stageIndex, stageName, baseWithArg) - stageName = baseWithArg - // If --from= convert index to name - if index, err := strconv.Atoi(stageName); err == nil && index >= 0 && index < stageIndex { - stageName = stages[index].Name - } - // Check if selected base is not an additional - // build context and if base is a valid stage - // add it to current stage's dependency tree. - if _, ok := b.additionalBuildContexts[stageName]; !ok { - if _, ok := dependencyMap[stageName]; ok { + logrus.Debugf("stage %d name: %q resolves to %q", stageIndex, copyFrom, copyFromWithArg) + // Check if this "from" is a stage; add it + // to the current stage's dependency tree + // unless it's been replaced by an + // additional context. + if _, ok := b.additionalBuildContexts[copyFromWithArg]; !ok { + // Treat the "from" as a rootfs we need to preserve + if otherStageIndex, _ := b.stageIndex(copyFromWithArg, stages[:stageIndex]); otherStageIndex != -1 { + b.rootfsMap[otherStageIndex] = struct{}{} // update current stage's dependency info - currentStageInfo := dependencyMap[stage.Name] - currentStageInfo.Needs = append(currentStageInfo.Needs, stageName) + depInfo, ok := dependencyMap[copyFromWithArg] + if !ok { + return "", nil, fmt.Errorf("COPY --from=%s: stage %q not found", copyFrom, copyFromWithArg) + } + if depInfo.Position >= stageIndex { + return "", nil, fmt.Errorf("COPY --from=%s: cannot depend on later stage %q", copyFrom, copyFromWithArg) + } + currentStageInfo.Needs = append(currentStageInfo.Needs, strconv.Itoa(otherStageIndex)) } } } @@ -1018,21 +1029,29 @@ func (b *executor) Build(ctx context.Context, stages imagebuilder.Stages) (image // if it is using `--mount` and `from=` field is set // and `from=` points to a stage consider it in // dependency calculation. - if strings.HasPrefix(flag, "--mount=") && strings.Contains(flag, "from") { - mountFlags := strings.TrimPrefix(flag, "--mount=") - fields := strings.SplitSeq(mountFlags, ",") - for field := range fields { - if mountFrom, hasFrom := strings.CutPrefix(field, "from="); hasFrom { - // Check if this base is a stage if yes + if mountFlags, ok := strings.CutPrefix(flag, "--mount="); ok { + for field := range strings.SplitSeq(mountFlags, ",") { + if mountFrom, ok := strings.CutPrefix(field, "from="); ok { + builtinArgs := argsMapToSlice(stage.Builder.BuiltinArgDefaults) + headingArgs := argsMapToSlice(stage.Builder.HeadingArgs) + userArgs := argsMapToSlice(stage.Builder.Args) + localScopeArgs := argsMapToSlice(stageLocalScopeArgs) + // ProcessWord uses first match; put highest priority first so + // --build-arg overrides stage ARG overrides header ARG overrides builtin. + userArgs = slices.Concat(userArgs, localScopeArgs, headingArgs, builtinArgs) + mountFromWithArg, err := imagebuilder.ProcessWord(mountFrom, userArgs) + if err != nil { + return "", nil, fmt.Errorf("while replacing arg variables with values for format %q: %w", mountFrom, err) + } + // Check if this "from" is a stage; if yes // add base to current stage's dependency tree - // but also confirm if this is not in additional context. - if _, ok := b.additionalBuildContexts[mountFrom]; !ok { - // Treat from as a rootfs we need to preserve - b.rootfsMap[mountFrom] = struct{}{} - if _, ok := dependencyMap[mountFrom]; ok { + // unless it's been replaced by an additional context. + if _, ok := b.additionalBuildContexts[mountFromWithArg]; !ok { + // Treat the "from" as a rootfs we need to preserve + if mountStageIndex, _ := b.stageIndex(mountFromWithArg, stages[:stageIndex]); mountStageIndex != -1 { + b.rootfsMap[mountStageIndex] = struct{}{} // update current stage's dependency info - currentStageInfo := dependencyMap[stage.Name] - currentStageInfo.Needs = append(currentStageInfo.Needs, mountFrom) + currentStageInfo.Needs = append(currentStageInfo.Needs, strconv.Itoa(mountStageIndex)) } } } @@ -1043,10 +1062,8 @@ func (b *executor) Build(ctx context.Context, stages imagebuilder.Stages) (image } node = node.Next // next line } - // Last stage is always target stage. - // Since last/target stage is processed - // let's calculate dependency map of stages - // so we can mark stages which can be skipped. + // Last stage is always target stage. Since last/target stage is processed let's + // calculate dependency map of stages so we can mark stages which can be skipped. if stage.Position == (len(stages) - 1) { markDependencyStagesForTarget(dependencyMap, stage.Name) } @@ -1111,7 +1128,7 @@ func (b *executor) Build(ctx context.Context, stages imagebuilder.Stages) (image // is not set to `false`. if stageDependencyInfo, ok := dependencyMap[stages[index].Name]; ok { if !stageDependencyInfo.NeededByTarget && b.skipUnusedStages != types.OptionalBoolFalse { - logrus.Debugf("Skipping stage with Name %q and index %d since its not needed by the target stage", stages[index].Name, index) + logrus.Debugf("Skipping stage with name %q and index %d since it's not needed by the target stage", stages[index].Name, index) ch <- Result{ Index: index, Error: nil, @@ -1119,7 +1136,8 @@ func (b *executor) Build(ctx context.Context, stages imagebuilder.Stages) (image return } } - stageID, stageResults, stageOnlyBaseImage, stageErr := b.buildStage(ctx, cleanupStages, stages, index) + + stageID, stageResults, stageOnlyBaseImage, stageErr := b.buildStage(ctx, cleanupStages, stages, index, afterDependency) if stageErr != nil { cancel = true ch <- Result{ @@ -1149,8 +1167,7 @@ func (b *executor) Build(ctx context.Context, stages imagebuilder.Stages) (image stage := stages[r.Index] b.stagesLock.Lock() - b.terminatedStage[stage.Name] = r.Error - b.terminatedStage[strconv.Itoa(stage.Position)] = r.Error + b.terminatedStage[stage.Position] = r.Error if r.Error != nil { b.stagesLock.Unlock() @@ -1161,7 +1178,7 @@ func (b *executor) Build(ctx context.Context, stages imagebuilder.Stages) (image // If this is an intermediate stage, make a note of the ID, so // that we can look it up later. if r.Index < len(stages)-1 && r.ImageID != "" { - b.imageMap[stage.Name] = r.ImageID + b.imageMap[stage.Position] = r.ImageID // We're not populating the cache with intermediate // images, so add this one to the list of images that // we'll remove later. diff --git a/imagebuildah/stage_executor.go b/imagebuildah/stage_executor.go index 67c728b621d..80f7413938b 100644 --- a/imagebuildah/stage_executor.go +++ b/imagebuildah/stage_executor.go @@ -542,9 +542,9 @@ func (s *stageExecutor) performCopy(excludes []string, copies ...imagebuilder.Co if isStage, err := s.executor.waitForStage(s.ctx, from, s.stages[:s.index]); isStage && err != nil { return err } - if other, ok := s.executor.stages[from]; ok && other.index < s.index { - contextDir = other.mountPoint - idMappingOptions = &other.builder.IDMappingOptions + if otherStageIndex, otherStage := s.executor.stageIndex(from, s.stages[:s.index]); otherStageIndex != -1 { + contextDir = otherStage.mountPoint + idMappingOptions = &otherStage.builder.IDMappingOptions } else if builder, ok := s.executor.containerMap[copy.From]; ok { contextDir = builder.MountPoint idMappingOptions = &builder.IDMappingOptions @@ -759,7 +759,7 @@ func (s *stageExecutor) runStageMountPoints(mountList []string) (map[string]inte } // If the source's name is a stage, return a // pointer to its rootfs. - if otherStage, ok := s.executor.stages[from]; ok && otherStage.index < s.index { + if otherStageIndex, otherStage := s.executor.stageIndex(from, s.stages[:s.index]); otherStageIndex != -1 { stageMountPoints[from] = internal.StageMountDetails{ IsStage: true, DidExecute: otherStage.didExecute, @@ -1029,11 +1029,8 @@ func (s *stageExecutor) prepare(ctx context.Context, from string, initializeIBCo if s.executor.sourcePolicy != nil && from != "scratch" { // Check if 'from' references a previous stage by name, index, or image ID isStageRef := false - for i, st := range s.stages[:s.index] { - if st.Name == from || strconv.Itoa(i) == from { - isStageRef = true - break - } + if stageIndex, _ := s.executor.stageIndex(from, s.stages[:s.index]); stageIndex != -1 { + isStageRef = true } // Also check if 'from' is an image ID that was created by a previous stage // (this happens when execute() resolves stage names to image IDs before calling prepare) @@ -1298,8 +1295,7 @@ func (s *stageExecutor) execute(ctx context.Context, base string) (imgID string, lastStage := !moreStages onlyBaseImage := false imageIsUsedLater := moreStages && (internalUtil.SetHas(s.executor.baseMap, stage.Name) || internalUtil.SetHas(s.executor.baseMap, strconv.Itoa(stage.Position))) - rootfsIsUsedLater := moreStages && (internalUtil.SetHas(s.executor.rootfsMap, stage.Name) || internalUtil.SetHas(s.executor.rootfsMap, strconv.Itoa(stage.Position))) - + rootfsIsUsedLater := moreStages && internalUtil.SetHas(s.executor.rootfsMap, stage.Position) // If the base image's name corresponds to the result of an earlier // stage, make sure that stage has finished building an image, and // substitute that image's ID for the base image's name here and force @@ -1314,10 +1310,12 @@ func (s *stageExecutor) execute(ctx context.Context, base string) (imgID string, pullPolicy := s.executor.pullPolicy s.executor.stagesLock.Lock() var preserveBaseImageAnnotationsAtStageStart bool - if stageImage, isPreviousStage := s.executor.imageMap[base]; isPreviousStage { - base = stageImage - pullPolicy = define.PullNever - preserveBaseImageAnnotationsAtStageStart = true + if otherStageIndex, _ := s.executor.stageIndexUnlocked(base, s.stages[:s.index]); otherStageIndex != -1 { + if stageImage, isPreviousStage := s.executor.imageMap[otherStageIndex]; isPreviousStage { + base = stageImage + pullPolicy = define.PullNever + preserveBaseImageAnnotationsAtStageStart = true + } } s.executor.stagesLock.Unlock() @@ -1540,7 +1538,7 @@ func (s *stageExecutor) execute(ctx context.Context, base string) (imgID string, if isStage, err := s.executor.waitForStage(ctx, from, s.stages[:s.index]); isStage && err != nil { return "", nil, false, err } - if otherStage, ok := s.executor.stages[from]; ok && otherStage.index < s.index { + if otherStageIndex, _ := s.executor.stageIndex(from, s.stages[:s.index]); otherStageIndex != -1 { break } else if _, err = s.getImageRootfs(ctx, from); err != nil { return "", nil, false, fmt.Errorf("%s --from=%s: no stage or image found with that name", command, from) @@ -2075,9 +2073,9 @@ func (s *stageExecutor) getCreatedBy(node *parser.Node, addedContentSummary stri } // Source specified is part of stage, image or additional-build-context. if mountOptionFrom != "" { - if stage, ok := s.executor.stages[mountOptionFrom]; ok { - // If source is a previous stage then checksum is image digest for that stage - if image, isPreviousStage := s.executor.imageMap[stage.name]; isPreviousStage { + if otherStageIndex, _ := s.executor.stageIndex(mountOptionFrom, s.stages[:s.stage.Position]); otherStageIndex != -1 { + // If source is a previous stage then checksum is the image digest for that stage + if image, isPreviousStage := s.executor.imageMap[otherStageIndex]; isPreviousStage { mountCheckSum = image } } else { diff --git a/tests/bud.bats b/tests/bud.bats index 0b2c3f3ad14..8b884c131fd 100644 --- a/tests/bud.bats +++ b/tests/bud.bats @@ -10247,3 +10247,97 @@ _EOF # Verify final image IDs match (complete cache hit) assert "$first_build_final_image_id" "==" "$second_build_final_image_id" "final image ID should match when cache is fully reused" } + +@test "bud-duplicate-stage-names-target" { + local contextdir="${TEST_SCRATCH_DIR}"/context + mkdir -p $contextdir + cat > $contextdir/Dockerfile << EOF + FROM scratch AS stage0 + LABEL x="really stage 0" + + FROM scratch AS stage0 + LABEL x="actually stage 1" + + FROM stage0 +EOF + local target=image0 + for njobs in "" --jobs=3 ; do + for layers in "" --layers ; do + for through in "" --target=stage0 ; do + echo building with options: $layers $through $njobs + run_buildah build $WITH_POLICY_JSON --identity-label=false $layers $through $njobs -t "${target}" "${contextdir}" + run_buildah inspect --format='{{.OCIv1.Config.Labels}}' "${target}" + expect_output 'map[x:actually stage 1]' + run_buildah rmi -a -f + done + done + done +} + +@test "bud-duplicate-stage-names-copy-from" { + _prefetch busybox + + local target=image0 + + local contextdir="${TEST_SCRATCH_DIR}"/context + mkdir -p $contextdir + cat > $contextdir/Dockerfile.1 <<_EOF + FROM busybox AS stage0 + COPY < $contextdir/Dockerfile.2 <<_EOF + FROM ${target} + RUN cat /stage.txt +_EOF + + for njobs in "" --jobs=3 ; do + for layers in "" "--layers --no-cache" ; do + echo building with option: $layers + run_buildah build $WITH_POLICY_JSON $layers $njobs -t "${target}" -f ${contextdir}/Dockerfile.1 "${contextdir}" + run_buildah build $WITH_POLICY_JSON $layers $njobs -t "${target}"-derived -f ${contextdir}/Dockerfile.2 "${contextdir}" + expect_output --substring "actually stage 1" + run_buildah rmi "${target}"-derived + run_buildah rmi "${target}" + done + done +} + +@test "bud-duplicate-stage-names-mount-from" { + _prefetch busybox + + local target=image0 + + local contextdir="${TEST_SCRATCH_DIR}"/context + mkdir -p $contextdir + cat > $contextdir/Dockerfile <<_EOF + FROM busybox AS stage0 + COPY <