Skip to content

[balsa] Parametrize IntegrationTest on HTTP/1 implementation. #24254

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
KBaichoo merged 6 commits into from
Dec 16, 2022
Merged

[balsa] Parametrize IntegrationTest on HTTP/1 implementation. #24254

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ecdb4d0
Parametrize IntegrationTest on HTTP/1 implementation.
bnc-google Nov 18, 2022
75a3f28
Reviewer suggestions, plus adding using directives.
bnc-google Dec 7, 2022
6d8bad6
Merge remote-tracking branch 'upstream/main' into integrationtest
bnc-google Dec 7, 2022
f90d1c8
Merge remote-tracking branch 'upstream/main' into integrationtest
bnc-google Dec 13, 2022
c9ea113
Merge remote-tracking branch 'upstream/main' into integrationtest
bnc-google Dec 14, 2022
01ffd25
Merge remote-tracking branch 'upstream/main' into integrationtest
bnc-google Dec 15, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 56 additions & 14 deletions test/integration/integration_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,20 @@ void setAllowHttp10WithDefaultHost(
hcm.mutable_http_protocol_options()->set_default_host_for_http_10("default.com");
}

std::string testParamToString(
const ::testing::TestParamInfo<std::tuple<Network::Address::IpVersion, Http1ParserImpl>>&

@diannahu diannahu Dec 2, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I think there's a preference here to avoid fully qualifying names with the leading "::" if possible, unless there's a using declaration or namespace alias. I like using the decision tree at internal TOTW 151 as a reference (unfortunately, there is no analogue at https://abseil.io/tips/ - the closest thing there seems to be https://abseil.io/tips/130).

@bencebeky bencebeky Dec 7, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. I was not aware of that guidance, thank you for digging it up.

@bencebeky bencebeky Dec 7, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On that note, also adding using directives for Combine, Values, and ValuesIn.

params) {
return absl::StrCat(TestUtility::ipVersionToString(std::get<0>(params.param)),
TestUtility::http1ParserImplToString(std::get<1>(params.param)));
}

} // namespace

INSTANTIATE_TEST_SUITE_P(IpVersions, IntegrationTest,
testing::ValuesIn(TestEnvironment::getIpVersionsForTest()),
TestUtility::ipTestParamsToString);
INSTANTIATE_TEST_SUITE_P(
IpVersions, IntegrationTest,

@diannahu diannahu Dec 2, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

optional: s/IpVersions/IpVersionsAndHttp1ParserVersions/ or similar.

@bencebeky bencebeky Dec 7, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about IpVersionsAndHttp1Parser as a compromise for brevity?

testing::Combine(testing::ValuesIn(TestEnvironment::getIpVersionsForTest()),
testing::Values(Http1ParserImpl::HttpParser, Http1ParserImpl::BalsaParser)),
testParamToString);

