diff --git a/source/extensions/access_loggers/grpc/BUILD b/source/extensions/access_loggers/grpc/BUILD index 1174056f7b79..4d73da7b49ad 100644 --- a/source/extensions/access_loggers/grpc/BUILD +++ b/source/extensions/access_loggers/grpc/BUILD @@ -97,11 +97,6 @@ envoy_cc_extension( name = "http_config", srcs = ["http_config.cc"], hdrs = ["http_config.h"], - # TODO(#9953) clean up. - extra_visibility = [ - "//test/common/access_log:__subpackages__", - "//test/integration:__subpackages__", - ], deps = [ ":config_utils", "//envoy/server:access_log_config_interface", @@ -118,11 +113,6 @@ envoy_cc_extension( name = "tcp_config", srcs = ["tcp_config.cc"], hdrs = ["tcp_config.h"], - # TODO(#9953) clean up. - extra_visibility = [ - "//test/common/access_log:__subpackages__", - "//test/integration:__subpackages__", - ], deps = [ ":config_utils", "//envoy/server:access_log_config_interface", diff --git a/test/common/access_log/BUILD b/test/common/access_log/BUILD index bc8eb168dccd..97af03fc5e15 100644 --- a/test/common/access_log/BUILD +++ b/test/common/access_log/BUILD @@ -22,8 +22,6 @@ envoy_cc_test( "//source/common/stream_info:utility_lib", "//source/extensions/access_loggers/file:config", "//source/extensions/access_loggers/filters/cel:config", - "//source/extensions/access_loggers/grpc:http_config", - "//source/extensions/access_loggers/grpc:tcp_config", "//source/extensions/access_loggers/stream:config", "//test/common/stream_info:test_util", "//test/common/upstream:utility_lib", diff --git a/test/extensions/access_loggers/grpc/http_grpc_access_log_integration_test.cc b/test/extensions/access_loggers/grpc/http_grpc_access_log_integration_test.cc index a7479bcd2ff4..117316e07476 100644 --- a/test/extensions/access_loggers/grpc/http_grpc_access_log_integration_test.cc +++ b/test/extensions/access_loggers/grpc/http_grpc_access_log_integration_test.cc @@ -15,6 +15,7 @@ #include "gtest/gtest.h" using testing::AssertionResult; +using testing::HasSubstr; namespace Envoy { namespace { @@ -222,5 +223,128 @@ TEST_P(AccessLogIntegrationTest, BasicAccessLogFlow) { cleanup(); } +// Regression test to make sure that configuring upstream logs over gRPC will not crash Envoy. +// TODO(asraa): Test output of the upstream logs. +// See https://github.com/envoyproxy/envoy/issues/8828. +TEST_P(AccessLogIntegrationTest, ConfigureHttpOverGrpcLogs) { + setUpstreamProtocol(Http::CodecType::HTTP2); + setDownstreamProtocol(Http::CodecType::HTTP2); + config_helper_.addConfigModifier( + [&](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager& + hcm) -> void { + // Configure just enough of an upstream access log to reference the upstream headers. + const std::string yaml_string = R"EOF( +name: router +typed_config: + "@type": type.googleapis.com/envoy.extensions.filters.http.router.v3.Router + upstream_log: + name: grpc_accesslog + filter: + not_health_check_filter: {} + typed_config: + "@type": type.googleapis.com/envoy.extensions.access_loggers.grpc.v3.HttpGrpcAccessLogConfig + common_config: + log_name: foo + transport_api_version: V3 + grpc_service: + envoy_grpc: + cluster_name: cluster_0 + )EOF"; + // Replace the terminal envoy.router. + hcm.clear_http_filters(); + TestUtility::loadFromYaml(yaml_string, *hcm.add_http_filters()); + }); + + initialize(); + + // Send the request. + codec_client_ = makeHttpConnection(lookupPort("http")); + auto response = codec_client_->makeHeaderOnlyRequest(default_request_headers_); + waitForNextUpstreamRequest(); + + // Send the response headers. + upstream_request_->encodeHeaders(default_response_headers_, true); + ASSERT_TRUE(response->waitForEndStream()); + EXPECT_EQ("200", response->headers().getStatusValue()); +} + +// Verify the grpc cached logger is available after the initial logger filter is destroyed. +// Regression test for https://github.com/envoyproxy/envoy/issues/18066 +TEST_P(AccessLogIntegrationTest, GrpcLoggerSurvivesAfterReloadConfig) { + autonomous_upstream_ = true; + // The grpc access logger connection never closes. It's ok to see an incomplete logging stream. + autonomous_allow_incomplete_streams_ = true; + + const std::string grpc_logger_string = R"EOF( + name: grpc_accesslog + typed_config: + "@type": type.googleapis.com/envoy.extensions.access_loggers.grpc.v3.HttpGrpcAccessLogConfig + common_config: + log_name: bar + transport_api_version: V3 + grpc_service: + envoy_grpc: + cluster_name: cluster_0 + )EOF"; + + config_helper_.addConfigModifier([](envoy::config::bootstrap::v3::Bootstrap& bootstrap) { + auto* listener = bootstrap.mutable_static_resources()->mutable_listeners(0); + listener->set_stat_prefix("listener_0"); + }); + config_helper_.addConfigModifier( + [&](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager& + hcm) { TestUtility::loadFromYaml(grpc_logger_string, *hcm.add_access_log()); }); + initialize(); + // Given we're using LDS in this test, initialize() will not complete until + // the initial LDS file has loaded. + EXPECT_EQ(1, test_server_->counter("listener_manager.lds.update_success")->value()); + + // HTTP 1.1 is allowed and the connection is kept open until the listener update. + std::string response; + auto connection = + createConnectionDriver(lookupPort("http"), "GET / HTTP/1.1\r\nHost: host\r\n\r\n", + [&response, &dispatcher = *dispatcher_]( + Network::ClientConnection&, const Buffer::Instance& data) -> void { + response.append(data.toString()); + if (response.find("\r\n\r\n") != std::string::npos) { + dispatcher.exit(); + } + }); + connection->run(); + EXPECT_TRUE(response.find("HTTP/1.1 200") == 0); + + test_server_->waitForCounterEq("access_logs.grpc_access_log.logs_written", 2); + + // Create a new config with HTTP/1.0 proxying. The goal is to trigger a listener update. + ConfigHelper new_config_helper( + version_, *api_, MessageUtil::getJsonStringFromMessageOrDie(config_helper_.bootstrap())); + new_config_helper.addConfigModifier( + [&](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager& + hcm) { + hcm.mutable_http_protocol_options()->set_accept_http_10(true); + hcm.mutable_http_protocol_options()->set_default_host_for_http_10("default.com"); + }); + + // Create an LDS response with the new config, and reload config. + new_config_helper.setLds("1"); + test_server_->waitForCounterGe("listener_manager.listener_in_place_updated", 1); + test_server_->waitForCounterEq("listener_manager.lds.update_success", 2); + + // Wait until the http 1.1 connection is destroyed due to the listener update. It indicates the + // listener starts draining. + test_server_->waitForGaugeEq("listener.listener_0.downstream_cx_active", 0); + // Wait until all the draining filter chain is gone. It indicates the old listener and filter + // chains are destroyed. + test_server_->waitForGaugeEq("listener_manager.total_filter_chains_draining", 0); + + // Verify that the new listener config is applied. + std::string response2; + sendRawHttpAndWaitForResponse(lookupPort("http"), "GET / HTTP/1.0\r\n\r\n", &response2, true); + EXPECT_THAT(response2, HasSubstr("HTTP/1.0 200 OK\r\n")); + + // Verify that the grpc access logger is available after the listener update. + test_server_->waitForCounterEq("access_logs.grpc_access_log.logs_written", 4); +} + } // namespace } // namespace Envoy diff --git a/test/integration/BUILD b/test/integration/BUILD index 559540999670..5504fcaea4d2 100644 --- a/test/integration/BUILD +++ b/test/integration/BUILD @@ -544,7 +544,6 @@ envoy_cc_test( deps = [ ":http_protocol_integration_lib", "//source/common/http:header_map_lib", - "//source/extensions/access_loggers/grpc:http_config", "//source/extensions/filters/http/buffer:config", "//test/integration/filters:encoder_decoder_buffer_filter_lib", "//test/integration/filters:random_pause_filter_lib", @@ -1413,7 +1412,6 @@ envoy_cc_test( deps = [ ":http_integration_lib", ":http_protocol_integration_lib", - "//source/extensions/access_loggers/grpc:http_config", "//source/extensions/filters/listener/tls_inspector:config", "//source/extensions/filters/listener/tls_inspector:tls_inspector_lib", "//source/extensions/filters/network/tcp_proxy:config", diff --git a/test/integration/multiplexed_upstream_integration_test.cc b/test/integration/multiplexed_upstream_integration_test.cc index 2b0bebba5b71..836ca8de7290 100644 --- a/test/integration/multiplexed_upstream_integration_test.cc +++ b/test/integration/multiplexed_upstream_integration_test.cc @@ -548,49 +548,6 @@ TEST_P(MultiplexedUpstreamIntegrationTest, LargeResponseHeadersRejected) { EXPECT_EQ("503", response->headers().getStatusValue()); } -// Regression test to make sure that configuring upstream logs over gRPC will not crash Envoy. -// TODO(asraa): Test output of the upstream logs. -// See https://github.com/envoyproxy/envoy/issues/8828. -TEST_P(MultiplexedUpstreamIntegrationTest, ConfigureHttpOverGrpcLogs) { - config_helper_.addConfigModifier( - [&](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager& - hcm) -> void { - // Configure just enough of an upstream access log to reference the upstream headers. - const std::string yaml_string = R"EOF( -name: router -typed_config: - "@type": type.googleapis.com/envoy.extensions.filters.http.router.v3.Router - upstream_log: - name: grpc_accesslog - filter: - not_health_check_filter: {} - typed_config: - "@type": type.googleapis.com/envoy.extensions.access_loggers.grpc.v3.HttpGrpcAccessLogConfig - common_config: - log_name: foo - transport_api_version: V3 - grpc_service: - envoy_grpc: - cluster_name: cluster_0 - )EOF"; - // Replace the terminal envoy.router. - hcm.clear_http_filters(); - TestUtility::loadFromYaml(yaml_string, *hcm.add_http_filters()); - }); - - initialize(); - - // Send the request. - codec_client_ = makeHttpConnection(lookupPort("http")); - auto response = codec_client_->makeHeaderOnlyRequest(default_request_headers_); - waitForNextUpstreamRequest(); - - // Send the response headers. - upstream_request_->encodeHeaders(default_response_headers_, true); - ASSERT_TRUE(response->waitForEndStream()); - EXPECT_EQ("200", response->headers().getStatusValue()); -} - // Regression test for https://github.com/envoyproxy/envoy/issues/13933 TEST_P(MultiplexedUpstreamIntegrationTest, MultipleRequestsLowStreamLimit) { autonomous_upstream_ = true; diff --git a/test/integration/xds_integration_test.cc b/test/integration/xds_integration_test.cc index b225c4e1f527..8e866132782b 100644 --- a/test/integration/xds_integration_test.cc +++ b/test/integration/xds_integration_test.cc @@ -610,84 +610,6 @@ TEST_P(LdsIntegrationTest, NewListenerWithBadPostListenSocketOption) { test_server_->waitForCounterGe("listener_manager.listener_create_failure", 1); } -// Verify the grpc cached logger is available after the initial logger filter is destroyed. -// Regression test for https://github.com/envoyproxy/envoy/issues/18066 -TEST_P(LdsIntegrationTest, GrpcLoggerSurvivesAfterReloadConfig) { - autonomous_upstream_ = true; - // The grpc access logger connection never closes. It's ok to see an incomplete logging stream. - autonomous_allow_incomplete_streams_ = true; - - const std::string grpc_logger_string = R"EOF( - name: grpc_accesslog - typed_config: - "@type": type.googleapis.com/envoy.extensions.access_loggers.grpc.v3.HttpGrpcAccessLogConfig - common_config: - log_name: bar - transport_api_version: V3 - grpc_service: - envoy_grpc: - cluster_name: cluster_0 - )EOF"; - - config_helper_.addConfigModifier([](envoy::config::bootstrap::v3::Bootstrap& bootstrap) { - auto* listener = bootstrap.mutable_static_resources()->mutable_listeners(0); - listener->set_stat_prefix("listener_0"); - }); - config_helper_.addConfigModifier( - [&](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager& - hcm) { TestUtility::loadFromYaml(grpc_logger_string, *hcm.add_access_log()); }); - initialize(); - // Given we're using LDS in this test, initialize() will not complete until - // the initial LDS file has loaded. - EXPECT_EQ(1, test_server_->counter("listener_manager.lds.update_success")->value()); - - // HTTP 1.1 is allowed and the connection is kept open until the listener update. - std::string response; - auto connection = - createConnectionDriver(lookupPort("http"), "GET / HTTP/1.1\r\nHost: host\r\n\r\n", - [&response, &dispatcher = *dispatcher_]( - Network::ClientConnection&, const Buffer::Instance& data) -> void { - response.append(data.toString()); - if (response.find("\r\n\r\n") != std::string::npos) { - dispatcher.exit(); - } - }); - connection->run(); - EXPECT_TRUE(response.find("HTTP/1.1 200") == 0); - - test_server_->waitForCounterEq("access_logs.grpc_access_log.logs_written", 1); - - // Create a new config with HTTP/1.0 proxying. The goal is to trigger a listener update. - ConfigHelper new_config_helper( - version_, *api_, MessageUtil::getJsonStringFromMessageOrDie(config_helper_.bootstrap())); - new_config_helper.addConfigModifier( - [&](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager& - hcm) { - hcm.mutable_http_protocol_options()->set_accept_http_10(true); - hcm.mutable_http_protocol_options()->set_default_host_for_http_10("default.com"); - }); - - // Create an LDS response with the new config, and reload config. - new_config_helper.setLds("1"); - test_server_->waitForCounterGe("listener_manager.listener_in_place_updated", 1); - test_server_->waitForCounterEq("listener_manager.lds.update_success", 2); - - // Wait until the http 1.1 connection is destroyed due to the listener update. It indicates the - // listener starts draining. - test_server_->waitForGaugeEq("listener.listener_0.downstream_cx_active", 0); - // Wait until all the draining filter chain is gone. It indicates the old listener and filter - // chains are destroyed. - test_server_->waitForGaugeEq("listener_manager.total_filter_chains_draining", 0); - - // Verify that the new listener config is applied. - std::string response2; - sendRawHttpAndWaitForResponse(lookupPort("http"), "GET / HTTP/1.0\r\n\r\n", &response2, true); - EXPECT_THAT(response2, HasSubstr("HTTP/1.0 200 OK\r\n")); - - // Verify that the grpc access logger is available after the listener update. - test_server_->waitForCounterEq("access_logs.grpc_access_log.logs_written", 2); -} - // Sample test making sure our config framework informs on listener failure. TEST_P(LdsIntegrationTest, FailConfigLoad) { config_helper_.addConfigModifier([&](envoy::config::bootstrap::v3::Bootstrap& bootstrap) -> void {