From 48a2010ac32c21ef207a32877279d436cc39b4ae Mon Sep 17 00:00:00 2001 From: Richard Scothern Date: Tue, 14 Jul 2015 16:25:37 -0700 Subject: [PATCH] Allow conditional fetching of manifests with the registry client. Add a functional argument to pass a digest to (ManifestService).GetByTag(). If the digest matches an empty manifest and nil error are returned. See 1bc740b0d59a31cef7779dab7ddc522a4020de7c for server implementation. Signed-off-by: Richard Scothern --- notifications/listener.go | 4 +-- registry.go | 5 ++- registry/client/repository.go | 36 +++++++++++++++++-- registry/client/repository_test.go | 56 ++++++++++++++++++++++-------- registry/storage/manifeststore.go | 9 ++++- testutil/handler.go | 30 ++++++++++++++-- 6 files changed, 116 insertions(+), 24 deletions(-) diff --git a/notifications/listener.go b/notifications/listener.go index 9b2762cdd..7b3e6b21a 100644 --- a/notifications/listener.go +++ b/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) { - sm, err := msl.ManifestService.GetByTag(tag) +func (msl *manifestServiceListener) GetByTag(tag string, options ...distribution.ManifestServiceOption) (*manifest.SignedManifest, error) { + 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) diff --git a/registry.go b/registry.go index bdca8bc49..85940824e 100644 --- a/registry.go +++ b/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: diff --git a/registry/client/repository.go b/registry/client/repository.go index 4a66f70b7..1f360ec86 100644 --- a/registry/client/repository.go +++ b/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) } diff --git a/registry/client/repository_test.go b/registry/client/repository_test.go index 7dbe97cf7..26d92d8ea 100644 --- a/registry/client/repository_test.go +++ b/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 { - t.Fatal("Manifest does not exist") - } - - manifest, err := ms.GetByTag("latest") - if err != nil { - t.Fatal(err) - } - if err := checkEqualManifest(manifest, m1); err != nil { - t.Fatal(err) + if m2 != nil { + t.Fatal("Expected empty manifest for matching etag") } } diff --git a/registry/storage/manifeststore.go b/registry/storage/manifeststore.go index 07f8de3c8..8f6c35626 100644 --- a/registry/storage/manifeststore.go +++ b/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 { diff --git a/testutil/handler.go b/testutil/handler.go index 10850e241..cf5c1b7e2 100644 --- a/testutil/handler.go +++ b/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()]