From 8dd290cba7911aa1b979999e13e405f50bdf0618 Mon Sep 17 00:00:00 2001 From: Snow Pettersen Date: Wed, 18 Mar 2020 19:47:28 -0700 Subject: [PATCH 1/9] inherit upstream connection filter state when creating upstream filter state Signed-off-by: Snow Pettersen --- include/envoy/stream_info/filter_state.h | 17 ++---- include/envoy/stream_info/stream_info.h | 3 +- .../common/access_log/access_log_formatter.cc | 2 +- source/common/router/header_formatter.cc | 2 +- source/common/router/router.cc | 7 +-- source/common/router/router.h | 4 ++ source/common/router/upstream_request.cc | 5 ++ source/common/stream_info/stream_info_impl.h | 5 +- test/common/router/router_test.cc | 58 ++++++++++++++++++- test/common/stream_info/test_util.h | 5 +- test/mocks/stream_info/mocks.cc | 1 - test/mocks/stream_info/mocks.h | 3 +- 12 files changed, 83 insertions(+), 29 deletions(-) diff --git a/include/envoy/stream_info/filter_state.h b/include/envoy/stream_info/filter_state.h index 0e41d967cf19f..4fe1aeb34b854 100644 --- a/include/envoy/stream_info/filter_state.h +++ b/include/envoy/stream_info/filter_state.h @@ -30,21 +30,16 @@ class FilterState { // // - FilterChain has the shortest life span, which is as long as the filter chain lives. // - // - DownstreamRequest is longer than FilterChain. When internal redirect is enabled, one - // downstream request may create multiple filter chains. DownstreamRequest allows an object to - // survive across filter chains for bookkeeping needs. + // - Request is longer than FilterChain. When internal redirect is enabled, one + // downstream request may create multiple filter chains. Request allows an object to + // survive across filter chains for bookkeeping needs. This is not used for the upstream case. // - // - DownstreamConnection makes an object survive the entire duration of a downstream connection. + // - Connection makes an object survive the entire duration of a connection. // Any stream within this connection can see the same object. // // Note that order matters in this enum because it's assumed that life span grows as enum number // grows. - enum LifeSpan { - FilterChain, - DownstreamRequest, - DownstreamConnection, - TopSpan = DownstreamConnection - }; + enum LifeSpan { FilterChain, Request, Connection, TopSpan = Connection }; class Object { public: @@ -65,7 +60,7 @@ class FilterState { * @param data an owning pointer to the data to be stored. * @param state_type indicates whether the object is mutable or not. * @param life_span indicates the life span of the object: bound to the filter chain, a - * downstream request, or a downstream connection. + * request, or a connection. * * Note that it is an error to call setData() twice with the same * data_name, if the existing object is immutable. Similarly, it is an diff --git a/include/envoy/stream_info/stream_info.h b/include/envoy/stream_info/stream_info.h index 6a0bbd05e2669..7a9197864ce36 100644 --- a/include/envoy/stream_info/stream_info.h +++ b/include/envoy/stream_info/stream_info.h @@ -479,8 +479,7 @@ class StreamInfo { * filters (append only). Both object types can be consumed by multiple filters. * @return the filter state associated with this request. */ - virtual const FilterStateSharedPtr& filterState() PURE; - virtual const FilterState& filterState() const PURE; + virtual const FilterStateSharedPtr& filterState() const PURE; /** * Filter State object to be shared between upstream and downstream filters. diff --git a/source/common/access_log/access_log_formatter.cc b/source/common/access_log/access_log_formatter.cc index 7237fb061fc42..20c4841bdbb9c 100644 --- a/source/common/access_log/access_log_formatter.cc +++ b/source/common/access_log/access_log_formatter.cc @@ -914,7 +914,7 @@ FilterStateFormatter::FilterStateFormatter(const std::string& key, ProtobufTypes::MessagePtr FilterStateFormatter::filterState(const StreamInfo::StreamInfo& stream_info) const { - const StreamInfo::FilterState& filter_state = stream_info.filterState(); + const StreamInfo::FilterState& filter_state = *stream_info.filterState(); if (!filter_state.hasDataWithName(key_)) { return nullptr; } diff --git a/source/common/router/header_formatter.cc b/source/common/router/header_formatter.cc index aacf4d9a37120..d0a6e0164a300 100644 --- a/source/common/router/header_formatter.cc +++ b/source/common/router/header_formatter.cc @@ -144,7 +144,7 @@ parsePerRequestStateField(absl::string_view param_str) { std::string param(modified_param_str); return [param](const Envoy::StreamInfo::StreamInfo& stream_info) -> std::string { - const Envoy::StreamInfo::FilterState& filter_state = stream_info.filterState(); + const Envoy::StreamInfo::FilterState& filter_state = *stream_info.filterState(); // No such value means don't output anything. if (!filter_state.hasDataWithName(param)) { diff --git a/source/common/router/router.cc b/source/common/router/router.cc index 89210734c2d06..5392497944066 100644 --- a/source/common/router/router.cc +++ b/source/common/router/router.cc @@ -83,10 +83,9 @@ bool convertRequestHeadersForInternalRedirect(Http::RequestHeaderMap& downstream // Make sure that performing the redirect won't result in exceeding the configured number of // redirects allowed for this route. if (!filter_state.hasData(NumInternalRedirectsFilterStateName)) { - filter_state.setData(NumInternalRedirectsFilterStateName, - std::make_shared(0), - StreamInfo::FilterState::StateType::Mutable, - StreamInfo::FilterState::LifeSpan::DownstreamRequest); + filter_state.setData( + NumInternalRedirectsFilterStateName, std::make_shared(0), + StreamInfo::FilterState::StateType::Mutable, StreamInfo::FilterState::LifeSpan::Request); } StreamInfo::UInt32Accessor& num_internal_redirect = filter_state.getDataMutable(NumInternalRedirectsFilterStateName); diff --git a/source/common/router/router.h b/source/common/router/router.h index 62bdb9d212f6b..85036250a2c0d 100644 --- a/source/common/router/router.h +++ b/source/common/router/router.h @@ -448,7 +448,11 @@ class Filter : Logger::Loggable, TimeSource& timeSource() { return config_.timeSource(); } Http::Context& httpContext() { return config_.http_context_; } + // This makes it accessible to the RouterTest class. +protected: FilterConfig& config_; + +private: Http::StreamDecoderFilterCallbacks* callbacks_{}; RouteConstSharedPtr route_; const RouteEntry* route_entry_{}; diff --git a/source/common/router/upstream_request.cc b/source/common/router/upstream_request.cc index 400639a1e88e8..7cf9eb6f8e336 100644 --- a/source/common/router/upstream_request.cc +++ b/source/common/router/upstream_request.cc @@ -324,6 +324,11 @@ void UpstreamRequest::onPoolReady(Http::RequestEncoder& request_encoder, onUpstreamHostSelected(host); + ENVOY_LOG_MISC(info, "info.filterstate: {}", info.filterState()->hasDataWithName("foo")); + stream_info_.setUpstreamFilterState(std::make_shared( + info.filterState()->parent()->parent(), StreamInfo::FilterState::LifeSpan::Request)); + ENVOY_LOG_MISC(info, "stream info upstream: {}", + stream_info_.upstreamFilterState()->hasDataWithName("foo")); stream_info_.setUpstreamLocalAddress(request_encoder.getStream().connectionLocalAddress()); parent_.callbacks_->streamInfo().setUpstreamLocalAddress( request_encoder.getStream().connectionLocalAddress()); diff --git a/source/common/stream_info/stream_info_impl.h b/source/common/stream_info/stream_info_impl.h index 00707bf9c6b87..091c90ad268aa 100644 --- a/source/common/stream_info/stream_info_impl.h +++ b/source/common/stream_info/stream_info_impl.h @@ -32,7 +32,7 @@ struct StreamInfoImpl : public StreamInfo { start_time_monotonic_(time_source.monotonicTime()), protocol_(protocol), filter_state_(std::make_shared( FilterStateImpl::LazyCreateAncestor(parent_filter_state, - FilterState::LifeSpan::DownstreamConnection), + FilterState::LifeSpan::Connection), FilterState::LifeSpan::FilterChain)) {} SystemTime startTime() const override { return start_time_; } @@ -216,8 +216,7 @@ struct StreamInfoImpl : public StreamInfo { (*metadata_.mutable_filter_metadata())[name].MergeFrom(value); }; - const FilterStateSharedPtr& filterState() override { return filter_state_; } - const FilterState& filterState() const override { return *filter_state_; } + const FilterStateSharedPtr& filterState() const override { return filter_state_; } const FilterStateSharedPtr& upstreamFilterState() const override { return upstream_filter_state_; diff --git a/test/common/router/router_test.cc b/test/common/router/router_test.cc index 208d01f0f68ef..4ac7604d6a4fe 100644 --- a/test/common/router/router_test.cc +++ b/test/common/router/router_test.cc @@ -1,5 +1,6 @@ #include #include +#include #include #include "envoy/config/core/v3/base.pb.h" @@ -83,6 +84,8 @@ class RouterTestFilter : public Filter { return &downstream_connection_; } + FilterConfig& config() { return config_; } + NiceMock downstream_connection_; MockRetryState* retry_state_{}; bool reject_all_hosts_ = false; @@ -238,8 +241,7 @@ class RouterTestBase : public testing::Test { callbacks_.streamInfo().filterState()->setData( "num_internal_redirects", std::make_shared(num_previous_redirects), - StreamInfo::FilterState::StateType::Mutable, - StreamInfo::FilterState::LifeSpan::DownstreamRequest); + StreamInfo::FilterState::StateType::Mutable, StreamInfo::FilterState::LifeSpan::Request); } void enableHedgeOnPerTryTimeout() { @@ -3826,6 +3828,58 @@ TEST_F(RouterTest, DirectResponseWithoutLocation) { EXPECT_EQ(1UL, config_.stats_.rq_direct_response_.value()); } +// Allows verifying the state of the upstream StreamInfo +class TestAccessLog : public AccessLog::Instance { +public: + explicit TestAccessLog(std::function func) : func_(func) {} + + void log(const Http::RequestHeaderMap*, const Http::ResponseHeaderMap*, + const Http::ResponseTrailerMap*, const StreamInfo::StreamInfo& info) override { + func_(info); + } + +private: + std::function func_; +}; + +// Verifies that we propagate the upstream connection filter state to the upstream request filter +// state. +TEST_F(RouterTest, PropagatesUpstreamFilterState) { + NiceMock encoder; + Http::ResponseDecoder* response_decoder = nullptr; + + // This pattern helps ensure that we're actually invoking the callback. + bool filter_state_verified = false; + router_.config().upstream_logs_.push_back( + std::make_shared([&](const auto& stream_info) { + filter_state_verified = stream_info.upstreamFilterState()->hasDataWithName("foo"); + })); + + upstream_stream_info_.filterState()->setData( + "foo", std::make_unique(123), + StreamInfo::FilterState::StateType::ReadOnly, StreamInfo::FilterState::LifeSpan::Connection); + expectResponseTimerCreate(); + EXPECT_CALL(cm_.conn_pool_, newStream(_, _)) + .WillOnce(Invoke( + [&](Http::ResponseDecoder& decoder, + Http::ConnectionPool::Callbacks& callbacks) -> Http::ConnectionPool::Cancellable* { + response_decoder = &decoder; + callbacks.onPoolReady(encoder, cm_.conn_pool_.host_, upstream_stream_info_); + return nullptr; + })); + + Http::TestRequestHeaderMapImpl headers{}; + HttpTestUtility::addDefaultHeaders(headers); + router_.decodeHeaders(headers, true); + + Http::ResponseHeaderMapPtr response_headers( + new Http::TestResponseHeaderMapImpl{{":status", "200"}}); + response_decoder->decodeHeaders(std::move(response_headers), true); + EXPECT_TRUE(verifyHostUpstreamStats(1, 0)); + + EXPECT_TRUE(filter_state_verified); +} + TEST_F(RouterTest, UpstreamSSLConnection) { NiceMock encoder; Http::ResponseDecoder* response_decoder = nullptr; diff --git a/test/common/stream_info/test_util.h b/test/common/stream_info/test_util.h index 81d47b23924ec..bf7a08b534ffa 100644 --- a/test/common/stream_info/test_util.h +++ b/test/common/stream_info/test_util.h @@ -176,8 +176,9 @@ class TestStreamInfo : public StreamInfo::StreamInfo { (*metadata_.mutable_filter_metadata())[name].MergeFrom(value); }; - const Envoy::StreamInfo::FilterStateSharedPtr& filterState() override { return filter_state_; } - const Envoy::StreamInfo::FilterState& filterState() const override { return *filter_state_; } + const Envoy::StreamInfo::FilterStateSharedPtr& filterState() const override { + return filter_state_; + } const Envoy::StreamInfo::FilterStateSharedPtr& upstreamFilterState() const override { return upstream_filter_state_; diff --git a/test/mocks/stream_info/mocks.cc b/test/mocks/stream_info/mocks.cc index 59219bffcc182..377ee15bb7cfd 100644 --- a/test/mocks/stream_info/mocks.cc +++ b/test/mocks/stream_info/mocks.cc @@ -109,7 +109,6 @@ MockStreamInfo::MockStreamInfo() ON_CALL(*this, dynamicMetadata()).WillByDefault(ReturnRef(metadata_)); ON_CALL(Const(*this), dynamicMetadata()).WillByDefault(ReturnRef(metadata_)); ON_CALL(*this, filterState()).WillByDefault(ReturnRef(filter_state_)); - ON_CALL(Const(*this), filterState()).WillByDefault(ReturnRef(*filter_state_)); ON_CALL(*this, upstreamFilterState()).WillByDefault(ReturnRef(upstream_filter_state_)); ON_CALL(*this, setUpstreamFilterState(_)) .WillByDefault(Invoke([this](const FilterStateSharedPtr& filter_state) { diff --git a/test/mocks/stream_info/mocks.h b/test/mocks/stream_info/mocks.h index 9a49ad4220027..ce48b79a5dbe3 100644 --- a/test/mocks/stream_info/mocks.h +++ b/test/mocks/stream_info/mocks.h @@ -79,8 +79,7 @@ class MockStreamInfo : public StreamInfo { MOCK_METHOD(void, setDynamicMetadata, (const std::string&, const ProtobufWkt::Struct&)); MOCK_METHOD(void, setDynamicMetadata, (const std::string&, const std::string&, const std::string&)); - MOCK_METHOD(const FilterStateSharedPtr&, filterState, ()); - MOCK_METHOD(const FilterState&, filterState, (), (const)); + MOCK_METHOD(const FilterStateSharedPtr&, filterState, (), (const)); MOCK_METHOD(const FilterStateSharedPtr&, upstreamFilterState, (), (const)); MOCK_METHOD(void, setUpstreamFilterState, (const FilterStateSharedPtr&)); MOCK_METHOD(void, setRequestedServerName, (const absl::string_view)); From dcf99282c63826921a2f89be97faf90e6f78d98f Mon Sep 17 00:00:00 2001 From: Snow Pettersen Date: Thu, 19 Mar 2020 13:28:50 -0700 Subject: [PATCH 2/9] update references to lifespan enum Signed-off-by: Snow Pettersen --- source/common/http/conn_manager_impl.cc | 2 +- .../network/sni_cluster/sni_cluster.cc | 3 +- test/common/http/conn_manager_impl_test.cc | 4 +- .../stream_info/filter_state_impl_test.cc | 67 ++++++++----------- test/common/tcp_proxy/tcp_proxy_test.cc | 15 ++--- 5 files changed, 36 insertions(+), 55 deletions(-) diff --git a/source/common/http/conn_manager_impl.cc b/source/common/http/conn_manager_impl.cc index 946d61a779b7b..42022b513c0a4 100644 --- a/source/common/http/conn_manager_impl.cc +++ b/source/common/http/conn_manager_impl.cc @@ -2366,7 +2366,7 @@ bool ConnectionManagerImpl::ActiveStreamDecoderFilter::recreateStream() { // store any objects with a LifeSpan at or above DownstreamRequest. This is to avoid unnecessary // heap allocation. if (parent_.stream_info_.filter_state_->hasDataAtOrAboveLifeSpan( - StreamInfo::FilterState::LifeSpan::DownstreamRequest)) { + StreamInfo::FilterState::LifeSpan::Request)) { (*parent_.connection_manager_.streams_.begin())->stream_info_.filter_state_ = std::make_shared( parent_.stream_info_.filter_state_->parent(), diff --git a/source/extensions/filters/network/sni_cluster/sni_cluster.cc b/source/extensions/filters/network/sni_cluster/sni_cluster.cc index eb1694647b75c..876eca758a942 100644 --- a/source/extensions/filters/network/sni_cluster/sni_cluster.cc +++ b/source/extensions/filters/network/sni_cluster/sni_cluster.cc @@ -21,8 +21,7 @@ Network::FilterStatus SniClusterFilter::onNewConnection() { read_callbacks_->connection().streamInfo().filterState()->setData( TcpProxy::PerConnectionCluster::key(), std::make_unique(sni), - StreamInfo::FilterState::StateType::Mutable, - StreamInfo::FilterState::LifeSpan::DownstreamConnection); + StreamInfo::FilterState::StateType::Mutable, StreamInfo::FilterState::LifeSpan::Connection); } return Network::FilterStatus::Continue; diff --git a/test/common/http/conn_manager_impl_test.cc b/test/common/http/conn_manager_impl_test.cc index 451adf7fe90b8..3f6ecc4522777 100644 --- a/test/common/http/conn_manager_impl_test.cc +++ b/test/common/http/conn_manager_impl_test.cc @@ -5330,11 +5330,11 @@ TEST_F(HttpConnectionManagerImplTest, ConnectionFilterState) { decoder_filters_[0]->callbacks_->streamInfo().filterState()->setData( "per_downstream_request", std::make_unique(2), StreamInfo::FilterState::StateType::ReadOnly, - StreamInfo::FilterState::LifeSpan::DownstreamRequest); + StreamInfo::FilterState::LifeSpan::Request); decoder_filters_[0]->callbacks_->streamInfo().filterState()->setData( "per_downstream_connection", std::make_unique(3), StreamInfo::FilterState::StateType::ReadOnly, - StreamInfo::FilterState::LifeSpan::DownstreamConnection); + StreamInfo::FilterState::LifeSpan::Connection); return FilterHeadersStatus::StopIteration; })); EXPECT_CALL(*decoder_filters_[1], decodeHeaders(_, true)) diff --git a/test/common/stream_info/filter_state_impl_test.cc b/test/common/stream_info/filter_state_impl_test.cc index 4c2659463adae..24590fa7c3715 100644 --- a/test/common/stream_info/filter_state_impl_test.cc +++ b/test/common/stream_info/filter_state_impl_test.cc @@ -249,15 +249,13 @@ TEST_F(FilterStateImplTest, LifeSpanInitFromParent) { filter_state().setData("test_2", std::make_unique(2), FilterState::StateType::Mutable, FilterState::LifeSpan::FilterChain); filter_state().setData("test_3", std::make_unique(3), - FilterState::StateType::ReadOnly, - FilterState::LifeSpan::DownstreamRequest); + FilterState::StateType::ReadOnly, FilterState::LifeSpan::Request); filter_state().setData("test_4", std::make_unique(4), FilterState::StateType::Mutable, - FilterState::LifeSpan::DownstreamRequest); + FilterState::LifeSpan::Request); filter_state().setData("test_5", std::make_unique(5), - FilterState::StateType::ReadOnly, - FilterState::LifeSpan::DownstreamConnection); + FilterState::StateType::ReadOnly, FilterState::LifeSpan::Connection); filter_state().setData("test_6", std::make_unique(6), FilterState::StateType::Mutable, - FilterState::LifeSpan::DownstreamConnection); + FilterState::LifeSpan::Connection); FilterStateImpl new_filter_state(filter_state().parent(), FilterState::LifeSpan::FilterChain); EXPECT_FALSE(new_filter_state.hasDataWithName("test_1")); @@ -282,15 +280,13 @@ TEST_F(FilterStateImplTest, LifeSpanInitFromGrandparent) { filter_state().setData("test_2", std::make_unique(2), FilterState::StateType::Mutable, FilterState::LifeSpan::FilterChain); filter_state().setData("test_3", std::make_unique(3), - FilterState::StateType::ReadOnly, - FilterState::LifeSpan::DownstreamRequest); + FilterState::StateType::ReadOnly, FilterState::LifeSpan::Request); filter_state().setData("test_4", std::make_unique(4), FilterState::StateType::Mutable, - FilterState::LifeSpan::DownstreamRequest); + FilterState::LifeSpan::Request); filter_state().setData("test_5", std::make_unique(5), - FilterState::StateType::ReadOnly, - FilterState::LifeSpan::DownstreamConnection); + FilterState::StateType::ReadOnly, FilterState::LifeSpan::Connection); filter_state().setData("test_6", std::make_unique(6), FilterState::StateType::Mutable, - FilterState::LifeSpan::DownstreamConnection); + FilterState::LifeSpan::Connection); FilterStateImpl new_filter_state(filter_state().parent()->parent(), FilterState::LifeSpan::FilterChain); @@ -312,18 +308,15 @@ TEST_F(FilterStateImplTest, LifeSpanInitFromNonParent) { filter_state().setData("test_2", std::make_unique(2), FilterState::StateType::Mutable, FilterState::LifeSpan::FilterChain); filter_state().setData("test_3", std::make_unique(3), - FilterState::StateType::ReadOnly, - FilterState::LifeSpan::DownstreamRequest); + FilterState::StateType::ReadOnly, FilterState::LifeSpan::Request); filter_state().setData("test_4", std::make_unique(4), FilterState::StateType::Mutable, - FilterState::LifeSpan::DownstreamRequest); + FilterState::LifeSpan::Request); filter_state().setData("test_5", std::make_unique(5), - FilterState::StateType::ReadOnly, - FilterState::LifeSpan::DownstreamConnection); + FilterState::StateType::ReadOnly, FilterState::LifeSpan::Connection); filter_state().setData("test_6", std::make_unique(6), FilterState::StateType::Mutable, - FilterState::LifeSpan::DownstreamConnection); + FilterState::LifeSpan::Connection); - FilterStateImpl new_filter_state(filter_state().parent(), - FilterState::LifeSpan::DownstreamRequest); + FilterStateImpl new_filter_state(filter_state().parent(), FilterState::LifeSpan::Request); EXPECT_FALSE(new_filter_state.hasDataWithName("test_1")); EXPECT_FALSE(new_filter_state.hasDataWithName("test_2")); EXPECT_FALSE(new_filter_state.hasDataWithName("test_3")); @@ -336,29 +329,25 @@ TEST_F(FilterStateImplTest, HasDataAtOrAboveLifeSpan) { filter_state().setData("test_1", std::make_unique(1), FilterState::StateType::ReadOnly, FilterState::LifeSpan::FilterChain); EXPECT_TRUE(filter_state().hasDataAtOrAboveLifeSpan(FilterState::LifeSpan::FilterChain)); - EXPECT_FALSE(filter_state().hasDataAtOrAboveLifeSpan(FilterState::LifeSpan::DownstreamRequest)); - EXPECT_FALSE( - filter_state().hasDataAtOrAboveLifeSpan(FilterState::LifeSpan::DownstreamConnection)); + EXPECT_FALSE(filter_state().hasDataAtOrAboveLifeSpan(FilterState::LifeSpan::Request)); + EXPECT_FALSE(filter_state().hasDataAtOrAboveLifeSpan(FilterState::LifeSpan::Connection)); filter_state().setData("test_2", std::make_unique(2), - FilterState::StateType::ReadOnly, - FilterState::LifeSpan::DownstreamRequest); + FilterState::StateType::ReadOnly, FilterState::LifeSpan::Request); EXPECT_TRUE(filter_state().hasDataAtOrAboveLifeSpan(FilterState::LifeSpan::FilterChain)); - EXPECT_TRUE(filter_state().hasDataAtOrAboveLifeSpan(FilterState::LifeSpan::DownstreamRequest)); - EXPECT_FALSE( - filter_state().hasDataAtOrAboveLifeSpan(FilterState::LifeSpan::DownstreamConnection)); + EXPECT_TRUE(filter_state().hasDataAtOrAboveLifeSpan(FilterState::LifeSpan::Request)); + EXPECT_FALSE(filter_state().hasDataAtOrAboveLifeSpan(FilterState::LifeSpan::Connection)); filter_state().setData("test_3", std::make_unique(3), - FilterState::StateType::ReadOnly, - FilterState::LifeSpan::DownstreamConnection); + FilterState::StateType::ReadOnly, FilterState::LifeSpan::Connection); EXPECT_TRUE(filter_state().hasDataAtOrAboveLifeSpan(FilterState::LifeSpan::FilterChain)); - EXPECT_TRUE(filter_state().hasDataAtOrAboveLifeSpan(FilterState::LifeSpan::DownstreamRequest)); - EXPECT_TRUE(filter_state().hasDataAtOrAboveLifeSpan(FilterState::LifeSpan::DownstreamConnection)); + EXPECT_TRUE(filter_state().hasDataAtOrAboveLifeSpan(FilterState::LifeSpan::Request)); + EXPECT_TRUE(filter_state().hasDataAtOrAboveLifeSpan(FilterState::LifeSpan::Connection)); } TEST_F(FilterStateImplTest, SetSameDataWithDifferentLifeSpan) { filter_state().setData("test_1", std::make_unique(1), FilterState::StateType::Mutable, - FilterState::LifeSpan::DownstreamConnection); + FilterState::LifeSpan::Connection); // Test reset on smaller LifeSpan EXPECT_THROW_WITH_MESSAGE( filter_state().setData("test_1", std::make_unique(2), @@ -367,18 +356,17 @@ TEST_F(FilterStateImplTest, SetSameDataWithDifferentLifeSpan) { "FilterState::setData called twice with conflicting life_span on the same data_name."); EXPECT_THROW_WITH_MESSAGE( filter_state().setData("test_1", std::make_unique(2), - FilterState::StateType::Mutable, - FilterState::LifeSpan::DownstreamRequest), + FilterState::StateType::Mutable, FilterState::LifeSpan::Request), EnvoyException, "FilterState::setData called twice with conflicting life_span on the same data_name."); // Still mutable on the correct LifeSpan. filter_state().setData("test_1", std::make_unique(2), FilterState::StateType::Mutable, - FilterState::LifeSpan::DownstreamConnection); + FilterState::LifeSpan::Connection); EXPECT_EQ(2, filter_state().getDataMutable("test_1").access()); filter_state().setData("test_2", std::make_unique(1), FilterState::StateType::Mutable, - FilterState::LifeSpan::DownstreamRequest); + FilterState::LifeSpan::Request); // Test reset on smaller and greater LifeSpan EXPECT_THROW_WITH_MESSAGE( filter_state().setData("test_2", std::make_unique(2), @@ -387,14 +375,13 @@ TEST_F(FilterStateImplTest, SetSameDataWithDifferentLifeSpan) { "FilterState::setData called twice with conflicting life_span on the same data_name."); EXPECT_THROW_WITH_MESSAGE( filter_state().setData("test_2", std::make_unique(2), - FilterState::StateType::Mutable, - FilterState::LifeSpan::DownstreamConnection), + FilterState::StateType::Mutable, FilterState::LifeSpan::Connection), EnvoyException, "FilterState::setData called twice with conflicting life_span on the same data_name."); // Still mutable on the correct LifeSpan. filter_state().setData("test_2", std::make_unique(2), FilterState::StateType::Mutable, - FilterState::LifeSpan::DownstreamRequest); + FilterState::LifeSpan::Request); EXPECT_EQ(2, filter_state().getDataMutable("test_2").access()); } diff --git a/test/common/tcp_proxy/tcp_proxy_test.cc b/test/common/tcp_proxy/tcp_proxy_test.cc index 80d676f5b97a0..835941eb193b8 100644 --- a/test/common/tcp_proxy/tcp_proxy_test.cc +++ b/test/common/tcp_proxy/tcp_proxy_test.cc @@ -752,8 +752,7 @@ TEST(ConfigTest, PerConnectionClusterWithTopLevelMetadataMatchConfig) { NiceMock connection; connection.stream_info_.filterState()->setData( "envoy.tcp_proxy.cluster", std::make_unique("filter_state_cluster"), - StreamInfo::FilterState::StateType::Mutable, - StreamInfo::FilterState::LifeSpan::DownstreamConnection); + StreamInfo::FilterState::StateType::Mutable, StreamInfo::FilterState::LifeSpan::Connection); const auto route = config_obj.getRouteFromEntries(connection); EXPECT_NE(nullptr, route); @@ -1742,8 +1741,7 @@ TEST_F(TcpProxyTest, ShareFilterState) { upstream_connections_.at(0)->streamInfo().filterState()->setData( "envoy.tcp_proxy.cluster", std::make_unique("filter_state_cluster"), - StreamInfo::FilterState::StateType::Mutable, - StreamInfo::FilterState::LifeSpan::DownstreamConnection); + StreamInfo::FilterState::StateType::Mutable, StreamInfo::FilterState::LifeSpan::Connection); raiseEventUpstreamConnected(0); EXPECT_EQ("filter_state_cluster", filter_callbacks_.connection_.streamInfo() @@ -1851,8 +1849,7 @@ TEST_F(TcpProxyRoutingTest, DEPRECATED_FEATURE_TEST(UseClusterFromPerConnectionC connection_.streamInfo().filterState()->setData( "envoy.tcp_proxy.cluster", std::make_unique("filter_state_cluster"), - StreamInfo::FilterState::StateType::Mutable, - StreamInfo::FilterState::LifeSpan::DownstreamConnection); + StreamInfo::FilterState::StateType::Mutable, StreamInfo::FilterState::LifeSpan::Connection); // Expect filter to try to open a connection to specified cluster. EXPECT_CALL(factory_context_.cluster_manager_, @@ -1869,8 +1866,7 @@ TEST_F(TcpProxyRoutingTest, DEPRECATED_FEATURE_TEST(UpstreamServerName)) { connection_.streamInfo().filterState()->setData( "envoy.network.upstream_server_name", std::make_unique("www.example.com"), - StreamInfo::FilterState::StateType::ReadOnly, - StreamInfo::FilterState::LifeSpan::DownstreamConnection); + StreamInfo::FilterState::StateType::ReadOnly, StreamInfo::FilterState::LifeSpan::Connection); // Expect filter to try to open a connection to a cluster with the transport socket options with // override-server-name @@ -1901,8 +1897,7 @@ TEST_F(TcpProxyRoutingTest, DEPRECATED_FEATURE_TEST(ApplicationProtocols)) { connection_.streamInfo().filterState()->setData( Network::ApplicationProtocols::key(), std::make_unique(std::vector{"foo", "bar"}), - StreamInfo::FilterState::StateType::ReadOnly, - StreamInfo::FilterState::LifeSpan::DownstreamConnection); + StreamInfo::FilterState::StateType::ReadOnly, StreamInfo::FilterState::LifeSpan::Connection); // Expect filter to try to open a connection to a cluster with the transport socket options with // override-application-protocol From 736c181d085eef55b0bfb45441eff8696680de73 Mon Sep 17 00:00:00 2001 From: Snow Pettersen Date: Thu, 19 Mar 2020 13:58:39 -0700 Subject: [PATCH 3/9] filterState() now returns a pointer Signed-off-by: Snow Pettersen --- .../extensions/access_loggers/grpc/grpc_access_log_utils.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source/extensions/access_loggers/grpc/grpc_access_log_utils.cc b/source/extensions/access_loggers/grpc/grpc_access_log_utils.cc index 1d187bc299852..ae2875a628d90 100644 --- a/source/extensions/access_loggers/grpc/grpc_access_log_utils.cc +++ b/source/extensions/access_loggers/grpc/grpc_access_log_utils.cc @@ -239,9 +239,9 @@ void Utility::extractCommonAccessLogProperties( } for (const auto& key : config.filter_state_objects_to_log()) { - if (stream_info.filterState().hasDataWithName(key)) { + if (stream_info.filterState()->hasDataWithName(key)) { const auto& obj = - stream_info.filterState().getDataReadOnly(key); + stream_info.filterState()->getDataReadOnly(key); ProtobufTypes::MessagePtr serialized_proto = obj.serializeAsProto(); if (serialized_proto != nullptr) { auto& filter_state_objects = *common_access_log.mutable_filter_state_objects(); From 0a4d2d1a55ecd8304ca3e5ca2715bab95d00a384 Mon Sep 17 00:00:00 2001 From: Snow Pettersen Date: Thu, 19 Mar 2020 14:35:40 -0700 Subject: [PATCH 4/9] more ptr access Signed-off-by: Snow Pettersen --- source/common/tcp_proxy/tcp_proxy.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/common/tcp_proxy/tcp_proxy.cc b/source/common/tcp_proxy/tcp_proxy.cc index bd299ff35c9e3..e3e9f77bfd8a3 100644 --- a/source/common/tcp_proxy/tcp_proxy.cc +++ b/source/common/tcp_proxy/tcp_proxy.cc @@ -407,7 +407,7 @@ Network::FilterStatus Filter::initializeUpstreamConnection() { if (downstreamConnection()) { transport_socket_options_ = Network::TransportSocketOptionsUtility::fromFilterState( - downstreamConnection()->streamInfo().filterState()); + *downstreamConnection()->streamInfo().filterState()); } Tcp::ConnectionPool::Instance* conn_pool = cluster_manager_.tcpConnPoolForCluster( From 0f7901cf598ffd922895ee5e5ee9907e85d9d2c1 Mon Sep 17 00:00:00 2001 From: Snow Pettersen Date: Thu, 19 Mar 2020 14:47:15 -0700 Subject: [PATCH 5/9] remove debug logs Signed-off-by: Snow Pettersen --- source/common/router/upstream_request.cc | 3 --- 1 file changed, 3 deletions(-) diff --git a/source/common/router/upstream_request.cc b/source/common/router/upstream_request.cc index 7cf9eb6f8e336..28ffef1ddbe69 100644 --- a/source/common/router/upstream_request.cc +++ b/source/common/router/upstream_request.cc @@ -324,11 +324,8 @@ void UpstreamRequest::onPoolReady(Http::RequestEncoder& request_encoder, onUpstreamHostSelected(host); - ENVOY_LOG_MISC(info, "info.filterstate: {}", info.filterState()->hasDataWithName("foo")); stream_info_.setUpstreamFilterState(std::make_shared( info.filterState()->parent()->parent(), StreamInfo::FilterState::LifeSpan::Request)); - ENVOY_LOG_MISC(info, "stream info upstream: {}", - stream_info_.upstreamFilterState()->hasDataWithName("foo")); stream_info_.setUpstreamLocalAddress(request_encoder.getStream().connectionLocalAddress()); parent_.callbacks_->streamInfo().setUpstreamLocalAddress( request_encoder.getStream().connectionLocalAddress()); From e436589cffda9ba39cb5517e60e5433e6e547272 Mon Sep 17 00:00:00 2001 From: Snow Pettersen Date: Thu, 19 Mar 2020 15:26:33 -0700 Subject: [PATCH 6/9] remove redundant mocks Signed-off-by: Snow Pettersen --- test/common/router/header_formatter_test.cc | 4 ---- 1 file changed, 4 deletions(-) diff --git a/test/common/router/header_formatter_test.cc b/test/common/router/header_formatter_test.cc index 8f87363613e38..59e3251f0f6a6 100644 --- a/test/common/router/header_formatter_test.cc +++ b/test/common/router/header_formatter_test.cc @@ -580,7 +580,6 @@ TEST_F(StreamInfoHeaderFormatterTest, TestFormatWithPerRequestStateVariable) { NiceMock stream_info; ON_CALL(stream_info, filterState()).WillByDefault(ReturnRef(filter_state)); - ON_CALL(Const(stream_info), filterState()).WillByDefault(ReturnRef(*filter_state)); testFormatting(stream_info, "PER_REQUEST_STATE(testing)", "test_value"); testFormatting(stream_info, "PER_REQUEST_STATE(testing2)", ""); @@ -598,7 +597,6 @@ TEST_F(StreamInfoHeaderFormatterTest, TestFormatWithNonStringPerRequestStateVari NiceMock stream_info; ON_CALL(stream_info, filterState()).WillByDefault(ReturnRef(filter_state)); - ON_CALL(Const(stream_info), filterState()).WillByDefault(ReturnRef(*filter_state)); testFormatting(stream_info, "PER_REQUEST_STATE(testing)", ""); } @@ -852,7 +850,6 @@ TEST(HeaderParserTest, TestParseInternal) { StreamInfo::FilterState::StateType::ReadOnly, StreamInfo::FilterState::LifeSpan::FilterChain); ON_CALL(stream_info, filterState()).WillByDefault(ReturnRef(filter_state)); - ON_CALL(Const(stream_info), filterState()).WillByDefault(ReturnRef(*filter_state)); for (const auto& test_case : test_cases) { Protobuf::RepeatedPtrField to_add; @@ -1023,7 +1020,6 @@ request_headers_to_remove: ["x-nope"] StreamInfo::FilterState::StateType::ReadOnly, StreamInfo::FilterState::LifeSpan::FilterChain); ON_CALL(stream_info, filterState()).WillByDefault(ReturnRef(filter_state)); - ON_CALL(Const(stream_info), filterState()).WillByDefault(ReturnRef(*filter_state)); req_header_parser->evaluateHeaders(header_map, stream_info); From 7d37607f04b4e16febcb8e2064bbda71b934a752 Mon Sep 17 00:00:00 2001 From: Snow Pettersen Date: Mon, 30 Mar 2020 17:45:07 -0700 Subject: [PATCH 7/9] pass non-const handle in onPoolReady Signed-off-by: Snow Pettersen --- include/envoy/stream_info/stream_info.h | 3 ++- source/common/access_log/access_log_formatter.cc | 2 +- source/common/router/header_formatter.cc | 2 +- source/common/router/upstream_request.cc | 2 +- source/common/stream_info/stream_info_impl.h | 3 ++- source/common/tcp_proxy/tcp_proxy.cc | 2 +- test/common/router/header_formatter_test.cc | 4 ++++ test/common/router/router_test.cc | 4 ++-- test/common/stream_info/test_util.h | 5 ++--- test/mocks/stream_info/mocks.cc | 1 + test/mocks/stream_info/mocks.h | 3 ++- 11 files changed, 19 insertions(+), 12 deletions(-) diff --git a/include/envoy/stream_info/stream_info.h b/include/envoy/stream_info/stream_info.h index e641befe118b2..1afda3a2336a3 100644 --- a/include/envoy/stream_info/stream_info.h +++ b/include/envoy/stream_info/stream_info.h @@ -487,7 +487,8 @@ class StreamInfo { * filters (append only). Both object types can be consumed by multiple filters. * @return the filter state associated with this request. */ - virtual const FilterStateSharedPtr& filterState() const PURE; + virtual const FilterStateSharedPtr& filterState() PURE; + virtual const FilterState& filterState() const PURE; /** * Filter State object to be shared between upstream and downstream filters. diff --git a/source/common/access_log/access_log_formatter.cc b/source/common/access_log/access_log_formatter.cc index 0c9e3c543674b..66b9d353158cf 100644 --- a/source/common/access_log/access_log_formatter.cc +++ b/source/common/access_log/access_log_formatter.cc @@ -931,7 +931,7 @@ FilterStateFormatter::FilterStateFormatter(const std::string& key, ProtobufTypes::MessagePtr FilterStateFormatter::filterState(const StreamInfo::StreamInfo& stream_info) const { - const StreamInfo::FilterState& filter_state = *stream_info.filterState(); + const StreamInfo::FilterState& filter_state = stream_info.filterState(); if (!filter_state.hasDataWithName(key_)) { return nullptr; } diff --git a/source/common/router/header_formatter.cc b/source/common/router/header_formatter.cc index 3c706ea756c9a..fd84519f722dd 100644 --- a/source/common/router/header_formatter.cc +++ b/source/common/router/header_formatter.cc @@ -144,7 +144,7 @@ parsePerRequestStateField(absl::string_view param_str) { std::string param(modified_param_str); return [param](const Envoy::StreamInfo::StreamInfo& stream_info) -> std::string { - const Envoy::StreamInfo::FilterState& filter_state = *stream_info.filterState(); + const Envoy::StreamInfo::FilterState& filter_state = stream_info.filterState(); // No such value means don't output anything. if (!filter_state.hasDataWithName(param)) { diff --git a/source/common/router/upstream_request.cc b/source/common/router/upstream_request.cc index e741258cf69d9..38b8636b71ddf 100644 --- a/source/common/router/upstream_request.cc +++ b/source/common/router/upstream_request.cc @@ -330,7 +330,7 @@ void UpstreamRequest::onPoolReady( onUpstreamHostSelected(host); stream_info_.setUpstreamFilterState(std::make_shared( - info.filterState()->parent()->parent(), StreamInfo::FilterState::LifeSpan::Request)); + info.filterState().parent()->parent(), StreamInfo::FilterState::LifeSpan::Request)); stream_info_.setUpstreamLocalAddress(upstream_local_address); parent_.callbacks_->streamInfo().setUpstreamLocalAddress(upstream_local_address); diff --git a/source/common/stream_info/stream_info_impl.h b/source/common/stream_info/stream_info_impl.h index c0ce992b045da..dd41982e2abfb 100644 --- a/source/common/stream_info/stream_info_impl.h +++ b/source/common/stream_info/stream_info_impl.h @@ -217,7 +217,8 @@ struct StreamInfoImpl : public StreamInfo { (*metadata_.mutable_filter_metadata())[name].MergeFrom(value); }; - const FilterStateSharedPtr& filterState() const override { return filter_state_; } + const FilterStateSharedPtr& filterState() override { return filter_state_; } + const FilterState& filterState() const override { return *filter_state_; } const FilterStateSharedPtr& upstreamFilterState() const override { return upstream_filter_state_; diff --git a/source/common/tcp_proxy/tcp_proxy.cc b/source/common/tcp_proxy/tcp_proxy.cc index 0fe8e5d5f0106..06d0615c49a6c 100644 --- a/source/common/tcp_proxy/tcp_proxy.cc +++ b/source/common/tcp_proxy/tcp_proxy.cc @@ -421,7 +421,7 @@ Network::FilterStatus Filter::initializeUpstreamConnection() { if (downstreamConnection()) { transport_socket_options_ = Network::TransportSocketOptionsUtility::fromFilterState( - *downstreamConnection()->streamInfo().filterState()); + downstreamConnection()->streamInfo().filterState()); } if (!config_->tunnelingConfig()) { diff --git a/test/common/router/header_formatter_test.cc b/test/common/router/header_formatter_test.cc index ffa026fd7a256..81044fddb558b 100644 --- a/test/common/router/header_formatter_test.cc +++ b/test/common/router/header_formatter_test.cc @@ -600,6 +600,7 @@ TEST_F(StreamInfoHeaderFormatterTest, TestFormatWithPerRequestStateVariable) { NiceMock stream_info; ON_CALL(stream_info, filterState()).WillByDefault(ReturnRef(filter_state)); + ON_CALL(Const(stream_info), filterState()).WillByDefault(ReturnRef(*filter_state)); testFormatting(stream_info, "PER_REQUEST_STATE(testing)", "test_value"); testFormatting(stream_info, "PER_REQUEST_STATE(testing2)", ""); @@ -617,6 +618,7 @@ TEST_F(StreamInfoHeaderFormatterTest, TestFormatWithNonStringPerRequestStateVari NiceMock stream_info; ON_CALL(stream_info, filterState()).WillByDefault(ReturnRef(filter_state)); + ON_CALL(Const(stream_info), filterState()).WillByDefault(ReturnRef(*filter_state)); testFormatting(stream_info, "PER_REQUEST_STATE(testing)", ""); } @@ -871,6 +873,7 @@ TEST(HeaderParserTest, TestParseInternal) { StreamInfo::FilterState::StateType::ReadOnly, StreamInfo::FilterState::LifeSpan::FilterChain); ON_CALL(stream_info, filterState()).WillByDefault(ReturnRef(filter_state)); + ON_CALL(Const(stream_info), filterState()).WillByDefault(ReturnRef(*filter_state)); for (const auto& test_case : test_cases) { Protobuf::RepeatedPtrField to_add; @@ -1041,6 +1044,7 @@ request_headers_to_remove: ["x-nope"] StreamInfo::FilterState::StateType::ReadOnly, StreamInfo::FilterState::LifeSpan::FilterChain); ON_CALL(stream_info, filterState()).WillByDefault(ReturnRef(filter_state)); + ON_CALL(Const(stream_info), filterState()).WillByDefault(ReturnRef(*filter_state)); req_header_parser->evaluateHeaders(header_map, stream_info); diff --git a/test/common/router/router_test.cc b/test/common/router/router_test.cc index 5c575fb613508..e3e24a131a41a 100644 --- a/test/common/router/router_test.cc +++ b/test/common/router/router_test.cc @@ -4358,11 +4358,11 @@ TEST_F(RouterTest, PropagatesUpstreamFilterState) { bool filter_state_verified = false; router_.config().upstream_logs_.push_back( std::make_shared([&](const auto& stream_info) { - filter_state_verified = stream_info.upstreamFilterState()->hasDataWithName("foo"); + filter_state_verified = stream_info.upstreamFilterState()->hasDataWithName("upstream data"); })); upstream_stream_info_.filterState()->setData( - "foo", std::make_unique(123), + "upstream data", std::make_unique(123), StreamInfo::FilterState::StateType::ReadOnly, StreamInfo::FilterState::LifeSpan::Connection); expectResponseTimerCreate(); EXPECT_CALL(cm_.conn_pool_, newStream(_, _)) diff --git a/test/common/stream_info/test_util.h b/test/common/stream_info/test_util.h index 1da2eb9b47f04..b141abeb0c2eb 100644 --- a/test/common/stream_info/test_util.h +++ b/test/common/stream_info/test_util.h @@ -179,9 +179,8 @@ class TestStreamInfo : public StreamInfo::StreamInfo { (*metadata_.mutable_filter_metadata())[name].MergeFrom(value); }; - const Envoy::StreamInfo::FilterStateSharedPtr& filterState() const override { - return filter_state_; - } + const Envoy::StreamInfo::FilterStateSharedPtr& filterState() override { return filter_state_; } + const Envoy::StreamInfo::FilterState& filterState() const override { return *filter_state_; } const Envoy::StreamInfo::FilterStateSharedPtr& upstreamFilterState() const override { return upstream_filter_state_; diff --git a/test/mocks/stream_info/mocks.cc b/test/mocks/stream_info/mocks.cc index e7dec2e445970..79cb4f41763f7 100644 --- a/test/mocks/stream_info/mocks.cc +++ b/test/mocks/stream_info/mocks.cc @@ -112,6 +112,7 @@ MockStreamInfo::MockStreamInfo() ON_CALL(*this, dynamicMetadata()).WillByDefault(ReturnRef(metadata_)); ON_CALL(Const(*this), dynamicMetadata()).WillByDefault(ReturnRef(metadata_)); ON_CALL(*this, filterState()).WillByDefault(ReturnRef(filter_state_)); + ON_CALL(Const(*this), filterState()).WillByDefault(ReturnRef(*filter_state_)); ON_CALL(*this, upstreamFilterState()).WillByDefault(ReturnRef(upstream_filter_state_)); ON_CALL(*this, setUpstreamFilterState(_)) .WillByDefault(Invoke([this](const FilterStateSharedPtr& filter_state) { diff --git a/test/mocks/stream_info/mocks.h b/test/mocks/stream_info/mocks.h index cc6b84e680b00..2c5b09562e965 100644 --- a/test/mocks/stream_info/mocks.h +++ b/test/mocks/stream_info/mocks.h @@ -80,7 +80,8 @@ class MockStreamInfo : public StreamInfo { MOCK_METHOD(void, setDynamicMetadata, (const std::string&, const ProtobufWkt::Struct&)); MOCK_METHOD(void, setDynamicMetadata, (const std::string&, const std::string&, const std::string&)); - MOCK_METHOD(const FilterStateSharedPtr&, filterState, (), (const)); + MOCK_METHOD(const FilterStateSharedPtr&, filterState, ()); + MOCK_METHOD(const FilterState&, filterState, (), (const)); MOCK_METHOD(const FilterStateSharedPtr&, upstreamFilterState, (), (const)); MOCK_METHOD(void, setUpstreamFilterState, (const FilterStateSharedPtr&)); MOCK_METHOD(void, setRequestedServerName, (const absl::string_view)); From cae70905dba8090a388888878f01968ef0bea9f2 Mon Sep 17 00:00:00 2001 From: Snow Pettersen Date: Tue, 31 Mar 2020 11:07:52 -0700 Subject: [PATCH 8/9] revert grpc access log Signed-off-by: Snow Pettersen --- .../extensions/access_loggers/grpc/grpc_access_log_utils.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source/extensions/access_loggers/grpc/grpc_access_log_utils.cc b/source/extensions/access_loggers/grpc/grpc_access_log_utils.cc index ae2875a628d90..1d187bc299852 100644 --- a/source/extensions/access_loggers/grpc/grpc_access_log_utils.cc +++ b/source/extensions/access_loggers/grpc/grpc_access_log_utils.cc @@ -239,9 +239,9 @@ void Utility::extractCommonAccessLogProperties( } for (const auto& key : config.filter_state_objects_to_log()) { - if (stream_info.filterState()->hasDataWithName(key)) { + if (stream_info.filterState().hasDataWithName(key)) { const auto& obj = - stream_info.filterState()->getDataReadOnly(key); + stream_info.filterState().getDataReadOnly(key); ProtobufTypes::MessagePtr serialized_proto = obj.serializeAsProto(); if (serialized_proto != nullptr) { auto& filter_state_objects = *common_access_log.mutable_filter_state_objects(); From 70bade80ad931033ccc2204723d601eaaea24199 Mon Sep 17 00:00:00 2001 From: Snow Pettersen Date: Thu, 2 Apr 2020 12:45:27 -0700 Subject: [PATCH 9/9] remove test specific config() call - the real class has one now Signed-off-by: Snow Pettersen --- source/common/router/router.h | 4 ---- test/common/router/router_test.cc | 2 -- 2 files changed, 6 deletions(-) diff --git a/source/common/router/router.h b/source/common/router/router.h index d56fa6a9110af..fb5e529454694 100644 --- a/source/common/router/router.h +++ b/source/common/router/router.h @@ -505,11 +505,7 @@ class Filter : Logger::Loggable, uint64_t grpc_to_http_status); Http::Context& httpContext() { return config_.http_context_; } - // This makes it accessible to the RouterTest class. -protected: FilterConfig& config_; - -private: Http::StreamDecoderFilterCallbacks* callbacks_{}; RouteConstSharedPtr route_; const RouteEntry* route_entry_{}; diff --git a/test/common/router/router_test.cc b/test/common/router/router_test.cc index e3e24a131a41a..2a55c005e0984 100644 --- a/test/common/router/router_test.cc +++ b/test/common/router/router_test.cc @@ -84,8 +84,6 @@ class RouterTestFilter : public Filter { return &downstream_connection_; } - FilterConfig& config() { return config_; } - NiceMock downstream_connection_; MockRetryState* retry_state_{}; bool reject_all_hosts_ = false;