httpcaddyfile: Allow `hostnames` & logger name overrides for log directive (#5643)

* httpcaddyfile: Allow `hostnames` override for log directive

* Implement access logger name overrides

* Fix panic & default logger clobbering edgecase

3 files changed, 94 insertions, 29 deletions

diff --git a/caddyconfig/httpcaddyfile/builtins.go b/caddyconfig/httpcaddyfile/builtins.go
index d6cdf84..5b0c5fb 100644
--- a/caddyconfig/httpcaddyfile/builtins.go
+++ b/caddyconfig/httpcaddyfile/builtins.go
@@ -788,7 +788,8 @@ func parseInvoke(h Helper) (caddyhttp.MiddlewareHandler, error) {
 
 // parseLog parses the log directive. Syntax:
 //
-//	log {
+//	log <logger_name> {
+//	    hostnames <hostnames...>
 //	    output <writer_module> ...
 //	    format <encoder_module> ...
 //	    level  <level>
@@ -809,11 +810,13 @@ func parseLogHelper(h Helper, globalLogNames map[string]struct{}) ([]ConfigValue
 	var configValues []ConfigValue
 	for h.Next() {
 		// Logic below expects that a name is always present when a
-		// global option is being parsed.
-		var globalLogName string
+		// global option is being parsed; or an optional override
+		// is supported for access logs.
+		var logName string
+
 		if parseAsGlobalOption {
 			if h.NextArg() {
-				globalLogName = h.Val()
+				logName = h.Val()
 
 				// Only a single argument is supported.
 				if h.NextArg() {
@@ -824,26 +827,47 @@ func parseLogHelper(h Helper, globalLogNames map[string]struct{}) ([]ConfigValue
 				// reference the default logger. See the
 				// setupNewDefault function in the logging
 				// package for where this is configured.
-				globalLogName = caddy.DefaultLoggerName
+				logName = caddy.DefaultLoggerName
 			}
 
 			// Verify this name is unused.
-			_, used := globalLogNames[globalLogName]
+			_, used := globalLogNames[logName]
 			if used {
-				return nil, h.Err("duplicate global log option for: " + globalLogName)
+				return nil, h.Err("duplicate global log option for: " + logName)
 			}
-			globalLogNames[globalLogName] = struct{}{}
+			globalLogNames[logName] = struct{}{}
 		} else {
-			// No arguments are supported for the server block log directive
+			// An optional override of the logger name can be provided;
+			// otherwise a default will be used, like "log0", "log1", etc.
 			if h.NextArg() {
-				return nil, h.ArgErr()
+				logName = h.Val()
+
+				// Only a single argument is supported.
+				if h.NextArg() {
+					return nil, h.ArgErr()
+				}
 			}
 		}
 
 		cl := new(caddy.CustomLog)
 
+		// allow overriding the current site block's hostnames for this logger;
+		// this is useful for setting up loggers per subdomain in a site block
+		// with a wildcard domain
+		customHostnames := []string{}
+
 		for h.NextBlock(0) {
 			switch h.Val() {
+			case "hostnames":
+				if parseAsGlobalOption {
+					return nil, h.Err("hostnames is not allowed in the log global options")
+				}
+				args := h.RemainingArgs()
+				if len(args) == 0 {
+					return nil, h.ArgErr()
+				}
+				customHostnames = append(customHostnames, args...)
+
 			case "output":
 				if !h.NextArg() {
 					return nil, h.ArgErr()
@@ -902,18 +926,16 @@ func parseLogHelper(h Helper, globalLogNames map[string]struct{}) ([]ConfigValue
 				}
 
 			case "include":
-				// This configuration is only allowed in the global options
 				if !parseAsGlobalOption {
-					return nil, h.ArgErr()
+					return nil, h.Err("include is not allowed in the log directive")
 				}
 				for h.NextArg() {
 					cl.Include = append(cl.Include, h.Val())
 				}
 
 			case "exclude":
-				// This configuration is only allowed in the global options
 				if !parseAsGlobalOption {
-					return nil, h.ArgErr()
+					return nil, h.Err("exclude is not allowed in the log directive")
 				}
 				for h.NextArg() {
 					cl.Exclude = append(cl.Exclude, h.Val())
@@ -925,24 +947,34 @@ func parseLogHelper(h Helper, globalLogNames map[string]struct{}) ([]ConfigValue
 		}
 
 		var val namedCustomLog
+		val.hostnames = customHostnames
+
+		isEmptyConfig := reflect.DeepEqual(cl, new(caddy.CustomLog))
+
 		// Skip handling of empty logging configs
-		if !reflect.DeepEqual(cl, new(caddy.CustomLog)) {
-			if parseAsGlobalOption {
-				// Use indicated name for global log options
-				val.name = globalLogName
-				val.log = cl
-			} else {
+
+		if parseAsGlobalOption {
+			// Use indicated name for global log options
+			val.name = logName
+		} else {
+			if logName != "" {
+				val.name = logName
+			} else if !isEmptyConfig {
 				// Construct a log name for server log streams
 				logCounter, ok := h.State["logCounter"].(int)
 				if !ok {
 					logCounter = 0
 				}
 				val.name = fmt.Sprintf("log%d", logCounter)
-				cl.Include = []string{"http.log.access." + val.name}
-				val.log = cl
 				logCounter++
 				h.State["logCounter"] = logCounter
 			}
+			if val.name != "" {
+				cl.Include = []string{"http.log.access." + val.name}
+			}
+		}
+		if !isEmptyConfig {
+			val.log = cl
 		}
 		configValues = append(configValues, ConfigValue{
 			Class: "custom_log",
diff --git a/caddyconfig/httpcaddyfile/builtins_test.go b/caddyconfig/httpcaddyfile/builtins_test.go
index 4cdb6ce..70f347d 100644
--- a/caddyconfig/httpcaddyfile/builtins_test.go
+++ b/caddyconfig/httpcaddyfile/builtins_test.go
@@ -52,12 +52,13 @@ func TestLogDirectiveSyntax(t *testing.T) {
 		},
 		{
 			input: `:8080 {
-				log invalid {
+				log name-override {
 					output file foo.log
 				}
 			}
 			`,
-			expectError: true,
+			output:      `{"logging":{"logs":{"default":{"exclude":["http.log.access.name-override"]},"name-override":{"writer":{"filename":"foo.log","output":"file"},"include":["http.log.access.name-override"]}}},"apps":{"http":{"servers":{"srv0":{"listen":[":8080"],"logs":{"default_logger_name":"name-override"}}}}}}`,
+			expectError: false,
 		},
 	} {
 
diff --git a/caddyconfig/httpcaddyfile/httptype.go b/caddyconfig/httpcaddyfile/httptype.go
index c7aeb94..78a380c 100644
--- a/caddyconfig/httpcaddyfile/httptype.go
+++ b/caddyconfig/httpcaddyfile/httptype.go
@@ -30,6 +30,7 @@ import (
 	"github.com/caddyserver/caddy/v2/modules/caddypki"
 	"github.com/caddyserver/caddy/v2/modules/caddytls"
 	"go.uber.org/zap"
+	"golang.org/x/exp/slices"
 )
 
 func init() {
@@ -241,7 +242,7 @@ func (st ServerType) Setup(
 		if ncl.name == caddy.DefaultLoggerName {
 			hasDefaultLog = true
 		}
-		if _, ok := options["debug"]; ok && ncl.log.Level == "" {
+		if _, ok := options["debug"]; ok && ncl.log != nil && ncl.log.Level == "" {
 			ncl.log.Level = zap.DebugLevel.CapitalString()
 		}
 		customLogs = append(customLogs, ncl)
@@ -324,7 +325,21 @@ func (st ServerType) Setup(
 				Logs: make(map[string]*caddy.CustomLog),
 			}
 		}
+
+		// Add the default log first if defined, so that it doesn't
+		// accidentally get re-created below due to the Exclude logic
+		for _, ncl := range customLogs {
+			if ncl.name == caddy.DefaultLoggerName && ncl.log != nil {
+				cfg.Logging.Logs[caddy.DefaultLoggerName] = ncl.log
+				break
+			}
+		}
+
+		// Add the rest of the custom logs
 		for _, ncl := range customLogs {
+			if ncl.log == nil || ncl.name == caddy.DefaultLoggerName {
+				continue
+			}
 			if ncl.name != "" {
 				cfg.Logging.Logs[ncl.name] = ncl.log
 			}
@@ -338,8 +353,16 @@ func (st ServerType) Setup(
 					cfg.Logging.Logs[caddy.DefaultLoggerName] = defaultLog
 				}
 				defaultLog.Exclude = append(defaultLog.Exclude, ncl.log.Include...)
+
+				// avoid duplicates by sorting + compacting
+				slices.Sort[string](defaultLog.Exclude)
+				defaultLog.Exclude = slices.Compact[[]string, string](defaultLog.Exclude)
 			}
 		}
+		// we may have not actually added anything, so remove if empty
+		if len(cfg.Logging.Logs) == 0 {
+			cfg.Logging = nil
+		}
 	}
 
 	return cfg, warnings, nil
@@ -770,12 +793,20 @@ func (st *ServerType) serversFromPairings(
 			sblockLogHosts := sblock.hostsFromKeys(true)
 			for _, cval := range sblock.pile["custom_log"] {
 				ncl := cval.Value.(namedCustomLog)
-				if sblock.hasHostCatchAllKey() {
+				if sblock.hasHostCatchAllKey() && len(ncl.hostnames) == 0 {
 					// 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 if len(ncl.hostnames) > 0 {
+					// if the logger overrides the hostnames, map that to the logger name
+					for _, h := range ncl.hostnames {
+						if srv.Logs.LoggerNames == nil {
+							srv.Logs.LoggerNames = make(map[string]string)
+						}
+						srv.Logs.LoggerNames[h] = ncl.name
+					}
 				} else {
-					// map each host to the user's desired logger name
+					// otherwise, map each host to the logger name
 					for _, h := range sblockLogHosts {
 						if srv.Logs.LoggerNames == nil {
 							srv.Logs.LoggerNames = make(map[string]string)
@@ -1564,8 +1595,9 @@ func (c counter) nextGroup() string {
 }
 
 type namedCustomLog struct {
-	name string
-	log  *caddy.CustomLog
+	name      string
+	hostnames []string
+	log       *caddy.CustomLog
 }
 
 // sbAddrAssociation is a mapping from a list of