From 805f9507035a6b879082c811bab229f3fcc37f11 Mon Sep 17 00:00:00 2001 From: Otto van der Schaaf Date: Wed, 9 Sep 2020 18:59:22 +0200 Subject: [PATCH 1/5] Add HttpFilterIntegrationTestBase - Extracts shared needs between tested extensions. - Sanitized api, better naming. - Doc comments. - Support for testing POST request methods and entity bodies to check the alternative flow that will trigger in extensions when using that. Prelude to #512, which includes it and provides a means to see the end goal. Signed-off-by: Otto van der Schaaf --- test/server/BUILD | 15 ++- .../http_filter_integration_test_base.cc | 72 +++++++++++ .../http_filter_integration_test_base.h | 122 ++++++++++++++++++ 3 files changed, 207 insertions(+), 2 deletions(-) create mode 100644 test/server/http_filter_integration_test_base.cc create mode 100644 test/server/http_filter_integration_test_base.h diff --git a/test/server/BUILD b/test/server/BUILD index 0ea078851..68717484b 100644 --- a/test/server/BUILD +++ b/test/server/BUILD @@ -1,6 +1,7 @@ load( "@envoy//bazel:envoy_build_system.bzl", "envoy_cc_test", + "envoy_cc_test_library", "envoy_package", ) @@ -8,6 +9,16 @@ licenses(["notice"]) # Apache 2 envoy_package() +envoy_cc_test_library( + name = "http_filter_integration_test_base_lib", + srcs = ["http_filter_integration_test_base.cc"], + hdrs = ["http_filter_integration_test_base.h"], + repository = "@envoy", + deps = [ + "@envoy//test/integration:http_integration_lib", + ], +) + envoy_cc_test( name = "http_test_server_filter_integration_test", srcs = ["http_test_server_filter_integration_test.cc"], @@ -25,9 +36,9 @@ envoy_cc_test( srcs = ["http_dynamic_delay_filter_integration_test.cc"], repository = "@envoy", deps = [ + ":http_filter_integration_test_base_lib", "//source/server:http_dynamic_delay_filter_config", "@envoy//source/common/api:api_lib_with_external_headers", - "@envoy//test/integration:http_integration_lib", ], ) @@ -36,10 +47,10 @@ envoy_cc_test( srcs = ["http_time_tracking_filter_integration_test.cc"], repository = "@envoy", deps = [ + ":http_filter_integration_test_base_lib", "//source/server:http_time_tracking_filter_config", "@envoy//include/envoy/upstream:cluster_manager_interface_with_external_headers", "@envoy//source/common/api:api_lib_with_external_headers", - "@envoy//test/integration:http_integration_lib", "@envoy//test/test_common:simulated_time_system_lib", ], ) diff --git a/test/server/http_filter_integration_test_base.cc b/test/server/http_filter_integration_test_base.cc new file mode 100644 index 000000000..44c849224 --- /dev/null +++ b/test/server/http_filter_integration_test_base.cc @@ -0,0 +1,72 @@ +#include "test/server/http_filter_integration_test_base.h" + +#include "gtest/gtest.h" + +namespace Nighthawk { + +HttpFilterIntegrationTestBase::HttpFilterIntegrationTestBase( + Envoy::Network::Address::IpVersion ip_version) + : HttpIntegrationTest(Envoy::Http::CodecClient::Type::HTTP1, ip_version), + request_headers_({{":method", "GET"}, {":path", "/"}, {":authority", "host"}}) {} + +void HttpFilterIntegrationTestBase::setup(const std::string& config) { + config_helper_.addFilter(config); + HttpIntegrationTest::initialize(); +} + +Envoy::IntegrationStreamDecoderPtr HttpFilterIntegrationTestBase::getResponseFromUpstream() { + return getResponse(request_headers_, true); +} + +Envoy::IntegrationStreamDecoderPtr +HttpFilterIntegrationTestBase::getResponseFromUpstream(absl::string_view request_level_config) { + return getResponse(request_level_config, true); +} + +Envoy::IntegrationStreamDecoderPtr +HttpFilterIntegrationTestBase::getResponseFromExtension(absl::string_view request_level_config) { + return getResponse(request_level_config, false); +} + +Envoy::IntegrationStreamDecoderPtr HttpFilterIntegrationTestBase::getResponseFromUpstream( + const Envoy::Http::TestRequestHeaderMapImpl& request_headers) { + return getResponse(request_headers, true); +} + +Envoy::IntegrationStreamDecoderPtr HttpFilterIntegrationTestBase::getResponseFromExtension( + const Envoy::Http::TestRequestHeaderMapImpl& request_headers) { + return getResponse(request_headers, false); +} + +Envoy::IntegrationStreamDecoderPtr +HttpFilterIntegrationTestBase::getResponse(absl::string_view request_level_config, + bool setup_for_upstream_request) { + const Envoy::Http::LowerCaseString key("x-nighthawk-test-server-config"); + Envoy::Http::TestRequestHeaderMapImpl request_headers = request_headers_; + request_headers.setCopy(key, request_level_config); + return getResponse(request_headers, setup_for_upstream_request); +} + +Envoy::IntegrationStreamDecoderPtr HttpFilterIntegrationTestBase::getResponse( + const Envoy::Http::TestRequestHeaderMapImpl& request_headers, bool setup_for_upstream_request) { + cleanupUpstreamAndDownstream(); + codec_client_ = makeHttpConnection(lookupPort("http")); + Envoy::IntegrationStreamDecoderPtr response; + const bool is_post = request_headers.Method()->value().getStringView() == + Envoy::Http::Headers::get().MethodValues.Post; + const uint64_t request_body_size = is_post ? 1024 : 0; + if (setup_for_upstream_request) { + response = sendRequestAndWaitForResponse(request_headers, request_body_size, + default_response_headers_, /*response_body_size*/ 0); + } else { + if (is_post) { + response = codec_client_->makeRequestWithBody(request_headers, request_body_size); + } else { + response = codec_client_->makeHeaderOnlyRequest(request_headers); + } + response->waitForEndStream(); + } + return response; +} + +} // namespace Nighthawk diff --git a/test/server/http_filter_integration_test_base.h b/test/server/http_filter_integration_test_base.h new file mode 100644 index 000000000..0b9db9ec3 --- /dev/null +++ b/test/server/http_filter_integration_test_base.h @@ -0,0 +1,122 @@ +#pragma once + +#include + +#include "external/envoy/test/integration/http_integration.h" + +namespace Nighthawk { + +/** + * Base class with shared functionality for testing Nighthawk test server http filter extensions. + */ +class HttpFilterIntegrationTestBase : public Envoy::HttpIntegrationTest { +protected: + /** + * @brief Construct a new HttpFilterIntegrationTestBase instance. + * + * @param ip_version Specify the ip version that the integration test server will use to listen + * for connections. + */ + HttpFilterIntegrationTestBase(Envoy::Network::Address::IpVersion ip_version); + + /** + * We don't override SetUp(): tests using this fixture must call setup() instead. + * This is to avoid imposing the need to create a fixture per filter configuration. + * + * @param config configuration to pass to Envoy::HttpIntegrationTest::config_helper_.addFilter. + */ + void setup(const std::string& config); + + /** + * @brief Fetch a response with the default request headers, and set up a fake upstream to supply + * the response. + * + * @return Envoy::IntegrationStreamDecoderPtr Pointer to the integration stream decoder, which can + * be used to inspect the response. + */ + Envoy::IntegrationStreamDecoderPtr getResponseFromUpstream(); + + /** + * @brief Fetches a response with request-level configuration set in the request header, and set + * up a fake upstream to supply the response. + * + * @param request_level_config Configuration to be delivered by request header. For example + * "{{response_body_size:1024}". + * @return Envoy::IntegrationStreamDecoderPtr Pointer to the integration stream decoder, which can + * be used to inspect the response. + */ + Envoy::IntegrationStreamDecoderPtr + getResponseFromUpstream(absl::string_view request_level_config); + + /** + * @brief Fetches a response with request-level configuration set in the request header. The + * extension under test should supply the response. + * + * @param request_level_config Configuration to be delivered by request header. For example + * "{{response_body_size:1024}". + * @return Envoy::IntegrationStreamDecoderPtr Pointer to the integration stream decoder, which can + * be used to inspect the response. + */ + Envoy::IntegrationStreamDecoderPtr + getResponseFromExtension(absl::string_view request_level_config); + + /** + * @brief Fetch a response using the provided request headers, and set up a fake upstream to + * supply a response. + * + * @param request_headers Supply a full set of request headers. If the request method is set to + * POST, an entity body will be send after the request headers. + * @return Envoy::IntegrationStreamDecoderPtr Pointer to the integration stream decoder, which can + * be used to inspect the response. + */ + Envoy::IntegrationStreamDecoderPtr + getResponseFromUpstream(const Envoy::Http::TestRequestHeaderMapImpl& request_headers); + + /** + * @brief Fetch a response using the provided request headers. The extension under test must + * supply a response. + * + * @param request_headers Supply a full set of request headers. If the request method is set to + * POST, an entity body will be send after the request headers. + * @return Envoy::IntegrationStreamDecoderPtr Pointer to the integration stream decoder, which can + * be used to inspect the response. + */ + Envoy::IntegrationStreamDecoderPtr + getResponseFromExtension(const Envoy::Http::TestRequestHeaderMapImpl& request_headers); + +private: + /** + * @brief Fetches a response with request-level configuration set in the request header. + * + * @param request_level_config Configuration to be delivered by request header. For example + * "{{response_body_size:1024}". + * @param setup_for_upstream_request Set to true iff the filter extension under test is expected + * to short-circuit and supply a response directly. For example because it couldn't parse the + * supplied request-level configuration. Otherwise this should be set to true, and a stock + * response will be yielded by the integration test server through an upstream request. + * @return Envoy::IntegrationStreamDecoderPtr Pointer to the integration stream decoder, which can + * be used to inspect the response. + */ + Envoy::IntegrationStreamDecoderPtr getResponse(absl::string_view request_level_config, + bool setup_for_upstream_request); + + /** + * @brief Fetch a response using the provided request headers. + * + * @param request_headers Supply a full set of request headers. If the request method is set to + * POST, an entity body will be send after the request headers. + * @param setup_for_upstream_request Set to true iff the filter extension under test is expected + * to short-circuit and supply a response directly. For example because it couldn't parse the + * supplied request-level configuration. Otherwise this should be set to true, and a stock + * response will be yielded by the integration test server through an upstream request. + * @return Envoy::IntegrationStreamDecoderPtr Pointer to the integration stream decoder, which can + * be used to inspect the response. + */ + Envoy::IntegrationStreamDecoderPtr + getResponse(const Envoy::Http::TestRequestHeaderMapImpl& request_headers, + bool setup_for_upstream_request); + + const Envoy::Http::TestRequestHeaderMapImpl request_headers_; +}; + +} // namespace Nighthawk From 8d30820f3b254e59b7872cab3dc1e8768a0355c0 Mon Sep 17 00:00:00 2001 From: Otto van der Schaaf Date: Sat, 12 Sep 2020 01:48:23 +0200 Subject: [PATCH 2/5] Review feedback Signed-off-by: Otto van der Schaaf --- test/server/BUILD | 1 + .../http_filter_integration_test_base.cc | 50 +++------- .../http_filter_integration_test_base.h | 98 ++++--------------- 3 files changed, 37 insertions(+), 112 deletions(-) diff --git a/test/server/BUILD b/test/server/BUILD index 68717484b..71ae178ab 100644 --- a/test/server/BUILD +++ b/test/server/BUILD @@ -15,6 +15,7 @@ envoy_cc_test_library( hdrs = ["http_filter_integration_test_base.h"], repository = "@envoy", deps = [ + "//source/server:well_known_headers_lib", "@envoy//test/integration:http_integration_lib", ], ) diff --git a/test/server/http_filter_integration_test_base.cc b/test/server/http_filter_integration_test_base.cc index 44c849224..c6f525db6 100644 --- a/test/server/http_filter_integration_test_base.cc +++ b/test/server/http_filter_integration_test_base.cc @@ -1,5 +1,7 @@ #include "test/server/http_filter_integration_test_base.h" +#include "server/well_known_headers.h" + #include "gtest/gtest.h" namespace Nighthawk { @@ -14,55 +16,33 @@ void HttpFilterIntegrationTestBase::setup(const std::string& config) { HttpIntegrationTest::initialize(); } -Envoy::IntegrationStreamDecoderPtr HttpFilterIntegrationTestBase::getResponseFromUpstream() { - return getResponse(request_headers_, true); +void HttpFilterIntegrationTestBase::updateRequestLevelConfiguration( + absl::string_view request_level_config) { + request_headers_.setCopy(Server::TestServer::HeaderNames::get().TestServerConfig, + request_level_config); } -Envoy::IntegrationStreamDecoderPtr -HttpFilterIntegrationTestBase::getResponseFromUpstream(absl::string_view request_level_config) { - return getResponse(request_level_config, true); +void HttpFilterIntegrationTestBase::switchToPostWithEntityBody() { + request_headers_.setCopy(Envoy::Http::Headers::get().Method, + Envoy::Http::Headers::get().MethodValues.Post); } Envoy::IntegrationStreamDecoderPtr -HttpFilterIntegrationTestBase::getResponseFromExtension(absl::string_view request_level_config) { - return getResponse(request_level_config, false); -} - -Envoy::IntegrationStreamDecoderPtr HttpFilterIntegrationTestBase::getResponseFromUpstream( - const Envoy::Http::TestRequestHeaderMapImpl& request_headers) { - return getResponse(request_headers, true); -} - -Envoy::IntegrationStreamDecoderPtr HttpFilterIntegrationTestBase::getResponseFromExtension( - const Envoy::Http::TestRequestHeaderMapImpl& request_headers) { - return getResponse(request_headers, false); -} - -Envoy::IntegrationStreamDecoderPtr -HttpFilterIntegrationTestBase::getResponse(absl::string_view request_level_config, - bool setup_for_upstream_request) { - const Envoy::Http::LowerCaseString key("x-nighthawk-test-server-config"); - Envoy::Http::TestRequestHeaderMapImpl request_headers = request_headers_; - request_headers.setCopy(key, request_level_config); - return getResponse(request_headers, setup_for_upstream_request); -} - -Envoy::IntegrationStreamDecoderPtr HttpFilterIntegrationTestBase::getResponse( - const Envoy::Http::TestRequestHeaderMapImpl& request_headers, bool setup_for_upstream_request) { +HttpFilterIntegrationTestBase::getResponse(ResponseOrigin expected_origin) { cleanupUpstreamAndDownstream(); codec_client_ = makeHttpConnection(lookupPort("http")); Envoy::IntegrationStreamDecoderPtr response; - const bool is_post = request_headers.Method()->value().getStringView() == + const bool is_post = request_headers_.Method()->value().getStringView() == Envoy::Http::Headers::get().MethodValues.Post; const uint64_t request_body_size = is_post ? 1024 : 0; - if (setup_for_upstream_request) { - response = sendRequestAndWaitForResponse(request_headers, request_body_size, + if (expected_origin == ResponseOrigin::UPSTREAM) { + response = sendRequestAndWaitForResponse(request_headers_, request_body_size, default_response_headers_, /*response_body_size*/ 0); } else { if (is_post) { - response = codec_client_->makeRequestWithBody(request_headers, request_body_size); + response = codec_client_->makeRequestWithBody(request_headers_, request_body_size); } else { - response = codec_client_->makeHeaderOnlyRequest(request_headers); + response = codec_client_->makeHeaderOnlyRequest(request_headers_); } response->waitForEndStream(); } diff --git a/test/server/http_filter_integration_test_base.h b/test/server/http_filter_integration_test_base.h index 0b9db9ec3..c3d72ca18 100644 --- a/test/server/http_filter_integration_test_base.h +++ b/test/server/http_filter_integration_test_base.h @@ -12,7 +12,11 @@ namespace Nighthawk { class HttpFilterIntegrationTestBase : public Envoy::HttpIntegrationTest { protected: /** - * @brief Construct a new HttpFilterIntegrationTestBase instance. + * Indicate the expected response origin. + */ + enum class ResponseOrigin { UPSTREAM, EXTENSION }; + /** + * Construct a new HttpFilterIntegrationTestBase instance. * * @param ip_version Specify the ip version that the integration test server will use to listen * for connections. @@ -28,95 +32,35 @@ class HttpFilterIntegrationTestBase : public Envoy::HttpIntegrationTest { void setup(const std::string& config); /** - * @brief Fetch a response with the default request headers, and set up a fake upstream to supply - * the response. - * - * @return Envoy::IntegrationStreamDecoderPtr Pointer to the integration stream decoder, which can - * be used to inspect the response. - */ - Envoy::IntegrationStreamDecoderPtr getResponseFromUpstream(); - - /** - * @brief Fetches a response with request-level configuration set in the request header, and set - * up a fake upstream to supply the response. + * Make getResponse send request-level configuration. Test server extensions read that + * configuration and merge it with their static configuration to determine a final effective + * configuration. See TestServerConfig in well_known_headers.h for the up to date header name. * - * @param request_level_config Configuration to be delivered by request header. For example - * "{{response_body_size:1024}". - * @return Envoy::IntegrationStreamDecoderPtr Pointer to the integration stream decoder, which can - * be used to inspect the response. + * @param request_level_config Configuration to be delivered by request-header in future calls to + * getResponse(). For example: "{response_body_size:1024}". */ - Envoy::IntegrationStreamDecoderPtr - getResponseFromUpstream(absl::string_view request_level_config); + void updateRequestLevelConfiguration(absl::string_view request_level_config); /** - * @brief Fetches a response with request-level configuration set in the request header. The - * extension under test should supply the response. - * - * @param request_level_config Configuration to be delivered by request header. For example - * "{{response_body_size:1024}". - * @return Envoy::IntegrationStreamDecoderPtr Pointer to the integration stream decoder, which can - * be used to inspect the response. + * Switch getResponse() to use the POST request method with an entity body. + * Doing so will make tests hit a different code paths in extensions. */ - Envoy::IntegrationStreamDecoderPtr - getResponseFromExtension(absl::string_view request_level_config); + void switchToPostWithEntityBody(); /** - * @brief Fetch a response using the provided request headers, and set up a fake upstream to - * supply a response. + * Fetch a response. The request headers default to a minimal GET, but this may be changed + * via other methods in this class. * - * @param request_headers Supply a full set of request headers. If the request method is set to - * POST, an entity body will be send after the request headers. + * @param expected_origin Indicate which component will be expected to reply: the extension or + * a fake upstream. Will cause a test failure upon mismatch. Can be used to verify that an + * extension short circuits and directly responds when expected. * @return Envoy::IntegrationStreamDecoderPtr Pointer to the integration stream decoder, which can * be used to inspect the response. */ - Envoy::IntegrationStreamDecoderPtr - getResponseFromUpstream(const Envoy::Http::TestRequestHeaderMapImpl& request_headers); - - /** - * @brief Fetch a response using the provided request headers. The extension under test must - * supply a response. - * - * @param request_headers Supply a full set of request headers. If the request method is set to - * POST, an entity body will be send after the request headers. - * @return Envoy::IntegrationStreamDecoderPtr Pointer to the integration stream decoder, which can - * be used to inspect the response. - */ - Envoy::IntegrationStreamDecoderPtr - getResponseFromExtension(const Envoy::Http::TestRequestHeaderMapImpl& request_headers); + Envoy::IntegrationStreamDecoderPtr getResponse(ResponseOrigin expected_origin); private: - /** - * @brief Fetches a response with request-level configuration set in the request header. - * - * @param request_level_config Configuration to be delivered by request header. For example - * "{{response_body_size:1024}". - * @param setup_for_upstream_request Set to true iff the filter extension under test is expected - * to short-circuit and supply a response directly. For example because it couldn't parse the - * supplied request-level configuration. Otherwise this should be set to true, and a stock - * response will be yielded by the integration test server through an upstream request. - * @return Envoy::IntegrationStreamDecoderPtr Pointer to the integration stream decoder, which can - * be used to inspect the response. - */ - Envoy::IntegrationStreamDecoderPtr getResponse(absl::string_view request_level_config, - bool setup_for_upstream_request); - - /** - * @brief Fetch a response using the provided request headers. - * - * @param request_headers Supply a full set of request headers. If the request method is set to - * POST, an entity body will be send after the request headers. - * @param setup_for_upstream_request Set to true iff the filter extension under test is expected - * to short-circuit and supply a response directly. For example because it couldn't parse the - * supplied request-level configuration. Otherwise this should be set to true, and a stock - * response will be yielded by the integration test server through an upstream request. - * @return Envoy::IntegrationStreamDecoderPtr Pointer to the integration stream decoder, which can - * be used to inspect the response. - */ - Envoy::IntegrationStreamDecoderPtr - getResponse(const Envoy::Http::TestRequestHeaderMapImpl& request_headers, - bool setup_for_upstream_request); - - const Envoy::Http::TestRequestHeaderMapImpl request_headers_; + Envoy::Http::TestRequestHeaderMapImpl request_headers_; }; } // namespace Nighthawk From 7eeb7e9f8bb7873ab2ee6f0870d74ad7dbc9fb4f Mon Sep 17 00:00:00 2001 From: Otto van der Schaaf Date: Mon, 14 Sep 2020 10:25:02 +0200 Subject: [PATCH 3/5] Add setRequestHeader Signed-off-by: Otto van der Schaaf --- test/server/http_filter_integration_test_base.cc | 12 ++++++++---- test/server/http_filter_integration_test_base.h | 9 +++++++++ 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/test/server/http_filter_integration_test_base.cc b/test/server/http_filter_integration_test_base.cc index c6f525db6..ee0239fc5 100644 --- a/test/server/http_filter_integration_test_base.cc +++ b/test/server/http_filter_integration_test_base.cc @@ -18,13 +18,17 @@ void HttpFilterIntegrationTestBase::setup(const std::string& config) { void HttpFilterIntegrationTestBase::updateRequestLevelConfiguration( absl::string_view request_level_config) { - request_headers_.setCopy(Server::TestServer::HeaderNames::get().TestServerConfig, - request_level_config); + setRequestHeader(Server::TestServer::HeaderNames::get().TestServerConfig, request_level_config); } void HttpFilterIntegrationTestBase::switchToPostWithEntityBody() { - request_headers_.setCopy(Envoy::Http::Headers::get().Method, - Envoy::Http::Headers::get().MethodValues.Post); + setRequestHeader(Envoy::Http::Headers::get().Method, + Envoy::Http::Headers::get().MethodValues.Post); +} + +void HttpFilterIntegrationTestBase::setRequestHeader( + const Envoy::Http::LowerCaseString& header_name, absl::string_view header_value) { + request_headers_.setCopy(header_name, header_value); } Envoy::IntegrationStreamDecoderPtr diff --git a/test/server/http_filter_integration_test_base.h b/test/server/http_filter_integration_test_base.h index c3d72ca18..1d6809a1f 100644 --- a/test/server/http_filter_integration_test_base.h +++ b/test/server/http_filter_integration_test_base.h @@ -47,6 +47,15 @@ class HttpFilterIntegrationTestBase : public Envoy::HttpIntegrationTest { */ void switchToPostWithEntityBody(); + /** + * Set a request header value. Overwrites any existing value. + * + * @param header_name Name of the request header to set. + * @param header_value Value to set for the request header. + */ + void setRequestHeader(const Envoy::Http::LowerCaseString& header_name, + absl::string_view header_value); + /** * Fetch a response. The request headers default to a minimal GET, but this may be changed * via other methods in this class. From ca17f41e3cbf7bbf3ac606e9f96acafd6c7db414 Mon Sep 17 00:00:00 2001 From: Otto van der Schaaf Date: Mon, 14 Sep 2020 23:39:39 +0200 Subject: [PATCH 4/5] Review feedback Signed-off-by: Otto van der Schaaf --- test/server/http_filter_integration_test_base.cc | 11 +++++++++-- test/server/http_filter_integration_test_base.h | 2 +- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/test/server/http_filter_integration_test_base.cc b/test/server/http_filter_integration_test_base.cc index ee0239fc5..c85fddbaf 100644 --- a/test/server/http_filter_integration_test_base.cc +++ b/test/server/http_filter_integration_test_base.cc @@ -11,8 +11,8 @@ HttpFilterIntegrationTestBase::HttpFilterIntegrationTestBase( : HttpIntegrationTest(Envoy::Http::CodecClient::Type::HTTP1, ip_version), request_headers_({{":method", "GET"}, {":path", "/"}, {":authority", "host"}}) {} -void HttpFilterIntegrationTestBase::setup(const std::string& config) { - config_helper_.addFilter(config); +void HttpFilterIntegrationTestBase::initializeConfig(absl::string_view config) { + config_helper_.addFilter(std::string(config)); HttpIntegrationTest::initialize(); } @@ -38,7 +38,14 @@ HttpFilterIntegrationTestBase::getResponse(ResponseOrigin expected_origin) { Envoy::IntegrationStreamDecoderPtr response; const bool is_post = request_headers_.Method()->value().getStringView() == Envoy::Http::Headers::get().MethodValues.Post; + // Upon observing a POST request method, we inject a content body, as promised in the header file. + // This is useful, because emitting an entity body will hit distinct code in extensions. Hence we + // facilitate that. const uint64_t request_body_size = is_post ? 1024 : 0; + // An extension can either act as an origin and synthesize a reply, or delegate that + // responsibility to an upstream. This behavior may change from request to request. For example, + // an extension is designed to transform input from an upstream, may start acting as an origin on + // misconfiguration. if (expected_origin == ResponseOrigin::UPSTREAM) { response = sendRequestAndWaitForResponse(request_headers_, request_body_size, default_response_headers_, /*response_body_size*/ 0); diff --git a/test/server/http_filter_integration_test_base.h b/test/server/http_filter_integration_test_base.h index 1d6809a1f..92c45ab86 100644 --- a/test/server/http_filter_integration_test_base.h +++ b/test/server/http_filter_integration_test_base.h @@ -29,7 +29,7 @@ class HttpFilterIntegrationTestBase : public Envoy::HttpIntegrationTest { * * @param config configuration to pass to Envoy::HttpIntegrationTest::config_helper_.addFilter. */ - void setup(const std::string& config); + void initializeConfig(absl::string_view config); /** * Make getResponse send request-level configuration. Test server extensions read that From 2aff84afee12feca90daa28b7cdfc11753c1bcb8 Mon Sep 17 00:00:00 2001 From: Otto van der Schaaf Date: Wed, 16 Sep 2020 22:32:54 +0200 Subject: [PATCH 5/5] Review feedback Signed-off-by: Otto van der Schaaf --- .../http_filter_integration_test_base.cc | 4 +-- .../http_filter_integration_test_base.h | 30 ++++++++++++------- 2 files changed, 22 insertions(+), 12 deletions(-) diff --git a/test/server/http_filter_integration_test_base.cc b/test/server/http_filter_integration_test_base.cc index c85fddbaf..d204133ad 100644 --- a/test/server/http_filter_integration_test_base.cc +++ b/test/server/http_filter_integration_test_base.cc @@ -11,12 +11,12 @@ HttpFilterIntegrationTestBase::HttpFilterIntegrationTestBase( : HttpIntegrationTest(Envoy::Http::CodecClient::Type::HTTP1, ip_version), request_headers_({{":method", "GET"}, {":path", "/"}, {":authority", "host"}}) {} -void HttpFilterIntegrationTestBase::initializeConfig(absl::string_view config) { +void HttpFilterIntegrationTestBase::initializeFilterConfiguration(absl::string_view config) { config_helper_.addFilter(std::string(config)); HttpIntegrationTest::initialize(); } -void HttpFilterIntegrationTestBase::updateRequestLevelConfiguration( +void HttpFilterIntegrationTestBase::setRequestLevelConfiguration( absl::string_view request_level_config) { setRequestHeader(Server::TestServer::HeaderNames::get().TestServerConfig, request_level_config); } diff --git a/test/server/http_filter_integration_test_base.h b/test/server/http_filter_integration_test_base.h index 92c45ab86..8027753ec 100644 --- a/test/server/http_filter_integration_test_base.h +++ b/test/server/http_filter_integration_test_base.h @@ -8,13 +8,23 @@ namespace Nighthawk { /** * Base class with shared functionality for testing Nighthawk test server http filter extensions. + * The class is stateful, and not safe to use from multiple threads. */ class HttpFilterIntegrationTestBase : public Envoy::HttpIntegrationTest { protected: /** - * Indicate the expected response origin. + * Indicate the expected response origin. A test failure will occur upon mismatch. */ - enum class ResponseOrigin { UPSTREAM, EXTENSION }; + enum class ResponseOrigin { + /** + * The upstream will supply the response, and not the extension under test. + */ + UPSTREAM, + /** + * The extension under test will suplly a response, and no upstream will be set up to do that. + */ + EXTENSION + }; /** * Construct a new HttpFilterIntegrationTestBase instance. * @@ -24,12 +34,13 @@ class HttpFilterIntegrationTestBase : public Envoy::HttpIntegrationTest { HttpFilterIntegrationTestBase(Envoy::Network::Address::IpVersion ip_version); /** - * We don't override SetUp(): tests using this fixture must call setup() instead. - * This is to avoid imposing the need to create a fixture per filter configuration. + * We don't override SetUp(): tests using this fixture must call initializeFilterConfiguration() + * instead. This is to avoid imposing the need to create a fixture per filter configuration. * - * @param config configuration to pass to Envoy::HttpIntegrationTest::config_helper_.addFilter. + * @param filter_configuration Pass configuration for the filter under test. Will be handed off to + * Envoy::HttpIntegrationTest::config_helper_.addFilter. */ - void initializeConfig(absl::string_view config); + void initializeFilterConfiguration(absl::string_view filter_configuration); /** * Make getResponse send request-level configuration. Test server extensions read that @@ -39,7 +50,7 @@ class HttpFilterIntegrationTestBase : public Envoy::HttpIntegrationTest { * @param request_level_config Configuration to be delivered by request-header in future calls to * getResponse(). For example: "{response_body_size:1024}". */ - void updateRequestLevelConfiguration(absl::string_view request_level_config); + void setRequestLevelConfiguration(absl::string_view request_level_config); /** * Switch getResponse() to use the POST request method with an entity body. @@ -57,9 +68,8 @@ class HttpFilterIntegrationTestBase : public Envoy::HttpIntegrationTest { absl::string_view header_value); /** - * Fetch a response. The request headers default to a minimal GET, but this may be changed - * via other methods in this class. - * + * Fetch a response, according to the options specified by the class methods. By default, + * simulates a GET request with minimal headers. * @param expected_origin Indicate which component will be expected to reply: the extension or * a fake upstream. Will cause a test failure upon mismatch. Can be used to verify that an * extension short circuits and directly responds when expected.