caddyconfig: add global option for configuring loggers (#4028)

This change is aimed at enhancing the logging module within the
Caddyfile directive to allow users to configure logs other than the HTTP
access log stream, which is the current capability of the Caddyfile [1].
The intent here is to leverage the same syntax as the server log
directive at a global level, so that similar customizations can be added
without needing to resort to a JSON-based configuration.

Discussion for this approach happened in the referenced issue.

Closes https://github.com/caddyserver/caddy/issues/3958

[1] https://caddyserver.com/docs/caddyfile/directives/log

caddyconfig/httpcaddyfile/builtins.go

@@ -619,11 +619,50 @@ func parseHandleErrors(h Helper) ([]ConfigValue, error) {
 //     }
 //
 func parseLog(h Helper) ([]ConfigValue, error) {
+	return parseLogHelper(h, nil)
+}
+
+// parseLogHelper is used both for the parseLog directive within Server Blocks,
+// as well as the global "log" option for configuring loggers at the global
+// level. The parseAsGlobalOption parameter is used to distinguish any differing logic
+// between the two.
+func parseLogHelper(h Helper, globalLogNames map[string]struct{}) ([]ConfigValue, error) {
+	// When the globalLogNames parameter is passed in, we make
+	// modifications to the parsing behavior.
+	parseAsGlobalOption := globalLogNames != nil
+
 	var configValues []ConfigValue
 	for h.Next() {
-		// log does not currently support any arguments
-		if h.NextArg() {
-			return nil, h.ArgErr()
+		// Logic below expects that a name is always present when a
+		// global option is being parsed.
+		var globalLogName string
+		if parseAsGlobalOption {
+			if h.NextArg() {
+				globalLogName = h.Val()
+
+				// Only a single argument is supported.
+				if h.NextArg() {
+					return nil, h.ArgErr()
+				}
+			} else {
+				// If there is no log name specified, we
+				// reference the default logger. See the
+				// setupNewDefault function in the logging
+				// package for where this is configured.
+				globalLogName = "default"
+			}
+
+			// Verify this name is unused.
+			_, used := globalLogNames[globalLogName]
+			if used {
+				return nil, h.Err("duplicate global log option for: " + globalLogName)
+			}
+			globalLogNames[globalLogName] = struct{}{}
+		} else {
+			// No arguments are supported for the server block log directive
+			if h.NextArg() {
+				return nil, h.ArgErr()
+			}
 		}
 
 		cl := new(caddy.CustomLog)
@@ -687,22 +726,48 @@ func parseLog(h Helper) ([]ConfigValue, error) {
 					return nil, h.ArgErr()
 				}
 
+			case "include":
+				// This configuration is only allowed in the global options
+				if !parseAsGlobalOption {
+					return nil, h.ArgErr()
+				}
+				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()
+				}
+				for h.NextArg() {
+					cl.Exclude = append(cl.Exclude, h.Val())
+				}
+
 			default:
 				return nil, h.Errf("unrecognized subdirective: %s", h.Val())
 			}
 		}
 
 		var val namedCustomLog
+		// Skip handling of empty logging configs
 		if !reflect.DeepEqual(cl, new(caddy.CustomLog)) {
-			logCounter, ok := h.State["logCounter"].(int)
-			if !ok {
-				logCounter = 0
+			if parseAsGlobalOption {
+				// Use indicated name for global log options
+				val.name = globalLogName
+				val.log = cl
+			} else {
+				// 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
 			}
-			val.name = fmt.Sprintf("log%d", logCounter)
-			cl.Include = []string{"http.log.access." + val.name}
-			val.log = cl
-			logCounter++
-			h.State["logCounter"] = logCounter
 		}
 		configValues = append(configValues, ConfigValue{
 			Class: "custom_log",

caddyconfig/httpcaddyfile/builtins_test.go

@@ -10,6 +10,7 @@ import (
 func TestLogDirectiveSyntax(t *testing.T) {
 	for i, tc := range []struct {
 		input       string
+		output      string
 		expectError bool
 	}{
 		{
@@ -17,6 +18,7 @@ func TestLogDirectiveSyntax(t *testing.T) {
 				log
 			}
 			`,
+			output:      `{"apps":{"http":{"servers":{"srv0":{"listen":[":8080"],"logs":{}}}}}}`,
 			expectError: false,
 		},
 		{
@@ -26,11 +28,31 @@ func TestLogDirectiveSyntax(t *testing.T) {
 				}
 			}
 			`,
+			output:      `{"logging":{"logs":{"default":{"exclude":["http.log.access.log0"]},"log0":{"writer":{"filename":"foo.log","output":"file"},"include":["http.log.access.log0"]}}},"apps":{"http":{"servers":{"srv0":{"listen":[":8080"],"logs":{"default_logger_name":"log0"}}}}}}`,
 			expectError: false,
 		},
 		{
 			input: `:8080 {
-				log /foo {
+				log {
+					format filter {
+						wrap console
+						fields {
+							common_log delete
+							request>remote_addr ip_mask {
+								ipv4 24
+								ipv6 32
+							}
+						}
+					}
+				}
+			}
+			`,
+			output:      `{"logging":{"logs":{"default":{"exclude":["http.log.access.log0"]},"log0":{"encoder":{"fields":{"common_log":{"filter":"delete"},"request\u003eremote_addr":{"filter":"ip_mask","ipv4_cidr":24,"ipv6_cidr":32}},"format":"filter","wrap":{"format":"console"}},"include":["http.log.access.log0"]}}},"apps":{"http":{"servers":{"srv0":{"listen":[":8080"],"logs":{"default_logger_name":"log0"}}}}}}`,
+			expectError: false,
+		},
+		{
+			input: `:8080 {
+				log invalid {
 					output file foo.log
 				}
 			}
@@ -43,12 +65,16 @@ func TestLogDirectiveSyntax(t *testing.T) {
 			ServerType: ServerType{},
 		}
 
-		_, _, err := adapter.Adapt([]byte(tc.input), nil)
+		out, _, err := adapter.Adapt([]byte(tc.input), nil)
 
 		if err != nil != tc.expectError {
 			t.Errorf("Test %d error expectation failed Expected: %v, got %s", i, tc.expectError, err)
 			continue
 		}
+
+		if string(out) != tc.output {
+			t.Errorf("Test %d error output mismatch Expected: %s, got %s", i, tc.output, out)
+		}
 	}
 }
 

caddyconfig/httpcaddyfile/httptype.go

@@ -240,20 +240,29 @@ func (st ServerType) Setup(inputServerBlocks []caddyfile.ServerBlock,
 	// extract any custom logs, and enforce configured levels
 	var customLogs []namedCustomLog
 	var hasDefaultLog bool
+	addCustomLog := func(ncl namedCustomLog) {
+		if ncl.name == "" {
+			return
+		}
+		if ncl.name == "default" {
+			hasDefaultLog = true
+		}
+		if _, ok := options["debug"]; ok && ncl.log.Level == "" {
+			ncl.log.Level = "DEBUG"
+		}
+		customLogs = append(customLogs, ncl)
+	}
+	// Apply global log options, when set
+	if options["log"] != nil {
+		for _, logValue := range options["log"].([]ConfigValue) {
+			addCustomLog(logValue.Value.(namedCustomLog))
+		}
+	}
+	// Apply server-specific log options
 	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)
+				addCustomLog(clVal.Value.(namedCustomLog))
 			}
 		}
 	}
@@ -313,7 +322,7 @@ func (st ServerType) Setup(inputServerBlocks []caddyfile.ServerBlock,
 			// 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 {
+			if ncl.name != "default" && len(ncl.log.Include) > 0 {
 				defaultLog, ok := cfg.Logging.Logs["default"]
 				if !ok {
 					defaultLog = new(caddy.CustomLog)
@@ -362,11 +371,25 @@ func (ServerType) evaluateGlobalOptionsBlock(serverBlocks []serverBlock, options
 			}
 			serverOpts, ok := val.(serverOptions)
 			if !ok {
-				return nil, fmt.Errorf("unexpected type from 'servers' global options")
+				return nil, fmt.Errorf("unexpected type from 'servers' global options: %T", val)
 			}
 			options[opt] = append(existingOpts, serverOpts)
 			continue
 		}
+		// Additionally, fold multiple "log" options together into an
+		// array so that multiple loggers can be configured.
+		if opt == "log" {
+			existingOpts, ok := options[opt].([]ConfigValue)
+			if !ok {
+				existingOpts = []ConfigValue{}
+			}
+			logOpts, ok := val.([]ConfigValue)
+			if !ok {
+				return nil, fmt.Errorf("unexpected type from 'log' global options: %T", val)
+			}
+			options[opt] = append(existingOpts, logOpts...)
+			continue
+		}
 
 		options[opt] = val
 	}

caddyconfig/httpcaddyfile/options.go

@@ -18,6 +18,7 @@ import (
 	"strconv"
 
 	"github.com/caddyserver/caddy/v2"
+	"github.com/caddyserver/caddy/v2/caddyconfig"
 	"github.com/caddyserver/caddy/v2/caddyconfig/caddyfile"
 	"github.com/caddyserver/caddy/v2/modules/caddytls"
 	"github.com/caddyserver/certmagic"
@@ -44,6 +45,7 @@ func init() {
 	RegisterGlobalOption("auto_https", parseOptAutoHTTPS)
 	RegisterGlobalOption("servers", parseServerOptions)
 	RegisterGlobalOption("ocsp_stapling", parseOCSPStaplingOptions)
+	RegisterGlobalOption("log", parseLogOptions)
 }
 
 func parseOptTrue(d *caddyfile.Dispenser, _ interface{}) (interface{}, error) { return true, nil }
@@ -385,3 +387,51 @@ func parseOCSPStaplingOptions(d *caddyfile.Dispenser, _ interface{}) (interface{
 		DisableStapling: val == "off",
 	}, nil
 }
+
+// parseLogOptions parses the global log option. Syntax:
+//
+//     log [name] {
+//         output  <writer_module> ...
+//         format  <encoder_module> ...
+//         level   <level>
+//         include <namespaces...>
+//         exclude <namespaces...>
+//     }
+//
+// When the name argument is unspecified, this directive modifies the default
+// logger.
+//
+func parseLogOptions(d *caddyfile.Dispenser, existingVal interface{}) (interface{}, error) {
+	currentNames := make(map[string]struct{})
+	if existingVal != nil {
+		innerVals, ok := existingVal.([]ConfigValue)
+		if !ok {
+			return nil, d.Errf("existing log values of unexpected type: %T", existingVal)
+		}
+		for _, rawVal := range innerVals {
+			val, ok := rawVal.Value.(namedCustomLog)
+			if !ok {
+				return nil, d.Errf("existing log value of unexpected type: %T", existingVal)
+			}
+			currentNames[val.name] = struct{}{}
+		}
+	}
+
+	var warnings []caddyconfig.Warning
+	// Call out the same parser that handles server-specific log configuration.
+	configValues, err := parseLogHelper(
+		Helper{
+			Dispenser: d,
+			warnings:  &warnings,
+		},
+		currentNames,
+	)
+	if err != nil {
+		return nil, err
+	}
+	if len(warnings) > 0 {
+		return nil, d.Errf("warnings found in parsing global log options: %+v", warnings)
+	}
+
+	return configValues, nil
+}

caddyconfig/httpcaddyfile/options_test.go

@@ -0,0 +1,64 @@
+package httpcaddyfile
+
+import (
+	"testing"
+
+	"github.com/caddyserver/caddy/v2/caddyconfig/caddyfile"
+	_ "github.com/caddyserver/caddy/v2/modules/logging"
+)
+
+func TestGlobalLogOptionSyntax(t *testing.T) {
+	for i, tc := range []struct {
+		input       string
+		output      string
+		expectError bool
+	}{
+		// NOTE: Additional test cases of successful Caddyfile parsing
+		// are present in: caddytest/integration/caddyfile_adapt/
+		{
+			input: `{
+				log default
+			}
+			`,
+			output:      `{}`,
+			expectError: false,
+		},
+		{
+			input: `{
+				log example {
+					output file foo.log
+				}
+				log example {
+					format json
+				}
+			}
+			`,
+			expectError: true,
+		},
+		{
+			input: `{
+				log example /foo {
+					output file foo.log
+				}
+			}
+			`,
+			expectError: true,
+		},
+	} {
+
+		adapter := caddyfile.Adapter{
+			ServerType: ServerType{},
+		}
+
+		out, _, err := adapter.Adapt([]byte(tc.input), nil)
+
+		if err != nil != tc.expectError {
+			t.Errorf("Test %d error expectation failed Expected: %v, got %v", i, tc.expectError, err)
+			continue
+		}
+
+		if string(out) != tc.output {
+			t.Errorf("Test %d error output mismatch Expected: %s, got %s", i, tc.output, out)
+		}
+	}
+}

caddytest/integration/caddyfile_adapt/global_options_log_and_site.txt

@@ -0,0 +1,77 @@
+{
+	log {
+		output file caddy.log
+		include some-log-source
+		exclude admin.api admin2.api
+	}
+	log custom-logger {
+		output file caddy.log
+		level WARN
+		include custom-log-source
+	}
+}
+
+:8884 {
+	log {
+		format json
+		output file access.log
+	}
+}
+----------
+{
+	"logging": {
+		"logs": {
+			"custom-logger": {
+				"writer": {
+					"filename": "caddy.log",
+					"output": "file"
+				},
+				"level": "WARN",
+				"include": [
+					"custom-log-source"
+				]
+			},
+			"default": {
+				"writer": {
+					"filename": "caddy.log",
+					"output": "file"
+				},
+				"include": [
+					"some-log-source"
+				],
+				"exclude": [
+					"admin.api",
+					"admin2.api",
+					"custom-log-source",
+					"http.log.access.log0"
+				]
+			},
+			"log0": {
+				"writer": {
+					"filename": "access.log",
+					"output": "file"
+				},
+				"encoder": {
+					"format": "json"
+				},
+				"include": [
+					"http.log.access.log0"
+				]
+			}
+		}
+	},
+	"apps": {
+		"http": {
+			"servers": {
+				"srv0": {
+					"listen": [
+						":8884"
+					],
+					"logs": {
+						"default_logger_name": "log0"
+					}
+				}
+			}
+		}
+	}
+}

caddytest/integration/caddyfile_adapt/global_options_log_basic.txt

@@ -0,0 +1,18 @@
+{
+	log {
+		output file foo.log
+	}
+}
+----------
+{
+	"logging": {
+		"logs": {
+			"default": {
+				"writer": {
+					"filename": "foo.log",
+					"output": "file"
+				}
+			}
+		}
+	}
+}

caddytest/integration/caddyfile_adapt/global_options_log_custom.txt

@@ -0,0 +1,39 @@
+{
+	log custom-logger {
+		format filter {
+			wrap console
+			fields {
+				common_log delete
+				request>remote_addr ip_mask {
+					ipv4 24
+					ipv6 32
+				}
+			}
+		}
+	}
+}
+----------
+{
+	"logging": {
+		"logs": {
+			"custom-logger": {
+				"encoder": {
+					"fields": {
+						"common_log": {
+							"filter": "delete"
+						},
+						"request\u003eremote_addr": {
+							"filter": "ip_mask",
+							"ipv4_cidr": 24,
+							"ipv6_cidr": 32
+						}
+					},
+					"format": "filter",
+					"wrap": {
+						"format": "console"
+					}
+				}
+			}
+		}
+	}
+}

caddytest/integration/caddyfile_adapt/global_options_log_multi.txt

@@ -0,0 +1,26 @@
+{
+	log first {
+		output file foo.log
+	}
+	log second {
+		format json
+	}
+}
+----------
+{
+	"logging": {
+		"logs": {
+			"first": {
+				"writer": {
+					"filename": "foo.log",
+					"output": "file"
+				}
+			},
+			"second": {
+				"encoder": {
+					"format": "json"
+				}
+			}
+		}
+	}
+}