From 2c889636aba408e2a7ab6466c82522ce7fb107b6 Mon Sep 17 00:00:00 2001 From: BossZou <40255591+BossZou@users.noreply.github.com> Date: Thu, 30 Apr 2020 01:23:32 +0800 Subject: [PATCH] Check Storage permission (#2174) * add class storage checker Signed-off-by: Yhz * code format Signed-off-by: Yhz * code format Signed-off-by: Yhz * add cpu instruction sets check (#2055) Signed-off-by: Yhz * add more details for storage Signed-off-by: Yhz * Check storage path permission (fix #2173) Signed-off-by: Yhz * Fix logs path access fail issue Signed-off-by: Yhz Co-authored-by: Jin Hai --- CHANGELOG.md | 1 + core/src/server/Server.cpp | 6 ++ core/src/server/init/StorageChecker.cpp | 106 ++++++++++++++++++++++++ core/src/server/init/StorageChecker.h | 26 ++++++ core/src/utils/LogUtil.cpp | 15 ++-- core/unittest/server/test_check.cpp | 60 +++++++++++++- core/unittest/server/test_web.cpp | 8 +- 7 files changed, 211 insertions(+), 11 deletions(-) create mode 100644 core/src/server/init/StorageChecker.cpp create mode 100644 core/src/server/init/StorageChecker.h diff --git a/CHANGELOG.md b/CHANGELOG.md index a77a09f6..cf9b8282 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,7 @@ Please mark all change in change log and use the issue from GitHub - \#2149 Merge server_cpu_config.template and server_gpu_config.template - \#2153 Upgrade thirdparty oatpp to v1.0.0 - \#2167 Merge log_config.conf with server_config.yaml +- \#2173 Check storage permission - \#2178 Using elkan K-Means to improve IVF ## Task diff --git a/core/src/server/Server.cpp b/core/src/server/Server.cpp index 692459a9..f2a8566f 100644 --- a/core/src/server/Server.cpp +++ b/core/src/server/Server.cpp @@ -25,6 +25,7 @@ #include "server/grpc_impl/GrpcServer.h" #include "server/init/CpuChecker.h" #include "server/init/GpuChecker.h" +#include "server/init/StorageChecker.h" #include "server/web_impl/WebServer.h" #include "src/version.h" //#include "storage/s3/S3ClientWrapper.h" @@ -289,6 +290,11 @@ Server::Start() { #else LOG_SERVER_INFO_ << "CPU edition"; #endif + s = StorageChecker::CheckStoragePermission(); + if (!s.ok()) { + return s; + } + s = CpuChecker::CheckCpuInstructionSet(); if (!s.ok()) { return s; diff --git a/core/src/server/init/StorageChecker.cpp b/core/src/server/init/StorageChecker.cpp new file mode 100644 index 00000000..d64b2e26 --- /dev/null +++ b/core/src/server/init/StorageChecker.cpp @@ -0,0 +1,106 @@ +// Copyright (C) 2019-2020 Zilliz. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software distributed under the License +// is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express +// or implied. See the License for the specific language governing permissions and limitations under the License. + +#include "server/init/StorageChecker.h" + +#include + +#include +#include + +#include + +#include "config/Config.h" +#include "utils/Log.h" +#include "utils/StringHelpFunctions.h" + +namespace milvus { +namespace server { + +Status +StorageChecker::CheckStoragePermission() { + auto& config = Config::GetInstance(); + /* Check log file write permission */ + std::string logs_path; + auto status = config.GetLogsPath(logs_path); + if (!status.ok()) { + return status; + } + int ret = access(logs_path.c_str(), F_OK | R_OK | W_OK); + fiu_do_on("StorageChecker.CheckStoragePermission.logs_path_access_fail", ret = -1); + if (0 != ret) { + std::string err_msg = + " Access log path " + logs_path + " fail. " + strerror(errno) + "(code: " + std::to_string(errno) + ")"; + LOG_SERVER_FATAL_ << err_msg; + std::cerr << err_msg << std::endl; + return Status(SERVER_UNEXPECTED_ERROR, err_msg); + } + + /* Check db directory write permission */ + std::string primary_path; + status = config.GetStorageConfigPrimaryPath(primary_path); + if (!status.ok()) { + return status; + } + + ret = access(primary_path.c_str(), F_OK | R_OK | W_OK); + fiu_do_on("StorageChecker.CheckStoragePermission.db_primary_path_access_fail", ret = -1); + if (0 != ret) { + std::string err_msg = " Access DB storage primary path " + primary_path + " fail. " + strerror(errno) + + "(code: " + std::to_string(errno) + ")"; + LOG_SERVER_FATAL_ << err_msg; + std::cerr << err_msg << std::endl; + return Status(SERVER_UNEXPECTED_ERROR, err_msg); + } + + std::string secondary_paths; + status = config.GetStorageConfigSecondaryPath(secondary_paths); + if (!status.ok()) { + return status; + } + + if (!secondary_paths.empty()) { + std::vector secondary_path_vector; + StringHelpFunctions::SplitStringByDelimeter(secondary_paths, ",", secondary_path_vector); + for (auto& path : secondary_path_vector) { + ret = access(path.c_str(), F_OK | R_OK | W_OK); + fiu_do_on("StorageChecker.CheckStoragePermission.db_secondary_path_access_fail", ret = -1); + if (0 != ret) { + std::string err_msg = " Access DB storage secondary path " + path + " fail. " + strerror(errno) + + "(code: " + std::to_string(errno) + ")"; + LOG_SERVER_FATAL_ << err_msg; + std::cerr << err_msg << std::endl; + return Status(SERVER_UNEXPECTED_ERROR, err_msg); + } + } + } + + /* Check wal directory write permission */ + std::string wal_path; + status = config.GetWalConfigWalPath(wal_path); + if (!status.ok()) { + return status; + } + ret = access(wal_path.c_str(), F_OK | R_OK | W_OK); + fiu_do_on("StorageChecker.CheckStoragePermission.wal_path_access_fail", ret = -1); + if (0 != ret) { + std::string err_msg = " Access WAL storage path " + wal_path + " fail. " + strerror(errno) + + "(code: " + std::to_string(errno) + ")"; + LOG_SERVER_FATAL_ << err_msg; + std::cerr << err_msg << std::endl; + return Status(SERVER_UNEXPECTED_ERROR, err_msg); + } + + return Status::OK(); +} + +} // namespace server +} // namespace milvus diff --git a/core/src/server/init/StorageChecker.h b/core/src/server/init/StorageChecker.h new file mode 100644 index 00000000..b55ffdc7 --- /dev/null +++ b/core/src/server/init/StorageChecker.h @@ -0,0 +1,26 @@ +// Copyright (C) 2019-2020 Zilliz. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software distributed under the License +// is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express +// or implied. See the License for the specific language governing permissions and limitations under the License. + +#pragma once + +#include "utils/Status.h" + +namespace milvus { +namespace server { + +class StorageChecker { + public: + static Status + CheckStoragePermission(); +}; + +} // namespace server +} // namespace milvus diff --git a/core/src/utils/LogUtil.cpp b/core/src/utils/LogUtil.cpp index 865070d1..e8d70a28 100644 --- a/core/src/utils/LogUtil.cpp +++ b/core/src/utils/LogUtil.cpp @@ -94,11 +94,12 @@ InitLog(bool trace_enable, bool debug_enable, bool info_enable, bool warning_ena defaultConf.setGlobally(el::ConfigurationType::PerformanceTracking, "false"); defaultConf.setGlobally(el::ConfigurationType::MaxLogFileSize, "209715200"); - std::string global_log_path = logs_path + "milvus-%datetime{%y-%M-%d-%H:%m}-global.log"; + std::string logs_reg_path = logs_path.rfind('/') == logs_path.length() - 1 ? logs_path : logs_path + "/"; + std::string global_log_path = logs_reg_path + "milvus-%datetime{%y-%M-%d-%H:%m}-global.log"; defaultConf.set(el::Level::Global, el::ConfigurationType::Filename, global_log_path.c_str()); defaultConf.set(el::Level::Global, el::ConfigurationType::Enabled, "true"); - std::string info_log_path = logs_path + "milvus-%datetime{%y-%M-%d-%H:%m}-info.log"; + std::string info_log_path = logs_reg_path + "milvus-%datetime{%y-%M-%d-%H:%m}-info.log"; defaultConf.set(el::Level::Info, el::ConfigurationType::Filename, info_log_path.c_str()); if (info_enable) { defaultConf.set(el::Level::Info, el::ConfigurationType::Enabled, "true"); @@ -106,7 +107,7 @@ InitLog(bool trace_enable, bool debug_enable, bool info_enable, bool warning_ena defaultConf.set(el::Level::Info, el::ConfigurationType::Enabled, "false"); } - std::string debug_log_path = logs_path + "milvus-%datetime{%y-%M-%d-%H:%m}-debug.log"; + std::string debug_log_path = logs_reg_path + "milvus-%datetime{%y-%M-%d-%H:%m}-debug.log"; defaultConf.set(el::Level::Debug, el::ConfigurationType::Filename, debug_log_path.c_str()); if (debug_enable) { defaultConf.set(el::Level::Debug, el::ConfigurationType::Enabled, "true"); @@ -114,7 +115,7 @@ InitLog(bool trace_enable, bool debug_enable, bool info_enable, bool warning_ena defaultConf.set(el::Level::Debug, el::ConfigurationType::Enabled, "false"); } - std::string warning_log_path = logs_path + "milvus-%datetime{%y-%M-%d-%H:%m}-warning.log"; + std::string warning_log_path = logs_reg_path + "milvus-%datetime{%y-%M-%d-%H:%m}-warning.log"; defaultConf.set(el::Level::Warning, el::ConfigurationType::Filename, warning_log_path.c_str()); if (warning_enable) { defaultConf.set(el::Level::Warning, el::ConfigurationType::Enabled, "true"); @@ -122,7 +123,7 @@ InitLog(bool trace_enable, bool debug_enable, bool info_enable, bool warning_ena defaultConf.set(el::Level::Warning, el::ConfigurationType::Enabled, "false"); } - std::string trace_log_path = logs_path + "milvus-%datetime{%y-%M-%d-%H:%m}-trace.log"; + std::string trace_log_path = logs_reg_path + "milvus-%datetime{%y-%M-%d-%H:%m}-trace.log"; defaultConf.set(el::Level::Trace, el::ConfigurationType::Filename, trace_log_path.c_str()); if (trace_enable) { defaultConf.set(el::Level::Trace, el::ConfigurationType::Enabled, "true"); @@ -130,7 +131,7 @@ InitLog(bool trace_enable, bool debug_enable, bool info_enable, bool warning_ena defaultConf.set(el::Level::Trace, el::ConfigurationType::Enabled, "false"); } - std::string error_log_path = logs_path + "milvus-%datetime{%y-%M-%d-%H:%m}-error.log"; + std::string error_log_path = logs_reg_path + "milvus-%datetime{%y-%M-%d-%H:%m}-error.log"; defaultConf.set(el::Level::Error, el::ConfigurationType::Filename, error_log_path.c_str()); if (error_enable) { defaultConf.set(el::Level::Error, el::ConfigurationType::Enabled, "true"); @@ -138,7 +139,7 @@ InitLog(bool trace_enable, bool debug_enable, bool info_enable, bool warning_ena defaultConf.set(el::Level::Error, el::ConfigurationType::Enabled, "false"); } - std::string fatal_log_path = logs_path + "milvus-%datetime{%y-%M-%d-%H:%m}-fatal.log"; + std::string fatal_log_path = logs_reg_path + "milvus-%datetime{%y-%M-%d-%H:%m}-fatal.log"; defaultConf.set(el::Level::Fatal, el::ConfigurationType::Filename, fatal_log_path.c_str()); if (fatal_enable) { defaultConf.set(el::Level::Fatal, el::ConfigurationType::Enabled, "true"); diff --git a/core/unittest/server/test_check.cpp b/core/unittest/server/test_check.cpp index 95131708..e00312be 100644 --- a/core/unittest/server/test_check.cpp +++ b/core/unittest/server/test_check.cpp @@ -9,6 +9,8 @@ // is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express // or implied. See the License for the specific language governing permissions and limitations under the License. +#include + #include #include #include @@ -18,6 +20,7 @@ #ifdef MILVUS_GPU_VERSION #include "server/init/GpuChecker.h" #endif +#include "server/init/StorageChecker.h" namespace ms = milvus::server; @@ -25,16 +28,71 @@ class ServerCheckerTest : public testing::Test { protected: void SetUp() override { + auto& config = ms::Config::GetInstance(); + + logs_path = "/tmp/milvus-test/logs"; + boost::filesystem::create_directories(logs_path); + config.SetLogsPath(logs_path); + + db_primary_path = "/tmp/milvus-test/db"; + boost::filesystem::create_directories(db_primary_path); + config.SetStorageConfigPrimaryPath(db_primary_path); + + db_secondary_path = "/tmp/milvus-test/db-secondary"; + boost::filesystem::create_directories(db_secondary_path); + config.SetStorageConfigSecondaryPath(db_secondary_path); + + wal_path = "/tmp/milvus-test/wal"; + boost::filesystem::create_directories(wal_path); + config.SetWalConfigWalPath(wal_path); } void TearDown() override { + boost::filesystem::remove_all(logs_path); + boost::filesystem::remove_all(db_primary_path); + boost::filesystem::remove_all(db_secondary_path); + boost::filesystem::remove_all(wal_path); } + + protected: + std::string logs_path; + std::string db_primary_path; + std::string db_secondary_path; + std::string wal_path; }; +TEST_F(ServerCheckerTest, STORAGE_TEST) { + auto status = ms::StorageChecker::CheckStoragePermission(); + ASSERT_TRUE(status.ok()) << status.message(); +} + +TEST_F(ServerCheckerTest, STORAGE_FAIL_TEST) { + fiu_enable("StorageChecker.CheckStoragePermission.logs_path_access_fail", 1, NULL, 0); + ASSERT_FALSE(ms::StorageChecker::CheckStoragePermission().ok()); + fiu_disable("StorageChecker.CheckStoragePermission.logs_path_access_fail"); + + fiu_enable("StorageChecker.CheckStoragePermission.db_primary_path_access_fail", 1, NULL, 0); + ASSERT_FALSE(ms::StorageChecker::CheckStoragePermission().ok()); + fiu_disable("StorageChecker.CheckStoragePermission.db_primary_path_access_fail"); + + auto& config = ms::Config::GetInstance(); + std::string storage_secondary_path; + ASSERT_TRUE(config.GetStorageConfigSecondaryPath(storage_secondary_path).ok()); + ASSERT_TRUE(config.SetStorageConfigSecondaryPath("/tmp/milvus-test01,/tmp/milvus-test02").ok()); + fiu_enable("StorageChecker.CheckStoragePermission.db_secondary_path_access_fail", 1, NULL, 0); + ASSERT_FALSE(ms::StorageChecker::CheckStoragePermission().ok()); + fiu_disable("StorageChecker.CheckStoragePermission.db_secondary_path_access_fail"); + ASSERT_TRUE(config.SetStorageConfigSecondaryPath(storage_secondary_path).ok()); + + fiu_enable("StorageChecker.CheckStoragePermission.wal_path_access_fail", 1, NULL, 0); + ASSERT_FALSE(ms::StorageChecker::CheckStoragePermission().ok()); + fiu_disable("StorageChecker.CheckStoragePermission.wal_path_access_fail"); +} + TEST_F(ServerCheckerTest, CPU_TEST) { auto status = ms::CpuChecker::CheckCpuInstructionSet(); - ASSERT_TRUE(status.ok()); + ASSERT_TRUE(status.ok()) << status.message(); } TEST_F(ServerCheckerTest, CPU_FAIL_TEST) { diff --git a/core/unittest/server/test_web.cpp b/core/unittest/server/test_web.cpp index 1c2cd4ff..b07e4518 100644 --- a/core/unittest/server/test_web.cpp +++ b/core/unittest/server/test_web.cpp @@ -38,9 +38,9 @@ #include "server/web_impl/dto/TableDto.hpp" #include "server/web_impl/dto/VectorDto.hpp" #include "server/web_impl/handler/WebRequestHandler.h" +#include "src/version.h" #include "unittest/server/utils.h" #include "utils/CommonUtil.h" -#include "version.h" static const char* COLLECTION_NAME = "test_web"; @@ -711,9 +711,11 @@ class TestClient : public oatpp::web::client::ApiClient { API_CALL("POST", "/hybrid_collections", createHybridCollection, BODY_STRING(String, body_str)) - API_CALL("POST", "/hybrid_collections/{collection_name}/entities", InsertEntity, PATH(String, collection_name), BODY_STRING(String, body)) + API_CALL("POST", "/hybrid_collections/{collection_name}/entities", InsertEntity, + PATH(String, collection_name), BODY_STRING(String, body)) -// API_CALL("POST", "/hybrid_collections/{collection_name}/vectors", HybridSearch, PATH(String, collection_name), BODY_STRING(String, body)) +// API_CALL("POST", "/hybrid_collections/{collection_name}/vectors", HybridSearch, +// PATH(String, collection_name), BODY_STRING(String, body)) #include OATPP_CODEGEN_END(ApiClient) }; -- GitLab