Skip to content

fix: parsing of the OCI manifests #569

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
hweawer wants to merge 8 commits into
base: master
Choose a base branch
from
Open

fix: parsing of the OCI manifests #569

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
d388e0e
fix: parsing of the OCI manifests
hweawer Feb 16, 2026
fcfeb11
fix: Puller accepts different formats
hweawer Feb 16, 2026
a7d0b46
fix: parse manifest
hweawer Feb 16, 2026
53c4578
fix: Update content
hweawer Feb 16, 2026
9d1d258
fix
hweawer Feb 16, 2026
1906201
fix: Size
hweawer Feb 16, 2026
3693cda
fix
hweawer Feb 16, 2026
316dfdc
fix
hweawer Feb 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions lib/dockerregistry/storage_driver.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,8 @@ func NewReadWriteStorageDriver(
config Config,
cas *store.CAStore,
transferer transfer.ImageTransferer,
verification func(repo string, digest core.Digest, blob store.FileReader) (SignatureVerificationDecision, error)) *KrakenStorageDriver {
verification func(repo string, digest core.Digest, blob store.FileReader) (SignatureVerificationDecision, error),
) *KrakenStorageDriver {
return &KrakenStorageDriver{
config: config,
transferer: transferer,
Expand All @@ -162,7 +163,8 @@ func NewReadOnlyStorageDriver(
config Config,
bs BlobStore,
transferer transfer.ImageTransferer,
verification func(repo string, digest core.Digest, blob store.FileReader) (SignatureVerificationDecision, error)) *KrakenStorageDriver {
verification func(repo string, digest core.Digest, blob store.FileReader) (SignatureVerificationDecision, error),
) *KrakenStorageDriver {
return &KrakenStorageDriver{
config: config,
transferer: transferer,
Expand Down
13 changes: 7 additions & 6 deletions tools/bin/puller/pull.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (
"github.com/docker/distribution/manifest/schema2"
"github.com/opencontainers/go-digest"
"github.com/uber/kraken/utils/closers"
"github.com/uber/kraken/utils/dockerutil"
"github.com/uber/kraken/utils/errutil"
"github.com/uber/kraken/utils/log"
)
Expand Down Expand Up @@ -100,10 +101,8 @@ func pullManifest(client http.Client, source string, name string, reference stri
if err != nil {
return nil, err
}
// Add `Accept` header to indicate schema2 is supported
req.Header.Add("Accept", schema2.MediaTypeManifest)
req.Header.Add("Accept", fmt.Sprintf("%s,%s", schema2.MediaTypeManifest, "application/vnd.oci.image.manifest.v1+json"))
resp, err := client.Do(req)

if err != nil {
return nil, err
}
Expand All @@ -117,14 +116,16 @@ func pullManifest(client http.Client, source string, name string, reference stri
return nil, fmt.Errorf("server returned %v", resp.Status)
}

version := resp.Header.Get("Content-Type")
body, err := io.ReadAll(resp.Body)
if err != nil {
return nil, err
}
manifest, _, err := distribution.UnmarshalManifest(version, body)

// Use dockerutil.ParseManifest which handles Docker v2, v2 list, and OCI formats
// This avoids issues with Content-Type headers and schema1 signature verification
manifest, _, err := dockerutil.ParseManifest(bytes.NewReader(body))
if err != nil {
return nil, err
return nil, fmt.Errorf("parse manifest: %w", err)
}

return manifest, nil
Expand Down
95 changes: 92 additions & 3 deletions utils/dockerutil/dockerutil.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,19 +14,22 @@
package dockerutil

import (
"encoding/json"
"errors"
"fmt"
"io"

"github.com/docker/distribution"
"github.com/docker/distribution/manifest/manifestlist"
"github.com/docker/distribution/manifest/schema2"
"github.com/opencontainers/go-digest"
"github.com/uber/kraken/core"
)

const (
_v2ManifestType = "application/vnd.docker.distribution.manifest.v2+json"
_v2ManifestListType = "application/vnd.docker.distribution.manifest.list.v2+json"
_ociManifestType = "application/vnd.oci.image.manifest.v1+json"
)

func ParseManifest(r io.Reader) (distribution.Manifest, core.Digest, error) {
Expand All @@ -35,13 +38,20 @@ func ParseManifest(r io.Reader) (distribution.Manifest, core.Digest, error) {
return nil, core.Digest{}, fmt.Errorf("read: %s", err)
}

// Try Docker v2 manifest first
manifest, d, err := ParseManifestV2(b)
if err == nil {
return manifest, d, err
}

// Retry with v2 manifest list.
return ParseManifestV2List(b)
// Try Docker v2 manifest list
manifest, d, err = ParseManifestV2List(b)
if err == nil {
return manifest, d, err
}

// Try OCI manifest v1 (same structure as Docker v2)
return ParseOCIManifest(b)
Comment on lines +42 to +55
Copy link

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ParseManifest function's fallback logic has been improved, but there's a subtle issue: when ParseManifestV2 succeeds but ParseManifestV2List fails, the error from ParseManifestV2List is being ignored and the function falls through to ParseOCIManifest. This could mask legitimate errors. Consider whether the original design pattern (where ParseManifestV2List was the final fallback) was intentional, and whether the error from the previous attempt should be preserved or logged when falling through to the next parser.

Copilot uses AI. Check for mistakes.
Comment on lines +54 to +55
Copy link

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The implementation only handles OCI image manifests (application/vnd.oci.image.manifest.v1+json) but does not support OCI image indexes (application/vnd.oci.image.index.v1+json), which are the OCI equivalent of Docker manifest lists. This means multi-platform OCI images using the index format will fail to parse. Consider adding support for OCI image indexes or documenting this limitation.

Copilot uses AI. Check for mistakes.
}

// ParseManifestV2 returns a parsed v2 manifest and its digest.
Expand Down Expand Up @@ -99,6 +109,85 @@ func GetManifestReferences(manifest distribution.Manifest) ([]core.Digest, error
return refs, nil
}

// ParseOCIManifest parses an OCI image manifest v1.
// OCI manifests have the same structure as Docker v2 manifests, so we can parse them similarly.
func ParseOCIManifest(bytes []byte) (distribution.Manifest, core.Digest, error) {
Comment on lines +124 to +126
Copy link

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are existing unit tests for dockerutil manifest parsing, but the new OCI parsing path isn’t covered. Adding a fixture + test case that feeds an OCI manifest (with and without mediaType) into ParseManifest/ParseOCIManifest would help prevent regressions.

Copilot uses AI. Check for mistakes.
// First, try to parse as Docker v2 manifest (they have the same structure)
// The Docker Distribution library might accept it if we use schema2.MediaTypeManifest
manifest, desc, err := distribution.UnmarshalManifest(schema2.MediaTypeManifest, bytes)
if err == nil {
// Verify it's actually a schema2 manifest
if _, ok := manifest.(*schema2.DeserializedManifest); ok {
d, err := core.ParseSHA256Digest(string(desc.Digest))
if err != nil {
return nil, core.Digest{}, fmt.Errorf("parse digest: %s", err)
}
return manifest, d, nil
Copy link

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The first attempt to parse as Docker v2 manifest (lines 117-127) doesn't properly handle the case where UnmarshalManifest succeeds but returns a non-schema2 manifest type. In this scenario, the function silently proceeds to manual parsing without returning an error, which could mask issues. Consider either returning an error when the manifest type is unexpected, or adding a comment explaining why this is intentional fallback behavior.

Suggested change
return manifest, d, nil
return manifest, d, nil
} else {
// If UnmarshalManifest succeeds but returns a non-schema2 manifest,
// treat this as an unsupported type for Docker v2 and fall back to
// manual OCI parsing below. This is intentional to avoid silently
// accepting an unexpected manifest representation.

Copilot uses AI. Check for mistakes.
}
Copy link

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ParseOCIManifest returns successfully after UnmarshalManifest(schema2.MediaTypeManifest, ...) without validating SchemaVersion==2 (unlike ParseManifestV2). Adding the same schema-version check here helps avoid accepting malformed manifests and keeps behavior consistent across parsers.

Copilot uses AI. Check for mistakes.
}

// If that fails, parse manually by checking the mediaType in the JSON
Copy link

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The first attempt to parse using distribution.UnmarshalManifest with schema2.MediaTypeManifest will likely fail for genuine OCI manifests because they have a different mediaType field. This means the code will always fall through to the manual parsing path for actual OCI manifests, making lines 115-127 effectively dead code for the OCI use case. While not harmful, this adds unnecessary complexity. Consider whether this initial attempt is needed, or document why it's included (e.g., for compatibility with manifests that are incorrectly labeled).

Suggested change
// First, try to parse as Docker v2 manifest (they have the same structure)
// The Docker Distribution library might accept it if we use schema2.MediaTypeManifest
manifest, desc, err := distribution.UnmarshalManifest(schema2.MediaTypeManifest, bytes)
if err == nil {
// Verify it's actually a schema2 manifest
if _, ok := manifest.(*schema2.DeserializedManifest); ok {
d, err := core.ParseSHA256Digest(string(desc.Digest))
if err != nil {
return nil, core.Digest{}, fmt.Errorf("parse digest: %s", err)
}
return manifest, d, nil
}
}
// If that fails, parse manually by checking the mediaType in the JSON
// Parse by checking the mediaType in the JSON

Copilot uses AI. Check for mistakes.
var manifestJSON struct {
MediaType string `json:"mediaType"`
Config struct {
Digest string `json:"digest"`
} `json:"config"`
Layers []struct {
Digest string `json:"digest"`
} `json:"layers"`
}
if err := json.Unmarshal(bytes, &manifestJSON); err != nil {
return nil, core.Digest{}, fmt.Errorf("unmarshal oci manifest json: %s", err)
}
Comment on lines +128 to +141
Copy link

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The OCI manifest JSON structure is incomplete. OCI manifests include 'size' fields for both config and layers, which should be parsed and used when constructing the descriptors. The current implementation sets Size to 0 on lines 164 and 176, which may cause issues downstream. Update the struct to include size fields and populate them in the distribution.Descriptor objects.

Copilot uses AI. Check for mistakes.

if manifestJSON.MediaType != _ociManifestType {
return nil, core.Digest{}, fmt.Errorf("expected oci manifest type %s, got %s", _ociManifestType, manifestJSON.MediaType)
Comment on lines +144 to +145
Copy link

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The manual JSON fallback requires mediaType to equal the OCI manifest media type. In OCI image manifests, mediaType can be omitted; treating an empty mediaType as OCI (or checking presence more leniently) would make parsing robust for valid manifests that don't include this field.

Suggested change
if manifestJSON.MediaType != _ociManifestType {
return nil, core.Digest{}, fmt.Errorf("expected oci manifest type %s, got %s", _ociManifestType, manifestJSON.MediaType)
// In OCI image manifests, mediaType may be omitted. Treat an empty mediaType
// as OCI, but reject manifests that explicitly specify a non-OCI media type.
if manifestJSON.MediaType != "" && manifestJSON.MediaType != _ociManifestType {
return nil, core.Digest{}, fmt.Errorf("expected oci manifest type %s or empty, got %s", _ociManifestType, manifestJSON.MediaType)

Copilot uses AI. Check for mistakes.
}

// Calculate digest of the manifest
d, err := core.NewDigester().FromBytes(bytes)
if err != nil {
return nil, core.Digest{}, fmt.Errorf("calculate manifest digest: %s", err)
}

// Parse digests from OCI format (they're already in "sha256:hex" format)
configDigest, err := core.ParseSHA256Digest(manifestJSON.Config.Digest)
if err != nil {
return nil, core.Digest{}, fmt.Errorf("parse config digest: %s", err)
}

// Convert to Docker v2 format by creating a schema2 manifest
// We need to convert the OCI structure to schema2 format
configDesc := distribution.Descriptor{
Digest: digest.Digest(configDigest.String()),
MediaType: schema2.MediaTypeImageConfig,
Size: 0, // Size not available in OCI manifest
}

layers := make([]distribution.Descriptor, 0, len(manifestJSON.Layers))
for _, layer := range manifestJSON.Layers {
layerDigest, err := core.ParseSHA256Digest(layer.Digest)
if err != nil {
return nil, core.Digest{}, fmt.Errorf("parse layer digest: %s", err)
}
layers = append(layers, distribution.Descriptor{
Digest: digest.Digest(layerDigest.String()),
MediaType: schema2.MediaTypeLayer,
Size: 0, // Size not available in OCI manifest
Copy link

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Setting Size to 0 for descriptors is problematic. While the comment indicates that size is "not available in OCI manifest", OCI manifests do include size fields for both config and layers according to the OCI Image Manifest Specification. The manifestJSON struct should be extended to capture these size fields, and they should be used when creating the descriptors. Size 0 could cause issues with download progress tracking, storage allocation, or validation in downstream components.

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The manifestJSON struct is missing the MediaType field for config and layers. OCI manifests include mediaType fields for both the config and each layer, and these should be preserved when converting to schema2 format. Different media types indicate different layer formats (e.g., tar.gzip vs tar+gzip, foreign layers, etc.), and using the wrong media type could cause processing errors. The struct should capture these fields and use them in the distribution.Descriptor objects instead of hardcoding schema2.MediaTypeImageConfig and schema2.MediaTypeLayer.

Copilot uses AI. Check for mistakes.
})
Comment on lines +168 to +186
Copy link

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The manual parsing of the OCI manifest doesn't preserve the original mediaType values from the config and layers. OCI manifests may have different mediaType values than Docker v2 (e.g., 'application/vnd.oci.image.layer.v1.tar+gzip' vs 'application/vnd.docker.image.rootfs.diff.tar.gzip'). By hardcoding schema2.MediaTypeImageConfig and schema2.MediaTypeLayer, the function loses this information. Consider extracting and using the actual mediaType values from the manifest JSON.

Copilot uses AI. Check for mistakes.
}

ociManifest := &schema2.DeserializedManifest{
Manifest: schema2.Manifest{
Versioned: manifestlist.SchemaVersion,
Copy link

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Versioned field is being set to manifestlist.SchemaVersion, but this appears to be incorrect. Based on the codebase patterns in ParseManifestV2 (lines 67-69), Docker v2 manifests should have SchemaVersion 2. The manifestlist.SchemaVersion constant is intended for manifest lists, not individual manifests. This will create a manifest with an incorrect schema version field, which could cause validation issues or unexpected behavior when the manifest is used.

Copilot uses AI. Check for mistakes.
Config: configDesc,
Layers: layers,
},
Copy link

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the manually-built schema2.DeserializedManifest, Versioned: manifestlist.SchemaVersion does not populate the embedded schema version/media type information (and may not match the expected type for the embedded Versioned field). This can yield a manifest with SchemaVersion=0 / empty MediaType (or fail to compile depending on types). Set the version info explicitly (SchemaVersion=2 and an appropriate MediaType) using the correct embedded version struct so downstream consumers see a valid v2/OCI-equivalent manifest.

Copilot uses AI. Check for mistakes.
}

return ociManifest, d, nil
}
Comment on lines +124 to +240
Copy link

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on the test coverage pattern in this codebase (dockerutil_test.go has tests for ParseManifestV2List), the new ParseOCIManifest function should have corresponding test coverage. Tests should include: valid OCI manifest parsing, invalid OCI manifest handling, digest calculation verification, and proper conversion to schema2 format. This ensures the OCI manifest parsing logic works correctly and prevents regressions.

Copilot uses AI. Check for mistakes.
Comment on lines +124 to +240
Copy link

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new ParseOCIManifest function lacks test coverage. The existing tests in dockerutil_test.go test ParseManifestV2List but don't cover OCI manifest parsing. Consider adding tests that verify OCI manifest parsing works correctly, including edge cases like missing fields or invalid digest formats.

Copilot uses AI. Check for mistakes.
Comment on lines +124 to +240
Copy link

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new ParseOCIManifest function lacks test coverage. The existing test file utils/dockerutil/dockerutil_test.go has tests for ParseManifestV2List but no tests for the new OCI manifest parsing functionality. Given that this is a critical new feature for parsing OCI manifests, comprehensive test coverage should be added to ensure correct behavior, including tests for valid OCI manifests, invalid media types, malformed JSON, and edge cases.

Copilot uses AI. Check for mistakes.

func GetSupportedManifestTypes() string {
return fmt.Sprintf("%s,%s", _v2ManifestType, _v2ManifestListType)
return fmt.Sprintf("%s,%s,%s", _v2ManifestType, _v2ManifestListType, _ociManifestType)
}
Loading