Tests for the X-Registry-Supports-Signatures extension

docker/docker_client.go

@@ -67,6 +67,9 @@ var (
 	}
 )
 
+// httpWrapper allows replacing the used http.RoundTripper.  This is internal-only, used for github.com/dnaeon/go-vcr/recorder -based tests.
+type httpWrapper func(http.RoundTripper) http.RoundTripper
+
 // extensionSignature and extensionSignatureList come from github.com/openshift/origin/pkg/dockerregistry/server/signaturedispatcher.go:
 // signature represents a Docker image signature.
 type extensionSignature struct {
@@ -92,8 +95,9 @@ type bearerToken struct {
 // dockerClient is configuration for dealing with a single Docker registry.
 type dockerClient struct {
 	// The following members are set by newDockerClient and do not change afterwards.
-	sys      *types.SystemContext
-	registry string
+	sys         *types.SystemContext
+	httpWrapper httpWrapper // nil unless running tests
+	registry    string
 
 	// tlsClientConfig is setup by newDockerClient and will be used and updated
 	// by detectProperties(). Callers can edit tlsClientConfig.InsecureSkipVerify in the meantime.
@@ -210,7 +214,7 @@ func dockerCertDir(sys *types.SystemContext, hostPort string) (string, error) {
 // newDockerClientFromRef returns a new dockerClient instance for refHostname (a host a specified in the Docker image reference, not canonicalized to dockerRegistry)
 // “write” specifies whether the client will be used for "write" access (in particular passed to lookaside.go:toplevelFromSection)
 // signatureBase is always set in the return value
-func newDockerClientFromRef(sys *types.SystemContext, ref dockerReference, write bool, actions string) (*dockerClient, error) {
+func newDockerClientFromRef(sys *types.SystemContext, ref dockerReference, write bool, actions string, httpWrapper httpWrapper) (*dockerClient, error) {
 	registry := reference.Domain(ref.ref)
 	auth, err := config.GetCredentials(sys, registry)
 	if err != nil {
@@ -222,7 +226,7 @@ func newDockerClientFromRef(sys *types.SystemContext, ref dockerReference, write
 		return nil, err
 	}
 
-	client, err := newDockerClient(sys, registry, ref.ref.Name())
+	client, err := newDockerClient(sys, registry, ref.ref.Name(), httpWrapper)
 	if err != nil {
 		return nil, err
 	}
@@ -242,7 +246,7 @@ func newDockerClientFromRef(sys *types.SystemContext, ref dockerReference, write
 // (e.g., "registry.com[:5000][/some/namespace]/repo").
 // Please note that newDockerClient does not set all members of dockerClient
 // (e.g., username and password); those must be set by callers if necessary.
-func newDockerClient(sys *types.SystemContext, registry, reference string) (*dockerClient, error) {
+func newDockerClient(sys *types.SystemContext, registry, reference string, httpWrapper httpWrapper) (*dockerClient, error) {
 	hostName := registry
 	if registry == dockerHostname {
 		registry = dockerRegistry
@@ -279,6 +283,7 @@ func newDockerClient(sys *types.SystemContext, registry, reference string) (*doc
 
 	return &dockerClient{
 		sys:             sys,
+		httpWrapper:     httpWrapper,
 		registry:        registry,
 		tlsClientConfig: tlsClientConfig,
 	}, nil
@@ -287,7 +292,7 @@ func newDockerClient(sys *types.SystemContext, registry, reference string) (*doc
 // CheckAuth validates the credentials by attempting to log into the registry
 // returns an error if an error occurred while making the http request or the status code received was 401
 func CheckAuth(ctx context.Context, sys *types.SystemContext, username, password, registry string) error {
-	client, err := newDockerClient(sys, registry, registry)
+	client, err := newDockerClient(sys, registry, registry, nil)
 	if err != nil {
 		return errors.Wrapf(err, "error creating new docker client")
 	}
@@ -349,7 +354,7 @@ func SearchRegistry(ctx context.Context, sys *types.SystemContext, registry, ima
 		hostname = dockerV1Hostname
 	}
 
-	client, err := newDockerClient(sys, hostname, registry)
+	client, err := newDockerClient(sys, hostname, registry, nil)
 	if err != nil {
 		return nil, errors.Wrapf(err, "error creating new docker client")
 	}
@@ -723,7 +728,11 @@ func (c *dockerClient) detectPropertiesHelper(ctx context.Context) error {
 	}
 	tr := tlsclientconfig.NewTransport()
 	tr.TLSClientConfig = c.tlsClientConfig
-	c.client = &http.Client{Transport: tr}
+	rt := http.RoundTripper(tr)
+	if c.httpWrapper != nil {
+		rt = c.httpWrapper(rt)
+	}
+	c.client = &http.Client{Transport: rt}
 
 	ping := func(scheme string) error {
 		url := fmt.Sprintf(resolvedPingV2URL, scheme, c.registry)

docker/docker_client_test.go

@@ -1,12 +1,26 @@
 package docker
 
+// Many of these tests are made using github.com/dnaeon/go-vcr/recorder, and under ordinary
+// operation are expected to run completely off-line from the recorded interactions.
+//
+// To update an individual test, set up a registry server as needed (usually just
+// allow access to docker.io; special setup will be described with individual tests),
+// temporarily edit the test to use recorder.ModeRecording, run the test.
+// Don’t forget to revert to recorder.ModeReplaying!
+
 import (
+	"context"
 	"fmt"
+	"io/ioutil"
+	"net/http"
+	"os"
 	"path/filepath"
 	"testing"
 	"time"
 
 	"github.com/containers/image/v5/types"
+	"github.com/dnaeon/go-vcr/recorder"
+	digest "github.com/opencontainers/go-digest"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -151,3 +165,138 @@ func assertBearerTokensEqual(t *testing.T, expected, subject *bearerToken) {
 		t.Fatalf("expected [%s] to equal [%s], it did not", subject.IssuedAt, expected.IssuedAt)
 	}
 }
+
+// prepareVCR is a shared helper for setting up HTTP request/response recordings using recordingBaseName.
+// It returns the result of preparing ctx and ref, a httpWrapper and a cleanup callback.
+func prepareVCR(t *testing.T, ctx *types.SystemContext, recordingBaseName string, mode recorder.Mode,
+	ref string) (*types.SystemContext, httpWrapper, func(), dockerReference) {
+	// Always set ctx.DockerAuthConfig so that we don’t depend on $HOME.
+	ourCtx := types.SystemContext{}
+	if ctx != nil {
+		ourCtx = *ctx
+	}
+	if ourCtx.DockerAuthConfig == nil {
+		ourCtx.DockerAuthConfig = &types.DockerAuthConfig{}
+	}
+
+	parsedRef, err := ParseReference(ref)
+	require.NoError(t, err)
+	dockerRef, ok := parsedRef.(dockerReference)
+	require.True(t, ok)
+
+	// dockerClient creates a new http.Client in each call to getBearerToken, so we need
+	// not just a single recording with a given name, but a sequence of recordings.
+	recorderNo := 0
+	allRecorders := []*recorder.Recorder{}
+	httpWrapper := func(rt http.RoundTripper) http.RoundTripper {
+		recordingName := fmt.Sprintf("fixtures/recording-%s-%d", recordingBaseName, recorderNo)
+		recorderNo++
+
+		// Always create the file first; without that, even with mode == recorder.ModeReplaying,
+		// the recorder is in recording mode.  We want to ensure that recording happens only
+		// as an intentional decision.
+		recordingFileName := recordingName + ".yaml"
+		f, err := os.OpenFile(recordingFileName, os.O_RDWR|os.O_CREATE, 0600)
+		require.NoError(t, err)
+		f.Close()
+
+		r, err := recorder.NewAsMode(recordingName, mode, rt)
+		require.NoError(t, err)
+		allRecorders = append(allRecorders, r)
+		return r
+	}
+
+	closeRecorders := func() {
+		for _, r := range allRecorders {
+			err := r.Stop()
+			require.NoError(t, err)
+		}
+	}
+
+	return &ourCtx, httpWrapper, closeRecorders, dockerRef
+}
+
+// vcrDockerClient creates a dockerClient using a series of HTTP request/response recordings
+// using recordingBaseName.
+// It returns a dockerClient and a cleanup callback, and the parsed version of ref.
+func vcrDockerClient(t *testing.T, ctx *types.SystemContext, recordingBaseName string, mode recorder.Mode,
+	ref string, write bool, actions string) (*dockerClient, func(), dockerReference) {
+	ctx, httpWrapper, cleanup, dockerRef := prepareVCR(t, ctx, recordingBaseName, mode,
+		ref)
+
+	client, err := newDockerClientFromRef(ctx, dockerRef, write, actions, httpWrapper)
+	require.NoError(t, err)
+	return client, cleanup, dockerRef
+}
+
+func TestDockerClientDetectProperties(t *testing.T) {
+	// Success, against the Docker Hub
+	client, cleanup, _ := vcrDockerClient(t, nil, "detectProperties-docker.io", recorder.ModeReplaying,
+		"//busybox:latest", false, "pull")
+	defer cleanup()
+	err := client.detectProperties(context.Background())
+	require.NoError(t, err)
+	assert.Equal(t, "https", client.scheme)
+	assert.Equal(t, []challenge{{
+		Scheme:     "bearer",
+		Parameters: map[string]string{"realm": "https://auth.docker.io/token", "service": "registry.docker.io"},
+	}}, client.challenges)
+	assert.False(t, client.supportsSignatures)
+
+	// Success, against Atomic Registry.
+	// See the comment above TestDockerClientGetExtensionsSignatures for instructions on setting up the recording.
+	openshiftCtx := &types.SystemContext{
+		DockerInsecureSkipTLSVerify: types.OptionalBoolTrue,
+	}
+	client, cleanup, _ = vcrDockerClient(t, openshiftCtx, "detectProperties-openshift", recorder.ModeReplaying,
+		"//localhost:5000/myns/personal:personal", false, "pull")
+	defer cleanup()
+	err = client.detectProperties(context.Background())
+	require.NoError(t, err)
+	assert.Equal(t, "http", client.scheme)
+	assert.Equal(t, []challenge{{
+		Scheme:     "bearer",
+		Parameters: map[string]string{"realm": "http://localhost:5000/openshift/token"},
+	}}, client.challenges)
+	assert.True(t, client.supportsSignatures)
+
+	// TODO? Test the various other cases, e.g. a schema1 registry
+}
+
+// To record the the X-Registry-Supports-Signatures tests,
+// use skopeo's integration tests to set up an Atomic Registry per https://github.com/projectatomic/skopeo/pull/320
+// except running the container with -p 5000:5000, e.g.
+// (sudo docker run --rm -i  -t -p 5000:5000 "skopeo-dev:openshift-shell" bash)
+// Then set:
+// - the username:password values obtained by decoding "auth" from the in-container  ~/.docker/config.json
+// - the manifest digest reference e.g. from (oc get istag personal:personal) value image.dockerImageReference in-container.
+// - the signature name from the same (oc get istag personal:personal)
+func TestDockerClientGetExtensionsSignatures(t *testing.T) {
+	ctx := &types.SystemContext{
+		DockerAuthConfig: &types.DockerAuthConfig{
+			Username: "unused",
+			Password: "dh2juhu6LbGYGSHKMUa5BFEpyoPMYDVA59hxd3FCfbU",
+		},
+		DockerInsecureSkipTLSVerify: types.OptionalBoolTrue,
+	}
+
+	// Success
+	manifestDigest := digest.Digest("sha256:8d7fe3e157e56648ab790794970fbdfe82c84af79e807443b98df92c822a9b9b")
+	client, cleanup, dockerRef := vcrDockerClient(t, ctx, "getExtensionsSignatures-success", recorder.ModeReplaying,
+		"//localhost:5000/myns/personal:personal", false, "pull")
+	defer cleanup()
+	esl, err := client.getExtensionsSignatures(context.Background(), dockerRef, manifestDigest)
+	require.NoError(t, err)
+	expectedSignature, err := ioutil.ReadFile("fixtures/extension-personal-personal.signature")
+	require.NoError(t, err)
+	assert.Equal(t, &extensionSignatureList{
+		Signatures: []extensionSignature{{
+			Version: extensionSignatureSchemaVersion,
+			Name:    manifestDigest.String() + "@809439d23da88df57186b0f2fce91e9a",
+			Type:    extensionSignatureTypeAtomic,
+			Content: expectedSignature,
+		}},
+	}, esl)
+
+	// TODO? Test the various failure modes.
+}

docker/docker_image.go

@@ -27,7 +27,7 @@ type Image struct {
 // a client to the registry hosting the given image.
 // The caller must call .Close() on the returned Image.
 func newImage(ctx context.Context, sys *types.SystemContext, ref dockerReference) (types.ImageCloser, error) {
-	s, err := newImageSource(ctx, sys, ref)
+	s, err := newImageSource(ctx, sys, ref, nil)
 	if err != nil {
 		return nil, err
 	}
@@ -60,7 +60,7 @@ func GetRepositoryTags(ctx context.Context, sys *types.SystemContext, ref types.
 	}
 
 	path := fmt.Sprintf(tagsPath, reference.Path(dr.ref))
-	client, err := newDockerClientFromRef(sys, dr, false, "pull")
+	client, err := newDockerClientFromRef(sys, dr, false, "pull", nil)
 	if err != nil {
 		return nil, errors.Wrap(err, "failed to create client")
 	}
@@ -124,7 +124,7 @@ func GetDigest(ctx context.Context, sys *types.SystemContext, ref types.ImageRef
 		return "", err
 	}
 
-	client, err := newDockerClientFromRef(sys, dr, false, "pull")
+	client, err := newDockerClientFromRef(sys, dr, false, "pull", nil)
 	if err != nil {
 		return "", errors.Wrap(err, "failed to create client")
 	}

docker/docker_image_dest.go

@@ -37,8 +37,8 @@ type dockerImageDestination struct {
 }
 
 // newImageDestination creates a new ImageDestination for the specified image reference.
-func newImageDestination(sys *types.SystemContext, ref dockerReference) (types.ImageDestination, error) {
-	c, err := newDockerClientFromRef(sys, ref, true, "pull,push")
+func newImageDestination(sys *types.SystemContext, ref dockerReference, httpWrapper httpWrapper) (*dockerImageDestination, error) {
+	c, err := newDockerClientFromRef(sys, ref, true, "pull,push", httpWrapper)
 	if err != nil {
 		return nil, err
 	}

docker/docker_image_dest_test.go

@@ -0,0 +1,80 @@
+package docker
+
+// Many of these tests are made using github.com/dnaeon/go-vcr/recorder.
+// See docker_client_test.go for more instructions.
+
+import (
+	"context"
+	"io/ioutil"
+	"testing"
+
+	"github.com/containers/image/v5/types"
+	"github.com/dnaeon/go-vcr/recorder"
+	digest "github.com/opencontainers/go-digest"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// TODO: Tests for quite a few methods.
+
+// vcrImageDestination creates a dockerImageDestination using a series of HTTP request/response recordings
+// using recordingBaseName.
+// It returns the imageDestination and a cleanup callback
+func vcrImageDestination(t *testing.T, ctx *types.SystemContext, recordingBaseName string, mode recorder.Mode,
+	ref string) (*dockerImageDestination, func()) {
+	ctx, httpWrapper, cleanup, dockerRef := prepareVCR(t, ctx, recordingBaseName, mode,
+		ref)
+
+	dest, err := newImageDestination(ctx, dockerRef, httpWrapper)
+	require.NoError(t, err)
+	return dest, cleanup
+}
+
+// See the comment above TestDockerClientGetExtensionsSignatures for instructions on setting up the recording.
+func TestDockerImageDestinationPutSignaturesToAPIExtension(t *testing.T) {
+	ctx := &types.SystemContext{
+		DockerAuthConfig: &types.DockerAuthConfig{
+			Username: "unused",
+			Password: "dh2juhu6LbGYGSHKMUa5BFEpyoPMYDVA59hxd3FCfbU",
+		},
+		DockerInsecureSkipTLSVerify: types.OptionalBoolTrue,
+	}
+	expectedSignature1, err := ioutil.ReadFile("fixtures/extension-personal-personal.signature")
+	require.NoError(t, err)
+
+	// Success
+	dest, cleanup := vcrImageDestination(t, ctx, "putSignaturesToAPIExtension-success", recorder.ModeReplaying,
+		"//localhost:5000/myns/personal:personal")
+	defer cleanup()
+	// The value can be obtained e.g. from (oc get istag personal:personal) value image.dockerImageReference in-container.
+	manifestDigest := digest.Digest("sha256:8d7fe3e157e56648ab790794970fbdfe82c84af79e807443b98df92c822a9b9b")
+	sig2 := []byte("This is not really a signature")
+	err = dest.putSignaturesToAPIExtension(context.Background(), [][]byte{sig2}, manifestDigest)
+	require.NoError(t, err)
+	// Verify that this preserves the original signature and creates a new one.
+	esl, err := dest.c.getExtensionsSignatures(context.Background(), dest.ref, manifestDigest)
+	require.NoError(t, err)
+	// We do not know what extensionSignature.Name has been randomly generated,
+	// so only verify that it has the expected format, and then replace it for the purposes of equality comparison.
+	require.Len(t, esl.Signatures, 2)
+	assert.Regexp(t, manifestDigest.String()+"@.{32}", esl.Signatures[1].Name)
+	assert.Equal(t, &extensionSignatureList{
+		Signatures: []extensionSignature{
+			{
+				Version: extensionSignatureSchemaVersion,
+				Name:    manifestDigest.String() + "@809439d23da88df57186b0f2fce91e9a",
+				Type:    extensionSignatureTypeAtomic,
+				Content: expectedSignature1,
+			},
+			{
+				Version: extensionSignatureSchemaVersion,
+				Name:    esl.Signatures[1].Name, // This is comparing the value with itself, i.e. ignoring the comparison; we have checked the format above.
+				Type:    extensionSignatureTypeAtomic,
+				Content: sig2,
+			},
+		},
+	}, esl)
+
+	// TODO? Test that unknown signature kinds are silently ignored.
+	// TODO? Test the various failure modes.
+}