cmd-import: pull disk images from OCI#4591
Conversation
|
Skipping CI for Draft Pull Request. |
There was a problem hiding this comment.
Code Review
This pull request adds support to cosa import for automatically discovering and downloading disk image artifacts from an OCI image index, introducing a new --download command-line option. The review feedback focuses on improving the robustness of the implementation, specifically by ensuring that the oras CLI tool is validated before use, handling cases where requested downloads are unavailable, and adding defensive checks to prevent crashes when parsing potentially malformed JSON payloads or missing manifest keys.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
/retest |
|
/gemini-review |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthrough
ChangesDisk image discovery and download
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant cosa_import
participant OCI_registry
participant oras_client
participant build_storage
cosa_import->>OCI_registry: Probe srcimg for OCI index
OCI_registry-->>cosa_import: Return index manifest
cosa_import->>cosa_import: Discover and select disk image artifacts
cosa_import->>oras_client: Fetch platform manifest and blob
oras_client-->>cosa_import: Return downloaded disk image
cosa_import->>build_storage: Update metadata and finalize files
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/cmd-import`:
- Around line 236-258: Validate registry-controlled artifact fields before they
are used to build filenames or populate build_meta['images']: in the cmd-import
flow where platform = entry.get('platform', {}), oci_goarch_to_basearch(), and
artifact_type_to_extension() are used, reject empty or unsafe platform.os,
platform.architecture, and artifactType-derived suffixes before appending to
artifacts or downloading. Add explicit checks in the manifest-processing path to
fail fast on unsupported platform names and only allow values that are valid for
the build metadata schema, so downstream path/key construction never sees
untrusted components.
- Around line 68-75: The `--download` path in `cmd-import` compares `args.arch`
directly against `disk_image_artifacts` entries, but those entries are
normalized with `oci_goarch_to_basearch()`, so mismatched aliases like `arm64`
vs `aarch64` can fail to match. Update the `download_arch` selection and the
artifact filtering in the `--download` branch to apply the same architecture
normalization used for OCI index entries, likely by normalizing `args.arch`
before comparing in the list comprehension and error message.
- Around line 60-71: The current try_fetch_oci_index()/--download flow is
masking real registry fetch failures as “not an OCI image index.” Update the
src/cmd-import logic around try_fetch_oci_index, registry_base_from_ref, and the
--download check so only known non-registry transports are treated as a missing
index, while actual oras manifest fetch/auth/transport errors are allowed to
surface when args.download is set. Keep the early
disk_image_artifacts/index_manifest handling, but distinguish between “not
supported transport” and genuine fetch failure before printing the OCI index
error.
- Around line 201-207: The ref normalization in registry_base_from_ref() still
leaves tags on refs like registry/repo:tag@sha256:..., so update the stripping
logic to remove the digest first and then always strip any tag from the final
path segment. Keep the fix localized to registry_base_from_ref() and ensure the
returned base is tag-free before later ORAS fetches use it.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 680e01e9-32b8-4731-99f0-fab7de0db016
📒 Files selected for processing (2)
src/cmd-importsrc/deps.txt
Support using coreos/coreos-assembler#4591 to download the pre-built QEMU image from the konflux builds instead of building it manually.
joelcapitao
left a comment
There was a problem hiding this comment.
LGTM
I left an in-line comment that can be addressed in a follow-up patch
There is also a fixup typo in the commit message but I would not trigger pipelines again only for its removal (just in case there is another edit before the merge)
Support using coreos/coreos-assembler#4591 to download the pre-built QEMU image from the konflux builds instead of building it manually.
437b7ab to
6a07621
Compare
In Konflux we are building the disk images using image-builder and
storing them as intermediary artifacts in OCI.
Abusing the OCI manifest spec a little bit we describe the disk images
types under `platform.os` with the artifact name.
This allows to create a big manifest list referencing the regular OCI
container images from the base build alongside all the disk images.
Here is an example:
```json
{
"schemaVersion": 2,
"mediaType": "application/vnd.oci.image.index.v1+json",
"manifests": [
{
"mediaType": "application/vnd.oci.image.manifest.v1+json",
"digest": "sha256:1e247...",
"size": 771,
"platform": {
"architecture": "amd64",
"os": "qemu"
},
"artifactType": "application/vnd.diskimage.qcow2"
},
...
]
}
```
With this change, `cosa import` knows how to find a disk image from such
an index and download them the `--download` flag.
e.g.
`cosa import docker://quay.io/konflux-fedora/coreos-tenant/disk-images-rawhide:on-pr-2d54f6ac04172313b4a3eec102d55bbd9def3097 --download qemu`
will result in a build dir populated with the OCI artifact and the
qemu artifact:
```
cosa list
45.20260701.91.1
Timestamp: 2026-07-01T06:09:39Z (8:15:24 ago)
Artifacts: ostree oci-manifest qemu
Config: c632a672e1b70cc3de53843db2bee58a4edaa367
```
Downloaded blobs are integrity-checked against the layer digest from
the manifest.
The --arch flag allows downloading disk images for a different
architecture than the host (defaults to host basearch).
Non-registry sources (oci-archive:, containers-storage:) are unaffected
and continue to use the standard skopeo-based flow.
Uses the oras Python bindings (python3-oras) for registry interactions
(manifest fetching and blob downloads) instead of shelling out to the
oras CLI tool.
Assisted-by: Opencode.ai <Opus 4.6>
Authored-by: Opencode.ai <claude-opus-4-6>
Add the ability to pull disk images to an already imported build. When --download is used on a build that already exists, the new augment_existing_build() path adds disk images without re-importing the OCI container image. Platforms already present in the build are skipped. The platform selection logic is also extracted into select_platforms_to_download() for reuse by both code paths. Authored-by: Opencode.ai <claude-opus-4-6>
6a07621 to
9bb1b4f
Compare
|
@coderabbitai help |
ChatThere are 3 ways to chat with CodeRabbit:
CodeRabbit commands
Other keywords and placeholders
CodeRabbit configuration file (
|
|
@coderabbitai full review |
✅ Action performedFull review finished. |
In Konflux we are building the disk images using image-builder and storing them as intermediary artifacts in OCI.
Abusing the OCI manifest spec a little bit we describe the disk images types under
platform.oswith the artifact name.This allows to create a big manifest list referencing the regular OCI container images from the base build alongside all the disk images. Here is an example:
{ "schemaVersion": 2, "mediaType": "application/vnd.oci.image.index.v1+json", "manifests": [ { "mediaType": "application/vnd.oci.image.manifest.v1+json", "digest": "sha256:1e24720d2ba7aba2a1cb8d82eb40e4c3fdde109c40d9a0eb118889ce5bedf6e3", "size": 771, "platform": { "architecture": "amd64", "os": "qemu" }, "artifactType": "application/vnd.diskimage.qcow2" }, { "mediaType": "application/vnd.oci.image.manifest.v1+json", "digest": "sha256:c7e32f00573f696796dec43df101a00db23e0b4702271fa6c7b8f00dbacca6ea", "size": 773, "platform": { "architecture": "amd64", "os": "metal" }, "artifactType": "application/vnd.diskimage.raw.gzip" }, { "mediaType": "application/vnd.oci.image.manifest.v1+json", "digest": "sha256:2f568edad2c832b24a67e362c4056eec49a658228f3f00ac597c9ce206c80261", "size": 11457, "platform": { "architecture": "amd64", "os": "linux" } } ] }With this change,
cosa importknows how to pull a disk image from such a manifest with the--downloadflag, passing the artifact name. e.g.cosa import docker://quay.io/konflux-fedora/coreos-tenant/disk-images-rawhide:on-pr-2d54f6ac04172313b4a3eec102d55bbd9def3097 --download qemuwill result in a build dir populated with the OCI artifact and the qemu artifact:Assisted-by: Opencode.ai <Opus 4.6>
Summary by CodeRabbit
cosa importcan now detect disk image artifacts inside OCI image indexes, report discovered disk images, and optionally download them into the imported build.--download(comma-separated platforms orall) and--archto control which disk images are fetched and added.--downloadnow augments it with the requested disk images instead of blocking.