Move manifest MIME type selection from GetManifest to ImageSource creation

This allows the selection to be consistent across GetManifest and
GetSignatures (which will be needed by Docker lookaside).

The API change causes lots of churn, but ultimately it just moves the
real origin of the value from image.FromSource() to transport.NewImageSource(),
both of which are static for the life of the ImageSource.

Does not change behavior.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
This commit is contained in:
Miloslav Trmač 2016-08-16 20:47:40 +02:00
parent 8ad0cad4ea
commit dff447c638
19 changed files with 79 additions and 58 deletions

View File

@ -25,7 +25,7 @@ func (s *dirImageSource) Reference() types.ImageReference {
}
// it's up to the caller to determine the MIME type of the returned manifest's bytes
func (s *dirImageSource) GetManifest(_ []string) ([]byte, string, error) {
func (s *dirImageSource) GetManifest() ([]byte, string, error) {
m, err := ioutil.ReadFile(s.ref.manifestPath())
if err != nil {
return nil, "", err

View File

@ -31,9 +31,9 @@ func TestGetPutManifest(t *testing.T) {
err = dest.PutManifest(man)
assert.NoError(t, err)
src, err := ref.NewImageSource(nil)
src, err := ref.NewImageSource(nil, nil)
require.NoError(t, err)
m, mt, err := src.GetManifest(nil)
m, mt, err := src.GetManifest()
assert.NoError(t, err)
assert.Equal(t, man, m)
assert.Equal(t, "", mt)
@ -50,7 +50,7 @@ func TestGetPutBlob(t *testing.T) {
err = dest.PutBlob(digest, bytes.NewReader(blob))
assert.NoError(t, err)
src, err := ref.NewImageSource(nil)
src, err := ref.NewImageSource(nil, nil)
require.NoError(t, err)
rc, size, err := src.GetBlob(digest)
assert.NoError(t, err)
@ -120,7 +120,7 @@ func TestGetPutSignatures(t *testing.T) {
err = dest.PutSignatures(signatures)
assert.NoError(t, err)
src, err := ref.NewImageSource(nil)
src, err := ref.NewImageSource(nil, nil)
require.NoError(t, err)
sigs, err := src.GetSignatures()
assert.NoError(t, err)
@ -131,7 +131,7 @@ func TestDelete(t *testing.T) {
ref, tmpDir := refToTempDir(t)
defer os.RemoveAll(tmpDir)
src, err := ref.NewImageSource(nil)
src, err := ref.NewImageSource(nil, nil)
require.NoError(t, err)
err = src.Delete()
assert.Error(t, err)
@ -141,7 +141,7 @@ func TestSourceReference(t *testing.T) {
ref, tmpDir := refToTempDir(t)
defer os.RemoveAll(tmpDir)
src, err := ref.NewImageSource(nil)
src, err := ref.NewImageSource(nil, nil)
require.NoError(t, err)
ref2 := src.Reference()
assert.Equal(t, tmpDir, ref2.StringWithinTransport())

View File

@ -130,11 +130,13 @@ func (ref dirReference) PolicyConfigurationNamespaces() []string {
// NewImage returns a types.Image for this reference.
func (ref dirReference) NewImage(ctx *types.SystemContext) (types.Image, error) {
src := newImageSource(ref)
return image.FromSource(src, nil), nil
return image.FromSource(src), nil
}
// NewImageSource returns a types.ImageSource for this reference.
func (ref dirReference) NewImageSource(ctx *types.SystemContext) (types.ImageSource, error) {
// NewImageSource returns a types.ImageSource for this reference,
// asking the backend to use a manifest from requestedManifestMIMETypes if possible
// nil requestedManifestMIMETypes means manifest.DefaultRequestedManifestMIMETypes.
func (ref dirReference) NewImageSource(ctx *types.SystemContext, requestedManifestMIMETypes []string) (types.ImageSource, error) {
return newImageSource(ref), nil
}

View File

@ -156,7 +156,7 @@ func TestReferenceNewImage(t *testing.T) {
func TestReferenceNewImageSource(t *testing.T) {
ref, tmpDir := refToTempDir(t)
defer os.RemoveAll(tmpDir)
_, err := ref.NewImageSource(nil)
_, err := ref.NewImageSource(nil, nil)
assert.NoError(t, err)
}

View File

@ -19,11 +19,11 @@ type Image struct {
// newImage returns a new Image interface type after setting up
// a client to the registry hosting the given image.
func newImage(ctx *types.SystemContext, ref dockerReference) (types.Image, error) {
s, err := newImageSource(ctx, ref)
s, err := newImageSource(ctx, ref, nil)
if err != nil {
return nil, err
}
return &Image{Image: image.FromSource(s, nil), src: s}, nil
return &Image{Image: image.FromSource(s), src: s}, nil
}
// SourceRefFullName returns a fully expanded name for the repository this image is in.

View File

@ -23,19 +23,26 @@ func (e errFetchManifest) Error() string {
}
type dockerImageSource struct {
ref dockerReference
c *dockerClient
ref dockerReference
requestedManifestMIMETypes []string
c *dockerClient
}
// newImageSource creates a new ImageSource for the specified image reference.
func newImageSource(ctx *types.SystemContext, ref dockerReference) (*dockerImageSource, error) {
// newImageSource creates a new ImageSource for the specified image reference,
// asking the backend to use a manifest from requestedManifestMIMETypes if possible
// nil requestedManifestMIMETypes means manifest.DefaultRequestedManifestMIMETypes.
func newImageSource(ctx *types.SystemContext, ref dockerReference, requestedManifestMIMETypes []string) (*dockerImageSource, error) {
c, err := newDockerClient(ctx, ref.ref.Hostname())
if err != nil {
return nil, err
}
if requestedManifestMIMETypes == nil {
requestedManifestMIMETypes = manifest.DefaultRequestedManifestMIMETypes
}
return &dockerImageSource{
ref: ref,
c: c,
requestedManifestMIMETypes: requestedManifestMIMETypes,
c: c,
}, nil
}
@ -58,7 +65,7 @@ func simplifyContentType(contentType string) string {
return mimeType
}
func (s *dockerImageSource) GetManifest(mimetypes []string) ([]byte, string, error) {
func (s *dockerImageSource) GetManifest() ([]byte, string, error) {
reference, err := s.ref.tagOrDigest()
if err != nil {
return nil, "", err
@ -67,7 +74,7 @@ func (s *dockerImageSource) GetManifest(mimetypes []string) ([]byte, string, err
// TODO(runcom) set manifest version header! schema1 for now - then schema2 etc etc and v1
// TODO(runcom) NO, switch on the resulter manifest like Docker is doing
headers := make(map[string][]string)
headers["Accept"] = mimetypes
headers["Accept"] = s.requestedManifestMIMETypes
res, err := s.c.makeRequest("GET", url, headers, nil)
if err != nil {
return nil, "", err

View File

@ -120,9 +120,11 @@ func (ref dockerReference) NewImage(ctx *types.SystemContext) (types.Image, erro
return newImage(ctx, ref)
}
// NewImageSource returns a types.ImageSource for this reference.
func (ref dockerReference) NewImageSource(ctx *types.SystemContext) (types.ImageSource, error) {
return newImageSource(ctx, ref)
// NewImageSource returns a types.ImageSource for this reference,
// asking the backend to use a manifest from requestedManifestMIMETypes if possible
// nil requestedManifestMIMETypes means manifest.DefaultRequestedManifestMIMETypes.
func (ref dockerReference) NewImageSource(ctx *types.SystemContext, requestedManifestMIMETypes []string) (types.ImageSource, error) {
return newImageSource(ctx, ref, requestedManifestMIMETypes)
}
// NewImageDestination returns a types.ImageDestination for this reference.

View File

@ -167,7 +167,7 @@ func TestReferenceNewImage(t *testing.T) {
func TestReferenceNewImageSource(t *testing.T) {
ref, err := ParseReference("//busybox")
require.NoError(t, err)
_, err = ref.NewImageSource(nil)
_, err = ref.NewImageSource(nil, nil)
assert.NoError(t, err)
}

View File

@ -33,21 +33,12 @@ type genericImage struct {
// this field is valid only if cachedManifest is not nil
cachedManifestMIMEType string
// private cache for Signatures(); nil if not yet known.
cachedSignatures [][]byte
requestedManifestMIMETypes []string
cachedSignatures [][]byte
}
// FromSource returns a types.Image implementation for source.
func FromSource(src types.ImageSource, requestedManifestMIMETypes []string) types.Image {
if len(requestedManifestMIMETypes) == 0 {
requestedManifestMIMETypes = []string{
manifest.OCIV1ImageManifestMIMEType,
manifest.DockerV2Schema2MIMEType,
manifest.DockerV2Schema1SignedMIMEType,
manifest.DockerV2Schema1MIMEType,
}
}
return &genericImage{src: src, requestedManifestMIMETypes: requestedManifestMIMETypes}
func FromSource(src types.ImageSource) types.Image {
return &genericImage{src: src}
}
// Reference returns the reference used to set up this source, _as specified by the user_
@ -60,7 +51,7 @@ func (i *genericImage) Reference() types.ImageReference {
// NOTE: It is essential for signature verification that Manifest returns the manifest from which BlobDigests is computed.
func (i *genericImage) Manifest() ([]byte, string, error) {
if i.cachedManifest == nil {
m, mt, err := i.src.GetManifest(i.requestedManifestMIMETypes)
m, mt, err := i.src.GetManifest()
if err != nil {
return nil, "", err
}

View File

@ -33,6 +33,15 @@ const (
OCIV1ImageSerializationConfigMIMEType = "application/vnd.oci.image.serialization.config.v1+json"
)
// DefaultRequestedManifestMIMETypes is a list of MIME types a types.ImageSource
// should request from the backend unless directed otherwise.
var DefaultRequestedManifestMIMETypes = []string{
OCIV1ImageManifestMIMEType,
DockerV2Schema2MIMEType,
DockerV2Schema1SignedMIMEType,
DockerV2Schema1MIMEType,
}
// GuessMIMEType guesses MIME type of a manifest and returns it _if it is recognized_, or "" if unknown or unrecognized.
// FIXME? We should, in general, prefer out-of-band MIME type instead of blindly parsing the manifest,
// but we may not have such metadata available (e.g. when the manifest is a local file).

View File

@ -169,8 +169,10 @@ func (ref ociReference) NewImage(ctx *types.SystemContext) (types.Image, error)
return nil, errors.New("Full Image support not implemented for oci: image names")
}
// NewImageSource returns a types.ImageSource for this reference.
func (ref ociReference) NewImageSource(ctx *types.SystemContext) (types.ImageSource, error) {
// NewImageSource returns a types.ImageSource for this reference,
// asking the backend to use a manifest from requestedManifestMIMETypes if possible
// nil requestedManifestMIMETypes means manifest.DefaultRequestedManifestMIMETypes.
func (ref ociReference) NewImageSource(ctx *types.SystemContext, requestedManifestMIMETypes []string) (types.ImageSource, error) {
return nil, errors.New("Reading images not implemented for oci: image names")
}

View File

@ -212,7 +212,7 @@ func TestReferenceNewImage(t *testing.T) {
func TestReferenceNewImageSource(t *testing.T) {
ref, tmpDir := refToTempOCI(t)
defer os.RemoveAll(tmpDir)
_, err := ref.NewImageSource(nil)
_, err := ref.NewImageSource(nil, nil)
assert.Error(t, err)
}

View File

@ -171,14 +171,17 @@ func (c *openshiftClient) dockerRegistryHostPart() string {
type openshiftImageSource struct {
client *openshiftClient
// Values specific to this image
ctx *types.SystemContext
ctx *types.SystemContext
requestedManifestMIMETypes []string
// State
docker types.ImageSource // The Docker Registry endpoint, or nil if not resolved yet
imageStreamImageName string // Resolved image identifier, or "" if not known yet
}
// newImageSource creates a new ImageSource for the specified reference.
func newImageSource(ctx *types.SystemContext, ref openshiftReference) (types.ImageSource, error) {
// newImageSource creates a new ImageSource for the specified reference,
// asking the backend to use a manifest from requestedManifestMIMETypes if possible
// nil requestedManifestMIMETypes means manifest.DefaultRequestedManifestMIMETypes.
func newImageSource(ctx *types.SystemContext, ref openshiftReference, requestedManifestMIMETypes []string) (types.ImageSource, error) {
client, err := newOpenshiftClient(ref)
if err != nil {
return nil, err
@ -187,6 +190,7 @@ func newImageSource(ctx *types.SystemContext, ref openshiftReference) (types.Ima
return &openshiftImageSource{
client: client,
ctx: ctx,
requestedManifestMIMETypes: requestedManifestMIMETypes,
}, nil
}
@ -196,11 +200,11 @@ func (s *openshiftImageSource) Reference() types.ImageReference {
return s.client.ref
}
func (s *openshiftImageSource) GetManifest(mimetypes []string) ([]byte, string, error) {
func (s *openshiftImageSource) GetManifest() ([]byte, string, error) {
if err := s.ensureImageIsResolved(); err != nil {
return nil, "", err
}
return s.docker.GetManifest(mimetypes)
return s.docker.GetManifest()
}
func (s *openshiftImageSource) GetBlob(digest string) (io.ReadCloser, int64, error) {
@ -268,7 +272,7 @@ func (s *openshiftImageSource) ensureImageIsResolved() error {
if err != nil {
return err
}
d, err := dockerRef.NewImageSource(s.ctx)
d, err := dockerRef.NewImageSource(s.ctx, s.requestedManifestMIMETypes)
if err != nil {
return err
}

View File

@ -158,9 +158,11 @@ func (ref openshiftReference) NewImage(ctx *types.SystemContext) (types.Image, e
return nil, errors.New("Full Image support not implemented for atomic: image names")
}
// NewImageSource returns a types.ImageSource for this reference.
func (ref openshiftReference) NewImageSource(ctx *types.SystemContext) (types.ImageSource, error) {
return newImageSource(ctx, ref)
// NewImageSource returns a types.ImageSource for this reference,
// asking the backend to use a manifest from requestedManifestMIMETypes if possible
// nil requestedManifestMIMETypes means manifest.DefaultRequestedManifestMIMETypes.
func (ref openshiftReference) NewImageSource(ctx *types.SystemContext, requestedManifestMIMETypes []string) (types.ImageSource, error) {
return newImageSource(ctx, ref, requestedManifestMIMETypes)
}
// NewImageDestination returns a types.ImageDestination for this reference.

View File

@ -25,12 +25,12 @@ func dirImageMock(t *testing.T, dir, dockerReference string) types.Image {
func dirImageMockWithRef(t *testing.T, dir string, ref types.ImageReference) types.Image {
srcRef, err := directory.NewReference(dir)
require.NoError(t, err)
src, err := srcRef.NewImageSource(nil)
src, err := srcRef.NewImageSource(nil, nil)
require.NoError(t, err)
return image.FromSource(&dirImageSourceMock{
ImageSource: src,
ref: ref,
}, nil)
})
}
// dirImageSourceMock inherits dirImageSource, but overrides its Reference method.

View File

@ -37,7 +37,7 @@ func (ref nameOnlyImageReferenceMock) PolicyConfigurationNamespaces() []string {
func (ref nameOnlyImageReferenceMock) NewImage(ctx *types.SystemContext) (types.Image, error) {
panic("unexpected call to a mock function")
}
func (ref nameOnlyImageReferenceMock) NewImageSource(ctx *types.SystemContext) (types.ImageSource, error) {
func (ref nameOnlyImageReferenceMock) NewImageSource(ctx *types.SystemContext, requestedManifestMIMETypes []string) (types.ImageSource, error) {
panic("unexpected call to a mock function")
}
func (ref nameOnlyImageReferenceMock) NewImageDestination(ctx *types.SystemContext) (types.ImageDestination, error) {

View File

@ -93,7 +93,7 @@ func (ref pcImageReferenceMock) PolicyConfigurationNamespaces() []string {
func (ref pcImageReferenceMock) NewImage(ctx *types.SystemContext) (types.Image, error) {
panic("unexpected call to a mock function")
}
func (ref pcImageReferenceMock) NewImageSource(ctx *types.SystemContext) (types.ImageSource, error) {
func (ref pcImageReferenceMock) NewImageSource(ctx *types.SystemContext, requestedManifestMIMETypes []string) (types.ImageSource, error) {
panic("unexpected call to a mock function")
}
func (ref pcImageReferenceMock) NewImageDestination(ctx *types.SystemContext) (types.ImageDestination, error) {

View File

@ -104,7 +104,7 @@ func (ref refImageReferenceMock) PolicyConfigurationNamespaces() []string {
func (ref refImageReferenceMock) NewImage(ctx *types.SystemContext) (types.Image, error) {
panic("unexpected call to a mock function")
}
func (ref refImageReferenceMock) NewImageSource(ctx *types.SystemContext) (types.ImageSource, error) {
func (ref refImageReferenceMock) NewImageSource(ctx *types.SystemContext, requestedManifestMIMETypes []string) (types.ImageSource, error) {
panic("unexpected call to a mock function")
}
func (ref refImageReferenceMock) NewImageDestination(ctx *types.SystemContext) (types.ImageDestination, error) {

View File

@ -72,8 +72,10 @@ type ImageReference interface {
// NewImage returns a types.Image for this reference.
NewImage(ctx *SystemContext) (Image, error)
// NewImageSource returns a types.ImageSource for this reference.
NewImageSource(ctx *SystemContext) (ImageSource, error)
// NewImageSource returns a types.ImageSource for this reference,
// asking the backend to use a manifest from requestedManifestMIMETypes if possible
// nil requestedManifestMIMETypes means manifest.DefaultRequestedManifestMIMETypes.
NewImageSource(ctx *SystemContext, requestedManifestMIMETypes []string) (ImageSource, error)
// NewImageDestination returns a types.ImageDestination for this reference.
NewImageDestination(ctx *SystemContext) (ImageDestination, error)
}
@ -85,9 +87,9 @@ type ImageSource interface {
// Reference returns the reference used to set up this source, _as specified by the user_
// (not as the image itself, or its underlying storage, claims). This can be used e.g. to determine which public keys are trusted for this image.
Reference() ImageReference
// GetManifest returns the image's manifest along with its MIME type. The empty string is returned if the MIME type is unknown. The slice parameter indicates the supported mime types the manifest should be when getting it.
// GetManifest returns the image's manifest along with its MIME type. The empty string is returned if the MIME type is unknown.
// It may use a remote (= slow) service.
GetManifest([]string) ([]byte, string, error)
GetManifest() ([]byte, string, error)
// Note: Calling GetBlob() may have ordering dependencies WRT other methods of this type. FIXME: How does this work with (docker save) on stdin?
// the second return value is the size of the blob. If not known 0 is returned
GetBlob(digest string) (io.ReadCloser, int64, error)