Skip to content

Conversation

crazy-max
Copy link
Member

@crazy-max crazy-max commented May 12, 2023

When using imagetools create and combining multiple sources we should check the media type of each manifest and set the right media type for the manifest list.

If there is a mismatch we set OCI index as best effort.

@crazy-max crazy-max added this to the v0.11.0 milestone May 12, 2023
}

var mt string
if cnt, ok := mfstmt[ocispec.MediaTypeImageManifest]; ok && cnt == len(newDescs) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Obviously the edgiest of edge cases 😄

But I think this does the wrong thing if we have a nested index? e.g. if we have oci indexes nested inside an oci index? In that case, we should still use the oci type.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hum right we don't handle nested indexes in addDesc atm

Copy link
Collaborator

Choose a reason for hiding this comment

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

What about this as an alternative: we should check for a prefix of application/vnd.docker to see if all the types are docker types, then use a docker manifest list in that case. If they are all OCI or even just mix-and-matched, then we should default to OCI.

Copy link
Member Author

Choose a reason for hiding this comment

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

As discussed yesterday, I'm fine defaulting to OCI.

When using imagetools create and combining multiple sources
we should check the media type of each manifest and set
the right media type for the manifest list.

If there is a mismatch we set OCI index as best effort.

Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
@crazy-max crazy-max requested a review from jedevc May 16, 2023 09:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants