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

1 files changed, 62 insertions, 25 deletions

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