error checks when reading /proc/[pid]/exe don't account for kernel processes or docker permissions shenanigans
#135
Assignees
Labels
Comments
|
Blocked as required elastic/beats#38241 first |
4 tasks
fearful-symmetry
added a commit
that referenced
this issue
Apr 18, 2024
…tainer (#140) ## What does this PR do? Closes elastic/beats#38241 This adds a lightweight test framework that runs a set of system tests under a container with the goal of monitoring the host system. The goal with these tests is to catch the numerous edge cases that happen when the system metrics function from a `/hostfs` path inside a container. The tests have a fairly large matrix of configurations, as we need to test both a wide variety of container permission settings, as well as differences in how linux distros will configure cgroups. The framework here was designed with the goal of being relatively idiomatic; you can just run the framework with `go test` as you would normally. You can run the tests yourself with `go test -v ./tests` As you may have noticed, there's a non-zero amount of TODO statements here, since these tests were built to aggravate a bunch of existing bugs, so certain parts of the tests will remain un-implemented until those bugs are fixed. ## Why is it important? See elastic/beats#38241, we really need test for this particular case. ## List of bugs that are responsible for TODO statements in the tests: - #141 - #135 - #139 - #132 - elastic/go-sysinfo#12 ## Checklist - [x] My code follows the style guidelines of this project - [x] I have commented my code, particularly in hard-to-understand areas - [x] I have added tests that prove my fix is effective or that my feature works - [ ] I have added an entry in `CHANGELOG.md`
fearful-symmetry
added a commit
that referenced
this issue
May 1, 2024
## What does this PR do? Closes #135 This fixes an issue where our permissions checks for fetching `/proc/pid/exe` could fail for kernel procs and certain docker configs. Also re-enables one of the tests. ## Why is it important? This is a bug that causes data loss. ## Checklist - [x] My code follows the style guidelines of this project - [x] I have commented my code, particularly in hard-to-understand areas - [x] I have added tests that prove my fix is effective or that my feature works - [ ] I have added an entry in `CHANGELOG.md`
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Right now, under Linux we check to see if we have a permission denied error when we resolve the symlink for
exe:elastic-agent-system-metrics/metric/system/process/process_linux_common.go
Line 209 in 8a02eaa
We do this so we can skip any errors related to permissions issues. However, this doesn't work under two cases:
exesymlink is non-existentPermission deinedorfile not founddepending on the permissions docker is running with.The permissions check in
process_linux_common.goshould check for both permission errors, and file not found errors.The text was updated successfully, but these errors were encountered: