Skip to content

Commit ca8572e

Browse files
jmillikin-stripemattklein123
authored andcommitted
Support bootstrap configs in text proto format, and improve errors. (#1593)
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.
1 parent b2114cd commit ca8572e

File tree

2 files changed

+39
-3
lines changed

2 files changed

+39
-3
lines changed

source/common/protobuf/utility.cc

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,22 @@ void MessageUtil::loadFromJson(const std::string& json, Protobuf::Message& messa
2323

2424
void MessageUtil::loadFromFile(const std::string& path, Protobuf::Message& message) {
2525
const std::string contents = Filesystem::fileReadToEnd(path);
26-
// If the filename ends with .pb, do a best-effort attempt to parse it as a proto.
26+
// If the filename ends with .pb, attempt to parse it as a binary proto.
2727
if (StringUtil::endsWith(path, ".pb")) {
2828
// Attempt to parse the binary format.
2929
if (message.ParseFromString(contents)) {
3030
return;
3131
}
32-
throw EnvoyException("Unable to parse proto " + path);
32+
throw EnvoyException("Unable to parse file \"" + path + "\" as a binary protobuf (type " +
33+
message.GetTypeName() + ")");
34+
}
35+
// If the filename ends with .pb_text, attempt to parse it as a text proto.
36+
if (StringUtil::endsWith(path, ".pb_text")) {
37+
if (Protobuf::TextFormat::ParseFromString(contents, &message)) {
38+
return;
39+
}
40+
throw EnvoyException("Unable to parse file \"" + path + "\" as a text protobuf (type " +
41+
message.GetTypeName() + ")");
3342
}
3443
if (StringUtil::endsWith(path, ".yaml")) {
3544
const std::string json = Json::Factory::loadFromYamlString(contents)->asJsonString();

test/common/protobuf/utility_test.cc

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88

99
namespace Envoy {
1010

11-
TEST(UtilityTest, LoadProtoFromFile) {
11+
TEST(UtilityTest, LoadBinaryProtoFromFile) {
1212
envoy::api::v2::Bootstrap bootstrap;
1313
bootstrap.mutable_cluster_manager()
1414
->mutable_upstream_bind_config()
@@ -23,4 +23,31 @@ TEST(UtilityTest, LoadProtoFromFile) {
2323
EXPECT_TRUE(TestUtility::protoEqual(bootstrap, proto_from_file));
2424
}
2525

26+
TEST(UtilityTest, LoadTextProtoFromFile) {
27+
envoy::api::v2::Bootstrap bootstrap;
28+
bootstrap.mutable_cluster_manager()
29+
->mutable_upstream_bind_config()
30+
->mutable_source_address()
31+
->set_address("1.1.1.1");
32+
33+
std::string bootstrap_text;
34+
ASSERT_TRUE(Protobuf::TextFormat::PrintToString(bootstrap, &bootstrap_text));
35+
const std::string filename =
36+
TestEnvironment::writeStringToFileForTest("proto.pb_text", bootstrap_text);
37+
38+
envoy::api::v2::Bootstrap proto_from_file;
39+
MessageUtil::loadFromFile(filename, proto_from_file);
40+
EXPECT_TRUE(TestUtility::protoEqual(bootstrap, proto_from_file));
41+
}
42+
43+
TEST(UtilityTest, LoadTextProtoFromFile_Failure) {
44+
const std::string filename =
45+
TestEnvironment::writeStringToFileForTest("proto.pb_text", "invalid {");
46+
47+
envoy::api::v2::Bootstrap proto_from_file;
48+
EXPECT_THROW_WITH_MESSAGE(MessageUtil::loadFromFile(filename, proto_from_file), EnvoyException,
49+
"Unable to parse file \"" + filename +
50+
"\" as a text protobuf (type envoy.api.v2.Bootstrap)");
51+
}
52+
2653
} // namespace Envoy

0 commit comments

Comments
 (0)