From 924913b4c33ffec4d13a2854375b03c21b3b13d5 Mon Sep 17 00:00:00 2001 From: Richard Scothern Date: Fri, 18 Sep 2015 11:00:44 -0700 Subject: [PATCH 1/2] Avoid returning nil, nil when fetching a manifest by tag by introducing a new error ErrManifestNotModified which can be checked by clients. Signed-off-by: Richard Scothern --- errors.go | 7 ++++++- registry/client/repository.go | 2 +- registry/client/repository_test.go | 7 ++----- registry/proxy/proxymanifeststore.go | 4 ++-- 4 files changed, 11 insertions(+), 9 deletions(-) diff --git a/errors.go b/errors.go index 4511fde6..eb332d1b 100644 --- a/errors.go +++ b/errors.go @@ -1,15 +1,20 @@ package distribution import ( + "errors" "fmt" "strings" "github.com/docker/distribution/digest" ) +// ErrManifestNotModified is returned when a conditional manifest GetByTag +// returns nil due to the client indicating it has the latest version +var ErrManifestNotModified = errors.New("manifest not modified") + // ErrUnsupported is returned when an unimplemented or unsupported action is // performed -var ErrUnsupported = fmt.Errorf("operation unsupported") +var ErrUnsupported = errors.New("operation unsupported") // ErrRepositoryUnknown is returned if the named repository is not known by // the registry. diff --git a/registry/client/repository.go b/registry/client/repository.go index 2d198314..0fcb17dc 100644 --- a/registry/client/repository.go +++ b/registry/client/repository.go @@ -288,7 +288,7 @@ func (ms *manifests) GetByTag(tag string, options ...distribution.ManifestServic } defer resp.Body.Close() if resp.StatusCode == http.StatusNotModified { - return nil, nil + return nil, distribution.ErrManifestNotModified } else if SuccessStatus(resp.StatusCode) { var sm schema1.SignedManifest decoder := json.NewDecoder(resp.Body) diff --git a/registry/client/repository_test.go b/registry/client/repository_test.go index b211b1f9..6e4a017e 100644 --- a/registry/client/repository_test.go +++ b/registry/client/repository_test.go @@ -603,13 +603,10 @@ func TestManifestFetchWithEtag(t *testing.T) { t.Fatal(err) } - m2, err := ms.GetByTag("latest", AddEtagToTag("latest", d1.String())) - if err != nil { + _, err = ms.GetByTag("latest", AddEtagToTag("latest", d1.String())) + if err != distribution.ErrManifestNotModified { t.Fatal(err) } - if m2 != nil { - t.Fatal("Expected empty manifest for matching etag") - } } func TestManifestDelete(t *testing.T) { diff --git a/registry/proxy/proxymanifeststore.go b/registry/proxy/proxymanifeststore.go index 1400cf02..610d695e 100644 --- a/registry/proxy/proxymanifeststore.go +++ b/registry/proxy/proxymanifeststore.go @@ -102,11 +102,11 @@ func (pms proxyManifestStore) GetByTag(tag string, options ...distribution.Manif fromremote: var sm *schema1.SignedManifest sm, err = pms.remoteManifests.GetByTag(tag, client.AddEtagToTag(tag, localDigest.String())) - if err != nil { + if err != nil && err != distribution.ErrManifestNotModified { return nil, err } - if sm == nil { + if err == distribution.ErrManifestNotModified { context.GetLogger(pms.ctx).Debugf("Local manifest for %q is latest, dgst=%s", tag, localDigest.String()) return localManifest, nil } From f36ab5a834cdf9e569e47f4f6e5037a3ade2f711 Mon Sep 17 00:00:00 2001 From: Richard Scothern Date: Fri, 18 Sep 2015 11:26:34 -0700 Subject: [PATCH 2/2] Don't return a nil array and a nil error if the Tags endpoint cannot be found Signed-off-by: Richard Scothern --- registry/client/repository.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/registry/client/repository.go b/registry/client/repository.go index 0fcb17dc..1e189438 100644 --- a/registry/client/repository.go +++ b/registry/client/repository.go @@ -211,8 +211,6 @@ func (ms *manifests) Tags() ([]string, error) { } return tagsResponse.Tags, nil - } else if resp.StatusCode == http.StatusNotFound { - return nil, nil } return nil, handleErrorResponse(resp) }