From 0362ec5c5c5abecf7fda0cc5365615d7840263d4 Mon Sep 17 00:00:00 2001
From: Jakub Sobon <mumak@google.com>
Date: Fri, 4 Dec 2020 14:42:02 -0500
Subject: [PATCH 1/4] Deprecating v2 primitive in request source service.proto.
Signed-off-by: Jakub Sobon <mumak@google.com>
---
api/request_source/BUILD | 1 +
api/request_source/service.proto | 18 ++++++++++++++----
2 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/api/request_source/BUILD b/api/request_source/BUILD
index 9cf50e7ff..2473d6a2e 100644
--- a/api/request_source/BUILD
+++ b/api/request_source/BUILD
@@ -13,6 +13,7 @@ api_cc_py_proto_library(
"@envoy_api//envoy/api/v2/auth:pkg",
"@envoy_api//envoy/api/v2/cluster:pkg",
"@envoy_api//envoy/api/v2/core:pkg",
+ "@envoy_api//envoy/config/core/v3:pkg",
],
)
diff --git a/api/request_source/service.proto b/api/request_source/service.proto
index 9ab934558..e1acdb135 100644
--- a/api/request_source/service.proto
+++ b/api/request_source/service.proto
@@ -2,8 +2,9 @@ syntax = "proto3";
package nighthawk.request_source;
-import "google/protobuf/wrappers.proto";
import "envoy/api/v2/core/base.proto";
+import "envoy/config/core/v3/base.proto";
+import "google/protobuf/wrappers.proto";
// Used to request a RequestStreamResponse.
message RequestStreamRequest {
@@ -37,9 +38,18 @@ message RequestSpecifier {
// Request content length. The client will transfer the number of bytes specified here for the
// request body.
google.protobuf.UInt32Value content_length = 4;
- // Request header replacements. Any existing header(s) with the same name will be removed
- // before setting.
- envoy.api.v2.core.HeaderMap headers = 5;
+
+ oneof oneof_headers {
+ // Request header replacements. Any existing header(s) with the same name will be removed
+ // before setting.
+ //
+ // Envoy deprecated its v2 API, prefer to use v3_headers instead.
+ envoy.api.v2.core.HeaderMap headers = 5 [deprecated = true];
+
+ // Request header replacements. Any existing header(s) with the same name will be removed
+ // before setting.
+ envoy.config.core.v3.HeaderMap v3_headers = 6;
+ }
// TODO(oschaaf): nice to have
// google.protobuf.StringValue sni_hostname = 10;
}
From 2a8579a1f28c842ea117daddea44fa6c64e8bd1c Mon Sep 17 00:00:00 2001
From: Jakub Sobon <mumak@google.com>
Date: Fri, 4 Dec 2020 14:43:11 -0500
Subject: [PATCH 2/4] gRPC client of request source can consume both v2 and v3
headers.
Signed-off-by: Jakub Sobon <mumak@google.com>
---
source/common/BUILD | 2 +
.../common/request_stream_grpc_client_impl.cc | 26 +++++++++--
test/BUILD | 3 ++
test/request_stream_grpc_client_test.cc | 45 ++++++++++++++++---
4 files changed, 65 insertions(+), 11 deletions(-)
diff --git a/source/common/BUILD b/source/common/BUILD
index fd8cc3701..951c2a411 100644
--- a/source/common/BUILD
+++ b/source/common/BUILD
@@ -60,6 +60,8 @@ envoy_cc_library(
"@envoy//source/common/grpc:typed_async_client_lib_with_external_headers",
"@envoy//source/common/http:header_map_lib_with_external_headers",
"@envoy//source/common/http:headers_lib_with_external_headers",
+ "@envoy_api//envoy/api/v2/core:pkg_cc_proto",
+ "@envoy_api//envoy/config/core/v3:pkg_cc_proto",
],
)
diff --git a/source/common/request_stream_grpc_client_impl.cc b/source/common/request_stream_grpc_client_impl.cc
index da0095042..31db65a69 100644
--- a/source/common/request_stream_grpc_client_impl.cc
+++ b/source/common/request_stream_grpc_client_impl.cc
@@ -2,16 +2,22 @@
#include <string>
+#include "envoy/api/v2/core/base.pb.h"
+#include "envoy/config/core/v3/base.pb.h"
#include "envoy/stats/scope.h"
#include "external/envoy/source/common/common/assert.h"
#include "external/envoy/source/common/http/header_map_impl.h"
#include "external/envoy/source/common/http/headers.h"
+#include "api/request_source/service.pb.h"
+
#include "common/request_impl.h"
namespace Nighthawk {
+using ::nighthawk::request_source::RequestSpecifier;
+
const std::string RequestStreamGrpcClientImpl::METHOD_NAME =
"nighthawk.request_source.NighthawkRequestSourceService.RequestStream";
@@ -75,15 +81,27 @@ RequestPtr ProtoRequestHelper::messageToRequest(
RequestPtr request = std::make_unique<RequestImpl>(header);
if (message.has_request_specifier()) {
- const auto& request_specifier = message.request_specifier();
- if (request_specifier.has_headers()) {
- const auto& message_request_headers = request_specifier.headers();
- for (const auto& message_header : message_request_headers.headers()) {
+ const RequestSpecifier& request_specifier = message.request_specifier();
+
+ if (request_specifier.has_v3_headers()) {
+ const envoy::config::core::v3::HeaderMap& message_request_headers =
+ request_specifier.v3_headers();
+ for (const envoy::config::core::v3::HeaderValue& message_header :
+ message_request_headers.headers()) {
+ Envoy::Http::LowerCaseString header_name(message_header.key());
+ header->remove(header_name);
+ header->addCopy(header_name, message_header.value());
+ }
+ } else if (request_specifier.has_headers()) {
+ const envoy::api::v2::core::HeaderMap& message_request_headers = request_specifier.headers();
+ for (const envoy::api::v2::core::HeaderValue& message_header :
+ message_request_headers.headers()) {
Envoy::Http::LowerCaseString header_name(message_header.key());
header->remove(header_name);
header->addCopy(header_name, message_header.value());
}
}
+
if (request_specifier.has_content_length()) {
std::string s_content_length = absl::StrCat("", request_specifier.content_length().value());
header->remove(Envoy::Http::Headers::get().ContentLength);
diff --git a/test/BUILD b/test/BUILD
index 326d971ee..92190a3c6 100644
--- a/test/BUILD
+++ b/test/BUILD
@@ -325,8 +325,11 @@ envoy_cc_test(
srcs = ["request_stream_grpc_client_test.cc"],
repository = "@envoy",
deps = [
+ "//api/request_source:grpc_request_source_service_lib",
"//source/common:request_stream_grpc_client_lib",
"@envoy//test/test_common:utility_lib",
+ "@envoy_api//envoy/api/v2/core:pkg_cc_proto",
+ "@envoy_api//envoy/config/core/v3:pkg_cc_proto",
],
)
diff --git a/test/request_stream_grpc_client_test.cc b/test/request_stream_grpc_client_test.cc
index 91661ea52..81f298eac 100644
--- a/test/request_stream_grpc_client_test.cc
+++ b/test/request_stream_grpc_client_test.cc
@@ -1,5 +1,10 @@
+#include "envoy/api/v2/core/base.pb.h"
+#include "envoy/config/core/v3/base.pb.h"
+
#include "external/envoy/test/test_common/utility.h"
+#include "api/request_source/service.pb.h"
+
#include "common/request_impl.h"
#include "common/request_stream_grpc_client_impl.h"
@@ -8,6 +13,9 @@
using namespace testing;
namespace Nighthawk {
+namespace {
+
+using ::nighthawk::request_source::RequestSpecifier;
// The grpc client itself is tested via the python based integration tests.
// It is convenient to test message translation here.
@@ -18,7 +26,9 @@ class ProtoRequestHelperTest : public Test {
// We test for equality. If we observe mismatch, we use EXPECT_EQ which is guaranteed
// to fail -- but will provide much more helpful output.
if (!Envoy::TestUtility::headerMapEqualIgnoreOrder(expected_header_, *request->header())) {
- EXPECT_EQ(expected_header_, *request->header());
+ EXPECT_EQ(expected_header_, *request->header()) << "expected headers:\n"
+ << expected_header_ << "\nactual headers:\n"
+ << *request->header() << "\n";
};
}
@@ -42,14 +52,34 @@ TEST_F(ProtoRequestHelperTest, ExplicitFields) {
translateExpectingEqual();
}
-// Test the generic header api we offer in the proto api.
-TEST_F(ProtoRequestHelperTest, GenericHeaderFields) {
- auto* request_specifier = response_.mutable_request_specifier();
- auto* headers = request_specifier->mutable_headers();
- auto* header_1 = headers->add_headers();
+// Test the generic header api we offer in the proto api using Envoy API v2
+// primitives.
+TEST_F(ProtoRequestHelperTest, GenericHeaderFieldsUsingDeprecatedEnvoyV2Api) {
+ RequestSpecifier* request_specifier = response_.mutable_request_specifier();
+ envoy::api::v2::core::HeaderMap* headers = request_specifier->mutable_headers();
+ envoy::api::v2::core::HeaderValue* header_1 = headers->add_headers();
+ header_1->set_key("header1");
+ header_1->set_value("value1");
+ envoy::api::v2::core::HeaderValue* header_2 = headers->add_headers();
+ header_2->set_key("header2");
+ header_2->set_value("value2");
+ // We re-add the same header, but do not expect that to show up in the translation because we
+ // always replace.
+ headers->add_headers()->MergeFrom(*header_2);
+ expected_header_ =
+ Envoy::Http::TestRequestHeaderMapImpl{{"header1", "value1"}, {"header2", "value2"}};
+ translateExpectingEqual();
+}
+
+// Test the generic header api we offer in the proto api using Envoy API v3
+// primitives.
+TEST_F(ProtoRequestHelperTest, GenericHeaderFieldsUsingDeprecatedEnvoyV3Api) {
+ RequestSpecifier* request_specifier = response_.mutable_request_specifier();
+ envoy::config::core::v3::HeaderMap* headers = request_specifier->mutable_v3_headers();
+ envoy::config::core::v3::HeaderValue* header_1 = headers->add_headers();
header_1->set_key("header1");
header_1->set_value("value1");
- auto* header_2 = headers->add_headers();
+ envoy::config::core::v3::HeaderValue* header_2 = headers->add_headers();
header_2->set_key("header2");
header_2->set_value("value2");
// We re-add the same header, but do not expect that to show up in the translation because we
@@ -74,4 +104,5 @@ TEST_F(ProtoRequestHelperTest, AmbiguousHost) {
translateExpectingEqual();
}
+} // namespace
} // namespace Nighthawk
From 0faf9caad9f32a280bf5b1fb6b1cd40267727b72 Mon Sep 17 00:00:00 2001
From: Jakub Sobon <mumak@google.com>
Date: Fri, 4 Dec 2020 14:54:49 -0500
Subject: [PATCH 3/4] Migrate our dummy request source to Envoy API v3
primitives.
Signed-off-by: Jakub Sobon <mumak@google.com>
---
source/client/BUILD | 1 +
source/client/service_impl.cc | 8 +++++---
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/source/client/BUILD b/source/client/BUILD
index b89c90bd7..4fe97d170 100644
--- a/source/client/BUILD
+++ b/source/client/BUILD
@@ -156,6 +156,7 @@ envoy_cc_library(
"//api/client:grpc_service_lib",
"//api/request_source:grpc_request_source_service_lib",
"@envoy//source/common/common:thread_lib_with_external_headers",
+ "@envoy_api//envoy/config/core/v3:pkg_cc_proto",
],
)
diff --git a/source/client/service_impl.cc b/source/client/service_impl.cc
index 693955a79..fda3d69d7 100644
--- a/source/client/service_impl.cc
+++ b/source/client/service_impl.cc
@@ -2,6 +2,8 @@
#include <grpc++/grpc++.h>
+#include "envoy/config/core/v3/base.pb.h"
+
#include "common/request_source_impl.h"
#include "client/client.h"
@@ -106,7 +108,7 @@ ::grpc::Status ServiceImpl::ExecutionStream(
}
namespace {
-void addHeader(envoy::api::v2::core::HeaderMap* map, absl::string_view key,
+void addHeader(envoy::config::core::v3::HeaderMap* map, absl::string_view key,
absl::string_view value) {
auto* request_header = map->add_headers();
request_header->set_key(std::string(key));
@@ -144,7 +146,7 @@ ::grpc::Status RequestSourceServiceImpl::RequestStream(
HeaderMapPtr headers = request->header();
nighthawk::request_source::RequestStreamResponse response;
auto* request_specifier = response.mutable_request_specifier();
- auto* request_headers = request_specifier->mutable_headers();
+ auto* request_headers = request_specifier->mutable_v3_headers();
headers->iterate([&request_headers](const Envoy::Http::HeaderEntry& header)
-> Envoy::Http::HeaderMap::Iterate {
addHeader(request_headers, header.key().getStringView(), header.value().getStringView());
@@ -163,4 +165,4 @@ ::grpc::Status RequestSourceServiceImpl::RequestStream(
}
} // namespace Client
-} // namespace Nighthawk
\ No newline at end of file
+} // namespace Nighthawk
From 32d51238cf10e70ae8420ac44986d9ed026abd3b Mon Sep 17 00:00:00 2001
From: Jakub Sobon <mumak@google.com>
Date: Mon, 7 Dec 2020 23:51:42 -0500
Subject: [PATCH 4/4] Removing incorrect word "deprecated" from a test case
name.
Signed-off-by: Jakub Sobon <mumak@google.com>
---
test/request_stream_grpc_client_test.cc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/test/request_stream_grpc_client_test.cc b/test/request_stream_grpc_client_test.cc
index 81f298eac..b78b32f32 100644
--- a/test/request_stream_grpc_client_test.cc
+++ b/test/request_stream_grpc_client_test.cc
@@ -73,7 +73,7 @@ TEST_F(ProtoRequestHelperTest, GenericHeaderFieldsUsingDeprecatedEnvoyV2Api) {
// Test the generic header api we offer in the proto api using Envoy API v3
// primitives.
-TEST_F(ProtoRequestHelperTest, GenericHeaderFieldsUsingDeprecatedEnvoyV3Api) {
+TEST_F(ProtoRequestHelperTest, GenericHeaderFieldsUsingEnvoyV3Api) {
RequestSpecifier* request_specifier = response_.mutable_request_specifier();
envoy::config::core::v3::HeaderMap* headers = request_specifier->mutable_v3_headers();
envoy::config::core::v3::HeaderValue* header_1 = headers->add_headers();