diff --git a/.golangci.yml b/.golangci.yml index 7217c6da2b1..a1b215ae8fd 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -192,13 +192,9 @@ linters-settings: - typeUnparen - commentFormatting - deferInLoop # - - sprintfQuotedString # - whyNoLint - equalFold # - unnecessaryBlock # - - ptrToRefParam # - - stringXbytes # - - appendAssign # - tooManyResultsChecker - unnecessaryDefer - docStub diff --git a/pkg/acquisition/acquisition.go b/pkg/acquisition/acquisition.go index 6ac47a8cdf1..4e233aad616 100644 --- a/pkg/acquisition/acquisition.go +++ b/pkg/acquisition/acquisition.go @@ -116,7 +116,7 @@ func setupLogger(source, name string, level *log.Level) (*log.Entry, error) { // if the configuration is not valid it returns an error. // If the datasource can't be run (eg. journalctl not available), it still returns an error which // can be checked for the appropriate action. -func DataSourceConfigure(commonConfig configuration.DataSourceCommonCfg, metricsLevel int) (*DataSource, error) { +func DataSourceConfigure(commonConfig configuration.DataSourceCommonCfg, metricsLevel int) (DataSource, error) { // we dump it back to []byte, because we want to decode the yaml blob twice: // once to DataSourceCommonCfg, and then later to the dedicated type of the datasource yamlConfig, err := yaml.Marshal(commonConfig) @@ -143,7 +143,7 @@ func DataSourceConfigure(commonConfig configuration.DataSourceCommonCfg, metrics return nil, err } - return &dataSrc, nil + return dataSrc, nil } // detectBackwardCompatAcquis: try to magically detect the type for backward compat (type was not mandatory then) @@ -309,7 +309,7 @@ func LoadAcquisitionFromFile(config *csconfig.CrowdsecServiceCfg, prom *csconfig transformRuntimes[uniqueId] = vm } - sources = append(sources, *src) + sources = append(sources, src) } } diff --git a/pkg/acquisition/acquisition_test.go b/pkg/acquisition/acquisition_test.go index 671426d344b..cfe1e74c612 100644 --- a/pkg/acquisition/acquisition_test.go +++ b/pkg/acquisition/acquisition_test.go @@ -193,19 +193,19 @@ wowo: ajsajasjas switch tc.TestName { case "basic_valid_config": - mock := (*ds).Dump().(*MockSource) + mock := ds.Dump().(*MockSource) assert.Equal(t, "test_value1", mock.Toto) assert.Equal(t, "cat", mock.Mode) assert.Equal(t, log.InfoLevel, mock.logger.Logger.Level) assert.Equal(t, map[string]string{"test": "foobar"}, mock.Labels) case "basic_debug_config": - mock := (*ds).Dump().(*MockSource) + mock := ds.Dump().(*MockSource) assert.Equal(t, "test_value1", mock.Toto) assert.Equal(t, "cat", mock.Mode) assert.Equal(t, log.DebugLevel, mock.logger.Logger.Level) assert.Equal(t, map[string]string{"test": "foobar"}, mock.Labels) case "basic_tailmode_config": - mock := (*ds).Dump().(*MockSource) + mock := ds.Dump().(*MockSource) assert.Equal(t, "test_value1", mock.Toto) assert.Equal(t, "tail", mock.Mode) assert.Equal(t, log.DebugLevel, mock.logger.Logger.Level) diff --git a/pkg/acquisition/modules/journalctl/journalctl.go b/pkg/acquisition/modules/journalctl/journalctl.go index 27f20b9f446..47d90e2b3a0 100644 --- a/pkg/acquisition/modules/journalctl/journalctl.go +++ b/pkg/acquisition/modules/journalctl/journalctl.go @@ -53,15 +53,18 @@ func readLine(scanner *bufio.Scanner, out chan string, errChan chan error) error txt := scanner.Text() out <- txt } + if errChan != nil && scanner.Err() != nil { errChan <- scanner.Err() close(errChan) // the error is already consumed by runJournalCtl return nil //nolint:nilerr } + if errChan != nil { close(errChan) } + return nil } @@ -69,15 +72,17 @@ func (j *JournalCtlSource) runJournalCtl(ctx context.Context, out chan types.Eve ctx, cancel := context.WithCancel(ctx) cmd := exec.CommandContext(ctx, journalctlCmd, j.args...) + stdout, err := cmd.StdoutPipe() if err != nil { cancel() - return fmt.Errorf("could not get journalctl stdout: %s", err) + return fmt.Errorf("could not get journalctl stdout: %w", err) } + stderr, err := cmd.StderrPipe() if err != nil { cancel() - return fmt.Errorf("could not get journalctl stderr: %s", err) + return fmt.Errorf("could not get journalctl stderr: %w", err) } stderrChan := make(chan string) @@ -87,6 +92,7 @@ func (j *JournalCtlSource) runJournalCtl(ctx context.Context, out chan types.Eve logger := j.logger.WithField("src", j.src) logger.Infof("Running journalctl command: %s %s", cmd.Path, cmd.Args) + err = cmd.Start() if err != nil { cancel() @@ -109,9 +115,11 @@ func (j *JournalCtlSource) runJournalCtl(ctx context.Context, out chan types.Eve cmd.Wait() return errors.New("failed to create stderr scanner") } + t.Go(func() error { return readLine(stdoutscanner, stdoutChan, errChan) }) + t.Go(func() error { // looks like journalctl closes stderr quite early, so ignore its status (but not its output) return readLine(stderrScanner, stderrChan, nil) @@ -123,6 +131,7 @@ func (j *JournalCtlSource) runJournalCtl(ctx context.Context, out chan types.Eve logger.Infof("journalctl datasource %s stopping", j.src) cancel() cmd.Wait() // avoid zombie process + return nil case stdoutLine := <-stdoutChan: l := types.Line{} @@ -133,6 +142,7 @@ func (j *JournalCtlSource) runJournalCtl(ctx context.Context, out chan types.Eve l.Src = j.src l.Process = true l.Module = j.GetName() + if j.metricsLevel != configuration.METRICS_NONE { linesRead.With(prometheus.Labels{"source": j.src}).Inc() } @@ -149,6 +159,7 @@ func (j *JournalCtlSource) runJournalCtl(ctx context.Context, out chan types.Eve logger.Debugf("errChan is closed, quitting") t.Kill(nil) } + if errScanner != nil { t.Kill(errScanner) } @@ -170,6 +181,7 @@ func (j *JournalCtlSource) GetAggregMetrics() []prometheus.Collector { func (j *JournalCtlSource) UnmarshalConfig(yamlConfig []byte) error { j.config = JournalCtlConfiguration{} + err := yaml.UnmarshalStrict(yamlConfig, &j.config) if err != nil { return fmt.Errorf("cannot parse JournalCtlSource configuration: %w", err) @@ -189,8 +201,11 @@ func (j *JournalCtlSource) UnmarshalConfig(yamlConfig []byte) error { if len(j.config.Filters) == 0 { return errors.New("journalctl_filter is required") } - j.args = append(args, j.config.Filters...) - j.src = fmt.Sprintf("journalctl-%s", strings.Join(j.config.Filters, ".")) + + args = append(args, j.config.Filters...) + + j.args = args + j.src = "journalctl-%s" + strings.Join(j.config.Filters, ".") return nil } @@ -226,8 +241,9 @@ func (j *JournalCtlSource) ConfigureByDSN(dsn string, labels map[string]string, params, err := url.ParseQuery(qs) if err != nil { - return fmt.Errorf("could not parse journalctl DSN : %s", err) + return fmt.Errorf("could not parse journalctl DSN: %w", err) } + for key, value := range params { switch key { case "filters": @@ -236,10 +252,12 @@ func (j *JournalCtlSource) ConfigureByDSN(dsn string, labels map[string]string, if len(value) != 1 { return errors.New("expected zero or one value for 'log_level'") } + lvl, err := log.ParseLevel(value[0]) if err != nil { return fmt.Errorf("unknown level %s: %w", value[0], err) } + j.logger.Logger.SetLevel(lvl) case "since": j.args = append(j.args, "--since", value[0]) @@ -247,7 +265,9 @@ func (j *JournalCtlSource) ConfigureByDSN(dsn string, labels map[string]string, return fmt.Errorf("unsupported key %s in journalctl DSN", key) } } + j.args = append(j.args, j.config.Filters...) + return nil } @@ -261,8 +281,10 @@ func (j *JournalCtlSource) GetName() string { func (j *JournalCtlSource) OneShotAcquisition(ctx context.Context, out chan types.Event, t *tomb.Tomb) error { defer trace.CatchPanic("crowdsec/acquis/journalctl/oneshot") + err := j.runJournalCtl(ctx, out, t) j.logger.Debug("Oneshot journalctl acquisition is done") + return err } @@ -271,6 +293,7 @@ func (j *JournalCtlSource) StreamingAcquisition(ctx context.Context, out chan ty defer trace.CatchPanic("crowdsec/acquis/journalctl/streaming") return j.runJournalCtl(ctx, out, t) }) + return nil } diff --git a/pkg/acquisition/modules/journalctl/journalctl_test.go b/pkg/acquisition/modules/journalctl/journalctl_test.go index 687067c1881..fedbed6b707 100644 --- a/pkg/acquisition/modules/journalctl/journalctl_test.go +++ b/pkg/acquisition/modules/journalctl/journalctl_test.go @@ -81,7 +81,7 @@ func TestConfigureDSN(t *testing.T) { }, { dsn: "journalctl://filters=%ZZ", - expectedErr: "could not parse journalctl DSN : invalid URL escape \"%ZZ\"", + expectedErr: "could not parse journalctl DSN: invalid URL escape \"%ZZ\"", }, { dsn: "journalctl://filters=_UID=42?log_level=warn", @@ -191,6 +191,7 @@ journalctl_filter: func TestStreaming(t *testing.T) { ctx := context.Background() + if runtime.GOOS == "windows" { t.Skip("Skipping test on windows") } @@ -270,7 +271,7 @@ journalctl_filter: tomb.Wait() output, _ := exec.Command("pgrep", "-x", "journalctl").CombinedOutput() - if string(output) != "" { + if len(output) != 0 { t.Fatalf("Found a journalctl process after killing the tomb !") } diff --git a/pkg/apiserver/apiserver.go b/pkg/apiserver/apiserver.go index 05f9150b037..e1d9ce95349 100644 --- a/pkg/apiserver/apiserver.go +++ b/pkg/apiserver/apiserver.go @@ -209,7 +209,7 @@ func NewServer(ctx context.Context, config *csconfig.LocalApiServerCfg) (*APISer gin.DefaultWriter = clog.Writer() router.Use(gin.LoggerWithFormatter(func(param gin.LogFormatterParams) string { - return fmt.Sprintf("%s - [%s] \"%s %s %s %d %s \"%s\" %s\"\n", + return fmt.Sprintf("%s - [%s] \"%s %s %s %d %s %q %s\"\n", param.ClientIP, param.TimeStamp.Format(time.RFC1123), param.Method, diff --git a/pkg/csprofiles/csprofiles_test.go b/pkg/csprofiles/csprofiles_test.go index 0247243ddd3..d09bf25d95b 100644 --- a/pkg/csprofiles/csprofiles_test.go +++ b/pkg/csprofiles/csprofiles_test.go @@ -132,7 +132,7 @@ func TestEvaluateProfile(t *testing.T) { name: "simple pass single expr", args: args{ profileCfg: &csconfig.ProfileCfg{ - Filters: []string{fmt.Sprintf("Alert.GetScenario() == \"%s\"", scenario)}, + Filters: []string{fmt.Sprintf("Alert.GetScenario() == %q", scenario)}, Debug: &boolFalse, }, Alert: &models.Alert{Remediation: true, Scenario: &scenario}, @@ -199,17 +199,22 @@ func TestEvaluateProfile(t *testing.T) { profilesCfg := []*csconfig.ProfileCfg{ tt.args.profileCfg, } + profile, err := NewProfile(profilesCfg) if err != nil { t.Errorf("failed to get newProfile : %+v", err) } + got, got1, _ := profile[0].EvaluateProfile(tt.args.Alert) + if !reflect.DeepEqual(len(got), tt.expectedDecisionCount) { t.Errorf("EvaluateProfile() got = %+v, want %+v", got, tt.expectedDecisionCount) } + if got1 != tt.expectedMatchStatus { t.Errorf("EvaluateProfile() got1 = %v, want %v", got1, tt.expectedMatchStatus) } + if tt.expectedDuration != "" { require.Equal(t, tt.expectedDuration, *got[0].Duration, "The two durations should be the same") } diff --git a/pkg/hubtest/parser_assert.go b/pkg/hubtest/parser_assert.go index be4fdbdb5e6..90d952506d1 100644 --- a/pkg/hubtest/parser_assert.go +++ b/pkg/hubtest/parser_assert.go @@ -270,7 +270,7 @@ func (p *ParserAssert) AutoGenParserAssert() string { continue } - base := fmt.Sprintf(`results["%s"]["%s"][%d].Evt.Unmarshaled["%s"]`, stage, parser, pidx, ukey) + base := fmt.Sprintf("results[%q][%q][%d].Evt.Unmarshaled[%q]", stage, parser, pidx, ukey) for _, line := range p.buildUnmarshaledAssert(base, uval) { ret += line @@ -295,11 +295,11 @@ func (p *ParserAssert) buildUnmarshaledAssert(ekey string, eval interface{}) []s switch val := eval.(type) { case map[string]interface{}: for k, v := range val { - ret = append(ret, p.buildUnmarshaledAssert(fmt.Sprintf(`%s["%s"]`, ekey, k), v)...) + ret = append(ret, p.buildUnmarshaledAssert(fmt.Sprintf("%s[%q]", ekey, k), v)...) } case map[interface{}]interface{}: for k, v := range val { - ret = append(ret, p.buildUnmarshaledAssert(fmt.Sprintf(`%s["%s"]`, ekey, k), v)...) + ret = append(ret, p.buildUnmarshaledAssert(fmt.Sprintf("%s[%q]", ekey, k), v)...) } case []interface{}: case string: