Add support for OCI image manifests and indexes#584
Add support for OCI image manifests and indexes#584aman-coder03 wants to merge 2 commits intouber:masterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds OCI image manifest (application/vnd.oci.image.manifest.v1+json) and OCI image index (application/vnd.oci.image.index.v1+json) support so Kraken can parse and advertise OCI media types in registry interactions.
Changes:
- Extend manifest parsing to fall back from Docker v2 manifest/list to OCI manifest/index formats.
- Include OCI media types in preheat media-type matching and in Accept headers via
GetSupportedManifestTypes(). - Add unit tests for OCI manifest/index parsing and promote
opencontainers/image-specto a direct dependency.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| utils/dockerutil/dockerutil.go | Adds OCI manifest/index parsing and expands supported manifest media types. |
| utils/dockerutil/dockerutil_test.go | Adds unit tests for OCI manifest/index parsing paths. |
| proxy/proxyserver/preheat.go | Updates manifest media type regex to recognize OCI manifest/index events. |
| go.mod | Promotes github.com/opencontainers/image-spec to a direct dependency. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
utils/dockerutil/dockerutil.go
Outdated
| // Try OCI image index. | ||
| return ParseManifestOCIIndex(b) |
There was a problem hiding this comment.
ParseManifest falls through multiple unmarshal attempts but, if all attempts fail, it returns only the final OCI-index error. This can produce misleading error messages (e.g., reporting an OCI-index parse failure for non-OCI inputs) and makes debugging harder. Consider returning an aggregated error that includes failures from each attempted format, or detect the manifest mediaType from JSON first and unmarshal only the matching type.
| _, ok := manifestIndex.(*manifestlist.DeserializedManifestList) | ||
| if !ok { | ||
| return nil, core.Digest{}, errors.New("expected manifestlist.DeserializedManifestList for OCI index") | ||
| } |
There was a problem hiding this comment.
ParseManifestOCIIndex asserts the returned type but does not validate SchemaVersion (unlike ParseManifestV2List, which checks SchemaVersion==2 on the same DeserializedManifestList type). For consistency and to avoid accepting unsupported index versions, store the cast result and validate SchemaVersion before returning.
| _, ok := manifestIndex.(*manifestlist.DeserializedManifestList) | |
| if !ok { | |
| return nil, core.Digest{}, errors.New("expected manifestlist.DeserializedManifestList for OCI index") | |
| } | |
| deserializedManifestIndex, ok := manifestIndex.(*manifestlist.DeserializedManifestList) | |
| if !ok { | |
| return nil, core.Digest{}, errors.New("expected manifestlist.DeserializedManifestList for OCI index") | |
| } | |
| version := deserializedManifestIndex.SchemaVersion | |
| if version != 2 { | |
| return nil, core.Digest{}, fmt.Errorf("unsupported OCI image index version: %d", version) | |
| } |
| var _manifestRegexp = regexp.MustCompile( | ||
| `^application/vnd\.docker\.distribution\.manifest\.v\d\+(json|prettyjws)` + | ||
| `|^application/vnd\.oci\.image\.manifest\.v1\+json` + | ||
| `|^application/vnd\.oci\.image\.index\.v1\+json`, | ||
| ) |
There was a problem hiding this comment.
Adding OCI index media types to the preheat event filter means Kraken will start processing push events whose manifest.References() typically include only child manifests (not layer/config blobs). With the current process() implementation, this can result in preheating only nested manifests rather than the actual layers unless additional manifest push events are also received. Consider either keeping index/list types out of this filter, or recursively fetching referenced manifests and preheating their references to ensure layers are warmed.
| manifest, d, err := dockerutil.ParseManifestOCI(testOCIManifestBytes) | ||
| require.NoError(err) | ||
| mediaType, _, err := manifest.Payload() | ||
| require.NoError(err) | ||
| require.Equal("application/vnd.oci.image.manifest.v1+json", mediaType) |
There was a problem hiding this comment.
This test only asserts that the returned digest uses the sha256 algorithm, which would still pass if the digest value is incorrect. Consider asserting the full digest string matches the expected digest of testOCIManifestBytes (and adding a negative test case to ensure ParseManifestOCI rejects non-OCI media types).
| manifest, d, err := dockerutil.ParseManifestOCIIndex(testOCIIndexBytes) | ||
| require.NoError(err) | ||
| mediaType, _, err := manifest.Payload() | ||
| require.NoError(err) | ||
| require.Equal("application/vnd.oci.image.index.v1+json", mediaType) |
There was a problem hiding this comment.
This test only checks the digest algorithm rather than the full digest value, and it doesn't exercise error paths (e.g., passing a manifest instead of an index). Consider asserting the exact digest string for testOCIIndexBytes and adding a failure-case test to confirm ParseManifestOCIIndex rejects non-index inputs.
| } | ||
| _, ok := manifestIndex.(*manifestlist.DeserializedManifestList) | ||
| if !ok { | ||
| return nil, core.Digest{}, errors.New("expected manifestlist.DeserializedManifestList for OCI index") |
There was a problem hiding this comment.
The error message here exposes the internal Docker distribution type name rather than describing the OCI concept being validated. Consider rewording to something user-facing like "expected OCI image index" (and optionally including the actual type via %T) to make troubleshooting easier.
| return nil, core.Digest{}, errors.New("expected manifestlist.DeserializedManifestList for OCI index") | |
| return nil, core.Digest{}, fmt.Errorf("expected OCI image index, got %T", manifestIndex) |
|
good point, the current |
What this PR does
Kraken currently only supports Docker v2 manifest formats. This PR adds support for OCI image manifests and image indexes so that images using OCI media types can be handled correctly
Changes made
application/vnd.oci.image.manifest.v1+jsonapplication/vnd.oci.image.index.v1+jsonpreheat.goto recognize OCI typesGetSupportedManifestTypesso they are sent in Accept headersgithub.com/opencontainers/image-specto a direct dependency for using official media type constantsTesting
ParseManifestcorrectly handles OCI inputs end-to-endWhy this is needed
OCI image formats are widely used across modern container registries. Without this support, Kraken fails to process such images. This change ensures compatibility with OCI-compliant registries while keeping existing Docker support intact
fixes #574