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.

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

Jump to bottom

runtime: removing envoy.reloadable_features.grpc_json_transcoder_adhe… #18696

Merged
merged 1 commit into from
Oct 27, 2021
Merged

runtime: removing envoy.reloadable_features.grpc_json_transcoder_adhe… #18696

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
1 change: 0 additions & 1 deletion source/common/runtime/runtime_features.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
11 changes: 0 additions & 11 deletions source/extensions/filters/http/grpc_json_transcoder/json_transcoder_filter.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,6 @@ using RcDetails = ConstSingleton<RcDetailsValues>;

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");
}
Expand Down Expand Up @@ -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 "
Expand All @@ -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 "
Expand Down
67 changes: 0 additions & 67 deletions test/extensions/filters/http/grpc_json_transcoder/grpc_json_transcoder_integration_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<bookstore::CreateShelfRequest, bookstore::Shelf>(
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<bookstore::CreateShelfRequest, bookstore::Shelf>(
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