Skip to content

Commit

Permalink
Remove duplicated platform field from oci index
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
brackendawson committed Jun 1, 2023
1 parent 973bfbb commit 9d1a8fc
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 62 deletions.
25 changes: 5 additions & 20 deletions manifest/ocischema/index.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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,
Expand All @@ -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{
Expand Down
47 changes: 16 additions & 31 deletions manifest/ocischema/index_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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)
}
}

Expand Down Expand Up @@ -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)
Expand Down
14 changes: 3 additions & 11 deletions registry/storage/manifeststore_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 9d1a8fc

Please sign in to comment.