From e72294d07594e683507b7a8053f62ead4285a036 Mon Sep 17 00:00:00 2001 From: Bracken Dawson Date: Fri, 31 Mar 2023 11:35:30 +0100 Subject: [PATCH 1/4] Split OCI Image Index from Docker Manifest List Move implementation of the index from the manifestlist package to the ocischema package so that other modules making empty imports support the manifest types their authors would expect. This is a breaking change to distribution as a library but not the registry. As OCI 1.0 released the manifest and index together, that is a good package from which to initialise both manifests. The docker manifest and manifest list remain in separate packages because one was released later. The image index and manifest list still share common code in many functions not intended for import by other modules. Signed-off-by: Bracken Dawson --- manifest/manifestlist/manifestlist.go | 72 +------ manifest/manifestlist/manifestlist_test.go | 238 ++++----------------- manifest/ocischema/index.go | 168 +++++++++++++++ manifest/ocischema/index_test.go | 222 +++++++++++++++++++ manifest/ocischema/manifest.go | 10 +- manifest/ocischema/manifest_test.go | 64 +++--- registry/storage/manifestlisthandler.go | 23 +- registry/storage/manifeststore.go | 19 +- registry/storage/manifeststore_test.go | 32 ++- registry/storage/ociindexhandler.go | 28 +++ registry/storage/registry.go | 15 +- 11 files changed, 569 insertions(+), 322 deletions(-) create mode 100644 manifest/ocischema/index.go create mode 100644 manifest/ocischema/index_test.go create mode 100644 registry/storage/ociindexhandler.go diff --git a/manifest/manifestlist/manifestlist.go b/manifest/manifestlist/manifestlist.go index 52e67c731..41f49775c 100644 --- a/manifest/manifestlist/manifestlist.go +++ b/manifest/manifestlist/manifestlist.go @@ -23,13 +23,6 @@ var SchemaVersion = manifest.Versioned{ MediaType: MediaTypeManifestList, } -// OCISchemaVersion provides a pre-initialized version structure for this -// packages OCIschema version of the manifest. -var OCISchemaVersion = manifest.Versioned{ - SchemaVersion: 2, - MediaType: v1.MediaTypeImageIndex, -} - func init() { manifestListFunc := func(b []byte) (distribution.Manifest, distribution.Descriptor, error) { m := new(DeserializedManifestList) @@ -52,31 +45,6 @@ func init() { if err != nil { panic(fmt.Sprintf("Unable to register manifest: %s", err)) } - - imageIndexFunc := func(b []byte) (distribution.Manifest, distribution.Descriptor, error) { - if err := validateIndex(b); err != nil { - return nil, distribution.Descriptor{}, err - } - m := new(DeserializedManifestList) - err := m.UnmarshalJSON(b) - if err != nil { - return nil, distribution.Descriptor{}, err - } - - if m.MediaType != "" && m.MediaType != v1.MediaTypeImageIndex { - err = fmt.Errorf("if present, mediaType in image index should be '%s' not '%s'", - v1.MediaTypeImageIndex, m.MediaType) - - return nil, distribution.Descriptor{}, err - } - - dgst := digest.FromBytes(b) - return m, distribution.Descriptor{Digest: dgst, Size: int64(len(b)), MediaType: v1.MediaTypeImageIndex}, err - } - err = distribution.RegisterManifestSchema(v1.MediaTypeImageIndex, imageIndexFunc) - if err != nil { - panic(fmt.Sprintf("Unable to register OCI Image Index: %s", err)) - } } // PlatformSpec specifies a platform where a particular image manifest is @@ -154,18 +122,11 @@ type DeserializedManifestList struct { // DeserializedManifestList which contains the resulting manifest list // and its JSON representation. func FromDescriptors(descriptors []ManifestDescriptor) (*DeserializedManifestList, error) { - var mediaType string - if len(descriptors) > 0 && descriptors[0].Descriptor.MediaType == v1.MediaTypeImageManifest { - mediaType = v1.MediaTypeImageIndex - } else { - mediaType = MediaTypeManifestList - } - - return FromDescriptorsWithMediaType(descriptors, mediaType) + return fromDescriptorsWithMediaType(descriptors, MediaTypeManifestList) } -// FromDescriptorsWithMediaType is for testing purposes, it's useful to be able to specify the media type explicitly -func FromDescriptorsWithMediaType(descriptors []ManifestDescriptor, mediaType string) (*DeserializedManifestList, error) { +// fromDescriptorsWithMediaType is for testing purposes, it's useful to be able to specify the media type explicitly +func fromDescriptorsWithMediaType(descriptors []ManifestDescriptor, mediaType string) (*DeserializedManifestList, error) { m := ManifestList{ Versioned: manifest.Versioned{ SchemaVersion: 2, @@ -215,32 +176,21 @@ func (m *DeserializedManifestList) MarshalJSON() ([]byte, error) { // Payload returns the raw content of the manifest list. The contents can be // used to calculate the content identifier. func (m DeserializedManifestList) Payload() (string, []byte, error) { - var mediaType string - if m.MediaType == "" { - mediaType = v1.MediaTypeImageIndex - } else { - mediaType = m.MediaType - } - - return mediaType, m.canonical, nil + return m.MediaType, m.canonical, nil } -// unknownDocument represents a manifest, manifest list, or index that has not -// yet been validated -type unknownDocument struct { - Config interface{} `json:"config,omitempty"` - Layers interface{} `json:"layers,omitempty"` -} - -// validateIndex returns an error if the byte slice is invalid JSON or if it +// validateManifestList returns an error if the byte slice is invalid JSON or if it // contains fields that belong to a manifest -func validateIndex(b []byte) error { - var doc unknownDocument +func validateManifestList(b []byte) error { + var doc struct { + Config interface{} `json:"config,omitempty"` + Layers interface{} `json:"layers,omitempty"` + } if err := json.Unmarshal(b, &doc); err != nil { return err } if doc.Config != nil || doc.Layers != nil { - return errors.New("index: expected index but found manifest") + return errors.New("manifestlist: expected list but found manifest") } return nil } diff --git a/manifest/manifestlist/manifestlist_test.go b/manifest/manifestlist/manifestlist_test.go index 842ae310b..f52485ed9 100644 --- a/manifest/manifestlist/manifestlist_test.go +++ b/manifest/manifestlist/manifestlist_test.go @@ -7,7 +7,7 @@ import ( "testing" "github.com/distribution/distribution/v3" - "github.com/distribution/distribution/v3/manifest/ocischema" + "github.com/distribution/distribution/v3/manifest/schema2" v1 "github.com/opencontainers/image-spec/specs-go/v1" ) @@ -67,7 +67,7 @@ func makeTestManifestList(t *testing.T, mediaType string) ([]ManifestDescriptor, }, } - deserialized, err := FromDescriptorsWithMediaType(manifestDescriptors, mediaType) + deserialized, err := fromDescriptorsWithMediaType(manifestDescriptors, mediaType) if err != nil { t.Fatalf("error creating DeserializedManifestList: %v", err) } @@ -130,223 +130,69 @@ func TestManifestList(t *testing.T) { } } -// TODO (mikebrow): add annotations on the manifest list (index) and support for -// empty platform structs (move to Platform *Platform `json:"platform,omitempty"` -// from current Platform PlatformSpec `json:"platform"`) in the manifest descriptor. -// Requires changes to distribution/distribution/manifest/manifestlist.ManifestList and .ManifestDescriptor -// and associated serialization APIs in manifestlist.go. Or split the OCI index and -// docker manifest list implementations, which would require a lot of refactoring. -const expectedOCIImageIndexSerialization = `{ - "schemaVersion": 2, - "mediaType": "application/vnd.oci.image.index.v1+json", - "manifests": [ - { - "mediaType": "application/vnd.oci.image.manifest.v1+json", - "digest": "sha256:1a9ec845ee94c202b2d5da74a24f0ed2058318bfa9879fa541efaecba272e86b", - "size": 985, - "platform": { - "architecture": "amd64", - "os": "linux", - "features": [ - "sse4" - ] - } - }, - { - "mediaType": "application/vnd.oci.image.manifest.v1+json", - "digest": "sha256:1a9ec845ee94c202b2d5da74a24f0ed2058318bfa9879fa541efaecba272e86b", - "size": 985, - "annotations": { - "platform": "none" - }, - "platform": { - "architecture": "", - "os": "" - } - }, - { - "mediaType": "application/vnd.oci.image.manifest.v1+json", - "digest": "sha256:6346340964309634683409684360934680934608934608934608934068934608", - "size": 2392, - "annotations": { - "what": "for" - }, - "platform": { - "architecture": "sun4m", - "os": "sunos" - } - } - ] -}` - -func makeTestOCIImageIndex(t *testing.T, mediaType string) ([]ManifestDescriptor, *DeserializedManifestList) { - manifestDescriptors := []ManifestDescriptor{ - { - Descriptor: distribution.Descriptor{ - MediaType: "application/vnd.oci.image.manifest.v1+json", - Digest: "sha256:1a9ec845ee94c202b2d5da74a24f0ed2058318bfa9879fa541efaecba272e86b", - Size: 985, - }, - Platform: PlatformSpec{ - Architecture: "amd64", - OS: "linux", - Features: []string{"sse4"}, - }, - }, - { - Descriptor: distribution.Descriptor{ - MediaType: "application/vnd.oci.image.manifest.v1+json", - Digest: "sha256:1a9ec845ee94c202b2d5da74a24f0ed2058318bfa9879fa541efaecba272e86b", - Size: 985, - Annotations: map[string]string{"platform": "none"}, - }, - }, - { - Descriptor: distribution.Descriptor{ - MediaType: "application/vnd.oci.image.manifest.v1+json", - Digest: "sha256:6346340964309634683409684360934680934608934608934608934068934608", - Size: 2392, - Annotations: map[string]string{"what": "for"}, - }, - Platform: PlatformSpec{ - Architecture: "sun4m", - OS: "sunos", - }, - }, - } - - deserialized, err := FromDescriptorsWithMediaType(manifestDescriptors, mediaType) - if err != nil { - t.Fatalf("error creating DeserializedManifestList: %v", err) - } - - return manifestDescriptors, deserialized -} - -func TestOCIImageIndex(t *testing.T) { - manifestDescriptors, deserialized := makeTestOCIImageIndex(t, v1.MediaTypeImageIndex) - - mediaType, canonical, _ := deserialized.Payload() - - if mediaType != v1.MediaTypeImageIndex { - t.Fatalf("unexpected media type: %s", mediaType) - } - - // Check that the canonical field is the same as json.MarshalIndent - // with these parameters. - expected, err := json.MarshalIndent(&deserialized.ManifestList, "", " ") - if err != nil { - t.Fatalf("error marshaling manifest list: %v", err) - } - if !bytes.Equal(expected, canonical) { - t.Fatalf("manifest bytes not equal:\nexpected:\n%s\nactual:\n%s\n", string(expected), string(canonical)) - } - - // Check that the canonical field has the expected value. - if !bytes.Equal([]byte(expectedOCIImageIndexSerialization), canonical) { - t.Fatalf("manifest bytes not equal:\nexpected:\n%s\nactual:\n%s\n", expectedOCIImageIndexSerialization, string(canonical)) - } - - var unmarshalled DeserializedManifestList - if err := json.Unmarshal(deserialized.canonical, &unmarshalled); err != nil { - t.Fatalf("error unmarshaling manifest: %v", err) - } - - if !reflect.DeepEqual(&unmarshalled, deserialized) { - t.Fatalf("manifests are different after unmarshaling: %v != %v", unmarshalled, *deserialized) - } - - references := deserialized.References() - if len(references) != 3 { - t.Fatalf("unexpected number of references: %d", len(references)) - } - for i := range references { - platform := manifestDescriptors[i].Platform - expectedPlatform := &v1.Platform{ - Architecture: platform.Architecture, - OS: platform.OS, - OSFeatures: platform.OSFeatures, - OSVersion: platform.OSVersion, - Variant: platform.Variant, - } - if !reflect.DeepEqual(references[i].Platform, expectedPlatform) { - t.Fatalf("unexpected value %d returned by References: %v", i, references[i]) - } - references[i].Platform = nil - if !reflect.DeepEqual(references[i], manifestDescriptors[i].Descriptor) { - t.Fatalf("unexpected value %d returned by References: %v", i, references[i]) - } - } -} - -func mediaTypeTest(t *testing.T, contentType string, mediaType string, shouldError bool) { - var m *DeserializedManifestList - if contentType == MediaTypeManifestList { +func mediaTypeTest(contentType string, mediaType string, shouldError bool) func(*testing.T) { + return func(t *testing.T) { + var m *DeserializedManifestList _, m = makeTestManifestList(t, mediaType) - } else { - _, m = makeTestOCIImageIndex(t, mediaType) - } - _, canonical, err := m.Payload() - if err != nil { - t.Fatalf("error getting payload, %v", err) - } - - unmarshalled, descriptor, err := distribution.UnmarshalManifest( - contentType, - canonical) - - if shouldError { - if err == nil { - t.Fatalf("bad content type should have produced error") - } - } else { + _, canonical, err := m.Payload() if err != nil { - t.Fatalf("error unmarshaling manifest, %v", err) + t.Fatalf("error getting payload, %v", err) } - asManifest := unmarshalled.(*DeserializedManifestList) - if asManifest.MediaType != mediaType { - t.Fatalf("Bad media type '%v' as unmarshalled", asManifest.MediaType) - } + unmarshalled, descriptor, err := distribution.UnmarshalManifest( + contentType, + canonical) - if descriptor.MediaType != contentType { - t.Fatalf("Bad media type '%v' for descriptor", descriptor.MediaType) - } + if shouldError { + if err == nil { + t.Fatalf("bad content type should have produced error") + } + } else { + if err != nil { + t.Fatalf("error unmarshaling manifest, %v", err) + } - unmarshalledMediaType, _, _ := unmarshalled.Payload() - if unmarshalledMediaType != contentType { - t.Fatalf("Bad media type '%v' for payload", unmarshalledMediaType) + asManifest := unmarshalled.(*DeserializedManifestList) + if asManifest.MediaType != mediaType { + t.Fatalf("Bad media type '%v' as unmarshalled", asManifest.MediaType) + } + + if descriptor.MediaType != contentType { + t.Fatalf("Bad media type '%v' for descriptor", descriptor.MediaType) + } + + unmarshalledMediaType, _, _ := unmarshalled.Payload() + if unmarshalledMediaType != contentType { + t.Fatalf("Bad media type '%v' for payload", unmarshalledMediaType) + } } } } func TestMediaTypes(t *testing.T) { - mediaTypeTest(t, MediaTypeManifestList, "", true) - mediaTypeTest(t, MediaTypeManifestList, MediaTypeManifestList, false) - mediaTypeTest(t, MediaTypeManifestList, MediaTypeManifestList+"XXX", true) - mediaTypeTest(t, v1.MediaTypeImageIndex, "", false) - mediaTypeTest(t, v1.MediaTypeImageIndex, v1.MediaTypeImageIndex, false) - mediaTypeTest(t, v1.MediaTypeImageIndex, v1.MediaTypeImageIndex+"XXX", true) + t.Run("ManifestList_No_MediaType", mediaTypeTest(MediaTypeManifestList, "", true)) + t.Run("ManifestList", mediaTypeTest(MediaTypeManifestList, MediaTypeManifestList, false)) + t.Run("ManifestList_Bad_MediaType", mediaTypeTest(MediaTypeManifestList, MediaTypeManifestList+"XXX", true)) } -func TestValidateManifest(t *testing.T) { - manifest := ocischema.Manifest{ +func TestValidateManifestList(t *testing.T) { + manifest := schema2.Manifest{ Config: distribution.Descriptor{Size: 1}, Layers: []distribution.Descriptor{{Size: 2}}, } - index := ManifestList{ + manifestList := ManifestList{ Manifests: []ManifestDescriptor{ {Descriptor: distribution.Descriptor{Size: 3}}, }, } t.Run("valid", func(t *testing.T) { - b, err := json.Marshal(index) + b, err := json.Marshal(manifestList) if err != nil { - t.Fatal("unexpected error marshaling index", err) + t.Fatal("unexpected error marshaling manifest list", err) } - if err := validateIndex(b); err != nil { - t.Error("index should be valid", err) + if err := validateManifestList(b); err != nil { + t.Error("list should be valid", err) } }) t.Run("invalid", func(t *testing.T) { @@ -354,7 +200,7 @@ func TestValidateManifest(t *testing.T) { if err != nil { t.Fatal("unexpected error marshaling manifest", err) } - if err := validateIndex(b); err == nil { + if err := validateManifestList(b); err == nil { t.Error("manifest should not be valid") } }) diff --git a/manifest/ocischema/index.go b/manifest/ocischema/index.go new file mode 100644 index 000000000..97b3943a6 --- /dev/null +++ b/manifest/ocischema/index.go @@ -0,0 +1,168 @@ +package ocischema + +import ( + "encoding/json" + "errors" + "fmt" + + "github.com/distribution/distribution/v3" + "github.com/distribution/distribution/v3/manifest" + "github.com/opencontainers/go-digest" + v1 "github.com/opencontainers/image-spec/specs-go/v1" +) + +// OCISchemaVersion provides a pre-initialized version structure for this +// packages OCIschema version of the manifest. +var OCISchemaVersion = manifest.Versioned{ + SchemaVersion: 2, + MediaType: v1.MediaTypeImageIndex, +} + +func init() { + imageIndexFunc := func(b []byte) (distribution.Manifest, distribution.Descriptor, error) { + if err := validateIndex(b); err != nil { + return nil, distribution.Descriptor{}, err + } + m := new(DeserializedImageIndex) + err := m.UnmarshalJSON(b) + if err != nil { + return nil, distribution.Descriptor{}, err + } + + if m.MediaType != "" && m.MediaType != v1.MediaTypeImageIndex { + err = fmt.Errorf("if present, mediaType in image index should be '%s' not '%s'", + v1.MediaTypeImageIndex, m.MediaType) + + return nil, distribution.Descriptor{}, err + } + + dgst := digest.FromBytes(b) + return m, distribution.Descriptor{Digest: dgst, Size: int64(len(b)), MediaType: v1.MediaTypeImageIndex}, err + } + err := distribution.RegisterManifestSchema(v1.MediaTypeImageIndex, imageIndexFunc) + if err != nil { + panic(fmt.Sprintf("Unable to register OCI Image Index: %s", err)) + } +} + +// A ManifestDescriptor references a platform-specific manifest. +type ManifestDescriptor struct { + distribution.Descriptor + + // Platform specifies which platform the manifest pointed to by the + // descriptor runs on. + Platform *v1.Platform `json:"platform,omitempty"` +} + +// ImageIndex references manifests for various platforms. +type ImageIndex struct { + manifest.Versioned + + // Manifests references a list of manifests + Manifests []ManifestDescriptor `json:"manifests"` +} + +// References returns the distribution descriptors for the referenced image +// manifests. +func (ii ImageIndex) References() []distribution.Descriptor { + dependencies := make([]distribution.Descriptor, len(ii.Manifests)) + for i := range ii.Manifests { + dependencies[i] = ii.Manifests[i].Descriptor + dependencies[i].Platform = ii.Manifests[i].Platform + } + + return dependencies +} + +// DeserializedImageIndex wraps ManifestList with a copy of the original +// JSON. +type DeserializedImageIndex struct { + ImageIndex + + // canonical is the canonical byte representation of the Manifest. + canonical []byte +} + +// FromDescriptors takes a slice of descriptors, and returns a +// DeserializedManifestList which contains the resulting manifest list +// and its JSON representation. +func FromDescriptors(descriptors []ManifestDescriptor) (*DeserializedImageIndex, error) { + return fromDescriptorsWithMediaType(descriptors, v1.MediaTypeImageIndex) +} + +// fromDescriptorsWithMediaType is for testing purposes, it's useful to be able to specify the media type explicitly +func fromDescriptorsWithMediaType(descriptors []ManifestDescriptor, mediaType string) (*DeserializedImageIndex, error) { + m := ImageIndex{ + Versioned: manifest.Versioned{ + SchemaVersion: 2, + MediaType: mediaType, + }, + } + + m.Manifests = make([]ManifestDescriptor, len(descriptors)) + copy(m.Manifests, descriptors) + + deserialized := DeserializedImageIndex{ + ImageIndex: m, + } + + var err error + deserialized.canonical, err = json.MarshalIndent(&m, "", " ") + return &deserialized, err +} + +// UnmarshalJSON populates a new ManifestList struct from JSON data. +func (m *DeserializedImageIndex) UnmarshalJSON(b []byte) error { + m.canonical = make([]byte, len(b)) + // store manifest list in canonical + copy(m.canonical, b) + + // Unmarshal canonical JSON into ManifestList object + var manifestList ImageIndex + if err := json.Unmarshal(m.canonical, &manifestList); err != nil { + return err + } + + m.ImageIndex = manifestList + + return nil +} + +// MarshalJSON returns the contents of canonical. If canonical is empty, +// marshals the inner contents. +func (m *DeserializedImageIndex) MarshalJSON() ([]byte, error) { + if len(m.canonical) > 0 { + return m.canonical, nil + } + + return nil, errors.New("JSON representation not initialized in DeserializedImageIndex") +} + +// Payload returns the raw content of the manifest list. The contents can be +// used to calculate the content identifier. +func (m DeserializedImageIndex) Payload() (string, []byte, error) { + var mediaType string + if m.MediaType == "" { + mediaType = v1.MediaTypeImageIndex + } else { + mediaType = m.MediaType + } + + return mediaType, m.canonical, nil +} + +// validateIndex returns an error if the byte slice is invalid JSON or if it +// contains fields that belong to a manifest +func validateIndex(b []byte) error { + var doc struct { + Config interface{} `json:"config,omitempty"` + Layers interface{} `json:"layers,omitempty"` + } + if err := json.Unmarshal(b, &doc); err != nil { + return err + } + if doc.Config != nil || doc.Layers != nil { + return errors.New("index: expected index but found manifest") + } + return nil +} diff --git a/manifest/ocischema/index_test.go b/manifest/ocischema/index_test.go new file mode 100644 index 000000000..90d9f5861 --- /dev/null +++ b/manifest/ocischema/index_test.go @@ -0,0 +1,222 @@ +package ocischema + +import ( + "bytes" + "encoding/json" + "reflect" + "testing" + + "github.com/distribution/distribution/v3" + "github.com/distribution/distribution/v3/manifest/schema2" + v1 "github.com/opencontainers/image-spec/specs-go/v1" +) + +// TODO (mikebrow): add annotations on the manifest list (index) and support for +// empty platform structs (move to Platform *Platform `json:"platform,omitempty"` +// from current Platform PlatformSpec `json:"platform"`) in the manifest descriptor. +// Requires changes to distribution/distribution/manifest/manifestlist.ManifestList and .ManifestDescriptor +// and associated serialization APIs in manifestlist.go. Or split the OCI index and +// docker manifest list implementations, which would require a lot of refactoring. +const expectedOCIImageIndexSerialization = `{ + "schemaVersion": 2, + "mediaType": "application/vnd.oci.image.index.v1+json", + "manifests": [ + { + "mediaType": "application/vnd.oci.image.manifest.v1+json", + "digest": "sha256:1a9ec845ee94c202b2d5da74a24f0ed2058318bfa9879fa541efaecba272e86b", + "size": 985, + "platform": { + "architecture": "amd64", + "os": "linux" + } + }, + { + "mediaType": "application/vnd.oci.image.manifest.v1+json", + "digest": "sha256:1a9ec845ee94c202b2d5da74a24f0ed2058318bfa9879fa541efaecba272e86b", + "size": 985, + "annotations": { + "platform": "none" + } + }, + { + "mediaType": "application/vnd.oci.image.manifest.v1+json", + "digest": "sha256:6346340964309634683409684360934680934608934608934608934068934608", + "size": 2392, + "annotations": { + "what": "for" + }, + "platform": { + "architecture": "sun4m", + "os": "sunos" + } + } + ] +}` + +func makeTestOCIImageIndex(t *testing.T, mediaType string) ([]ManifestDescriptor, *DeserializedImageIndex) { + manifestDescriptors := []ManifestDescriptor{ + { + Descriptor: distribution.Descriptor{ + MediaType: "application/vnd.oci.image.manifest.v1+json", + Digest: "sha256:1a9ec845ee94c202b2d5da74a24f0ed2058318bfa9879fa541efaecba272e86b", + Size: 985, + }, + Platform: &v1.Platform{ + Architecture: "amd64", + OS: "linux", + }, + }, + { + Descriptor: distribution.Descriptor{ + MediaType: "application/vnd.oci.image.manifest.v1+json", + Digest: "sha256:1a9ec845ee94c202b2d5da74a24f0ed2058318bfa9879fa541efaecba272e86b", + Size: 985, + Annotations: map[string]string{"platform": "none"}, + }, + }, + { + Descriptor: distribution.Descriptor{ + MediaType: "application/vnd.oci.image.manifest.v1+json", + Digest: "sha256:6346340964309634683409684360934680934608934608934608934068934608", + Size: 2392, + Annotations: map[string]string{"what": "for"}, + }, + Platform: &v1.Platform{ + Architecture: "sun4m", + OS: "sunos", + }, + }, + } + + deserialized, err := fromDescriptorsWithMediaType(manifestDescriptors, mediaType) + if err != nil { + t.Fatalf("error creating DeserializedManifestList: %v", err) + } + + return manifestDescriptors, deserialized +} + +func TestOCIImageIndex(t *testing.T) { + manifestDescriptors, deserialized := makeTestOCIImageIndex(t, v1.MediaTypeImageIndex) + + mediaType, canonical, _ := deserialized.Payload() + + if mediaType != v1.MediaTypeImageIndex { + t.Fatalf("unexpected media type: %s", mediaType) + } + + // Check that the canonical field is the same as json.MarshalIndent + // with these parameters. + expected, err := json.MarshalIndent(&deserialized.ImageIndex, "", " ") + if err != nil { + t.Fatalf("error marshaling manifest list: %v", err) + } + if !bytes.Equal(expected, canonical) { + t.Fatalf("manifest bytes not equal:\nexpected:\n%s\nactual:\n%s\n", string(expected), string(canonical)) + } + + // Check that the canonical field has the expected value. + if !bytes.Equal([]byte(expectedOCIImageIndexSerialization), canonical) { + t.Fatalf("manifest bytes not equal:\nexpected:\n%s\nactual:\n%s\n", expectedOCIImageIndexSerialization, string(canonical)) + } + + var unmarshalled DeserializedImageIndex + if err := json.Unmarshal(deserialized.canonical, &unmarshalled); err != nil { + t.Fatalf("error unmarshaling manifest: %v", err) + } + + if !reflect.DeepEqual(&unmarshalled, deserialized) { + t.Fatalf("manifests are different after unmarshaling: %v != %v", unmarshalled, *deserialized) + } + + references := deserialized.References() + if len(references) != 3 { + t.Fatalf("unexpected number of references: %d", len(references)) + } + for i := range references { + expectedPlatform := manifestDescriptors[i].Platform + if !reflect.DeepEqual(references[i].Platform, expectedPlatform) { + t.Fatalf("unexpected value %d returned by References: %v", i, references[i]) + } + references[i].Platform = nil + if !reflect.DeepEqual(references[i], manifestDescriptors[i].Descriptor) { + t.Fatalf("unexpected value %d returned by References: %v", i, references[i]) + } + } +} + +func indexMediaTypeTest(contentType string, mediaType string, shouldError bool) func(*testing.T) { + return func(t *testing.T) { + var m *DeserializedImageIndex + _, m = makeTestOCIImageIndex(t, mediaType) + + _, canonical, err := m.Payload() + if err != nil { + t.Fatalf("error getting payload, %v", err) + } + + unmarshalled, descriptor, err := distribution.UnmarshalManifest( + contentType, + canonical) + + if shouldError { + if err == nil { + t.Fatalf("bad content type should have produced error") + } + } else { + if err != nil { + t.Fatalf("error unmarshaling manifest, %v", err) + } + + asManifest := unmarshalled.(*DeserializedImageIndex) + if asManifest.MediaType != mediaType { + t.Fatalf("Bad media type '%v' as unmarshalled", asManifest.MediaType) + } + + if descriptor.MediaType != contentType { + t.Fatalf("Bad media type '%v' for descriptor", descriptor.MediaType) + } + + unmarshalledMediaType, _, _ := unmarshalled.Payload() + if unmarshalledMediaType != contentType { + t.Fatalf("Bad media type '%v' for payload", unmarshalledMediaType) + } + } + } +} + +func TestIndexMediaTypes(t *testing.T) { + t.Run("No_MediaType", indexMediaTypeTest(v1.MediaTypeImageIndex, "", false)) + t.Run("ImageIndex", indexMediaTypeTest(v1.MediaTypeImageIndex, v1.MediaTypeImageIndex, false)) + t.Run("Bad_MediaType", indexMediaTypeTest(v1.MediaTypeImageIndex, v1.MediaTypeImageIndex+"XXX", true)) +} + +func TestValidateIndex(t *testing.T) { + manifest := schema2.Manifest{ + Config: distribution.Descriptor{Size: 1}, + Layers: []distribution.Descriptor{{Size: 2}}, + } + index := ImageIndex{ + Manifests: []ManifestDescriptor{ + {Descriptor: distribution.Descriptor{Size: 3}}, + }, + } + t.Run("valid", func(t *testing.T) { + b, err := json.Marshal(index) + if err != nil { + t.Fatal("unexpected error marshaling index", err) + } + if err := validateIndex(b); err != nil { + t.Error("index should be valid", err) + } + }) + t.Run("invalid", func(t *testing.T) { + b, err := json.Marshal(manifest) + if err != nil { + t.Fatal("unexpected error marshaling manifest", err) + } + if err := validateIndex(b); err == nil { + t.Error("manifest should not be valid") + } + }) +} diff --git a/manifest/ocischema/manifest.go b/manifest/ocischema/manifest.go index 40b0bd839..b5133e84e 100644 --- a/manifest/ocischema/manifest.go +++ b/manifest/ocischema/manifest.go @@ -124,16 +124,12 @@ func (m DeserializedManifest) Payload() (string, []byte, error) { return v1.MediaTypeImageManifest, m.canonical, nil } -// unknownDocument represents a manifest, manifest list, or index that has not -// yet been validated -type unknownDocument struct { - Manifests interface{} `json:"manifests,omitempty"` -} - // validateManifest returns an error if the byte slice is invalid JSON or if it // contains fields that belong to a index func validateManifest(b []byte) error { - var doc unknownDocument + var doc struct { + Manifests interface{} `json:"manifests,omitempty"` + } if err := json.Unmarshal(b, &doc); err != nil { return err } diff --git a/manifest/ocischema/manifest_test.go b/manifest/ocischema/manifest_test.go index a906eab9e..7cde85a31 100644 --- a/manifest/ocischema/manifest_test.go +++ b/manifest/ocischema/manifest_test.go @@ -142,47 +142,49 @@ func TestManifest(t *testing.T) { } } -func mediaTypeTest(t *testing.T, mediaType string, shouldError bool) { - mfst := makeTestManifest(mediaType) +func manifestMediaTypeTest(mediaType string, shouldError bool) func(*testing.T) { + return func(t *testing.T) { + mfst := makeTestManifest(mediaType) - deserialized, err := FromStruct(mfst) - if err != nil { - t.Fatalf("error creating DeserializedManifest: %v", err) - } - - unmarshalled, descriptor, err := distribution.UnmarshalManifest( - v1.MediaTypeImageManifest, - deserialized.canonical) - - if shouldError { - if err == nil { - t.Fatalf("bad content type should have produced error") - } - } else { + deserialized, err := FromStruct(mfst) if err != nil { - t.Fatalf("error unmarshaling manifest, %v", err) + t.Fatalf("error creating DeserializedManifest: %v", err) } - asManifest := unmarshalled.(*DeserializedManifest) - if asManifest.MediaType != mediaType { - t.Fatalf("Bad media type '%v' as unmarshalled", asManifest.MediaType) - } + unmarshalled, descriptor, err := distribution.UnmarshalManifest( + v1.MediaTypeImageManifest, + deserialized.canonical) - if descriptor.MediaType != v1.MediaTypeImageManifest { - t.Fatalf("Bad media type '%v' for descriptor", descriptor.MediaType) - } + if shouldError { + if err == nil { + t.Fatalf("bad content type should have produced error") + } + } else { + if err != nil { + t.Fatalf("error unmarshaling manifest, %v", err) + } - unmarshalledMediaType, _, _ := unmarshalled.Payload() - if unmarshalledMediaType != v1.MediaTypeImageManifest { - t.Fatalf("Bad media type '%v' for payload", unmarshalledMediaType) + asManifest := unmarshalled.(*DeserializedManifest) + if asManifest.MediaType != mediaType { + t.Fatalf("Bad media type '%v' as unmarshalled", asManifest.MediaType) + } + + if descriptor.MediaType != v1.MediaTypeImageManifest { + t.Fatalf("Bad media type '%v' for descriptor", descriptor.MediaType) + } + + unmarshalledMediaType, _, _ := unmarshalled.Payload() + if unmarshalledMediaType != v1.MediaTypeImageManifest { + t.Fatalf("Bad media type '%v' for payload", unmarshalledMediaType) + } } } } -func TestMediaTypes(t *testing.T) { - mediaTypeTest(t, "", false) - mediaTypeTest(t, v1.MediaTypeImageManifest, false) - mediaTypeTest(t, v1.MediaTypeImageManifest+"XXX", true) +func TestManifestMediaTypes(t *testing.T) { + t.Run("No_MediaType", manifestMediaTypeTest("", false)) + t.Run("ImageManifest", manifestMediaTypeTest(v1.MediaTypeImageManifest, false)) + t.Run("Bad_MediaType", manifestMediaTypeTest(v1.MediaTypeImageManifest+"XXX", true)) } func TestValidateManifest(t *testing.T) { diff --git a/registry/storage/manifestlisthandler.go b/registry/storage/manifestlisthandler.go index e9c71d4c0..1c27bccfd 100644 --- a/registry/storage/manifestlisthandler.go +++ b/registry/storage/manifestlisthandler.go @@ -7,6 +7,7 @@ import ( "github.com/distribution/distribution/v3" dcontext "github.com/distribution/distribution/v3/context" "github.com/distribution/distribution/v3/manifest/manifestlist" + "github.com/distribution/distribution/v3/manifest/ocischema" "github.com/opencontainers/go-digest" ) @@ -33,16 +34,24 @@ func (ms *manifestListHandler) Unmarshal(ctx context.Context, dgst digest.Digest func (ms *manifestListHandler) Put(ctx context.Context, manifestList distribution.Manifest, skipDependencyVerification bool) (digest.Digest, error) { dcontext.GetLogger(ms.ctx).Debug("(*manifestListHandler).Put") - m, ok := manifestList.(*manifestlist.DeserializedManifestList) - if !ok { + var schemaVersion int + switch m := manifestList.(type) { + case *manifestlist.DeserializedManifestList: + schemaVersion = m.SchemaVersion + case *ocischema.DeserializedImageIndex: + schemaVersion = m.SchemaVersion + default: return "", fmt.Errorf("wrong type put to manifestListHandler: %T", manifestList) } + if schemaVersion != 2 { + return "", fmt.Errorf("unrecognized manifest list schema version %d", schemaVersion) + } - if err := ms.verifyManifest(ms.ctx, *m, skipDependencyVerification); err != nil { + if err := ms.verifyManifest(ms.ctx, manifestList, skipDependencyVerification); err != nil { return "", err } - mt, payload, err := m.Payload() + mt, payload, err := manifestList.Payload() if err != nil { return "", err } @@ -60,13 +69,9 @@ func (ms *manifestListHandler) Put(ctx context.Context, manifestList distributio // perspective of the registry. As a policy, the registry only tries to // store valid content, leaving trust policies of that content up to // consumers. -func (ms *manifestListHandler) verifyManifest(ctx context.Context, mnfst manifestlist.DeserializedManifestList, skipDependencyVerification bool) error { +func (ms *manifestListHandler) verifyManifest(ctx context.Context, mnfst distribution.Manifest, skipDependencyVerification bool) error { var errs distribution.ErrManifestVerification - if mnfst.SchemaVersion != 2 { - return fmt.Errorf("unrecognized manifest list schema version %d", mnfst.SchemaVersion) - } - if !skipDependencyVerification { // This manifest service is different from the blob service // returned by Blob. It uses a linked blob store to ensure that diff --git a/registry/storage/manifeststore.go b/registry/storage/manifeststore.go index 0daa92dfe..1610fd579 100644 --- a/registry/storage/manifeststore.go +++ b/registry/storage/manifeststore.go @@ -48,10 +48,11 @@ type manifestStore struct { skipDependencyVerification bool - schema1Handler ManifestHandler - schema2Handler ManifestHandler - ocischemaHandler ManifestHandler - manifestListHandler ManifestHandler + schema1Handler ManifestHandler + schema2Handler ManifestHandler + manifestListHandler ManifestHandler + ocischemaHandler ManifestHandler + ocischemaIndexHandler ManifestHandler } var _ distribution.ManifestService = &manifestStore{} @@ -104,14 +105,16 @@ func (ms *manifestStore) Get(ctx context.Context, dgst digest.Digest, options .. return ms.schema2Handler.Unmarshal(ctx, dgst, content) case v1.MediaTypeImageManifest: return ms.ocischemaHandler.Unmarshal(ctx, dgst, content) - case manifestlist.MediaTypeManifestList, v1.MediaTypeImageIndex: + case manifestlist.MediaTypeManifestList: return ms.manifestListHandler.Unmarshal(ctx, dgst, content) + case v1.MediaTypeImageIndex: + return ms.ocischemaIndexHandler.Unmarshal(ctx, dgst, content) case "": // OCI image or image index - no media type in the content // First see if it looks like an image index - res, err := ms.manifestListHandler.Unmarshal(ctx, dgst, content) - resIndex := res.(*manifestlist.DeserializedManifestList) + res, err := ms.ocischemaIndexHandler.Unmarshal(ctx, dgst, content) + resIndex := res.(*ocischema.DeserializedImageIndex) if err == nil && resIndex.Manifests != nil { return resIndex, nil } @@ -138,6 +141,8 @@ func (ms *manifestStore) Put(ctx context.Context, manifest distribution.Manifest return ms.ocischemaHandler.Put(ctx, manifest, ms.skipDependencyVerification) case *manifestlist.DeserializedManifestList: return ms.manifestListHandler.Put(ctx, manifest, ms.skipDependencyVerification) + case *ocischema.DeserializedImageIndex: + return ms.ocischemaIndexHandler.Put(ctx, manifest, ms.skipDependencyVerification) } return "", fmt.Errorf("unrecognized manifest type %T", manifest) diff --git a/registry/storage/manifeststore_test.go b/registry/storage/manifeststore_test.go index efcc80b6f..dd51c2f41 100644 --- a/registry/storage/manifeststore_test.go +++ b/registry/storage/manifeststore_test.go @@ -3,13 +3,13 @@ package storage import ( "bytes" "context" + "encoding/json" "io" "reflect" "testing" "github.com/distribution/distribution/v3" "github.com/distribution/distribution/v3/manifest" - "github.com/distribution/distribution/v3/manifest/manifestlist" "github.com/distribution/distribution/v3/manifest/ocischema" "github.com/distribution/distribution/v3/manifest/schema1" "github.com/distribution/distribution/v3/reference" @@ -465,19 +465,19 @@ func testOCIManifestStorage(t *testing.T, testname string, includeMediaTypes boo } descriptor.MediaType = v1.MediaTypeImageManifest - platformSpec := manifestlist.PlatformSpec{ + platformSpec := v1.Platform{ Architecture: "atari2600", OS: "CP/M", } - manifestDescriptors := []manifestlist.ManifestDescriptor{ + manifestDescriptors := []ocischema.ManifestDescriptor{ { Descriptor: descriptor, - Platform: platformSpec, + Platform: &platformSpec, }, } - imageIndex, err := manifestlist.FromDescriptorsWithMediaType(manifestDescriptors, indexMediaType) + imageIndex, err := ociIndexFromDesriptorsWithMediaType(manifestDescriptors, indexMediaType) if err != nil { t.Fatalf("%s: unexpected error creating image index: %v", testname, err) } @@ -523,7 +523,7 @@ func testOCIManifestStorage(t *testing.T, testname string, includeMediaTypes boo t.Fatalf("%s: unexpected error fetching image index: %v", testname, err) } - fetchedIndex, ok := fromStore.(*manifestlist.DeserializedManifestList) + fetchedIndex, ok := fromStore.(*ocischema.DeserializedImageIndex) if !ok { t.Fatalf("%s: unexpected type for fetched manifest", testname) } @@ -574,3 +574,23 @@ func TestLinkPathFuncs(t *testing.T) { } } } + +func ociIndexFromDesriptorsWithMediaType(descriptors []ocischema.ManifestDescriptor, mediaType string) (*ocischema.DeserializedImageIndex, error) { + manifest, err := ocischema.FromDescriptors(descriptors) + if err != nil { + return nil, err + } + manifest.ImageIndex.MediaType = mediaType + + rawManifest, err := json.Marshal(manifest.ImageIndex) + if err != nil { + return nil, err + } + + var d ocischema.DeserializedImageIndex + if err := d.UnmarshalJSON(rawManifest); err != nil { + return nil, err + } + + return &d, nil +} diff --git a/registry/storage/ociindexhandler.go b/registry/storage/ociindexhandler.go new file mode 100644 index 000000000..01864f061 --- /dev/null +++ b/registry/storage/ociindexhandler.go @@ -0,0 +1,28 @@ +package storage + +import ( + "context" + + "github.com/distribution/distribution/v3" + dcontext "github.com/distribution/distribution/v3/context" + "github.com/distribution/distribution/v3/manifest/ocischema" + "github.com/opencontainers/go-digest" +) + +// ocischemaIndexHandler is a ManifestHandler that covers the OCI Image Index. +type ocischemaIndexHandler struct { + *manifestListHandler +} + +var _ ManifestHandler = &manifestListHandler{} + +func (ms *ocischemaIndexHandler) Unmarshal(ctx context.Context, dgst digest.Digest, content []byte) (distribution.Manifest, error) { + dcontext.GetLogger(ms.ctx).Debug("(*ociIndexHandler).Unmarshal") + + m := &ocischema.DeserializedImageIndex{} + if err := m.UnmarshalJSON(content); err != nil { + return nil, err + } + + return m, nil +} diff --git a/registry/storage/registry.go b/registry/storage/registry.go index c49b6dbdd..7bf244878 100644 --- a/registry/storage/registry.go +++ b/registry/storage/registry.go @@ -259,6 +259,12 @@ func (repo *repository) Manifests(ctx context.Context, options ...distribution.M } } + manifestListHandler := &manifestListHandler{ + ctx: ctx, + repository: repo, + blobStore: blobStore, + } + ms := &manifestStore{ ctx: ctx, repository: repo, @@ -270,17 +276,16 @@ func (repo *repository) Manifests(ctx context.Context, options ...distribution.M blobStore: blobStore, manifestURLs: repo.registry.manifestURLs, }, - manifestListHandler: &manifestListHandler{ - ctx: ctx, - repository: repo, - blobStore: blobStore, - }, + manifestListHandler: manifestListHandler, ocischemaHandler: &ocischemaManifestHandler{ ctx: ctx, repository: repo, blobStore: blobStore, manifestURLs: repo.registry.manifestURLs, }, + ocischemaIndexHandler: &ocischemaIndexHandler{ + manifestListHandler: manifestListHandler, + }, } // Apply options From 88646f54da4c507a7624acfc88eec0a2efc0f69d Mon Sep 17 00:00:00 2001 From: Bracken Dawson Date: Fri, 31 Mar 2023 14:01:15 +0100 Subject: [PATCH 2/4] Support annotations in the OCI Image Index Empty platform structs were already supported after splitting OCI Image Index out from Docker Manifest List. Signed-off-by: Bracken Dawson --- manifest/ocischema/index.go | 18 ++++++++++++------ manifest/ocischema/index_test.go | 18 ++++++++++-------- registry/storage/manifeststore_test.go | 2 +- 3 files changed, 23 insertions(+), 15 deletions(-) diff --git a/manifest/ocischema/index.go b/manifest/ocischema/index.go index 97b3943a6..16b7b6873 100644 --- a/manifest/ocischema/index.go +++ b/manifest/ocischema/index.go @@ -60,6 +60,10 @@ type ImageIndex struct { // Manifests references a list of manifests Manifests []ManifestDescriptor `json:"manifests"` + + // Annotations is an optional field that contains arbitrary metadata for the + // image index + Annotations map[string]string `json:"annotations,omitempty"` } // References returns the distribution descriptors for the referenced image @@ -83,20 +87,22 @@ type DeserializedImageIndex struct { canonical []byte } -// FromDescriptors takes a slice of descriptors, and returns a -// DeserializedManifestList which contains the resulting manifest list -// and its JSON representation. -func FromDescriptors(descriptors []ManifestDescriptor) (*DeserializedImageIndex, error) { - return fromDescriptorsWithMediaType(descriptors, v1.MediaTypeImageIndex) +// FromDescriptors takes a slice of descriptors and a map of annotations, and +// returns a DeserializedManifestList which contains the resulting manifest list +// and its JSON representation. If annotations is nil or empty then the +// annotations property will be omitted from the JSON representation. +func FromDescriptors(descriptors []ManifestDescriptor, annotations map[string]string) (*DeserializedImageIndex, error) { + return fromDescriptorsWithMediaType(descriptors, annotations, v1.MediaTypeImageIndex) } // fromDescriptorsWithMediaType is for testing purposes, it's useful to be able to specify the media type explicitly -func fromDescriptorsWithMediaType(descriptors []ManifestDescriptor, mediaType string) (*DeserializedImageIndex, error) { +func fromDescriptorsWithMediaType(descriptors []ManifestDescriptor, annotations map[string]string, mediaType string) (*DeserializedImageIndex, error) { m := ImageIndex{ Versioned: manifest.Versioned{ SchemaVersion: 2, MediaType: mediaType, }, + Annotations: annotations, } m.Manifests = make([]ManifestDescriptor, len(descriptors)) diff --git a/manifest/ocischema/index_test.go b/manifest/ocischema/index_test.go index 90d9f5861..06af34f9a 100644 --- a/manifest/ocischema/index_test.go +++ b/manifest/ocischema/index_test.go @@ -11,12 +11,6 @@ import ( v1 "github.com/opencontainers/image-spec/specs-go/v1" ) -// TODO (mikebrow): add annotations on the manifest list (index) and support for -// empty platform structs (move to Platform *Platform `json:"platform,omitempty"` -// from current Platform PlatformSpec `json:"platform"`) in the manifest descriptor. -// Requires changes to distribution/distribution/manifest/manifestlist.ManifestList and .ManifestDescriptor -// and associated serialization APIs in manifestlist.go. Or split the OCI index and -// docker manifest list implementations, which would require a lot of refactoring. const expectedOCIImageIndexSerialization = `{ "schemaVersion": 2, "mediaType": "application/vnd.oci.image.index.v1+json", @@ -50,7 +44,11 @@ const expectedOCIImageIndexSerialization = `{ "os": "sunos" } } - ] + ], + "annotations": { + "com.example.favourite-colour": "blue", + "com.example.locale": "en_GB" + } }` func makeTestOCIImageIndex(t *testing.T, mediaType string) ([]ManifestDescriptor, *DeserializedImageIndex) { @@ -87,8 +85,12 @@ func makeTestOCIImageIndex(t *testing.T, mediaType string) ([]ManifestDescriptor }, }, } + annotations := map[string]string{ + "com.example.favourite-colour": "blue", + "com.example.locale": "en_GB", + } - deserialized, err := fromDescriptorsWithMediaType(manifestDescriptors, mediaType) + deserialized, err := fromDescriptorsWithMediaType(manifestDescriptors, annotations, mediaType) if err != nil { t.Fatalf("error creating DeserializedManifestList: %v", err) } diff --git a/registry/storage/manifeststore_test.go b/registry/storage/manifeststore_test.go index dd51c2f41..bba3538e1 100644 --- a/registry/storage/manifeststore_test.go +++ b/registry/storage/manifeststore_test.go @@ -576,7 +576,7 @@ func TestLinkPathFuncs(t *testing.T) { } func ociIndexFromDesriptorsWithMediaType(descriptors []ocischema.ManifestDescriptor, mediaType string) (*ocischema.DeserializedImageIndex, error) { - manifest, err := ocischema.FromDescriptors(descriptors) + manifest, err := ocischema.FromDescriptors(descriptors, nil) if err != nil { return nil, err } From 973bfbb676963aacb6c114943e461a18b74dfe24 Mon Sep 17 00:00:00 2001 From: Bracken Dawson Date: Fri, 21 Apr 2023 14:43:21 +0100 Subject: [PATCH 3/4] Fix Go Idioms - DRY out SchemaVersion literals - Better name the predefined Versioned struct for the Image Index - Var names, declarations, else cases. Co-authored-by: Milos Gajdos Signed-off-by: Bracken Dawson --- manifest/manifestlist/manifestlist.go | 2 +- manifest/ocischema/index.go | 23 ++++++++++------------- manifest/ocischema/manifest.go | 6 +++--- registry/storage/manifestlisthandler.go | 8 +++++--- 4 files changed, 19 insertions(+), 20 deletions(-) diff --git a/manifest/manifestlist/manifestlist.go b/manifest/manifestlist/manifestlist.go index 41f49775c..25ffbe525 100644 --- a/manifest/manifestlist/manifestlist.go +++ b/manifest/manifestlist/manifestlist.go @@ -129,7 +129,7 @@ func FromDescriptors(descriptors []ManifestDescriptor) (*DeserializedManifestLis func fromDescriptorsWithMediaType(descriptors []ManifestDescriptor, mediaType string) (*DeserializedManifestList, error) { m := ManifestList{ Versioned: manifest.Versioned{ - SchemaVersion: 2, + SchemaVersion: SchemaVersion.SchemaVersion, MediaType: mediaType, }, } diff --git a/manifest/ocischema/index.go b/manifest/ocischema/index.go index 16b7b6873..55daaa881 100644 --- a/manifest/ocischema/index.go +++ b/manifest/ocischema/index.go @@ -11,9 +11,9 @@ import ( v1 "github.com/opencontainers/image-spec/specs-go/v1" ) -// OCISchemaVersion provides a pre-initialized version structure for this -// packages OCIschema version of the manifest. -var OCISchemaVersion = manifest.Versioned{ +// IndexSchemaVersion provides a pre-initialized version structure for OCI Image +// Indices. +var IndexSchemaVersion = manifest.Versioned{ SchemaVersion: 2, MediaType: v1.MediaTypeImageIndex, } @@ -69,13 +69,13 @@ type ImageIndex struct { // References returns the distribution descriptors for the referenced image // manifests. func (ii ImageIndex) References() []distribution.Descriptor { - dependencies := make([]distribution.Descriptor, len(ii.Manifests)) + references := make([]distribution.Descriptor, len(ii.Manifests)) for i := range ii.Manifests { - dependencies[i] = ii.Manifests[i].Descriptor - dependencies[i].Platform = ii.Manifests[i].Platform + references[i] = ii.Manifests[i].Descriptor + references[i].Platform = ii.Manifests[i].Platform } - return dependencies + return references } // DeserializedImageIndex wraps ManifestList with a copy of the original @@ -96,10 +96,10 @@ func FromDescriptors(descriptors []ManifestDescriptor, annotations map[string]st } // fromDescriptorsWithMediaType is for testing purposes, it's useful to be able to specify the media type explicitly -func fromDescriptorsWithMediaType(descriptors []ManifestDescriptor, annotations map[string]string, mediaType string) (*DeserializedImageIndex, error) { +func fromDescriptorsWithMediaType(descriptors []ManifestDescriptor, annotations map[string]string, mediaType string) (_ *DeserializedImageIndex, err error) { m := ImageIndex{ Versioned: manifest.Versioned{ - SchemaVersion: 2, + SchemaVersion: IndexSchemaVersion.SchemaVersion, MediaType: mediaType, }, Annotations: annotations, @@ -112,7 +112,6 @@ func fromDescriptorsWithMediaType(descriptors []ManifestDescriptor, annotations ImageIndex: m, } - var err error deserialized.canonical, err = json.MarshalIndent(&m, "", " ") return &deserialized, err } @@ -147,11 +146,9 @@ func (m *DeserializedImageIndex) MarshalJSON() ([]byte, error) { // Payload returns the raw content of the manifest list. The contents can be // used to calculate the content identifier. func (m DeserializedImageIndex) Payload() (string, []byte, error) { - var mediaType string + mediaType := m.MediaType if m.MediaType == "" { mediaType = v1.MediaTypeImageIndex - } else { - mediaType = m.MediaType } return mediaType, m.canonical, nil diff --git a/manifest/ocischema/manifest.go b/manifest/ocischema/manifest.go index b5133e84e..973cd64ae 100644 --- a/manifest/ocischema/manifest.go +++ b/manifest/ocischema/manifest.go @@ -11,10 +11,10 @@ import ( v1 "github.com/opencontainers/image-spec/specs-go/v1" ) -// SchemaVersion provides a pre-initialized version structure for this -// packages version of the manifest. +// SchemaVersion provides a pre-initialized version structure for OCI Image +// Manifests var SchemaVersion = manifest.Versioned{ - SchemaVersion: 2, // historical value here.. does not pertain to OCI or docker version + SchemaVersion: 2, MediaType: v1.MediaTypeImageManifest, } diff --git a/registry/storage/manifestlisthandler.go b/registry/storage/manifestlisthandler.go index 1c27bccfd..1fc7aac7a 100644 --- a/registry/storage/manifestlisthandler.go +++ b/registry/storage/manifestlisthandler.go @@ -34,17 +34,19 @@ func (ms *manifestListHandler) Unmarshal(ctx context.Context, dgst digest.Digest func (ms *manifestListHandler) Put(ctx context.Context, manifestList distribution.Manifest, skipDependencyVerification bool) (digest.Digest, error) { dcontext.GetLogger(ms.ctx).Debug("(*manifestListHandler).Put") - var schemaVersion int + var schemaVersion, expectedSchemaVersion int switch m := manifestList.(type) { case *manifestlist.DeserializedManifestList: + expectedSchemaVersion = manifestlist.SchemaVersion.SchemaVersion schemaVersion = m.SchemaVersion case *ocischema.DeserializedImageIndex: + expectedSchemaVersion = ocischema.IndexSchemaVersion.SchemaVersion schemaVersion = m.SchemaVersion default: return "", fmt.Errorf("wrong type put to manifestListHandler: %T", manifestList) } - if schemaVersion != 2 { - return "", fmt.Errorf("unrecognized manifest list schema version %d", schemaVersion) + if schemaVersion != expectedSchemaVersion { + return "", fmt.Errorf("unrecognized manifest list schema version %d, expected %d", schemaVersion, expectedSchemaVersion) } if err := ms.verifyManifest(ms.ctx, manifestList, skipDependencyVerification); err != nil { From 9d1a8fc929be3281ee5e915c0951d2b73e4a9a2c Mon Sep 17 00:00:00 2001 From: Bracken Dawson Date: Thu, 1 Jun 2023 11:38:31 +0100 Subject: [PATCH 4/4] Remove duplicated platform field from oci index It is desirable to remove Platform from distribution.Descriptor because it doesn't really belong there. However this would be a further breaking change because the References() call would no longer be returning plaform information when it reurns descriptors of manifests, which is started to for OCI Indices after c94f288 and this feature was added to Docker Manifest Lists in 1a059fe. I don't want to take away something people clearly want. Signed-off-by: Bracken Dawson --- manifest/ocischema/index.go | 25 +++----------- manifest/ocischema/index_test.go | 47 +++++++++----------------- registry/storage/manifeststore_test.go | 14 ++------ 3 files changed, 24 insertions(+), 62 deletions(-) diff --git a/manifest/ocischema/index.go b/manifest/ocischema/index.go index 55daaa881..3ae824f40 100644 --- a/manifest/ocischema/index.go +++ b/manifest/ocischema/index.go @@ -45,21 +45,12 @@ func init() { } } -// A ManifestDescriptor references a platform-specific manifest. -type ManifestDescriptor struct { - distribution.Descriptor - - // Platform specifies which platform the manifest pointed to by the - // descriptor runs on. - Platform *v1.Platform `json:"platform,omitempty"` -} - // ImageIndex references manifests for various platforms. type ImageIndex struct { manifest.Versioned // Manifests references a list of manifests - Manifests []ManifestDescriptor `json:"manifests"` + Manifests []distribution.Descriptor `json:"manifests"` // Annotations is an optional field that contains arbitrary metadata for the // image index @@ -69,13 +60,7 @@ type ImageIndex struct { // References returns the distribution descriptors for the referenced image // manifests. func (ii ImageIndex) References() []distribution.Descriptor { - references := make([]distribution.Descriptor, len(ii.Manifests)) - for i := range ii.Manifests { - references[i] = ii.Manifests[i].Descriptor - references[i].Platform = ii.Manifests[i].Platform - } - - return references + return ii.Manifests } // DeserializedImageIndex wraps ManifestList with a copy of the original @@ -91,12 +76,12 @@ type DeserializedImageIndex struct { // returns a DeserializedManifestList which contains the resulting manifest list // and its JSON representation. If annotations is nil or empty then the // annotations property will be omitted from the JSON representation. -func FromDescriptors(descriptors []ManifestDescriptor, annotations map[string]string) (*DeserializedImageIndex, error) { +func FromDescriptors(descriptors []distribution.Descriptor, annotations map[string]string) (*DeserializedImageIndex, error) { return fromDescriptorsWithMediaType(descriptors, annotations, v1.MediaTypeImageIndex) } // fromDescriptorsWithMediaType is for testing purposes, it's useful to be able to specify the media type explicitly -func fromDescriptorsWithMediaType(descriptors []ManifestDescriptor, annotations map[string]string, mediaType string) (_ *DeserializedImageIndex, err error) { +func fromDescriptorsWithMediaType(descriptors []distribution.Descriptor, annotations map[string]string, mediaType string) (_ *DeserializedImageIndex, err error) { m := ImageIndex{ Versioned: manifest.Versioned{ SchemaVersion: IndexSchemaVersion.SchemaVersion, @@ -105,7 +90,7 @@ func fromDescriptorsWithMediaType(descriptors []ManifestDescriptor, annotations Annotations: annotations, } - m.Manifests = make([]ManifestDescriptor, len(descriptors)) + m.Manifests = make([]distribution.Descriptor, len(descriptors)) copy(m.Manifests, descriptors) deserialized := DeserializedImageIndex{ diff --git a/manifest/ocischema/index_test.go b/manifest/ocischema/index_test.go index 06af34f9a..cb07bb17a 100644 --- a/manifest/ocischema/index_test.go +++ b/manifest/ocischema/index_test.go @@ -51,34 +51,28 @@ const expectedOCIImageIndexSerialization = `{ } }` -func makeTestOCIImageIndex(t *testing.T, mediaType string) ([]ManifestDescriptor, *DeserializedImageIndex) { - manifestDescriptors := []ManifestDescriptor{ +func makeTestOCIImageIndex(t *testing.T, mediaType string) ([]distribution.Descriptor, *DeserializedImageIndex) { + manifestDescriptors := []distribution.Descriptor{ { - Descriptor: distribution.Descriptor{ - MediaType: "application/vnd.oci.image.manifest.v1+json", - Digest: "sha256:1a9ec845ee94c202b2d5da74a24f0ed2058318bfa9879fa541efaecba272e86b", - Size: 985, - }, + MediaType: "application/vnd.oci.image.manifest.v1+json", + Digest: "sha256:1a9ec845ee94c202b2d5da74a24f0ed2058318bfa9879fa541efaecba272e86b", + Size: 985, Platform: &v1.Platform{ Architecture: "amd64", OS: "linux", }, }, { - Descriptor: distribution.Descriptor{ - MediaType: "application/vnd.oci.image.manifest.v1+json", - Digest: "sha256:1a9ec845ee94c202b2d5da74a24f0ed2058318bfa9879fa541efaecba272e86b", - Size: 985, - Annotations: map[string]string{"platform": "none"}, - }, + MediaType: "application/vnd.oci.image.manifest.v1+json", + Digest: "sha256:1a9ec845ee94c202b2d5da74a24f0ed2058318bfa9879fa541efaecba272e86b", + Size: 985, + Annotations: map[string]string{"platform": "none"}, }, { - Descriptor: distribution.Descriptor{ - MediaType: "application/vnd.oci.image.manifest.v1+json", - Digest: "sha256:6346340964309634683409684360934680934608934608934608934068934608", - Size: 2392, - Annotations: map[string]string{"what": "for"}, - }, + MediaType: "application/vnd.oci.image.manifest.v1+json", + Digest: "sha256:6346340964309634683409684360934680934608934608934608934068934608", + Size: 2392, + Annotations: map[string]string{"what": "for"}, Platform: &v1.Platform{ Architecture: "sun4m", OS: "sunos", @@ -135,15 +129,8 @@ func TestOCIImageIndex(t *testing.T) { if len(references) != 3 { t.Fatalf("unexpected number of references: %d", len(references)) } - for i := range references { - expectedPlatform := manifestDescriptors[i].Platform - if !reflect.DeepEqual(references[i].Platform, expectedPlatform) { - t.Fatalf("unexpected value %d returned by References: %v", i, references[i]) - } - references[i].Platform = nil - if !reflect.DeepEqual(references[i], manifestDescriptors[i].Descriptor) { - t.Fatalf("unexpected value %d returned by References: %v", i, references[i]) - } + if !reflect.DeepEqual(references, manifestDescriptors) { + t.Errorf("expected references:\n%v\nbut got:\n%v", references, manifestDescriptors) } } @@ -199,9 +186,7 @@ func TestValidateIndex(t *testing.T) { Layers: []distribution.Descriptor{{Size: 2}}, } index := ImageIndex{ - Manifests: []ManifestDescriptor{ - {Descriptor: distribution.Descriptor{Size: 3}}, - }, + Manifests: []distribution.Descriptor{{Size: 3}}, } t.Run("valid", func(t *testing.T) { b, err := json.Marshal(index) diff --git a/registry/storage/manifeststore_test.go b/registry/storage/manifeststore_test.go index bba3538e1..402d5c945 100644 --- a/registry/storage/manifeststore_test.go +++ b/registry/storage/manifeststore_test.go @@ -464,20 +464,12 @@ func testOCIManifestStorage(t *testing.T, testname string, includeMediaTypes boo t.Fatalf("%s: unexpected error getting manifest descriptor", testname) } descriptor.MediaType = v1.MediaTypeImageManifest - - platformSpec := v1.Platform{ + descriptor.Platform = &v1.Platform{ Architecture: "atari2600", OS: "CP/M", } - manifestDescriptors := []ocischema.ManifestDescriptor{ - { - Descriptor: descriptor, - Platform: &platformSpec, - }, - } - - imageIndex, err := ociIndexFromDesriptorsWithMediaType(manifestDescriptors, indexMediaType) + imageIndex, err := ociIndexFromDesriptorsWithMediaType([]distribution.Descriptor{descriptor}, indexMediaType) if err != nil { t.Fatalf("%s: unexpected error creating image index: %v", testname, err) } @@ -575,7 +567,7 @@ func TestLinkPathFuncs(t *testing.T) { } } -func ociIndexFromDesriptorsWithMediaType(descriptors []ocischema.ManifestDescriptor, mediaType string) (*ocischema.DeserializedImageIndex, error) { +func ociIndexFromDesriptorsWithMediaType(descriptors []distribution.Descriptor, mediaType string) (*ocischema.DeserializedImageIndex, error) { manifest, err := ocischema.FromDescriptors(descriptors, nil) if err != nil { return nil, err