From 9f2bb040358d561bccecc84dd109d7f8fc512c56 Mon Sep 17 00:00:00 2001 From: Alyssa Wilk Date: Wed, 20 Oct 2021 14:36:39 -0400 Subject: [PATCH] runtime: removing envoy.reloadable_features.grpc_json_transcoder_adhere_to_buffer_limits Signed-off-by: Alyssa Wilk --- docs/root/version_history/current.rst | 1 + source/common/runtime/runtime_features.cc | 1 - .../json_transcoder_filter.cc | 11 --- .../grpc_json_transcoder_integration_test.cc | 67 ------------------- 4 files changed, 1 insertion(+), 79 deletions(-) diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index ea5ea5527770..a5a06743abed 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -29,6 +29,7 @@ Removed Config or Runtime * http: removed ``envoy.reloadable_features.add_and_validate_scheme_header`` and legacy code paths. * http: removed ``envoy.reloadable_features.check_unsupported_typed_per_filter_config``, Envoy will always check unsupported typed per filter config if the filter isn't optional. * http: removed ``envoy.reloadable_features.dont_add_content_length_for_bodiless_requests deprecation`` and legacy code paths. +* http: removed ``envoy.reloadable_features.grpc_json_transcoder_adhere_to_buffer_limits`` and legacy code paths. * http: removed ``envoy.reloadable_features.http2_skip_encoding_empty_trailers`` and legacy code paths. Envoy will always encode empty trailers by sending empty data with ``end_stream`` true (instead of sending empty trailers) for HTTP/2. * http: removed ``envoy.reloadable_features.improved_stream_limit_handling`` and legacy code paths. * http: removed ``envoy.reloadable_features.remove_forked_chromium_url`` and legacy code paths. diff --git a/source/common/runtime/runtime_features.cc b/source/common/runtime/runtime_features.cc index 287e0537b832..1d76f9321f67 100644 --- a/source/common/runtime/runtime_features.cc +++ b/source/common/runtime/runtime_features.cc @@ -63,7 +63,6 @@ constexpr const char* runtime_features[] = { "envoy.reloadable_features.fix_added_trailers", "envoy.reloadable_features.grpc_bridge_stats_disabled", "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.health_check.graceful_goaway_handling", "envoy.reloadable_features.http2_consume_stream_refused_errors", diff --git a/source/extensions/filters/http/grpc_json_transcoder/json_transcoder_filter.cc b/source/extensions/filters/http/grpc_json_transcoder/json_transcoder_filter.cc index 32014e2ed3d8..3bb7f57c5637 100644 --- a/source/extensions/filters/http/grpc_json_transcoder/json_transcoder_filter.cc +++ b/source/extensions/filters/http/grpc_json_transcoder/json_transcoder_filter.cc @@ -62,9 +62,6 @@ using RcDetails = ConstSingleton; namespace { -constexpr absl::string_view buffer_limits_runtime_feature = - "envoy.reloadable_features.grpc_json_transcoder_adhere_to_buffer_limits"; - const Http::LowerCaseString& trailerHeader() { CONSTRUCT_ON_FIRST_USE(Http::LowerCaseString, "trailer"); } @@ -893,10 +890,6 @@ bool JsonTranscoderFilter::maybeConvertGrpcStatus(Grpc::Status::GrpcStatus grpc_ } bool JsonTranscoderFilter::decoderBufferLimitReached(uint64_t buffer_length) { - if (!Runtime::runtimeFeatureEnabled(buffer_limits_runtime_feature)) { - return false; - } - if (buffer_length > decoder_callbacks_->decoderBufferLimit()) { ENVOY_LOG(debug, "Request rejected because the transcoder's internal buffer size exceeds the " @@ -915,10 +908,6 @@ bool JsonTranscoderFilter::decoderBufferLimitReached(uint64_t buffer_length) { } bool JsonTranscoderFilter::encoderBufferLimitReached(uint64_t buffer_length) { - if (!Runtime::runtimeFeatureEnabled(buffer_limits_runtime_feature)) { - return false; - } - if (buffer_length > encoder_callbacks_->encoderBufferLimit()) { ENVOY_LOG(debug, "Response not transcoded because the transcoder's internal buffer size exceeds the " diff --git a/test/extensions/filters/http/grpc_json_transcoder/grpc_json_transcoder_integration_test.cc b/test/extensions/filters/http/grpc_json_transcoder/grpc_json_transcoder_integration_test.cc index fccc8902d00c..474d74e96aa8 100644 --- a/test/extensions/filters/http/grpc_json_transcoder/grpc_json_transcoder_integration_test.cc +++ b/test/extensions/filters/http/grpc_json_transcoder/grpc_json_transcoder_integration_test.cc @@ -1321,72 +1321,5 @@ TEST_P(OverrideConfigGrpcJsonTranscoderIntegrationTest, RouteOverride) { R"({"shelves":[{"id":"20","theme":"Children"},{"id":"1","theme":"Foo"}]})"); }; -// Tests to ensure transcoding buffer limits do not apply when the runtime feature is disabled. -class BufferLimitsDisabledGrpcJsonTranscoderIntegrationTest - : public GrpcJsonTranscoderIntegrationTest { -public: - void SetUp() override { - setUpstreamProtocol(Http::CodecType::HTTP2); - const std::string filter = - R"EOF( - name: grpc_json_transcoder - typed_config: - "@type": type.googleapis.com/envoy.extensions.filters.http.grpc_json_transcoder.v3.GrpcJsonTranscoder - proto_descriptor : "{}" - services : "bookstore.Bookstore" - )EOF"; - config_helper_.prependFilter( - fmt::format(filter, TestEnvironment::runfilesPath("test/proto/bookstore.descriptor"))); - - // Disable runtime feature. - config_helper_.addRuntimeOverride( - "envoy.reloadable_features.grpc_json_transcoder_adhere_to_buffer_limits", "false"); - } -}; -INSTANTIATE_TEST_SUITE_P(IpVersions, BufferLimitsDisabledGrpcJsonTranscoderIntegrationTest, - testing::ValuesIn(TestEnvironment::getIpVersionsForTest()), - TestUtility::ipTestParamsToString); - -TEST_P(BufferLimitsDisabledGrpcJsonTranscoderIntegrationTest, UnaryPostRequestExceedsBufferLimit) { - // Request body is more than 20 bytes. - config_helper_.setBufferLimits(2 << 20, 20); - HttpIntegrationTest::initialize(); - - // Transcoding succeeds. - testTranscoding( - Http::TestRequestHeaderMapImpl{{":method", "POST"}, - {":path", "/shelf"}, - {":authority", "host"}, - {"content-type", "application/json"}}, - R"({"theme": "Children 0123456789 0123456789 0123456789 0123456789"})", - {R"(shelf { theme: "Children 0123456789 0123456789 0123456789 0123456789" })"}, {R"(id: 1)"}, - Status(), - Http::TestResponseHeaderMapImpl{{":status", "200"}, - {"content-type", "application/json"}, - {"content-length", "10"}, - {"grpc-status", "0"}}, - R"({"id":"1"})"); -} - -TEST_P(BufferLimitsDisabledGrpcJsonTranscoderIntegrationTest, UnaryPostResponseExceedsBufferLimit) { - // Request body is less than 35 bytes. - // Response body is more than 35 bytes. - config_helper_.setBufferLimits(2 << 20, 35); - HttpIntegrationTest::initialize(); - - // Transcoding succeeds. However, the downstream client is unable to buffer the full response. - // We can tell these errors are NOT from the transcoder because the response body is too generic. - testTranscoding( - Http::TestRequestHeaderMapImpl{{":method", "POST"}, - {":path", "/shelf"}, - {":authority", "host"}, - {"content-type", "application/json"}}, - R"({"theme": "Children"})", {R"(shelf { theme: "Children" })"}, - {R"(id: 20 theme: "Children 0123456789 0123456789 0123456789 0123456789" )"}, Status(), - Http::TestResponseHeaderMapImpl{ - {":status", "500"}, {"content-type", "text/plain"}, {"content-length", "21"}}, - R"(Internal Server Error)"); -} - } // namespace } // namespace Envoy