diff --git a/docs/root/configuration/http/http_filters/composite_filter.rst b/docs/root/configuration/http/http_filters/composite_filter.rst index 8515e51e1f71..2f2851e8864d 100644 --- a/docs/root/configuration/http/http_filters/composite_filter.rst +++ b/docs/root/configuration/http/http_filters/composite_filter.rst @@ -20,6 +20,7 @@ incremented. This filter adds a map of the delegated filter name (of the action that is matched )and the root config filter name to the filter state with key ``envoy.extensions.filters.http.composite.matched_actions`` +This filter state is not emitted when the filter is configured in the upstream filter chain. Contains a map of pairs `FILTER_CONFIG_NAME:ACTION_NAME`: diff --git a/source/extensions/filters/http/composite/config.cc b/source/extensions/filters/http/composite/config.cc index 074b80d06ad2..7cf994a393df 100644 --- a/source/extensions/filters/http/composite/config.cc +++ b/source/extensions/filters/http/composite/config.cc @@ -19,8 +19,8 @@ absl::StatusOr CompositeFilterFactory::createFilterFactor auto stats = std::make_shared( FilterStats{ALL_COMPOSITE_FILTER_STATS(POOL_COUNTER_PREFIX(dual_info.scope, prefix))}); - return [stats](Http::FilterChainFactoryCallbacks& callbacks) -> void { - auto filter = std::make_shared(*stats, callbacks.dispatcher()); + return [stats, dual_info](Http::FilterChainFactoryCallbacks& callbacks) -> void { + auto filter = std::make_shared(*stats, callbacks.dispatcher(), dual_info.is_upstream); callbacks.addStreamFilter(filter); callbacks.addAccessLogHandler(filter); }; diff --git a/source/extensions/filters/http/composite/filter.cc b/source/extensions/filters/http/composite/filter.cc index 99c28e0bec52..e8e1711a14a1 100644 --- a/source/extensions/filters/http/composite/filter.cc +++ b/source/extensions/filters/http/composite/filter.cc @@ -142,6 +142,9 @@ void Filter::onMatchCallback(const Matcher::Action& action) { void Filter::updateFilterState(Http::StreamFilterCallbacks* callback, const std::string& filter_name, const std::string& action_name) { + if (isUpstream()) { + return; + } auto* info = callback->streamInfo().filterState()->getDataMutable( MatchedActionsFilterStateKey); if (info != nullptr) { diff --git a/source/extensions/filters/http/composite/filter.h b/source/extensions/filters/http/composite/filter.h index 3a07d7015c31..dfe76ec43c1a 100644 --- a/source/extensions/filters/http/composite/filter.h +++ b/source/extensions/filters/http/composite/filter.h @@ -56,8 +56,9 @@ class Filter : public Http::StreamFilter, public AccessLog::Instance, Logger::Loggable { public: - Filter(FilterStats& stats, Event::Dispatcher& dispatcher) - : dispatcher_(dispatcher), decoded_headers_(false), stats_(stats) {} + Filter(FilterStats& stats, Event::Dispatcher& dispatcher, bool is_upstream) + : dispatcher_(dispatcher), decoded_headers_(false), stats_(stats), is_upstream_(is_upstream) { + } // Http::StreamDecoderFilter Http::FilterHeadersStatus decodeHeaders(Http::RequestHeaderMap& headers, @@ -109,6 +110,8 @@ class Filter : public Http::StreamFilter, } } + bool isUpstream() const { return is_upstream_; } + private: friend FactoryCallbacksWrapper; @@ -165,6 +168,8 @@ class Filter : public Http::StreamFilter, Http::StreamEncoderFilterCallbacks* encoder_callbacks_{}; Http::StreamDecoderFilterCallbacks* decoder_callbacks_{}; FilterStats& stats_; + // Filter in the upstream filter chain. + bool is_upstream_; }; } // namespace Composite diff --git a/test/extensions/filters/http/composite/filter_test.cc b/test/extensions/filters/http/composite/filter_test.cc index 3ad6cf6cc566..32e17d7bc676 100644 --- a/test/extensions/filters/http/composite/filter_test.cc +++ b/test/extensions/filters/http/composite/filter_test.cc @@ -20,9 +20,10 @@ namespace { using Envoy::Protobuf::util::MessageDifferencer; -class FilterTest : public ::testing::Test { +class CompositeFilterTest : public ::testing::Test { public: - FilterTest() : filter_(stats_, decoder_callbacks_.dispatcher()) { + CompositeFilterTest(bool is_upstream) + : filter_(stats_, decoder_callbacks_.dispatcher(), is_upstream) { filter_.setDecoderFilterCallbacks(decoder_callbacks_); filter_.setEncoderFilterCallbacks(encoder_callbacks_); } @@ -100,6 +101,11 @@ class FilterTest : public ::testing::Test { {"response-trailer", "something-else"}}; }; +class FilterTest : public CompositeFilterTest { +public: + FilterTest() : CompositeFilterTest(false) {} +}; + TEST_F(FilterTest, StreamEncoderFilterDelegation) { auto stream_filter = std::make_shared(); StreamInfo::FilterStateSharedPtr filter_state = @@ -640,6 +646,41 @@ TEST_F(FilterTest, MatchingActionShouldNotCollitionWithOtherRootFilter) { filter_.onDestroy(); } +class UpstreamFilterTest : public CompositeFilterTest { +public: + UpstreamFilterTest() : CompositeFilterTest(true) {} +}; + +TEST_F(UpstreamFilterTest, StreamEncoderFilterDelegationUpstream) { + auto stream_filter = std::make_shared(); + StreamInfo::FilterStateSharedPtr filter_state = + std::make_shared(StreamInfo::FilterState::LifeSpan::Connection); + ON_CALL(encoder_callbacks_, filterConfigName()).WillByDefault(testing::Return("rootFilterName")); + ON_CALL(encoder_callbacks_.stream_info_, filterState()) + .WillByDefault(testing::ReturnRef(filter_state)); + + auto factory_callback = [&](Http::FilterChainFactoryCallbacks& cb) { + cb.addStreamEncoderFilter(stream_filter); + }; + + EXPECT_CALL(*stream_filter, setEncoderFilterCallbacks(_)); + ExecuteFilterAction action(factory_callback, "actionName"); + EXPECT_CALL(success_counter_, inc()); + filter_.onMatchCallback(action); + + auto* info = filter_state->getDataMutable(MatchedActionsFilterStateKey); + EXPECT_EQ(nullptr, info); + + doAllDecodingCallbacks(); + expectDelegatedEncoding(*stream_filter); + doAllEncodingCallbacks(); + + EXPECT_CALL(*stream_filter, onStreamComplete()); + EXPECT_CALL(*stream_filter, onDestroy()); + filter_.onStreamComplete(); + filter_.onDestroy(); +} + } // namespace } // namespace Composite } // namespace HttpFilters