Merge pull request #14828 from vitlibar/change-columns-of-system-user-directories

Add 'params' column to system.user_directories table.

src/Access/AccessControlManager.cpp

@@ -186,7 +186,7 @@ void AccessControlManager::addUsersConfigStorage(
     {
         if (auto users_config_storage = typeid_cast<std::shared_ptr<UsersConfigAccessStorage>>(storage))
         {
-            if (users_config_storage->getStoragePath() == users_config_path_)
+            if (users_config_storage->isPathEqual(users_config_path_))
                 return;
         }
     }
@@ -229,7 +229,7 @@ void AccessControlManager::addDiskStorage(const String & storage_name_, const St
     {
         if (auto disk_storage = typeid_cast<std::shared_ptr<DiskAccessStorage>>(storage))
         {
-            if (disk_storage->isStoragePathEqual(directory_))
+            if (disk_storage->isPathEqual(directory_))
             {
                 if (readonly_)
                     disk_storage->setReadOnly(readonly_);

src/Access/DiskAccessStorage.cpp

@@ -33,6 +33,9 @@
 #include <Interpreters/InterpreterShowGrantsQuery.h>
 #include <Common/quoteString.h>
 #include <Core/Defines.h>
+#include <Poco/JSON/JSON.h>
+#include <Poco/JSON/Object.h>
+#include <Poco/JSON/Stringifier.h>
 #include <boost/range/adaptor/map.hpp>
 #include <boost/range/algorithm/copy.hpp>
 #include <boost/range/algorithm_ext/push_back.hpp>
@@ -342,9 +345,22 @@ DiskAccessStorage::~DiskAccessStorage()
 }
 
 
-bool DiskAccessStorage::isStoragePathEqual(const String & directory_path_) const
+String DiskAccessStorage::getStorageParamsJSON() const
 {
-    return getStoragePath() == makeDirectoryPathCanonical(directory_path_);
+    std::lock_guard lock{mutex};
+    Poco::JSON::Object json;
+    json.set("path", directory_path);
+    if (readonly)
+        json.set("readonly", readonly.load());
+    std::ostringstream oss;
+    Poco::JSON::Stringifier::stringify(json, oss);
+    return oss.str();
+}
+
+
+bool DiskAccessStorage::isPathEqual(const String & directory_path_) const
+{
+    return getPath() == makeDirectoryPathCanonical(directory_path_);
 }
 
 

src/Access/DiskAccessStorage.h

@@ -18,12 +18,13 @@ public:
     ~DiskAccessStorage() override;
 
     const char * getStorageType() const override { return STORAGE_TYPE; }
+    String getStorageParamsJSON() const override;
 
-    String getStoragePath() const override { return directory_path; }
-    bool isStoragePathEqual(const String & directory_path_) const;
+    String getPath() const { return directory_path; }
+    bool isPathEqual(const String & directory_path_) const;
 
     void setReadOnly(bool readonly_) { readonly = readonly_; }
-    bool isStorageReadOnly() const override { return readonly; }
+    bool isReadOnly() const { return readonly; }
 
 private:
     std::optional<UUID> findImpl(EntityType type, const String & name) const override;

src/Access/IAccessStorage.h

@@ -25,8 +25,9 @@ public:
     /// Returns the name of this storage.
     const String & getStorageName() const { return storage_name; }
     virtual const char * getStorageType() const = 0;
-    virtual String getStoragePath() const { return {}; }
-    virtual bool isStorageReadOnly() const { return false; }
+
+    /// Returns a JSON with the parameters of the storage. It's up to the storage type to fill the JSON.
+    virtual String getStorageParamsJSON() const { return "{}"; }
 
     using EntityType = IAccessEntity::Type;
     using EntityTypeInfo = IAccessEntity::TypeInfo;

src/Access/UsersConfigAccessStorage.cpp

@@ -10,6 +10,9 @@
 #include <Core/Settings.h>
 #include <Poco/Util/AbstractConfiguration.h>
 #include <Poco/MD5Engine.h>
+#include <Poco/JSON/JSON.h>
+#include <Poco/JSON/Object.h>
+#include <Poco/JSON/Stringifier.h>
 #include <common/logger_useful.h>
 #include <boost/range/algorithm/copy.hpp>
 #include <boost/range/adaptor/map.hpp>
@@ -482,12 +485,29 @@ UsersConfigAccessStorage::UsersConfigAccessStorage(const String & storage_name_,
 UsersConfigAccessStorage::~UsersConfigAccessStorage() = default;
 
 
-String UsersConfigAccessStorage::getStoragePath() const
+String UsersConfigAccessStorage::getStorageParamsJSON() const
+{
+    std::lock_guard lock{load_mutex};
+    Poco::JSON::Object json;
+    if (!path.empty())
+        json.set("path", path);
+    std::ostringstream oss;
+    Poco::JSON::Stringifier::stringify(json, oss);
+    return oss.str();
+}
+
+
+String UsersConfigAccessStorage::getPath() const
 {
     std::lock_guard lock{load_mutex};
     return path;
 }
 
+bool UsersConfigAccessStorage::isPathEqual(const String & path_) const
+{
+    return getPath() == path_;
+}
+
 
 void UsersConfigAccessStorage::setConfig(const Poco::Util::AbstractConfiguration & config)
 {

src/Access/UsersConfigAccessStorage.h

@@ -26,8 +26,10 @@ public:
     ~UsersConfigAccessStorage() override;
 
     const char * getStorageType() const override { return STORAGE_TYPE; }
-    String getStoragePath() const override;
-    bool isStorageReadOnly() const override { return true; }
+    String getStorageParamsJSON() const override;
+
+    String getPath() const;
+    bool isPathEqual(const String & path_) const;
 
     void setConfig(const Poco::Util::AbstractConfiguration & config);
 

src/Storages/System/StorageSystemUserDirectories.cpp

@@ -15,8 +15,7 @@ NamesAndTypesList StorageSystemUserDirectories::getNamesAndTypes()
     NamesAndTypesList names_and_types{
         {"name", std::make_shared<DataTypeString>()},
         {"type", std::make_shared<DataTypeString>()},
-        {"path", std::make_shared<DataTypeString>()},
-        {"readonly", std::make_shared<DataTypeUInt8>()},
+        {"params", std::make_shared<DataTypeString>()},
         {"precedence", std::make_shared<DataTypeUInt64>()},
     };
     return names_and_types;
@@ -31,21 +30,18 @@ void StorageSystemUserDirectories::fillData(MutableColumns & res_columns, const
     size_t column_index = 0;
     auto & column_name = assert_cast<ColumnString &>(*res_columns[column_index++]);
     auto & column_type = assert_cast<ColumnString &>(*res_columns[column_index++]);
-    auto & column_path = assert_cast<ColumnString &>(*res_columns[column_index++]);
-    auto & column_readonly = assert_cast<ColumnUInt8 &>(*res_columns[column_index++]);
+    auto & column_params = assert_cast<ColumnString &>(*res_columns[column_index++]);
     auto & column_precedence = assert_cast<ColumnUInt64 &>(*res_columns[column_index++]);
 
     auto add_row = [&](const IAccessStorage & storage, size_t precedence)
     {
         const String & name = storage.getStorageName();
         std::string_view type = storage.getStorageType();
-        const String & path = storage.getStoragePath();
-        bool readonly = storage.isStorageReadOnly();
+        String params = storage.getStorageParamsJSON();
 
         column_name.insertData(name.data(), name.length());
         column_type.insertData(type.data(), type.length());
-        column_path.insertData(path.data(), path.length());
-        column_readonly.insert(readonly);
+        column_params.insertData(params.data(), params.length());
         column_precedence.insert(precedence);
     };
 

tests/integration/test_user_directories/test.py

@@ -24,40 +24,40 @@ def started_cluster():
 def test_old_style():
     node.copy_file_to_container(os.path.join(SCRIPT_DIR, "configs/old_style.xml"), '/etc/clickhouse-server/config.d/z.xml')
     node.restart_clickhouse()
-    assert node.query("SELECT * FROM system.user_directories") == TSV([["users.xml",       "users.xml",       "/etc/clickhouse-server/users2.xml", 1, 1],
-                                                                       ["local directory", "local directory", "/var/lib/clickhouse/access2/",      0, 2]])
+    assert node.query("SELECT * FROM system.user_directories") == TSV([["users.xml",       "users.xml",       '{"path":"\\\\/etc\\\\/clickhouse-server\\\\/users2.xml"}',    1],
+                                                                       ["local directory", "local directory", '{"path":"\\\\/var\\\\/lib\\\\/clickhouse\\\\/access2\\\\/"}', 2]])
 
 
 def test_local_directories():
     node.copy_file_to_container(os.path.join(SCRIPT_DIR, "configs/local_directories.xml"), '/etc/clickhouse-server/config.d/z.xml')
     node.restart_clickhouse()
-    assert node.query("SELECT * FROM system.user_directories") == TSV([["users.xml",            "users.xml",       "/etc/clickhouse-server/users3.xml", 1, 1],
-                                                                       ["local directory",      "local directory", "/var/lib/clickhouse/access3/",      0, 2],
-                                                                       ["local directory (ro)", "local directory", "/var/lib/clickhouse/access3-ro/",   1, 3]])
+    assert node.query("SELECT * FROM system.user_directories") == TSV([["users.xml",            "users.xml",       '{"path":"\\\\/etc\\\\/clickhouse-server\\\\/users3.xml"}',                       1],
+                                                                       ["local directory",      "local directory", '{"path":"\\\\/var\\\\/lib\\\\/clickhouse\\\\/access3\\\\/"}',                    2],
+                                                                       ["local directory (ro)", "local directory", '{"path":"\\\\/var\\\\/lib\\\\/clickhouse\\\\/access3-ro\\\\/","readonly":true}', 3]])
 
 
 def test_relative_path():
     node.copy_file_to_container(os.path.join(SCRIPT_DIR, "configs/relative_path.xml"), '/etc/clickhouse-server/config.d/z.xml')
     node.restart_clickhouse()
-    assert node.query("SELECT * FROM system.user_directories") == TSV([["users.xml", "users.xml", "/etc/clickhouse-server/users4.xml", 1, 1]])
+    assert node.query("SELECT * FROM system.user_directories") == TSV([["users.xml", "users.xml", '{"path":"\\\\/etc\\\\/clickhouse-server\\\\/users4.xml"}', 1]])
 
 
 def test_memory():
     node.copy_file_to_container(os.path.join(SCRIPT_DIR, "configs/memory.xml"), '/etc/clickhouse-server/config.d/z.xml')
     node.restart_clickhouse()
-    assert node.query("SELECT * FROM system.user_directories") == TSV([["users.xml", "users.xml", "/etc/clickhouse-server/users5.xml", 1, 1],
-                                                                       ["memory",    "memory",    "",                                  0, 2]])
+    assert node.query("SELECT * FROM system.user_directories") == TSV([["users.xml", "users.xml", '{"path":"\\\\/etc\\\\/clickhouse-server\\\\/users5.xml"}', 1],
+                                                                       ["memory",    "memory",    '{}',                                                       2]])
 
 def test_mixed_style():
     node.copy_file_to_container(os.path.join(SCRIPT_DIR, "configs/mixed_style.xml"), '/etc/clickhouse-server/config.d/z.xml')
     node.restart_clickhouse()
-    assert node.query("SELECT * FROM system.user_directories") == TSV([["users.xml",       "users.xml",       "/etc/clickhouse-server/users6.xml", 1, 1],
-                                                                       ["local directory", "local directory", "/var/lib/clickhouse/access6/",      0, 2],
-                                                                       ["local directory", "local directory", "/var/lib/clickhouse/access6a/",     0, 3],
-                                                                       ["memory",          "memory",          "",                                  0, 4]])
+    assert node.query("SELECT * FROM system.user_directories") == TSV([["users.xml",       "users.xml",       '{"path":"\\\\/etc\\\\/clickhouse-server\\\\/users6.xml"}',     1],
+                                                                       ["local directory", "local directory", '{"path":"\\\\/var\\\\/lib\\\\/clickhouse\\\\/access6\\\\/"}',  2],
+                                                                       ["local directory", "local directory", '{"path":"\\\\/var\\\\/lib\\\\/clickhouse\\\\/access6a\\\\/"}', 3],
+                                                                       ["memory",          "memory",          '{}',                                                           4]])
 
 def test_duplicates():
     node.copy_file_to_container(os.path.join(SCRIPT_DIR, "configs/duplicates.xml"), '/etc/clickhouse-server/config.d/z.xml')
     node.restart_clickhouse()
-    assert node.query("SELECT * FROM system.user_directories") == TSV([["users.xml",       "users.xml",       "/etc/clickhouse-server/users7.xml", 1, 1],
-                                                                       ["local directory", "local directory", "/var/lib/clickhouse/access7/",      0, 2]])
+    assert node.query("SELECT * FROM system.user_directories") == TSV([["users.xml",       "users.xml",       '{"path":"\\\\/etc\\\\/clickhouse-server\\\\/users7.xml"}',    1],
+                                                                       ["local directory", "local directory", '{"path":"\\\\/var\\\\/lib\\\\/clickhouse\\\\/access7\\\\/"}', 2]])