Skip to content
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

Jump to bottom

Support for image indexes with multiple manifests #175

Open
matthewpi opened this issue May 2, 2023 · 5 comments · May be fixed by #176
Open

Support for image indexes with multiple manifests #175

matthewpi opened this issue May 2, 2023 · 5 comments · May be fixed by #176
Labels
enhancement New feature or request

Comments

@matthewpi
Copy link

matthewpi commented May 2, 2023 •
edited
Loading

What would you like to be added:

Currently stereoscope will throw an error when using image providers when they are passed an image index that contains references to multiple manifests. I'd like to request that this behaviour is changed (or a new feature is implemented) to support image indexes with references to multiple manifests.

Why is this needed:

Multi manifest image indexes are becoming increasingly more common, especially with the prominence of multiple supported architectures for containers (primarily amd64 and arm64). If stereoscope is passed one of these multi manifest indexes, it errors out with no ability to filter or select one of the manifests from the index (at least with the oci-dir provider).

My primary use-case for this change is to be able to use Syft to generate SBOMs for each manifest within a multi-arch oci-dir. While I'm unsure exactly how the SBOMs will be generated for multi-arch images, the changes I am requesting here seem to be a prerequisite to be able to support any type of multi-arch SBOM (or generating SBOMs per image manifest) when given a multi-manifest index.

Additional context:

I have already prototyped the code for supporting this and plan to open a PR but thought I would open an issue before doing so. My main concerns about making this change is to what extent is changing or breaking the existing API allowed.

My current implementation adds a new Index struct, a new IndexProvider interface with a ProvideIndex method so providers can optionally add support for multi-manifest indexes, however this feels very awkward for two reasons.

The first is providers will still error out if you call Provide with a multi-manifest index, this is due to the existing API not being changed at all and Provide still only supporting a single image.

Secondly, there would now another entire code path to support multi-manifest indexes. While this code path does still fully support single-manifest indexes (or just single images if the format doesn't have the concept of multiple manifests), it does mean users of this library will need to implement support for a different API and new users may be confused about using the Index vs non-index functions.
@matthewpi matthewpi added the enhancement New feature or request label May 2, 2023
@matthewpi matthewpi linked a pull request May 2, 2023 that will close this issue
Open
@sophiewigmore
Copy link

sophiewigmore commented Jun 14, 2023 •
edited
Loading

Hey y'all! My team has a similar desire for support for multiple manifests as we have begun shipping multi-arch images in our project. @matthewpi are you still looking into this?

@matthewpi
Copy link
Author

Hey y'all! My team has a similar desire for support for multiple manifests as we have begun shipping multi-arch images in our project. @matthewpi are you still looking into this?

I was but had to take time to work on some other things, the PR I opened works correctly but I am still unsure about it's API design. Ignoring any potential changes to the API design, the code works and is ready to be merged. I will see about looking over the code myself later today and ensuring it is ready for review.

Following that, we would need to get any consumers of the library to update and change their code-paths to support the new image index provider and types.

@sophiewigmore
Copy link

Cool, thanks for the update. In the meantime we've found a workaround of pushing our multi-arch OCI archives to a local docker registry, and then generating the SBOM via syft on the local registry images which seems to work.

@wagoodman wagoodman added this to OSS Jun 27, 2023
@JonZeolla JonZeolla mentioned this issue Aug 18, 2023
Merged
4 tasks
@kzantow
Copy link
Contributor

kzantow commented Aug 24, 2023

This is related to the analogous issue in Syft: anchore/syft#1683

@kzantow kzantow moved this to In Progress in OSS Aug 24, 2023
@spiffcs spiffcs moved this from In Progress to Ready in OSS Feb 14, 2024
@wagoodman wagoodman moved this from Ready to In Progress in OSS Feb 14, 2024
@wagoodman wagoodman removed their assignment Feb 26, 2024
@spiffcs
Copy link
Contributor

spiffcs commented Aug 29, 2024

👋 Sorry that this issue has been stale for so long!

We came upon it when doing some review of issues during our live stream:
https://www.youtube.com/channel/UC3HczVqyiAqz1aNxBIMS3pQ

Here is the plan that we came up with - We want a solution to an oci index with multiple manifests that is narrowly scoped to the same behavior as the other stereoscope providers.

This means two things:

  1. We focus the solution to stereoscope providing a single answer from the manifest. Multiple answers from the index to generate multiple sbom by syft side is not in scope for this change.

Below is how the docker daemon provider works for stereoscope when trying to select the correct

stereoscope/pkg/image/containerd/daemon_provider.go

Lines 238 to 270 in e6d086e

// let's make certain that the image we found is for the platform we want (but the hard way!)
desc := img.Target()
switch desc.MediaType {
case images.MediaTypeDockerSchema2Manifest, ocispec.MediaTypeImageManifest:
return processManifest(imageStr, desc)
case images.MediaTypeDockerSchema2ManifestList, ocispec.MediaTypeImageIndex:
img = nil
// let's find the digest for the manifest for the specific architecture we want
by, err := content.ReadBlob(ctx, client.ContentStore(), desc)
if err != nil {
return "", nil, fmt.Errorf("unable to fetch manifest list: %w", err)
}
var index ocispec.Index
if err := json.Unmarshal(by, &index); err != nil {
return "", nil, fmt.Errorf("unable to unmarshal manifest list: %w", err)
}
platformObj, err := platforms.Parse(p.platform.String())
if err != nil {
return "", nil, fmt.Errorf("unable to parse platform: %w", err)
}
platformMatcher := platforms.NewMatcher(platformObj)
for _, manifestDesc := range index.Manifests {
if manifestDesc.Platform == nil {
continue
}
if platformMatcher.Match(*manifestDesc.Platform) {
return processManifest(imageStr, manifestDesc)
}
}

The OCI provider currently selects position 0 from the manifest list:

stereoscope/pkg/image/oci/directory_provider.go

Lines 62 to 66 in e6d086e

manifest := indexManifest.Manifests[0]
img, err := pathObj.Image(manifest.Digest)
if err != nil {
return nil, fmt.Errorf("unable to parse OCI directory as an image: %w", err)
}

It should be updated to an API where syft can ask for a certain platform/arch from the index and expect to get a single *image.Image answer.

We did not have time to conclude if this input from syft should be required, or if some other default stereoscope behavior should exist to try and "guess" which image to respond with.

@spiffcs spiffcs moved this from In Progress to Stalled in OSS Oct 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement New feature or request
Projects
OSS
Status: Stalled
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

image: multi-manifest index support matthewpi/stereoscope
5 participants
@wagoodman @kzantow @sophiewigmore @matthewpi @spiffcs