diff --git a/api/envoy/data/tap/v2alpha/http.proto b/api/envoy/data/tap/v2alpha/http.proto index f9c4bb483ad7..92b731c2ec97 100644 --- a/api/envoy/data/tap/v2alpha/http.proto +++ b/api/envoy/data/tap/v2alpha/http.proto @@ -9,9 +9,18 @@ import "envoy/api/v2/core/base.proto"; // A fully buffered HTTP trace message. message HttpBufferedTrace { - // Request headers. - repeated api.v2.core.HeaderValue request_headers = 2; + // HTTP message wrapper. + message Message { + // Message headers. + repeated api.v2.core.HeaderValue headers = 1; - // Response headers. - repeated api.v2.core.HeaderValue response_headers = 3; + // Message trailers. + repeated api.v2.core.HeaderValue trailers = 2; + } + + // Request message. + Message request = 1; + + // Response message. + Message response = 2; } diff --git a/api/envoy/service/tap/v2alpha/common.proto b/api/envoy/service/tap/v2alpha/common.proto index e4bd6d4a0aa5..47f9b53f7e26 100644 --- a/api/envoy/service/tap/v2alpha/common.proto +++ b/api/envoy/service/tap/v2alpha/common.proto @@ -48,23 +48,23 @@ message MatchPredicate { // The match configuration will always match. bool any_match = 4 [(validate.rules).bool.const = true]; - // HTTP request match configuration. - HttpRequestMatch http_request_match = 5; + // HTTP request headers match configuration. + HttpHeadersMatch http_request_headers_match = 5; - // HTTP response match configuration. - HttpResponseMatch http_response_match = 6; - } -} + // HTTP request trailers match configuration. + HttpHeadersMatch http_request_trailers_match = 6; -// HTTP request match configuration. -message HttpRequestMatch { - // HTTP request headers to match. - repeated api.v2.route.HeaderMatcher headers = 1; + // HTTP response headers match configuration. + HttpHeadersMatch http_response_headers_match = 7; + + // HTTP response trailers match configuration. + HttpHeadersMatch http_response_trailers_match = 8; + } } -// HTTP response match configuration. -message HttpResponseMatch { - // HTTP response headers to match. +// HTTP headers match configuration. +message HttpHeadersMatch { + // HTTP headers to match. repeated api.v2.route.HeaderMatcher headers = 1; } diff --git a/docs/root/configuration/http_filters/tap_filter.rst b/docs/root/configuration/http_filters/tap_filter.rst index e447ce5a1dde..6982a3384a95 100644 --- a/docs/root/configuration/http_filters/tap_filter.rst +++ b/docs/root/configuration/http_filters/tap_filter.rst @@ -65,11 +65,11 @@ An example POST body: match_config: and_match: rules: - - http_request_match: + - http_request_headers_match: headers: - name: foo exact_match: bar - - http_response_match: + - http_response_headers_match: headers: - name: bar exact_match: baz @@ -90,11 +90,11 @@ Another example POST body: match_config: or_match: rules: - - http_request_match: + - http_request_headers_match: headers: - name: foo exact_match: bar - - http_response_match: + - http_response_headers_match: headers: - name: bar exact_match: baz diff --git a/source/common/router/router.cc b/source/common/router/router.cc index 5cab1a9106d5..af5e8b6faa14 100644 --- a/source/common/router/router.cc +++ b/source/common/router/router.cc @@ -447,6 +447,7 @@ Http::FilterDataStatus Filter::decodeData(Buffer::Instance& data, bool end_strea } Http::FilterTrailersStatus Filter::decodeTrailers(Http::HeaderMap& trailers) { + ENVOY_STREAM_LOG(debug, "router decoding trailers:\n{}", *callbacks_, trailers); downstream_trailers_ = &trailers; upstream_request_->encodeTrailers(trailers); onRequestComplete(); diff --git a/source/extensions/common/tap/tap_matcher.cc b/source/extensions/common/tap/tap_matcher.cc index b97568d4060f..39114a122994 100644 --- a/source/extensions/common/tap/tap_matcher.cc +++ b/source/extensions/common/tap/tap_matcher.cc @@ -32,12 +32,21 @@ void buildMatcher(const envoy::service::tap::v2alpha::MatchPredicate& match_conf case envoy::service::tap::v2alpha::MatchPredicate::kAnyMatch: new_matcher = std::make_unique(matchers); break; - case envoy::service::tap::v2alpha::MatchPredicate::kHttpRequestMatch: - new_matcher = std::make_unique(match_config.http_request_match(), matchers); + case envoy::service::tap::v2alpha::MatchPredicate::kHttpRequestHeadersMatch: + new_matcher = std::make_unique( + match_config.http_request_headers_match(), matchers); break; - case envoy::service::tap::v2alpha::MatchPredicate::kHttpResponseMatch: - new_matcher = - std::make_unique(match_config.http_response_match(), matchers); + case envoy::service::tap::v2alpha::MatchPredicate::kHttpRequestTrailersMatch: + new_matcher = std::make_unique( + match_config.http_request_trailers_match(), matchers); + break; + case envoy::service::tap::v2alpha::MatchPredicate::kHttpResponseHeadersMatch: + new_matcher = std::make_unique( + match_config.http_response_headers_match(), matchers); + break; + case envoy::service::tap::v2alpha::MatchPredicate::kHttpResponseTrailersMatch: + new_matcher = std::make_unique( + match_config.http_response_trailers_match(), matchers); break; default: NOT_REACHED_GCOVR_EXCL_LINE; @@ -50,14 +59,19 @@ void buildMatcher(const envoy::service::tap::v2alpha::MatchPredicate& match_conf SetLogicMatcher::SetLogicMatcher( const envoy::service::tap::v2alpha::MatchPredicate::MatchSet& configs, std::vector& matchers, Type type) - : Matcher(matchers), matchers_(matchers), type_(type) { + : LogicMatcherBase(matchers), matchers_(matchers), type_(type) { for (const auto& config : configs.rules()) { indexes_.push_back(matchers_.size()); buildMatcher(config, matchers_); } } -bool SetLogicMatcher::updateLocalStatus(std::vector& statuses) const { +bool SetLogicMatcher::updateLocalStatus(std::vector& statuses, + const UpdateFunctor& functor) const { + for (size_t index : indexes_) { + statuses[index] = functor(*matchers_[index], statuses); + } + auto predicate = [&statuses](size_t index) { return statuses[index]; }; if (type_ == Type::And) { statuses[my_index_] = std::all_of(indexes_.begin(), indexes_.end(), predicate); @@ -69,81 +83,30 @@ bool SetLogicMatcher::updateLocalStatus(std::vector& statuses) const { return statuses[my_index_]; } -bool SetLogicMatcher::onNewStream(std::vector& statuses) const { - for (size_t index : indexes_) { - statuses[index] = matchers_[index]->onNewStream(statuses); - } - - return updateLocalStatus(statuses); -} - -bool SetLogicMatcher::onHttpRequestHeaders(const Http::HeaderMap& request_headers, - std::vector& statuses) const { - for (size_t index : indexes_) { - statuses[index] = matchers_[index]->onHttpRequestHeaders(request_headers, statuses); - } - - return updateLocalStatus(statuses); -} - -bool SetLogicMatcher::onHttpResponseHeaders(const Http::HeaderMap& response_headers, - std::vector& statuses) const { - for (size_t index : indexes_) { - statuses[index] = matchers_[index]->onHttpResponseHeaders(response_headers, statuses); - } - - return updateLocalStatus(statuses); -} - NotMatcher::NotMatcher(const envoy::service::tap::v2alpha::MatchPredicate& config, std::vector& matchers) - : Matcher(matchers), matchers_(matchers), not_index_(matchers.size()) { + : LogicMatcherBase(matchers), matchers_(matchers), not_index_(matchers.size()) { buildMatcher(config, matchers); } -bool NotMatcher::onNewStream(std::vector& statuses) const { - statuses[my_index_] = !matchers_[not_index_]->onNewStream(statuses); - return statuses[my_index_]; -} - -bool NotMatcher::onHttpRequestHeaders(const Http::HeaderMap& request_headers, - std::vector& statuses) const { - statuses[my_index_] = !matchers_[not_index_]->onHttpRequestHeaders(request_headers, statuses); - return statuses[my_index_]; -} - -bool NotMatcher::onHttpResponseHeaders(const Http::HeaderMap& response_headers, - std::vector& statuses) const { - statuses[my_index_] = !matchers_[not_index_]->onHttpResponseHeaders(response_headers, statuses); - return statuses[my_index_]; -} - -HttpRequestMatcher::HttpRequestMatcher(const envoy::service::tap::v2alpha::HttpRequestMatch& config, - const std::vector& matchers) - : Matcher(matchers) { - for (const auto& header_match : config.headers()) { - headers_to_match_.emplace_back(header_match); - } -} - -bool HttpRequestMatcher::onHttpRequestHeaders(const Http::HeaderMap& request_headers, - std::vector& statuses) const { - statuses[my_index_] = Http::HeaderUtility::matchHeaders(request_headers, headers_to_match_); +bool NotMatcher::updateLocalStatus(std::vector& statuses, + const UpdateFunctor& functor) const { + statuses[my_index_] = !functor(*matchers_[not_index_], statuses); return statuses[my_index_]; } -HttpResponseMatcher::HttpResponseMatcher( - const envoy::service::tap::v2alpha::HttpResponseMatch& config, +HttpHeaderMatcherBase::HttpHeaderMatcherBase( + const envoy::service::tap::v2alpha::HttpHeadersMatch& config, const std::vector& matchers) - : Matcher(matchers) { + : SimpleMatcher(matchers) { for (const auto& header_match : config.headers()) { headers_to_match_.emplace_back(header_match); } } -bool HttpResponseMatcher::onHttpResponseHeaders(const Http::HeaderMap& response_headers, - std::vector& statuses) const { - statuses[my_index_] = Http::HeaderUtility::matchHeaders(response_headers, headers_to_match_); +bool HttpHeaderMatcherBase::matchHeaders(const Http::HeaderMap& headers, + std::vector& statuses) const { + statuses[my_index_] = Http::HeaderUtility::matchHeaders(headers, headers_to_match_); return statuses[my_index_]; } diff --git a/source/extensions/common/tap/tap_matcher.h b/source/extensions/common/tap/tap_matcher.h index 0380967c14dd..622933a96d65 100644 --- a/source/extensions/common/tap/tap_matcher.h +++ b/source/extensions/common/tap/tap_matcher.h @@ -38,6 +38,16 @@ using MatcherPtr = std::unique_ptr; */ class Matcher { public: + /** + * Base class constructor for a matcher. + * @param matchers supplies the match tree vector being built. + */ + Matcher(const std::vector& matchers) + // NOTE: This code assumes that the index for the matcher being constructed has already been + // allocated, which is why my_index_ is set to size() - 1. See buildMatcher() in + // tap_matcher.cc. + : my_index_(matchers.size() - 1) {} + virtual ~Matcher() = default; /** @@ -46,7 +56,7 @@ class Matcher { size_t index() { return my_index_; } /** - * Update match status when a stream is created. This might be an HTTP stream, a TCP connectin, + * Update match status when a stream is created. This might be an HTTP stream, a TCP connection, * etc. This allows any matchers to flip to an initial state of true if applicable. */ virtual bool onNewStream(std::vector& statuses) const PURE; @@ -59,6 +69,16 @@ class Matcher { */ virtual bool onHttpRequestHeaders(const Http::HeaderMap& request_headers, std::vector& statuses) const PURE; + + /** + * Update match status given HTTP request trailers. + * @param request_trailers supplies the request trailers. + * @param statuses supplies the per-stream-request match status vector which must be the same + * size as the match tree vector (see above). + */ + virtual bool onHttpRequestTrailers(const Http::HeaderMap& request_trailers, + std::vector& statuses) const PURE; + /** * Update match status given HTTP response headers. * @param response_headers supplies the response headers. @@ -69,23 +89,22 @@ class Matcher { std::vector& statuses) const PURE; /** - * @return whether given currently available information, the matcher matches. + * Update match status given HTTP response trailers. + * @param response_headers supplies the response trailers. * @param statuses supplies the per-stream-request match status vector which must be the same * size as the match tree vector (see above). */ - bool matches(const std::vector& statuses) const { return statuses[my_index_]; } + virtual bool onHttpResponseTrailers(const Http::HeaderMap& response_trailers, + std::vector& statuses) const PURE; -protected: /** - * Base class constructor for a matcher. - * @param matchers supplies the match tree vector being built. + * @return whether given currently available information, the matcher matches. + * @param statuses supplies the per-stream-request match status vector which must be the same + * size as the match tree vector (see above). */ - Matcher(const std::vector& matchers) - // NOTE: This code assumes that the index for the matcher being constructed has already been - // allocated, which is why my_index_ is set to size() - 1. See buildMatcher() in - // tap_matcher.cc. - : my_index_(matchers.size() - 1) {} + bool matches(const std::vector& statuses) const { return statuses[my_index_]; } +protected: const size_t my_index_; }; @@ -97,25 +116,64 @@ class Matcher { void buildMatcher(const envoy::service::tap::v2alpha::MatchPredicate& match_config, std::vector& matchers); +/** + * Base class for logic matchers that need to forward update calls to child matchers. + */ +class LogicMatcherBase : public Matcher { +public: + using Matcher::Matcher; + + // Extensions::Common::Tap::Matcher + bool onNewStream(std::vector& statuses) const override { + return updateLocalStatus( + statuses, [](Matcher& m, std::vector& statuses) { return m.onNewStream(statuses); }); + } + bool onHttpRequestHeaders(const Http::HeaderMap& request_headers, + std::vector& statuses) const override { + return updateLocalStatus(statuses, [&request_headers](Matcher& m, std::vector& statuses) { + return m.onHttpRequestHeaders(request_headers, statuses); + }); + } + bool onHttpRequestTrailers(const Http::HeaderMap& request_trailers, + std::vector& statuses) const override { + return updateLocalStatus(statuses, + [&request_trailers](Matcher& m, std::vector& statuses) { + return m.onHttpRequestTrailers(request_trailers, statuses); + }); + } + bool onHttpResponseHeaders(const Http::HeaderMap& response_headers, + std::vector& statuses) const override { + return updateLocalStatus(statuses, + [&response_headers](Matcher& m, std::vector& statuses) { + return m.onHttpResponseHeaders(response_headers, statuses); + }); + } + bool onHttpResponseTrailers(const Http::HeaderMap& response_trailers, + std::vector& statuses) const override { + return updateLocalStatus(statuses, + [&response_trailers](Matcher& m, std::vector& statuses) { + return m.onHttpResponseTrailers(response_trailers, statuses); + }); + } + +protected: + using UpdateFunctor = std::function&)>; + virtual bool updateLocalStatus(std::vector& statuses, + const UpdateFunctor& functor) const PURE; +}; + /** * Matcher for implementing set logic. */ -class SetLogicMatcher : public Matcher { +class SetLogicMatcher : public LogicMatcherBase { public: enum class Type { And, Or }; SetLogicMatcher(const envoy::service::tap::v2alpha::MatchPredicate::MatchSet& configs, std::vector& matchers, Type type); - // Extensions::Common::Tap::Matcher - bool onNewStream(std::vector& statuses) const override; - bool onHttpRequestHeaders(const Http::HeaderMap& request_headers, - std::vector& statuses) const override; - bool onHttpResponseHeaders(const Http::HeaderMap& response_headers, - std::vector& statuses) const override; - private: - bool updateLocalStatus(std::vector& statuses) const; + bool updateLocalStatus(std::vector& statuses, const UpdateFunctor& functor) const override; std::vector& matchers_; std::vector indexes_; @@ -125,81 +183,124 @@ class SetLogicMatcher : public Matcher { /** * Not matcher. */ -class NotMatcher : public Matcher { +class NotMatcher : public LogicMatcherBase { public: NotMatcher(const envoy::service::tap::v2alpha::MatchPredicate& config, std::vector& matchers); - // Extensions::Common::Tap::Matcher - bool onNewStream(std::vector& statuses) const override; - bool onHttpRequestHeaders(const Http::HeaderMap& request_headers, - std::vector& statuses) const override; - bool onHttpResponseHeaders(const Http::HeaderMap& response_headers, - std::vector& statuses) const override; - private: + bool updateLocalStatus(std::vector& statuses, const UpdateFunctor& functor) const override; + std::vector& matchers_; const size_t not_index_; }; +/** + * A base class for a matcher that generally wants to return default values, but might override + * a single update function. + */ +class SimpleMatcher : public Matcher { +public: + using Matcher::Matcher; + + // Extensions::Common::Tap::Matcher + bool onNewStream(std::vector& statuses) const { return statuses[my_index_]; } + bool onHttpRequestHeaders(const Http::HeaderMap&, std::vector& statuses) const { + return statuses[my_index_]; + } + bool onHttpRequestTrailers(const Http::HeaderMap&, std::vector& statuses) const { + return statuses[my_index_]; + } + bool onHttpResponseHeaders(const Http::HeaderMap&, std::vector& statuses) const { + return statuses[my_index_]; + } + bool onHttpResponseTrailers(const Http::HeaderMap&, std::vector& statuses) const { + return statuses[my_index_]; + } +}; + /** * Any matcher (always matches). */ -class AnyMatcher : public Matcher { +class AnyMatcher : public SimpleMatcher { public: - AnyMatcher(std::vector& matchers) : Matcher(matchers) {} + using SimpleMatcher::SimpleMatcher; // Extensions::Common::Tap::Matcher bool onNewStream(std::vector& statuses) const override { statuses[my_index_] = true; return true; } - bool onHttpRequestHeaders(const Http::HeaderMap&, std::vector&) const override { - return true; - } - bool onHttpResponseHeaders(const Http::HeaderMap&, std::vector&) const override { - return true; - } }; /** - * HTTP request matcher. + * Base class for the various HTTP header matchers. */ -class HttpRequestMatcher : public Matcher { +class HttpHeaderMatcherBase : public SimpleMatcher { public: - HttpRequestMatcher(const envoy::service::tap::v2alpha::HttpRequestMatch& config, - const std::vector& matchers); + HttpHeaderMatcherBase(const envoy::service::tap::v2alpha::HttpHeadersMatch& config, + const std::vector& matchers); + +protected: + bool matchHeaders(const Http::HeaderMap& headers, std::vector& statuses) const; + + std::vector headers_to_match_; +}; + +/** + * HTTP request headers matcher. + */ +class HttpRequestHeadersMatcher : public HttpHeaderMatcherBase { +public: + using HttpHeaderMatcherBase::HttpHeaderMatcherBase; // Extensions::Common::Tap::Matcher - bool onNewStream(std::vector&) const override { return false; } bool onHttpRequestHeaders(const Http::HeaderMap& request_headers, - std::vector& statuses) const override; - bool onHttpResponseHeaders(const Http::HeaderMap&, std::vector& statuses) const override { - return statuses[my_index_]; + std::vector& statuses) const override { + return matchHeaders(request_headers, statuses); } - -private: - std::vector headers_to_match_; }; /** - * HTTP response matcher. + * HTTP request trailers matcher. */ -class HttpResponseMatcher : public Matcher { +class HttpRequestTrailersMatcher : public HttpHeaderMatcherBase { public: - HttpResponseMatcher(const envoy::service::tap::v2alpha::HttpResponseMatch& config, - const std::vector& matchers); + using HttpHeaderMatcherBase::HttpHeaderMatcherBase; // Extensions::Common::Tap::Matcher - bool onNewStream(std::vector&) const override { return false; } - bool onHttpRequestHeaders(const Http::HeaderMap&, std::vector&) const override { - return false; + bool onHttpRequestTrailers(const Http::HeaderMap& request_trailers, + std::vector& statuses) const override { + return matchHeaders(request_trailers, statuses); } +}; + +/** + * HTTP response headers matcher. + */ +class HttpResponseHeadersMatcher : public HttpHeaderMatcherBase { +public: + using HttpHeaderMatcherBase::HttpHeaderMatcherBase; + + // Extensions::Common::Tap::Matcher bool onHttpResponseHeaders(const Http::HeaderMap& response_headers, - std::vector& statuses) const override; + std::vector& statuses) const override { + return matchHeaders(response_headers, statuses); + } +}; -private: - std::vector headers_to_match_; +/** + * HTTP response trailers matcher. + */ +class HttpResponseTrailersMatcher : public HttpHeaderMatcherBase { +public: + using HttpHeaderMatcherBase::HttpHeaderMatcherBase; + + // Extensions::Common::Tap::Matcher + bool onHttpResponseTrailers(const Http::HeaderMap& response_trailers, + std::vector& statuses) const override { + return matchHeaders(response_trailers, statuses); + } }; } // namespace Tap diff --git a/source/extensions/filters/http/tap/tap_config.h b/source/extensions/filters/http/tap/tap_config.h index ef7e42faa94f..3444f213b37d 100644 --- a/source/extensions/filters/http/tap/tap_config.h +++ b/source/extensions/filters/http/tap/tap_config.h @@ -24,17 +24,29 @@ class HttpPerRequestTapper { */ virtual void onRequestHeaders(const Http::HeaderMap& headers) PURE; + /** + * Called when request trailers are received. + */ + virtual void onRequestTrailers(const Http::HeaderMap& trailers) PURE; + /** * Called when response headers are received. */ virtual void onResponseHeaders(const Http::HeaderMap& headers) PURE; + /** + * Called when response trailers are received. + */ + virtual void onResponseTrailers(const Http::HeaderMap& headers) PURE; + /** * Called when the request is being destroyed and is being logged. * @return whether the request was tapped or not. */ virtual bool onDestroyLog(const Http::HeaderMap* request_headers, - const Http::HeaderMap* response_headers) PURE; + const Http::HeaderMap* request_trailers, + const Http::HeaderMap* response_headers, + const Http::HeaderMap* response_trailers) PURE; }; using HttpPerRequestTapperPtr = std::unique_ptr; diff --git a/source/extensions/filters/http/tap/tap_config_impl.cc b/source/extensions/filters/http/tap/tap_config_impl.cc index 1880bf6176a4..3ed61680a1dd 100644 --- a/source/extensions/filters/http/tap/tap_config_impl.cc +++ b/source/extensions/filters/http/tap/tap_config_impl.cc @@ -22,10 +22,18 @@ void HttpPerRequestTapperImpl::onRequestHeaders(const Http::HeaderMap& headers) config_->rootMatcher().onHttpRequestHeaders(headers, statuses_); } +void HttpPerRequestTapperImpl::onRequestTrailers(const Http::HeaderMap& trailers) { + config_->rootMatcher().onHttpRequestTrailers(trailers, statuses_); +} + void HttpPerRequestTapperImpl::onResponseHeaders(const Http::HeaderMap& headers) { config_->rootMatcher().onHttpResponseHeaders(headers, statuses_); } +void HttpPerRequestTapperImpl::onResponseTrailers(const Http::HeaderMap& trailers) { + config_->rootMatcher().onHttpResponseTrailers(trailers, statuses_); +} + namespace { Http::HeaderMap::Iterate fillHeaderList(const Http::HeaderEntry& header, void* context) { Protobuf::RepeatedPtrField& header_list = @@ -38,16 +46,24 @@ Http::HeaderMap::Iterate fillHeaderList(const Http::HeaderEntry& header, void* c } // namespace bool HttpPerRequestTapperImpl::onDestroyLog(const Http::HeaderMap* request_headers, - const Http::HeaderMap* response_headers) { + const Http::HeaderMap* request_trailers, + const Http::HeaderMap* response_headers, + const Http::HeaderMap* response_trailers) { if (!config_->rootMatcher().matches(statuses_)) { return false; } auto trace = std::make_shared(); auto& http_trace = *trace->mutable_http_buffered_trace(); - request_headers->iterate(fillHeaderList, http_trace.mutable_request_headers()); + request_headers->iterate(fillHeaderList, http_trace.mutable_request()->mutable_headers()); + if (request_trailers != nullptr) { + request_trailers->iterate(fillHeaderList, http_trace.mutable_request()->mutable_trailers()); + } if (response_headers != nullptr) { - response_headers->iterate(fillHeaderList, http_trace.mutable_response_headers()); + response_headers->iterate(fillHeaderList, http_trace.mutable_response()->mutable_headers()); + } + if (response_trailers != nullptr) { + response_trailers->iterate(fillHeaderList, http_trace.mutable_response()->mutable_trailers()); } ENVOY_LOG(debug, "submitting buffered trace sink"); diff --git a/source/extensions/filters/http/tap/tap_config_impl.h b/source/extensions/filters/http/tap/tap_config_impl.h index 56e465d80acc..8c0fa527188d 100644 --- a/source/extensions/filters/http/tap/tap_config_impl.h +++ b/source/extensions/filters/http/tap/tap_config_impl.h @@ -34,9 +34,12 @@ class HttpPerRequestTapperImpl : public HttpPerRequestTapper, Logger::LoggableonRequestTrailers(trailers); + } + return Http::FilterTrailersStatus::Continue; +} + Http::FilterHeadersStatus Filter::encodeHeaders(Http::HeaderMap& headers, bool) { if (tapper_ != nullptr) { tapper_->onResponseHeaders(headers); @@ -39,9 +48,17 @@ Http::FilterHeadersStatus Filter::encodeHeaders(Http::HeaderMap& headers, bool) return Http::FilterHeadersStatus::Continue; } +Http::FilterTrailersStatus Filter::encodeTrailers(Http::HeaderMap& trailers) { + if (tapper_ != nullptr) { + tapper_->onResponseTrailers(trailers); + } + return Http::FilterTrailersStatus::Continue; +} + void Filter::log(const Http::HeaderMap* request_headers, const Http::HeaderMap* response_headers, - const Http::HeaderMap*, const StreamInfo::StreamInfo&) { - if (tapper_ != nullptr && tapper_->onDestroyLog(request_headers, response_headers)) { + const Http::HeaderMap* response_trailers, const StreamInfo::StreamInfo&) { + if (tapper_ != nullptr && tapper_->onDestroyLog(request_headers, request_trailers_, + response_headers, response_trailers)) { config_->stats().rq_tapped_.inc(); } } diff --git a/source/extensions/filters/http/tap/tap_filter.h b/source/extensions/filters/http/tap/tap_filter.h index 5b05d7128d29..326dcd846c0c 100644 --- a/source/extensions/filters/http/tap/tap_filter.h +++ b/source/extensions/filters/http/tap/tap_filter.h @@ -86,9 +86,7 @@ class Filter : public Http::StreamFilter, public AccessLog::Instance { Http::FilterDataStatus decodeData(Buffer::Instance&, bool) override { return Http::FilterDataStatus::Continue; } - Http::FilterTrailersStatus decodeTrailers(Http::HeaderMap&) override { - return Http::FilterTrailersStatus::Continue; - } + Http::FilterTrailersStatus decodeTrailers(Http::HeaderMap& trailers) override; void setDecoderFilterCallbacks(Http::StreamDecoderFilterCallbacks& callbacks) override { HttpTapConfigSharedPtr config = config_->currentConfig(); tapper_ = config ? config->createPerRequestTapper(callbacks.streamId()) : nullptr; @@ -102,9 +100,7 @@ class Filter : public Http::StreamFilter, public AccessLog::Instance { Http::FilterDataStatus encodeData(Buffer::Instance&, bool) override { return Http::FilterDataStatus::Continue; } - Http::FilterTrailersStatus encodeTrailers(Http::HeaderMap&) override { - return Http::FilterTrailersStatus::Continue; - } + Http::FilterTrailersStatus encodeTrailers(Http::HeaderMap& trailers) override; Http::FilterMetadataStatus encodeMetadata(Http::MetadataMap&) override { return Http::FilterMetadataStatus::Continue; } @@ -118,6 +114,7 @@ class Filter : public Http::StreamFilter, public AccessLog::Instance { private: FilterConfigSharedPtr config_; HttpPerRequestTapperPtr tapper_; + const Http::HeaderMap* request_trailers_{}; }; } // namespace TapFilter diff --git a/test/extensions/filters/http/tap/tap_filter_integration_test.cc b/test/extensions/filters/http/tap/tap_filter_integration_test.cc index 26a79ea9c5c8..dc258d3ef67a 100644 --- a/test/extensions/filters/http/tap/tap_filter_integration_test.cc +++ b/test/extensions/filters/http/tap/tap_filter_integration_test.cc @@ -16,7 +16,11 @@ class TapIntegrationTest : public HttpIntegrationTest, // HTTP/1 on OSX. In this test we close the admin /tap stream when we don't want any // more data, and without immediate close detection we can't have a flake free test. // Thus, we use HTTP/2 for everything here. - : HttpIntegrationTest(Http::CodecClient::Type::HTTP2, GetParam(), realTime()) {} + : HttpIntegrationTest(Http::CodecClient::Type::HTTP2, GetParam(), realTime()) { + + // Also use HTTP/2 for upstream so that we can fully test trailers. + setUpstreamProtocol(FakeHttpConnection::Type::HTTP2); + } void initializeFilter(const std::string& filter_config) { config_helper_.addFilter(filter_config); @@ -36,13 +40,38 @@ class TapIntegrationTest : public HttpIntegrationTest, } void makeRequest(const Http::TestHeaderMapImpl& request_headers, - const Http::TestHeaderMapImpl& response_headers) { - IntegrationStreamDecoderPtr decoder = codec_client_->makeHeaderOnlyRequest(request_headers); + const Http::TestHeaderMapImpl* request_trailers, + const Http::TestHeaderMapImpl& response_headers, + const Http::TestHeaderMapImpl* response_trailers) { + IntegrationStreamDecoderPtr decoder; + if (request_trailers == nullptr) { + decoder = codec_client_->makeHeaderOnlyRequest(request_headers); + } else { + auto result = codec_client_->startRequest(request_headers); + decoder = std::move(result.second); + result.first.encodeTrailers(*request_trailers); + } + waitForNextUpstreamRequest(); - upstream_request_->encodeHeaders(response_headers, true); + + upstream_request_->encodeHeaders(response_headers, response_trailers == nullptr); + if (response_trailers != nullptr) { + upstream_request_->encodeTrailers(*response_trailers); + } + decoder->waitForEndStream(); } + void startAdminRequest(const std::string& admin_request_yaml) { + admin_client_ = makeHttpConnection(makeClientConnection(lookupPort("admin"))); + const Http::TestHeaderMapImpl admin_request_headers{ + {":method", "POST"}, {":path", "/tap"}, {":scheme", "http"}, {":authority", "host"}}; + admin_response_ = admin_client_->makeRequestWithBody(admin_request_headers, admin_request_yaml); + admin_response_->waitForHeaders(); + EXPECT_STREQ("200", admin_response_->headers().Status()->value().c_str()); + EXPECT_FALSE(admin_response_->complete()); + } + const Http::TestHeaderMapImpl request_headers_tap_{{":method", "GET"}, {":path", "/"}, {":scheme", "http"}, @@ -55,6 +84,18 @@ class TapIntegrationTest : public HttpIntegrationTest, const Http::TestHeaderMapImpl response_headers_tap_{{":status", "200"}, {"bar", "baz"}}; const Http::TestHeaderMapImpl response_headers_no_tap_{{":status", "200"}}; + + const std::string admin_filter_config_ = + R"EOF( +name: envoy.filters.http.tap +config: + common_config: + admin_config: + config_id: test_config_id +)EOF"; + + IntegrationCodecClientPtr admin_client_; + IntegrationStreamDecoderPtr admin_response_; }; INSTANTIATE_TEST_SUITE_P(IpVersions, TapIntegrationTest, @@ -84,7 +125,7 @@ name: envoy.filters.http.tap // Initial request/response with tap. codec_client_ = makeHttpConnection(makeClientConnection(lookupPort("http"))); - makeRequest(request_headers_tap_, response_headers_no_tap_); + makeRequest(request_headers_tap_, nullptr, response_headers_no_tap_, nullptr); codec_client_->close(); test_server_->waitForCounterGe("http.config_test.downstream_cx_destroy", 1); @@ -101,20 +142,11 @@ name: envoy.filters.http.tap // Verify a basic tap flow using the admin handler. TEST_P(TapIntegrationTest, AdminBasicFlow) { - const std::string filter_config = - R"EOF( -name: envoy.filters.http.tap -config: - common_config: - admin_config: - config_id: test_config_id -)EOF"; - - initializeFilter(filter_config); + initializeFilter(admin_filter_config_); // Initial request/response with no tap. codec_client_ = makeHttpConnection(makeClientConnection(lookupPort("http"))); - makeRequest(request_headers_tap_, response_headers_no_tap_); + makeRequest(request_headers_tap_, nullptr, response_headers_no_tap_, nullptr); const std::string admin_request_yaml = R"EOF( @@ -123,11 +155,11 @@ config_id: test_config_id match_config: or_match: rules: - - http_request_match: + - http_request_headers_match: headers: - name: foo exact_match: bar - - http_response_match: + - http_response_headers_match: headers: - name: bar exact_match: baz @@ -137,55 +169,43 @@ config_id: test_config_id )EOF"; // Setup a tap and disconnect it without any request/response. - IntegrationCodecClientPtr admin_client_ = - makeHttpConnection(makeClientConnection(lookupPort("admin"))); - const Http::TestHeaderMapImpl admin_request_headers{ - {":method", "POST"}, {":path", "/tap"}, {":scheme", "http"}, {":authority", "host"}}; - IntegrationStreamDecoderPtr admin_response = - admin_client_->makeRequestWithBody(admin_request_headers, admin_request_yaml); - admin_response->waitForHeaders(); - EXPECT_STREQ("200", admin_response->headers().Status()->value().c_str()); - EXPECT_FALSE(admin_response->complete()); + startAdminRequest(admin_request_yaml); admin_client_->close(); test_server_->waitForGaugeEq("http.admin.downstream_rq_active", 0); // Second request/response with no tap. - makeRequest(request_headers_tap_, response_headers_no_tap_); + makeRequest(request_headers_tap_, nullptr, response_headers_no_tap_, nullptr); // Setup the tap again and leave it open. - admin_client_ = makeHttpConnection(makeClientConnection(lookupPort("admin"))); - admin_response = admin_client_->makeRequestWithBody(admin_request_headers, admin_request_yaml); - admin_response->waitForHeaders(); - EXPECT_STREQ("200", admin_response->headers().Status()->value().c_str()); - EXPECT_FALSE(admin_response->complete()); + startAdminRequest(admin_request_yaml); // Do a request which should tap, matching on request headers. - makeRequest(request_headers_tap_, response_headers_no_tap_); + makeRequest(request_headers_tap_, nullptr, response_headers_no_tap_, nullptr); // Wait for the tap message. - admin_response->waitForBodyData(1); + admin_response_->waitForBodyData(1); envoy::data::tap::v2alpha::BufferedTraceWrapper trace; - MessageUtil::loadFromYaml(admin_response->body(), trace); - EXPECT_EQ(trace.http_buffered_trace().request_headers().size(), 8); - EXPECT_EQ(trace.http_buffered_trace().response_headers().size(), 5); - admin_response->clearBody(); + MessageUtil::loadFromYaml(admin_response_->body(), trace); + EXPECT_EQ(trace.http_buffered_trace().request().headers().size(), 8); + EXPECT_EQ(trace.http_buffered_trace().response().headers().size(), 4); + admin_response_->clearBody(); // Do a request which should not tap. - makeRequest(request_headers_no_tap_, response_headers_no_tap_); + makeRequest(request_headers_no_tap_, nullptr, response_headers_no_tap_, nullptr); // Do a request which should tap, matching on response headers. - makeRequest(request_headers_no_tap_, response_headers_tap_); + makeRequest(request_headers_no_tap_, nullptr, response_headers_tap_, nullptr); // Wait for the tap message. - admin_response->waitForBodyData(1); - MessageUtil::loadFromYaml(admin_response->body(), trace); - EXPECT_EQ(trace.http_buffered_trace().request_headers().size(), 7); + admin_response_->waitForBodyData(1); + MessageUtil::loadFromYaml(admin_response_->body(), trace); + EXPECT_EQ(trace.http_buffered_trace().request().headers().size(), 7); EXPECT_EQ( "http", - findHeader("x-forwarded-proto", trace.http_buffered_trace().request_headers())->value()); - EXPECT_EQ(trace.http_buffered_trace().response_headers().size(), 6); - EXPECT_NE(nullptr, findHeader("date", trace.http_buffered_trace().response_headers())); - EXPECT_EQ("baz", findHeader("bar", trace.http_buffered_trace().response_headers())->value()); + findHeader("x-forwarded-proto", trace.http_buffered_trace().request().headers())->value()); + EXPECT_EQ(trace.http_buffered_trace().response().headers().size(), 5); + EXPECT_NE(nullptr, findHeader("date", trace.http_buffered_trace().response().headers())); + EXPECT_EQ("baz", findHeader("bar", trace.http_buffered_trace().response().headers())->value()); admin_client_->close(); test_server_->waitForGaugeEq("http.admin.downstream_rq_active", 0); @@ -198,11 +218,11 @@ config_id: test_config_id match_config: and_match: rules: - - http_request_match: + - http_request_headers_match: headers: - name: foo exact_match: bar - - http_response_match: + - http_response_headers_match: headers: - name: bar exact_match: baz @@ -211,28 +231,67 @@ config_id: test_config_id - streaming_admin: {} )EOF"; - admin_client_ = makeHttpConnection(makeClientConnection(lookupPort("admin"))); - admin_response = admin_client_->makeRequestWithBody(admin_request_headers, admin_request_yaml2); - admin_response->waitForHeaders(); - EXPECT_STREQ("200", admin_response->headers().Status()->value().c_str()); - EXPECT_FALSE(admin_response->complete()); + startAdminRequest(admin_request_yaml2); // Do a request that matches, but the response does not match. No tap. - makeRequest(request_headers_tap_, response_headers_no_tap_); + makeRequest(request_headers_tap_, nullptr, response_headers_no_tap_, nullptr); // Do a request that doesn't match, but the response does match. No tap. - makeRequest(request_headers_no_tap_, response_headers_tap_); + makeRequest(request_headers_no_tap_, nullptr, response_headers_tap_, nullptr); // Do a request that matches and a response that matches. Should tap. - makeRequest(request_headers_tap_, response_headers_tap_); + makeRequest(request_headers_tap_, nullptr, response_headers_tap_, nullptr); // Wait for the tap message. - admin_response->waitForBodyData(1); - MessageUtil::loadFromYaml(admin_response->body(), trace); + admin_response_->waitForBodyData(1); + MessageUtil::loadFromYaml(admin_response_->body(), trace); admin_client_->close(); EXPECT_EQ(3UL, test_server_->counter("http.config_test.tap.rq_tapped")->value()); } +// Verify both request and response trailer matching works. +TEST_P(TapIntegrationTest, AdminTrailers) { + initializeFilter(admin_filter_config_); + + const std::string admin_request_yaml = + R"EOF( +config_id: test_config_id +tap_config: + match_config: + and_match: + rules: + - http_request_trailers_match: + headers: + - name: foo_trailer + exact_match: bar + - http_response_trailers_match: + headers: + - name: bar_trailer + exact_match: baz + output_config: + sinks: + - streaming_admin: {} +)EOF"; + + startAdminRequest(admin_request_yaml); + + codec_client_ = makeHttpConnection(makeClientConnection(lookupPort("http"))); + const Http::TestHeaderMapImpl request_trailers{{"foo_trailer", "bar"}}; + const Http::TestHeaderMapImpl response_trailers{{"bar_trailer", "baz"}}; + makeRequest(request_headers_no_tap_, &request_trailers, response_headers_no_tap_, + &response_trailers); + + envoy::data::tap::v2alpha::BufferedTraceWrapper trace; + admin_response_->waitForBodyData(1); + MessageUtil::loadFromYaml(admin_response_->body(), trace); + EXPECT_EQ("bar", + findHeader("foo_trailer", trace.http_buffered_trace().request().trailers())->value()); + EXPECT_EQ("baz", + findHeader("bar_trailer", trace.http_buffered_trace().response().trailers())->value()); + + admin_client_->close(); +} + } // namespace } // namespace Envoy diff --git a/test/extensions/filters/http/tap/tap_filter_test.cc b/test/extensions/filters/http/tap/tap_filter_test.cc index e48e13fd94fc..d605488ea68b 100644 --- a/test/extensions/filters/http/tap/tap_filter_test.cc +++ b/test/extensions/filters/http/tap/tap_filter_test.cc @@ -37,9 +37,13 @@ class MockHttpTapConfig : public HttpTapConfig { class MockHttpPerRequestTapper : public HttpPerRequestTapper { public: MOCK_METHOD1(onRequestHeaders, void(const Http::HeaderMap& headers)); + MOCK_METHOD1(onRequestTrailers, void(const Http::HeaderMap& headers)); MOCK_METHOD1(onResponseHeaders, void(const Http::HeaderMap& headers)); - MOCK_METHOD2(onDestroyLog, bool(const Http::HeaderMap* request_headers, - const Http::HeaderMap* response_headers)); + MOCK_METHOD1(onResponseTrailers, void(const Http::HeaderMap& headers)); + MOCK_METHOD4(onDestroyLog, + bool(const Http::HeaderMap* request_headers, const Http::HeaderMap* request_trailers, + const Http::HeaderMap* response_headers, + const Http::HeaderMap* response_trailers)); }; class TapFilterTest : public testing::Test { @@ -107,6 +111,7 @@ TEST_F(TapFilterTest, Config) { Buffer::OwnedImpl request_body; EXPECT_EQ(Http::FilterDataStatus::Continue, filter_->decodeData(request_body, false)); Http::TestHeaderMapImpl request_trailers; + EXPECT_CALL(*http_per_request_tapper_, onRequestTrailers(_)); EXPECT_EQ(Http::FilterTrailersStatus::Continue, filter_->decodeTrailers(request_trailers)); Http::TestHeaderMapImpl response_headers; @@ -117,9 +122,11 @@ TEST_F(TapFilterTest, Config) { Buffer::OwnedImpl response_body; EXPECT_EQ(Http::FilterDataStatus::Continue, filter_->encodeData(response_body, false)); Http::TestHeaderMapImpl response_trailers; + EXPECT_CALL(*http_per_request_tapper_, onResponseTrailers(_)); EXPECT_EQ(Http::FilterTrailersStatus::Continue, filter_->encodeTrailers(response_trailers)); - EXPECT_CALL(*http_per_request_tapper_, onDestroyLog(&request_headers, &response_headers)) + EXPECT_CALL(*http_per_request_tapper_, onDestroyLog(&request_headers, &request_trailers, + &response_headers, &response_trailers)) .WillOnce(Return(true)); filter_->log(&request_headers, &response_headers, &response_trailers, stream_info_); EXPECT_EQ(1UL, filter_config_->stats_.rq_tapped_.value());