CNCF packaging unpack

[ilopezluna]

Minor fixes to unpacking CNCF models

[gemini-code-assist]

Code Review

This pull request introduces support for the CNCF ModelPack artifact format, allowing users to package models using the --format cncf flag. The changes include updates to the CLI, builder logic, and distribution packages to handle CNCF-specific media types and metadata, alongside existing Docker-format support. A critical concern was raised regarding the downgrade of OpenTelemetry dependencies in go.mod, which could potentially re-introduce security vulnerabilities.

I am having trouble creating individual review comments. Click here to see my feedback.

go.mod (132-133)

This change downgrades go.opentelemetry.io/otel/sdk and go.opentelemetry.io/otel/sdk/metric from v1.43.0 to v1.41.0. Downgrading dependencies can re-introduce security vulnerabilities that were fixed in newer versions, posing a supply chain risk. Please confirm that this downgrade is necessary due to constraints from other dependencies (like the new modelpack/model-spec). If it is, please confirm that this older version does not contain any known critical vulnerabilities. It's recommended to run a dependency vulnerability scan to ensure the safety of this change.

References

1. The style guide requires reviewers to consider security, including supply chain risks. A dependency downgrade is a potential supply chain risk that should be carefully evaluated. As this could be a security flaw, it is considered a 'Critical' issue. ^(link)

[sourcery-ai]

Hey - I've found 1 issue, and left some high level feedback:

• In remoteLayer.DiffID, any JSON parsing error or malformed config now causes a silent fallback to l.desc.Digest instead of returning an error as before; consider distinguishing fatal parse errors from simple "no diffID found" so callers can detect corrupt configs rather than quietly proceeding.
• The extractDiffIDs helper currently conflates "index out of range" and "config structure not present/parseable" by returning a zero hash in both cases; if these scenarios should be handled differently, consider returning a typed error or an explicit found bool to signal the reason for the fallback.
• With isCNCFModel now relying solely on manifest.ArtifactType, you may want to keep a defensive check on Config.MediaType as a secondary guard (or at least log unexpected combinations) to avoid misclassifying artifacts that incorrectly set the artifactType.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- In `remoteLayer.DiffID`, any JSON parsing error or malformed config now causes a silent fallback to `l.desc.Digest` instead of returning an error as before; consider distinguishing fatal parse errors from simple "no diffID found" so callers can detect corrupt configs rather than quietly proceeding.
- The `extractDiffIDs` helper currently conflates "index out of range" and "config structure not present/parseable" by returning a zero hash in both cases; if these scenarios should be handled differently, consider returning a typed error or an explicit `found bool` to signal the reason for the fallback.
- With `isCNCFModel` now relying solely on `manifest.ArtifactType`, you may want to keep a defensive check on `Config.MediaType` as a secondary guard (or at least log unexpected combinations) to avoid misclassifying artifacts that incorrectly set the artifactType.

## Individual Comments

### Comment 1
<location path="pkg/distribution/internal/bundle/unpack.go" line_range="47" />
<code_context>
+// CNCF model-spec ("application/vnd.cncf.model.manifest.v1+json").
 // CNCF ModelPack uses a layer-per-file approach with filepath annotations,
 // similar to V0.2, so it can be unpacked using UnpackFromLayers.
 func isCNCFModel(model types.ModelArtifact) bool {
</code_context>
<issue_to_address>
**issue (bug_risk):** Switching isCNCFModel to rely solely on ArtifactType may regress detection for older/partial manifests.

This used to rely on `manifest.Config.MediaType == modelpack.MediaTypeModelConfigV1`, but now depends only on `ArtifactType == modelpack.ArtifactTypeModelManifest`. For older manifests where `artifactType` wasn’t set, or for registries/tools that drop it, CNCF models will no longer be detected even though the config media type is still correct. Please add a backward-compatible fallback, e.g. `artifactType == ... || manifest.Config.MediaType == modelpack.MediaTypeModelConfigV1`, so legacy bundles continue to be recognized.
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}