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

6 files changed, 342 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
diff --git a/caddytest/integration/caddyfile_adapt/log_override_hostname.txt b/caddytest/integration/caddyfile_adapt/log_override_hostname.txt
new file mode 100644
index 0000000..4511fd4
--- /dev/null
+++ b/caddytest/integration/caddyfile_adapt/log_override_hostname.txt
@@ -0,0 +1,71 @@
+*.example.com {
+	log {
+		hostnames foo.example.com bar.example.com
+		output file /foo-bar.txt
+	}
+	log {
+		hostnames baz.example.com
+		output file /baz.txt
+	}
+}
+----------
+{
+	"logging": {
+		"logs": {
+			"default": {
+				"exclude": [
+					"http.log.access.log0",
+					"http.log.access.log1"
+				]
+			},
+			"log0": {
+				"writer": {
+					"filename": "/foo-bar.txt",
+					"output": "file"
+				},
+				"include": [
+					"http.log.access.log0"
+				]
+			},
+			"log1": {
+				"writer": {
+					"filename": "/baz.txt",
+					"output": "file"
+				},
+				"include": [
+					"http.log.access.log1"
+				]
+			}
+		}
+	},
+	"apps": {
+		"http": {
+			"servers": {
+				"srv0": {
+					"listen": [
+						":443"
+					],
+					"routes": [
+						{
+							"match": [
+								{
+									"host": [
+										"*.example.com"
+									]
+								}
+							],
+							"terminal": true
+						}
+					],
+					"logs": {
+						"logger_names": {
+							"bar.example.com": "log0",
+							"baz.example.com": "log1",
+							"foo.example.com": "log0"
+						}
+					}
+				}
+			}
+		}
+	}
+}
\ No newline at end of file
diff --git a/caddytest/integration/caddyfile_adapt/log_override_name_multiaccess.txt b/caddytest/integration/caddyfile_adapt/log_override_name_multiaccess.txt
new file mode 100644
index 0000000..a3b0cec
--- /dev/null
+++ b/caddytest/integration/caddyfile_adapt/log_override_name_multiaccess.txt
@@ -0,0 +1,86 @@
+{
+	log access-console {
+		include http.log.access.foo
+		output file access-localhost.log
+		format console
+	}
+
+	log access-json {
+		include http.log.access.foo
+		output file access-localhost.json
+		format json
+	}
+}
+
+http://localhost:8881 {
+	log foo
+}
+----------
+{
+	"logging": {
+		"logs": {
+			"access-console": {
+				"writer": {
+					"filename": "access-localhost.log",
+					"output": "file"
+				},
+				"encoder": {
+					"format": "console"
+				},
+				"include": [
+					"http.log.access.foo"
+				]
+			},
+			"access-json": {
+				"writer": {
+					"filename": "access-localhost.json",
+					"output": "file"
+				},
+				"encoder": {
+					"format": "json"
+				},
+				"include": [
+					"http.log.access.foo"
+				]
+			},
+			"default": {
+				"exclude": [
+					"http.log.access.foo"
+				]
+			}
+		}
+	},
+	"apps": {
+		"http": {
+			"servers": {
+				"srv0": {
+					"listen": [
+						":8881"
+					],
+					"routes": [
+						{
+							"match": [
+								{
+									"host": [
+										"localhost"
+									]
+								}
+							],
+							"terminal": true
+						}
+					],
+					"automatic_https": {
+						"skip": [
+							"localhost"
+						]
+					},
+					"logs": {
+						"logger_names": {
+							"localhost:8881": "foo"
+						}
+					}
+				}
+			}
+		}
+	}
+}
\ No newline at end of file
diff --git a/caddytest/integration/caddyfile_adapt/log_override_name_multiaccess_debug.txt b/caddytest/integration/caddyfile_adapt/log_override_name_multiaccess_debug.txt
new file mode 100644
index 0000000..e6698e4
--- /dev/null
+++ b/caddytest/integration/caddyfile_adapt/log_override_name_multiaccess_debug.txt
@@ -0,0 +1,91 @@
+{
+	debug
+
+	log access-console {
+		include http.log.access.foo
+		output file access-localhost.log
+		format console
+	}
+
+	log access-json {
+		include http.log.access.foo
+		output file access-localhost.json
+		format json
+	}
+}
+
+http://localhost:8881 {
+	log foo
+}
+----------
+{
+	"logging": {
+		"logs": {
+			"access-console": {
+				"writer": {
+					"filename": "access-localhost.log",
+					"output": "file"
+				},
+				"encoder": {
+					"format": "console"
+				},
+				"level": "DEBUG",
+				"include": [
+					"http.log.access.foo"
+				]
+			},
+			"access-json": {
+				"writer": {
+					"filename": "access-localhost.json",
+					"output": "file"
+				},
+				"encoder": {
+					"format": "json"
+				},
+				"level": "DEBUG",
+				"include": [
+					"http.log.access.foo"
+				]
+			},
+			"default": {
+				"level": "DEBUG",
+				"exclude": [
+					"http.log.access.foo"
+				]
+			}
+		}
+	},
+	"apps": {
+		"http": {
+			"servers": {
+				"srv0": {
+					"listen": [
+						":8881"
+					],
+					"routes": [
+						{
+							"match": [
+								{
+									"host": [
+										"localhost"
+									]
+								}
+							],
+							"terminal": true
+						}
+					],
+					"automatic_https": {
+						"skip": [
+							"localhost"
+						]
+					},
+					"logs": {
+						"logger_names": {
+							"localhost:8881": "foo"
+						}
+					}
+				}
+			}
+		}
+	}
+}
\ No newline at end of file