Check Storage permission (#2174)

* add class storage checker
Signed-off-by: NYhz <yinghao.zou@zilliz.com>

* code format
Signed-off-by: NYhz <yinghao.zou@zilliz.com>

* code format
Signed-off-by: NYhz <yinghao.zou@zilliz.com>

* add cpu instruction sets check (#2055)
Signed-off-by: NYhz <yinghao.zou@zilliz.com>

* add more details for storage
Signed-off-by: NYhz <yinghao.zou@zilliz.com>

* Check storage path permission (fix #2173)
Signed-off-by: NYhz <yinghao.zou@zilliz.com>

* Fix logs path access fail issue
Signed-off-by: NYhz <yinghao.zou@zilliz.com>
Co-authored-by: NJin Hai <hai.jin@zilliz.com>

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

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;

core/src/server/init/StorageChecker.cpp

+// 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 <unistd.h>
+
+#include <string>
+#include <vector>
+
+#include <fiu-local.h>
+
+#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<std::string> 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

core/src/server/init/StorageChecker.h

+// 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

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");

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 <boost/filesystem.hpp>
+
 #include <fiu-control.h>
 #include <fiu-local.h>
 #include <gtest/gtest.h>
@@ -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) {

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)
 };