Skip to content

Commit f291c9a

Browse files
author
Christoph Wurm
authored
Improve error catching for recursiveWatcher (#8087) (#8342)
There have been spurious test failures in test_file_integrity.Test.test_recursive (#7731). This makes sure all errors encountered in recursiveWatcher are caught and logged, and also adds a debug message when a new recursive watch is added. (cherry picked from commit e5f16e2)
1 parent 95d661f commit f291c9a

File tree

3 files changed

+34
-6
lines changed

3 files changed

+34
-6
lines changed

auditbeat/module/file_integrity/monitor/monitor.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,10 @@ import (
2121
"github.com/fsnotify/fsnotify"
2222
)
2323

24+
const (
25+
moduleName = "file_integrity"
26+
)
27+
2428
// Watcher is an interface for a file watcher akin to fsnotify.Watcher
2529
// with an additional Start method.
2630
type Watcher interface {

auditbeat/module/file_integrity/monitor/recursive.go

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ import (
2424
"github.com/fsnotify/fsnotify"
2525
"github.com/joeshaw/multierror"
2626
"github.com/pkg/errors"
27+
28+
"github.com/elastic/beats/libbeat/logp"
2729
)
2830

2931
type recursiveWatcher struct {
@@ -33,6 +35,7 @@ type recursiveWatcher struct {
3335
done chan bool
3436
addC chan string
3537
addErrC chan error
38+
log *logp.Logger
3639
}
3740

3841
func newRecursiveWatcher(inner *fsnotify.Watcher) *recursiveWatcher {
@@ -42,6 +45,7 @@ func newRecursiveWatcher(inner *fsnotify.Watcher) *recursiveWatcher {
4245
eventC: make(chan fsnotify.Event, 1),
4346
addC: make(chan string),
4447
addErrC: make(chan error),
48+
log: logp.NewLogger(moduleName),
4549
}
4650
}
4751

@@ -101,6 +105,8 @@ func (watcher *recursiveWatcher) addRecursive(path string) error {
101105
}
102106
return err
103107
})
108+
watcher.log.Debugw("Added recursive watch", "path", path)
109+
104110
if err != nil {
105111
errs = append(errs, errors.Wrapf(err, "failed to walk path '%s'", path))
106112
}
@@ -147,33 +153,47 @@ func (watcher *recursiveWatcher) forwardEvents() error {
147153
}
148154
switch event.Op {
149155
case fsnotify.Create:
150-
if err := watcher.addRecursive(event.Name); err != nil {
151-
watcher.inner.Errors <- errors.Wrapf(err, "unable to recurse path '%s'", event.Name)
156+
err := watcher.addRecursive(event.Name)
157+
if err != nil {
158+
watcher.inner.Errors <- errors.Wrapf(err, "failed to add created path '%s'", event.Name)
152159
}
153-
watcher.tree.Visit(event.Name, PreOrder, func(path string, _ bool) error {
160+
err = watcher.tree.Visit(event.Name, PreOrder, func(path string, _ bool) error {
154161
watcher.deliver(fsnotify.Event{
155162
Name: path,
156163
Op: event.Op,
157164
})
158165
return nil
159166
})
167+
if err != nil {
168+
watcher.inner.Errors <- errors.Wrapf(err, "failed to visit created path '%s'", event.Name)
169+
}
160170

161171
case fsnotify.Remove:
162-
watcher.tree.Visit(event.Name, PostOrder, func(path string, _ bool) error {
172+
err := watcher.tree.Visit(event.Name, PostOrder, func(path string, _ bool) error {
163173
watcher.deliver(fsnotify.Event{
164174
Name: path,
165175
Op: event.Op,
166176
})
167177
return nil
168178
})
169-
watcher.tree.Remove(event.Name)
179+
if err != nil {
180+
watcher.inner.Errors <- errors.Wrapf(err, "failed to visit removed path '%s'", event.Name)
181+
}
182+
183+
err = watcher.tree.Remove(event.Name)
184+
if err != nil {
185+
watcher.inner.Errors <- errors.Wrapf(err, "failed to visit removed path '%s'", event.Name)
186+
}
170187

171188
// Handling rename (move) as a special case to give this recursion
172189
// the same semantics as macOS FSEvents:
173190
// - Removal of a dir notifies removal for all files inside it
174191
// - Moving a dir away sends only one notification for this dir
175192
case fsnotify.Rename:
176-
watcher.tree.Remove(event.Name)
193+
err := watcher.tree.Remove(event.Name)
194+
if err != nil {
195+
watcher.inner.Errors <- errors.Wrapf(err, "failed to remove path '%s'", event.Name)
196+
}
177197
fallthrough
178198

179199
default:

auditbeat/tests/system/test_file_integrity.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,13 +154,17 @@ def test_recursive(self):
154154
self.wait_log_contains(escape_path(dirs[0]), max_timeout=30, ignore_case=True)
155155
self.wait_log_contains("\"recursive\": true")
156156

157+
# auditbeat_test/subdir/
157158
subdir = os.path.join(dirs[0], "subdir")
158159
os.mkdir(subdir)
160+
# auditbeat_test/subdir/file.txt
159161
file1 = os.path.join(subdir, "file.txt")
160162
self.create_file(file1, "hello world!")
161163

164+
# auditbeat_test/subdir/other/
162165
subdir2 = os.path.join(subdir, "other")
163166
os.mkdir(subdir2)
167+
# auditbeat_test/subdir/other/more.txt
164168
file2 = os.path.join(subdir2, "more.txt")
165169
self.create_file(file2, "")
166170

0 commit comments

Comments
 (0)