From 9d1a8fc929be3281ee5e915c0951d2b73e4a9a2c Mon Sep 17 00:00:00 2001 From: Bracken Dawson Date: Thu, 1 Jun 2023 11:38:31 +0100 Subject: [PATCH] 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