Merge pull request #699 from RichardScothern/client-manifest-etags-clean

Allow conditional fetching of manifests with the registry client.

notifications/listener.go

@@ -93,8 +93,8 @@ func (msl *manifestServiceListener) Put(sm *manifest.SignedManifest) error {
 	return err
 }
 
-func (msl *manifestServiceListener) GetByTag(tag string) (*manifest.SignedManifest, error) {
+func (msl *manifestServiceListener) GetByTag(tag string, options ...distribution.ManifestServiceOption) (*manifest.SignedManifest, error) {
-	sm, err := msl.ManifestService.GetByTag(tag)
+	sm, err := msl.ManifestService.GetByTag(tag, options...)
 	if err == nil {
 		if err := msl.parent.listener.ManifestPulled(msl.parent.Repository.Name(), sm); err != nil {
 			logrus.Errorf("error dispatching manifest pull to listener: %v", err)

registry.go

@@ -37,6 +37,9 @@ type Namespace interface {
 	Repository(ctx context.Context, name string) (Repository, error)
 }
 
+// ManifestServiceOption is a function argument for Manifest Service methods
+type ManifestServiceOption func(ManifestService) error
+
 // Repository is a named collection of manifests and layers.
 type Repository interface {
 	// Name returns the name of the repository.
@@ -84,7 +87,7 @@ type ManifestService interface {
 	ExistsByTag(tag string) (bool, error)
 
 	// GetByTag retrieves the named manifest, if it exists.
-	GetByTag(tag string) (*manifest.SignedManifest, error)
+	GetByTag(tag string, options ...ManifestServiceOption) (*manifest.SignedManifest, error)
 
 	// TODO(stevvooe): There are several changes that need to be done to this
 	// interface:

registry/client/repository.go

@@ -75,6 +75,7 @@ func (r *repository) Manifests() distribution.ManifestService {
 		name:   r.Name(),
 		ub:     r.ub,
 		client: r.client,
+		etags:  make(map[string]string),
 	}
 }
 
@@ -104,6 +105,7 @@ type manifests struct {
 	name   string
 	ub     *v2.URLBuilder
 	client *http.Client
+	etags  map[string]string
 }
 
 func (ms *manifests) Tags() ([]string, error) {
@@ -173,13 +175,40 @@ func (ms *manifests) Get(dgst digest.Digest) (*manifest.SignedManifest, error) {
 	return ms.GetByTag(dgst.String())
 }
 
-func (ms *manifests) GetByTag(tag string) (*manifest.SignedManifest, error) {
+// AddEtagToTag allows a client to supply an eTag to GetByTag which will
+// be used for a conditional HTTP request.  If the eTag matches, a nil
+// manifest and nil error will be returned.
+func AddEtagToTag(tagName, dgst string) distribution.ManifestServiceOption {
+	return func(ms distribution.ManifestService) error {
+		if ms, ok := ms.(*manifests); ok {
+			ms.etags[tagName] = dgst
+			return nil
+		}
+		return fmt.Errorf("etag options is a client-only option")
+	}
+}
+
+func (ms *manifests) GetByTag(tag string, options ...distribution.ManifestServiceOption) (*manifest.SignedManifest, error) {
+	for _, option := range options {
+		err := option(ms)
+		if err != nil {
+			return nil, err
+		}
+	}
+
 	u, err := ms.ub.BuildManifestURL(ms.name, tag)
 	if err != nil {
 		return nil, err
 	}
+	req, err := http.NewRequest("GET", u, nil)
+	if err != nil {
+		return nil, err
+	}
 
-	resp, err := ms.client.Get(u)
+	if _, ok := ms.etags[tag]; ok {
+		req.Header.Set("eTag", ms.etags[tag])
+	}
+	resp, err := ms.client.Do(req)
 	if err != nil {
 		return nil, err
 	}
@@ -193,8 +222,9 @@ func (ms *manifests) GetByTag(tag string) (*manifest.SignedManifest, error) {
 		if err := decoder.Decode(&sm); err != nil {
 			return nil, err
 		}
-
 		return &sm, nil
+	case http.StatusNotModified:
+		return nil, nil
 	default:
 		return nil, handleErrorResponse(resp)
 	}

registry/client/repository_test.go

@@ -46,6 +46,7 @@ func newRandomBlob(size int) (digest.Digest, []byte) {
 }
 
 func addTestFetch(repo string, dgst digest.Digest, content []byte, m *testutil.RequestResponseMap) {
+
 	*m = append(*m, testutil.RequestResponseMapping{
 		Request: testutil.Request{
 			Method: "GET",
@@ -60,6 +61,7 @@ func addTestFetch(repo string, dgst digest.Digest, content []byte, m *testutil.R
 			}),
 		},
 	})
+
 	*m = append(*m, testutil.RequestResponseMapping{
 		Request: testutil.Request{
 			Method: "HEAD",
@@ -398,6 +400,40 @@ func newRandomSchemaV1Manifest(name, tag string, blobCount int) (*manifest.Signe
 	return m, dgst
 }
 
+func addTestManifestWithEtag(repo, reference string, content []byte, m *testutil.RequestResponseMap, dgst string) {
+	actualDigest, _ := digest.FromBytes(content)
+	getReqWithEtag := testutil.Request{
+		Method: "GET",
+		Route:  "/v2/" + repo + "/manifests/" + reference,
+		Headers: http.Header(map[string][]string{
+			"Etag": {dgst},
+		}),
+	}
+
+	var getRespWithEtag testutil.Response
+	if actualDigest.String() == dgst {
+		getRespWithEtag = testutil.Response{
+			StatusCode: http.StatusNotModified,
+			Body:       []byte{},
+			Headers: http.Header(map[string][]string{
+				"Content-Length": {"0"},
+				"Last-Modified":  {time.Now().Add(-1 * time.Second).Format(time.ANSIC)},
+			}),
+		}
+	} else {
+		getRespWithEtag = testutil.Response{
+			StatusCode: http.StatusOK,
+			Body:       content,
+			Headers: http.Header(map[string][]string{
+				"Content-Length": {fmt.Sprint(len(content))},
+				"Last-Modified":  {time.Now().Add(-1 * time.Second).Format(time.ANSIC)},
+			}),
+		}
+
+	}
+	*m = append(*m, testutil.RequestResponseMapping{Request: getReqWithEtag, Response: getRespWithEtag})
+}
+
 func addTestManifest(repo, reference string, content []byte, m *testutil.RequestResponseMap) {
 	*m = append(*m, testutil.RequestResponseMapping{
 		Request: testutil.Request{
@@ -487,11 +523,11 @@ func TestManifestFetch(t *testing.T) {
 	}
 }
 
-func TestManifestFetchByTag(t *testing.T) {
+func TestManifestFetchWithEtag(t *testing.T) {
 	repo := "test.example.com/repo/by/tag"
-	m1, _ := newRandomSchemaV1Manifest(repo, "latest", 6)
+	m1, d1 := newRandomSchemaV1Manifest(repo, "latest", 6)
 	var m testutil.RequestResponseMap
-	addTestManifest(repo, "latest", m1.Raw, &m)
+	addTestManifestWithEtag(repo, "latest", m1.Raw, &m, d1.String())
 
 	e, c := testServer(m)
 	defer c()
@@ -502,20 +538,12 @@ func TestManifestFetchByTag(t *testing.T) {
 	}
 
 	ms := r.Manifests()
-	ok, err := ms.ExistsByTag("latest")
+	m2, err := ms.GetByTag("latest", AddEtagToTag("latest", d1.String()))
 	if err != nil {
 		t.Fatal(err)
 	}
-	if !ok {
+	if m2 != nil {
-		t.Fatal("Manifest does not exist")
+		t.Fatal("Expected empty manifest for matching etag")
-	}
-
-	manifest, err := ms.GetByTag("latest")
-	if err != nil {
-		t.Fatal(err)
-	}
-	if err := checkEqualManifest(manifest, m1); err != nil {
-		t.Fatal(err)
 	}
 }
 

registry/storage/manifeststore.go

@@ -73,7 +73,14 @@ func (ms *manifestStore) ExistsByTag(tag string) (bool, error) {
 	return ms.tagStore.exists(tag)
 }
 
-func (ms *manifestStore) GetByTag(tag string) (*manifest.SignedManifest, error) {
+func (ms *manifestStore) GetByTag(tag string, options ...distribution.ManifestServiceOption) (*manifest.SignedManifest, error) {
+	for _, option := range options {
+		err := option(ms)
+		if err != nil {
+			return nil, err
+		}
+	}
+
 	context.GetLogger(ms.ctx).Debug("(*manifestStore).GetByTag")
 	dgst, err := ms.tagStore.resolve(tag)
 	if err != nil {

testutil/handler.go

@@ -21,8 +21,6 @@ type RequestResponseMapping struct {
 	Response Response
 }
 
-// TODO(bbland): add support for request headers
-
 // Request is a simplified http.Request object
 type Request struct {
 	// Method is the http method of the request, for example GET
@@ -36,6 +34,9 @@ type Request struct {
 
 	// Body is the byte contents of the http request
 	Body []byte
+
+	// Headers are the header for this request
+	Headers http.Header
 }
 
 func (r Request) String() string {
@@ -54,7 +55,22 @@ func (r Request) String() string {
 		}
 		queryString = "?" + strings.Join(queryParts, "&")
 	}
-	return fmt.Sprintf("%s %s%s\n%s", r.Method, r.Route, queryString, r.Body)
+	var headers []string
+	if len(r.Headers) > 0 {
+		var headerKeys []string
+		for k := range r.Headers {
+			headerKeys = append(headerKeys, k)
+		}
+		sort.Strings(headerKeys)
+
+		for _, k := range headerKeys {
+			for _, val := range r.Headers[k] {
+				headers = append(headers, fmt.Sprintf("%s:%s", k, val))
+			}
+		}
+
+	}
+	return fmt.Sprintf("%s %s%s\n%s\n%s", r.Method, r.Route, queryString, headers, r.Body)
 }
 
 // Response is a simplified http.Response object
@@ -101,6 +117,14 @@ func (app *testHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		Route:       r.URL.Path,
 		QueryParams: r.URL.Query(),
 		Body:        requestBody,
+		Headers:     make(map[string][]string),
+	}
+
+	// Add headers of interest here
+	for k, v := range r.Header {
+		if k == "Etag" {
+			request.Headers[k] = v
+		}
 	}
 
 	responses, ok := app.responseMap[request.String()]