Skip to content
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.

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

runtime: remove HCM stream error runtime override #16040

Merged
merged 6 commits into from
Apr 22, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions docs/root/version_history/current.rst
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ Removed Config or Runtime
-------------------------
*Normally occurs at the end of the* :ref:`deprecation period <deprecated>`

* http: removed `envoy.reloadable_features.hcm_stream_error_on_invalid_message` for disabling closing HTTP/1.1 connections on error. Connection-closing can still be disabled by setting the HTTP/1 configuration :ref:`override_stream_error_on_invalid_http_message <envoy_v3_api_field_config.core.v3.Http1ProtocolOptions.override_stream_error_on_invalid_http_message>`.
* http: removed `envoy.reloadable_features.overload_manager_disable_keepalive_drain_http2`; Envoy will now always send GOAWAY to HTTP2 downstreams when the :ref:`disable_keepalive <config_overload_manager_overload_actions>` overload action is active.
* tls: removed `envoy.reloadable_features.tls_use_io_handle_bio` runtime guard and legacy code path.

Expand Down
4 changes: 1 addition & 3 deletions source/common/http/conn_manager_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1312,9 +1312,7 @@ absl::optional<Router::ConfigConstSharedPtr> ConnectionManagerImpl::ActiveStream

void ConnectionManagerImpl::ActiveStream::onLocalReply(Code code) {
// The BadRequest error code indicates there has been a messaging error.
if (Runtime::runtimeFeatureEnabled(
"envoy.reloadable_features.hcm_stream_error_on_invalid_message") &&
code == Http::Code::BadRequest && connection_manager_.codec_->protocol() < Protocol::Http2 &&
if (code == Http::Code::BadRequest && connection_manager_.codec_->protocol() < Protocol::Http2 &&
!response_encoder_->streamErrorOnInvalidHttpMessage()) {
state_.saw_connection_close_ = true;
}
Expand Down
8 changes: 2 additions & 6 deletions source/common/http/utility.cc
Original file line number Diff line number Diff line change
Expand Up @@ -128,9 +128,7 @@ initializeAndValidateOptions(const envoy::config::core::v3::Http2ProtocolOptions
bool hcm_stream_error_set,
const Protobuf::BoolValue& hcm_stream_error) {
auto ret = initializeAndValidateOptions(options);
if (Runtime::runtimeFeatureEnabled(
"envoy.reloadable_features.hcm_stream_error_on_invalid_message") &&
!options.has_override_stream_error_on_invalid_http_message() && hcm_stream_error_set) {
if (!options.has_override_stream_error_on_invalid_http_message() && hcm_stream_error_set) {
ret.mutable_override_stream_error_on_invalid_http_message()->set_value(
hcm_stream_error.value());
}
Expand Down Expand Up @@ -211,9 +209,7 @@ initializeAndValidateOptions(const envoy::config::core::v3::Http3ProtocolOptions
return options;
}
envoy::config::core::v3::Http3ProtocolOptions options_clone(options);
if (Runtime::runtimeFeatureEnabled(
"envoy.reloadable_features.hcm_stream_error_on_invalid_message") &&
hcm_stream_error_set) {
if (hcm_stream_error_set) {
options_clone.mutable_override_stream_error_on_invalid_http_message()->set_value(
hcm_stream_error.value());
} else {
Expand Down
1 change: 0 additions & 1 deletion source/common/runtime/runtime_features.cc
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ constexpr const char* runtime_features[] = {
"envoy.reloadable_features.grpc_web_fix_non_proto_encoded_response_handling",
"envoy.reloadable_features.grpc_json_transcoder_adhere_to_buffer_limits",
"envoy.reloadable_features.hash_multiple_header_values",
"envoy.reloadable_features.hcm_stream_error_on_invalid_message",
"envoy.reloadable_features.health_check.graceful_goaway_handling",
"envoy.reloadable_features.health_check.immediate_failure_exclude_from_cluster",
"envoy.reloadable_features.http_match_on_all_headers",
Expand Down
12 changes: 1 addition & 11 deletions test/common/http/utility_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@
#include "common/network/address_impl.h"

#include "test/mocks/http/mocks.h"
#include "test/mocks/protobuf/mocks.h"
#include "test/test_common/printers.h"
#include "test/test_common/test_runtime.h"
#include "test/test_common/utility.h"

#include "gtest/gtest.h"
Expand Down Expand Up @@ -409,16 +409,6 @@ TEST(HttpUtility, ValidateStreamErrorsWithHcm) {
EXPECT_TRUE(Envoy::Http2::Utility::initializeAndValidateOptions(http2_options, true, hcm_value)
.override_stream_error_on_invalid_http_message()
.value());

{
// With runtime flipped, override is ignored.
asraa marked this conversation as resolved.
Show resolved Hide resolved
TestScopedRuntime scoped_runtime;
Runtime::LoaderSingleton::getExisting()->mergeValues(
{{"envoy.reloadable_features.hcm_stream_error_on_invalid_message", "false"}});
EXPECT_TRUE(Envoy::Http2::Utility::initializeAndValidateOptions(http2_options, true, hcm_value)
.override_stream_error_on_invalid_http_message()
.value());
}
}

TEST(HttpUtility, ValidateStreamErrorConfigurationForHttp1) {
Expand Down
6 changes: 3 additions & 3 deletions test/integration/integration_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -964,9 +964,9 @@ TEST_P(IntegrationTest, PipelineWithTrailers) {
// an inline sendLocalReply to make sure the "kick" works under the call stack
// of dispatch as well as when a response is proxied from upstream.
TEST_P(IntegrationTest, PipelineInline) {
// When deprecating this flag, set hcm.mutable_stream_error_on_invalid_http_message true.
config_helper_.addRuntimeOverride("envoy.reloadable_features.hcm_stream_error_on_invalid_message",
"false");
config_helper_.addConfigModifier(
[](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager&
hcm) { hcm.mutable_stream_error_on_invalid_http_message()->set_value(true); });

autonomous_upstream_ = true;
initialize();
Expand Down