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.

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

v1.ImageIndex needs a Blob(Hash) method #819

Open
jonjohnsonjr opened this issue Nov 6, 2020 · 3 comments
Open

v1.ImageIndex needs a Blob(Hash) method #819

jonjohnsonjr opened this issue Nov 6, 2020 · 3 comments

Comments

@jonjohnsonjr
Copy link
Collaborator

Given that a descriptor can point to arbitrary content, we need a way to access that content if it doesn't conform to expected media-types. layout.layoutIndex already implements this.

We may want to have an option v1.ImageIndex.Layer(Hash) as well (and use it for known layer media types), because of how MountableLayer works.

We also need to update a lot of places where we handle v1.ImageIndex. A few that spring to mind...

In mutate, you can't append non-manifests to an index:

} else {
logs.Warn.Printf("Unexpected index addendum: %T", add.Add)
}

In remote.Write, we just ignore non-manifests:

switch desc.MediaType {
case types.OCIImageIndex, types.DockerManifestList:
ii, err := ii.ImageIndex(desc.Digest)
if err != nil {
return err
}
if err := WriteIndex(ref, ii, WithAuth(o.auth), WithTransport(o.transport)); err != nil {
return err
}
case types.OCIManifestSchema1, types.DockerManifestSchema2:
img, err := ii.Image(desc.Digest)
if err != nil {
return err
}
if err := Write(ref, img, WithAuth(o.auth), WithTransport(o.transport)); err != nil {
return err
}
}

In remote.MultiWrite, we return an error:

return nil, fmt.Errorf("unknown media type: %v", desc.MediaType)

In layout.Write, we ignore non-manifests:

default:
// TODO: The layout could reference arbitrary things, which we should
// probably just pass through.

In validate.Index, we log a warning:

default:
logs.Warn.Printf("Unexpected manifest: %s", desc.MediaType)

In compare.Indexes, we don't bother:

// TODO(jonjohnsonjr): Iterate over Manifest and compare Image and ImageIndex results.

@jonjohnsonjr
Copy link
Collaborator Author

This came up in #814 (comment)

@jonjohnsonjr
Copy link
Collaborator Author

We might want to add a Manifests() method that mirrors v1.Image's Layers().

Unfortunately, there's not a great return type for that. We need something that can be any of v1.Image, v1.ImageIndex, v1.Layer, or just an io.ReadCloser (perhaps this can just be a v1.Layer?). Related to #835

I need to go through all the callsites to see what we actually end up doing with this stuff to see if there's an obvious common set of functionality.

In the short term, we could do some typechecking for Blob(Hash) and Layer(hash) for unexpected mediatypes.

@github-actions
Copy link

github-actions bot commented May 4, 2021

This issue is stale because it has been open for 90 days with no
activity. It will automatically close after 30 more days of
inactivity. Reopen the issue with /reopen. Mark the issue as
fresh by adding the comment /remove-lifecycle stale.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant