From 60522f85e3de401f48da007269d9a57c6070a5dd Mon Sep 17 00:00:00 2001 From: Evan Anderson Date: Mon, 13 Feb 2023 09:35:47 -0800 Subject: [PATCH 1/3] Cache authenticators between registry operations to speed up large copies. Signed-off-by: Evan Anderson --- pkg/imgpkg/registry/registry.go | 36 +++++++++++++++++++++++---------- 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/pkg/imgpkg/registry/registry.go b/pkg/imgpkg/registry/registry.go index c5429416..8240338c 100644 --- a/pkg/imgpkg/registry/registry.go +++ b/pkg/imgpkg/registry/registry.go @@ -126,6 +126,7 @@ type SimpleRegistry struct { remoteOpts []regremote.Option refOpts []regname.Option keychain regauthn.Keychain + authn map[string]regauthn.Authenticator roundTrippers RoundTripperStorage transportAccess *sync.Mutex } @@ -195,6 +196,7 @@ func NewSimpleRegistryWithTransport(opts Opts, rTripper http.RoundTripper, regOp refOpts: refOpts, keychain: keychain, roundTrippers: NewMultiRoundTripperStorage(baseRoundTripper), + authn: map[string]regauthn.Authenticator{}, transportAccess: &sync.Mutex{}, }, nil } @@ -219,6 +221,7 @@ func (r SimpleRegistry) CloneWithSingleAuth(imageRef regname.Tag) (Registry, err refOpts: r.refOpts, keychain: keychain, roundTrippers: NewSingleTripperStorage(rt), + authn: map[string]regauthn.Authenticator{}, transportAccess: &sync.Mutex{}, }, nil } @@ -237,42 +240,53 @@ func (r SimpleRegistry) CloneWithLogger(_ util.ProgressLogger) Registry { // readOpts Returns the readOpts + the keychain func (r *SimpleRegistry) readOpts(ref regname.Reference) ([]regremote.Option, error) { - rt, err := r.transport(ref, ref.Scope(transport.PullScope)) + rt, authn, err := r.transport(ref, ref.Scope(transport.PullScope)) if err != nil { return nil, err } - return append([]regremote.Option{regremote.WithAuthFromKeychain(r.keychain), regremote.WithTransport(rt)}, r.remoteOpts...), nil + return append([]regremote.Option{regremote.WithAuth(authn), regremote.WithTransport(rt)}, r.remoteOpts...), nil } // writeOpts Returns the writeOpts + the keychain func (r *SimpleRegistry) writeOpts(ref regname.Reference) ([]regremote.Option, error) { - rt, err := r.transport(ref, ref.Scope(transport.PushScope)) + rt, auth, err := r.transport(ref, ref.Scope(transport.PushScope)) if err != nil { return nil, err } - return append([]regremote.Option{regremote.WithAuthFromKeychain(r.keychain), regremote.WithTransport(rt)}, r.remoteOpts...), nil + return append([]regremote.Option{regremote.WithAuth(auth), regremote.WithTransport(rt)}, r.remoteOpts...), nil } // transport Retrieve the RoundTripper that can be used to access the repository -func (r *SimpleRegistry) transport(ref regname.Reference, scope string) (http.RoundTripper, error) { +func (r *SimpleRegistry) transport(ref regname.Reference, scope string) (http.RoundTripper, regauthn.Authenticator, error) { + registry := ref.Context() // The idea is that we can only retrieve 1 RoundTripper at a time to ensure that we do not create // the same RoundTripper multiple times r.transportAccess.Lock() defer r.transportAccess.Unlock() - rt := r.roundTrippers.RoundTripper(ref.Context(), scope) + rt := r.roundTrippers.RoundTripper(registry, scope) if rt == nil { - resolvedAuth, err := r.keychain.Resolve(ref.Context()) + resolvedAuth, err := r.keychain.Resolve(registry) if err != nil { - return nil, fmt.Errorf("Unable retrieve credentials for registry: %s", err) + return nil, nil, fmt.Errorf("Unable retrieve credentials for registry: %s", err) } - rt, err = r.roundTrippers.CreateRoundTripper(ref.Context().Registry, resolvedAuth, scope) + rt, err = r.roundTrippers.CreateRoundTripper(registry.Registry, resolvedAuth, scope) if err != nil { - return nil, fmt.Errorf("Error while preparing a transport to talk with the registry: %s", err) + return nil, nil, fmt.Errorf("Error while preparing a transport to talk with the registry: %s", err) } } - return rt, nil + auth, ok := r.authn[registry.Name()] + if !ok { + regauth, err := r.keychain.Resolve(registry) + if err != nil { + return nil, nil, fmt.Errorf("Unable to get authenticator for registry %s: %s", ref.Context().Name(), err) + } + r.authn[registry.Name()] = regauth + auth = regauth + } + + return rt, auth, nil } // Get Retrieve Image descriptor for an Image reference From 57ecc28c21bd47d44f0ccb79bce0a4730acd80d4 Mon Sep 17 00:00:00 2001 From: Evan Anderson Date: Mon, 13 Feb 2023 10:11:38 -0800 Subject: [PATCH 2/3] Fix a missed map initialization. Also defend against this error in depth. Signed-off-by: Evan Anderson --- pkg/imgpkg/registry/registry.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pkg/imgpkg/registry/registry.go b/pkg/imgpkg/registry/registry.go index 8240338c..13091f21 100644 --- a/pkg/imgpkg/registry/registry.go +++ b/pkg/imgpkg/registry/registry.go @@ -234,6 +234,7 @@ func (r SimpleRegistry) CloneWithLogger(_ util.ProgressLogger) Registry { refOpts: r.refOpts, keychain: r.keychain, roundTrippers: r.roundTrippers, + authn: map[string]regauthn.Authenticator{}, transportAccess: &sync.Mutex{}, } } @@ -276,6 +277,10 @@ func (r *SimpleRegistry) transport(ref regname.Reference, scope string) (http.Ro } } + if r.authn == nil { + // This shouldn't happen, but let's defend in depth + r.authn = map[string]regauthn.Authenticator{} + } auth, ok := r.authn[registry.Name()] if !ok { regauth, err := r.keychain.Resolve(registry) From 08b722e9548b8c728f82497e6c45ee887644eeea Mon Sep 17 00:00:00 2001 From: Evan Anderson Date: Mon, 13 Feb 2023 10:43:58 -0800 Subject: [PATCH 3/3] Store results from `Resolve` during RoundTripper creation in the Authenticator cache. Signed-off-by: Evan Anderson --- pkg/imgpkg/registry/registry.go | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/pkg/imgpkg/registry/registry.go b/pkg/imgpkg/registry/registry.go index 13091f21..b99508b3 100644 --- a/pkg/imgpkg/registry/registry.go +++ b/pkg/imgpkg/registry/registry.go @@ -261,33 +261,36 @@ func (r *SimpleRegistry) writeOpts(ref regname.Reference) ([]regremote.Option, e // transport Retrieve the RoundTripper that can be used to access the repository func (r *SimpleRegistry) transport(ref regname.Reference, scope string) (http.RoundTripper, regauthn.Authenticator, error) { registry := ref.Context() + registryKey := registry.Name() // The idea is that we can only retrieve 1 RoundTripper at a time to ensure that we do not create // the same RoundTripper multiple times r.transportAccess.Lock() defer r.transportAccess.Unlock() + if r.authn == nil { + // This shouldn't happen, but let's defend in depth + r.authn = map[string]regauthn.Authenticator{} + } + rt := r.roundTrippers.RoundTripper(registry, scope) if rt == nil { resolvedAuth, err := r.keychain.Resolve(registry) if err != nil { return nil, nil, fmt.Errorf("Unable retrieve credentials for registry: %s", err) } + r.authn[registryKey] = resolvedAuth rt, err = r.roundTrippers.CreateRoundTripper(registry.Registry, resolvedAuth, scope) if err != nil { return nil, nil, fmt.Errorf("Error while preparing a transport to talk with the registry: %s", err) } } - if r.authn == nil { - // This shouldn't happen, but let's defend in depth - r.authn = map[string]regauthn.Authenticator{} - } - auth, ok := r.authn[registry.Name()] + auth, ok := r.authn[registryKey] if !ok { regauth, err := r.keychain.Resolve(registry) if err != nil { return nil, nil, fmt.Errorf("Unable to get authenticator for registry %s: %s", ref.Context().Name(), err) } - r.authn[registry.Name()] = regauth + r.authn[registryKey] = regauth auth = regauth }