From cd28f169066b928ed3933de1fe901a187794d4ff Mon Sep 17 00:00:00 2001 From: Richard Scothern Date: Thu, 4 Feb 2016 10:58:06 -0800 Subject: [PATCH 1/8] update version file Signed-off-by: Richard Scothern --- version/version.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/version/version.go b/version/version.go index 450d15c2..a46d1b0c 100644 --- a/version/version.go +++ b/version/version.go @@ -8,4 +8,4 @@ var Package = "github.com/docker/distribution" // the latest release tag by hand, always suffixed by "+unknown". During // build, it will be replaced by the actual version. The value here will be // used if the registry is run after a go get based install. -var Version = "v2.1.0+unknown" +var Version = "v2.3.0+unknown" From 099622876197e3b24d627a72053d1bcc8968076a Mon Sep 17 00:00:00 2001 From: Richard Scothern Date: Thu, 4 Feb 2016 10:36:15 -0800 Subject: [PATCH 2/8] Update maintainers and authors Signed-off-by: Richard Scothern --- .mailmap | 3 ++- AUTHORS | 9 +++++++++ MAINTAINERS | 5 +++++ 3 files changed, 16 insertions(+), 1 deletion(-) diff --git a/.mailmap b/.mailmap index 191e60cd..e4e50ee7 100644 --- a/.mailmap +++ b/.mailmap @@ -2,6 +2,7 @@ Stephen J Day Stephen Day Stephen Day Olivier Gambier Olivier Gambier Brian Bland Brian Bland +Brian Bland Brian Bland Josh Hawn Josh Hawn Richard Scothern Richard Richard Scothern Richard Scothern @@ -11,4 +12,4 @@ Jessie Frazelle Sharif Nassar Sharif Nassar Sven Dowideit Sven Dowideit Vincent Giersch Vincent Giersch -davidli davidli \ No newline at end of file +davidli davidli diff --git a/AUTHORS b/AUTHORS index 4b97cd78..a44266b0 100644 --- a/AUTHORS +++ b/AUTHORS @@ -21,6 +21,7 @@ Ben Firshman bin liu Brian Bland burnettk +Carson A Chris Dillon Daisuke Fujita Darren Shepherd @@ -33,11 +34,13 @@ davidli Dejan Golja Derek McGowan Diogo Mónica +DJ Enriquez Donald Huang Doug Davis farmerworking Florentin Raud Frederick F. Kautz IV +gabriell nascimento harche Henri Gomez Hu Keping @@ -55,7 +58,9 @@ Josh Hawn Julien Fernandez Kelsey Hightower Kenneth Lim +Kenny Leung Li Yi +Liu Hua Louis Kottmann Luke Carpenter Mary Anthony @@ -76,7 +81,9 @@ Olivier Jacques Patrick Devine Philip Misiowiec Richard Scothern +Rodolfo Carvalho Rusty Conover +Sean Boran Sebastiaan van Stijn Sharif Nassar Shawn Falkner-Horine @@ -93,11 +100,13 @@ Thomas Sjögren Tianon Gravi Tibor Vass Tonis Tiigi +Trevor Pounds Troels Thomsen Vincent Batts Vincent Demeester Vincent Giersch W. Trevor King +weiyuan.yl xg.song xiekeyang Yann ROBERT diff --git a/MAINTAINERS b/MAINTAINERS index bda40015..97f415db 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -32,6 +32,11 @@ Email = "aaron.lehmann@docker.com" GitHub = "aaronlehmann" + [people.brianbland] + Name = "Brian Bland" + Email = "brian.bland@docker.com" + GitHub = "BrianBland" + [people.dmcgowan] Name = "Derek McGowan" Email = "derek@mcgstyle.net" From 34c3acf8a8d800524ac06444290b26ed96d4dac0 Mon Sep 17 00:00:00 2001 From: Aaron Lehmann Date: Fri, 5 Feb 2016 16:00:12 -0800 Subject: [PATCH 3/8] Allow uppercase characters in hostnames This allows hostnames to contain uppercase characters, matching behavior in Docker versions before 1.10. It does not attempt to canonicalize hostnames into a lowercase format before parsing, since this could lead to corner cases (for example, making Hostname.Domain.Com/ref ambiguous on a daemon which contains references for both hostname.domain.com/ref and Hostname.Domain.Com/ref). Fixes: #1433 Signed-off-by: Aaron Lehmann --- reference/reference.go | 2 +- reference/regexp.go | 2 +- reference/regexp_test.go | 12 ++++++++++++ 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/reference/reference.go b/reference/reference.go index c188472a..6f079cbb 100644 --- a/reference/reference.go +++ b/reference/reference.go @@ -6,7 +6,7 @@ // reference := repository [ ":" tag ] [ "@" digest ] // name := [hostname '/'] component ['/' component]* // hostname := hostcomponent ['.' hostcomponent]* [':' port-number] -// hostcomponent := /([a-z0-9]|[a-z0-9][a-z0-9-]*[a-z0-9])/ +// hostcomponent := /([a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9-]*[a-zA-Z0-9])/ // port-number := /[0-9]+/ // component := alpha-numeric [separator alpha-numeric]* // alpha-numeric := /[a-z0-9]+/ diff --git a/reference/regexp.go b/reference/regexp.go index a4ffe5b6..b465abf5 100644 --- a/reference/regexp.go +++ b/reference/regexp.go @@ -22,7 +22,7 @@ var ( // hostnameComponentRegexp restricts the registry hostname component of a // repository name to start with a component as defined by hostnameRegexp // and followed by an optional port. - hostnameComponentRegexp = match(`(?:[a-z0-9]|[a-z0-9][a-z0-9-]*[a-z0-9])`) + hostnameComponentRegexp = match(`(?:[a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9-]*[a-zA-Z0-9])`) // hostnameRegexp defines the structure of potential hostname components // that may be part of image names. This is purposely a subset of what is diff --git a/reference/regexp_test.go b/reference/regexp_test.go index 33944918..2ec39377 100644 --- a/reference/regexp_test.go +++ b/reference/regexp_test.go @@ -111,6 +111,10 @@ func TestHostRegexp(t *testing.T) { input: "xn--n3h.com", // ☃.com in punycode match: true, }, + { + input: "Asdf.com", // uppercase character + match: true, + }, } r := regexp.MustCompile(`^` + hostnameRegexp.String() + `$`) for i := range hostcases { @@ -399,6 +403,14 @@ func TestFullNameRegexp(t *testing.T) { match: true, subs: []string{"registry.io", "foo/project--id.module--name.ver---sion--name"}, }, + { + input: "Asdf.com/foo/bar", // uppercase character in hostname + match: true, + }, + { + input: "Foo/FarB", // uppercase characters in remote name + match: false, + }, } for i := range testcases { checkRegexp(t, anchoredNameRegexp, testcases[i]) From d7eb5d118a6a14a6f320062296b1808506c35241 Mon Sep 17 00:00:00 2001 From: Derek McGowan Date: Tue, 9 Feb 2016 18:28:43 -0800 Subject: [PATCH 4/8] Fix schema1 manifest etag and docker content digest header When schema2 manifests are rewritten as schema1 currently the etag and docker content digest header keep the value for the schema2 manifest. Fixes #1444 Signed-off-by: Derek McGowan (github: dmcgowan) --- registry/handlers/api_test.go | 62 ++++++++++++++++++++++------------- registry/handlers/images.go | 1 + 2 files changed, 41 insertions(+), 22 deletions(-) diff --git a/registry/handlers/api_test.go b/registry/handlers/api_test.go index 5fffaa5a..1f18173f 100644 --- a/registry/handlers/api_test.go +++ b/registry/handlers/api_test.go @@ -1378,19 +1378,28 @@ func testManifestAPISchema2(t *testing.T, env *testEnv, imageName reference.Name } defer resp.Body.Close() - checkResponse(t, "fetching uploaded manifest as schema1", resp, http.StatusOK) - checkHeaders(t, resp, http.Header{ - "Docker-Content-Digest": []string{dgst.String()}, - "ETag": []string{fmt.Sprintf(`"%s"`, dgst)}, - }) - - var fetchedSchema1Manifest schema1.SignedManifest - dec = json.NewDecoder(resp.Body) - - if err := dec.Decode(&fetchedSchema1Manifest); err != nil { - t.Fatalf("error decoding fetched schema1 manifest: %v", err) + manifestBytes, err := ioutil.ReadAll(resp.Body) + if err != nil { + t.Fatalf("error reading response body: %v", err) } + checkResponse(t, "fetching uploaded manifest as schema1", resp, http.StatusOK) + + m, desc, err := distribution.UnmarshalManifest(schema1.MediaTypeManifest, manifestBytes) + if err != nil { + t.Fatalf("unexpected error unmarshalling manifest: %v", err) + } + + fetchedSchema1Manifest, ok := m.(*schema1.SignedManifest) + if !ok { + t.Fatalf("expecting schema1 manifest") + } + + checkHeaders(t, resp, http.Header{ + "Docker-Content-Digest": []string{desc.Digest.String()}, + "ETag": []string{fmt.Sprintf(`"%s"`, desc.Digest)}, + }) + if fetchedSchema1Manifest.Manifest.SchemaVersion != 1 { t.Fatal("wrong schema version") } @@ -1603,19 +1612,28 @@ func testManifestAPIManifestList(t *testing.T, env *testEnv, args manifestArgs) } defer resp.Body.Close() - checkResponse(t, "fetching uploaded manifest list as schema1", resp, http.StatusOK) - checkHeaders(t, resp, http.Header{ - "Docker-Content-Digest": []string{dgst.String()}, - "ETag": []string{fmt.Sprintf(`"%s"`, dgst)}, - }) - - var fetchedSchema1Manifest schema1.SignedManifest - dec = json.NewDecoder(resp.Body) - - if err := dec.Decode(&fetchedSchema1Manifest); err != nil { - t.Fatalf("error decoding fetched schema1 manifest: %v", err) + manifestBytes, err := ioutil.ReadAll(resp.Body) + if err != nil { + t.Fatalf("error reading response body: %v", err) } + checkResponse(t, "fetching uploaded manifest list as schema1", resp, http.StatusOK) + + m, desc, err := distribution.UnmarshalManifest(schema1.MediaTypeManifest, manifestBytes) + if err != nil { + t.Fatalf("unexpected error unmarshalling manifest: %v", err) + } + + fetchedSchema1Manifest, ok := m.(*schema1.SignedManifest) + if !ok { + t.Fatalf("expecting schema1 manifest") + } + + checkHeaders(t, resp, http.Header{ + "Docker-Content-Digest": []string{desc.Digest.String()}, + "ETag": []string{fmt.Sprintf(`"%s"`, desc.Digest)}, + }) + if fetchedSchema1Manifest.Manifest.SchemaVersion != 1 { t.Fatal("wrong schema version") } diff --git a/registry/handlers/images.go b/registry/handlers/images.go index 808ead54..b41037ba 100644 --- a/registry/handlers/images.go +++ b/registry/handlers/images.go @@ -196,6 +196,7 @@ func (imh *imageManifestHandler) convertSchema2Manifest(schema2Manifest *schema2 imh.Errors = append(imh.Errors, v2.ErrorCodeManifestInvalid.WithDetail(err)) return nil, err } + imh.Digest = digest.FromBytes(manifest.(*schema1.SignedManifest).Canonical) return manifest, nil } From d1c173078fe47f45c7f7b96098410d4f13dd68ab Mon Sep 17 00:00:00 2001 From: Derek McGowan Date: Wed, 10 Feb 2016 15:20:39 -0800 Subject: [PATCH 5/8] Add option to disable signatures Add option for specifying trust key for signing schema1 manifests. Since schema1 signature key identifiers are not verified anywhere and deprecated, storing signatures is no longer a requirement. Furthermore in schema2 there is no signature, requiring the registry to already add signatures to generated schema1 manifests. Signed-off-by: Derek McGowan (github: dmcgowan) --- configuration/configuration.go | 15 ++++++ manifest/schema1/manifest.go | 2 +- registry/handlers/app.go | 22 ++++++-- registry/storage/manifeststore_test.go | 61 +++++++++++++++++------ registry/storage/registry.go | 26 +++++++++- registry/storage/signedmanifesthandler.go | 39 +++++++++++---- 6 files changed, 131 insertions(+), 34 deletions(-) diff --git a/configuration/configuration.go b/configuration/configuration.go index 3dff32f8..62d914cd 100644 --- a/configuration/configuration.go +++ b/configuration/configuration.go @@ -145,6 +145,21 @@ type Configuration struct { Health Health `yaml:"health,omitempty"` Proxy Proxy `yaml:"proxy,omitempty"` + + // Compatibility is used for configurations of working with older or deprecated features. + Compatibility struct { + // Schema1 configures how schema1 manifests will be handled + Schema1 struct { + // TrustKey is the signing key to use for adding the signature to + // schema1 manifests. + TrustKey string `yaml:"signingkeyfile,omitempty"` + + // DisableSignatureStore will cause all signatures attached to schema1 manifests + // to be ignored. Signatures will be generated on all schema1 manifest requests + // rather than only requests which converted schema2 to schema1. + DisableSignatureStore bool `yaml:"disablesignaturestore,omitempty"` + } `yaml:"schema1,omitempty"` + } `yaml:"compatibility,omitempty"` } // LogHook is composed of hook Level and Type. diff --git a/manifest/schema1/manifest.go b/manifest/schema1/manifest.go index 160f9cd9..bff47bde 100644 --- a/manifest/schema1/manifest.go +++ b/manifest/schema1/manifest.go @@ -102,7 +102,7 @@ type SignedManifest struct { Canonical []byte `json:"-"` // all contains the byte representation of the Manifest including signatures - // and is retuend by Payload() + // and is returned by Payload() all []byte } diff --git a/registry/handlers/app.go b/registry/handlers/app.go index 70b7417f..95c72550 100644 --- a/registry/handlers/app.go +++ b/registry/handlers/app.go @@ -146,11 +146,18 @@ func NewApp(ctx context.Context, configuration *configuration.Configuration) *Ap app.configureRedis(configuration) app.configureLogHook(configuration) - // Generate an ephemeral key to be used for signing converted manifests - // for clients that don't support schema2. - app.trustKey, err = libtrust.GenerateECP256PrivateKey() - if err != nil { - panic(err) + if configuration.Compatibility.Schema1.TrustKey != "" { + app.trustKey, err = libtrust.LoadKeyFile(configuration.Compatibility.Schema1.TrustKey) + if err != nil { + panic(fmt.Sprintf(`could not load schema1 "signingkey" parameter: %v`, err)) + } + } else { + // Generate an ephemeral key to be used for signing converted manifests + // for clients that don't support schema2. + app.trustKey, err = libtrust.GenerateECP256PrivateKey() + if err != nil { + panic(err) + } } if configuration.HTTP.Host != "" { @@ -167,6 +174,11 @@ func NewApp(ctx context.Context, configuration *configuration.Configuration) *Ap options = append(options, storage.DisableDigestResumption) } + if configuration.Compatibility.Schema1.DisableSignatureStore { + options = append(options, storage.DisableSchema1Signatures) + options = append(options, storage.Schema1SigningKey(app.trustKey)) + } + // configure deletion if d, ok := configuration.Storage["delete"]; ok { e, ok := d["enabled"] diff --git a/registry/storage/manifeststore_test.go b/registry/storage/manifeststore_test.go index 7885c466..fcb5adf9 100644 --- a/registry/storage/manifeststore_test.go +++ b/registry/storage/manifeststore_test.go @@ -28,11 +28,10 @@ type manifestStoreTestEnv struct { tag string } -func newManifestStoreTestEnv(t *testing.T, name reference.Named, tag string) *manifestStoreTestEnv { +func newManifestStoreTestEnv(t *testing.T, name reference.Named, tag string, options ...RegistryOption) *manifestStoreTestEnv { ctx := context.Background() driver := inmemory.New() - registry, err := NewRegistry(ctx, driver, BlobDescriptorCacheProvider( - memory.NewInMemoryBlobDescriptorCacheProvider()), EnableDelete, EnableRedirect) + registry, err := NewRegistry(ctx, driver, options...) if err != nil { t.Fatalf("error creating registry: %v", err) } @@ -53,13 +52,26 @@ func newManifestStoreTestEnv(t *testing.T, name reference.Named, tag string) *ma } func TestManifestStorage(t *testing.T) { + testManifestStorage(t, BlobDescriptorCacheProvider(memory.NewInMemoryBlobDescriptorCacheProvider()), EnableDelete, EnableRedirect) +} + +func TestManifestStorageDisabledSignatures(t *testing.T) { + k, err := libtrust.GenerateECP256PrivateKey() + if err != nil { + t.Fatal(err) + } + testManifestStorage(t, BlobDescriptorCacheProvider(memory.NewInMemoryBlobDescriptorCacheProvider()), EnableDelete, EnableRedirect, DisableSchema1Signatures, Schema1SigningKey(k)) +} + +func testManifestStorage(t *testing.T, options ...RegistryOption) { repoName, _ := reference.ParseNamed("foo/bar") - env := newManifestStoreTestEnv(t, repoName, "thetag") + env := newManifestStoreTestEnv(t, repoName, "thetag", options...) ctx := context.Background() ms, err := env.repository.Manifests(ctx) if err != nil { t.Fatal(err) } + equalSignatures := env.registry.(*registry).schema1SignaturesEnabled m := schema1.Manifest{ Versioned: manifest.Versioned{ @@ -159,8 +171,14 @@ func TestManifestStorage(t *testing.T) { t.Fatalf("unexpected manifest type from signedstore") } - if !reflect.DeepEqual(fetchedManifest, sm) { - t.Fatalf("fetched manifest not equal: %#v != %#v", fetchedManifest, sm) + if !bytes.Equal(fetchedManifest.Canonical, sm.Canonical) { + t.Fatalf("fetched payload does not match original payload: %q != %q", fetchedManifest.Canonical, sm.Canonical) + } + + if equalSignatures { + if !reflect.DeepEqual(fetchedManifest, sm) { + t.Fatalf("fetched manifest not equal: %#v != %#v", fetchedManifest.Manifest, sm.Manifest) + } } _, pl, err := fetchedManifest.Payload() @@ -196,8 +214,19 @@ func TestManifestStorage(t *testing.T) { t.Fatalf("unexpected error fetching manifest by digest: %v", err) } - if !reflect.DeepEqual(fetchedByDigest, fetchedManifest) { - t.Fatalf("fetched manifest not equal: %#v != %#v", fetchedByDigest, fetchedManifest) + byDigestManifest, ok := fetchedByDigest.(*schema1.SignedManifest) + if !ok { + t.Fatalf("unexpected manifest type from signedstore") + } + + if !bytes.Equal(byDigestManifest.Canonical, fetchedManifest.Canonical) { + t.Fatalf("fetched manifest not equal: %q != %q", byDigestManifest.Canonical, fetchedManifest.Canonical) + } + + if equalSignatures { + if !reflect.DeepEqual(fetchedByDigest, fetchedManifest) { + t.Fatalf("fetched manifest not equal: %#v != %#v", fetchedByDigest, fetchedManifest) + } } sigs, err := fetchedJWS.Signatures() @@ -286,14 +315,16 @@ func TestManifestStorage(t *testing.T) { t.Fatalf("payloads are not equal") } - receivedSigs, err := receivedJWS.Signatures() - if err != nil { - t.Fatalf("error getting signatures: %v", err) - } + if equalSignatures { + receivedSigs, err := receivedJWS.Signatures() + if err != nil { + t.Fatalf("error getting signatures: %v", err) + } - for i, sig := range receivedSigs { - if !bytes.Equal(sig, expectedSigs[i]) { - t.Fatalf("mismatched signatures from remote: %v != %v", string(sig), string(expectedSigs[i])) + for i, sig := range receivedSigs { + if !bytes.Equal(sig, expectedSigs[i]) { + t.Fatalf("mismatched signatures from remote: %v != %v", string(sig), string(expectedSigs[i])) + } } } diff --git a/registry/storage/registry.go b/registry/storage/registry.go index be570cbc..26fadf02 100644 --- a/registry/storage/registry.go +++ b/registry/storage/registry.go @@ -6,6 +6,7 @@ import ( "github.com/docker/distribution/reference" "github.com/docker/distribution/registry/storage/cache" storagedriver "github.com/docker/distribution/registry/storage/driver" + "github.com/docker/libtrust" ) // registry is the top-level implementation of Registry for use in the storage @@ -17,6 +18,8 @@ type registry struct { blobDescriptorCacheProvider cache.BlobDescriptorCacheProvider deleteEnabled bool resumableDigestEnabled bool + schema1SignaturesEnabled bool + schema1SigningKey libtrust.PrivateKey } // RegistryOption is the type used for functional options for NewRegistry. @@ -43,6 +46,24 @@ func DisableDigestResumption(registry *registry) error { return nil } +// DisableSchema1Signatures is a functional option for NewRegistry. It disables +// signature storage and ensures all schema1 manifests will only be returned +// with a signature from a provided signing key. +func DisableSchema1Signatures(registry *registry) error { + registry.schema1SignaturesEnabled = false + return nil +} + +// Schema1SigningKey returns a functional option for NewRegistry. It sets the +// signing key for adding a signature to all schema1 manifests. This should be +// used in conjunction with disabling signature store. +func Schema1SigningKey(key libtrust.PrivateKey) RegistryOption { + return func(registry *registry) error { + registry.schema1SigningKey = key + return nil + } +} + // BlobDescriptorCacheProvider returns a functional option for // NewRegistry. It creates a cached blob statter for use by the // registry. @@ -85,8 +106,9 @@ func NewRegistry(ctx context.Context, driver storagedriver.StorageDriver, option statter: statter, pathFn: bs.path, }, - statter: statter, - resumableDigestEnabled: true, + statter: statter, + resumableDigestEnabled: true, + schema1SignaturesEnabled: true, } for _, option := range options { diff --git a/registry/storage/signedmanifesthandler.go b/registry/storage/signedmanifesthandler.go index 02663226..8e13dd93 100644 --- a/registry/storage/signedmanifesthandler.go +++ b/registry/storage/signedmanifesthandler.go @@ -25,10 +25,17 @@ var _ ManifestHandler = &signedManifestHandler{} func (ms *signedManifestHandler) Unmarshal(ctx context.Context, dgst digest.Digest, content []byte) (distribution.Manifest, error) { context.GetLogger(ms.ctx).Debug("(*signedManifestHandler).Unmarshal") - // Fetch the signatures for the manifest - signatures, err := ms.signatures.Get(dgst) - if err != nil { - return nil, err + + var ( + signatures [][]byte + err error + ) + if ms.repository.schema1SignaturesEnabled { + // Fetch the signatures for the manifest + signatures, err = ms.signatures.Get(dgst) + if err != nil { + return nil, err + } } jsig, err := libtrust.NewJSONSignature(content, signatures...) @@ -36,6 +43,14 @@ func (ms *signedManifestHandler) Unmarshal(ctx context.Context, dgst digest.Dige return nil, err } + if ms.repository.schema1SigningKey != nil { + if err := jsig.Sign(ms.repository.schema1SigningKey); err != nil { + return nil, err + } + } else if !ms.repository.schema1SignaturesEnabled { + return nil, fmt.Errorf("missing signing key with signature store disabled") + } + // Extract the pretty JWS raw, err := jsig.PrettySignature("signatures") if err != nil { @@ -75,14 +90,16 @@ func (ms *signedManifestHandler) Put(ctx context.Context, manifest distribution. return "", err } - // Grab each json signature and store them. - signatures, err := sm.Signatures() - if err != nil { - return "", err - } + if ms.repository.schema1SignaturesEnabled { + // Grab each json signature and store them. + signatures, err := sm.Signatures() + if err != nil { + return "", err + } - if err := ms.signatures.Put(revision.Digest, signatures...); err != nil { - return "", err + if err := ms.signatures.Put(revision.Digest, signatures...); err != nil { + return "", err + } } return revision.Digest, nil From 740ed699f40e1522faacb2a706e44fa1560af8ea Mon Sep 17 00:00:00 2001 From: Richard Scothern Date: Wed, 10 Feb 2016 18:07:28 -0800 Subject: [PATCH 6/8] To avoid any network use unless necessary, delay establishing authorization challenges with the upstream until any proxied data is found not to be local. Implement auth challenges behind an interface and add to unit tests. Also, remove a non-sensical unit test. Signed-off-by: Richard Scothern --- registry/proxy/proxyauth.go | 9 +-- registry/proxy/proxyblobstore.go | 9 +++ registry/proxy/proxyblobstore_test.go | 6 ++ registry/proxy/proxymanifeststore.go | 9 ++- registry/proxy/proxymanifeststore_test.go | 37 +++++++++-- registry/proxy/proxyregistry.go | 80 ++++++++++++++++++----- registry/proxy/proxytagservice.go | 27 +++++--- registry/proxy/proxytagservice_test.go | 23 ++++++- 8 files changed, 156 insertions(+), 44 deletions(-) diff --git a/registry/proxy/proxyauth.go b/registry/proxy/proxyauth.go index e4bec75a..bcfa7aab 100644 --- a/registry/proxy/proxyauth.go +++ b/registry/proxy/proxyauth.go @@ -8,6 +8,7 @@ import ( ) const tokenURL = "https://auth.docker.io/token" +const challengeHeader = "Docker-Distribution-Api-Version" type userpass struct { username string @@ -24,12 +25,8 @@ func (c credentials) Basic(u *url.URL) (string, string) { return up.username, up.password } -// ConfigureAuth authorizes with the upstream registry -func ConfigureAuth(remoteURL, username, password string, cm auth.ChallengeManager) (auth.CredentialStore, error) { - if err := ping(cm, remoteURL+"/v2/", "Docker-Distribution-Api-Version"); err != nil { - return nil, err - } - +// ConfigureAuth stores credentials for challenge responses +func configureAuth(username, password string) (auth.CredentialStore, error) { creds := map[string]userpass{ tokenURL: { username: username, diff --git a/registry/proxy/proxyblobstore.go b/registry/proxy/proxyblobstore.go index 1d7dfbc6..5f1a9c50 100644 --- a/registry/proxy/proxyblobstore.go +++ b/registry/proxy/proxyblobstore.go @@ -22,6 +22,7 @@ type proxyBlobStore struct { remoteStore distribution.BlobService scheduler *scheduler.TTLExpirationScheduler repositoryName reference.Named + authChallenger authChallenger } var _ distribution.BlobStore = &proxyBlobStore{} @@ -121,6 +122,10 @@ func (pbs *proxyBlobStore) ServeBlob(ctx context.Context, w http.ResponseWriter, return nil } + if err := pbs.authChallenger.tryEstablishChallenges(ctx); err != nil { + return err + } + mu.Lock() _, ok := inflight[dgst] if ok { @@ -162,6 +167,10 @@ func (pbs *proxyBlobStore) Stat(ctx context.Context, dgst digest.Digest) (distri return distribution.Descriptor{}, err } + if err := pbs.authChallenger.tryEstablishChallenges(ctx); err != nil { + return distribution.Descriptor{}, err + } + return pbs.remoteStore.Stat(ctx, dgst) } diff --git a/registry/proxy/proxyblobstore_test.go b/registry/proxy/proxyblobstore_test.go index 3054ef0b..4d63aa42 100644 --- a/registry/proxy/proxyblobstore_test.go +++ b/registry/proxy/proxyblobstore_test.go @@ -168,6 +168,7 @@ func makeTestEnv(t *testing.T, name string) *testEnv { remoteStore: truthBlobs, localStore: localBlobs, scheduler: s, + authChallenger: &mockChallenger{}, } te := &testEnv{ @@ -242,6 +243,11 @@ func TestProxyStoreStat(t *testing.T) { if (*remoteStats)["stat"] != remoteBlobCount { t.Errorf("Unexpected remote stat count") } + + if te.store.authChallenger.(*mockChallenger).count != len(te.inRemote) { + t.Fatalf("Unexpected auth challenge count, got %#v", te.store.authChallenger) + } + } func TestProxyStoreServeHighConcurrency(t *testing.T) { diff --git a/registry/proxy/proxymanifeststore.go b/registry/proxy/proxymanifeststore.go index 0b5532d4..b8109667 100644 --- a/registry/proxy/proxymanifeststore.go +++ b/registry/proxy/proxymanifeststore.go @@ -19,6 +19,7 @@ type proxyManifestStore struct { remoteManifests distribution.ManifestService repositoryName reference.Named scheduler *scheduler.TTLExpirationScheduler + authChallenger authChallenger } var _ distribution.ManifestService = &proxyManifestStore{} @@ -31,7 +32,9 @@ func (pms proxyManifestStore) Exists(ctx context.Context, dgst digest.Digest) (b if exists { return true, nil } - + if err := pms.authChallenger.tryEstablishChallenges(ctx); err != nil { + return false, err + } return pms.remoteManifests.Exists(ctx, dgst) } @@ -41,6 +44,10 @@ func (pms proxyManifestStore) Get(ctx context.Context, dgst digest.Digest, optio var fromRemote bool manifest, err := pms.localManifests.Get(ctx, dgst, options...) if err != nil { + if err := pms.authChallenger.tryEstablishChallenges(ctx); err != nil { + return nil, err + } + manifest, err = pms.remoteManifests.Get(ctx, dgst, options...) if err != nil { return nil, err diff --git a/registry/proxy/proxymanifeststore_test.go b/registry/proxy/proxymanifeststore_test.go index 00f9daf9..e16fa6f5 100644 --- a/registry/proxy/proxymanifeststore_test.go +++ b/registry/proxy/proxymanifeststore_test.go @@ -2,6 +2,7 @@ package proxy import ( "io" + "sync" "testing" "github.com/docker/distribution" @@ -64,6 +65,20 @@ func (sm statsManifest) Put(ctx context.Context, manifest distribution.Manifest, } */ +type mockChallenger struct { + sync.Mutex + count int +} + +// Called for remote operations only +func (mc *mockChallenger) tryEstablishChallenges(context.Context) error { + mc.Lock() + defer mc.Unlock() + + mc.count++ + return nil +} + func newManifestStoreTestEnv(t *testing.T, name, tag string) *manifestStoreTestEnv { nameRef, err := reference.ParseNamed(name) if err != nil { @@ -120,6 +135,7 @@ func newManifestStoreTestEnv(t *testing.T, name, tag string) *manifestStoreTestE remoteManifests: truthManifests, scheduler: s, repositoryName: nameRef, + authChallenger: &mockChallenger{}, }, } } @@ -198,6 +214,10 @@ func TestProxyManifests(t *testing.T) { t.Errorf("Unexpected exists count : \n%v \n%v", localStats, remoteStats) } + if env.manifests.authChallenger.(*mockChallenger).count != 1 { + t.Fatalf("Expected 1 auth challenge, got %#v", env.manifests.authChallenger) + } + // Get - should succeed and pull manifest into local _, err = env.manifests.Get(ctx, env.manifestDigest) if err != nil { @@ -212,6 +232,10 @@ func TestProxyManifests(t *testing.T) { t.Errorf("Expected local put") } + if env.manifests.authChallenger.(*mockChallenger).count != 2 { + t.Fatalf("Expected 2 auth challenges, got %#v", env.manifests.authChallenger) + } + // Stat - should only go to local exists, err = env.manifests.Exists(ctx, env.manifestDigest) if err != nil { @@ -225,17 +249,18 @@ func TestProxyManifests(t *testing.T) { t.Errorf("Unexpected exists count") } - // Get - should get from remote, to test freshness + if env.manifests.authChallenger.(*mockChallenger).count != 2 { + t.Fatalf("Expected 2 auth challenges, got %#v", env.manifests.authChallenger) + } + + // Get proxied - won't require another authchallenge _, err = env.manifests.Get(ctx, env.manifestDigest) if err != nil { t.Fatal(err) } - if (*remoteStats)["get"] != 2 && (*remoteStats)["exists"] != 1 && (*localStats)["put"] != 1 { - t.Errorf("Unexpected get count") + if env.manifests.authChallenger.(*mockChallenger).count != 2 { + t.Fatalf("Expected 2 auth challenges, got %#v", env.manifests.authChallenger) } -} - -func TestProxyTagService(t *testing.T) { } diff --git a/registry/proxy/proxyregistry.go b/registry/proxy/proxyregistry.go index 43c1486e..0f5248ff 100644 --- a/registry/proxy/proxyregistry.go +++ b/registry/proxy/proxyregistry.go @@ -1,10 +1,11 @@ package proxy import ( + "fmt" "net/http" "net/url" + "sync" - "fmt" "github.com/docker/distribution" "github.com/docker/distribution/configuration" "github.com/docker/distribution/context" @@ -19,13 +20,10 @@ import ( // proxyingRegistry fetches content from a remote registry and caches it locally type proxyingRegistry struct { - embedded distribution.Namespace // provides local registry functionality - - scheduler *scheduler.TTLExpirationScheduler - - remoteURL string - credentialStore auth.CredentialStore - challengeManager auth.ChallengeManager + embedded distribution.Namespace // provides local registry functionality + scheduler *scheduler.TTLExpirationScheduler + remoteURL string + authChallenger authChallenger } // NewRegistryPullThroughCache creates a registry acting as a pull through cache @@ -93,18 +91,20 @@ func NewRegistryPullThroughCache(ctx context.Context, registry distribution.Name return nil, err } - challengeManager := auth.NewSimpleChallengeManager() - cs, err := ConfigureAuth(config.RemoteURL, config.Username, config.Password, challengeManager) + cs, err := configureAuth(config.Username, config.Password) if err != nil { return nil, err } return &proxyingRegistry{ - embedded: registry, - scheduler: s, - challengeManager: challengeManager, - credentialStore: cs, - remoteURL: config.RemoteURL, + embedded: registry, + scheduler: s, + remoteURL: config.RemoteURL, + authChallenger: &remoteAuthChallenger{ + remoteURL: config.RemoteURL, + challengeManager: auth.NewSimpleChallengeManager(), + credentialStore: cs, + }, }, nil } @@ -117,8 +117,13 @@ func (pr *proxyingRegistry) Repositories(ctx context.Context, repos []string, la } func (pr *proxyingRegistry) Repository(ctx context.Context, name reference.Named) (distribution.Repository, error) { + hcm, ok := pr.authChallenger.(*remoteAuthChallenger) + if !ok { + return nil, fmt.Errorf("unexpected challenge manager type %T", pr.authChallenger) + } + tr := transport.NewTransport(http.DefaultTransport, - auth.NewAuthorizer(pr.challengeManager, auth.NewTokenHandler(http.DefaultTransport, pr.credentialStore, name.Name(), "pull"))) + auth.NewAuthorizer(hcm.challengeManager, auth.NewTokenHandler(http.DefaultTransport, hcm.credentialStore, name.Name(), "pull"))) localRepo, err := pr.embedded.Repository(ctx, name) if err != nil { @@ -145,6 +150,7 @@ func (pr *proxyingRegistry) Repository(ctx context.Context, name reference.Named remoteStore: remoteRepo.Blobs(ctx), scheduler: pr.scheduler, repositoryName: name, + authChallenger: pr.authChallenger, }, manifests: &proxyManifestStore{ repositoryName: name, @@ -152,15 +158,53 @@ func (pr *proxyingRegistry) Repository(ctx context.Context, name reference.Named remoteManifests: remoteManifests, ctx: ctx, scheduler: pr.scheduler, + authChallenger: pr.authChallenger, }, name: name, tags: &proxyTagService{ - localTags: localRepo.Tags(ctx), - remoteTags: remoteRepo.Tags(ctx), + localTags: localRepo.Tags(ctx), + remoteTags: remoteRepo.Tags(ctx), + authChallenger: pr.authChallenger, }, }, nil } +// authChallenger encapsulates a request to the upstream to establish credential challenges +type authChallenger interface { + tryEstablishChallenges(context.Context) error +} + +type remoteAuthChallenger struct { + remoteURL string + sync.Mutex + challengeManager auth.ChallengeManager + credentialStore auth.CredentialStore +} + +// tryEstablishChallenges will attempt to get a challenge types for the upstream if none currently exist +func (hcm *remoteAuthChallenger) tryEstablishChallenges(ctx context.Context) error { + hcm.Lock() + defer hcm.Unlock() + + remoteURL := hcm.remoteURL + "/v2/" + challenges, err := hcm.challengeManager.GetChallenges(remoteURL) + if err != nil { + return err + } + + if len(challenges) > 0 { + return nil + } + + // establish challenge type with upstream + if err := ping(hcm.challengeManager, remoteURL, challengeHeader); err != nil { + return err + } + + context.GetLogger(ctx).Infof("Challenge established with upstream : %s %s", remoteURL, hcm.challengeManager) + return nil +} + // proxiedRepository uses proxying blob and manifest services to serve content // locally, or pulling it through from a remote and caching it locally if it doesn't // already exist diff --git a/registry/proxy/proxytagservice.go b/registry/proxy/proxytagservice.go index c52460c4..a8273030 100644 --- a/registry/proxy/proxytagservice.go +++ b/registry/proxy/proxytagservice.go @@ -7,8 +7,9 @@ import ( // proxyTagService supports local and remote lookup of tags. type proxyTagService struct { - localTags distribution.TagService - remoteTags distribution.TagService + localTags distribution.TagService + remoteTags distribution.TagService + authChallenger authChallenger } var _ distribution.TagService = proxyTagService{} @@ -17,16 +18,19 @@ var _ distribution.TagService = proxyTagService{} // tag service first and then caching it locally. If the remote is unavailable // the local association is returned func (pt proxyTagService) Get(ctx context.Context, tag string) (distribution.Descriptor, error) { - desc, err := pt.remoteTags.Get(ctx, tag) + err := pt.authChallenger.tryEstablishChallenges(ctx) if err == nil { - err := pt.localTags.Tag(ctx, tag, desc) - if err != nil { - return distribution.Descriptor{}, err + desc, err := pt.remoteTags.Get(ctx, tag) + if err == nil { + err := pt.localTags.Tag(ctx, tag, desc) + if err != nil { + return distribution.Descriptor{}, err + } + return desc, nil } - return desc, nil } - desc, err = pt.localTags.Get(ctx, tag) + desc, err := pt.localTags.Get(ctx, tag) if err != nil { return distribution.Descriptor{}, err } @@ -46,9 +50,12 @@ func (pt proxyTagService) Untag(ctx context.Context, tag string) error { } func (pt proxyTagService) All(ctx context.Context) ([]string, error) { - tags, err := pt.remoteTags.All(ctx) + err := pt.authChallenger.tryEstablishChallenges(ctx) if err == nil { - return tags, err + tags, err := pt.remoteTags.All(ctx) + if err == nil { + return tags, err + } } return pt.localTags.All(ctx) } diff --git a/registry/proxy/proxytagservice_test.go b/registry/proxy/proxytagservice_test.go index 8d9518c0..a446645c 100644 --- a/registry/proxy/proxytagservice_test.go +++ b/registry/proxy/proxytagservice_test.go @@ -69,8 +69,9 @@ func testProxyTagService(local, remote map[string]distribution.Descriptor) *prox remote = make(map[string]distribution.Descriptor) } return &proxyTagService{ - localTags: &mockTagStore{mapping: local}, - remoteTags: &mockTagStore{mapping: remote}, + localTags: &mockTagStore{mapping: local}, + remoteTags: &mockTagStore{mapping: remote}, + authChallenger: &mockChallenger{}, } } @@ -87,6 +88,10 @@ func TestGet(t *testing.T) { t.Fatal(err) } + if proxyTags.authChallenger.(*mockChallenger).count != 1 { + t.Fatalf("Expected 1 auth challenge call, got %#v", proxyTags.authChallenger) + } + if d != remoteDesc { t.Fatal("unable to get put tag") } @@ -112,6 +117,10 @@ func TestGet(t *testing.T) { t.Fatal(err) } + if proxyTags.authChallenger.(*mockChallenger).count != 2 { + t.Fatalf("Expected 2 auth challenge calls, got %#v", proxyTags.authChallenger) + } + if d != newRemoteDesc { t.Fatal("unable to get put tag") } @@ -142,7 +151,11 @@ func TestGet(t *testing.T) { t.Fatal("untagged tag should be pulled through") } - // Add another tag. Ensure both tags appear in enumerate + if proxyTags.authChallenger.(*mockChallenger).count != 3 { + t.Fatalf("Expected 3 auth challenge calls, got %#v", proxyTags.authChallenger) + } + + // Add another tag. Ensure both tags appear in 'All' err = proxyTags.remoteTags.Tag(ctx, "funtag", distribution.Descriptor{Size: 42}) if err != nil { t.Fatal(err) @@ -161,4 +174,8 @@ func TestGet(t *testing.T) { if all[0] != "funtag" && all[1] != "remote" { t.Fatalf("Unexpected tags returned from All() : %v ", all) } + + if proxyTags.authChallenger.(*mockChallenger).count != 4 { + t.Fatalf("Expected 4 auth challenge calls, got %#v", proxyTags.authChallenger) + } } From 16445b67679e91eab408a40a34625aa1f4dabfd1 Mon Sep 17 00:00:00 2001 From: Richard Scothern Date: Wed, 17 Feb 2016 10:42:34 -0800 Subject: [PATCH 7/8] Extend authChallenger interface to remove type cast. Signed-off-by: Richard Scothern --- registry/proxy/proxyauth.go | 2 +- registry/proxy/proxymanifeststore_test.go | 17 ++++++--- registry/proxy/proxyregistry.go | 43 +++++++++++++---------- 3 files changed, 39 insertions(+), 23 deletions(-) diff --git a/registry/proxy/proxyauth.go b/registry/proxy/proxyauth.go index bcfa7aab..6f0eb005 100644 --- a/registry/proxy/proxyauth.go +++ b/registry/proxy/proxyauth.go @@ -25,7 +25,7 @@ func (c credentials) Basic(u *url.URL) (string, string) { return up.username, up.password } -// ConfigureAuth stores credentials for challenge responses +// configureAuth stores credentials for challenge responses func configureAuth(username, password string) (auth.CredentialStore, error) { creds := map[string]userpass{ tokenURL: { diff --git a/registry/proxy/proxymanifeststore_test.go b/registry/proxy/proxymanifeststore_test.go index e16fa6f5..312eb343 100644 --- a/registry/proxy/proxymanifeststore_test.go +++ b/registry/proxy/proxymanifeststore_test.go @@ -11,6 +11,7 @@ import ( "github.com/docker/distribution/manifest" "github.com/docker/distribution/manifest/schema1" "github.com/docker/distribution/reference" + "github.com/docker/distribution/registry/client/auth" "github.com/docker/distribution/registry/proxy/scheduler" "github.com/docker/distribution/registry/storage" "github.com/docker/distribution/registry/storage/cache/memory" @@ -71,11 +72,19 @@ type mockChallenger struct { } // Called for remote operations only -func (mc *mockChallenger) tryEstablishChallenges(context.Context) error { - mc.Lock() - defer mc.Unlock() +func (m *mockChallenger) tryEstablishChallenges(context.Context) error { + m.Lock() + defer m.Unlock() - mc.count++ + m.count++ + return nil +} + +func (m *mockChallenger) credentialStore() auth.CredentialStore { + return nil +} + +func (m *mockChallenger) challengeManager() auth.ChallengeManager { return nil } diff --git a/registry/proxy/proxyregistry.go b/registry/proxy/proxyregistry.go index 0f5248ff..0c51a022 100644 --- a/registry/proxy/proxyregistry.go +++ b/registry/proxy/proxyregistry.go @@ -101,9 +101,9 @@ func NewRegistryPullThroughCache(ctx context.Context, registry distribution.Name scheduler: s, remoteURL: config.RemoteURL, authChallenger: &remoteAuthChallenger{ - remoteURL: config.RemoteURL, - challengeManager: auth.NewSimpleChallengeManager(), - credentialStore: cs, + remoteURL: config.RemoteURL, + cm: auth.NewSimpleChallengeManager(), + cs: cs, }, }, nil } @@ -117,13 +117,10 @@ func (pr *proxyingRegistry) Repositories(ctx context.Context, repos []string, la } func (pr *proxyingRegistry) Repository(ctx context.Context, name reference.Named) (distribution.Repository, error) { - hcm, ok := pr.authChallenger.(*remoteAuthChallenger) - if !ok { - return nil, fmt.Errorf("unexpected challenge manager type %T", pr.authChallenger) - } + c := pr.authChallenger tr := transport.NewTransport(http.DefaultTransport, - auth.NewAuthorizer(hcm.challengeManager, auth.NewTokenHandler(http.DefaultTransport, hcm.credentialStore, name.Name(), "pull"))) + auth.NewAuthorizer(c.challengeManager(), auth.NewTokenHandler(http.DefaultTransport, c.credentialStore(), name.Name(), "pull"))) localRepo, err := pr.embedded.Repository(ctx, name) if err != nil { @@ -172,22 +169,32 @@ func (pr *proxyingRegistry) Repository(ctx context.Context, name reference.Named // authChallenger encapsulates a request to the upstream to establish credential challenges type authChallenger interface { tryEstablishChallenges(context.Context) error + challengeManager() auth.ChallengeManager + credentialStore() auth.CredentialStore } type remoteAuthChallenger struct { remoteURL string sync.Mutex - challengeManager auth.ChallengeManager - credentialStore auth.CredentialStore + cm auth.ChallengeManager + cs auth.CredentialStore } -// tryEstablishChallenges will attempt to get a challenge types for the upstream if none currently exist -func (hcm *remoteAuthChallenger) tryEstablishChallenges(ctx context.Context) error { - hcm.Lock() - defer hcm.Unlock() +func (r *remoteAuthChallenger) credentialStore() auth.CredentialStore { + return r.cs +} - remoteURL := hcm.remoteURL + "/v2/" - challenges, err := hcm.challengeManager.GetChallenges(remoteURL) +func (r *remoteAuthChallenger) challengeManager() auth.ChallengeManager { + return r.cm +} + +// tryEstablishChallenges will attempt to get a challenge type for the upstream if none currently exist +func (r *remoteAuthChallenger) tryEstablishChallenges(ctx context.Context) error { + r.Lock() + defer r.Unlock() + + remoteURL := r.remoteURL + "/v2/" + challenges, err := r.cm.GetChallenges(remoteURL) if err != nil { return err } @@ -197,11 +204,11 @@ func (hcm *remoteAuthChallenger) tryEstablishChallenges(ctx context.Context) err } // establish challenge type with upstream - if err := ping(hcm.challengeManager, remoteURL, challengeHeader); err != nil { + if err := ping(r.cm, remoteURL, challengeHeader); err != nil { return err } - context.GetLogger(ctx).Infof("Challenge established with upstream : %s %s", remoteURL, hcm.challengeManager) + context.GetLogger(ctx).Infof("Challenge established with upstream : %s %s", remoteURL, r.cm) return nil } From 36936218c2e6a4527fc99a5c04126bb1f4c7d55c Mon Sep 17 00:00:00 2001 From: Richard Scothern Date: Wed, 17 Feb 2016 16:32:23 -0800 Subject: [PATCH 8/8] Enable proxying registries to downgrade fetched manifests to Schema 1. Ensure Accept headers are sent with TagService.Get (which hits manifest endpoints). Add support for remote Get and Put for the proxied blobstore. Signed-off-by: Richard Scothern --- registry/client/repository.go | 24 ++++++++++++++++--- registry/proxy/proxyblobstore.go | 26 ++++++++++++++++---- registry/proxy/proxyblobstore_test.go | 34 +++++++++++++++++++++++++++ 3 files changed, 77 insertions(+), 7 deletions(-) diff --git a/registry/client/repository.go b/registry/client/repository.go index 1f777add..b5a03ca0 100644 --- a/registry/client/repository.go +++ b/registry/client/repository.go @@ -257,9 +257,18 @@ func (t *tags) Get(ctx context.Context, tag string) (distribution.Descriptor, er if err != nil { return distribution.Descriptor{}, err } - var attempts int - resp, err := t.client.Head(u) + req, err := http.NewRequest("HEAD", u, nil) + if err != nil { + return distribution.Descriptor{}, err + } + + for _, t := range distribution.ManifestMediaTypes() { + req.Header.Add("Accept", t) + } + + var attempts int + resp, err := t.client.Do(req) check: if err != nil { return distribution.Descriptor{}, err @@ -269,7 +278,16 @@ check: case resp.StatusCode >= 200 && resp.StatusCode < 400: return descriptorFromResponse(resp) case resp.StatusCode == http.StatusMethodNotAllowed: - resp, err = t.client.Get(u) + req, err = http.NewRequest("GET", u, nil) + if err != nil { + return distribution.Descriptor{}, err + } + + for _, t := range distribution.ManifestMediaTypes() { + req.Header.Add("Accept", t) + } + + resp, err = t.client.Do(req) attempts++ if attempts > 1 { return distribution.Descriptor{}, err diff --git a/registry/proxy/proxyblobstore.go b/registry/proxy/proxyblobstore.go index 5f1a9c50..7a6d7ea2 100644 --- a/registry/proxy/proxyblobstore.go +++ b/registry/proxy/proxyblobstore.go @@ -174,6 +174,28 @@ func (pbs *proxyBlobStore) Stat(ctx context.Context, dgst digest.Digest) (distri return pbs.remoteStore.Stat(ctx, dgst) } +func (pbs *proxyBlobStore) Get(ctx context.Context, dgst digest.Digest) ([]byte, error) { + blob, err := pbs.localStore.Get(ctx, dgst) + if err == nil { + return blob, nil + } + + if err := pbs.authChallenger.tryEstablishChallenges(ctx); err != nil { + return []byte{}, err + } + + blob, err = pbs.remoteStore.Get(ctx, dgst) + if err != nil { + return []byte{}, err + } + + _, err = pbs.localStore.Put(ctx, "", blob) + if err != nil { + return []byte{}, err + } + return blob, nil +} + // Unsupported functions func (pbs *proxyBlobStore) Put(ctx context.Context, mediaType string, p []byte) (distribution.Descriptor, error) { return distribution.Descriptor{}, distribution.ErrUnsupported @@ -195,10 +217,6 @@ func (pbs *proxyBlobStore) Open(ctx context.Context, dgst digest.Digest) (distri return nil, distribution.ErrUnsupported } -func (pbs *proxyBlobStore) Get(ctx context.Context, dgst digest.Digest) ([]byte, error) { - return nil, distribution.ErrUnsupported -} - func (pbs *proxyBlobStore) Delete(ctx context.Context, dgst digest.Digest) error { return distribution.ErrUnsupported } diff --git a/registry/proxy/proxyblobstore_test.go b/registry/proxy/proxyblobstore_test.go index 4d63aa42..b93b5343 100644 --- a/registry/proxy/proxyblobstore_test.go +++ b/registry/proxy/proxyblobstore_test.go @@ -218,6 +218,40 @@ func populate(t *testing.T, te *testEnv, blobCount, size, numUnique int) { te.inRemote = inRemote te.numUnique = numUnique } +func TestProxyStoreGet(t *testing.T) { + te := makeTestEnv(t, "foo/bar") + + localStats := te.LocalStats() + remoteStats := te.RemoteStats() + + populate(t, te, 1, 10, 1) + _, err := te.store.Get(te.ctx, te.inRemote[0].Digest) + if err != nil { + t.Fatal(err) + } + + if (*localStats)["get"] != 1 && (*localStats)["put"] != 1 { + t.Errorf("Unexpected local counts") + } + + if (*remoteStats)["get"] != 1 { + t.Errorf("Unexpected remote get count") + } + + _, err = te.store.Get(te.ctx, te.inRemote[0].Digest) + if err != nil { + t.Fatal(err) + } + + if (*localStats)["get"] != 2 && (*localStats)["put"] != 1 { + t.Errorf("Unexpected local counts") + } + + if (*remoteStats)["get"] != 1 { + t.Errorf("Unexpected remote get count") + } + +} func TestProxyStoreStat(t *testing.T) { te := makeTestEnv(t, "foo/bar")