From 860b28c5b9698bb7e9d5e9b9bc83f15319425723 Mon Sep 17 00:00:00 2001 From: Stephen J Day Date: Mon, 14 Aug 2017 18:30:46 -0700 Subject: [PATCH] registry: remove dependency on logrus for client To simplify the vendoring story for the client, we have now removed the requirement for `logrus` and the forked `context` package (usually imported as `dcontext`). We inject the logger via the metrics tracker for the blob cache and via options on the token handler. We preserve logs on the proxy cache for that case. Clients expecting these log messages may need to be updated accordingly. Signed-off-by: Stephen J Day --- registry/client/auth/session.go | 21 ++++++++++++--- registry/proxy/proxyregistry.go | 15 ++++++++++- registry/storage/blobcachemetrics.go | 6 +++++ .../cache/cachedblobdescriptorstore.go | 27 ++++++++++++++++--- 4 files changed, 61 insertions(+), 8 deletions(-) diff --git a/registry/client/auth/session.go b/registry/client/auth/session.go index be474d82..bc7b66db 100644 --- a/registry/client/auth/session.go +++ b/registry/client/auth/session.go @@ -13,7 +13,6 @@ import ( "github.com/docker/distribution/registry/client" "github.com/docker/distribution/registry/client/auth/challenge" "github.com/docker/distribution/registry/client/transport" - "github.com/sirupsen/logrus" ) var ( @@ -135,6 +134,8 @@ type tokenHandler struct { tokenLock sync.Mutex tokenCache string tokenExpiration time.Time + + logger Logger } // Scope is a type which is serializable to a string @@ -176,6 +177,18 @@ func (rs RegistryScope) String() string { return fmt.Sprintf("registry:%s:%s", rs.Name, strings.Join(rs.Actions, ",")) } +// Logger defines the injectable logging interface, used on TokenHandlers. +type Logger interface { + Debugf(format string, args ...interface{}) +} + +func logDebugf(logger Logger, format string, args ...interface{}) { + if logger == nil { + return + } + logger.Debugf(format, args...) +} + // TokenHandlerOptions is used to configure a new token handler type TokenHandlerOptions struct { Transport http.RoundTripper @@ -185,6 +198,7 @@ type TokenHandlerOptions struct { ForceOAuth bool ClientID string Scopes []Scope + Logger Logger } // An implementation of clock for providing real time data. @@ -220,6 +234,7 @@ func NewTokenHandlerWithOptions(options TokenHandlerOptions) AuthenticationHandl clientID: options.ClientID, scopes: options.Scopes, clock: realClock{}, + logger: options.Logger, } return handler @@ -348,7 +363,7 @@ func (th *tokenHandler) fetchTokenWithOAuth(realm *url.URL, refreshToken, servic if tr.ExpiresIn < minimumTokenLifetimeSeconds { // The default/minimum lifetime. tr.ExpiresIn = minimumTokenLifetimeSeconds - logrus.Debugf("Increasing token expiration to: %d seconds", tr.ExpiresIn) + logDebugf(th.logger, "Increasing token expiration to: %d seconds", tr.ExpiresIn) } if tr.IssuedAt.IsZero() { @@ -439,7 +454,7 @@ func (th *tokenHandler) fetchTokenWithBasicAuth(realm *url.URL, service string, if tr.ExpiresIn < minimumTokenLifetimeSeconds { // The default/minimum lifetime. tr.ExpiresIn = minimumTokenLifetimeSeconds - logrus.Debugf("Increasing token expiration to: %d seconds", tr.ExpiresIn) + logDebugf(th.logger, "Increasing token expiration to: %d seconds", tr.ExpiresIn) } if tr.IssuedAt.IsZero() { diff --git a/registry/proxy/proxyregistry.go b/registry/proxy/proxyregistry.go index aec220e8..a6413fda 100644 --- a/registry/proxy/proxyregistry.go +++ b/registry/proxy/proxyregistry.go @@ -121,8 +121,21 @@ func (pr *proxyingRegistry) Repositories(ctx context.Context, repos []string, la func (pr *proxyingRegistry) Repository(ctx context.Context, name reference.Named) (distribution.Repository, error) { c := pr.authChallenger + tkopts := auth.TokenHandlerOptions{ + Transport: http.DefaultTransport, + Credentials: c.credentialStore(), + Scopes: []auth.Scope{ + auth.RepositoryScope{ + Repository: name.Name(), + Actions: []string{"pull"}, + }, + }, + Logger: dcontext.GetLogger(ctx), + } + tr := transport.NewTransport(http.DefaultTransport, - auth.NewAuthorizer(c.challengeManager(), auth.NewTokenHandler(http.DefaultTransport, c.credentialStore(), name.Name(), "pull"))) + auth.NewAuthorizer(c.challengeManager(), + auth.NewTokenHandlerWithOptions(tkopts))) localRepo, err := pr.embedded.Repository(ctx, name) if err != nil { diff --git a/registry/storage/blobcachemetrics.go b/registry/storage/blobcachemetrics.go index fad0a77a..238b5806 100644 --- a/registry/storage/blobcachemetrics.go +++ b/registry/storage/blobcachemetrics.go @@ -1,9 +1,11 @@ package storage import ( + "context" "expvar" "sync/atomic" + dcontext "github.com/docker/distribution/context" "github.com/docker/distribution/registry/storage/cache" ) @@ -25,6 +27,10 @@ func (bsc *blobStatCollector) Metrics() cache.Metrics { return bsc.metrics } +func (bsc *blobStatCollector) Logger(ctx context.Context) cache.Logger { + return dcontext.GetLogger(ctx) +} + // blobStatterCacheMetrics keeps track of cache metrics for blob descriptor // cache requests. Note this is kept globally and made available via expvar. // For more detailed metrics, its recommend to instrument a particular cache diff --git a/registry/storage/cache/cachedblobdescriptorstore.go b/registry/storage/cache/cachedblobdescriptorstore.go index 668eebda..cdc34f5f 100644 --- a/registry/storage/cache/cachedblobdescriptorstore.go +++ b/registry/storage/cache/cachedblobdescriptorstore.go @@ -4,7 +4,6 @@ import ( "context" "github.com/docker/distribution" - dcontext "github.com/docker/distribution/context" "github.com/opencontainers/go-digest" ) @@ -17,12 +16,20 @@ type Metrics struct { Misses uint64 } +// Logger can be provided on the MetricsTracker to log errors. +// +// Usually, this is just a proxy to dcontext.GetLogger. +type Logger interface { + Errorf(format string, args ...interface{}) +} + // MetricsTracker represents a metric tracker // which simply counts the number of hits and misses. type MetricsTracker interface { Hit() Miss() Metrics() Metrics + Logger(context.Context) Logger } type cachedBlobStatter struct { @@ -54,7 +61,7 @@ func (cbds *cachedBlobStatter) Stat(ctx context.Context, dgst digest.Digest) (di desc, err := cbds.cache.Stat(ctx, dgst) if err != nil { if err != distribution.ErrBlobUnknown { - dcontext.GetLogger(ctx).Errorf("error retrieving descriptor from cache: %v", err) + logErrorf(ctx, cbds.tracker, "error retrieving descriptor from cache: %v", err) } goto fallback @@ -74,7 +81,7 @@ fallback: } if err := cbds.cache.SetDescriptor(ctx, dgst, desc); err != nil { - dcontext.GetLogger(ctx).Errorf("error adding descriptor %v to cache: %v", desc.Digest, err) + logErrorf(ctx, cbds.tracker, "error adding descriptor %v to cache: %v", desc.Digest, err) } return desc, err @@ -96,7 +103,19 @@ func (cbds *cachedBlobStatter) Clear(ctx context.Context, dgst digest.Digest) er func (cbds *cachedBlobStatter) SetDescriptor(ctx context.Context, dgst digest.Digest, desc distribution.Descriptor) error { if err := cbds.cache.SetDescriptor(ctx, dgst, desc); err != nil { - dcontext.GetLogger(ctx).Errorf("error adding descriptor %v to cache: %v", desc.Digest, err) + logErrorf(ctx, cbds.tracker, "error adding descriptor %v to cache: %v", desc.Digest, err) } return nil } + +func logErrorf(ctx context.Context, tracker MetricsTracker, format string, args ...interface{}) { + if tracker == nil { + return + } + + logger := tracker.Logger(ctx) + if logger == nil { + return + } + logger.Errorf(format, args...) +}