[cgroups2] Device controller tests.#538
Conversation
DevinLeamy
force-pushed
the
device-tests
branch
from
4b457d1 to
1d42c8c
Compare
| // read-write is blocked | ||
| vector<Access>{{os::DEV_NULL, O_RDWR}} | ||
| }, | ||
| DeviceControllerTestParams{ |
There was a problem hiding this comment.
what's this one? This?
// Allow write-only access to /dev/null using any device type.
There was a problem hiding this comment.
Allows read access to /dev/null. The check that read-only is allowed and that read-write is not allowed.
There was a problem hiding this comment.
It's probably easier to just have one comment that explains what each test case here does rather than these fine grained comments, for example there's no comment on the "b 1:3 rwm" test, so it's hard for the reader to know what that case is checking (e.g. different device type so not actually /dev/null?)
| DeviceControllerTestParams{ | ||
| vector<devices::Entry>{*devices::Entry::parse("b 1:3 r")}, | ||
| vector<devices::Entry>{}, | ||
| vector<Access>{}, | ||
| // /dev/null is blocked | ||
| vector<Access>{{os::DEV_NULL, O_RDWR}, {os::DEV_NULL, O_RDONLY}} | ||
| } |
There was a problem hiding this comment.
Is this what we're testing?
// Allow access to /dev/null but with the wrong device type, therefore access is denied.
There was a problem hiding this comment.
/dev/null is a character device. This tests that if we allow a block device to be read, and then try and read from a character device with the same type, then that request will be blocked.
| if (pid == 0) { | ||
| // Check that we can only do the "allowedAccesses". | ||
| foreach(const Access& access, allowedAccesses) { | ||
| ASSERT_SOME(os::open(access.first, access.second)); | ||
| } | ||
| foreach(const Access& access, blockedAccesses) { | ||
| ASSERT_ERROR(os::open(access.first, access.second)); | ||
| } | ||
|
|
||
| ASSERT_SOME(ebpf::cgroups2::detach(path, attached->at(0))); | ||
|
|
||
| // Check that we can do both the "allowedAccesses" and "blockedAccesses". | ||
| foreach(const Access& access, allowedAccesses) { | ||
| ASSERT_SOME(os::open(access.first, access.second)); | ||
| } | ||
| foreach(const Access& access, blockedAccesses) { | ||
| ASSERT_SOME(os::open(access.first, access.second)); | ||
| } |
There was a problem hiding this comment.
We can't use ASSERT in the child process, as they won't fail the test. Looks like the strategy in cgroups_tests.cpp is to use a pipe and communicate back to the parent process to indicate whether the child process' tests succeeded.
DevinLeamy
force-pushed
the
device-tests
branch
3 times, most recently
from
4e5e8bb to
840b97e
Compare
Added tests for the programs created by the the Device Controller.
DevinLeamy
force-pushed
the
device-tests
branch
from
840b97e to
ce62bcc
Compare
bmahler
left a comment
bmahler
left a comment
There was a problem hiding this comment.
Nice, the test looks clean! We should really have some tests that actually use multiple device rules though?
| #include <vector> | ||
|
|
||
| #include <process/reap.hpp> | ||
| #include <process/gmock.hpp> |
There was a problem hiding this comment.
doesn't look like this is used?
| public ::testing::WithParamInterface<DeviceControllerTestParams> {}; | ||
|
|
||
|
|
||
| TEST_P(DeviceControllerTestFixture, ROOT_CGROUPS2_DeviceController) { |
| ASSERT_SOME(attached); | ||
| ASSERT_EQ(0u, attached->size()); | ||
|
|
||
| EXPECT_SOME(devices::configure(cgroup, allow, deny)); |
There was a problem hiding this comment.
ASSERT since we don't want to continue the test if this fails?
| DeviceControllerTestParams{ | ||
| vector<devices::Entry>{}, | ||
| vector<devices::Entry>{}, | ||
| vector<OpenArgs>{}, | ||
| // Block accesses by default. | ||
| vector<OpenArgs>{{os::DEV_NULL, O_RDWR}, {os::DEV_NULL, O_RDWR}} | ||
| }, | ||
| DeviceControllerTestParams{ | ||
| vector<devices::Entry>{}, | ||
| vector<devices::Entry>{*devices::Entry::parse("c 1:3 rwm")}, |
There was a problem hiding this comment.
indentation is inconsistent here
| vector<devices::Entry>{*devices::Entry::parse("b 1:3 rwm")}, | ||
| vector<devices::Entry>{}, | ||
| vector<OpenArgs>{}, | ||
| // /dev/null is blocked |
There was a problem hiding this comment.
only because the entry has the wrong device type though, which isn't very clear here
2 participants
Added tests for the programs created by the the Device Controller.