From 10db57027df6925c1afc82db1e7ddd0ff2cc53a2 Mon Sep 17 00:00:00 2001 From: Matt Holt Date: Tue, 28 Apr 2020 08:32:04 -0600 Subject: caddyhttp: General improvements to access logging (#3301) * httpcaddyfile: Exclude access logs written to files from default log Even though any logs can just be ignored, most users don't seem to like configuring an access log to go to a file only to have it doubly appear in the default log. Related to: - #3294 - https://caddy.community/t/v2-logging-format/7642/4?u=matt - https://caddy.community/t/caddyfile-questions/7651/3?u=matt * caddyhttp: General improvements to access log controls (fixes #3310) * caddyhttp: Move log config nil check higher * Rename LoggerName -> DefaultLoggerName --- caddyconfig/httpcaddyfile/httptype.go | 87 +++++++++++++++++++++++++---------- 1 file changed, 62 insertions(+), 25 deletions(-) (limited to 'caddyconfig/httpcaddyfile/httptype.go') diff --git a/caddyconfig/httpcaddyfile/httptype.go b/caddyconfig/httpcaddyfile/httptype.go index a22dd40..eb067bc 100644 --- a/caddyconfig/httpcaddyfile/httptype.go +++ b/caddyconfig/httpcaddyfile/httptype.go @@ -38,7 +38,7 @@ type ServerType struct { } // Setup makes a config from the tokens. -func (st ServerType) Setup(originalServerBlocks []caddyfile.ServerBlock, +func (st ServerType) Setup(inputServerBlocks []caddyfile.ServerBlock, options map[string]interface{}) (*caddy.Config, []caddyconfig.Warning, error) { var warnings []caddyconfig.Warning gc := counter{new(int)} @@ -50,15 +50,15 @@ func (st ServerType) Setup(originalServerBlocks []caddyfile.ServerBlock, // chosen to handle a request - we actually will make each // server block's route terminal so that only one will run sbKeys := make(map[string]struct{}) - serverBlocks := make([]serverBlock, 0, len(originalServerBlocks)) - for i, sblock := range originalServerBlocks { + originalServerBlocks := make([]serverBlock, 0, len(inputServerBlocks)) + for i, sblock := range inputServerBlocks { for j, k := range sblock.Keys { if _, ok := sbKeys[k]; ok { return nil, warnings, fmt.Errorf("duplicate site address not allowed: '%s' in %v (site block %d, key %d)", k, sblock.Keys, i, j) } sbKeys[k] = struct{}{} } - serverBlocks = append(serverBlocks, serverBlock{ + originalServerBlocks = append(originalServerBlocks, serverBlock{ block: sblock, pile: make(map[string][]ConfigValue), }) @@ -66,12 +66,12 @@ func (st ServerType) Setup(originalServerBlocks []caddyfile.ServerBlock, // apply any global options var err error - serverBlocks, err = st.evaluateGlobalOptionsBlock(serverBlocks, options) + originalServerBlocks, err = st.evaluateGlobalOptionsBlock(originalServerBlocks, options) if err != nil { return nil, warnings, err } - for _, sb := range serverBlocks { + for _, sb := range originalServerBlocks { // replace shorthand placeholders (which are // convenient when writing a Caddyfile) with // their actual placeholder identifiers or @@ -155,7 +155,7 @@ func (st ServerType) Setup(originalServerBlocks []caddyfile.ServerBlock, } // map - sbmap, err := st.mapAddressToServerBlocks(serverBlocks, options) + sbmap, err := st.mapAddressToServerBlocks(originalServerBlocks, options) if err != nil { return nil, warnings, err } @@ -193,21 +193,24 @@ func (st ServerType) Setup(originalServerBlocks []caddyfile.ServerBlock, // extract any custom logs, and enforce configured levels var customLogs []namedCustomLog var hasDefaultLog bool - for _, sb := range serverBlocks { - for _, clVal := range sb.pile["custom_log"] { - ncl := clVal.Value.(namedCustomLog) - if ncl.name == "" { - continue - } - if ncl.name == "default" { - hasDefaultLog = true - } - if _, ok := options["debug"]; ok && ncl.log.Level == "" { - ncl.log.Level = "DEBUG" + for _, p := range pairings { + for _, sb := range p.serverBlocks { + for _, clVal := range sb.pile["custom_log"] { + ncl := clVal.Value.(namedCustomLog) + if ncl.name == "" { + continue + } + if ncl.name == "default" { + hasDefaultLog = true + } + if _, ok := options["debug"]; ok && ncl.log.Level == "" { + ncl.log.Level = "DEBUG" + } + customLogs = append(customLogs, ncl) } - customLogs = append(customLogs, ncl) } } + if !hasDefaultLog { // if the default log was not customized, ensure we // configure it with any applicable options @@ -250,6 +253,17 @@ func (st ServerType) Setup(originalServerBlocks []caddyfile.ServerBlock, if ncl.name != "" { cfg.Logging.Logs[ncl.name] = ncl.log } + // most users seem to prefer not writing access logs + // to the default log when they are directed to a + // file or have any other special customization + if len(ncl.log.Include) > 0 { + defaultLog, ok := cfg.Logging.Logs["default"] + if !ok { + defaultLog = new(caddy.CustomLog) + cfg.Logging.Logs["default"] = defaultLog + } + defaultLog.Exclude = append(defaultLog.Exclude, ncl.log.Include...) + } } } @@ -451,23 +465,46 @@ func (st *ServerType) serversFromPairings( } // add log associations + // see https://github.com/caddyserver/caddy/issues/3310 + sblockLogHosts := sblock.hostsFromKeys(true) for _, cval := range sblock.pile["custom_log"] { ncl := cval.Value.(namedCustomLog) if srv.Logs == nil { - srv.Logs = &caddyhttp.ServerLogConfig{ - LoggerNames: make(map[string]string), - } + srv.Logs = new(caddyhttp.ServerLogConfig) } if sblock.hasHostCatchAllKey() { - srv.Logs.LoggerName = ncl.name + // all requests for hosts not able to be listed should use + // this log because it's a catch-all-hosts server block + srv.Logs.DefaultLoggerName = ncl.name } else { - for _, h := range sblock.hostsFromKeys(true) { - if ncl.name != "" { + // map each host to the user's desired logger name + for _, h := range sblockLogHosts { + // if the custom logger name is non-empty, add it to + // the map; otherwise, only map to an empty logger + // name if the server block has a catch-all host (in + // which case only requests with mapped hostnames will + // be access-logged, so it'll be necessary to add them + // to the map even if they use default logger) + if ncl.name != "" || len(hosts) == 0 { + if srv.Logs.LoggerNames == nil { + srv.Logs.LoggerNames = make(map[string]string) + } srv.Logs.LoggerNames[h] = ncl.name } } } } + if srv.Logs != nil && len(sblock.pile["custom_log"]) == 0 { + // server has access logs enabled, but this server block does not + // enable access logs; therefore, all hosts of this server block + // should not be access-logged + if len(hosts) == 0 { + // if the server block has a catch-all-hosts key, then we should + // not log reqs to any host unless it appears in the map + srv.Logs.SkipUnmappedHosts = true + } + srv.Logs.SkipHosts = append(srv.Logs.SkipHosts, sblockLogHosts...) + } } // a server cannot (natively) serve both HTTP and HTTPS at the -- cgit v1.2.3