bmahler commented on code in PR #538:
URL: https://github.com/apache/mesos/pull/538#discussion_r1543872868
##########
src/tests/containerizer/cgroups2_tests.cpp:
##########
@@ -210,6 +220,148 @@ TEST_F(Cgroups2Test, ROOT_CGROUPS2_EnableAndDisable)
EXPECT_EQ(0u, enabled->count("cpu"));
}
+// Arguments for os::open(). Combination of a path and an access type.
+typedef pair<string, int> OpenArgs;
+
+using DeviceControllerTestParams = tuple<
+ vector<devices::Entry>,
+ vector<devices::Entry>,
+ vector<OpenArgs>,
+ vector<OpenArgs>>;
+
+class DeviceControllerTestFixture :
+ public Cgroups2Test,
+ public ::testing::WithParamInterface<DeviceControllerTestParams> {};
+
+
+TEST_P(DeviceControllerTestFixture, ROOT_CGROUPS2_DeviceController) {
+ const string& cgroup = TEST_CGROUP;
+
+ auto params = GetParam();
+ const vector<devices::Entry>& allow = std::get<0>(params);
+ const vector<devices::Entry>& deny = std::get<1>(params);
+ const vector<OpenArgs>& allowedAccesses = std::get<2>(params);
+ const vector<OpenArgs>& blockedAccesses = std::get<3>(params);
+
+ ASSERT_SOME(cgroups2::create(cgroup));
+ string path = cgroups2::path(cgroup);
+
+ Try<vector<uint32_t>> attached = ebpf::cgroups2::attached(path);
+ ASSERT_SOME(attached);
+ ASSERT_EQ(0u, attached->size());
+
+ EXPECT_SOME(devices::configure(cgroup, allow, deny));
Review Comment:
ASSERT since we don't want to continue the test if this fails?
##########
src/tests/containerizer/cgroups2_tests.cpp:
##########
@@ -16,23 +16,33 @@
#include <set>
#include <string>
+#include <tuple>
+#include <utility>
#include <vector>
#include <process/reap.hpp>
+#include <process/gmock.hpp>
Review Comment:
doesn't look like this is used?
##########
src/tests/containerizer/cgroups2_tests.cpp:
##########
@@ -210,6 +220,148 @@ TEST_F(Cgroups2Test, ROOT_CGROUPS2_EnableAndDisable)
EXPECT_EQ(0u, enabled->count("cpu"));
}
+// Arguments for os::open(). Combination of a path and an access type.
+typedef pair<string, int> OpenArgs;
+
+using DeviceControllerTestParams = tuple<
+ vector<devices::Entry>,
+ vector<devices::Entry>,
+ vector<OpenArgs>,
+ vector<OpenArgs>>;
+
+class DeviceControllerTestFixture :
+ public Cgroups2Test,
+ public ::testing::WithParamInterface<DeviceControllerTestParams> {};
+
+
+TEST_P(DeviceControllerTestFixture, ROOT_CGROUPS2_DeviceController) {
+ const string& cgroup = TEST_CGROUP;
+
+ auto params = GetParam();
+ const vector<devices::Entry>& allow = std::get<0>(params);
+ const vector<devices::Entry>& deny = std::get<1>(params);
+ const vector<OpenArgs>& allowedAccesses = std::get<2>(params);
+ const vector<OpenArgs>& blockedAccesses = std::get<3>(params);
+
+ ASSERT_SOME(cgroups2::create(cgroup));
+ string path = cgroups2::path(cgroup);
+
+ Try<vector<uint32_t>> attached = ebpf::cgroups2::attached(path);
+ ASSERT_SOME(attached);
+ ASSERT_EQ(0u, attached->size());
+
+ EXPECT_SOME(devices::configure(cgroup, allow, deny));
+ attached = ebpf::cgroups2::attached(path);
+ ASSERT_SOME(attached);
+ ASSERT_EQ(1u, attached->size());
+
+ pid_t pid = ::fork();
+ ASSERT_NE(-1, pid);
+
+ if (pid == 0) {
+ // Move the child process into the newly created cgroup.
+ Try<Nothing> assign = cgroups2::assign(cgroup, ::getpid());
+ if (assign.isError()) {
+ SAFE_EXIT(EXIT_FAILURE, "Failed to assign child process to cgroup");
+ }
+
+ // Check that we can only do the "allowedAccesses".
+ foreach(const OpenArgs& args, allowedAccesses) {
+ if (os::open(args.first, args.second).isError()) {
+ SAFE_EXIT(EXIT_FAILURE, "Expected allowed read to succeed");
+ }
+ }
+ foreach(const OpenArgs& args, blockedAccesses) {
+ if (os::open(args.first, args.second).isSome()) {
+ SAFE_EXIT(EXIT_FAILURE, "Expected blocked read to fail");
+ }
+ }
+
+ ASSERT_SOME(ebpf::cgroups2::detach(path, attached->at(0)));
+
+ // Check that we can do both the "allowedAccesses" and "blockedAccesses".
+ foreach(const OpenArgs& args, allowedAccesses) {
+ if (os::open(args.first, args.second).isError()) {
+ SAFE_EXIT(EXIT_FAILURE, "Expected successful read after detaching"
+ " device controller program");
+ }
+ }
+ foreach(const OpenArgs& args, blockedAccesses) {
+ if (os::open(args.first, args.second).isError()) {
+ SAFE_EXIT(EXIT_FAILURE, "Expected successful read after detaching"
+ " device controller program");
+ }
+ }
+
+ ::_exit(EXIT_SUCCESS);
+ }
+
+ AWAIT_EXPECT_WEXITSTATUS_EQ(EXIT_SUCCESS, process::reap(pid));
+}
+
+
+INSTANTIATE_TEST_CASE_P(
+ DeviceControllerTestParams,
+ DeviceControllerTestFixture,
+ ::testing::Values<DeviceControllerTestParams>(
+ 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")},
Review Comment:
indentation is inconsistent here
##########
src/tests/containerizer/cgroups2_tests.cpp:
##########
@@ -210,6 +220,148 @@ TEST_F(Cgroups2Test, ROOT_CGROUPS2_EnableAndDisable)
EXPECT_EQ(0u, enabled->count("cpu"));
}
+// Arguments for os::open(). Combination of a path and an access type.
+typedef pair<string, int> OpenArgs;
+
+using DeviceControllerTestParams = tuple<
+ vector<devices::Entry>,
+ vector<devices::Entry>,
+ vector<OpenArgs>,
+ vector<OpenArgs>>;
+
+class DeviceControllerTestFixture :
+ public Cgroups2Test,
+ public ::testing::WithParamInterface<DeviceControllerTestParams> {};
+
+
+TEST_P(DeviceControllerTestFixture, ROOT_CGROUPS2_DeviceController) {
Review Comment:
nit: brace on next line
##########
src/tests/containerizer/cgroups2_tests.cpp:
##########
@@ -210,6 +220,148 @@ TEST_F(Cgroups2Test, ROOT_CGROUPS2_EnableAndDisable)
EXPECT_EQ(0u, enabled->count("cpu"));
}
+// Arguments for os::open(). Combination of a path and an access type.
+typedef pair<string, int> OpenArgs;
+
+using DeviceControllerTestParams = tuple<
+ vector<devices::Entry>,
+ vector<devices::Entry>,
+ vector<OpenArgs>,
+ vector<OpenArgs>>;
+
+class DeviceControllerTestFixture :
+ public Cgroups2Test,
+ public ::testing::WithParamInterface<DeviceControllerTestParams> {};
+
+
+TEST_P(DeviceControllerTestFixture, ROOT_CGROUPS2_DeviceController) {
+ const string& cgroup = TEST_CGROUP;
+
+ auto params = GetParam();
+ const vector<devices::Entry>& allow = std::get<0>(params);
+ const vector<devices::Entry>& deny = std::get<1>(params);
+ const vector<OpenArgs>& allowedAccesses = std::get<2>(params);
+ const vector<OpenArgs>& blockedAccesses = std::get<3>(params);
+
+ ASSERT_SOME(cgroups2::create(cgroup));
+ string path = cgroups2::path(cgroup);
+
+ Try<vector<uint32_t>> attached = ebpf::cgroups2::attached(path);
+ ASSERT_SOME(attached);
+ ASSERT_EQ(0u, attached->size());
+
+ EXPECT_SOME(devices::configure(cgroup, allow, deny));
+ attached = ebpf::cgroups2::attached(path);
+ ASSERT_SOME(attached);
+ ASSERT_EQ(1u, attached->size());
+
+ pid_t pid = ::fork();
+ ASSERT_NE(-1, pid);
+
+ if (pid == 0) {
+ // Move the child process into the newly created cgroup.
+ Try<Nothing> assign = cgroups2::assign(cgroup, ::getpid());
+ if (assign.isError()) {
+ SAFE_EXIT(EXIT_FAILURE, "Failed to assign child process to cgroup");
+ }
+
+ // Check that we can only do the "allowedAccesses".
+ foreach(const OpenArgs& args, allowedAccesses) {
+ if (os::open(args.first, args.second).isError()) {
+ SAFE_EXIT(EXIT_FAILURE, "Expected allowed read to succeed");
+ }
+ }
+ foreach(const OpenArgs& args, blockedAccesses) {
+ if (os::open(args.first, args.second).isSome()) {
+ SAFE_EXIT(EXIT_FAILURE, "Expected blocked read to fail");
+ }
+ }
+
+ ASSERT_SOME(ebpf::cgroups2::detach(path, attached->at(0)));
+
+ // Check that we can do both the "allowedAccesses" and "blockedAccesses".
+ foreach(const OpenArgs& args, allowedAccesses) {
+ if (os::open(args.first, args.second).isError()) {
+ SAFE_EXIT(EXIT_FAILURE, "Expected successful read after detaching"
+ " device controller program");
+ }
+ }
+ foreach(const OpenArgs& args, blockedAccesses) {
+ if (os::open(args.first, args.second).isError()) {
+ SAFE_EXIT(EXIT_FAILURE, "Expected successful read after detaching"
+ " device controller program");
+ }
+ }
+
+ ::_exit(EXIT_SUCCESS);
+ }
+
+ AWAIT_EXPECT_WEXITSTATUS_EQ(EXIT_SUCCESS, process::reap(pid));
+}
+
+
+INSTANTIATE_TEST_CASE_P(
+ DeviceControllerTestParams,
+ DeviceControllerTestFixture,
+ ::testing::Values<DeviceControllerTestParams>(
+ 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")},
+ vector<OpenArgs>{},
+ vector<OpenArgs>{{os::DEV_NULL, O_RDWR}}
+ },
+ DeviceControllerTestParams{
+ // allow /dev/null
+ vector<devices::Entry>{*devices::Entry::parse("c 1:3 rwm")},
+ vector<devices::Entry>{},
+ // read-write allowed
+ vector<OpenArgs>{{os::DEV_NULL, O_RDWR}},
+ vector<OpenArgs>{}
+ },
+ DeviceControllerTestParams{
+ // allow /dev/null
+ vector<devices::Entry>{*devices::Entry::parse("c 1:3 r")},
+ vector<devices::Entry>{},
+ // read-only allowed
+ vector<OpenArgs>{{os::DEV_NULL, O_RDONLY}},
+ // read-write is blocked
+ vector<OpenArgs>{{os::DEV_NULL, O_RDWR}}
+ },
+ DeviceControllerTestParams{
+ vector<devices::Entry>{*devices::Entry::parse("c 1:3 w")},
+ vector<devices::Entry>{},
+ // write-only allowed
+ vector<OpenArgs>{{os::DEV_NULL, O_WRONLY}},
+ // read is blocked
+ vector<OpenArgs>{{os::DEV_NULL, O_RDWR}, {os::DEV_NULL, O_RDONLY}}
+ },
+ DeviceControllerTestParams{
+ vector<devices::Entry>{*devices::Entry::parse("b 1:3 rwm")},
+ vector<devices::Entry>{},
+ vector<OpenArgs>{},
+ // /dev/null is blocked
Review Comment:
only because the entry has the wrong device type though, which isn't very
clear here
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
