From e43b6d81782ef79f22058179d8793f40cea89556 Mon Sep 17 00:00:00 2001 From: Matthew Holt Date: Fri, 16 Sep 2022 16:55:30 -0600 Subject: [PATCH] core: Variadic Context.Logger(); soft deprecation Ideally I'd just remove the parameter to caddy.Context.Logger(), but this would break most Caddy plugins. Instead, I'm making it variadic and marking it as partially deprecated. In the future, I might completely remove the parameter once most plugins have updated. --- caddyconfig/httploader.go | 4 +- context.go | 43 +++++++++++-------- modules/caddyevents/app.go | 2 +- modules/caddyhttp/app.go | 2 +- modules/caddyhttp/caddyauth/caddyauth.go | 2 +- modules/caddyhttp/caddyauth/hashes.go | 2 +- modules/caddyhttp/celmatcher.go | 16 +++---- modules/caddyhttp/fileserver/matcher.go | 2 +- modules/caddyhttp/fileserver/staticfiles.go | 2 +- modules/caddyhttp/matchers.go | 2 +- modules/caddyhttp/push/handler.go | 2 +- .../caddyhttp/reverseproxy/fastcgi/fastcgi.go | 2 +- .../caddyhttp/reverseproxy/httptransport.go | 2 +- .../caddyhttp/reverseproxy/reverseproxy.go | 2 +- modules/caddyhttp/reverseproxy/upstreams.go | 4 +- modules/caddyhttp/rewrite/rewrite.go | 2 +- modules/caddyhttp/tracing/module.go | 9 ++-- modules/caddypki/acmeserver/acmeserver.go | 2 +- modules/caddypki/adminapi.go | 2 +- modules/caddypki/pki.go | 2 +- modules/caddytls/acmeissuer.go | 2 +- modules/caddytls/certmanagers.go | 2 +- modules/caddytls/internalissuer.go | 13 +++--- modules/caddytls/matchers.go | 2 +- modules/caddytls/tls.go | 2 +- modules/caddytls/zerosslissuer.go | 2 +- modules/metrics/metrics.go | 9 ++-- 27 files changed, 71 insertions(+), 67 deletions(-) diff --git a/caddyconfig/httploader.go b/caddyconfig/httploader.go index e4d815915..80eab4469 100644 --- a/caddyconfig/httploader.go +++ b/caddyconfig/httploader.go @@ -113,7 +113,7 @@ func (hl HTTPLoader) LoadConfig(ctx caddy.Context) ([]byte, error) { return nil, err } for _, warn := range warnings { - ctx.Logger(hl).Warn(warn.String()) + ctx.Logger().Warn(warn.String()) } return result, nil @@ -129,7 +129,7 @@ func (hl HTTPLoader) makeClient(ctx caddy.Context) (*http.Client, error) { // client authentication if hl.TLS.UseServerIdentity { - certs, err := ctx.IdentityCredentials(ctx.Logger(hl)) + certs, err := ctx.IdentityCredentials(ctx.Logger()) if err != nil { return nil, fmt.Errorf("getting server identity credentials: %v", err) } diff --git a/context.go b/context.go index e850b73df..e55e40cef 100644 --- a/context.go +++ b/context.go @@ -442,10 +442,27 @@ func (ctx Context) Storage() certmagic.Storage { return ctx.cfg.storage } -// TODO: aw man, can I please change this? -// Logger returns a logger that can be used by mod. -func (ctx Context) Logger(mod Module) *zap.Logger { - // TODO: if mod is nil, use ctx.Module() instead... +// Logger returns a logger that is intended for use by the most +// recent module associated with the context. Callers should not +// pass in any arguments unless they want to associate with a +// different module; it panics if more than 1 value is passed in. +// +// Originally, this method's signature was `Logger(mod Module)`, +// requiring that an instance of a Caddy module be passsed in. +// However, that is no longer necessary, as the closest module +// most recently associated with the context will be automatically +// assumed. To prevent a sudden breaking change, this method's +// signature has been changed to be variadic, but we may remove +// the parameter altogether in the future. Callers should not +// pass in any argument. If there is valid need to specify a +// different module, please open an issue to discuss. +// +// PARTIALLY DEPRECATED: The Logger(module) form is deprecated and +// may be removed in the future. Do not pass in any arguments. +func (ctx Context) Logger(module ...Module) *zap.Logger { + if len(module) > 1 { + panic("more than 1 module passed in") + } if ctx.cfg == nil { // often the case in tests; just use a dev logger l, err := zap.NewDevelopment() @@ -454,23 +471,13 @@ func (ctx Context) Logger(mod Module) *zap.Logger { } return l } + mod := ctx.Module() + if len(module) > 0 { + mod = module[0] + } return ctx.cfg.Logging.Logger(mod) } -// TODO: use this -// // Logger returns a logger that can be used by the current module. -// func (ctx Context) Log() *zap.Logger { -// if ctx.cfg == nil { -// // often the case in tests; just use a dev logger -// l, err := zap.NewDevelopment() -// if err != nil { -// panic("config missing, unable to create dev logger: " + err.Error()) -// } -// return l -// } -// return ctx.cfg.Logging.Logger(ctx.Module()) -// } - // Modules returns the lineage of modules that this context provisioned, // with the most recent/current module being last in the list. func (ctx Context) Modules() []Module { diff --git a/modules/caddyevents/app.go b/modules/caddyevents/app.go index 3ae40e813..db2192ce3 100644 --- a/modules/caddyevents/app.go +++ b/modules/caddyevents/app.go @@ -119,7 +119,7 @@ func (App) CaddyModule() caddy.ModuleInfo { // Provision sets up the app. func (app *App) Provision(ctx caddy.Context) error { - app.logger = ctx.Logger(app) + app.logger = ctx.Logger() app.subscriptions = make(map[string]map[caddy.ModuleID][]Handler) for _, sub := range app.Subscriptions { diff --git a/modules/caddyhttp/app.go b/modules/caddyhttp/app.go index 0253521a6..290a4083e 100644 --- a/modules/caddyhttp/app.go +++ b/modules/caddyhttp/app.go @@ -160,7 +160,7 @@ func (app *App) Provision(ctx caddy.Context) error { } app.tlsApp = tlsAppIface.(*caddytls.TLS) app.ctx = ctx - app.logger = ctx.Logger(app) + app.logger = ctx.Logger() eventsAppIface, err := ctx.App("events") if err != nil { diff --git a/modules/caddyhttp/caddyauth/caddyauth.go b/modules/caddyhttp/caddyauth/caddyauth.go index ae30a0863..b2bdbc22f 100644 --- a/modules/caddyhttp/caddyauth/caddyauth.go +++ b/modules/caddyhttp/caddyauth/caddyauth.go @@ -56,7 +56,7 @@ func (Authentication) CaddyModule() caddy.ModuleInfo { // Provision sets up a. func (a *Authentication) Provision(ctx caddy.Context) error { - a.logger = ctx.Logger(a) + a.logger = ctx.Logger() a.Providers = make(map[string]Authenticator) mods, err := ctx.LoadModule(a, "ProvidersRaw") if err != nil { diff --git a/modules/caddyhttp/caddyauth/hashes.go b/modules/caddyhttp/caddyauth/hashes.go index 6505d187c..6a651f0d9 100644 --- a/modules/caddyhttp/caddyauth/hashes.go +++ b/modules/caddyhttp/caddyauth/hashes.go @@ -92,7 +92,7 @@ func (ScryptHash) CaddyModule() caddy.ModuleInfo { // Provision sets up s. func (s *ScryptHash) Provision(ctx caddy.Context) error { s.SetDefaults() - ctx.Logger(s).Warn("use of 'scrypt' is deprecated, please use 'bcrypt' instead") + ctx.Logger().Warn("use of 'scrypt' is deprecated, please use 'bcrypt' instead") return nil } diff --git a/modules/caddyhttp/celmatcher.go b/modules/caddyhttp/celmatcher.go index e7067c324..7f1cc02ef 100644 --- a/modules/caddyhttp/celmatcher.go +++ b/modules/caddyhttp/celmatcher.go @@ -90,7 +90,7 @@ func (m *MatchExpression) UnmarshalJSON(data []byte) error { // Provision sets ups m. func (m *MatchExpression) Provision(ctx caddy.Context) error { - m.log = ctx.Logger(m) + m.log = ctx.Logger() // replace placeholders with a function call - this is just some // light (and possibly naïve) syntactic sugar @@ -294,13 +294,13 @@ type CELLibraryProducer interface { // CELMatcherImpl creates a new cel.Library based on the following pieces of // data: // -// - macroName: the function name to be used within CEL. This will be a macro -// and not a function proper. -// - funcName: the function overload name generated by the CEL macro used to -// represent the matcher. -// - matcherDataTypes: the argument types to the macro. -// - fac: a matcherFactory implementation which converts from CEL constant -// values to a Matcher instance. +// - macroName: the function name to be used within CEL. This will be a macro +// and not a function proper. +// - funcName: the function overload name generated by the CEL macro used to +// represent the matcher. +// - matcherDataTypes: the argument types to the macro. +// - fac: a matcherFactory implementation which converts from CEL constant +// values to a Matcher instance. // // Note, macro names and function names must not collide with other macros or // functions exposed within CEL expressions, or an error will be produced diff --git a/modules/caddyhttp/fileserver/matcher.go b/modules/caddyhttp/fileserver/matcher.go index 87a152433..2989e4b20 100644 --- a/modules/caddyhttp/fileserver/matcher.go +++ b/modules/caddyhttp/fileserver/matcher.go @@ -256,7 +256,7 @@ func celFileMatcherMacroExpander() parser.MacroExpander { // Provision sets up m's defaults. func (m *MatchFile) Provision(ctx caddy.Context) error { - m.logger = ctx.Logger(m) + m.logger = ctx.Logger() // establish the file system to use if len(m.FileSystemRaw) > 0 { diff --git a/modules/caddyhttp/fileserver/staticfiles.go b/modules/caddyhttp/fileserver/staticfiles.go index 0639d979f..84743388e 100644 --- a/modules/caddyhttp/fileserver/staticfiles.go +++ b/modules/caddyhttp/fileserver/staticfiles.go @@ -167,7 +167,7 @@ func (FileServer) CaddyModule() caddy.ModuleInfo { // Provision sets up the static files responder. func (fsrv *FileServer) Provision(ctx caddy.Context) error { - fsrv.logger = ctx.Logger(fsrv) + fsrv.logger = ctx.Logger() // establish which file system (possibly a virtual one) we'll be using if len(fsrv.FileSystemRaw) > 0 { diff --git a/modules/caddyhttp/matchers.go b/modules/caddyhttp/matchers.go index a01f8f985..bca94be8e 100644 --- a/modules/caddyhttp/matchers.go +++ b/modules/caddyhttp/matchers.go @@ -1326,7 +1326,7 @@ func (MatchRemoteIP) CELLibrary(ctx caddy.Context) (cel.Library, error) { // Provision parses m's IP ranges, either from IP or CIDR expressions. func (m *MatchRemoteIP) Provision(ctx caddy.Context) error { - m.logger = ctx.Logger(m) + m.logger = ctx.Logger() for _, str := range m.Ranges { // Exclude the zone_id from the IP if strings.Contains(str, "%") { diff --git a/modules/caddyhttp/push/handler.go b/modules/caddyhttp/push/handler.go index 27652ef08..60eadd0c6 100644 --- a/modules/caddyhttp/push/handler.go +++ b/modules/caddyhttp/push/handler.go @@ -61,7 +61,7 @@ func (Handler) CaddyModule() caddy.ModuleInfo { // Provision sets up h. func (h *Handler) Provision(ctx caddy.Context) error { - h.logger = ctx.Logger(h) + h.logger = ctx.Logger() if h.Headers != nil { err := h.Headers.Provision(ctx) if err != nil { diff --git a/modules/caddyhttp/reverseproxy/fastcgi/fastcgi.go b/modules/caddyhttp/reverseproxy/fastcgi/fastcgi.go index 6ff6ff4a9..ec194e727 100644 --- a/modules/caddyhttp/reverseproxy/fastcgi/fastcgi.go +++ b/modules/caddyhttp/reverseproxy/fastcgi/fastcgi.go @@ -94,7 +94,7 @@ func (Transport) CaddyModule() caddy.ModuleInfo { // Provision sets up t. func (t *Transport) Provision(ctx caddy.Context) error { - t.logger = ctx.Logger(t) + t.logger = ctx.Logger() if t.Root == "" { t.Root = "{http.vars.root}" diff --git a/modules/caddyhttp/reverseproxy/httptransport.go b/modules/caddyhttp/reverseproxy/httptransport.go index e9c7ddd0a..ec5d2f23a 100644 --- a/modules/caddyhttp/reverseproxy/httptransport.go +++ b/modules/caddyhttp/reverseproxy/httptransport.go @@ -195,7 +195,7 @@ func (h *HTTPTransport) NewTransport(caddyCtx caddy.Context) (*http.Transport, e TCPConn: tcpConn, readTimeout: time.Duration(h.ReadTimeout), writeTimeout: time.Duration(h.WriteTimeout), - logger: caddyCtx.Logger(h), + logger: caddyCtx.Logger(), } } diff --git a/modules/caddyhttp/reverseproxy/reverseproxy.go b/modules/caddyhttp/reverseproxy/reverseproxy.go index dc1458d11..4c8b29b13 100644 --- a/modules/caddyhttp/reverseproxy/reverseproxy.go +++ b/modules/caddyhttp/reverseproxy/reverseproxy.go @@ -217,7 +217,7 @@ func (h *Handler) Provision(ctx caddy.Context) error { } h.events = eventAppIface.(*caddyevents.App) h.ctx = ctx - h.logger = ctx.Logger(h) + h.logger = ctx.Logger() h.connections = make(map[io.ReadWriteCloser]openConnection) h.connectionsMu = new(sync.Mutex) diff --git a/modules/caddyhttp/reverseproxy/upstreams.go b/modules/caddyhttp/reverseproxy/upstreams.go index b9f85a2ce..7a90016d0 100644 --- a/modules/caddyhttp/reverseproxy/upstreams.go +++ b/modules/caddyhttp/reverseproxy/upstreams.go @@ -76,7 +76,7 @@ func (SRVUpstreams) CaddyModule() caddy.ModuleInfo { } func (su *SRVUpstreams) Provision(ctx caddy.Context) error { - su.logger = ctx.Logger(su) + su.logger = ctx.Logger() if su.Refresh == 0 { su.Refresh = caddy.Duration(time.Minute) } @@ -383,7 +383,7 @@ func (MultiUpstreams) CaddyModule() caddy.ModuleInfo { } func (mu *MultiUpstreams) Provision(ctx caddy.Context) error { - mu.logger = ctx.Logger(mu) + mu.logger = ctx.Logger() if mu.SourcesRaw != nil { mod, err := ctx.LoadModule(mu, "SourcesRaw") diff --git a/modules/caddyhttp/rewrite/rewrite.go b/modules/caddyhttp/rewrite/rewrite.go index 4316b5afd..95bc50468 100644 --- a/modules/caddyhttp/rewrite/rewrite.go +++ b/modules/caddyhttp/rewrite/rewrite.go @@ -101,7 +101,7 @@ func (Rewrite) CaddyModule() caddy.ModuleInfo { // Provision sets up rewr. func (rewr *Rewrite) Provision(ctx caddy.Context) error { - rewr.logger = ctx.Logger(rewr) + rewr.logger = ctx.Logger() for i, rep := range rewr.PathRegexp { if rep.Find == "" { diff --git a/modules/caddyhttp/tracing/module.go b/modules/caddyhttp/tracing/module.go index 7cce6692b..e3eb84d20 100644 --- a/modules/caddyhttp/tracing/module.go +++ b/modules/caddyhttp/tracing/module.go @@ -42,7 +42,7 @@ func (Tracing) CaddyModule() caddy.ModuleInfo { // Provision implements caddy.Provisioner. func (ot *Tracing) Provision(ctx caddy.Context) error { - ot.logger = ctx.Logger(ot) + ot.logger = ctx.Logger() var err error ot.otel, err = newOpenTelemetryWrapper(ctx, ot.SpanName) @@ -66,10 +66,9 @@ func (ot *Tracing) ServeHTTP(w http.ResponseWriter, r *http.Request, next caddyh // UnmarshalCaddyfile sets up the module from Caddyfile tokens. Syntax: // -// tracing { -// [span ] -// } -// +// tracing { +// [span ] +// } func (ot *Tracing) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { setParameter := func(d *caddyfile.Dispenser, val *string) error { if d.NextArg() { diff --git a/modules/caddypki/acmeserver/acmeserver.go b/modules/caddypki/acmeserver/acmeserver.go index ff021b4ff..921d0b879 100644 --- a/modules/caddypki/acmeserver/acmeserver.go +++ b/modules/caddypki/acmeserver/acmeserver.go @@ -87,7 +87,7 @@ func (Handler) CaddyModule() caddy.ModuleInfo { // Provision sets up the ACME server handler. func (ash *Handler) Provision(ctx caddy.Context) error { - ash.logger = ctx.Logger(ash) + ash.logger = ctx.Logger() // set some defaults if ash.CA == "" { ash.CA = caddypki.DefaultCAID diff --git a/modules/caddypki/adminapi.go b/modules/caddypki/adminapi.go index b4d269516..f03c6b638 100644 --- a/modules/caddypki/adminapi.go +++ b/modules/caddypki/adminapi.go @@ -47,7 +47,7 @@ func (adminAPI) CaddyModule() caddy.ModuleInfo { // Provision sets up the adminAPI module. func (a *adminAPI) Provision(ctx caddy.Context) error { a.ctx = ctx - a.log = ctx.Logger(a) + a.log = ctx.Logger(a) // TODO: passing in 'a' is a hack until the admin API is officially extensible (see #5032) // First check if the PKI app was configured, because // a.ctx.App() has the side effect of instantiating diff --git a/modules/caddypki/pki.go b/modules/caddypki/pki.go index 4fd0bb508..2caeb2b59 100644 --- a/modules/caddypki/pki.go +++ b/modules/caddypki/pki.go @@ -54,7 +54,7 @@ func (PKI) CaddyModule() caddy.ModuleInfo { // Provision sets up the configuration for the PKI app. func (p *PKI) Provision(ctx caddy.Context) error { p.ctx = ctx - p.log = ctx.Logger(p) + p.log = ctx.Logger() for caID, ca := range p.CAs { err := ca.Provision(ctx, caID, p.log) diff --git a/modules/caddytls/acmeissuer.go b/modules/caddytls/acmeissuer.go index 2fe4004fa..2f752ed05 100644 --- a/modules/caddytls/acmeissuer.go +++ b/modules/caddytls/acmeissuer.go @@ -103,7 +103,7 @@ func (ACMEIssuer) CaddyModule() caddy.ModuleInfo { // Provision sets up iss. func (iss *ACMEIssuer) Provision(ctx caddy.Context) error { - iss.logger = ctx.Logger(iss) + iss.logger = ctx.Logger() repl := caddy.NewReplacer() diff --git a/modules/caddytls/certmanagers.go b/modules/caddytls/certmanagers.go index de7abf973..1b701ab26 100644 --- a/modules/caddytls/certmanagers.go +++ b/modules/caddytls/certmanagers.go @@ -43,7 +43,7 @@ func (Tailscale) CaddyModule() caddy.ModuleInfo { } func (ts *Tailscale) Provision(ctx caddy.Context) error { - ts.logger = ctx.Logger(ts) + ts.logger = ctx.Logger() return nil } diff --git a/modules/caddytls/internalissuer.go b/modules/caddytls/internalissuer.go index ba6055edd..3dd6c359f 100644 --- a/modules/caddytls/internalissuer.go +++ b/modules/caddytls/internalissuer.go @@ -65,7 +65,7 @@ func (InternalIssuer) CaddyModule() caddy.ModuleInfo { // Provision sets up the issuer. func (iss *InternalIssuer) Provision(ctx caddy.Context) error { - iss.logger = ctx.Logger(iss) + iss.logger = ctx.Logger() // set some defaults if iss.CA == "" { @@ -148,12 +148,11 @@ func (iss InternalIssuer) Issue(ctx context.Context, csr *x509.CertificateReques // UnmarshalCaddyfile deserializes Caddyfile tokens into iss. // -// ... internal { -// ca -// lifetime -// sign_with_root -// } -// +// ... internal { +// ca +// lifetime +// sign_with_root +// } func (iss *InternalIssuer) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { for d.Next() { for d.NextBlock(0) { diff --git a/modules/caddytls/matchers.go b/modules/caddytls/matchers.go index 4a22bc09c..f541220fc 100644 --- a/modules/caddytls/matchers.go +++ b/modules/caddytls/matchers.go @@ -81,7 +81,7 @@ func (MatchRemoteIP) CaddyModule() caddy.ModuleInfo { // Provision parses m's IP ranges, either from IP or CIDR expressions. func (m *MatchRemoteIP) Provision(ctx caddy.Context) error { - m.logger = ctx.Logger(m) + m.logger = ctx.Logger() for _, str := range m.Ranges { cidrs, err := m.parseIPRange(str) if err != nil { diff --git a/modules/caddytls/tls.go b/modules/caddytls/tls.go index 2e532ed91..8051653ee 100644 --- a/modules/caddytls/tls.go +++ b/modules/caddytls/tls.go @@ -94,7 +94,7 @@ func (t *TLS) Provision(ctx caddy.Context) error { } t.events = eventsAppIface.(*caddyevents.App) t.ctx = ctx - t.logger = ctx.Logger(t) + t.logger = ctx.Logger() repl := caddy.NewReplacer() // set up a new certificate cache; this (re)loads all certificates diff --git a/modules/caddytls/zerosslissuer.go b/modules/caddytls/zerosslissuer.go index c7ff70fbd..02092944b 100644 --- a/modules/caddytls/zerosslissuer.go +++ b/modules/caddytls/zerosslissuer.go @@ -66,7 +66,7 @@ func (*ZeroSSLIssuer) CaddyModule() caddy.ModuleInfo { // Provision sets up iss. func (iss *ZeroSSLIssuer) Provision(ctx caddy.Context) error { - iss.logger = ctx.Logger(iss) + iss.logger = ctx.Logger() if iss.ACMEIssuer == nil { iss.ACMEIssuer = new(ACMEIssuer) } diff --git a/modules/metrics/metrics.go b/modules/metrics/metrics.go index 1ad392c1e..d4ad97798 100644 --- a/modules/metrics/metrics.go +++ b/modules/metrics/metrics.go @@ -62,7 +62,7 @@ func (l *zapLogger) Println(v ...any) { // Provision sets up m. func (m *Metrics) Provision(ctx caddy.Context) error { - log := ctx.Logger(m) + log := ctx.Logger() m.metricsHandler = createMetricsHandler(&zapLogger{log}, !m.DisableOpenMetrics) return nil } @@ -75,10 +75,9 @@ func parseCaddyfile(h httpcaddyfile.Helper) (caddyhttp.MiddlewareHandler, error) // UnmarshalCaddyfile sets up the handler from Caddyfile tokens. Syntax: // -// metrics [] { -// disable_openmetrics -// } -// +// metrics [] { +// disable_openmetrics +// } func (m *Metrics) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { for d.Next() { args := d.RemainingArgs()