From bcce4afe5eb32e6b81d0e385c4f5d3f8aa1c91b2 Mon Sep 17 00:00:00 2001 From: Laurence Jones Date: Fri, 7 Mar 2025 13:42:08 +0000 Subject: [PATCH] enhance: Flags now superceed all log levels (#3496) * enhance: Flags now superceed all log levels * enhance: remove global var for local scope * test --------- Co-authored-by: marco --- cmd/crowdsec/main.go | 15 +++++++++------ pkg/acquisition/acquisition.go | 6 +----- pkg/alertcontext/alertcontext.go | 2 +- pkg/apiserver/apiserver.go | 6 +----- pkg/apiserver/apiserver_test.go | 4 ++-- pkg/apiserver/papi.go | 4 +--- pkg/cache/cache.go | 3 +-- pkg/csplugin/broker.go | 4 ++-- pkg/csprofiles/csprofiles.go | 4 ++-- pkg/database/database.go | 6 +----- pkg/exprhelpers/crowdsec_cti.go | 5 +---- pkg/leakybucket/manager_load.go | 4 ++-- pkg/parser/node.go | 4 ++-- pkg/types/utils.go | 16 +++++++++++----- test/bats/01_crowdsec.bats | 18 ++++++++++++++++++ 15 files changed, 55 insertions(+), 46 deletions(-) diff --git a/cmd/crowdsec/main.go b/cmd/crowdsec/main.go index 02220e152..188116fb2 100644 --- a/cmd/crowdsec/main.go +++ b/cmd/crowdsec/main.go @@ -186,9 +186,10 @@ func (f *Flags) Parse() { flag.Parse() } -func newLogLevel(curLevelPtr *log.Level, f *Flags) *log.Level { +func newLogLevel(curLevelPtr *log.Level, f *Flags) (*log.Level, bool) { // mother of all defaults ret := log.InfoLevel + logLevelViaFlag := true // keep if already set if curLevelPtr != nil { @@ -210,14 +211,16 @@ func newLogLevel(curLevelPtr *log.Level, f *Flags) *log.Level { case f.LogLevelFatal: ret = log.FatalLevel default: + // We set logLevelViaFlag to false in default cause no flag was provided + logLevelViaFlag = false } if curLevelPtr != nil && ret == *curLevelPtr { // avoid returning a new ptr to the same value - return curLevelPtr + return curLevelPtr, logLevelViaFlag } - return &ret + return &ret, logLevelViaFlag } // LoadConfig returns a configuration parsed from configuration file @@ -230,8 +233,8 @@ func LoadConfig(configFile string, disableAgent bool, disableAPI bool, quiet boo if err := trace.Init(filepath.Join(cConfig.ConfigPaths.DataDir, "trace")); err != nil { return nil, fmt.Errorf("while setting up trace directory: %w", err) } - - cConfig.Common.LogLevel = newLogLevel(cConfig.Common.LogLevel, flags) + var logLevelViaFlag bool + cConfig.Common.LogLevel, logLevelViaFlag = newLogLevel(cConfig.Common.LogLevel, flags) if dumpFolder != "" { parser.ParseDump = true @@ -250,7 +253,7 @@ func LoadConfig(configFile string, disableAgent bool, disableAPI bool, quiet boo cConfig.Common.LogDir, *cConfig.Common.LogLevel, cConfig.Common.LogMaxSize, cConfig.Common.LogMaxFiles, cConfig.Common.LogMaxAge, cConfig.Common.LogFormat, cConfig.Common.CompressLogs, - cConfig.Common.ForceColorLogs); err != nil { + cConfig.Common.ForceColorLogs, logLevelViaFlag); err != nil { return nil, err } diff --git a/pkg/acquisition/acquisition.go b/pkg/acquisition/acquisition.go index d39282705..4462d8df0 100644 --- a/pkg/acquisition/acquisition.go +++ b/pkg/acquisition/acquisition.go @@ -92,14 +92,10 @@ func registerDataSource(dataSourceType string, dsGetter func() DataSource) { // setupLogger creates a logger for the datasource to use at runtime. func setupLogger(source, name string, level *log.Level) (*log.Entry, error) { clog := log.New() - if err := types.ConfigureLogger(clog); err != nil { + if err := types.ConfigureLogger(clog, level); err != nil { return nil, fmt.Errorf("while configuring datasource logger: %w", err) } - if level != nil { - clog.SetLevel(*level) - } - fields := log.Fields{ "type": source, } diff --git a/pkg/alertcontext/alertcontext.go b/pkg/alertcontext/alertcontext.go index 0b38336a6..3c5a3e10c 100644 --- a/pkg/alertcontext/alertcontext.go +++ b/pkg/alertcontext/alertcontext.go @@ -45,7 +45,7 @@ func ValidateContextExpr(key string, expressions []string) error { func NewAlertContext(contextToSend map[string][]string, valueLength int) error { clog := log.New() - if err := types.ConfigureLogger(clog); err != nil { + if err := types.ConfigureLogger(clog, nil); err != nil { return fmt.Errorf("couldn't create logger for alert context: %w", err) } diff --git a/pkg/apiserver/apiserver.go b/pkg/apiserver/apiserver.go index a14e656fa..a14aeac09 100644 --- a/pkg/apiserver/apiserver.go +++ b/pkg/apiserver/apiserver.go @@ -118,14 +118,10 @@ func CustomRecoveryWithWriter() gin.HandlerFunc { func newGinLogger(config *csconfig.LocalApiServerCfg) (*log.Logger, string, error) { clog := log.New() - if err := types.ConfigureLogger(clog); err != nil { + if err := types.ConfigureLogger(clog, config.LogLevel); err != nil { return nil, "", fmt.Errorf("while configuring gin logger: %w", err) } - if config.LogLevel != nil { - clog.SetLevel(*config.LogLevel) - } - if config.LogMedia != "file" { return clog, "", nil } diff --git a/pkg/apiserver/apiserver_test.go b/pkg/apiserver/apiserver_test.go index fe250aa37..01a8588df 100644 --- a/pkg/apiserver/apiserver_test.go +++ b/pkg/apiserver/apiserver_test.go @@ -388,7 +388,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.LogFormat, cfg.CompressLogs, false) + err := types.SetDefaultLoggerConfig(cfg.LogMedia, cfg.LogDir, *cfg.LogLevel, cfg.LogMaxSize, cfg.LogMaxFiles, cfg.LogMaxAge, cfg.LogFormat, cfg.CompressLogs, false, false) require.NoError(t, err) api, err := NewServer(ctx, &cfg) @@ -440,7 +440,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.LogFormat, cfg.CompressLogs, false) + err := types.SetDefaultLoggerConfig(cfg.LogMedia, cfg.LogDir, *cfg.LogLevel, cfg.LogMaxSize, cfg.LogMaxFiles, cfg.LogMaxAge, cfg.LogFormat, cfg.CompressLogs, false, false) require.NoError(t, err) api, err := NewServer(ctx, &cfg) diff --git a/pkg/apiserver/papi.go b/pkg/apiserver/papi.go index 6f50a08fa..442c57295 100644 --- a/pkg/apiserver/papi.go +++ b/pkg/apiserver/papi.go @@ -83,12 +83,10 @@ type PapiPermCheckSuccess struct { func NewPAPI(apic *apic, dbClient *database.Client, consoleConfig *csconfig.ConsoleConfig, logLevel log.Level) (*Papi, error) { logger := log.New() - if err := types.ConfigureLogger(logger); err != nil { + if err := types.ConfigureLogger(logger, &logLevel); err != nil { return &Papi{}, fmt.Errorf("creating papi logger: %w", err) } - logger.SetLevel(logLevel) - papiUrl := *apic.apiClient.PapiURL papiUrl.Path = fmt.Sprintf("%s%s", types.PAPIVersion, types.PAPIPollUrl) diff --git a/pkg/cache/cache.go b/pkg/cache/cache.go index 8a696caf1..3885294c5 100644 --- a/pkg/cache/cache.go +++ b/pkg/cache/cache.go @@ -59,11 +59,10 @@ func CacheInit(cfg CacheCfg) error { clog := log.New() - if err := types.ConfigureLogger(clog); err != nil { + if err := types.ConfigureLogger(clog, cfg.LogLevel); err != nil { return fmt.Errorf("while creating cache logger: %w", err) } - clog.SetLevel(*cfg.LogLevel) cfg.Logger = clog.WithField("cache", cfg.Name) tmpCache := gcache.New(cfg.Size) diff --git a/pkg/csplugin/broker.go b/pkg/csplugin/broker.go index 3d0404596..df78d258d 100644 --- a/pkg/csplugin/broker.go +++ b/pkg/csplugin/broker.go @@ -20,6 +20,7 @@ import ( "gopkg.in/yaml.v2" "github.com/crowdsecurity/go-cs-lib/csstring" + "github.com/crowdsecurity/go-cs-lib/ptr" "github.com/crowdsecurity/go-cs-lib/slicetools" "github.com/crowdsecurity/crowdsec/pkg/csconfig" @@ -321,13 +322,12 @@ func (pb *PluginBroker) loadNotificationPlugin(name string, binaryPath string) ( pb.pluginMap[name] = &NotifierPlugin{} l := log.New() - err = types.ConfigureLogger(l) + err = types.ConfigureLogger(l, ptr.Of(log.TraceLevel)) if err != nil { return nil, err } // We set the highest level to permit plugins to set their own log level // without that, crowdsec log level is controlling plugins level - l.SetLevel(log.TraceLevel) logger := NewHCLogAdapter(l, "") c := plugin.NewClient(&plugin.ClientConfig{ HandshakeConfig: handshake, diff --git a/pkg/csprofiles/csprofiles.go b/pkg/csprofiles/csprofiles.go index c509fb448..c9a7cda79 100644 --- a/pkg/csprofiles/csprofiles.go +++ b/pkg/csprofiles/csprofiles.go @@ -12,6 +12,7 @@ import ( "github.com/crowdsecurity/crowdsec/pkg/exprhelpers" "github.com/crowdsecurity/crowdsec/pkg/models" "github.com/crowdsecurity/crowdsec/pkg/types" + "github.com/crowdsecurity/go-cs-lib/ptr" ) type Runtime struct { @@ -34,11 +35,10 @@ func NewProfile(profilesCfg []*csconfig.ProfileCfg) ([]*Runtime, error) { runtime := &Runtime{} xlog := log.New() - if err := types.ConfigureLogger(xlog); err != nil { + if err := types.ConfigureLogger(xlog, ptr.Of(log.InfoLevel)); err != nil { return nil, fmt.Errorf("while configuring profiles-specific logger: %w", err) } - xlog.SetLevel(log.InfoLevel) runtime.Logger = xlog.WithFields(log.Fields{ "type": "profile", "name": profile.Name, diff --git a/pkg/database/database.go b/pkg/database/database.go index 804797107..d5186a76d 100644 --- a/pkg/database/database.go +++ b/pkg/database/database.go @@ -52,14 +52,10 @@ func NewClient(ctx context.Context, config *csconfig.DatabaseCfg) (*Client, erro } /*The logger that will be used by db operations*/ clog := log.New() - if err := types.ConfigureLogger(clog); err != nil { + if err := types.ConfigureLogger(clog, config.LogLevel); err != nil { return nil, fmt.Errorf("while configuring db logger: %w", err) } - if config.LogLevel != nil { - clog.SetLevel(*config.LogLevel) - } - entLogger := clog.WithField("context", "ent") entOpt := ent.Log(entLogger.Debug) diff --git a/pkg/exprhelpers/crowdsec_cti.go b/pkg/exprhelpers/crowdsec_cti.go index 900bd7824..4d720dc1b 100644 --- a/pkg/exprhelpers/crowdsec_cti.go +++ b/pkg/exprhelpers/crowdsec_cti.go @@ -44,12 +44,9 @@ func InitCrowdsecCTI(key *string, ttl *time.Duration, size *int, logLevel *log.L *ttl = 5 * time.Minute } clog := log.New() - if err := types.ConfigureLogger(clog); err != nil { + if err := types.ConfigureLogger(clog, logLevel); err != nil { return fmt.Errorf("while configuring datasource logger: %w", err) } - if logLevel != nil { - clog.SetLevel(*logLevel) - } subLogger := clog.WithField("type", "crowdsec-cti") CrowdsecCTIInitCache(*size, *ttl) ctiClient = cticlient.NewCrowdsecCTIClient(cticlient.WithAPIKey(CTIApiKey), cticlient.WithLogger(subLogger)) diff --git a/pkg/leakybucket/manager_load.go b/pkg/leakybucket/manager_load.go index 9216c7f67..474f0fe5e 100644 --- a/pkg/leakybucket/manager_load.go +++ b/pkg/leakybucket/manager_load.go @@ -24,6 +24,7 @@ import ( "github.com/crowdsecurity/crowdsec/pkg/cwversion/constraint" "github.com/crowdsecurity/crowdsec/pkg/exprhelpers" "github.com/crowdsecurity/crowdsec/pkg/types" + "github.com/crowdsecurity/go-cs-lib/ptr" ) // BucketFactory struct holds all fields for any bucket configuration. This is to have a @@ -345,11 +346,10 @@ func LoadBucket(bucketFactory *BucketFactory, tomb *tomb.Tomb) error { if bucketFactory.Debug { clog := log.New() - if err = types.ConfigureLogger(clog); err != nil { + if err = types.ConfigureLogger(clog, ptr.Of(log.DebugLevel)); err != nil { return fmt.Errorf("while creating bucket-specific logger: %w", err) } - clog.SetLevel(log.DebugLevel) bucketFactory.logger = clog.WithFields(log.Fields{ "cfg": bucketFactory.BucketName, "name": bucketFactory.Name, diff --git a/pkg/parser/node.go b/pkg/parser/node.go index 1229a0f44..7332356b9 100644 --- a/pkg/parser/node.go +++ b/pkg/parser/node.go @@ -14,6 +14,7 @@ import ( log "github.com/sirupsen/logrus" yaml "gopkg.in/yaml.v2" + "github.com/crowdsecurity/go-cs-lib/ptr" "github.com/crowdsecurity/grokky" "github.com/crowdsecurity/crowdsec/pkg/cache" @@ -462,11 +463,10 @@ func (n *Node) compile(pctx *UnixParserCtx, ectx EnricherCtx) error { that will be used only for processing this node ;) */ if n.Debug { clog := log.New() - if err = types.ConfigureLogger(clog); err != nil { + if err = types.ConfigureLogger(clog, ptr.Of(log.DebugLevel)); err != nil { return fmt.Errorf("while creating bucket-specific logger: %w", err) } - clog.SetLevel(log.DebugLevel) n.Logger = clog.WithField("id", n.rn) n.Logger.Infof("%s has debug enabled", n.Name) } else { diff --git a/pkg/types/utils.go b/pkg/types/utils.go index d5e4ac6f9..6b1506f0d 100644 --- a/pkg/types/utils.go +++ b/pkg/types/utils.go @@ -11,12 +11,13 @@ import ( ) var ( - logFormatter log.Formatter - LogOutput *lumberjack.Logger // io.Writer - logLevel log.Level + logFormatter log.Formatter + LogOutput *lumberjack.Logger // io.Writer + logLevel log.Level + logLevelViaFlag bool ) -func SetDefaultLoggerConfig(cfgMode string, cfgFolder string, cfgLevel log.Level, maxSize int, maxFiles int, maxAge int, format string, compress *bool, forceColors bool) error { +func SetDefaultLoggerConfig(cfgMode string, cfgFolder string, cfgLevel log.Level, maxSize int, maxFiles int, maxAge int, format string, compress *bool, forceColors bool, levelViaFlag bool) error { if format == "" { format = "text" } @@ -68,13 +69,14 @@ func SetDefaultLoggerConfig(cfgMode string, cfgFolder string, cfgLevel log.Level } logLevel = cfgLevel + logLevelViaFlag = levelViaFlag log.SetLevel(logLevel) log.SetFormatter(logFormatter) return nil } -func ConfigureLogger(clog *log.Logger) error { +func ConfigureLogger(clog *log.Logger, level *log.Level) error { /*Configure logs*/ if LogOutput != nil { clog.SetOutput(LogOutput) @@ -86,6 +88,10 @@ func ConfigureLogger(clog *log.Logger) error { clog.SetLevel(logLevel) + if level != nil && !logLevelViaFlag { + clog.SetLevel(*level) + } + return nil } diff --git a/test/bats/01_crowdsec.bats b/test/bats/01_crowdsec.bats index 3df0b42a0..2d2807b39 100644 --- a/test/bats/01_crowdsec.bats +++ b/test/bats/01_crowdsec.bats @@ -101,6 +101,24 @@ teardown() { assert_stderr --regexp 'FATAL.* you must run at least the API Server or crowdsec$' } +@test "crowdsec - pass log level flag to apiserver" { + LOCAL_API_CREDENTIALS=$(config_get '.api.client.credentials_path') + config_set "$LOCAL_API_CREDENTIALS" '.password="badpassword"' + + config_set '.common.log_media="stdout"' + rune -1 "$CROWDSEC" + + # info + assert_stderr --partial "/v1/watchers/login" + # fatal + assert_stderr --partial "incorrect Username or Password" + + config_set '.common.log_media="stdout"' + rune -1 "$CROWDSEC" -error + + refute_stderr --partial "/v1/watchers/login" +} + @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"