-
Notifications
You must be signed in to change notification settings - Fork 351
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
Readonly paths #582
Readonly paths #582
Conversation
crates/integration_test/src/tests/readonly_paths/readonly_paths.rs
Outdated
Show resolved
Hide resolved
crates/integration_test/src/tests/readonly_paths/readonly_paths.rs
Outdated
Show resolved
Hide resolved
crates/integration_test/src/tests/readonly_paths/readonly_paths.rs
Outdated
Show resolved
Hide resolved
Remove unnecessary unsafe{} Remove libc direct dependency and use nix instead
Codecov Report
@@ Coverage Diff @@
## main #582 +/- ##
=======================================
Coverage 70.18% 70.18%
=======================================
Files 82 82
Lines 10909 10909
=======================================
Hits 7657 7657
Misses 3252 3252 |
This PR is adds an important Part of the integration tests : The test_inside_container function, which allows testing the restrictions from inside the container ; along with its counterpart the runtimetest . The PR contains the readonly_paths test, as well as a new crate for runtimetest, which is the binary which runs inside the container process and runs tests inside. Apologies for such a big PR, but it needed to add both of the things together, as to make sure runtimetest works, we need a test which will run test_inside_container and to run a test_inside_container, we need runtiemtest, thus both of those ended up being in this same PR. As the runtimetest tool and its configuration can be tricky, as well as I had several issues while figuring out how all works and fits together for the original Go tests, I have added detailed explanations in the Readme as well as have added docs for it as well. Also I would like to apologies to @utam0k because this PR will likely make even more conflicts for #514 . If there is any way I can help with resolving the conflicts, or reducing the issue, please let me know I would like to help. If we need to wait for it to be merged before this, that is fine as well, we can review this and keep it in queue to be merged after that PR. Please take a look, thanks :) |
There is no problem. As this pr which I made is too large to merge, I'll try to make some smaller PRs.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome!
However, this PR will inevitably have a large impact on other people's work. Therefore, I'll ignore the details and merge this once. Let me improve some of them on myend.
Add readonly paths integration tests