// Verify that we gracefully handle an invalid pre-bind socket option when using reuse_port.
TEST_P(IntegrationTest, BadPrebindSocketOptionWithReusePort) {
Expand Down Expand Up @@ -131,7 +140,7 @@ TEST_P(IntegrationTest, PerWorkerStatsAndBalancing) {

// Per-worker listener stats.
auto check_listener_stats = [this](uint64_t cx_active, uint64_t cx_total) {
if (GetParam() == Network::Address::IpVersion::v4) {
if (ip_version_ == Network::Address::IpVersion::v4) {
test_server_->waitForGaugeEq("listener.127.0.0.1_0.worker_0.downstream_cx_active", cx_active);
test_server_->waitForGaugeEq("listener.127.0.0.1_0.worker_1.downstream_cx_active", cx_active);
test_server_->waitForCounterEq("listener.127.0.0.1_0.worker_0.downstream_cx_total", cx_total);
Expand Down Expand Up @@ -201,7 +210,7 @@ TEST_P(IntegrationTest, ConnectionBalanceFactory) {
initialize();

auto check_listener_stats = [this](uint64_t cx_active, uint64_t cx_total) {
if (GetParam() == Network::Address::IpVersion::v4) {
if (ip_version_ == Network::Address::IpVersion::v4) {
test_server_->waitForGaugeEq("listener.127.0.0.1_0.worker_0.downstream_cx_active", cx_active);
test_server_->waitForGaugeEq("listener.127.0.0.1_0.worker_1.downstream_cx_active", cx_active);
test_server_->waitForCounterEq("listener.127.0.0.1_0.worker_0.downstream_cx_total", cx_total);
Expand Down Expand Up @@ -240,7 +249,7 @@ TEST_P(IntegrationTest, AllWorkersAreHandlingLoad) {
initialize();

std::string worker0_stat_name, worker1_stat_name;
if (GetParam() == Network::Address::IpVersion::v4) {
if (ip_version_ == Network::Address::IpVersion::v4) {
worker0_stat_name = "listener.127.0.0.1_0.worker_0.downstream_cx_total";
worker1_stat_name = "listener.127.0.0.1_0.worker_1.downstream_cx_total";
} else {
Expand Down Expand Up @@ -975,6 +984,11 @@ TEST_P(IntegrationTest, TestClientAllowChunkedLength) {
}

TEST_P(IntegrationTest, BadFirstline) {
if (http1_implementation_ == Http1ParserImpl::BalsaParser) {
// TODO(#21245): Re-enable this test for BalsaParser.
return;
}

initialize();
std::string response;
sendRawHttpAndWaitForResponse(lookupPort("http"), "hello", &response);
Expand Down Expand Up @@ -1003,6 +1017,11 @@ TEST_P(IntegrationTest, InvalidCharacterInFirstline) {
}

TEST_P(IntegrationTest, InvalidVersion) {
if (http1_implementation_ == Http1ParserImpl::BalsaParser) {
// TODO(#21245): Re-enable this test for BalsaParser.
return;
}

initialize();
std::string response;
sendRawHttpAndWaitForResponse(lookupPort("http"), "GET / HTTP/1.01\r\nHost: host\r\n\r\n",
Expand All @@ -1012,6 +1031,11 @@ TEST_P(IntegrationTest, InvalidVersion) {

// Expect that malformed trailers to break the connection
TEST_P(IntegrationTest, BadTrailer) {
if (http1_implementation_ == Http1ParserImpl::BalsaParser) {
// TODO(#21245): Re-enable this test for BalsaParser.
return;
}

initialize();
std::string response;
sendRawHttpAndWaitForResponse(lookupPort("http"),
Expand All @@ -1028,6 +1052,11 @@ TEST_P(IntegrationTest, BadTrailer) {

// Expect malformed headers to break the connection
TEST_P(IntegrationTest, BadHeader) {
if (http1_implementation_ == Http1ParserImpl::BalsaParser) {
// TODO(#21245): Re-enable this test for BalsaParser.
return;
}

initialize();
std::string response;
sendRawHttpAndWaitForResponse(lookupPort("http"),
Expand Down Expand Up @@ -1093,6 +1122,11 @@ TEST_P(IntegrationTest, Http09Enabled) {
}

TEST_P(IntegrationTest, Http09WithKeepalive) {
if (http1_implementation_ == Http1ParserImpl::BalsaParser) {
// TODO(#21245): Re-enable this test for BalsaParser.
return;
}

#ifdef ENVOY_ENABLE_UHV
// TODO(#23287) - Determine HTTP/0.9 and HTTP/1.0 support within UHV
return;
Expand Down Expand Up @@ -1440,7 +1474,7 @@ TEST_P(IntegrationTest, AbsolutePathUsingHttpsAllowedInternally) {
TEST_P(IntegrationTest, TestHostWithAddress) {
useAccessLog("%REQ(Host)%");
std::string address_string;
if (GetParam() == Network::Address::IpVersion::v4) {
if (ip_version_ == Network::Address::IpVersion::v4) {
address_string = TestUtility::getIpv4Loopback();
} else {
address_string = "[::1]";
Expand Down Expand Up @@ -1524,7 +1558,13 @@ TEST_P(IntegrationTest, Connect) {
}

// Test that Envoy returns HTTP code 502 on upstream protocol error.
TEST_P(IntegrationTest, UpstreamProtocolError) { testRouterUpstreamProtocolError("502", "UPE"); }
TEST_P(IntegrationTest, UpstreamProtocolError) {
if (http1_implementation_ == Http1ParserImpl::BalsaParser) {
// TODO(#21245): Re-enable this test for BalsaParser.
return;
}
testRouterUpstreamProtocolError("502", "UPE");
}

TEST_P(IntegrationTest, TestHead) {
initialize();
Expand Down Expand Up @@ -1588,7 +1628,7 @@ TEST_P(IntegrationTest, TestHeadWithExplicitTE) {

TEST_P(IntegrationTest, TestBind) {
std::string address_string;
if (GetParam() == Network::Address::IpVersion::v4) {
if (ip_version_ == Network::Address::IpVersion::v4) {
address_string = TestUtility::getIpv4Loopback();
} else {
address_string = "::1";
Expand Down Expand Up @@ -1843,14 +1883,16 @@ TEST_P(IntegrationTest, TrailersDroppedDownstream) {
testTrailers(10, 10, false, false);
}

INSTANTIATE_TEST_SUITE_P(IpVersions, UpstreamEndpointIntegrationTest,
testing::ValuesIn(TestEnvironment::getIpVersionsForTest()),
TestUtility::ipTestParamsToString);
INSTANTIATE_TEST_SUITE_P(
IpVersions, UpstreamEndpointIntegrationTest,
testing::Combine(testing::ValuesIn(TestEnvironment::getIpVersionsForTest()),
testing::Values(Http1ParserImpl::HttpParser, Http1ParserImpl::BalsaParser)),
testParamToString);

TEST_P(UpstreamEndpointIntegrationTest, TestUpstreamEndpointAddress) {
initialize();
EXPECT_STREQ(fake_upstreams_[0]->localAddress()->ip()->addressAsString().c_str(),
Network::Test::getLoopbackAddressString(GetParam()).c_str());
Network::Test::getLoopbackAddressString(ip_version_).c_str());
}

// Send continuous pipelined requests while not reading responses, to check
Expand Down Expand Up @@ -2435,7 +2477,7 @@ TEST_P(IntegrationTest, SetRouteToDelegatingRouteWithClusterOverride) {
initialize();

const std::string ip_port_pair =
absl::StrCat(Network::Test::getLoopbackAddressUrlString(GetParam()), ":",
absl::StrCat(Network::Test::getLoopbackAddressUrlString(ip_version_), ":",
fake_upstreams_[1]->localAddress()->ip()->port());

Http::TestRequestHeaderMapImpl request_headers{
Expand Down
32 changes: 25 additions & 7 deletions test/integration/integration_test.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,27 +1,45 @@
#pragma once

#include "test/integration/http_integration.h"
#include "test/test_common/utility.h"

#include "gtest/gtest.h"

// A test class for testing HTTP/1.1 upstream and downstreams
namespace Envoy {
class IntegrationTest : public testing::TestWithParam<Network::Address::IpVersion>,
public HttpIntegrationTest {
class IntegrationTest
: public testing::TestWithParam<std::tuple<Network::Address::IpVersion, Http1ParserImpl>>,
public HttpIntegrationTest {
public:
IntegrationTest() : HttpIntegrationTest(Http::CodecType::HTTP1, GetParam()) {}
IntegrationTest()
: HttpIntegrationTest(Http::CodecType::HTTP1, std::get<0>(GetParam())),
ip_version_(std::get<0>(GetParam())), http1_implementation_(std::get<1>(GetParam())) {
setupHttp1ImplOverrides(http1_implementation_);
}

protected:
const Network::Address::IpVersion ip_version_;
const Http1ParserImpl http1_implementation_;
};

class UpstreamEndpointIntegrationTest : public testing::TestWithParam<Network::Address::IpVersion>,
public HttpIntegrationTest {
class UpstreamEndpointIntegrationTest
: public testing::TestWithParam<std::tuple<Network::Address::IpVersion, Http1ParserImpl>>,
public HttpIntegrationTest {
public:
UpstreamEndpointIntegrationTest()
: HttpIntegrationTest(
Http::CodecType::HTTP1,
[](int) {
return Network::Utility::parseInternetAddress(
Network::Test::getLoopbackAddressString(GetParam()), 0);
Network::Test::getLoopbackAddressString(std::get<0>(GetParam())), 0);
},
GetParam()) {}
std::get<0>(GetParam())),
ip_version_(std::get<0>(GetParam())), http1_implementation_(std::get<1>(GetParam())) {
setupHttp1ImplOverrides(http1_implementation_);
}

protected:
const Network::Address::IpVersion ip_version_;
const Http1ParserImpl http1_implementation_;
};
} // namespace Envoy