wh2

apache · Feb 15, 2023 · 42992e0 · 42992e0
1 parent 6431345
commit 42992e0
Show file tree

Hide file tree

Showing 2 changed files with 8 additions and 7 deletions.
diff --git a/src/runtime/ranger/ranger_resource_policy_manager.cpp b/src/runtime/ranger/ranger_resource_policy_manager.cpp
@@ -272,7 +272,9 @@ dsn::error_code ranger_resource_policy_manager::update_policies_from_ranger_serv
     auto err_code = load_policies_from_json(ranger_policies);
     if (err_code == dsn::ERR_RANGER_POLICIES_NO_NEED_UPDATE) {
         LOG_DEBUG("Skip to update local policies.");
-        // TODO(yingchun): remove this
+        // for the newly created table, its app envs must be empty. This needs to be executed
+        // periodically to update the table's app envs, regardless of whether the Ranger policy is
+        // updated or not.
         CHECK_EQ_MSG(dsn::ERR_OK, sync_policies_to_app_envs(), "Sync policies to app envs failed.");
         LOG_DEBUG("Sync policies to app envs succeeded.");
         return dsn::ERR_OK;
@@ -298,7 +300,6 @@ void ranger_resource_policy_manager::start_to_dump_and_sync_policies()
                 sync_task->enqueue();
                 return;
             }
-            // TODO(yingchun): is it the only error here?
             CHECK_EQ(err, dsn::ERR_TIMEOUT);
             LOG_ERROR("Create Ranger policy meta root timeout, try it later.");
             dsn::tasking::enqueue(LPC_CM_GET_RANGER_POLICY,
@@ -372,14 +373,12 @@ dsn::error_code ranger_resource_policy_manager::sync_policies_to_app_envs()
     _meta_svc->get_server_state()->list_apps(list_req, list_resp);
     ERR_LOG_AND_RETURN_NOT_OK(list_resp.err, "list_apps failed.");
     for (const auto &app : list_resp.infos) {
-        // TODO(yingchun): check the rule
         std::string database_name = get_database_name_from_app_name(app.app_name);
         std::string table_name;
         if (database_name.empty()) {
             database_name = "*";
             table_name = app.app_name;
         } else {
-            // TODO(yingchun): optimize this
             table_name = app.app_name.substr(database_name.size());
         }
 
@@ -389,12 +388,12 @@ dsn::error_code ranger_resource_policy_manager::sync_policies_to_app_envs()
             {dsn::replication::replica_envs::REPLICA_ACCESS_CONTROLLER_RANGER_POLICIES});
         bool has_match_policy = false;
         for (const auto &policy : table_policies->second) {
-            // TODO(yingchun): check the rule
             if (policy.database_names.count(database_name) == 0) {
                 continue;
             }
 
-            // TODO(yingchun): there must be a default policy to match "*" ?
+            // 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) {
                 has_match_policy = true;
                 req->__set_op(dsn::replication::app_env_operation::type::APP_ENV_OP_SET);
@@ -572,6 +571,8 @@ dsn::error_code ranger_resource_policy_manager::load_policies_from_json(const st
         resource_type rt = UNKNOWN;
         do {
             // TODO(yingchun): refactor the following code
+            // parse Ranger policies json string into `values_of_resource_type`, distinguish
+            // resource types by `values_of_resource_type.size()`
             if (values_of_resource_type.size() == 1) {
                 auto iter = values_of_resource_type.find("global");
                 if (iter != values_of_resource_type.end()) {

diff --git a/src/runtime/test/ranger_resource_policy_test.cpp b/src/runtime/test/ranger_resource_policy_test.cpp
@@ -78,4 +78,4 @@ TEST(ranger_resource_policy_test, acl_policies_allowed)
     }
 }
 } // namespace security
-} // namespace ranger
+} // namespace ranger