mz480: multitarget builds - part 1

[mzihlmann]

Fixes #480

Description

First part of multi-target builds. This change allows building of multiple stages, but so far still only pushing a single stage. This allows for use-cases where after build we want to run tests. So far these use-cases had to rely on --skip-unused-stages=false, which we're going to deprecate.

FROM base AS final

FROM final AS test

In this case we could now run --target=final,test. Note that the order is important. The first stage in that list is the stage we're going to push, but it doesn't necessarily need to be the last stage we're going to build.

[mzihlmann]

let's also add plan and the new "golden tests" here or in a part - 0

[mzihlmann]

@Bixilon I think that would cover your use-case?

[Bixilon]

oh yes, precisely!

[mzihlmann]

depends on #486 for its integration tests

[mzihlmann]

AI summary review

Overview

Adds support for building multiple stages via --target=stage1 --target=stage2. The first target in the list is the stage that gets pushed; subsequent targets are built but not pushed. Enables test-stage use-cases previously requiring --skip-unused-stages=false.

---

Code Quality

Good:

• Clean separation of Push (first target, gets pushed) vs Final (last target, ends the loop). Previously Final was overloaded for both.
• pushStage is captured before slices.Sort/slices.Compact, which correctly preserves user ordering intent.
• Deduplication via slices.Sort + slices.Compact handles repeated targets gracefully (including case-insensitive duplicates, since index lookup uses EqualFold).
• squash() correctly propagates the new Push field (Push: b.Push).
• Error message upgraded from %s to %q for invalid targets — better UX.

Concerns:

• Ordering is implicit and fragile. The push stage is whichever appears first in the --target list, not whichever is declared first in the Dockerfile. A user writing --target=test --target=final will push the test stage. This is documented in the PR description, but there's no validation or warning if the push stage comes after the final stage in the Dockerfile (which would currently panic — see below).

• logrus.Panic("pushImage is nil") — while this path is provably unreachable today (since pushStage <= finalStage always holds after sorting), a Panic is a harsh failure mode for what would otherwise be a user error. A defensive return nil, fmt.Errorf(...) would be safer here.

• SaveStage override in onlyUsedStages sets s.SaveStage = stagesDependencies[i] > 0, which means a buildTargets stage with no dependents gets SaveStage=false. This is correct, but it silently overrides whatever saveStage(i, stages) returned earlier — worth a comment.

• TODO comments in test.go — three TODOs flag known inefficiencies ("clean after base stage is unnecessary", "saving the final stage is unnecessary"). These are honest, but the second one appears incorrect: the test stage does FROM final, so saving final is required. Should be double-checked before merging.

---

Specific Suggestions

Location	Issue
pkg/dockerfile/dockerfile.go:300	Add a validation: if pushStage > finalStage (impossible today but would panic silently), return a clear error instead of relying on the panic
pkg/executor/build.go:961	Replace logrus.Panic with return nil, fmt.Errorf(...) — unreachable but safer
golden/testdata/test_issue_mz480/test.go:23	TODO "saving the 'final' stage is unnecessary" looks wrong — test stage unpacks it via UNPACK /kaniko/stages/2
pkg/dockerfile/dockerfile.go:406	Comment why buildTargets[i] stages get SaveStage = stagesDependencies[i] > 0 (overriding the earlier saveStage() call)

---

Tests

• Golden tests cover the key cases: single target, redundant target (build already a dependency of final), and multi-target with a trailing test stage.
• Unit test for targetStages is adapted correctly from the single-target version.
• Missing: a test where the push stage is listed after the final stage (e.g., --target=test --target=final) to document and validate that behavior.

---

Summary

Solid, focused implementation. The Push/Final split is the right abstraction. Main action items before merge: fix or remove the incorrect TODO, replace the panic with a real error return, and consider adding a test for the push-stage ordering edge case.

[mzihlmann]

I chose panic on purpose because this is not indicative of a runtime error, but a programming mistake.

[0hlov3]

LGTM