From cca29316048999e19aa65a5c67c8cad326e3a154 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Thu, 6 Oct 2016 22:11:08 +0200 Subject: [PATCH 01/11] In copy.Image, do signature verification before doing _anything_ else MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In particular, do not even look at the manifest type; we might not event want to fetch the manifest if required signatures are missing, so loading it and possibly guessing the MIME type is not needed either. Signed-off-by: Miloslav Trmač --- copy/copy.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/copy/copy.go b/copy/copy.go index 450827cc4d..09c463d97d 100644 --- a/copy/copy.go +++ b/copy/copy.go @@ -112,6 +112,11 @@ func Image(ctx *types.SystemContext, policyContext *signature.PolicyContext, des src := image.FromSource(rawSource) defer src.Close() + // Please keep this policy check BEFORE reading any other information about the image. + if allowed, err := policyContext.IsRunningImageAllowed(src); !allowed || err != nil { // Be paranoid and fail if either return value indicates so. + return fmt.Errorf("Source image rejected: %v", err) + } + multiImage, err := src.IsMultiImage() if err != nil { return err @@ -120,11 +125,6 @@ func Image(ctx *types.SystemContext, policyContext *signature.PolicyContext, des return fmt.Errorf("can not copy %s: manifest contains multiple images", transports.ImageName(srcRef)) } - // Please keep this policy check BEFORE reading any other information about the image. - if allowed, err := policyContext.IsRunningImageAllowed(src); !allowed || err != nil { // Be paranoid and fail if either return value indicates so. - return fmt.Errorf("Source image rejected: %v", err) - } - writeReport("Getting image source manifest\n") manifest, _, err := src.Manifest() if err != nil { From b8751d89dc253a50a0916fffcd674994b62ed238 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Thu, 6 Oct 2016 21:33:32 +0200 Subject: [PATCH 02/11] Add types.UnparsedImage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently we use a types.Image, which supports lots of parsing, for verification processing in in signature.PolicyContext. In the future, we will want that types.Image to do significantly more processing at initialization time (e.g. determine manifest type and fully parse it), which is undesirable for signature verification — there we would _really_ prefer to first find a signature which cryptographically verifies, before even _downloading_ the manifest, let alone processing it in any way. So, split the minimum functionality desired for processing unsigned images (manifest and signature caching) into a separate UnparsedImage type. Right now, this does not affect any Image or UnparsedImage implementation (apart from dropping a few panic()ing mock functions). (Note that for some more advanced processing, signature/* may create a types.Image out of the given types.UnparsedImage in the future — but that would be an intentional action after the signature code determines that there is enough presumed trust to even start parsing anything.) Signed-off-by: Miloslav Trmač --- signature/policy_eval.go | 10 +++--- signature/policy_eval_baselayer.go | 4 +-- signature/policy_eval_signedby.go | 4 +-- signature/policy_eval_signedby_test.go | 11 +++--- signature/policy_eval_simple.go | 8 ++--- signature/policy_eval_simple_test.go | 6 +--- signature/policy_eval_test.go | 4 +-- signature/policy_reference_match.go | 10 +++--- signature/policy_reference_match_test.go | 46 ++---------------------- types/types.go | 20 +++++++---- 10 files changed, 43 insertions(+), 80 deletions(-) diff --git a/signature/policy_eval.go b/signature/policy_eval.go index ab3a50f908..4828ecc81b 100644 --- a/signature/policy_eval.go +++ b/signature/policy_eval.go @@ -54,14 +54,14 @@ type PolicyRequirement interface { // a container based on this image; use IsRunningImageAllowed instead. // - Just because a signature is accepted does not automatically mean the contents of the // signature are authorized to run code as root, or to affect system or cluster configuration. - isSignatureAuthorAccepted(image types.Image, sig []byte) (signatureAcceptanceResult, *Signature, error) + isSignatureAuthorAccepted(image types.UnparsedImage, sig []byte) (signatureAcceptanceResult, *Signature, error) // isRunningImageAllowed returns true if the requirement allows running an image. // If it returns false, err must be non-nil, and should be an PolicyRequirementError if evaluation // succeeded but the result was rejection. // WARNING: This validates signatures and the manifest, but does not download or validate the // layers. Users must validate that the layers match their expected digests. - isRunningImageAllowed(image types.Image) (bool, error) + isRunningImageAllowed(image types.UnparsedImage) (bool, error) } // PolicyReferenceMatch specifies a set of image identities accepted in PolicyRequirement. @@ -70,7 +70,7 @@ type PolicyReferenceMatch interface { // matchesDockerReference decides whether a specific image identity is accepted for an image // (or, usually, for the image's Reference().DockerReference()). Note that // image.Reference().DockerReference() may be nil. - matchesDockerReference(image types.Image, signatureDockerReference string) bool + matchesDockerReference(image types.UnparsedImage, signatureDockerReference string) bool } // PolicyContext encapsulates a policy and possible cached state @@ -174,7 +174,7 @@ func (pc *PolicyContext) requirementsForImageRef(ref types.ImageReference) Polic // a container based on this image; use IsRunningImageAllowed instead. // - Just because a signature is accepted does not automatically mean the contents of the // signature are authorized to run code as root, or to affect system or cluster configuration. -func (pc *PolicyContext) GetSignaturesWithAcceptedAuthor(image types.Image) (sigs []*Signature, finalErr error) { +func (pc *PolicyContext) GetSignaturesWithAcceptedAuthor(image types.UnparsedImage) (sigs []*Signature, finalErr error) { if err := pc.changeState(pcReady, pcInUse); err != nil { return nil, err } @@ -254,7 +254,7 @@ func (pc *PolicyContext) GetSignaturesWithAcceptedAuthor(image types.Image) (sig // succeeded but the result was rejection. // WARNING: This validates signatures and the manifest, but does not download or validate the // layers. Users must validate that the layers match their expected digests. -func (pc *PolicyContext) IsRunningImageAllowed(image types.Image) (res bool, finalErr error) { +func (pc *PolicyContext) IsRunningImageAllowed(image types.UnparsedImage) (res bool, finalErr error) { if err := pc.changeState(pcReady, pcInUse); err != nil { return false, err } diff --git a/signature/policy_eval_baselayer.go b/signature/policy_eval_baselayer.go index 8e5e628aea..dec84c93c1 100644 --- a/signature/policy_eval_baselayer.go +++ b/signature/policy_eval_baselayer.go @@ -7,11 +7,11 @@ import ( "github.com/containers/image/types" ) -func (pr *prSignedBaseLayer) isSignatureAuthorAccepted(image types.Image, sig []byte) (signatureAcceptanceResult, *Signature, error) { +func (pr *prSignedBaseLayer) isSignatureAuthorAccepted(image types.UnparsedImage, sig []byte) (signatureAcceptanceResult, *Signature, error) { return sarUnknown, nil, nil } -func (pr *prSignedBaseLayer) isRunningImageAllowed(image types.Image) (bool, error) { +func (pr *prSignedBaseLayer) isRunningImageAllowed(image types.UnparsedImage) (bool, error) { // FIXME? Reject this at policy parsing time already? logrus.Errorf("signedBaseLayer not implemented yet!") return false, PolicyRequirementError("signedBaseLayer not implemented yet!") diff --git a/signature/policy_eval_signedby.go b/signature/policy_eval_signedby.go index e6889c8965..7b1ffac710 100644 --- a/signature/policy_eval_signedby.go +++ b/signature/policy_eval_signedby.go @@ -13,7 +13,7 @@ import ( "github.com/containers/image/types" ) -func (pr *prSignedBy) isSignatureAuthorAccepted(image types.Image, sig []byte) (signatureAcceptanceResult, *Signature, error) { +func (pr *prSignedBy) isSignatureAuthorAccepted(image types.UnparsedImage, sig []byte) (signatureAcceptanceResult, *Signature, error) { switch pr.KeyType { case SBKeyTypeGPGKeys: case SBKeyTypeSignedByGPGKeys, SBKeyTypeX509Certificates, SBKeyTypeSignedByX509CAs: @@ -97,7 +97,7 @@ func (pr *prSignedBy) isSignatureAuthorAccepted(image types.Image, sig []byte) ( return sarAccepted, signature, nil } -func (pr *prSignedBy) isRunningImageAllowed(image types.Image) (bool, error) { +func (pr *prSignedBy) isRunningImageAllowed(image types.UnparsedImage) (bool, error) { sigs, err := image.Signatures() if err != nil { return false, err diff --git a/signature/policy_eval_signedby_test.go b/signature/policy_eval_signedby_test.go index a3c89f9950..22f6c82138 100644 --- a/signature/policy_eval_signedby_test.go +++ b/signature/policy_eval_signedby_test.go @@ -14,16 +14,17 @@ import ( "github.com/stretchr/testify/require" ) -// dirImageMock returns a types.Image for a directory, claiming a specified dockerReference. -func dirImageMock(t *testing.T, dir, dockerReference string) types.Image { +// dirImageMock returns a types.UnparsedImage for a directory, claiming a specified dockerReference. +// The caller must call .Close() on the returned UnparsedImage. +func dirImageMock(t *testing.T, dir, dockerReference string) types.UnparsedImage { ref, err := reference.ParseNamed(dockerReference) require.NoError(t, err) return dirImageMockWithRef(t, dir, refImageReferenceMock{ref}) } -// dirImageMockWithRef returns a types.Image for a directory, claiming a specified ref. -// The caller must call .Close() on the returned Image. -func dirImageMockWithRef(t *testing.T, dir string, ref types.ImageReference) types.Image { +// dirImageMockWithRef returns a types.UnparsedImage for a directory, claiming a specified ref. +// The caller must call .Close() on the returned UnparsedImage. +func dirImageMockWithRef(t *testing.T, dir string, ref types.ImageReference) types.UnparsedImage { srcRef, err := directory.NewReference(dir) require.NoError(t, err) src, err := srcRef.NewImageSource(nil, nil) diff --git a/signature/policy_eval_simple.go b/signature/policy_eval_simple.go index 5d479f23bc..19a71e6d99 100644 --- a/signature/policy_eval_simple.go +++ b/signature/policy_eval_simple.go @@ -9,20 +9,20 @@ import ( "github.com/containers/image/types" ) -func (pr *prInsecureAcceptAnything) isSignatureAuthorAccepted(image types.Image, sig []byte) (signatureAcceptanceResult, *Signature, error) { +func (pr *prInsecureAcceptAnything) isSignatureAuthorAccepted(image types.UnparsedImage, sig []byte) (signatureAcceptanceResult, *Signature, error) { // prInsecureAcceptAnything semantics: Every image is allowed to run, // but this does not consider the signature as verified. return sarUnknown, nil, nil } -func (pr *prInsecureAcceptAnything) isRunningImageAllowed(image types.Image) (bool, error) { +func (pr *prInsecureAcceptAnything) isRunningImageAllowed(image types.UnparsedImage) (bool, error) { return true, nil } -func (pr *prReject) isSignatureAuthorAccepted(image types.Image, sig []byte) (signatureAcceptanceResult, *Signature, error) { +func (pr *prReject) isSignatureAuthorAccepted(image types.UnparsedImage, sig []byte) (signatureAcceptanceResult, *Signature, error) { return sarRejected, nil, PolicyRequirementError(fmt.Sprintf("Any signatures for image %s are rejected by policy.", transports.ImageName(image.Reference()))) } -func (pr *prReject) isRunningImageAllowed(image types.Image) (bool, error) { +func (pr *prReject) isRunningImageAllowed(image types.UnparsedImage) (bool, error) { return false, PolicyRequirementError(fmt.Sprintf("Running image %s is rejected by policy.", transports.ImageName(image.Reference()))) } diff --git a/signature/policy_eval_simple_test.go b/signature/policy_eval_simple_test.go index 633396ffe5..b3ef759f49 100644 --- a/signature/policy_eval_simple_test.go +++ b/signature/policy_eval_simple_test.go @@ -7,15 +7,11 @@ import ( "github.com/docker/docker/reference" ) -// nameOnlyImageMock is a mock of types.Image which only allows transports.ImageName to work +// nameOnlyImageMock is a mock of types.UnparsedImage which only allows transports.ImageName to work type nameOnlyImageMock struct { forbiddenImageMock } -func (nameOnlyImageMock) IsMultiImage() (bool, error) { - panic("unexpected call to a mock function") -} - func (nameOnlyImageMock) Reference() types.ImageReference { return nameOnlyImageReferenceMock("== StringWithinTransport mock") } diff --git a/signature/policy_eval_test.go b/signature/policy_eval_test.go index 8d6b211cea..15bb13df6e 100644 --- a/signature/policy_eval_test.go +++ b/signature/policy_eval_test.go @@ -171,9 +171,9 @@ func TestPolicyContextRequirementsForImageRef(t *testing.T) { } } -// pcImageMock returns a types.Image for a directory, claiming a specified dockerReference and implementing PolicyConfigurationIdentity/PolicyConfigurationNamespaces. +// pcImageMock returns a types.UnparsedImage for a directory, claiming a specified dockerReference and implementing PolicyConfigurationIdentity/PolicyConfigurationNamespaces. // The caller must call .Close() on the returned Image. -func pcImageMock(t *testing.T, dir, dockerReference string) types.Image { +func pcImageMock(t *testing.T, dir, dockerReference string) types.UnparsedImage { ref, err := reference.ParseNamed(dockerReference) require.NoError(t, err) return dirImageMockWithRef(t, dir, pcImageReferenceMock{"docker", ref}) diff --git a/signature/policy_reference_match.go b/signature/policy_reference_match.go index 2b2d410996..df8eacea4f 100644 --- a/signature/policy_reference_match.go +++ b/signature/policy_reference_match.go @@ -11,7 +11,7 @@ import ( ) // parseImageAndDockerReference converts an image and a reference string into two parsed entities, failing on any error and handling unidentified images. -func parseImageAndDockerReference(image types.Image, s2 string) (reference.Named, reference.Named, error) { +func parseImageAndDockerReference(image types.UnparsedImage, s2 string) (reference.Named, reference.Named, error) { r1 := image.Reference().DockerReference() if r1 == nil { return nil, nil, PolicyRequirementError(fmt.Sprintf("Docker reference match attempted on image %s with no known Docker reference identity", @@ -24,7 +24,7 @@ func parseImageAndDockerReference(image types.Image, s2 string) (reference.Named return r1, r2, nil } -func (prm *prmMatchExact) matchesDockerReference(image types.Image, signatureDockerReference string) bool { +func (prm *prmMatchExact) matchesDockerReference(image types.UnparsedImage, signatureDockerReference string) bool { intended, signature, err := parseImageAndDockerReference(image, signatureDockerReference) if err != nil { return false @@ -36,7 +36,7 @@ func (prm *prmMatchExact) matchesDockerReference(image types.Image, signatureDoc return signature.String() == intended.String() } -func (prm *prmMatchRepository) matchesDockerReference(image types.Image, signatureDockerReference string) bool { +func (prm *prmMatchRepository) matchesDockerReference(image types.UnparsedImage, signatureDockerReference string) bool { intended, signature, err := parseImageAndDockerReference(image, signatureDockerReference) if err != nil { return false @@ -57,7 +57,7 @@ func parseDockerReferences(s1, s2 string) (reference.Named, reference.Named, err return r1, r2, nil } -func (prm *prmExactReference) matchesDockerReference(image types.Image, signatureDockerReference string) bool { +func (prm *prmExactReference) matchesDockerReference(image types.UnparsedImage, signatureDockerReference string) bool { intended, signature, err := parseDockerReferences(prm.DockerReference, signatureDockerReference) if err != nil { return false @@ -69,7 +69,7 @@ func (prm *prmExactReference) matchesDockerReference(image types.Image, signatur return signature.String() == intended.String() } -func (prm *prmExactRepository) matchesDockerReference(image types.Image, signatureDockerReference string) bool { +func (prm *prmExactRepository) matchesDockerReference(image types.UnparsedImage, signatureDockerReference string) bool { intended, signature, err := parseDockerReferences(prm.DockerRepository, signatureDockerReference) if err != nil { return false diff --git a/signature/policy_reference_match_test.go b/signature/policy_reference_match_test.go index 52893c471a..a0967e713e 100644 --- a/signature/policy_reference_match_test.go +++ b/signature/policy_reference_match_test.go @@ -50,42 +50,21 @@ func TestParseImageAndDockerReference(t *testing.T) { } } -// refImageMock is a mock of types.Image which returns itself in Reference().DockerReference. +// refImageMock is a mock of types.UnparsedImage which returns itself in Reference().DockerReference. type refImageMock struct{ reference.Named } func (ref refImageMock) Reference() types.ImageReference { return refImageReferenceMock{ref.Named} } -func (ref refImageMock) IsMultiImage() (bool, error) { - panic("unexpected call to a mock function") -} func (ref refImageMock) Close() { panic("unexpected call to a mock function") } func (ref refImageMock) Manifest() ([]byte, string, error) { panic("unexpected call to a mock function") } -func (ref refImageMock) ManifestMatchesDigest(expectedDigest string) (bool, error) { - panic("unexpected call to a mock function") -} func (ref refImageMock) Signatures() ([][]byte, error) { panic("unexpected call to a mock function") } -func (ref refImageMock) ConfigInfo() (types.BlobInfo, error) { - panic("unexpected call to a mock function") -} -func (ref refImageMock) LayerInfos() ([]types.BlobInfo, error) { - panic("unexpected call to a mock function") -} -func (ref refImageMock) Inspect() (*types.ImageInspectInfo, error) { - panic("unexpected call to a mock function") -} -func (ref refImageMock) UpdatedManifest(options types.ManifestUpdateOptions) ([]byte, error) { - panic("unexpected call to a mock function") -} -func (ref refImageMock) GetRepositoryTags() ([]string, error) { - panic("unexpected call to a mock function") -} // refImageReferenceMock is a mock of types.ImageReference which returns itself in DockerReference. type refImageReferenceMock struct{ reference.Named } @@ -267,12 +246,9 @@ func TestParseDockerReferences(t *testing.T) { } } -// forbiddenImageMock is a mock of types.Image which ensures Reference is not called +// forbiddenImageMock is a mock of types.UnparsedImage which ensures Reference is not called type forbiddenImageMock struct{} -func (ref forbiddenImageMock) IsMultiImage() (bool, error) { - panic("unexpected call to a mock function") -} func (ref forbiddenImageMock) Reference() types.ImageReference { panic("unexpected call to a mock function") } @@ -282,27 +258,9 @@ func (ref forbiddenImageMock) Close() { func (ref forbiddenImageMock) Manifest() ([]byte, string, error) { panic("unexpected call to a mock function") } -func (ref forbiddenImageMock) ManifestMatchesDigest(expectedDigest string) (bool, error) { - panic("unexpected call to a mock function") -} func (ref forbiddenImageMock) Signatures() ([][]byte, error) { panic("unexpected call to a mock function") } -func (ref forbiddenImageMock) ConfigInfo() (types.BlobInfo, error) { - panic("unexpected call to a mock function") -} -func (ref forbiddenImageMock) LayerInfos() ([]types.BlobInfo, error) { - panic("unexpected call to a mock function") -} -func (ref forbiddenImageMock) Inspect() (*types.ImageInspectInfo, error) { - panic("unexpected call to a mock function") -} -func (ref forbiddenImageMock) UpdatedManifest(options types.ManifestUpdateOptions) ([]byte, error) { - panic("unexpected call to a mock function") -} -func (ref forbiddenImageMock) GetRepositoryTags() ([]string, error) { - panic("unexpected call to a mock function") -} func TestPRMExactReferenceMatchesDockerReference(t *testing.T) { for _, test := range prmExactMatchTestTable { diff --git a/types/types.go b/types/types.go index ab185a6f9e..b52799cc85 100644 --- a/types/types.go +++ b/types/types.go @@ -156,20 +156,28 @@ type ImageDestination interface { Commit() error } -// Image is the primary API for inspecting properties of images. -// Each Image should eventually be closed by calling Close(). -type Image interface { +// UnparsedImage is an Image-to-be; until it is verified and accepted, it only caries its identity and caches manifest and signature blobs. +// Thus, an UnparsedImage can be created from an ImageSource simply by fetching blobs without interpreting them, +// allowing cryptographic signature verification to happen first, before even fetching the manifest, or parsing anything else. +// This also makes the UnparsedImage→Image conversion an explicitly visible step. +// Each UnparsedImage should eventually be closed by calling Close(). +type UnparsedImage 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 - // Close removes resources associated with an initialized Image, if any. + // Close removes resources associated with an initialized UnparsedImage, if any. Close() - // ref to repository? // Manifest is like ImageSource.GetManifest, but the result is cached; it is OK to call this however often you need. - // NOTE: It is essential for signature verification that Manifest returns the manifest from which ConfigInfo and LayerInfos is computed. Manifest() ([]byte, string, error) // Signatures is like ImageSource.GetSignatures, but the result is cached; it is OK to call this however often you need. Signatures() ([][]byte, error) +} + +// Image is the primary API for inspecting properties of images. +// Each Image should eventually be closed by calling Close(). +type Image interface { + // NOTE: It is essential for signature verification that Manifest returns the manifest from which ConfigInfo and LayerInfos is computed. + UnparsedImage // ConfigInfo returns a complete BlobInfo for the separate config object, or a BlobInfo{Digest:""} if there isn't a separate object. // NOTE: It is essential for signature verification that ConfigInfo is computed from the same manifest which is returned by Manifest(). ConfigInfo() (BlobInfo, error) From ad294f1c4b7fb0f3991663e7a1704d5fff754128 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Thu, 6 Oct 2016 22:06:03 +0200 Subject: [PATCH 03/11] Split image.UnparsedImage from image.genericImage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Now that types.UnparsedImage is split from types.Image, also split the implementations. In particular, copy.Image uses an UnparsedImage for signature verification. This structural separation allows us to remove the “It is essential for signature verification”… comments all over the place in favor of a single one in the choke point where an UnparsedImage turns into a full genericImage. Also, split the manifest type guessing (which involves parsing) so that it does not happen in UnparsedImage. This needs ugly fields like trueManifestMIMETypeSet, which will go away momentarily. Signed-off-by: Miloslav Trmač --- copy/copy.go | 13 +++- directory/directory_transport.go | 4 +- docker/docker_transport.go | 4 +- image/image.go | 93 ++++++++++++-------------- image/unparsed.go | 60 +++++++++++++++++ oci/layout/oci_transport.go | 4 +- openshift/openshift_transport.go | 4 +- signature/policy_eval_signedby_test.go | 2 +- types/types.go | 7 +- 9 files changed, 129 insertions(+), 62 deletions(-) create mode 100644 image/unparsed.go diff --git a/copy/copy.go b/copy/copy.go index 09c463d97d..f990b81911 100644 --- a/copy/copy.go +++ b/copy/copy.go @@ -109,13 +109,20 @@ func Image(ctx *types.SystemContext, policyContext *signature.PolicyContext, des if err != nil { return fmt.Errorf("Error initializing source %s: %v", transports.ImageName(srcRef), err) } - src := image.FromSource(rawSource) - defer src.Close() + unparsedImage := image.UnparsedFromSource(rawSource) + defer func() { + if unparsedImage != nil { + unparsedImage.Close() + } + }() // Please keep this policy check BEFORE reading any other information about the image. - if allowed, err := policyContext.IsRunningImageAllowed(src); !allowed || err != nil { // Be paranoid and fail if either return value indicates so. + if allowed, err := policyContext.IsRunningImageAllowed(unparsedImage); !allowed || err != nil { // Be paranoid and fail if either return value indicates so. return fmt.Errorf("Source image rejected: %v", err) } + src := image.FromUnparsedImage(unparsedImage) + unparsedImage = nil + defer src.Close() multiImage, err := src.IsMultiImage() if err != nil { diff --git a/directory/directory_transport.go b/directory/directory_transport.go index 4979572de6..71ab47f0cd 100644 --- a/directory/directory_transport.go +++ b/directory/directory_transport.go @@ -127,8 +127,10 @@ func (ref dirReference) PolicyConfigurationNamespaces() []string { return res } -// NewImage returns a types.Image for this reference. +// NewImage returns a types.Image for this reference, possibly specialized for this ImageTransport. // The caller must call .Close() on the returned Image. +// NOTE: If any kind of signature verification should happen, build an UnparsedImage from the value returned by NewImageSource, +// verify that UnparsedImage, and convert it into a real Image via image.FromUnparsedImage. func (ref dirReference) NewImage(ctx *types.SystemContext) (types.Image, error) { src := newImageSource(ref) return image.FromSource(src), nil diff --git a/docker/docker_transport.go b/docker/docker_transport.go index 5ab5efe3ef..52c4ab73cf 100644 --- a/docker/docker_transport.go +++ b/docker/docker_transport.go @@ -115,8 +115,10 @@ func (ref dockerReference) PolicyConfigurationNamespaces() []string { return policyconfiguration.DockerReferenceNamespaces(ref.ref) } -// NewImage returns a types.Image for this reference. +// NewImage returns a types.Image for this reference, possibly specialized for this ImageTransport. // The caller must call .Close() on the returned Image. +// NOTE: If any kind of signature verification should happen, build an UnparsedImage from the value returned by NewImageSource, +// verify that UnparsedImage, and convert it into a real Image via image.FromUnparsedImage. func (ref dockerReference) NewImage(ctx *types.SystemContext) (types.Image, error) { return newImage(ctx, ref) } diff --git a/image/image.go b/image/image.go index 23c2a0a2ba..1856f1adc2 100644 --- a/image/image.go +++ b/image/image.go @@ -12,23 +12,6 @@ import ( "github.com/containers/image/types" ) -// genericImage is a general set of utilities for working with container images, -// whatever is their underlying location (i.e. dockerImageSource-independent). -// Note the existence of skopeo/docker.Image: some instances of a `types.Image` -// may not be a `genericImage` directly. However, most users of `types.Image` -// do not care, and those who care about `skopeo/docker.Image` know they do. -type genericImage struct { - src types.ImageSource - // private cache for Manifest(); nil if not yet known. - cachedManifest []byte - // private cache for the manifest media type w/o having to guess it - // this may be the empty string in case the MIME Type wasn't guessed correctly - // this field is valid only if cachedManifest is not nil - cachedManifestMIMEType string - // private cache for Signatures(); nil if not yet known. - cachedSignatures [][]byte -} - // FromSource returns a types.Image implementation for source. // The caller must call .Close() on the returned Image. // @@ -36,30 +19,54 @@ type genericImage struct { // when the image is closed. (This does not prevent callers from using both the // Image and ImageSource objects simultaneously, but it means that they only need to // the Image.) +// +// NOTE: If any kind of signature verification should happen, build an UnparsedImage from the value returned by NewImageSource, +// verify that UnparsedImage, and convert it into a real Image via image.FromUnparsedImage instead of calling this function. func FromSource(src types.ImageSource) types.Image { - return &genericImage{src: src} + return FromUnparsedImage(UnparsedFromSource(src)) } -// 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. -func (i *genericImage) Reference() types.ImageReference { - return i.src.Reference() +// genericImage is a general set of utilities for working with container images, +// whatever is their underlying location (i.e. dockerImageSource-independent). +// Note the existence of skopeo/docker.Image: some instances of a `types.Image` +// may not be a `genericImage` directly. However, most users of `types.Image` +// do not care, and those who care about `skopeo/docker.Image` know they do. +type genericImage struct { + *UnparsedImage + trueManifestMIMETypeSet bool // A private cache for Manifest + trueManifestMIMEType string // A private cache for Manifest, valid only if trueManifestMIMETypeSet } -// Close removes resources associated with an initialized Image, if any. -func (i *genericImage) Close() { - i.src.Close() +// FromUnparsedImage returns a types.Image implementation for unparsed. +// The caller must call .Close() on the returned Image. +// +// FromSource “takes ownership” of the input UnparsedImage and will call uparsed.Close() +// when the image is closed. (This does not prevent callers from using both the +// UnparsedImage and ImageSource objects simultaneously, but it means that they only need to +// keep a reference to the Image.) +func FromUnparsedImage(unparsed *UnparsedImage) types.Image { + // Note that the input parameter above is specifically *image.UnparsedImage, not types.UnparsedImage: + // we want to be able to use unparsed.src. We could make that an explicit interface, but, well, + // this is the only UnparsedImage implementation around, anyway. + + // Also, we do not explicitly implement types.Image.Close; we let the implementation fall through to + // unparsed.Close. + + // NOTE: It is essential for signature verification that all parsing done in this object happens on the same manifest which is returned by unparsed.Manifest(). + return &genericImage{ + UnparsedImage: unparsed, + trueManifestMIMETypeSet: false, + trueManifestMIMEType: "", + } } -// Manifest is like ImageSource.GetManifest, but the result is cached; it is OK to call this however often you need. -// NOTE: It is essential for signature verification that Manifest returns the manifest from which ConfigInfo and LayerInfos is computed. +// Manifest overrides the UnparsedImage.Manifest to add guessing and overrides, which we don't want to do before signature verification. func (i *genericImage) Manifest() ([]byte, string, error) { - if i.cachedManifest == nil { - m, mt, err := i.src.GetManifest() - if err != nil { - return nil, "", err - } - i.cachedManifest = m + m, mt, err := i.UnparsedImage.Manifest() + if err != nil { + return nil, "", err + } + if !i.trueManifestMIMETypeSet { if mt == "" || mt == "text/plain" { // Crane registries can return "text/plain". // This makes no real sense, but it happens @@ -68,21 +75,10 @@ func (i *genericImage) Manifest() ([]byte, string, error) { // network which is configured that way. mt = manifest.GuessMIMEType(i.cachedManifest) } - i.cachedManifestMIMEType = mt - } - return i.cachedManifest, i.cachedManifestMIMEType, nil -} - -// Signatures is like ImageSource.GetSignatures, but the result is cached; it is OK to call this however often you need. -func (i *genericImage) Signatures() ([][]byte, error) { - if i.cachedSignatures == nil { - sigs, err := i.src.GetSignatures() - if err != nil { - return nil, err - } - i.cachedSignatures = sigs + i.trueManifestMIMEType = mt + i.trueManifestMIMETypeSet = true } - return i.cachedSignatures, nil + return m, mt, nil } type config struct { @@ -114,7 +110,6 @@ type genericManifest interface { // getParsedManifest parses the manifest into a data structure, cleans it up, and returns it. // NOTE: The manifest may have been modified in the process; DO NOT reserialize and store the return value // if you want to preserve the original manifest; use the blob returned by Manifest() directly. -// NOTE: It is essential for signature verification that the object is computed from the same manifest which is returned by Manifest(). func (i *genericImage) getParsedManifest() (genericManifest, error) { manblob, mt, err := i.Manifest() if err != nil { @@ -168,7 +163,6 @@ func (i *genericImage) Inspect() (*types.ImageInspectInfo, error) { } // ConfigInfo returns a complete BlobInfo for the separate config object, or a BlobInfo{Digest:""} if there isn't a separate object. -// NOTE: It is essential for signature verification that ConfigInfo is computed from the same manifest which is returned by Manifest(). func (i *genericImage) ConfigInfo() (types.BlobInfo, error) { m, err := i.getParsedManifest() if err != nil { @@ -179,7 +173,6 @@ func (i *genericImage) ConfigInfo() (types.BlobInfo, error) { // LayerInfos returns a list of BlobInfos of layers referenced by this image, in order (the root layer first, and then successive layered layers). // The Digest field is guaranteed to be provided; Size may be -1. -// NOTE: It is essential for signature verification that LayerInfos is computed from the same manifest which is returned by Manifest(). // WARNING: The list may contain duplicates, and they are semantically relevant. func (i *genericImage) LayerInfos() ([]types.BlobInfo, error) { m, err := i.getParsedManifest() diff --git a/image/unparsed.go b/image/unparsed.go new file mode 100644 index 0000000000..2682a5d851 --- /dev/null +++ b/image/unparsed.go @@ -0,0 +1,60 @@ +package image + +import "github.com/containers/image/types" + +// UnparsedImage implements types.UnparsedImage . +type UnparsedImage struct { + src types.ImageSource + cachedManifest []byte // A private cache for Manifest(); nil if not yet known. + // A private cache for Manifest(), may be the empty string if guessing failed. + // Valid iff cachedManifest is not nil. + cachedManifestMIMEType string + cachedSignatures [][]byte // A private cache for Signatures(); nil if not yet known. +} + +// UnparsedFromSource returns a types.UnparsedImage implementation for source. +// The caller must call .Close() on the returned UnparsedImage. +// +// UnparsedFromSource “takes ownership” of the input ImageSource and will call src.Close() +// when the image is closed. (This does not prevent callers from using both the +// UnparsedImage and ImageSource objects simultaneously, but it means that they only need to +// keep a reference to the UnparsedImage.) +func UnparsedFromSource(src types.ImageSource) *UnparsedImage { + return &UnparsedImage{src: src} +} + +// 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. +func (i *UnparsedImage) Reference() types.ImageReference { + return i.src.Reference() +} + +// Close removes resources associated with an initialized UnparsedImage, if any. +func (i *UnparsedImage) Close() { + i.src.Close() +} + +// Manifest is like ImageSource.GetManifest, but the result is cached; it is OK to call this however often you need. +func (i *UnparsedImage) Manifest() ([]byte, string, error) { + if i.cachedManifest == nil { + m, mt, err := i.src.GetManifest() + if err != nil { + return nil, "", err + } + i.cachedManifest = m + i.cachedManifestMIMEType = mt + } + return i.cachedManifest, i.cachedManifestMIMEType, nil +} + +// Signatures is like ImageSource.GetSignatures, but the result is cached; it is OK to call this however often you need. +func (i *UnparsedImage) Signatures() ([][]byte, error) { + if i.cachedSignatures == nil { + sigs, err := i.src.GetSignatures() + if err != nil { + return nil, err + } + i.cachedSignatures = sigs + } + return i.cachedSignatures, nil +} diff --git a/oci/layout/oci_transport.go b/oci/layout/oci_transport.go index eee79957f7..fcd4c53145 100644 --- a/oci/layout/oci_transport.go +++ b/oci/layout/oci_transport.go @@ -164,8 +164,10 @@ func (ref ociReference) PolicyConfigurationNamespaces() []string { return res } -// NewImage returns a types.Image for this reference. +// NewImage returns a types.Image for this reference, possibly specialized for this ImageTransport. // The caller must call .Close() on the returned Image. +// NOTE: If any kind of signature verification should happen, build an UnparsedImage from the value returned by NewImageSource, +// verify that UnparsedImage, and convert it into a real Image via image.FromUnparsedImage. func (ref ociReference) NewImage(ctx *types.SystemContext) (types.Image, error) { return nil, errors.New("Full Image support not implemented for oci: image names") } diff --git a/openshift/openshift_transport.go b/openshift/openshift_transport.go index ae30d24c99..550f998f22 100644 --- a/openshift/openshift_transport.go +++ b/openshift/openshift_transport.go @@ -118,8 +118,10 @@ func (ref openshiftReference) PolicyConfigurationNamespaces() []string { return policyconfiguration.DockerReferenceNamespaces(ref.dockerReference) } -// NewImage returns a types.Image for this reference. +// NewImage returns a types.Image for this reference, possibly specialized for this ImageTransport. // The caller must call .Close() on the returned Image. +// NOTE: If any kind of signature verification should happen, build an UnparsedImage from the value returned by NewImageSource, +// verify that UnparsedImage, and convert it into a real Image via image.FromUnparsedImage. func (ref openshiftReference) NewImage(ctx *types.SystemContext) (types.Image, error) { src, err := newImageSource(ctx, ref, nil) if err != nil { diff --git a/signature/policy_eval_signedby_test.go b/signature/policy_eval_signedby_test.go index 22f6c82138..cc9af6f987 100644 --- a/signature/policy_eval_signedby_test.go +++ b/signature/policy_eval_signedby_test.go @@ -29,7 +29,7 @@ func dirImageMockWithRef(t *testing.T, dir string, ref types.ImageReference) typ require.NoError(t, err) src, err := srcRef.NewImageSource(nil, nil) require.NoError(t, err) - return image.FromSource(&dirImageSourceMock{ + return image.UnparsedFromSource(&dirImageSourceMock{ ImageSource: src, ref: ref, }) diff --git a/types/types.go b/types/types.go index b52799cc85..28352770c2 100644 --- a/types/types.go +++ b/types/types.go @@ -70,8 +70,10 @@ type ImageReference interface { // and each following element to be a prefix of the element preceding it. PolicyConfigurationNamespaces() []string - // NewImage returns a types.Image for this reference. + // NewImage returns a types.Image for this reference, possibly specialized for this ImageTransport. // The caller must call .Close() on the returned Image. + // NOTE: If any kind of signature verification should happen, build an UnparsedImage from the value returned by NewImageSource, + // verify that UnparsedImage, and convert it into a real Image via image.FromUnparsedImage. NewImage(ctx *SystemContext) (Image, error) // NewImageSource returns a types.ImageSource for this reference, // asking the backend to use a manifest from requestedManifestMIMETypes if possible. @@ -176,14 +178,11 @@ type UnparsedImage interface { // Image is the primary API for inspecting properties of images. // Each Image should eventually be closed by calling Close(). type Image interface { - // NOTE: It is essential for signature verification that Manifest returns the manifest from which ConfigInfo and LayerInfos is computed. UnparsedImage // ConfigInfo returns a complete BlobInfo for the separate config object, or a BlobInfo{Digest:""} if there isn't a separate object. - // NOTE: It is essential for signature verification that ConfigInfo is computed from the same manifest which is returned by Manifest(). ConfigInfo() (BlobInfo, error) // LayerInfos returns a list of BlobInfos of layers referenced by this image, in order (the root layer first, and then successive layered layers). // The Digest field is guaranteed to be provided; Size may be -1. - // NOTE: It is essential for signature verification that LayerInfos is computed from the same manifest which is returned by Manifest(). // WARNING: The list may contain duplicates, and they are semantically relevant. LayerInfos() ([]BlobInfo, error) // Inspect returns various information for (skopeo inspect) parsed from the manifest and configuration. From 5d9d834a53dac3828026a1ea1ca9793ea57c01ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Thu, 6 Oct 2016 23:16:50 +0200 Subject: [PATCH 04/11] Move genericManifest-related code to image/manifest.go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit image/manifest.go now contains genericManifest without caring about genericImage, and image/image.go contains genericImage only. Signed-off-by: Miloslav Trmač --- image/image.go | 48 ----------------------------------------- image/manifest.go | 54 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 48 deletions(-) create mode 100644 image/manifest.go diff --git a/image/image.go b/image/image.go index 1856f1adc2..820023af21 100644 --- a/image/image.go +++ b/image/image.go @@ -4,10 +4,6 @@ package image import ( - "errors" - "fmt" - "time" - "github.com/containers/image/manifest" "github.com/containers/image/types" ) @@ -81,32 +77,6 @@ func (i *genericImage) Manifest() ([]byte, string, error) { return m, mt, nil } -type config struct { - Labels map[string]string -} - -type v1Image struct { - // Config is the configuration of the container received from the client - Config *config `json:"config,omitempty"` - // DockerVersion specifies version on which image is built - DockerVersion string `json:"docker_version,omitempty"` - // Created timestamp when image was created - Created time.Time `json:"created"` - // Architecture is the hardware that the image is build and runs on - Architecture string `json:"architecture,omitempty"` - // OS is the operating system used to build and run the image - OS string `json:"os,omitempty"` -} - -// will support v1 one day... -type genericManifest interface { - Config() ([]byte, error) - ConfigInfo() types.BlobInfo - LayerInfos() []types.BlobInfo - ImageInspectInfo() (*types.ImageInspectInfo, error) // The caller will need to fill in Layers - UpdatedManifest(types.ManifestUpdateOptions) ([]byte, error) -} - // getParsedManifest parses the manifest into a data structure, cleans it up, and returns it. // NOTE: The manifest may have been modified in the process; DO NOT reserialize and store the return value // if you want to preserve the original manifest; use the blob returned by Manifest() directly. @@ -126,24 +96,6 @@ func (i *genericImage) IsMultiImage() (bool, error) { return mt == manifest.DockerV2ListMediaType, nil } -func manifestInstanceFromBlob(src types.ImageSource, manblob []byte, mt string) (genericManifest, error) { - switch mt { - // "application/json" is a valid v2s1 value per https://github.com/docker/distribution/blob/master/docs/spec/manifest-v2-1.md . - // This works for now, when nothing else seems to return "application/json"; if that were not true, the mapping/detection might - // need to happen within the ImageSource. - case manifest.DockerV2Schema1MediaType, manifest.DockerV2Schema1SignedMediaType, "application/json": - return manifestSchema1FromManifest(manblob) - case manifest.DockerV2Schema2MediaType: - return manifestSchema2FromManifest(src, manblob) - case manifest.DockerV2ListMediaType: - return manifestSchema2FromManifestList(src, manblob) - case "": - return nil, errors.New("could not guess manifest media type") - default: - return nil, fmt.Errorf("unsupported manifest media type %s", mt) - } -} - func (i *genericImage) Inspect() (*types.ImageInspectInfo, error) { // TODO(runcom): unused version param for now, default to docker v2-1 m, err := i.getParsedManifest() diff --git a/image/manifest.go b/image/manifest.go new file mode 100644 index 0000000000..696314cbe1 --- /dev/null +++ b/image/manifest.go @@ -0,0 +1,54 @@ +package image + +import ( + "errors" + "fmt" + "time" + + "github.com/containers/image/manifest" + "github.com/containers/image/types" +) + +type config struct { + Labels map[string]string +} + +type v1Image struct { + // Config is the configuration of the container received from the client + Config *config `json:"config,omitempty"` + // DockerVersion specifies version on which image is built + DockerVersion string `json:"docker_version,omitempty"` + // Created timestamp when image was created + Created time.Time `json:"created"` + // Architecture is the hardware that the image is build and runs on + Architecture string `json:"architecture,omitempty"` + // OS is the operating system used to build and run the image + OS string `json:"os,omitempty"` +} + +// will support v1 one day... +type genericManifest interface { + Config() ([]byte, error) + ConfigInfo() types.BlobInfo + LayerInfos() []types.BlobInfo + ImageInspectInfo() (*types.ImageInspectInfo, error) // The caller will need to fill in Layers + UpdatedManifest(types.ManifestUpdateOptions) ([]byte, error) +} + +func manifestInstanceFromBlob(src types.ImageSource, manblob []byte, mt string) (genericManifest, error) { + switch mt { + // "application/json" is a valid v2s1 value per https://github.com/docker/distribution/blob/master/docs/spec/manifest-v2-1.md . + // This works for now, when nothing else seems to return "application/json"; if that were not true, the mapping/detection might + // need to happen within the ImageSource. + case manifest.DockerV2Schema1MediaType, manifest.DockerV2Schema1SignedMediaType, "application/json": + return manifestSchema1FromManifest(manblob) + case manifest.DockerV2Schema2MediaType: + return manifestSchema2FromManifest(src, manblob) + case manifest.DockerV2ListMediaType: + return manifestSchema2FromManifestList(src, manblob) + case "": + return nil, errors.New("could not guess manifest media type") + default: + return nil, fmt.Errorf("unsupported manifest media type %s", mt) + } +} From 115856315dddd7d7304a364ee861014f9f3e19e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Thu, 6 Oct 2016 23:19:08 +0200 Subject: [PATCH 05/11] Move IsMultiImage implementation to match the interface order MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Just a bit of pointless pedantry, sorry :) Signed-off-by: Miloslav Trmač --- image/image.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/image/image.go b/image/image.go index 820023af21..66f27961f2 100644 --- a/image/image.go +++ b/image/image.go @@ -88,14 +88,6 @@ func (i *genericImage) getParsedManifest() (genericManifest, error) { return manifestInstanceFromBlob(i.src, manblob, mt) } -func (i *genericImage) IsMultiImage() (bool, error) { - _, mt, err := i.Manifest() - if err != nil { - return false, err - } - return mt == manifest.DockerV2ListMediaType, nil -} - func (i *genericImage) Inspect() (*types.ImageInspectInfo, error) { // TODO(runcom): unused version param for now, default to docker v2-1 m, err := i.getParsedManifest() @@ -143,3 +135,11 @@ func (i *genericImage) UpdatedManifest(options types.ManifestUpdateOptions) ([]b } return m.UpdatedManifest(options) } + +func (i *genericImage) IsMultiImage() (bool, error) { + _, mt, err := i.Manifest() + if err != nil { + return false, err + } + return mt == manifest.DockerV2ListMediaType, nil +} From 41f9cdc9378f25079be27f4c9e55b884d904a7aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Thu, 6 Oct 2016 22:49:16 +0200 Subject: [PATCH 06/11] Fetch a manifest already in image.FromUnparsedImage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This means that image.FromUnparsedImage can now fail, OTOH later methods which refer to a manifest cannot. This is an intermediate step to also parsing the manifest at that time already. Signed-off-by: Miloslav Trmač --- copy/copy.go | 11 +++-- directory/directory_transport.go | 2 +- directory/directory_transport_test.go | 9 ++++ docker/docker_image.go | 6 ++- image/image.go | 63 +++++++++++---------------- openshift/openshift_transport.go | 2 +- types/types.go | 2 +- 7 files changed, 48 insertions(+), 47 deletions(-) diff --git a/copy/copy.go b/copy/copy.go index f990b81911..e264905a76 100644 --- a/copy/copy.go +++ b/copy/copy.go @@ -120,15 +120,14 @@ func Image(ctx *types.SystemContext, policyContext *signature.PolicyContext, des if allowed, err := policyContext.IsRunningImageAllowed(unparsedImage); !allowed || err != nil { // Be paranoid and fail if either return value indicates so. return fmt.Errorf("Source image rejected: %v", err) } - src := image.FromUnparsedImage(unparsedImage) + src, err := image.FromUnparsedImage(unparsedImage) + if err != nil { + return fmt.Errorf("Error initializing image from source %s: %v", transports.ImageName(srcRef), err) + } unparsedImage = nil defer src.Close() - multiImage, err := src.IsMultiImage() - if err != nil { - return err - } - if multiImage { + if src.IsMultiImage() { return fmt.Errorf("can not copy %s: manifest contains multiple images", transports.ImageName(srcRef)) } diff --git a/directory/directory_transport.go b/directory/directory_transport.go index 71ab47f0cd..5703127f19 100644 --- a/directory/directory_transport.go +++ b/directory/directory_transport.go @@ -133,7 +133,7 @@ func (ref dirReference) PolicyConfigurationNamespaces() []string { // verify that UnparsedImage, and convert it into a real Image via image.FromUnparsedImage. func (ref dirReference) NewImage(ctx *types.SystemContext) (types.Image, error) { src := newImageSource(ref) - return image.FromSource(src), nil + return image.FromSource(src) } // NewImageSource returns a types.ImageSource for this reference, diff --git a/directory/directory_transport_test.go b/directory/directory_transport_test.go index 6dce1b03e0..534fa503c1 100644 --- a/directory/directory_transport_test.go +++ b/directory/directory_transport_test.go @@ -149,6 +149,15 @@ func TestReferencePolicyConfigurationNamespaces(t *testing.T) { func TestReferenceNewImage(t *testing.T) { ref, tmpDir := refToTempDir(t) defer os.RemoveAll(tmpDir) + + dest, err := ref.NewImageDestination(nil) + require.NoError(t, err) + defer dest.Close() + err = dest.PutManifest([]byte(`{"schemaVersion":2}`)) + assert.NoError(t, err) + err = dest.Commit() + assert.NoError(t, err) + img, err := ref.NewImage(nil) assert.NoError(t, err) defer img.Close() diff --git a/docker/docker_image.go b/docker/docker_image.go index b02a8cd9a6..5946875563 100644 --- a/docker/docker_image.go +++ b/docker/docker_image.go @@ -24,7 +24,11 @@ func newImage(ctx *types.SystemContext, ref dockerReference) (types.Image, error if err != nil { return nil, err } - return &Image{Image: image.FromSource(s), src: s}, nil + img, err := image.FromSource(s) + if err != nil { + return nil, err + } + return &Image{Image: img, src: s}, nil } // SourceRefFullName returns a fully expanded name for the repository this image is in. diff --git a/image/image.go b/image/image.go index 66f27961f2..35c5f1b52f 100644 --- a/image/image.go +++ b/image/image.go @@ -18,7 +18,7 @@ import ( // // NOTE: If any kind of signature verification should happen, build an UnparsedImage from the value returned by NewImageSource, // verify that UnparsedImage, and convert it into a real Image via image.FromUnparsedImage instead of calling this function. -func FromSource(src types.ImageSource) types.Image { +func FromSource(src types.ImageSource) (types.Image, error) { return FromUnparsedImage(UnparsedFromSource(src)) } @@ -29,8 +29,8 @@ func FromSource(src types.ImageSource) types.Image { // do not care, and those who care about `skopeo/docker.Image` know they do. type genericImage struct { *UnparsedImage - trueManifestMIMETypeSet bool // A private cache for Manifest - trueManifestMIMEType string // A private cache for Manifest, valid only if trueManifestMIMETypeSet + manifestBlob []byte + manifestMIMEType string } // FromUnparsedImage returns a types.Image implementation for unparsed. @@ -40,7 +40,7 @@ type genericImage struct { // when the image is closed. (This does not prevent callers from using both the // UnparsedImage and ImageSource objects simultaneously, but it means that they only need to // keep a reference to the Image.) -func FromUnparsedImage(unparsed *UnparsedImage) types.Image { +func FromUnparsedImage(unparsed *UnparsedImage) (types.Image, error) { // Note that the input parameter above is specifically *image.UnparsedImage, not types.UnparsedImage: // we want to be able to use unparsed.src. We could make that an explicit interface, but, well, // this is the only UnparsedImage implementation around, anyway. @@ -49,43 +49,36 @@ func FromUnparsedImage(unparsed *UnparsedImage) types.Image { // unparsed.Close. // NOTE: It is essential for signature verification that all parsing done in this object happens on the same manifest which is returned by unparsed.Manifest(). - return &genericImage{ - UnparsedImage: unparsed, - trueManifestMIMETypeSet: false, - trueManifestMIMEType: "", + manifestBlob, manifestMIMEType, err := unparsed.Manifest() + if err != nil { + return nil, err + } + if manifestMIMEType == "" || manifestMIMEType == "text/plain" { + // Crane registries can return "text/plain". + // This makes no real sense, but it happens + // because requests for manifests are + // redirected to a content distribution + // network which is configured that way. + manifestMIMEType = manifest.GuessMIMEType(manifestBlob) } + + return &genericImage{ + UnparsedImage: unparsed, + manifestBlob: manifestBlob, + manifestMIMEType: manifestMIMEType, + }, nil } -// Manifest overrides the UnparsedImage.Manifest to add guessing and overrides, which we don't want to do before signature verification. +// Manifest overrides the UnparsedImage.Manifest to use the fields which we have already fetched, after guessing and overrides. func (i *genericImage) Manifest() ([]byte, string, error) { - m, mt, err := i.UnparsedImage.Manifest() - if err != nil { - return nil, "", err - } - if !i.trueManifestMIMETypeSet { - if mt == "" || mt == "text/plain" { - // Crane registries can return "text/plain". - // This makes no real sense, but it happens - // because requests for manifests are - // redirected to a content distribution - // network which is configured that way. - mt = manifest.GuessMIMEType(i.cachedManifest) - } - i.trueManifestMIMEType = mt - i.trueManifestMIMETypeSet = true - } - return m, mt, nil + return i.manifestBlob, i.manifestMIMEType, nil } // getParsedManifest parses the manifest into a data structure, cleans it up, and returns it. // NOTE: The manifest may have been modified in the process; DO NOT reserialize and store the return value // if you want to preserve the original manifest; use the blob returned by Manifest() directly. func (i *genericImage) getParsedManifest() (genericManifest, error) { - manblob, mt, err := i.Manifest() - if err != nil { - return nil, err - } - return manifestInstanceFromBlob(i.src, manblob, mt) + return manifestInstanceFromBlob(i.src, i.manifestBlob, i.manifestMIMEType) } func (i *genericImage) Inspect() (*types.ImageInspectInfo, error) { @@ -136,10 +129,6 @@ func (i *genericImage) UpdatedManifest(options types.ManifestUpdateOptions) ([]b return m.UpdatedManifest(options) } -func (i *genericImage) IsMultiImage() (bool, error) { - _, mt, err := i.Manifest() - if err != nil { - return false, err - } - return mt == manifest.DockerV2ListMediaType, nil +func (i *genericImage) IsMultiImage() bool { + return i.manifestMIMEType == manifest.DockerV2ListMediaType } diff --git a/openshift/openshift_transport.go b/openshift/openshift_transport.go index 550f998f22..213f1f72be 100644 --- a/openshift/openshift_transport.go +++ b/openshift/openshift_transport.go @@ -127,7 +127,7 @@ func (ref openshiftReference) NewImage(ctx *types.SystemContext) (types.Image, e if err != nil { return nil, err } - return genericImage.FromSource(src), nil + return genericImage.FromSource(src) } // NewImageSource returns a types.ImageSource for this reference, diff --git a/types/types.go b/types/types.go index 28352770c2..9f09231158 100644 --- a/types/types.go +++ b/types/types.go @@ -191,7 +191,7 @@ type Image interface { // This does not change the state of the Image object. UpdatedManifest(options ManifestUpdateOptions) ([]byte, error) // IsMultiImage returns true if the image's manifest is a list of images, false otherwise. - IsMultiImage() (bool, error) + IsMultiImage() bool } // ManifestUpdateOptions is a way to pass named optional arguments to Image.UpdatedManifest From 9602e629c5adce4ec2e957914c246dfae54912aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Thu, 6 Oct 2016 23:10:35 +0200 Subject: [PATCH 07/11] Parse a manifest into getParsedManifest already in image.FromUnparsedImage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Another step towards simplifying the types.Image/genericManifest relationship: Now every genericImage always has a genericManifest. Signed-off-by: Miloslav Trmač --- image/image.go | 44 +++++++++++++++----------------------------- 1 file changed, 15 insertions(+), 29 deletions(-) diff --git a/image/image.go b/image/image.go index 35c5f1b52f..a56ae678ce 100644 --- a/image/image.go +++ b/image/image.go @@ -31,6 +31,10 @@ type genericImage struct { *UnparsedImage manifestBlob []byte manifestMIMEType string + // parsedManifest contains data corresponding to manifestBlob. + // NOTE: The manifest may have been modified in the process; DO NOT reserialize and store parsedManifest + // if you want to preserve the original manifest; use manifestBlob directly. + parsedManifest genericManifest } // FromUnparsedImage returns a types.Image implementation for unparsed. @@ -62,10 +66,16 @@ func FromUnparsedImage(unparsed *UnparsedImage) (types.Image, error) { manifestMIMEType = manifest.GuessMIMEType(manifestBlob) } + parsedManifest, err := manifestInstanceFromBlob(unparsed.src, manifestBlob, manifestMIMEType) + if err != nil { + return nil, err + } + return &genericImage{ UnparsedImage: unparsed, manifestBlob: manifestBlob, manifestMIMEType: manifestMIMEType, + parsedManifest: parsedManifest, }, nil } @@ -74,24 +84,12 @@ func (i *genericImage) Manifest() ([]byte, string, error) { return i.manifestBlob, i.manifestMIMEType, nil } -// getParsedManifest parses the manifest into a data structure, cleans it up, and returns it. -// NOTE: The manifest may have been modified in the process; DO NOT reserialize and store the return value -// if you want to preserve the original manifest; use the blob returned by Manifest() directly. -func (i *genericImage) getParsedManifest() (genericManifest, error) { - return manifestInstanceFromBlob(i.src, i.manifestBlob, i.manifestMIMEType) -} - func (i *genericImage) Inspect() (*types.ImageInspectInfo, error) { - // TODO(runcom): unused version param for now, default to docker v2-1 - m, err := i.getParsedManifest() - if err != nil { - return nil, err - } - info, err := m.ImageInspectInfo() + info, err := i.parsedManifest.ImageInspectInfo() if err != nil { return nil, err } - layers := m.LayerInfos() + layers := i.parsedManifest.LayerInfos() info.Layers = make([]string, len(layers)) for i, layer := range layers { info.Layers[i] = layer.Digest @@ -101,32 +99,20 @@ func (i *genericImage) Inspect() (*types.ImageInspectInfo, error) { // ConfigInfo returns a complete BlobInfo for the separate config object, or a BlobInfo{Digest:""} if there isn't a separate object. func (i *genericImage) ConfigInfo() (types.BlobInfo, error) { - m, err := i.getParsedManifest() - if err != nil { - return types.BlobInfo{}, err - } - return m.ConfigInfo(), nil + return i.parsedManifest.ConfigInfo(), nil } // LayerInfos returns a list of BlobInfos of layers referenced by this image, in order (the root layer first, and then successive layered layers). // The Digest field is guaranteed to be provided; Size may be -1. // WARNING: The list may contain duplicates, and they are semantically relevant. func (i *genericImage) LayerInfos() ([]types.BlobInfo, error) { - m, err := i.getParsedManifest() - if err != nil { - return nil, err - } - return m.LayerInfos(), nil + return i.parsedManifest.LayerInfos(), nil } // UpdatedManifest returns the image's manifest modified according to updateOptions. // This does not change the state of the Image object. func (i *genericImage) UpdatedManifest(options types.ManifestUpdateOptions) ([]byte, error) { - m, err := i.getParsedManifest() - if err != nil { - return nil, err - } - return m.UpdatedManifest(options) + return i.parsedManifest.UpdatedManifest(options) } func (i *genericImage) IsMultiImage() bool { From 5abb8c13b7ce73cf42e73a0a2ab8c7bee30e5b83 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Thu, 6 Oct 2016 23:14:44 +0200 Subject: [PATCH 08/11] Embed genericManifest into genericImage instead of explicitly forwarding MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Update types.Image to drop unnecessary error return values, and modify genericManifest to be more suitable; then embed a genericManfest into genericImage as an anonymous member implementing most of the interface. genericImage has now become a tiny wrapper over UnparsedImage and genericManifest. Signed-off-by: Miloslav Trmač --- copy/copy.go | 11 ++--------- image/docker_schema1.go | 12 +++++++++--- image/docker_schema2.go | 12 +++++++++--- image/image.go | 30 ++++++------------------------ image/manifest.go | 13 +++++++++++-- types/types.go | 4 ++-- 6 files changed, 39 insertions(+), 43 deletions(-) diff --git a/copy/copy.go b/copy/copy.go index e264905a76..f9a02bb5d5 100644 --- a/copy/copy.go +++ b/copy/copy.go @@ -156,11 +156,7 @@ func Image(ctx *types.SystemContext, policyContext *signature.PolicyContext, des } canModifyManifest := len(sigs) == 0 - writeReport("Getting image source configuration\n") - srcConfigInfo, err := src.ConfigInfo() - if err != nil { - return fmt.Errorf("Error parsing manifest: %v", err) - } + srcConfigInfo := src.ConfigInfo() if srcConfigInfo.Digest != "" { writeReport("Uploading blob %s\n", srcConfigInfo.Digest) destConfigInfo, err := copyBlob(dest, rawSource, srcConfigInfo, false, reportWriter) @@ -172,10 +168,7 @@ func Image(ctx *types.SystemContext, policyContext *signature.PolicyContext, des } } - srcLayerInfos, err := src.LayerInfos() - if err != nil { - return fmt.Errorf("Error parsing manifest: %v", err) - } + srcLayerInfos := src.LayerInfos() destLayerInfos := []types.BlobInfo{} copiedLayers := map[string]types.BlobInfo{} for _, srcLayer := range srcLayerInfos { diff --git a/image/docker_schema1.go b/image/docker_schema1.go index c3915fe70b..f93b09b269 100644 --- a/image/docker_schema1.go +++ b/image/docker_schema1.go @@ -47,10 +47,14 @@ func manifestSchema1FromManifest(manifest []byte) (genericManifest, error) { return mschema1, nil } +// ConfigInfo returns a complete BlobInfo for the separate config object, or a BlobInfo{Digest:""} if there isn't a separate object. func (m *manifestSchema1) ConfigInfo() types.BlobInfo { return types.BlobInfo{} } +// LayerInfos returns a list of BlobInfos of layers referenced by this image, in order (the root layer first, and then successive layered layers). +// The Digest field is guaranteed to be provided; Size may be -1. +// WARNING: The list may contain duplicates, and they are semantically relevant. func (m *manifestSchema1) LayerInfos() []types.BlobInfo { layers := make([]types.BlobInfo, len(m.FSLayers)) for i, layer := range m.FSLayers { // NOTE: This includes empty layers (where m.History.V1Compatibility->ThrowAway) @@ -59,13 +63,13 @@ func (m *manifestSchema1) LayerInfos() []types.BlobInfo { return layers } -func (m *manifestSchema1) Config() ([]byte, error) { +func (m *manifestSchema1) config() ([]byte, error) { return []byte(m.History[0].V1Compatibility), nil } -func (m *manifestSchema1) ImageInspectInfo() (*types.ImageInspectInfo, error) { +func (m *manifestSchema1) imageInspectInfo() (*types.ImageInspectInfo, error) { v1 := &v1Image{} - config, err := m.Config() + config, err := m.config() if err != nil { return nil, err } @@ -82,6 +86,8 @@ func (m *manifestSchema1) ImageInspectInfo() (*types.ImageInspectInfo, error) { }, nil } +// UpdatedManifest returns the image's manifest modified according to options. +// This does not change the state of the Image object. func (m *manifestSchema1) UpdatedManifest(options types.ManifestUpdateOptions) ([]byte, error) { copy := *m if options.LayerInfos != nil { diff --git a/image/docker_schema2.go b/image/docker_schema2.go index 448dc38c55..68981e814a 100644 --- a/image/docker_schema2.go +++ b/image/docker_schema2.go @@ -30,10 +30,14 @@ func manifestSchema2FromManifest(src types.ImageSource, manifest []byte) (generi return &v2s2, nil } +// ConfigInfo returns a complete BlobInfo for the separate config object, or a BlobInfo{Digest:""} if there isn't a separate object. func (m *manifestSchema2) ConfigInfo() types.BlobInfo { return types.BlobInfo{Digest: m.ConfigDescriptor.Digest, Size: m.ConfigDescriptor.Size} } +// LayerInfos returns a list of BlobInfos of layers referenced by this image, in order (the root layer first, and then successive layered layers). +// The Digest field is guaranteed to be provided; Size may be -1. +// WARNING: The list may contain duplicates, and they are semantically relevant. func (m *manifestSchema2) LayerInfos() []types.BlobInfo { blobs := []types.BlobInfo{} for _, layer := range m.LayersDescriptors { @@ -42,7 +46,7 @@ func (m *manifestSchema2) LayerInfos() []types.BlobInfo { return blobs } -func (m *manifestSchema2) Config() ([]byte, error) { +func (m *manifestSchema2) config() ([]byte, error) { rawConfig, _, err := m.src.GetBlob(m.ConfigDescriptor.Digest) if err != nil { return nil, err @@ -52,8 +56,8 @@ func (m *manifestSchema2) Config() ([]byte, error) { return config, err } -func (m *manifestSchema2) ImageInspectInfo() (*types.ImageInspectInfo, error) { - config, err := m.Config() +func (m *manifestSchema2) imageInspectInfo() (*types.ImageInspectInfo, error) { + config, err := m.config() if err != nil { return nil, err } @@ -70,6 +74,8 @@ func (m *manifestSchema2) ImageInspectInfo() (*types.ImageInspectInfo, error) { }, nil } +// UpdatedManifest returns the image's manifest modified according to options. +// This does not change the state of the Image object. func (m *manifestSchema2) UpdatedManifest(options types.ManifestUpdateOptions) ([]byte, error) { copy := *m if options.LayerInfos != nil { diff --git a/image/image.go b/image/image.go index a56ae678ce..dffb9fedc2 100644 --- a/image/image.go +++ b/image/image.go @@ -31,10 +31,10 @@ type genericImage struct { *UnparsedImage manifestBlob []byte manifestMIMEType string - // parsedManifest contains data corresponding to manifestBlob. - // NOTE: The manifest may have been modified in the process; DO NOT reserialize and store parsedManifest + // genericManifest contains data corresponding to manifestBlob. + // NOTE: The manifest may have been modified in the process; DO NOT reserialize and store genericManifest // if you want to preserve the original manifest; use manifestBlob directly. - parsedManifest genericManifest + genericManifest } // FromUnparsedImage returns a types.Image implementation for unparsed. @@ -75,7 +75,7 @@ func FromUnparsedImage(unparsed *UnparsedImage) (types.Image, error) { UnparsedImage: unparsed, manifestBlob: manifestBlob, manifestMIMEType: manifestMIMEType, - parsedManifest: parsedManifest, + genericManifest: parsedManifest, }, nil } @@ -85,11 +85,11 @@ func (i *genericImage) Manifest() ([]byte, string, error) { } func (i *genericImage) Inspect() (*types.ImageInspectInfo, error) { - info, err := i.parsedManifest.ImageInspectInfo() + info, err := i.genericManifest.imageInspectInfo() if err != nil { return nil, err } - layers := i.parsedManifest.LayerInfos() + layers := i.LayerInfos() info.Layers = make([]string, len(layers)) for i, layer := range layers { info.Layers[i] = layer.Digest @@ -97,24 +97,6 @@ func (i *genericImage) Inspect() (*types.ImageInspectInfo, error) { return info, nil } -// ConfigInfo returns a complete BlobInfo for the separate config object, or a BlobInfo{Digest:""} if there isn't a separate object. -func (i *genericImage) ConfigInfo() (types.BlobInfo, error) { - return i.parsedManifest.ConfigInfo(), nil -} - -// LayerInfos returns a list of BlobInfos of layers referenced by this image, in order (the root layer first, and then successive layered layers). -// The Digest field is guaranteed to be provided; Size may be -1. -// WARNING: The list may contain duplicates, and they are semantically relevant. -func (i *genericImage) LayerInfos() ([]types.BlobInfo, error) { - return i.parsedManifest.LayerInfos(), nil -} - -// UpdatedManifest returns the image's manifest modified according to updateOptions. -// This does not change the state of the Image object. -func (i *genericImage) UpdatedManifest(options types.ManifestUpdateOptions) ([]byte, error) { - return i.parsedManifest.UpdatedManifest(options) -} - func (i *genericImage) IsMultiImage() bool { return i.manifestMIMEType == manifest.DockerV2ListMediaType } diff --git a/image/manifest.go b/image/manifest.go index 696314cbe1..ce4c93134c 100644 --- a/image/manifest.go +++ b/image/manifest.go @@ -26,12 +26,21 @@ type v1Image struct { OS string `json:"os,omitempty"` } +// genericManifest is an interface for parsing, modifying image manifests and related data. +// Note that the public methods are intended to be a subset of types.Image +// so that embedding a genericManifest into structs works. // will support v1 one day... type genericManifest interface { - Config() ([]byte, error) + config() ([]byte, error) + // ConfigInfo returns a complete BlobInfo for the separate config object, or a BlobInfo{Digest:""} if there isn't a separate object. ConfigInfo() types.BlobInfo + // LayerInfos returns a list of BlobInfos of layers referenced by this image, in order (the root layer first, and then successive layered layers). + // The Digest field is guaranteed to be provided; Size may be -1. + // WARNING: The list may contain duplicates, and they are semantically relevant. LayerInfos() []types.BlobInfo - ImageInspectInfo() (*types.ImageInspectInfo, error) // The caller will need to fill in Layers + imageInspectInfo() (*types.ImageInspectInfo, error) // The caller will need to fill in Layers + // UpdatedManifest returns the image's manifest modified according to options. + // This does not change the state of the Image object. UpdatedManifest(types.ManifestUpdateOptions) ([]byte, error) } diff --git a/types/types.go b/types/types.go index 9f09231158..7947487c84 100644 --- a/types/types.go +++ b/types/types.go @@ -180,11 +180,11 @@ type UnparsedImage interface { type Image interface { UnparsedImage // ConfigInfo returns a complete BlobInfo for the separate config object, or a BlobInfo{Digest:""} if there isn't a separate object. - ConfigInfo() (BlobInfo, error) + ConfigInfo() BlobInfo // LayerInfos returns a list of BlobInfos of layers referenced by this image, in order (the root layer first, and then successive layered layers). // The Digest field is guaranteed to be provided; Size may be -1. // WARNING: The list may contain duplicates, and they are semantically relevant. - LayerInfos() ([]BlobInfo, error) + LayerInfos() []BlobInfo // Inspect returns various information for (skopeo inspect) parsed from the manifest and configuration. Inspect() (*ImageInspectInfo, error) // UpdatedManifest returns the image's manifest modified according to options. From 6344e64fef8ddc06f1668792117eed91cc795f0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Fri, 7 Oct 2016 00:04:13 +0200 Subject: [PATCH 09/11] Rename genericImage to sourcedImage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit … immediately before we introduce another kind of image. Signed-off-by: Miloslav Trmač --- image/{image.go => sourced.go} | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) rename image/{image.go => sourced.go} (91%) diff --git a/image/image.go b/image/sourced.go similarity index 91% rename from image/image.go rename to image/sourced.go index dffb9fedc2..c84f0f9f51 100644 --- a/image/image.go +++ b/image/sourced.go @@ -22,12 +22,12 @@ func FromSource(src types.ImageSource) (types.Image, error) { return FromUnparsedImage(UnparsedFromSource(src)) } -// genericImage is a general set of utilities for working with container images, +// sourcedImage is a general set of utilities for working with container images, // whatever is their underlying location (i.e. dockerImageSource-independent). // Note the existence of skopeo/docker.Image: some instances of a `types.Image` -// may not be a `genericImage` directly. However, most users of `types.Image` +// may not be a `sourcedImage` directly. However, most users of `types.Image` // do not care, and those who care about `skopeo/docker.Image` know they do. -type genericImage struct { +type sourcedImage struct { *UnparsedImage manifestBlob []byte manifestMIMEType string @@ -71,7 +71,7 @@ func FromUnparsedImage(unparsed *UnparsedImage) (types.Image, error) { return nil, err } - return &genericImage{ + return &sourcedImage{ UnparsedImage: unparsed, manifestBlob: manifestBlob, manifestMIMEType: manifestMIMEType, @@ -80,11 +80,11 @@ func FromUnparsedImage(unparsed *UnparsedImage) (types.Image, error) { } // Manifest overrides the UnparsedImage.Manifest to use the fields which we have already fetched, after guessing and overrides. -func (i *genericImage) Manifest() ([]byte, string, error) { +func (i *sourcedImage) Manifest() ([]byte, string, error) { return i.manifestBlob, i.manifestMIMEType, nil } -func (i *genericImage) Inspect() (*types.ImageInspectInfo, error) { +func (i *sourcedImage) Inspect() (*types.ImageInspectInfo, error) { info, err := i.genericManifest.imageInspectInfo() if err != nil { return nil, err @@ -97,6 +97,6 @@ func (i *genericImage) Inspect() (*types.ImageInspectInfo, error) { return info, nil } -func (i *genericImage) IsMultiImage() bool { +func (i *sourcedImage) IsMultiImage() bool { return i.manifestMIMEType == manifest.DockerV2ListMediaType } From 908a6f0235d816f5d94269d7e307a3e0e7b5ef2f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Fri, 7 Oct 2016 00:35:02 +0200 Subject: [PATCH 10/11] Finally, use UpdatedImage instead of UpdatedManifest MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Right now this does not really do anything, except add a whole lot of do-nothing infrastructure in memoryImage, but it will make it much easier to return extra data (like a newly generated config.json) from UpdatedImage. Signed-off-by: Miloslav Trmač --- copy/copy.go | 13 ++++---- image/docker_schema1.go | 22 ++++++++------ image/docker_schema2.go | 13 +++++--- image/manifest.go | 23 +++++++++++--- image/memory.go | 67 +++++++++++++++++++++++++++++++++++++++++ image/sourced.go | 11 +------ types/types.go | 7 +++-- 7 files changed, 119 insertions(+), 37 deletions(-) create mode 100644 image/memory.go diff --git a/copy/copy.go b/copy/copy.go index f9a02bb5d5..647e97b4bf 100644 --- a/copy/copy.go +++ b/copy/copy.go @@ -131,12 +131,6 @@ func Image(ctx *types.SystemContext, policyContext *signature.PolicyContext, des return fmt.Errorf("can not copy %s: manifest contains multiple images", transports.ImageName(srcRef)) } - writeReport("Getting image source manifest\n") - manifest, _, err := src.Manifest() - if err != nil { - return fmt.Errorf("Error reading manifest: %v", err) - } - var sigs [][]byte if options != nil && options.RemoveSignatures { sigs = [][]byte{} @@ -189,15 +183,20 @@ func Image(ctx *types.SystemContext, policyContext *signature.PolicyContext, des manifestUpdates.LayerInfos = destLayerInfos } + pendingImage := src if !reflect.DeepEqual(manifestUpdates, types.ManifestUpdateOptions{}) { if !canModifyManifest { return fmt.Errorf("Internal error: copy needs an updated manifest but that was known to be forbidden") } - manifest, err = src.UpdatedManifest(manifestUpdates) + pendingImage, err = src.UpdatedImage(manifestUpdates) if err != nil { return fmt.Errorf("Error creating an updated manifest: %v", err) } } + manifest, _, err := pendingImage.Manifest() + if err != nil { + return fmt.Errorf("Error reading manifest: %v", err) + } if options != nil && options.SignBy != "" { mech, err := signature.NewGPGSigningMechanism() diff --git a/image/docker_schema1.go b/image/docker_schema1.go index f93b09b269..29dac3b0a7 100644 --- a/image/docker_schema1.go +++ b/image/docker_schema1.go @@ -47,6 +47,15 @@ func manifestSchema1FromManifest(manifest []byte) (genericManifest, error) { return mschema1, nil } +func (m *manifestSchema1) serialize() ([]byte, error) { + // docker/distribution requires a signature even if the incoming data uses the nominally unsigned DockerV2Schema1MediaType. + unsigned, err := json.Marshal(*m) + if err != nil { + return nil, err + } + return manifest.AddDummyV2S1Signature(unsigned) +} + // ConfigInfo returns a complete BlobInfo for the separate config object, or a BlobInfo{Digest:""} if there isn't a separate object. func (m *manifestSchema1) ConfigInfo() types.BlobInfo { return types.BlobInfo{} @@ -86,9 +95,9 @@ func (m *manifestSchema1) imageInspectInfo() (*types.ImageInspectInfo, error) { }, nil } -// UpdatedManifest returns the image's manifest modified according to options. -// This does not change the state of the Image object. -func (m *manifestSchema1) UpdatedManifest(options types.ManifestUpdateOptions) ([]byte, error) { +// UpdatedImage returns a types.Image modified according to options. +// This does not change the state of the original Image object. +func (m *manifestSchema1) UpdatedImage(options types.ManifestUpdateOptions) (types.Image, error) { copy := *m if options.LayerInfos != nil { // Our LayerInfos includes empty layers (where m.History.V1Compatibility->ThrowAway), so expect them to be included here as well. @@ -102,12 +111,7 @@ func (m *manifestSchema1) UpdatedManifest(options types.ManifestUpdateOptions) ( copy.FSLayers[(len(options.LayerInfos)-1)-i].BlobSum = info.Digest } } - // docker/distribution requires a signature even if the incoming data uses the nominally unsigned DockerV2Schema1MediaType. - unsigned, err := json.Marshal(copy) - if err != nil { - return nil, err - } - return manifest.AddDummyV2S1Signature(unsigned) + return memoryImageFromManifest(©, manifest.DockerV2Schema1SignedMediaType), nil } // fixManifestLayers, after validating the supplied manifest diff --git a/image/docker_schema2.go b/image/docker_schema2.go index 68981e814a..4d19288115 100644 --- a/image/docker_schema2.go +++ b/image/docker_schema2.go @@ -5,6 +5,7 @@ import ( "fmt" "io/ioutil" + "github.com/containers/image/manifest" "github.com/containers/image/types" ) @@ -30,6 +31,10 @@ func manifestSchema2FromManifest(src types.ImageSource, manifest []byte) (generi return &v2s2, nil } +func (m *manifestSchema2) serialize() ([]byte, error) { + return json.Marshal(*m) +} + // ConfigInfo returns a complete BlobInfo for the separate config object, or a BlobInfo{Digest:""} if there isn't a separate object. func (m *manifestSchema2) ConfigInfo() types.BlobInfo { return types.BlobInfo{Digest: m.ConfigDescriptor.Digest, Size: m.ConfigDescriptor.Size} @@ -74,9 +79,9 @@ func (m *manifestSchema2) imageInspectInfo() (*types.ImageInspectInfo, error) { }, nil } -// UpdatedManifest returns the image's manifest modified according to options. -// This does not change the state of the Image object. -func (m *manifestSchema2) UpdatedManifest(options types.ManifestUpdateOptions) ([]byte, error) { +// UpdatedImage returns a types.Image modified according to options. +// This does not change the state of the original Image object. +func (m *manifestSchema2) UpdatedImage(options types.ManifestUpdateOptions) (types.Image, error) { copy := *m if options.LayerInfos != nil { if len(copy.LayersDescriptors) != len(options.LayerInfos) { @@ -87,5 +92,5 @@ func (m *manifestSchema2) UpdatedManifest(options types.ManifestUpdateOptions) ( copy.LayersDescriptors[i].Size = info.Size } } - return json.Marshal(copy) + return memoryImageFromManifest(©, manifest.DockerV2Schema2MediaType), nil } diff --git a/image/manifest.go b/image/manifest.go index ce4c93134c..392d8baff5 100644 --- a/image/manifest.go +++ b/image/manifest.go @@ -31,6 +31,7 @@ type v1Image struct { // so that embedding a genericManifest into structs works. // will support v1 one day... type genericManifest interface { + serialize() ([]byte, error) config() ([]byte, error) // ConfigInfo returns a complete BlobInfo for the separate config object, or a BlobInfo{Digest:""} if there isn't a separate object. ConfigInfo() types.BlobInfo @@ -38,10 +39,10 @@ type genericManifest interface { // The Digest field is guaranteed to be provided; Size may be -1. // WARNING: The list may contain duplicates, and they are semantically relevant. LayerInfos() []types.BlobInfo - imageInspectInfo() (*types.ImageInspectInfo, error) // The caller will need to fill in Layers - // UpdatedManifest returns the image's manifest modified according to options. - // This does not change the state of the Image object. - UpdatedManifest(types.ManifestUpdateOptions) ([]byte, error) + imageInspectInfo() (*types.ImageInspectInfo, error) // To be called by inspectManifest + // UpdatedImage returns a types.Image modified according to options. + // This does not change the state of the original Image object. + UpdatedImage(options types.ManifestUpdateOptions) (types.Image, error) } func manifestInstanceFromBlob(src types.ImageSource, manblob []byte, mt string) (genericManifest, error) { @@ -61,3 +62,17 @@ func manifestInstanceFromBlob(src types.ImageSource, manblob []byte, mt string) return nil, fmt.Errorf("unsupported manifest media type %s", mt) } } + +// inspectManifest is an implementation of types.Image.Inspect +func inspectManifest(m genericManifest) (*types.ImageInspectInfo, error) { + info, err := m.imageInspectInfo() + if err != nil { + return nil, err + } + layers := m.LayerInfos() + info.Layers = make([]string, len(layers)) + for i, layer := range layers { + info.Layers[i] = layer.Digest + } + return info, nil +} diff --git a/image/memory.go b/image/memory.go new file mode 100644 index 0000000000..0e02a51bf7 --- /dev/null +++ b/image/memory.go @@ -0,0 +1,67 @@ +package image + +import ( + "errors" + + "github.com/containers/image/types" +) + +// memoryImage is a mostly-implementation of types.Image assembled from data +// created in memory, used primarily as a return value of types.Image.UpdatedImage +// as a way to carry various structured information in a type-safe and easy-to-use way. +// Note that this _only_ carries the immediate metadata; it is _not_ a stand-alone +// collection of all related information, e.g. there is no way to get layer blobs +// from a memoryImage. +type memoryImage struct { + genericManifest + manifestMIMEType string + serializedManifest []byte // A private cache for Manifest() +} + +func memoryImageFromManifest(m genericManifest, mimeType string) types.Image { + return &memoryImage{ + genericManifest: m, + manifestMIMEType: mimeType, + serializedManifest: nil, + } +} + +// 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. +func (i *memoryImage) Reference() types.ImageReference { + // It would really be inappropriate to return the ImageReference of the image this was based on. + return nil +} + +// Close removes resources associated with an initialized UnparsedImage, if any. +func (i *memoryImage) Close() { +} + +// Manifest is like ImageSource.GetManifest, but the result is cached; it is OK to call this however often you need. +func (i *memoryImage) Manifest() ([]byte, string, error) { + if i.serializedManifest == nil { + m, err := i.genericManifest.serialize() + if err != nil { + return nil, "", err + } + i.serializedManifest = m + } + return i.serializedManifest, i.manifestMIMEType, nil +} + +// Signatures is like ImageSource.GetSignatures, but the result is cached; it is OK to call this however often you need. +func (i *memoryImage) Signatures() ([][]byte, error) { + // Modifying an image invalidates signatures; a caller asking the updated image for signatures + // is probably confused. + return nil, errors.New("Internal error: Image.Signatures() is not supported for images modified in memory") +} + +// Inspect returns various information for (skopeo inspect) parsed from the manifest and configuration. +func (i *memoryImage) Inspect() (*types.ImageInspectInfo, error) { + return inspectManifest(i.genericManifest) +} + +// IsMultiImage returns true if the image's manifest is a list of images, false otherwise. +func (i *memoryImage) IsMultiImage() bool { + return false +} diff --git a/image/sourced.go b/image/sourced.go index c84f0f9f51..1f21c68f50 100644 --- a/image/sourced.go +++ b/image/sourced.go @@ -85,16 +85,7 @@ func (i *sourcedImage) Manifest() ([]byte, string, error) { } func (i *sourcedImage) Inspect() (*types.ImageInspectInfo, error) { - info, err := i.genericManifest.imageInspectInfo() - if err != nil { - return nil, err - } - layers := i.LayerInfos() - info.Layers = make([]string, len(layers)) - for i, layer := range layers { - info.Layers[i] = layer.Digest - } - return info, nil + return inspectManifest(i.genericManifest) } func (i *sourcedImage) IsMultiImage() bool { diff --git a/types/types.go b/types/types.go index 7947487c84..e1c7ec019c 100644 --- a/types/types.go +++ b/types/types.go @@ -178,6 +178,7 @@ type UnparsedImage interface { // Image is the primary API for inspecting properties of images. // Each Image should eventually be closed by calling Close(). type Image interface { + // Note that Reference may return nil in the return value of UpdatedImage! UnparsedImage // ConfigInfo returns a complete BlobInfo for the separate config object, or a BlobInfo{Digest:""} if there isn't a separate object. ConfigInfo() BlobInfo @@ -187,9 +188,9 @@ type Image interface { LayerInfos() []BlobInfo // Inspect returns various information for (skopeo inspect) parsed from the manifest and configuration. Inspect() (*ImageInspectInfo, error) - // UpdatedManifest returns the image's manifest modified according to options. - // This does not change the state of the Image object. - UpdatedManifest(options ManifestUpdateOptions) ([]byte, error) + // UpdatedImage returns a types.Image modified according to options. + // This does not change the state of the original Image object. + UpdatedImage(options ManifestUpdateOptions) (Image, error) // IsMultiImage returns true if the image's manifest is a list of images, false otherwise. IsMultiImage() bool } From 1e99e60bc329d014bb276d590751e0f0a2df4ea0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Fri, 7 Oct 2016 17:40:15 +0200 Subject: [PATCH 11/11] Remove manifestMIMEType from memoryImage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Instead, make it a responsibility of genericManifest to provide the MIME type. This simplifies creation of a memoryImage, and removes a field which may be redundant, e.g. with manifestSchema2.MediaType. Signed-off-by: Miloslav Trmač --- image/docker_schema1.go | 6 +++++- image/docker_schema2.go | 7 +++++-- image/manifest.go | 1 + image/memory.go | 6 ++---- 4 files changed, 13 insertions(+), 7 deletions(-) diff --git a/image/docker_schema1.go b/image/docker_schema1.go index 29dac3b0a7..7b3f894f49 100644 --- a/image/docker_schema1.go +++ b/image/docker_schema1.go @@ -56,6 +56,10 @@ func (m *manifestSchema1) serialize() ([]byte, error) { return manifest.AddDummyV2S1Signature(unsigned) } +func (m *manifestSchema1) manifestMIMEType() string { + return manifest.DockerV2Schema1SignedMediaType +} + // ConfigInfo returns a complete BlobInfo for the separate config object, or a BlobInfo{Digest:""} if there isn't a separate object. func (m *manifestSchema1) ConfigInfo() types.BlobInfo { return types.BlobInfo{} @@ -111,7 +115,7 @@ func (m *manifestSchema1) UpdatedImage(options types.ManifestUpdateOptions) (typ copy.FSLayers[(len(options.LayerInfos)-1)-i].BlobSum = info.Digest } } - return memoryImageFromManifest(©, manifest.DockerV2Schema1SignedMediaType), nil + return memoryImageFromManifest(©), nil } // fixManifestLayers, after validating the supplied manifest diff --git a/image/docker_schema2.go b/image/docker_schema2.go index 4d19288115..b803f1c763 100644 --- a/image/docker_schema2.go +++ b/image/docker_schema2.go @@ -5,7 +5,6 @@ import ( "fmt" "io/ioutil" - "github.com/containers/image/manifest" "github.com/containers/image/types" ) @@ -35,6 +34,10 @@ func (m *manifestSchema2) serialize() ([]byte, error) { return json.Marshal(*m) } +func (m *manifestSchema2) manifestMIMEType() string { + return m.MediaType +} + // ConfigInfo returns a complete BlobInfo for the separate config object, or a BlobInfo{Digest:""} if there isn't a separate object. func (m *manifestSchema2) ConfigInfo() types.BlobInfo { return types.BlobInfo{Digest: m.ConfigDescriptor.Digest, Size: m.ConfigDescriptor.Size} @@ -92,5 +95,5 @@ func (m *manifestSchema2) UpdatedImage(options types.ManifestUpdateOptions) (typ copy.LayersDescriptors[i].Size = info.Size } } - return memoryImageFromManifest(©, manifest.DockerV2Schema2MediaType), nil + return memoryImageFromManifest(©), nil } diff --git a/image/manifest.go b/image/manifest.go index 392d8baff5..17b1223578 100644 --- a/image/manifest.go +++ b/image/manifest.go @@ -32,6 +32,7 @@ type v1Image struct { // will support v1 one day... type genericManifest interface { serialize() ([]byte, error) + manifestMIMEType() string config() ([]byte, error) // ConfigInfo returns a complete BlobInfo for the separate config object, or a BlobInfo{Digest:""} if there isn't a separate object. ConfigInfo() types.BlobInfo diff --git a/image/memory.go b/image/memory.go index 0e02a51bf7..568f855acb 100644 --- a/image/memory.go +++ b/image/memory.go @@ -14,14 +14,12 @@ import ( // from a memoryImage. type memoryImage struct { genericManifest - manifestMIMEType string serializedManifest []byte // A private cache for Manifest() } -func memoryImageFromManifest(m genericManifest, mimeType string) types.Image { +func memoryImageFromManifest(m genericManifest) types.Image { return &memoryImage{ genericManifest: m, - manifestMIMEType: mimeType, serializedManifest: nil, } } @@ -46,7 +44,7 @@ func (i *memoryImage) Manifest() ([]byte, string, error) { } i.serializedManifest = m } - return i.serializedManifest, i.manifestMIMEType, nil + return i.serializedManifest, i.genericManifest.manifestMIMEType(), nil } // Signatures is like ImageSource.GetSignatures, but the result is cached; it is OK to call this however often you need.