Merge pull request #21374 from markdroth/xds_bootstrap_server

Support multiple xds servers in bootstrap file.

src/core/ext/filters/client_channel/xds/xds_bootstrap.cc

@@ -58,23 +58,23 @@ XdsBootstrap::XdsBootstrap(grpc_slice contents, grpc_error** error)
     return;
   }
   InlinedVector<grpc_error*, 1> error_list;
-  bool seen_xds_server = false;
+  bool seen_xds_servers = false;
   bool seen_node = false;
   for (grpc_json* child = tree_->child; child != nullptr; child = child->next) {
     if (child->key == nullptr) {
       error_list.push_back(
           GRPC_ERROR_CREATE_FROM_STATIC_STRING("JSON key is null"));
-    } else if (strcmp(child->key, "xds_server") == 0) {
-      if (child->type != GRPC_JSON_OBJECT) {
+    } else if (strcmp(child->key, "xds_servers") == 0) {
+      if (child->type != GRPC_JSON_ARRAY) {
         error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING(
-            "\"xds_server\" field is not an object"));
+            "\"xds_servers\" field is not an array"));
       }
-      if (seen_xds_server) {
+      if (seen_xds_servers) {
         error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING(
-            "duplicate \"xds_server\" field"));
+            "duplicate \"xds_servers\" field"));
       }
-      seen_xds_server = true;
-      grpc_error* parse_error = ParseXdsServer(child);
+      seen_xds_servers = true;
+      grpc_error* parse_error = ParseXdsServerList(child);
       if (parse_error != GRPC_ERROR_NONE) error_list.push_back(parse_error);
     } else if (strcmp(child->key, "node") == 0) {
       if (child->type != GRPC_JSON_OBJECT) {
@@ -90,9 +90,9 @@ XdsBootstrap::XdsBootstrap(grpc_slice contents, grpc_error** error)
       if (parse_error != GRPC_ERROR_NONE) error_list.push_back(parse_error);
     }
   }
-  if (!seen_xds_server) {
+  if (!seen_xds_servers) {
     error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING(
-        "\"xds_server\" field not present"));
+        "\"xds_servers\" field not present"));
   }
   *error = GRPC_ERROR_CREATE_FROM_VECTOR("errors parsing xds bootstrap file",
                                          &error_list);
@@ -103,9 +103,33 @@ XdsBootstrap::~XdsBootstrap() {
   grpc_slice_unref_internal(contents_);
 }
 
