Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Auditbeat/FIM/fsnotify]: prevent losing events for recursive mode on OS X #39362

Conversation

pkoutsovasilis
Copy link
Contributor

Proposed commit message

This PR prevents FIM from losing events for recursive mode on Mac OS X even when the watchFile of the root dir, added to be monitored, returns an error by always walking the dir. Specifically, this discrepancy, between Linux and OS X oses, is due to the fact that in the latter the underlying library, namely fsnotify, when you add a watch of a directory it walks the directory and adds the respective sub-dir watchers. If any of these fail, e.g. with EACCESS, it returns an error. However, auditbeat's wrapper of this library emits created events by walking the directory. So, in order not to lose any events we need to guarantee that we won't interrupt this flow - we will always walk the directory - and accumulate any errors during this process which we will return only at the end.

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have made corresponding change to the default configuration files
  • I have added tests that prove my fix is effective or that my feature works
  • I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

Author's Checklist

N/A

How to test this PR locally

Already tested here

Related issues

Use cases

N/A

Screenshots

N/A

Logs

N/A

@pkoutsovasilis pkoutsovasilis added bug backport-7.17 Automated backport to the 7.17 branch with mergify Team:Security-Linux Platform Linux Platform Team in Security Solution backport-v8.13.0 Automated backport with mergify backport-v8.14.0 Automated backport with mergify labels May 2, 2024
@pkoutsovasilis pkoutsovasilis self-assigned this May 2, 2024
@botelastic botelastic bot added needs_team Indicates that the issue/PR needs a Team:* label and removed needs_team Indicates that the issue/PR needs a Team:* label labels May 2, 2024
@pkoutsovasilis pkoutsovasilis force-pushed the pkoutsovasilis/fix_integ_tests_for_mac_osx branch from 33504f4 to dbfd308 Compare May 2, 2024 12:18
@pkoutsovasilis pkoutsovasilis marked this pull request as ready for review May 2, 2024 12:18
@pkoutsovasilis pkoutsovasilis requested a review from a team as a code owner May 2, 2024 12:18
@elasticmachine
Copy link
Collaborator

Pinging @elastic/sec-linux-platform (Team:Security-Linux Platform)

Copy link
Contributor

@nicholasberlin nicholasberlin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM if it works.

@pkoutsovasilis
Copy link
Contributor Author

run docs-build

Copy link
Contributor

mergify bot commented May 2, 2024

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b pkoutsovasilis/fix_integ_tests_for_mac_osx upstream/pkoutsovasilis/fix_integ_tests_for_mac_osx
git merge upstream/main
git push upstream pkoutsovasilis/fix_integ_tests_for_mac_osx

@pkoutsovasilis pkoutsovasilis force-pushed the pkoutsovasilis/fix_integ_tests_for_mac_osx branch from dbfd308 to 8ed3203 Compare May 2, 2024 13:06
@elasticmachine
Copy link
Collaborator

elasticmachine commented May 2, 2024

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Duration: 91 min 23 sec

❕ Flaky test report

No test was executed to be analysed.

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

  • /package : Generate the packages and run the E2E tests.

  • /beats-tester : Run the installation tests with beats-tester.

  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

@pkoutsovasilis pkoutsovasilis merged commit bbf8746 into elastic:main May 2, 2024
34 checks passed
@pkoutsovasilis pkoutsovasilis deleted the pkoutsovasilis/fix_integ_tests_for_mac_osx branch May 2, 2024 14:53
mergify bot pushed a commit that referenced this pull request May 2, 2024
… OS X (#39362)

* fix(auditbeat/fim/fsnotify): do not return error immediately as this causes losing events on mac

* doc: update CHANGELOG.next.asciidoc

(cherry picked from commit bbf8746)
mergify bot pushed a commit that referenced this pull request May 2, 2024
… OS X (#39362)

* fix(auditbeat/fim/fsnotify): do not return error immediately as this causes losing events on mac

* doc: update CHANGELOG.next.asciidoc

(cherry picked from commit bbf8746)
mergify bot pushed a commit that referenced this pull request May 2, 2024
… OS X (#39362)

* fix(auditbeat/fim/fsnotify): do not return error immediately as this causes losing events on mac

* doc: update CHANGELOG.next.asciidoc

(cherry picked from commit bbf8746)
pkoutsovasilis added a commit that referenced this pull request May 2, 2024
… OS X (#39362) (#39375)

* fix(auditbeat/fim/fsnotify): do not return error immediately as this causes losing events on mac

* doc: update CHANGELOG.next.asciidoc

(cherry picked from commit bbf8746)

Co-authored-by: Panos Koutsovasilis <panos.koutsovasilis@elastic.co>
pkoutsovasilis added a commit that referenced this pull request May 7, 2024
…ts for recursive mode on OS X (#39374)

* [Auditbeat/FIM/fsnotify]: prevent losing events for recursive mode on OS X (#39362)

* fix(auditbeat/fim/fsnotify): do not return error immediately as this causes losing events on mac

* doc: update CHANGELOG.next.asciidoc

(cherry picked from commit bbf8746)

* doc: remove redundant changes from CHANGELOG.next.asciidoc

---------

Co-authored-by: Panos Koutsovasilis <panos.koutsovasilis@elastic.co>
pkoutsovasilis added a commit that referenced this pull request May 7, 2024
… OS X (#39362) (#39376)

* fix(auditbeat/fim/fsnotify): do not return error immediately as this causes losing events on mac

* doc: update CHANGELOG.next.asciidoc

(cherry picked from commit bbf8746)

Co-authored-by: Panos Koutsovasilis <panos.koutsovasilis@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-7.17 Automated backport to the 7.17 branch with mergify backport-v8.13.0 Automated backport with mergify backport-v8.14.0 Automated backport with mergify bug Team:Security-Linux Platform Linux Platform Team in Security Solution
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants