From 1e73f1053e589224d69cb80f41bc2e3f7c82d847 Mon Sep 17 00:00:00 2001 From: Pelinthecoder Date: Thu, 13 Feb 2025 10:25:25 -0500 Subject: [PATCH] Ensure request body is correctly propagated in Nighthawk Request Source. (#1289) * Ensure request body is correctly propagated in Nighthawk Request Source. Previously, requests generated by `OptionsListRequestSource` did not correctly set the request body, leading to empty payloads when using `json_body`. This CL ensures that `json_body` is correctly propagated from `RequestOptionsList` to `RequestImpl`, aligning request behavior with expectations. ### Changes: - **Updated Request interface (`request.h`)**: Added `body()` as a pure virtual method. - **Modified `RequestImpl` (`request_impl.h` & `request_impl.cc`)**: - Updated the constructor to accept `json_body_` as an argument. - Implemented `body()` to return `json_body_` instead of an empty string. - **Fixed `OptionsListRequestSource::get()`**: - Now correctly assigns `json_body` when creating `RequestImpl`. - Ensures request bodies are set properly instead of being empty. - **Fixed `RemoteRequestSourceImpl::get()`**: - Ensures that request bodies from gRPC sources are also correctly populated. - **Updated `request_source_plugin_test.cc`**: - Modified tests to verify that `request3->body()` is correctly set. - Ensured `request3` is not `nullptr` and contains the expected `json_body`. ### **Why This Change?** Without this fix, request bodies remain empty despite being set in the configuration (`RequestOptionsList`). This caused inconsistencies in request generation. This CL ensures that the body is always populated correctly, improving reliability. ### **Testing:** - Verified that `json_body` is correctly loaded from the YAML file. - Confirmed `request3->body()` now returns the expected JSON payload. - Ran tests on Cider-V, all passed successfully. This change ensures that request generation works as expected and aligns with the intended behavior. Signed-off-by: pelinthecoder * Update request_source_impl.cc Signed-off-by: Pelinthecoder * Update request_source_plugin_test.cc Signed-off-by: Pelinthecoder * Add json body for OptionsListRequestSource::get Signed-off-by: pelinthecoder * Remove move for string value Signed-off-by: pelinthecoder * Add json body under options.proto Signed-off-by: pelinthecoder * Fixes the include statements under the test file Signed-off-by: pelinthecoder * Fixes external envoy includes in the test Signed-off-by: pelinthecoder * add semicolon Signed-off-by: pelinthecoder * modify request body size to account for json body Signed-off-by: pelinthecoder * modifying the request body size again Signed-off-by: pelinthecoder * Fix test for json body Signed-off-by: pelinthecoder * test option request body size to reflect json length Signed-off-by: pelinthecoder * fixing the format Signed-off-by: pelinthecoder * Added tests for json bodies Signed-off-by: pelinthecoder * fix header import order to satisfy lint Signed-off-by: pelinthecoder * Recovered the working request_source_impl, added tests Signed-off-by: pelinthecoder * Fixes format Signed-off-by: pelinthecoder --------- Signed-off-by: pelinthecoder Signed-off-by: Pelinthecoder --- include/nighthawk/common/request.h | 1 + source/common/request_impl.h | 9 +++++++- .../request_options_list_plugin_impl.cc | 4 +++- .../request_source_plugin_test.cc | 22 +++++++++++++++++++ .../test_data/test-config-ab.yaml | 6 +++-- 5 files changed, 38 insertions(+), 4 deletions(-) diff --git a/include/nighthawk/common/request.h b/include/nighthawk/common/request.h index d123b8736..1a5f16134 100644 --- a/include/nighthawk/common/request.h +++ b/include/nighthawk/common/request.h @@ -20,6 +20,7 @@ class Request { * @return HeaderMapPtr shared pointer to a request header specification. */ virtual HeaderMapPtr header() const PURE; + virtual const std::string& body() const PURE; // TODO(oschaaf): expectations }; diff --git a/source/common/request_impl.h b/source/common/request_impl.h index 930175dc5..9cc2742f3 100644 --- a/source/common/request_impl.h +++ b/source/common/request_impl.h @@ -1,5 +1,8 @@ #pragma once +#include +#include + #include "envoy/http/header_map.h" #include "nighthawk/common/request.h" @@ -8,11 +11,15 @@ namespace Nighthawk { class RequestImpl : public Request { public: - RequestImpl(HeaderMapPtr header) : header_(std::move(header)) {} + RequestImpl(HeaderMapPtr header, std::string json_body = "") + : header_(std::move(header)), json_body_(json_body) {} + HeaderMapPtr header() const override { return header_; } + const std::string& body() const override { return json_body_; } private: HeaderMapPtr header_; + std::string json_body_; }; } // namespace Nighthawk diff --git a/source/request_source/request_options_list_plugin_impl.cc b/source/request_source/request_options_list_plugin_impl.cc index 59c1a96a6..6e7539fc4 100644 --- a/source/request_source/request_options_list_plugin_impl.cc +++ b/source/request_source/request_options_list_plugin_impl.cc @@ -1,5 +1,7 @@ #include "source/request_source/request_options_list_plugin_impl.h" +#include + #include "external/envoy/source/common/protobuf/message_validator_impl.h" #include "external/envoy/source/common/protobuf/protobuf.h" #include "external/envoy/source/common/protobuf/utility.h" @@ -109,7 +111,7 @@ RequestGenerator OptionsListRequestSource::get() { auto lower_case_key = Envoy::Http::LowerCaseString(std::string(option_header.header().key())); header->setCopy(lower_case_key, std::string(option_header.header().value())); } - return std::make_unique(std::move(header)); + return std::make_unique(std::move(header), request_option.json_body()); }; return request_generator; } diff --git a/test/request_source/request_source_plugin_test.cc b/test/request_source/request_source_plugin_test.cc index 4416f88a2..5f9957063 100644 --- a/test/request_source/request_source_plugin_test.cc +++ b/test/request_source/request_source_plugin_test.cc @@ -1,6 +1,20 @@ +#include +#include +#include + +#include "envoy/api/api.h" #include "envoy/common/exception.h" +#include "envoy/http/header_map.h" + +#include "nighthawk/common/exception.h" +#include "nighthawk/common/request.h" +#include "nighthawk/common/request_source.h" +#include "nighthawk/request_source/request_source_plugin_config_factory.h" #include "external/envoy/source/common/config/utility.h" +#include "external/envoy/source/common/http/header_map_impl.h" +#include "external/envoy/source/common/protobuf/message_validator_impl.h" +#include "external/envoy/source/common/protobuf/protobuf.h" #include "external/envoy/test/mocks/api/mocks.h" #include "external/envoy/test/mocks/stats/mocks.h" #include "external/envoy/test/test_common/file_system_for_test.h" @@ -171,6 +185,10 @@ TEST_F(FileBasedRequestSourcePluginTest, Nighthawk::HeaderMapPtr header2 = request2->header(); EXPECT_EQ(header1->getPathValue(), "/a"); EXPECT_EQ(header2->getPathValue(), "/b"); + std::string body1 = request1->body(); + std::string body2 = request2->body(); + EXPECT_EQ(body1, R"({"message": "hello1"})"); + EXPECT_EQ(body2, R"({"message": "hello2"})"); EXPECT_EQ(request3, nullptr); } @@ -353,6 +371,10 @@ TEST_F(InLineRequestSourcePluginTest, Nighthawk::HeaderMapPtr header2 = request2->header(); EXPECT_EQ(header1->getPathValue(), "/a"); EXPECT_EQ(header2->getPathValue(), "/b"); + std::string body1 = request1->body(); + std::string body2 = request2->body(); + EXPECT_EQ(body1, R"({"message": "hello1"})"); + EXPECT_EQ(body2, R"({"message": "hello2"})"); EXPECT_EQ(request3, nullptr); } diff --git a/test/request_source/test_data/test-config-ab.yaml b/test/request_source/test_data/test-config-ab.yaml index b78bcba64..e03fdd318 100644 --- a/test/request_source/test_data/test-config-ab.yaml +++ b/test/request_source/test_data/test-config-ab.yaml @@ -1,11 +1,13 @@ options: - request_method: 1 - request_body_size: 10 + request_body_size: 23 + json_body: '{"message": "hello1"}' request_headers: - { header: { key: ":path", value: "/a" } } - { header: { key: "x-nighthawk-test-server-config", value: "{response_body_size:13}" } } - request_method: 1 - request_body_size: 10 + request_body_size: 23 + json_body: '{"message": "hello2"}' request_headers: - { header: { key: ":path", value: "/b" } } - { header: { key: "x-nighthawk-test-server-config", value: "{response_body_size:17}" } } \ No newline at end of file