From c5fd291e7e8a2cf84a16123167d97c827c8c07c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Thu, 11 Aug 2016 19:44:27 +0200 Subject: [PATCH 1/4] Add SystemContext.RootForImplicitAbsolutePaths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As we add more config files, having a single knob to override them all may simplify usage. Note that this is not a chroot; it will override e.g. hard-coded /etc, but $HOME is unaffected. Signed-off-by: Miloslav Trmač --- signature/policy_config.go | 10 ++++++-- signature/policy_config_test.go | 44 +++++++++++++++++++++++++-------- types/types.go | 8 +++++- 3 files changed, 49 insertions(+), 13 deletions(-) diff --git a/signature/policy_config.go b/signature/policy_config.go index 20821a3509..f50a75cc88 100644 --- a/signature/policy_config.go +++ b/signature/policy_config.go @@ -18,6 +18,7 @@ import ( "errors" "fmt" "io/ioutil" + "path/filepath" "github.com/containers/image/transports" "github.com/containers/image/types" @@ -54,8 +55,13 @@ func DefaultPolicy(ctx *types.SystemContext) (*Policy, error) { // defaultPolicyPath returns a path to the default policy of the system. func defaultPolicyPath(ctx *types.SystemContext) string { - if ctx != nil && ctx.SignaturePolicyPath != "" { - return ctx.SignaturePolicyPath + if ctx != nil { + if ctx.SignaturePolicyPath != "" { + return ctx.SignaturePolicyPath + } + if ctx.RootForImplicitAbsolutePaths != "" { + return filepath.Join(ctx.RootForImplicitAbsolutePaths, systemDefaultPolicyPath) + } } return systemDefaultPolicyPath } diff --git a/signature/policy_config_test.go b/signature/policy_config_test.go index 7a0c15777d..8af3003d24 100644 --- a/signature/policy_config_test.go +++ b/signature/policy_config_test.go @@ -4,6 +4,7 @@ import ( "bytes" "encoding/json" "io/ioutil" + "path/filepath" "testing" "github.com/containers/image/directory" @@ -84,17 +85,40 @@ func TestDefaultPolicy(t *testing.T) { } func TestDefaultPolicyPath(t *testing.T) { - // The common case - path := defaultPolicyPath(nil) - assert.Equal(t, systemDefaultPolicyPath, path) - // There is a context, but it does not override the path. - path = defaultPolicyPath(&types.SystemContext{}) - assert.Equal(t, systemDefaultPolicyPath, path) - - // Path overridden + const nondefaultPath = "/this/is/not/the/default/path.json" - path = defaultPolicyPath(&types.SystemContext{SignaturePolicyPath: nondefaultPath}) - assert.Equal(t, nondefaultPath, path) + const variableReference = "$HOME" + const rootPrefix = "/root/prefix" + + for _, c := range []struct { + ctx *types.SystemContext + expected string + }{ + // The common case + {nil, systemDefaultPolicyPath}, + // There is a context, but it does not override the path. + {&types.SystemContext{}, systemDefaultPolicyPath}, + // Path overridden + {&types.SystemContext{SignaturePolicyPath: nondefaultPath}, nondefaultPath}, + // Root overridden + { + &types.SystemContext{RootForImplicitAbsolutePaths: rootPrefix}, + filepath.Join(rootPrefix, systemDefaultPolicyPath), + }, + // Root and path overrides present simultaneously, + { + &types.SystemContext{ + RootForImplicitAbsolutePaths: rootPrefix, + SignaturePolicyPath: nondefaultPath, + }, + nondefaultPath, + }, + // No environment expansion happens in the overridden paths + {&types.SystemContext{SignaturePolicyPath: variableReference}, variableReference}, + } { + path := defaultPolicyPath(c.ctx) + assert.Equal(t, c.expected, path) + } } func TestNewPolicyFromFile(t *testing.T) { diff --git a/types/types.go b/types/types.go index c5eb5ab65b..c02ba96ba9 100644 --- a/types/types.go +++ b/types/types.go @@ -154,5 +154,11 @@ type ImageInspectInfo struct { // the same; if in doubt, add a new field. // It is always OK to pass nil instead of a SystemContext. type SystemContext struct { - SignaturePolicyPath string // If not "", overrides the system's default path for signature.Policy configuration. + // If not "", prefixed to any absolute paths used by default by the library (e.g. in /etc/). + // Not used for any of the more specific path overrides available in this struct. + // Not used for any paths specified by users in config files (even if the location of the config file _was_ affected by it). + // NOTE: This does NOT affect paths starting by $HOME. + RootForImplicitAbsolutePaths string + // If not "", overrides the system's default path for signature.Policy configuration. + SignaturePolicyPath string } From 8ad0cad4eabb4ff46c897d6bfd24867cbf069af9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Thu, 11 Aug 2016 20:06:17 +0200 Subject: [PATCH 2/4] Use types.SystemContext in NewImage* MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ... instead of Docker-specific certPath and tlsVerify. Also invert the sense of tlsVerify to make the default secure. Signed-off-by: Miloslav Trmač --- directory/directory_test.go | 20 ++++++++++---------- directory/directory_transport.go | 6 +++--- directory/directory_transport_test.go | 6 +++--- doc.go | 2 +- docker/docker_client.go | 11 ++++++----- docker/docker_image.go | 4 ++-- docker/docker_image_dest.go | 6 +++--- docker/docker_image_src.go | 6 +++--- docker/docker_transport.go | 12 ++++++------ docker/docker_transport_test.go | 6 +++--- oci/oci_dest_test.go | 2 +- oci/oci_transport.go | 6 +++--- oci/oci_transport_test.go | 6 +++--- openshift/openshift.go | 20 +++++++++----------- openshift/openshift_transport.go | 10 +++++----- openshift/openshift_transport_test.go | 2 +- signature/policy_eval_signedby_test.go | 2 +- signature/policy_eval_simple_test.go | 6 +++--- signature/policy_eval_test.go | 6 +++--- signature/policy_reference_match_test.go | 6 +++--- types/types.go | 12 +++++++++--- 21 files changed, 81 insertions(+), 76 deletions(-) diff --git a/directory/directory_test.go b/directory/directory_test.go index 8d2c31002e..26e19a20fb 100644 --- a/directory/directory_test.go +++ b/directory/directory_test.go @@ -15,7 +15,7 @@ func TestDestinationReference(t *testing.T) { ref, tmpDir := refToTempDir(t) defer os.RemoveAll(tmpDir) - dest, err := ref.NewImageDestination("", true) + dest, err := ref.NewImageDestination(nil) require.NoError(t, err) ref2 := dest.Reference() assert.Equal(t, tmpDir, ref2.StringWithinTransport()) @@ -26,12 +26,12 @@ func TestGetPutManifest(t *testing.T) { defer os.RemoveAll(tmpDir) man := []byte("test-manifest") - dest, err := ref.NewImageDestination("", true) + dest, err := ref.NewImageDestination(nil) require.NoError(t, err) err = dest.PutManifest(man) assert.NoError(t, err) - src, err := ref.NewImageSource("", true) + src, err := ref.NewImageSource(nil) require.NoError(t, err) m, mt, err := src.GetManifest(nil) assert.NoError(t, err) @@ -45,12 +45,12 @@ func TestGetPutBlob(t *testing.T) { digest := "digest-test" blob := []byte("test-blob") - dest, err := ref.NewImageDestination("", true) + dest, err := ref.NewImageDestination(nil) require.NoError(t, err) err = dest.PutBlob(digest, bytes.NewReader(blob)) assert.NoError(t, err) - src, err := ref.NewImageSource("", true) + src, err := ref.NewImageSource(nil) require.NoError(t, err) rc, size, err := src.GetBlob(digest) assert.NoError(t, err) @@ -96,7 +96,7 @@ func TestPutBlobDigestFailure(t *testing.T) { return 0, fmt.Errorf(digestErrorString) }) - dest, err := ref.NewImageDestination("", true) + dest, err := ref.NewImageDestination(nil) require.NoError(t, err) err = dest.PutBlob(blobDigest, reader) assert.Error(t, err) @@ -111,7 +111,7 @@ func TestGetPutSignatures(t *testing.T) { ref, tmpDir := refToTempDir(t) defer os.RemoveAll(tmpDir) - dest, err := ref.NewImageDestination("", true) + dest, err := ref.NewImageDestination(nil) require.NoError(t, err) signatures := [][]byte{ []byte("sig1"), @@ -120,7 +120,7 @@ func TestGetPutSignatures(t *testing.T) { err = dest.PutSignatures(signatures) assert.NoError(t, err) - src, err := ref.NewImageSource("", true) + src, err := ref.NewImageSource(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("", true) + src, err := ref.NewImageSource(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("", true) + src, err := ref.NewImageSource(nil) require.NoError(t, err) ref2 := src.Reference() assert.Equal(t, tmpDir, ref2.StringWithinTransport()) diff --git a/directory/directory_transport.go b/directory/directory_transport.go index c5547f7c56..7b12c0068c 100644 --- a/directory/directory_transport.go +++ b/directory/directory_transport.go @@ -128,18 +128,18 @@ func (ref dirReference) PolicyConfigurationNamespaces() []string { } // NewImage returns a types.Image for this reference. -func (ref dirReference) NewImage(certPath string, tlsVerify bool) (types.Image, error) { +func (ref dirReference) NewImage(ctx *types.SystemContext) (types.Image, error) { src := newImageSource(ref) return image.FromSource(src, nil), nil } // NewImageSource returns a types.ImageSource for this reference. -func (ref dirReference) NewImageSource(certPath string, tlsVerify bool) (types.ImageSource, error) { +func (ref dirReference) NewImageSource(ctx *types.SystemContext) (types.ImageSource, error) { return newImageSource(ref), nil } // NewImageDestination returns a types.ImageDestination for this reference. -func (ref dirReference) NewImageDestination(certPath string, tlsVerify bool) (types.ImageDestination, error) { +func (ref dirReference) NewImageDestination(ctx *types.SystemContext) (types.ImageDestination, error) { return newImageDestination(ref), nil } diff --git a/directory/directory_transport_test.go b/directory/directory_transport_test.go index cba4c0d625..9403185779 100644 --- a/directory/directory_transport_test.go +++ b/directory/directory_transport_test.go @@ -149,21 +149,21 @@ func TestReferencePolicyConfigurationNamespaces(t *testing.T) { func TestReferenceNewImage(t *testing.T) { ref, tmpDir := refToTempDir(t) defer os.RemoveAll(tmpDir) - _, err := ref.NewImage("/this/doesn't/exist", true) + _, err := ref.NewImage(nil) assert.NoError(t, err) } func TestReferenceNewImageSource(t *testing.T) { ref, tmpDir := refToTempDir(t) defer os.RemoveAll(tmpDir) - _, err := ref.NewImageSource("/this/doesn't/exist", true) + _, err := ref.NewImageSource(nil) assert.NoError(t, err) } func TestReferenceNewImageDestination(t *testing.T) { ref, tmpDir := refToTempDir(t) defer os.RemoveAll(tmpDir) - _, err := ref.NewImageDestination("/this/doesn't/exist", true) + _, err := ref.NewImageDestination(nil) assert.NoError(t, err) } diff --git a/doc.go b/doc.go index 870bb408dd..3f990071c9 100644 --- a/doc.go +++ b/doc.go @@ -13,7 +13,7 @@ // if err != nil { // panic(err) // } -// img, err := ref.NewImage("", true) +// img, err := ref.NewImage(nil) // if err != nil { // panic(err) // } diff --git a/docker/docker_client.go b/docker/docker_client.go index acaca313fa..fd016565c2 100644 --- a/docker/docker_client.go +++ b/docker/docker_client.go @@ -14,6 +14,7 @@ import ( "time" "github.com/Sirupsen/logrus" + "github.com/containers/image/types" "github.com/docker/docker/pkg/homedir" ) @@ -44,7 +45,7 @@ type dockerClient struct { } // newDockerClient returns a new dockerClient instance for refHostname (a host a specified in the Docker image reference, not canonicalized to dockerRegistry) -func newDockerClient(refHostname, certPath string, tlsVerify bool) (*dockerClient, error) { +func newDockerClient(ctx *types.SystemContext, refHostname string) (*dockerClient, error) { var registry string if refHostname == dockerHostname { registry = dockerRegistry @@ -56,17 +57,17 @@ func newDockerClient(refHostname, certPath string, tlsVerify bool) (*dockerClien return nil, err } var tr *http.Transport - if certPath != "" || !tlsVerify { + if ctx != nil && (ctx.DockerCertPath != "" || ctx.DockerInsecureSkipTLSVerify) { tlsc := &tls.Config{} - if certPath != "" { - cert, err := tls.LoadX509KeyPair(filepath.Join(certPath, "cert.pem"), filepath.Join(certPath, "key.pem")) + if ctx.DockerCertPath != "" { + cert, err := tls.LoadX509KeyPair(filepath.Join(ctx.DockerCertPath, "cert.pem"), filepath.Join(ctx.DockerCertPath, "key.pem")) if err != nil { return nil, fmt.Errorf("Error loading x509 key pair: %s", err) } tlsc.Certificates = append(tlsc.Certificates, cert) } - tlsc.InsecureSkipVerify = !tlsVerify + tlsc.InsecureSkipVerify = ctx.DockerInsecureSkipTLSVerify tr = &http.Transport{ TLSClientConfig: tlsc, } diff --git a/docker/docker_image.go b/docker/docker_image.go index 1a5b39187f..233c87ef7e 100644 --- a/docker/docker_image.go +++ b/docker/docker_image.go @@ -18,8 +18,8 @@ type Image struct { // newImage returns a new Image interface type after setting up // a client to the registry hosting the given image. -func newImage(ref dockerReference, certPath string, tlsVerify bool) (types.Image, error) { - s, err := newImageSource(ref, certPath, tlsVerify) +func newImage(ctx *types.SystemContext, ref dockerReference) (types.Image, error) { + s, err := newImageSource(ctx, ref) if err != nil { return nil, err } diff --git a/docker/docker_image_dest.go b/docker/docker_image_dest.go index 86a38e64a5..1e5ab49877 100644 --- a/docker/docker_image_dest.go +++ b/docker/docker_image_dest.go @@ -17,9 +17,9 @@ type dockerImageDestination struct { c *dockerClient } -// newImageDestination creates a new ImageDestination for the specified image reference and connection specification. -func newImageDestination(ref dockerReference, certPath string, tlsVerify bool) (types.ImageDestination, error) { - c, err := newDockerClient(ref.ref.Hostname(), certPath, tlsVerify) +// newImageDestination creates a new ImageDestination for the specified image reference. +func newImageDestination(ctx *types.SystemContext, ref dockerReference) (types.ImageDestination, error) { + c, err := newDockerClient(ctx, ref.ref.Hostname()) if err != nil { return nil, err } diff --git a/docker/docker_image_src.go b/docker/docker_image_src.go index 9284fd4222..8c4e70154b 100644 --- a/docker/docker_image_src.go +++ b/docker/docker_image_src.go @@ -27,9 +27,9 @@ type dockerImageSource struct { c *dockerClient } -// newImageSource creates a new ImageSource for the specified image reference and connection specification. -func newImageSource(ref dockerReference, certPath string, tlsVerify bool) (*dockerImageSource, error) { - c, err := newDockerClient(ref.ref.Hostname(), certPath, tlsVerify) +// newImageSource creates a new ImageSource for the specified image reference. +func newImageSource(ctx *types.SystemContext, ref dockerReference) (*dockerImageSource, error) { + c, err := newDockerClient(ctx, ref.ref.Hostname()) if err != nil { return nil, err } diff --git a/docker/docker_transport.go b/docker/docker_transport.go index 68cb0060b8..03224387b4 100644 --- a/docker/docker_transport.go +++ b/docker/docker_transport.go @@ -116,18 +116,18 @@ func (ref dockerReference) PolicyConfigurationNamespaces() []string { } // NewImage returns a types.Image for this reference. -func (ref dockerReference) NewImage(certPath string, tlsVerify bool) (types.Image, error) { - return newImage(ref, certPath, tlsVerify) +func (ref dockerReference) NewImage(ctx *types.SystemContext) (types.Image, error) { + return newImage(ctx, ref) } // NewImageSource returns a types.ImageSource for this reference. -func (ref dockerReference) NewImageSource(certPath string, tlsVerify bool) (types.ImageSource, error) { - return newImageSource(ref, certPath, tlsVerify) +func (ref dockerReference) NewImageSource(ctx *types.SystemContext) (types.ImageSource, error) { + return newImageSource(ctx, ref) } // NewImageDestination returns a types.ImageDestination for this reference. -func (ref dockerReference) NewImageDestination(certPath string, tlsVerify bool) (types.ImageDestination, error) { - return newImageDestination(ref, certPath, tlsVerify) +func (ref dockerReference) NewImageDestination(ctx *types.SystemContext) (types.ImageDestination, error) { + return newImageDestination(ctx, ref) } // tagOrDigest returns a tag or digest from the reference. diff --git a/docker/docker_transport_test.go b/docker/docker_transport_test.go index cc2078e75b..d5950809b8 100644 --- a/docker/docker_transport_test.go +++ b/docker/docker_transport_test.go @@ -160,21 +160,21 @@ func TestReferencePolicyConfigurationNamespaces(t *testing.T) { func TestReferenceNewImage(t *testing.T) { ref, err := ParseReference("//busybox") require.NoError(t, err) - _, err = ref.NewImage("", true) + _, err = ref.NewImage(nil) assert.NoError(t, err) } func TestReferenceNewImageSource(t *testing.T) { ref, err := ParseReference("//busybox") require.NoError(t, err) - _, err = ref.NewImageSource("", true) + _, err = ref.NewImageSource(nil) assert.NoError(t, err) } func TestReferenceNewImageDestination(t *testing.T) { ref, err := ParseReference("//busybox") require.NoError(t, err) - _, err = ref.NewImageDestination("", true) + _, err = ref.NewImageDestination(nil) assert.NoError(t, err) } diff --git a/oci/oci_dest_test.go b/oci/oci_dest_test.go index 5d672bd3c8..f7b2aed334 100644 --- a/oci/oci_dest_test.go +++ b/oci/oci_dest_test.go @@ -44,7 +44,7 @@ func TestPutBlobDigestFailure(t *testing.T) { return 0, fmt.Errorf(digestErrorString) }) - dest, err := ref.NewImageDestination("", true) + dest, err := ref.NewImageDestination(nil) require.NoError(t, err) err = dest.PutBlob(blobDigest, reader) assert.Error(t, err) diff --git a/oci/oci_transport.go b/oci/oci_transport.go index be96a85e77..61288a32bf 100644 --- a/oci/oci_transport.go +++ b/oci/oci_transport.go @@ -165,17 +165,17 @@ func (ref ociReference) PolicyConfigurationNamespaces() []string { } // NewImage returns a types.Image for this reference. -func (ref ociReference) NewImage(certPath string, tlsVerify bool) (types.Image, error) { +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(certPath string, tlsVerify bool) (types.ImageSource, error) { +func (ref ociReference) NewImageSource(ctx *types.SystemContext) (types.ImageSource, error) { return nil, errors.New("Reading images not implemented for oci: image names") } // NewImageDestination returns a types.ImageDestination for this reference. -func (ref ociReference) NewImageDestination(certPath string, tlsVerify bool) (types.ImageDestination, error) { +func (ref ociReference) NewImageDestination(ctx *types.SystemContext) (types.ImageDestination, error) { return newImageDestination(ref), nil } diff --git a/oci/oci_transport_test.go b/oci/oci_transport_test.go index 4636805e92..90a3a3a1ba 100644 --- a/oci/oci_transport_test.go +++ b/oci/oci_transport_test.go @@ -205,21 +205,21 @@ func TestReferencePolicyConfigurationNamespaces(t *testing.T) { func TestReferenceNewImage(t *testing.T) { ref, tmpDir := refToTempOCI(t) defer os.RemoveAll(tmpDir) - _, err := ref.NewImage("/this/doesn't/exist", true) + _, err := ref.NewImage(nil) assert.Error(t, err) } func TestReferenceNewImageSource(t *testing.T) { ref, tmpDir := refToTempOCI(t) defer os.RemoveAll(tmpDir) - _, err := ref.NewImageSource("/this/doesn't/exist", true) + _, err := ref.NewImageSource(nil) assert.Error(t, err) } func TestReferenceNewImageDestination(t *testing.T) { ref, tmpDir := refToTempOCI(t) defer os.RemoveAll(tmpDir) - _, err := ref.NewImageDestination("/this/doesn't/exist", true) + _, err := ref.NewImageDestination(nil) assert.NoError(t, err) } diff --git a/openshift/openshift.go b/openshift/openshift.go index 75080b223e..7d981ba544 100644 --- a/openshift/openshift.go +++ b/openshift/openshift.go @@ -171,24 +171,22 @@ func (c *openshiftClient) dockerRegistryHostPart() string { type openshiftImageSource struct { client *openshiftClient // Values specific to this image - certPath string // Only for parseDockerImageSource - tlsVerify bool // Only for parseDockerImageSource + ctx *types.SystemContext // 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 and connection specification. -func newImageSource(ref openshiftReference, certPath string, tlsVerify bool) (types.ImageSource, error) { +// newImageSource creates a new ImageSource for the specified reference. +func newImageSource(ctx *types.SystemContext, ref openshiftReference) (types.ImageSource, error) { client, err := newOpenshiftClient(ref) if err != nil { return nil, err } return &openshiftImageSource{ - client: client, - certPath: certPath, - tlsVerify: tlsVerify, + client: client, + ctx: ctx, }, nil } @@ -270,7 +268,7 @@ func (s *openshiftImageSource) ensureImageIsResolved() error { if err != nil { return err } - d, err := dockerRef.NewImageSource(s.certPath, s.tlsVerify) + d, err := dockerRef.NewImageSource(s.ctx) if err != nil { return err } @@ -286,8 +284,8 @@ type openshiftImageDestination struct { imageStreamImageName string // "" if not yet known } -// newImageDestination creates a new ImageDestination for the specified reference and connection specification. -func newImageDestination(ref openshiftReference, certPath string, tlsVerify bool) (types.ImageDestination, error) { +// newImageDestination creates a new ImageDestination for the specified reference. +func newImageDestination(ctx *types.SystemContext, ref openshiftReference) (types.ImageDestination, error) { client, err := newOpenshiftClient(ref) if err != nil { return nil, err @@ -301,7 +299,7 @@ func newImageDestination(ref openshiftReference, certPath string, tlsVerify bool if err != nil { return nil, err } - docker, err := dockerRef.NewImageDestination(certPath, tlsVerify) + docker, err := dockerRef.NewImageDestination(ctx) if err != nil { return nil, err } diff --git a/openshift/openshift_transport.go b/openshift/openshift_transport.go index 96b5f085bd..f7c2879d40 100644 --- a/openshift/openshift_transport.go +++ b/openshift/openshift_transport.go @@ -154,16 +154,16 @@ func (ref openshiftReference) PolicyConfigurationNamespaces() []string { } // NewImage returns a types.Image for this reference. -func (ref openshiftReference) NewImage(certPath string, tlsVerify bool) (types.Image, error) { +func (ref openshiftReference) NewImage(ctx *types.SystemContext) (types.Image, error) { 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(certPath string, tlsVerify bool) (types.ImageSource, error) { - return newImageSource(ref, certPath, tlsVerify) +func (ref openshiftReference) NewImageSource(ctx *types.SystemContext) (types.ImageSource, error) { + return newImageSource(ctx, ref) } // NewImageDestination returns a types.ImageDestination for this reference. -func (ref openshiftReference) NewImageDestination(certPath string, tlsVerify bool) (types.ImageDestination, error) { - return newImageDestination(ref, certPath, tlsVerify) +func (ref openshiftReference) NewImageDestination(ctx *types.SystemContext) (types.ImageDestination, error) { + return newImageDestination(ctx, ref) } diff --git a/openshift/openshift_transport_test.go b/openshift/openshift_transport_test.go index 17aba5f6be..d57a3d58b3 100644 --- a/openshift/openshift_transport_test.go +++ b/openshift/openshift_transport_test.go @@ -113,7 +113,7 @@ func TestReferencePolicyConfigurationNamespaces(t *testing.T) { func TestReferenceNewImage(t *testing.T) { ref, err := NewReference(testBaseURL, "ns", "stream", "notlatest") require.NoError(t, err) - _, err = ref.NewImage("", true) + _, err = ref.NewImage(nil) assert.Error(t, err) } diff --git a/signature/policy_eval_signedby_test.go b/signature/policy_eval_signedby_test.go index 34376efd3b..7abf800029 100644 --- a/signature/policy_eval_signedby_test.go +++ b/signature/policy_eval_signedby_test.go @@ -25,7 +25,7 @@ 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("", true) + src, err := srcRef.NewImageSource(nil) require.NoError(t, err) return image.FromSource(&dirImageSourceMock{ ImageSource: src, diff --git a/signature/policy_eval_simple_test.go b/signature/policy_eval_simple_test.go index 3f23bdf789..788f414f45 100644 --- a/signature/policy_eval_simple_test.go +++ b/signature/policy_eval_simple_test.go @@ -34,13 +34,13 @@ func (ref nameOnlyImageReferenceMock) PolicyConfigurationIdentity() string { func (ref nameOnlyImageReferenceMock) PolicyConfigurationNamespaces() []string { panic("unexpected call to a mock function") } -func (ref nameOnlyImageReferenceMock) NewImage(certPath string, tlsVerify bool) (types.Image, error) { +func (ref nameOnlyImageReferenceMock) NewImage(ctx *types.SystemContext) (types.Image, error) { panic("unexpected call to a mock function") } -func (ref nameOnlyImageReferenceMock) NewImageSource(certPath string, tlsVerify bool) (types.ImageSource, error) { +func (ref nameOnlyImageReferenceMock) NewImageSource(ctx *types.SystemContext) (types.ImageSource, error) { panic("unexpected call to a mock function") } -func (ref nameOnlyImageReferenceMock) NewImageDestination(certPath string, tlsVerify bool) (types.ImageDestination, error) { +func (ref nameOnlyImageReferenceMock) NewImageDestination(ctx *types.SystemContext) (types.ImageDestination, error) { panic("unexpected call to a mock function") } diff --git a/signature/policy_eval_test.go b/signature/policy_eval_test.go index a4a87afb66..406ee7260a 100644 --- a/signature/policy_eval_test.go +++ b/signature/policy_eval_test.go @@ -90,13 +90,13 @@ func (ref pcImageReferenceMock) PolicyConfigurationNamespaces() []string { } return policyconfiguration.DockerReferenceNamespaces(ref.ref) } -func (ref pcImageReferenceMock) NewImage(certPath string, tlsVerify bool) (types.Image, error) { +func (ref pcImageReferenceMock) NewImage(ctx *types.SystemContext) (types.Image, error) { panic("unexpected call to a mock function") } -func (ref pcImageReferenceMock) NewImageSource(certPath string, tlsVerify bool) (types.ImageSource, error) { +func (ref pcImageReferenceMock) NewImageSource(ctx *types.SystemContext) (types.ImageSource, error) { panic("unexpected call to a mock function") } -func (ref pcImageReferenceMock) NewImageDestination(certPath string, tlsVerify bool) (types.ImageDestination, error) { +func (ref pcImageReferenceMock) NewImageDestination(ctx *types.SystemContext) (types.ImageDestination, error) { panic("unexpected call to a mock function") } diff --git a/signature/policy_reference_match_test.go b/signature/policy_reference_match_test.go index e342ea0559..fafa2dd71f 100644 --- a/signature/policy_reference_match_test.go +++ b/signature/policy_reference_match_test.go @@ -101,13 +101,13 @@ func (ref refImageReferenceMock) PolicyConfigurationIdentity() string { func (ref refImageReferenceMock) PolicyConfigurationNamespaces() []string { panic("unexpected call to a mock function") } -func (ref refImageReferenceMock) NewImage(certPath string, tlsVerify bool) (types.Image, error) { +func (ref refImageReferenceMock) NewImage(ctx *types.SystemContext) (types.Image, error) { panic("unexpected call to a mock function") } -func (ref refImageReferenceMock) NewImageSource(certPath string, tlsVerify bool) (types.ImageSource, error) { +func (ref refImageReferenceMock) NewImageSource(ctx *types.SystemContext) (types.ImageSource, error) { panic("unexpected call to a mock function") } -func (ref refImageReferenceMock) NewImageDestination(certPath string, tlsVerify bool) (types.ImageDestination, error) { +func (ref refImageReferenceMock) NewImageDestination(ctx *types.SystemContext) (types.ImageDestination, error) { panic("unexpected call to a mock function") } diff --git a/types/types.go b/types/types.go index c02ba96ba9..fabc593f49 100644 --- a/types/types.go +++ b/types/types.go @@ -71,11 +71,11 @@ type ImageReference interface { PolicyConfigurationNamespaces() []string // NewImage returns a types.Image for this reference. - NewImage(certPath string, tlsVerify bool) (Image, error) + NewImage(ctx *SystemContext) (Image, error) // NewImageSource returns a types.ImageSource for this reference. - NewImageSource(certPath string, tlsVerify bool) (ImageSource, error) + NewImageSource(ctx *SystemContext) (ImageSource, error) // NewImageDestination returns a types.ImageDestination for this reference. - NewImageDestination(certPath string, tlsVerify bool) (ImageDestination, error) + NewImageDestination(ctx *SystemContext) (ImageDestination, error) } // ImageSource is a service, possibly remote (= slow), to download components of a single image. @@ -159,6 +159,12 @@ type SystemContext struct { // Not used for any paths specified by users in config files (even if the location of the config file _was_ affected by it). // NOTE: This does NOT affect paths starting by $HOME. RootForImplicitAbsolutePaths string + + // === Global configuration overrides === // If not "", overrides the system's default path for signature.Policy configuration. SignaturePolicyPath string + + // === docker.Transport overrides === + DockerCertPath string // If not "", a directory containing "cert.pem" and "key.pem" used when talking to a Docker Registry + DockerInsecureSkipTLSVerify bool } From dff447c6383f7a3e351ce4a0566dad192357c3aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Tue, 16 Aug 2016 20:47:40 +0200 Subject: [PATCH 3/4] Move manifest MIME type selection from GetManifest to ImageSource creation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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č --- directory/directory_src.go | 2 +- directory/directory_test.go | 12 ++++++------ directory/directory_transport.go | 8 +++++--- directory/directory_transport_test.go | 2 +- docker/docker_image.go | 4 ++-- docker/docker_image_src.go | 21 ++++++++++++++------- docker/docker_transport.go | 8 +++++--- docker/docker_transport_test.go | 2 +- image/image.go | 17 ++++------------- manifest/manifest.go | 9 +++++++++ oci/oci_transport.go | 6 ++++-- oci/oci_transport_test.go | 2 +- openshift/openshift.go | 16 ++++++++++------ openshift/openshift_transport.go | 8 +++++--- signature/policy_eval_signedby_test.go | 4 ++-- signature/policy_eval_simple_test.go | 2 +- signature/policy_eval_test.go | 2 +- signature/policy_reference_match_test.go | 2 +- types/types.go | 10 ++++++---- 19 files changed, 79 insertions(+), 58 deletions(-) diff --git a/directory/directory_src.go b/directory/directory_src.go index f265c6279f..3e96f3ff8f 100644 --- a/directory/directory_src.go +++ b/directory/directory_src.go @@ -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 diff --git a/directory/directory_test.go b/directory/directory_test.go index 26e19a20fb..aabb0a9634 100644 --- a/directory/directory_test.go +++ b/directory/directory_test.go @@ -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()) diff --git a/directory/directory_transport.go b/directory/directory_transport.go index 7b12c0068c..9adefecfef 100644 --- a/directory/directory_transport.go +++ b/directory/directory_transport.go @@ -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 } diff --git a/directory/directory_transport_test.go b/directory/directory_transport_test.go index 9403185779..188700e4ea 100644 --- a/directory/directory_transport_test.go +++ b/directory/directory_transport_test.go @@ -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) } diff --git a/docker/docker_image.go b/docker/docker_image.go index 233c87ef7e..57216e8f89 100644 --- a/docker/docker_image.go +++ b/docker/docker_image.go @@ -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. diff --git a/docker/docker_image_src.go b/docker/docker_image_src.go index 8c4e70154b..88c676f79a 100644 --- a/docker/docker_image_src.go +++ b/docker/docker_image_src.go @@ -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 diff --git a/docker/docker_transport.go b/docker/docker_transport.go index 03224387b4..64962a6093 100644 --- a/docker/docker_transport.go +++ b/docker/docker_transport.go @@ -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. diff --git a/docker/docker_transport_test.go b/docker/docker_transport_test.go index d5950809b8..e5995cc6a5 100644 --- a/docker/docker_transport_test.go +++ b/docker/docker_transport_test.go @@ -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) } diff --git a/image/image.go b/image/image.go index ff7769e323..ee64ce28f8 100644 --- a/image/image.go +++ b/image/image.go @@ -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 } diff --git a/manifest/manifest.go b/manifest/manifest.go index 9345b5563b..1d5d63a799 100644 --- a/manifest/manifest.go +++ b/manifest/manifest.go @@ -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). diff --git a/oci/oci_transport.go b/oci/oci_transport.go index 61288a32bf..2b9c016442 100644 --- a/oci/oci_transport.go +++ b/oci/oci_transport.go @@ -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") } diff --git a/oci/oci_transport_test.go b/oci/oci_transport_test.go index 90a3a3a1ba..40d80bdfc9 100644 --- a/oci/oci_transport_test.go +++ b/oci/oci_transport_test.go @@ -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) } diff --git a/openshift/openshift.go b/openshift/openshift.go index 7d981ba544..e9e8ec4f7e 100644 --- a/openshift/openshift.go +++ b/openshift/openshift.go @@ -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 } diff --git a/openshift/openshift_transport.go b/openshift/openshift_transport.go index f7c2879d40..fd0ac1299f 100644 --- a/openshift/openshift_transport.go +++ b/openshift/openshift_transport.go @@ -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. diff --git a/signature/policy_eval_signedby_test.go b/signature/policy_eval_signedby_test.go index 7abf800029..e993bb6f2c 100644 --- a/signature/policy_eval_signedby_test.go +++ b/signature/policy_eval_signedby_test.go @@ -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. diff --git a/signature/policy_eval_simple_test.go b/signature/policy_eval_simple_test.go index 788f414f45..d52269706b 100644 --- a/signature/policy_eval_simple_test.go +++ b/signature/policy_eval_simple_test.go @@ -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) { diff --git a/signature/policy_eval_test.go b/signature/policy_eval_test.go index 406ee7260a..c7be98e983 100644 --- a/signature/policy_eval_test.go +++ b/signature/policy_eval_test.go @@ -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) { diff --git a/signature/policy_reference_match_test.go b/signature/policy_reference_match_test.go index fafa2dd71f..86f5d29b55 100644 --- a/signature/policy_reference_match_test.go +++ b/signature/policy_reference_match_test.go @@ -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) { diff --git a/types/types.go b/types/types.go index fabc593f49..d091d6d3b0 100644 --- a/types/types.go +++ b/types/types.go @@ -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) From be7e92f900dd804bb7da7e93baaa3687ed976eff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Thu, 25 Aug 2016 00:07:47 +0200 Subject: [PATCH 4/4] Move deleting images from ImageSource to ImageReference MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For lookaside signature store, and separating the read and write URLs, we need to set up read-only and read-write states differently; having read-write “delete” in dockerImageSource is incovenient. In tue future, ImageSource.Delete will be a really poor fit for docker-daemon:, where initializing the ImageSource causes the tarball to be copied from the daemon. We could instead implement the docker-daemon source so that it only copies the tarball on demand, but not sharing the object is much simpler. This leaves the Docker implementation in docker_image_src.go to make reviewing easier. Signed-off-by: Miloslav Trmač --- directory/directory_src.go | 5 ----- directory/directory_test.go | 10 ---------- directory/directory_transport.go | 5 +++++ directory/directory_transport_test.go | 7 +++++++ docker/docker_image_src.go | 24 ++++++++++++++---------- docker/docker_transport.go | 5 +++++ oci/oci_transport.go | 5 +++++ oci/oci_transport_test.go | 7 +++++++ openshift/openshift.go | 4 ---- openshift/openshift_transport.go | 5 +++++ openshift/openshift_transport_test.go | 7 +++++++ signature/policy_eval_simple_test.go | 3 +++ signature/policy_eval_test.go | 3 +++ signature/policy_reference_match_test.go | 3 +++ types/types.go | 5 +++-- 15 files changed, 67 insertions(+), 31 deletions(-) diff --git a/directory/directory_src.go b/directory/directory_src.go index 3e96f3ff8f..1f343597cc 100644 --- a/directory/directory_src.go +++ b/directory/directory_src.go @@ -1,7 +1,6 @@ package directory import ( - "fmt" "io" "io/ioutil" "os" @@ -59,7 +58,3 @@ func (s *dirImageSource) GetSignatures() ([][]byte, error) { } return signatures, nil } - -func (s *dirImageSource) Delete() error { - return fmt.Errorf("directory#dirImageSource.Delete() not implmented") -} diff --git a/directory/directory_test.go b/directory/directory_test.go index aabb0a9634..08af57b70e 100644 --- a/directory/directory_test.go +++ b/directory/directory_test.go @@ -127,16 +127,6 @@ func TestGetPutSignatures(t *testing.T) { assert.Equal(t, signatures, sigs) } -func TestDelete(t *testing.T) { - ref, tmpDir := refToTempDir(t) - defer os.RemoveAll(tmpDir) - - src, err := ref.NewImageSource(nil, nil) - require.NoError(t, err) - err = src.Delete() - assert.Error(t, err) -} - func TestSourceReference(t *testing.T) { ref, tmpDir := refToTempDir(t) defer os.RemoveAll(tmpDir) diff --git a/directory/directory_transport.go b/directory/directory_transport.go index 9adefecfef..1fd30cfd07 100644 --- a/directory/directory_transport.go +++ b/directory/directory_transport.go @@ -145,6 +145,11 @@ func (ref dirReference) NewImageDestination(ctx *types.SystemContext) (types.Ima return newImageDestination(ref), nil } +// DeleteImage deletes the named image from the registry, if supported. +func (ref dirReference) DeleteImage(ctx *types.SystemContext) error { + return fmt.Errorf("Deleting images not implemented for dir: images") +} + // manifestPath returns a path for the manifest within a directory using our conventions. func (ref dirReference) manifestPath() string { return filepath.Join(ref.path, "manifest.json") diff --git a/directory/directory_transport_test.go b/directory/directory_transport_test.go index 188700e4ea..b202a186d3 100644 --- a/directory/directory_transport_test.go +++ b/directory/directory_transport_test.go @@ -167,6 +167,13 @@ func TestReferenceNewImageDestination(t *testing.T) { assert.NoError(t, err) } +func TestReferenceDeleteImage(t *testing.T) { + ref, tmpDir := refToTempDir(t) + defer os.RemoveAll(tmpDir) + err := ref.DeleteImage(nil) + assert.Error(t, err) +} + func TestReferenceManifestPath(t *testing.T) { ref, tmpDir := refToTempDir(t) defer os.RemoveAll(tmpDir) diff --git a/docker/docker_image_src.go b/docker/docker_image_src.go index 88c676f79a..943918f94b 100644 --- a/docker/docker_image_src.go +++ b/docker/docker_image_src.go @@ -113,42 +113,46 @@ func (s *dockerImageSource) GetSignatures() ([][]byte, error) { return [][]byte{}, nil } -func (s *dockerImageSource) Delete() error { - var body []byte +// deleteImage deletes the named image from the registry, if supported. +func deleteImage(ctx *types.SystemContext, ref dockerReference) error { + c, err := newDockerClient(ctx, ref.ref.Hostname()) + if err != nil { + return err + } // When retrieving the digest from a registry >= 2.3 use the following header: // "Accept": "application/vnd.docker.distribution.manifest.v2+json" headers := make(map[string][]string) headers["Accept"] = []string{manifest.DockerV2Schema2MIMEType} - reference, err := s.ref.tagOrDigest() + reference, err := ref.tagOrDigest() if err != nil { return err } - getURL := fmt.Sprintf(manifestURL, s.ref.ref.RemoteName(), reference) - get, err := s.c.makeRequest("GET", getURL, headers, nil) + getURL := fmt.Sprintf(manifestURL, ref.ref.RemoteName(), reference) + get, err := c.makeRequest("GET", getURL, headers, nil) if err != nil { return err } defer get.Body.Close() - body, err = ioutil.ReadAll(get.Body) + body, err := ioutil.ReadAll(get.Body) if err != nil { return err } switch get.StatusCode { case http.StatusOK: case http.StatusNotFound: - return fmt.Errorf("Unable to delete %v. Image may not exist or is not stored with a v2 Schema in a v2 registry.", s.ref.ref) + return fmt.Errorf("Unable to delete %v. Image may not exist or is not stored with a v2 Schema in a v2 registry.", ref.ref) default: - return fmt.Errorf("Failed to delete %v: %s (%v)", s.ref.ref, string(body), get.Status) + return fmt.Errorf("Failed to delete %v: %s (%v)", ref.ref, string(body), get.Status) } digest := get.Header.Get("Docker-Content-Digest") - deleteURL := fmt.Sprintf(manifestURL, s.ref.ref.RemoteName(), digest) + deleteURL := fmt.Sprintf(manifestURL, ref.ref.RemoteName(), digest) // When retrieving the digest from a registry >= 2.3 use the following header: // "Accept": "application/vnd.docker.distribution.manifest.v2+json" - delete, err := s.c.makeRequest("DELETE", deleteURL, headers, nil) + delete, err := c.makeRequest("DELETE", deleteURL, headers, nil) if err != nil { return err } diff --git a/docker/docker_transport.go b/docker/docker_transport.go index 64962a6093..20f77dffcc 100644 --- a/docker/docker_transport.go +++ b/docker/docker_transport.go @@ -132,6 +132,11 @@ func (ref dockerReference) NewImageDestination(ctx *types.SystemContext) (types. return newImageDestination(ctx, ref) } +// DeleteImage deletes the named image from the registry, if supported. +func (ref dockerReference) DeleteImage(ctx *types.SystemContext) error { + return deleteImage(ctx, ref) +} + // tagOrDigest returns a tag or digest from the reference. func (ref dockerReference) tagOrDigest() (string, error) { if ref, ok := ref.ref.(reference.Canonical); ok { diff --git a/oci/oci_transport.go b/oci/oci_transport.go index 2b9c016442..e6d92a39d1 100644 --- a/oci/oci_transport.go +++ b/oci/oci_transport.go @@ -181,6 +181,11 @@ func (ref ociReference) NewImageDestination(ctx *types.SystemContext) (types.Ima return newImageDestination(ref), nil } +// DeleteImage deletes the named image from the registry, if supported. +func (ref ociReference) DeleteImage(ctx *types.SystemContext) error { + return fmt.Errorf("Deleting images not implemented for oci: images") +} + // ociLayoutPathPath returns a path for the oci-layout within a directory using OCI conventions. func (ref ociReference) ociLayoutPath() string { return filepath.Join(ref.dir, "oci-layout") diff --git a/oci/oci_transport_test.go b/oci/oci_transport_test.go index 40d80bdfc9..f6064d6ee8 100644 --- a/oci/oci_transport_test.go +++ b/oci/oci_transport_test.go @@ -223,6 +223,13 @@ func TestReferenceNewImageDestination(t *testing.T) { assert.NoError(t, err) } +func TestReferenceDeleteImage(t *testing.T) { + ref, tmpDir := refToTempOCI(t) + defer os.RemoveAll(tmpDir) + err := ref.DeleteImage(nil) + assert.Error(t, err) +} + func TestReferenceOCILayoutPath(t *testing.T) { ref, tmpDir := refToTempOCI(t) defer os.RemoveAll(tmpDir) diff --git a/openshift/openshift.go b/openshift/openshift.go index e9e8ec4f7e..3e9514409f 100644 --- a/openshift/openshift.go +++ b/openshift/openshift.go @@ -517,7 +517,3 @@ type status struct { // Details *StatusDetails `json:"details,omitempty"` Code int32 `json:"code,omitempty"` } - -func (s *openshiftImageSource) Delete() error { - return fmt.Errorf("openshift#openshiftImageSource.Delete() not implmented") -} diff --git a/openshift/openshift_transport.go b/openshift/openshift_transport.go index fd0ac1299f..de8dd9d7e4 100644 --- a/openshift/openshift_transport.go +++ b/openshift/openshift_transport.go @@ -169,3 +169,8 @@ func (ref openshiftReference) NewImageSource(ctx *types.SystemContext, requested func (ref openshiftReference) NewImageDestination(ctx *types.SystemContext) (types.ImageDestination, error) { return newImageDestination(ctx, ref) } + +// DeleteImage deletes the named image from the registry, if supported. +func (ref openshiftReference) DeleteImage(ctx *types.SystemContext) error { + return fmt.Errorf("Deleting images not implemented for atomic: images") +} diff --git a/openshift/openshift_transport_test.go b/openshift/openshift_transport_test.go index d57a3d58b3..a5d9c6559d 100644 --- a/openshift/openshift_transport_test.go +++ b/openshift/openshift_transport_test.go @@ -119,3 +119,10 @@ func TestReferenceNewImage(t *testing.T) { // openshiftReference.NewImageSource, openshiftReference.NewImageDestination untested because they depend // on per-user configuration when initializing httpClient. + +func TestReferenceDeleteImage(t *testing.T) { + ref, err := NewReference(testBaseURL, "ns", "stream", "notlatest") + require.NoError(t, err) + err = ref.DeleteImage(nil) + assert.Error(t, err) +} diff --git a/signature/policy_eval_simple_test.go b/signature/policy_eval_simple_test.go index d52269706b..90c25b8e06 100644 --- a/signature/policy_eval_simple_test.go +++ b/signature/policy_eval_simple_test.go @@ -43,6 +43,9 @@ func (ref nameOnlyImageReferenceMock) NewImageSource(ctx *types.SystemContext, r func (ref nameOnlyImageReferenceMock) NewImageDestination(ctx *types.SystemContext) (types.ImageDestination, error) { panic("unexpected call to a mock function") } +func (ref nameOnlyImageReferenceMock) DeleteImage(ctx *types.SystemContext) error { + panic("unexpected call to a mock function") +} func TestPRInsecureAcceptAnythingIsSignatureAuthorAccepted(t *testing.T) { pr := NewPRInsecureAcceptAnything() diff --git a/signature/policy_eval_test.go b/signature/policy_eval_test.go index c7be98e983..df6c785b75 100644 --- a/signature/policy_eval_test.go +++ b/signature/policy_eval_test.go @@ -99,6 +99,9 @@ func (ref pcImageReferenceMock) NewImageSource(ctx *types.SystemContext, request func (ref pcImageReferenceMock) NewImageDestination(ctx *types.SystemContext) (types.ImageDestination, error) { panic("unexpected call to a mock function") } +func (ref pcImageReferenceMock) DeleteImage(ctx *types.SystemContext) error { + panic("unexpected call to a mock function") +} func TestPolicyContextRequirementsForImageRef(t *testing.T) { ktGPG := SBKeyTypeGPGKeys diff --git a/signature/policy_reference_match_test.go b/signature/policy_reference_match_test.go index 86f5d29b55..6b5735cde0 100644 --- a/signature/policy_reference_match_test.go +++ b/signature/policy_reference_match_test.go @@ -110,6 +110,9 @@ func (ref refImageReferenceMock) NewImageSource(ctx *types.SystemContext, reques func (ref refImageReferenceMock) NewImageDestination(ctx *types.SystemContext) (types.ImageDestination, error) { panic("unexpected call to a mock function") } +func (ref refImageReferenceMock) DeleteImage(ctx *types.SystemContext) error { + panic("unexpected call to a mock function") +} // nameImageTransportMock is a mock of types.ImageTransport which returns itself in Name. type nameImageTransportMock string diff --git a/types/types.go b/types/types.go index d091d6d3b0..eee1cd49da 100644 --- a/types/types.go +++ b/types/types.go @@ -78,6 +78,9 @@ type ImageReference interface { NewImageSource(ctx *SystemContext, requestedManifestMIMETypes []string) (ImageSource, error) // NewImageDestination returns a types.ImageDestination for this reference. NewImageDestination(ctx *SystemContext) (ImageDestination, error) + + // DeleteImage deletes the named image from the registry, if supported. + DeleteImage(ctx *SystemContext) error } // ImageSource is a service, possibly remote (= slow), to download components of a single image. @@ -95,8 +98,6 @@ type ImageSource interface { GetBlob(digest string) (io.ReadCloser, int64, error) // GetSignatures returns the image's signatures. It may use a remote (= slow) service. GetSignatures() ([][]byte, error) - // Delete image from registry, if operation is supported - Delete() error } // ImageDestination is a service, possibly remote (= slow), to store components of a single image.