From 2a127ac3d1650052f39e93f277e1dfd8c7c730e4 Mon Sep 17 00:00:00 2001 From: Aaron Taylor Date: Fri, 12 Mar 2021 15:00:02 -0500 Subject: 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 | 87 +++++++++++++++++++--- caddyconfig/httpcaddyfile/builtins_test.go | 30 +++++++- caddyconfig/httpcaddyfile/httptype.go | 49 ++++++++---- caddyconfig/httpcaddyfile/options.go | 50 +++++++++++++ caddyconfig/httpcaddyfile/options_test.go | 64 ++++++++++++++++ .../global_options_log_and_site.txt | 77 +++++++++++++++++++ .../caddyfile_adapt/global_options_log_basic.txt | 18 +++++ .../caddyfile_adapt/global_options_log_custom.txt | 39 ++++++++++ .../caddyfile_adapt/global_options_log_multi.txt | 26 +++++++ 9 files changed, 414 insertions(+), 26 deletions(-) create mode 100644 caddyconfig/httpcaddyfile/options_test.go create mode 100644 caddytest/integration/caddyfile_adapt/global_options_log_and_site.txt create mode 100644 caddytest/integration/caddyfile_adapt/global_options_log_basic.txt create mode 100644 caddytest/integration/caddyfile_adapt/global_options_log_custom.txt create mode 100644 caddytest/integration/caddyfile_adapt/global_options_log_multi.txt diff --git a/caddyconfig/httpcaddyfile/builtins.go b/caddyconfig/httpcaddyfile/builtins.go index 8a8f3cc..8830f52 100644 --- a/caddyconfig/httpcaddyfile/builtins.go +++ b/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", diff --git a/caddyconfig/httpcaddyfile/builtins_test.go b/caddyconfig/httpcaddyfile/builtins_test.go index 09bb4ed..4962383 100644 --- a/caddyconfig/httpcaddyfile/builtins_test.go +++ b/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) + } } } diff --git a/caddyconfig/httpcaddyfile/httptype.go b/caddyconfig/httpcaddyfile/httptype.go index f9195a6..4288076 100644 --- a/caddyconfig/httpcaddyfile/httptype.go +++ b/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 } diff --git a/caddyconfig/httpcaddyfile/options.go b/caddyconfig/httpcaddyfile/options.go index 54672a6..9dd215f 100644 --- a/caddyconfig/httpcaddyfile/options.go +++ b/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 ... +// format ... +// level +// include +// exclude +// } +// +// 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 +} diff --git a/caddyconfig/httpcaddyfile/options_test.go b/caddyconfig/httpcaddyfile/options_test.go new file mode 100644 index 0000000..bc9e881 --- /dev/null +++ b/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) + } + } +} diff --git a/caddytest/integration/caddyfile_adapt/global_options_log_and_site.txt b/caddytest/integration/caddyfile_adapt/global_options_log_and_site.txt new file mode 100644 index 0000000..037c8b6 --- /dev/null +++ b/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" + } + } + } + } + } +} diff --git a/caddytest/integration/caddyfile_adapt/global_options_log_basic.txt b/caddytest/integration/caddyfile_adapt/global_options_log_basic.txt new file mode 100644 index 0000000..b8d32dc --- /dev/null +++ b/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" + } + } + } + } +} diff --git a/caddytest/integration/caddyfile_adapt/global_options_log_custom.txt b/caddytest/integration/caddyfile_adapt/global_options_log_custom.txt new file mode 100644 index 0000000..abbca19 --- /dev/null +++ b/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" + } + } + } + } + } +} diff --git a/caddytest/integration/caddyfile_adapt/global_options_log_multi.txt b/caddytest/integration/caddyfile_adapt/global_options_log_multi.txt new file mode 100644 index 0000000..c20251a --- /dev/null +++ b/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" + } + } + } + } +} -- cgit v1.2.3