Implement build image index

[mkosiarc]

No description provided.

[snyk-io]

✅ Snyk checks have passed. No issues have been found so far.

Status	Scan Engine	Critical	High	Medium	Low	Total (0)
✅	Open Source Security	0	0	0	0	0 issues
✅	Licenses	0	0	0	0	0 issues
✅	Code Security	0	0	0	0	0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

[qodo-code-review]

Review Summary by Qodo

Implement build-image-index command for multi-architecture container images

✨ Enhancement

Walkthroughs

Description

• Implement build-image-index command for multi-architecture image support
• Extend buildah CLI wrapper with manifest management methods
• Add comprehensive parameter validation and format consistency checks
• Include full test coverage for new functionality

Diagram

flowchart LR
  A["User Command<br/>build-image-index"] --> B["BuildImageIndex<br/>Command Handler"]
  B --> C["Manifest Operations"]
  C --> D["ManifestCreate"]
  C --> E["ManifestAdd"]
  C --> F["ManifestInspect"]
  C --> G["ManifestPush"]
  D --> H["Buildah CLI Wrapper"]
  E --> H
  F --> H
  G --> H
  H --> I["Registry Push<br/>with Digest"]

Loading

File Changes

1. cmd/image.go ✨ Enhancement +1/-0

Register build-image-index command

cmd/image.go

---

2. cmd/image/build_image_index.go ✨ Enhancement +49/-0

Implement build-image-index CLI command

cmd/image/build_image_index.go

---

3. pkg/cliwrappers/buildah.go ✨ Enhancement +150/-0

Add manifest management methods to buildah wrapper

pkg/cliwrappers/buildah.go

---

View more (6)

4. pkg/cliwrappers/buildah_test.go 🧪 Tests +331/-0

Test manifest create, add, inspect, push operations

pkg/cliwrappers/buildah_test.go

---

5. pkg/commands/build_image_index.go ✨ Enhancement +363/-0

Implement build image index command logic

pkg/commands/build_image_index.go

---

6. pkg/commands/build_image_index_test.go 🧪 Tests +315/-0

Test parameter validation and format consistency

pkg/commands/build_image_index_test.go

---

7. pkg/commands/cli_mocks_test.go 🧪 Tests +34/-2

Add mock methods for manifest operations

pkg/commands/cli_mocks_test.go

---

8. pkg/common/image_ref.go ✨ Enhancement +47/-0

Add image reference normalization utilities

pkg/common/image_ref.go

---

9. pkg/common/image_ref_test.go 🧪 Tests +91/-0

Test image reference normalization functions

pkg/common/image_ref_test.go

---

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (4) 📘 Rule violations (0) 📎 Requirement gaps (0)

1. Skip mode never pushes 🐞 Bug ✓ Correctness

Description

When always-build-index=false, the command still creates a local manifest list and then returns
early without pushing the requested --image (and without processing
--additional-tags/--output-manifest-path), yet results still report image_url=--image.

Code

pkg/commands/build_image_index.go[R167-187]

+func (c *BuildImageIndex) buildManifestIndex() error {
+	l.Logger.Infof("Creating manifest list: %s", c.Params.Image)
+	err := c.CliWrappers.BuildahCli.ManifestCreate(&cliwrappers.BuildahManifestCreateArgs{
+		ManifestName: c.Params.Image,
+	})
+	if err != nil {
+		return err
+	}
+
+	for _, imageRef := range c.Params.Images {
+		// Normalize the image reference to strip tag if both tag and digest are present
+		// buildah doesn't support the repository:tag@digest format
+		normalizedRef := common.NormalizeImageRefWithDigest(imageRef)
+
+		// Special case: single image with always-build-index=false
+		if !c.Params.AlwaysBuildIndex {
+			l.Logger.Info("Skipping image index generation. Returning results for single image.")
+			c.images = normalizedRef
+			c.imageDigest = common.GetImageDigest(normalizedRef)
+			return nil
+		}

Evidence

buildManifestIndex() always calls ManifestCreate() first, then returns early for
!AlwaysBuildIndex, skipping all push/tag/output-manifest work. Run() unconditionally reports
results as if --image was produced/pushed.

pkg/commands/build_image_index.go[153-156]
pkg/commands/build_image_index.go[167-187]

Agent prompt

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

### Issue description
`always-build-index=false` is meant to skip creating an index for a single image, but the implementation currently:
1) still runs `buildah manifest create`, and
2) returns early without pushing/tagging `--image` or handling `--additional-tags` / `--output-manifest-path`, while still emitting results pointing at `--image`.

