Support bootstrap configs in text proto format, and improve errors. (#1593)

jmillikin-stripe · mattklein123 · commit ca8572eb6c33 · 2017-09-05T15:44:44.000-07:00
This commit allows `.pb_text` configs to be parsed as text, and improves
the parse failure error message to include (1) text or binary mode, and
(2) the message type being parsed.
diff --git a/source/common/protobuf/utility.cc b/source/common/protobuf/utility.cc
@@ -23,13 +23,22 @@ void MessageUtil::loadFromJson(const std::string& json, Protobuf::Message& messa
 
 void MessageUtil::loadFromFile(const std::string& path, Protobuf::Message& message) {
   const std::string contents = Filesystem::fileReadToEnd(path);
-  // If the filename ends with .pb, do a best-effort attempt to parse it as a proto.
+  // If the filename ends with .pb, attempt to parse it as a binary proto.
   if (StringUtil::endsWith(path, ".pb")) {
     // Attempt to parse the binary format.
     if (message.ParseFromString(contents)) {
       return;
     }
-    throw EnvoyException("Unable to parse proto " + path);
+    throw EnvoyException("Unable to parse file \"" + path + "\" as a binary protobuf (type " +
+                         message.GetTypeName() + ")");
+  }
+  // If the filename ends with .pb_text, attempt to parse it as a text proto.
+  if (StringUtil::endsWith(path, ".pb_text")) {
+    if (Protobuf::TextFormat::ParseFromString(contents, &message)) {
+      return;
+    }
+    throw EnvoyException("Unable to parse file \"" + path + "\" as a text protobuf (type " +
+                         message.GetTypeName() + ")");
   }
   if (StringUtil::endsWith(path, ".yaml")) {
     const std::string json = Json::Factory::loadFromYamlString(contents)->asJsonString();
diff --git a/test/common/protobuf/utility_test.cc b/test/common/protobuf/utility_test.cc
@@ -8,7 +8,7 @@
 
 namespace Envoy {
 
-TEST(UtilityTest, LoadProtoFromFile) {
+TEST(UtilityTest, LoadBinaryProtoFromFile) {
   envoy::api::v2::Bootstrap bootstrap;
   bootstrap.mutable_cluster_manager()
       ->mutable_upstream_bind_config()
@@ -23,4 +23,31 @@ TEST(UtilityTest, LoadProtoFromFile) {
   EXPECT_TRUE(TestUtility::protoEqual(bootstrap, proto_from_file));
 }
 
+TEST(UtilityTest, LoadTextProtoFromFile) {
+  envoy::api::v2::Bootstrap bootstrap;
+  bootstrap.mutable_cluster_manager()
+      ->mutable_upstream_bind_config()
+      ->mutable_source_address()
+      ->set_address("1.1.1.1");
+
+  std::string bootstrap_text;
+  ASSERT_TRUE(Protobuf::TextFormat::PrintToString(bootstrap, &bootstrap_text));
+  const std::string filename =
+      TestEnvironment::writeStringToFileForTest("proto.pb_text", bootstrap_text);
+
+  envoy::api::v2::Bootstrap proto_from_file;
+  MessageUtil::loadFromFile(filename, proto_from_file);
+  EXPECT_TRUE(TestUtility::protoEqual(bootstrap, proto_from_file));
+}
+
+TEST(UtilityTest, LoadTextProtoFromFile_Failure) {
+  const std::string filename =
+      TestEnvironment::writeStringToFileForTest("proto.pb_text", "invalid {");
+
+  envoy::api::v2::Bootstrap proto_from_file;
+  EXPECT_THROW_WITH_MESSAGE(MessageUtil::loadFromFile(filename, proto_from_file), EnvoyException,
+                            "Unable to parse file \"" + filename +
+                                "\" as a text protobuf (type envoy.api.v2.Bootstrap)");
+}
+
 } // namespace Envoy
