fix(ISV-7284): detect and reject duplicate stage aliases in Containerfile by ezopezo · Pull Request #44 · konflux-ci/capo

ezopezo · 2026-06-16T13:42:27Z

Parse() now returns ErrDuplicateAlias when two stages share the same alias. Buildah behavior for duplicates is undefined (podman-container-tools/buildah#6731), so we fail early instead of silently producing incorrect results. Once buildah defines and stabilizes the behavior, we can remove this guard and adapt accordingly.

qodo-app-for-konflux-ci · 2026-06-16T13:43:57Z

PR Summary by Qodo

Reject duplicate stage aliases when parsing Containerfiles
🐞 Bug fix 🧪 Tests 🕐 10-20 Minutes

Description

• Fail parsing early when two stages reuse the same alias (undefined buildah behavior).
• Introduce a dedicated ErrDuplicateAlias error for callers to detect and handle.
• Update probe/containerfile tests to assert duplicate-alias inputs are rejected.

Diagram

graph TD
  A["Containerfile text"] --> B["containerfile.Parse()"] --> C["Stages + refs"] --> D["probe.Probe()"] --> E["Build metadata"]
  B --> F["ErrDuplicateAlias"]

High-Level Assessment

The following are alternative approaches to this PR:

1. Match buildah’s current (inconsistent) behavior

➕ No behavior change for users relying on today’s buildah outcomes
➕ Avoids introducing a new error surface area
➖ Build results can be silently wrong/unstable across buildah versions
➖ Harder to reason about provenance of --from references when aliases collide

2. Define deterministic resolution (e.g., “last alias wins” or “first wins”)

➕ Provides stable behavior without failing builds
➕ Can be documented as this project’s contract independent of buildah
➖ May diverge from future buildah semantics and surprise users
➖ Still risks masking authoring mistakes and producing unintended artifacts

3. Allow duplicates but require disambiguation via numeric stage indices

➕ Keeps advanced workflows possible while preventing ambiguous alias lookup
➕ Provides a clear migration path for existing Containerfiles
➖ Requires additional validation logic and documentation
➖ Doesn’t help when Dockerfile syntax relies on alias names (common case)

Recommendation: Failing fast with a typed ErrDuplicateAlias is the safest approach while upstream buildah behavior is undefined: it prevents silently incorrect metadata and forces Containerfile authors to fix ambiguity. If/when buildah defines stable semantics, reassess whether to align exactly or keep a stricter contract.

Files changed (3) +53 / -30

Bug fix (1) +9 / -1

containerfile.goAdd ErrDuplicateAlias and reject reused stage aliases in Parse() +9/-1

Add ErrDuplicateAlias and reject reused stage aliases in Parse()

• Introduces ErrDuplicateAlias to represent duplicate stage alias usage. Parse() now checks for alias reuse while iterating stages and returns a wrapped ErrDuplicateAlias error instead of continuing with ambiguous stage-name resolution.

pkg/containerfile/containerfile.go

Tests (2) +44 / -29

containerfile_test.goAdd unit test asserting Parse() fails on duplicate aliases +15/-0

Add unit test asserting Parse() fails on duplicate aliases
• Adds TestParseDuplicateAlias to ensure parsing returns ErrDuplicateAlias when two stages share the same AS name. Imports errors to validate the wrapped error via errors.Is.
pkg/containerfile/containerfile_test.go

probe_test.goUpdate probe tests to expect duplicate-alias inputs to fail +29/-29

Update probe tests to expect duplicate-alias inputs to fail

• Removes the prior test that asserted metadata extraction with duplicate stage names (matching inconsistent buildah behavior). Adds TestProbeDuplicateAlias to confirm Probe() propagates containerfile.ErrDuplicateAlias, and updates imports accordingly.

pkg/probe/probe_test.go

qodo-app-for-konflux-ci · 2026-06-16T13:45:29Z

CI Feedback 🧐

(Feedback updated until commit `4c2a028`)

A test triggered by this PR failed. Here is an AI-generated analysis of the failure:

Action: ci
Failed stage: Run tests with coverage [❌]
Failed test name: ""
Failure summary: The action failed during `mage coverage` because `go test` could not compile the code due to Go build errors in `pkg/containerfile/containerfile.go`: - `pkg/containerfile/containerfile.go:157:6`: variable `i` is declared but not used - `pkg/containerfile/containerfile.go:163:14`, `164:17`, `175:24`: `index` is referenced but is undefined These compilation errors caused multiple packages (including `github.com/konflux-ci/capo/pkg/containerfile`) to fail building, so `go test -coverprofile=... ./...` exited with code 1.
Relevant error logs: 1: ##[group]Runner Image Provisioner 2: Hosted Compute Agent ... 207: GOTOOLCHAIN: local 208: ##[endgroup] 209: ##[group]Run mage tidy 210: �[36;1mmage tidy�[0m 211: shell: /usr/bin/bash -e {0} 212: env: 213: GOTOOLCHAIN: local 214: ##[endgroup] 215: ##[group]Run mage coverage 216: �[36;1mmage coverage�[0m 217: shell: /usr/bin/bash -e {0} 218: env: 219: GOTOOLCHAIN: local 220: ##[endgroup] 221: # github.com/konflux-ci/capo/pkg/containerfile 222: ##[error]pkg/containerfile/containerfile.go:157:6: declared and not used: i 223: ##[error]pkg/containerfile/containerfile.go:163:14: undefined: index 224: ##[error]pkg/containerfile/containerfile.go:164:17: undefined: index 225: ##[error]pkg/containerfile/containerfile.go:175:24: undefined: index 226: FAIL github.com/konflux-ci/capo/cmd/buildprobe [build failed] 227: FAIL github.com/konflux-ci/capo/cmd/capo [build failed] 228: github.com/konflux-ci/capo/internal/sbom coverage: 0.0% of statements 229: github.com/konflux-ci/capo/internal/testutils coverage: 0.0% of statements 230: FAIL github.com/konflux-ci/capo/pkg [build failed] 231: ok github.com/konflux-ci/capo/pkg/buildargs 0.004s coverage: 92.6% of statements 232: FAIL github.com/konflux-ci/capo/pkg/containerfile [build failed] 233: FAIL github.com/konflux-ci/capo/pkg/probe [build failed] 234: ok github.com/konflux-ci/capo/pkg/storageclient 0.004s coverage: 17.4% of statements 235: FAIL 236: Error: running "go test -coverprofile=coverage.out -covermode=atomic -tags=unit,exclude_graphdriver_btrfs ./..." failed with exit code 1 237: ##[error]Process completed with exit code 1. 238: Post job cleanup.

qodo-app-for-konflux-ci · 2026-06-16T13:48:17Z

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (1) 📜 Skill insights (0)

Context used

✅ Compliance rules (platform): 11 rules

1. ErrDuplicateAlias missing code prefix 📘 Rule violation ⚙ Maintainability

Description

The new sentinel error ErrDuplicateAlias in pkg/ does not start with a bracketed
machine-readable ALL-CAPS code prefix. This breaks the required error message convention and can
hinder reliable identification/classification of errors downstream.

Code

pkg/containerfile/containerfile.go[R113-116]

+// ErrDuplicateAlias is returned when two stages in a Containerfile share
+// the same alias. Buildah behavior for duplicate aliases is undefined for now.
+// (see https://github.com/containers/buildah/issues/6731).
+var ErrDuplicateAlias = errors.New("duplicate stage alias")

Evidence
PR Compliance ID 1356 requires all sentinel errors in pkg/ to start with a bracketed
machine-readable ALL-CAPS code prefix. The added sentinel ErrDuplicateAlias is defined with the
message duplicate stage alias, which lacks any leading bracketed code.
Rule 1356: Sentinel errors in pkg/ must include a bracketed machine-readable code prefix
pkg/containerfile/containerfile.go[113-116]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A new sentinel error `ErrDuplicateAlias` was added under `pkg/`, but its message does not begin with the required bracketed machine-readable code (e.g. `"[ERR_SOMETHING] ..."`).

## Issue Context
Compliance requires sentinel errors in `pkg/` to use the format `^\[[A-Z0-9_]+\] .+` so errors can be reliably detected/triaged.

## Fix Focus Areas
- pkg/containerfile/containerfile.go[113-116]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

2. Index variable mismatch 🐞 Bug ≡ Correctness

Description

Parse() changed the range index variable to i but still references index (and
pullspecs[index]), which will not compile and blocks all builds/tests that import this package.

Code

pkg/containerfile/containerfile.go[R157-163]

+	for i, s := range rawStages {
+		if slices.Contains(stageNames, s.Name) {
+			return nil, fmt.Errorf("%w: stage alias %q is used more than once", ErrDuplicateAlias, s.Name)
+		}
		stageNames = append(stageNames, s.Name)

		isFinal := index == len(rawStages)-1

Evidence

The loop declares i but the body still uses index, which is undefined after the rename, causing
compilation to fail.

pkg/containerfile/containerfile.go[157-176]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`Parse()` uses `for i, s := range rawStages` but still refers to `index` in the loop body (`isFinal`, `stageIndex`, and `baseRef := pullspecs[index]`). This results in compile-time errors (`undefined: index` and unused `i`).

### Issue Context
The PR introduced a duplicate-alias guard in the stage loop and renamed the loop index, but the remaining references were not updated.

### Fix Focus Areas
- pkg/containerfile/containerfile.go[157-176]

### Implementation notes
- Either revert to `for index, s := range rawStages` OR replace all `index` references in the loop with `i`:
 - `isFinal := i == len(rawStages)-1`
 - `stageIndex := i`
 - `baseRef := pullspecs[i]`
 - and any other `index` occurrences within the loop.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

3. O(n²) alias detection 🐞 Bug ➹ Performance

Description

Duplicate-alias detection scans stageNames with slices.Contains on each iteration, making it
O(n²) in the number of stages and mixing alias-uniqueness tracking with the stageNames list used
for stage reference resolution.

Code

pkg/containerfile/containerfile.go[R157-162]

+	for i, s := range rawStages {
+		if slices.Contains(stageNames, s.Name) {
+			return nil, fmt.Errorf("%w: stage alias %q is used more than once", ErrDuplicateAlias, s.Name)
+		}
		stageNames = append(stageNames, s.Name)

Evidence

The added code uses slices.Contains(stageNames, s.Name) prior to appending, which implies a linear
scan each iteration over a growing slice.

pkg/containerfile/containerfile.go[157-162]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The new duplicate-alias guard uses `slices.Contains(stageNames, s.Name)` inside the stage loop. Since `stageNames` grows every iteration, this is quadratic and also ties alias uniqueness checking to `stageNames`, which is primarily used for stage-ref resolution.

### Issue Context
The same function already maintains maps (e.g., `aliasToBase`), so introducing a dedicated `seenAliases` map is straightforward and keeps responsibilities separate.

### Fix Focus Areas
- pkg/containerfile/containerfile.go[157-162]

### Implementation notes
- Add `seenAliases := make(map[string]struct{}, len(rawStages))` before the loop.
- In the loop, do:
 - `if _, ok := seenAliases[s.Name]; ok { ...ErrDuplicateAlias... }`
 - `seenAliases[s.Name] = struct{}{}`
- Keep `stageNames = append(stageNames, s.Name)` as-is since it serves stage ref parsing.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

codecov-commenter · 2026-06-16T14:00:07Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 62.57%. Comparing base (f5f8a7a) to head (31f1eaa).

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #44      +/-   ##
==========================================
+ Coverage   62.34%   62.57%   +0.22%     
==========================================
  Files           9        9              
  Lines        1166     1173       +7     
==========================================
+ Hits          727      734       +7     
  Misses        331      331              
  Partials      108      108

Flag	Coverage Δ
integration-tests	`68.91% <28.57%> (-0.34%)`	⬇️
unit-tests	`35.97% <100.00%> (+0.55%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
pkg/scan.go	`78.50% <100.00%> (+0.45%)`	⬆️

Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f5f8a7a...31f1eaa. Read the comment docs.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Scan() now returns ErrDuplicateAlias when two stages share the same alias. Detection is in checkUnsupportedFeatures() rather than Parse(), because buildprobe must remain unchanged it needs to accept duplicates to match konflux-build-cli functionality. Buildah behavior for duplicates is undefined (podman-container-tools/buildah#6731), so capo fails early instead of trying to resolve builder content. Once buildah resolves this, capo will be adapted accordingly. Signed-off-by: Erik Mravec <emravec@redhat.com> Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>

bclindner

very straightforward, great work

BorekZnovustvoritel

We may want to take another look at Brian's point. This may jumpscare us with unexpected bugs if we ever change some Capo internals.

- stage.Index instead of FinalStage alias comparison for duplicate check. - permalink for konflux-build-cli reference in test comment. Signed-off-by: Erik Mravec <emravec@redhat.com>

qodo-app-for-konflux-ci Bot reviewed Jun 16, 2026

View reviewed changes

Comment thread pkg/containerfile/containerfile.go Outdated

Comment thread pkg/containerfile/containerfile.go Outdated

ezopezo force-pushed the ISV-7284 branch 2 times, most recently from a2b62bf to a18fbd1 Compare June 16, 2026 13:56

ezopezo force-pushed the ISV-7284 branch 7 times, most recently from fc78c3c to fc62e18 Compare June 16, 2026 15:19

ezopezo force-pushed the ISV-7284 branch from fc62e18 to dc3fc0c Compare June 16, 2026 15:20

ezopezo requested review from BorekZnovustvoritel and bclindner June 17, 2026 15:19

bclindner reviewed Jun 17, 2026

View reviewed changes

Comment thread pkg/probe/probe_test.go Outdated

bclindner approved these changes Jun 17, 2026

View reviewed changes

Comment thread pkg/scan.go Outdated

BorekZnovustvoritel reviewed Jun 18, 2026

View reviewed changes

Comment thread pkg/scan.go Outdated

fix(ISV-7284): address review comments

31f1eaa

- stage.Index instead of FinalStage alias comparison for duplicate check. - permalink for konflux-build-cli reference in test comment. Signed-off-by: Erik Mravec <emravec@redhat.com>

BorekZnovustvoritel approved these changes Jun 18, 2026

View reviewed changes

ezopezo merged commit 757a1cb into main Jun 18, 2026
4 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(ISV-7284): detect and reject duplicate stage aliases in Containerfile#44

fix(ISV-7284): detect and reject duplicate stage aliases in Containerfile#44
ezopezo merged 2 commits into
mainfrom
ISV-7284

ezopezo commented Jun 16, 2026

Uh oh!

qodo-app-for-konflux-ci Bot commented Jun 16, 2026

Uh oh!

qodo-app-for-konflux-ci Bot commented Jun 16, 2026 •

edited

Loading

Uh oh!

qodo-app-for-konflux-ci Bot commented Jun 16, 2026

Uh oh!

Uh oh!

Uh oh!

codecov-commenter commented Jun 16, 2026 •

edited

Loading

Uh oh!

Uh oh!

bclindner left a comment

Uh oh!

Uh oh!

BorekZnovustvoritel left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

ezopezo commented Jun 16, 2026

Uh oh!

qodo-app-for-konflux-ci Bot commented Jun 16, 2026

PR Summary by Qodo

Uh oh!

qodo-app-for-konflux-ci Bot commented Jun 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CI Feedback 🧐

(Feedback updated until commit 4c2a028)

Uh oh!

qodo-app-for-konflux-ci Bot commented Jun 16, 2026

Code Review by Qodo

Uh oh!

Uh oh!

Uh oh!

codecov-commenter commented Jun 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

bclindner left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

BorekZnovustvoritel left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

qodo-app-for-konflux-ci Bot commented Jun 16, 2026 •

edited

Loading

(Feedback updated until commit `4c2a028`)

codecov-commenter commented Jun 16, 2026 •

edited

Loading