From 9780841f324f71cbe2d2273a7a972275def9e09d Mon Sep 17 00:00:00 2001 From: marco Date: Wed, 28 Aug 2024 17:14:37 +0200 Subject: [PATCH 1/2] reduce if nesting --- .golangci.yml | 2 +- pkg/acquisition/modules/file/file.go | 174 +++++++++--------- .../internal/parser/rfc3164/parse_test.go | 157 +++++----------- 3 files changed, 140 insertions(+), 193 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index c59ab372799..78b666d25b4 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -27,7 +27,7 @@ linters-settings: nestif: # lower this after refactoring - min-complexity: 20 + min-complexity: 19 nlreturn: block-size: 5 diff --git a/pkg/acquisition/modules/file/file.go b/pkg/acquisition/modules/file/file.go index 34a7052f46f..840e73c626b 100644 --- a/pkg/acquisition/modules/file/file.go +++ b/pkg/acquisition/modules/file/file.go @@ -426,118 +426,120 @@ func (f *FileSource) monitorNewFiles(out chan types.Event, t *tomb.Tomb) error { return nil } - if event.Op&fsnotify.Create == fsnotify.Create { - fi, err := os.Stat(event.Name) - if err != nil { - logger.Errorf("Could not stat() new file %s, ignoring it : %s", event.Name, err) - continue - } + if event.Op&fsnotify.Create != fsnotify.Create { + continue + } - if fi.IsDir() { - continue - } + fi, err := os.Stat(event.Name) + if err != nil { + logger.Errorf("Could not stat() new file %s, ignoring it : %s", event.Name, err) + continue + } - logger.Debugf("Detected new file %s", event.Name) + if fi.IsDir() { + continue + } - matched := false + logger.Debugf("Detected new file %s", event.Name) - for _, pattern := range f.config.Filenames { - logger.Debugf("Matching %s with %s", pattern, event.Name) + matched := false - matched, err = filepath.Match(pattern, event.Name) - if err != nil { - logger.Errorf("Could not match pattern : %s", err) - continue - } + for _, pattern := range f.config.Filenames { + logger.Debugf("Matching %s with %s", pattern, event.Name) - if matched { - logger.Debugf("Matched %s with %s", pattern, event.Name) - break - } + matched, err = filepath.Match(pattern, event.Name) + if err != nil { + logger.Errorf("Could not match pattern : %s", err) + continue } - if !matched { - continue + if matched { + logger.Debugf("Matched %s with %s", pattern, event.Name) + break } + } - // before opening the file, check if we need to specifically avoid it. (XXX) - skip := false + if !matched { + continue + } - for _, pattern := range f.exclude_regexps { - if pattern.MatchString(event.Name) { - f.logger.Infof("file %s matches exclusion pattern %s, skipping", event.Name, pattern.String()) + // before opening the file, check if we need to specifically avoid it. (XXX) + skip := false - skip = true + for _, pattern := range f.exclude_regexps { + if pattern.MatchString(event.Name) { + f.logger.Infof("file %s matches exclusion pattern %s, skipping", event.Name, pattern.String()) - break - } - } + skip = true - if skip { - continue + break } + } - f.tailMapMutex.RLock() - if f.tails[event.Name] { - f.tailMapMutex.RUnlock() - // we already have a tail on it, do not start a new one - logger.Debugf("Already tailing file %s, not creating a new tail", event.Name) + if skip { + continue + } - break - } + f.tailMapMutex.RLock() + if f.tails[event.Name] { f.tailMapMutex.RUnlock() - // cf. https://github.com/crowdsecurity/crowdsec/issues/1168 - // do not rely on stat, reclose file immediately as it's opened by Tail - fd, err := os.Open(event.Name) - if err != nil { - f.logger.Errorf("unable to read %s : %s", event.Name, err) - continue - } - if err := fd.Close(); err != nil { - f.logger.Errorf("unable to close %s : %s", event.Name, err) - continue - } + // we already have a tail on it, do not start a new one + logger.Debugf("Already tailing file %s, not creating a new tail", event.Name) - pollFile := false - if f.config.PollWithoutInotify != nil { - pollFile = *f.config.PollWithoutInotify - } else { - networkFS, fsType, err := types.IsNetworkFS(event.Name) - if err != nil { - f.logger.Warningf("Could not get fs type for %s : %s", event.Name, err) - } - f.logger.Debugf("fs for %s is network: %t (%s)", event.Name, networkFS, fsType) - if networkFS { - pollFile = true - } - } - - filink, err := os.Lstat(event.Name) + break + } + f.tailMapMutex.RUnlock() + // cf. https://github.com/crowdsecurity/crowdsec/issues/1168 + // do not rely on stat, reclose file immediately as it's opened by Tail + fd, err := os.Open(event.Name) + if err != nil { + f.logger.Errorf("unable to read %s : %s", event.Name, err) + continue + } + if err := fd.Close(); err != nil { + f.logger.Errorf("unable to close %s : %s", event.Name, err) + continue + } + pollFile := false + if f.config.PollWithoutInotify != nil { + pollFile = *f.config.PollWithoutInotify + } else { + networkFS, fsType, err := types.IsNetworkFS(event.Name) if err != nil { - logger.Errorf("Could not lstat() new file %s, ignoring it : %s", event.Name, err) - continue + f.logger.Warningf("Could not get fs type for %s : %s", event.Name, err) } - - if filink.Mode()&os.ModeSymlink == os.ModeSymlink && !pollFile { - logger.Warnf("File %s is a symlink, but inotify polling is enabled. Crowdsec will not be able to detect rotation. Consider setting poll_without_inotify to true in your configuration", event.Name) + f.logger.Debugf("fs for %s is network: %t (%s)", event.Name, networkFS, fsType) + if networkFS { + pollFile = true } + } - //Slightly different parameters for Location, as we want to read the first lines of the newly created file - tail, err := tail.TailFile(event.Name, tail.Config{ReOpen: true, Follow: true, Poll: pollFile, Location: &tail.SeekInfo{Offset: 0, Whence: io.SeekStart}}) - if err != nil { - logger.Errorf("Could not start tailing file %s : %s", event.Name, err) - break - } + filink, err := os.Lstat(event.Name) + + if err != nil { + logger.Errorf("Could not lstat() new file %s, ignoring it : %s", event.Name, err) + continue + } + + if filink.Mode()&os.ModeSymlink == os.ModeSymlink && !pollFile { + logger.Warnf("File %s is a symlink, but inotify polling is enabled. Crowdsec will not be able to detect rotation. Consider setting poll_without_inotify to true in your configuration", event.Name) + } - f.tailMapMutex.Lock() - f.tails[event.Name] = true - f.tailMapMutex.Unlock() - t.Go(func() error { - defer trace.CatchPanic("crowdsec/acquis/tailfile") - return f.tailFile(out, t, tail) - }) + //Slightly different parameters for Location, as we want to read the first lines of the newly created file + tail, err := tail.TailFile(event.Name, tail.Config{ReOpen: true, Follow: true, Poll: pollFile, Location: &tail.SeekInfo{Offset: 0, Whence: io.SeekStart}}) + if err != nil { + logger.Errorf("Could not start tailing file %s : %s", event.Name, err) + break } + + f.tailMapMutex.Lock() + f.tails[event.Name] = true + f.tailMapMutex.Unlock() + t.Go(func() error { + defer trace.CatchPanic("crowdsec/acquis/tailfile") + return f.tailFile(out, t, tail) + }) case err, ok := <-f.watcher.Errors: if !ok { return nil diff --git a/pkg/acquisition/modules/syslog/internal/parser/rfc3164/parse_test.go b/pkg/acquisition/modules/syslog/internal/parser/rfc3164/parse_test.go index 8fb5089a61f..1a836b4a12b 100644 --- a/pkg/acquisition/modules/syslog/internal/parser/rfc3164/parse_test.go +++ b/pkg/acquisition/modules/syslog/internal/parser/rfc3164/parse_test.go @@ -4,6 +4,9 @@ import ( "fmt" "testing" "time" + + "github.com/stretchr/testify/assert" + "github.com/crowdsecurity/go-cs-lib/cstest" ) func TestPri(t *testing.T) { @@ -26,22 +29,15 @@ func TestPri(t *testing.T) { r := &RFC3164{} r.buf = []byte(test.input) r.len = len(r.buf) + err := r.parsePRI() - if err != nil { - if test.expectedErr != "" { - if err.Error() != test.expectedErr { - t.Errorf("expected error %s, got %s", test.expectedErr, err) - } - } else { - t.Errorf("unexpected error: %s", err) - } - } else { - if test.expectedErr != "" { - t.Errorf("expected error %s, got no error", test.expectedErr) - } else if r.PRI != test.expected { - t.Errorf("expected %d, got %d", test.expected, r.PRI) - } + cstest.RequireErrorContains(t, err, test.expectedErr) + + if test.expectedErr != "" { + return } + + assert.Equal(t, test.expected, r.PRI) }) } } @@ -71,22 +67,15 @@ func TestTimestamp(t *testing.T) { r := NewRFC3164Parser(opts...) r.buf = []byte(test.input) r.len = len(r.buf) + err := r.parseTimestamp() - if err != nil { - if test.expectedErr != "" { - if err.Error() != test.expectedErr { - t.Errorf("expected error %s, got %s", test.expectedErr, err) - } - } else { - t.Errorf("unexpected error: %s", err) - } - } else { - if test.expectedErr != "" { - t.Errorf("expected error %s, got no error", test.expectedErr) - } else if r.Timestamp.Format(time.RFC3339) != test.expected { - t.Errorf("expected %s, got %s", test.expected, r.Timestamp.Format(time.RFC3339)) - } + cstest.RequireErrorContains(t, err, test.expectedErr) + + if test.expectedErr != "" { + return } + + assert.Equal(t, test.expected, r.Timestamp.Format(time.RFC3339)) }) } } @@ -124,22 +113,15 @@ func TestHostname(t *testing.T) { r := NewRFC3164Parser(opts...) r.buf = []byte(test.input) r.len = len(r.buf) + err := r.parseHostname() - if err != nil { - if test.expectedErr != "" { - if err.Error() != test.expectedErr { - t.Errorf("expected error %s, got %s", test.expectedErr, err) - } - } else { - t.Errorf("unexpected error: %s", err) - } - } else { - if test.expectedErr != "" { - t.Errorf("expected error %s, got no error", test.expectedErr) - } else if r.Hostname != test.expected { - t.Errorf("expected %s, got %s", test.expected, r.Hostname) - } + cstest.RequireErrorContains(t, err, test.expectedErr) + + if test.expectedErr != "" { + return } + + assert.Equal(t, test.expected, r.Hostname) }) } } @@ -164,27 +146,16 @@ func TestTag(t *testing.T) { r := &RFC3164{} r.buf = []byte(test.input) r.len = len(r.buf) + err := r.parseTag() - if err != nil { - if test.expectedErr != "" { - if err.Error() != test.expectedErr { - t.Errorf("expected error %s, got %s", test.expectedErr, err) - } - } else { - t.Errorf("unexpected error: %s", err) - } - } else { - if test.expectedErr != "" { - t.Errorf("expected error %s, got no error", test.expectedErr) - } else { - if r.Tag != test.expected { - t.Errorf("expected %s, got %s", test.expected, r.Tag) - } - if r.PID != test.expectedPID { - t.Errorf("expected %s, got %s", test.expected, r.Message) - } - } + cstest.RequireErrorContains(t, err, test.expectedErr) + + if test.expectedErr != "" { + return } + + assert.Equal(t, test.expected, r.Tag) + assert.Equal(t, test.expectedPID, r.PID) }) } } @@ -207,22 +178,15 @@ func TestMessage(t *testing.T) { r := &RFC3164{} r.buf = []byte(test.input) r.len = len(r.buf) + err := r.parseMessage() - if err != nil { - if test.expectedErr != "" { - if err.Error() != test.expectedErr { - t.Errorf("expected error %s, got %s", test.expectedErr, err) - } - } else { - t.Errorf("unexpected error: %s", err) - } - } else { - if test.expectedErr != "" { - t.Errorf("expected error %s, got no error", test.expectedErr) - } else if r.Message != test.expected { - t.Errorf("expected message %s, got %s", test.expected, r.Tag) - } + cstest.RequireErrorContains(t, err, test.expectedErr) + + if test.expectedErr != "" { + return } + + assert.Equal(t, test.expected, r.Message) }) } } @@ -326,39 +290,20 @@ func TestParse(t *testing.T) { for _, test := range tests { t.Run(test.input, func(t *testing.T) { r := NewRFC3164Parser(test.opts...) + err := r.Parse([]byte(test.input)) - if err != nil { - if test.expectedErr != "" { - if err.Error() != test.expectedErr { - t.Errorf("expected error '%s', got '%s'", test.expectedErr, err) - } - } else { - t.Errorf("unexpected error: '%s'", err) - } - } else { - if test.expectedErr != "" { - t.Errorf("expected error '%s', got no error", test.expectedErr) - } else { - if r.Timestamp != test.expected.Timestamp { - t.Errorf("expected timestamp '%s', got '%s'", test.expected.Timestamp, r.Timestamp) - } - if r.Hostname != test.expected.Hostname { - t.Errorf("expected hostname '%s', got '%s'", test.expected.Hostname, r.Hostname) - } - if r.Tag != test.expected.Tag { - t.Errorf("expected tag '%s', got '%s'", test.expected.Tag, r.Tag) - } - if r.PID != test.expected.PID { - t.Errorf("expected pid '%s', got '%s'", test.expected.PID, r.PID) - } - if r.Message != test.expected.Message { - t.Errorf("expected message '%s', got '%s'", test.expected.Message, r.Message) - } - if r.PRI != test.expected.PRI { - t.Errorf("expected pri '%d', got '%d'", test.expected.PRI, r.PRI) - } - } + cstest.RequireErrorContains(t, err, test.expectedErr) + + if test.expectedErr != "" { + return } + + assert.Equal(t, test.expected.Timestamp, r.Timestamp) + assert.Equal(t, test.expected.Hostname, r.Hostname) + assert.Equal(t, test.expected.Tag, r.Tag) + assert.Equal(t, test.expected.PID, r.PID) + assert.Equal(t, test.expected.Message, r.Message) + assert.Equal(t, test.expected.PRI, r.PRI) }) } } From f7826fa822467c4a5ae1b180e39761f6e3d3a923 Mon Sep 17 00:00:00 2001 From: marco Date: Thu, 13 Jun 2024 11:43:25 +0200 Subject: [PATCH 2/2] lint: gocritic (nestingReduce) --- cmd/crowdsec-cli/cliconsole/console.go | 27 ++++++------ pkg/exprhelpers/debugger_test.go | 15 ++++--- pkg/leakybucket/manager_load.go | 61 +++++++++++++------------- 3 files changed, 53 insertions(+), 50 deletions(-) diff --git a/cmd/crowdsec-cli/cliconsole/console.go b/cmd/crowdsec-cli/cliconsole/console.go index 995a082c514..f5758791399 100644 --- a/cmd/crowdsec-cli/cliconsole/console.go +++ b/cmd/crowdsec-cli/cliconsole/console.go @@ -88,23 +88,24 @@ func (cli *cliConsole) enroll(key string, name string, overwrite bool, tags []st } for _, availableOpt := range csconfig.CONSOLE_CONFIGS { - if opt == availableOpt { - valid = true - enable := true - - for _, enabledOpt := range enableOpts { - if opt == enabledOpt { - enable = false - continue - } - } + if opt != availableOpt { + continue + } + valid = true + enable := true - if enable { - enableOpts = append(enableOpts, opt) + for _, enabledOpt := range enableOpts { + if opt == enabledOpt { + enable = false + continue } + } - break + if enable { + enableOpts = append(enableOpts, opt) } + + break } if !valid { diff --git a/pkg/exprhelpers/debugger_test.go b/pkg/exprhelpers/debugger_test.go index efdcbc1a769..e52d52b4d11 100644 --- a/pkg/exprhelpers/debugger_test.go +++ b/pkg/exprhelpers/debugger_test.go @@ -332,14 +332,15 @@ func TestBaseDbg(t *testing.T) { } for i, out := range outdbg { - if !reflect.DeepEqual(out, test.ExpectedOutputs[i]) { - spew.Config.DisableMethods = true - t.Errorf("failed test %s", test.Name) - t.Errorf("expected : %#v", test.ExpectedOutputs[i]) - t.Errorf("got : %#v", out) - t.Fatalf("%d/%d : mismatch", i, len(outdbg)) + if reflect.DeepEqual(out, test.ExpectedOutputs[i]) { + //DisplayExprDebug(prog, outdbg, logger, ret) + continue } - //DisplayExprDebug(prog, outdbg, logger, ret) + spew.Config.DisableMethods = true + t.Errorf("failed test %s", test.Name) + t.Errorf("expected : %#v", test.ExpectedOutputs[i]) + t.Errorf("got : %#v", out) + t.Fatalf("%d/%d : mismatch", i, len(outdbg)) } } } diff --git a/pkg/leakybucket/manager_load.go b/pkg/leakybucket/manager_load.go index 1d523759f2b..8c207e7334a 100644 --- a/pkg/leakybucket/manager_load.go +++ b/pkg/leakybucket/manager_load.go @@ -509,37 +509,38 @@ func LoadBucketsState(file string, buckets *Buckets, bucketFactories []BucketFac found := false for _, h := range bucketFactories { - if h.Name == v.Name { - log.Debugf("found factory %s/%s -> %s", h.Author, h.Name, h.Description) - // check in which mode the bucket was - if v.Mode == types.TIMEMACHINE { - tbucket = NewTimeMachine(h) - } else if v.Mode == types.LIVE { - tbucket = NewLeaky(h) - } else { - log.Errorf("Unknown bucket type : %d", v.Mode) - } - /*Trying to restore queue state*/ - tbucket.Queue = v.Queue - /*Trying to set the limiter to the saved values*/ - tbucket.Limiter.Load(v.SerializedState) - tbucket.In = make(chan *types.Event) - tbucket.Mapkey = k - tbucket.Signal = make(chan bool, 1) - tbucket.First_ts = v.First_ts - tbucket.Last_ts = v.Last_ts - tbucket.Ovflw_ts = v.Ovflw_ts - tbucket.Total_count = v.Total_count - buckets.Bucket_map.Store(k, tbucket) - h.tomb.Go(func() error { - return LeakRoutine(tbucket) - }) - <-tbucket.Signal - - found = true - - break + if h.Name != v.Name { + continue + } + log.Debugf("found factory %s/%s -> %s", h.Author, h.Name, h.Description) + // check in which mode the bucket was + if v.Mode == types.TIMEMACHINE { + tbucket = NewTimeMachine(h) + } else if v.Mode == types.LIVE { + tbucket = NewLeaky(h) + } else { + log.Errorf("Unknown bucket type : %d", v.Mode) } + /*Trying to restore queue state*/ + tbucket.Queue = v.Queue + /*Trying to set the limiter to the saved values*/ + tbucket.Limiter.Load(v.SerializedState) + tbucket.In = make(chan *types.Event) + tbucket.Mapkey = k + tbucket.Signal = make(chan bool, 1) + tbucket.First_ts = v.First_ts + tbucket.Last_ts = v.Last_ts + tbucket.Ovflw_ts = v.Ovflw_ts + tbucket.Total_count = v.Total_count + buckets.Bucket_map.Store(k, tbucket) + h.tomb.Go(func() error { + return LeakRoutine(tbucket) + }) + <-tbucket.Signal + + found = true + + break } if !found {