From 466f39b88027d77045fe724f6e80b6f28a2cec67 Mon Sep 17 00:00:00 2001 From: victoredvardsson <62360867+victoredvardsson@users.noreply.github.com> Date: Mon, 23 Dec 2024 11:08:33 +0100 Subject: [PATCH] Add possibility to configure log format #799 (#2941) * make it possible to enable json log * fix * fix typo * fix typo * fix typo * fix typo * fix typo * fix typo * Add error handling * Add log_format to default config * Fix syntax error in if statement * Fix typo * Fix typo * Fix some typos and change naming from native to text, makes more sense * Set same timestamp format for json logging * Fix formatting * Move in if statement under previous * Fix some formatting that got messed up * Default to text formatter, if log_format is not configured. * defining logFormatter outside if statement so that log.SetFormatter(logFormatter) is not undefined when function is called * Add variables that were undefined * Argument were missing when calling SetDefaultLoggerConfig function * Fix order of arguments passed * Fix order of arguments passed * Fix typo * Implicit log_format = "text" * functional test * ignore log_format in FatalHook * make it possible to enable json log * fix * fix typo * fix typo * fix typo * fix typo * fix typo * fix typo * Add error handling * Add log_format to default config * Fix syntax error in if statement * Fix typo * Fix typo * Fix some typos and change naming from native to text, makes more sense * Set same timestamp format for json logging * Fix formatting * Move in if statement under previous * Fix some formatting that got messed up * Default to text formatter, if log_format is not configured. * defining logFormatter outside if statement so that log.SetFormatter(logFormatter) is not undefined when function is called * Add variables that were undefined * Argument were missing when calling SetDefaultLoggerConfig function * Fix order of arguments passed * Fix order of arguments passed * Fix typo * Implicit log_format = "text" * functional test * ignore log_format in FatalHook * lint * fix func test * lint * remove < > characters from log --------- Co-authored-by: Victor Edvardsson Co-authored-by: marco Co-authored-by: Thibault "bui" Koechlin --- .golangci.yml | 5 +++++ cmd/crowdsec-cli/main.go | 5 ++++- cmd/crowdsec/fatalhook.go | 24 ++++++++++++++++++-- cmd/crowdsec/main.go | 7 ++---- pkg/apiserver/apiserver_test.go | 4 ++-- pkg/csconfig/api.go | 4 +++- pkg/csconfig/common.go | 8 +++++-- pkg/csconfig/fflag.go | 2 +- pkg/types/utils.go | 30 ++++++++++++++++++++++--- test/bats/01_crowdsec.bats | 34 +++++++++++++++++++++++++++++ test/bats/crowdsec-acquisition.bats | 2 +- 11 files changed, 107 insertions(+), 18 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index d0fdd3b37f4..b51f17df489 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -459,3 +459,8 @@ issues: - gocritic path: "pkg/(appsec|acquisition|dumps|alertcontext|leakybucket|exprhelpers)" text: "rangeValCopy: .*" + + - linters: + - revive + path: "pkg/types/utils.go" + text: "argument-limit: .*" diff --git a/cmd/crowdsec-cli/main.go b/cmd/crowdsec-cli/main.go index 8b3077a579e..87e9d82fea2 100644 --- a/cmd/crowdsec-cli/main.go +++ b/cmd/crowdsec-cli/main.go @@ -146,7 +146,10 @@ func (cli *cliRoot) initialize() error { return fmt.Errorf("output format '%s' not supported: must be one of human, json, raw", csConfig.Cscli.Output) } - log.SetFormatter(&log.TextFormatter{DisableTimestamp: true}) + log.SetFormatter(&log.TextFormatter{ + DisableTimestamp: true, + DisableLevelTruncation: true, + }) if csConfig.Cscli.Output == "json" { log.SetFormatter(&log.JSONFormatter{}) diff --git a/cmd/crowdsec/fatalhook.go b/cmd/crowdsec/fatalhook.go index 84a57406a21..56e945c84a5 100644 --- a/cmd/crowdsec/fatalhook.go +++ b/cmd/crowdsec/fatalhook.go @@ -2,6 +2,7 @@ package main import ( "io" + "os" log "github.com/sirupsen/logrus" ) @@ -9,16 +10,35 @@ import ( // FatalHook is used to log fatal messages to stderr when the rest goes to a file type FatalHook struct { Writer io.Writer + Formatter log.Formatter LogLevels []log.Level } +func newFatalHook() *FatalHook { + return &FatalHook{ + Writer: os.Stderr, + Formatter: &log.TextFormatter{ + DisableTimestamp: true, + // XXX: logrus.TextFormatter has either key pairs with no colors, + // or "LEVEL [optional timestamp] message", with colors. + // We force colors to make sure we get the latter, even if + // the output is not a terminal. + // There are more flexible formatters that don't conflate the two concepts, + // or we can write our own. + ForceColors: true, + DisableLevelTruncation: true, + }, + LogLevels: []log.Level{log.FatalLevel, log.PanicLevel}, + } +} + func (hook *FatalHook) Fire(entry *log.Entry) error { - line, err := entry.String() + line, err := hook.Formatter.Format(entry) if err != nil { return err } - _, err = hook.Writer.Write([]byte(line)) + _, err = hook.Writer.Write(line) return err } diff --git a/cmd/crowdsec/main.go b/cmd/crowdsec/main.go index 518bd8e9c0d..02220e15216 100644 --- a/cmd/crowdsec/main.go +++ b/cmd/crowdsec/main.go @@ -249,16 +249,13 @@ func LoadConfig(configFile string, disableAgent bool, disableAPI bool, quiet boo if err := types.SetDefaultLoggerConfig(cConfig.Common.LogMedia, cConfig.Common.LogDir, *cConfig.Common.LogLevel, cConfig.Common.LogMaxSize, cConfig.Common.LogMaxFiles, - cConfig.Common.LogMaxAge, cConfig.Common.CompressLogs, + cConfig.Common.LogMaxAge, cConfig.Common.LogFormat, cConfig.Common.CompressLogs, cConfig.Common.ForceColorLogs); err != nil { return nil, err } if cConfig.Common.LogMedia != "stdout" { - log.AddHook(&FatalHook{ - Writer: os.Stderr, - LogLevels: []log.Level{log.FatalLevel, log.PanicLevel}, - }) + log.AddHook(newFatalHook()) } if err := csconfig.LoadFeatureFlagsFile(configFile, log.StandardLogger()); err != nil { diff --git a/pkg/apiserver/apiserver_test.go b/pkg/apiserver/apiserver_test.go index cf4c91dedda..d8f24add75e 100644 --- a/pkg/apiserver/apiserver_test.go +++ b/pkg/apiserver/apiserver_test.go @@ -387,7 +387,7 @@ func TestLoggingDebugToFileConfig(t *testing.T) { cfg.LogLevel = ptr.Of(log.DebugLevel) // Configure logging - err := types.SetDefaultLoggerConfig(cfg.LogMedia, cfg.LogDir, *cfg.LogLevel, cfg.LogMaxSize, cfg.LogMaxFiles, cfg.LogMaxAge, cfg.CompressLogs, false) + err := types.SetDefaultLoggerConfig(cfg.LogMedia, cfg.LogDir, *cfg.LogLevel, cfg.LogMaxSize, cfg.LogMaxFiles, cfg.LogMaxAge, cfg.LogFormat, cfg.CompressLogs, false) require.NoError(t, err) api, err := NewServer(ctx, &cfg) @@ -439,7 +439,7 @@ func TestLoggingErrorToFileConfig(t *testing.T) { cfg.LogLevel = ptr.Of(log.ErrorLevel) // Configure logging - err := types.SetDefaultLoggerConfig(cfg.LogMedia, cfg.LogDir, *cfg.LogLevel, cfg.LogMaxSize, cfg.LogMaxFiles, cfg.LogMaxAge, cfg.CompressLogs, false) + err := types.SetDefaultLoggerConfig(cfg.LogMedia, cfg.LogDir, *cfg.LogLevel, cfg.LogMaxSize, cfg.LogMaxFiles, cfg.LogMaxAge, cfg.LogFormat, cfg.CompressLogs, false) require.NoError(t, err) api, err := NewServer(ctx, &cfg) diff --git a/pkg/csconfig/api.go b/pkg/csconfig/api.go index 5f2f8f9248b..d94d90aaf19 100644 --- a/pkg/csconfig/api.go +++ b/pkg/csconfig/api.go @@ -271,6 +271,7 @@ type LocalApiServerCfg struct { LogMaxSize int `yaml:"-"` LogMaxAge int `yaml:"-"` LogMaxFiles int `yaml:"-"` + LogFormat string `yaml:"-"` TrustedIPs []string `yaml:"trusted_ips,omitempty"` PapiLogLevel *log.Level `yaml:"papi_log_level"` DisableRemoteLapiRegistration bool `yaml:"disable_remote_lapi_registration,omitempty"` @@ -351,7 +352,7 @@ func (c *Config) LoadAPIServer(inCli bool) error { log.Printf("push and pull to Central API disabled") } - //Set default values for CAPI push/pull + // Set default values for CAPI push/pull if c.API.Server.OnlineClient != nil { if c.API.Server.OnlineClient.PullConfig.Community == nil { c.API.Server.OnlineClient.PullConfig.Community = ptr.Of(true) @@ -391,6 +392,7 @@ func (c *Config) LoadAPIServer(inCli bool) error { c.API.Server.CompressLogs = c.Common.CompressLogs c.API.Server.LogMaxSize = c.Common.LogMaxSize c.API.Server.LogMaxAge = c.Common.LogMaxAge + c.API.Server.LogFormat = c.Common.LogFormat c.API.Server.LogMaxFiles = c.Common.LogMaxFiles if c.API.Server.UseForwardedForHeaders && c.API.Server.TrustedProxies == nil { diff --git a/pkg/csconfig/common.go b/pkg/csconfig/common.go index 7e1ef6e5c98..e312756ce20 100644 --- a/pkg/csconfig/common.go +++ b/pkg/csconfig/common.go @@ -12,11 +12,12 @@ type CommonCfg struct { Daemonize bool PidDir string `yaml:"pid_dir,omitempty"` // TODO: This is just for backward compat. Remove this later LogMedia string `yaml:"log_media"` - LogDir string `yaml:"log_dir,omitempty"` //if LogMedia = file + LogDir string `yaml:"log_dir,omitempty"` // if LogMedia = file LogLevel *log.Level `yaml:"log_level"` WorkingDir string `yaml:"working_dir,omitempty"` // TODO: This is just for backward compat. Remove this later CompressLogs *bool `yaml:"compress_logs,omitempty"` LogMaxSize int `yaml:"log_max_size,omitempty"` + LogFormat string `yaml:"log_format,omitempty"` LogMaxAge int `yaml:"log_max_age,omitempty"` LogMaxFiles int `yaml:"log_max_files,omitempty"` ForceColorLogs bool `yaml:"force_color_logs,omitempty"` @@ -24,6 +25,7 @@ type CommonCfg struct { func (c *Config) loadCommon() error { var err error + if c.Common == nil { c.Common = &CommonCfg{} } @@ -32,13 +34,15 @@ func (c *Config) loadCommon() error { c.Common.LogMedia = "stdout" } - var CommonCleanup = []*string{ + CommonCleanup := []*string{ &c.Common.LogDir, } + for _, k := range CommonCleanup { if *k == "" { continue } + *k, err = filepath.Abs(*k) if err != nil { return fmt.Errorf("failed to get absolute path of '%s': %w", *k, err) diff --git a/pkg/csconfig/fflag.go b/pkg/csconfig/fflag.go index c86686889eb..ec1282c5a04 100644 --- a/pkg/csconfig/fflag.go +++ b/pkg/csconfig/fflag.go @@ -38,7 +38,7 @@ func LoadFeatureFlagsFile(configPath string, logger *log.Logger) error { func ListFeatureFlags() string { enabledFeatures := fflag.Crowdsec.GetEnabledFeatures() - msg := "" + msg := "none" if len(enabledFeatures) > 0 { msg = strings.Join(enabledFeatures, ", ") } diff --git a/pkg/types/utils.go b/pkg/types/utils.go index 3e1ae4f7547..d5e4ac6f986 100644 --- a/pkg/types/utils.go +++ b/pkg/types/utils.go @@ -16,21 +16,40 @@ var ( logLevel log.Level ) -func SetDefaultLoggerConfig(cfgMode string, cfgFolder string, cfgLevel log.Level, maxSize int, maxFiles int, maxAge int, compress *bool, forceColors bool) error { - /*Configure logs*/ +func SetDefaultLoggerConfig(cfgMode string, cfgFolder string, cfgLevel log.Level, maxSize int, maxFiles int, maxAge int, format string, compress *bool, forceColors bool) error { + if format == "" { + format = "text" + } + + switch format { + case "text": + logFormatter = &log.TextFormatter{ + TimestampFormat: time.RFC3339, + FullTimestamp: true, + ForceColors: forceColors, + } + case "json": + logFormatter = &log.JSONFormatter{TimestampFormat: time.RFC3339} + default: + return fmt.Errorf("unknown log_format '%s'", format) + } + if cfgMode == "file" { _maxsize := 500 if maxSize != 0 { _maxsize = maxSize } + _maxfiles := 3 if maxFiles != 0 { _maxfiles = maxFiles } + _maxage := 28 if maxAge != 0 { _maxage = maxAge } + _compress := true if compress != nil { _compress = *compress @@ -47,10 +66,11 @@ func SetDefaultLoggerConfig(cfgMode string, cfgFolder string, cfgLevel log.Level } else if cfgMode != "stdout" { return fmt.Errorf("log mode '%s' unknown", cfgMode) } + logLevel = cfgLevel log.SetLevel(logLevel) - logFormatter = &log.TextFormatter{TimestampFormat: time.RFC3339, FullTimestamp: true, ForceColors: forceColors} log.SetFormatter(logFormatter) + return nil } @@ -63,7 +83,9 @@ func ConfigureLogger(clog *log.Logger) error { if logFormatter != nil { clog.SetFormatter(logFormatter) } + clog.SetLevel(logLevel) + return nil } @@ -76,6 +98,8 @@ func IsNetworkFS(path string) (bool, string, error) { if err != nil { return false, "", err } + fsType = strings.ToLower(fsType) + return fsType == "nfs" || fsType == "cifs" || fsType == "smb" || fsType == "smb2", fsType, nil } diff --git a/test/bats/01_crowdsec.bats b/test/bats/01_crowdsec.bats index a768a8d4d28..3df0b42a0f2 100644 --- a/test/bats/01_crowdsec.bats +++ b/test/bats/01_crowdsec.bats @@ -67,6 +67,40 @@ teardown() { refute_output } +@test "crowdsec - log format" { + # fail early + config_disable_lapi + config_disable_agent + + config_set '.common.log_media="stdout"' + + config_set '.common.log_format=""' + rune -0 wait-for --err "you must run at least the API Server or crowdsec" "$CROWDSEC" + assert_stderr --partial 'level=fatal msg="you must run at least the API Server or crowdsec"' + + config_set '.common.log_format="text"' + rune -0 wait-for --err "you must run at least the API Server or crowdsec" "$CROWDSEC" + assert_stderr --partial 'level=fatal msg="you must run at least the API Server or crowdsec"' + + config_set '.common.log_format="json"' + rune -0 wait-for --err "you must run at least the API Server or crowdsec" "$CROWDSEC" + rune -0 jq -c 'select(.msg=="you must run at least the API Server or crowdsec") | .level' <(stderr | grep "^{") + assert_output '"fatal"' + + # If log_media='file', a hook to stderr is added only for fatal messages, + # with a predefined formatter (level + msg, no timestamp, ignore log_format) + + config_set '.common.log_media="file"' + + config_set '.common.log_format="text"' + rune -0 wait-for --err "you must run at least the API Server or crowdsec" "$CROWDSEC" + assert_stderr --regexp 'FATAL.* you must run at least the API Server or crowdsec$' + + config_set '.common.log_format="json"' + rune -0 wait-for --err "you must run at least the API Server or crowdsec" "$CROWDSEC" + assert_stderr --regexp 'FATAL.* you must run at least the API Server or crowdsec$' +} + @test "CS_LAPI_SECRET not strong enough" { CS_LAPI_SECRET=foo rune -1 wait-for "$CROWDSEC" assert_stderr --partial "api server init: unable to run local API: controller init: CS_LAPI_SECRET not strong enough" diff --git a/test/bats/crowdsec-acquisition.bats b/test/bats/crowdsec-acquisition.bats index 5189790f01f..1a92624b4c4 100644 --- a/test/bats/crowdsec-acquisition.bats +++ b/test/bats/crowdsec-acquisition.bats @@ -33,7 +33,7 @@ teardown() { EOT rune -1 "$CROWDSEC" -t - assert_stderr --partial "crowdsec init: while loading acquisition config: while configuring datasource of type file from $ACQUIS_DIR/file.yaml (position 0): cannot parse FileAcquisition configuration: yaml: unmarshal errors:\n line 6: cannot unmarshal !!seq into string" + assert_stderr --partial "crowdsec init: while loading acquisition config: while configuring datasource of type file from $ACQUIS_DIR/file.yaml (position 0): cannot parse FileAcquisition configuration: yaml: unmarshal errors:" } @test "datasource type detection" {