-grpc_error* XdsBootstrap::ParseXdsServer(grpc_json* json) {
+grpc_error* XdsBootstrap::ParseXdsServerList(grpc_json* json) {
   InlinedVector<grpc_error*, 1> error_list;
-  server_uri_ = nullptr;
+  size_t idx = 0;
+  for (grpc_json *child = json->child; child != nullptr;
+       child = child->next, ++idx) {
+    if (child->key != nullptr) {
+      char* msg;
+      gpr_asprintf(&msg, "array element %" PRIuPTR " key is not null", idx);
+      error_list.push_back(GRPC_ERROR_CREATE_FROM_COPIED_STRING(msg));
+    }
+    if (child->type != GRPC_JSON_OBJECT) {
+      char* msg;
+      gpr_asprintf(&msg, "array element %" PRIuPTR " is not an object", idx);
+      error_list.push_back(GRPC_ERROR_CREATE_FROM_COPIED_STRING(msg));
+    } else {
+      grpc_error* parse_error = ParseXdsServer(child, idx);
+      if (parse_error != GRPC_ERROR_NONE) error_list.push_back(parse_error);
+    }
+  }
+  return GRPC_ERROR_CREATE_FROM_VECTOR("errors parsing \"xds_servers\" array",
+                                       &error_list);
+}
+
+grpc_error* XdsBootstrap::ParseXdsServer(grpc_json* json, size_t idx) {
+  InlinedVector<grpc_error*, 1> error_list;
+  servers_.emplace_back();
+  XdsServer& server = servers_[servers_.size() - 1];
   bool seen_channel_creds = false;
   for (grpc_json* child = json->child; child != nullptr; child = child->next) {
     if (child->key == nullptr) {
@@ -116,11 +140,11 @@ grpc_error* XdsBootstrap::ParseXdsServer(grpc_json* json) {
         error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING(
             "\"server_uri\" field is not a string"));
       }
-      if (server_uri_ != nullptr) {
+      if (server.server_uri != nullptr) {
         error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING(
             "duplicate \"server_uri\" field"));
       }
-      server_uri_ = child->value;
+      server.server_uri = child->value;
     } else if (strcmp(child->key, "channel_creds") == 0) {
       if (child->type != GRPC_JSON_ARRAY) {
         error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING(
@@ -131,19 +155,29 @@ grpc_error* XdsBootstrap::ParseXdsServer(grpc_json* json) {
             "duplicate \"channel_creds\" field"));
       }
       seen_channel_creds = true;
-      grpc_error* parse_error = ParseChannelCredsArray(child);
+      grpc_error* parse_error = ParseChannelCredsArray(child, &server);
       if (parse_error != GRPC_ERROR_NONE) error_list.push_back(parse_error);
     }
   }
-  if (server_uri_ == nullptr) {
+  if (server.server_uri == nullptr) {
     error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING(
         "\"server_uri\" field not present"));
   }
-  return GRPC_ERROR_CREATE_FROM_VECTOR("errors parsing \"xds_server\" object",
-                                       &error_list);
+  // Can't use GRPC_ERROR_CREATE_FROM_VECTOR() here, because the error
+  // string is not static in this case.
+  if (error_list.empty()) return GRPC_ERROR_NONE;
+  char* msg;
+  gpr_asprintf(&msg, "errors parsing index %" PRIuPTR, idx);
+  grpc_error* error = GRPC_ERROR_CREATE_FROM_COPIED_STRING(msg);
+  gpr_free(msg);
+  for (size_t i = 0; i < error_list.size(); ++i) {
+    error = grpc_error_add_child(error, error_list[i]);
+  }
+  return error;
 }
 
