acelyc111 commented on code in PR #1507:
URL:
https://github.com/apache/incubator-pegasus/pull/1507#discussion_r1214196269
##########
src/runtime/ranger/ranger_resource_policy_manager.cpp:
##########
@@ -590,24 +599,21 @@ dsn::error_code
ranger_resource_policy_manager::sync_policies_to_app_envs()
{dsn::replication::replica_envs::REPLICA_ACCESS_CONTROLLER_RANGER_POLICIES});
bool is_policy_matched = false;
for (const auto &policy : table_policies->second) {
- if (policy.database_names.count(database_name) == 0) {
+ // this table does not match any database, app Ranger policy will
be cleaned up
Review Comment:
```suggestion
// If the table does not match any database, its Ranger policies
will be cleaned up.
```
##########
src/runtime/ranger/ranger_resource_policy_manager.cpp:
##########
@@ -241,20 +245,24 @@ bool ranger_resource_policy_manager::allowed(const int
rpc_code,
break;
}
+ // legacy table belongs to the default database.
+ std::string db_name =
+ database_name.empty() ?
FLAGS_legacy_table_database_mapping_policy_name : database_name;
+
// Check if it is allowed by any DATABASE policy.
utils::auto_read_lock l(_database_policies_lock);
for (const auto &policy : _database_policies_cache) {
if (!policy.policies.allowed(ac_type->second, user_name)) {
continue;
}
- // Legacy tables may don't contain database section.
- if (database_name.empty() && policy.database_names.count("*") !=
0) {
- return true;
- }
- if (policy.database_names.count(database_name) != 0) {
+ // "*" can match any table, including legacy table and new table.
+ if (policy.database_names.count("*") != 0 ||
policy.database_names.count(db_name)) {
Review Comment:
```suggestion
if (policy.database_names.count("*") != 0 ||
policy.database_names.count(db_name) != 0) {
```
##########
src/runtime/ranger/ranger_resource_policy_manager.cpp:
##########
@@ -72,6 +72,10 @@ DSN_DEFINE_string(ranger,
ranger_service_name,
"",
"The name of the policies defined in the Ranger service.");
+DSN_DEFINE_string(ranger,
+ legacy_table_database_mapping_policy_name,
+ "__default__",
+ "The name of the Ranger database policy matched by the
legacy table");
Review Comment:
It would be better to describe what is "legacy table".
##########
src/runtime/ranger/ranger_resource_policy_manager.cpp:
##########
@@ -590,24 +599,21 @@ dsn::error_code
ranger_resource_policy_manager::sync_policies_to_app_envs()
{dsn::replication::replica_envs::REPLICA_ACCESS_CONTROLLER_RANGER_POLICIES});
bool is_policy_matched = false;
for (const auto &policy : table_policies->second) {
- if (policy.database_names.count(database_name) == 0) {
+ // this table does not match any database, app Ranger policy will
be cleaned up
+ if (policy.database_names.count(database_name) == 0 &&
+ policy.database_names.count("*") == 0) {
continue;
}
- // if table name does not conform to the naming
rules(database_name.table_name),
- // database is defined by "*" in ranger for acl matching
- if (policy.table_names.count("*") != 0 ||
policy.table_names.count(table_name) != 0) {
- is_policy_matched = true;
-
req->__set_op(dsn::replication::app_env_operation::type::APP_ENV_OP_SET);
- req->__set_values(
-
{json::json_forwarder<acl_policies>::encode(policy.policies).to_string()});
-
- dsn::replication::update_app_env_rpc rpc(std::move(req),
-
LPC_USE_RANGER_ACCESS_CONTROL);
- _meta_svc->get_server_state()->set_app_envs(rpc);
- LOG_AND_RETURN_NOT_OK(ERROR, rpc.response().err, "set_app_envs
failed.");
- break;
- }
+ is_policy_matched = true;
+
req->__set_op(dsn::replication::app_env_operation::type::APP_ENV_OP_SET);
+ req->__set_values(
+
{json::json_forwarder<acl_policies>::encode(policy.policies).to_string()});
+
+ dsn::replication::update_app_env_rpc rpc(std::move(req),
LPC_USE_RANGER_ACCESS_CONTROL);
+ _meta_svc->get_server_state()->set_app_envs(rpc);
+ LOG_AND_RETURN_NOT_OK(ERROR, rpc.response().err, "set_app_envs
failed.");
+ break;
}
// There is no matched policy, clear app Ranger policy
Review Comment:
```suggestion
// There is no matched policy, clear the table's Ranger policies.
```
##########
src/runtime/ranger/ranger_resource_policy_manager.cpp:
##########
@@ -590,24 +599,21 @@ dsn::error_code
ranger_resource_policy_manager::sync_policies_to_app_envs()
{dsn::replication::replica_envs::REPLICA_ACCESS_CONTROLLER_RANGER_POLICIES});
bool is_policy_matched = false;
for (const auto &policy : table_policies->second) {
- if (policy.database_names.count(database_name) == 0) {
+ // this table does not match any database, app Ranger policy will
be cleaned up
+ if (policy.database_names.count(database_name) == 0 &&
+ policy.database_names.count("*") == 0) {
continue;
}
- // if table name does not conform to the naming
rules(database_name.table_name),
- // database is defined by "*" in ranger for acl matching
- if (policy.table_names.count("*") != 0 ||
policy.table_names.count(table_name) != 0) {
- is_policy_matched = true;
-
req->__set_op(dsn::replication::app_env_operation::type::APP_ENV_OP_SET);
- req->__set_values(
-
{json::json_forwarder<acl_policies>::encode(policy.policies).to_string()});
-
- dsn::replication::update_app_env_rpc rpc(std::move(req),
-
LPC_USE_RANGER_ACCESS_CONTROL);
- _meta_svc->get_server_state()->set_app_envs(rpc);
- LOG_AND_RETURN_NOT_OK(ERROR, rpc.response().err, "set_app_envs
failed.");
- break;
- }
+ is_policy_matched = true;
+
req->__set_op(dsn::replication::app_env_operation::type::APP_ENV_OP_SET);
+ req->__set_values(
+
{json::json_forwarder<acl_policies>::encode(policy.policies).to_string()});
+
+ dsn::replication::update_app_env_rpc rpc(std::move(req),
LPC_USE_RANGER_ACCESS_CONTROL);
+ _meta_svc->get_server_state()->set_app_envs(rpc);
+ LOG_AND_RETURN_NOT_OK(ERROR, rpc.response().err, "set_app_envs
failed.");
+ break;
Review Comment:
Is it possible that a table match multiple policies?
--
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]
