empiredan commented on code in PR #1518:
URL:
https://github.com/apache/incubator-pegasus/pull/1518#discussion_r1228941458
##########
src/runtime/ranger/ranger_resource_policy_manager.cpp:
##########
@@ -216,9 +216,9 @@ void ranger_resource_policy_manager::start()
std::chrono::milliseconds(1));
}
-bool ranger_resource_policy_manager::allowed(const int rpc_code,
- const std::string &user_name,
- const std::string &database_name)
+access_control_result ranger_resource_policy_manager::allowed(const int
rpc_code,
+ const
std::string &user_name,
+ const
std::string &database_name)
Review Comment:
Could be declared as `const`, while both `_global_policies_lock` and
`_database_policies_lock` could be declared `mutable`.
##########
src/runtime/test/ranger_resource_policy_manager_test.cpp:
##########
@@ -193,27 +193,142 @@ TEST(ranger_resource_policy_manager_test,
ranger_resource_policy_serialized_test
{
access_type ac_type;
std::string user_name;
- bool expected_result;
- } tests[] = {{access_type::kRead, "user", false},
{access_type::kRead, "user1", true},
- {access_type::kWrite, "user1", true},
{access_type::kCreate, "user1", false},
- {access_type::kDrop, "user1", false},
{access_type::kList, "user1", true},
- {access_type::kMetadata, "user1", false},
{access_type::kControl, "user1", false},
- {access_type::kRead, "user2", true},
{access_type::kWrite, "user2", false},
- {access_type::kCreate, "user2", false},
{access_type::kDrop, "user2", false},
- {access_type::kList, "user2", true},
{access_type::kMetadata, "user2", false},
- {access_type::kControl, "user2", false},
{access_type::kRead, "user3", false},
- {access_type::kWrite, "user3", false},
{access_type::kCreate, "user3", false},
- {access_type::kDrop, "user3", false},
{access_type::kList, "user3", true},
- {access_type::kMetadata, "user3", false},
{access_type::kControl, "user3", false},
- {access_type::kRead, "user4", true},
{access_type::kWrite, "user4", false},
- {access_type::kCreate, "user4", false},
{access_type::kDrop, "user4", false},
- {access_type::kList, "user4", true},
{access_type::kMetadata, "user4", false},
- {access_type::kControl, "user4", false}};
+ policy_check_type check_type;
+ policy_check_status expected_result;
+ } tests[] = {
Review Comment:
How about providing some simple description for each case ?
##########
src/runtime/ranger/ranger_resource_policy_manager.h:
##########
@@ -74,9 +74,10 @@ class ranger_resource_policy_manager
// When using Ranger for ACL, periodically pull policies from Ranger
service.
void start();
- // Return true if the 'user_name' is allowed to access 'database_name' via
'rpc_code'.
- bool
- allowed(const int rpc_code, const std::string &user_name, const
std::string &database_name);
+ // Return 'access_control_result::kAllowed' if the 'user_name' is allowed
to access
+ // 'database_name' via 'rpc_code'.
+ access_control_result
+ allowed(const int rpc_code, const std::string &user_name, const
std::string &app_name);
Review Comment:
Why did `database_name` changed to `app_name` ?
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
