From ecf47e24cdc3e94e9c84028247f17c263b72209c Mon Sep 17 00:00:00 2001 From: Snow Pettersen Date: Fri, 4 Dec 2020 01:28:47 +0000 Subject: [PATCH 01/32] http: add support for skip filter match action Signed-off-by: Snow Pettersen --- .../config/common/matcher/v3/action.proto | 19 ++++ .../config/common/matcher/v3/matcher.proto | 11 +++ .../common/matcher/v4alpha/action.proto | 20 ++++ .../common/matcher/v4alpha/matcher.proto | 14 +++ .../config/common/matcher/v3/action.proto | 19 ++++ .../config/common/matcher/v3/matcher.proto | 11 +++ .../common/matcher/v4alpha/action.proto | 20 ++++ .../common/matcher/v4alpha/matcher.proto | 14 +++ include/envoy/http/BUILD | 1 + include/envoy/http/filter.h | 17 ++++ include/envoy/http/header_map.h | 3 + include/envoy/matcher/matcher.h | 3 + source/common/http/BUILD | 2 + source/common/http/filter_manager.cc | 16 +++- source/common/http/filter_manager.h | 94 +++++++++++++++++-- source/common/http/header_utility.cc | 31 +++--- source/common/http/header_utility.h | 2 + test/common/http/filter_manager_test.cc | 43 +++++++++ test/common/router/router_test.cc | 3 +- 19 files changed, 322 insertions(+), 21 deletions(-) create mode 100644 api/envoy/config/common/matcher/v3/action.proto create mode 100644 api/envoy/config/common/matcher/v4alpha/action.proto create mode 100644 generated_api_shadow/envoy/config/common/matcher/v3/action.proto create mode 100644 generated_api_shadow/envoy/config/common/matcher/v4alpha/action.proto diff --git a/api/envoy/config/common/matcher/v3/action.proto b/api/envoy/config/common/matcher/v3/action.proto new file mode 100644 index 0000000000000..28afda10ffa1e --- /dev/null +++ b/api/envoy/config/common/matcher/v3/action.proto @@ -0,0 +1,19 @@ +syntax = "proto3"; + +package envoy.config.common.matcher.v3; + +import "udpa/annotations/migrate.proto"; +import "udpa/annotations/status.proto"; +import "udpa/annotations/versioning.proto"; +import "validate/validate.proto"; + +option java_package = "io.envoyproxy.envoy.config.common.matcher.v3"; +option java_outer_classname = "ActionProto"; +option java_multiple_files = true; +option (udpa.annotations.file_status).package_version_status = ACTIVE; + +// [#protodoc-title: Common Match Actions] + +// Indicates that the associated action should be skipped. +message SkipAction { +} diff --git a/api/envoy/config/common/matcher/v3/matcher.proto b/api/envoy/config/common/matcher/v3/matcher.proto index 24ed5dafbdc54..23cad325d7c36 100644 --- a/api/envoy/config/common/matcher/v3/matcher.proto +++ b/api/envoy/config/common/matcher/v3/matcher.proto @@ -140,6 +140,17 @@ message Matcher { OnMatch on_no_match = 3; } +// Wrapper around an existing extension that provides an associated matcher. This allows +// decorating an existing extension with a matcher, which can be used to match against +// relevant protocol data. +message ExtensionWithMatcher { + // The associated matcher. + Matcher matcher = 1 [(validate.rules).message = {required: true}]; + + // The underlying extension config. + core.v3.TypedExtensionConfig typed_config = 2 [(validate.rules).message = {required: true}]; +} + // Match configuration. This is a recursive structure which allows complex nested match // configurations to be built using various logical operators. // [#next-free-field: 11] diff --git a/api/envoy/config/common/matcher/v4alpha/action.proto b/api/envoy/config/common/matcher/v4alpha/action.proto new file mode 100644 index 0000000000000..4c047cd2f3900 --- /dev/null +++ b/api/envoy/config/common/matcher/v4alpha/action.proto @@ -0,0 +1,20 @@ +syntax = "proto3"; + +package envoy.config.common.matcher.v4alpha; + +import "udpa/annotations/status.proto"; +import "udpa/annotations/versioning.proto"; +import "validate/validate.proto"; + +option java_package = "io.envoyproxy.envoy.config.common.matcher.v4alpha"; +option java_outer_classname = "ActionProto"; +option java_multiple_files = true; +option (udpa.annotations.file_status).package_version_status = NEXT_MAJOR_VERSION_CANDIDATE; + +// [#protodoc-title: Common Match Actions] + +// Indicates that the associated action should be skipped. +message SkipAction { + option (udpa.annotations.versioning).previous_message_type = + "envoy.config.common.matcher.v3.SkipAction"; +} diff --git a/api/envoy/config/common/matcher/v4alpha/matcher.proto b/api/envoy/config/common/matcher/v4alpha/matcher.proto index ab1f4de6cccc1..6f20d3d907c80 100644 --- a/api/envoy/config/common/matcher/v4alpha/matcher.proto +++ b/api/envoy/config/common/matcher/v4alpha/matcher.proto @@ -166,6 +166,20 @@ message Matcher { OnMatch on_no_match = 3; } +// Wrapper around an existing extension that provides an associated matcher. This allows +// decorating an existing extension with a matcher, which can be used to match against +// relevant protocol data. +message ExtensionWithMatcher { + option (udpa.annotations.versioning).previous_message_type = + "envoy.config.common.matcher.v3.ExtensionWithMatcher"; + + // The associated matcher. + Matcher matcher = 1 [(validate.rules).message = {required: true}]; + + // The underlying extension config. + core.v4alpha.TypedExtensionConfig typed_config = 2 [(validate.rules).message = {required: true}]; +} + // Match configuration. This is a recursive structure which allows complex nested match // configurations to be built using various logical operators. // [#next-free-field: 11] diff --git a/generated_api_shadow/envoy/config/common/matcher/v3/action.proto b/generated_api_shadow/envoy/config/common/matcher/v3/action.proto new file mode 100644 index 0000000000000..28afda10ffa1e --- /dev/null +++ b/generated_api_shadow/envoy/config/common/matcher/v3/action.proto @@ -0,0 +1,19 @@ +syntax = "proto3"; + +package envoy.config.common.matcher.v3; + +import "udpa/annotations/migrate.proto"; +import "udpa/annotations/status.proto"; +import "udpa/annotations/versioning.proto"; +import "validate/validate.proto"; + +option java_package = "io.envoyproxy.envoy.config.common.matcher.v3"; +option java_outer_classname = "ActionProto"; +option java_multiple_files = true; +option (udpa.annotations.file_status).package_version_status = ACTIVE; + +// [#protodoc-title: Common Match Actions] + +// Indicates that the associated action should be skipped. +message SkipAction { +} diff --git a/generated_api_shadow/envoy/config/common/matcher/v3/matcher.proto b/generated_api_shadow/envoy/config/common/matcher/v3/matcher.proto index 24ed5dafbdc54..23cad325d7c36 100644 --- a/generated_api_shadow/envoy/config/common/matcher/v3/matcher.proto +++ b/generated_api_shadow/envoy/config/common/matcher/v3/matcher.proto @@ -140,6 +140,17 @@ message Matcher { OnMatch on_no_match = 3; } +// Wrapper around an existing extension that provides an associated matcher. This allows +// decorating an existing extension with a matcher, which can be used to match against +// relevant protocol data. +message ExtensionWithMatcher { + // The associated matcher. + Matcher matcher = 1 [(validate.rules).message = {required: true}]; + + // The underlying extension config. + core.v3.TypedExtensionConfig typed_config = 2 [(validate.rules).message = {required: true}]; +} + // Match configuration. This is a recursive structure which allows complex nested match // configurations to be built using various logical operators. // [#next-free-field: 11] diff --git a/generated_api_shadow/envoy/config/common/matcher/v4alpha/action.proto b/generated_api_shadow/envoy/config/common/matcher/v4alpha/action.proto new file mode 100644 index 0000000000000..4c047cd2f3900 --- /dev/null +++ b/generated_api_shadow/envoy/config/common/matcher/v4alpha/action.proto @@ -0,0 +1,20 @@ +syntax = "proto3"; + +package envoy.config.common.matcher.v4alpha; + +import "udpa/annotations/status.proto"; +import "udpa/annotations/versioning.proto"; +import "validate/validate.proto"; + +option java_package = "io.envoyproxy.envoy.config.common.matcher.v4alpha"; +option java_outer_classname = "ActionProto"; +option java_multiple_files = true; +option (udpa.annotations.file_status).package_version_status = NEXT_MAJOR_VERSION_CANDIDATE; + +// [#protodoc-title: Common Match Actions] + +// Indicates that the associated action should be skipped. +message SkipAction { + option (udpa.annotations.versioning).previous_message_type = + "envoy.config.common.matcher.v3.SkipAction"; +} diff --git a/generated_api_shadow/envoy/config/common/matcher/v4alpha/matcher.proto b/generated_api_shadow/envoy/config/common/matcher/v4alpha/matcher.proto index ab1f4de6cccc1..6f20d3d907c80 100644 --- a/generated_api_shadow/envoy/config/common/matcher/v4alpha/matcher.proto +++ b/generated_api_shadow/envoy/config/common/matcher/v4alpha/matcher.proto @@ -166,6 +166,20 @@ message Matcher { OnMatch on_no_match = 3; } +// Wrapper around an existing extension that provides an associated matcher. This allows +// decorating an existing extension with a matcher, which can be used to match against +// relevant protocol data. +message ExtensionWithMatcher { + option (udpa.annotations.versioning).previous_message_type = + "envoy.config.common.matcher.v3.ExtensionWithMatcher"; + + // The associated matcher. + Matcher matcher = 1 [(validate.rules).message = {required: true}]; + + // The underlying extension config. + core.v4alpha.TypedExtensionConfig typed_config = 2 [(validate.rules).message = {required: true}]; +} + // Match configuration. This is a recursive structure which allows complex nested match // configurations to be built using various logical operators. // [#next-free-field: 11] diff --git a/include/envoy/http/BUILD b/include/envoy/http/BUILD index f17ce1cb5e146..7e93a12e310a5 100644 --- a/include/envoy/http/BUILD +++ b/include/envoy/http/BUILD @@ -79,6 +79,7 @@ envoy_cc_library( "//include/envoy/event:dispatcher_interface", "//include/envoy/grpc:status", "//include/envoy/router:router_interface", + "//include/envoy/matcher:matcher_interface", "//include/envoy/ssl:connection_interface", "//include/envoy/stream_info:stream_info_interface", "//include/envoy/tracing:http_tracer_interface", diff --git a/include/envoy/http/filter.h b/include/envoy/http/filter.h index f9710a45eb0ae..6a9ba6b0e9db6 100644 --- a/include/envoy/http/filter.h +++ b/include/envoy/http/filter.h @@ -5,6 +5,7 @@ #include #include +#include "envoy/matcher/matcher.h" #include "envoy/access_log/access_log.h" #include "envoy/common/scope_tracker.h" #include "envoy/event/dispatcher.h" @@ -864,6 +865,14 @@ class StreamFilter : public virtual StreamDecoderFilter, public virtual StreamEn using StreamFilterSharedPtr = std::shared_ptr; +class HttpMatchingData { +public: + virtual ~HttpMatchingData() = default; + + virtual RequestHeaderMapOptConstRef requestHeaders() const PURE; + virtual ResponseHeaderMapOptConstRef responseHeaders() const PURE; +}; + /** * These callbacks are provided by the connection manager to the factory so that the factory can * build the filter chain in an application specific way. @@ -878,6 +887,14 @@ class FilterChainFactoryCallbacks { */ virtual void addStreamDecoderFilter(Http::StreamDecoderFilterSharedPtr filter) PURE; + /** + * Add a decoder filter that is used when reading stream data. + * @param filter supplies the filter to add. + */ + virtual void + addStreamDecoderFilter(Http::StreamDecoderFilterSharedPtr filter, + Matcher::MatchTreeSharedPtr match_tree) PURE; + /** * Add an encoder filter that is used when writing stream data. * @param filter supplies the filter to add. diff --git a/include/envoy/http/header_map.h b/include/envoy/http/header_map.h index 26b6bdf926b66..e5d064612caaa 100644 --- a/include/envoy/http/header_map.h +++ b/include/envoy/http/header_map.h @@ -749,6 +749,8 @@ class RequestHeaderMap }; using RequestHeaderMapPtr = std::unique_ptr; using RequestHeaderMapOptRef = absl::optional>; +using RequestHeaderMapOptConstRef = absl::optional>; +using RequestHeaderMapOptRef = absl::optional>; // Request trailers. class RequestTrailerMap @@ -775,6 +777,7 @@ class ResponseHeaderMap }; using ResponseHeaderMapPtr = std::unique_ptr; using ResponseHeaderMapOptRef = absl::optional>; +using ResponseHeaderMapOptConstRef = absl::optional>; // Response trailers. class ResponseTrailerMap diff --git a/include/envoy/matcher/matcher.h b/include/envoy/matcher/matcher.h index dec9cf6ce76ad..cb4ec68a981ee 100644 --- a/include/envoy/matcher/matcher.h +++ b/include/envoy/matcher/matcher.h @@ -120,6 +120,9 @@ template class MatchTree { virtual MatchResult match(const DataType& matching_data) PURE; }; +template +using MatchTreeSharedPtr = std::shared_ptr>; + // InputMatcher provides the interface for determining whether an input value matches. class InputMatcher { public: diff --git a/source/common/http/BUILD b/source/common/http/BUILD index 01798486ffef6..99701c3e4b4ca 100644 --- a/source/common/http/BUILD +++ b/source/common/http/BUILD @@ -170,6 +170,8 @@ envoy_cc_library( "//include/envoy/http:filter_interface", "//source/common/buffer:watermark_buffer_lib", "//source/common/common:linked_object", + "//include/envoy/matcher:matcher_interface", + "//source/common/matcher:matcher_lib", "//source/common/common:scope_tracker", "//source/common/grpc:common_lib", "//source/common/local_reply:local_reply_lib", diff --git a/source/common/http/filter_manager.cc b/source/common/http/filter_manager.cc index 36aa653a90062..0e992b731bd18 100644 --- a/source/common/http/filter_manager.cc +++ b/source/common/http/filter_manager.cc @@ -9,6 +9,7 @@ #include "common/http/header_utility.h" #include "common/http/utility.h" #include "common/runtime/runtime_features.h" +#include "include/envoy/matcher/_virtual_includes/matcher_interface/envoy/matcher/matcher.h" namespace Envoy { namespace Http { @@ -385,8 +386,8 @@ void ActiveStreamDecoderFilter::requestDataTooLarge() { } void FilterManager::addStreamDecoderFilterWorker(StreamDecoderFilterSharedPtr filter, - bool dual_filter) { - ActiveStreamDecoderFilterPtr wrapper(new ActiveStreamDecoderFilter(*this, filter, dual_filter)); + Matcher::MatchTreeSharedPtr matcher, bool dual_filter) { + ActiveStreamDecoderFilterPtr wrapper(new ActiveStreamDecoderFilter(*this, filter, matcher, dual_filter)); filter->setDecoderFilterCallbacks(*wrapper); // Note: configured decoder filters are appended to decoder_filters_. // This means that if filters are configured in the following order (assume all three filters are @@ -439,6 +440,17 @@ void FilterManager::decodeHeaders(ActiveStreamDecoderFilter* filter, RequestHead std::list::iterator continue_data_entry = decoder_filters_.end(); for (; entry != decoder_filters_.end(); entry++) { + if ((*entry)->matcher_) { + (*entry)->matching_data_.onRequestHeaders(headers); + const auto match_result = Matcher::evaluateMatch(*(*entry)->matcher_, (*entry)->matching_data_); + if (match_result.match_state_ == Matcher::MatchState::MatchComplete && match_result.result_) { + if (dynamic_cast(match_result.result_.get())) { + (*entry)->skip_ = true; + continue; + } + } + } + ASSERT(!(state_.filter_call_state_ & FilterCallState::DecodeHeaders)); state_.filter_call_state_ |= FilterCallState::DecodeHeaders; (*entry)->end_stream_ = (end_stream && continue_data_entry == decoder_filters_.end()); diff --git a/source/common/http/filter_manager.h b/source/common/http/filter_manager.h index 9f1de48ef3a4a..3ebf80f9f979e 100644 --- a/source/common/http/filter_manager.h +++ b/source/common/http/filter_manager.h @@ -1,5 +1,7 @@ #pragma once +#include "common/http/filter_manager.h" +#include "common/http/header_utility.h" #include "envoy/http/filter.h" #include "envoy/http/header_map.h" @@ -9,13 +11,84 @@ #include "common/common/logger.h" #include "common/grpc/common.h" #include "common/http/headers.h" +#include "common/matcher/matcher.h" #include "common/local_reply/local_reply.h" +#include "envoy/config/common/matcher/v3/action.pb.h" +#include "envoy/matcher/matcher.h" + namespace Envoy { namespace Http { class FilterManager; +class HttpMatchingDataImpl : public HttpMatchingData { +public: + static absl::string_view name() { return "http"; } + + void onRequestHeaders(const Http::RequestHeaderMap& request_headers) { + request_headers_ = &request_headers; + } + + void onResponseHeaders(const Http::ResponseHeaderMap& response_headers) { + response_headers_ = &response_headers; + } + + Http::RequestHeaderMapOptConstRef requestHeaders() const { + if (request_headers_) { + return absl::make_optional(std::cref(*request_headers_)); + } + + return absl::nullopt; + } + + Http::ResponseHeaderMapOptConstRef responseHeaders() const { + if (response_headers_) { + return absl::make_optional(std::cref(*response_headers_)); + } + + return absl::nullopt; + } + +private: + const Http::RequestHeaderMap* request_headers_{}; + const Http::ResponseHeaderMap* response_headers_{}; +}; + +class HttpRequestHeadersDataInput : public Matcher::DataInput { +public: + explicit HttpRequestHeadersDataInput(const std::string& name) : name_(name) {} + + Matcher::DataInputGetResult get(const HttpMatchingData& data) override { + const auto maybe_headers = data.requestHeaders(); + + if (!maybe_headers) { + return {Matcher::DataInputGetResult::DataAvailability::NotAvailable, absl::nullopt}; + } + + auto header = maybe_headers->get().get(name_); + if (header.empty()) { + return {Matcher::DataInputGetResult::DataAvailability::AllDataAvailable, absl::nullopt}; + } + + if (header_as_string_result_) { + return {Matcher::DataInputGetResult::DataAvailability::AllDataAvailable, + header_as_string_result_->result()}; + } + + header_as_string_result_ = HeaderUtility::getAllOfHeaderAsString(header, ","); + + return {Matcher::DataInputGetResult::DataAvailability::AllDataAvailable, + header_as_string_result_->result()}; + } + +private: + const LowerCaseString name_; + absl::optional header_as_string_result_; +}; + +class SkipAction : public Matcher::ActionBase {}; + /** * Base class wrapper for both stream encoder and decoder filters. */ @@ -25,7 +98,7 @@ struct ActiveStreamFilterBase : public virtual StreamFilterCallbacks, : parent_(parent), iteration_state_(IterationState::Continue), iterate_from_current_filter_(false), headers_continued_(false), continue_headers_continued_(false), end_stream_(false), dual_filter_(dual_filter), - decode_headers_called_(false), encode_headers_called_(false) {} + decode_headers_called_(false), encode_headers_called_(false), skip_(false) {} // Functions in the following block are called after the filter finishes processing // corresponding data. Those functions handle state updates and data storage (if needed) @@ -124,6 +197,7 @@ struct ActiveStreamFilterBase : public virtual StreamFilterCallbacks, const bool dual_filter_ : 1; bool decode_headers_called_ : 1; bool encode_headers_called_ : 1; + bool skip_ : 1; }; /** @@ -133,8 +207,8 @@ struct ActiveStreamDecoderFilter : public ActiveStreamFilterBase, public StreamDecoderFilterCallbacks, LinkedObject { ActiveStreamDecoderFilter(FilterManager& parent, StreamDecoderFilterSharedPtr filter, - bool dual_filter) - : ActiveStreamFilterBase(parent, dual_filter), handle_(filter) {} + Matcher::MatchTreeSharedPtr matcher, bool dual_filter) + : ActiveStreamFilterBase(parent, dual_filter), handle_(filter), matcher_(matcher) {} // ActiveStreamFilterBase bool canContinue() override; @@ -207,6 +281,8 @@ struct ActiveStreamDecoderFilter : public ActiveStreamFilterBase, absl::optional routeConfig(); StreamDecoderFilterSharedPtr handle_; + Matcher::MatchTreeSharedPtr matcher_; + HttpMatchingDataImpl matching_data_; bool is_grpc_request_{}; }; @@ -517,13 +593,17 @@ class FilterManager : public ScopeTrackedObject, // Http::FilterChainFactoryCallbacks void addStreamDecoderFilter(StreamDecoderFilterSharedPtr filter) override { - addStreamDecoderFilterWorker(filter, false); + addStreamDecoderFilterWorker(filter, nullptr, false); + } + void addStreamDecoderFilter(StreamDecoderFilterSharedPtr filter, + Matcher::MatchTreeSharedPtr match_tree) override { + addStreamDecoderFilterWorker(filter, match_tree, false); } void addStreamEncoderFilter(StreamEncoderFilterSharedPtr filter) override { addStreamEncoderFilterWorker(filter, false); } void addStreamFilter(StreamFilterSharedPtr filter) override { - addStreamDecoderFilterWorker(filter, true); + addStreamDecoderFilterWorker(filter, nullptr, true); addStreamEncoderFilterWorker(filter, true); } void addAccessLogHandler(AccessLog::InstanceSharedPtr handler) override; @@ -606,7 +686,9 @@ class FilterManager : public ScopeTrackedObject, void decodeMetadata(MetadataMap& metadata_map) { decodeMetadata(nullptr, metadata_map); } // TODO(snowp): Make private as filter chain construction is moved into FM. - void addStreamDecoderFilterWorker(StreamDecoderFilterSharedPtr filter, bool dual_filter); + void addStreamDecoderFilterWorker(StreamDecoderFilterSharedPtr filter, + Matcher::MatchTreeSharedPtr matcher, + bool dual_filter); void addStreamEncoderFilterWorker(StreamEncoderFilterSharedPtr filter, bool dual_filter); void disarmRequestTimeout(); diff --git a/source/common/http/header_utility.cc b/source/common/http/header_utility.cc index c59efda824dbe..26f6106bdb8fb 100644 --- a/source/common/http/header_utility.cc +++ b/source/common/http/header_utility.cc @@ -94,6 +94,25 @@ bool HeaderUtility::matchHeaders(const HeaderMap& request_headers, return true; } +HeaderUtility::GetAllOfHeaderAsStringResult +HeaderUtility::getAllOfHeaderAsString(const HeaderMap::GetResult& header_value, + absl::string_view seperator) { + GetAllOfHeaderAsStringResult result; + // In this case we concatenate all found headers using a delimiter before performing the + // final match. We use an InlinedVector of absl::string_view to invoke the optimized join + // algorithm. This requires a copying phase before we invoke join. The 3 used as the inline + // size has been arbitrarily chosen. + // TODO(mattklein123): Do we need to normalize any whitespace here? + absl::InlinedVector string_view_vector; + string_view_vector.reserve(header_value.size()); + for (size_t i = 0; i < header_value.size(); i++) { + string_view_vector.push_back(header_value[i]->value().getStringView()); + } + result.result_backing_string_ = absl::StrJoin(string_view_vector, seperator); + + return result; +} + HeaderUtility::GetAllOfHeaderAsStringResult HeaderUtility::getAllOfHeaderAsString(const HeaderMap& headers, const Http::LowerCaseString& key, absl::string_view separator) { @@ -108,17 +127,7 @@ HeaderUtility::getAllOfHeaderAsString(const HeaderMap& headers, const Http::Lowe "envoy.reloadable_features.http_match_on_all_headers")) { result.result_ = header_value[0]->value().getStringView(); } else { - // In this case we concatenate all found headers using a delimiter before performing the - // final match. We use an InlinedVector of absl::string_view to invoke the optimized join - // algorithm. This requires a copying phase before we invoke join. The 3 used as the inline - // size has been arbitrarily chosen. - // TODO(mattklein123): Do we need to normalize any whitespace here? - absl::InlinedVector string_view_vector; - string_view_vector.reserve(header_value.size()); - for (size_t i = 0; i < header_value.size(); i++) { - string_view_vector.push_back(header_value[i]->value().getStringView()); - } - result.result_backing_string_ = absl::StrJoin(string_view_vector, separator); + return getAllOfHeaderAsString(header_value, separator); } return result; diff --git a/source/common/http/header_utility.h b/source/common/http/header_utility.h index 864b1d942ad86..ff320408532ee 100644 --- a/source/common/http/header_utility.h +++ b/source/common/http/header_utility.h @@ -47,6 +47,8 @@ class HeaderUtility { friend class HeaderUtility; }; + static GetAllOfHeaderAsStringResult getAllOfHeaderAsString(const HeaderMap::GetResult& header, + absl::string_view separator = ","); static GetAllOfHeaderAsStringResult getAllOfHeaderAsString(const HeaderMap& headers, const Http::LowerCaseString& key, absl::string_view separator = ","); diff --git a/test/common/http/filter_manager_test.cc b/test/common/http/filter_manager_test.cc index fdc6e6a4b6beb..7200238de0d0f 100644 --- a/test/common/http/filter_manager_test.cc +++ b/test/common/http/filter_manager_test.cc @@ -1,9 +1,12 @@ +#include "envoy/http/header_map.h" #include "envoy/stream_info/filter_state.h" #include "common/http/filter_manager.h" #include "common/stream_info/filter_state_impl.h" #include "common/stream_info/stream_info_impl.h" +#include "include/envoy/matcher/_virtual_includes/matcher_interface/envoy/matcher/matcher.h" +#include "source/common/matcher/_virtual_includes/exact_map_matcher_lib/common/matcher/exact_map_matcher.h" #include "test/mocks/event/mocks.h" #include "test/mocks/http/mocks.h" #include "test/mocks/local_reply/mocks.h" @@ -139,6 +142,46 @@ TEST_F(FilterManagerTest, SendLocalReplyDuringEncodingGrpcClassiciation) { filter_manager_->decodeHeaders(*grpc_headers, true); filter_manager_->destroyFilters(); } + +Matcher::MatchTreeSharedPtr createRequestTreeMatchingTree() { + auto tree = std::make_shared>( + std::make_unique("match-header"), + absl::nullopt); + + tree->addChild("match", Matcher::OnMatch{ + []() { return std::make_unique(); }, nullptr}); + + return tree; +} + +TEST_F(FilterManagerTest, MatchTreeSkipAction) { + initialize(); + + // The filter is added, but since we match on the request header we skip the filter. + std::shared_ptr decoder_filter(new MockStreamDecoderFilter()); + EXPECT_CALL(*decoder_filter, setDecoderFilterCallbacks(_)); + EXPECT_CALL(*decoder_filter, onDestroy()); + + EXPECT_CALL(filter_factory_, createFilterChain(_)) + .WillRepeatedly(Invoke([&](FilterChainFactoryCallbacks& callbacks) -> void { + callbacks.addStreamDecoderFilter(decoder_filter, createRequestTreeMatchingTree()); + })); + + RequestHeaderMapPtr grpc_headers{ + new TestRequestHeaderMapImpl{{":authority", "host"}, + {":path", "/"}, + {":method", "GET"}, + {"match-header", "match"}, + {"content-type", "application/grpc"}}}; + + ON_CALL(filter_manager_callbacks_, requestHeaders()) + .WillByDefault(Return(absl::make_optional(std::ref(*grpc_headers)))); + filter_manager_->createFilterChain(); + + filter_manager_->requestHeadersInitialized(); + filter_manager_->decodeHeaders(*grpc_headers, true); + filter_manager_->destroyFilters(); +} } // namespace } // namespace Http } // namespace Envoy \ No newline at end of file diff --git a/test/common/router/router_test.cc b/test/common/router/router_test.cc index 3b59a864b240f..e5bca652da856 100644 --- a/test/common/router/router_test.cc +++ b/test/common/router/router_test.cc @@ -54,7 +54,6 @@ using testing::AtLeast; using testing::Eq; using testing::InSequence; using testing::Invoke; -using testing::Matcher; using testing::MockFunction; using testing::NiceMock; using testing::Property; @@ -5969,7 +5968,7 @@ TEST(RouterFilterUtilityTest, ShouldShadow) { TestShadowPolicy policy("cluster", "foo", fractional_percent); NiceMock runtime; EXPECT_CALL(runtime.snapshot_, - featureEnabled("foo", Matcher(_), 3)) + featureEnabled("foo", testing::Matcher(_), 3)) .WillOnce(Return(true)); EXPECT_TRUE(FilterUtility::shouldShadow(policy, runtime, 3)); } From 910d3f148e6e0e96fa945b0ddb3fb75bfb063160 Mon Sep 17 00:00:00 2001 From: Snow Pettersen Date: Fri, 4 Dec 2020 14:36:27 +0000 Subject: [PATCH 02/32] support skipping filter execution ; Signed-off-by: Snow Pettersen --- include/envoy/http/BUILD | 2 +- include/envoy/http/filter.h | 17 +++- include/envoy/http/header_map.h | 3 +- include/envoy/matcher/matcher.h | 3 +- source/common/http/BUILD | 5 +- source/common/http/filter_manager.cc | 75 ++++++++++++---- source/common/http/filter_manager.h | 110 ++++++++++++++++++------ source/common/http/header_utility.cc | 2 +- test/common/http/filter_manager_test.cc | 82 ++++++++++++++++-- test/common/router/router_test.cc | 5 +- 10 files changed, 245 insertions(+), 59 deletions(-) diff --git a/include/envoy/http/BUILD b/include/envoy/http/BUILD index 7e93a12e310a5..a350d27f3e61c 100644 --- a/include/envoy/http/BUILD +++ b/include/envoy/http/BUILD @@ -78,8 +78,8 @@ envoy_cc_library( "//include/envoy/common:scope_tracker_interface", "//include/envoy/event:dispatcher_interface", "//include/envoy/grpc:status", - "//include/envoy/router:router_interface", "//include/envoy/matcher:matcher_interface", + "//include/envoy/router:router_interface", "//include/envoy/ssl:connection_interface", "//include/envoy/stream_info:stream_info_interface", "//include/envoy/tracing:http_tracer_interface", diff --git a/include/envoy/http/filter.h b/include/envoy/http/filter.h index 6a9ba6b0e9db6..2738231e7d61e 100644 --- a/include/envoy/http/filter.h +++ b/include/envoy/http/filter.h @@ -5,13 +5,13 @@ #include #include -#include "envoy/matcher/matcher.h" #include "envoy/access_log/access_log.h" #include "envoy/common/scope_tracker.h" #include "envoy/event/dispatcher.h" #include "envoy/grpc/status.h" #include "envoy/http/codec.h" #include "envoy/http/header_map.h" +#include "envoy/matcher/matcher.h" #include "envoy/router/router.h" #include "envoy/ssl/connection.h" #include "envoy/tracing/http_tracer.h" @@ -901,12 +901,27 @@ class FilterChainFactoryCallbacks { */ virtual void addStreamEncoderFilter(Http::StreamEncoderFilterSharedPtr filter) PURE; + /** + * Add an encoder filter that is used when writing stream data. + * @param filter supplies the filter to add. + */ + virtual void + addStreamEncoderFilter(Http::StreamEncoderFilterSharedPtr filter, + Matcher::MatchTreeSharedPtr match_tree) PURE; + /** * Add a decoder/encoder filter that is used both when reading and writing stream data. * @param filter supplies the filter to add. */ virtual void addStreamFilter(Http::StreamFilterSharedPtr filter) PURE; + /** + * Add a decoder/encoder filter that is used both when reading and writing stream data. + * @param filter supplies the filter to add. + */ + virtual void addStreamFilter(Http::StreamFilterSharedPtr filter, + Matcher::MatchTreeSharedPtr match_tree) PURE; + /** * Add an access log handler that is called when the stream is destroyed. * @param handler supplies the handler to add. diff --git a/include/envoy/http/header_map.h b/include/envoy/http/header_map.h index e5d064612caaa..06bbe4f7a91b4 100644 --- a/include/envoy/http/header_map.h +++ b/include/envoy/http/header_map.h @@ -777,7 +777,8 @@ class ResponseHeaderMap }; using ResponseHeaderMapPtr = std::unique_ptr; using ResponseHeaderMapOptRef = absl::optional>; -using ResponseHeaderMapOptConstRef = absl::optional>; +using ResponseHeaderMapOptConstRef = + absl::optional>; // Response trailers. class ResponseTrailerMap diff --git a/include/envoy/matcher/matcher.h b/include/envoy/matcher/matcher.h index cb4ec68a981ee..613274ce389a2 100644 --- a/include/envoy/matcher/matcher.h +++ b/include/envoy/matcher/matcher.h @@ -120,8 +120,7 @@ template class MatchTree { virtual MatchResult match(const DataType& matching_data) PURE; }; -template -using MatchTreeSharedPtr = std::shared_ptr>; +template using MatchTreeSharedPtr = std::shared_ptr>; // InputMatcher provides the interface for determining whether an input value matches. class InputMatcher { diff --git a/source/common/http/BUILD b/source/common/http/BUILD index 99701c3e4b4ca..f8f99bce38fc6 100644 --- a/source/common/http/BUILD +++ b/source/common/http/BUILD @@ -168,13 +168,14 @@ envoy_cc_library( deps = [ ":headers_lib", "//include/envoy/http:filter_interface", + "//include/envoy/matcher:matcher_interface", "//source/common/buffer:watermark_buffer_lib", "//source/common/common:linked_object", - "//include/envoy/matcher:matcher_interface", - "//source/common/matcher:matcher_lib", "//source/common/common:scope_tracker", "//source/common/grpc:common_lib", "//source/common/local_reply:local_reply_lib", + "//source/common/matcher:matcher_lib", + "@envoy_api//envoy/config/common/matcher/v3:pkg_cc_proto", ], ) diff --git a/source/common/http/filter_manager.cc b/source/common/http/filter_manager.cc index 0e992b731bd18..e1608321f7d11 100644 --- a/source/common/http/filter_manager.cc +++ b/source/common/http/filter_manager.cc @@ -1,6 +1,7 @@ #include "common/http/filter_manager.h" #include "envoy/http/header_map.h" +#include "envoy/matcher/matcher.h" #include "common/common/enum_to_int.h" #include "common/common/scope_tracker.h" @@ -9,7 +10,6 @@ #include "common/http/header_utility.h" #include "common/http/utility.h" #include "common/runtime/runtime_features.h" -#include "include/envoy/matcher/_virtual_includes/matcher_interface/envoy/matcher/matcher.h" namespace Envoy { namespace Http { @@ -242,6 +242,25 @@ void ActiveStreamFilterBase::clearRouteCache() { parent_.filter_manager_callbacks_.clearRouteCache(); } +void ActiveStreamFilterBase::evaluateMatchTreeWithNewData( + std::function update_func) { + if (match_tree_evaluated_ || !matching_data_) { + return; + } + + update_func(*matching_data_); + + const auto match_result = Matcher::evaluateMatch(*match_tree_, *matching_data_); + + match_tree_evaluated_ = match_result.match_state_ == Matcher::MatchState::MatchComplete; + + if (match_tree_evaluated_ && match_result.result_) { + if (dynamic_cast(match_result.result_.get())) { + skip_ = true; + } + } +} + bool ActiveStreamDecoderFilter::canContinue() { // It is possible for the connection manager to respond directly to a request even while // a filter is trying to continue. If a response has already happened, we should not @@ -385,9 +404,11 @@ void ActiveStreamDecoderFilter::requestDataTooLarge() { } } -void FilterManager::addStreamDecoderFilterWorker(StreamDecoderFilterSharedPtr filter, - Matcher::MatchTreeSharedPtr matcher, bool dual_filter) { - ActiveStreamDecoderFilterPtr wrapper(new ActiveStreamDecoderFilter(*this, filter, matcher, dual_filter)); +void FilterManager::addStreamDecoderFilterWorker( + StreamDecoderFilterSharedPtr filter, Matcher::MatchTreeSharedPtr matcher, + HttpMatchingDataImplSharedPtr matching_data, bool dual_filter) { + ActiveStreamDecoderFilterPtr wrapper(new ActiveStreamDecoderFilter( + *this, filter, std::move(matcher), std::move(matching_data), dual_filter)); filter->setDecoderFilterCallbacks(*wrapper); // Note: configured decoder filters are appended to decoder_filters_. // This means that if filters are configured in the following order (assume all three filters are @@ -400,9 +421,11 @@ void FilterManager::addStreamDecoderFilterWorker(StreamDecoderFilterSharedPtr fi LinkedList::moveIntoListBack(std::move(wrapper), decoder_filters_); } -void FilterManager::addStreamEncoderFilterWorker(StreamEncoderFilterSharedPtr filter, - bool dual_filter) { - ActiveStreamEncoderFilterPtr wrapper(new ActiveStreamEncoderFilter(*this, filter, dual_filter)); +void FilterManager::addStreamEncoderFilterWorker( + StreamEncoderFilterSharedPtr filter, Matcher::MatchTreeSharedPtr match_tree, + HttpMatchingDataImplSharedPtr matching_data, bool dual_filter) { + ActiveStreamEncoderFilterPtr wrapper(new ActiveStreamEncoderFilter( + *this, filter, std::move(match_tree), std::move(matching_data), dual_filter)); filter->setEncoderFilterCallbacks(*wrapper); // Note: configured encoder filters are prepended to encoder_filters_. // This means that if filters are configured in the following order (assume all three filters are @@ -440,15 +463,10 @@ void FilterManager::decodeHeaders(ActiveStreamDecoderFilter* filter, RequestHead std::list::iterator continue_data_entry = decoder_filters_.end(); for (; entry != decoder_filters_.end(); entry++) { - if ((*entry)->matcher_) { - (*entry)->matching_data_.onRequestHeaders(headers); - const auto match_result = Matcher::evaluateMatch(*(*entry)->matcher_, (*entry)->matching_data_); - if (match_result.match_state_ == Matcher::MatchState::MatchComplete && match_result.result_) { - if (dynamic_cast(match_result.result_.get())) { - (*entry)->skip_ = true; - continue; - } - } + (*entry)->evaluateMatchTreeWithNewData( + [&](auto& matching_data) { matching_data.onRequestHeaders(headers); }); + if ((*entry)->skip_) { + continue; } ASSERT(!(state_.filter_call_state_ & FilterCallState::DecodeHeaders)); @@ -527,6 +545,9 @@ void FilterManager::decodeData(ActiveStreamDecoderFilter* filter, Buffer::Instan commonDecodePrefix(filter, filter_iteration_start_state); for (; entry != decoder_filters_.end(); entry++) { + if ((*entry)->skip_) { + continue; + } // If the filter pointed by entry has stopped for all frame types, return now. if (handleDataIfStopAll(**entry, data, state_.decoder_filters_streaming_)) { return; @@ -658,6 +679,10 @@ void FilterManager::decodeTrailers(ActiveStreamDecoderFilter* filter, RequestTra commonDecodePrefix(filter, FilterIterationStartState::CanStartFromCurrent); for (; entry != decoder_filters_.end(); entry++) { + if ((*entry)->skip_) { + continue; + } + // If the filter pointed by entry has stopped for all frame type, return now. if ((*entry)->stoppedAll()) { return; @@ -686,6 +711,9 @@ void FilterManager::decodeMetadata(ActiveStreamDecoderFilter* filter, MetadataMa commonDecodePrefix(filter, FilterIterationStartState::CanStartFromCurrent); for (; entry != decoder_filters_.end(); entry++) { + if ((*entry)->skip_) { + continue; + } // If the filter pointed by entry has stopped for all frame type, stores metadata and returns. // If the filter pointed by entry hasn't returned from decodeHeaders, stores newly added // metadata in case decodeHeaders returns StopAllIteration. The latter can happen when headers @@ -897,6 +925,10 @@ void FilterManager::encode100ContinueHeaders(ActiveStreamEncoderFilter* filter, std::list::iterator entry = commonEncodePrefix(filter, false, FilterIterationStartState::AlwaysStartFromNext); for (; entry != encoder_filters_.end(); entry++) { + if ((*entry)->skip_) { + continue; + } + ASSERT(!(state_.filter_call_state_ & FilterCallState::Encode100ContinueHeaders)); state_.filter_call_state_ |= FilterCallState::Encode100ContinueHeaders; FilterHeadersStatus status = (*entry)->handle_->encode100ContinueHeaders(headers); @@ -938,6 +970,11 @@ void FilterManager::encodeHeaders(ActiveStreamEncoderFilter* filter, ResponseHea std::list::iterator continue_data_entry = encoder_filters_.end(); for (; entry != encoder_filters_.end(); entry++) { + (*entry)->evaluateMatchTreeWithNewData( + [&headers](auto& matching_data) { matching_data.onResponseHeaders(headers); }); + if ((*entry)->skip_) { + continue; + } ASSERT(!(state_.filter_call_state_ & FilterCallState::EncodeHeaders)); state_.filter_call_state_ |= FilterCallState::EncodeHeaders; (*entry)->end_stream_ = (end_stream && continue_data_entry == encoder_filters_.end()); @@ -992,6 +1029,9 @@ void FilterManager::encodeMetadata(ActiveStreamEncoderFilter* filter, commonEncodePrefix(filter, false, FilterIterationStartState::CanStartFromCurrent); for (; entry != encoder_filters_.end(); entry++) { + if ((*entry)->skip_) { + continue; + } // If the filter pointed by entry has stopped for all frame type, stores metadata and returns. // If the filter pointed by entry hasn't returned from encodeHeaders, stores newly added // metadata in case encodeHeaders returns StopAllIteration. The latter can happen when headers @@ -1060,6 +1100,9 @@ void FilterManager::encodeData(ActiveStreamEncoderFilter* filter, Buffer::Instan const bool trailers_exists_at_start = filter_manager_callbacks_.responseTrailers().has_value(); for (; entry != encoder_filters_.end(); entry++) { + if ((*entry)->skip_) { + continue; + } // If the filter pointed by entry has stopped for all frame type, return now. if (handleDataIfStopAll(**entry, data, state_.encoder_filters_streaming_)) { return; diff --git a/source/common/http/filter_manager.h b/source/common/http/filter_manager.h index 3ebf80f9f979e..bbd7bd8480b60 100644 --- a/source/common/http/filter_manager.h +++ b/source/common/http/filter_manager.h @@ -1,21 +1,20 @@ #pragma once -#include "common/http/filter_manager.h" -#include "common/http/header_utility.h" +#include "envoy/config/common/matcher/v3/action.pb.h" #include "envoy/http/filter.h" #include "envoy/http/header_map.h" +#include "envoy/matcher/matcher.h" #include "common/buffer/watermark_buffer.h" #include "common/common/dump_state_utils.h" #include "common/common/linked_object.h" #include "common/common/logger.h" #include "common/grpc/common.h" +#include "common/http/filter_manager.h" +#include "common/http/header_utility.h" #include "common/http/headers.h" -#include "common/matcher/matcher.h" #include "common/local_reply/local_reply.h" - -#include "envoy/config/common/matcher/v3/action.pb.h" -#include "envoy/matcher/matcher.h" +#include "common/matcher/matcher.h" namespace Envoy { namespace Http { @@ -55,12 +54,17 @@ class HttpMatchingDataImpl : public HttpMatchingData { const Http::ResponseHeaderMap* response_headers_{}; }; -class HttpRequestHeadersDataInput : public Matcher::DataInput { +using HttpMatchingDataImplSharedPtr = std::shared_ptr; + +class HttpHeadersDataInputBase : public Matcher::DataInput { public: - explicit HttpRequestHeadersDataInput(const std::string& name) : name_(name) {} + explicit HttpHeadersDataInputBase(const std::string& name) : name_(name) {} + + virtual absl::optional> + headerMap(const HttpMatchingData& data) const PURE; Matcher::DataInputGetResult get(const HttpMatchingData& data) override { - const auto maybe_headers = data.requestHeaders(); + const auto maybe_headers = headerMap(data); if (!maybe_headers) { return {Matcher::DataInputGetResult::DataAvailability::NotAvailable, absl::nullopt}; @@ -87,6 +91,26 @@ class HttpRequestHeadersDataInput : public Matcher::DataInput absl::optional header_as_string_result_; }; +class HttpRequestHeadersDataInput : public HttpHeadersDataInputBase { +public: + explicit HttpRequestHeadersDataInput(const std::string& name) : HttpHeadersDataInputBase(name) {} + + absl::optional> + headerMap(const HttpMatchingData& data) const override { + return data.requestHeaders(); + } +}; + +class HttpResponseHeadersDataInput : public HttpHeadersDataInputBase { +public: + explicit HttpResponseHeadersDataInput(const std::string& name) : HttpHeadersDataInputBase(name) {} + + absl::optional> + headerMap(const HttpMatchingData& data) const override { + return data.responseHeaders(); + } +}; + class SkipAction : public Matcher::ActionBase {}; /** @@ -94,11 +118,15 @@ class SkipAction : public Matcher::ActionBase { - ActiveStreamFilterBase(FilterManager& parent, bool dual_filter) + ActiveStreamFilterBase(FilterManager& parent, bool dual_filter, + Matcher::MatchTreeSharedPtr match_tree, + HttpMatchingDataImplSharedPtr matching_data) : parent_(parent), iteration_state_(IterationState::Continue), + match_tree_(std::move(match_tree)), matching_data_(std::move(matching_data)), iterate_from_current_filter_(false), headers_continued_(false), continue_headers_continued_(false), end_stream_(false), dual_filter_(dual_filter), - decode_headers_called_(false), encode_headers_called_(false), skip_(false) {} + decode_headers_called_(false), encode_headers_called_(false), match_tree_evaluated_(false), + skip_(false) {} // Functions in the following block are called after the filter finishes processing // corresponding data. Those functions handle state updates and data storage (if needed) @@ -168,6 +196,8 @@ struct ActiveStreamFilterBase : public virtual StreamFilterCallbacks, return saved_response_metadata_.get(); } + void evaluateMatchTreeWithNewData(std::function update_func); + // A vector to save metadata when the current filter's [de|en]codeMetadata() can not be called, // either because [de|en]codeHeaders() of the current filter returns StopAllIteration or because // [de|en]codeHeaders() adds new metadata to [de|en]code, but we don't know @@ -185,6 +215,10 @@ struct ActiveStreamFilterBase : public virtual StreamFilterCallbacks, }; FilterManager& parent_; IterationState iteration_state_; + + Matcher::MatchTreeSharedPtr match_tree_; + HttpMatchingDataImplSharedPtr matching_data_; + // If the filter resumes iteration from a StopAllBuffer/Watermark state, the current filter // hasn't parsed data and trailers. As a result, the filter iteration should start with the // current filter instead of the next one. If true, filter iteration starts with the current @@ -197,6 +231,7 @@ struct ActiveStreamFilterBase : public virtual StreamFilterCallbacks, const bool dual_filter_ : 1; bool decode_headers_called_ : 1; bool encode_headers_called_ : 1; + bool match_tree_evaluated_ : 1; bool skip_ : 1; }; @@ -207,8 +242,11 @@ struct ActiveStreamDecoderFilter : public ActiveStreamFilterBase, public StreamDecoderFilterCallbacks, LinkedObject { ActiveStreamDecoderFilter(FilterManager& parent, StreamDecoderFilterSharedPtr filter, - Matcher::MatchTreeSharedPtr matcher, bool dual_filter) - : ActiveStreamFilterBase(parent, dual_filter), handle_(filter), matcher_(matcher) {} + Matcher::MatchTreeSharedPtr match_tree, + HttpMatchingDataImplSharedPtr matching_data, bool dual_filter) + : ActiveStreamFilterBase(parent, dual_filter, std::move(match_tree), + std::move(matching_data)), + handle_(filter) {} // ActiveStreamFilterBase bool canContinue() override; @@ -281,8 +319,6 @@ struct ActiveStreamDecoderFilter : public ActiveStreamFilterBase, absl::optional routeConfig(); StreamDecoderFilterSharedPtr handle_; - Matcher::MatchTreeSharedPtr matcher_; - HttpMatchingDataImpl matching_data_; bool is_grpc_request_{}; }; @@ -295,8 +331,11 @@ struct ActiveStreamEncoderFilter : public ActiveStreamFilterBase, public StreamEncoderFilterCallbacks, LinkedObject { ActiveStreamEncoderFilter(FilterManager& parent, StreamEncoderFilterSharedPtr filter, - bool dual_filter) - : ActiveStreamFilterBase(parent, dual_filter), handle_(filter) {} + Matcher::MatchTreeSharedPtr match_tree, + HttpMatchingDataImplSharedPtr matching_data, bool dual_filter) + : ActiveStreamFilterBase(parent, dual_filter, std::move(match_tree), + std::move(matching_data)), + handle_(filter) {} // ActiveStreamFilterBase bool canContinue() override { return true; } @@ -593,18 +632,37 @@ class FilterManager : public ScopeTrackedObject, // Http::FilterChainFactoryCallbacks void addStreamDecoderFilter(StreamDecoderFilterSharedPtr filter) override { - addStreamDecoderFilterWorker(filter, nullptr, false); + addStreamDecoderFilterWorker(filter, nullptr, nullptr, false); } void addStreamDecoderFilter(StreamDecoderFilterSharedPtr filter, Matcher::MatchTreeSharedPtr match_tree) override { - addStreamDecoderFilterWorker(filter, match_tree, false); + auto matching_data = match_tree ? std::make_shared() : nullptr; + + addStreamDecoderFilterWorker(filter, std::move(match_tree), std::move(matching_data), false); } void addStreamEncoderFilter(StreamEncoderFilterSharedPtr filter) override { - addStreamEncoderFilterWorker(filter, false); + addStreamEncoderFilterWorker(filter, nullptr, nullptr, false); + } + void addStreamEncoderFilter(StreamEncoderFilterSharedPtr filter, + Matcher::MatchTreeSharedPtr match_tree) override { + auto matching_data = match_tree ? std::make_shared() : nullptr; + + addStreamEncoderFilterWorker(filter, std::move(match_tree), std::move(matching_data), false); } void addStreamFilter(StreamFilterSharedPtr filter) override { - addStreamDecoderFilterWorker(filter, nullptr, true); - addStreamEncoderFilterWorker(filter, true); + addStreamDecoderFilterWorker(filter, nullptr, nullptr, true); + addStreamEncoderFilterWorker(filter, nullptr, nullptr, true); + } + void addStreamFilter(StreamFilterSharedPtr filter, + Matcher::MatchTreeSharedPtr match_tree) override { + // Note that we share the match data and tree between the two filters to allow things like + // matching on both request and response data. + // TODO(snowp): The match tree might be fully evaluated twice, ideally we should expose + // the result to both filters after the first match evaluation. + auto matching_data = match_tree ? std::make_shared() : nullptr; + + addStreamDecoderFilterWorker(filter, match_tree, matching_data, true); + addStreamEncoderFilterWorker(filter, std::move(match_tree), std::move(matching_data), true); } void addAccessLogHandler(AccessLog::InstanceSharedPtr handler) override; @@ -687,9 +745,11 @@ class FilterManager : public ScopeTrackedObject, // TODO(snowp): Make private as filter chain construction is moved into FM. void addStreamDecoderFilterWorker(StreamDecoderFilterSharedPtr filter, - Matcher::MatchTreeSharedPtr matcher, - bool dual_filter); - void addStreamEncoderFilterWorker(StreamEncoderFilterSharedPtr filter, bool dual_filter); + Matcher::MatchTreeSharedPtr match_tree, + HttpMatchingDataImplSharedPtr matching_data, bool dual_filter); + void addStreamEncoderFilterWorker(StreamEncoderFilterSharedPtr filter, + Matcher::MatchTreeSharedPtr match_tree, + HttpMatchingDataImplSharedPtr matching_data, bool dual_filter); void disarmRequestTimeout(); diff --git a/source/common/http/header_utility.cc b/source/common/http/header_utility.cc index 26f6106bdb8fb..a3c2747b294cb 100644 --- a/source/common/http/header_utility.cc +++ b/source/common/http/header_utility.cc @@ -96,7 +96,7 @@ bool HeaderUtility::matchHeaders(const HeaderMap& request_headers, HeaderUtility::GetAllOfHeaderAsStringResult HeaderUtility::getAllOfHeaderAsString(const HeaderMap::GetResult& header_value, - absl::string_view seperator) { + absl::string_view seperator) { GetAllOfHeaderAsStringResult result; // In this case we concatenate all found headers using a delimiter before performing the // final match. We use an InlinedVector of absl::string_view to invoke the optimized join diff --git a/test/common/http/filter_manager_test.cc b/test/common/http/filter_manager_test.cc index 7200238de0d0f..08a34e7054f3e 100644 --- a/test/common/http/filter_manager_test.cc +++ b/test/common/http/filter_manager_test.cc @@ -1,12 +1,13 @@ +#include "envoy/http/filter.h" #include "envoy/http/header_map.h" +#include "envoy/matcher/matcher.h" #include "envoy/stream_info/filter_state.h" #include "common/http/filter_manager.h" +#include "common/matcher/exact_map_matcher.h" #include "common/stream_info/filter_state_impl.h" #include "common/stream_info/stream_info_impl.h" -#include "include/envoy/matcher/_virtual_includes/matcher_interface/envoy/matcher/matcher.h" -#include "source/common/matcher/_virtual_includes/exact_map_matcher_lib/common/matcher/exact_map_matcher.h" #include "test/mocks/event/mocks.h" #include "test/mocks/http/mocks.h" #include "test/mocks/local_reply/mocks.h" @@ -143,18 +144,28 @@ TEST_F(FilterManagerTest, SendLocalReplyDuringEncodingGrpcClassiciation) { filter_manager_->destroyFilters(); } -Matcher::MatchTreeSharedPtr createRequestTreeMatchingTree() { +Matcher::MatchTreeSharedPtr createRequestMatchingTree() { auto tree = std::make_shared>( - std::make_unique("match-header"), - absl::nullopt); + std::make_unique("match-header"), absl::nullopt); tree->addChild("match", Matcher::OnMatch{ - []() { return std::make_unique(); }, nullptr}); + []() { return std::make_unique(); }, nullptr}); return tree; } -TEST_F(FilterManagerTest, MatchTreeSkipAction) { +Matcher::MatchTreeSharedPtr createRequestAndResponseMatchingTree() { + auto tree = std::make_shared>( + std::make_unique("match-header"), absl::nullopt); + + tree->addChild("match", + Matcher::OnMatch{[]() { return std::make_unique(); }, + createRequestMatchingTree()}); + + return tree; +} + +TEST_F(FilterManagerTest, MatchTreeSkipActionDecodingHeaders) { initialize(); // The filter is added, but since we match on the request header we skip the filter. @@ -164,7 +175,7 @@ TEST_F(FilterManagerTest, MatchTreeSkipAction) { EXPECT_CALL(filter_factory_, createFilterChain(_)) .WillRepeatedly(Invoke([&](FilterChainFactoryCallbacks& callbacks) -> void { - callbacks.addStreamDecoderFilter(decoder_filter, createRequestTreeMatchingTree()); + callbacks.addStreamDecoderFilter(decoder_filter, createRequestMatchingTree()); })); RequestHeaderMapPtr grpc_headers{ @@ -182,6 +193,61 @@ TEST_F(FilterManagerTest, MatchTreeSkipAction) { filter_manager_->decodeHeaders(*grpc_headers, true); filter_manager_->destroyFilters(); } + +TEST_F(FilterManagerTest, MatchTreeSkipActionRequestAndResponseHeaders) { + initialize(); + + EXPECT_CALL(dispatcher_, setTrackedObject(_)).Times(2); + + // This stream filter will skip further callbacks once it sees both the request and response + // header. As such, it should see the decoding callbacks but none of the encoding callbacks. + auto stream_filter = std::make_shared(); + EXPECT_CALL(*stream_filter, setDecoderFilterCallbacks(_)); + EXPECT_CALL(*stream_filter, setEncoderFilterCallbacks(_)); + EXPECT_CALL(*stream_filter, onDestroy()); + EXPECT_CALL(*stream_filter, decodeHeaders(_, false)) + .WillOnce(Return(FilterHeadersStatus::Continue)); + EXPECT_CALL(*stream_filter, decodeData(_, true)).WillOnce(Return(FilterDataStatus::Continue)); + + auto decoder_filter = std::make_shared>(); + EXPECT_CALL(*decoder_filter, decodeHeaders(_, false)) + .WillOnce(Return(FilterHeadersStatus::StopIteration)); + EXPECT_CALL(*decoder_filter, decodeData(_, true)) + .WillOnce(Invoke([&](auto&, bool) -> FilterDataStatus { + ResponseHeaderMapPtr headers{new TestResponseHeaderMapImpl{ + {":status", "200"}, {"match-header", "match"}, {"content-type", "application/grpc"}}}; + decoder_filter->callbacks_->encodeHeaders(std::move(headers), false, "details"); + + Buffer::OwnedImpl data("data"); + decoder_filter->callbacks_->encodeData(data, true); + return FilterDataStatus::StopIterationNoBuffer; + })); + + EXPECT_CALL(filter_factory_, createFilterChain(_)) + .WillRepeatedly(Invoke([&](FilterChainFactoryCallbacks& callbacks) -> void { + callbacks.addStreamFilter(stream_filter, createRequestAndResponseMatchingTree()); + callbacks.addStreamDecoderFilter(decoder_filter); + })); + + RequestHeaderMapPtr headers{new TestRequestHeaderMapImpl{{":authority", "host"}, + {":path", "/"}, + {":method", "GET"}, + {"match-header", "match"}, + {"content-type", "application/grpc"}}}; + Buffer::OwnedImpl data("data"); + + ON_CALL(filter_manager_callbacks_, requestHeaders()) + .WillByDefault(Return(absl::make_optional(std::ref(*headers)))); + filter_manager_->createFilterChain(); + + EXPECT_CALL(filter_manager_callbacks_, encodeHeaders(_, _)); + EXPECT_CALL(filter_manager_callbacks_, endStream()); + + filter_manager_->requestHeadersInitialized(); + filter_manager_->decodeHeaders(*headers, false); + filter_manager_->decodeData(data, true); + filter_manager_->destroyFilters(); +} } // namespace } // namespace Http } // namespace Envoy \ No newline at end of file diff --git a/test/common/router/router_test.cc b/test/common/router/router_test.cc index e5bca652da856..1d9d5fd6cb305 100644 --- a/test/common/router/router_test.cc +++ b/test/common/router/router_test.cc @@ -5967,8 +5967,9 @@ TEST(RouterFilterUtilityTest, ShouldShadow) { fractional_percent.set_denominator(envoy::type::v3::FractionalPercent::TEN_THOUSAND); TestShadowPolicy policy("cluster", "foo", fractional_percent); NiceMock runtime; - EXPECT_CALL(runtime.snapshot_, - featureEnabled("foo", testing::Matcher(_), 3)) + EXPECT_CALL( + runtime.snapshot_, + featureEnabled("foo", testing::Matcher(_), 3)) .WillOnce(Return(true)); EXPECT_TRUE(FilterUtility::shouldShadow(policy, runtime, 3)); } From 5734a5c133f1a764030d2fe4c4745f6781f0ec98 Mon Sep 17 00:00:00 2001 From: Snow Pettersen Date: Mon, 7 Dec 2020 19:32:14 +0000 Subject: [PATCH 03/32] move protos Signed-off-by: Snow Pettersen --- api/BUILD | 2 ++ .../config/common/matcher/v3/action.proto | 19 ------------ .../config/common/matcher/v3/matcher.proto | 1 - .../common/matcher/v3/skip_action.proto | 6 ++-- .../common/matcher/v4alpha/matcher.proto | 1 - .../{action.proto => skip_action.proto} | 8 ++--- api/envoy/extensions/common/matching/v3/BUILD | 13 ++++++++ .../matching/v3/extension_matcher.proto | 27 +++++++++++++++++ .../extensions/common/matching/v4alpha/BUILD | 14 +++++++++ .../matching/v4alpha/extension_matcher.proto | 30 +++++++++++++++++++ .../filters/common/matching/v3/BUILD | 9 ++++++ .../common/matching/v3/skip_action.proto | 19 ++++++++++++ api/versioning/BUILD | 2 ++ .../config/common/matcher/v3/matcher.proto | 1 - .../common/matcher/v3/skip_action.proto | 19 ++++++++++++ .../common/matcher/v4alpha/matcher.proto | 1 - .../{action.proto => skip_action.proto} | 8 ++--- .../envoy/extensions/common/matching/v3/BUILD | 13 ++++++++ .../matching/v3/extension_matcher.proto | 27 +++++++++++++++++ .../extensions/common/matching/v4alpha/BUILD | 14 +++++++++ .../matching/v4alpha/extension_matcher.proto | 30 +++++++++++++++++++ .../filters/common/matching/v3/BUILD | 9 ++++++ .../common/matching/v3/skip_action.proto | 19 ++++++++++++ include/envoy/http/filter.h | 3 ++ 24 files changed, 261 insertions(+), 34 deletions(-) rename generated_api_shadow/envoy/config/common/matcher/v3/action.proto => api/envoy/config/common/matcher/v3/skip_action.proto (76%) rename api/envoy/config/common/matcher/v4alpha/{action.proto => skip_action.proto} (71%) create mode 100644 api/envoy/extensions/common/matching/v3/BUILD create mode 100644 api/envoy/extensions/common/matching/v3/extension_matcher.proto create mode 100644 api/envoy/extensions/common/matching/v4alpha/BUILD create mode 100644 api/envoy/extensions/common/matching/v4alpha/extension_matcher.proto create mode 100644 api/envoy/extensions/filters/common/matching/v3/BUILD create mode 100644 api/envoy/extensions/filters/common/matching/v3/skip_action.proto create mode 100644 generated_api_shadow/envoy/config/common/matcher/v3/skip_action.proto rename generated_api_shadow/envoy/config/common/matcher/v4alpha/{action.proto => skip_action.proto} (71%) create mode 100644 generated_api_shadow/envoy/extensions/common/matching/v3/BUILD create mode 100644 generated_api_shadow/envoy/extensions/common/matching/v3/extension_matcher.proto create mode 100644 generated_api_shadow/envoy/extensions/common/matching/v4alpha/BUILD create mode 100644 generated_api_shadow/envoy/extensions/common/matching/v4alpha/extension_matcher.proto create mode 100644 generated_api_shadow/envoy/extensions/filters/common/matching/v3/BUILD create mode 100644 generated_api_shadow/envoy/extensions/filters/common/matching/v3/skip_action.proto diff --git a/api/BUILD b/api/BUILD index cc41b7c0d8aa3..2249d224fc5b8 100644 --- a/api/BUILD +++ b/api/BUILD @@ -160,11 +160,13 @@ proto_library( "//envoy/extensions/clusters/dynamic_forward_proxy/v3:pkg", "//envoy/extensions/clusters/redis/v3:pkg", "//envoy/extensions/common/dynamic_forward_proxy/v3:pkg", + "//envoy/extensions/common/matching/v3:pkg", "//envoy/extensions/common/ratelimit/v3:pkg", "//envoy/extensions/common/tap/v3:pkg", "//envoy/extensions/compression/gzip/compressor/v3:pkg", "//envoy/extensions/compression/gzip/decompressor/v3:pkg", "//envoy/extensions/filters/common/fault/v3:pkg", + "//envoy/extensions/filters/common/matching/v3:pkg", "//envoy/extensions/filters/http/adaptive_concurrency/v3:pkg", "//envoy/extensions/filters/http/admission_control/v3alpha:pkg", "//envoy/extensions/filters/http/aws_lambda/v3:pkg", diff --git a/api/envoy/config/common/matcher/v3/action.proto b/api/envoy/config/common/matcher/v3/action.proto index 28afda10ffa1e..e69de29bb2d1d 100644 --- a/api/envoy/config/common/matcher/v3/action.proto +++ b/api/envoy/config/common/matcher/v3/action.proto @@ -1,19 +0,0 @@ -syntax = "proto3"; - -package envoy.config.common.matcher.v3; - -import "udpa/annotations/migrate.proto"; -import "udpa/annotations/status.proto"; -import "udpa/annotations/versioning.proto"; -import "validate/validate.proto"; - -option java_package = "io.envoyproxy.envoy.config.common.matcher.v3"; -option java_outer_classname = "ActionProto"; -option java_multiple_files = true; -option (udpa.annotations.file_status).package_version_status = ACTIVE; - -// [#protodoc-title: Common Match Actions] - -// Indicates that the associated action should be skipped. -message SkipAction { -} diff --git a/api/envoy/config/common/matcher/v3/matcher.proto b/api/envoy/config/common/matcher/v3/matcher.proto index 23cad325d7c36..0468fb28d2ce8 100644 --- a/api/envoy/config/common/matcher/v3/matcher.proto +++ b/api/envoy/config/common/matcher/v3/matcher.proto @@ -25,7 +25,6 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE; // might repeat several times until the final OnMatch (or no match) is decided. // // This API is a work in progress. -// [#not-implemented-hide:] message Matcher { // What to do if a match is successful. message OnMatch { diff --git a/generated_api_shadow/envoy/config/common/matcher/v3/action.proto b/api/envoy/config/common/matcher/v3/skip_action.proto similarity index 76% rename from generated_api_shadow/envoy/config/common/matcher/v3/action.proto rename to api/envoy/config/common/matcher/v3/skip_action.proto index 28afda10ffa1e..f5408f92dc3c8 100644 --- a/generated_api_shadow/envoy/config/common/matcher/v3/action.proto +++ b/api/envoy/config/common/matcher/v3/skip_action.proto @@ -8,12 +8,12 @@ import "udpa/annotations/versioning.proto"; import "validate/validate.proto"; option java_package = "io.envoyproxy.envoy.config.common.matcher.v3"; -option java_outer_classname = "ActionProto"; +option java_outer_classname = "SkipActionProto"; option java_multiple_files = true; option (udpa.annotations.file_status).package_version_status = ACTIVE; // [#protodoc-title: Common Match Actions] -// Indicates that the associated action should be skipped. -message SkipAction { +// Indicates that the associated filter should be skipped. +message SkipFilterMatchAction { } diff --git a/api/envoy/config/common/matcher/v4alpha/matcher.proto b/api/envoy/config/common/matcher/v4alpha/matcher.proto index 6f20d3d907c80..7769ea590463e 100644 --- a/api/envoy/config/common/matcher/v4alpha/matcher.proto +++ b/api/envoy/config/common/matcher/v4alpha/matcher.proto @@ -24,7 +24,6 @@ option (udpa.annotations.file_status).package_version_status = NEXT_MAJOR_VERSIO // might repeat several times until the final OnMatch (or no match) is decided. // // This API is a work in progress. -// [#not-implemented-hide:] message Matcher { option (udpa.annotations.versioning).previous_message_type = "envoy.config.common.matcher.v3.Matcher"; diff --git a/api/envoy/config/common/matcher/v4alpha/action.proto b/api/envoy/config/common/matcher/v4alpha/skip_action.proto similarity index 71% rename from api/envoy/config/common/matcher/v4alpha/action.proto rename to api/envoy/config/common/matcher/v4alpha/skip_action.proto index 4c047cd2f3900..4d0e930fe137c 100644 --- a/api/envoy/config/common/matcher/v4alpha/action.proto +++ b/api/envoy/config/common/matcher/v4alpha/skip_action.proto @@ -7,14 +7,14 @@ import "udpa/annotations/versioning.proto"; import "validate/validate.proto"; option java_package = "io.envoyproxy.envoy.config.common.matcher.v4alpha"; -option java_outer_classname = "ActionProto"; +option java_outer_classname = "SkipActionProto"; option java_multiple_files = true; option (udpa.annotations.file_status).package_version_status = NEXT_MAJOR_VERSION_CANDIDATE; // [#protodoc-title: Common Match Actions] -// Indicates that the associated action should be skipped. -message SkipAction { +// Indicates that the associated filter should be skipped. +message SkipFilterMatchAction { option (udpa.annotations.versioning).previous_message_type = - "envoy.config.common.matcher.v3.SkipAction"; + "envoy.config.common.matcher.v3.SkipFilterMatchAction"; } diff --git a/api/envoy/extensions/common/matching/v3/BUILD b/api/envoy/extensions/common/matching/v3/BUILD new file mode 100644 index 0000000000000..5fa93360e6558 --- /dev/null +++ b/api/envoy/extensions/common/matching/v3/BUILD @@ -0,0 +1,13 @@ +# DO NOT EDIT. This file is generated by tools/proto_format/proto_sync.py. + +load("@envoy_api//bazel:api_build_system.bzl", "api_proto_package") + +licenses(["notice"]) # Apache 2 + +api_proto_package( + deps = [ + "//envoy/config/common/matcher/v3:pkg", + "//envoy/config/core/v3:pkg", + "@com_github_cncf_udpa//udpa/annotations:pkg", + ], +) diff --git a/api/envoy/extensions/common/matching/v3/extension_matcher.proto b/api/envoy/extensions/common/matching/v3/extension_matcher.proto new file mode 100644 index 0000000000000..370444fd70421 --- /dev/null +++ b/api/envoy/extensions/common/matching/v3/extension_matcher.proto @@ -0,0 +1,27 @@ +syntax = "proto3"; + +package envoy.extensions.common.matching.v3; + +import "envoy/config/common/matcher/v3/matcher.proto"; +import "envoy/config/core/v3/extension.proto"; + +import "udpa/annotations/migrate.proto"; +import "udpa/annotations/status.proto"; +import "validate/validate.proto"; + +option java_package = "io.envoyproxy.envoy.extensions.common.matching.v3"; +option java_outer_classname = "ExtensionMatcherProto"; +option java_multiple_files = true; +option (udpa.annotations.file_status).package_version_status = ACTIVE; + +// Wrapper around an existing extension that provides an associated matcher. This allows +// decorating an existing extension with a matcher, which can be used to match against +// relevant protocol data. +message ExtensionWithMatcher { + // The associated matcher. + config.common.matcher.v3.Matcher matcher = 1 [(validate.rules).message = {required: true}]; + + // The underlying extension config. + config.core.v3.TypedExtensionConfig extension_config = 2 + [(validate.rules).message = {required: true}]; +} diff --git a/api/envoy/extensions/common/matching/v4alpha/BUILD b/api/envoy/extensions/common/matching/v4alpha/BUILD new file mode 100644 index 0000000000000..95ccc22a554af --- /dev/null +++ b/api/envoy/extensions/common/matching/v4alpha/BUILD @@ -0,0 +1,14 @@ +# DO NOT EDIT. This file is generated by tools/proto_format/proto_sync.py. + +load("@envoy_api//bazel:api_build_system.bzl", "api_proto_package") + +licenses(["notice"]) # Apache 2 + +api_proto_package( + deps = [ + "//envoy/config/common/matcher/v4alpha:pkg", + "//envoy/config/core/v4alpha:pkg", + "//envoy/extensions/common/matching/v3:pkg", + "@com_github_cncf_udpa//udpa/annotations:pkg", + ], +) diff --git a/api/envoy/extensions/common/matching/v4alpha/extension_matcher.proto b/api/envoy/extensions/common/matching/v4alpha/extension_matcher.proto new file mode 100644 index 0000000000000..c6a996363bc2b --- /dev/null +++ b/api/envoy/extensions/common/matching/v4alpha/extension_matcher.proto @@ -0,0 +1,30 @@ +syntax = "proto3"; + +package envoy.extensions.common.matching.v4alpha; + +import "envoy/config/common/matcher/v4alpha/matcher.proto"; +import "envoy/config/core/v4alpha/extension.proto"; + +import "udpa/annotations/status.proto"; +import "udpa/annotations/versioning.proto"; +import "validate/validate.proto"; + +option java_package = "io.envoyproxy.envoy.extensions.common.matching.v4alpha"; +option java_outer_classname = "ExtensionMatcherProto"; +option java_multiple_files = true; +option (udpa.annotations.file_status).package_version_status = NEXT_MAJOR_VERSION_CANDIDATE; + +// Wrapper around an existing extension that provides an associated matcher. This allows +// decorating an existing extension with a matcher, which can be used to match against +// relevant protocol data. +message ExtensionWithMatcher { + option (udpa.annotations.versioning).previous_message_type = + "envoy.extensions.common.matching.v3.ExtensionWithMatcher"; + + // The associated matcher. + config.common.matcher.v4alpha.Matcher matcher = 1 [(validate.rules).message = {required: true}]; + + // The underlying extension config. + config.core.v4alpha.TypedExtensionConfig extension_config = 2 + [(validate.rules).message = {required: true}]; +} diff --git a/api/envoy/extensions/filters/common/matching/v3/BUILD b/api/envoy/extensions/filters/common/matching/v3/BUILD new file mode 100644 index 0000000000000..ee92fb652582e --- /dev/null +++ b/api/envoy/extensions/filters/common/matching/v3/BUILD @@ -0,0 +1,9 @@ +# DO NOT EDIT. This file is generated by tools/proto_format/proto_sync.py. + +load("@envoy_api//bazel:api_build_system.bzl", "api_proto_package") + +licenses(["notice"]) # Apache 2 + +api_proto_package( + deps = ["@com_github_cncf_udpa//udpa/annotations:pkg"], +) diff --git a/api/envoy/extensions/filters/common/matching/v3/skip_action.proto b/api/envoy/extensions/filters/common/matching/v3/skip_action.proto new file mode 100644 index 0000000000000..ab7e18e7cf34f --- /dev/null +++ b/api/envoy/extensions/filters/common/matching/v3/skip_action.proto @@ -0,0 +1,19 @@ +syntax = "proto3"; + +package envoy.extensions.filters.common.matching.v3; + +import "udpa/annotations/migrate.proto"; +import "udpa/annotations/status.proto"; +import "udpa/annotations/versioning.proto"; +import "validate/validate.proto"; + +option java_package = "io.envoyproxy.envoy.extensions.filters.common.matching.v3"; +option java_outer_classname = "SkipActionProto"; +option java_multiple_files = true; +option (udpa.annotations.file_status).package_version_status = ACTIVE; + +// [#protodoc-title: Common Match Actions] + +// Indicates that the associated filter should be skipped. +message SkipFilterMatchAction { +} diff --git a/api/versioning/BUILD b/api/versioning/BUILD index d5b6098680bd4..2b0bd46f11264 100644 --- a/api/versioning/BUILD +++ b/api/versioning/BUILD @@ -43,11 +43,13 @@ proto_library( "//envoy/extensions/clusters/dynamic_forward_proxy/v3:pkg", "//envoy/extensions/clusters/redis/v3:pkg", "//envoy/extensions/common/dynamic_forward_proxy/v3:pkg", + "//envoy/extensions/common/matching/v3:pkg", "//envoy/extensions/common/ratelimit/v3:pkg", "//envoy/extensions/common/tap/v3:pkg", "//envoy/extensions/compression/gzip/compressor/v3:pkg", "//envoy/extensions/compression/gzip/decompressor/v3:pkg", "//envoy/extensions/filters/common/fault/v3:pkg", + "//envoy/extensions/filters/common/matching/v3:pkg", "//envoy/extensions/filters/http/adaptive_concurrency/v3:pkg", "//envoy/extensions/filters/http/admission_control/v3alpha:pkg", "//envoy/extensions/filters/http/aws_lambda/v3:pkg", diff --git a/generated_api_shadow/envoy/config/common/matcher/v3/matcher.proto b/generated_api_shadow/envoy/config/common/matcher/v3/matcher.proto index 23cad325d7c36..0468fb28d2ce8 100644 --- a/generated_api_shadow/envoy/config/common/matcher/v3/matcher.proto +++ b/generated_api_shadow/envoy/config/common/matcher/v3/matcher.proto @@ -25,7 +25,6 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE; // might repeat several times until the final OnMatch (or no match) is decided. // // This API is a work in progress. -// [#not-implemented-hide:] message Matcher { // What to do if a match is successful. message OnMatch { diff --git a/generated_api_shadow/envoy/config/common/matcher/v3/skip_action.proto b/generated_api_shadow/envoy/config/common/matcher/v3/skip_action.proto new file mode 100644 index 0000000000000..f5408f92dc3c8 --- /dev/null +++ b/generated_api_shadow/envoy/config/common/matcher/v3/skip_action.proto @@ -0,0 +1,19 @@ +syntax = "proto3"; + +package envoy.config.common.matcher.v3; + +import "udpa/annotations/migrate.proto"; +import "udpa/annotations/status.proto"; +import "udpa/annotations/versioning.proto"; +import "validate/validate.proto"; + +option java_package = "io.envoyproxy.envoy.config.common.matcher.v3"; +option java_outer_classname = "SkipActionProto"; +option java_multiple_files = true; +option (udpa.annotations.file_status).package_version_status = ACTIVE; + +// [#protodoc-title: Common Match Actions] + +// Indicates that the associated filter should be skipped. +message SkipFilterMatchAction { +} diff --git a/generated_api_shadow/envoy/config/common/matcher/v4alpha/matcher.proto b/generated_api_shadow/envoy/config/common/matcher/v4alpha/matcher.proto index 6f20d3d907c80..7769ea590463e 100644 --- a/generated_api_shadow/envoy/config/common/matcher/v4alpha/matcher.proto +++ b/generated_api_shadow/envoy/config/common/matcher/v4alpha/matcher.proto @@ -24,7 +24,6 @@ option (udpa.annotations.file_status).package_version_status = NEXT_MAJOR_VERSIO // might repeat several times until the final OnMatch (or no match) is decided. // // This API is a work in progress. -// [#not-implemented-hide:] message Matcher { option (udpa.annotations.versioning).previous_message_type = "envoy.config.common.matcher.v3.Matcher"; diff --git a/generated_api_shadow/envoy/config/common/matcher/v4alpha/action.proto b/generated_api_shadow/envoy/config/common/matcher/v4alpha/skip_action.proto similarity index 71% rename from generated_api_shadow/envoy/config/common/matcher/v4alpha/action.proto rename to generated_api_shadow/envoy/config/common/matcher/v4alpha/skip_action.proto index 4c047cd2f3900..4d0e930fe137c 100644 --- a/generated_api_shadow/envoy/config/common/matcher/v4alpha/action.proto +++ b/generated_api_shadow/envoy/config/common/matcher/v4alpha/skip_action.proto @@ -7,14 +7,14 @@ import "udpa/annotations/versioning.proto"; import "validate/validate.proto"; option java_package = "io.envoyproxy.envoy.config.common.matcher.v4alpha"; -option java_outer_classname = "ActionProto"; +option java_outer_classname = "SkipActionProto"; option java_multiple_files = true; option (udpa.annotations.file_status).package_version_status = NEXT_MAJOR_VERSION_CANDIDATE; // [#protodoc-title: Common Match Actions] -// Indicates that the associated action should be skipped. -message SkipAction { +// Indicates that the associated filter should be skipped. +message SkipFilterMatchAction { option (udpa.annotations.versioning).previous_message_type = - "envoy.config.common.matcher.v3.SkipAction"; + "envoy.config.common.matcher.v3.SkipFilterMatchAction"; } diff --git a/generated_api_shadow/envoy/extensions/common/matching/v3/BUILD b/generated_api_shadow/envoy/extensions/common/matching/v3/BUILD new file mode 100644 index 0000000000000..5fa93360e6558 --- /dev/null +++ b/generated_api_shadow/envoy/extensions/common/matching/v3/BUILD @@ -0,0 +1,13 @@ +# DO NOT EDIT. This file is generated by tools/proto_format/proto_sync.py. + +load("@envoy_api//bazel:api_build_system.bzl", "api_proto_package") + +licenses(["notice"]) # Apache 2 + +api_proto_package( + deps = [ + "//envoy/config/common/matcher/v3:pkg", + "//envoy/config/core/v3:pkg", + "@com_github_cncf_udpa//udpa/annotations:pkg", + ], +) diff --git a/generated_api_shadow/envoy/extensions/common/matching/v3/extension_matcher.proto b/generated_api_shadow/envoy/extensions/common/matching/v3/extension_matcher.proto new file mode 100644 index 0000000000000..370444fd70421 --- /dev/null +++ b/generated_api_shadow/envoy/extensions/common/matching/v3/extension_matcher.proto @@ -0,0 +1,27 @@ +syntax = "proto3"; + +package envoy.extensions.common.matching.v3; + +import "envoy/config/common/matcher/v3/matcher.proto"; +import "envoy/config/core/v3/extension.proto"; + +import "udpa/annotations/migrate.proto"; +import "udpa/annotations/status.proto"; +import "validate/validate.proto"; + +option java_package = "io.envoyproxy.envoy.extensions.common.matching.v3"; +option java_outer_classname = "ExtensionMatcherProto"; +option java_multiple_files = true; +option (udpa.annotations.file_status).package_version_status = ACTIVE; + +// Wrapper around an existing extension that provides an associated matcher. This allows +// decorating an existing extension with a matcher, which can be used to match against +// relevant protocol data. +message ExtensionWithMatcher { + // The associated matcher. + config.common.matcher.v3.Matcher matcher = 1 [(validate.rules).message = {required: true}]; + + // The underlying extension config. + config.core.v3.TypedExtensionConfig extension_config = 2 + [(validate.rules).message = {required: true}]; +} diff --git a/generated_api_shadow/envoy/extensions/common/matching/v4alpha/BUILD b/generated_api_shadow/envoy/extensions/common/matching/v4alpha/BUILD new file mode 100644 index 0000000000000..95ccc22a554af --- /dev/null +++ b/generated_api_shadow/envoy/extensions/common/matching/v4alpha/BUILD @@ -0,0 +1,14 @@ +# DO NOT EDIT. This file is generated by tools/proto_format/proto_sync.py. + +load("@envoy_api//bazel:api_build_system.bzl", "api_proto_package") + +licenses(["notice"]) # Apache 2 + +api_proto_package( + deps = [ + "//envoy/config/common/matcher/v4alpha:pkg", + "//envoy/config/core/v4alpha:pkg", + "//envoy/extensions/common/matching/v3:pkg", + "@com_github_cncf_udpa//udpa/annotations:pkg", + ], +) diff --git a/generated_api_shadow/envoy/extensions/common/matching/v4alpha/extension_matcher.proto b/generated_api_shadow/envoy/extensions/common/matching/v4alpha/extension_matcher.proto new file mode 100644 index 0000000000000..c6a996363bc2b --- /dev/null +++ b/generated_api_shadow/envoy/extensions/common/matching/v4alpha/extension_matcher.proto @@ -0,0 +1,30 @@ +syntax = "proto3"; + +package envoy.extensions.common.matching.v4alpha; + +import "envoy/config/common/matcher/v4alpha/matcher.proto"; +import "envoy/config/core/v4alpha/extension.proto"; + +import "udpa/annotations/status.proto"; +import "udpa/annotations/versioning.proto"; +import "validate/validate.proto"; + +option java_package = "io.envoyproxy.envoy.extensions.common.matching.v4alpha"; +option java_outer_classname = "ExtensionMatcherProto"; +option java_multiple_files = true; +option (udpa.annotations.file_status).package_version_status = NEXT_MAJOR_VERSION_CANDIDATE; + +// Wrapper around an existing extension that provides an associated matcher. This allows +// decorating an existing extension with a matcher, which can be used to match against +// relevant protocol data. +message ExtensionWithMatcher { + option (udpa.annotations.versioning).previous_message_type = + "envoy.extensions.common.matching.v3.ExtensionWithMatcher"; + + // The associated matcher. + config.common.matcher.v4alpha.Matcher matcher = 1 [(validate.rules).message = {required: true}]; + + // The underlying extension config. + config.core.v4alpha.TypedExtensionConfig extension_config = 2 + [(validate.rules).message = {required: true}]; +} diff --git a/generated_api_shadow/envoy/extensions/filters/common/matching/v3/BUILD b/generated_api_shadow/envoy/extensions/filters/common/matching/v3/BUILD new file mode 100644 index 0000000000000..ee92fb652582e --- /dev/null +++ b/generated_api_shadow/envoy/extensions/filters/common/matching/v3/BUILD @@ -0,0 +1,9 @@ +# DO NOT EDIT. This file is generated by tools/proto_format/proto_sync.py. + +load("@envoy_api//bazel:api_build_system.bzl", "api_proto_package") + +licenses(["notice"]) # Apache 2 + +api_proto_package( + deps = ["@com_github_cncf_udpa//udpa/annotations:pkg"], +) diff --git a/generated_api_shadow/envoy/extensions/filters/common/matching/v3/skip_action.proto b/generated_api_shadow/envoy/extensions/filters/common/matching/v3/skip_action.proto new file mode 100644 index 0000000000000..ab7e18e7cf34f --- /dev/null +++ b/generated_api_shadow/envoy/extensions/filters/common/matching/v3/skip_action.proto @@ -0,0 +1,19 @@ +syntax = "proto3"; + +package envoy.extensions.filters.common.matching.v3; + +import "udpa/annotations/migrate.proto"; +import "udpa/annotations/status.proto"; +import "udpa/annotations/versioning.proto"; +import "validate/validate.proto"; + +option java_package = "io.envoyproxy.envoy.extensions.filters.common.matching.v3"; +option java_outer_classname = "SkipActionProto"; +option java_multiple_files = true; +option (udpa.annotations.file_status).package_version_status = ACTIVE; + +// [#protodoc-title: Common Match Actions] + +// Indicates that the associated filter should be skipped. +message SkipFilterMatchAction { +} diff --git a/include/envoy/http/filter.h b/include/envoy/http/filter.h index 2738231e7d61e..4157dfc6819f7 100644 --- a/include/envoy/http/filter.h +++ b/include/envoy/http/filter.h @@ -890,6 +890,7 @@ class FilterChainFactoryCallbacks { /** * Add a decoder filter that is used when reading stream data. * @param filter supplies the filter to add. + * @param match_tree the MatchTree to associated with this filter. */ virtual void addStreamDecoderFilter(Http::StreamDecoderFilterSharedPtr filter, @@ -904,6 +905,7 @@ class FilterChainFactoryCallbacks { /** * Add an encoder filter that is used when writing stream data. * @param filter supplies the filter to add. + * @param match_tree the MatchTree to associated with this filter. */ virtual void addStreamEncoderFilter(Http::StreamEncoderFilterSharedPtr filter, @@ -918,6 +920,7 @@ class FilterChainFactoryCallbacks { /** * Add a decoder/encoder filter that is used both when reading and writing stream data. * @param filter supplies the filter to add. + * @param match_tree the MatchTree to associated with this filter. */ virtual void addStreamFilter(Http::StreamFilterSharedPtr filter, Matcher::MatchTreeSharedPtr match_tree) PURE; From ce3e50333280f762e02913c39b348fb1c4455b14 Mon Sep 17 00:00:00 2001 From: Snow Pettersen Date: Mon, 7 Dec 2020 19:58:11 +0000 Subject: [PATCH 04/32] move protos Signed-off-by: Snow Pettersen --- api/envoy/config/common/matcher/v3/action.proto | 0 source/common/http/BUILD | 2 +- source/common/http/filter_manager.h | 5 +++-- 3 files changed, 4 insertions(+), 3 deletions(-) delete mode 100644 api/envoy/config/common/matcher/v3/action.proto diff --git a/api/envoy/config/common/matcher/v3/action.proto b/api/envoy/config/common/matcher/v3/action.proto deleted file mode 100644 index e69de29bb2d1d..0000000000000 diff --git a/source/common/http/BUILD b/source/common/http/BUILD index f8f99bce38fc6..f5227788302a0 100644 --- a/source/common/http/BUILD +++ b/source/common/http/BUILD @@ -175,7 +175,7 @@ envoy_cc_library( "//source/common/grpc:common_lib", "//source/common/local_reply:local_reply_lib", "//source/common/matcher:matcher_lib", - "@envoy_api//envoy/config/common/matcher/v3:pkg_cc_proto", + "@envoy_api//envoy/extensions/filters/common/matching/v3:pkg_cc_proto", ], ) diff --git a/source/common/http/filter_manager.h b/source/common/http/filter_manager.h index bbd7bd8480b60..ee77f494f19f5 100644 --- a/source/common/http/filter_manager.h +++ b/source/common/http/filter_manager.h @@ -1,6 +1,6 @@ #pragma once -#include "envoy/config/common/matcher/v3/action.pb.h" +#include "envoy/extensions/filters/common/matching/v3/skip_action.pb.h" #include "envoy/http/filter.h" #include "envoy/http/header_map.h" #include "envoy/matcher/matcher.h" @@ -111,7 +111,8 @@ class HttpResponseHeadersDataInput : public HttpHeadersDataInputBase { } }; -class SkipAction : public Matcher::ActionBase {}; +class SkipAction : public Matcher::ActionBase< + envoy::extensions::filters::common::matching::v3::SkipFilterMatchAction> {}; /** * Base class wrapper for both stream encoder and decoder filters. From 6b407f55893d78601a0fe32fb0de32f7c2f5ce79 Mon Sep 17 00:00:00 2001 From: Snow Pettersen Date: Mon, 7 Dec 2020 23:38:36 +0000 Subject: [PATCH 05/32] spelling + title Signed-off-by: Snow Pettersen --- .../extensions/common/matching/v3/extension_matcher.proto | 2 ++ .../common/matching/v4alpha/extension_matcher.proto | 2 ++ .../extensions/common/matching/v3/extension_matcher.proto | 2 ++ .../common/matching/v4alpha/extension_matcher.proto | 2 ++ source/common/http/header_utility.cc | 4 ++-- 5 files changed, 10 insertions(+), 2 deletions(-) diff --git a/api/envoy/extensions/common/matching/v3/extension_matcher.proto b/api/envoy/extensions/common/matching/v3/extension_matcher.proto index 370444fd70421..47a065be64888 100644 --- a/api/envoy/extensions/common/matching/v3/extension_matcher.proto +++ b/api/envoy/extensions/common/matching/v3/extension_matcher.proto @@ -14,6 +14,8 @@ option java_outer_classname = "ExtensionMatcherProto"; option java_multiple_files = true; option (udpa.annotations.file_status).package_version_status = ACTIVE; +// [#protodoc-title: Extension Matcher] + // Wrapper around an existing extension that provides an associated matcher. This allows // decorating an existing extension with a matcher, which can be used to match against // relevant protocol data. diff --git a/api/envoy/extensions/common/matching/v4alpha/extension_matcher.proto b/api/envoy/extensions/common/matching/v4alpha/extension_matcher.proto index c6a996363bc2b..bb71cc5a095ff 100644 --- a/api/envoy/extensions/common/matching/v4alpha/extension_matcher.proto +++ b/api/envoy/extensions/common/matching/v4alpha/extension_matcher.proto @@ -14,6 +14,8 @@ option java_outer_classname = "ExtensionMatcherProto"; option java_multiple_files = true; option (udpa.annotations.file_status).package_version_status = NEXT_MAJOR_VERSION_CANDIDATE; +// [#protodoc-title: Extension Matcher] + // Wrapper around an existing extension that provides an associated matcher. This allows // decorating an existing extension with a matcher, which can be used to match against // relevant protocol data. diff --git a/generated_api_shadow/envoy/extensions/common/matching/v3/extension_matcher.proto b/generated_api_shadow/envoy/extensions/common/matching/v3/extension_matcher.proto index 370444fd70421..47a065be64888 100644 --- a/generated_api_shadow/envoy/extensions/common/matching/v3/extension_matcher.proto +++ b/generated_api_shadow/envoy/extensions/common/matching/v3/extension_matcher.proto @@ -14,6 +14,8 @@ option java_outer_classname = "ExtensionMatcherProto"; option java_multiple_files = true; option (udpa.annotations.file_status).package_version_status = ACTIVE; +// [#protodoc-title: Extension Matcher] + // Wrapper around an existing extension that provides an associated matcher. This allows // decorating an existing extension with a matcher, which can be used to match against // relevant protocol data. diff --git a/generated_api_shadow/envoy/extensions/common/matching/v4alpha/extension_matcher.proto b/generated_api_shadow/envoy/extensions/common/matching/v4alpha/extension_matcher.proto index c6a996363bc2b..bb71cc5a095ff 100644 --- a/generated_api_shadow/envoy/extensions/common/matching/v4alpha/extension_matcher.proto +++ b/generated_api_shadow/envoy/extensions/common/matching/v4alpha/extension_matcher.proto @@ -14,6 +14,8 @@ option java_outer_classname = "ExtensionMatcherProto"; option java_multiple_files = true; option (udpa.annotations.file_status).package_version_status = NEXT_MAJOR_VERSION_CANDIDATE; +// [#protodoc-title: Extension Matcher] + // Wrapper around an existing extension that provides an associated matcher. This allows // decorating an existing extension with a matcher, which can be used to match against // relevant protocol data. diff --git a/source/common/http/header_utility.cc b/source/common/http/header_utility.cc index a3c2747b294cb..2d22bde10d149 100644 --- a/source/common/http/header_utility.cc +++ b/source/common/http/header_utility.cc @@ -96,7 +96,7 @@ bool HeaderUtility::matchHeaders(const HeaderMap& request_headers, HeaderUtility::GetAllOfHeaderAsStringResult HeaderUtility::getAllOfHeaderAsString(const HeaderMap::GetResult& header_value, - absl::string_view seperator) { + absl::string_view separator) { GetAllOfHeaderAsStringResult result; // In this case we concatenate all found headers using a delimiter before performing the // final match. We use an InlinedVector of absl::string_view to invoke the optimized join @@ -108,7 +108,7 @@ HeaderUtility::getAllOfHeaderAsString(const HeaderMap::GetResult& header_value, for (size_t i = 0; i < header_value.size(); i++) { string_view_vector.push_back(header_value[i]->value().getStringView()); } - result.result_backing_string_ = absl::StrJoin(string_view_vector, seperator); + result.result_backing_string_ = absl::StrJoin(string_view_vector, separator); return result; } From 6fec69fbcdc59aa267daad7e30add7592aa35e77 Mon Sep 17 00:00:00 2001 From: Snow Pettersen Date: Tue, 8 Dec 2020 12:25:03 +0000 Subject: [PATCH 06/32] add to toc Signed-off-by: Snow Pettersen --- docs/root/api-v3/common_messages/common_messages.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/root/api-v3/common_messages/common_messages.rst b/docs/root/api-v3/common_messages/common_messages.rst index ceff6d6681ee9..6539b53b8577a 100644 --- a/docs/root/api-v3/common_messages/common_messages.rst +++ b/docs/root/api-v3/common_messages/common_messages.rst @@ -21,3 +21,5 @@ Common messages ../extensions/common/ratelimit/v3/ratelimit.proto ../extensions/filters/common/fault/v3/fault.proto ../extensions/network/socket_interface/v3/default_socket_interface.proto + ../extensions/common/matching/v3/extension_matcher.proto + ../extensions/filters/common/matching/v3/skip_action.proto From baff1bd53d26101410a28ae7b22a2385d4e95746 Mon Sep 17 00:00:00 2001 From: Snow Pettersen Date: Tue, 8 Dec 2020 09:36:50 -0500 Subject: [PATCH 07/32] templating to help clang do the conversions Signed-off-by: Snow Pettersen --- source/common/http/filter_manager.h | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/source/common/http/filter_manager.h b/source/common/http/filter_manager.h index ee77f494f19f5..b9718c7b1e7dd 100644 --- a/source/common/http/filter_manager.h +++ b/source/common/http/filter_manager.h @@ -56,11 +56,12 @@ class HttpMatchingDataImpl : public HttpMatchingData { using HttpMatchingDataImplSharedPtr = std::shared_ptr; +template class HttpHeadersDataInputBase : public Matcher::DataInput { public: explicit HttpHeadersDataInputBase(const std::string& name) : name_(name) {} - virtual absl::optional> + virtual absl::optional> headerMap(const HttpMatchingData& data) const PURE; Matcher::DataInputGetResult get(const HttpMatchingData& data) override { @@ -91,21 +92,21 @@ class HttpHeadersDataInputBase : public Matcher::DataInput { absl::optional header_as_string_result_; }; -class HttpRequestHeadersDataInput : public HttpHeadersDataInputBase { +class HttpRequestHeadersDataInput : public HttpHeadersDataInputBase { public: explicit HttpRequestHeadersDataInput(const std::string& name) : HttpHeadersDataInputBase(name) {} - absl::optional> + absl::optional> headerMap(const HttpMatchingData& data) const override { return data.requestHeaders(); } }; -class HttpResponseHeadersDataInput : public HttpHeadersDataInputBase { +class HttpResponseHeadersDataInput : public HttpHeadersDataInputBase { public: explicit HttpResponseHeadersDataInput(const std::string& name) : HttpHeadersDataInputBase(name) {} - absl::optional> + absl::optional> headerMap(const HttpMatchingData& data) const override { return data.responseHeaders(); } From c39c870e67aefda923b0f40280111163bcddd4ea Mon Sep 17 00:00:00 2001 From: Snow Pettersen Date: Tue, 8 Dec 2020 15:26:50 +0000 Subject: [PATCH 08/32] fix name collision Signed-off-by: Snow Pettersen --- test/common/router/config_impl_test.cc | 31 +++++++++++++++----------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/test/common/router/config_impl_test.cc b/test/common/router/config_impl_test.cc index 6439a43bc9f96..b22614af73c99 100644 --- a/test/common/router/config_impl_test.cc +++ b/test/common/router/config_impl_test.cc @@ -39,7 +39,6 @@ using testing::_; using testing::ContainerEq; using testing::Eq; -using testing::Matcher; using testing::MockFunction; using testing::NiceMock; using testing::Return; @@ -3036,15 +3035,17 @@ TEST_F(RouteMatcherTest, FractionalRuntime) { TestConfigImpl config(parseRouteConfigurationFromYaml(yaml), factory_context_, false); - EXPECT_CALL(snapshot, featureEnabled("bogus_key", - Matcher(_), 41)) + EXPECT_CALL(snapshot, + featureEnabled("bogus_key", + testing::Matcher(_), 41)) .WillRepeatedly(Return(true)); EXPECT_EQ( "something_else", config.route(genHeaders("www.lyft.com", "/foo", "GET"), 41)->routeEntry()->clusterName()); - EXPECT_CALL(snapshot, featureEnabled("bogus_key", - Matcher(_), 43)) + EXPECT_CALL(snapshot, + featureEnabled("bogus_key", + testing::Matcher(_), 43)) .WillRepeatedly(Return(false)); EXPECT_EQ( "www2", @@ -5616,11 +5617,13 @@ TEST_F(RoutePropertyTest, DEPRECATED_FEATURE_TEST(TestVHostCorsConfig)) { )EOF"; Runtime::MockSnapshot snapshot; - EXPECT_CALL(snapshot, featureEnabled("cors.www.enabled", - Matcher(_))) + EXPECT_CALL(snapshot, + featureEnabled("cors.www.enabled", + testing::Matcher(_))) .WillOnce(Return(false)); - EXPECT_CALL(snapshot, featureEnabled("cors.www.shadow_enabled", - Matcher(_))) + EXPECT_CALL(snapshot, + featureEnabled("cors.www.shadow_enabled", + testing::Matcher(_))) .WillOnce(Return(true)); EXPECT_CALL(factory_context_.runtime_loader_, snapshot()).WillRepeatedly(ReturnRef(snapshot)); @@ -5673,11 +5676,13 @@ TEST_F(RoutePropertyTest, TestRouteCorsConfig) { )EOF"; Runtime::MockSnapshot snapshot; - EXPECT_CALL(snapshot, featureEnabled("cors.www.enabled", - Matcher(_))) + EXPECT_CALL(snapshot, + featureEnabled("cors.www.enabled", + testing::Matcher(_))) .WillOnce(Return(false)); - EXPECT_CALL(snapshot, featureEnabled("cors.www.shadow_enabled", - Matcher(_))) + EXPECT_CALL(snapshot, + featureEnabled("cors.www.shadow_enabled", + testing::Matcher(_))) .WillOnce(Return(true)); EXPECT_CALL(factory_context_.runtime_loader_, snapshot()).WillRepeatedly(ReturnRef(snapshot)); From 571f096637d6cef1278d66e9d307942c815542f9 Mon Sep 17 00:00:00 2001 From: Snow Pettersen Date: Tue, 8 Dec 2020 16:11:14 +0000 Subject: [PATCH 09/32] update mocks Signed-off-by: Snow Pettersen --- api/versioning/BUILD | 1 + test/mocks/http/mocks.h | 10 ++++++++++ 2 files changed, 11 insertions(+) diff --git a/api/versioning/BUILD b/api/versioning/BUILD index 2b0bd46f11264..79e9a345828db 100644 --- a/api/versioning/BUILD +++ b/api/versioning/BUILD @@ -79,6 +79,7 @@ proto_library( "//envoy/extensions/filters/http/kill_request/v3:pkg", "//envoy/extensions/filters/http/local_ratelimit/v3:pkg", "//envoy/extensions/filters/http/lua/v3:pkg", + "//envoy/extensions/filters/http/noop:pkg", "//envoy/extensions/filters/http/oauth2/v3alpha:pkg", "//envoy/extensions/filters/http/on_demand/v3:pkg", "//envoy/extensions/filters/http/original_src/v3:pkg", diff --git a/test/mocks/http/mocks.h b/test/mocks/http/mocks.h index 407956f8aa93b..85496ffa21480 100644 --- a/test/mocks/http/mocks.h +++ b/test/mocks/http/mocks.h @@ -12,6 +12,7 @@ #include "envoy/http/codec.h" #include "envoy/http/conn_pool.h" #include "envoy/http/filter.h" +#include "envoy/matcher/matcher.h" #include "envoy/ssl/connection.h" #include "common/http/filter_manager.h" @@ -466,8 +467,17 @@ class MockFilterChainFactoryCallbacks : public Http::FilterChainFactoryCallbacks ~MockFilterChainFactoryCallbacks() override; MOCK_METHOD(void, addStreamDecoderFilter, (Http::StreamDecoderFilterSharedPtr filter)); + MOCK_METHOD(void, addStreamDecoderFilter, + (Http::StreamDecoderFilterSharedPtr filter, + Matcher::MatchTreeSharedPtr match_tree)); MOCK_METHOD(void, addStreamEncoderFilter, (Http::StreamEncoderFilterSharedPtr filter)); + MOCK_METHOD(void, addStreamEncoderFilter, + (Http::StreamEncoderFilterSharedPtr filter, + Matcher::MatchTreeSharedPtr match_tree)); MOCK_METHOD(void, addStreamFilter, (Http::StreamFilterSharedPtr filter)); + MOCK_METHOD(void, addStreamFilter, + (Http::StreamFilterSharedPtr filter, + Matcher::MatchTreeSharedPtr match_tree)); MOCK_METHOD(void, addAccessLogHandler, (AccessLog::InstanceSharedPtr handler)); }; From e83b3b2870af7c9acac44868d2dc51af2b657d79 Mon Sep 17 00:00:00 2001 From: Snow Pettersen Date: Tue, 8 Dec 2020 17:32:16 +0000 Subject: [PATCH 10/32] remove accidental line Signed-off-by: Snow Pettersen --- api/versioning/BUILD | 1 - 1 file changed, 1 deletion(-) diff --git a/api/versioning/BUILD b/api/versioning/BUILD index 79e9a345828db..2b0bd46f11264 100644 --- a/api/versioning/BUILD +++ b/api/versioning/BUILD @@ -79,7 +79,6 @@ proto_library( "//envoy/extensions/filters/http/kill_request/v3:pkg", "//envoy/extensions/filters/http/local_ratelimit/v3:pkg", "//envoy/extensions/filters/http/lua/v3:pkg", - "//envoy/extensions/filters/http/noop:pkg", "//envoy/extensions/filters/http/oauth2/v3alpha:pkg", "//envoy/extensions/filters/http/on_demand/v3:pkg", "//envoy/extensions/filters/http/original_src/v3:pkg", From 9afefeedd1e69554f1382ce149092f7d4dfda66a Mon Sep 17 00:00:00 2001 From: Snow Pettersen Date: Tue, 8 Dec 2020 19:31:40 +0000 Subject: [PATCH 11/32] wip filter for creating match trees Signed-off-by: Snow Pettersen --- .../filters/http/match_wrapper/BUILD | 28 ++++++ .../filters/http/match_wrapper/config.cc | 85 +++++++++++++++++++ .../filters/http/match_wrapper/config.h | 31 +++++++ .../filters/http/well_known_names.h | 2 + 4 files changed, 146 insertions(+) create mode 100644 source/extensions/filters/http/match_wrapper/BUILD create mode 100644 source/extensions/filters/http/match_wrapper/config.cc create mode 100644 source/extensions/filters/http/match_wrapper/config.h diff --git a/source/extensions/filters/http/match_wrapper/BUILD b/source/extensions/filters/http/match_wrapper/BUILD new file mode 100644 index 0000000000000..d0116e8ab2f31 --- /dev/null +++ b/source/extensions/filters/http/match_wrapper/BUILD @@ -0,0 +1,28 @@ +load( + "//bazel:envoy_build_system.bzl", + "envoy_cc_extension", + "envoy_cc_library", + "envoy_extension_package", +) + +licenses(["notice"]) # Apache 2 + +# Lua scripting L7 HTTP filter (https://www.lua.org/, http://luajit.org/) +# Public docs: docs/root/configuration/http_filters/lua_filter.rst + +envoy_extension_package() + +envoy_cc_extension( + name = "config", + srcs = ["config.cc"], + hdrs = ["config.h"], + security_posture = "unknown", + deps = [ + "//include/envoy/registry", + "//include/envoy/server:filter_config_interface", + "//source/extensions/filters/http:well_known_names", + "//source/extensions/filters/http/common:factory_base_lib", + "//source/common/matcher:matcher_lib", + "@envoy_api//envoy/extensions/common/matching/v3:pkg_cc_proto", + ], +) diff --git a/source/extensions/filters/http/match_wrapper/config.cc b/source/extensions/filters/http/match_wrapper/config.cc new file mode 100644 index 0000000000000..16dec7f59861b --- /dev/null +++ b/source/extensions/filters/http/match_wrapper/config.cc @@ -0,0 +1,85 @@ +#include "extensions/filters/http/match_wrapper/config.h" + +#include "envoy/http/filter.h" +#include "envoy/registry/registry.h" + +#include "common/matcher/matcher.h" +#include "include/envoy/matcher/_virtual_includes/matcher_interface/envoy/matcher/matcher.h" +#include + +namespace Envoy { +namespace Extensions { +namespace HttpFilters { +namespace MatchWrapper { + +namespace { +struct DelegatingFactoryCallbacks : public Http::FilterChainFactoryCallbacks { + DelegatingFactoryCallbacks(Http::FilterChainFactoryCallbacks& delegated_callbacks, + Matcher::MatchTreeSharedPtr match_tree) + : delegated_callbacks_(delegated_callbacks), match_tree_(std::move(match_tree)) {} + + void addStreamDecoderFilter(Http::StreamDecoderFilterSharedPtr filter) override { + delegated_callbacks_.addStreamDecoderFilter(std::move(filter), match_tree_); + } + void addStreamDecoderFilter(Http::StreamDecoderFilterSharedPtr filter, + Matcher::MatchTreeSharedPtr match_tree) override { + delegated_callbacks_.addStreamDecoderFilter(std::move(filter), std::move(match_tree)); + } + void addStreamEncoderFilter(Http::StreamEncoderFilterSharedPtr filter) override { + delegated_callbacks_.addStreamEncoderFilter(std::move(filter), match_tree_); + } + void addStreamEncoderFilter(Http::StreamEncoderFilterSharedPtr filter, + Matcher::MatchTreeSharedPtr match_tree) override { + delegated_callbacks_.addStreamEncoderFilter(std::move(filter), std::move(match_tree)); + } + void addStreamFilter(Http::StreamFilterSharedPtr filter) override { + delegated_callbacks_.addStreamFilter(std::move(filter), match_tree_); + } + void addStreamFilter(Http::StreamFilterSharedPtr filter, + Matcher::MatchTreeSharedPtr match_tree) override { + delegated_callbacks_.addStreamFilter(std::move(filter), std::move(match_tree)); + } + void addAccessLogHandler(AccessLog::InstanceSharedPtr handler) override { + delegated_callbacks_.addAccessLogHandler(std::move(handler)); + } + + Http::FilterChainFactoryCallbacks& delegated_callbacks_; + Matcher::MatchTreeSharedPtr match_tree_; +}; +} // namespace + +Http::FilterFactoryCb MatchWrapperConfig::createFilterFactoryFromProtoTyped( + const envoy::extensions::common::matching::v3::ExtensionWithMatcher& proto_config, + const std::string& prefix, Server::Configuration::FactoryContext& context) { + + if (proto_config.has_extension_config()) { + auto& factory = + Config::Utility::getAndCheckFactory( + proto_config.extension_config()); + + auto filter_factory = + factory.createFilterFactoryFromProto(proto_config.extension_config().typed_config(), prefix, context); + + auto match_tree = + Matcher::MatchTreeFactory(context.messageValidationVisitor()) + .create(proto_config.matcher()); + + return [filter_factory, match_tree](Http::FilterChainFactoryCallbacks& callbacks) -> void { + DelegatingFactoryCallbacks delegated_callbacks(callbacks, match_tree); + + return filter_factory(delegated_callbacks); + }; + } + + return [](Http::FilterChainFactoryCallbacks&) -> void {}; +} + +/** + * Static registration for the Lua filter. @see RegisterFactory. + */ +REGISTER_FACTORY(MatchWrapperConfig, Server::Configuration::NamedHttpFilterConfigFactory){"blah"}; + +} // namespace MatchWrapper +} // namespace HttpFilters +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/filters/http/match_wrapper/config.h b/source/extensions/filters/http/match_wrapper/config.h new file mode 100644 index 0000000000000..5b0373a4d15d3 --- /dev/null +++ b/source/extensions/filters/http/match_wrapper/config.h @@ -0,0 +1,31 @@ +#pragma once + +#include "envoy/extensions/common/matching/v3/extension_matcher.pb.validate.h" + +#include "envoy/server/filter_config.h" +#include "extensions/filters/http/common/factory_base.h" +#include "extensions/filters/http/well_known_names.h" + +namespace Envoy { +namespace Extensions { +namespace HttpFilters { +namespace MatchWrapper { + +/** + * Config registration for the Lua filter. @see NamedHttpFilterConfigFactory. + */ +class MatchWrapperConfig + : public Common::FactoryBase { +public: + MatchWrapperConfig() : FactoryBase(HttpFilterNames::get().MatchingFilter) {} + +private: + Http::FilterFactoryCb createFilterFactoryFromProtoTyped( + const envoy::extensions::common::matching::v3::ExtensionWithMatcher& proto_config, const std::string&, + Server::Configuration::FactoryContext& context) override; +}; + +} // namespace Lua +} // namespace HttpFilters +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/filters/http/well_known_names.h b/source/extensions/filters/http/well_known_names.h index 189640302f8db..44f45084ae42f 100644 --- a/source/extensions/filters/http/well_known_names.h +++ b/source/extensions/filters/http/well_known_names.h @@ -86,6 +86,8 @@ class HttpFilterNameValues { const std::string KillRequest = "envoy.filters.http.kill_request"; // External Processing filter const std::string ExternalProcessing = "envoy.filters.http.ext_proc"; + // Match tree wrapper + const std::string MatchingFilter = "envoy.filters.http.matching_filter"; }; using HttpFilterNames = ConstSingleton; From 3d223ad1272e7bbd8bc7b74fa8646cdd0b71a539 Mon Sep 17 00:00:00 2001 From: Snow Pettersen Date: Fri, 11 Dec 2020 01:21:03 +0000 Subject: [PATCH 12/32] wire things up a bit Signed-off-by: Snow Pettersen --- .../v3/http_connection_manager.proto | 8 ++ .../v4alpha/http_connection_manager.proto | 14 ++++ .../v3/http_connection_manager.proto | 8 ++ .../v4alpha/http_connection_manager.proto | 14 ++++ include/envoy/http/filter.h | 2 + source/common/http/BUILD | 1 + source/common/http/filter_manager.cc | 3 + source/common/http/filter_manager.h | 67 ++++++++++++--- .../http/match_wrapper/BUILD | 11 +-- .../http/match_wrapper/config.cc | 11 ++- .../http/match_wrapper/config.h | 0 .../extension_discovery_integration_test.cc | 82 +++++++++++++++++++ 12 files changed, 199 insertions(+), 22 deletions(-) rename source/{extensions/filters => common}/http/match_wrapper/BUILD (66%) rename source/{extensions/filters => common}/http/match_wrapper/config.cc (87%) rename source/{extensions/filters => common}/http/match_wrapper/config.h (100%) diff --git a/api/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto b/api/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto index f83cc368ea0c3..76516da35f769 100644 --- a/api/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto +++ b/api/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto @@ -834,3 +834,11 @@ message RequestIDExtension { // Request ID extension specific configuration. google.protobuf.Any typed_config = 1; } + +message HttpRequestHeaderMatchInput { + string header_name = 1; +} + +message HttpResponseHeaderMatchInput { + string header_name = 1; +} diff --git a/api/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto b/api/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto index 013b676b8a639..49a9a6405735f 100644 --- a/api/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto +++ b/api/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto @@ -840,3 +840,17 @@ message RequestIDExtension { // Request ID extension specific configuration. google.protobuf.Any typed_config = 1; } + +message HttpRequestHeaderMatchInput { + option (udpa.annotations.versioning).previous_message_type = + "envoy.extensions.filters.network.http_connection_manager.v3.HttpRequestHeaderMatchInput"; + + string header_name = 1; +} + +message HttpResponseHeaderMatchInput { + option (udpa.annotations.versioning).previous_message_type = + "envoy.extensions.filters.network.http_connection_manager.v3.HttpResponseHeaderMatchInput"; + + string header_name = 1; +} diff --git a/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto b/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto index fcec5a4e235bb..19a1e511312a7 100644 --- a/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto +++ b/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto @@ -839,3 +839,11 @@ message RequestIDExtension { // Request ID extension specific configuration. google.protobuf.Any typed_config = 1; } + +message HttpRequestHeaderMatchInput { + string header_name = 1; +} + +message HttpResponseHeaderMatchInput { + string header_name = 1; +} diff --git a/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto b/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto index 013b676b8a639..49a9a6405735f 100644 --- a/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto +++ b/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto @@ -840,3 +840,17 @@ message RequestIDExtension { // Request ID extension specific configuration. google.protobuf.Any typed_config = 1; } + +message HttpRequestHeaderMatchInput { + option (udpa.annotations.versioning).previous_message_type = + "envoy.extensions.filters.network.http_connection_manager.v3.HttpRequestHeaderMatchInput"; + + string header_name = 1; +} + +message HttpResponseHeaderMatchInput { + option (udpa.annotations.versioning).previous_message_type = + "envoy.extensions.filters.network.http_connection_manager.v3.HttpResponseHeaderMatchInput"; + + string header_name = 1; +} diff --git a/include/envoy/http/filter.h b/include/envoy/http/filter.h index 4157dfc6819f7..eacb6329d5b46 100644 --- a/include/envoy/http/filter.h +++ b/include/envoy/http/filter.h @@ -867,6 +867,8 @@ using StreamFilterSharedPtr = std::shared_ptr; class HttpMatchingData { public: + static absl::string_view name() { return "http"; } + virtual ~HttpMatchingData() = default; virtual RequestHeaderMapOptConstRef requestHeaders() const PURE; diff --git a/source/common/http/BUILD b/source/common/http/BUILD index f5227788302a0..79671b5955af1 100644 --- a/source/common/http/BUILD +++ b/source/common/http/BUILD @@ -237,6 +237,7 @@ envoy_cc_library( "//source/common/config:utility_lib", "//source/common/http/http1:codec_legacy_lib", "//source/common/http/http1:codec_lib", + "//source/common/http/match_wrapper:config", "//source/common/http/http2:codec_legacy_lib", "//source/common/http/http2:codec_lib", "//source/common/http/http3:quic_codec_factory_lib", diff --git a/source/common/http/filter_manager.cc b/source/common/http/filter_manager.cc index e1608321f7d11..d64fea30fe5cc 100644 --- a/source/common/http/filter_manager.cc +++ b/source/common/http/filter_manager.cc @@ -14,7 +14,10 @@ namespace Envoy { namespace Http { +// REGISTER_FACTORY(HttpResponseHeadersDataInputFactory, Matcher::DataInputFactory){"response-headers"}; namespace { +REGISTER_FACTORY(HttpRequestHeadersDataInputFactory, Matcher::DataInputFactory); +REGISTER_FACTORY(SkipActionFactory, Matcher::ActionFactory); template using FilterList = std::list>; diff --git a/source/common/http/filter_manager.h b/source/common/http/filter_manager.h index b9718c7b1e7dd..e34b526fe8837 100644 --- a/source/common/http/filter_manager.h +++ b/source/common/http/filter_manager.h @@ -1,6 +1,8 @@ #pragma once +#include "envoy/matcher/matcher.h" #include "envoy/extensions/filters/common/matching/v3/skip_action.pb.h" +#include "envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.pb.h" #include "envoy/http/filter.h" #include "envoy/http/header_map.h" #include "envoy/matcher/matcher.h" @@ -102,6 +104,23 @@ class HttpRequestHeadersDataInput : public HttpHeadersDataInputBase { +public: + std::string name() const override { return "request-headers"; } + Matcher::DataInputPtr + createDataInput(const Protobuf::Message& config) override { + const auto& typed_config = + dynamic_cast(config); + + return std::make_unique(typed_config.header_name()); + }; + ProtobufTypes::MessagePtr createEmptyConfigProto() override { + return std::make_unique(); + } +}; + class HttpResponseHeadersDataInput : public HttpHeadersDataInputBase { public: explicit HttpResponseHeadersDataInput(const std::string& name) : HttpHeadersDataInputBase(name) {} @@ -112,9 +131,37 @@ class HttpResponseHeadersDataInput : public HttpHeadersDataInputBase { +public: + std::string name() const override { return "response-headers"; } + Matcher::DataInputPtr + createDataInput(const Protobuf::Message& config) override { + const auto& typed_config = + dynamic_cast(config); + + return std::make_unique(typed_config.header_name()); + }; + ProtobufTypes::MessagePtr createEmptyConfigProto() override { + return std::make_unique(); + } +}; + class SkipAction : public Matcher::ActionBase< envoy::extensions::filters::common::matching::v3::SkipFilterMatchAction> {}; +class SkipActionFactory : public Matcher::ActionFactory { +public: + std::string name() const override { return "skip"; } + Matcher::ActionFactoryCb createActionFactoryCb(const Protobuf::Message&) override { + return []() { return std::make_unique(); }; + } + ProtobufTypes::MessagePtr createEmptyConfigProto() override { + return std::make_unique< + envoy::extensions::filters::common::matching::v3::SkipFilterMatchAction>(); + } +}; /** * Base class wrapper for both stream encoder and decoder filters. */ @@ -431,20 +478,20 @@ class FilterManagerCallbacks { virtual void setRequestTrailers(RequestTrailerMapPtr&& request_trailers) PURE; /** - * Passes ownership of received continue headers to the parent. This may be called multiple times - * in the case of multiple upstream calls. + * Passes ownership of received continue headers to the parent. This may be called multiple + * times in the case of multiple upstream calls. */ virtual void setContinueHeaders(ResponseHeaderMapPtr&& response_headers) PURE; /** - * Passes ownership of received response headers to the parent. This may be called multiple times - * in the case of multiple upstream calls. + * Passes ownership of received response headers to the parent. This may be called multiple + * times in the case of multiple upstream calls. */ virtual void setResponseHeaders(ResponseHeaderMapPtr&& response_headers) PURE; /** - * Passes ownership of received response trailers to the parent. This may be called multiple times - * in the case of multiple upstream calls. + * Passes ownership of received response trailers to the parent. This may be called multiple + * times in the case of multiple upstream calls. */ virtual void setResponseTrailers(ResponseTrailerMapPtr&& response_trailers) PURE; @@ -474,15 +521,15 @@ class FilterManagerCallbacks { /** * Retrieves a pointer to the response headers set via the last call to setResponseHeaders. * Note that response headers might be set multiple times (e.g. if a local reply is issued after - * headers have been received but before headers have been encoded), so it is not safe in general - * to assume that any set of headers will be valid for the duration of a stream. + * headers have been received but before headers have been encoded), so it is not safe in + * general to assume that any set of headers will be valid for the duration of a stream. */ virtual ResponseHeaderMapOptRef responseHeaders() PURE; /** * Retrieves a pointer to the last response trailers set via setResponseTrailers. - * Note that response trailers might be set multiple times, so it is not safe in general to assume - * that any set of trailers will be valid for the duration of the stream. + * Note that response trailers might be set multiple times, so it is not safe in general to + * assume that any set of trailers will be valid for the duration of the stream. */ virtual ResponseTrailerMapOptRef responseTrailers() PURE; diff --git a/source/extensions/filters/http/match_wrapper/BUILD b/source/common/http/match_wrapper/BUILD similarity index 66% rename from source/extensions/filters/http/match_wrapper/BUILD rename to source/common/http/match_wrapper/BUILD index d0116e8ab2f31..57a840061ab34 100644 --- a/source/extensions/filters/http/match_wrapper/BUILD +++ b/source/common/http/match_wrapper/BUILD @@ -1,22 +1,17 @@ load( "//bazel:envoy_build_system.bzl", - "envoy_cc_extension", "envoy_cc_library", - "envoy_extension_package", + "envoy_package", ) licenses(["notice"]) # Apache 2 -# Lua scripting L7 HTTP filter (https://www.lua.org/, http://luajit.org/) -# Public docs: docs/root/configuration/http_filters/lua_filter.rst +envoy_package() -envoy_extension_package() - -envoy_cc_extension( +envoy_cc_library( name = "config", srcs = ["config.cc"], hdrs = ["config.h"], - security_posture = "unknown", deps = [ "//include/envoy/registry", "//include/envoy/server:filter_config_interface", diff --git a/source/extensions/filters/http/match_wrapper/config.cc b/source/common/http/match_wrapper/config.cc similarity index 87% rename from source/extensions/filters/http/match_wrapper/config.cc rename to source/common/http/match_wrapper/config.cc index 16dec7f59861b..9770189583b56 100644 --- a/source/extensions/filters/http/match_wrapper/config.cc +++ b/source/common/http/match_wrapper/config.cc @@ -1,11 +1,10 @@ -#include "extensions/filters/http/match_wrapper/config.h" +#include "common/http/match_wrapper/config.h" #include "envoy/http/filter.h" #include "envoy/registry/registry.h" #include "common/matcher/matcher.h" -#include "include/envoy/matcher/_virtual_includes/matcher_interface/envoy/matcher/matcher.h" -#include +#include "envoy/matcher/matcher.h" namespace Envoy { namespace Extensions { @@ -57,8 +56,12 @@ Http::FilterFactoryCb MatchWrapperConfig::createFilterFactoryFromProtoTyped( Config::Utility::getAndCheckFactory( proto_config.extension_config()); + auto message = factory.createEmptyConfigProto(); + proto_config.extension_config().typed_config(); + Config::Utility::translateOpaqueConfig(proto_config.extension_config().typed_config(), ProtobufWkt::Struct(), + context.messageValidationVisitor(), *message); auto filter_factory = - factory.createFilterFactoryFromProto(proto_config.extension_config().typed_config(), prefix, context); + factory.createFilterFactoryFromProto(*message, prefix, context); auto match_tree = Matcher::MatchTreeFactory(context.messageValidationVisitor()) diff --git a/source/extensions/filters/http/match_wrapper/config.h b/source/common/http/match_wrapper/config.h similarity index 100% rename from source/extensions/filters/http/match_wrapper/config.h rename to source/common/http/match_wrapper/config.h diff --git a/test/integration/extension_discovery_integration_test.cc b/test/integration/extension_discovery_integration_test.cc index 5e4af865b6488..b4aa07d98bcc5 100644 --- a/test/integration/extension_discovery_integration_test.cc +++ b/test/integration/extension_discovery_integration_test.cc @@ -1,4 +1,5 @@ #include "envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.pb.h" +#include "envoy/extensions/common/matching/v3/extension_matcher.pb.h" #include "envoy/service/extension/v3/config_discovery.pb.h" #include "test/common/grpc/grpc_client_integration.h" @@ -18,6 +19,32 @@ std::string denyPrivateConfig() { )EOF"; } +std::string denyPrivateConfigWithMatcher() { + return R"EOF( + "@type": type.googleapis.com/envoy.extensions.common.matching.v3.ExtensionWithMatcher + extension_config: + name: response-filter-config + typed_config: + "@type": type.googleapis.com/test.integration.filters.SetResponseCodeFilterConfig + prefix: "/private" + code: 403 + matcher: + matcher_tree: + input: + name: request-headers + typed_config: + "@type": type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpRequestHeaderMatchInput + header_name: some-header + exact_match_map: + map: + match: + action: + name: skip + typed_config: + "@type": type.googleapis.com/envoy.extensions.filters.common.matching.v3.SkipFilterMatchAction + )EOF"; +} + std::string allowAllConfig() { return "code: 200"; } std::string invalidConfig() { return "code: 90"; } @@ -39,6 +66,8 @@ class ExtensionDiscoveryIntegrationTest : public Grpc::GrpcClientIntegrationPara auto* discovery = filter->mutable_config_discovery(); discovery->add_type_urls( "type.googleapis.com/test.integration.filters.SetResponseCodeFilterConfig"); + discovery->add_type_urls( + "type.googleapis.com/envoy.extensions.common.matching.v3.ExtensionWithMatcher"); if (set_default_config) { const auto default_configuration = TestUtility::parseYaml( @@ -123,6 +152,19 @@ class ExtensionDiscoveryIntegrationTest : public Grpc::GrpcClientIntegrationPara ecds_stream_->sendGrpcMessage(response); } + void sendXdsResponseWithFullYaml(const std::string& name, const std::string& version, + const std::string& full_yaml) { + envoy::service::discovery::v3::DiscoveryResponse response; + response.set_version_info(version); + response.set_type_url("type.googleapis.com/envoy.config.core.v3.TypedExtensionConfig"); + const auto configuration = TestUtility::parseYaml(full_yaml); + envoy::config::core::v3::TypedExtensionConfig typed_config; + typed_config.set_name(name); + typed_config.mutable_typed_config()->MergeFrom(configuration); + response.add_resources()->PackFrom(typed_config); + ecds_stream_->sendGrpcMessage(response); + } + FakeUpstream& getEcdsFakeUpstream() const { return *fake_upstreams_[1]; } FakeHttpConnectionPtr ecds_connection_{nullptr}; @@ -174,6 +216,46 @@ TEST_P(ExtensionDiscoveryIntegrationTest, BasicSuccess) { } } +TEST_P(ExtensionDiscoveryIntegrationTest, BasicSuccessWithMatcher) { + on_server_init_function_ = [&]() { waitXdsStream(); }; + addDynamicFilter("foo", false); + initialize(); + test_server_->waitForCounterGe("listener_manager.lds.update_success", 1); + EXPECT_EQ(test_server_->server().initManager().state(), Init::Manager::State::Initializing); + registerTestServerPorts({"http"}); + sendXdsResponseWithFullYaml("foo", "1", denyPrivateConfigWithMatcher()); + test_server_->waitForCounterGe("http.config_test.extension_config_discovery.foo.config_reload", + 1); + test_server_->waitUntilListenersReady(); + test_server_->waitForGaugeGe("listener_manager.workers_started", 1); + EXPECT_EQ(test_server_->server().initManager().state(), Init::Manager::State::Initialized); + codec_client_ = makeHttpConnection(makeClientConnection((lookupPort("http")))); + { + Http::TestRequestHeaderMapImpl request_headers{ + {":method", "GET"}, {":path", "/"}, {":scheme", "http"}, {":authority", "host"}}; + auto response = codec_client_->makeHeaderOnlyRequest(request_headers); + response->waitForEndStream(); + ASSERT_TRUE(response->complete()); + EXPECT_EQ("200", response->headers().getStatusValue()); + } + Http::TestRequestHeaderMapImpl banned_request_headers{ + {":method", "GET"}, {":path", "/private/key"}, {":scheme", "http"}, {":authority", "host"}}; + { + auto response = codec_client_->makeHeaderOnlyRequest(banned_request_headers); + response->waitForEndStream(); + ASSERT_TRUE(response->complete()); + EXPECT_EQ("403", response->headers().getStatusValue()); + } + Http::TestRequestHeaderMapImpl banned_request_headers_skipped{ + {":method", "GET"}, {":path", "/private/key"}, {"some-header", "match"}, {":scheme", "http"}, {":authority", "host"}}; + { + auto response = codec_client_->makeHeaderOnlyRequest(banned_request_headers); + response->waitForEndStream(); + ASSERT_TRUE(response->complete()); + EXPECT_EQ("200", response->headers().getStatusValue()); + } +} + TEST_P(ExtensionDiscoveryIntegrationTest, BasicFailWithDefault) { on_server_init_function_ = [&]() { waitXdsStream(); }; addDynamicFilter("foo", false); From ecb49e7f8ed586e708e01a2b46f854ce683c5294 Mon Sep 17 00:00:00 2001 From: Snow Pettersen Date: Tue, 15 Dec 2020 20:07:53 +0000 Subject: [PATCH 13/32] add test coverage Signed-off-by: Snow Pettersen --- source/common/http/BUILD | 3 +- source/common/http/filter_manager.cc | 3 +- source/common/http/filter_manager.h | 7 +- source/common/http/match_wrapper/BUILD | 2 +- source/common/http/match_wrapper/config.cc | 18 ++-- source/common/http/match_wrapper/config.h | 8 +- test/integration/BUILD | 3 + .../extension_discovery_integration_test.cc | 84 ++++++++++++++++--- test/integration/integration_test.cc | 52 ++++++++++++ 9 files changed, 150 insertions(+), 30 deletions(-) diff --git a/source/common/http/BUILD b/source/common/http/BUILD index f5e4b59d21196..116cf0911f9f2 100644 --- a/source/common/http/BUILD +++ b/source/common/http/BUILD @@ -177,6 +177,7 @@ envoy_cc_library( "//source/common/local_reply:local_reply_lib", "//source/common/matcher:matcher_lib", "@envoy_api//envoy/extensions/filters/common/matcher/action/v3:pkg_cc_proto", + "@envoy_api//envoy/extensions/filters/network/http_connection_manager/v3:pkg_cc_proto", ], ) @@ -238,11 +239,11 @@ envoy_cc_library( "//source/common/config:utility_lib", "//source/common/http/http1:codec_legacy_lib", "//source/common/http/http1:codec_lib", - "//source/common/http/match_wrapper:config", "//source/common/http/http2:codec_legacy_lib", "//source/common/http/http2:codec_lib", "//source/common/http/http3:quic_codec_factory_lib", "//source/common/http/http3:well_known_names", + "//source/common/http/match_wrapper:config", "//source/common/network:utility_lib", "//source/common/router:config_lib", "//source/common/router:scoped_rds_lib", diff --git a/source/common/http/filter_manager.cc b/source/common/http/filter_manager.cc index ec93f0b0684a2..3fd6f3683ad0a 100644 --- a/source/common/http/filter_manager.cc +++ b/source/common/http/filter_manager.cc @@ -14,7 +14,8 @@ namespace Envoy { namespace Http { -// REGISTER_FACTORY(HttpResponseHeadersDataInputFactory, Matcher::DataInputFactory){"response-headers"}; +// REGISTER_FACTORY(HttpResponseHeadersDataInputFactory, +// Matcher::DataInputFactory){"response-headers"}; namespace { REGISTER_FACTORY(HttpRequestHeadersDataInputFactory, Matcher::DataInputFactory); REGISTER_FACTORY(SkipActionFactory, Matcher::ActionFactory); diff --git a/source/common/http/filter_manager.h b/source/common/http/filter_manager.h index 9323f50aca972..40e77ff6142d6 100644 --- a/source/common/http/filter_manager.h +++ b/source/common/http/filter_manager.h @@ -1,9 +1,7 @@ #pragma once -#include "envoy/matcher/matcher.h" -#include "envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.pb.h" - #include "envoy/extensions/filters/common/matcher/action/v3/skip_action.pb.h" +#include "envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.pb.h" #include "envoy/http/filter.h" #include "envoy/http/header_map.h" #include "envoy/matcher/matcher.h" @@ -158,8 +156,7 @@ class SkipActionFactory : public Matcher::ActionFactory { return []() { return std::make_unique(); }; } ProtobufTypes::MessagePtr createEmptyConfigProto() override { - return std::make_unique< - envoy::extensions::filters::common::matcher::action::v3::SkipFilter>(); + return std::make_unique(); } }; /** diff --git a/source/common/http/match_wrapper/BUILD b/source/common/http/match_wrapper/BUILD index 57a840061ab34..1350d3db39e0b 100644 --- a/source/common/http/match_wrapper/BUILD +++ b/source/common/http/match_wrapper/BUILD @@ -15,9 +15,9 @@ envoy_cc_library( deps = [ "//include/envoy/registry", "//include/envoy/server:filter_config_interface", + "//source/common/matcher:matcher_lib", "//source/extensions/filters/http:well_known_names", "//source/extensions/filters/http/common:factory_base_lib", - "//source/common/matcher:matcher_lib", "@envoy_api//envoy/extensions/common/matching/v3:pkg_cc_proto", ], ) diff --git a/source/common/http/match_wrapper/config.cc b/source/common/http/match_wrapper/config.cc index 9770189583b56..b8e29a7129ac7 100644 --- a/source/common/http/match_wrapper/config.cc +++ b/source/common/http/match_wrapper/config.cc @@ -1,10 +1,10 @@ #include "common/http/match_wrapper/config.h" #include "envoy/http/filter.h" +#include "envoy/matcher/matcher.h" #include "envoy/registry/registry.h" #include "common/matcher/matcher.h" -#include "envoy/matcher/matcher.h" namespace Envoy { namespace Extensions { @@ -20,15 +20,17 @@ struct DelegatingFactoryCallbacks : public Http::FilterChainFactoryCallbacks { void addStreamDecoderFilter(Http::StreamDecoderFilterSharedPtr filter) override { delegated_callbacks_.addStreamDecoderFilter(std::move(filter), match_tree_); } - void addStreamDecoderFilter(Http::StreamDecoderFilterSharedPtr filter, - Matcher::MatchTreeSharedPtr match_tree) override { + void + addStreamDecoderFilter(Http::StreamDecoderFilterSharedPtr filter, + Matcher::MatchTreeSharedPtr match_tree) override { delegated_callbacks_.addStreamDecoderFilter(std::move(filter), std::move(match_tree)); } void addStreamEncoderFilter(Http::StreamEncoderFilterSharedPtr filter) override { delegated_callbacks_.addStreamEncoderFilter(std::move(filter), match_tree_); } - void addStreamEncoderFilter(Http::StreamEncoderFilterSharedPtr filter, - Matcher::MatchTreeSharedPtr match_tree) override { + void + addStreamEncoderFilter(Http::StreamEncoderFilterSharedPtr filter, + Matcher::MatchTreeSharedPtr match_tree) override { delegated_callbacks_.addStreamEncoderFilter(std::move(filter), std::move(match_tree)); } void addStreamFilter(Http::StreamFilterSharedPtr filter) override { @@ -58,10 +60,10 @@ Http::FilterFactoryCb MatchWrapperConfig::createFilterFactoryFromProtoTyped( auto message = factory.createEmptyConfigProto(); proto_config.extension_config().typed_config(); - Config::Utility::translateOpaqueConfig(proto_config.extension_config().typed_config(), ProtobufWkt::Struct(), + Config::Utility::translateOpaqueConfig(proto_config.extension_config().typed_config(), + ProtobufWkt::Struct(), context.messageValidationVisitor(), *message); - auto filter_factory = - factory.createFilterFactoryFromProto(*message, prefix, context); + auto filter_factory = factory.createFilterFactoryFromProto(*message, prefix, context); auto match_tree = Matcher::MatchTreeFactory(context.messageValidationVisitor()) diff --git a/source/common/http/match_wrapper/config.h b/source/common/http/match_wrapper/config.h index 5b0373a4d15d3..a3abb52108522 100644 --- a/source/common/http/match_wrapper/config.h +++ b/source/common/http/match_wrapper/config.h @@ -1,8 +1,8 @@ #pragma once #include "envoy/extensions/common/matching/v3/extension_matcher.pb.validate.h" - #include "envoy/server/filter_config.h" + #include "extensions/filters/http/common/factory_base.h" #include "extensions/filters/http/well_known_names.h" @@ -21,11 +21,11 @@ class MatchWrapperConfig private: Http::FilterFactoryCb createFilterFactoryFromProtoTyped( - const envoy::extensions::common::matching::v3::ExtensionWithMatcher& proto_config, const std::string&, - Server::Configuration::FactoryContext& context) override; + const envoy::extensions::common::matching::v3::ExtensionWithMatcher& proto_config, + const std::string&, Server::Configuration::FactoryContext& context) override; }; -} // namespace Lua +} // namespace MatchWrapper } // namespace HttpFilters } // namespace Extensions } // namespace Envoy diff --git a/test/integration/BUILD b/test/integration/BUILD index d433a40a9d30b..de814f6fb1b0f 100644 --- a/test/integration/BUILD +++ b/test/integration/BUILD @@ -882,6 +882,8 @@ envoy_cc_test( "//test/integration/filters:encoder_decoder_buffer_filter_lib", "//test/integration/filters:invalid_header_filter_lib", "//test/integration/filters:process_context_lib", + "//test/integration/filters:set_response_code_filter_config_proto_cc_proto", + "//test/integration/filters:set_response_code_filter_lib", "//test/integration/filters:stop_iteration_and_continue", "//test/mocks/http:http_mocks", "//test/test_common:utility_lib", @@ -1083,6 +1085,7 @@ envoy_cc_test( "//test/integration/filters:set_response_code_filter_config_proto_cc_proto", "//test/integration/filters:set_response_code_filter_lib", "//test/test_common:utility_lib", + "@envoy_api//envoy/extensions/common/matching/v3:pkg_cc_proto", "@envoy_api//envoy/extensions/filters/network/http_connection_manager/v3:pkg_cc_proto", "@envoy_api//envoy/service/extension/v3:pkg_cc_proto", ], diff --git a/test/integration/extension_discovery_integration_test.cc b/test/integration/extension_discovery_integration_test.cc index e7fa682af3424..64ce24fa8a1c0 100644 --- a/test/integration/extension_discovery_integration_test.cc +++ b/test/integration/extension_discovery_integration_test.cc @@ -1,5 +1,5 @@ -#include "envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.pb.h" #include "envoy/extensions/common/matching/v3/extension_matcher.pb.h" +#include "envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.pb.h" #include "envoy/service/extension/v3/config_discovery.pb.h" #include "test/common/grpc/grpc_client_integration.h" @@ -56,9 +56,10 @@ class ExtensionDiscoveryIntegrationTest : public Grpc::GrpcClientIntegrationPara : HttpIntegrationTest(Http::CodecClient::Type::HTTP1, ipVersion()) {} void addDynamicFilter(const std::string& name, bool apply_without_warming, - bool set_default_config = true, bool rate_limit = false) { + bool set_default_config = true, bool rate_limit = false, + bool use_default_matcher = false) { config_helper_.addConfigModifier( - [this, name, apply_without_warming, set_default_config, rate_limit]( + [this, name, apply_without_warming, set_default_config, rate_limit, use_default_matcher]( envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager& http_connection_manager) { auto* filter = http_connection_manager.mutable_http_filters()->Add(); @@ -69,10 +70,38 @@ class ExtensionDiscoveryIntegrationTest : public Grpc::GrpcClientIntegrationPara discovery->add_type_urls( "type.googleapis.com/envoy.extensions.common.matching.v3.ExtensionWithMatcher"); if (set_default_config) { - const auto default_configuration = - TestUtility::parseYaml( - "code: 403"); - discovery->mutable_default_config()->PackFrom(default_configuration); + if (use_default_matcher) { + const auto default_configuration = TestUtility::parseYaml< + envoy::extensions::common::matching::v3::ExtensionWithMatcher>( + R"EOF( + extension_config: + name: set-response-code + typed_config: + "@type": type.googleapis.com/test.integration.filters.SetResponseCodeFilterConfig + code: 403 + matcher: + matcher_tree: + input: + name: request-headers + typed_config: + "@type": type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpRequestHeaderMatchInput + header_name: default-matcher-header + exact_match_map: + map: + match: + action: + name: skip + typed_config: + "@type": type.googleapis.com/envoy.extensions.filters.common.matcher.action.v3.SkipFilter + )EOF"); + + discovery->mutable_default_config()->PackFrom(default_configuration); + } else { + const auto default_configuration = + TestUtility::parseYaml( + "code: 403"); + discovery->mutable_default_config()->PackFrom(default_configuration); + } } discovery->set_apply_default_config_without_warming(apply_without_warming); discovery->mutable_config_source()->set_resource_api_version( @@ -155,7 +184,7 @@ class ExtensionDiscoveryIntegrationTest : public Grpc::GrpcClientIntegrationPara } void sendXdsResponseWithFullYaml(const std::string& name, const std::string& version, - const std::string& full_yaml) { + const std::string& full_yaml) { envoy::service::discovery::v3::DiscoveryResponse response; response.set_version_info(version); response.set_type_url("type.googleapis.com/envoy.config.core.v3.TypedExtensionConfig"); @@ -248,8 +277,11 @@ TEST_P(ExtensionDiscoveryIntegrationTest, BasicSuccessWithMatcher) { ASSERT_TRUE(response->complete()); EXPECT_EQ("403", response->headers().getStatusValue()); } - Http::TestRequestHeaderMapImpl banned_request_headers_skipped{ - {":method", "GET"}, {":path", "/private/key"}, {"some-header", "match"}, {":scheme", "http"}, {":authority", "host"}}; + Http::TestRequestHeaderMapImpl banned_request_headers_skipped{{":method", "GET"}, + {":path", "/private/key"}, + {"some-header", "match"}, + {":scheme", "http"}, + {":authority", "host"}}; { auto response = codec_client_->makeHeaderOnlyRequest(banned_request_headers_skipped); response->waitForEndStream(); @@ -258,6 +290,38 @@ TEST_P(ExtensionDiscoveryIntegrationTest, BasicSuccessWithMatcher) { } } +TEST_P(ExtensionDiscoveryIntegrationTest, BasicDefaultMatcher) { + on_server_init_function_ = [&]() { waitXdsStream(); }; + addDynamicFilter("foo", false, true, false, true); + initialize(); + test_server_->waitForCounterGe("listener_manager.lds.update_success", 1); + EXPECT_EQ(test_server_->server().initManager().state(), Init::Manager::State::Initializing); + registerTestServerPorts({"http"}); + sendXdsResponse("foo", "1", invalidConfig()); + test_server_->waitForCounterGe("http.config_test.extension_config_discovery.foo.config_fail", 1); + test_server_->waitUntilListenersReady(); + test_server_->waitForGaugeGe("listener_manager.workers_started", 1); + EXPECT_EQ(test_server_->server().initManager().state(), Init::Manager::State::Initialized); + codec_client_ = makeHttpConnection(makeClientConnection((lookupPort("http")))); + { + Http::TestRequestHeaderMapImpl request_headers{ + {":method", "GET"}, {":path", "/"}, {":scheme", "http"}, {":authority", "host"}}; + auto response = codec_client_->makeHeaderOnlyRequest(request_headers); + response->waitForEndStream(); + ASSERT_TRUE(response->complete()); + EXPECT_EQ("403", response->headers().getStatusValue()); + } + Http::TestRequestHeaderMapImpl request_headers{{":method", "GET"}, + {"default-matcher-header", "match"}, + {":path", "/"}, + {":scheme", "http"}, + {":authority", "host"}}; + auto response = codec_client_->makeHeaderOnlyRequest(request_headers); + response->waitForEndStream(); + ASSERT_TRUE(response->complete()); + EXPECT_EQ("200", response->headers().getStatusValue()); +} + TEST_P(ExtensionDiscoveryIntegrationTest, BasicFailWithDefault) { on_server_init_function_ = [&]() { waitXdsStream(); }; addDynamicFilter("foo", false); diff --git a/test/integration/integration_test.cc b/test/integration/integration_test.cc index 15f7587a2d34f..bbc84394e6b8d 100644 --- a/test/integration/integration_test.cc +++ b/test/integration/integration_test.cc @@ -363,6 +363,58 @@ TEST_P(IntegrationTest, EnvoyProxying100ContinueWithDecodeDataPause) { testEnvoyProxying1xx(true); } +// Verifies that we can construct a match tree with a filter, and that we are able to skip +// filter invocation through the match tree. +TEST_P(IntegrationTest, MatchingHttpFilterConstruction) { + config_helper_.addFilter(R"EOF( +name: matcher +typed_config: + "@type": type.googleapis.com/envoy.extensions.common.matching.v3.ExtensionWithMatcher + extension_config: + name: set-response-code + typed_config: + "@type": type.googleapis.com/test.integration.filters.SetResponseCodeFilterConfig + code: 403 + matcher: + matcher_tree: + input: + name: request-headers + typed_config: + "@type": type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpRequestHeaderMatchInput + header_name: match-header + exact_match_map: + map: + match: + action: + name: skip + typed_config: + "@type": type.googleapis.com/envoy.extensions.filters.common.matcher.action.v3.SkipFilter +)EOF"); + + initialize(); + + codec_client_ = makeHttpConnection(lookupPort("http")); + + { + auto response = codec_client_->makeRequestWithBody(default_request_headers_, 1024); + response->waitForEndStream(); + EXPECT_THAT(response->headers(), HttpStatusIs("403")); + } + + codec_client_ = makeHttpConnection(lookupPort("http")); + Http::TestRequestHeaderMapImpl request_headers{ + {":method", "POST"}, {":path", "/test/long/url"}, {":scheme", "http"}, + {":authority", "host"}, {"match-header", "match"}, {"content-type", "application/grpc"}}; + auto response = codec_client_->makeRequestWithBody(request_headers, 1024); + waitForNextUpstreamRequest(); + upstream_request_->encodeHeaders(default_response_headers_, true); + + response->waitForEndStream(); + EXPECT_THAT(response->headers(), HttpStatusIs("200")); + + codec_client_->close(); +} + // This is a regression for https://github.com/envoyproxy/envoy/issues/2715 and validates that a // pending request is not sent on a connection that has been half-closed. TEST_P(IntegrationTest, UpstreamDisconnectWithTwoRequests) { From 13a552df107efdcf85c297dd87140fc3b9ac780c Mon Sep 17 00:00:00 2001 From: Snow Pettersen Date: Tue, 15 Dec 2020 20:14:55 +0000 Subject: [PATCH 14/32] remove old protos Signed-off-by: Snow Pettersen --- api/BUILD | 1 + .../config/common/matcher/v3/matcher.proto | 11 ---------- .../common/matcher/v3/skip_action.proto | 19 ------------------ .../common/matcher/v4alpha/matcher.proto | 14 ------------- .../common/matcher/v4alpha/skip_action.proto | 20 ------------------- api/versioning/BUILD | 1 + .../config/common/matcher/v3/matcher.proto | 11 ---------- .../common/matcher/v3/skip_action.proto | 19 ------------------ .../common/matcher/v4alpha/matcher.proto | 14 ------------- .../common/matcher/v4alpha/skip_action.proto | 20 ------------------- 10 files changed, 2 insertions(+), 128 deletions(-) delete mode 100644 api/envoy/config/common/matcher/v3/skip_action.proto delete mode 100644 api/envoy/config/common/matcher/v4alpha/skip_action.proto delete mode 100644 generated_api_shadow/envoy/config/common/matcher/v3/skip_action.proto delete mode 100644 generated_api_shadow/envoy/config/common/matcher/v4alpha/skip_action.proto diff --git a/api/BUILD b/api/BUILD index f7c75a2d7faea..ac87239a96be7 100644 --- a/api/BUILD +++ b/api/BUILD @@ -167,6 +167,7 @@ proto_library( "//envoy/extensions/compression/gzip/decompressor/v3:pkg", "//envoy/extensions/filters/common/fault/v3:pkg", "//envoy/extensions/filters/common/matcher/action/v3:pkg", + "//envoy/extensions/filters/common/matching/v3:pkg", "//envoy/extensions/filters/http/adaptive_concurrency/v3:pkg", "//envoy/extensions/filters/http/admission_control/v3alpha:pkg", "//envoy/extensions/filters/http/aws_lambda/v3:pkg", diff --git a/api/envoy/config/common/matcher/v3/matcher.proto b/api/envoy/config/common/matcher/v3/matcher.proto index 0468fb28d2ce8..2760be1b8bde8 100644 --- a/api/envoy/config/common/matcher/v3/matcher.proto +++ b/api/envoy/config/common/matcher/v3/matcher.proto @@ -139,17 +139,6 @@ message Matcher { OnMatch on_no_match = 3; } -// Wrapper around an existing extension that provides an associated matcher. This allows -// decorating an existing extension with a matcher, which can be used to match against -// relevant protocol data. -message ExtensionWithMatcher { - // The associated matcher. - Matcher matcher = 1 [(validate.rules).message = {required: true}]; - - // The underlying extension config. - core.v3.TypedExtensionConfig typed_config = 2 [(validate.rules).message = {required: true}]; -} - // Match configuration. This is a recursive structure which allows complex nested match // configurations to be built using various logical operators. // [#next-free-field: 11] diff --git a/api/envoy/config/common/matcher/v3/skip_action.proto b/api/envoy/config/common/matcher/v3/skip_action.proto deleted file mode 100644 index f5408f92dc3c8..0000000000000 --- a/api/envoy/config/common/matcher/v3/skip_action.proto +++ /dev/null @@ -1,19 +0,0 @@ -syntax = "proto3"; - -package envoy.config.common.matcher.v3; - -import "udpa/annotations/migrate.proto"; -import "udpa/annotations/status.proto"; -import "udpa/annotations/versioning.proto"; -import "validate/validate.proto"; - -option java_package = "io.envoyproxy.envoy.config.common.matcher.v3"; -option java_outer_classname = "SkipActionProto"; -option java_multiple_files = true; -option (udpa.annotations.file_status).package_version_status = ACTIVE; - -// [#protodoc-title: Common Match Actions] - -// Indicates that the associated filter should be skipped. -message SkipFilterMatchAction { -} diff --git a/api/envoy/config/common/matcher/v4alpha/matcher.proto b/api/envoy/config/common/matcher/v4alpha/matcher.proto index 7769ea590463e..7c846e1e93755 100644 --- a/api/envoy/config/common/matcher/v4alpha/matcher.proto +++ b/api/envoy/config/common/matcher/v4alpha/matcher.proto @@ -165,20 +165,6 @@ message Matcher { OnMatch on_no_match = 3; } -// Wrapper around an existing extension that provides an associated matcher. This allows -// decorating an existing extension with a matcher, which can be used to match against -// relevant protocol data. -message ExtensionWithMatcher { - option (udpa.annotations.versioning).previous_message_type = - "envoy.config.common.matcher.v3.ExtensionWithMatcher"; - - // The associated matcher. - Matcher matcher = 1 [(validate.rules).message = {required: true}]; - - // The underlying extension config. - core.v4alpha.TypedExtensionConfig typed_config = 2 [(validate.rules).message = {required: true}]; -} - // Match configuration. This is a recursive structure which allows complex nested match // configurations to be built using various logical operators. // [#next-free-field: 11] diff --git a/api/envoy/config/common/matcher/v4alpha/skip_action.proto b/api/envoy/config/common/matcher/v4alpha/skip_action.proto deleted file mode 100644 index 4d0e930fe137c..0000000000000 --- a/api/envoy/config/common/matcher/v4alpha/skip_action.proto +++ /dev/null @@ -1,20 +0,0 @@ -syntax = "proto3"; - -package envoy.config.common.matcher.v4alpha; - -import "udpa/annotations/status.proto"; -import "udpa/annotations/versioning.proto"; -import "validate/validate.proto"; - -option java_package = "io.envoyproxy.envoy.config.common.matcher.v4alpha"; -option java_outer_classname = "SkipActionProto"; -option java_multiple_files = true; -option (udpa.annotations.file_status).package_version_status = NEXT_MAJOR_VERSION_CANDIDATE; - -// [#protodoc-title: Common Match Actions] - -// Indicates that the associated filter should be skipped. -message SkipFilterMatchAction { - option (udpa.annotations.versioning).previous_message_type = - "envoy.config.common.matcher.v3.SkipFilterMatchAction"; -} diff --git a/api/versioning/BUILD b/api/versioning/BUILD index 41addf767671d..d2a0f7857d38a 100644 --- a/api/versioning/BUILD +++ b/api/versioning/BUILD @@ -50,6 +50,7 @@ proto_library( "//envoy/extensions/compression/gzip/decompressor/v3:pkg", "//envoy/extensions/filters/common/fault/v3:pkg", "//envoy/extensions/filters/common/matcher/action/v3:pkg", + "//envoy/extensions/filters/common/matching/v3:pkg", "//envoy/extensions/filters/http/adaptive_concurrency/v3:pkg", "//envoy/extensions/filters/http/admission_control/v3alpha:pkg", "//envoy/extensions/filters/http/aws_lambda/v3:pkg", diff --git a/generated_api_shadow/envoy/config/common/matcher/v3/matcher.proto b/generated_api_shadow/envoy/config/common/matcher/v3/matcher.proto index 0468fb28d2ce8..2760be1b8bde8 100644 --- a/generated_api_shadow/envoy/config/common/matcher/v3/matcher.proto +++ b/generated_api_shadow/envoy/config/common/matcher/v3/matcher.proto @@ -139,17 +139,6 @@ message Matcher { OnMatch on_no_match = 3; } -// Wrapper around an existing extension that provides an associated matcher. This allows -// decorating an existing extension with a matcher, which can be used to match against -// relevant protocol data. -message ExtensionWithMatcher { - // The associated matcher. - Matcher matcher = 1 [(validate.rules).message = {required: true}]; - - // The underlying extension config. - core.v3.TypedExtensionConfig typed_config = 2 [(validate.rules).message = {required: true}]; -} - // Match configuration. This is a recursive structure which allows complex nested match // configurations to be built using various logical operators. // [#next-free-field: 11] diff --git a/generated_api_shadow/envoy/config/common/matcher/v3/skip_action.proto b/generated_api_shadow/envoy/config/common/matcher/v3/skip_action.proto deleted file mode 100644 index f5408f92dc3c8..0000000000000 --- a/generated_api_shadow/envoy/config/common/matcher/v3/skip_action.proto +++ /dev/null @@ -1,19 +0,0 @@ -syntax = "proto3"; - -package envoy.config.common.matcher.v3; - -import "udpa/annotations/migrate.proto"; -import "udpa/annotations/status.proto"; -import "udpa/annotations/versioning.proto"; -import "validate/validate.proto"; - -option java_package = "io.envoyproxy.envoy.config.common.matcher.v3"; -option java_outer_classname = "SkipActionProto"; -option java_multiple_files = true; -option (udpa.annotations.file_status).package_version_status = ACTIVE; - -// [#protodoc-title: Common Match Actions] - -// Indicates that the associated filter should be skipped. -message SkipFilterMatchAction { -} diff --git a/generated_api_shadow/envoy/config/common/matcher/v4alpha/matcher.proto b/generated_api_shadow/envoy/config/common/matcher/v4alpha/matcher.proto index 7769ea590463e..7c846e1e93755 100644 --- a/generated_api_shadow/envoy/config/common/matcher/v4alpha/matcher.proto +++ b/generated_api_shadow/envoy/config/common/matcher/v4alpha/matcher.proto @@ -165,20 +165,6 @@ message Matcher { OnMatch on_no_match = 3; } -// Wrapper around an existing extension that provides an associated matcher. This allows -// decorating an existing extension with a matcher, which can be used to match against -// relevant protocol data. -message ExtensionWithMatcher { - option (udpa.annotations.versioning).previous_message_type = - "envoy.config.common.matcher.v3.ExtensionWithMatcher"; - - // The associated matcher. - Matcher matcher = 1 [(validate.rules).message = {required: true}]; - - // The underlying extension config. - core.v4alpha.TypedExtensionConfig typed_config = 2 [(validate.rules).message = {required: true}]; -} - // Match configuration. This is a recursive structure which allows complex nested match // configurations to be built using various logical operators. // [#next-free-field: 11] diff --git a/generated_api_shadow/envoy/config/common/matcher/v4alpha/skip_action.proto b/generated_api_shadow/envoy/config/common/matcher/v4alpha/skip_action.proto deleted file mode 100644 index 4d0e930fe137c..0000000000000 --- a/generated_api_shadow/envoy/config/common/matcher/v4alpha/skip_action.proto +++ /dev/null @@ -1,20 +0,0 @@ -syntax = "proto3"; - -package envoy.config.common.matcher.v4alpha; - -import "udpa/annotations/status.proto"; -import "udpa/annotations/versioning.proto"; -import "validate/validate.proto"; - -option java_package = "io.envoyproxy.envoy.config.common.matcher.v4alpha"; -option java_outer_classname = "SkipActionProto"; -option java_multiple_files = true; -option (udpa.annotations.file_status).package_version_status = NEXT_MAJOR_VERSION_CANDIDATE; - -// [#protodoc-title: Common Match Actions] - -// Indicates that the associated filter should be skipped. -message SkipFilterMatchAction { - option (udpa.annotations.versioning).previous_message_type = - "envoy.config.common.matcher.v3.SkipFilterMatchAction"; -} From a66c51f933e6a6e52168b5c08d73117c5065e761 Mon Sep 17 00:00:00 2001 From: Snow Pettersen Date: Tue, 15 Dec 2020 20:17:04 +0000 Subject: [PATCH 15/32] cleanup Signed-off-by: Snow Pettersen --- source/common/http/filter_manager.cc | 2 -- source/common/http/filter_manager.h | 20 ++++++++++---------- 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/source/common/http/filter_manager.cc b/source/common/http/filter_manager.cc index 3fd6f3683ad0a..6dac21f843749 100644 --- a/source/common/http/filter_manager.cc +++ b/source/common/http/filter_manager.cc @@ -14,8 +14,6 @@ namespace Envoy { namespace Http { -// REGISTER_FACTORY(HttpResponseHeadersDataInputFactory, -// Matcher::DataInputFactory){"response-headers"}; namespace { REGISTER_FACTORY(HttpRequestHeadersDataInputFactory, Matcher::DataInputFactory); REGISTER_FACTORY(SkipActionFactory, Matcher::ActionFactory); diff --git a/source/common/http/filter_manager.h b/source/common/http/filter_manager.h index 40e77ff6142d6..be521a99fc07e 100644 --- a/source/common/http/filter_manager.h +++ b/source/common/http/filter_manager.h @@ -479,20 +479,20 @@ class FilterManagerCallbacks { virtual void setRequestTrailers(RequestTrailerMapPtr&& request_trailers) PURE; /** - * Passes ownership of received continue headers to the parent. This may be called multiple - * times in the case of multiple upstream calls. + * Passes ownership of received continue headers to the parent. This may be called multiple times + * in the case of multiple upstream calls. */ virtual void setContinueHeaders(ResponseHeaderMapPtr&& response_headers) PURE; /** - * Passes ownership of received response headers to the parent. This may be called multiple - * times in the case of multiple upstream calls. + * Passes ownership of received response headers to the parent. This may be called multiple times + * in the case of multiple upstream calls. */ virtual void setResponseHeaders(ResponseHeaderMapPtr&& response_headers) PURE; /** - * Passes ownership of received response trailers to the parent. This may be called multiple - * times in the case of multiple upstream calls. + * Passes ownership of received response trailers to the parent. This may be called multiple times + * in the case of multiple upstream calls. */ virtual void setResponseTrailers(ResponseTrailerMapPtr&& response_trailers) PURE; @@ -522,15 +522,15 @@ class FilterManagerCallbacks { /** * Retrieves a pointer to the response headers set via the last call to setResponseHeaders. * Note that response headers might be set multiple times (e.g. if a local reply is issued after - * headers have been received but before headers have been encoded), so it is not safe in - * general to assume that any set of headers will be valid for the duration of a stream. + * headers have been received but before headers have been encoded), so it is not safe in general + * to assume that any set of headers will be valid for the duration of a stream. */ virtual ResponseHeaderMapOptRef responseHeaders() PURE; /** * Retrieves a pointer to the last response trailers set via setResponseTrailers. - * Note that response trailers might be set multiple times, so it is not safe in general to - * assume that any set of trailers will be valid for the duration of the stream. + * Note that response trailers might be set multiple times, so it is not safe in general to assume + * that any set of trailers will be valid for the duration of the stream. */ virtual ResponseTrailerMapOptRef responseTrailers() PURE; From b66ef325178cc3eb91a5d0112999383ce7c39e06 Mon Sep 17 00:00:00 2001 From: Snow Pettersen Date: Tue, 15 Dec 2020 20:25:17 +0000 Subject: [PATCH 16/32] more cleanups Signed-off-by: Snow Pettersen --- source/common/http/match_wrapper/config.cc | 44 +++++++++---------- source/common/http/match_wrapper/config.h | 19 ++++---- .../filters/http/well_known_names.h | 2 - 3 files changed, 30 insertions(+), 35 deletions(-) diff --git a/source/common/http/match_wrapper/config.cc b/source/common/http/match_wrapper/config.cc index b8e29a7129ac7..a28105f735711 100644 --- a/source/common/http/match_wrapper/config.cc +++ b/source/common/http/match_wrapper/config.cc @@ -7,49 +7,49 @@ #include "common/matcher/matcher.h" namespace Envoy { -namespace Extensions { -namespace HttpFilters { +namespace Common { +namespace Http { namespace MatchWrapper { namespace { -struct DelegatingFactoryCallbacks : public Http::FilterChainFactoryCallbacks { - DelegatingFactoryCallbacks(Http::FilterChainFactoryCallbacks& delegated_callbacks, - Matcher::MatchTreeSharedPtr match_tree) +struct DelegatingFactoryCallbacks : public Envoy::Http::FilterChainFactoryCallbacks { + DelegatingFactoryCallbacks(Envoy::Http::FilterChainFactoryCallbacks& delegated_callbacks, + Matcher::MatchTreeSharedPtr match_tree) : delegated_callbacks_(delegated_callbacks), match_tree_(std::move(match_tree)) {} - void addStreamDecoderFilter(Http::StreamDecoderFilterSharedPtr filter) override { + void addStreamDecoderFilter(Envoy::Http::StreamDecoderFilterSharedPtr filter) override { delegated_callbacks_.addStreamDecoderFilter(std::move(filter), match_tree_); } void - addStreamDecoderFilter(Http::StreamDecoderFilterSharedPtr filter, - Matcher::MatchTreeSharedPtr match_tree) override { + addStreamDecoderFilter(Envoy::Http::StreamDecoderFilterSharedPtr filter, + Matcher::MatchTreeSharedPtr match_tree) override { delegated_callbacks_.addStreamDecoderFilter(std::move(filter), std::move(match_tree)); } - void addStreamEncoderFilter(Http::StreamEncoderFilterSharedPtr filter) override { + void addStreamEncoderFilter(Envoy::Http::StreamEncoderFilterSharedPtr filter) override { delegated_callbacks_.addStreamEncoderFilter(std::move(filter), match_tree_); } void - addStreamEncoderFilter(Http::StreamEncoderFilterSharedPtr filter, - Matcher::MatchTreeSharedPtr match_tree) override { + addStreamEncoderFilter(Envoy::Http::StreamEncoderFilterSharedPtr filter, + Matcher::MatchTreeSharedPtr match_tree) override { delegated_callbacks_.addStreamEncoderFilter(std::move(filter), std::move(match_tree)); } - void addStreamFilter(Http::StreamFilterSharedPtr filter) override { + void addStreamFilter(Envoy::Http::StreamFilterSharedPtr filter) override { delegated_callbacks_.addStreamFilter(std::move(filter), match_tree_); } - void addStreamFilter(Http::StreamFilterSharedPtr filter, - Matcher::MatchTreeSharedPtr match_tree) override { + void addStreamFilter(Envoy::Http::StreamFilterSharedPtr filter, + Matcher::MatchTreeSharedPtr match_tree) override { delegated_callbacks_.addStreamFilter(std::move(filter), std::move(match_tree)); } void addAccessLogHandler(AccessLog::InstanceSharedPtr handler) override { delegated_callbacks_.addAccessLogHandler(std::move(handler)); } - Http::FilterChainFactoryCallbacks& delegated_callbacks_; - Matcher::MatchTreeSharedPtr match_tree_; + Envoy::Http::FilterChainFactoryCallbacks& delegated_callbacks_; + Matcher::MatchTreeSharedPtr match_tree_; }; } // namespace -Http::FilterFactoryCb MatchWrapperConfig::createFilterFactoryFromProtoTyped( +Envoy::Http::FilterFactoryCb MatchWrapperConfig::createFilterFactoryFromProtoTyped( const envoy::extensions::common::matching::v3::ExtensionWithMatcher& proto_config, const std::string& prefix, Server::Configuration::FactoryContext& context) { @@ -66,17 +66,17 @@ Http::FilterFactoryCb MatchWrapperConfig::createFilterFactoryFromProtoTyped( auto filter_factory = factory.createFilterFactoryFromProto(*message, prefix, context); auto match_tree = - Matcher::MatchTreeFactory(context.messageValidationVisitor()) + Matcher::MatchTreeFactory(context.messageValidationVisitor()) .create(proto_config.matcher()); - return [filter_factory, match_tree](Http::FilterChainFactoryCallbacks& callbacks) -> void { + return [filter_factory, match_tree](Envoy::Http::FilterChainFactoryCallbacks& callbacks) -> void { DelegatingFactoryCallbacks delegated_callbacks(callbacks, match_tree); return filter_factory(delegated_callbacks); }; } - return [](Http::FilterChainFactoryCallbacks&) -> void {}; + return [](Envoy::Http::FilterChainFactoryCallbacks&) -> void {}; } /** @@ -85,6 +85,6 @@ Http::FilterFactoryCb MatchWrapperConfig::createFilterFactoryFromProtoTyped( REGISTER_FACTORY(MatchWrapperConfig, Server::Configuration::NamedHttpFilterConfigFactory){"blah"}; } // namespace MatchWrapper -} // namespace HttpFilters -} // namespace Extensions +} // namespace Http +} // namespace Common } // namespace Envoy diff --git a/source/common/http/match_wrapper/config.h b/source/common/http/match_wrapper/config.h index a3abb52108522..c1417aaa6f247 100644 --- a/source/common/http/match_wrapper/config.h +++ b/source/common/http/match_wrapper/config.h @@ -7,25 +7,22 @@ #include "extensions/filters/http/well_known_names.h" namespace Envoy { -namespace Extensions { -namespace HttpFilters { +namespace Common { +namespace Http { namespace MatchWrapper { -/** - * Config registration for the Lua filter. @see NamedHttpFilterConfigFactory. - */ -class MatchWrapperConfig - : public Common::FactoryBase { +class MatchWrapperConfig : public Extensions::HttpFilters::Common::FactoryBase< + envoy::extensions::common::matching::v3::ExtensionWithMatcher> { public: - MatchWrapperConfig() : FactoryBase(HttpFilterNames::get().MatchingFilter) {} + MatchWrapperConfig() : FactoryBase("match-wrapper") {} private: - Http::FilterFactoryCb createFilterFactoryFromProtoTyped( + Envoy::Http::FilterFactoryCb createFilterFactoryFromProtoTyped( const envoy::extensions::common::matching::v3::ExtensionWithMatcher& proto_config, const std::string&, Server::Configuration::FactoryContext& context) override; }; } // namespace MatchWrapper -} // namespace HttpFilters -} // namespace Extensions +} // namespace Http +} // namespace Common } // namespace Envoy diff --git a/source/extensions/filters/http/well_known_names.h b/source/extensions/filters/http/well_known_names.h index 44f45084ae42f..189640302f8db 100644 --- a/source/extensions/filters/http/well_known_names.h +++ b/source/extensions/filters/http/well_known_names.h @@ -86,8 +86,6 @@ class HttpFilterNameValues { const std::string KillRequest = "envoy.filters.http.kill_request"; // External Processing filter const std::string ExternalProcessing = "envoy.filters.http.ext_proc"; - // Match tree wrapper - const std::string MatchingFilter = "envoy.filters.http.matching_filter"; }; using HttpFilterNames = ConstSingleton; From 01fced22182d2636e0f919c42620b93bf0661641 Mon Sep 17 00:00:00 2001 From: Snow Pettersen Date: Tue, 15 Dec 2020 20:27:21 +0000 Subject: [PATCH 17/32] format Signed-off-by: Snow Pettersen --- source/common/http/match_wrapper/config.cc | 26 ++++++++++++---------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/source/common/http/match_wrapper/config.cc b/source/common/http/match_wrapper/config.cc index a28105f735711..679c091aadf06 100644 --- a/source/common/http/match_wrapper/config.cc +++ b/source/common/http/match_wrapper/config.cc @@ -20,24 +20,25 @@ struct DelegatingFactoryCallbacks : public Envoy::Http::FilterChainFactoryCallba void addStreamDecoderFilter(Envoy::Http::StreamDecoderFilterSharedPtr filter) override { delegated_callbacks_.addStreamDecoderFilter(std::move(filter), match_tree_); } - void - addStreamDecoderFilter(Envoy::Http::StreamDecoderFilterSharedPtr filter, - Matcher::MatchTreeSharedPtr match_tree) override { + void addStreamDecoderFilter( + Envoy::Http::StreamDecoderFilterSharedPtr filter, + Matcher::MatchTreeSharedPtr match_tree) override { delegated_callbacks_.addStreamDecoderFilter(std::move(filter), std::move(match_tree)); } void addStreamEncoderFilter(Envoy::Http::StreamEncoderFilterSharedPtr filter) override { delegated_callbacks_.addStreamEncoderFilter(std::move(filter), match_tree_); } - void - addStreamEncoderFilter(Envoy::Http::StreamEncoderFilterSharedPtr filter, - Matcher::MatchTreeSharedPtr match_tree) override { + void addStreamEncoderFilter( + Envoy::Http::StreamEncoderFilterSharedPtr filter, + Matcher::MatchTreeSharedPtr match_tree) override { delegated_callbacks_.addStreamEncoderFilter(std::move(filter), std::move(match_tree)); } void addStreamFilter(Envoy::Http::StreamFilterSharedPtr filter) override { delegated_callbacks_.addStreamFilter(std::move(filter), match_tree_); } - void addStreamFilter(Envoy::Http::StreamFilterSharedPtr filter, - Matcher::MatchTreeSharedPtr match_tree) override { + void + addStreamFilter(Envoy::Http::StreamFilterSharedPtr filter, + Matcher::MatchTreeSharedPtr match_tree) override { delegated_callbacks_.addStreamFilter(std::move(filter), std::move(match_tree)); } void addAccessLogHandler(AccessLog::InstanceSharedPtr handler) override { @@ -69,11 +70,12 @@ Envoy::Http::FilterFactoryCb MatchWrapperConfig::createFilterFactoryFromProtoTyp Matcher::MatchTreeFactory(context.messageValidationVisitor()) .create(proto_config.matcher()); - return [filter_factory, match_tree](Envoy::Http::FilterChainFactoryCallbacks& callbacks) -> void { - DelegatingFactoryCallbacks delegated_callbacks(callbacks, match_tree); + return + [filter_factory, match_tree](Envoy::Http::FilterChainFactoryCallbacks& callbacks) -> void { + DelegatingFactoryCallbacks delegated_callbacks(callbacks, match_tree); - return filter_factory(delegated_callbacks); - }; + return filter_factory(delegated_callbacks); + }; } return [](Envoy::Http::FilterChainFactoryCallbacks&) -> void {}; From 142c70bd0247ea92131df43fdc274013f9d35751 Mon Sep 17 00:00:00 2001 From: Snow Pettersen Date: Tue, 15 Dec 2020 21:17:35 +0000 Subject: [PATCH 18/32] remove more old protos Signed-off-by: Snow Pettersen --- api/BUILD | 1 - .../filters/common/matching/v3/BUILD | 9 --------- .../common/matching/v3/skip_action.proto | 19 ------------------- api/versioning/BUILD | 1 - .../filters/common/matching/v3/BUILD | 9 --------- .../common/matching/v3/skip_action.proto | 19 ------------------- 6 files changed, 58 deletions(-) delete mode 100644 api/envoy/extensions/filters/common/matching/v3/BUILD delete mode 100644 api/envoy/extensions/filters/common/matching/v3/skip_action.proto delete mode 100644 generated_api_shadow/envoy/extensions/filters/common/matching/v3/BUILD delete mode 100644 generated_api_shadow/envoy/extensions/filters/common/matching/v3/skip_action.proto diff --git a/api/BUILD b/api/BUILD index ac87239a96be7..f7c75a2d7faea 100644 --- a/api/BUILD +++ b/api/BUILD @@ -167,7 +167,6 @@ proto_library( "//envoy/extensions/compression/gzip/decompressor/v3:pkg", "//envoy/extensions/filters/common/fault/v3:pkg", "//envoy/extensions/filters/common/matcher/action/v3:pkg", - "//envoy/extensions/filters/common/matching/v3:pkg", "//envoy/extensions/filters/http/adaptive_concurrency/v3:pkg", "//envoy/extensions/filters/http/admission_control/v3alpha:pkg", "//envoy/extensions/filters/http/aws_lambda/v3:pkg", diff --git a/api/envoy/extensions/filters/common/matching/v3/BUILD b/api/envoy/extensions/filters/common/matching/v3/BUILD deleted file mode 100644 index ee92fb652582e..0000000000000 --- a/api/envoy/extensions/filters/common/matching/v3/BUILD +++ /dev/null @@ -1,9 +0,0 @@ -# DO NOT EDIT. This file is generated by tools/proto_format/proto_sync.py. - -load("@envoy_api//bazel:api_build_system.bzl", "api_proto_package") - -licenses(["notice"]) # Apache 2 - -api_proto_package( - deps = ["@com_github_cncf_udpa//udpa/annotations:pkg"], -) diff --git a/api/envoy/extensions/filters/common/matching/v3/skip_action.proto b/api/envoy/extensions/filters/common/matching/v3/skip_action.proto deleted file mode 100644 index ab7e18e7cf34f..0000000000000 --- a/api/envoy/extensions/filters/common/matching/v3/skip_action.proto +++ /dev/null @@ -1,19 +0,0 @@ -syntax = "proto3"; - -package envoy.extensions.filters.common.matching.v3; - -import "udpa/annotations/migrate.proto"; -import "udpa/annotations/status.proto"; -import "udpa/annotations/versioning.proto"; -import "validate/validate.proto"; - -option java_package = "io.envoyproxy.envoy.extensions.filters.common.matching.v3"; -option java_outer_classname = "SkipActionProto"; -option java_multiple_files = true; -option (udpa.annotations.file_status).package_version_status = ACTIVE; - -// [#protodoc-title: Common Match Actions] - -// Indicates that the associated filter should be skipped. -message SkipFilterMatchAction { -} diff --git a/api/versioning/BUILD b/api/versioning/BUILD index d2a0f7857d38a..41addf767671d 100644 --- a/api/versioning/BUILD +++ b/api/versioning/BUILD @@ -50,7 +50,6 @@ proto_library( "//envoy/extensions/compression/gzip/decompressor/v3:pkg", "//envoy/extensions/filters/common/fault/v3:pkg", "//envoy/extensions/filters/common/matcher/action/v3:pkg", - "//envoy/extensions/filters/common/matching/v3:pkg", "//envoy/extensions/filters/http/adaptive_concurrency/v3:pkg", "//envoy/extensions/filters/http/admission_control/v3alpha:pkg", "//envoy/extensions/filters/http/aws_lambda/v3:pkg", diff --git a/generated_api_shadow/envoy/extensions/filters/common/matching/v3/BUILD b/generated_api_shadow/envoy/extensions/filters/common/matching/v3/BUILD deleted file mode 100644 index ee92fb652582e..0000000000000 --- a/generated_api_shadow/envoy/extensions/filters/common/matching/v3/BUILD +++ /dev/null @@ -1,9 +0,0 @@ -# DO NOT EDIT. This file is generated by tools/proto_format/proto_sync.py. - -load("@envoy_api//bazel:api_build_system.bzl", "api_proto_package") - -licenses(["notice"]) # Apache 2 - -api_proto_package( - deps = ["@com_github_cncf_udpa//udpa/annotations:pkg"], -) diff --git a/generated_api_shadow/envoy/extensions/filters/common/matching/v3/skip_action.proto b/generated_api_shadow/envoy/extensions/filters/common/matching/v3/skip_action.proto deleted file mode 100644 index ab7e18e7cf34f..0000000000000 --- a/generated_api_shadow/envoy/extensions/filters/common/matching/v3/skip_action.proto +++ /dev/null @@ -1,19 +0,0 @@ -syntax = "proto3"; - -package envoy.extensions.filters.common.matching.v3; - -import "udpa/annotations/migrate.proto"; -import "udpa/annotations/status.proto"; -import "udpa/annotations/versioning.proto"; -import "validate/validate.proto"; - -option java_package = "io.envoyproxy.envoy.extensions.filters.common.matching.v3"; -option java_outer_classname = "SkipActionProto"; -option java_multiple_files = true; -option (udpa.annotations.file_status).package_version_status = ACTIVE; - -// [#protodoc-title: Common Match Actions] - -// Indicates that the associated filter should be skipped. -message SkipFilterMatchAction { -} From 9ce852389f765005118ce611e28f6a73e9a438cb Mon Sep 17 00:00:00 2001 From: Snow Pettersen Date: Wed, 16 Dec 2020 13:34:49 +0000 Subject: [PATCH 19/32] add comments Signed-off-by: Snow Pettersen --- .../v3/http_connection_manager.proto | 6 ++++++ .../v4alpha/http_connection_manager.proto | 6 ++++++ .../v3/http_connection_manager.proto | 6 ++++++ .../v4alpha/http_connection_manager.proto | 6 ++++++ 4 files changed, 24 insertions(+) diff --git a/api/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto b/api/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto index b51a819fa5086..78bdebd2dbc05 100644 --- a/api/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto +++ b/api/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto @@ -839,10 +839,16 @@ message RequestIDExtension { google.protobuf.Any typed_config = 1; } +// Match input indicates that matching should be done on a specific request header. +// TODO(snowp): Link to unified matching docs. message HttpRequestHeaderMatchInput { + // The request header to match on. string header_name = 1; } +// Match input indicating that matching should be done on a specific response header. +// TODO(snowp): Link to unified matching docs. message HttpResponseHeaderMatchInput { + // The response header to match on. string header_name = 1; } diff --git a/api/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto b/api/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto index 9e8f46582cdfb..5a1802f354397 100644 --- a/api/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto +++ b/api/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto @@ -845,16 +845,22 @@ message RequestIDExtension { google.protobuf.Any typed_config = 1; } +// Match input indicates that matching should be done on a specific request header. +// TODO(snowp): Link to unified matching docs. message HttpRequestHeaderMatchInput { option (udpa.annotations.versioning).previous_message_type = "envoy.extensions.filters.network.http_connection_manager.v3.HttpRequestHeaderMatchInput"; + // The request header to match on. string header_name = 1; } +// Match input indicating that matching should be done on a specific response header. +// TODO(snowp): Link to unified matching docs. message HttpResponseHeaderMatchInput { option (udpa.annotations.versioning).previous_message_type = "envoy.extensions.filters.network.http_connection_manager.v3.HttpResponseHeaderMatchInput"; + // The response header to match on. string header_name = 1; } diff --git a/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto b/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto index fb948c9ac9c22..aa77825226f6f 100644 --- a/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto +++ b/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto @@ -844,10 +844,16 @@ message RequestIDExtension { google.protobuf.Any typed_config = 1; } +// Match input indicates that matching should be done on a specific request header. +// TODO(snowp): Link to unified matching docs. message HttpRequestHeaderMatchInput { + // The request header to match on. string header_name = 1; } +// Match input indicating that matching should be done on a specific response header. +// TODO(snowp): Link to unified matching docs. message HttpResponseHeaderMatchInput { + // The response header to match on. string header_name = 1; } diff --git a/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto b/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto index 9e8f46582cdfb..5a1802f354397 100644 --- a/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto +++ b/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto @@ -845,16 +845,22 @@ message RequestIDExtension { google.protobuf.Any typed_config = 1; } +// Match input indicates that matching should be done on a specific request header. +// TODO(snowp): Link to unified matching docs. message HttpRequestHeaderMatchInput { option (udpa.annotations.versioning).previous_message_type = "envoy.extensions.filters.network.http_connection_manager.v3.HttpRequestHeaderMatchInput"; + // The request header to match on. string header_name = 1; } +// Match input indicating that matching should be done on a specific response header. +// TODO(snowp): Link to unified matching docs. message HttpResponseHeaderMatchInput { option (udpa.annotations.versioning).previous_message_type = "envoy.extensions.filters.network.http_connection_manager.v3.HttpResponseHeaderMatchInput"; + // The response header to match on. string header_name = 1; } From be5f50b3a79b8f55d1737e07dcb9d2a378ec96d5 Mon Sep 17 00:00:00 2001 From: Snow Pettersen Date: Wed, 16 Dec 2020 15:26:10 +0000 Subject: [PATCH 20/32] test coverage Signed-off-by: Snow Pettersen --- test/common/http/match_wrapper/BUILD | 19 ++++ test/common/http/match_wrapper/config_test.cc | 88 +++++++++++++++++++ 2 files changed, 107 insertions(+) create mode 100644 test/common/http/match_wrapper/BUILD create mode 100644 test/common/http/match_wrapper/config_test.cc diff --git a/test/common/http/match_wrapper/BUILD b/test/common/http/match_wrapper/BUILD new file mode 100644 index 0000000000000..52425ab72baa7 --- /dev/null +++ b/test/common/http/match_wrapper/BUILD @@ -0,0 +1,19 @@ +load( + "//bazel:envoy_build_system.bzl", + "envoy_cc_test", + "envoy_package", +) + +licenses(["notice"]) # Apache 2 + +envoy_package() + +envoy_cc_test( + name = "config_test", + srcs = ["config_test.cc"], + deps = [ + "//source/common/http/match_wrapper:config", + "//test/mocks/server:factory_context_mocks", + "//test/test_common:registry_lib", + ], +) diff --git a/test/common/http/match_wrapper/config_test.cc b/test/common/http/match_wrapper/config_test.cc new file mode 100644 index 0000000000000..29d9a60bcb768 --- /dev/null +++ b/test/common/http/match_wrapper/config_test.cc @@ -0,0 +1,88 @@ +#include "envoy/http/filter.h" +#include "envoy/server/factory_context.h" +#include "envoy/server/filter_config.h" + +#include "common/http/match_wrapper/config.h" + +#include "test/mocks/server/factory_context.h" +#include "test/test_common/registry.h" + +#include "gtest/gtest.h" + +namespace Envoy { +namespace Common { +namespace Http { +namespace MatchWrapper { +namespace { + +struct TestFactory : public Envoy::Server::Configuration::NamedHttpFilterConfigFactory { + std::string name() const { return "test"; } + ProtobufTypes::MessagePtr createEmptyConfigProto() { + return std::make_unique(); + } + Envoy::Http::FilterFactoryCb + createFilterFactoryFromProto(const Protobuf::Message&, const std::string&, + Server::Configuration::FactoryContext&) { + return [](auto& callbacks) { + callbacks.addStreamDecoderFilter(nullptr); + callbacks.addStreamEncoderFilter(nullptr); + callbacks.addStreamFilter(nullptr); + + callbacks.addStreamDecoderFilter(nullptr, nullptr); + callbacks.addStreamEncoderFilter(nullptr, nullptr); + callbacks.addStreamFilter(nullptr, nullptr); + }; + } +}; + +TEST(MatchWrapper, WithMatcher) { + TestFactory test_factory; + Envoy::Registry::InjectFactory + inject_factory(test_factory); + + NiceMock factory_context; + + const auto config = + TestUtility::parseYaml(R"EOF( +extension_config: + name: test + typed_config: + "@type": type.googleapis.com/google.protobuf.StringValue +matcher: + matcher_tree: + input: + name: request-headers + typed_config: + "@type": type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpRequestHeaderMatchInput + header_name: default-matcher-header + exact_match_map: + map: + match: + action: + name: skip + typed_config: + "@type": type.googleapis.com/envoy.extensions.filters.common.matcher.action.v3.SkipFilter +)EOF"); + + MatchWrapperConfig match_wrapper_config; + auto cb = match_wrapper_config.createFilterFactoryFromProto(config, "", factory_context); + + Envoy::Http::MockFilterChainFactoryCallbacks factory_callbacks; + testing::InSequence s; + + // This matches the sequence of calls in the filter factory above: the ones that call the overload + // without a match tree has a match tree added, the other one does not. + EXPECT_CALL(factory_callbacks, addStreamDecoderFilter(_, testing::NotNull())); + EXPECT_CALL(factory_callbacks, addStreamEncoderFilter(_, testing::NotNull())); + EXPECT_CALL(factory_callbacks, addStreamFilter(_, testing::NotNull())); + EXPECT_CALL(factory_callbacks, addStreamDecoderFilter(_, testing::IsNull())); + EXPECT_CALL(factory_callbacks, addStreamEncoderFilter(_, testing::IsNull())); + EXPECT_CALL(factory_callbacks, addStreamFilter(_, testing::IsNull())); + cb(factory_callbacks); +} + +} // namespace +} // namespace MatchWrapper +} // namespace Http +} // namespace Common +} // namespace Envoy \ No newline at end of file From 711114b7a24c15d63d7caed33c2d3d8717daf229 Mon Sep 17 00:00:00 2001 From: Snow Pettersen Date: Thu, 17 Dec 2020 13:26:41 +0000 Subject: [PATCH 21/32] update comments Signed-off-by: Snow Pettersen --- source/common/http/match_wrapper/config.cc | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/source/common/http/match_wrapper/config.cc b/source/common/http/match_wrapper/config.cc index 679c091aadf06..6fbb4dab97730 100644 --- a/source/common/http/match_wrapper/config.cc +++ b/source/common/http/match_wrapper/config.cc @@ -82,9 +82,13 @@ Envoy::Http::FilterFactoryCb MatchWrapperConfig::createFilterFactoryFromProtoTyp } /** - * Static registration for the Lua filter. @see RegisterFactory. + * Static registration for the match wrapper filter. @see RegisterFactory. + * Note that we register this as a filter in order to serve as a drop in wrapper for other HTTP filters. + * While not a real filter, by being registered as one all the code paths that look up HTTP filters will + * look up this filter factory instead, which does the work to create and associate a match tree with the + * underlying filter. */ -REGISTER_FACTORY(MatchWrapperConfig, Server::Configuration::NamedHttpFilterConfigFactory){"blah"}; +REGISTER_FACTORY(MatchWrapperConfig, Server::Configuration::NamedHttpFilterConfigFactory); } // namespace MatchWrapper } // namespace Http From d8e5cde0571f4dbd189aef3acf383c8fcefc501e Mon Sep 17 00:00:00 2001 From: Snow Pettersen Date: Thu, 17 Dec 2020 13:45:07 +0000 Subject: [PATCH 22/32] use better helpers Signed-off-by: Snow Pettersen --- include/envoy/matcher/matcher.h | 3 ++- source/common/http/filter_manager.h | 21 +++++++++++++-------- source/common/http/match_wrapper/config.cc | 9 ++++----- source/common/matcher/matcher.h | 2 +- 4 files changed, 20 insertions(+), 15 deletions(-) diff --git a/include/envoy/matcher/matcher.h b/include/envoy/matcher/matcher.h index 613274ce389a2..1706a5dc10a71 100644 --- a/include/envoy/matcher/matcher.h +++ b/include/envoy/matcher/matcher.h @@ -10,6 +10,7 @@ #include "absl/strings/string_view.h" #include "absl/types/optional.h" +#include "envoy/protobuf/message_validator.h" namespace Envoy { namespace Matcher { @@ -209,7 +210,7 @@ template class DataInputFactory : public Config::TypedFactory { /** * Creates a DataInput from the provided config. */ - virtual DataInputPtr createDataInput(const Protobuf::Message& config) PURE; + virtual DataInputPtr createDataInput(const Protobuf::Message& config, ProtobufMessage::ValidationVisitor& validation_visitor) PURE; /** * The category of this factory depends on the DataType, so we require a name() function to exist diff --git a/source/common/http/filter_manager.h b/source/common/http/filter_manager.h index be521a99fc07e..4ad84a487c33b 100644 --- a/source/common/http/filter_manager.h +++ b/source/common/http/filter_manager.h @@ -1,7 +1,9 @@ #pragma once +#include "common/protobuf/utility.h" #include "envoy/extensions/filters/common/matcher/action/v3/skip_action.pb.h" #include "envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.pb.h" +#include "envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.pb.validate.h" #include "envoy/http/filter.h" #include "envoy/http/header_map.h" #include "envoy/matcher/matcher.h" @@ -15,6 +17,7 @@ #include "common/http/headers.h" #include "common/local_reply/local_reply.h" #include "common/matcher/matcher.h" +#include "envoy/protobuf/message_validator.h" namespace Envoy { namespace Http { @@ -106,10 +109,11 @@ class HttpRequestHeadersDataInputFactory : public Matcher::DataInputFactory - createDataInput(const Protobuf::Message& config) override { - const auto& typed_config = - dynamic_cast(config); + createDataInput(const Protobuf::Message& config, + ProtobufMessage::ValidationVisitor& validation_visitor) override { + const auto& typed_config = MessageUtil::downcastAndValidate< + const envoy::extensions::filters::network::http_connection_manager::v3:: + HttpRequestHeaderMatchInput&>(config, validation_visitor); return std::make_unique(typed_config.header_name()); }; @@ -133,10 +137,11 @@ class HttpResponseHeadersDataInputFactory : public Matcher::DataInputFactory - createDataInput(const Protobuf::Message& config) override { - const auto& typed_config = - dynamic_cast(config); + createDataInput(const Protobuf::Message& config, + ProtobufMessage::ValidationVisitor& validation_visitor) override { + const auto& typed_config = MessageUtil::downcastAndValidate< + const envoy::extensions::filters::network::http_connection_manager::v3:: + HttpResponseHeaderMatchInput&>(config, validation_visitor); return std::make_unique(typed_config.header_name()); }; diff --git a/source/common/http/match_wrapper/config.cc b/source/common/http/match_wrapper/config.cc index 6fbb4dab97730..5a06fb1e9eccc 100644 --- a/source/common/http/match_wrapper/config.cc +++ b/source/common/http/match_wrapper/config.cc @@ -5,6 +5,7 @@ #include "envoy/registry/registry.h" #include "common/matcher/matcher.h" +#include "common/config/utility.h" namespace Envoy { namespace Common { @@ -59,11 +60,9 @@ Envoy::Http::FilterFactoryCb MatchWrapperConfig::createFilterFactoryFromProtoTyp Config::Utility::getAndCheckFactory( proto_config.extension_config()); - auto message = factory.createEmptyConfigProto(); - proto_config.extension_config().typed_config(); - Config::Utility::translateOpaqueConfig(proto_config.extension_config().typed_config(), - ProtobufWkt::Struct(), - context.messageValidationVisitor(), *message); + auto message = + Config::Utility::translateAnyToFactoryConfig(proto_config.extension_config().typed_config(), + context.messageValidationVisitor(), factory); auto filter_factory = factory.createFilterFactoryFromProto(*message, prefix, context); auto match_tree = diff --git a/source/common/matcher/matcher.h b/source/common/matcher/matcher.h index 4e21e17ea463b..44ae0a26e787d 100644 --- a/source/common/matcher/matcher.h +++ b/source/common/matcher/matcher.h @@ -160,7 +160,7 @@ template class MatchTreeFactory { auto& factory = Config::Utility::getAndCheckFactory>(config); ProtobufTypes::MessagePtr message = Config::Utility::translateAnyToFactoryConfig( config.typed_config(), validation_visitor_, factory); - return factory.createDataInput(*message); + return factory.createDataInput(*message, validation_visitor_); } InputMatcherPtr createInputMatcher( From 465f9a47322e847acdfd5562715e493ea15f4622 Mon Sep 17 00:00:00 2001 From: Snow Pettersen Date: Thu, 17 Dec 2020 13:54:45 +0000 Subject: [PATCH 23/32] comments + format Signed-off-by: Snow Pettersen --- .../v3/http_connection_manager.proto | 16 +--------------- .../v4alpha/http_connection_manager.proto | 6 ++++++ .../v3/http_connection_manager.proto | 6 ++++++ .../v4alpha/http_connection_manager.proto | 6 ++++++ include/envoy/matcher/matcher.h | 6 ++++-- source/common/http/filter_manager.h | 4 ++-- source/common/http/match_wrapper/config.cc | 10 +++++----- 7 files changed, 30 insertions(+), 24 deletions(-) diff --git a/api/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto b/api/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto index 78bdebd2dbc05..92e14ac7a15aa 100644 --- a/api/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto +++ b/api/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto @@ -837,18 +837,4 @@ message RequestIDExtension { // Request ID extension specific configuration. google.protobuf.Any typed_config = 1; -} - -// Match input indicates that matching should be done on a specific request header. -// TODO(snowp): Link to unified matching docs. -message HttpRequestHeaderMatchInput { - // The request header to match on. - string header_name = 1; -} - -// Match input indicating that matching should be done on a specific response header. -// TODO(snowp): Link to unified matching docs. -message HttpResponseHeaderMatchInput { - // The response header to match on. - string header_name = 1; -} +} \ No newline at end of file diff --git a/api/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto b/api/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto index 5a1802f354397..7147831ab4e3c 100644 --- a/api/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto +++ b/api/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto @@ -846,6 +846,9 @@ message RequestIDExtension { } // Match input indicates that matching should be done on a specific request header. +// The resulting input string will be all headers for the given key joined by a comma, +// e.g. if the request contains two 'foo' headers with value 'bar' and 'baz', the input +// string will be 'bar,baz'. // TODO(snowp): Link to unified matching docs. message HttpRequestHeaderMatchInput { option (udpa.annotations.versioning).previous_message_type = @@ -856,6 +859,9 @@ message HttpRequestHeaderMatchInput { } // Match input indicating that matching should be done on a specific response header. +// The resulting input string will be all headers for the given key joined by a comma, +// e.g. if the response contains two 'foo' headers with value 'bar' and 'baz', the input +// string will be 'bar,baz'. // TODO(snowp): Link to unified matching docs. message HttpResponseHeaderMatchInput { option (udpa.annotations.versioning).previous_message_type = diff --git a/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto b/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto index aa77825226f6f..3f7c2f79b9d7e 100644 --- a/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto +++ b/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto @@ -845,6 +845,9 @@ message RequestIDExtension { } // Match input indicates that matching should be done on a specific request header. +// The resulting input string will be all headers for the given key joined by a comma, +// e.g. if the request contains two 'foo' headers with value 'bar' and 'baz', the input +// string will be 'bar,baz'. // TODO(snowp): Link to unified matching docs. message HttpRequestHeaderMatchInput { // The request header to match on. @@ -852,6 +855,9 @@ message HttpRequestHeaderMatchInput { } // Match input indicating that matching should be done on a specific response header. +// The resulting input string will be all headers for the given key joined by a comma, +// e.g. if the response contains two 'foo' headers with value 'bar' and 'baz', the input +// string will be 'bar,baz'. // TODO(snowp): Link to unified matching docs. message HttpResponseHeaderMatchInput { // The response header to match on. diff --git a/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto b/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto index 5a1802f354397..7147831ab4e3c 100644 --- a/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto +++ b/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto @@ -846,6 +846,9 @@ message RequestIDExtension { } // Match input indicates that matching should be done on a specific request header. +// The resulting input string will be all headers for the given key joined by a comma, +// e.g. if the request contains two 'foo' headers with value 'bar' and 'baz', the input +// string will be 'bar,baz'. // TODO(snowp): Link to unified matching docs. message HttpRequestHeaderMatchInput { option (udpa.annotations.versioning).previous_message_type = @@ -856,6 +859,9 @@ message HttpRequestHeaderMatchInput { } // Match input indicating that matching should be done on a specific response header. +// The resulting input string will be all headers for the given key joined by a comma, +// e.g. if the response contains two 'foo' headers with value 'bar' and 'baz', the input +// string will be 'bar,baz'. // TODO(snowp): Link to unified matching docs. message HttpResponseHeaderMatchInput { option (udpa.annotations.versioning).previous_message_type = diff --git a/include/envoy/matcher/matcher.h b/include/envoy/matcher/matcher.h index 1706a5dc10a71..678eed64023bc 100644 --- a/include/envoy/matcher/matcher.h +++ b/include/envoy/matcher/matcher.h @@ -7,10 +7,10 @@ #include "envoy/config/common/matcher/v3/matcher.pb.h" #include "envoy/config/core/v3/extension.pb.h" #include "envoy/config/typed_config.h" +#include "envoy/protobuf/message_validator.h" #include "absl/strings/string_view.h" #include "absl/types/optional.h" -#include "envoy/protobuf/message_validator.h" namespace Envoy { namespace Matcher { @@ -210,7 +210,9 @@ template class DataInputFactory : public Config::TypedFactory { /** * Creates a DataInput from the provided config. */ - virtual DataInputPtr createDataInput(const Protobuf::Message& config, ProtobufMessage::ValidationVisitor& validation_visitor) PURE; + virtual DataInputPtr + createDataInput(const Protobuf::Message& config, + ProtobufMessage::ValidationVisitor& validation_visitor) PURE; /** * The category of this factory depends on the DataType, so we require a name() function to exist diff --git a/source/common/http/filter_manager.h b/source/common/http/filter_manager.h index 4ad84a487c33b..29cfeb2453d2a 100644 --- a/source/common/http/filter_manager.h +++ b/source/common/http/filter_manager.h @@ -1,12 +1,12 @@ #pragma once -#include "common/protobuf/utility.h" #include "envoy/extensions/filters/common/matcher/action/v3/skip_action.pb.h" #include "envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.pb.h" #include "envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.pb.validate.h" #include "envoy/http/filter.h" #include "envoy/http/header_map.h" #include "envoy/matcher/matcher.h" +#include "envoy/protobuf/message_validator.h" #include "common/buffer/watermark_buffer.h" #include "common/common/dump_state_utils.h" @@ -17,7 +17,7 @@ #include "common/http/headers.h" #include "common/local_reply/local_reply.h" #include "common/matcher/matcher.h" -#include "envoy/protobuf/message_validator.h" +#include "common/protobuf/utility.h" namespace Envoy { namespace Http { diff --git a/source/common/http/match_wrapper/config.cc b/source/common/http/match_wrapper/config.cc index 5a06fb1e9eccc..695793f6257d5 100644 --- a/source/common/http/match_wrapper/config.cc +++ b/source/common/http/match_wrapper/config.cc @@ -4,8 +4,8 @@ #include "envoy/matcher/matcher.h" #include "envoy/registry/registry.h" -#include "common/matcher/matcher.h" #include "common/config/utility.h" +#include "common/matcher/matcher.h" namespace Envoy { namespace Common { @@ -82,10 +82,10 @@ Envoy::Http::FilterFactoryCb MatchWrapperConfig::createFilterFactoryFromProtoTyp /** * Static registration for the match wrapper filter. @see RegisterFactory. - * Note that we register this as a filter in order to serve as a drop in wrapper for other HTTP filters. - * While not a real filter, by being registered as one all the code paths that look up HTTP filters will - * look up this filter factory instead, which does the work to create and associate a match tree with the - * underlying filter. + * Note that we register this as a filter in order to serve as a drop in wrapper for other HTTP + * filters. While not a real filter, by being registered as one all the code paths that look up HTTP + * filters will look up this filter factory instead, which does the work to create and associate a + * match tree with the underlying filter. */ REGISTER_FACTORY(MatchWrapperConfig, Server::Configuration::NamedHttpFilterConfigFactory); From e88be1db6f4c3ef5efcf30f74f9addb81bca87f3 Mon Sep 17 00:00:00 2001 From: Snow Pettersen Date: Thu, 17 Dec 2020 14:53:03 +0000 Subject: [PATCH 24/32] move protos out of hcm config Signed-off-by: Snow Pettersen --- api/BUILD | 2 ++ .../common/matcher/input/http/v3/BUILD | 9 +++++ .../matcher/input/http/v3/http_inputs.proto | 35 +++++++++++++++++++ .../v3/http_connection_manager.proto | 2 +- .../v4alpha/http_connection_manager.proto | 26 -------------- api/versioning/BUILD | 2 ++ .../common/matcher/input/http/v3/BUILD | 9 +++++ .../matcher/input/http/v3/http_inputs.proto | 35 +++++++++++++++++++ .../v3/http_connection_manager.proto | 20 ----------- .../v4alpha/http_connection_manager.proto | 26 -------------- source/common/http/BUILD | 1 + source/common/http/filter_manager.h | 19 +++++----- source/common/http/match_wrapper/config.cc | 33 ++++++++--------- test/common/http/match_wrapper/config_test.cc | 5 ++- .../extension_discovery_integration_test.cc | 4 +-- test/integration/integration_test.cc | 2 +- 16 files changed, 126 insertions(+), 104 deletions(-) create mode 100644 api/envoy/extensions/filters/common/matcher/input/http/v3/BUILD create mode 100644 api/envoy/extensions/filters/common/matcher/input/http/v3/http_inputs.proto create mode 100644 generated_api_shadow/envoy/extensions/filters/common/matcher/input/http/v3/BUILD create mode 100644 generated_api_shadow/envoy/extensions/filters/common/matcher/input/http/v3/http_inputs.proto diff --git a/api/BUILD b/api/BUILD index f7c75a2d7faea..50353fed93cb5 100644 --- a/api/BUILD +++ b/api/BUILD @@ -167,6 +167,8 @@ proto_library( "//envoy/extensions/compression/gzip/decompressor/v3:pkg", "//envoy/extensions/filters/common/fault/v3:pkg", "//envoy/extensions/filters/common/matcher/action/v3:pkg", + "//envoy/extensions/filters/common/matcher/http/v3:pkg", + "//envoy/extensions/filters/common/matcher/input/http/v3:pkg", "//envoy/extensions/filters/http/adaptive_concurrency/v3:pkg", "//envoy/extensions/filters/http/admission_control/v3alpha:pkg", "//envoy/extensions/filters/http/aws_lambda/v3:pkg", diff --git a/api/envoy/extensions/filters/common/matcher/input/http/v3/BUILD b/api/envoy/extensions/filters/common/matcher/input/http/v3/BUILD new file mode 100644 index 0000000000000..ee92fb652582e --- /dev/null +++ b/api/envoy/extensions/filters/common/matcher/input/http/v3/BUILD @@ -0,0 +1,9 @@ +# DO NOT EDIT. This file is generated by tools/proto_format/proto_sync.py. + +load("@envoy_api//bazel:api_build_system.bzl", "api_proto_package") + +licenses(["notice"]) # Apache 2 + +api_proto_package( + deps = ["@com_github_cncf_udpa//udpa/annotations:pkg"], +) diff --git a/api/envoy/extensions/filters/common/matcher/input/http/v3/http_inputs.proto b/api/envoy/extensions/filters/common/matcher/input/http/v3/http_inputs.proto new file mode 100644 index 0000000000000..a9ed81cc95e37 --- /dev/null +++ b/api/envoy/extensions/filters/common/matcher/input/http/v3/http_inputs.proto @@ -0,0 +1,35 @@ +syntax = "proto3"; + +package envoy.extensions.filters.common.matcher.input.http.v3; + +import "udpa/annotations/migrate.proto"; +import "udpa/annotations/status.proto"; +import "udpa/annotations/versioning.proto"; +import "validate/validate.proto"; + +option java_package = "io.envoyproxy.envoy.extensions.filters.common.matcher.input.http.v3"; +option java_outer_classname = "HttpInputsProto"; +option java_multiple_files = true; +option (udpa.annotations.file_status).package_version_status = ACTIVE; + +// [#protodoc-title: Common HTTP Inputs] + +// Match input indicates that matching should be done on a specific request header. +// The resulting input string will be all headers for the given key joined by a comma, +// e.g. if the request contains two 'foo' headers with value 'bar' and 'baz', the input +// string will be 'bar,baz'. +// TODO(snowp): Link to unified matching docs. +message HttpRequestHeaderMatchInput { + // The request header to match on. + string header_name = 1; +} + +// Match input indicating that matching should be done on a specific response header. +// The resulting input string will be all headers for the given key joined by a comma, +// e.g. if the response contains two 'foo' headers with value 'bar' and 'baz', the input +// string will be 'bar,baz'. +// TODO(snowp): Link to unified matching docs. +message HttpResponseHeaderMatchInput { + // The response header to match on. + string header_name = 1; +} diff --git a/api/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto b/api/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto index 92e14ac7a15aa..5878beffa5f92 100644 --- a/api/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto +++ b/api/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto @@ -837,4 +837,4 @@ message RequestIDExtension { // Request ID extension specific configuration. google.protobuf.Any typed_config = 1; -} \ No newline at end of file +} diff --git a/api/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto b/api/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto index 7147831ab4e3c..51fdb6bf3dbf6 100644 --- a/api/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto +++ b/api/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto @@ -844,29 +844,3 @@ message RequestIDExtension { // Request ID extension specific configuration. google.protobuf.Any typed_config = 1; } - -// Match input indicates that matching should be done on a specific request header. -// The resulting input string will be all headers for the given key joined by a comma, -// e.g. if the request contains two 'foo' headers with value 'bar' and 'baz', the input -// string will be 'bar,baz'. -// TODO(snowp): Link to unified matching docs. -message HttpRequestHeaderMatchInput { - option (udpa.annotations.versioning).previous_message_type = - "envoy.extensions.filters.network.http_connection_manager.v3.HttpRequestHeaderMatchInput"; - - // The request header to match on. - string header_name = 1; -} - -// Match input indicating that matching should be done on a specific response header. -// The resulting input string will be all headers for the given key joined by a comma, -// e.g. if the response contains two 'foo' headers with value 'bar' and 'baz', the input -// string will be 'bar,baz'. -// TODO(snowp): Link to unified matching docs. -message HttpResponseHeaderMatchInput { - option (udpa.annotations.versioning).previous_message_type = - "envoy.extensions.filters.network.http_connection_manager.v3.HttpResponseHeaderMatchInput"; - - // The response header to match on. - string header_name = 1; -} diff --git a/api/versioning/BUILD b/api/versioning/BUILD index 41addf767671d..9a7e5afb42c3e 100644 --- a/api/versioning/BUILD +++ b/api/versioning/BUILD @@ -50,6 +50,8 @@ proto_library( "//envoy/extensions/compression/gzip/decompressor/v3:pkg", "//envoy/extensions/filters/common/fault/v3:pkg", "//envoy/extensions/filters/common/matcher/action/v3:pkg", + "//envoy/extensions/filters/common/matcher/http/v3:pkg", + "//envoy/extensions/filters/common/matcher/input/http/v3:pkg", "//envoy/extensions/filters/http/adaptive_concurrency/v3:pkg", "//envoy/extensions/filters/http/admission_control/v3alpha:pkg", "//envoy/extensions/filters/http/aws_lambda/v3:pkg", diff --git a/generated_api_shadow/envoy/extensions/filters/common/matcher/input/http/v3/BUILD b/generated_api_shadow/envoy/extensions/filters/common/matcher/input/http/v3/BUILD new file mode 100644 index 0000000000000..ee92fb652582e --- /dev/null +++ b/generated_api_shadow/envoy/extensions/filters/common/matcher/input/http/v3/BUILD @@ -0,0 +1,9 @@ +# DO NOT EDIT. This file is generated by tools/proto_format/proto_sync.py. + +load("@envoy_api//bazel:api_build_system.bzl", "api_proto_package") + +licenses(["notice"]) # Apache 2 + +api_proto_package( + deps = ["@com_github_cncf_udpa//udpa/annotations:pkg"], +) diff --git a/generated_api_shadow/envoy/extensions/filters/common/matcher/input/http/v3/http_inputs.proto b/generated_api_shadow/envoy/extensions/filters/common/matcher/input/http/v3/http_inputs.proto new file mode 100644 index 0000000000000..a9ed81cc95e37 --- /dev/null +++ b/generated_api_shadow/envoy/extensions/filters/common/matcher/input/http/v3/http_inputs.proto @@ -0,0 +1,35 @@ +syntax = "proto3"; + +package envoy.extensions.filters.common.matcher.input.http.v3; + +import "udpa/annotations/migrate.proto"; +import "udpa/annotations/status.proto"; +import "udpa/annotations/versioning.proto"; +import "validate/validate.proto"; + +option java_package = "io.envoyproxy.envoy.extensions.filters.common.matcher.input.http.v3"; +option java_outer_classname = "HttpInputsProto"; +option java_multiple_files = true; +option (udpa.annotations.file_status).package_version_status = ACTIVE; + +// [#protodoc-title: Common HTTP Inputs] + +// Match input indicates that matching should be done on a specific request header. +// The resulting input string will be all headers for the given key joined by a comma, +// e.g. if the request contains two 'foo' headers with value 'bar' and 'baz', the input +// string will be 'bar,baz'. +// TODO(snowp): Link to unified matching docs. +message HttpRequestHeaderMatchInput { + // The request header to match on. + string header_name = 1; +} + +// Match input indicating that matching should be done on a specific response header. +// The resulting input string will be all headers for the given key joined by a comma, +// e.g. if the response contains two 'foo' headers with value 'bar' and 'baz', the input +// string will be 'bar,baz'. +// TODO(snowp): Link to unified matching docs. +message HttpResponseHeaderMatchInput { + // The response header to match on. + string header_name = 1; +} diff --git a/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto b/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto index 3f7c2f79b9d7e..1cdb60df3ccb8 100644 --- a/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto +++ b/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto @@ -843,23 +843,3 @@ message RequestIDExtension { // Request ID extension specific configuration. google.protobuf.Any typed_config = 1; } - -// Match input indicates that matching should be done on a specific request header. -// The resulting input string will be all headers for the given key joined by a comma, -// e.g. if the request contains two 'foo' headers with value 'bar' and 'baz', the input -// string will be 'bar,baz'. -// TODO(snowp): Link to unified matching docs. -message HttpRequestHeaderMatchInput { - // The request header to match on. - string header_name = 1; -} - -// Match input indicating that matching should be done on a specific response header. -// The resulting input string will be all headers for the given key joined by a comma, -// e.g. if the response contains two 'foo' headers with value 'bar' and 'baz', the input -// string will be 'bar,baz'. -// TODO(snowp): Link to unified matching docs. -message HttpResponseHeaderMatchInput { - // The response header to match on. - string header_name = 1; -} diff --git a/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto b/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto index 7147831ab4e3c..51fdb6bf3dbf6 100644 --- a/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto +++ b/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto @@ -844,29 +844,3 @@ message RequestIDExtension { // Request ID extension specific configuration. google.protobuf.Any typed_config = 1; } - -// Match input indicates that matching should be done on a specific request header. -// The resulting input string will be all headers for the given key joined by a comma, -// e.g. if the request contains two 'foo' headers with value 'bar' and 'baz', the input -// string will be 'bar,baz'. -// TODO(snowp): Link to unified matching docs. -message HttpRequestHeaderMatchInput { - option (udpa.annotations.versioning).previous_message_type = - "envoy.extensions.filters.network.http_connection_manager.v3.HttpRequestHeaderMatchInput"; - - // The request header to match on. - string header_name = 1; -} - -// Match input indicating that matching should be done on a specific response header. -// The resulting input string will be all headers for the given key joined by a comma, -// e.g. if the response contains two 'foo' headers with value 'bar' and 'baz', the input -// string will be 'bar,baz'. -// TODO(snowp): Link to unified matching docs. -message HttpResponseHeaderMatchInput { - option (udpa.annotations.versioning).previous_message_type = - "envoy.extensions.filters.network.http_connection_manager.v3.HttpResponseHeaderMatchInput"; - - // The response header to match on. - string header_name = 1; -} diff --git a/source/common/http/BUILD b/source/common/http/BUILD index 116cf0911f9f2..8b9e476af5e8d 100644 --- a/source/common/http/BUILD +++ b/source/common/http/BUILD @@ -177,6 +177,7 @@ envoy_cc_library( "//source/common/local_reply:local_reply_lib", "//source/common/matcher:matcher_lib", "@envoy_api//envoy/extensions/filters/common/matcher/action/v3:pkg_cc_proto", + "@envoy_api//envoy/extensions/filters/common/matcher/input/http/v3:pkg_cc_proto", "@envoy_api//envoy/extensions/filters/network/http_connection_manager/v3:pkg_cc_proto", ], ) diff --git a/source/common/http/filter_manager.h b/source/common/http/filter_manager.h index 29cfeb2453d2a..06594ba994ee0 100644 --- a/source/common/http/filter_manager.h +++ b/source/common/http/filter_manager.h @@ -1,6 +1,7 @@ #pragma once #include "envoy/extensions/filters/common/matcher/action/v3/skip_action.pb.h" +#include "envoy/extensions/filters/common/matcher/input/http/v3/http_inputs.pb.validate.h" #include "envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.pb.h" #include "envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.pb.validate.h" #include "envoy/http/filter.h" @@ -111,14 +112,15 @@ class HttpRequestHeadersDataInputFactory : public Matcher::DataInputFactory createDataInput(const Protobuf::Message& config, ProtobufMessage::ValidationVisitor& validation_visitor) override { - const auto& typed_config = MessageUtil::downcastAndValidate< - const envoy::extensions::filters::network::http_connection_manager::v3:: - HttpRequestHeaderMatchInput&>(config, validation_visitor); + const auto& typed_config = + MessageUtil::downcastAndValidate( + config, validation_visitor); return std::make_unique(typed_config.header_name()); }; ProtobufTypes::MessagePtr createEmptyConfigProto() override { - return std::make_unique(); } }; @@ -139,14 +141,15 @@ class HttpResponseHeadersDataInputFactory : public Matcher::DataInputFactory createDataInput(const Protobuf::Message& config, ProtobufMessage::ValidationVisitor& validation_visitor) override { - const auto& typed_config = MessageUtil::downcastAndValidate< - const envoy::extensions::filters::network::http_connection_manager::v3:: - HttpResponseHeaderMatchInput&>(config, validation_visitor); + const auto& typed_config = + MessageUtil::downcastAndValidate( + config, validation_visitor); return std::make_unique(typed_config.header_name()); }; ProtobufTypes::MessagePtr createEmptyConfigProto() override { - return std::make_unique(); } }; diff --git a/source/common/http/match_wrapper/config.cc b/source/common/http/match_wrapper/config.cc index 695793f6257d5..7d5b75f9f47ad 100644 --- a/source/common/http/match_wrapper/config.cc +++ b/source/common/http/match_wrapper/config.cc @@ -55,29 +55,24 @@ Envoy::Http::FilterFactoryCb MatchWrapperConfig::createFilterFactoryFromProtoTyp const envoy::extensions::common::matching::v3::ExtensionWithMatcher& proto_config, const std::string& prefix, Server::Configuration::FactoryContext& context) { - if (proto_config.has_extension_config()) { - auto& factory = - Config::Utility::getAndCheckFactory( - proto_config.extension_config()); + ASSERT(proto_config.has_extension_config()); + auto& factory = + Config::Utility::getAndCheckFactory( + proto_config.extension_config()); - auto message = - Config::Utility::translateAnyToFactoryConfig(proto_config.extension_config().typed_config(), - context.messageValidationVisitor(), factory); - auto filter_factory = factory.createFilterFactoryFromProto(*message, prefix, context); + auto message = Config::Utility::translateAnyToFactoryConfig( + proto_config.extension_config().typed_config(), context.messageValidationVisitor(), factory); + auto filter_factory = factory.createFilterFactoryFromProto(*message, prefix, context); - auto match_tree = - Matcher::MatchTreeFactory(context.messageValidationVisitor()) - .create(proto_config.matcher()); + auto match_tree = + Matcher::MatchTreeFactory(context.messageValidationVisitor()) + .create(proto_config.matcher()); - return - [filter_factory, match_tree](Envoy::Http::FilterChainFactoryCallbacks& callbacks) -> void { - DelegatingFactoryCallbacks delegated_callbacks(callbacks, match_tree); + return [filter_factory, match_tree](Envoy::Http::FilterChainFactoryCallbacks& callbacks) -> void { + DelegatingFactoryCallbacks delegated_callbacks(callbacks, match_tree); - return filter_factory(delegated_callbacks); - }; - } - - return [](Envoy::Http::FilterChainFactoryCallbacks&) -> void {}; + return filter_factory(delegated_callbacks); + }; } /** diff --git a/test/common/http/match_wrapper/config_test.cc b/test/common/http/match_wrapper/config_test.cc index 29d9a60bcb768..0e82f1d6088e0 100644 --- a/test/common/http/match_wrapper/config_test.cc +++ b/test/common/http/match_wrapper/config_test.cc @@ -31,6 +31,8 @@ struct TestFactory : public Envoy::Server::Configuration::NamedHttpFilterConfigF callbacks.addStreamDecoderFilter(nullptr, nullptr); callbacks.addStreamEncoderFilter(nullptr, nullptr); callbacks.addStreamFilter(nullptr, nullptr); + + callbacks.addAccessLogHandler(nullptr); }; } }; @@ -53,7 +55,7 @@ TEST(MatchWrapper, WithMatcher) { input: name: request-headers typed_config: - "@type": type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpRequestHeaderMatchInput + "@type": type.googleapis.com/envoy.extensions.filters.common.matcher.input.http.v3.HttpRequestHeaderMatchInput header_name: default-matcher-header exact_match_map: map: @@ -78,6 +80,7 @@ TEST(MatchWrapper, WithMatcher) { EXPECT_CALL(factory_callbacks, addStreamDecoderFilter(_, testing::IsNull())); EXPECT_CALL(factory_callbacks, addStreamEncoderFilter(_, testing::IsNull())); EXPECT_CALL(factory_callbacks, addStreamFilter(_, testing::IsNull())); + EXPECT_CALL(factory_callbacks, addAccessLogHandler(_)); cb(factory_callbacks); } diff --git a/test/integration/extension_discovery_integration_test.cc b/test/integration/extension_discovery_integration_test.cc index 64ce24fa8a1c0..762d5f862e1b3 100644 --- a/test/integration/extension_discovery_integration_test.cc +++ b/test/integration/extension_discovery_integration_test.cc @@ -33,7 +33,7 @@ std::string denyPrivateConfigWithMatcher() { input: name: request-headers typed_config: - "@type": type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpRequestHeaderMatchInput + "@type": type.googleapis.com/envoy.extensions.filters.common.matcher.input.http.v3.HttpRequestHeaderMatchInput header_name: some-header exact_match_map: map: @@ -84,7 +84,7 @@ class ExtensionDiscoveryIntegrationTest : public Grpc::GrpcClientIntegrationPara input: name: request-headers typed_config: - "@type": type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpRequestHeaderMatchInput + "@type": type.googleapis.com/envoy.extensions.filters.common.matcher.input.http.v3.HttpRequestHeaderMatchInput header_name: default-matcher-header exact_match_map: map: diff --git a/test/integration/integration_test.cc b/test/integration/integration_test.cc index bbc84394e6b8d..fa8c4ea666496 100644 --- a/test/integration/integration_test.cc +++ b/test/integration/integration_test.cc @@ -380,7 +380,7 @@ name: matcher input: name: request-headers typed_config: - "@type": type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpRequestHeaderMatchInput + "@type": type.googleapis.com/envoy.extensions.filters.common.matcher.input.http.v3.HttpRequestHeaderMatchInput header_name: match-header exact_match_map: map: From 487822f27a15d0bcb465719bbe150b6c9d2d84a7 Mon Sep 17 00:00:00 2001 From: Snow Pettersen Date: Fri, 18 Dec 2020 00:23:30 +0000 Subject: [PATCH 25/32] move to envoy/type/matcher Signed-off-by: Snow Pettersen --- api/BUILD | 2 - .../common/matcher/input/http/v3/BUILD | 9 ----- .../matcher}/v3/http_inputs.proto | 8 ++-- .../type/matcher/v4alpha/http_inputs.proto | 40 +++++++++++++++++++ api/versioning/BUILD | 2 - .../common/matcher/input/http/v3/BUILD | 9 ----- .../matcher}/v3/http_inputs.proto | 8 ++-- .../type/matcher/v4alpha/http_inputs.proto | 40 +++++++++++++++++++ source/common/http/BUILD | 2 +- source/common/http/filter_manager.h | 20 ++++------ 10 files changed, 96 insertions(+), 44 deletions(-) delete mode 100644 api/envoy/extensions/filters/common/matcher/input/http/v3/BUILD rename api/envoy/{extensions/filters/common/matcher/input/http => type/matcher}/v3/http_inputs.proto (82%) create mode 100644 api/envoy/type/matcher/v4alpha/http_inputs.proto delete mode 100644 generated_api_shadow/envoy/extensions/filters/common/matcher/input/http/v3/BUILD rename generated_api_shadow/envoy/{extensions/filters/common/matcher/input/http => type/matcher}/v3/http_inputs.proto (82%) create mode 100644 generated_api_shadow/envoy/type/matcher/v4alpha/http_inputs.proto diff --git a/api/BUILD b/api/BUILD index 50353fed93cb5..f7c75a2d7faea 100644 --- a/api/BUILD +++ b/api/BUILD @@ -167,8 +167,6 @@ proto_library( "//envoy/extensions/compression/gzip/decompressor/v3:pkg", "//envoy/extensions/filters/common/fault/v3:pkg", "//envoy/extensions/filters/common/matcher/action/v3:pkg", - "//envoy/extensions/filters/common/matcher/http/v3:pkg", - "//envoy/extensions/filters/common/matcher/input/http/v3:pkg", "//envoy/extensions/filters/http/adaptive_concurrency/v3:pkg", "//envoy/extensions/filters/http/admission_control/v3alpha:pkg", "//envoy/extensions/filters/http/aws_lambda/v3:pkg", diff --git a/api/envoy/extensions/filters/common/matcher/input/http/v3/BUILD b/api/envoy/extensions/filters/common/matcher/input/http/v3/BUILD deleted file mode 100644 index ee92fb652582e..0000000000000 --- a/api/envoy/extensions/filters/common/matcher/input/http/v3/BUILD +++ /dev/null @@ -1,9 +0,0 @@ -# DO NOT EDIT. This file is generated by tools/proto_format/proto_sync.py. - -load("@envoy_api//bazel:api_build_system.bzl", "api_proto_package") - -licenses(["notice"]) # Apache 2 - -api_proto_package( - deps = ["@com_github_cncf_udpa//udpa/annotations:pkg"], -) diff --git a/api/envoy/extensions/filters/common/matcher/input/http/v3/http_inputs.proto b/api/envoy/type/matcher/v3/http_inputs.proto similarity index 82% rename from api/envoy/extensions/filters/common/matcher/input/http/v3/http_inputs.proto rename to api/envoy/type/matcher/v3/http_inputs.proto index a9ed81cc95e37..48eb4c43154a8 100644 --- a/api/envoy/extensions/filters/common/matcher/input/http/v3/http_inputs.proto +++ b/api/envoy/type/matcher/v3/http_inputs.proto @@ -1,13 +1,13 @@ syntax = "proto3"; -package envoy.extensions.filters.common.matcher.input.http.v3; +package envoy.type.matcher.v3; import "udpa/annotations/migrate.proto"; import "udpa/annotations/status.proto"; import "udpa/annotations/versioning.proto"; import "validate/validate.proto"; -option java_package = "io.envoyproxy.envoy.extensions.filters.common.matcher.input.http.v3"; +option java_package = "io.envoyproxy.envoy.type.matcher.v3"; option java_outer_classname = "HttpInputsProto"; option java_multiple_files = true; option (udpa.annotations.file_status).package_version_status = ACTIVE; @@ -18,7 +18,7 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE; // The resulting input string will be all headers for the given key joined by a comma, // e.g. if the request contains two 'foo' headers with value 'bar' and 'baz', the input // string will be 'bar,baz'. -// TODO(snowp): Link to unified matching docs. +// [#comment:TODO(snowp): Link to unified matching docs.] message HttpRequestHeaderMatchInput { // The request header to match on. string header_name = 1; @@ -28,7 +28,7 @@ message HttpRequestHeaderMatchInput { // The resulting input string will be all headers for the given key joined by a comma, // e.g. if the response contains two 'foo' headers with value 'bar' and 'baz', the input // string will be 'bar,baz'. -// TODO(snowp): Link to unified matching docs. +// [#comment:TODO(snowp): Link to unified matching docs.] message HttpResponseHeaderMatchInput { // The response header to match on. string header_name = 1; diff --git a/api/envoy/type/matcher/v4alpha/http_inputs.proto b/api/envoy/type/matcher/v4alpha/http_inputs.proto new file mode 100644 index 0000000000000..f15e9ceb70e2f --- /dev/null +++ b/api/envoy/type/matcher/v4alpha/http_inputs.proto @@ -0,0 +1,40 @@ +syntax = "proto3"; + +package envoy.type.matcher.v4alpha; + +import "udpa/annotations/status.proto"; +import "udpa/annotations/versioning.proto"; +import "validate/validate.proto"; + +option java_package = "io.envoyproxy.envoy.type.matcher.v4alpha"; +option java_outer_classname = "HttpInputsProto"; +option java_multiple_files = true; +option (udpa.annotations.file_status).package_version_status = NEXT_MAJOR_VERSION_CANDIDATE; + +// [#protodoc-title: Common HTTP Inputs] + +// Match input indicates that matching should be done on a specific request header. +// The resulting input string will be all headers for the given key joined by a comma, +// e.g. if the request contains two 'foo' headers with value 'bar' and 'baz', the input +// string will be 'bar,baz'. +// [#comment:TODO(snowp): Link to unified matching docs.] +message HttpRequestHeaderMatchInput { + option (udpa.annotations.versioning).previous_message_type = + "envoy.type.matcher.v3.HttpRequestHeaderMatchInput"; + + // The request header to match on. + string header_name = 1; +} + +// Match input indicating that matching should be done on a specific response header. +// The resulting input string will be all headers for the given key joined by a comma, +// e.g. if the response contains two 'foo' headers with value 'bar' and 'baz', the input +// string will be 'bar,baz'. +// [#comment:TODO(snowp): Link to unified matching docs.] +message HttpResponseHeaderMatchInput { + option (udpa.annotations.versioning).previous_message_type = + "envoy.type.matcher.v3.HttpResponseHeaderMatchInput"; + + // The response header to match on. + string header_name = 1; +} diff --git a/api/versioning/BUILD b/api/versioning/BUILD index 9a7e5afb42c3e..41addf767671d 100644 --- a/api/versioning/BUILD +++ b/api/versioning/BUILD @@ -50,8 +50,6 @@ proto_library( "//envoy/extensions/compression/gzip/decompressor/v3:pkg", "//envoy/extensions/filters/common/fault/v3:pkg", "//envoy/extensions/filters/common/matcher/action/v3:pkg", - "//envoy/extensions/filters/common/matcher/http/v3:pkg", - "//envoy/extensions/filters/common/matcher/input/http/v3:pkg", "//envoy/extensions/filters/http/adaptive_concurrency/v3:pkg", "//envoy/extensions/filters/http/admission_control/v3alpha:pkg", "//envoy/extensions/filters/http/aws_lambda/v3:pkg", diff --git a/generated_api_shadow/envoy/extensions/filters/common/matcher/input/http/v3/BUILD b/generated_api_shadow/envoy/extensions/filters/common/matcher/input/http/v3/BUILD deleted file mode 100644 index ee92fb652582e..0000000000000 --- a/generated_api_shadow/envoy/extensions/filters/common/matcher/input/http/v3/BUILD +++ /dev/null @@ -1,9 +0,0 @@ -# DO NOT EDIT. This file is generated by tools/proto_format/proto_sync.py. - -load("@envoy_api//bazel:api_build_system.bzl", "api_proto_package") - -licenses(["notice"]) # Apache 2 - -api_proto_package( - deps = ["@com_github_cncf_udpa//udpa/annotations:pkg"], -) diff --git a/generated_api_shadow/envoy/extensions/filters/common/matcher/input/http/v3/http_inputs.proto b/generated_api_shadow/envoy/type/matcher/v3/http_inputs.proto similarity index 82% rename from generated_api_shadow/envoy/extensions/filters/common/matcher/input/http/v3/http_inputs.proto rename to generated_api_shadow/envoy/type/matcher/v3/http_inputs.proto index a9ed81cc95e37..48eb4c43154a8 100644 --- a/generated_api_shadow/envoy/extensions/filters/common/matcher/input/http/v3/http_inputs.proto +++ b/generated_api_shadow/envoy/type/matcher/v3/http_inputs.proto @@ -1,13 +1,13 @@ syntax = "proto3"; -package envoy.extensions.filters.common.matcher.input.http.v3; +package envoy.type.matcher.v3; import "udpa/annotations/migrate.proto"; import "udpa/annotations/status.proto"; import "udpa/annotations/versioning.proto"; import "validate/validate.proto"; -option java_package = "io.envoyproxy.envoy.extensions.filters.common.matcher.input.http.v3"; +option java_package = "io.envoyproxy.envoy.type.matcher.v3"; option java_outer_classname = "HttpInputsProto"; option java_multiple_files = true; option (udpa.annotations.file_status).package_version_status = ACTIVE; @@ -18,7 +18,7 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE; // The resulting input string will be all headers for the given key joined by a comma, // e.g. if the request contains two 'foo' headers with value 'bar' and 'baz', the input // string will be 'bar,baz'. -// TODO(snowp): Link to unified matching docs. +// [#comment:TODO(snowp): Link to unified matching docs.] message HttpRequestHeaderMatchInput { // The request header to match on. string header_name = 1; @@ -28,7 +28,7 @@ message HttpRequestHeaderMatchInput { // The resulting input string will be all headers for the given key joined by a comma, // e.g. if the response contains two 'foo' headers with value 'bar' and 'baz', the input // string will be 'bar,baz'. -// TODO(snowp): Link to unified matching docs. +// [#comment:TODO(snowp): Link to unified matching docs.] message HttpResponseHeaderMatchInput { // The response header to match on. string header_name = 1; diff --git a/generated_api_shadow/envoy/type/matcher/v4alpha/http_inputs.proto b/generated_api_shadow/envoy/type/matcher/v4alpha/http_inputs.proto new file mode 100644 index 0000000000000..f15e9ceb70e2f --- /dev/null +++ b/generated_api_shadow/envoy/type/matcher/v4alpha/http_inputs.proto @@ -0,0 +1,40 @@ +syntax = "proto3"; + +package envoy.type.matcher.v4alpha; + +import "udpa/annotations/status.proto"; +import "udpa/annotations/versioning.proto"; +import "validate/validate.proto"; + +option java_package = "io.envoyproxy.envoy.type.matcher.v4alpha"; +option java_outer_classname = "HttpInputsProto"; +option java_multiple_files = true; +option (udpa.annotations.file_status).package_version_status = NEXT_MAJOR_VERSION_CANDIDATE; + +// [#protodoc-title: Common HTTP Inputs] + +// Match input indicates that matching should be done on a specific request header. +// The resulting input string will be all headers for the given key joined by a comma, +// e.g. if the request contains two 'foo' headers with value 'bar' and 'baz', the input +// string will be 'bar,baz'. +// [#comment:TODO(snowp): Link to unified matching docs.] +message HttpRequestHeaderMatchInput { + option (udpa.annotations.versioning).previous_message_type = + "envoy.type.matcher.v3.HttpRequestHeaderMatchInput"; + + // The request header to match on. + string header_name = 1; +} + +// Match input indicating that matching should be done on a specific response header. +// The resulting input string will be all headers for the given key joined by a comma, +// e.g. if the response contains two 'foo' headers with value 'bar' and 'baz', the input +// string will be 'bar,baz'. +// [#comment:TODO(snowp): Link to unified matching docs.] +message HttpResponseHeaderMatchInput { + option (udpa.annotations.versioning).previous_message_type = + "envoy.type.matcher.v3.HttpResponseHeaderMatchInput"; + + // The response header to match on. + string header_name = 1; +} diff --git a/source/common/http/BUILD b/source/common/http/BUILD index 8b9e476af5e8d..4eed92f6dd09c 100644 --- a/source/common/http/BUILD +++ b/source/common/http/BUILD @@ -177,7 +177,7 @@ envoy_cc_library( "//source/common/local_reply:local_reply_lib", "//source/common/matcher:matcher_lib", "@envoy_api//envoy/extensions/filters/common/matcher/action/v3:pkg_cc_proto", - "@envoy_api//envoy/extensions/filters/common/matcher/input/http/v3:pkg_cc_proto", + "@envoy_api//envoy/type/matcher/v3:pkg_cc_proto", "@envoy_api//envoy/extensions/filters/network/http_connection_manager/v3:pkg_cc_proto", ], ) diff --git a/source/common/http/filter_manager.h b/source/common/http/filter_manager.h index 06594ba994ee0..7bc0fc6f8701d 100644 --- a/source/common/http/filter_manager.h +++ b/source/common/http/filter_manager.h @@ -1,7 +1,7 @@ #pragma once #include "envoy/extensions/filters/common/matcher/action/v3/skip_action.pb.h" -#include "envoy/extensions/filters/common/matcher/input/http/v3/http_inputs.pb.validate.h" +#include "envoy/type/matcher/v3/http_inputs.pb.validate.h" #include "envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.pb.h" #include "envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.pb.validate.h" #include "envoy/http/filter.h" @@ -112,16 +112,13 @@ class HttpRequestHeadersDataInputFactory : public Matcher::DataInputFactory createDataInput(const Protobuf::Message& config, ProtobufMessage::ValidationVisitor& validation_visitor) override { - const auto& typed_config = - MessageUtil::downcastAndValidate( - config, validation_visitor); + const auto& typed_config = MessageUtil::downcastAndValidate< + const envoy::type::matcher::v3::HttpRequestHeaderMatchInput&>(config, validation_visitor); return std::make_unique(typed_config.header_name()); }; ProtobufTypes::MessagePtr createEmptyConfigProto() override { - return std::make_unique(); + return std::make_unique(); } }; @@ -141,16 +138,13 @@ class HttpResponseHeadersDataInputFactory : public Matcher::DataInputFactory createDataInput(const Protobuf::Message& config, ProtobufMessage::ValidationVisitor& validation_visitor) override { - const auto& typed_config = - MessageUtil::downcastAndValidate( - config, validation_visitor); + const auto& typed_config = MessageUtil::downcastAndValidate< + const envoy::type::matcher::v3::HttpRequestHeaderMatchInput&>(config, validation_visitor); return std::make_unique(typed_config.header_name()); }; ProtobufTypes::MessagePtr createEmptyConfigProto() override { - return std::make_unique(); + return std::make_unique(); } }; From 6cb618f38c31a31ff7b5f9b0a5e4ee8a0f7d46ef Mon Sep 17 00:00:00 2001 From: Snow Pettersen Date: Fri, 18 Dec 2020 01:14:33 +0000 Subject: [PATCH 26/32] update docs Signed-off-by: Snow Pettersen --- docs/root/api-v3/types/types.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/root/api-v3/types/types.rst b/docs/root/api-v3/types/types.rst index 3e6af53865bde..4321708bdb82a 100644 --- a/docs/root/api-v3/types/types.rst +++ b/docs/root/api-v3/types/types.rst @@ -21,5 +21,6 @@ Types ../type/matcher/v3/string.proto ../type/matcher/v3/struct.proto ../type/matcher/v3/value.proto + ../type/matcher/v3/http_inputs.proto ../type/metadata/v3/metadata.proto ../type/tracing/v3/custom_tag.proto From 2cfb10ce51c3aec883f7cedd5409fea2b58f1638 Mon Sep 17 00:00:00 2001 From: Snow Pettersen Date: Fri, 18 Dec 2020 01:28:19 +0000 Subject: [PATCH 27/32] format Signed-off-by: Snow Pettersen --- source/common/http/BUILD | 2 +- source/common/http/filter_manager.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/source/common/http/BUILD b/source/common/http/BUILD index 4eed92f6dd09c..1e52f08ebcb9f 100644 --- a/source/common/http/BUILD +++ b/source/common/http/BUILD @@ -177,8 +177,8 @@ envoy_cc_library( "//source/common/local_reply:local_reply_lib", "//source/common/matcher:matcher_lib", "@envoy_api//envoy/extensions/filters/common/matcher/action/v3:pkg_cc_proto", - "@envoy_api//envoy/type/matcher/v3:pkg_cc_proto", "@envoy_api//envoy/extensions/filters/network/http_connection_manager/v3:pkg_cc_proto", + "@envoy_api//envoy/type/matcher/v3:pkg_cc_proto", ], ) diff --git a/source/common/http/filter_manager.h b/source/common/http/filter_manager.h index 7bc0fc6f8701d..46be12b6884a4 100644 --- a/source/common/http/filter_manager.h +++ b/source/common/http/filter_manager.h @@ -1,13 +1,13 @@ #pragma once #include "envoy/extensions/filters/common/matcher/action/v3/skip_action.pb.h" -#include "envoy/type/matcher/v3/http_inputs.pb.validate.h" #include "envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.pb.h" #include "envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.pb.validate.h" #include "envoy/http/filter.h" #include "envoy/http/header_map.h" #include "envoy/matcher/matcher.h" #include "envoy/protobuf/message_validator.h" +#include "envoy/type/matcher/v3/http_inputs.pb.validate.h" #include "common/buffer/watermark_buffer.h" #include "common/common/dump_state_utils.h" From 0fb52fbdb2b9a4236719ccdfeede016a47f5ac4d Mon Sep 17 00:00:00 2001 From: Snow Pettersen Date: Fri, 18 Dec 2020 15:05:23 +0000 Subject: [PATCH 28/32] fix test failures Signed-off-by: Snow Pettersen --- test/common/matcher/test_utility.h | 4 +++- test/integration/extension_discovery_integration_test.cc | 4 ++-- test/integration/integration_test.cc | 2 +- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/test/common/matcher/test_utility.h b/test/common/matcher/test_utility.h index 623d8158c55ff..1348d9cd80d4c 100644 --- a/test/common/matcher/test_utility.h +++ b/test/common/matcher/test_utility.h @@ -1,6 +1,7 @@ #pragma once #include "envoy/matcher/matcher.h" +#include "envoy/protobuf/message_validator.h" #include "common/matcher/matcher.h" @@ -29,7 +30,8 @@ class TestDataInputFactory : public DataInputFactory { TestDataInputFactory(absl::string_view factory_name, absl::string_view data) : factory_name_(std::string(factory_name)), value_(std::string(data)), injection_(*this) {} - DataInputPtr createDataInput(const Protobuf::Message&) override { + DataInputPtr createDataInput(const Protobuf::Message&, + ProtobufMessage::ValidationVisitor&) override { return std::make_unique( DataInputGetResult{DataInputGetResult::DataAvailability::AllDataAvailable, value_}); } diff --git a/test/integration/extension_discovery_integration_test.cc b/test/integration/extension_discovery_integration_test.cc index 762d5f862e1b3..28f49421dacd3 100644 --- a/test/integration/extension_discovery_integration_test.cc +++ b/test/integration/extension_discovery_integration_test.cc @@ -33,7 +33,7 @@ std::string denyPrivateConfigWithMatcher() { input: name: request-headers typed_config: - "@type": type.googleapis.com/envoy.extensions.filters.common.matcher.input.http.v3.HttpRequestHeaderMatchInput + "@type": type.googleapis.com/envoy.type.matcher.v3.HttpRequestHeaderMatchInput header_name: some-header exact_match_map: map: @@ -84,7 +84,7 @@ class ExtensionDiscoveryIntegrationTest : public Grpc::GrpcClientIntegrationPara input: name: request-headers typed_config: - "@type": type.googleapis.com/envoy.extensions.filters.common.matcher.input.http.v3.HttpRequestHeaderMatchInput + "@type": type.googleapis.com/envoy.type.matcher.v3.HttpRequestHeaderMatchInput header_name: default-matcher-header exact_match_map: map: diff --git a/test/integration/integration_test.cc b/test/integration/integration_test.cc index fa8c4ea666496..9247e1760e825 100644 --- a/test/integration/integration_test.cc +++ b/test/integration/integration_test.cc @@ -380,7 +380,7 @@ name: matcher input: name: request-headers typed_config: - "@type": type.googleapis.com/envoy.extensions.filters.common.matcher.input.http.v3.HttpRequestHeaderMatchInput + "@type": type.googleapis.com/envoy.type.matcher.v3.HttpRequestHeaderMatchInput header_name: match-header exact_match_map: map: From 91e62afb075193c1480814fbec17923fa117c0d5 Mon Sep 17 00:00:00 2001 From: Snow Pettersen Date: Fri, 18 Dec 2020 19:10:41 +0000 Subject: [PATCH 29/32] fix last type url Signed-off-by: Snow Pettersen --- test/common/http/match_wrapper/config_test.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/common/http/match_wrapper/config_test.cc b/test/common/http/match_wrapper/config_test.cc index 0e82f1d6088e0..aad0a2d90445f 100644 --- a/test/common/http/match_wrapper/config_test.cc +++ b/test/common/http/match_wrapper/config_test.cc @@ -55,7 +55,7 @@ TEST(MatchWrapper, WithMatcher) { input: name: request-headers typed_config: - "@type": type.googleapis.com/envoy.extensions.filters.common.matcher.input.http.v3.HttpRequestHeaderMatchInput + "@type": type.googleapis.com/envoy.type.matcher.v3.HttpRequestHeaderMatchInput header_name: default-matcher-header exact_match_map: map: From b2c884f808eaf49fbbf1cf9cf7beaedf00e2901f Mon Sep 17 00:00:00 2001 From: Snow Pettersen Date: Fri, 18 Dec 2020 21:12:46 +0000 Subject: [PATCH 30/32] clang-tidy Signed-off-by: Snow Pettersen --- test/common/http/match_wrapper/config_test.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/common/http/match_wrapper/config_test.cc b/test/common/http/match_wrapper/config_test.cc index aad0a2d90445f..d29207ff3c6e3 100644 --- a/test/common/http/match_wrapper/config_test.cc +++ b/test/common/http/match_wrapper/config_test.cc @@ -16,13 +16,13 @@ namespace MatchWrapper { namespace { struct TestFactory : public Envoy::Server::Configuration::NamedHttpFilterConfigFactory { - std::string name() const { return "test"; } - ProtobufTypes::MessagePtr createEmptyConfigProto() { + std::string name() const override { return "test"; } + ProtobufTypes::MessagePtr createEmptyConfigProto() override { return std::make_unique(); } Envoy::Http::FilterFactoryCb createFilterFactoryFromProto(const Protobuf::Message&, const std::string&, - Server::Configuration::FactoryContext&) { + Server::Configuration::FactoryContext&) override { return [](auto& callbacks) { callbacks.addStreamDecoderFilter(nullptr); callbacks.addStreamEncoderFilter(nullptr); From e4987b5969f37e1e3f97cfc7483f89a778d2d0a1 Mon Sep 17 00:00:00 2001 From: Snow Pettersen Date: Fri, 18 Dec 2020 22:24:29 +0000 Subject: [PATCH 31/32] add include Signed-off-by: Snow Pettersen --- include/envoy/matcher/BUILD | 1 + 1 file changed, 1 insertion(+) diff --git a/include/envoy/matcher/BUILD b/include/envoy/matcher/BUILD index 838e7fd5e0c1c..fadfc109afb0c 100644 --- a/include/envoy/matcher/BUILD +++ b/include/envoy/matcher/BUILD @@ -17,6 +17,7 @@ envoy_cc_library( ], deps = [ "//include/envoy/config:typed_config_interface", + "//include/envoy/protobuf:message_validator_interface", "@envoy_api//envoy/config/common/matcher/v3:pkg_cc_proto", "@envoy_api//envoy/config/core/v3:pkg_cc_proto", ], From 899a365d08237e38710f71a1ea8c88c948b22e5c Mon Sep 17 00:00:00 2001 From: Snow Pettersen Date: Mon, 11 Jan 2021 15:43:38 +0000 Subject: [PATCH 32/32] introduce templated base class for factory Signed-off-by: Snow Pettersen --- source/common/http/filter_manager.h | 43 ++++++++++++++++------------- 1 file changed, 24 insertions(+), 19 deletions(-) diff --git a/source/common/http/filter_manager.h b/source/common/http/filter_manager.h index d056ad2eda8fe..502b225f106b2 100644 --- a/source/common/http/filter_manager.h +++ b/source/common/http/filter_manager.h @@ -101,20 +101,34 @@ class HttpRequestHeadersDataInput : public HttpHeadersDataInputBase { +template +class HttpHeadersDataInputFactoryBase : public Matcher::DataInputFactory { public: - std::string name() const override { return "request-headers"; } + explicit HttpHeadersDataInputFactoryBase(const std::string& name) : name_(name) {} + + std::string name() const override { return name_; } + Matcher::DataInputPtr createDataInput(const Protobuf::Message& config, ProtobufMessage::ValidationVisitor& validation_visitor) override { - const auto& typed_config = MessageUtil::downcastAndValidate< - const envoy::type::matcher::v3::HttpRequestHeaderMatchInput&>(config, validation_visitor); + const auto& typed_config = + MessageUtil::downcastAndValidate(config, validation_visitor); - return std::make_unique(typed_config.header_name()); + return std::make_unique(typed_config.header_name()); }; ProtobufTypes::MessagePtr createEmptyConfigProto() override { - return std::make_unique(); + return std::make_unique(); } + +private: + const std::string name_; +}; + +class HttpRequestHeadersDataInputFactory + : public HttpHeadersDataInputFactoryBase< + HttpRequestHeadersDataInput, envoy::type::matcher::v3::HttpRequestHeaderMatchInput> { +public: + HttpRequestHeadersDataInputFactory() : HttpHeadersDataInputFactoryBase("request-headers") {} }; class HttpResponseHeadersDataInput : public HttpHeadersDataInputBase { @@ -127,20 +141,11 @@ class HttpResponseHeadersDataInput : public HttpHeadersDataInputBase { +class HttpResponseHeadersDataInputFactory + : public HttpHeadersDataInputFactoryBase< + HttpResponseHeadersDataInput, envoy::type::matcher::v3::HttpResponseHeaderMatchInput> { public: - std::string name() const override { return "response-headers"; } - Matcher::DataInputPtr - createDataInput(const Protobuf::Message& config, - ProtobufMessage::ValidationVisitor& validation_visitor) override { - const auto& typed_config = MessageUtil::downcastAndValidate< - const envoy::type::matcher::v3::HttpRequestHeaderMatchInput&>(config, validation_visitor); - - return std::make_unique(typed_config.header_name()); - }; - ProtobufTypes::MessagePtr createEmptyConfigProto() override { - return std::make_unique(); - } + HttpResponseHeadersDataInputFactory() : HttpHeadersDataInputFactoryBase("response-headers") {} }; class SkipAction : public Matcher::ActionBase<