Skip access storages with same path while reading the main config.

This commit is contained in:
Vitaly Baranov 2020-09-11 19:54:22 +03:00
parent 6dd764bcfe
commit e2c2a679ef
6 changed files with 81 additions and 18 deletions

View File

@ -181,6 +181,15 @@ void AccessControlManager::addUsersConfigStorage(
const String & preprocessed_dir_,
const zkutil::GetZooKeeper & get_zookeeper_function_)
{
auto storages = getStoragesPtr();
for (const auto & storage : *storages)
{
if (auto users_config_storage = typeid_cast<std::shared_ptr<UsersConfigAccessStorage>>(storage))
{
if (users_config_storage->getStoragePath() == users_config_path_)
return;
}
}
auto check_setting_name_function = [this](const std::string_view & setting_name) { checkSettingNameIsAllowed(setting_name); };
auto new_storage = std::make_shared<UsersConfigAccessStorage>(storage_name_, check_setting_name_function);
new_storage->load(users_config_path_, include_from_path_, preprocessed_dir_, get_zookeeper_function_);
@ -210,17 +219,36 @@ void AccessControlManager::startPeriodicReloadingUsersConfigs()
void AccessControlManager::addDiskStorage(const String & directory_, bool readonly_)
{
addStorage(std::make_shared<DiskAccessStorage>(directory_, readonly_));
addDiskStorage(DiskAccessStorage::STORAGE_TYPE, directory_, readonly_);
}
void AccessControlManager::addDiskStorage(const String & storage_name_, const String & directory_, bool readonly_)
{
auto storages = getStoragesPtr();
for (const auto & storage : *storages)
{
if (auto disk_storage = typeid_cast<std::shared_ptr<DiskAccessStorage>>(storage))
{
if (disk_storage->isStoragePathEqual(directory_))
{
if (readonly_)
disk_storage->setReadOnly(readonly_);
return;
}
}
}
addStorage(std::make_shared<DiskAccessStorage>(storage_name_, directory_, readonly_));
}
void AccessControlManager::addMemoryStorage(const String & storage_name_)
{
auto storages = getStoragesPtr();
for (const auto & storage : *storages)
{
if (auto memory_storage = typeid_cast<std::shared_ptr<MemoryAccessStorage>>(storage))
return;
}
addStorage(std::make_shared<MemoryAccessStorage>(storage_name_));
}

View File

@ -218,6 +218,16 @@ namespace
}
/// Converts a path to an absolute path and append it with a separator.
String makeDirectoryPathCanonical(const String & directory_path)
{
auto canonical_directory_path = std::filesystem::weakly_canonical(directory_path);
if (canonical_directory_path.has_filename())
canonical_directory_path += std::filesystem::path::preferred_separator;
return canonical_directory_path;
}
/// Calculates the path to a file named <id>.sql for saving an access entity.
String getEntityFilePath(const String & directory_path, const UUID & id)
{
@ -298,22 +308,17 @@ DiskAccessStorage::DiskAccessStorage(const String & directory_path_, bool readon
{
}
DiskAccessStorage::DiskAccessStorage(const String & storage_name_, const String & directory_path_, bool readonly_)
: IAccessStorage(storage_name_)
{
auto canonical_directory_path = std::filesystem::weakly_canonical(directory_path_);
if (canonical_directory_path.has_filename())
canonical_directory_path += std::filesystem::path::preferred_separator;
directory_path = makeDirectoryPathCanonical(directory_path_);
readonly = readonly_;
std::error_code create_dir_error_code;
std::filesystem::create_directories(canonical_directory_path, create_dir_error_code);
std::filesystem::create_directories(directory_path, create_dir_error_code);
if (!std::filesystem::exists(canonical_directory_path) || !std::filesystem::is_directory(canonical_directory_path) || create_dir_error_code)
throw Exception("Couldn't create directory " + canonical_directory_path.string() + " reason: '" + create_dir_error_code.message() + "'", ErrorCodes::DIRECTORY_DOESNT_EXIST);
directory_path = canonical_directory_path;
readonly = readonly_;
if (!std::filesystem::exists(directory_path) || !std::filesystem::is_directory(directory_path) || create_dir_error_code)
throw Exception("Couldn't create directory " + directory_path + " reason: '" + create_dir_error_code.message() + "'", ErrorCodes::DIRECTORY_DOESNT_EXIST);
bool should_rebuild_lists = std::filesystem::exists(getNeedRebuildListsMarkFilePath(directory_path));
if (!should_rebuild_lists)
@ -337,6 +342,12 @@ DiskAccessStorage::~DiskAccessStorage()
}
bool DiskAccessStorage::isStoragePathEqual(const String & directory_path_) const
{
return getStoragePath() == makeDirectoryPathCanonical(directory_path_);
}
void DiskAccessStorage::clear()
{
entries_by_id.clear();

View File

@ -18,7 +18,11 @@ public:
~DiskAccessStorage() override;
const char * getStorageType() const override { return STORAGE_TYPE; }
String getStoragePath() const override { return directory_path; }
bool isStoragePathEqual(const String & directory_path_) const;
void setReadOnly(bool readonly_) { readonly = readonly_; }
bool isStorageReadOnly() const override { return readonly; }
private:
@ -67,7 +71,7 @@ private:
void prepareNotifications(const UUID & id, const Entry & entry, bool remove, Notifications & notifications) const;
String directory_path;
bool readonly;
std::atomic<bool> readonly;
std::unordered_map<UUID, Entry> entries_by_id;
std::unordered_map<std::string_view, Entry *> entries_by_name_and_type[static_cast<size_t>(EntityType::MAX)];
boost::container::flat_set<EntityType> types_of_lists_to_write;

View File

@ -0,0 +1,13 @@
<yandex>
<user_directories replace="replace">
<local_directory>
<path>/var/lib/clickhouse/access7/</path>
</local_directory>
<users_xml>
<path>/etc/clickhouse-server/users7.xml</path>
</users_xml>
</user_directories>
<users_config>/etc/clickhouse-server/users7.xml</users_config>
<access_control_path>/var/lib/clickhouse/access7/</access_control_path>
</yandex>

View File

@ -1,5 +1,8 @@
<yandex>
<user_directories replace="replace">
<local_directory>
<path>/var/lib/clickhouse/access6a/</path>
</local_directory>
<memory/>
</user_directories>

View File

@ -12,11 +12,8 @@ def started_cluster():
try:
cluster.start()
node.exec_in_container("cp /etc/clickhouse-server/users.xml /etc/clickhouse-server/users2.xml")
node.exec_in_container("cp /etc/clickhouse-server/users.xml /etc/clickhouse-server/users3.xml")
node.exec_in_container("cp /etc/clickhouse-server/users.xml /etc/clickhouse-server/users4.xml")
node.exec_in_container("cp /etc/clickhouse-server/users.xml /etc/clickhouse-server/users5.xml")
node.exec_in_container("cp /etc/clickhouse-server/users.xml /etc/clickhouse-server/users6.xml")
for i in range(2, 8):
node.exec_in_container("cp /etc/clickhouse-server/users.xml /etc/clickhouse-server/users{}.xml".format(i))
yield cluster
@ -56,4 +53,11 @@ def test_mixed_style():
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],
["memory", "memory", "", 0, 3]])
["local directory", "local directory", "/var/lib/clickhouse/access6a/", 0, 3],
["memory", "memory", "", 0, 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]])