ApiManager reads service configs from the server config (istio#367)

incfly · Jun 13, 2017 · 7f8caa5 · 7f8caa5
1 parent 0596a8a
commit 7f8caa5
Show file tree

Hide file tree

Showing 4 changed files with 35 additions and 38 deletions.
diff --git a/contrib/endpoints/include/api_manager/api_manager.h b/contrib/endpoints/include/api_manager/api_manager.h
@@ -102,7 +102,7 @@ class ApiManagerFactory {
   // Create an ApiManager object.
   std::shared_ptr<ApiManager> CreateApiManager(
       std::unique_ptr<ApiManagerEnvInterface> env,
-      const std::string &service_config, const std::string &server_config);
+      const std::string &server_config);
 };
 
 }  // namespace api_manager

diff --git a/contrib/endpoints/src/api_manager/api_manager_impl.cc b/contrib/endpoints/src/api_manager/api_manager_impl.cc
@@ -31,23 +31,19 @@ const std::string kConfigRolloutManaged("managed");
 }  // namespace anonymous
 
 ApiManagerImpl::ApiManagerImpl(std::unique_ptr<ApiManagerEnvInterface> env,
-                               const std::string &service_config,
                                const std::string &server_config)
     : global_context_(
           new context::GlobalContext(std::move(env), server_config)),
       config_loading_status_(
           utils::Status(Code::UNAVAILABLE, "Not initialized yet")) {
-  if (!service_config.empty()) {
-    std::string config_id;
-    if (AddConfig(service_config, false, &config_id).ok()) {
-      DeployConfigs({{config_id, 100}});
-      config_loading_status_ = utils::Status::OK;
-    } else {
-      config_loading_status_ =
-          utils::Status(Code::ABORTED, "Invalid service config");
-    }
-  } else if (global_context_->server_config()->init_service_configs_size() >
-             0) {
+  if (!global_context_->server_config()) {
+    std::string err_msg = "Invalid server config";
+    global_context_->env()->LogError(err_msg);
+    config_loading_status_ = utils::Status(Code::ABORTED, err_msg);
+    return;
+  }
+
+  if (global_context_->server_config()->init_service_configs_size() > 0) {
     std::vector<std::pair<std::string, int>> list;
     for (auto item : global_context_->server_config()->init_service_configs()) {
       std::ifstream config_file(item.service_config_file_full_path());
@@ -235,9 +231,9 @@ std::unique_ptr<RequestHandlerInterface> ApiManagerImpl::CreateRequestHandler(
 
 std::shared_ptr<ApiManager> ApiManagerFactory::CreateApiManager(
     std::unique_ptr<ApiManagerEnvInterface> env,
-    const std::string &service_config, const std::string &server_config) {
+    const std::string &server_config) {
   return std::shared_ptr<ApiManager>(
-      new ApiManagerImpl(std::move(env), service_config, server_config));
+      new ApiManagerImpl(std::move(env), server_config));
 }
 
 }  // namespace api_manager

diff --git a/contrib/endpoints/src/api_manager/api_manager_impl.h b/contrib/endpoints/src/api_manager/api_manager_impl.h
@@ -30,7 +30,6 @@ class CheckWorkflow;
 class ApiManagerImpl : public ApiManager {
  public:
   ApiManagerImpl(std::unique_ptr<ApiManagerEnvInterface> env,
-                 const std::string &service_config,
                  const std::string &server_config);
 
   bool Enabled() const override;

diff --git a/contrib/endpoints/src/api_manager/api_manager_test.cc b/contrib/endpoints/src/api_manager/api_manager_test.cc
@@ -133,10 +133,7 @@ class ApiManagerTest : public ::testing::Test {
  protected:
   ApiManagerTest() : callback_run_count_(0) {}
   std::shared_ptr<ApiManager> MakeApiManager(
-      std::unique_ptr<ApiManagerEnvInterface> env, const char *service_config);
-  std::shared_ptr<ApiManager> MakeApiManager(
-      std::unique_ptr<ApiManagerEnvInterface> env, const char *service_config,
-      const char *server_config);
+      std::unique_ptr<ApiManagerEnvInterface> env, const char *server_config);
 
   void SetUp() {
     callback_run_count_ = 0;
@@ -152,15 +149,8 @@ class ApiManagerTest : public ::testing::Test {
 };
 
 std::shared_ptr<ApiManager> ApiManagerTest::MakeApiManager(
-    std::unique_ptr<ApiManagerEnvInterface> env, const char *service_config) {
-  return factory_.CreateApiManager(std::move(env), service_config, "");
-}
-
-std::shared_ptr<ApiManager> ApiManagerTest::MakeApiManager(
-    std::unique_ptr<ApiManagerEnvInterface> env, const char *service_config,
-    const char *server_config) {
-  return factory_.CreateApiManager(std::move(env), service_config,
-                                   server_config);
+    std::unique_ptr<ApiManagerEnvInterface> env, const char *server_config) {
+  return factory_.CreateApiManager(std::move(env), server_config);
 }
 
 TEST_F(ApiManagerTest, EnvironmentLogging) {
@@ -179,14 +169,27 @@ TEST_F(ApiManagerTest, EnvironmentLogging) {
   env.LogError("error log");
 }
 
-TEST_F(ApiManagerTest, CorrectStatistics) {
+TEST_F(ApiManagerTest, InvalidServerConfig) {
   std::unique_ptr<ApiManagerEnvInterface> env(
       new ::testing::NiceMock<MockApiManagerEnvironment>());
 
   std::shared_ptr<ApiManagerImpl> api_manager(
       std::dynamic_pointer_cast<ApiManagerImpl>(
           MakeApiManager(std::move(env), kServiceForStatistics)));
   EXPECT_TRUE(api_manager);
+  EXPECT_FALSE(api_manager->Enabled());
+  api_manager->Init();
+  EXPECT_FALSE(api_manager->Enabled());
+}
+
+TEST_F(ApiManagerTest, CorrectStatistics) {
+  std::unique_ptr<ApiManagerEnvInterface> env(
+      new ::testing::NiceMock<MockApiManagerEnvironment>());
+
+  std::shared_ptr<ApiManagerImpl> api_manager(
+      std::dynamic_pointer_cast<ApiManagerImpl>(MakeApiManager(
+          std::move(env), kServerConfigWithSingleServiceConfig)));
+  EXPECT_TRUE(api_manager);
   EXPECT_TRUE(api_manager->Enabled());
   api_manager->Init();
   ApiManagerStatistics statistics;
@@ -209,9 +212,8 @@ TEST_F(ApiManagerTest, InitializedOnApiManagerInstanceCreation) {
   EXPECT_CALL(*(env.get()), DoRunHTTPRequest(_)).Times(0);
 
   std::shared_ptr<ApiManagerImpl> api_manager(
-      std::dynamic_pointer_cast<ApiManagerImpl>(
-          MakeApiManager(std::move(env), kServiceConfig1,
-                         kServerConfigWithSingleServiceConfig)));
+      std::dynamic_pointer_cast<ApiManagerImpl>(MakeApiManager(
+          std::move(env), kServerConfigWithSingleServiceConfig)));
 
   EXPECT_TRUE(api_manager);
   EXPECT_TRUE(api_manager->Enabled());
@@ -238,7 +240,7 @@ TEST_F(ApiManagerTest, InitializedByConfigManager) {
 
   std::shared_ptr<ApiManagerImpl> api_manager(
       std::dynamic_pointer_cast<ApiManagerImpl>(MakeApiManager(
-          std::move(env), "", kServerConfigWithSingleServiceConfig)));
+          std::move(env), kServerConfigWithSingleServiceConfig)));
 
   EXPECT_TRUE(api_manager);
   EXPECT_TRUE(api_manager->Enabled());
@@ -262,7 +264,7 @@ TEST_F(ApiManagerTest, kServerConfigWithPartialServiceConfig) {
 
   std::shared_ptr<ApiManagerImpl> api_manager(
       std::dynamic_pointer_cast<ApiManagerImpl>(MakeApiManager(
-          std::move(env), "", kServerConfigWithPartialServiceConfig)));
+          std::move(env), kServerConfigWithPartialServiceConfig)));
 
   EXPECT_TRUE(api_manager);
   EXPECT_TRUE(api_manager->Enabled());
@@ -294,7 +296,7 @@ TEST_F(ApiManagerTest, kServerConfigWithInvaludServiceConfig) {
 
   std::shared_ptr<ApiManagerImpl> api_manager(
       std::dynamic_pointer_cast<ApiManagerImpl>(MakeApiManager(
-          std::move(env), "", kServerConfigWithPartialServiceConfigFailed)));
+          std::move(env), kServerConfigWithPartialServiceConfigFailed)));
 
   EXPECT_TRUE(api_manager);
   EXPECT_FALSE(api_manager->Enabled());
@@ -309,8 +311,8 @@ TEST_F(ApiManagerTest, kServerConfigServiceConfigNotSpecifed) {
       new ::testing::NiceMock<MockApiManagerEnvironment>());
 
   std::shared_ptr<ApiManagerImpl> api_manager(
-      std::dynamic_pointer_cast<ApiManagerImpl>(MakeApiManager(
-          std::move(env), "", kServerConfigWithNoServiceConfig)));
+      std::dynamic_pointer_cast<ApiManagerImpl>(
+          MakeApiManager(std::move(env), kServerConfigWithNoServiceConfig)));
 
   EXPECT_TRUE(api_manager);
   EXPECT_FALSE(api_manager->Enabled());