-grpc_error* XdsBootstrap::ParseChannelCredsArray(grpc_json* json) {
+grpc_error* XdsBootstrap::ParseChannelCredsArray(grpc_json* json,
+                                                 XdsServer* server) {
   InlinedVector<grpc_error*, 1> error_list;
   size_t idx = 0;
   for (grpc_json *child = json->child; child != nullptr;
@@ -158,7 +192,7 @@ grpc_error* XdsBootstrap::ParseChannelCredsArray(grpc_json* json) {
       gpr_asprintf(&msg, "array element %" PRIuPTR " is not an object", idx);
       error_list.push_back(GRPC_ERROR_CREATE_FROM_COPIED_STRING(msg));
     } else {
-      grpc_error* parse_error = ParseChannelCreds(child, idx);
+      grpc_error* parse_error = ParseChannelCreds(child, idx, server);
       if (parse_error != GRPC_ERROR_NONE) error_list.push_back(parse_error);
     }
   }
@@ -166,7 +200,8 @@ grpc_error* XdsBootstrap::ParseChannelCredsArray(grpc_json* json) {
                                        &error_list);
 }
 
-grpc_error* XdsBootstrap::ParseChannelCreds(grpc_json* json, size_t idx) {
+grpc_error* XdsBootstrap::ParseChannelCreds(grpc_json* json, size_t idx,
+                                            XdsServer* server) {
   InlinedVector<grpc_error*, 1> error_list;
   ChannelCreds channel_creds;
   for (grpc_json* child = json->child; child != nullptr; child = child->next) {
@@ -195,7 +230,9 @@ grpc_error* XdsBootstrap::ParseChannelCreds(grpc_json* json, size_t idx) {
       channel_creds.config = child;
     }
   }
-  if (channel_creds.type != nullptr) channel_creds_.push_back(channel_creds);
+  if (channel_creds.type != nullptr) {
+    server->channel_creds.push_back(channel_creds);
+  }
   // Can't use GRPC_ERROR_CREATE_FROM_VECTOR() here, because the error
   // string is not static in this case.
   if (error_list.empty()) return GRPC_ERROR_NONE;

src/core/ext/filters/client_channel/xds/xds_bootstrap.h

@@ -58,6 +58,11 @@ class XdsBootstrap {
     grpc_json* config = nullptr;
   };
 
+  struct XdsServer {
+    const char* server_uri = nullptr;
+    InlinedVector<ChannelCreds, 1> channel_creds;
+  };
+
   // If *error is not GRPC_ERROR_NONE after returning, then there was an
   // error reading the file.
   static std::unique_ptr<XdsBootstrap> ReadFromFile(grpc_error** error);
@@ -66,16 +71,16 @@ class XdsBootstrap {
   XdsBootstrap(grpc_slice contents, grpc_error** error);
   ~XdsBootstrap();
 
-  const char* server_uri() const { return server_uri_; }
-  const InlinedVector<ChannelCreds, 1>& channel_creds() const {
-    return channel_creds_;
-  }
+  // TODO(roth): We currently support only one server. Fix this when we
+  // add support for fallback for the xds channel.
+  const XdsServer& server() const { return servers_[0]; }
   const Node* node() const { return node_.get(); }
 
  private:
-  grpc_error* ParseXdsServer(grpc_json* json);
-  grpc_error* ParseChannelCredsArray(grpc_json* json);
-  grpc_error* ParseChannelCreds(grpc_json* json, size_t idx);
+  grpc_error* ParseXdsServerList(grpc_json* json);
+  grpc_error* ParseXdsServer(grpc_json* json, size_t idx);
+  grpc_error* ParseChannelCredsArray(grpc_json* json, XdsServer* server);
+  grpc_error* ParseChannelCreds(grpc_json* json, size_t idx, XdsServer* server);
   grpc_error* ParseNode(grpc_json* json);
   grpc_error* ParseLocality(grpc_json* json);
 
@@ -90,8 +95,7 @@ class XdsBootstrap {
   grpc_slice contents_;
   grpc_json* tree_ = nullptr;
 
-  const char* server_uri_ = nullptr;
-  InlinedVector<ChannelCreds, 1> channel_creds_;
+  InlinedVector<XdsServer, 1> servers_;
   std::unique_ptr<Node> node_;
 };
 

src/core/ext/filters/client_channel/xds/xds_channel.cc

@@ -30,8 +30,9 @@ grpc_channel_args* ModifyXdsChannelArgs(grpc_channel_args* args) {
 
 grpc_channel* CreateXdsChannel(const XdsBootstrap& bootstrap,
                                const grpc_channel_args& args) {
-  if (!bootstrap.channel_creds().empty()) return nullptr;
-  return grpc_insecure_channel_create(bootstrap.server_uri(), &args, nullptr);
+  if (!bootstrap.server().channel_creds.empty()) return nullptr;
+  return grpc_insecure_channel_create(bootstrap.server().server_uri, &args,
+                                      nullptr);
 }
 
 }  // namespace grpc_core

src/core/ext/filters/client_channel/xds/xds_channel_secure.cc

@@ -67,12 +67,14 @@ grpc_channel* CreateXdsChannel(const XdsBootstrap& bootstrap,
                                const grpc_channel_args& args) {
   grpc_channel_credentials* creds = nullptr;
   RefCountedPtr<grpc_channel_credentials> creds_to_unref;
-  if (!bootstrap.channel_creds().empty()) {
-    for (size_t i = 0; i < bootstrap.channel_creds().size(); ++i) {
-      if (strcmp(bootstrap.channel_creds()[i].type, "google_default") == 0) {
+  if (!bootstrap.server().channel_creds.empty()) {
+    for (size_t i = 0; i < bootstrap.server().channel_creds.size(); ++i) {
+      if (strcmp(bootstrap.server().channel_creds[i].type, "google_default") ==
+          0) {
         creds = grpc_google_default_credentials_create();
         break;
-      } else if (strcmp(bootstrap.channel_creds()[i].type, "fake") == 0) {
+      } else if (strcmp(bootstrap.server().channel_creds[i].type, "fake") ==
+                 0) {
         creds = grpc_fake_transport_security_credentials_create();
         break;
       }
@@ -83,7 +85,7 @@ grpc_channel* CreateXdsChannel(const XdsBootstrap& bootstrap,
     creds = grpc_channel_credentials_find_in_args(&args);
     if (creds == nullptr) {
       // Built with security but parent channel is insecure.
-      return grpc_insecure_channel_create(bootstrap.server_uri(), &args,
+      return grpc_insecure_channel_create(bootstrap.server().server_uri, &args,
                                           nullptr);
     }
   }
@@ -91,7 +93,7 @@ grpc_channel* CreateXdsChannel(const XdsBootstrap& bootstrap,
   grpc_channel_args* new_args =
       grpc_channel_args_copy_and_remove(&args, &arg_to_remove, 1);
   grpc_channel* channel = grpc_secure_channel_create(
-      creds, bootstrap.server_uri(), new_args, nullptr);
+      creds, bootstrap.server().server_uri, new_args, nullptr);
   grpc_channel_args_destroy(new_args);
   return channel;
 }

src/core/ext/filters/client_channel/xds/xds_client.cc

@@ -1268,7 +1268,7 @@ XdsClient::XdsClient(Combiner* combiner, grpc_pollset_set* interested_parties,
   }
   if (GRPC_TRACE_FLAG_ENABLED(grpc_xds_client_trace)) {
     gpr_log(GPR_INFO, "[xds_client %p: creating channel to %s", this,
-            bootstrap_->server_uri());
+            bootstrap_->server().server_uri);
   }
   chand_ = MakeOrphanable<ChannelState>(
       Ref(DEBUG_LOCATION, "XdsClient+ChannelState"), channel_args);

test/core/client_channel/xds_bootstrap_test.cc

@@ -38,16 +38,28 @@ void VerifyRegexMatch(grpc_error* error, const std::regex& e) {
 TEST(XdsBootstrapTest, Basic) {
   const char* json =
       "{"
-      "  \"xds_server\": {"
-      "    \"server_uri\": \"fake:///lb\","
-      "    \"channel_creds\": ["
-      "      {"
-      "        \"type\": \"fake\","
-      "        \"ignore\": 0"
-      "      }"
-      "    ],"
-      "    \"ignore\": 0"
-      "  },"
+      "  \"xds_servers\": ["
+      "    {"
+      "      \"server_uri\": \"fake:///lb\","
+      "      \"channel_creds\": ["
+      "        {"
+      "          \"type\": \"fake\","
+      "          \"ignore\": 0"
+      "        }"
+      "      ],"
+      "      \"ignore\": 0"
+      "    },"
+      "    {"
+      "      \"server_uri\": \"ignored\","
+      "      \"channel_creds\": ["
+      "        {"
+      "          \"type\": \"ignored\","
+      "          \"ignore\": 0"
+      "        }"
+      "      ],"
+      "      \"ignore\": 0"
+      "    }"
+      "  ],"
       "  \"node\": {"
       "    \"id\": \"foo\","
       "    \"cluster\": \"bar\","
@@ -74,11 +86,11 @@ TEST(XdsBootstrapTest, Basic) {
   grpc_slice slice = grpc_slice_from_copied_string(json);
   grpc_error* error = GRPC_ERROR_NONE;
   grpc_core::XdsBootstrap bootstrap(slice, &error);
-  EXPECT_EQ(error, GRPC_ERROR_NONE);
-  EXPECT_STREQ(bootstrap.server_uri(), "fake:///lb");
-  ASSERT_EQ(bootstrap.channel_creds().size(), 1);
-  EXPECT_STREQ(bootstrap.channel_creds()[0].type, "fake");
-  EXPECT_EQ(bootstrap.channel_creds()[0].config, nullptr);
+  EXPECT_EQ(error, GRPC_ERROR_NONE) << grpc_error_string(error);
+  EXPECT_STREQ(bootstrap.server().server_uri, "fake:///lb");
+  ASSERT_EQ(bootstrap.server().channel_creds.size(), 1);
+  EXPECT_STREQ(bootstrap.server().channel_creds[0].type, "fake");
+  EXPECT_EQ(bootstrap.server().channel_creds[0].config, nullptr);
   ASSERT_NE(bootstrap.node(), nullptr);
   EXPECT_STREQ(bootstrap.node()->id, "foo");
   EXPECT_STREQ(bootstrap.node()->cluster, "bar");
@@ -152,16 +164,18 @@ TEST(XdsBootstrapTest, Basic) {
 TEST(XdsBootstrapTest, ValidWithoutChannelCredsAndNode) {
   const char* json =
       "{"
-      "  \"xds_server\": {"
-      "    \"server_uri\": \"fake:///lb\""
-      "  }"
+      "  \"xds_servers\": ["
+      "    {"
+      "      \"server_uri\": \"fake:///lb\""
+      "    }"
+      "  ]"
       "}";
   grpc_slice slice = grpc_slice_from_copied_string(json);
   grpc_error* error = GRPC_ERROR_NONE;
   grpc_core::XdsBootstrap bootstrap(slice, &error);
   EXPECT_EQ(error, GRPC_ERROR_NONE);
-  EXPECT_STREQ(bootstrap.server_uri(), "fake:///lb");
-  EXPECT_EQ(bootstrap.channel_creds().size(), 0);
+  EXPECT_STREQ(bootstrap.server().server_uri, "fake:///lb");
+  EXPECT_EQ(bootstrap.server().channel_creds.size(), 0);
   EXPECT_EQ(bootstrap.node(), nullptr);
 }
 
@@ -185,30 +199,31 @@ TEST(XdsBootstrapTest, MalformedJson) {
   VerifyRegexMatch(error, e);
 }
 
-TEST(XdsBootstrapTest, MissingXdsServer) {
+TEST(XdsBootstrapTest, MissingXdsServers) {
   grpc_slice slice = grpc_slice_from_copied_string("{}");
   grpc_error* error = GRPC_ERROR_NONE;
   grpc_core::XdsBootstrap bootstrap(slice, &error);
   gpr_log(GPR_ERROR, "%s", grpc_error_string(error));
   ASSERT_TRUE(error != GRPC_ERROR_NONE);
-  std::regex e(std::string("\"xds_server\" field not present"));
+  std::regex e(std::string("\"xds_servers\" field not present"));
   VerifyRegexMatch(error, e);
 }
 
-TEST(XdsBootstrapTest, BadXdsServer) {
+TEST(XdsBootstrapTest, BadXdsServers) {
   grpc_slice slice = grpc_slice_from_copied_string(
       "{"
-      "  \"xds_server\":1,"
-      "  \"xds_server\":{}"
+      "  \"xds_servers\":1,"
+      "  \"xds_servers\":[{}]"
       "}");
   grpc_error* error = GRPC_ERROR_NONE;
   grpc_core::XdsBootstrap bootstrap(slice, &error);
   gpr_log(GPR_ERROR, "%s", grpc_error_string(error));
   ASSERT_TRUE(error != GRPC_ERROR_NONE);
   std::regex e(
-      std::string("\"xds_server\" field is not an object(.*)"
-                  "duplicate \"xds_server\" field(.*)"
-                  "errors parsing \"xds_server\" object(.*)"
+      std::string("\"xds_servers\" field is not an array(.*)"
+                  "duplicate \"xds_servers\" field(.*)"
+                  "errors parsing \"xds_servers\" array(.*)"
+                  "errors parsing index 0(.*)"
                   "\"server_uri\" field not present"));
   VerifyRegexMatch(error, e);
 }
@@ -216,19 +231,22 @@ TEST(XdsBootstrapTest, BadXdsServer) {
 TEST(XdsBootstrapTest, BadXdsServerContents) {
   grpc_slice slice = grpc_slice_from_copied_string(
       "{"
-      "  \"xds_server\":{"
-      "    \"server_uri\":1,"
-      "    \"server_uri\":\"foo\","
-      "    \"channel_creds\":1,"
-      "    \"channel_creds\":{}"
-      "  }"
+      "  \"xds_servers\":["
+      "    {"
+      "      \"server_uri\":1,"
+      "      \"server_uri\":\"foo\","
+      "      \"channel_creds\":1,"
+      "      \"channel_creds\":{}"
+      "    }"
+      "  ]"
       "}");
   grpc_error* error = GRPC_ERROR_NONE;
   grpc_core::XdsBootstrap bootstrap(slice, &error);
   gpr_log(GPR_ERROR, "%s", grpc_error_string(error));
   ASSERT_TRUE(error != GRPC_ERROR_NONE);
   std::regex e(
-      std::string("errors parsing \"xds_server\" object(.*)"
+      std::string("errors parsing \"xds_servers\" array(.*)"
+                  "errors parsing index 0(.*)"
                   "\"server_uri\" field is not a string(.*)"
                   "duplicate \"server_uri\" field(.*)"
                   "\"channel_creds\" field is not an array(.*)"
@@ -240,24 +258,27 @@ TEST(XdsBootstrapTest, BadXdsServerContents) {
 TEST(XdsBootstrapTest, BadChannelCredsContents) {
   grpc_slice slice = grpc_slice_from_copied_string(
       "{"
-      "  \"xds_server\":{"
-      "    \"server_uri\":\"foo\","
-      "    \"channel_creds\":["
-      "      {"
-      "        \"type\":0,"
-      "        \"type\":\"fake\","
-      "        \"config\":1,"
-      "        \"config\":{}"
-      "      }"
-      "    ]"
-      "  }"
+      "  \"xds_servers\":["
+      "    {"
+      "      \"server_uri\":\"foo\","
+      "      \"channel_creds\":["
+      "        {"
+      "          \"type\":0,"
+      "          \"type\":\"fake\","
+      "          \"config\":1,"
+      "          \"config\":{}"
+      "        }"
+      "      ]"
+      "    }"
+      "  ]"
       "}");
   grpc_error* error = GRPC_ERROR_NONE;
   grpc_core::XdsBootstrap bootstrap(slice, &error);
   gpr_log(GPR_ERROR, "%s", grpc_error_string(error));
   ASSERT_TRUE(error != GRPC_ERROR_NONE);
   std::regex e(
-      std::string("errors parsing \"xds_server\" object(.*)"
+      std::string("errors parsing \"xds_servers\" array(.*)"
+                  "errors parsing index 0(.*)"
                   "errors parsing \"channel_creds\" array(.*)"
                   "errors parsing index 0(.*)"
                   "\"type\" field is not a string(.*)"

test/cpp/end2end/xds_end2end_test.cc

@@ -105,14 +105,16 @@ constexpr int kDefaultLocalityPriority = 0;
 
 constexpr char kBootstrapFile[] =
     "{\n"
-    "  \"xds_server\": {\n"
-    "    \"server_uri\": \"fake:///lb\",\n"
-    "    \"channel_creds\": [\n"
-    "      {\n"
-    "        \"type\": \"fake\"\n"
-    "      }\n"
-    "    ]\n"
-    "  },\n"
+    "  \"xds_servers\": [\n"
+    "    {\n"
+    "      \"server_uri\": \"fake:///lb\",\n"
+    "      \"channel_creds\": [\n"
+    "        {\n"
+    "          \"type\": \"fake\"\n"
+    "        }\n"
+    "      ]\n"
+    "    }\n"
+    "  ],\n"
     "  \"node\": {\n"
     "    \"id\": \"xds_end2end_test\",\n"
     "    \"cluster\": \"test\",\n"
@@ -129,14 +131,16 @@ constexpr char kBootstrapFile[] =
 
 constexpr char kBootstrapFileBad[] =
     "{\n"
-    "  \"xds_server\": {\n"
-    "    \"server_uri\": \"fake:///wrong_lb\",\n"
-    "    \"channel_creds\": [\n"
-    "      {\n"
-    "        \"type\": \"fake\"\n"
-    "      }\n"
-    "    ]\n"
-    "  },\n"
+    "  \"xds_servers\": [\n"
+    "    {\n"
+    "      \"server_uri\": \"fake:///wrong_lb\",\n"
+    "      \"channel_creds\": [\n"
+    "        {\n"
+    "          \"type\": \"fake\"\n"
+    "        }\n"
+    "      ]\n"
+    "    }\n"
+    "  ],\n"
     "  \"node\": {\n"
     "  }\n"
     "}\n";