This is an automated email from the ASF dual-hosted git repository. bmahler pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/mesos.git
commit fc25362836f7b3c922bdb13497dbbcbaeea10c20 Author: Jason Zhou <[email protected]> AuthorDate: Fri Jul 26 17:28:57 2024 -0400 [devices] Enforce normalization for DeviceManager configure & reconfigure. Currently in the configure() and reconfigure() functions in device manager, we do not ensure that the device access state at the end of the function call is normalized. So we incorporate normalized() and normalize() calls to ensure that the allow and deny lists are always normalized at the end of a configure() or reconfigure() call. Review: https://reviews.apache.org/r/75115/ --- .../device_manager/device_manager.cpp | 21 +++++++--------- src/tests/device_manager_tests.cpp | 29 ++++++++++++++-------- 2 files changed, 28 insertions(+), 22 deletions(-) diff --git a/src/slave/containerizer/device_manager/device_manager.cpp b/src/slave/containerizer/device_manager/device_manager.cpp index 800e50243..c25af0adc 100644 --- a/src/slave/containerizer/device_manager/device_manager.cpp +++ b/src/slave/containerizer/device_manager/device_manager.cpp @@ -81,6 +81,13 @@ public: const vector<DeviceManager::NonWildcardEntry>& non_wildcard_deny_list) { vector<Entry> deny_list = convert_to_entries(non_wildcard_deny_list); + + if (!cgroups2::devices::normalized(allow_list) + || !cgroups2::devices::normalized(deny_list)) { + return Failure("Failed to configure allow and deny devices:" + " the input allow or deny list is not normalized"); + } + foreach (const Entry& allow_entry, allow_list) { foreach (const Entry& deny_entry, deny_list) { if (deny_entry.encompasses(allow_entry)) { @@ -324,18 +331,8 @@ DeviceManager::CgroupDeviceAccess DeviceManager::apply_diff( } } - auto strip_empties = [](const vector<Entry>& entries) { - vector<Entry> res = {}; - foreach (const Entry& entry, entries) { - if (!entry.access.none()) { - res.push_back(entry); - } - } - return res; - }; - - new_state.allow_list = strip_empties(new_state.allow_list); - new_state.deny_list = strip_empties(new_state.deny_list); + new_state.allow_list = cgroups2::devices::normalize(new_state.allow_list); + new_state.deny_list = cgroups2::devices::normalize(new_state.deny_list); return new_state; } diff --git a/src/tests/device_manager_tests.cpp b/src/tests/device_manager_tests.cpp index b30618c98..da1ef64fe 100644 --- a/src/tests/device_manager_tests.cpp +++ b/src/tests/device_manager_tests.cpp @@ -352,7 +352,8 @@ INSTANTIATE_TEST_CASE_P( vector<devices::Entry>{}, vector<devices::Entry>{*devices::Entry::parse("c 3:1 rm")}, vector<devices::Entry>{*devices::Entry::parse("c 3:1 w")}, - vector<devices::Entry>{}}, + vector<devices::Entry>{} + }, // Remove existing deny entry accesses: DeviceManagerGetDiffStateTestParams{ vector<devices::Entry>{*devices::Entry::parse("c 3:* rwm")}, @@ -360,9 +361,10 @@ INSTANTIATE_TEST_CASE_P( vector<devices::Entry>{*devices::Entry::parse("c 3:1 rm")}, vector<devices::Entry>{}, vector<devices::Entry>{ - *devices::Entry::parse("c 3:* rwm"), - *devices::Entry::parse("c 3:1 rm")}, - vector<devices::Entry>{*devices::Entry::parse("c 3:1 w")}}, + *devices::Entry::parse("c 3:* rwm") + }, + vector<devices::Entry>{*devices::Entry::parse("c 3:1 w")} + }, // Remove entire existing allow entry: DeviceManagerGetDiffStateTestParams{ vector<devices::Entry>{*devices::Entry::parse("c 3:1 rm")}, @@ -370,7 +372,8 @@ INSTANTIATE_TEST_CASE_P( vector<devices::Entry>{}, vector<devices::Entry>{*devices::Entry::parse("c 3:1 rwm")}, vector<devices::Entry>{}, - vector<devices::Entry>{}}, + vector<devices::Entry>{} + }, // Remove entire existing deny entry: DeviceManagerGetDiffStateTestParams{ vector<devices::Entry>{*devices::Entry::parse("c 3:* rm")}, @@ -378,8 +381,10 @@ INSTANTIATE_TEST_CASE_P( vector<devices::Entry>{*devices::Entry::parse("c 3:1 rm")}, vector<devices::Entry>{}, vector<devices::Entry>{ - *devices::Entry::parse("c 3:* rm"), *devices::Entry::parse("c 3:1 rm")}, - vector<devices::Entry>{}}, + *devices::Entry::parse("c 3:* rm") + }, + vector<devices::Entry>{} + }, // Overlapping entries where none encompasses the other: DeviceManagerGetDiffStateTestParams{ vector<devices::Entry>{*devices::Entry::parse("c 3:* rm")}, @@ -387,8 +392,11 @@ INSTANTIATE_TEST_CASE_P( vector<devices::Entry>{*devices::Entry::parse("c 3:1 rw")}, vector<devices::Entry>{}, vector<devices::Entry>{ - *devices::Entry::parse("c 3:* rm"), *devices::Entry::parse("c 3:1 rw")}, - vector<devices::Entry>{*devices::Entry::parse("c 3:1 m")}}, + *devices::Entry::parse("c 3:* rm"), + *devices::Entry::parse("c 3:1 rwm") + }, + vector<devices::Entry>{*devices::Entry::parse("c 3:1 m")} + }, // Overlapping with non-encompassing wildcard: DeviceManagerGetDiffStateTestParams{ vector<devices::Entry>{*devices::Entry::parse("c 3:* rm")}, @@ -396,7 +404,8 @@ INSTANTIATE_TEST_CASE_P( vector<devices::Entry>{}, vector<devices::Entry>{*devices::Entry::parse("c 3:1 rw")}, vector<devices::Entry>{*devices::Entry::parse("c 3:* rm")}, - vector<devices::Entry>{*devices::Entry::parse("c 3:1 r")}})); + vector<devices::Entry>{*devices::Entry::parse("c 3:1 r")} + })); TEST(DeviceManagerCgroupDeviceAccessTest, IsAccessGrantedTest)
