From 17972da91572853491d6d8bd238ffdf2ae53aa42 Mon Sep 17 00:00:00 2001 From: Matt Klein Date: Tue, 25 Dec 2018 11:16:34 -0700 Subject: [PATCH 01/26] tap: introduce HTTP tap filter This is a MVP for the HTTP tap filter. It includes minimal infrastructure for the following: 1. Generic tap configuration which in the future will be used for static config, XDS config, etc. In this MVP the tap can be configured via a /tap admin endpoint. 2. Generic output configuration which in the future will be used for different output sinks such as files, gRPC API, etc. In this MVP the tap results are streamed back out the /tap admin endpoint. 3. Matching infrastructure. In this MVP only matching on request and response headers are implemented. Both logical AND and logical OR matches are possible. 4. In this MVP request/response body is not considered at all. 5. All docs are included and with all the caveats the filter is ready to use for the limited cases it supports (which are likely still to be useful). There is a lot of follow on work which I will do in subsequent PRs. This includes: 1. Merging the existing capture transport socket into this framework. 2. Implementing body support, both for matching on body contents as well as outputting body data. 3. Tap rate limiting so too many streams do not get tapped. 4. gRPC matching. Using reflection and loaded proto definitions, it will be possible to match on gRPC fields. 5. JSON matching. If the body parses as JSON, we can allow matching on JSON fields. Part of https://github.com/envoyproxy/envoy/issues/1413. Signed-off-by: Matt Klein --- api/docs/BUILD | 4 + api/envoy/admin/v2alpha/BUILD | 8 + api/envoy/admin/v2alpha/tap.proto | 18 ++ .../config/filter/http/tap/v2alpha/BUILD | 11 + .../config/filter/http/tap/v2alpha/tap.proto | 30 +++ api/envoy/data/tap/v2alpha/BUILD | 5 + api/envoy/data/tap/v2alpha/http.proto | 28 +++ api/envoy/service/tap/v2alpha/BUILD | 12 + api/envoy/service/tap/v2alpha/common.proto | 88 ++++++++ docs/build.sh | 4 + docs/root/api-v2/admin/admin.rst | 8 +- docs/root/api-v2/data/tap/tap.rst | 2 +- docs/root/api-v2/service/service.rst | 1 + .../http_filters/http_filters.rst | 1 + .../configuration/http_filters/tap_filter.rst | 122 ++++++++++ docs/root/operations/admin.rst | 8 + include/envoy/server/admin.h | 5 + include/envoy/singleton/BUILD | 1 + include/envoy/singleton/manager.h | 1 + source/common/common/logger.h | 1 + source/common/http/header_utility.cc | 2 +- source/common/http/header_utility.h | 2 +- source/extensions/common/tap/BUILD | 31 +++ source/extensions/common/tap/admin.cc | 113 ++++++++++ source/extensions/common/tap/admin.h | 76 +++++++ source/extensions/common/tap/tap.h | 61 +++++ .../extensions/common/tap/tap_config_base.h | 25 +++ source/extensions/extensions_build_config.bzl | 1 + source/extensions/filters/http/tap/BUILD | 61 +++++ source/extensions/filters/http/tap/config.cc | 46 ++++ source/extensions/filters/http/tap/config.h | 31 +++ .../extensions/filters/http/tap/tap_config.h | 76 +++++++ .../filters/http/tap/tap_config_impl.cc | 127 +++++++++++ .../filters/http/tap/tap_config_impl.h | 80 +++++++ .../extensions/filters/http/tap/tap_filter.cc | 86 +++++++ .../extensions/filters/http/tap/tap_filter.h | 139 ++++++++++++ .../filters/http/well_known_names.h | 2 + source/server/http/admin.cc | 6 +- source/server/http/admin.h | 1 + source/server/worker_impl.cc | 3 +- test/extensions/common/tap/BUILD | 18 ++ test/extensions/common/tap/admin_test.cc | 99 +++++++++ test/extensions/filters/http/tap/BUILD | 33 +++ .../http/tap/tap_filter_integration_test.cc | 210 ++++++++++++++++++ .../filters/http/tap/tap_filter_test.cc | 128 +++++++++++ test/integration/http_integration.cc | 8 +- test/integration/http_integration.h | 2 + test/integration/integration.cc | 7 +- test/integration/integration.h | 1 + test/mocks/server/mocks.cc | 6 +- test/mocks/server/mocks.h | 25 ++- test/server/http/admin_test.cc | 17 +- 52 files changed, 1840 insertions(+), 41 deletions(-) create mode 100644 api/envoy/admin/v2alpha/tap.proto create mode 100644 api/envoy/config/filter/http/tap/v2alpha/BUILD create mode 100644 api/envoy/config/filter/http/tap/v2alpha/tap.proto create mode 100644 api/envoy/data/tap/v2alpha/http.proto create mode 100644 api/envoy/service/tap/v2alpha/BUILD create mode 100644 api/envoy/service/tap/v2alpha/common.proto create mode 100644 docs/root/configuration/http_filters/tap_filter.rst create mode 100644 source/extensions/common/tap/BUILD create mode 100644 source/extensions/common/tap/admin.cc create mode 100644 source/extensions/common/tap/admin.h create mode 100644 source/extensions/common/tap/tap.h create mode 100644 source/extensions/common/tap/tap_config_base.h create mode 100644 source/extensions/filters/http/tap/BUILD create mode 100644 source/extensions/filters/http/tap/config.cc create mode 100644 source/extensions/filters/http/tap/config.h create mode 100644 source/extensions/filters/http/tap/tap_config.h create mode 100644 source/extensions/filters/http/tap/tap_config_impl.cc create mode 100644 source/extensions/filters/http/tap/tap_config_impl.h create mode 100644 source/extensions/filters/http/tap/tap_filter.cc create mode 100644 source/extensions/filters/http/tap/tap_filter.h create mode 100644 test/extensions/common/tap/BUILD create mode 100644 test/extensions/common/tap/admin_test.cc create mode 100644 test/extensions/filters/http/tap/BUILD create mode 100644 test/extensions/filters/http/tap/tap_filter_integration_test.cc create mode 100644 test/extensions/filters/http/tap/tap_filter_test.cc diff --git a/api/docs/BUILD b/api/docs/BUILD index bca5ebfbe680..24f5107b7a0e 100644 --- a/api/docs/BUILD +++ b/api/docs/BUILD @@ -18,6 +18,7 @@ proto_library( "//envoy/admin/v2alpha:memory", "//envoy/admin/v2alpha:mutex_stats", "//envoy/admin/v2alpha:server_info", + "//envoy/admin/v2alpha:tap", "//envoy/api/v2:cds", "//envoy/api/v2:discovery", "//envoy/api/v2:eds", @@ -46,6 +47,7 @@ proto_library( "//envoy/config/filter/http/rbac/v2:rbac", "//envoy/config/filter/http/router/v2:router", "//envoy/config/filter/http/squash/v2:squash", + "//envoy/config/filter/http/tap/v2alpha:tap", "//envoy/config/filter/http/transcoder/v2:transcoder", "//envoy/config/filter/network/client_ssl_auth/v2:client_ssl_auth", "//envoy/config/filter/network/ext_authz/v2:ext_authz", @@ -71,6 +73,7 @@ proto_library( "//envoy/data/accesslog/v2:accesslog", "//envoy/data/core/v2alpha:health_check_event", "//envoy/data/tap/v2alpha:capture", + "//envoy/data/tap/v2alpha:http", "//envoy/service/accesslog/v2:als", "//envoy/service/auth/v2alpha:attribute_context", "//envoy/service/auth/v2alpha:external_auth", @@ -78,6 +81,7 @@ proto_library( "//envoy/service/load_stats/v2:lrs", "//envoy/service/metrics/v2:metrics_service", "//envoy/service/ratelimit/v2:rls", + "//envoy/service/tap/v2alpha:common", "//envoy/type:percent", "//envoy/type:range", "//envoy/type/matcher:metadata", diff --git a/api/envoy/admin/v2alpha/BUILD b/api/envoy/admin/v2alpha/BUILD index 332c07058bb6..a6b403fdd23e 100644 --- a/api/envoy/admin/v2alpha/BUILD +++ b/api/envoy/admin/v2alpha/BUILD @@ -55,3 +55,11 @@ api_proto_library_internal( srcs = ["server_info.proto"], visibility = ["//visibility:public"], ) + +api_proto_library_internal( + name = "tap", + srcs = ["tap.proto"], + deps = [ + "//envoy/service/tap/v2alpha:common", + ], +) diff --git a/api/envoy/admin/v2alpha/tap.proto b/api/envoy/admin/v2alpha/tap.proto new file mode 100644 index 000000000000..e2534440b6da --- /dev/null +++ b/api/envoy/admin/v2alpha/tap.proto @@ -0,0 +1,18 @@ +syntax = "proto3"; + +import "envoy/service/tap/v2alpha/common.proto"; +import "validate/validate.proto"; + +package envoy.admin.v2alpha; +option java_package = "io.envoyproxy.envoy.admin.v2alpha"; +option java_multiple_files = true; + +// The /tap admin request body that is used to configure an active tap session. +message TapRequest { + // The opaque configuration ID used to match the configuration to a loaded extension. + // A tap extension configures a similar opaque ID that is used to match. + string config_id = 1 [(validate.rules).string.min_bytes = 1]; + + // The tap configuration to load. + service.tap.v2alpha.TapConfig tap_config = 2 [(validate.rules).message.required = true]; +} diff --git a/api/envoy/config/filter/http/tap/v2alpha/BUILD b/api/envoy/config/filter/http/tap/v2alpha/BUILD new file mode 100644 index 000000000000..acec4094f172 --- /dev/null +++ b/api/envoy/config/filter/http/tap/v2alpha/BUILD @@ -0,0 +1,11 @@ +load("//bazel:api_build_system.bzl", "api_proto_library_internal") + +licenses(["notice"]) # Apache 2 + +api_proto_library_internal( + name = "tap", + srcs = ["tap.proto"], + deps = [ + "//envoy/service/tap/v2alpha:common", + ], +) diff --git a/api/envoy/config/filter/http/tap/v2alpha/tap.proto b/api/envoy/config/filter/http/tap/v2alpha/tap.proto new file mode 100644 index 000000000000..b9337f8f14ea --- /dev/null +++ b/api/envoy/config/filter/http/tap/v2alpha/tap.proto @@ -0,0 +1,30 @@ +syntax = "proto3"; + +import "envoy/service/tap/v2alpha/common.proto"; + +import "validate/validate.proto"; + +package envoy.config.filter.http.tap.v2alpha; +option java_package = "io.envoyproxy.envoy.config.filter.http.tap.v2alpha"; +option java_multiple_files = true; + +// [#protodoc-title: Tap] +// Tap :ref:`configuration overview `. + +// Top level configuration for the tap filter. +message Tap { + oneof config_type { + option (validate.required) = true; + + // If specified, the tap filter will be configured via an admin handler. + AdminConfig admin_config = 1; + } +} + +// Configuration for the admin handler. See :ref:`here ` for +// more information. +message AdminConfig { + // Opaque configuration ID. When requests are made to the admin handler, the passed opaque ID is + // matched to the configured filter opaque ID to determine which filter to configure. + string config_id = 1 [(validate.rules).string.min_bytes = 1]; +} diff --git a/api/envoy/data/tap/v2alpha/BUILD b/api/envoy/data/tap/v2alpha/BUILD index 46de68e3a825..c0cdd05c7f11 100644 --- a/api/envoy/data/tap/v2alpha/BUILD +++ b/api/envoy/data/tap/v2alpha/BUILD @@ -7,3 +7,8 @@ api_proto_library_internal( srcs = ["capture.proto"], deps = ["//envoy/api/v2/core:address"], ) + +api_proto_library_internal( + name = "http", + srcs = ["http.proto"], +) diff --git a/api/envoy/data/tap/v2alpha/http.proto b/api/envoy/data/tap/v2alpha/http.proto new file mode 100644 index 000000000000..98f8b508b582 --- /dev/null +++ b/api/envoy/data/tap/v2alpha/http.proto @@ -0,0 +1,28 @@ +syntax = "proto3"; + +package envoy.data.tap.v2alpha; +option java_package = "io.envoyproxy.envoy.data.tap.v2alpha"; +option java_multiple_files = true; + +// [#protodoc-title: HTTP tap data] + +// A fully buffered HTTP trace message. +message HttpBufferedTrace { + message Header { + // Header key. + string key = 1; + + // Header value. + string value = 2; + } + + // The match ID specified in the :ref:`match_id + // ` field. + string match_id = 1; + + // Request headers. + repeated Header request_headers = 2; + + // Response headers. + repeated Header response_headers = 3; +} diff --git a/api/envoy/service/tap/v2alpha/BUILD b/api/envoy/service/tap/v2alpha/BUILD new file mode 100644 index 000000000000..0d46bdac25a8 --- /dev/null +++ b/api/envoy/service/tap/v2alpha/BUILD @@ -0,0 +1,12 @@ +load("//bazel:api_build_system.bzl", "api_proto_library_internal") + +licenses(["notice"]) # Apache 2 + +api_proto_library_internal( + name = "common", + srcs = ["common.proto"], + visibility = ["//visibility:public"], + deps = [ + "//envoy/api/v2/route", + ], +) diff --git a/api/envoy/service/tap/v2alpha/common.proto b/api/envoy/service/tap/v2alpha/common.proto new file mode 100644 index 000000000000..0462db7c2708 --- /dev/null +++ b/api/envoy/service/tap/v2alpha/common.proto @@ -0,0 +1,88 @@ +syntax = "proto3"; + +import "envoy/api/v2/route/route.proto"; + +import "validate/validate.proto"; + +package envoy.service.tap.v2alpha; +option java_package = "io.envoyproxy.envoy.service.tap.v2alpha"; +option java_multiple_files = true; + +// [#protodoc-title: Common tap configuration] + +// Tap configuration. +message TapConfig { + // The match conditions. One or more match configurations can be specified. If any of them + // match the data source being tapped, a tap will occur, with the result written to the + // configured output. + repeated MatchConfig match_configs = 1 [(validate.rules).repeated .min_items = 1]; + + // The tap output configuration. If a match configuration matches a data source being tapped, + // a tap will occur and the data will be written to the configured output. + OutputConfig output_config = 2 [(validate.rules).message.required = true]; + + // [#comment:TODO(mattklein123): Rate limiting] +} + +// Tap match configuration. Within a single match configuration, all conditions are evaluated as +// a logical AND. Logical OR matching can be implemented via multiple match configurations in the +// top level tap configuration. +message MatchConfig { + // Match ID used for correlating tap data with the match that produced it. + string match_id = 1 [(validate.rules).string.min_bytes = 1]; + + // HTTP match configuration. Only useful for HTTP aware tap extensions such as the + // :ref:`HTTP tap filter `. + HttpMatchConfig http_match_config = 2; +} + +// HTTP match configuration. +message HttpMatchConfig { + // HTTP request match configuration. + HttpRequestMatchConfig request_match_config = 1; + + // HTTP response match configuration. + HttpResponseMatchConfig response_match_config = 2; +} + +// HTTP request match configuration. +message HttpRequestMatchConfig { + // HTTP request headers to match. + repeated api.v2.route.HeaderMatcher headers = 1; +} + +// HTTP response match configuration. +message HttpResponseMatchConfig { + // HTTP response headers to match. + repeated api.v2.route.HeaderMatcher headers = 1; +} + +// Tap output configuration. +message OutputConfig { + // Output sinks for tap data. Currently a single sink is allowed in the list. Once multiple + // sink types are supported this constraint will be relaxed. + repeated OutputSink sinks = 1 [(validate.rules).repeated = {min_items: 1, max_items: 1}]; + + // [#comment:TODO(mattklein123): Output filtering. E.g., certain headers, truncated body, etc.] +} + +// Tap output sink configuration. +message OutputSink { + oneof output_sink_type { + option (validate.required) = true; + + // Tap output will be streamed out the :http:post:`/tap` admin endpoint. + // + // .. attention:: + // + // It is only allowed to specify the streaming admin output sink if the tap is being + // configured from the :http:post:`/tap` admin endpoint. Thus, if an extension has + // been configured to receive tap configuration from some other source (e.g., static + // file, XDS, etc.) configuring the streaming admin output type will fail. + StreamingAdminSink streaming_admin = 1; + } +} + +// Streaming admin sink configuration. +message StreamingAdminSink { +} diff --git a/docs/build.sh b/docs/build.sh index d2bed5cd023d..2d0ddd3a4ed8 100755 --- a/docs/build.sh +++ b/docs/build.sh @@ -54,6 +54,7 @@ PROTO_RST=" /envoy/admin/v2alpha/clusters/envoy/admin/v2alpha/metrics.proto.rst /envoy/admin/v2alpha/mutex_stats/envoy/admin/v2alpha/mutex_stats.proto.rst /envoy/admin/v2alpha/server_info/envoy/admin/v2alpha/server_info.proto.rst + /envoy/admin/v2alpha/tap/envoy/admin/v2alpha/tap.proto.rst /envoy/api/v2/core/address/envoy/api/v2/core/address.proto.rst /envoy/api/v2/core/base/envoy/api/v2/core/base.proto.rst /envoy/api/v2/core/http_uri/envoy/api/v2/core/http_uri.proto.rst @@ -95,6 +96,7 @@ PROTO_RST=" /envoy/config/filter/http/rbac/v2/rbac/envoy/config/filter/http/rbac/v2/rbac.proto.rst /envoy/config/filter/http/router/v2/router/envoy/config/filter/http/router/v2/router.proto.rst /envoy/config/filter/http/squash/v2/squash/envoy/config/filter/http/squash/v2/squash.proto.rst + /envoy/config/filter/http/tap/v2alpha/tap/envoy/config/filter/http/tap/v2alpha/tap.proto.rst /envoy/config/filter/http/transcoder/v2/transcoder/envoy/config/filter/http/transcoder/v2/transcoder.proto.rst /envoy/config/filter/network/client_ssl_auth/v2/client_ssl_auth/envoy/config/filter/network/client_ssl_auth/v2/client_ssl_auth.proto.rst /envoy/config/filter/network/ext_authz/v2/ext_authz/envoy/config/filter/network/ext_authz/v2/ext_authz.proto.rst @@ -117,10 +119,12 @@ PROTO_RST=" /envoy/data/accesslog/v2/accesslog/envoy/data/accesslog/v2/accesslog.proto.rst /envoy/data/core/v2alpha/health_check_event/envoy/data/core/v2alpha/health_check_event.proto.rst /envoy/data/tap/v2alpha/capture/envoy/data/tap/v2alpha/capture.proto.rst + /envoy/data/tap/v2alpha/http/envoy/data/tap/v2alpha/http.proto.rst /envoy/service/accesslog/v2/als/envoy/service/accesslog/v2/als.proto.rst /envoy/service/auth/v2alpha/external_auth/envoy/service/auth/v2alpha/attribute_context.proto.rst /envoy/service/auth/v2alpha/external_auth/envoy/service/auth/v2alpha/external_auth.proto.rst /envoy/service/ratelimit/v2/rls/envoy/service/ratelimit/v2/rls.proto.rst + /envoy/service/tap/v2alpha/common/envoy/service/tap/v2alpha/common.proto.rst /envoy/type/http_status/envoy/type/http_status.proto.rst /envoy/type/percent/envoy/type/percent.proto.rst /envoy/type/range/envoy/type/range.proto.rst diff --git a/docs/root/api-v2/admin/admin.rst b/docs/root/api-v2/admin/admin.rst index 4837d9d10a7a..0f5579251d93 100644 --- a/docs/root/api-v2/admin/admin.rst +++ b/docs/root/api-v2/admin/admin.rst @@ -5,10 +5,4 @@ Admin :glob: :maxdepth: 2 - ../admin/v2alpha/certs.proto - ../admin/v2alpha/config_dump.proto - ../admin/v2alpha/clusters.proto - ../admin/v2alpha/memory.proto - ../admin/v2alpha/metrics.proto - ../admin/v2alpha/mutex_stats.proto - ../admin/v2alpha/server_info.proto + ../admin/v2alpha/* diff --git a/docs/root/api-v2/data/tap/tap.rst b/docs/root/api-v2/data/tap/tap.rst index b6514156bcab..b1e7b3dd5017 100644 --- a/docs/root/api-v2/data/tap/tap.rst +++ b/docs/root/api-v2/data/tap/tap.rst @@ -5,4 +5,4 @@ Tap :glob: :maxdepth: 2 - v2alpha/capture.proto + v2alpha/* diff --git a/docs/root/api-v2/service/service.rst b/docs/root/api-v2/service/service.rst index 00d2698ca161..4cf1c5e9f284 100644 --- a/docs/root/api-v2/service/service.rst +++ b/docs/root/api-v2/service/service.rst @@ -7,3 +7,4 @@ Services accesslog/v2/* ratelimit/v2/* + tap/v2alpha/* diff --git a/docs/root/configuration/http_filters/http_filters.rst b/docs/root/configuration/http_filters/http_filters.rst index 75be91b9f844..6c2d38b8c81e 100644 --- a/docs/root/configuration/http_filters/http_filters.rst +++ b/docs/root/configuration/http_filters/http_filters.rst @@ -25,3 +25,4 @@ HTTP filters rbac_filter router_filter squash_filter + tap_filter diff --git a/docs/root/configuration/http_filters/tap_filter.rst b/docs/root/configuration/http_filters/tap_filter.rst new file mode 100644 index 000000000000..d7488c5852e6 --- /dev/null +++ b/docs/root/configuration/http_filters/tap_filter.rst @@ -0,0 +1,122 @@ +.. _config_http_filters_tap: + +Tap +=== + +* :ref:`v2 API reference ` +* This filter should be configured with the name *envoy.filters.http.tap*. + +.. attention:: + + The tap filter is experimental and is currently under active development. There is currently a + very limited set of match conditions, output configuration, output sinks, etc. Capabilities will + be expanded over time and the configuration structures are likely to change. + +The HTTP tap filter is used to tap HTTP traffic. At a high level, the configuration is composed of +two pieces: + +1. Match configuration: a list of conditions under which the filter will match an HTTP request + and begin a tap session. +2. Output configuration: a list of output sinks that the filter will write the matched and tapped + data to. + +Each of these concepts will be covered in the following example configuration section. + +Example configuration +--------------------- + +Example filter configuration: + +.. code-block:: yaml + + name: envoy.filters.http.tap + config: + admin_config: + config_id: test_config_id + +The previous snippet configures the filter for control via the :http:post:`/tap` admin handler. +See the following section for more details. + +.. _config_http_filters_tap_admin_handler: + +Admin handler +------------- + +When the HTTP filter specifies an :ref:`admin_config +`, it is configured for admin control and +the :http:post:`/tap` admin handler will be installed. The admin handler can be used for live +tapping and debugging of HTTP traffic. It works as follows: + +1. A POST request is used to provide a valid tap configuration. The POST request body can be either + the JSON or YAML representation of the :ref:`TapConfig + ` message. +2. If the POST request is accepted, Envoy will stream :ref:`HttpBufferedTrace + ` messages (serialized to JSON) until the admin + request is terminated. + +An example POST body: + +.. code-block:: yaml + + config_id: test_config_id + tap_config: + match_configs: + - match_id: foo_match_id + http_match_config: + request_match_config: + headers: + - name: foo + exact_match: bar + response_match_config: + headers: + - name: bar + exact_match: baz + output_config: + sinks: + - streaming_admin: {} + +The configuration instructs the tap filter to match any HTTP requests in which a request header +``foo: bar`` is present AND a response header ``bar: baz`` is present. If both of these conditions +are met, the request will be tapped and streamed out the admin endpoint. + +Another example POST body: + +.. code-block:: yaml + + config_id: test_config_id + tap_config: + match_configs: + - match_id: request_match_id + http_match_config: + request_match_config: + headers: + - name: foo + exact_match: bar + - match_id: response_match_id + http_match_config: + response_match_config: + headers: + - name: bar + exact_match: baz + output_config: + sinks: + - streaming_admin: {} + +The configuration instructs the tap filter to match any HTTP requests in which a request header +``foo: bar`` is present OR a response header ``bar: baz`` is present. If either of these conditions +are met, the request will be tapped and streamed out the admin endpoint. The difference between +the first example and the second is the use of multiple top level match configurations to produce +a logical OR. + +Statistics +---------- + +The tap filter outputs statistics in the *http..tap.* namespace. The :ref:`stat prefix +` +comes from the owning HTTP connection manager. + +.. csv-table:: + :header: Name, Type, Description + :widths: 1, 1, 2 + + rq_tapped, Counter, Total requests that matched and were tapped diff --git a/docs/root/operations/admin.rst b/docs/root/operations/admin.rst index a8016befefe7..032ee4a3090d 100644 --- a/docs/root/operations/admin.rst +++ b/docs/root/operations/admin.rst @@ -399,3 +399,11 @@ explanation of the output. set to '0'. * Latency information represents data since last flush. Mean latency is currently not available. + +.. http:post:: /tap + + This endpoint is used for configuring an active tap session. It is only + available if a valid tap extension has been configured, and that extension has + been configured to accept admin configuration. See: + + * :ref:`HTTP tap filter configuration ` diff --git a/include/envoy/server/admin.h b/include/envoy/server/admin.h index af14490f1216..7249e65cc2bd 100644 --- a/include/envoy/server/admin.h +++ b/include/envoy/server/admin.h @@ -39,6 +39,11 @@ class AdminStream { */ virtual Http::StreamDecoderFilterCallbacks& getDecoderFilterCallbacks() const PURE; + /** + * @return const Buffer::Instance* the fully buffered admin request if applicable. + */ + virtual const Buffer::Instance* getRequestBody() const PURE; + /** * @return Http::HeaderMap& to be used by handler to parse header information sent with the * request. diff --git a/include/envoy/singleton/BUILD b/include/envoy/singleton/BUILD index 330dc3f8905c..23fd60851570 100644 --- a/include/envoy/singleton/BUILD +++ b/include/envoy/singleton/BUILD @@ -18,5 +18,6 @@ envoy_cc_library( hdrs = ["manager.h"], deps = [ ":instance_interface", + "//include/envoy/registry", ], ) diff --git a/include/envoy/singleton/manager.h b/include/envoy/singleton/manager.h index 089c8cca2eb8..9721d9318ff8 100644 --- a/include/envoy/singleton/manager.h +++ b/include/envoy/singleton/manager.h @@ -5,6 +5,7 @@ #include #include "envoy/common/pure.h" +#include "envoy/registry/registry.h" #include "envoy/singleton/instance.h" namespace Envoy { diff --git a/source/common/common/logger.h b/source/common/common/logger.h index 341caa20c8c9..d97ffba7f943 100644 --- a/source/common/common/logger.h +++ b/source/common/common/logger.h @@ -47,6 +47,7 @@ namespace Logger { FUNCTION(runtime) \ FUNCTION(stats) \ FUNCTION(secret) \ + FUNCTION(tap) \ FUNCTION(testing) \ FUNCTION(thrift) \ FUNCTION(tracing) \ diff --git a/source/common/http/header_utility.cc b/source/common/http/header_utility.cc index b579ba10b2bd..1bb3d4d77c4b 100644 --- a/source/common/http/header_utility.cc +++ b/source/common/http/header_utility.cc @@ -66,7 +66,7 @@ HeaderUtility::HeaderData::HeaderData(const Json::Object& config) bool HeaderUtility::matchHeaders(const Http::HeaderMap& request_headers, const std::vector& config_headers) { - // TODO (rodaine): Should this really allow empty headers to always match? + // No headers to match is considered a match. if (!config_headers.empty()) { for (const HeaderData& cfg_header_data : config_headers) { if (!matchHeaders(request_headers, cfg_header_data)) { diff --git a/source/common/http/header_utility.h b/source/common/http/header_utility.h index ec061ee4fa76..a2af114bcd84 100644 --- a/source/common/http/header_utility.h +++ b/source/common/http/header_utility.h @@ -38,7 +38,7 @@ class HeaderUtility { * @param request_headers supplies the headers from the request. * @param config_headers supplies the list of configured header conditions on which to match. * @return bool true if all the headers (and values) in the config_headers are found in the - * request_headers + * request_headers. If no config_headers are specified, returns true. */ static bool matchHeaders(const Http::HeaderMap& request_headers, const std::vector& config_headers); diff --git a/source/extensions/common/tap/BUILD b/source/extensions/common/tap/BUILD new file mode 100644 index 000000000000..49c348ae40f9 --- /dev/null +++ b/source/extensions/common/tap/BUILD @@ -0,0 +1,31 @@ +licenses(["notice"]) # Apache 2 + +load( + "//bazel:envoy_build_system.bzl", + "envoy_cc_library", + "envoy_package", +) + +envoy_package() + +envoy_cc_library( + name = "tap_interface", + hdrs = ["tap.h"], +) + +envoy_cc_library( + name = "tap_config_base", + hdrs = ["tap_config_base.h"], +) + +envoy_cc_library( + name = "admin", + srcs = ["admin.cc"], + hdrs = ["admin.h"], + deps = [ + ":tap_interface", + "//include/envoy/server:admin_interface", + "//include/envoy/singleton:manager_interface", + "@envoy_api//envoy/admin/v2alpha:tap_cc", + ], +) diff --git a/source/extensions/common/tap/admin.cc b/source/extensions/common/tap/admin.cc new file mode 100644 index 000000000000..96fb4dfcf954 --- /dev/null +++ b/source/extensions/common/tap/admin.cc @@ -0,0 +1,113 @@ +#include "extensions/common/tap/admin.h" + +#include "envoy/admin/v2alpha/tap.pb.h" +#include "envoy/admin/v2alpha/tap.pb.validate.h" + +#include "common/buffer/buffer_impl.h" + +namespace Envoy { +namespace Extensions { +namespace Common { +namespace Tap { + +// Singleton registration via macro defined in envoy/singleton/manager.h +SINGLETON_MANAGER_REGISTRATION(tap_admin_handler); + +AdminHandlerSharedPtr AdminHandler::getSingleton(Server::Admin& admin, + Singleton::Manager& singleton_manager, + Event::Dispatcher& main_thread_dispatcher) { + return singleton_manager.getTyped( + SINGLETON_MANAGER_REGISTERED_NAME(tap_admin_handler), [&admin, &main_thread_dispatcher] { + return std::make_shared(admin, main_thread_dispatcher); + }); +} + +AdminHandler::AdminHandler(Server::Admin& admin, Event::Dispatcher& main_thread_dispatcher) + : admin_(admin), main_thread_dispatcher_(main_thread_dispatcher) { + bool rc = + admin_.addHandler("/tap", "tap filter control", MAKE_ADMIN_HANDLER(handler), true, true); + RELEASE_ASSERT(rc, "/tap admin endpoint is taken"); +} + +AdminHandler::~AdminHandler() { + bool rc = admin_.removeHandler("/tap"); + ASSERT(rc); +} + +Http::Code AdminHandler::handler(absl::string_view, Http::HeaderMap&, Buffer::Instance& response, + Server::AdminStream& admin_stream) { + if (attached_request_.has_value()) { + response.add("An attached /tap admin stream already exists. Detach it."); + return Http::Code::BadRequest; + } + + if (admin_stream.getRequestBody() == nullptr) { + response.add("/tap requires a JSON/YAML body"); + return Http::Code::BadRequest; + } + + envoy::admin::v2alpha::TapRequest tap_request; + try { + MessageUtil::loadFromYamlAndValidate(admin_stream.getRequestBody()->toString(), tap_request); + } catch (EnvoyException& e) { + response.add(e.what()); + return Http::Code::BadRequest; + } + + ENVOY_LOG(debug, "tap admin request for config_id={}", tap_request.config_id()); + if (config_id_map_.count(tap_request.config_id()) == 0) { + response.add(fmt::format("Unknown config id '{}'. No extension has registered with this id.", + tap_request.config_id())); + return Http::Code::BadRequest; + } + for (auto config : config_id_map_[tap_request.config_id()]) { + config->newTapConfig(std::move(*tap_request.mutable_tap_config()), this); + } + + admin_stream.setEndStreamOnComplete(false); + admin_stream.addOnDestroyCallback([this] { + for (auto config : config_id_map_[attached_request_.value().config_id_]) { + ENVOY_LOG(debug, "detach tap admin request for config_id={}", + attached_request_.value().config_id_); + config->clearTapConfig(); + attached_request_ = absl::nullopt; + } + }); + attached_request_.emplace(tap_request.config_id(), &admin_stream); + return Http::Code::OK; +} + +void AdminHandler::registerConfig(ExtensionConfig& config, const std::string& config_id) { + ASSERT(!config_id.empty()); + ASSERT(config_id_map_[config_id].count(&config) == 0); + config_id_map_[config_id].insert(&config); +} + +void AdminHandler::unregisterConfig(ExtensionConfig& config) { + ASSERT(!config.adminId().empty()); + ASSERT(config_id_map_[config.adminId()].count(&config) == 1); + config_id_map_[config.adminId()].erase(&config); + if (config_id_map_[config.adminId()].size() == 0) { + config_id_map_.erase(config.adminId()); + } +} + +void AdminHandler::submitBufferedTrace(std::shared_ptr trace) { + // TODO(mattklein123): The only reason this takes a shared_ptr is because currently a posted + // lambda cannot take a unique_ptr because we copy the lambda. If we allow posts to happen with + // a moved lambda we can fix this. I will fix this in a follow up. + ENVOY_LOG(debug, "admin submitting buffered trace to main thread"); + main_thread_dispatcher_.post([this, trace]() { + if (attached_request_.has_value()) { + ENVOY_LOG(debug, "admin writing buffered trace to response"); + Buffer::OwnedImpl json_trace{MessageUtil::getJsonStringFromMessage(*trace, true, true)}; + attached_request_.value().admin_stream_->getDecoderFilterCallbacks().encodeData(json_trace, + false); + } + }); +} + +} // namespace Tap +} // namespace Common +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/common/tap/admin.h b/source/extensions/common/tap/admin.h new file mode 100644 index 000000000000..a73a42ee8e58 --- /dev/null +++ b/source/extensions/common/tap/admin.h @@ -0,0 +1,76 @@ +#pragma once + +#include "envoy/server/admin.h" +#include "envoy/singleton/manager.h" + +#include "extensions/common/tap/tap.h" + +namespace Envoy { +namespace Extensions { +namespace Common { +namespace Tap { + +class AdminHandler; +typedef std::shared_ptr AdminHandlerSharedPtr; + +/** + * Singleton /tap admin handler for admin management of tap configurations and output. This + * handler is not installed and active unless the tap configuration specifically configures it. + * TODO(mattklein123): We should allow the admin handler to always be installed in read only mode + * so it's easier to debug the active tap configuration. + */ +class AdminHandler : public Singleton::Instance, + public Extensions::Common::Tap::Sink, + Logger::Loggable { +public: + AdminHandler(Server::Admin& admin, Event::Dispatcher& main_thread_dispatcher); + ~AdminHandler(); + + /** + * Get the singleton admin handler. The handler will be created if it doesn't already exist, + * otherwise the existing handler will be returned. + */ + static AdminHandlerSharedPtr getSingleton(Server::Admin& admin, + Singleton::Manager& singleton_manager, + Event::Dispatcher& main_thread_dispatcher); + + /** + * Register a new extension config to the handler so that it can be admin managed. + * @param config supplies the config to register. + * @param config_id supplies the ID to use for managing the configuration. Multiple extensions + * can use the same ID so they can be managed in aggregate (e.g., an HTTP filter on + * many listeners). + */ + void registerConfig(ExtensionConfig& config, const std::string& config_id); + + /** + * Unregister an extension config from the handler. + * @param config supplies the previously registered config. + */ + void unregisterConfig(ExtensionConfig& config); + + // Extensions::Common::Tap::Sink + void submitBufferedTrace(std::shared_ptr trace) override; + +private: + struct AttachedRequest { + AttachedRequest(const std::string& config_id, Server::AdminStream* admin_stream) + : config_id_(config_id), admin_stream_(admin_stream) {} + + std::string config_id_; + Server::AdminStream* admin_stream_; + }; + + Http::Code handler(absl::string_view path_and_query, Http::HeaderMap& response_headers, + Buffer::Instance& response, Server::AdminStream& admin_stream); + + Server::Admin& admin_; + Event::Dispatcher& main_thread_dispatcher_; + std::unordered_map> config_id_map_; + absl::optional attached_request_; +}; + +} // namespace Tap +} // namespace Common +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/common/tap/tap.h b/source/extensions/common/tap/tap.h new file mode 100644 index 000000000000..17a58e4f274c --- /dev/null +++ b/source/extensions/common/tap/tap.h @@ -0,0 +1,61 @@ +#pragma once + +#include "envoy/service/tap/v2alpha/common.pb.h" + +#include "common/protobuf/protobuf.h" + +namespace Envoy { +namespace Extensions { +namespace Common { +namespace Tap { + +/** + * Sink for sending tap messages. + */ +class Sink { +public: + virtual ~Sink() = default; + + /** + * Send a fully buffered trace to the sink. + * @param trace supplies the trace to send. The trace message is opaque and is assumed to be a + * discrete trace message (as opposed to a portion of a larger trace that should be + * aggregated). + */ + virtual void submitBufferedTrace(std::shared_ptr trace) PURE; +}; + +/** + * Generic configuration for a tap extension (filter, transport socket, etc.). + */ +class ExtensionConfig { +public: + virtual ~ExtensionConfig() = default; + + /** + * @return the ID to use for admin extension configuration tracking (if applicable). + */ + virtual const std::string& adminId() PURE; + + /** + * Clear any active tap configuration. + */ + virtual void clearTapConfig() PURE; + + /** + * Install a new tap configuration. + * @param proto_config supplies the generic tap config to install. Not all configuration fields + * may be applicable to an extension (e.g. HTTP fields). The extension is free to fail + * the configuration load via exception if it wishes. + * @param admin_streamer supplies the singleton admin sink to use for output if the configuration + * specifies that output type. May not be used if the configuration does not specify + * admin output. May be nullptr if admin is not used to supply the config. + */ + virtual void newTapConfig(envoy::service::tap::v2alpha::TapConfig&& proto_config, + Sink* admin_streamer) PURE; +}; + +} // namespace Tap +} // namespace Common +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/common/tap/tap_config_base.h b/source/extensions/common/tap/tap_config_base.h new file mode 100644 index 000000000000..49e71e59c48d --- /dev/null +++ b/source/extensions/common/tap/tap_config_base.h @@ -0,0 +1,25 @@ +#pragma once + +#include "envoy/service/tap/v2alpha/common.pb.h" + +namespace Envoy { +namespace Extensions { +namespace Common { +namespace Tap { + +/** + * Base class for all tap configurations. + * TODO(mattklein123): This class will handle common functionality such as rate limiting, etc. + */ +class TapConfigBaseImpl { +protected: + TapConfigBaseImpl(envoy::service::tap::v2alpha::TapConfig&& proto_config) + : proto_config_(std::move(proto_config)) {} + + const envoy::service::tap::v2alpha::TapConfig proto_config_; +}; + +} // namespace Tap +} // namespace Common +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/extensions_build_config.bzl b/source/extensions/extensions_build_config.bzl index 4cf8281e9852..4b4474642987 100644 --- a/source/extensions/extensions_build_config.bzl +++ b/source/extensions/extensions_build_config.bzl @@ -42,6 +42,7 @@ EXTENSIONS = { "envoy.filters.http.rbac": "//source/extensions/filters/http/rbac:config", "envoy.filters.http.router": "//source/extensions/filters/http/router:config", "envoy.filters.http.squash": "//source/extensions/filters/http/squash:config", + "envoy.filters.http.tap": "//source/extensions/filters/http/tap:config", # # Listener filters diff --git a/source/extensions/filters/http/tap/BUILD b/source/extensions/filters/http/tap/BUILD new file mode 100644 index 000000000000..84d259329bdc --- /dev/null +++ b/source/extensions/filters/http/tap/BUILD @@ -0,0 +1,61 @@ +licenses(["notice"]) # Apache 2 + +# L7 HTTP Tap filter +# Public docs: docs/root/configuration/http_filters/tap_filter.rst + +load( + "//bazel:envoy_build_system.bzl", + "envoy_cc_library", + "envoy_package", +) + +envoy_package() + +envoy_cc_library( + name = "tap_config_interface", + hdrs = ["tap_config.h"], + deps = [ + "//include/envoy/http:header_map_interface", + "//source/extensions/common/tap:tap_interface", + "@envoy_api//envoy/service/tap/v2alpha:common_cc", + ], +) + +envoy_cc_library( + name = "tap_config_impl", + srcs = ["tap_config_impl.cc"], + hdrs = ["tap_config_impl.h"], + deps = [ + ":tap_config_interface", + "//source/common/http:header_utility_lib", + "//source/extensions/common/tap:tap_config_base", + "@envoy_api//envoy/data/tap/v2alpha:http_cc", + ], +) + +envoy_cc_library( + name = "tap_filter_lib", + srcs = ["tap_filter.cc"], + hdrs = ["tap_filter.h"], + deps = [ + ":tap_config_interface", + "//include/envoy/http:filter_interface", + "//include/envoy/thread_local:thread_local_interface", + "//source/extensions/common/tap:admin", + "@envoy_api//envoy/config/filter/http/tap/v2alpha:tap_cc", + ], +) + +envoy_cc_library( + name = "config", + srcs = ["config.cc"], + hdrs = ["config.h"], + deps = [ + ":tap_config_impl", + ":tap_filter_lib", + "//include/envoy/registry", + "//source/extensions/filters/http:well_known_names", + "//source/extensions/filters/http/common:factory_base_lib", + "@envoy_api//envoy/config/filter/http/tap/v2alpha:tap_cc", + ], +) diff --git a/source/extensions/filters/http/tap/config.cc b/source/extensions/filters/http/tap/config.cc new file mode 100644 index 000000000000..184d539fd263 --- /dev/null +++ b/source/extensions/filters/http/tap/config.cc @@ -0,0 +1,46 @@ +#include "extensions/filters/http/tap/config.h" + +#include "envoy/registry/registry.h" + +#include "extensions/filters/http/tap/tap_config_impl.h" +#include "extensions/filters/http/tap/tap_filter.h" + +namespace Envoy { +namespace Extensions { +namespace HttpFilters { +namespace TapFilter { + +class HttpTapConfigFactoryImpl : public HttpTapConfigFactory { +public: + HttpTapConfigSharedPtr + createHttpConfigFromProto(envoy::service::tap::v2alpha::TapConfig&& proto_config, + Extensions::Common::Tap::Sink* admin_streamer) override { + return std::make_shared(std::move(proto_config), admin_streamer); + } +}; + +Http::FilterFactoryCb TapFilterFactory::createFilterFactoryFromProtoTyped( + const envoy::config::filter::http::tap::v2alpha::Tap& proto_config, + const std::string& stats_prefix, Server::Configuration::FactoryContext& context) { + + FilterConfigSharedPtr filter_config(new FilterConfigImpl( + proto_config, stats_prefix, std::make_unique(), context.scope(), + context.admin(), context.singletonManager(), context.threadLocal(), context.dispatcher())); + return [filter_config](Http::FilterChainFactoryCallbacks& callbacks) -> void { + auto filter = std::make_shared(filter_config); + callbacks.addStreamFilter(filter); + callbacks.addAccessLogHandler(filter); + }; +} + +/** + * Static registration for the tap filter. @see RegisterFactory. + */ +static Registry::RegisterFactory + register_; + +} // namespace TapFilter +} // namespace HttpFilters +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/filters/http/tap/config.h b/source/extensions/filters/http/tap/config.h new file mode 100644 index 000000000000..1819217ba3fc --- /dev/null +++ b/source/extensions/filters/http/tap/config.h @@ -0,0 +1,31 @@ +#pragma once + +#include "envoy/config/filter/http/tap/v2alpha/tap.pb.h" +#include "envoy/config/filter/http/tap/v2alpha/tap.pb.validate.h" + +#include "extensions/filters/http/common/factory_base.h" +#include "extensions/filters/http/well_known_names.h" + +namespace Envoy { +namespace Extensions { +namespace HttpFilters { +namespace TapFilter { + +/** + * Config registration for the tap filter. + */ +class TapFilterFactory + : public Common::FactoryBase { +public: + TapFilterFactory() : FactoryBase(HttpFilterNames::get().Tap) {} + +private: + Http::FilterFactoryCb createFilterFactoryFromProtoTyped( + const envoy::config::filter::http::tap::v2alpha::Tap& proto_config, + const std::string& stats_prefix, Server::Configuration::FactoryContext& context) override; +}; + +} // namespace TapFilter +} // namespace HttpFilters +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/filters/http/tap/tap_config.h b/source/extensions/filters/http/tap/tap_config.h new file mode 100644 index 000000000000..305fbbdfbd1d --- /dev/null +++ b/source/extensions/filters/http/tap/tap_config.h @@ -0,0 +1,76 @@ +#pragma once + +#include "envoy/service/tap/v2alpha/common.pb.h" + +#include "extensions/common/tap/tap.h" + +namespace Envoy { +namespace Extensions { +namespace HttpFilters { +namespace TapFilter { + +/** + * Per-request/stream HTTP tap implementation. Abstractly handles all request lifecycle events in + * order to tap if the configuration matches. + */ +class HttpPerRequestTapper { +public: + virtual ~HttpPerRequestTapper() = default; + + /** + * Called when request headers are received. + */ + virtual void onRequestHeaders(const Http::HeaderMap& headers) PURE; + + /** + * Called when response headers are received. + */ + virtual void onResponseHeaders(const Http::HeaderMap& headers) PURE; + + /** + * Called when the request is being destroyed and is being logged. + * @return whether the request was tapped or not. + */ + virtual bool onLog(const Http::HeaderMap* request_headers, + const Http::HeaderMap* response_headers) PURE; +}; + +typedef std::unique_ptr HttpPerRequestTapperPtr; + +/** + * Abstract HTTP tap configuration. + */ +class HttpTapConfig { +public: + virtual ~HttpTapConfig() = default; + + /** + * @return a new per-request HTTP tapper which is used to handle tapping of a discrete request. + */ + virtual HttpPerRequestTapperPtr newPerRequestTapper() PURE; +}; + +typedef std::shared_ptr HttpTapConfigSharedPtr; + +/** + * Configuration factory for the HTTP tap filter. + */ +class HttpTapConfigFactory { +public: + virtual ~HttpTapConfigFactory() = default; + + /** + * @return a new configuration given a raw tap service config proto. See + * Extensions::Common::Tap::ExtensionConfig::newTapConfig() for param info. + */ + virtual HttpTapConfigSharedPtr + createHttpConfigFromProto(envoy::service::tap::v2alpha::TapConfig&& proto_config, + Extensions::Common::Tap::Sink* admin_streamer) PURE; +}; + +typedef std::unique_ptr HttpTapConfigFactoryPtr; + +} // namespace TapFilter +} // namespace HttpFilters +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/filters/http/tap/tap_config_impl.cc b/source/extensions/filters/http/tap/tap_config_impl.cc new file mode 100644 index 000000000000..397577be39de --- /dev/null +++ b/source/extensions/filters/http/tap/tap_config_impl.cc @@ -0,0 +1,127 @@ +#include "extensions/filters/http/tap/tap_config_impl.h" + +#include "envoy/data/tap/v2alpha/http.pb.h" + +#include "common/common/assert.h" + +namespace Envoy { +namespace Extensions { +namespace HttpFilters { +namespace TapFilter { + +HttpTapConfigImpl::HttpTapConfigImpl(envoy::service::tap::v2alpha::TapConfig&& proto_config, + Common::Tap::Sink* admin_streamer) + : Extensions::Common::Tap::TapConfigBaseImpl(std::move(proto_config)), + admin_streamer_(admin_streamer) { + // TODO(mattklein123): The streaming admin output sink is the only currently supported sink. + ASSERT(admin_streamer); + ASSERT(proto_config_.output_config().sinks()[0].has_streaming_admin()); + + for (const auto& proto_match_config : proto_config_.match_configs()) { + match_configs_.emplace_back(); + MatchConfig& match_config = match_configs_.back(); + match_config.id_ = proto_match_config.match_id(); + + if (proto_match_config.http_match_config().has_request_match_config()) { + match_config.request_config_ = RequestMatchConfig(); + for (const auto& header_match : + proto_match_config.http_match_config().request_match_config().headers()) { + match_config.request_config_.value().headers_to_match_.emplace_back(header_match); + } + } + + if (proto_match_config.http_match_config().has_response_match_config()) { + match_config.response_config_ = ResponseMatchConfig(); + for (const auto& header_match : + proto_match_config.http_match_config().response_match_config().headers()) { + match_config.response_config_.value().headers_to_match_.emplace_back(header_match); + } + } + } +} + +void HttpTapConfigImpl::matchesRequestHeaders(const Http::HeaderMap& headers, + std::vector& statuses) { + ASSERT(match_configs_.size() == statuses.size()); + for (uint64_t i = 0; i < match_configs_.size(); i++) { + statuses[i].request_matched_ = true; + if (match_configs_[i].request_config_.has_value()) { + statuses[i].request_matched_ = Http::HeaderUtility::matchHeaders( + headers, match_configs_[i].request_config_.value().headers_to_match_); + } + } +} + +void HttpTapConfigImpl::matchesResponseHeaders(const Http::HeaderMap& headers, + std::vector& statuses) { + ASSERT(match_configs_.size() == statuses.size()); + for (uint64_t i = 0; i < match_configs_.size(); i++) { + statuses[i].response_matched_ = true; + if (match_configs_[i].response_config_.has_value()) { + statuses[i].response_matched_ = Http::HeaderUtility::matchHeaders( + headers, match_configs_[i].response_config_.value().headers_to_match_); + } + } +} + +HttpPerRequestTapperPtr HttpTapConfigImpl::newPerRequestTapper() { + return std::make_unique(shared_from_this()); +} + +void HttpPerRequestTapperImpl::onRequestHeaders(const Http::HeaderMap& headers) { + config_->matchesRequestHeaders(headers, statuses_); +} + +void HttpPerRequestTapperImpl::onResponseHeaders(const Http::HeaderMap& headers) { + config_->matchesResponseHeaders(headers, statuses_); +} + +bool HttpPerRequestTapperImpl::onLog(const Http::HeaderMap* request_headers, + const Http::HeaderMap* response_headers) { + absl::optional match_index; + for (uint64_t i = 0; i < statuses_.size(); i++) { + const auto& status = statuses_[i]; + if (status.request_matched_ && status.response_matched_) { + match_index = i; + break; + } + } + + if (!match_index.has_value()) { + return false; + } + + auto trace = std::make_shared(); + trace->set_match_id(config_->configs()[match_index.value()].id_); + request_headers->iterate( + [](const Http::HeaderEntry& header, void* context) -> Http::HeaderMap::Iterate { + envoy::data::tap::v2alpha::HttpBufferedTrace& trace = + *reinterpret_cast(context); + auto& new_header = *trace.add_request_headers(); + new_header.set_key(header.key().c_str()); + new_header.set_value(header.value().c_str()); + return Http::HeaderMap::Iterate::Continue; + }, + trace.get()); + if (response_headers != nullptr) { + response_headers->iterate( + [](const Http::HeaderEntry& header, void* context) -> Http::HeaderMap::Iterate { + envoy::data::tap::v2alpha::HttpBufferedTrace& trace = + *reinterpret_cast(context); + auto& new_header = *trace.add_response_headers(); + new_header.set_key(header.key().c_str()); + new_header.set_value(header.value().c_str()); + return Http::HeaderMap::Iterate::Continue; + }, + trace.get()); + } + + ENVOY_LOG(debug, "submitting buffered trace sink"); + config_->sink().submitBufferedTrace(trace); + return true; +} + +} // namespace TapFilter +} // namespace HttpFilters +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/filters/http/tap/tap_config_impl.h b/source/extensions/filters/http/tap/tap_config_impl.h new file mode 100644 index 000000000000..f1436a973a1f --- /dev/null +++ b/source/extensions/filters/http/tap/tap_config_impl.h @@ -0,0 +1,80 @@ +#pragma once + +#include "envoy/http/header_map.h" + +#include "common/common/logger.h" +#include "common/http/header_utility.h" + +#include "extensions/common/tap/tap_config_base.h" +#include "extensions/filters/http/tap/tap_config.h" + +namespace Envoy { +namespace Extensions { +namespace HttpFilters { +namespace TapFilter { + +class HttpTapConfigImpl : Extensions::Common::Tap::TapConfigBaseImpl, + public HttpTapConfig, + public std::enable_shared_from_this { +public: + struct RequestMatchConfig { + std::vector headers_to_match_; + }; + + struct ResponseMatchConfig { + std::vector headers_to_match_; + }; + + struct MatchConfig { + std::string id_; + absl::optional request_config_; + absl::optional response_config_; + }; + + struct MatchStatus { + bool request_matched_{}; + bool response_matched_{}; + }; + + HttpTapConfigImpl(envoy::service::tap::v2alpha::TapConfig&& proto_config, + Extensions::Common::Tap::Sink* admin_streamer); + + const std::vector& configs() { return match_configs_; } + void matchesRequestHeaders(const Http::HeaderMap& headers, std::vector& statuses); + void matchesResponseHeaders(const Http::HeaderMap& headers, std::vector& statuses); + Extensions::Common::Tap::Sink& sink() { + // TODO(mattklein123): When we support multiple sinks, select the right one. Right now + // it must be admin. + return *admin_streamer_; + } + + // TapFilter::HttpTapConfig + HttpPerRequestTapperPtr newPerRequestTapper() override; + +private: + Extensions::Common::Tap::Sink* admin_streamer_; + std::vector match_configs_; +}; + +typedef std::shared_ptr HttpTapConfigImplSharedPtr; + +class HttpPerRequestTapperImpl : public HttpPerRequestTapper, Logger::Loggable { +public: + HttpPerRequestTapperImpl(HttpTapConfigImplSharedPtr config) + : config_(config), statuses_(config_->configs().size()) {} + + // TapFilter::HttpPerRequestTapper + void onRequestHeaders(const Http::HeaderMap& headers) override; + void onResponseHeaders(const Http::HeaderMap& headers) override; + bool onLog(const Http::HeaderMap* request_headers, + const Http::HeaderMap* response_headers) override; + +private: + HttpTapConfigImplSharedPtr config_; + std::vector statuses_; +}; + +} // namespace TapFilter +} // namespace HttpFilters +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/filters/http/tap/tap_filter.cc b/source/extensions/filters/http/tap/tap_filter.cc new file mode 100644 index 000000000000..772a758e0ab0 --- /dev/null +++ b/source/extensions/filters/http/tap/tap_filter.cc @@ -0,0 +1,86 @@ +#include "extensions/filters/http/tap/tap_filter.h" + +namespace Envoy { +namespace Extensions { +namespace HttpFilters { +namespace TapFilter { + +FilterConfigImpl::FilterConfigImpl( + const envoy::config::filter::http::tap::v2alpha::Tap& proto_config, + const std::string& stats_prefix, HttpTapConfigFactoryPtr&& config_factory, Stats::Scope& scope, + Server::Admin& admin, Singleton::Manager& singleton_manager, ThreadLocal::SlotAllocator& tls, + Event::Dispatcher& main_thread_dispatcher) + : proto_config_(proto_config), stats_(Filter::generateStats(stats_prefix, scope)), + config_factory_(std::move(config_factory)), tls_slot_(tls.allocateSlot()) { + + // TODO(mattklein123): Admin is the only supported config type currently. + ASSERT(proto_config_.has_admin_config()); + + admin_handler_ = Extensions::Common::Tap::AdminHandler::getSingleton(admin, singleton_manager, + main_thread_dispatcher); + admin_handler_->registerConfig(*this, proto_config_.admin_config().config_id()); + ENVOY_LOG(debug, "initializing tap filter with admin endpoint (config_id={})", + proto_config_.admin_config().config_id()); + + tls_slot_->set([](Event::Dispatcher&) -> ThreadLocal::ThreadLocalObjectSharedPtr { + return std::make_shared(); + }); +} + +FilterConfigImpl::~FilterConfigImpl() { + if (admin_handler_) { + admin_handler_->unregisterConfig(*this); + } +} + +HttpTapConfigSharedPtr FilterConfigImpl::currentConfig() { + return tls_slot_->getTyped().config_; +} + +const std::string& FilterConfigImpl::adminId() { + ASSERT(proto_config_.has_admin_config()); + return proto_config_.admin_config().config_id(); +} + +void FilterConfigImpl::clearTapConfig() { + tls_slot_->runOnAllThreads([this] { tls_slot_->getTyped().config_ = nullptr; }); +} + +void FilterConfigImpl::newTapConfig(envoy::service::tap::v2alpha::TapConfig&& proto_config, + Common::Tap::Sink* admin_streamer) { + HttpTapConfigSharedPtr new_config = + config_factory_->createHttpConfigFromProto(std::move(proto_config), admin_streamer); + tls_slot_->runOnAllThreads( + [this, new_config] { tls_slot_->getTyped().config_ = new_config; }); +} + +FilterStats Filter::generateStats(const std::string& prefix, Stats::Scope& scope) { + std::string final_prefix = prefix + "tap."; + return {ALL_TAP_FILTER_STATS(POOL_COUNTER_PREFIX(scope, final_prefix))}; +} + +Http::FilterHeadersStatus Filter::decodeHeaders(Http::HeaderMap& headers, bool) { + if (tapper_ != nullptr) { + tapper_->onRequestHeaders(headers); + } + return Http::FilterHeadersStatus::Continue; +} + +Http::FilterHeadersStatus Filter::encodeHeaders(Http::HeaderMap& headers, bool) { + if (tapper_ != nullptr) { + tapper_->onResponseHeaders(headers); + } + return Http::FilterHeadersStatus::Continue; +} + +void Filter::log(const Http::HeaderMap* request_headers, const Http::HeaderMap* response_headers, + const Http::HeaderMap*, const StreamInfo::StreamInfo&) { + if (tapper_ != nullptr && tapper_->onLog(request_headers, response_headers)) { + config_->stats().rq_tapped_.inc(); + } +} + +} // namespace TapFilter +} // namespace HttpFilters +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/filters/http/tap/tap_filter.h b/source/extensions/filters/http/tap/tap_filter.h new file mode 100644 index 000000000000..e7d2953ae7aa --- /dev/null +++ b/source/extensions/filters/http/tap/tap_filter.h @@ -0,0 +1,139 @@ +#pragma once + +#include "envoy/config/filter/http/tap/v2alpha/tap.pb.h" +#include "envoy/http/filter.h" +#include "envoy/service/tap/v2alpha/common.pb.h" +#include "envoy/stats/scope.h" +#include "envoy/stats/stats_macros.h" +#include "envoy/thread_local/thread_local.h" + +#include "extensions/common/tap/admin.h" +#include "extensions/filters/http/tap/tap_config.h" + +namespace Envoy { +namespace Extensions { +namespace HttpFilters { +namespace TapFilter { + +/** + * All stats for the tap filter. @see stats_macros.h + */ +// clang-format off +#define ALL_TAP_FILTER_STATS(COUNTER) \ + COUNTER(rq_tapped) +// clang-format on + +/** + * Wrapper struct for tap filter stats. @see stats_macros.h + */ +struct FilterStats { + ALL_TAP_FILTER_STATS(GENERATE_COUNTER_STRUCT) +}; + +/** + * Abstract filter configuration. + */ +class FilterConfig { +public: + virtual ~FilterConfig() = default; + + /** + * @return the current tap configuration if there is one. + */ + virtual HttpTapConfigSharedPtr currentConfig() PURE; + + /** + * @return the filter stats. + */ + virtual FilterStats& stats() PURE; +}; + +typedef std::shared_ptr FilterConfigSharedPtr; + +/** + * Configuration for the tap filter. + */ +class FilterConfigImpl : public FilterConfig, + public Extensions::Common::Tap::ExtensionConfig, + Logger::Loggable { +public: + FilterConfigImpl(const envoy::config::filter::http::tap::v2alpha::Tap& proto_config, + const std::string& stats_prefix, HttpTapConfigFactoryPtr&& config_factory, + Stats::Scope& scope, Server::Admin& admin, Singleton::Manager& singleton_manager, + ThreadLocal::SlotAllocator& tls, Event::Dispatcher& main_thread_dispatcher); + ~FilterConfigImpl(); + + // FilterConfig + HttpTapConfigSharedPtr currentConfig() override; + FilterStats& stats() override { return stats_; } + + // Extensions::Common::Tap::ExtensionConfig + void clearTapConfig() override; + const std::string& adminId() override; + void newTapConfig(envoy::service::tap::v2alpha::TapConfig&& proto_config, + Extensions::Common::Tap::Sink* admin_streamer) override; + +private: + struct TlsFilterConfig : public ThreadLocal::ThreadLocalObject { + HttpTapConfigSharedPtr config_; + }; + + const envoy::config::filter::http::tap::v2alpha::Tap proto_config_; + FilterStats stats_; + HttpTapConfigFactoryPtr config_factory_; + ThreadLocal::SlotPtr tls_slot_; + Extensions::Common::Tap::AdminHandlerSharedPtr admin_handler_; +}; + +/** + * HTTP tap filter. + */ +class Filter : public Http::StreamFilter, public AccessLog::Instance { +public: + Filter(FilterConfigSharedPtr config) + : config_(config), + tapper_(config_->currentConfig() ? config_->currentConfig()->newPerRequestTapper() + : nullptr) {} + + static FilterStats generateStats(const std::string& prefix, Stats::Scope& scope); + + // Http::StreamFilterBase + void onDestroy() override {} + + // Http::StreamDecoderFilter + Http::FilterHeadersStatus decodeHeaders(Http::HeaderMap& headers, bool end_stream) override; + Http::FilterDataStatus decodeData(Buffer::Instance&, bool) override { + return Http::FilterDataStatus::Continue; + } + Http::FilterTrailersStatus decodeTrailers(Http::HeaderMap&) override { + return Http::FilterTrailersStatus::Continue; + } + void setDecoderFilterCallbacks(Http::StreamDecoderFilterCallbacks&) override {} + + // Http::StreamEncoderFilter + Http::FilterHeadersStatus encode100ContinueHeaders(Http::HeaderMap&) override { + return Http::FilterHeadersStatus::Continue; + } + Http::FilterHeadersStatus encodeHeaders(Http::HeaderMap& headers, bool end_stream) override; + Http::FilterDataStatus encodeData(Buffer::Instance&, bool) override { + return Http::FilterDataStatus::Continue; + } + Http::FilterTrailersStatus encodeTrailers(Http::HeaderMap&) override { + return Http::FilterTrailersStatus::Continue; + } + void setEncoderFilterCallbacks(Http::StreamEncoderFilterCallbacks&) override {} + + // AccessLog::Instance + void log(const Http::HeaderMap* request_headers, const Http::HeaderMap* response_headers, + const Http::HeaderMap* response_trailers, + const StreamInfo::StreamInfo& stream_info) override; + +private: + FilterConfigSharedPtr config_; + HttpPerRequestTapperPtr tapper_; +}; + +} // namespace TapFilter +} // 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 cda225da77a1..d3585225d752 100644 --- a/source/extensions/filters/http/well_known_names.h +++ b/source/extensions/filters/http/well_known_names.h @@ -50,6 +50,8 @@ class HttpFilterNameValues { const std::string JwtAuthn = "envoy.filters.http.jwt_authn"; // Header to metadata filter const std::string HeaderToMetadata = "envoy.filters.http.header_to_metadata"; + // Tap filter + const std::string Tap = "envoy.filters.http.tap"; // Converts names from v1 to v2 const Config::V1Converter v1_converter_; diff --git a/source/server/http/admin.cc b/source/server/http/admin.cc index 4eba0592cf02..5b6ec4be7838 100644 --- a/source/server/http/admin.cc +++ b/source/server/http/admin.cc @@ -197,7 +197,9 @@ Http::FilterDataStatus AdminFilter::decodeData(Buffer::Instance&, bool end_strea onComplete(); } - return Http::FilterDataStatus::StopIterationNoBuffer; + // Currently we generically buffer all admin request data in case a handler wants to use it. + // If we ever support streaming admin requests we may need to revisit this. + return Http::FilterDataStatus::StopIterationAndBuffer; } Http::FilterTrailersStatus AdminFilter::decodeTrailers(Http::HeaderMap&) { @@ -220,6 +222,8 @@ Http::StreamDecoderFilterCallbacks& AdminFilter::getDecoderFilterCallbacks() con return *callbacks_; } +const Buffer::Instance* AdminFilter::getRequestBody() const { return callbacks_->decodingBuffer(); } + const Http::HeaderMap& AdminFilter::getRequestHeaders() const { ASSERT(request_headers_ != nullptr); return *request_headers_; diff --git a/source/server/http/admin.h b/source/server/http/admin.h index 979be5a9c718..ed567c31c769 100644 --- a/source/server/http/admin.h +++ b/source/server/http/admin.h @@ -340,6 +340,7 @@ class AdminFilter : public Http::StreamDecoderFilter, void setEndStreamOnComplete(bool end_stream) override { end_stream_on_complete_ = end_stream; } void addOnDestroyCallback(std::function cb) override; Http::StreamDecoderFilterCallbacks& getDecoderFilterCallbacks() const override; + const Buffer::Instance* getRequestBody() const override; const Http::HeaderMap& getRequestHeaders() const override; private: diff --git a/source/server/worker_impl.cc b/source/server/worker_impl.cc index a7ec17545405..3f4c78aa751d 100644 --- a/source/server/worker_impl.cc +++ b/source/server/worker_impl.cc @@ -105,8 +105,7 @@ void WorkerImpl::threadRoutine(GuardDog& guard_dog) { // We must close all active connections before we actually exit the thread. This prevents any // destructors from running on the main thread which might reference thread locals. Destroying - // the handler does this as well as destroying the dispatcher which purges the delayed deletion - // list. + // the handler does this which additionally purges the dispatcher delayed deletion list. handler_.reset(); tls_.shutdownThread(); watchdog.reset(); diff --git a/test/extensions/common/tap/BUILD b/test/extensions/common/tap/BUILD new file mode 100644 index 000000000000..ccea6eba1fb3 --- /dev/null +++ b/test/extensions/common/tap/BUILD @@ -0,0 +1,18 @@ +licenses(["notice"]) # Apache 2 + +load( + "//bazel:envoy_build_system.bzl", + "envoy_cc_test", + "envoy_package", +) + +envoy_package() + +envoy_cc_test( + name = "admin_test", + srcs = ["admin_test.cc"], + deps = [ + "//source/extensions/common/tap:admin", + "//test/mocks/server:server_mocks", + ], +) diff --git a/test/extensions/common/tap/admin_test.cc b/test/extensions/common/tap/admin_test.cc new file mode 100644 index 000000000000..01e8a1d169cd --- /dev/null +++ b/test/extensions/common/tap/admin_test.cc @@ -0,0 +1,99 @@ +#include "extensions/common/tap/admin.h" + +#include "test/mocks/server/mocks.h" + +#include "gtest/gtest.h" + +using testing::_; +using testing::Return; +using testing::SaveArg; + +namespace Envoy { +namespace Extensions { +namespace Common { +namespace Tap { +namespace { + +class MockExtensionConfig : public ExtensionConfig { +public: + MOCK_METHOD0(adminId, const std::string&()); + MOCK_METHOD0(clearTapConfig, void()); + MOCK_METHOD2(newTapConfig, + void(envoy::service::tap::v2alpha::TapConfig&& proto_config, Sink* admin_streamer)); +}; + +class AdminHandlerTest : public testing::Test { +public: + AdminHandlerTest() { + EXPECT_CALL(admin_, addHandler("/tap", "tap filter control", _, true, true)) + .WillOnce(DoAll(SaveArg<2>(&cb_), Return(true))); + handler_ = std::make_unique(admin_, main_thread_dispatcher_); + } + + ~AdminHandlerTest() { EXPECT_CALL(admin_, removeHandler("/tap")).WillOnce(Return(true)); } + + Server::MockAdmin admin_; + Event::MockDispatcher main_thread_dispatcher_; + std::unique_ptr handler_; + Server::Admin::HandlerCb cb_; + Http::TestHeaderMapImpl response_headers_; + Buffer::OwnedImpl response_; + Server::MockAdminStream admin_stream_; + + const std::string admin_request_yaml_ = + R"EOF( +config_id: test_config_id +tap_config: + match_configs: + - match_id: foo_match_id + output_config: + sinks: + - streaming_admin: {} +)EOF"; +}; + +// Request with no config body. +TEST_F(AdminHandlerTest, NoBody) { + EXPECT_CALL(admin_stream_, getRequestBody()); + EXPECT_EQ(Http::Code::BadRequest, cb_("/tap", response_headers_, response_, admin_stream_)); + EXPECT_EQ("/tap requires a JSON/YAML body", response_.toString()); +} + +// Request with a config body that doesn't parse/verify. +TEST_F(AdminHandlerTest, BadBody) { + Buffer::OwnedImpl bad_body("hello"); + EXPECT_CALL(admin_stream_, getRequestBody()).WillRepeatedly(Return(&bad_body)); + EXPECT_EQ(Http::Code::BadRequest, cb_("/tap", response_headers_, response_, admin_stream_)); + EXPECT_EQ("Unable to convert YAML as JSON: hello", response_.toString()); +} + +// Request that references an unknown config ID. +TEST_F(AdminHandlerTest, UnknownConfigId) { + Buffer::OwnedImpl body(admin_request_yaml_); + EXPECT_CALL(admin_stream_, getRequestBody()).WillRepeatedly(Return(&body)); + EXPECT_EQ(Http::Code::BadRequest, cb_("/tap", response_headers_, response_, admin_stream_)); + EXPECT_EQ("Unknown config id 'test_config_id'. No extension has registered with this id.", + response_.toString()); +} + +// Request while there is already an active tap session. +TEST_F(AdminHandlerTest, RequestTapWhileAttached) { + MockExtensionConfig extension_config; + handler_->registerConfig(extension_config, "test_config_id"); + + Buffer::OwnedImpl body(admin_request_yaml_); + EXPECT_CALL(admin_stream_, getRequestBody()).WillRepeatedly(Return(&body)); + EXPECT_CALL(extension_config, newTapConfig(_, handler_.get())); + EXPECT_CALL(admin_stream_, setEndStreamOnComplete(false)); + EXPECT_CALL(admin_stream_, addOnDestroyCallback(_)); + EXPECT_EQ(Http::Code::OK, cb_("/tap", response_headers_, response_, admin_stream_)); + + EXPECT_EQ(Http::Code::BadRequest, cb_("/tap", response_headers_, response_, admin_stream_)); + EXPECT_EQ("An attached /tap admin stream already exists. Detach it.", response_.toString()); +} + +} // namespace +} // namespace Tap +} // namespace Common +} // namespace Extensions +} // namespace Envoy diff --git a/test/extensions/filters/http/tap/BUILD b/test/extensions/filters/http/tap/BUILD new file mode 100644 index 000000000000..a6e7f916d6c2 --- /dev/null +++ b/test/extensions/filters/http/tap/BUILD @@ -0,0 +1,33 @@ +licenses(["notice"]) # Apache 2 + +load( + "//bazel:envoy_build_system.bzl", + "envoy_package", +) +load( + "//test/extensions:extensions_build_system.bzl", + "envoy_extension_cc_test", +) + +envoy_package() + +envoy_extension_cc_test( + name = "tap_filter_test", + srcs = ["tap_filter_test.cc"], + extension_name = "envoy.filters.http.tap", + deps = [ + "//source/extensions/filters/http/tap:tap_filter_lib", + "//test/mocks/stream_info:stream_info_mocks", + "//test/test_common:utility_lib", + ], +) + +envoy_extension_cc_test( + name = "tap_filter_integration_test", + srcs = ["tap_filter_integration_test.cc"], + extension_name = "envoy.filters.http.tap", + deps = [ + "//source/extensions/filters/http/tap:config", + "//test/integration:http_integration_lib", + ], +) diff --git a/test/extensions/filters/http/tap/tap_filter_integration_test.cc b/test/extensions/filters/http/tap/tap_filter_integration_test.cc new file mode 100644 index 000000000000..a97ca09a79b5 --- /dev/null +++ b/test/extensions/filters/http/tap/tap_filter_integration_test.cc @@ -0,0 +1,210 @@ +#include "envoy/data/tap/v2alpha/http.pb.h" + +#include "test/integration/http_integration.h" + +#include "gtest/gtest.h" + +namespace Envoy { +namespace { + +class TapIntegrationTest : public HttpIntegrationTest, + public testing::TestWithParam { +public: + TapIntegrationTest() + : HttpIntegrationTest(Http::CodecClient::Type::HTTP1, GetParam(), realTime()) {} + + void initializeFilter(const std::string& filter_config) { + config_helper_.addFilter(filter_config); + initialize(); + } + + const envoy::data::tap::v2alpha::HttpBufferedTrace::Header* + findHeader(const std::string& key, + const Protobuf::RepeatedPtrField& + headers) { + for (const auto& header : headers) { + if (header.key() == key) { + return &header; + } + } + + return nullptr; + } +}; + +INSTANTIATE_TEST_CASE_P(IpVersions, TapIntegrationTest, + testing::ValuesIn(TestEnvironment::getIpVersionsForTest()), + TestUtility::ipTestParamsToString); + +// Verify a basic tap flow using the admin handler. +TEST_P(TapIntegrationTest, AdminBasicFlow) { + const std::string FILTER_CONFIG = + R"EOF( +name: envoy.filters.http.tap +config: + admin_config: + config_id: test_config_id +)EOF"; + + initializeFilter(FILTER_CONFIG); + + // Initial request/response with no tap. + codec_client_ = makeHttpConnection(makeClientConnection(lookupPort("http"))); + const Http::TestHeaderMapImpl request_headers_tap{{":method", "GET"}, + {":path", "/"}, + {":scheme", "http"}, + {":authority", "host"}, + {"foo", "bar"}}; + IntegrationStreamDecoderPtr decoder = codec_client_->makeHeaderOnlyRequest(request_headers_tap); + waitForNextUpstreamRequest(); + const Http::TestHeaderMapImpl response_headers_no_tap{{":status", "200"}}; + upstream_request_->encodeHeaders(response_headers_no_tap, true); + decoder->waitForEndStream(); + + const std::string admin_request_yaml = + R"EOF( +config_id: test_config_id +tap_config: + match_configs: + - match_id: request_match_id + http_match_config: + request_match_config: + headers: + - name: foo + exact_match: bar + - match_id: response_match_id + http_match_config: + response_match_config: + headers: + - name: bar + exact_match: baz + output_config: + sinks: + - streaming_admin: {} +)EOF"; + + // Setup a tap and disconnect it without any request/response. + IntegrationCodecClientPtr admin_client_ = + makeHttpConnection(makeClientConnection(lookupPort("admin"))); + const Http::TestHeaderMapImpl admin_request_headers{ + {":method", "POST"}, {":path", "/tap"}, {":scheme", "http"}, {":authority", "host"}}; + IntegrationStreamDecoderPtr admin_response = + admin_client_->makeRequestWithBody(admin_request_headers, admin_request_yaml); + admin_response->waitForHeaders(); + EXPECT_STREQ("200", admin_response->headers().Status()->value().c_str()); + EXPECT_FALSE(admin_response->complete()); + admin_client_->close(); + test_server_->waitForGaugeEq("http.admin.downstream_rq_active", 0); + + // Second request/response with no tap. + decoder = codec_client_->makeHeaderOnlyRequest(request_headers_tap); + waitForNextUpstreamRequest(); + upstream_request_->encodeHeaders(response_headers_no_tap, true); + decoder->waitForEndStream(); + + // Setup the tap again and leave it open. + admin_client_ = makeHttpConnection(makeClientConnection(lookupPort("admin"))); + admin_response = admin_client_->makeRequestWithBody(admin_request_headers, admin_request_yaml); + admin_response->waitForHeaders(); + EXPECT_STREQ("200", admin_response->headers().Status()->value().c_str()); + EXPECT_FALSE(admin_response->complete()); + + // Do a request which should tap, matching on request headers. + decoder = codec_client_->makeHeaderOnlyRequest(request_headers_tap); + waitForNextUpstreamRequest(); + upstream_request_->encodeHeaders(response_headers_no_tap, true); + decoder->waitForEndStream(); + + // Wait for the tap message. + admin_response->waitForBodyData(1); + envoy::data::tap::v2alpha::HttpBufferedTrace trace; + MessageUtil::loadFromYaml(admin_response->body(), trace); + EXPECT_EQ("request_match_id", trace.match_id()); + EXPECT_EQ(trace.request_headers().size(), 9); + EXPECT_EQ(trace.response_headers().size(), 5); + admin_response->clearBody(); + + // Do a request which should not tap. + const Http::TestHeaderMapImpl request_headers_no_tap{ + {":method", "GET"}, {":path", "/"}, {":scheme", "http"}, {":authority", "host"}}; + decoder = codec_client_->makeHeaderOnlyRequest(request_headers_no_tap); + waitForNextUpstreamRequest(); + upstream_request_->encodeHeaders(response_headers_no_tap, true); + decoder->waitForEndStream(); + + // Do a request which should tap, matching on response headers. + decoder = codec_client_->makeHeaderOnlyRequest(request_headers_no_tap); + waitForNextUpstreamRequest(); + const Http::TestHeaderMapImpl response_headers_tap{{":status", "200"}, {"bar", "baz"}}; + upstream_request_->encodeHeaders(response_headers_tap, true); + decoder->waitForEndStream(); + + // Wait for the tap message. + admin_response->waitForBodyData(1); + MessageUtil::loadFromYaml(admin_response->body(), trace); + EXPECT_EQ("response_match_id", trace.match_id()); + EXPECT_EQ(trace.request_headers().size(), 8); + EXPECT_EQ("0", findHeader("content-length", trace.request_headers())->value()); + EXPECT_EQ(trace.response_headers().size(), 6); + EXPECT_NE(nullptr, findHeader("date", trace.response_headers())); + EXPECT_EQ("baz", findHeader("bar", trace.response_headers())->value()); + + admin_client_->close(); + test_server_->waitForGaugeEq("http.admin.downstream_rq_active", 0); + + // Now setup a tap that matches on logical AND. + const std::string admin_request_yaml2 = + R"EOF( +config_id: test_config_id +tap_config: + match_configs: + - match_id: both_match_id + http_match_config: + request_match_config: + headers: + - name: foo + exact_match: bar + response_match_config: + headers: + - name: bar + exact_match: baz + output_config: + sinks: + - streaming_admin: {} +)EOF"; + + admin_client_ = makeHttpConnection(makeClientConnection(lookupPort("admin"))); + admin_response = admin_client_->makeRequestWithBody(admin_request_headers, admin_request_yaml2); + admin_response->waitForHeaders(); + EXPECT_STREQ("200", admin_response->headers().Status()->value().c_str()); + EXPECT_FALSE(admin_response->complete()); + + // Do a request that matches, but the response does not match. No tap. + decoder = codec_client_->makeHeaderOnlyRequest(request_headers_tap); + waitForNextUpstreamRequest(); + upstream_request_->encodeHeaders(response_headers_no_tap, true); + decoder->waitForEndStream(); + + // Do a request that doesn't match, but the response does match. No tap. + decoder = codec_client_->makeHeaderOnlyRequest(request_headers_no_tap); + waitForNextUpstreamRequest(); + upstream_request_->encodeHeaders(response_headers_tap, true); + decoder->waitForEndStream(); + + // Do a request that matches and a response that matches. Should tap. + decoder = codec_client_->makeHeaderOnlyRequest(request_headers_tap); + waitForNextUpstreamRequest(); + upstream_request_->encodeHeaders(response_headers_tap, true); + decoder->waitForEndStream(); + + // Wait for the tap message. + admin_response->waitForBodyData(1); + MessageUtil::loadFromYaml(admin_response->body(), trace); + EXPECT_EQ("both_match_id", trace.match_id()); + + admin_client_->close(); + EXPECT_EQ(3UL, test_server_->counter("http.config_test.tap.rq_tapped")->value()); +} + +} // namespace +} // namespace Envoy diff --git a/test/extensions/filters/http/tap/tap_filter_test.cc b/test/extensions/filters/http/tap/tap_filter_test.cc new file mode 100644 index 000000000000..2904d28183da --- /dev/null +++ b/test/extensions/filters/http/tap/tap_filter_test.cc @@ -0,0 +1,128 @@ +#include "extensions/filters/http/tap/tap_filter.h" + +#include "test/mocks/stream_info/mocks.h" +#include "test/test_common/utility.h" + +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +using testing::InSequence; +using testing::Return; + +namespace Envoy { +namespace Extensions { +namespace HttpFilters { +namespace TapFilter { +namespace { + +class MockFilterConfig : public FilterConfig { +public: + MOCK_METHOD0(currentConfig, HttpTapConfigSharedPtr()); + FilterStats& stats() override { return stats_; } + + Stats::IsolatedStoreImpl stats_store_; + FilterStats stats_{Filter::generateStats("foo", stats_store_)}; +}; + +class MockHttpTapConfig : public HttpTapConfig { +public: + HttpPerRequestTapperPtr newPerRequestTapper() override { + return HttpPerRequestTapperPtr{newPerRequestTapper_()}; + } + + MOCK_METHOD0(newPerRequestTapper_, HttpPerRequestTapper*()); +}; + +class MockHttpPerRequestTapper : public HttpPerRequestTapper { +public: + MOCK_METHOD1(onRequestHeaders, void(const Http::HeaderMap& headers)); + MOCK_METHOD1(onResponseHeaders, void(const Http::HeaderMap& headers)); + MOCK_METHOD2(onLog, bool(const Http::HeaderMap* request_headers, + const Http::HeaderMap* response_headers)); +}; + +class TapFilterTest : public testing::Test { +public: + void setup(bool has_config) { + if (has_config) { + http_tap_config_ = std::make_shared(); + } + + EXPECT_CALL(*filter_config_, currentConfig()).WillRepeatedly(Return(http_tap_config_)); + if (has_config) { + http_per_request_tapper_ = new MockHttpPerRequestTapper(); + EXPECT_CALL(*http_tap_config_, newPerRequestTapper_()) + .WillOnce(Return(http_per_request_tapper_)); + } + + filter_ = std::make_unique(filter_config_); + } + + std::shared_ptr filter_config_{new MockFilterConfig()}; + std::shared_ptr http_tap_config_; + MockHttpPerRequestTapper* http_per_request_tapper_; + std::unique_ptr filter_; + StreamInfo::MockStreamInfo stream_info_; +}; + +// Verify filter functionality when there is no tap config. +TEST_F(TapFilterTest, NoConfig) { + InSequence s; + setup(false); + + Http::TestHeaderMapImpl request_headers; + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers, false)); + Buffer::OwnedImpl request_body; + EXPECT_EQ(Http::FilterDataStatus::Continue, filter_->decodeData(request_body, false)); + Http::TestHeaderMapImpl request_trailers; + EXPECT_EQ(Http::FilterTrailersStatus::Continue, filter_->decodeTrailers(request_trailers)); + + Http::TestHeaderMapImpl response_headers; + EXPECT_EQ(Http::FilterHeadersStatus::Continue, + filter_->encode100ContinueHeaders(response_headers)); + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->encodeHeaders(response_headers, false)); + Buffer::OwnedImpl response_body; + EXPECT_EQ(Http::FilterDataStatus::Continue, filter_->encodeData(response_body, false)); + Http::TestHeaderMapImpl response_trailers; + EXPECT_EQ(Http::FilterTrailersStatus::Continue, filter_->encodeTrailers(response_trailers)); + + filter_->log(&request_headers, &response_headers, &response_trailers, stream_info_); +} + +// Verify filter functionality when there is a tap config. +TEST_F(TapFilterTest, Config) { + InSequence s; + setup(true); + + Http::TestHeaderMapImpl request_headers; + EXPECT_CALL(*http_per_request_tapper_, onRequestHeaders(_)); + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers, false)); + Buffer::OwnedImpl request_body; + EXPECT_EQ(Http::FilterDataStatus::Continue, filter_->decodeData(request_body, false)); + Http::TestHeaderMapImpl request_trailers; + EXPECT_EQ(Http::FilterTrailersStatus::Continue, filter_->decodeTrailers(request_trailers)); + + Http::TestHeaderMapImpl response_headers; + EXPECT_EQ(Http::FilterHeadersStatus::Continue, + filter_->encode100ContinueHeaders(response_headers)); + EXPECT_CALL(*http_per_request_tapper_, onResponseHeaders(_)); + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->encodeHeaders(response_headers, false)); + Buffer::OwnedImpl response_body; + EXPECT_EQ(Http::FilterDataStatus::Continue, filter_->encodeData(response_body, false)); + Http::TestHeaderMapImpl response_trailers; + EXPECT_EQ(Http::FilterTrailersStatus::Continue, filter_->encodeTrailers(response_trailers)); + + EXPECT_CALL(*http_per_request_tapper_, onLog(&request_headers, &response_headers)) + .WillOnce(Return(true)); + filter_->log(&request_headers, &response_headers, &response_trailers, stream_info_); + EXPECT_EQ(1UL, filter_config_->stats_.rq_tapped_.value()); + + // Workaround InSequence/shared_ptr mock leak. + EXPECT_TRUE(testing::Mock::VerifyAndClearExpectations(http_tap_config_.get())); +} + +} // namespace +} // namespace TapFilter +} // namespace HttpFilters +} // namespace Extensions +} // namespace Envoy diff --git a/test/integration/http_integration.cc b/test/integration/http_integration.cc index 5d2017c02f5a..5c87937619e3 100644 --- a/test/integration/http_integration.cc +++ b/test/integration/http_integration.cc @@ -103,11 +103,17 @@ IntegrationCodecClient::makeHeaderOnlyRequest(const Http::HeaderMap& headers) { IntegrationStreamDecoderPtr IntegrationCodecClient::makeRequestWithBody(const Http::HeaderMap& headers, uint64_t body_size) { + return makeRequestWithBody(headers, std::string(body_size, 'a')); +} + +IntegrationStreamDecoderPtr +IntegrationCodecClient::makeRequestWithBody(const Http::HeaderMap& headers, + const std::string& body) { auto response = std::make_unique(dispatcher_); Http::StreamEncoder& encoder = newStream(*response); encoder.getStream().addCallbacks(*response); encoder.encodeHeaders(headers, false); - Buffer::OwnedImpl data(std::string(body_size, 'a')); + Buffer::OwnedImpl data(body); encoder.encodeData(data, true); flushWrite(); return response; diff --git a/test/integration/http_integration.h b/test/integration/http_integration.h index 1d5357bbd828..0c06732832fb 100644 --- a/test/integration/http_integration.h +++ b/test/integration/http_integration.h @@ -25,6 +25,8 @@ class IntegrationCodecClient : public Http::CodecClientProd { IntegrationStreamDecoderPtr makeHeaderOnlyRequest(const Http::HeaderMap& headers); IntegrationStreamDecoderPtr makeRequestWithBody(const Http::HeaderMap& headers, uint64_t body_size); + IntegrationStreamDecoderPtr makeRequestWithBody(const Http::HeaderMap& headers, + const std::string& body); bool sawGoAway() const { return saw_goaway_; } bool connected() const { return connected_; } void sendData(Http::StreamEncoder& encoder, absl::string_view data, bool end_stream); diff --git a/test/integration/integration.cc b/test/integration/integration.cc index 94bea9dcd7b8..7bb9a470bed5 100644 --- a/test/integration/integration.cc +++ b/test/integration/integration.cc @@ -91,12 +91,7 @@ void IntegrationStreamDecoder::decodeHeaders(Http::HeaderMapPtr&& headers, bool void IntegrationStreamDecoder::decodeData(Buffer::Instance& data, bool end_stream) { saw_end_stream_ = end_stream; - uint64_t num_slices = data.getRawSlices(nullptr, 0); - STACK_ARRAY(slices, Buffer::RawSlice, num_slices); - data.getRawSlices(slices.begin(), num_slices); - for (const Buffer::RawSlice& slice : slices) { - body_.append(static_cast(slice.mem_), slice.len_); - } + body_ += data.toString(); if (end_stream && waiting_for_end_stream_) { dispatcher_.exit(); diff --git a/test/integration/integration.h b/test/integration/integration.h index 85d7c93d39ac..3c6a2f0d09c7 100644 --- a/test/integration/integration.h +++ b/test/integration/integration.h @@ -40,6 +40,7 @@ class IntegrationStreamDecoder : public Http::StreamDecoder, public Http::Stream void waitForBodyData(uint64_t size); void waitForEndStream(); void waitForReset(); + void clearBody() { body_.clear(); } // Http::StreamDecoder void decode100ContinueHeaders(Http::HeaderMapPtr&& headers) override; diff --git a/test/mocks/server/mocks.cc b/test/mocks/server/mocks.cc index 5729d1608a3e..ddc8dc3b1a77 100644 --- a/test/mocks/server/mocks.cc +++ b/test/mocks/server/mocks.cc @@ -56,6 +56,9 @@ MockAdmin::MockAdmin() { } MockAdmin::~MockAdmin() {} +MockAdminStream::MockAdminStream() {} +MockAdminStream::~MockAdminStream() {} + MockDrainManager::MockDrainManager() { ON_CALL(*this, startDrainSequence(_)).WillByDefault(SaveArg<0>(&drain_sequence_completion_)); } @@ -202,9 +205,6 @@ MockHealthCheckerFactoryContext::MockHealthCheckerFactoryContext() { MockHealthCheckerFactoryContext::~MockHealthCheckerFactoryContext() {} -MockAdminStream::MockAdminStream() {} -MockAdminStream::~MockAdminStream() {} - } // namespace Configuration } // namespace Server } // namespace Envoy diff --git a/test/mocks/server/mocks.h b/test/mocks/server/mocks.h index 9e929c826bbd..927182bb70eb 100644 --- a/test/mocks/server/mocks.h +++ b/test/mocks/server/mocks.h @@ -138,6 +138,19 @@ class MockAdmin : public Admin { NiceMock config_tracker_; }; +class MockAdminStream : public AdminStream { +public: + MockAdminStream(); + ~MockAdminStream(); + + MOCK_METHOD1(setEndStreamOnComplete, void(bool)); + MOCK_METHOD1(addOnDestroyCallback, void(std::function)); + MOCK_CONST_METHOD0(getRequestBody, const Buffer::Instance*()); + MOCK_CONST_METHOD0(getRequestHeaders, Http::HeaderMap&()); + MOCK_CONST_METHOD0(getDecoderFilterCallbacks, + NiceMock&()); +}; + class MockDrainManager : public DrainManager { public: MockDrainManager(); @@ -501,18 +514,6 @@ class MockHealthCheckerFactoryContext : public virtual HealthCheckerFactoryConte testing::NiceMock* event_logger_{}; }; -class MockAdminStream : public AdminStream { -public: - MockAdminStream(); - ~MockAdminStream(); - - MOCK_METHOD1(setEndStreamOnComplete, void(bool)); - MOCK_METHOD1(addOnDestroyCallback, void(std::function)); - MOCK_CONST_METHOD0(getRequestHeaders, Http::HeaderMap&()); - MOCK_CONST_METHOD0(getDecoderFilterCallbacks, - NiceMock&()); -}; - } // namespace Configuration } // namespace Server } // namespace Envoy diff --git a/test/server/http/admin_test.cc b/test/server/http/admin_test.cc index 0a0c2eec2a47..39687bf54988 100644 --- a/test/server/http/admin_test.cc +++ b/test/server/http/admin_test.cc @@ -558,22 +558,27 @@ INSTANTIATE_TEST_CASE_P(IpVersions, AdminFilterTest, TEST_P(AdminFilterTest, HeaderOnly) { EXPECT_CALL(callbacks_, encodeHeaders_(_, false)); - filter_.decodeHeaders(request_headers_, true); + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + filter_.decodeHeaders(request_headers_, true)); } TEST_P(AdminFilterTest, Body) { - filter_.decodeHeaders(request_headers_, false); + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + filter_.decodeHeaders(request_headers_, false)); Buffer::OwnedImpl data("hello"); EXPECT_CALL(callbacks_, encodeHeaders_(_, false)); - filter_.decodeData(data, true); + EXPECT_EQ(Http::FilterDataStatus::StopIterationAndBuffer, filter_.decodeData(data, true)); } TEST_P(AdminFilterTest, Trailers) { - filter_.decodeHeaders(request_headers_, false); + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + filter_.decodeHeaders(request_headers_, false)); Buffer::OwnedImpl data("hello"); - filter_.decodeData(data, false); + EXPECT_EQ(Http::FilterDataStatus::StopIterationAndBuffer, filter_.decodeData(data, false)); EXPECT_CALL(callbacks_, encodeHeaders_(_, false)); - filter_.decodeTrailers(request_headers_); + EXPECT_CALL(callbacks_, decodingBuffer()); + filter_.getRequestBody(); + EXPECT_EQ(Http::FilterTrailersStatus::StopIteration, filter_.decodeTrailers(request_headers_)); } class AdminInstanceTest : public testing::TestWithParam { From 68a8371014e800724f6f932c711108ed4e927210 Mon Sep 17 00:00:00 2001 From: Matt Klein Date: Mon, 7 Jan 2019 09:07:19 -0700 Subject: [PATCH 02/26] release notes Signed-off-by: Matt Klein --- docs/root/intro/version_history.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst index 39de547552fc..4296cc96c30a 100644 --- a/docs/root/intro/version_history.rst +++ b/docs/root/intro/version_history.rst @@ -8,6 +8,7 @@ Version history * config: removed deprecated_v1 sds_config from :ref:`Bootstrap config `. * cors: added :ref:`filter_enabled & shadow_enabled RuntimeFractionalPercent flags ` to filter. * http: added new grpc_http1_reverse_bridge filter for converting gRPC requests into HTTP/1.1 requests. +* tap: added new :ref:`HTTP tap filter `. * tls: enabled TLS 1.3 on the server-side (non-FIPS builds). 1.9.0 From 67334c8ac1c5a55c31728696af22dbf9ad2fbe20 Mon Sep 17 00:00:00 2001 From: Matt Klein Date: Mon, 7 Jan 2019 10:04:36 -0700 Subject: [PATCH 03/26] fix build Signed-off-by: Matt Klein --- test/extensions/stats_sinks/hystrix/hystrix_test.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/extensions/stats_sinks/hystrix/hystrix_test.cc b/test/extensions/stats_sinks/hystrix/hystrix_test.cc index acf9754dd5ab..baf24727a0b3 100644 --- a/test/extensions/stats_sinks/hystrix/hystrix_test.cc +++ b/test/extensions/stats_sinks/hystrix/hystrix_test.cc @@ -519,7 +519,7 @@ TEST_F(HystrixSinkTest, HystrixEventStreamHandler) { Http::HeaderMapImpl response_headers; - NiceMock admin_stream_mock; + NiceMock admin_stream_mock; NiceMock connection_mock; auto addr_instance_ = Envoy::Network::Utility::parseInternetAddress("2.3.4.5", 123, false); From 426b10d1e5ec1d52bb34d09a109afbbf443feeb3 Mon Sep 17 00:00:00 2001 From: Matt Klein Date: Mon, 7 Jan 2019 10:53:08 -0700 Subject: [PATCH 04/26] tidy fixes Signed-off-by: Matt Klein --- source/extensions/common/tap/admin.h | 8 +-- .../extensions/filters/http/tap/tap_config.h | 6 +-- .../filters/http/tap/tap_config_impl.h | 4 +- .../extensions/filters/http/tap/tap_filter.cc | 13 ++--- .../extensions/filters/http/tap/tap_filter.h | 8 +-- test/mocks/server/mocks.cc | 49 +++++++++---------- 6 files changed, 44 insertions(+), 44 deletions(-) diff --git a/source/extensions/common/tap/admin.h b/source/extensions/common/tap/admin.h index a73a42ee8e58..eb7dfef19dae 100644 --- a/source/extensions/common/tap/admin.h +++ b/source/extensions/common/tap/admin.h @@ -11,7 +11,7 @@ namespace Common { namespace Tap { class AdminHandler; -typedef std::shared_ptr AdminHandlerSharedPtr; +using AdminHandlerSharedPtr = std::shared_ptr; /** * Singleton /tap admin handler for admin management of tap configurations and output. This @@ -24,7 +24,7 @@ class AdminHandler : public Singleton::Instance, Logger::Loggable { public: AdminHandler(Server::Admin& admin, Event::Dispatcher& main_thread_dispatcher); - ~AdminHandler(); + ~AdminHandler() override; /** * Get the singleton admin handler. The handler will be created if it doesn't already exist, @@ -54,8 +54,8 @@ class AdminHandler : public Singleton::Instance, private: struct AttachedRequest { - AttachedRequest(const std::string& config_id, Server::AdminStream* admin_stream) - : config_id_(config_id), admin_stream_(admin_stream) {} + AttachedRequest(std::string config_id, Server::AdminStream* admin_stream) + : config_id_(std::move(config_id)), admin_stream_(admin_stream) {} std::string config_id_; Server::AdminStream* admin_stream_; diff --git a/source/extensions/filters/http/tap/tap_config.h b/source/extensions/filters/http/tap/tap_config.h index 305fbbdfbd1d..c82615743e08 100644 --- a/source/extensions/filters/http/tap/tap_config.h +++ b/source/extensions/filters/http/tap/tap_config.h @@ -35,7 +35,7 @@ class HttpPerRequestTapper { const Http::HeaderMap* response_headers) PURE; }; -typedef std::unique_ptr HttpPerRequestTapperPtr; +using HttpPerRequestTapperPtr = std::unique_ptr; /** * Abstract HTTP tap configuration. @@ -50,7 +50,7 @@ class HttpTapConfig { virtual HttpPerRequestTapperPtr newPerRequestTapper() PURE; }; -typedef std::shared_ptr HttpTapConfigSharedPtr; +using HttpTapConfigSharedPtr = std::shared_ptr; /** * Configuration factory for the HTTP tap filter. @@ -68,7 +68,7 @@ class HttpTapConfigFactory { Extensions::Common::Tap::Sink* admin_streamer) PURE; }; -typedef std::unique_ptr HttpTapConfigFactoryPtr; +using HttpTapConfigFactoryPtr = std::unique_ptr; } // namespace TapFilter } // namespace HttpFilters diff --git a/source/extensions/filters/http/tap/tap_config_impl.h b/source/extensions/filters/http/tap/tap_config_impl.h index f1436a973a1f..6450227fe6c1 100644 --- a/source/extensions/filters/http/tap/tap_config_impl.h +++ b/source/extensions/filters/http/tap/tap_config_impl.h @@ -56,12 +56,12 @@ class HttpTapConfigImpl : Extensions::Common::Tap::TapConfigBaseImpl, std::vector match_configs_; }; -typedef std::shared_ptr HttpTapConfigImplSharedPtr; +using HttpTapConfigImplSharedPtr = std::shared_ptr; class HttpPerRequestTapperImpl : public HttpPerRequestTapper, Logger::Loggable { public: HttpPerRequestTapperImpl(HttpTapConfigImplSharedPtr config) - : config_(config), statuses_(config_->configs().size()) {} + : config_(std::move(config)), statuses_(config_->configs().size()) {} // TapFilter::HttpPerRequestTapper void onRequestHeaders(const Http::HeaderMap& headers) override; diff --git a/source/extensions/filters/http/tap/tap_filter.cc b/source/extensions/filters/http/tap/tap_filter.cc index 772a758e0ab0..e838c9fadabb 100644 --- a/source/extensions/filters/http/tap/tap_filter.cc +++ b/source/extensions/filters/http/tap/tap_filter.cc @@ -5,12 +5,13 @@ namespace Extensions { namespace HttpFilters { namespace TapFilter { -FilterConfigImpl::FilterConfigImpl( - const envoy::config::filter::http::tap::v2alpha::Tap& proto_config, - const std::string& stats_prefix, HttpTapConfigFactoryPtr&& config_factory, Stats::Scope& scope, - Server::Admin& admin, Singleton::Manager& singleton_manager, ThreadLocal::SlotAllocator& tls, - Event::Dispatcher& main_thread_dispatcher) - : proto_config_(proto_config), stats_(Filter::generateStats(stats_prefix, scope)), +FilterConfigImpl::FilterConfigImpl(envoy::config::filter::http::tap::v2alpha::Tap proto_config, + const std::string& stats_prefix, + HttpTapConfigFactoryPtr&& config_factory, Stats::Scope& scope, + Server::Admin& admin, Singleton::Manager& singleton_manager, + ThreadLocal::SlotAllocator& tls, + Event::Dispatcher& main_thread_dispatcher) + : proto_config_(std::move(proto_config)), stats_(Filter::generateStats(stats_prefix, scope)), config_factory_(std::move(config_factory)), tls_slot_(tls.allocateSlot()) { // TODO(mattklein123): Admin is the only supported config type currently. diff --git a/source/extensions/filters/http/tap/tap_filter.h b/source/extensions/filters/http/tap/tap_filter.h index e7d2953ae7aa..45362ae7198e 100644 --- a/source/extensions/filters/http/tap/tap_filter.h +++ b/source/extensions/filters/http/tap/tap_filter.h @@ -48,7 +48,7 @@ class FilterConfig { virtual FilterStats& stats() PURE; }; -typedef std::shared_ptr FilterConfigSharedPtr; +using FilterConfigSharedPtr = std::shared_ptr; /** * Configuration for the tap filter. @@ -57,11 +57,11 @@ class FilterConfigImpl : public FilterConfig, public Extensions::Common::Tap::ExtensionConfig, Logger::Loggable { public: - FilterConfigImpl(const envoy::config::filter::http::tap::v2alpha::Tap& proto_config, + FilterConfigImpl(envoy::config::filter::http::tap::v2alpha::Tap proto_config, const std::string& stats_prefix, HttpTapConfigFactoryPtr&& config_factory, Stats::Scope& scope, Server::Admin& admin, Singleton::Manager& singleton_manager, ThreadLocal::SlotAllocator& tls, Event::Dispatcher& main_thread_dispatcher); - ~FilterConfigImpl(); + ~FilterConfigImpl() override; // FilterConfig HttpTapConfigSharedPtr currentConfig() override; @@ -91,7 +91,7 @@ class FilterConfigImpl : public FilterConfig, class Filter : public Http::StreamFilter, public AccessLog::Instance { public: Filter(FilterConfigSharedPtr config) - : config_(config), + : config_(std::move(config)), tapper_(config_->currentConfig() ? config_->currentConfig()->newPerRequestTapper() : nullptr) {} diff --git a/test/mocks/server/mocks.cc b/test/mocks/server/mocks.cc index ded10464e8bb..617080c63d79 100644 --- a/test/mocks/server/mocks.cc +++ b/test/mocks/server/mocks.cc @@ -39,7 +39,7 @@ MockOptions::MockOptions(const std::string& config_path) : config_path_(config_p return std::make_unique(); })); } -MockOptions::~MockOptions() {} +MockOptions::~MockOptions() = default; MockConfigTracker::MockConfigTracker() { ON_CALL(*this, add_(_, _)) @@ -49,40 +49,40 @@ MockConfigTracker::MockConfigTracker() { return new MockEntryOwner(); })); } -MockConfigTracker::~MockConfigTracker() {} +MockConfigTracker::~MockConfigTracker() = default; MockAdmin::MockAdmin() { ON_CALL(*this, getConfigTracker()).WillByDefault(testing::ReturnRef(config_tracker_)); } -MockAdmin::~MockAdmin() {} +MockAdmin::~MockAdmin() = default; -MockAdminStream::MockAdminStream() {} -MockAdminStream::~MockAdminStream() {} +MockAdminStream::MockAdminStream() = default; +MockAdminStream::~MockAdminStream() = default; MockDrainManager::MockDrainManager() { ON_CALL(*this, startDrainSequence(_)).WillByDefault(SaveArg<0>(&drain_sequence_completion_)); } -MockDrainManager::~MockDrainManager() {} +MockDrainManager::~MockDrainManager() = default; -MockWatchDog::MockWatchDog() {} -MockWatchDog::~MockWatchDog() {} +MockWatchDog::MockWatchDog() = default; +MockWatchDog::~MockWatchDog() = default; MockGuardDog::MockGuardDog() : watch_dog_(new NiceMock()) { ON_CALL(*this, createWatchDog(_)).WillByDefault(Return(watch_dog_)); } -MockGuardDog::~MockGuardDog() {} +MockGuardDog::~MockGuardDog() = default; MockHotRestart::MockHotRestart() { ON_CALL(*this, logLock()).WillByDefault(ReturnRef(log_lock_)); ON_CALL(*this, accessLogLock()).WillByDefault(ReturnRef(access_log_lock_)); ON_CALL(*this, statsAllocator()).WillByDefault(ReturnRef(stats_allocator_)); } -MockHotRestart::~MockHotRestart() {} +MockHotRestart::~MockHotRestart() = default; MockOverloadManager::MockOverloadManager() { ON_CALL(*this, getThreadLocalOverloadState()).WillByDefault(ReturnRef(overload_state_)); } -MockOverloadManager::~MockOverloadManager() {} +MockOverloadManager::~MockOverloadManager() = default; MockListenerComponentFactory::MockListenerComponentFactory() : socket_(std::make_shared>()) { @@ -97,13 +97,13 @@ MockListenerComponentFactory::MockListenerComponentFactory() return socket_; })); } -MockListenerComponentFactory::~MockListenerComponentFactory() {} +MockListenerComponentFactory::~MockListenerComponentFactory() = default; -MockListenerManager::MockListenerManager() {} -MockListenerManager::~MockListenerManager() {} +MockListenerManager::MockListenerManager() = default; +MockListenerManager::~MockListenerManager() = default; -MockWorkerFactory::MockWorkerFactory() {} -MockWorkerFactory::~MockWorkerFactory() {} +MockWorkerFactory::MockWorkerFactory() = default; +MockWorkerFactory::~MockWorkerFactory() = default; MockWorker::MockWorker() { ON_CALL(*this, addListener(_, _)) @@ -120,7 +120,7 @@ MockWorker::MockWorker() { remove_listener_completion_ = completion; })); } -MockWorker::~MockWorker() {} +MockWorker::~MockWorker() = default; MockInstance::MockInstance() : secret_manager_(new Secret::SecretManagerImpl()), cluster_manager_(timeSystem()), @@ -150,7 +150,7 @@ MockInstance::MockInstance() // ON_CALL(*this, timeSystem()).WillByDefault(ReturnRef(test_time_.timeSystem()));; } -MockInstance::~MockInstance() {} +MockInstance::~MockInstance() = default; namespace Configuration { @@ -162,7 +162,7 @@ MockMain::MockMain(int wd_miss, int wd_megamiss, int wd_kill, int wd_multikill) ON_CALL(*this, wdMultiKillTimeout()).WillByDefault(Return(wd_multikill_)); } -MockMain::~MockMain() {} +MockMain::~MockMain() = default; MockFactoryContext::MockFactoryContext() : singleton_manager_( @@ -184,16 +184,15 @@ MockFactoryContext::MockFactoryContext() ON_CALL(*this, overloadManager()).WillByDefault(ReturnRef(overload_manager_)); } -MockFactoryContext::~MockFactoryContext() {} +MockFactoryContext::~MockFactoryContext() = default; MockTransportSocketFactoryContext::MockTransportSocketFactoryContext() : secret_manager_(new Secret::SecretManagerImpl()) {} -MockTransportSocketFactoryContext::~MockTransportSocketFactoryContext() {} +MockTransportSocketFactoryContext::~MockTransportSocketFactoryContext() = default; -MockListenerFactoryContext::MockListenerFactoryContext() {} - -MockListenerFactoryContext::~MockListenerFactoryContext() {} +MockListenerFactoryContext::MockListenerFactoryContext() = default; +MockListenerFactoryContext::~MockListenerFactoryContext() = default; MockHealthCheckerFactoryContext::MockHealthCheckerFactoryContext() { event_logger_ = new NiceMock(); @@ -204,7 +203,7 @@ MockHealthCheckerFactoryContext::MockHealthCheckerFactoryContext() { ON_CALL(*this, eventLogger_()).WillByDefault(Return(event_logger_)); } -MockHealthCheckerFactoryContext::~MockHealthCheckerFactoryContext() {} +MockHealthCheckerFactoryContext::~MockHealthCheckerFactoryContext() = default; } // namespace Configuration } // namespace Server From 0280cac46ddbb517b9320bfe6d8713c306ef5b55 Mon Sep 17 00:00:00 2001 From: Matt Klein Date: Mon, 7 Jan 2019 12:43:11 -0700 Subject: [PATCH 05/26] bump googletest to try to workaround clang-tidy warning Signed-off-by: Matt Klein --- bazel/repository_locations.bzl | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/bazel/repository_locations.bzl b/bazel/repository_locations.bzl index 77a2925bfe4b..7483eb17b16f 100644 --- a/bazel/repository_locations.bzl +++ b/bazel/repository_locations.bzl @@ -146,9 +146,10 @@ REPOSITORY_LOCATIONS = dict( urls = ["https://files.pythonhosted.org/packages/f9/e7/4f80d582578f8489226370762d2cf6bc9381175d1929eba1754e03f70708/twitter.common.finagle-thrift-0.3.9.tar.gz"], ), com_google_googletest = dict( - sha256 = "9bf1fe5182a604b4135edc1a425ae356c9ad15e9b23f9f12a02e80184c3a249c", - strip_prefix = "googletest-release-1.8.1", - urls = ["https://github.com/google/googletest/archive/release-1.8.1.tar.gz"], + sha256 = "ed328c14e8b2e6555ad0d4b8d205c32fbd63962941a050ee0ff797fa3ab9b77c", + strip_prefix = "googletest-644319b9f06f6ca9bf69fe791be399061044bc3d", + # 2019-01-07: attempt to workaround clang-tidy warnings. + urls = ["https://github.com/google/googletest/archive/644319b9f06f6ca9bf69fe791be399061044bc3d.tar.gz"], ), com_google_protobuf = dict( sha256 = "3d610ac90f8fa16e12490088605c248b85fdaf23114ce4b3605cdf81f7823604", From e2c5c2b00795c49ea4d79e537349a6fad84da250 Mon Sep 17 00:00:00 2001 From: Matt Klein Date: Mon, 7 Jan 2019 13:33:19 -0700 Subject: [PATCH 06/26] comments Signed-off-by: Matt Klein --- api/envoy/data/tap/v2alpha/BUILD | 1 + api/envoy/data/tap/v2alpha/http.proto | 14 ++++---------- source/extensions/common/tap/BUILD | 4 ++++ .../http/tap/tap_filter_integration_test.cc | 5 ++--- 4 files changed, 11 insertions(+), 13 deletions(-) diff --git a/api/envoy/data/tap/v2alpha/BUILD b/api/envoy/data/tap/v2alpha/BUILD index c0cdd05c7f11..9aa8958228db 100644 --- a/api/envoy/data/tap/v2alpha/BUILD +++ b/api/envoy/data/tap/v2alpha/BUILD @@ -11,4 +11,5 @@ api_proto_library_internal( api_proto_library_internal( name = "http", srcs = ["http.proto"], + deps = ["//envoy/api/v2/core:base"], ) diff --git a/api/envoy/data/tap/v2alpha/http.proto b/api/envoy/data/tap/v2alpha/http.proto index 98f8b508b582..2c0a1a256df2 100644 --- a/api/envoy/data/tap/v2alpha/http.proto +++ b/api/envoy/data/tap/v2alpha/http.proto @@ -4,25 +4,19 @@ package envoy.data.tap.v2alpha; option java_package = "io.envoyproxy.envoy.data.tap.v2alpha"; option java_multiple_files = true; +import "envoy/api/v2/core/base.proto"; + // [#protodoc-title: HTTP tap data] // A fully buffered HTTP trace message. message HttpBufferedTrace { - message Header { - // Header key. - string key = 1; - - // Header value. - string value = 2; - } - // The match ID specified in the :ref:`match_id // ` field. string match_id = 1; // Request headers. - repeated Header request_headers = 2; + repeated api.v2.core.HeaderValue request_headers = 2; // Response headers. - repeated Header response_headers = 3; + repeated api.v2.core.HeaderValue response_headers = 3; } diff --git a/source/extensions/common/tap/BUILD b/source/extensions/common/tap/BUILD index 49c348ae40f9..b61783ef0950 100644 --- a/source/extensions/common/tap/BUILD +++ b/source/extensions/common/tap/BUILD @@ -11,6 +11,10 @@ envoy_package() envoy_cc_library( name = "tap_interface", hdrs = ["tap.h"], + deps = [ + "//source/common/protobuf", + "@envoy_api//envoy/service/tap/v2alpha:common_cc", + ], ) envoy_cc_library( diff --git a/test/extensions/filters/http/tap/tap_filter_integration_test.cc b/test/extensions/filters/http/tap/tap_filter_integration_test.cc index a97ca09a79b5..25bef9dbf43b 100644 --- a/test/extensions/filters/http/tap/tap_filter_integration_test.cc +++ b/test/extensions/filters/http/tap/tap_filter_integration_test.cc @@ -18,10 +18,9 @@ class TapIntegrationTest : public HttpIntegrationTest, initialize(); } - const envoy::data::tap::v2alpha::HttpBufferedTrace::Header* + const envoy::api::v2::core::HeaderValue* findHeader(const std::string& key, - const Protobuf::RepeatedPtrField& - headers) { + const Protobuf::RepeatedPtrField& headers) { for (const auto& header : headers) { if (header.key() == key) { return &header; From 9d6ee459bfdcbe5ec38d2c2825ff8c5c6fd5950b Mon Sep 17 00:00:00 2001 From: Matt Klein Date: Mon, 7 Jan 2019 13:49:19 -0700 Subject: [PATCH 07/26] revert googletest update Signed-off-by: Matt Klein --- bazel/repository_locations.bzl | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/bazel/repository_locations.bzl b/bazel/repository_locations.bzl index 7483eb17b16f..77a2925bfe4b 100644 --- a/bazel/repository_locations.bzl +++ b/bazel/repository_locations.bzl @@ -146,10 +146,9 @@ REPOSITORY_LOCATIONS = dict( urls = ["https://files.pythonhosted.org/packages/f9/e7/4f80d582578f8489226370762d2cf6bc9381175d1929eba1754e03f70708/twitter.common.finagle-thrift-0.3.9.tar.gz"], ), com_google_googletest = dict( - sha256 = "ed328c14e8b2e6555ad0d4b8d205c32fbd63962941a050ee0ff797fa3ab9b77c", - strip_prefix = "googletest-644319b9f06f6ca9bf69fe791be399061044bc3d", - # 2019-01-07: attempt to workaround clang-tidy warnings. - urls = ["https://github.com/google/googletest/archive/644319b9f06f6ca9bf69fe791be399061044bc3d.tar.gz"], + sha256 = "9bf1fe5182a604b4135edc1a425ae356c9ad15e9b23f9f12a02e80184c3a249c", + strip_prefix = "googletest-release-1.8.1", + urls = ["https://github.com/google/googletest/archive/release-1.8.1.tar.gz"], ), com_google_protobuf = dict( sha256 = "3d610ac90f8fa16e12490088605c248b85fdaf23114ce4b3605cdf81f7823604", From 4b9a6693959198e1907871eaa97d7ae82fc6b16f Mon Sep 17 00:00:00 2001 From: Matt Klein Date: Mon, 7 Jan 2019 16:45:20 -0700 Subject: [PATCH 08/26] tidy fixes Signed-off-by: Matt Klein --- source/extensions/common/tap/BUILD | 3 +++ source/extensions/common/tap/tap.h | 1 + source/extensions/filters/http/tap/tap_config.h | 1 + 3 files changed, 5 insertions(+) diff --git a/source/extensions/common/tap/BUILD b/source/extensions/common/tap/BUILD index b61783ef0950..5a80b993baed 100644 --- a/source/extensions/common/tap/BUILD +++ b/source/extensions/common/tap/BUILD @@ -20,6 +20,9 @@ envoy_cc_library( envoy_cc_library( name = "tap_config_base", hdrs = ["tap_config_base.h"], + deps = [ + "@envoy_api//envoy/service/tap/v2alpha:common_cc", + ], ) envoy_cc_library( diff --git a/source/extensions/common/tap/tap.h b/source/extensions/common/tap/tap.h index 17a58e4f274c..1a94be2df68d 100644 --- a/source/extensions/common/tap/tap.h +++ b/source/extensions/common/tap/tap.h @@ -1,5 +1,6 @@ #pragma once +#include "envoy/common/pure.h" #include "envoy/service/tap/v2alpha/common.pb.h" #include "common/protobuf/protobuf.h" diff --git a/source/extensions/filters/http/tap/tap_config.h b/source/extensions/filters/http/tap/tap_config.h index c82615743e08..a29fa5021d78 100644 --- a/source/extensions/filters/http/tap/tap_config.h +++ b/source/extensions/filters/http/tap/tap_config.h @@ -1,5 +1,6 @@ #pragma once +#include "envoy/common/pure.h" #include "envoy/service/tap/v2alpha/common.pb.h" #include "extensions/common/tap/tap.h" From 6f5da99310201e8c89fbb82aacd7d543eb14e5b4 Mon Sep 17 00:00:00 2001 From: Matt Klein Date: Mon, 7 Jan 2019 18:38:43 -0700 Subject: [PATCH 09/26] tidy fix Signed-off-by: Matt Klein --- source/extensions/filters/http/tap/tap_config.h | 1 + 1 file changed, 1 insertion(+) diff --git a/source/extensions/filters/http/tap/tap_config.h b/source/extensions/filters/http/tap/tap_config.h index a29fa5021d78..1104b4f2f21d 100644 --- a/source/extensions/filters/http/tap/tap_config.h +++ b/source/extensions/filters/http/tap/tap_config.h @@ -1,6 +1,7 @@ #pragma once #include "envoy/common/pure.h" +#include "envoy/http/header_map.h" #include "envoy/service/tap/v2alpha/common.pb.h" #include "extensions/common/tap/tap.h" From 4941d3d051fbf5d5fec016387dc78c2f23fd07f9 Mon Sep 17 00:00:00 2001 From: Matt Klein Date: Mon, 7 Jan 2019 20:49:02 -0700 Subject: [PATCH 10/26] fix test flake Signed-off-by: Matt Klein --- test/integration/integration.cc | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/integration/integration.cc b/test/integration/integration.cc index 7bb9a470bed5..d8c7d56e17b9 100644 --- a/test/integration/integration.cc +++ b/test/integration/integration.cc @@ -56,8 +56,10 @@ void IntegrationStreamDecoder::waitForHeaders() { void IntegrationStreamDecoder::waitForBodyData(uint64_t size) { ASSERT(body_data_waiting_length_ == 0); - body_data_waiting_length_ = size; - dispatcher_.run(Event::Dispatcher::RunType::Block); + if (body_.size() < size) { + body_data_waiting_length_ = size; + dispatcher_.run(Event::Dispatcher::RunType::Block); + } } void IntegrationStreamDecoder::waitForEndStream() { From 0f4f392556c9ac9ee95be48c02671919d9cd980f Mon Sep 17 00:00:00 2001 From: Matt Klein Date: Mon, 7 Jan 2019 22:05:05 -0700 Subject: [PATCH 11/26] another try at test fix Signed-off-by: Matt Klein --- test/integration/integration.cc | 5 +++-- test/integration/integration.h | 3 +++ test/integration/websocket_integration_test.cc | 2 +- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/test/integration/integration.cc b/test/integration/integration.cc index d8c7d56e17b9..6247e2bcaa85 100644 --- a/test/integration/integration.cc +++ b/test/integration/integration.cc @@ -56,8 +56,9 @@ void IntegrationStreamDecoder::waitForHeaders() { void IntegrationStreamDecoder::waitForBodyData(uint64_t size) { ASSERT(body_data_waiting_length_ == 0); - if (body_.size() < size) { - body_data_waiting_length_ = size; + body_data_waiting_length_ = size; + body_data_waiting_length_ -= std::min(body_data_waiting_length_, body_.size()); + if (body_data_waiting_length_ > 0) { dispatcher_.run(Event::Dispatcher::RunType::Block); } } diff --git a/test/integration/integration.h b/test/integration/integration.h index 3c6a2f0d09c7..7150d2e74d37 100644 --- a/test/integration/integration.h +++ b/test/integration/integration.h @@ -37,6 +37,9 @@ class IntegrationStreamDecoder : public Http::StreamDecoder, public Http::Stream const Http::MetadataMap& metadata_map() { return *metadata_map_; } void waitForContinueHeaders(); void waitForHeaders(); + // This function waits until body_ has at least size bytes in it (it might have more). clearBody() + // can be used if the previous body data is not relevant and the test wants to wait for a specific + // amount of new data without considering the existing body size. void waitForBodyData(uint64_t size); void waitForEndStream(); void waitForReset(); diff --git a/test/integration/websocket_integration_test.cc b/test/integration/websocket_integration_test.cc index 1ce08d0376d1..2be39ceb339e 100644 --- a/test/integration/websocket_integration_test.cc +++ b/test/integration/websocket_integration_test.cc @@ -444,7 +444,7 @@ TEST_P(WebsocketIntegrationTest, BidirectionalChunkedData) { codec_client_->sendData(*request_encoder_, "FinalClientPayload", false); ASSERT_TRUE(upstream_request_->waitForData(*dispatcher_, request_payload + "FinalClientPayload")); upstream_request_->encodeData("FinalServerPayload", false); - response_->waitForBodyData(5); + response_->waitForBodyData(response_->body().size() + 5); EXPECT_EQ(response_payload + "FinalServerPayload", response_->body()); // Clean up. From 149e9089a8716671049596e2ce761bb40ee60acf Mon Sep 17 00:00:00 2001 From: Matt Klein Date: Tue, 8 Jan 2019 08:43:33 -0700 Subject: [PATCH 12/26] fix osx Signed-off-by: Matt Klein --- test/integration/integration.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/integration/integration.cc b/test/integration/integration.cc index 6247e2bcaa85..9488b3f8af5a 100644 --- a/test/integration/integration.cc +++ b/test/integration/integration.cc @@ -57,7 +57,8 @@ void IntegrationStreamDecoder::waitForHeaders() { void IntegrationStreamDecoder::waitForBodyData(uint64_t size) { ASSERT(body_data_waiting_length_ == 0); body_data_waiting_length_ = size; - body_data_waiting_length_ -= std::min(body_data_waiting_length_, body_.size()); + body_data_waiting_length_ -= + std::min(body_data_waiting_length_, static_cast(body_.size())); if (body_data_waiting_length_ > 0) { dispatcher_.run(Event::Dispatcher::RunType::Block); } From eb25d484dff1e39b0dd3ddbb2f3b36a449622269 Mon Sep 17 00:00:00 2001 From: Matt Klein Date: Tue, 8 Jan 2019 19:34:48 -0700 Subject: [PATCH 13/26] use HTTP/2 for admin Signed-off-by: Matt Klein --- source/common/http/conn_manager_utility.cc | 70 ++++++++++++++----- source/common/http/conn_manager_utility.h | 33 +++++++-- source/common/http/http2/codec_impl.h | 4 +- .../network/http_connection_manager/config.cc | 29 +------- .../network/http_connection_manager/config.h | 14 ---- source/server/http/BUILD | 1 - source/server/http/admin.cc | 20 +++--- test/common/http/conn_manager_utility_test.cc | 45 ++++++++++++ .../http/tap/tap_filter_integration_test.cc | 12 ++-- .../http_connection_manager/config_test.cc | 44 ------------ test/server/http/admin_test.cc | 12 +++- 11 files changed, 155 insertions(+), 129 deletions(-) diff --git a/source/common/http/conn_manager_utility.cc b/source/common/http/conn_manager_utility.cc index 737b0eea54bd..5cdecd35ea43 100644 --- a/source/common/http/conn_manager_utility.cc +++ b/source/common/http/conn_manager_utility.cc @@ -8,6 +8,8 @@ #include "common/common/empty_string.h" #include "common/common/utility.h" #include "common/http/headers.h" +#include "common/http/http1/codec_impl.h" +#include "common/http/http2/codec_impl.h" #include "common/http/utility.h" #include "common/network/utility.h" #include "common/runtime/uuid_util.h" @@ -18,10 +20,40 @@ namespace Envoy { namespace Http { +std::string ConnectionManagerUtility::determineNextProtocol(Network::Connection& connection, + const Buffer::Instance& data) { + if (!connection.nextProtocol().empty()) { + return connection.nextProtocol(); + } + + // See if the data we have so far shows the HTTP/2 prefix. We ignore the case where someone sends + // us the first few bytes of the HTTP/2 prefix since in all public cases we use SSL/ALPN. For + // internal cases this should practically never happen. + if (-1 != data.search(Http2::CLIENT_MAGIC_PREFIX.c_str(), Http2::CLIENT_MAGIC_PREFIX.size(), 0)) { + return Http2::ALPN_STRING; + } + + return ""; +} + +ServerConnectionPtr ConnectionManagerUtility::autoCreateCodec(Network::Connection& connection, + const Buffer::Instance& data, + ServerConnectionCallbacks& callbacks, + Stats::Scope& scope, + const Http1Settings& http1_settings, + const Http2Settings& http2_settings) { + if (determineNextProtocol(connection, data) == Http2::ALPN_STRING) { + return ServerConnectionPtr{ + new Http2::ServerConnectionImpl(connection, callbacks, scope, http2_settings)}; + } else { + return ServerConnectionPtr{ + new Http1::ServerConnectionImpl(connection, callbacks, http1_settings)}; + } +} + Network::Address::InstanceConstSharedPtr ConnectionManagerUtility::mutateRequestHeaders( - Http::HeaderMap& request_headers, Network::Connection& connection, - ConnectionManagerConfig& config, const Router::Config& route_config, - Runtime::RandomGenerator& random, Runtime::Loader& runtime, + HeaderMap& request_headers, Network::Connection& connection, ConnectionManagerConfig& config, + const Router::Config& route_config, Runtime::RandomGenerator& random, Runtime::Loader& runtime, const LocalInfo::LocalInfo& local_info) { // If this is a Upgrade request, do not remove the Connection and Upgrade headers, // as we forward them verbatim to the upstream hosts. @@ -139,7 +171,7 @@ Network::Address::InstanceConstSharedPtr ConnectionManagerUtility::mutateRequest request_headers.removeEnvoyForceTrace(); request_headers.removeEnvoyIpTags(); - for (const Http::LowerCaseString& header : route_config.internalOnlyHeaders()) { + for (const LowerCaseString& header : route_config.internalOnlyHeaders()) { request_headers.remove(header); } } @@ -184,7 +216,7 @@ Network::Address::InstanceConstSharedPtr ConnectionManagerUtility::mutateRequest return final_remote_address; } -void ConnectionManagerUtility::mutateTracingRequestHeader(Http::HeaderMap& request_headers, +void ConnectionManagerUtility::mutateTracingRequestHeader(HeaderMap& request_headers, Runtime::Loader& runtime, ConnectionManagerConfig& config) { if (!config.tracingConfig() || !request_headers.RequestId()) { @@ -221,22 +253,22 @@ void ConnectionManagerUtility::mutateTracingRequestHeader(Http::HeaderMap& reque request_headers.RequestId()->value(x_request_id); } -void ConnectionManagerUtility::mutateXfccRequestHeader(Http::HeaderMap& request_headers, +void ConnectionManagerUtility::mutateXfccRequestHeader(HeaderMap& request_headers, Network::Connection& connection, ConnectionManagerConfig& config) { // When AlwaysForwardOnly is set, always forward the XFCC header without modification. - if (config.forwardClientCert() == Http::ForwardClientCertType::AlwaysForwardOnly) { + if (config.forwardClientCert() == ForwardClientCertType::AlwaysForwardOnly) { return; } // When Sanitize is set, or the connection is not mutual TLS, remove the XFCC header. - if (config.forwardClientCert() == Http::ForwardClientCertType::Sanitize || + if (config.forwardClientCert() == ForwardClientCertType::Sanitize || !(connection.ssl() && connection.ssl()->peerCertificatePresented())) { request_headers.removeForwardedClientCert(); return; } // When ForwardOnly is set, always forward the XFCC header without modification. - if (config.forwardClientCert() == Http::ForwardClientCertType::ForwardOnly) { + if (config.forwardClientCert() == ForwardClientCertType::ForwardOnly) { return; } @@ -246,8 +278,8 @@ void ConnectionManagerUtility::mutateXfccRequestHeader(Http::HeaderMap& request_ std::vector client_cert_details; // When AppendForward or SanitizeSet is set, the client certificate information should be set into // the XFCC header. - if (config.forwardClientCert() == Http::ForwardClientCertType::AppendForward || - config.forwardClientCert() == Http::ForwardClientCertType::SanitizeSet) { + if (config.forwardClientCert() == ForwardClientCertType::AppendForward || + config.forwardClientCert() == ForwardClientCertType::SanitizeSet) { const std::string uri_san_local_cert = connection.ssl()->uriSanLocalCertificate(); if (!uri_san_local_cert.empty()) { client_cert_details.push_back("By=" + uri_san_local_cert); @@ -258,23 +290,23 @@ void ConnectionManagerUtility::mutateXfccRequestHeader(Http::HeaderMap& request_ } for (const auto& detail : config.setCurrentClientCertDetails()) { switch (detail) { - case Http::ClientCertDetailsType::Cert: { + case ClientCertDetailsType::Cert: { const std::string peer_cert = connection.ssl()->urlEncodedPemEncodedPeerCertificate(); if (!peer_cert.empty()) { client_cert_details.push_back("Cert=\"" + peer_cert + "\""); } break; } - case Http::ClientCertDetailsType::Subject: + case ClientCertDetailsType::Subject: // The "Subject" key still exists even if the subject is empty. client_cert_details.push_back("Subject=\"" + connection.ssl()->subjectPeerCertificate() + "\""); break; - case Http::ClientCertDetailsType::URI: + case ClientCertDetailsType::URI: // The "URI" key still exists even if the URI is empty. client_cert_details.push_back("URI=" + connection.ssl()->uriSanPeerCertificate()); break; - case Http::ClientCertDetailsType::DNS: { + case ClientCertDetailsType::DNS: { const std::vector dns_sans = connection.ssl()->dnsSansPeerCertificate(); if (!dns_sans.empty()) { for (const std::string& dns : dns_sans) { @@ -288,18 +320,18 @@ void ConnectionManagerUtility::mutateXfccRequestHeader(Http::HeaderMap& request_ } const std::string client_cert_details_str = absl::StrJoin(client_cert_details, ";"); - if (config.forwardClientCert() == Http::ForwardClientCertType::AppendForward) { + if (config.forwardClientCert() == ForwardClientCertType::AppendForward) { HeaderMapImpl::appendToHeader(request_headers.insertForwardedClientCert().value(), client_cert_details_str); - } else if (config.forwardClientCert() == Http::ForwardClientCertType::SanitizeSet) { + } else if (config.forwardClientCert() == ForwardClientCertType::SanitizeSet) { request_headers.insertForwardedClientCert().value(client_cert_details_str); } else { NOT_REACHED_GCOVR_EXCL_LINE; } } -void ConnectionManagerUtility::mutateResponseHeaders(Http::HeaderMap& response_headers, - const Http::HeaderMap* request_headers, +void ConnectionManagerUtility::mutateResponseHeaders(HeaderMap& response_headers, + const HeaderMap* request_headers, const std::string& via) { if (request_headers != nullptr && Utility::isUpgrade(*request_headers) && Utility::isUpgrade(response_headers)) { diff --git a/source/common/http/conn_manager_utility.h b/source/common/http/conn_manager_utility.h index a17f7cde81a8..7ba81d8eeb52 100644 --- a/source/common/http/conn_manager_utility.h +++ b/source/common/http/conn_manager_utility.h @@ -16,6 +16,28 @@ namespace Http { */ class ConnectionManagerUtility { public: + /** + * Determine the next protocol to used based both on ALPN as well as protocol inspection. + * @param connection supplies the connection to determine a protocol for. + * @param data supplies the currently available read data on the connection. + */ + static std::string determineNextProtocol(Network::Connection& connection, + const Buffer::Instance& data); + + /** + * Create an HTTP codec given the connection and the beginning of the incoming data. + * @param connection supplies the connection. + * @param data supplies the initial data supplied by the client. + * @param callbacks supplies the codec callbacks. + * @param scope supplies the stats scope for codec stats. + * @param http1_settings supplies the HTTP/1 settings to use if HTTP/1 is chosen. + * @param http2_settings supplies the HTTP/2 settings to use if HTTP/2 is chosen. + */ + static ServerConnectionPtr + autoCreateCodec(Network::Connection& connection, const Buffer::Instance& data, + ServerConnectionCallbacks& callbacks, Stats::Scope& scope, + const Http1Settings& http1_settings, const Http2Settings& http2_settings); + /** * Mutates request headers in various ways. This functionality is broken out because of its * complexity for ease of testing. See the method itself for detailed comments on what @@ -28,23 +50,22 @@ class ConnectionManagerUtility { * existence of the x-forwarded-for header. Again see the method for more details. */ static Network::Address::InstanceConstSharedPtr - mutateRequestHeaders(Http::HeaderMap& request_headers, Network::Connection& connection, + mutateRequestHeaders(HeaderMap& request_headers, Network::Connection& connection, ConnectionManagerConfig& config, const Router::Config& route_config, Runtime::RandomGenerator& random, Runtime::Loader& runtime, const LocalInfo::LocalInfo& local_info); - static void mutateResponseHeaders(Http::HeaderMap& response_headers, - const Http::HeaderMap* request_headers, const std::string& via); + static void mutateResponseHeaders(HeaderMap& response_headers, const HeaderMap* request_headers, + const std::string& via); private: /** * Mutate request headers if request needs to be traced. */ - static void mutateTracingRequestHeader(Http::HeaderMap& request_headers, Runtime::Loader& runtime, + static void mutateTracingRequestHeader(HeaderMap& request_headers, Runtime::Loader& runtime, ConnectionManagerConfig& config); - static void mutateXfccRequestHeader(Http::HeaderMap& request_headers, - Network::Connection& connection, + static void mutateXfccRequestHeader(HeaderMap& request_headers, Network::Connection& connection, ConnectionManagerConfig& config); }; diff --git a/source/common/http/http2/codec_impl.h b/source/common/http/http2/codec_impl.h index 16d7139c30ca..663f8f55965a 100644 --- a/source/common/http/http2/codec_impl.h +++ b/source/common/http/http2/codec_impl.h @@ -11,17 +11,15 @@ #include "envoy/network/connection.h" #include "envoy/stats/scope.h" -//#include "envoy/stats/stats_macros.h" - #include "common/buffer/buffer_impl.h" #include "common/buffer/watermark_buffer.h" #include "common/common/linked_object.h" #include "common/common/logger.h" #include "common/http/codec_helper.h" #include "common/http/header_map_impl.h" -#include "common/http/utility.h" #include "common/http/http2/metadata_decoder.h" #include "common/http/http2/metadata_encoder.h" +#include "common/http/utility.h" #include "absl/types/optional.h" #include "nghttp2/nghttp2.h" diff --git a/source/extensions/filters/network/http_connection_manager/config.cc b/source/extensions/filters/network/http_connection_manager/config.cc index 6dd5ef35b878..f9faf22d2ccf 100644 --- a/source/extensions/filters/network/http_connection_manager/config.cc +++ b/source/extensions/filters/network/http_connection_manager/config.cc @@ -14,6 +14,7 @@ #include "common/common/fmt.h" #include "common/config/filter_json.h" #include "common/config/utility.h" +#include "common/http/conn_manager_utility.h" #include "common/http/date_provider_impl.h" #include "common/http/default_server_string.h" #include "common/http/http1/codec_impl.h" @@ -117,24 +118,6 @@ static Registry::RegisterFactory registered_; -std::string -HttpConnectionManagerConfigUtility::determineNextProtocol(Network::Connection& connection, - const Buffer::Instance& data) { - if (!connection.nextProtocol().empty()) { - return connection.nextProtocol(); - } - - // See if the data we have so far shows the HTTP/2 prefix. We ignore the case where someone sends - // us the first few bytes of the HTTP/2 prefix since in all public cases we use SSL/ALPN. For - // internal cases this should practically never happen. - if (-1 != data.search(Http::Http2::CLIENT_MAGIC_PREFIX.c_str(), - Http::Http2::CLIENT_MAGIC_PREFIX.size(), 0)) { - return Http::Http2::ALPN_STRING; - } - - return ""; -} - InternalAddressConfig::InternalAddressConfig( const envoy::config::filter::network::http_connection_manager::v2::HttpConnectionManager:: InternalAddressConfig& config) @@ -344,14 +327,8 @@ HttpConnectionManagerConfig::createCodec(Network::Connection& connection, return Http::ServerConnectionPtr{new Http::Http2::ServerConnectionImpl( connection, callbacks, context_.scope(), http2_settings_)}; case CodecType::AUTO: - if (HttpConnectionManagerConfigUtility::determineNextProtocol(connection, data) == - Http::Http2::ALPN_STRING) { - return Http::ServerConnectionPtr{new Http::Http2::ServerConnectionImpl( - connection, callbacks, context_.scope(), http2_settings_)}; - } else { - return Http::ServerConnectionPtr{ - new Http::Http1::ServerConnectionImpl(connection, callbacks, http1_settings_)}; - } + return Http::ConnectionManagerUtility::autoCreateCodec( + connection, data, callbacks, context_.scope(), http1_settings_, http2_settings_); } NOT_REACHED_GCOVR_EXCL_LINE; diff --git a/source/extensions/filters/network/http_connection_manager/config.h b/source/extensions/filters/network/http_connection_manager/config.h index 917b2ee3b99a..43172d7802e3 100644 --- a/source/extensions/filters/network/http_connection_manager/config.h +++ b/source/extensions/filters/network/http_connection_manager/config.h @@ -46,20 +46,6 @@ class HttpConnectionManagerFilterConfigFactory Server::Configuration::FactoryContext& context) override; }; -/** - * Utilities for the HTTP connection manager that facilitate testing. - */ -class HttpConnectionManagerConfigUtility { -public: - /** - * Determine the next protocol to used based both on ALPN as well as protocol inspection. - * @param connection supplies the connection to determine a protocol for. - * @param data supplies the currently available read data on the connection. - */ - static std::string determineNextProtocol(Network::Connection& connection, - const Buffer::Instance& data); -}; - /** * Determines if an address is internal based on user provided config. */ diff --git a/source/server/http/BUILD b/source/server/http/BUILD index 1ccaa0bf106d..b6761f286585 100644 --- a/source/server/http/BUILD +++ b/source/server/http/BUILD @@ -47,7 +47,6 @@ envoy_cc_library( "//source/common/http:header_map_lib", "//source/common/http:headers_lib", "//source/common/http:utility_lib", - "//source/common/http/http1:codec_lib", "//source/common/memory:stats_lib", "//source/common/network:listen_socket_lib", "//source/common/network:raw_buffer_socket_lib", diff --git a/source/server/http/admin.cc b/source/server/http/admin.cc index 5b6ec4be7838..e6504dd184ad 100644 --- a/source/server/http/admin.cc +++ b/source/server/http/admin.cc @@ -37,9 +37,9 @@ #include "common/common/version.h" #include "common/html/utility.h" #include "common/http/codes.h" +#include "common/http/conn_manager_utility.h" #include "common/http/header_map_impl.h" #include "common/http/headers.h" -#include "common/http/http1/codec_impl.h" #include "common/json/json_loader.h" #include "common/memory/stats.h" #include "common/network/listen_socket_impl.h" @@ -192,14 +192,16 @@ Http::FilterHeadersStatus AdminFilter::decodeHeaders(Http::HeaderMap& headers, b return Http::FilterHeadersStatus::StopIteration; } -Http::FilterDataStatus AdminFilter::decodeData(Buffer::Instance&, bool end_stream) { +Http::FilterDataStatus AdminFilter::decodeData(Buffer::Instance& data, bool end_stream) { + // Currently we generically buffer all admin request data in case a handler wants to use it. + // If we ever support streaming admin requests we may need to revisit this. + callbacks_->addDecodedData(data, false); + if (end_stream) { onComplete(); } - // Currently we generically buffer all admin request data in case a handler wants to use it. - // If we ever support streaming admin requests we may need to revisit this. - return Http::FilterDataStatus::StopIterationAndBuffer; + return Http::FilterDataStatus::StopIterationNoBuffer; } Http::FilterTrailersStatus AdminFilter::decodeTrailers(Http::HeaderMap&) { @@ -1052,11 +1054,11 @@ AdminImpl::AdminImpl(const std::string& profile_path, Server::Instance& server) admin_filter_chain_(std::make_shared()) {} Http::ServerConnectionPtr AdminImpl::createCodec(Network::Connection& connection, - const Buffer::Instance&, + const Buffer::Instance& data, Http::ServerConnectionCallbacks& callbacks) { - return Http::ServerConnectionPtr{ - new Http::Http1::ServerConnectionImpl(connection, callbacks, Http::Http1Settings())}; -} + return Http::ConnectionManagerUtility::autoCreateCodec( + connection, data, callbacks, server_.stats(), Http::Http1Settings(), Http::Http2Settings()); +} // namespace Server bool AdminImpl::createNetworkFilterChain(Network::Connection& connection, const std::vector&) { diff --git a/test/common/http/conn_manager_utility_test.cc b/test/common/http/conn_manager_utility_test.cc index 06983d561b2f..df63b2d66184 100644 --- a/test/common/http/conn_manager_utility_test.cc +++ b/test/common/http/conn_manager_utility_test.cc @@ -127,6 +127,51 @@ class ConnectionManagerUtilityTest : public testing::Test { std::string via_; }; +// Tests for ConnectionManagerUtility::determineNextProtocol. +TEST_F(ConnectionManagerUtilityTest, DetermineNextProtocol) { + { + Network::MockConnection connection; + EXPECT_CALL(connection, nextProtocol()).WillRepeatedly(Return("hello")); + Buffer::OwnedImpl data(""); + EXPECT_EQ("hello", ConnectionManagerUtility::determineNextProtocol(connection, data)); + } + + { + Network::MockConnection connection; + EXPECT_CALL(connection, nextProtocol()).WillRepeatedly(Return("")); + Buffer::OwnedImpl data(""); + EXPECT_EQ("", ConnectionManagerUtility::determineNextProtocol(connection, data)); + } + + { + Network::MockConnection connection; + EXPECT_CALL(connection, nextProtocol()).WillRepeatedly(Return("")); + Buffer::OwnedImpl data("GET / HTTP/1.1"); + EXPECT_EQ("", ConnectionManagerUtility::determineNextProtocol(connection, data)); + } + + { + Network::MockConnection connection; + EXPECT_CALL(connection, nextProtocol()).WillRepeatedly(Return("")); + Buffer::OwnedImpl data("PRI * HTTP/2.0\r\n"); + EXPECT_EQ("h2", ConnectionManagerUtility::determineNextProtocol(connection, data)); + } + + { + Network::MockConnection connection; + EXPECT_CALL(connection, nextProtocol()).WillRepeatedly(Return("")); + Buffer::OwnedImpl data("PRI * HTTP/2"); + EXPECT_EQ("h2", ConnectionManagerUtility::determineNextProtocol(connection, data)); + } + + { + Network::MockConnection connection; + EXPECT_CALL(connection, nextProtocol()).WillRepeatedly(Return("")); + Buffer::OwnedImpl data("PRI * HTTP/"); + EXPECT_EQ("", ConnectionManagerUtility::determineNextProtocol(connection, data)); + } +} + // Verify external request and XFF is set when we are using remote address and the address is // external. TEST_F(ConnectionManagerUtilityTest, UseRemoteAddressWhenNotLocalHostRemoteAddress) { diff --git a/test/extensions/filters/http/tap/tap_filter_integration_test.cc b/test/extensions/filters/http/tap/tap_filter_integration_test.cc index 25bef9dbf43b..ef489d625e87 100644 --- a/test/extensions/filters/http/tap/tap_filter_integration_test.cc +++ b/test/extensions/filters/http/tap/tap_filter_integration_test.cc @@ -11,7 +11,11 @@ class TapIntegrationTest : public HttpIntegrationTest, public testing::TestWithParam { public: TapIntegrationTest() - : HttpIntegrationTest(Http::CodecClient::Type::HTTP1, GetParam(), realTime()) {} + // Note: This test must use HTTP/2 because of the lack of early close detection for + // HTTP/1 on OSX. In this test we close the admin /tap stream when we don't want any + // more data, and without immediate close detection we can't have a flake free test. + // Thus, we use HTTP/2 for everything here. + : HttpIntegrationTest(Http::CodecClient::Type::HTTP2, GetParam(), realTime()) {} void initializeFilter(const std::string& filter_config) { config_helper_.addFilter(filter_config); @@ -119,7 +123,7 @@ config_id: test_config_id envoy::data::tap::v2alpha::HttpBufferedTrace trace; MessageUtil::loadFromYaml(admin_response->body(), trace); EXPECT_EQ("request_match_id", trace.match_id()); - EXPECT_EQ(trace.request_headers().size(), 9); + EXPECT_EQ(trace.request_headers().size(), 8); EXPECT_EQ(trace.response_headers().size(), 5); admin_response->clearBody(); @@ -142,8 +146,8 @@ config_id: test_config_id admin_response->waitForBodyData(1); MessageUtil::loadFromYaml(admin_response->body(), trace); EXPECT_EQ("response_match_id", trace.match_id()); - EXPECT_EQ(trace.request_headers().size(), 8); - EXPECT_EQ("0", findHeader("content-length", trace.request_headers())->value()); + EXPECT_EQ(trace.request_headers().size(), 7); + EXPECT_EQ("http", findHeader("x-forwarded-proto", trace.request_headers())->value()); EXPECT_EQ(trace.response_headers().size(), 6); EXPECT_NE(nullptr, findHeader("date", trace.response_headers())); EXPECT_EQ("baz", findHeader("bar", trace.response_headers())->value()); diff --git a/test/extensions/filters/network/http_connection_manager/config_test.cc b/test/extensions/filters/network/http_connection_manager/config_test.cc index 9a6cc70d30da..aec4be3bf194 100644 --- a/test/extensions/filters/network/http_connection_manager/config_test.cc +++ b/test/extensions/filters/network/http_connection_manager/config_test.cc @@ -246,50 +246,6 @@ TEST_F(HttpConnectionManagerConfigTest, SingleDateProvider) { Network::FilterFactoryCb cb2 = factory.createFilterFactory(*json_config, context_); } -TEST(HttpConnectionManagerConfigUtilityTest, DetermineNextProtocol) { - { - Network::MockConnection connection; - EXPECT_CALL(connection, nextProtocol()).WillRepeatedly(Return("hello")); - Buffer::OwnedImpl data(""); - EXPECT_EQ("hello", HttpConnectionManagerConfigUtility::determineNextProtocol(connection, data)); - } - - { - Network::MockConnection connection; - EXPECT_CALL(connection, nextProtocol()).WillRepeatedly(Return("")); - Buffer::OwnedImpl data(""); - EXPECT_EQ("", HttpConnectionManagerConfigUtility::determineNextProtocol(connection, data)); - } - - { - Network::MockConnection connection; - EXPECT_CALL(connection, nextProtocol()).WillRepeatedly(Return("")); - Buffer::OwnedImpl data("GET / HTTP/1.1"); - EXPECT_EQ("", HttpConnectionManagerConfigUtility::determineNextProtocol(connection, data)); - } - - { - Network::MockConnection connection; - EXPECT_CALL(connection, nextProtocol()).WillRepeatedly(Return("")); - Buffer::OwnedImpl data("PRI * HTTP/2.0\r\n"); - EXPECT_EQ("h2", HttpConnectionManagerConfigUtility::determineNextProtocol(connection, data)); - } - - { - Network::MockConnection connection; - EXPECT_CALL(connection, nextProtocol()).WillRepeatedly(Return("")); - Buffer::OwnedImpl data("PRI * HTTP/2"); - EXPECT_EQ("h2", HttpConnectionManagerConfigUtility::determineNextProtocol(connection, data)); - } - - { - Network::MockConnection connection; - EXPECT_CALL(connection, nextProtocol()).WillRepeatedly(Return("")); - Buffer::OwnedImpl data("PRI * HTTP/"); - EXPECT_EQ("", HttpConnectionManagerConfigUtility::determineNextProtocol(connection, data)); - } -} - TEST_F(HttpConnectionManagerConfigTest, BadHttpConnectionMangerConfig) { std::string json_string = R"EOF( { diff --git a/test/server/http/admin_test.cc b/test/server/http/admin_test.cc index 438e75def0f0..8545bf691e8e 100644 --- a/test/server/http/admin_test.cc +++ b/test/server/http/admin_test.cc @@ -563,21 +563,27 @@ TEST_P(AdminFilterTest, HeaderOnly) { } TEST_P(AdminFilterTest, Body) { + InSequence s; + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, filter_.decodeHeaders(request_headers_, false)); Buffer::OwnedImpl data("hello"); + EXPECT_CALL(callbacks_, addDecodedData(_, false)); EXPECT_CALL(callbacks_, encodeHeaders_(_, false)); - EXPECT_EQ(Http::FilterDataStatus::StopIterationAndBuffer, filter_.decodeData(data, true)); + EXPECT_EQ(Http::FilterDataStatus::StopIterationNoBuffer, filter_.decodeData(data, true)); } TEST_P(AdminFilterTest, Trailers) { + InSequence s; + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, filter_.decodeHeaders(request_headers_, false)); Buffer::OwnedImpl data("hello"); - EXPECT_EQ(Http::FilterDataStatus::StopIterationAndBuffer, filter_.decodeData(data, false)); - EXPECT_CALL(callbacks_, encodeHeaders_(_, false)); + EXPECT_CALL(callbacks_, addDecodedData(_, false)); + EXPECT_EQ(Http::FilterDataStatus::StopIterationNoBuffer, filter_.decodeData(data, false)); EXPECT_CALL(callbacks_, decodingBuffer()); filter_.getRequestBody(); + EXPECT_CALL(callbacks_, encodeHeaders_(_, false)); EXPECT_EQ(Http::FilterTrailersStatus::StopIteration, filter_.decodeTrailers(request_headers_)); } From 407ac60f1a8fa47fcbc3473ed4df7477f14472a6 Mon Sep 17 00:00:00 2001 From: Matt Klein Date: Tue, 8 Jan 2019 21:11:00 -0700 Subject: [PATCH 14/26] fix Signed-off-by: Matt Klein --- source/server/http/admin.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/server/http/admin.cc b/source/server/http/admin.cc index e6504dd184ad..759c5d826ffc 100644 --- a/source/server/http/admin.cc +++ b/source/server/http/admin.cc @@ -1058,7 +1058,7 @@ Http::ServerConnectionPtr AdminImpl::createCodec(Network::Connection& connection Http::ServerConnectionCallbacks& callbacks) { return Http::ConnectionManagerUtility::autoCreateCodec( connection, data, callbacks, server_.stats(), Http::Http1Settings(), Http::Http2Settings()); -} // namespace Server +} bool AdminImpl::createNetworkFilterChain(Network::Connection& connection, const std::vector&) { From ca0b1de08479dfbe1350a4ee99cce9cbb4eda4b2 Mon Sep 17 00:00:00 2001 From: Matt Klein Date: Tue, 8 Jan 2019 21:21:14 -0700 Subject: [PATCH 15/26] run HTTP/2 integration tests for admin Signed-off-by: Matt Klein --- test/integration/BUILD | 2 +- test/integration/integration_admin_test.cc | 6 +++--- test/integration/integration_admin_test.h | 8 ++------ 3 files changed, 6 insertions(+), 10 deletions(-) diff --git a/test/integration/BUILD b/test/integration/BUILD index 2e793df5535b..5957b9d3c7e7 100644 --- a/test/integration/BUILD +++ b/test/integration/BUILD @@ -160,7 +160,7 @@ envoy_cc_test( "integration_admin_test.h", ], deps = [ - ":http_integration_lib", + ":http_protocol_integration_lib", "//include/envoy/http:header_map_interface", "//source/common/stats:stats_matcher_lib", "//source/extensions/filters/http/buffer:config", diff --git a/test/integration/integration_admin_test.cc b/test/integration/integration_admin_test.cc index 969b65d5dfca..a44394e30e8c 100644 --- a/test/integration/integration_admin_test.cc +++ b/test/integration/integration_admin_test.cc @@ -16,9 +16,9 @@ namespace Envoy { -INSTANTIATE_TEST_CASE_P(IpVersions, IntegrationAdminTest, - testing::ValuesIn(TestEnvironment::getIpVersionsForTest()), - TestUtility::ipTestParamsToString); +INSTANTIATE_TEST_CASE_P(Protocols, IntegrationAdminTest, + testing::ValuesIn(HttpProtocolIntegrationTest::getProtocolTestParams()), + HttpProtocolIntegrationTest::protocolTestParamsToString); TEST_P(IntegrationAdminTest, HealthCheck) { initialize(); diff --git a/test/integration/integration_admin_test.h b/test/integration/integration_admin_test.h index 3a0487d7d60e..0bc34fbe72d4 100644 --- a/test/integration/integration_admin_test.h +++ b/test/integration/integration_admin_test.h @@ -2,18 +2,14 @@ #include "common/json/json_loader.h" -#include "test/integration/http_integration.h" +#include "test/integration/http_protocol_integration.h" #include "gtest/gtest.h" namespace Envoy { -class IntegrationAdminTest : public HttpIntegrationTest, - public testing::TestWithParam { +class IntegrationAdminTest : public HttpProtocolIntegrationTest { public: - IntegrationAdminTest() - : HttpIntegrationTest(Http::CodecClient::Type::HTTP1, GetParam(), realTime()) {} - void initialize() override { config_helper_.addFilter(ConfigHelper::DEFAULT_HEALTH_CHECK_FILTER); HttpIntegrationTest::initialize(); From 7563e54f32f63ce28c93fcfa5a271df36115df9e Mon Sep 17 00:00:00 2001 From: Matt Klein Date: Wed, 9 Jan 2019 15:22:05 -0700 Subject: [PATCH 16/26] alpha Signed-off-by: Matt Klein --- docs/root/intro/version_history.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst index f66a061f22e0..33f4978e0fda 100644 --- a/docs/root/intro/version_history.rst +++ b/docs/root/intro/version_history.rst @@ -10,9 +10,9 @@ Version history * config: removed REST_LEGACY as a valid :ref:`ApiType `. * cors: added :ref:`filter_enabled & shadow_enabled RuntimeFractionalPercent flags ` to filter. * http: added new grpc_http1_reverse_bridge filter for converting gRPC requests into HTTP/1.1 requests. -* tap: added new :ref:`HTTP tap filter `. * router: added ability to configure a :ref:`retry policy ` at the virtual host level. +* tap: added new :ref:`HTTP tap filter `. * tls: enabled TLS 1.3 on the server-side (non-FIPS builds). 1.9.0 From 6888f9bfd8d12f9e45b0693d87f57d706255851a Mon Sep 17 00:00:00 2001 From: Matt Klein Date: Wed, 9 Jan 2019 15:40:15 -0700 Subject: [PATCH 17/26] comments Signed-off-by: Matt Klein --- source/extensions/common/tap/admin.cc | 5 +-- .../filters/http/tap/tap_config_impl.cc | 33 ++++++++----------- 2 files changed, 14 insertions(+), 24 deletions(-) diff --git a/source/extensions/common/tap/admin.cc b/source/extensions/common/tap/admin.cc index 96fb4dfcf954..9b7faa1fbb11 100644 --- a/source/extensions/common/tap/admin.cc +++ b/source/extensions/common/tap/admin.cc @@ -87,15 +87,12 @@ void AdminHandler::unregisterConfig(ExtensionConfig& config) { ASSERT(!config.adminId().empty()); ASSERT(config_id_map_[config.adminId()].count(&config) == 1); config_id_map_[config.adminId()].erase(&config); - if (config_id_map_[config.adminId()].size() == 0) { + if (config_id_map_[config.adminId()].empty()) { config_id_map_.erase(config.adminId()); } } void AdminHandler::submitBufferedTrace(std::shared_ptr trace) { - // TODO(mattklein123): The only reason this takes a shared_ptr is because currently a posted - // lambda cannot take a unique_ptr because we copy the lambda. If we allow posts to happen with - // a moved lambda we can fix this. I will fix this in a follow up. ENVOY_LOG(debug, "admin submitting buffered trace to main thread"); main_thread_dispatcher_.post([this, trace]() { if (attached_request_.has_value()) { diff --git a/source/extensions/filters/http/tap/tap_config_impl.cc b/source/extensions/filters/http/tap/tap_config_impl.cc index 397577be39de..8e2c8c878acd 100644 --- a/source/extensions/filters/http/tap/tap_config_impl.cc +++ b/source/extensions/filters/http/tap/tap_config_impl.cc @@ -76,6 +76,17 @@ void HttpPerRequestTapperImpl::onResponseHeaders(const Http::HeaderMap& headers) config_->matchesResponseHeaders(headers, statuses_); } +namespace { +Http::HeaderMap::Iterate fillHeaderList(const Http::HeaderEntry& header, void* context) { + Protobuf::RepeatedPtrField& header_list = + *reinterpret_cast*>(context); + auto& new_header = *header_list.Add(); + new_header.set_key(header.key().c_str()); + new_header.set_value(header.value().c_str()); + return Http::HeaderMap::Iterate::Continue; +} +} // namespace + bool HttpPerRequestTapperImpl::onLog(const Http::HeaderMap* request_headers, const Http::HeaderMap* response_headers) { absl::optional match_index; @@ -93,27 +104,9 @@ bool HttpPerRequestTapperImpl::onLog(const Http::HeaderMap* request_headers, auto trace = std::make_shared(); trace->set_match_id(config_->configs()[match_index.value()].id_); - request_headers->iterate( - [](const Http::HeaderEntry& header, void* context) -> Http::HeaderMap::Iterate { - envoy::data::tap::v2alpha::HttpBufferedTrace& trace = - *reinterpret_cast(context); - auto& new_header = *trace.add_request_headers(); - new_header.set_key(header.key().c_str()); - new_header.set_value(header.value().c_str()); - return Http::HeaderMap::Iterate::Continue; - }, - trace.get()); + request_headers->iterate(fillHeaderList, trace->mutable_request_headers()); if (response_headers != nullptr) { - response_headers->iterate( - [](const Http::HeaderEntry& header, void* context) -> Http::HeaderMap::Iterate { - envoy::data::tap::v2alpha::HttpBufferedTrace& trace = - *reinterpret_cast(context); - auto& new_header = *trace.add_response_headers(); - new_header.set_key(header.key().c_str()); - new_header.set_value(header.value().c_str()); - return Http::HeaderMap::Iterate::Continue; - }, - trace.get()); + response_headers->iterate(fillHeaderList, trace->mutable_response_headers()); } ENVOY_LOG(debug, "submitting buffered trace sink"); From 4b0acb44915d027fc44217884b3ab3ca9ef4dee6 Mon Sep 17 00:00:00 2001 From: Matt Klein Date: Sun, 13 Jan 2019 20:53:34 -0700 Subject: [PATCH 18/26] fix release notes Signed-off-by: Matt Klein --- docs/root/intro/version_history.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst index a19a5dcdb2e0..e95fafcfecf7 100644 --- a/docs/root/intro/version_history.rst +++ b/docs/root/intro/version_history.rst @@ -3,10 +3,10 @@ Version history 1.10.0 (pending) ================ -* config: added support of using google.protobuf.Any in opaque configs for extensions. * access log: added a new flag for upstream retry count exceeded. * admin: the admin server can now be accessed via HTTP/2 (prior knowledge). -* buffer: fix vulnerabilities when allocation fails +* buffer: fix vulnerabilities when allocation fails. +* config: added support of using google.protobuf.Any in opaque configs for extensions. * config: removed deprecated_v1 sds_config from :ref:`Bootstrap config `. * config: removed REST_LEGACY as a valid :ref:`ApiType `. * cors: added :ref:`filter_enabled & shadow_enabled RuntimeFractionalPercent flags ` to filter. @@ -20,7 +20,7 @@ Version history * redis: added :ref:`success and error stats ` for commands. * router: added ability to configure a :ref:`retry policy ` at the virtual host level. -* tap: added new :ref:`HTTP tap filter `. +* tap: added new alpha :ref:`HTTP tap filter `. * tls: enabled TLS 1.3 on the server-side (non-FIPS builds). * upstream: add hash_function to specify the hash function for :ref:`ring hash` as either xxHash or `murmurHash2 `_. MurmurHash2 is compatible with std::hash in GNU libstdc++ 3.4.20 or above. This is typically the case when compiled on Linux and not macOS. From 7515eef38e5268e63382a91debcf9acc7d943feb Mon Sep 17 00:00:00 2001 From: Matt Klein Date: Mon, 14 Jan 2019 09:41:10 -0700 Subject: [PATCH 19/26] comments Signed-off-by: Matt Klein --- .../configuration/http_filters/tap_filter.rst | 15 ++++++++------- source/extensions/common/tap/admin.cc | 6 ++++-- source/extensions/common/tap/admin.h | 4 ++-- source/extensions/filters/http/tap/config.cc | 1 - source/extensions/filters/http/tap/tap_config.h | 6 +++--- .../filters/http/tap/tap_config_impl.cc | 6 +++--- .../extensions/filters/http/tap/tap_config_impl.h | 6 +++--- source/extensions/filters/http/tap/tap_filter.cc | 2 +- source/extensions/filters/http/tap/tap_filter.h | 2 +- .../filters/http/tap/tap_filter_test.cc | 14 +++++++------- 10 files changed, 32 insertions(+), 30 deletions(-) diff --git a/docs/root/configuration/http_filters/tap_filter.rst b/docs/root/configuration/http_filters/tap_filter.rst index d7488c5852e6..88039984878d 100644 --- a/docs/root/configuration/http_filters/tap_filter.rst +++ b/docs/root/configuration/http_filters/tap_filter.rst @@ -12,15 +12,16 @@ Tap very limited set of match conditions, output configuration, output sinks, etc. Capabilities will be expanded over time and the configuration structures are likely to change. -The HTTP tap filter is used to tap HTTP traffic. At a high level, the configuration is composed of -two pieces: +The HTTP tap filter is used to interpose on and record HTTP traffic. At a high level, the +configuration is composed of two pieces: -1. Match configuration: a list of conditions under which the filter will match an HTTP request - and begin a tap session. -2. Output configuration: a list of output sinks that the filter will write the matched and tapped - data to. +1. :ref:`Match configuration `: a list of conditions + under which the filter will match an HTTP request and begin a tap session. +2. :ref:`Output configuration `: a list of output + sinks that the filter will write the matched and tapped data to. -Each of these concepts will be covered in the following example configuration section. +Each of these concepts will be covered incrementally over the course of several example +configurations in the following section. Example configuration --------------------- diff --git a/source/extensions/common/tap/admin.cc b/source/extensions/common/tap/admin.cc index 9b7faa1fbb11..aadb9b6172cf 100644 --- a/source/extensions/common/tap/admin.cc +++ b/source/extensions/common/tap/admin.cc @@ -24,19 +24,21 @@ AdminHandlerSharedPtr AdminHandler::getSingleton(Server::Admin& admin, AdminHandler::AdminHandler(Server::Admin& admin, Event::Dispatcher& main_thread_dispatcher) : admin_(admin), main_thread_dispatcher_(main_thread_dispatcher) { - bool rc = + const bool rc = admin_.addHandler("/tap", "tap filter control", MAKE_ADMIN_HANDLER(handler), true, true); RELEASE_ASSERT(rc, "/tap admin endpoint is taken"); } AdminHandler::~AdminHandler() { - bool rc = admin_.removeHandler("/tap"); + const bool rc = admin_.removeHandler("/tap"); ASSERT(rc); } Http::Code AdminHandler::handler(absl::string_view, Http::HeaderMap&, Buffer::Instance& response, Server::AdminStream& admin_stream) { if (attached_request_.has_value()) { + // TODO(mattlklein123): Consider supporting concurrent admin /tap streams. Right now we support + // a single stream as a simplification. response.add("An attached /tap admin stream already exists. Detach it."); return Http::Code::BadRequest; } diff --git a/source/extensions/common/tap/admin.h b/source/extensions/common/tap/admin.h index eb7dfef19dae..a0900450d163 100644 --- a/source/extensions/common/tap/admin.h +++ b/source/extensions/common/tap/admin.h @@ -57,8 +57,8 @@ class AdminHandler : public Singleton::Instance, AttachedRequest(std::string config_id, Server::AdminStream* admin_stream) : config_id_(std::move(config_id)), admin_stream_(admin_stream) {} - std::string config_id_; - Server::AdminStream* admin_stream_; + const std::string config_id_; + const Server::AdminStream* admin_stream_; }; Http::Code handler(absl::string_view path_and_query, Http::HeaderMap& response_headers, diff --git a/source/extensions/filters/http/tap/config.cc b/source/extensions/filters/http/tap/config.cc index 184d539fd263..746d26e5854b 100644 --- a/source/extensions/filters/http/tap/config.cc +++ b/source/extensions/filters/http/tap/config.cc @@ -22,7 +22,6 @@ class HttpTapConfigFactoryImpl : public HttpTapConfigFactory { Http::FilterFactoryCb TapFilterFactory::createFilterFactoryFromProtoTyped( const envoy::config::filter::http::tap::v2alpha::Tap& proto_config, const std::string& stats_prefix, Server::Configuration::FactoryContext& context) { - FilterConfigSharedPtr filter_config(new FilterConfigImpl( proto_config, stats_prefix, std::make_unique(), context.scope(), context.admin(), context.singletonManager(), context.threadLocal(), context.dispatcher())); diff --git a/source/extensions/filters/http/tap/tap_config.h b/source/extensions/filters/http/tap/tap_config.h index 1104b4f2f21d..7270a7b935a1 100644 --- a/source/extensions/filters/http/tap/tap_config.h +++ b/source/extensions/filters/http/tap/tap_config.h @@ -33,8 +33,8 @@ class HttpPerRequestTapper { * Called when the request is being destroyed and is being logged. * @return whether the request was tapped or not. */ - virtual bool onLog(const Http::HeaderMap* request_headers, - const Http::HeaderMap* response_headers) PURE; + virtual bool onDestroyLog(const Http::HeaderMap* request_headers, + const Http::HeaderMap* response_headers) PURE; }; using HttpPerRequestTapperPtr = std::unique_ptr; @@ -49,7 +49,7 @@ class HttpTapConfig { /** * @return a new per-request HTTP tapper which is used to handle tapping of a discrete request. */ - virtual HttpPerRequestTapperPtr newPerRequestTapper() PURE; + virtual HttpPerRequestTapperPtr createPerRequestTapper() PURE; }; using HttpTapConfigSharedPtr = std::shared_ptr; diff --git a/source/extensions/filters/http/tap/tap_config_impl.cc b/source/extensions/filters/http/tap/tap_config_impl.cc index 8e2c8c878acd..2acf21f488d3 100644 --- a/source/extensions/filters/http/tap/tap_config_impl.cc +++ b/source/extensions/filters/http/tap/tap_config_impl.cc @@ -64,7 +64,7 @@ void HttpTapConfigImpl::matchesResponseHeaders(const Http::HeaderMap& headers, } } -HttpPerRequestTapperPtr HttpTapConfigImpl::newPerRequestTapper() { +HttpPerRequestTapperPtr HttpTapConfigImpl::createPerRequestTapper() { return std::make_unique(shared_from_this()); } @@ -87,8 +87,8 @@ Http::HeaderMap::Iterate fillHeaderList(const Http::HeaderEntry& header, void* c } } // namespace -bool HttpPerRequestTapperImpl::onLog(const Http::HeaderMap* request_headers, - const Http::HeaderMap* response_headers) { +bool HttpPerRequestTapperImpl::onDestroyLog(const Http::HeaderMap* request_headers, + const Http::HeaderMap* response_headers) { absl::optional match_index; for (uint64_t i = 0; i < statuses_.size(); i++) { const auto& status = statuses_[i]; diff --git a/source/extensions/filters/http/tap/tap_config_impl.h b/source/extensions/filters/http/tap/tap_config_impl.h index 6450227fe6c1..7ef223ddf12a 100644 --- a/source/extensions/filters/http/tap/tap_config_impl.h +++ b/source/extensions/filters/http/tap/tap_config_impl.h @@ -49,7 +49,7 @@ class HttpTapConfigImpl : Extensions::Common::Tap::TapConfigBaseImpl, } // TapFilter::HttpTapConfig - HttpPerRequestTapperPtr newPerRequestTapper() override; + HttpPerRequestTapperPtr createPerRequestTapper() override; private: Extensions::Common::Tap::Sink* admin_streamer_; @@ -66,8 +66,8 @@ class HttpPerRequestTapperImpl : public HttpPerRequestTapper, Logger::LoggableonLog(request_headers, response_headers)) { + if (tapper_ != nullptr && tapper_->onDestroyLog(request_headers, response_headers)) { config_->stats().rq_tapped_.inc(); } } diff --git a/source/extensions/filters/http/tap/tap_filter.h b/source/extensions/filters/http/tap/tap_filter.h index 45362ae7198e..f6dbde56ea4d 100644 --- a/source/extensions/filters/http/tap/tap_filter.h +++ b/source/extensions/filters/http/tap/tap_filter.h @@ -92,7 +92,7 @@ class Filter : public Http::StreamFilter, public AccessLog::Instance { public: Filter(FilterConfigSharedPtr config) : config_(std::move(config)), - tapper_(config_->currentConfig() ? config_->currentConfig()->newPerRequestTapper() + tapper_(config_->currentConfig() ? config_->currentConfig()->createPerRequestTapper() : nullptr) {} static FilterStats generateStats(const std::string& prefix, Stats::Scope& scope); diff --git a/test/extensions/filters/http/tap/tap_filter_test.cc b/test/extensions/filters/http/tap/tap_filter_test.cc index 2904d28183da..66dbb54d737e 100644 --- a/test/extensions/filters/http/tap/tap_filter_test.cc +++ b/test/extensions/filters/http/tap/tap_filter_test.cc @@ -26,19 +26,19 @@ class MockFilterConfig : public FilterConfig { class MockHttpTapConfig : public HttpTapConfig { public: - HttpPerRequestTapperPtr newPerRequestTapper() override { - return HttpPerRequestTapperPtr{newPerRequestTapper_()}; + HttpPerRequestTapperPtr createPerRequestTapper() override { + return HttpPerRequestTapperPtr{createPerRequestTapper_()}; } - MOCK_METHOD0(newPerRequestTapper_, HttpPerRequestTapper*()); + MOCK_METHOD0(createPerRequestTapper_, HttpPerRequestTapper*()); }; class MockHttpPerRequestTapper : public HttpPerRequestTapper { public: MOCK_METHOD1(onRequestHeaders, void(const Http::HeaderMap& headers)); MOCK_METHOD1(onResponseHeaders, void(const Http::HeaderMap& headers)); - MOCK_METHOD2(onLog, bool(const Http::HeaderMap* request_headers, - const Http::HeaderMap* response_headers)); + MOCK_METHOD2(onDestroyLog, bool(const Http::HeaderMap* request_headers, + const Http::HeaderMap* response_headers)); }; class TapFilterTest : public testing::Test { @@ -51,7 +51,7 @@ class TapFilterTest : public testing::Test { EXPECT_CALL(*filter_config_, currentConfig()).WillRepeatedly(Return(http_tap_config_)); if (has_config) { http_per_request_tapper_ = new MockHttpPerRequestTapper(); - EXPECT_CALL(*http_tap_config_, newPerRequestTapper_()) + EXPECT_CALL(*http_tap_config_, createPerRequestTapper_()) .WillOnce(Return(http_per_request_tapper_)); } @@ -112,7 +112,7 @@ TEST_F(TapFilterTest, Config) { Http::TestHeaderMapImpl response_trailers; EXPECT_EQ(Http::FilterTrailersStatus::Continue, filter_->encodeTrailers(response_trailers)); - EXPECT_CALL(*http_per_request_tapper_, onLog(&request_headers, &response_headers)) + EXPECT_CALL(*http_per_request_tapper_, onDestroyLog(&request_headers, &response_headers)) .WillOnce(Return(true)); filter_->log(&request_headers, &response_headers, &response_trailers, stream_info_); EXPECT_EQ(1UL, filter_config_->stats_.rq_tapped_.value()); From 937092a61c3dd7f4f31e3137d784c29de63b85f7 Mon Sep 17 00:00:00 2001 From: Matt Klein Date: Mon, 14 Jan 2019 20:21:55 -0700 Subject: [PATCH 20/26] rewrite tap matchers Signed-off-by: Matt Klein --- api/envoy/data/tap/v2alpha/http.proto | 4 - api/envoy/service/tap/v2alpha/common.proto | 55 +++++--- .../configuration/http_filters/tap_filter.rst | 77 +++++----- source/extensions/common/tap/BUILD | 16 ++- source/extensions/common/tap/admin.cc | 21 +-- source/extensions/common/tap/admin.h | 1 + source/extensions/common/tap/tap.h | 63 +++++++++ .../extensions/common/tap/tap_config_base.cc | 32 +++++ .../extensions/common/tap/tap_config_base.h | 19 ++- source/extensions/common/tap/tap_matcher.cc | 132 ++++++++++++++++++ source/extensions/common/tap/tap_matcher.h | 111 +++++++++++++++ source/extensions/filters/http/tap/BUILD | 1 - .../filters/http/tap/tap_config_impl.cc | 69 +-------- .../filters/http/tap/tap_config_impl.h | 39 +----- test/extensions/common/tap/BUILD | 8 ++ test/extensions/common/tap/admin_test.cc | 4 +- .../extensions/common/tap/tap_matcher_test.cc | 51 +++++++ .../http/tap/tap_filter_integration_test.cc | 49 +++---- 18 files changed, 550 insertions(+), 202 deletions(-) create mode 100644 source/extensions/common/tap/tap_config_base.cc create mode 100644 source/extensions/common/tap/tap_matcher.cc create mode 100644 source/extensions/common/tap/tap_matcher.h create mode 100644 test/extensions/common/tap/tap_matcher_test.cc diff --git a/api/envoy/data/tap/v2alpha/http.proto b/api/envoy/data/tap/v2alpha/http.proto index 2c0a1a256df2..9d15046ea91a 100644 --- a/api/envoy/data/tap/v2alpha/http.proto +++ b/api/envoy/data/tap/v2alpha/http.proto @@ -10,10 +10,6 @@ import "envoy/api/v2/core/base.proto"; // A fully buffered HTTP trace message. message HttpBufferedTrace { - // The match ID specified in the :ref:`match_id - // ` field. - string match_id = 1; - // Request headers. repeated api.v2.core.HeaderValue request_headers = 2; diff --git a/api/envoy/service/tap/v2alpha/common.proto b/api/envoy/service/tap/v2alpha/common.proto index 0462db7c2708..774690e827dd 100644 --- a/api/envoy/service/tap/v2alpha/common.proto +++ b/api/envoy/service/tap/v2alpha/common.proto @@ -12,10 +12,9 @@ option java_multiple_files = true; // Tap configuration. message TapConfig { - // The match conditions. One or more match configurations can be specified. If any of them - // match the data source being tapped, a tap will occur, with the result written to the - // configured output. - repeated MatchConfig match_configs = 1 [(validate.rules).repeated .min_items = 1]; + // The match configuration. If the configuration matches the data source being tapped, a tap will + // occur, with the result written to the configured output. + MatchConfig match_config = 1 [(validate.rules).message.required = true]; // The tap output configuration. If a match configuration matches a data source being tapped, // a tap will occur and the data will be written to the configured output. @@ -24,35 +23,49 @@ message TapConfig { // [#comment:TODO(mattklein123): Rate limiting] } -// Tap match configuration. Within a single match configuration, all conditions are evaluated as -// a logical AND. Logical OR matching can be implemented via multiple match configurations in the -// top level tap configuration. +// Tap match configuration. This is a recursive structure which allows complex nested match +// configurations to be built using various logical operators. message MatchConfig { - // Match ID used for correlating tap data with the match that produced it. - string match_id = 1 [(validate.rules).string.min_bytes = 1]; + // A set of match configurations used for logical operations. + message Set { + // The list of rules that make up the set. + repeated MatchConfig rules = 1 [(validate.rules).repeated .min_items = 2]; + } - // HTTP match configuration. Only useful for HTTP aware tap extensions such as the - // :ref:`HTTP tap filter `. - HttpMatchConfig http_match_config = 2; -} + oneof rule { + option (validate.required) = true; + + // A set that describes a logical OR. If any member of the set matches, the match configuration + // matches. + Set or_match = 1; -// HTTP match configuration. -message HttpMatchConfig { - // HTTP request match configuration. - HttpRequestMatchConfig request_match_config = 1; + // A set that describes a logical AND. If all members of the set match, the match configuration + // matches. + Set and_match = 2; - // HTTP response match configuration. - HttpResponseMatchConfig response_match_config = 2; + // A negation match. The match configuration will match if the negated match condition does not + // match. + MatchConfig not_match = 3; + + // The match configuration will always match. + bool any_match = 4 [(validate.rules).bool.const = true]; + + // HTTP request match configuration. + HttpRequestMatch http_request_match = 5; + + // HTTP response match configuration. + HttpResponseMatch http_response_match = 6; + } } // HTTP request match configuration. -message HttpRequestMatchConfig { +message HttpRequestMatch { // HTTP request headers to match. repeated api.v2.route.HeaderMatcher headers = 1; } // HTTP response match configuration. -message HttpResponseMatchConfig { +message HttpResponseMatch { // HTTP response headers to match. repeated api.v2.route.HeaderMatcher headers = 1; } diff --git a/docs/root/configuration/http_filters/tap_filter.rst b/docs/root/configuration/http_filters/tap_filter.rst index 88039984878d..402f3b9d2ae8 100644 --- a/docs/root/configuration/http_filters/tap_filter.rst +++ b/docs/root/configuration/http_filters/tap_filter.rst @@ -20,7 +20,7 @@ configuration is composed of two pieces: 2. :ref:`Output configuration `: a list of output sinks that the filter will write the matched and tapped data to. -Each of these concepts will be covered incrementally over the course of several example +Each of these concepts will be covered incrementally over the course of several example configurations in the following section. Example configuration @@ -61,24 +61,24 @@ An example POST body: config_id: test_config_id tap_config: - match_configs: - - match_id: foo_match_id - http_match_config: - request_match_config: - headers: - - name: foo - exact_match: bar - response_match_config: - headers: - - name: bar - exact_match: baz + match_config: + and_match: + rules: + - http_request_match: + headers: + - name: foo + exact_match: bar + - http_response_match: + headers: + - name: bar + exact_match: baz output_config: sinks: - streaming_admin: {} -The configuration instructs the tap filter to match any HTTP requests in which a request header -``foo: bar`` is present AND a response header ``bar: baz`` is present. If both of these conditions -are met, the request will be tapped and streamed out the admin endpoint. +The preceding configuration instructs the tap filter to match any HTTP requests in which a request +header ``foo: bar`` is present AND a response header ``bar: baz`` is present. If both of these +conditions are met, the request will be tapped and streamed out the admin endpoint. Another example POST body: @@ -86,28 +86,39 @@ Another example POST body: config_id: test_config_id tap_config: - match_configs: - - match_id: request_match_id - http_match_config: - request_match_config: - headers: - - name: foo - exact_match: bar - - match_id: response_match_id - http_match_config: - response_match_config: - headers: - - name: bar - exact_match: baz + match_config: + or_match: + rules: + - http_request_match: + headers: + - name: foo + exact_match: bar + - http_response_match: + headers: + - name: bar + exact_match: baz output_config: sinks: - streaming_admin: {} -The configuration instructs the tap filter to match any HTTP requests in which a request header -``foo: bar`` is present OR a response header ``bar: baz`` is present. If either of these conditions -are met, the request will be tapped and streamed out the admin endpoint. The difference between -the first example and the second is the use of multiple top level match configurations to produce -a logical OR. +The preceding configuration instructs the tap filter to match any HTTP requests in which a request +header ``foo: bar`` is present OR a response header ``bar: baz`` is present. If either of these +conditions are met, the request will be tapped and streamed out the admin endpoint. + +Another example POST body: + +.. code-block:: yaml + + config_id: test_config_id + tap_config: + match_config: + any_match: true + output_config: + sinks: + - streaming_admin: {} + +The preceding configuration instructs the tap filter to match any HTTP requests. All requests will +be tapped and streamed out the admin endpoint. Statistics ---------- diff --git a/source/extensions/common/tap/BUILD b/source/extensions/common/tap/BUILD index 5a80b993baed..6e770e140ae2 100644 --- a/source/extensions/common/tap/BUILD +++ b/source/extensions/common/tap/BUILD @@ -12,6 +12,7 @@ envoy_cc_library( name = "tap_interface", hdrs = ["tap.h"], deps = [ + "//include/envoy/http:header_map_interface", "//source/common/protobuf", "@envoy_api//envoy/service/tap/v2alpha:common_cc", ], @@ -19,9 +20,22 @@ envoy_cc_library( envoy_cc_library( name = "tap_config_base", + srcs = ["tap_config_base.cc"], hdrs = ["tap_config_base.h"], deps = [ - "@envoy_api//envoy/service/tap/v2alpha:common_cc", + ":tap_interface", + ":tap_matcher", + "//source/common/common:assert_lib", + ], +) + +envoy_cc_library( + name = "tap_matcher", + srcs = ["tap_matcher.cc"], + hdrs = ["tap_matcher.h"], + deps = [ + ":tap_interface", + "//source/common/http:header_utility_lib", ], ) diff --git a/source/extensions/common/tap/admin.cc b/source/extensions/common/tap/admin.cc index aadb9b6172cf..ef12e44ae846 100644 --- a/source/extensions/common/tap/admin.cc +++ b/source/extensions/common/tap/admin.cc @@ -39,28 +39,25 @@ Http::Code AdminHandler::handler(absl::string_view, Http::HeaderMap&, Buffer::In if (attached_request_.has_value()) { // TODO(mattlklein123): Consider supporting concurrent admin /tap streams. Right now we support // a single stream as a simplification. - response.add("An attached /tap admin stream already exists. Detach it."); - return Http::Code::BadRequest; + return badRequest(response, "An attached /tap admin stream already exists. Detach it."); } if (admin_stream.getRequestBody() == nullptr) { - response.add("/tap requires a JSON/YAML body"); - return Http::Code::BadRequest; + return badRequest(response, "/tap requires a JSON/YAML body"); } envoy::admin::v2alpha::TapRequest tap_request; try { MessageUtil::loadFromYamlAndValidate(admin_stream.getRequestBody()->toString(), tap_request); } catch (EnvoyException& e) { - response.add(e.what()); - return Http::Code::BadRequest; + return badRequest(response, e.what()); } ENVOY_LOG(debug, "tap admin request for config_id={}", tap_request.config_id()); if (config_id_map_.count(tap_request.config_id()) == 0) { - response.add(fmt::format("Unknown config id '{}'. No extension has registered with this id.", - tap_request.config_id())); - return Http::Code::BadRequest; + return badRequest( + response, fmt::format("Unknown config id '{}'. No extension has registered with this id.", + tap_request.config_id())); } for (auto config : config_id_map_[tap_request.config_id()]) { config->newTapConfig(std::move(*tap_request.mutable_tap_config()), this); @@ -79,6 +76,12 @@ Http::Code AdminHandler::handler(absl::string_view, Http::HeaderMap&, Buffer::In return Http::Code::OK; } +Http::Code AdminHandler::badRequest(Buffer::Instance& response, absl::string_view error) { + ENVOY_LOG(debug, "handler bad request: {}", error); + response.add(error); + return Http::Code::BadRequest; +} + void AdminHandler::registerConfig(ExtensionConfig& config, const std::string& config_id) { ASSERT(!config_id.empty()); ASSERT(config_id_map_[config_id].count(&config) == 0); diff --git a/source/extensions/common/tap/admin.h b/source/extensions/common/tap/admin.h index a0900450d163..dc9a1639b459 100644 --- a/source/extensions/common/tap/admin.h +++ b/source/extensions/common/tap/admin.h @@ -63,6 +63,7 @@ class AdminHandler : public Singleton::Instance, Http::Code handler(absl::string_view path_and_query, Http::HeaderMap& response_headers, Buffer::Instance& response, Server::AdminStream& admin_stream); + Http::Code badRequest(Buffer::Instance& response, absl::string_view error); Server::Admin& admin_; Event::Dispatcher& main_thread_dispatcher_; diff --git a/source/extensions/common/tap/tap.h b/source/extensions/common/tap/tap.h index 1a94be2df68d..414ad893564b 100644 --- a/source/extensions/common/tap/tap.h +++ b/source/extensions/common/tap/tap.h @@ -1,6 +1,7 @@ #pragma once #include "envoy/common/pure.h" +#include "envoy/http/header_map.h" #include "envoy/service/tap/v2alpha/common.pb.h" #include "common/protobuf/protobuf.h" @@ -56,6 +57,68 @@ class ExtensionConfig { Sink* admin_streamer) PURE; }; +class Matcher; +using MatcherPtr = std::unique_ptr; + +/** + * Base class for all tap matchers. + * + * A high level note on the design of tap matching which is different from other matching in Envoy + * due to a requirement to support streaming matching (match as new data arrives versus + * calculating the match given all available data at once). + * - The matching system is composed of a constant matching configuration. This is essentially + * a tree of matchers given logical AND, OR, NOT, etc. + * - A per-stream/request matching status must be kept in order to compute interim match status. + * - In order to make this computationally efficient, the matching tree is kept in a vector, with + * all references to other matchers implemented using an index into the vector. + * - The previous point allows the creation of a per-stream/request vector of booleans of the same + * size as the matcher vector. Then, when match status is updated given new information, the + * vector of booleans can be easily updated using the same indexes as in the constant match + * configuration. + * - Finally, a matches() function can be trivially implemented by looking in the status vector at + * the index position that the current matcher is located in. + */ +class Matcher { +public: + virtual ~Matcher() = default; + + /** + * @return the matcher's index in the match tree vector (see above). + */ + size_t index() { return my_index_; } + + /** + * Update match status given new information. + * @param request_headers supplies the request headers, if available. + * @param response_headers supplies the response headers, if available. + * @param statuses supplies the per-stream-request match status vector which must be the same + * size as the match tree vector (see above). + */ + virtual bool updateMatchStatus(const Http::HeaderMap* request_headers, + const Http::HeaderMap* response_headers, + std::vector& statuses) const PURE; + + /** + * @return whether given currently available information, the matcher matches. + * @param statuses supplies the per-stream-request match status vector which must be the same + * size as the match tree vector (see above). + */ + bool matches(const std::vector& statuses) const { return statuses[my_index_]; } + +protected: + /** + * Base class constructor for a matcher. + * @param matchers supplies the match tree vector being built. + */ + Matcher(const std::vector& matchers) + // NOTE: This code assumes that the index for the matcher being constructed has already been + // allocated, which is why my_index_ is set to size() - 1. See buildMatcher() in + // tap_matcher.cc. + : my_index_(matchers.size() - 1) {} + + const size_t my_index_; +}; + } // namespace Tap } // namespace Common } // namespace Extensions diff --git a/source/extensions/common/tap/tap_config_base.cc b/source/extensions/common/tap/tap_config_base.cc new file mode 100644 index 000000000000..3bcb95b04ed6 --- /dev/null +++ b/source/extensions/common/tap/tap_config_base.cc @@ -0,0 +1,32 @@ +#include "extensions/common/tap/tap_config_base.h" + +#include "common/common/assert.h" + +#include "extensions/common/tap/tap_matcher.h" + +namespace Envoy { +namespace Extensions { +namespace Common { +namespace Tap { + +TapConfigBaseImpl::TapConfigBaseImpl(envoy::service::tap::v2alpha::TapConfig&& proto_config, + Common::Tap::Sink* admin_streamer) + : admin_streamer_(admin_streamer) { + + // TODO(mattklein123): The streaming admin output sink is the only currently supported sink. This + // is validated by schema. + ASSERT(admin_streamer != nullptr); + ASSERT(proto_config.output_config().sinks()[0].has_streaming_admin()); + + buildMatcher(proto_config.match_config(), matchers_); +} + +Matcher& TapConfigBaseImpl::rootMatcher() { + ASSERT(matchers_.size() >= 1); + return *matchers_[0]; +} + +} // namespace Tap +} // namespace Common +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/common/tap/tap_config_base.h b/source/extensions/common/tap/tap_config_base.h index 49e71e59c48d..4a775d93314b 100644 --- a/source/extensions/common/tap/tap_config_base.h +++ b/source/extensions/common/tap/tap_config_base.h @@ -2,6 +2,8 @@ #include "envoy/service/tap/v2alpha/common.pb.h" +#include "extensions/common/tap/tap.h" + namespace Envoy { namespace Extensions { namespace Common { @@ -12,11 +14,22 @@ namespace Tap { * TODO(mattklein123): This class will handle common functionality such as rate limiting, etc. */ class TapConfigBaseImpl { +public: + size_t numMatchers() { return matchers_.size(); } + Matcher& rootMatcher(); + Extensions::Common::Tap::Sink& sink() { + // TODO(mattklein123): When we support multiple sinks, select the right one. Right now + // it must be admin. + return *admin_streamer_; + } + protected: - TapConfigBaseImpl(envoy::service::tap::v2alpha::TapConfig&& proto_config) - : proto_config_(std::move(proto_config)) {} + TapConfigBaseImpl(envoy::service::tap::v2alpha::TapConfig&& proto_config, + Common::Tap::Sink* admin_streamer); - const envoy::service::tap::v2alpha::TapConfig proto_config_; +private: + Sink* admin_streamer_; + std::vector matchers_; }; } // namespace Tap diff --git a/source/extensions/common/tap/tap_matcher.cc b/source/extensions/common/tap/tap_matcher.cc new file mode 100644 index 000000000000..489d6788c260 --- /dev/null +++ b/source/extensions/common/tap/tap_matcher.cc @@ -0,0 +1,132 @@ +#include "extensions/common/tap/tap_matcher.h" + +#include "common/common/assert.h" + +namespace Envoy { +namespace Extensions { +namespace Common { +namespace Tap { + +void buildMatcher(const envoy::service::tap::v2alpha::MatchConfig& match_config, + std::vector& matchers) { + // In order to store indexes and build our matcher tree inline, we must reserve a slot where + // the matcher we are about to create will go. This allows us to know its future index and still + // construct more of the tree in each called constructor (e.g., multiple OR/AND conditions). + // Once fully constructed, we move the matcher into its position below. See the tap matcher + // overview in tap.h for more information. + matchers.emplace_back(nullptr); + + MatcherPtr new_matcher; + switch (match_config.rule_case()) { + case envoy::service::tap::v2alpha::MatchConfig::kOrMatch: + new_matcher = std::make_unique(match_config.or_match(), matchers, + SetLogicMatcher::Type::Or); + break; + case envoy::service::tap::v2alpha::MatchConfig::kAndMatch: + new_matcher = std::make_unique(match_config.and_match(), matchers, + SetLogicMatcher::Type::And); + break; + case envoy::service::tap::v2alpha::MatchConfig::kNotMatch: + new_matcher = std::make_unique(match_config.not_match(), matchers); + break; + case envoy::service::tap::v2alpha::MatchConfig::kAnyMatch: + new_matcher = std::make_unique(matchers); + break; + case envoy::service::tap::v2alpha::MatchConfig::kHttpRequestMatch: + new_matcher = std::make_unique(match_config.http_request_match(), matchers); + break; + case envoy::service::tap::v2alpha::MatchConfig::kHttpResponseMatch: + new_matcher = + std::make_unique(match_config.http_response_match(), matchers); + break; + default: + NOT_REACHED_GCOVR_EXCL_LINE; + } + + // Per above, move the matcher into its position. + matchers[new_matcher->index()] = std::move(new_matcher); +} + +SetLogicMatcher::SetLogicMatcher(const envoy::service::tap::v2alpha::MatchConfig::Set& configs, + std::vector& matchers, Type type) + : Matcher(matchers), matchers_(matchers), type_(type) { + for (const auto& config : configs.rules()) { + indexes_.push_back(matchers_.size()); + buildMatcher(config, matchers_); + } +} + +bool SetLogicMatcher::updateMatchStatus(const Http::HeaderMap* request_headers, + const Http::HeaderMap* response_headers, + std::vector& statuses) const { + for (size_t index : indexes_) { + statuses[index] = + matchers_[index]->updateMatchStatus(request_headers, response_headers, statuses); + } + + auto predicate = [&statuses](size_t index) { return statuses[index]; }; + if (type_ == Type::And) { + statuses[my_index_] = std::all_of(indexes_.begin(), indexes_.end(), predicate); + } else { + ASSERT(type_ == Type::Or); + statuses[my_index_] = std::any_of(indexes_.begin(), indexes_.end(), predicate); + } + + return statuses[my_index_]; +} + +NotMatcher::NotMatcher(const envoy::service::tap::v2alpha::MatchConfig& config, + std::vector& matchers) + : Matcher(matchers), matchers_(matchers), not_index_(matchers.size()) { + buildMatcher(config, matchers); +} + +bool NotMatcher::updateMatchStatus(const Http::HeaderMap* request_headers, + const Http::HeaderMap* response_headers, + std::vector& statuses) const { + statuses[my_index_] = + !matchers_[not_index_]->updateMatchStatus(request_headers, response_headers, statuses); + return statuses[my_index_]; +} + +HttpRequestMatcher::HttpRequestMatcher(const envoy::service::tap::v2alpha::HttpRequestMatch& config, + const std::vector& matchers) + : Matcher(matchers) { + for (const auto& header_match : config.headers()) { + headers_to_match_.emplace_back(header_match); + } +} + +bool HttpRequestMatcher::updateMatchStatus(const Http::HeaderMap* request_headers, + const Http::HeaderMap*, + std::vector& statuses) const { + if (request_headers != nullptr) { + statuses[my_index_] = Http::HeaderUtility::matchHeaders(*request_headers, headers_to_match_); + } + + return statuses[my_index_]; +} + +HttpResponseMatcher::HttpResponseMatcher( + const envoy::service::tap::v2alpha::HttpResponseMatch& config, + const std::vector& matchers) + : Matcher(matchers) { + for (const auto& header_match : config.headers()) { + headers_to_match_.emplace_back(header_match); + } +} + +bool HttpResponseMatcher::updateMatchStatus(const Http::HeaderMap*, + const Http::HeaderMap* response_headers, + std::vector& statuses) const { + if (response_headers != nullptr) { + statuses[my_index_] = Http::HeaderUtility::matchHeaders(*response_headers, headers_to_match_); + } + + return statuses[my_index_]; +} + +} // namespace Tap +} // namespace Common +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/common/tap/tap_matcher.h b/source/extensions/common/tap/tap_matcher.h new file mode 100644 index 000000000000..908c546cec7a --- /dev/null +++ b/source/extensions/common/tap/tap_matcher.h @@ -0,0 +1,111 @@ +#pragma once + +#include "common/http/header_utility.h" + +#include "extensions/common/tap/tap.h" + +namespace Envoy { +namespace Extensions { +namespace Common { +namespace Tap { + +/** + * Factory method to build a matcher given a match config. Calling this function may end + * up recursively building many matchers, which will all be added to the passed in vector + * of matchers. See the comments in tap.h for the general structure of how tap matchers work. + */ +void buildMatcher(const envoy::service::tap::v2alpha::MatchConfig& match_config, + std::vector& matchers); + +/** + * Matcher for implementing set logic. + */ +class SetLogicMatcher : public Matcher { +public: + enum class Type { And, Or }; + + SetLogicMatcher(const envoy::service::tap::v2alpha::MatchConfig::Set& configs, + std::vector& matchers, Type type); + + // Extensions::Common::Tap::Matcher + bool updateMatchStatus(const Http::HeaderMap* request_headers, + const Http::HeaderMap* response_headers, + std::vector& statuses) const override; + +private: + std::vector& matchers_; + std::vector indexes_; + const Type type_; +}; + +/** + * Not matcher. + */ +class NotMatcher : public Matcher { +public: + NotMatcher(const envoy::service::tap::v2alpha::MatchConfig& config, + std::vector& matchers); + + // Extensions::Common::Tap::Matcher + bool updateMatchStatus(const Http::HeaderMap* request_headers, + const Http::HeaderMap* response_headers, + std::vector& statuses) const override; + +private: + std::vector& matchers_; + const size_t not_index_; +}; + +/** + * Any matcher (always matches). + */ +class AnyMatcher : public Matcher { +public: + AnyMatcher(std::vector& matchers) : Matcher(matchers) {} + + // Extensions::Common::Tap::Matcher + bool updateMatchStatus(const Http::HeaderMap*, const Http::HeaderMap*, + std::vector& statuses) const override { + statuses[my_index_] = true; + return true; + } +}; + +/** + * HTTP request matcher. + */ +class HttpRequestMatcher : public Matcher { +public: + HttpRequestMatcher(const envoy::service::tap::v2alpha::HttpRequestMatch& config, + const std::vector& matchers); + + // Extensions::Common::Tap::Matcher + bool updateMatchStatus(const Http::HeaderMap* request_headers, + const Http::HeaderMap* response_headers, + std::vector& statuses) const override; + +private: + std::vector headers_to_match_; +}; + +/** + * HTTP response matcher. + */ +class HttpResponseMatcher : public Matcher { +public: + HttpResponseMatcher(const envoy::service::tap::v2alpha::HttpResponseMatch& config, + const std::vector& matchers); + + // Extensions::Common::Tap::Matcher + bool updateMatchStatus(const Http::HeaderMap* request_headers, + const Http::HeaderMap* response_headers, + std::vector& statuses) const override; + +private: + std::vector headers_to_match_; +}; + +} // namespace Tap +} // namespace Common +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/filters/http/tap/BUILD b/source/extensions/filters/http/tap/BUILD index 84d259329bdc..d44f143b0dfd 100644 --- a/source/extensions/filters/http/tap/BUILD +++ b/source/extensions/filters/http/tap/BUILD @@ -27,7 +27,6 @@ envoy_cc_library( hdrs = ["tap_config_impl.h"], deps = [ ":tap_config_interface", - "//source/common/http:header_utility_lib", "//source/extensions/common/tap:tap_config_base", "@envoy_api//envoy/data/tap/v2alpha:http_cc", ], diff --git a/source/extensions/filters/http/tap/tap_config_impl.cc b/source/extensions/filters/http/tap/tap_config_impl.cc index 2acf21f488d3..884e1dd97bba 100644 --- a/source/extensions/filters/http/tap/tap_config_impl.cc +++ b/source/extensions/filters/http/tap/tap_config_impl.cc @@ -11,69 +11,18 @@ namespace TapFilter { HttpTapConfigImpl::HttpTapConfigImpl(envoy::service::tap::v2alpha::TapConfig&& proto_config, Common::Tap::Sink* admin_streamer) - : Extensions::Common::Tap::TapConfigBaseImpl(std::move(proto_config)), - admin_streamer_(admin_streamer) { - // TODO(mattklein123): The streaming admin output sink is the only currently supported sink. - ASSERT(admin_streamer); - ASSERT(proto_config_.output_config().sinks()[0].has_streaming_admin()); - - for (const auto& proto_match_config : proto_config_.match_configs()) { - match_configs_.emplace_back(); - MatchConfig& match_config = match_configs_.back(); - match_config.id_ = proto_match_config.match_id(); - - if (proto_match_config.http_match_config().has_request_match_config()) { - match_config.request_config_ = RequestMatchConfig(); - for (const auto& header_match : - proto_match_config.http_match_config().request_match_config().headers()) { - match_config.request_config_.value().headers_to_match_.emplace_back(header_match); - } - } - - if (proto_match_config.http_match_config().has_response_match_config()) { - match_config.response_config_ = ResponseMatchConfig(); - for (const auto& header_match : - proto_match_config.http_match_config().response_match_config().headers()) { - match_config.response_config_.value().headers_to_match_.emplace_back(header_match); - } - } - } -} - -void HttpTapConfigImpl::matchesRequestHeaders(const Http::HeaderMap& headers, - std::vector& statuses) { - ASSERT(match_configs_.size() == statuses.size()); - for (uint64_t i = 0; i < match_configs_.size(); i++) { - statuses[i].request_matched_ = true; - if (match_configs_[i].request_config_.has_value()) { - statuses[i].request_matched_ = Http::HeaderUtility::matchHeaders( - headers, match_configs_[i].request_config_.value().headers_to_match_); - } - } -} - -void HttpTapConfigImpl::matchesResponseHeaders(const Http::HeaderMap& headers, - std::vector& statuses) { - ASSERT(match_configs_.size() == statuses.size()); - for (uint64_t i = 0; i < match_configs_.size(); i++) { - statuses[i].response_matched_ = true; - if (match_configs_[i].response_config_.has_value()) { - statuses[i].response_matched_ = Http::HeaderUtility::matchHeaders( - headers, match_configs_[i].response_config_.value().headers_to_match_); - } - } -} + : Extensions::Common::Tap::TapConfigBaseImpl(std::move(proto_config), admin_streamer) {} HttpPerRequestTapperPtr HttpTapConfigImpl::createPerRequestTapper() { return std::make_unique(shared_from_this()); } void HttpPerRequestTapperImpl::onRequestHeaders(const Http::HeaderMap& headers) { - config_->matchesRequestHeaders(headers, statuses_); + config_->rootMatcher().updateMatchStatus(&headers, nullptr, statuses_); } void HttpPerRequestTapperImpl::onResponseHeaders(const Http::HeaderMap& headers) { - config_->matchesResponseHeaders(headers, statuses_); + config_->rootMatcher().updateMatchStatus(nullptr, &headers, statuses_); } namespace { @@ -89,21 +38,11 @@ Http::HeaderMap::Iterate fillHeaderList(const Http::HeaderEntry& header, void* c bool HttpPerRequestTapperImpl::onDestroyLog(const Http::HeaderMap* request_headers, const Http::HeaderMap* response_headers) { - absl::optional match_index; - for (uint64_t i = 0; i < statuses_.size(); i++) { - const auto& status = statuses_[i]; - if (status.request_matched_ && status.response_matched_) { - match_index = i; - break; - } - } - - if (!match_index.has_value()) { + if (!config_->rootMatcher().matches(statuses_)) { return false; } auto trace = std::make_shared(); - trace->set_match_id(config_->configs()[match_index.value()].id_); request_headers->iterate(fillHeaderList, trace->mutable_request_headers()); if (response_headers != nullptr) { response_headers->iterate(fillHeaderList, trace->mutable_response_headers()); diff --git a/source/extensions/filters/http/tap/tap_config_impl.h b/source/extensions/filters/http/tap/tap_config_impl.h index 7ef223ddf12a..b8f18e8fed96 100644 --- a/source/extensions/filters/http/tap/tap_config_impl.h +++ b/source/extensions/filters/http/tap/tap_config_impl.h @@ -3,7 +3,6 @@ #include "envoy/http/header_map.h" #include "common/common/logger.h" -#include "common/http/header_utility.h" #include "extensions/common/tap/tap_config_base.h" #include "extensions/filters/http/tap/tap_config.h" @@ -13,47 +12,15 @@ namespace Extensions { namespace HttpFilters { namespace TapFilter { -class HttpTapConfigImpl : Extensions::Common::Tap::TapConfigBaseImpl, +class HttpTapConfigImpl : public Extensions::Common::Tap::TapConfigBaseImpl, public HttpTapConfig, public std::enable_shared_from_this { public: - struct RequestMatchConfig { - std::vector headers_to_match_; - }; - - struct ResponseMatchConfig { - std::vector headers_to_match_; - }; - - struct MatchConfig { - std::string id_; - absl::optional request_config_; - absl::optional response_config_; - }; - - struct MatchStatus { - bool request_matched_{}; - bool response_matched_{}; - }; - HttpTapConfigImpl(envoy::service::tap::v2alpha::TapConfig&& proto_config, Extensions::Common::Tap::Sink* admin_streamer); - const std::vector& configs() { return match_configs_; } - void matchesRequestHeaders(const Http::HeaderMap& headers, std::vector& statuses); - void matchesResponseHeaders(const Http::HeaderMap& headers, std::vector& statuses); - Extensions::Common::Tap::Sink& sink() { - // TODO(mattklein123): When we support multiple sinks, select the right one. Right now - // it must be admin. - return *admin_streamer_; - } - // TapFilter::HttpTapConfig HttpPerRequestTapperPtr createPerRequestTapper() override; - -private: - Extensions::Common::Tap::Sink* admin_streamer_; - std::vector match_configs_; }; using HttpTapConfigImplSharedPtr = std::shared_ptr; @@ -61,7 +28,7 @@ using HttpTapConfigImplSharedPtr = std::shared_ptr; class HttpPerRequestTapperImpl : public HttpPerRequestTapper, Logger::Loggable { public: HttpPerRequestTapperImpl(HttpTapConfigImplSharedPtr config) - : config_(std::move(config)), statuses_(config_->configs().size()) {} + : config_(std::move(config)), statuses_(config_->numMatchers()) {} // TapFilter::HttpPerRequestTapper void onRequestHeaders(const Http::HeaderMap& headers) override; @@ -71,7 +38,7 @@ class HttpPerRequestTapperImpl : public HttpPerRequestTapper, Logger::Loggable statuses_; + std::vector statuses_; }; } // namespace TapFilter diff --git a/test/extensions/common/tap/BUILD b/test/extensions/common/tap/BUILD index ccea6eba1fb3..a60a8f29f2a3 100644 --- a/test/extensions/common/tap/BUILD +++ b/test/extensions/common/tap/BUILD @@ -16,3 +16,11 @@ envoy_cc_test( "//test/mocks/server:server_mocks", ], ) + +envoy_cc_test( + name = "tap_matcher_test", + srcs = ["tap_matcher_test.cc"], + deps = [ + "//source/extensions/common/tap:tap_matcher", + ], +) diff --git a/test/extensions/common/tap/admin_test.cc b/test/extensions/common/tap/admin_test.cc index 01e8a1d169cd..2440c844afc8 100644 --- a/test/extensions/common/tap/admin_test.cc +++ b/test/extensions/common/tap/admin_test.cc @@ -44,8 +44,8 @@ class AdminHandlerTest : public testing::Test { R"EOF( config_id: test_config_id tap_config: - match_configs: - - match_id: foo_match_id + match_config: + any_match: true output_config: sinks: - streaming_admin: {} diff --git a/test/extensions/common/tap/tap_matcher_test.cc b/test/extensions/common/tap/tap_matcher_test.cc new file mode 100644 index 000000000000..70ebd3d5f241 --- /dev/null +++ b/test/extensions/common/tap/tap_matcher_test.cc @@ -0,0 +1,51 @@ +#include "common/protobuf/utility.h" + +#include "extensions/common/tap/tap_matcher.h" + +#include "gtest/gtest.h" + +namespace Envoy { +namespace Extensions { +namespace Common { +namespace Tap { +namespace { + +class TapMatcherTest : public testing::Test { +public: + std::vector matchers_; + std::vector statuses_; + envoy::service::tap::v2alpha::MatchConfig config_; +}; + +TEST_F(TapMatcherTest, Any) { + const std::string matcher_yaml = + R"EOF( +any_match: true +)EOF"; + + MessageUtil::loadFromYaml(matcher_yaml, config_); + buildMatcher(config_, matchers_); + EXPECT_EQ(1, matchers_.size()); + statuses_.resize(matchers_.size()); + EXPECT_TRUE(matchers_[0]->updateMatchStatus(nullptr, nullptr, statuses_)); +} + +TEST_F(TapMatcherTest, Not) { + const std::string matcher_yaml = + R"EOF( +not_match: + any_match: true +)EOF"; + + MessageUtil::loadFromYaml(matcher_yaml, config_); + buildMatcher(config_, matchers_); + EXPECT_EQ(2, matchers_.size()); + statuses_.resize(matchers_.size()); + EXPECT_FALSE(matchers_[0]->updateMatchStatus(nullptr, nullptr, statuses_)); +} + +} // namespace +} // namespace Tap +} // namespace Common +} // namespace Extensions +} // namespace Envoy diff --git a/test/extensions/filters/http/tap/tap_filter_integration_test.cc b/test/extensions/filters/http/tap/tap_filter_integration_test.cc index ef489d625e87..0157fb487f93 100644 --- a/test/extensions/filters/http/tap/tap_filter_integration_test.cc +++ b/test/extensions/filters/http/tap/tap_filter_integration_test.cc @@ -68,19 +68,17 @@ name: envoy.filters.http.tap R"EOF( config_id: test_config_id tap_config: - match_configs: - - match_id: request_match_id - http_match_config: - request_match_config: - headers: - - name: foo - exact_match: bar - - match_id: response_match_id - http_match_config: - response_match_config: - headers: - - name: bar - exact_match: baz + match_config: + or_match: + rules: + - http_request_match: + headers: + - name: foo + exact_match: bar + - http_response_match: + headers: + - name: bar + exact_match: baz output_config: sinks: - streaming_admin: {} @@ -122,7 +120,6 @@ config_id: test_config_id admin_response->waitForBodyData(1); envoy::data::tap::v2alpha::HttpBufferedTrace trace; MessageUtil::loadFromYaml(admin_response->body(), trace); - EXPECT_EQ("request_match_id", trace.match_id()); EXPECT_EQ(trace.request_headers().size(), 8); EXPECT_EQ(trace.response_headers().size(), 5); admin_response->clearBody(); @@ -145,7 +142,6 @@ config_id: test_config_id // Wait for the tap message. admin_response->waitForBodyData(1); MessageUtil::loadFromYaml(admin_response->body(), trace); - EXPECT_EQ("response_match_id", trace.match_id()); EXPECT_EQ(trace.request_headers().size(), 7); EXPECT_EQ("http", findHeader("x-forwarded-proto", trace.request_headers())->value()); EXPECT_EQ(trace.response_headers().size(), 6); @@ -160,17 +156,17 @@ config_id: test_config_id R"EOF( config_id: test_config_id tap_config: - match_configs: - - match_id: both_match_id - http_match_config: - request_match_config: - headers: - - name: foo - exact_match: bar - response_match_config: - headers: - - name: bar - exact_match: baz + match_config: + and_match: + rules: + - http_request_match: + headers: + - name: foo + exact_match: bar + - http_response_match: + headers: + - name: bar + exact_match: baz output_config: sinks: - streaming_admin: {} @@ -203,7 +199,6 @@ config_id: test_config_id // Wait for the tap message. admin_response->waitForBodyData(1); MessageUtil::loadFromYaml(admin_response->body(), trace); - EXPECT_EQ("both_match_id", trace.match_id()); admin_client_->close(); EXPECT_EQ(3UL, test_server_->counter("http.config_test.tap.rq_tapped")->value()); From 433c8f1e5a2f0f8c992f96fa6a09e165564c5082 Mon Sep 17 00:00:00 2001 From: Matt Klein Date: Tue, 15 Jan 2019 13:29:46 -0700 Subject: [PATCH 21/26] fix release notes Signed-off-by: Matt Klein --- docs/root/intro/version_history.rst | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst index d4da4cd3c32b..e76858c969bd 100644 --- a/docs/root/intro/version_history.rst +++ b/docs/root/intro/version_history.rst @@ -3,12 +3,11 @@ Version history 1.10.0 (pending) ================ -* config: added support of using google.protobuf.Any in opaque configs for extensions. -* config: removed deprecated --v2-config-only from command line config. * access log: added a new flag for upstream retry count exceeded. * admin: the admin server can now be accessed via HTTP/2 (prior knowledge). * buffer: fix vulnerabilities when allocation fails. * config: added support of using google.protobuf.Any in opaque configs for extensions. +* config: removed deprecated --v2-config-only from command line config. * config: removed deprecated_v1 sds_config from :ref:`Bootstrap config `. * config: removed REST_LEGACY as a valid :ref:`ApiType `. * cors: added :ref:`filter_enabled & shadow_enabled RuntimeFractionalPercent flags ` to filter. From f355b5c1575d8a6bbbfefe772f950156779d9184 Mon Sep 17 00:00:00 2001 From: Matt Klein Date: Tue, 15 Jan 2019 13:50:17 -0700 Subject: [PATCH 22/26] fix build Signed-off-by: Matt Klein --- source/extensions/filters/http/tap/tap_filter.h | 3 +++ test/extensions/filters/http/tap/tap_filter_test.cc | 2 ++ 2 files changed, 5 insertions(+) diff --git a/source/extensions/filters/http/tap/tap_filter.h b/source/extensions/filters/http/tap/tap_filter.h index f6dbde56ea4d..144e601fcaff 100644 --- a/source/extensions/filters/http/tap/tap_filter.h +++ b/source/extensions/filters/http/tap/tap_filter.h @@ -121,6 +121,9 @@ class Filter : public Http::StreamFilter, public AccessLog::Instance { Http::FilterTrailersStatus encodeTrailers(Http::HeaderMap&) override { return Http::FilterTrailersStatus::Continue; } + Http::FilterMetadataStatus encodeMetadata(Http::MetadataMap&) override { + return Http::FilterMetadataStatus::Continue; + } void setEncoderFilterCallbacks(Http::StreamEncoderFilterCallbacks&) override {} // AccessLog::Instance diff --git a/test/extensions/filters/http/tap/tap_filter_test.cc b/test/extensions/filters/http/tap/tap_filter_test.cc index 66dbb54d737e..cf9dce466354 100644 --- a/test/extensions/filters/http/tap/tap_filter_test.cc +++ b/test/extensions/filters/http/tap/tap_filter_test.cc @@ -85,6 +85,8 @@ TEST_F(TapFilterTest, NoConfig) { EXPECT_EQ(Http::FilterDataStatus::Continue, filter_->encodeData(response_body, false)); Http::TestHeaderMapImpl response_trailers; EXPECT_EQ(Http::FilterTrailersStatus::Continue, filter_->encodeTrailers(response_trailers)); + Http::MetadataMap metadata; + EXPECT_EQ(Http::FilterMetadataStatus::Continue, filter_->encodeMetadata(metadata)); filter_->log(&request_headers, &response_headers, &response_trailers, stream_info_); } From 259384c15e245a3a60883553814bce340a93af8e Mon Sep 17 00:00:00 2001 From: Matt Klein Date: Wed, 16 Jan 2019 14:57:34 -0700 Subject: [PATCH 23/26] comments Signed-off-by: Matt Klein --- api/envoy/service/tap/v2alpha/common.proto | 17 +++++++-------- source/extensions/common/tap/tap_matcher.cc | 21 ++++++++++--------- source/extensions/common/tap/tap_matcher.h | 6 +++--- .../extensions/filters/http/tap/tap_filter.cc | 2 ++ .../extensions/common/tap/tap_matcher_test.cc | 2 +- 5 files changed, 25 insertions(+), 23 deletions(-) diff --git a/api/envoy/service/tap/v2alpha/common.proto b/api/envoy/service/tap/v2alpha/common.proto index 774690e827dd..b44d715ab24e 100644 --- a/api/envoy/service/tap/v2alpha/common.proto +++ b/api/envoy/service/tap/v2alpha/common.proto @@ -14,7 +14,7 @@ option java_multiple_files = true; message TapConfig { // The match configuration. If the configuration matches the data source being tapped, a tap will // occur, with the result written to the configured output. - MatchConfig match_config = 1 [(validate.rules).message.required = true]; + MatchPredicate match_config = 1 [(validate.rules).message.required = true]; // The tap output configuration. If a match configuration matches a data source being tapped, // a tap will occur and the data will be written to the configured output. @@ -25,11 +25,11 @@ message TapConfig { // Tap match configuration. This is a recursive structure which allows complex nested match // configurations to be built using various logical operators. -message MatchConfig { +message MatchPredicate { // A set of match configurations used for logical operations. - message Set { + message MatchSet { // The list of rules that make up the set. - repeated MatchConfig rules = 1 [(validate.rules).repeated .min_items = 2]; + repeated MatchPredicate rules = 1 [(validate.rules).repeated .min_items = 2]; } oneof rule { @@ -37,15 +37,14 @@ message MatchConfig { // A set that describes a logical OR. If any member of the set matches, the match configuration // matches. - Set or_match = 1; + MatchSet or_match = 1; // A set that describes a logical AND. If all members of the set match, the match configuration // matches. - Set and_match = 2; + MatchSet and_match = 2; - // A negation match. The match configuration will match if the negated match condition does not - // match. - MatchConfig not_match = 3; + // A negation match. The match configuration will match if the negated match condition matches. + MatchPredicate not_match = 3; // The match configuration will always match. bool any_match = 4 [(validate.rules).bool.const = true]; diff --git a/source/extensions/common/tap/tap_matcher.cc b/source/extensions/common/tap/tap_matcher.cc index 489d6788c260..33d77c927bcc 100644 --- a/source/extensions/common/tap/tap_matcher.cc +++ b/source/extensions/common/tap/tap_matcher.cc @@ -7,7 +7,7 @@ namespace Extensions { namespace Common { namespace Tap { -void buildMatcher(const envoy::service::tap::v2alpha::MatchConfig& match_config, +void buildMatcher(const envoy::service::tap::v2alpha::MatchPredicate& match_config, std::vector& matchers) { // In order to store indexes and build our matcher tree inline, we must reserve a slot where // the matcher we are about to create will go. This allows us to know its future index and still @@ -18,24 +18,24 @@ void buildMatcher(const envoy::service::tap::v2alpha::MatchConfig& match_config, MatcherPtr new_matcher; switch (match_config.rule_case()) { - case envoy::service::tap::v2alpha::MatchConfig::kOrMatch: + case envoy::service::tap::v2alpha::MatchPredicate::kOrMatch: new_matcher = std::make_unique(match_config.or_match(), matchers, SetLogicMatcher::Type::Or); break; - case envoy::service::tap::v2alpha::MatchConfig::kAndMatch: + case envoy::service::tap::v2alpha::MatchPredicate::kAndMatch: new_matcher = std::make_unique(match_config.and_match(), matchers, SetLogicMatcher::Type::And); break; - case envoy::service::tap::v2alpha::MatchConfig::kNotMatch: + case envoy::service::tap::v2alpha::MatchPredicate::kNotMatch: new_matcher = std::make_unique(match_config.not_match(), matchers); break; - case envoy::service::tap::v2alpha::MatchConfig::kAnyMatch: + case envoy::service::tap::v2alpha::MatchPredicate::kAnyMatch: new_matcher = std::make_unique(matchers); break; - case envoy::service::tap::v2alpha::MatchConfig::kHttpRequestMatch: + case envoy::service::tap::v2alpha::MatchPredicate::kHttpRequestMatch: new_matcher = std::make_unique(match_config.http_request_match(), matchers); break; - case envoy::service::tap::v2alpha::MatchConfig::kHttpResponseMatch: + case envoy::service::tap::v2alpha::MatchPredicate::kHttpResponseMatch: new_matcher = std::make_unique(match_config.http_response_match(), matchers); break; @@ -47,8 +47,9 @@ void buildMatcher(const envoy::service::tap::v2alpha::MatchConfig& match_config, matchers[new_matcher->index()] = std::move(new_matcher); } -SetLogicMatcher::SetLogicMatcher(const envoy::service::tap::v2alpha::MatchConfig::Set& configs, - std::vector& matchers, Type type) +SetLogicMatcher::SetLogicMatcher( + const envoy::service::tap::v2alpha::MatchPredicate::MatchSet& configs, + std::vector& matchers, Type type) : Matcher(matchers), matchers_(matchers), type_(type) { for (const auto& config : configs.rules()) { indexes_.push_back(matchers_.size()); @@ -75,7 +76,7 @@ bool SetLogicMatcher::updateMatchStatus(const Http::HeaderMap* request_headers, return statuses[my_index_]; } -NotMatcher::NotMatcher(const envoy::service::tap::v2alpha::MatchConfig& config, +NotMatcher::NotMatcher(const envoy::service::tap::v2alpha::MatchPredicate& config, std::vector& matchers) : Matcher(matchers), matchers_(matchers), not_index_(matchers.size()) { buildMatcher(config, matchers); diff --git a/source/extensions/common/tap/tap_matcher.h b/source/extensions/common/tap/tap_matcher.h index 908c546cec7a..f34aaf34e51c 100644 --- a/source/extensions/common/tap/tap_matcher.h +++ b/source/extensions/common/tap/tap_matcher.h @@ -14,7 +14,7 @@ namespace Tap { * up recursively building many matchers, which will all be added to the passed in vector * of matchers. See the comments in tap.h for the general structure of how tap matchers work. */ -void buildMatcher(const envoy::service::tap::v2alpha::MatchConfig& match_config, +void buildMatcher(const envoy::service::tap::v2alpha::MatchPredicate& match_config, std::vector& matchers); /** @@ -24,7 +24,7 @@ class SetLogicMatcher : public Matcher { public: enum class Type { And, Or }; - SetLogicMatcher(const envoy::service::tap::v2alpha::MatchConfig::Set& configs, + SetLogicMatcher(const envoy::service::tap::v2alpha::MatchPredicate::MatchSet& configs, std::vector& matchers, Type type); // Extensions::Common::Tap::Matcher @@ -43,7 +43,7 @@ class SetLogicMatcher : public Matcher { */ class NotMatcher : public Matcher { public: - NotMatcher(const envoy::service::tap::v2alpha::MatchConfig& config, + NotMatcher(const envoy::service::tap::v2alpha::MatchPredicate& config, std::vector& matchers); // Extensions::Common::Tap::Matcher diff --git a/source/extensions/filters/http/tap/tap_filter.cc b/source/extensions/filters/http/tap/tap_filter.cc index 44955f406914..318d9672120e 100644 --- a/source/extensions/filters/http/tap/tap_filter.cc +++ b/source/extensions/filters/http/tap/tap_filter.cc @@ -56,6 +56,8 @@ void FilterConfigImpl::newTapConfig(envoy::service::tap::v2alpha::TapConfig&& pr } FilterStats Filter::generateStats(const std::string& prefix, Stats::Scope& scope) { + // TODO(mattklein123): Consider whether we want to additionally namespace the stats on the + // filter's configured opaque ID. std::string final_prefix = prefix + "tap."; return {ALL_TAP_FILTER_STATS(POOL_COUNTER_PREFIX(scope, final_prefix))}; } diff --git a/test/extensions/common/tap/tap_matcher_test.cc b/test/extensions/common/tap/tap_matcher_test.cc index 70ebd3d5f241..b6798f0a0a76 100644 --- a/test/extensions/common/tap/tap_matcher_test.cc +++ b/test/extensions/common/tap/tap_matcher_test.cc @@ -14,7 +14,7 @@ class TapMatcherTest : public testing::Test { public: std::vector matchers_; std::vector statuses_; - envoy::service::tap::v2alpha::MatchConfig config_; + envoy::service::tap::v2alpha::MatchPredicate config_; }; TEST_F(TapMatcherTest, Any) { From e9abe73dfdd2522acb60c5a547b8d76e4ba4c2b8 Mon Sep 17 00:00:00 2001 From: Matt Klein Date: Wed, 16 Jan 2019 15:20:53 -0700 Subject: [PATCH 24/26] use wrapper message for trace output Signed-off-by: Matt Klein --- api/docs/BUILD | 1 + api/envoy/data/tap/v2alpha/BUILD | 6 +++++ api/envoy/data/tap/v2alpha/capture.proto | 2 +- api/envoy/data/tap/v2alpha/wrapper.proto | 22 +++++++++++++++++++ docs/build.sh | 1 + .../configuration/http_filters/tap_filter.rst | 4 ++-- source/extensions/common/tap/BUILD | 2 +- source/extensions/common/tap/admin.cc | 3 ++- source/extensions/common/tap/admin.h | 3 ++- source/extensions/common/tap/tap.h | 11 +++++----- .../filters/http/tap/tap_config_impl.cc | 8 ++++--- .../http/tap/tap_filter_integration_test.cc | 20 +++++++++-------- 12 files changed, 59 insertions(+), 24 deletions(-) create mode 100644 api/envoy/data/tap/v2alpha/wrapper.proto diff --git a/api/docs/BUILD b/api/docs/BUILD index 6a2ae4795039..df855569bb3b 100644 --- a/api/docs/BUILD +++ b/api/docs/BUILD @@ -75,6 +75,7 @@ proto_library( "//envoy/data/core/v2alpha:health_check_event", "//envoy/data/tap/v2alpha:capture", "//envoy/data/tap/v2alpha:http", + "//envoy/data/tap/v2alpha:wrapper", "//envoy/service/accesslog/v2:als", "//envoy/service/auth/v2alpha:attribute_context", "//envoy/service/auth/v2alpha:external_auth", diff --git a/api/envoy/data/tap/v2alpha/BUILD b/api/envoy/data/tap/v2alpha/BUILD index 9aa8958228db..97c4a0b82827 100644 --- a/api/envoy/data/tap/v2alpha/BUILD +++ b/api/envoy/data/tap/v2alpha/BUILD @@ -13,3 +13,9 @@ api_proto_library_internal( srcs = ["http.proto"], deps = ["//envoy/api/v2/core:base"], ) + +api_proto_library_internal( + name = "wrapper", + srcs = ["wrapper.proto"], + deps = [":http"], +) diff --git a/api/envoy/data/tap/v2alpha/capture.proto b/api/envoy/data/tap/v2alpha/capture.proto index aea51a19a594..e294a82846dd 100644 --- a/api/envoy/data/tap/v2alpha/capture.proto +++ b/api/envoy/data/tap/v2alpha/capture.proto @@ -1,6 +1,6 @@ syntax = "proto3"; -// [#protodoc-title: Common TAP] +// [#protodoc-title: Common tap] // Trace capture format for the capture transport socket extension. This dumps plain text read/write // sequences on a socket. diff --git a/api/envoy/data/tap/v2alpha/wrapper.proto b/api/envoy/data/tap/v2alpha/wrapper.proto new file mode 100644 index 000000000000..c8e73d3b3369 --- /dev/null +++ b/api/envoy/data/tap/v2alpha/wrapper.proto @@ -0,0 +1,22 @@ +syntax = "proto3"; + +import "envoy/data/tap/v2alpha/http.proto"; + +import "validate/validate.proto"; + +package envoy.data.tap.v2alpha; +option java_package = "io.envoyproxy.envoy.data.tap.v2alpha"; +option java_multiple_files = true; + +// [#protodoc-title: Tap data wrappers] + +// Wrapper for all fully buffered tap traces that Envoy emits. This is required for sending traces +// over gRPC APIs or more easily persisting binary messages to files. +message BufferedTraceWrapper { + oneof trace { + option (validate.required) = true; + + // An HTTP buffered tap trace. + HttpBufferedTrace http_buffered_trace = 1; + } +} diff --git a/docs/build.sh b/docs/build.sh index a644d3ee3c90..bf5fafcf31d4 100755 --- a/docs/build.sh +++ b/docs/build.sh @@ -121,6 +121,7 @@ PROTO_RST=" /envoy/data/core/v2alpha/health_check_event/envoy/data/core/v2alpha/health_check_event.proto.rst /envoy/data/tap/v2alpha/capture/envoy/data/tap/v2alpha/capture.proto.rst /envoy/data/tap/v2alpha/http/envoy/data/tap/v2alpha/http.proto.rst + /envoy/data/tap/v2alpha/wrapper/envoy/data/tap/v2alpha/wrapper.proto.rst /envoy/service/accesslog/v2/als/envoy/service/accesslog/v2/als.proto.rst /envoy/service/auth/v2alpha/external_auth/envoy/service/auth/v2alpha/attribute_context.proto.rst /envoy/service/auth/v2alpha/external_auth/envoy/service/auth/v2alpha/external_auth.proto.rst diff --git a/docs/root/configuration/http_filters/tap_filter.rst b/docs/root/configuration/http_filters/tap_filter.rst index 402f3b9d2ae8..2d3fdf408120 100644 --- a/docs/root/configuration/http_filters/tap_filter.rst +++ b/docs/root/configuration/http_filters/tap_filter.rst @@ -15,8 +15,8 @@ Tap The HTTP tap filter is used to interpose on and record HTTP traffic. At a high level, the configuration is composed of two pieces: -1. :ref:`Match configuration `: a list of conditions - under which the filter will match an HTTP request and begin a tap session. +1. :ref:`Match configuration `: a list of + conditions under which the filter will match an HTTP request and begin a tap session. 2. :ref:`Output configuration `: a list of output sinks that the filter will write the matched and tapped data to. diff --git a/source/extensions/common/tap/BUILD b/source/extensions/common/tap/BUILD index 6e770e140ae2..cb5d2028c0ff 100644 --- a/source/extensions/common/tap/BUILD +++ b/source/extensions/common/tap/BUILD @@ -13,7 +13,7 @@ envoy_cc_library( hdrs = ["tap.h"], deps = [ "//include/envoy/http:header_map_interface", - "//source/common/protobuf", + "@envoy_api//envoy/data/tap/v2alpha:wrapper_cc", "@envoy_api//envoy/service/tap/v2alpha:common_cc", ], ) diff --git a/source/extensions/common/tap/admin.cc b/source/extensions/common/tap/admin.cc index ef12e44ae846..d2a0369c21bb 100644 --- a/source/extensions/common/tap/admin.cc +++ b/source/extensions/common/tap/admin.cc @@ -97,7 +97,8 @@ void AdminHandler::unregisterConfig(ExtensionConfig& config) { } } -void AdminHandler::submitBufferedTrace(std::shared_ptr trace) { +void AdminHandler::submitBufferedTrace( + std::shared_ptr trace) { ENVOY_LOG(debug, "admin submitting buffered trace to main thread"); main_thread_dispatcher_.post([this, trace]() { if (attached_request_.has_value()) { diff --git a/source/extensions/common/tap/admin.h b/source/extensions/common/tap/admin.h index dc9a1639b459..b86cc1e232f8 100644 --- a/source/extensions/common/tap/admin.h +++ b/source/extensions/common/tap/admin.h @@ -50,7 +50,8 @@ class AdminHandler : public Singleton::Instance, void unregisterConfig(ExtensionConfig& config); // Extensions::Common::Tap::Sink - void submitBufferedTrace(std::shared_ptr trace) override; + void submitBufferedTrace( + std::shared_ptr trace) override; private: struct AttachedRequest { diff --git a/source/extensions/common/tap/tap.h b/source/extensions/common/tap/tap.h index 414ad893564b..e1eab1785786 100644 --- a/source/extensions/common/tap/tap.h +++ b/source/extensions/common/tap/tap.h @@ -1,11 +1,10 @@ #pragma once #include "envoy/common/pure.h" +#include "envoy/data/tap/v2alpha/wrapper.pb.h" #include "envoy/http/header_map.h" #include "envoy/service/tap/v2alpha/common.pb.h" -#include "common/protobuf/protobuf.h" - namespace Envoy { namespace Extensions { namespace Common { @@ -20,11 +19,11 @@ class Sink { /** * Send a fully buffered trace to the sink. - * @param trace supplies the trace to send. The trace message is opaque and is assumed to be a - * discrete trace message (as opposed to a portion of a larger trace that should be - * aggregated). + * @param trace supplies the trace to send. The trace message is a discrete trace message (as + * opposed to a portion of a larger trace that should be aggregated). */ - virtual void submitBufferedTrace(std::shared_ptr trace) PURE; + virtual void + submitBufferedTrace(std::shared_ptr trace) PURE; }; /** diff --git a/source/extensions/filters/http/tap/tap_config_impl.cc b/source/extensions/filters/http/tap/tap_config_impl.cc index 884e1dd97bba..9732b776559e 100644 --- a/source/extensions/filters/http/tap/tap_config_impl.cc +++ b/source/extensions/filters/http/tap/tap_config_impl.cc @@ -3,6 +3,7 @@ #include "envoy/data/tap/v2alpha/http.pb.h" #include "common/common/assert.h" +#include "common/protobuf/protobuf.h" namespace Envoy { namespace Extensions { @@ -42,10 +43,11 @@ bool HttpPerRequestTapperImpl::onDestroyLog(const Http::HeaderMap* request_heade return false; } - auto trace = std::make_shared(); - request_headers->iterate(fillHeaderList, trace->mutable_request_headers()); + auto trace = std::make_shared(); + auto& http_trace = *trace->mutable_http_buffered_trace(); + request_headers->iterate(fillHeaderList, http_trace.mutable_request_headers()); if (response_headers != nullptr) { - response_headers->iterate(fillHeaderList, trace->mutable_response_headers()); + response_headers->iterate(fillHeaderList, http_trace.mutable_response_headers()); } ENVOY_LOG(debug, "submitting buffered trace sink"); diff --git a/test/extensions/filters/http/tap/tap_filter_integration_test.cc b/test/extensions/filters/http/tap/tap_filter_integration_test.cc index 0157fb487f93..0f6c0111efdc 100644 --- a/test/extensions/filters/http/tap/tap_filter_integration_test.cc +++ b/test/extensions/filters/http/tap/tap_filter_integration_test.cc @@ -1,4 +1,4 @@ -#include "envoy/data/tap/v2alpha/http.pb.h" +#include "envoy/data/tap/v2alpha/wrapper.pb.h" #include "test/integration/http_integration.h" @@ -118,10 +118,10 @@ config_id: test_config_id // Wait for the tap message. admin_response->waitForBodyData(1); - envoy::data::tap::v2alpha::HttpBufferedTrace trace; + envoy::data::tap::v2alpha::BufferedTraceWrapper trace; MessageUtil::loadFromYaml(admin_response->body(), trace); - EXPECT_EQ(trace.request_headers().size(), 8); - EXPECT_EQ(trace.response_headers().size(), 5); + EXPECT_EQ(trace.http_buffered_trace().request_headers().size(), 8); + EXPECT_EQ(trace.http_buffered_trace().response_headers().size(), 5); admin_response->clearBody(); // Do a request which should not tap. @@ -142,11 +142,13 @@ config_id: test_config_id // Wait for the tap message. admin_response->waitForBodyData(1); MessageUtil::loadFromYaml(admin_response->body(), trace); - EXPECT_EQ(trace.request_headers().size(), 7); - EXPECT_EQ("http", findHeader("x-forwarded-proto", trace.request_headers())->value()); - EXPECT_EQ(trace.response_headers().size(), 6); - EXPECT_NE(nullptr, findHeader("date", trace.response_headers())); - EXPECT_EQ("baz", findHeader("bar", trace.response_headers())->value()); + EXPECT_EQ(trace.http_buffered_trace().request_headers().size(), 7); + EXPECT_EQ( + "http", + findHeader("x-forwarded-proto", trace.http_buffered_trace().request_headers())->value()); + EXPECT_EQ(trace.http_buffered_trace().response_headers().size(), 6); + EXPECT_NE(nullptr, findHeader("date", trace.http_buffered_trace().response_headers())); + EXPECT_EQ("baz", findHeader("bar", trace.http_buffered_trace().response_headers())->value()); admin_client_->close(); test_server_->waitForGaugeEq("http.admin.downstream_rq_active", 0); From 9092655c2e3124937edc5e314a799c08355ea3db Mon Sep 17 00:00:00 2001 From: Matt Klein Date: Fri, 18 Jan 2019 08:57:56 -0700 Subject: [PATCH 25/26] fix format Signed-off-by: Matt Klein --- api/envoy/admin/v2alpha/tap.proto | 1 - api/envoy/config/filter/http/tap/v2alpha/tap.proto | 1 - api/envoy/data/tap/v2alpha/http.proto | 1 - api/envoy/data/tap/v2alpha/wrapper.proto | 1 - api/envoy/service/tap/v2alpha/common.proto | 1 - test/proto/helloworld.proto | 1 - 6 files changed, 6 deletions(-) diff --git a/api/envoy/admin/v2alpha/tap.proto b/api/envoy/admin/v2alpha/tap.proto index e2534440b6da..e35ef3cc5aec 100644 --- a/api/envoy/admin/v2alpha/tap.proto +++ b/api/envoy/admin/v2alpha/tap.proto @@ -5,7 +5,6 @@ import "validate/validate.proto"; package envoy.admin.v2alpha; option java_package = "io.envoyproxy.envoy.admin.v2alpha"; -option java_multiple_files = true; // The /tap admin request body that is used to configure an active tap session. message TapRequest { diff --git a/api/envoy/config/filter/http/tap/v2alpha/tap.proto b/api/envoy/config/filter/http/tap/v2alpha/tap.proto index b9337f8f14ea..8018d5f07f92 100644 --- a/api/envoy/config/filter/http/tap/v2alpha/tap.proto +++ b/api/envoy/config/filter/http/tap/v2alpha/tap.proto @@ -6,7 +6,6 @@ import "validate/validate.proto"; package envoy.config.filter.http.tap.v2alpha; option java_package = "io.envoyproxy.envoy.config.filter.http.tap.v2alpha"; -option java_multiple_files = true; // [#protodoc-title: Tap] // Tap :ref:`configuration overview `. diff --git a/api/envoy/data/tap/v2alpha/http.proto b/api/envoy/data/tap/v2alpha/http.proto index 9d15046ea91a..f9c4bb483ad7 100644 --- a/api/envoy/data/tap/v2alpha/http.proto +++ b/api/envoy/data/tap/v2alpha/http.proto @@ -2,7 +2,6 @@ syntax = "proto3"; package envoy.data.tap.v2alpha; option java_package = "io.envoyproxy.envoy.data.tap.v2alpha"; -option java_multiple_files = true; import "envoy/api/v2/core/base.proto"; diff --git a/api/envoy/data/tap/v2alpha/wrapper.proto b/api/envoy/data/tap/v2alpha/wrapper.proto index c8e73d3b3369..12a527e866f8 100644 --- a/api/envoy/data/tap/v2alpha/wrapper.proto +++ b/api/envoy/data/tap/v2alpha/wrapper.proto @@ -6,7 +6,6 @@ import "validate/validate.proto"; package envoy.data.tap.v2alpha; option java_package = "io.envoyproxy.envoy.data.tap.v2alpha"; -option java_multiple_files = true; // [#protodoc-title: Tap data wrappers] diff --git a/api/envoy/service/tap/v2alpha/common.proto b/api/envoy/service/tap/v2alpha/common.proto index b44d715ab24e..a70bacf50d78 100644 --- a/api/envoy/service/tap/v2alpha/common.proto +++ b/api/envoy/service/tap/v2alpha/common.proto @@ -6,7 +6,6 @@ import "validate/validate.proto"; package envoy.service.tap.v2alpha; option java_package = "io.envoyproxy.envoy.service.tap.v2alpha"; -option java_multiple_files = true; // [#protodoc-title: Common tap configuration] diff --git a/test/proto/helloworld.proto b/test/proto/helloworld.proto index 2372a603b49f..007991440cd0 100644 --- a/test/proto/helloworld.proto +++ b/test/proto/helloworld.proto @@ -30,7 +30,6 @@ syntax = "proto3"; option cc_generic_services = true; -option java_multiple_files = true; option java_package = "io.grpc.examples.helloworld"; option java_outer_classname = "HelloWorldProto"; From 96e87487abed5690c652116c492b51581b771bd9 Mon Sep 17 00:00:00 2001 From: Matt Klein Date: Fri, 18 Jan 2019 14:24:53 -0700 Subject: [PATCH 26/26] comments Signed-off-by: Matt Klein --- .../configuration/http_filters/tap_filter.rst | 14 ++++ source/extensions/common/tap/BUILD | 2 +- source/extensions/common/tap/tap.h | 62 ----------------- .../extensions/common/tap/tap_config_base.h | 1 + source/extensions/common/tap/tap_matcher.h | 67 ++++++++++++++++++- 5 files changed, 81 insertions(+), 65 deletions(-) diff --git a/docs/root/configuration/http_filters/tap_filter.rst b/docs/root/configuration/http_filters/tap_filter.rst index 2d3fdf408120..f2fe8bd83ff0 100644 --- a/docs/root/configuration/http_filters/tap_filter.rst +++ b/docs/root/configuration/http_filters/tap_filter.rst @@ -120,6 +120,20 @@ Another example POST body: The preceding configuration instructs the tap filter to match any HTTP requests. All requests will be tapped and streamed out the admin endpoint. +Streaming matching +------------------ + +The tap filter supports "streaming matching." This means that instead of waiting until the end of +the request/response sequence, the filter will match incrementally as the request proceeds. I.e., +first the request headers will be matched, then the request body if present, then the request +trailers if present, then the response headers if present, etc. + +In the future, the filter will support streaming output. Currently only :ref:`fully buffered output +` is implemented. However, even in the current +implementation, if a tap is configured to match request headers and the request headers match, +even if there is no response (upstream failure, etc.) the request will still be tapped and sent +to the configured output. + Statistics ---------- diff --git a/source/extensions/common/tap/BUILD b/source/extensions/common/tap/BUILD index cb5d2028c0ff..5362c729b470 100644 --- a/source/extensions/common/tap/BUILD +++ b/source/extensions/common/tap/BUILD @@ -34,8 +34,8 @@ envoy_cc_library( srcs = ["tap_matcher.cc"], hdrs = ["tap_matcher.h"], deps = [ - ":tap_interface", "//source/common/http:header_utility_lib", + "@envoy_api//envoy/service/tap/v2alpha:common_cc", ], ) diff --git a/source/extensions/common/tap/tap.h b/source/extensions/common/tap/tap.h index e1eab1785786..4d2d6bea469b 100644 --- a/source/extensions/common/tap/tap.h +++ b/source/extensions/common/tap/tap.h @@ -56,68 +56,6 @@ class ExtensionConfig { Sink* admin_streamer) PURE; }; -class Matcher; -using MatcherPtr = std::unique_ptr; - -/** - * Base class for all tap matchers. - * - * A high level note on the design of tap matching which is different from other matching in Envoy - * due to a requirement to support streaming matching (match as new data arrives versus - * calculating the match given all available data at once). - * - The matching system is composed of a constant matching configuration. This is essentially - * a tree of matchers given logical AND, OR, NOT, etc. - * - A per-stream/request matching status must be kept in order to compute interim match status. - * - In order to make this computationally efficient, the matching tree is kept in a vector, with - * all references to other matchers implemented using an index into the vector. - * - The previous point allows the creation of a per-stream/request vector of booleans of the same - * size as the matcher vector. Then, when match status is updated given new information, the - * vector of booleans can be easily updated using the same indexes as in the constant match - * configuration. - * - Finally, a matches() function can be trivially implemented by looking in the status vector at - * the index position that the current matcher is located in. - */ -class Matcher { -public: - virtual ~Matcher() = default; - - /** - * @return the matcher's index in the match tree vector (see above). - */ - size_t index() { return my_index_; } - - /** - * Update match status given new information. - * @param request_headers supplies the request headers, if available. - * @param response_headers supplies the response headers, if available. - * @param statuses supplies the per-stream-request match status vector which must be the same - * size as the match tree vector (see above). - */ - virtual bool updateMatchStatus(const Http::HeaderMap* request_headers, - const Http::HeaderMap* response_headers, - std::vector& statuses) const PURE; - - /** - * @return whether given currently available information, the matcher matches. - * @param statuses supplies the per-stream-request match status vector which must be the same - * size as the match tree vector (see above). - */ - bool matches(const std::vector& statuses) const { return statuses[my_index_]; } - -protected: - /** - * Base class constructor for a matcher. - * @param matchers supplies the match tree vector being built. - */ - Matcher(const std::vector& matchers) - // NOTE: This code assumes that the index for the matcher being constructed has already been - // allocated, which is why my_index_ is set to size() - 1. See buildMatcher() in - // tap_matcher.cc. - : my_index_(matchers.size() - 1) {} - - const size_t my_index_; -}; - } // namespace Tap } // namespace Common } // namespace Extensions diff --git a/source/extensions/common/tap/tap_config_base.h b/source/extensions/common/tap/tap_config_base.h index 4a775d93314b..d273b28a2c94 100644 --- a/source/extensions/common/tap/tap_config_base.h +++ b/source/extensions/common/tap/tap_config_base.h @@ -3,6 +3,7 @@ #include "envoy/service/tap/v2alpha/common.pb.h" #include "extensions/common/tap/tap.h" +#include "extensions/common/tap/tap_matcher.h" namespace Envoy { namespace Extensions { diff --git a/source/extensions/common/tap/tap_matcher.h b/source/extensions/common/tap/tap_matcher.h index f34aaf34e51c..e01ff292316f 100644 --- a/source/extensions/common/tap/tap_matcher.h +++ b/source/extensions/common/tap/tap_matcher.h @@ -1,14 +1,77 @@ #pragma once -#include "common/http/header_utility.h" +#include "envoy/service/tap/v2alpha/common.pb.h" -#include "extensions/common/tap/tap.h" +#include "common/http/header_utility.h" namespace Envoy { namespace Extensions { namespace Common { namespace Tap { +class Matcher; +using MatcherPtr = std::unique_ptr; + +/** + * Base class for all tap matchers. + * + * A high level note on the design of tap matching which is different from other matching in Envoy + * due to a requirement to support streaming matching (match as new data arrives versus + * calculating the match given all available data at once). + * - The matching system is composed of a constant matching configuration. This is essentially + * a tree of matchers given logical AND, OR, NOT, etc. + * - A per-stream/request matching status must be kept in order to compute interim match status. + * - In order to make this computationally efficient, the matching tree is kept in a vector, with + * all references to other matchers implemented using an index into the vector. The vector is + * effectively a flattened N-ary tree. + * - The previous point allows the creation of a per-stream/request vector of booleans of the same + * size as the matcher vector. Then, when match status is updated given new information, the + * vector of booleans can be easily updated using the same indexes as in the constant match + * configuration. + * - Finally, a matches() function can be trivially implemented by looking in the status vector at + * the index position that the current matcher is located in. + */ +class Matcher { +public: + virtual ~Matcher() = default; + + /** + * @return the matcher's index in the match tree vector (see above). + */ + size_t index() { return my_index_; } + + /** + * Update match status given new information. + * @param request_headers supplies the request headers, if available. + * @param response_headers supplies the response headers, if available. + * @param statuses supplies the per-stream-request match status vector which must be the same + * size as the match tree vector (see above). + */ + virtual bool updateMatchStatus(const Http::HeaderMap* request_headers, + const Http::HeaderMap* response_headers, + std::vector& statuses) const PURE; + + /** + * @return whether given currently available information, the matcher matches. + * @param statuses supplies the per-stream-request match status vector which must be the same + * size as the match tree vector (see above). + */ + bool matches(const std::vector& statuses) const { return statuses[my_index_]; } + +protected: + /** + * Base class constructor for a matcher. + * @param matchers supplies the match tree vector being built. + */ + Matcher(const std::vector& matchers) + // NOTE: This code assumes that the index for the matcher being constructed has already been + // allocated, which is why my_index_ is set to size() - 1. See buildMatcher() in + // tap_matcher.cc. + : my_index_(matchers.size() - 1) {} + + const size_t my_index_; +}; + /** * Factory method to build a matcher given a match config. Calling this function may end * up recursively building many matchers, which will all be added to the passed in vector