You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pegasus.apache.org by wa...@apache.org on 2023/06/08 12:31:23 UTC

[incubator-pegasus] branch master updated: feat(Ranger): add a default database policy name for legacy table (#1507)

This is an automated email from the ASF dual-hosted git repository.

wangdan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-pegasus.git


The following commit(s) were added to refs/heads/master by this push:
     new f3d13881a feat(Ranger): add a default database policy name for legacy table (#1507)
f3d13881a is described below

commit f3d13881a134d5821d1629b0cc9845a0441d2626
Author: WHBANG <38...@users.noreply.github.com>
AuthorDate: Thu Jun 8 20:31:16 2023 +0800

    feat(Ranger): add a default database policy name for legacy table (#1507)
    
    https://github.com/apache/incubator-pegasus/issues/1054
    
    This patch adds a new conf item `legacy_table_database_mapping_policy_name`,
    the legacy table (the tables which are created before Ranger ACL enabled) will be
    matched to the database named `legacy_table_database_mapping_policy_name`
    for ACL.
    
    "*" can match any table, including legacy tables and tables named by new rules.
---
 .../ranger/ranger_resource_policy_manager.cpp      | 56 +++++++++++++---------
 .../ranger/ranger_resource_policy_manager.h        |  5 +-
 .../test/ranger_resource_policy_manager_test.cpp   | 18 ++++++-
 3 files changed, 52 insertions(+), 27 deletions(-)

diff --git a/src/runtime/ranger/ranger_resource_policy_manager.cpp b/src/runtime/ranger/ranger_resource_policy_manager.cpp
index 7f708f808..44d22d789 100644
--- a/src/runtime/ranger/ranger_resource_policy_manager.cpp
+++ b/src/runtime/ranger/ranger_resource_policy_manager.cpp
@@ -72,6 +72,11 @@ 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(The table "
+                  "name does not follow the naming rules of {database_name}.{table_name})");
 
 #define RETURN_ERR_IF_MISSING_MEMBER(obj, member)                                                  \
     do {                                                                                           \
@@ -241,20 +246,25 @@ 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) != 0) {
                 return true;
             }
         }
+
+        // The check that does not match any DATABASE policy returns false.
+        return false;
     } while (false);
 
     // The check that does not match any policy returns false.
@@ -578,9 +588,10 @@ dsn::error_code ranger_resource_policy_manager::sync_policies_to_app_envs()
     LOG_AND_RETURN_NOT_OK(ERROR, list_resp.err, "list_apps failed.");
     for (const auto &app : list_resp.infos) {
         std::string database_name = get_database_name_from_app_name(app.app_name);
-        // Use "*" for table name of invalid Ranger rules to match datdabase resources.
+        // Use 'legacy_table_database_mapping_policy_name' for table name of invalid Ranger rules to
+        // match datdabase resources.
         if (database_name.empty()) {
-            database_name = "*";
+            database_name = FLAGS_legacy_table_database_mapping_policy_name;
         }
         std::string table_name = get_table_name_from_app_name(app.app_name);
 
@@ -590,32 +601,31 @@ 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) {
+            // If this table does not match any database, its Ranger policies 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);
+            _meta_svc->get_server_state()->wait_all_task();
+            LOG_AND_RETURN_NOT_OK(ERROR, rpc.response().err, "set_app_envs failed.");
+            break;
         }
 
-        // There is no matched policy, clear app Ranger policy
+        // There is no matched policy, clear the table's Ranger policies.
         if (!is_policy_matched) {
             req->__set_op(dsn::replication::app_env_operation::type::APP_ENV_OP_DEL);
 
             dsn::replication::update_app_env_rpc rpc(std::move(req), LPC_USE_RANGER_ACCESS_CONTROL);
             _meta_svc->get_server_state()->del_app_envs(rpc);
+            _meta_svc->get_server_state()->wait_all_task();
             LOG_AND_RETURN_NOT_OK(ERROR, rpc.response().err, "del_app_envs failed.");
         }
     }
diff --git a/src/runtime/ranger/ranger_resource_policy_manager.h b/src/runtime/ranger/ranger_resource_policy_manager.h
index 3b548bc67..7e46bec31 100644
--- a/src/runtime/ranger/ranger_resource_policy_manager.h
+++ b/src/runtime/ranger/ranger_resource_policy_manager.h
@@ -74,8 +74,9 @@ public:
     // When using Ranger for ACL, periodically pull policies from Ranger service.
     void start();
 
-    // Return true if the 'user_name' is allowed to access 'app_name' via 'rpc_code'.
-    bool allowed(const int rpc_code, const std::string &user_name, const std::string &app_name);
+    // 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);
 
 private:
     // Parse Ranger ACL policies from 'data' in JSON format into 'policies'.
diff --git a/src/runtime/test/ranger_resource_policy_manager_test.cpp b/src/runtime/test/ranger_resource_policy_manager_test.cpp
index e9fc87984..6660cab42 100644
--- a/src/runtime/test/ranger_resource_policy_manager_test.cpp
+++ b/src/runtime/test/ranger_resource_policy_manager_test.cpp
@@ -34,9 +34,11 @@
 #include "runtime/ranger/ranger_resource_policy_manager.h"
 #include "runtime/task/task_code.h"
 #include "utils/blob.h"
+#include "utils/flags.h"
 
 namespace dsn {
 namespace ranger {
+DSN_DECLARE_string(legacy_table_database_mapping_policy_name);
 
 TEST(ranger_resource_policy_manager_test, parse_policies_from_json_for_test)
 {
@@ -286,8 +288,17 @@ public:
               {{access_type::kCreate, {"user6"}}},
               {},
               {}}});
+        ranger_resource_policy fake_default_ranger_resource_policy(
+            {"",
+             {FLAGS_legacy_table_database_mapping_policy_name},
+             {},
+             {{{access_type::kCreate, {"user5", "user6"}}},
+              {{access_type::kCreate, {"user5"}}},
+              {},
+              {}}});
         _database_policies_cache = {fake_ranger_resource_policy_1,
                                     fake_ranger_resource_policy_2,
+                                    fake_default_ranger_resource_policy,
                                     fake_ranger_resource_policy_3};
 
         ranger_resource_policy fake_ranger_resource_policy_4(
@@ -331,9 +342,12 @@ TEST_F(ranger_resource_policy_manager_function_test, allowed)
                  {"RPC_CM_START_BACKUP_APP", "user3", "database2", true},
                  {"RPC_CM_START_BACKUP_APP", "user4", "database2", false},
                  {"TASK_CODE_INVALID", "user5", "", false},
+                 // Next two case matched to the default database policy and "*" database.
                  {"RPC_CM_CREATE_APP", "user5", "", true},
-                 {"RPC_CM_CREATE_APP", "user5", "database2", false},
-                 {"RPC_CM_CREATE_APP", "user6", "", false},
+                 {"RPC_CM_CREATE_APP", "user6", "", true},
+                 // Next two case matched to the database policy named "*".
+                 {"RPC_CM_CREATE_APP", "user5", "any_database_name", true},
+                 {"RPC_CM_CREATE_APP", "user6", "any_database_name", false},
                  {"RPC_CM_CREATE_APP", "user6", "database2", false},
                  {"TASK_CODE_INVALID", "user7", "database3", false},
                  {"RPC_CM_LIST_NODES", "user7", "database3", true},


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pegasus.apache.org
For additional commands, e-mail: commits-help@pegasus.apache.org