### Issue Context
This can cause unnecessary failures (manifest create requires buildah/local storage), and it can mislead downstream consumers that expect the output image/tag to exist.

### Fix Focus Areas
- pkg/commands/build_image_index.go[167-187]
- pkg/commands/build_image_index.go[153-156]

### Suggested fix approach
- Move the `!AlwaysBuildIndex` handling to the start of `buildManifestIndex()` (before `ManifestCreate`).
- Decide semantics for skip mode:
 - If skip mode should still publish `--image` and additional tags, implement a path that tags/copies the single input image to `--image` (+additional tags) and obtains a real digest.
 - If skip mode should not publish anything, adjust results so `image_url`/`image_ref` reflect the input image, not `--image`.
- Ensure `--output-manifest-path` behavior is consistent (either write something meaningful or reject in skip mode).

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

---

2. Skip mode can emit empty digest 🐞 Bug ✓ Correctness

Description

In the skip branch (always-build-index=false), digest is derived from --images[0] via
GetImageDigest(), which returns empty for tag-only refs; validation does not require digests, so
the command can emit invalid image_ref like repo@ and empty image_digest.

Code

pkg/commands/build_image_index.go[R181-186]

+		// Special case: single image with always-build-index=false
+		if !c.Params.AlwaysBuildIndex {
+			l.Logger.Info("Skipping image index generation. Returning results for single image.")
+			c.images = normalizedRef
+			c.imageDigest = common.GetImageDigest(normalizedRef)
+			return nil

Evidence

GetImageDigest() returns an empty string when the reference has no digest. The CLI examples show
--images inputs as digest-pinned, but validateParams() currently only validates image names, not
digest presence/validity, so empty digests can slip through in skip mode and then are concatenated
into image_ref.

pkg/commands/build_image_index.go[181-186]
pkg/common/image_ref.go[72-86]
pkg/commands/build_image_index.go[270-296]
cmd/image/build_image_index.go[19-24]
pkg/commands/push_containerfile.go[251-258]

Agent prompt

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

### Issue description
In `always-build-index=false` mode, the command sets `image_digest` from `GetImageDigest(--images[0])`. For tag-only inputs this returns empty, leading to invalid outputs (e.g., `image_ref` becomes `name@`).

### Issue Context
The CLI examples for `--images` use digest-pinned inputs, but `validateParams()` does not enforce that. Other commands in the repo explicitly validate digests.

### Fix Focus Areas
- pkg/commands/build_image_index.go[181-186]
- pkg/commands/build_image_index.go[270-296]
- pkg/common/image_ref.go[72-86]

### Suggested fix approach
- In `validateParams()`, when `always-build-index=false`:
 - require `len(images)==1` (already enforced) AND
 - require that `common.GetImageDigest(images[0])` is non-empty AND `common.IsImageDigestValid(...)`.
- Consider also validating digests for all `--images` when the command’s contract expects manifest-by-digest inputs.
- If tag-only inputs must be supported, add a resolution step (e.g., inspect/resolve digest) and use that digest for results.

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

---

3. Additional tags not validated 🐞 Bug ⛯ Reliability

Description

--additional-tags are concatenated directly into repo:tag destinations without validation, so
invalid or empty tags fail at runtime; this is inconsistent with other commands that validate tag
syntax.

Code

pkg/commands/build_image_index.go[R270-296]

+func (c *BuildImageIndex) validateParams() error {
+	imageName := common.GetImageName(c.Params.Image)
+	if !common.IsImageNameValid(imageName) {
+		return fmt.Errorf("image name '%s' is invalid", c.Params.Image)
+	}
+
+	if len(c.Params.Images) == 0 {
+		return fmt.Errorf("at least one image must be provided via --images")
+	}
+
+	validFormats := map[string]bool{"oci": true, "docker": true}
+	if !validFormats[c.Params.BuildahFormat] {
+		return fmt.Errorf("format must be 'oci' or 'docker', got '%s'", c.Params.BuildahFormat)
+	}
+
+	for _, img := range c.Params.Images {
+		imgName := common.GetImageName(img)
+		if !common.IsImageNameValid(imgName) {
+			return fmt.Errorf("invalid image reference: %s", img)
+		}
+	}
+
+	if len(c.Params.Images) != 1 && !c.Params.AlwaysBuildIndex {
+		return fmt.Errorf("multiple images provided but always-build-index=false; supplying multiple image inputs is unsupported without always-build-index=true")
+	}
+
+	return nil

Evidence

validateParams() does not validate AdditionalTags at all, while ApplyTags validates tag
strings using common.IsImageTagValid, indicating this repo expects early tag validation.

pkg/commands/build_image_index.go[235-249]
pkg/commands/build_image_index.go[270-296]
pkg/commands/apply_tags.go[294-298]
pkg/common/image_ref.go[27-33]

Agent prompt

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

### Issue description
`--additional-tags` are used to construct `repo:tag` destinations without validation, which can cause late failures and unclear error messages.

### Issue Context
The repo already has a canonical tag validator (`common.IsImageTagValid`) and uses it in `ApplyTags`.

### Fix Focus Areas
- pkg/commands/build_image_index.go[270-296]
- pkg/commands/build_image_index.go[235-249]
- pkg/common/image_ref.go[27-33]

### Suggested fix approach
- Add a loop in `validateParams()` to validate `c.Params.AdditionalTags`:
 - reject empty strings
 - call `common.IsImageTagValid(tag)`
 - optionally dedupe to avoid pushing the same tag multiple times.
- Add unit tests for invalid additional tags.

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

---

4. --image digest not rejected 🐞 Bug ✓ Correctness

Description

validateParams() validates only the trimmed image name, so --image values containing a digest
(e.g., repo:tag@sha256:...) can pass validation but later be used as the manifest
name/destination, likely breaking buildah operations and contradicting the help text (“target image
and tag”).

Code

pkg/commands/build_image_index.go[R167-174]

+func (c *BuildImageIndex) buildManifestIndex() error {
+	l.Logger.Infof("Creating manifest list: %s", c.Params.Image)
+	err := c.CliWrappers.BuildahCli.ManifestCreate(&cliwrappers.BuildahManifestCreateArgs{
+		ManifestName: c.Params.Image,
+	})
+	if err != nil {
+		return err
+	}

Evidence

The code validates --image by trimming it to a name (GetImageName) and checking that name only,
but later passes the original c.Params.Image directly to buildah manifest create and `buildah
manifest push` destination construction.

pkg/commands/build_image_index.go[270-274]
pkg/common/image_ref.go[10-20]
pkg/commands/build_image_index.go[169-171]
pkg/commands/build_image_index.go[221-224]

Agent prompt

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

### Issue description
`--image` is documented as the target image+tag, but validation only checks the trimmed repository name. This allows inputs like `repo:tag@sha256:...` to pass validation yet be used verbatim as the buildah manifest name/destination.

### Issue Context
`common.GetImageName()` intentionally strips tags/digests, so validating only that value does not guarantee the original `--image` is suitable for buildah manifest operations.

### Fix Focus Areas
- pkg/commands/build_image_index.go[270-274]
- pkg/commands/build_image_index.go[169-171]
- pkg/commands/build_image_index.go[221-224]

### Suggested fix approach
- Update `validateParams()` to validate the full `c.Params.Image` reference shape:
 - reject digested refs for `--image` (require tag form)
 - optionally ensure a tag is present if that’s required by the command contract.
- Alternatively, normalize `--image` similarly to `NormalizeImageRefWithDigest` if you want to support `tag@digest` inputs by stripping the digest before use.
- Add tests covering `--image` with digest and `tag@digest` forms.

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

---

ⓘ The new review experience is currently in Beta. Learn more

[mkosiarc]

Related (draft) pr in build-definitions konflux-ci/build-definitions#3321

[chmeliik]

The code mostly LGTM (with one suspicion of a small bug), found some integration test issues though

[chmeliik]

Just a couple smaller issues left, otherwise LGTM. Feel free to ignore the less substantial ones 😄

[chmeliik]

LGTM 🎉