diff --git a/GOVERNANCE.md b/GOVERNANCE.md index fcbcae9bab016..13342260c7bc1 100644 --- a/GOVERNANCE.md +++ b/GOVERNANCE.md @@ -37,8 +37,8 @@ * Triage GitHub issues and perform pull request reviews for other maintainers and the community. The areas of specialization listed in [OWNERS.md](OWNERS.md) can be used to help with routing an issue/question to the right person. -* Triage build issues - file issues for known flaky builds or bugs, and either fix or find someone - to fix any main build breakages. +* Triage build and CI issues. Monitor #envoy-ci and #test-flaky and file issues for failing builds, + flaky tests or new bugs, and either fix or find someone to fix any main build breakages. * During GitHub issue triage, apply all applicable [labels](https://github.com/envoyproxy/envoy/labels) to each new issue. Labels are extremely useful for future issue follow up. Which labels to apply is somewhat subjective so just use your best judgment. A few of the most important labels that are diff --git a/api/envoy/config/core/v3/base.proto b/api/envoy/config/core/v3/base.proto index f75d918bfc656..d6c507b8dec9a 100644 --- a/api/envoy/config/core/v3/base.proto +++ b/api/envoy/config/core/v3/base.proto @@ -236,7 +236,19 @@ message Metadata { // Key is the reverse DNS filter name, e.g. com.acme.widget. The envoy.* // namespace is reserved for Envoy's built-in filters. + // If both *filter_metadata* and + // :ref:`typed_filter_metadata ` + // fields are present in the metadata with same keys, + // only *typed_filter_metadata* field will be parsed. map filter_metadata = 1; + + // Key is the reverse DNS filter name, e.g. com.acme.widget. The envoy.* + // namespace is reserved for Envoy's built-in filters. + // The value is encoded as google.protobuf.Any. + // If both :ref:`filter_metadata ` + // and *typed_filter_metadata* fields are present in the metadata with same keys, + // only *typed_filter_metadata* field will be parsed. + map typed_filter_metadata = 2; } // Runtime derived uint32 with a default when not specified. diff --git a/api/envoy/config/core/v3/protocol.proto b/api/envoy/config/core/v3/protocol.proto index fb7c86245c6c8..bb1998828464e 100644 --- a/api/envoy/config/core/v3/protocol.proto +++ b/api/envoy/config/core/v3/protocol.proto @@ -71,6 +71,28 @@ message UpstreamHttpProtocolOptions { bool auto_san_validation = 2; } +// Configures the alternate protocols cache which tracks alternate protocols that can be used to +// make an HTTP connection to an origin server. See https://tools.ietf.org/html/rfc7838 for +// HTTP Alternate Services and https://datatracker.ietf.org/doc/html/draft-ietf-dnsop-svcb-https-04 +// for the "HTTPS" DNS resource record. +message AlternateProtocolsCacheOptions { + // The name of the cache. Multiple named caches allow independent alternate protocols cache + // configurations to operate within a single Envoy process using different configurations. All + // alternate protocols cache options with the same name *must* be equal in all fields when + // referenced from different configuration components. Configuration will fail to load if this is + // not the case. + string name = 1 [(validate.rules).string = {min_len: 1}]; + + // The maximum number of entries that the cache will hold. If not specified defaults to 1024. + // + // .. note: + // + // The implementation is approximate and enforced independently on each worker thread, thus + // it is possible for the maximum entries in the cache to go slightly above the configured + // value depending on timing. This is similar to how other circuit breakers work. + google.protobuf.UInt32Value max_entries = 2 [(validate.rules).uint32 = {gt: 0}]; +} + // [#next-free-field: 6] message HttpProtocolOptions { option (udpa.annotations.versioning).previous_message_type = diff --git a/api/envoy/config/core/v4alpha/base.proto b/api/envoy/config/core/v4alpha/base.proto index 1eb3c2c6e0834..b9980eff49ca5 100644 --- a/api/envoy/config/core/v4alpha/base.proto +++ b/api/envoy/config/core/v4alpha/base.proto @@ -226,7 +226,19 @@ message Metadata { // Key is the reverse DNS filter name, e.g. com.acme.widget. The envoy.* // namespace is reserved for Envoy's built-in filters. + // If both *filter_metadata* and + // :ref:`typed_filter_metadata ` + // fields are present in the metadata with same keys, + // only *typed_filter_metadata* field will be parsed. map filter_metadata = 1; + + // Key is the reverse DNS filter name, e.g. com.acme.widget. The envoy.* + // namespace is reserved for Envoy's built-in filters. + // The value is encoded as google.protobuf.Any. + // If both :ref:`filter_metadata ` + // and *typed_filter_metadata* fields are present in the metadata with same keys, + // only *typed_filter_metadata* field will be parsed. + map typed_filter_metadata = 2; } // Runtime derived uint32 with a default when not specified. diff --git a/api/envoy/config/core/v4alpha/protocol.proto b/api/envoy/config/core/v4alpha/protocol.proto index 7d09eb016c9aa..2c55569e0c0ae 100644 --- a/api/envoy/config/core/v4alpha/protocol.proto +++ b/api/envoy/config/core/v4alpha/protocol.proto @@ -73,6 +73,31 @@ message UpstreamHttpProtocolOptions { bool auto_san_validation = 2; } +// Configures the alternate protocols cache which tracks alternate protocols that can be used to +// make an HTTP connection to an origin server. See https://tools.ietf.org/html/rfc7838 for +// HTTP Alternate Services and https://datatracker.ietf.org/doc/html/draft-ietf-dnsop-svcb-https-04 +// for the "HTTPS" DNS resource record. +message AlternateProtocolsCacheOptions { + option (udpa.annotations.versioning).previous_message_type = + "envoy.config.core.v3.AlternateProtocolsCacheOptions"; + + // The name of the cache. Multiple named caches allow independent alternate protocols cache + // configurations to operate within a single Envoy process using different configurations. All + // alternate protocols cache options with the same name *must* be equal in all fields when + // referenced from different configuration components. Configuration will fail to load if this is + // not the case. + string name = 1 [(validate.rules).string = {min_len: 1}]; + + // The maximum number of entries that the cache will hold. If not specified defaults to 1024. + // + // .. note: + // + // The implementation is approximate and enforced independently on each worker thread, thus + // it is possible for the maximum entries in the cache to go slightly above the configured + // value depending on timing. This is similar to how other circuit breakers work. + google.protobuf.UInt32Value max_entries = 2 [(validate.rules).uint32 = {gt: 0}]; +} + // [#next-free-field: 6] message HttpProtocolOptions { option (udpa.annotations.versioning).previous_message_type = diff --git a/api/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto b/api/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto index 90f4b276dc3cd..df141cb13087b 100644 --- a/api/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto +++ b/api/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto @@ -979,7 +979,7 @@ message HttpFilter { // If true, clients that do not support this filter may ignore the // filter but otherwise accept the config. // Otherwise, clients that do not support this filter must reject the config. - // [#not-implemented-hide:] + // This is also same with typed per filter config. bool is_optional = 6; } diff --git a/api/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto b/api/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto index 4f61ffb8ccca3..37f15c7d49633 100644 --- a/api/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto +++ b/api/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto @@ -964,7 +964,7 @@ message HttpFilter { // If true, clients that do not support this filter may ignore the // filter but otherwise accept the config. // Otherwise, clients that do not support this filter must reject the config. - // [#not-implemented-hide:] + // This is also same with typed per filter config. bool is_optional = 6; } diff --git a/api/envoy/extensions/upstreams/http/v3/http_protocol_options.proto b/api/envoy/extensions/upstreams/http/v3/http_protocol_options.proto index 2c8790c84faf5..f99f4059166b5 100644 --- a/api/envoy/extensions/upstreams/http/v3/http_protocol_options.proto +++ b/api/envoy/extensions/upstreams/http/v3/http_protocol_options.proto @@ -114,6 +114,14 @@ message HttpProtocolOptions { // AutoHttpConfig config is undergoing especially rapid change and as it // is alpha is not guaranteed to be API-stable. config.core.v3.Http3ProtocolOptions http3_protocol_options = 3; + + // [#not-implemented-hide:] + // The presence of alternate protocols cache options causes the use of the + // alternate protocols cache, which is responsible for parsing and caching + // HTTP Alt-Svc headers. This enables the use of HTTP/3 for origins that + // advertise supporting it. + // TODO(RyanTheOptimist): Make this field required when HTTP/3 is enabled. + config.core.v3.AlternateProtocolsCacheOptions alternate_protocols_cache_options = 4; } // This contains options common across HTTP/1 and HTTP/2 diff --git a/api/envoy/extensions/upstreams/http/v4alpha/http_protocol_options.proto b/api/envoy/extensions/upstreams/http/v4alpha/http_protocol_options.proto index 9ceadcec907c6..10971c2587f04 100644 --- a/api/envoy/extensions/upstreams/http/v4alpha/http_protocol_options.proto +++ b/api/envoy/extensions/upstreams/http/v4alpha/http_protocol_options.proto @@ -127,6 +127,14 @@ message HttpProtocolOptions { // AutoHttpConfig config is undergoing especially rapid change and as it // is alpha is not guaranteed to be API-stable. config.core.v4alpha.Http3ProtocolOptions http3_protocol_options = 3; + + // [#not-implemented-hide:] + // The presence of alternate protocols cache options causes the use of the + // alternate protocols cache, which is responsible for parsing and caching + // HTTP Alt-Svc headers. This enables the use of HTTP/3 for origins that + // advertise supporting it. + // TODO(RyanTheOptimist): Make this field required when HTTP/3 is enabled. + config.core.v4alpha.AlternateProtocolsCacheOptions alternate_protocols_cache_options = 4; } // This contains options common across HTTP/1 and HTTP/2 diff --git a/ci/flaky_test/process_xml.py b/ci/flaky_test/process_xml.py index 1445248490f54..9cbfa56bfbb7e 100755 --- a/ci/flaky_test/process_xml.py +++ b/ci/flaky_test/process_xml.py @@ -102,9 +102,15 @@ def parse_and_print_test_suite_error(testsuite, log_path): # parsed into the XML metadata. Here we attempt to extract those names from the log by # finding the last test case to run. The expected format of that is: # "[ RUN ] /.\n". + last_test_fullname = test_output.split('[ RUN ]')[-1].splitlines()[0] - last_testsuite = last_test_fullname.split('/')[1].split('.')[0] - last_testcase = last_test_fullname.split('.')[1] + last_test_fullname_splitted_on_dot = last_test_fullname.split('.') + if len(last_test_fullname_splitted_on_dot) == 2: + last_testcase = last_test_fullname_splitted_on_dot[1] + last_testsuite = last_test_fullname_splitted_on_dot[0].split('/')[-1] + else: + last_testcase = "Could not retrieve last test case" + last_testsuite = "Could not retrieve last test suite" if error_msg != "": return print_test_suite_error( diff --git a/docs/requirements.txt b/docs/requirements.txt index 9ae3006b4a475..0eae9f8c01043 100644 --- a/docs/requirements.txt +++ b/docs/requirements.txt @@ -41,9 +41,9 @@ gitdb==4.0.7 \ # via # -r docs/requirements.txt # gitpython -gitpython==3.1.14 \ - --hash=sha256:3283ae2fba31c913d857e12e5ba5f9a7772bbc064ae2bb09efafa71b0dd4939b \ - --hash=sha256:be27633e7509e58391f10207cd32b2a6cf5b908f92d9cd30da2e514e1137af61 +gitpython==3.1.17 \ + --hash=sha256:29fe82050709760081f588dd50ce83504feddbebdc4da6956d02351552b1c135 \ + --hash=sha256:ee24bdc93dce357630764db659edaf6b8d664d4ff5447ccfeedd2dc5c253f41e # via -r docs/requirements.txt idna==2.10 \ --hash=sha256:b307872f855b18632ce0c21c5e45be78c0ea7ae4c15c828c20788b26921eb3f6 \ diff --git a/docs/root/configuration/listeners/listener_filters/original_dst_filter.rst b/docs/root/configuration/listeners/listener_filters/original_dst_filter.rst index 05051629c94f3..533e218d35772 100644 --- a/docs/root/configuration/listeners/listener_filters/original_dst_filter.rst +++ b/docs/root/configuration/listeners/listener_filters/original_dst_filter.rst @@ -4,14 +4,14 @@ Original Destination ==================== Linux -=============== +----- Original destination listener filter reads the SO_ORIGINAL_DST socket option set when a connection has been redirected by an iptables REDIRECT target, or by an iptables TPROXY target in combination with setting the listener's :ref:`transparent ` option. Windows -=============== +------- Original destination listener filter reads the SO_ORIGINAL_DST socket option set when a connection has been redirected by an `HNS `_ diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index b00d52cbde733..2ba5d8a0489a6 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -31,7 +31,7 @@ Minor Behavior Changes * http: serve HEAD requests from cache. * listener: respect the :ref:`connection balance config ` defined within the listener where the sockets are redirected to. Clear that field to restore the previous behavior. - +* tcp: switched to the new connection pool by default. Any unexpected behavioral changes can be reverted by setting runtime guard ``envoy.reloadable_features.new_tcp_connection_pool`` to false. Bug Fixes @@ -60,7 +60,12 @@ Removed Config or Runtime New Features ------------ +* http: a new field `is_optional` is added to `extensions.filters.network.http_connection_manager.v3.HttpFilter`. When + value is `true`, the unsupported http filter will be ignored by envoy. This is also same with unsupported http filter + in the typed per filter config. For more information, please reference + :ref:`HttpFilter `. +* crash support: restore crash context when continuing to processing requests or responses as a result of an asynchronous callback that invokes a filter directly. This is unlike the call stacks that go through the various network layers, to eventually reach the filter. For a concrete example see: ``Envoy::Extensions::HttpFilters::Cache::CacheFilter::getHeaders`` which posts a callback on the dispatcher that will invoke the filter directly. * http: added support for :ref:`original IP detection extensions`. Two initial extensions were added, the :ref:`custom header ` extension and the :ref:`xff ` extension. @@ -68,6 +73,7 @@ New Features * http: added upstream and downstream alpha HTTP/3 support! See :ref:`quic_options ` for downstream and the new http3_protocol_options in :ref:`http_protocol_options ` for upstream HTTP/3. * listener: added ability to change an existing listener's address. * metric service: added support for sending metric tags as labels. This can be enabled by setting the :ref:`emit_tags_as_labels ` field to true. +* tcp: added support for :ref:`preconnecting `. Preconnecting is off by default, but recommended for clusters serving latency-sensitive traffic. * udp_proxy: added :ref:`key ` as another hash policy to support hash based routing on any given key. Deprecated diff --git a/examples/grpc-bridge/client/requirements.txt b/examples/grpc-bridge/client/requirements.txt index 327544f83036d..bda4a54f6297d 100644 --- a/examples/grpc-bridge/client/requirements.txt +++ b/examples/grpc-bridge/client/requirements.txt @@ -1,4 +1,4 @@ requests>=2.22.0 grpcio grpcio-tools -protobuf==3.16.0 +protobuf==3.17.0 diff --git a/generated_api_shadow/envoy/config/core/v3/base.proto b/generated_api_shadow/envoy/config/core/v3/base.proto index 69d019daeecc2..9b1ca815723b2 100644 --- a/generated_api_shadow/envoy/config/core/v3/base.proto +++ b/generated_api_shadow/envoy/config/core/v3/base.proto @@ -235,7 +235,19 @@ message Metadata { // Key is the reverse DNS filter name, e.g. com.acme.widget. The envoy.* // namespace is reserved for Envoy's built-in filters. + // If both *filter_metadata* and + // :ref:`typed_filter_metadata ` + // fields are present in the metadata with same keys, + // only *typed_filter_metadata* field will be parsed. map filter_metadata = 1; + + // Key is the reverse DNS filter name, e.g. com.acme.widget. The envoy.* + // namespace is reserved for Envoy's built-in filters. + // The value is encoded as google.protobuf.Any. + // If both :ref:`filter_metadata ` + // and *typed_filter_metadata* fields are present in the metadata with same keys, + // only *typed_filter_metadata* field will be parsed. + map typed_filter_metadata = 2; } // Runtime derived uint32 with a default when not specified. diff --git a/generated_api_shadow/envoy/config/core/v3/protocol.proto b/generated_api_shadow/envoy/config/core/v3/protocol.proto index fb7c86245c6c8..bb1998828464e 100644 --- a/generated_api_shadow/envoy/config/core/v3/protocol.proto +++ b/generated_api_shadow/envoy/config/core/v3/protocol.proto @@ -71,6 +71,28 @@ message UpstreamHttpProtocolOptions { bool auto_san_validation = 2; } +// Configures the alternate protocols cache which tracks alternate protocols that can be used to +// make an HTTP connection to an origin server. See https://tools.ietf.org/html/rfc7838 for +// HTTP Alternate Services and https://datatracker.ietf.org/doc/html/draft-ietf-dnsop-svcb-https-04 +// for the "HTTPS" DNS resource record. +message AlternateProtocolsCacheOptions { + // The name of the cache. Multiple named caches allow independent alternate protocols cache + // configurations to operate within a single Envoy process using different configurations. All + // alternate protocols cache options with the same name *must* be equal in all fields when + // referenced from different configuration components. Configuration will fail to load if this is + // not the case. + string name = 1 [(validate.rules).string = {min_len: 1}]; + + // The maximum number of entries that the cache will hold. If not specified defaults to 1024. + // + // .. note: + // + // The implementation is approximate and enforced independently on each worker thread, thus + // it is possible for the maximum entries in the cache to go slightly above the configured + // value depending on timing. This is similar to how other circuit breakers work. + google.protobuf.UInt32Value max_entries = 2 [(validate.rules).uint32 = {gt: 0}]; +} + // [#next-free-field: 6] message HttpProtocolOptions { option (udpa.annotations.versioning).previous_message_type = diff --git a/generated_api_shadow/envoy/config/core/v4alpha/base.proto b/generated_api_shadow/envoy/config/core/v4alpha/base.proto index 6c23e0e8022e1..99ce121ddf63f 100644 --- a/generated_api_shadow/envoy/config/core/v4alpha/base.proto +++ b/generated_api_shadow/envoy/config/core/v4alpha/base.proto @@ -235,7 +235,19 @@ message Metadata { // Key is the reverse DNS filter name, e.g. com.acme.widget. The envoy.* // namespace is reserved for Envoy's built-in filters. + // If both *filter_metadata* and + // :ref:`typed_filter_metadata ` + // fields are present in the metadata with same keys, + // only *typed_filter_metadata* field will be parsed. map filter_metadata = 1; + + // Key is the reverse DNS filter name, e.g. com.acme.widget. The envoy.* + // namespace is reserved for Envoy's built-in filters. + // The value is encoded as google.protobuf.Any. + // If both :ref:`filter_metadata ` + // and *typed_filter_metadata* fields are present in the metadata with same keys, + // only *typed_filter_metadata* field will be parsed. + map typed_filter_metadata = 2; } // Runtime derived uint32 with a default when not specified. diff --git a/generated_api_shadow/envoy/config/core/v4alpha/protocol.proto b/generated_api_shadow/envoy/config/core/v4alpha/protocol.proto index e682602c350c1..ba1c933eeb4d8 100644 --- a/generated_api_shadow/envoy/config/core/v4alpha/protocol.proto +++ b/generated_api_shadow/envoy/config/core/v4alpha/protocol.proto @@ -74,6 +74,31 @@ message UpstreamHttpProtocolOptions { bool auto_san_validation = 2; } +// Configures the alternate protocols cache which tracks alternate protocols that can be used to +// make an HTTP connection to an origin server. See https://tools.ietf.org/html/rfc7838 for +// HTTP Alternate Services and https://datatracker.ietf.org/doc/html/draft-ietf-dnsop-svcb-https-04 +// for the "HTTPS" DNS resource record. +message AlternateProtocolsCacheOptions { + option (udpa.annotations.versioning).previous_message_type = + "envoy.config.core.v3.AlternateProtocolsCacheOptions"; + + // The name of the cache. Multiple named caches allow independent alternate protocols cache + // configurations to operate within a single Envoy process using different configurations. All + // alternate protocols cache options with the same name *must* be equal in all fields when + // referenced from different configuration components. Configuration will fail to load if this is + // not the case. + string name = 1 [(validate.rules).string = {min_len: 1}]; + + // The maximum number of entries that the cache will hold. If not specified defaults to 1024. + // + // .. note: + // + // The implementation is approximate and enforced independently on each worker thread, thus + // it is possible for the maximum entries in the cache to go slightly above the configured + // value depending on timing. This is similar to how other circuit breakers work. + google.protobuf.UInt32Value max_entries = 2 [(validate.rules).uint32 = {gt: 0}]; +} + // [#next-free-field: 6] message HttpProtocolOptions { option (udpa.annotations.versioning).previous_message_type = diff --git a/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto b/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto index 1ac96a5a292cc..76c8488c32be6 100644 --- a/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto +++ b/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto @@ -991,7 +991,7 @@ message HttpFilter { // If true, clients that do not support this filter may ignore the // filter but otherwise accept the config. // Otherwise, clients that do not support this filter must reject the config. - // [#not-implemented-hide:] + // This is also same with typed per filter config. bool is_optional = 6; } diff --git a/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto b/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto index 7582fc2611ee3..e2f85d4b3e15d 100644 --- a/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto +++ b/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto @@ -988,7 +988,7 @@ message HttpFilter { // If true, clients that do not support this filter may ignore the // filter but otherwise accept the config. // Otherwise, clients that do not support this filter must reject the config. - // [#not-implemented-hide:] + // This is also same with typed per filter config. bool is_optional = 6; } diff --git a/generated_api_shadow/envoy/extensions/upstreams/http/v3/http_protocol_options.proto b/generated_api_shadow/envoy/extensions/upstreams/http/v3/http_protocol_options.proto index 2c8790c84faf5..f99f4059166b5 100644 --- a/generated_api_shadow/envoy/extensions/upstreams/http/v3/http_protocol_options.proto +++ b/generated_api_shadow/envoy/extensions/upstreams/http/v3/http_protocol_options.proto @@ -114,6 +114,14 @@ message HttpProtocolOptions { // AutoHttpConfig config is undergoing especially rapid change and as it // is alpha is not guaranteed to be API-stable. config.core.v3.Http3ProtocolOptions http3_protocol_options = 3; + + // [#not-implemented-hide:] + // The presence of alternate protocols cache options causes the use of the + // alternate protocols cache, which is responsible for parsing and caching + // HTTP Alt-Svc headers. This enables the use of HTTP/3 for origins that + // advertise supporting it. + // TODO(RyanTheOptimist): Make this field required when HTTP/3 is enabled. + config.core.v3.AlternateProtocolsCacheOptions alternate_protocols_cache_options = 4; } // This contains options common across HTTP/1 and HTTP/2 diff --git a/generated_api_shadow/envoy/extensions/upstreams/http/v4alpha/http_protocol_options.proto b/generated_api_shadow/envoy/extensions/upstreams/http/v4alpha/http_protocol_options.proto index 9ceadcec907c6..10971c2587f04 100644 --- a/generated_api_shadow/envoy/extensions/upstreams/http/v4alpha/http_protocol_options.proto +++ b/generated_api_shadow/envoy/extensions/upstreams/http/v4alpha/http_protocol_options.proto @@ -127,6 +127,14 @@ message HttpProtocolOptions { // AutoHttpConfig config is undergoing especially rapid change and as it // is alpha is not guaranteed to be API-stable. config.core.v4alpha.Http3ProtocolOptions http3_protocol_options = 3; + + // [#not-implemented-hide:] + // The presence of alternate protocols cache options causes the use of the + // alternate protocols cache, which is responsible for parsing and caching + // HTTP Alt-Svc headers. This enables the use of HTTP/3 for origins that + // advertise supporting it. + // TODO(RyanTheOptimist): Make this field required when HTTP/3 is enabled. + config.core.v4alpha.AlternateProtocolsCacheOptions alternate_protocols_cache_options = 4; } // This contains options common across HTTP/1 and HTTP/2 diff --git a/include/envoy/config/typed_metadata.h b/include/envoy/config/typed_metadata.h index a05ca8235d645..9406e6a99baa8 100644 --- a/include/envoy/config/typed_metadata.h +++ b/include/envoy/config/typed_metadata.h @@ -65,6 +65,17 @@ class TypedMetadataFactory : public UntypedFactory { virtual std::unique_ptr parse(const ProtobufWkt::Struct& data) const PURE; + /** + * Convert the google.protobuf.Any into an instance of TypedMetadata::Object. + * It should throw an EnvoyException in case the conversion can't be completed. + * @param data config data stored as a protobuf any. + * @return a derived class object pointer of TypedMetadata or return nullptr if + * one doesn't implement parse() method. + * @throw EnvoyException if the parsing can't be done. + */ + virtual std::unique_ptr + parse(const ProtobufWkt::Any& data) const PURE; + std::string category() const override { return "envoy.typed_metadata"; } }; diff --git a/include/envoy/event/dispatcher.h b/include/envoy/event/dispatcher.h index 741c6a4af5ac8..1618c9236d178 100644 --- a/include/envoy/event/dispatcher.h +++ b/include/envoy/event/dispatcher.h @@ -116,6 +116,11 @@ class DispatcherBase { */ virtual void popTrackedObject(const ScopeTrackedObject* expected_object) PURE; + /** + * Whether the tracked object stack is empty. + */ + virtual bool trackedObjectStackIsEmpty() const PURE; + /** * Validates that an operation is thread-safe with respect to this dispatcher; i.e. that the * current thread of execution is on the same thread upon which the dispatcher loop is running. diff --git a/include/envoy/http/BUILD b/include/envoy/http/BUILD index 749355e04c5dc..de8f76446a1dd 100644 --- a/include/envoy/http/BUILD +++ b/include/envoy/http/BUILD @@ -11,7 +11,10 @@ envoy_package() envoy_cc_library( name = "alternate_protocols_cache_interface", hdrs = ["alternate_protocols_cache.h"], - deps = ["//include/envoy/common:time_interface"], + deps = [ + "//include/envoy/common:time_interface", + "@envoy_api//envoy/config/core/v3:pkg_cc_proto", + ], ) envoy_cc_library( @@ -90,6 +93,7 @@ envoy_cc_library( "//include/envoy/ssl:connection_interface", "//include/envoy/stream_info:stream_info_interface", "//include/envoy/tracing:http_tracer_interface", + "//source/common/common:scope_tracked_object_stack", ], ) diff --git a/include/envoy/http/alternate_protocols_cache.h b/include/envoy/http/alternate_protocols_cache.h index 667a2163a9ab9..6d94f32eafcb6 100644 --- a/include/envoy/http/alternate_protocols_cache.h +++ b/include/envoy/http/alternate_protocols_cache.h @@ -1,12 +1,14 @@ #pragma once #include +#include #include #include #include #include "envoy/common/optref.h" #include "envoy/common/time.h" +#include "envoy/config/core/v3/protocol.pb.h" #include "absl/strings/string_view.h" @@ -26,7 +28,8 @@ class AlternateProtocolsCache { */ struct Origin { public: - Origin(absl::string_view scheme, absl::string_view hostname, uint32_t port); + Origin(absl::string_view scheme, absl::string_view hostname, uint32_t port) + : scheme_(scheme), hostname_(hostname), port_(port) {} bool operator==(const Origin& other) const { return std::tie(scheme_, hostname_, port_) == @@ -65,7 +68,8 @@ class AlternateProtocolsCache { */ struct AlternateProtocol { public: - AlternateProtocol(absl::string_view alpn, absl::string_view hostname, uint32_t port); + AlternateProtocol(absl::string_view alpn, absl::string_view hostname, uint32_t port) + : alpn_(alpn), hostname_(hostname), port_(port) {} bool operator==(const AlternateProtocol& other) const { return std::tie(alpn_, hostname_, port_) == @@ -94,9 +98,9 @@ class AlternateProtocolsCache { /** * Returns the possible alternative protocols which can be used to connect to the - * specified origin, or nullptr if not alternatives are found. The returned pointer - * is owned by the AlternateProtocolsCacheImpl and is valid until the next operation on - * AlternateProtocolsCacheImpl. + * specified origin, or nullptr if not alternatives are found. The returned reference + * is owned by the AlternateProtocolsCache and is valid until the next operation on the + * AlternateProtocolsCache. * @param origin The origin to find alternate protocols for. * @return An optional list of alternate protocols for the given origin. */ @@ -109,5 +113,38 @@ class AlternateProtocolsCache { virtual size_t size() const PURE; }; +using AlternateProtocolsCacheSharedPtr = std::shared_ptr; + +/** + * A manager for multiple alternate protocols caches. + */ +class AlternateProtocolsCacheManager { +public: + virtual ~AlternateProtocolsCacheManager() = default; + + /** + * Get an alternate protocols cache. + * @param config supplies the cache parameters. If a cache exists with the same parameters it + * will be returned, otherwise a new one will be created. + */ + virtual AlternateProtocolsCacheSharedPtr + getCache(const envoy::config::core::v3::AlternateProtocolsCacheOptions& config) PURE; +}; + +using AlternateProtocolsCacheManagerSharedPtr = std::shared_ptr; + +/** + * Factory for getting an alternate protocols cache manager. + */ +class AlternateProtocolsCacheManagerFactory { +public: + virtual ~AlternateProtocolsCacheManagerFactory() = default; + + /** + * Get the alternate protocols cache manager. + */ + virtual AlternateProtocolsCacheManagerSharedPtr get() PURE; +}; + } // namespace Http } // namespace Envoy diff --git a/include/envoy/http/codec.h b/include/envoy/http/codec.h index 70544787ca44d..9e91a4d9df4a0 100644 --- a/include/envoy/http/codec.h +++ b/include/envoy/http/codec.h @@ -208,14 +208,13 @@ class RequestDecoder : public virtual StreamDecoder { /** * Called if the codec needs to send a protocol error. - * @param is_grpc_request indicates if the request is a gRPC request * @param code supplies the HTTP error code to send. * @param body supplies an optional body to send with the local reply. * @param modify_headers supplies a way to edit headers before they are sent downstream. * @param grpc_status an optional gRPC status for gRPC requests * @param details details about the source of the error, for debug purposes */ - virtual void sendLocalReply(bool is_grpc_request, Code code, absl::string_view body, + virtual void sendLocalReply(Code code, absl::string_view body, const std::function& modify_headers, const absl::optional grpc_status, absl::string_view details) PURE; diff --git a/include/envoy/http/filter.h b/include/envoy/http/filter.h index f26348cd5e0a3..48a23a7e8e7b0 100644 --- a/include/envoy/http/filter.h +++ b/include/envoy/http/filter.h @@ -17,6 +17,8 @@ #include "envoy/tracing/http_tracer.h" #include "envoy/upstream/upstream.h" +#include "common/common/scope_tracked_object_stack.h" + #include "absl/types/optional.h" namespace Envoy { @@ -283,6 +285,16 @@ class StreamFilterCallbacks { * @return the ScopeTrackedObject for this stream. */ virtual const ScopeTrackedObject& scope() PURE; + + /** + * Should be used when we continue processing a request or response by invoking a filter directly + * from an asynchronous callback to restore crash context. If not explicitly used by the filter + * itself, this gets invoked in ActiveStreamFilterBase::commonContinue(). + * + * @param tracked_object_stack ScopeTrackedObjectStack where relevant ScopeTrackedObjects will be + * added to. + */ + virtual void restoreContextOnContinue(ScopeTrackedObjectStack& tracked_object_stack) PURE; }; /** diff --git a/include/envoy/network/connection.h b/include/envoy/network/connection.h index f70fe1a569077..620a5b2e1ec31 100644 --- a/include/envoy/network/connection.h +++ b/include/envoy/network/connection.h @@ -7,6 +7,7 @@ #include "envoy/buffer/buffer.h" #include "envoy/common/pure.h" +#include "envoy/common/scope_tracker.h" #include "envoy/event/deferred_deletable.h" #include "envoy/network/address.h" #include "envoy/network/filter.h" @@ -74,7 +75,9 @@ enum class ConnectionCloseType { /** * An abstract raw connection. Free the connection or call close() to disconnect. */ -class Connection : public Event::DeferredDeletable, public FilterManager { +class Connection : public Event::DeferredDeletable, + public FilterManager, + public ScopeTrackedObject { public: enum class State { Open, Closing, Closed }; diff --git a/include/envoy/router/route_config_provider_manager.h b/include/envoy/router/route_config_provider_manager.h index 67a184f2ba8e6..e83ad69225f1f 100644 --- a/include/envoy/router/route_config_provider_manager.h +++ b/include/envoy/router/route_config_provider_manager.h @@ -25,6 +25,7 @@ namespace Router { class RouteConfigProviderManager { public: virtual ~RouteConfigProviderManager() = default; + using OptionalHttpFilters = absl::flat_hash_set; /** * Get a RouteConfigProviderPtr for a route from RDS. Ownership of the RouteConfigProvider is the @@ -33,6 +34,7 @@ class RouteConfigProviderManager { * the RouteConfigProvider. This method creates a RouteConfigProvider which may share the * underlying RDS subscription with the same (route_config_name, cluster). * @param rds supplies the proto configuration of an RDS-configured RouteConfigProvider. + * @param optional_http_filters a set of optional http filter names. * @param factory_context is the context to use for the route config provider. * @param stat_prefix supplies the stat_prefix to use for the provider stats. * @param init_manager the Init::Manager used to coordinate initialization of a the underlying RDS @@ -40,6 +42,7 @@ class RouteConfigProviderManager { */ virtual RouteConfigProviderSharedPtr createRdsRouteConfigProvider( const envoy::extensions::filters::network::http_connection_manager::v3::Rds& rds, + const OptionalHttpFilters& optional_http_filters, Server::Configuration::ServerFactoryContext& factory_context, const std::string& stat_prefix, Init::Manager& init_manager) PURE; @@ -47,11 +50,13 @@ class RouteConfigProviderManager { * Get a RouteConfigSharedPtr for a statically defined route. Ownership is as described for * getRdsRouteConfigProvider above. This method always create a new RouteConfigProvider. * @param route_config supplies the RouteConfiguration for this route + * @param optional_http_filters a set of optional http filter names. * @param factory_context is the context to use for the route config provider. * @param validator is the message validator for route config. */ virtual RouteConfigProviderPtr createStaticRouteConfigProvider(const envoy::config::route::v3::RouteConfiguration& route_config, + const OptionalHttpFilters& optional_http_filters, Server::Configuration::ServerFactoryContext& factory_context, ProtobufMessage::ValidationVisitor& validator) PURE; }; diff --git a/include/envoy/upstream/cluster_manager.h b/include/envoy/upstream/cluster_manager.h index 14744078e69a9..e06a45c14ffa3 100644 --- a/include/envoy/upstream/cluster_manager.h +++ b/include/envoy/upstream/cluster_manager.h @@ -12,6 +12,7 @@ #include "envoy/config/cluster/v3/cluster.pb.h" #include "envoy/config/core/v3/address.pb.h" #include "envoy/config/core/v3/config_source.pb.h" +#include "envoy/config/core/v3/protocol.pb.h" #include "envoy/config/grpc_mux.h" #include "envoy/config/subscription_factory.h" #include "envoy/grpc/async_client_manager.h" @@ -359,6 +360,8 @@ class ClusterManagerFactory { virtual Http::ConnectionPool::InstancePtr allocateConnPool(Event::Dispatcher& dispatcher, HostConstSharedPtr host, ResourcePriority priority, std::vector& protocol, + const absl::optional& + alternate_protocol_options, const Network::ConnectionSocket::OptionsSharedPtr& options, const Network::TransportSocketOptionsSharedPtr& transport_socket_options, TimeSource& time_source, ClusterConnectivityState& state) PURE; diff --git a/include/envoy/upstream/upstream.h b/include/envoy/upstream/upstream.h index dc628e6139dbc..d238be74aad29 100644 --- a/include/envoy/upstream/upstream.h +++ b/include/envoy/upstream/upstream.h @@ -985,6 +985,12 @@ class ClusterInfo { virtual const absl::optional& upstreamHttpProtocolOptions() const PURE; + /** + * @return alternate protocols cache options for upstream connections. + */ + virtual const absl::optional& + alternateProtocolsCacheOptions() const PURE; + /** * @return the Http1 Codec Stats. */ diff --git a/source/common/common/BUILD b/source/common/common/BUILD index 4d8b5b5cc0228..664e2564e3fa3 100644 --- a/source/common/common/BUILD +++ b/source/common/common/BUILD @@ -324,6 +324,14 @@ envoy_cc_library( ], ) +envoy_cc_library( + name = "scope_tracked_object_stack", + hdrs = ["scope_tracked_object_stack.h"], + deps = [ + "//include/envoy/common:scope_tracker_interface", + ], +) + envoy_cc_library( name = "stl_helpers", hdrs = ["stl_helpers.h"], diff --git a/source/common/common/scope_tracked_object_stack.h b/source/common/common/scope_tracked_object_stack.h new file mode 100644 index 0000000000000..8e6ee276084e8 --- /dev/null +++ b/source/common/common/scope_tracked_object_stack.h @@ -0,0 +1,35 @@ +#pragma once + +#include +#include + +#include "envoy/common/scope_tracker.h" + +namespace Envoy { + +// Encapsulates zero or more ScopeTrackedObjects. +// +// This is currently used to restore the underlying request context if a +// filter continues processing a request or response due to being invoked directly from an +// asynchronous callback. +class ScopeTrackedObjectStack : public ScopeTrackedObject { +public: + ScopeTrackedObjectStack() = default; + + // Not copyable or movable + ScopeTrackedObjectStack(const ScopeTrackedObjectStack&) = delete; + ScopeTrackedObjectStack& operator=(const ScopeTrackedObjectStack&) = delete; + + void add(const ScopeTrackedObject& object) { tracked_objects_.push_back(object); } + + void dumpState(std::ostream& os, int indent_level) const override { + for (auto iter = tracked_objects_.rbegin(); iter != tracked_objects_.rend(); ++iter) { + iter->get().dumpState(os, indent_level); + } + } + +private: + std::vector> tracked_objects_; +}; + +} // namespace Envoy diff --git a/source/common/config/metadata.h b/source/common/config/metadata.h index efac4eff7e59d..9b4ddbbcb0f43 100644 --- a/source/common/config/metadata.h +++ b/source/common/config/metadata.h @@ -116,8 +116,20 @@ template class TypedMetadataImpl : public TypedMetadata */ void populateFrom(const envoy::config::core::v3::Metadata& metadata) { auto& data_by_key = metadata.filter_metadata(); + auto& typed_data_by_key = metadata.typed_filter_metadata(); for (const auto& [factory_name, factory] : Registry::FactoryRegistry::factories()) { + const auto& typed_meta_iter = typed_data_by_key.find(factory_name); + // If the key exists in Any metadata, and parse() does not return nullptr, + // populate data_. + if (typed_meta_iter != typed_data_by_key.end()) { + auto result = factory->parse(typed_meta_iter->second); + if (result != nullptr) { + data_[factory->name()] = std::move(result); + continue; + } + } + // Fall back cases to parsing Struct metadata and populate data_. const auto& meta_iter = data_by_key.find(factory_name); if (meta_iter != data_by_key.end()) { data_[factory->name()] = factory->parse(meta_iter->second); diff --git a/source/common/config/utility.h b/source/common/config/utility.h index 187be996622ca..93304377ccf4b 100644 --- a/source/common/config/utility.h +++ b/source/common/config/utility.h @@ -249,21 +249,33 @@ class Utility { * Get a Factory from the registry with a particular name (and templated type) with error checking * to ensure the name and factory are valid. * @param name string identifier for the particular implementation. + * @param is_optional exception will be throw when the value is false and no factory found. * @return factory the factory requested or nullptr if it does not exist. */ - template static Factory& getAndCheckFactoryByName(const std::string& name) { + template + static Factory* getAndCheckFactoryByName(const std::string& name, bool is_optional) { if (name.empty()) { ExceptionUtil::throwEnvoyException("Provided name for static registration lookup was empty."); } Factory* factory = Registry::FactoryRegistry::getFactory(name); - if (factory == nullptr) { + if (factory == nullptr && !is_optional) { ExceptionUtil::throwEnvoyException( fmt::format("Didn't find a registered implementation for name: '{}'", name)); } - return *factory; + return factory; + } + + /** + * Get a Factory from the registry with a particular name (and templated type) with error checking + * to ensure the name and factory are valid. + * @param name string identifier for the particular implementation. + * @return factory the factory requested or nullptr if it does not exist. + */ + template static Factory& getAndCheckFactoryByName(const std::string& name) { + return *getAndCheckFactoryByName(name, false); } /** @@ -294,17 +306,29 @@ class Utility { /** * Get a Factory from the registry with error checking to ensure the name and the factory are - * valid. + * valid. And a flag to control return nullptr or throw an exception. * @param message proto that contains fields 'name' and 'typed_config'. + * @param is_optional an exception will be throw when the value is true and no factory found. + * @return factory the factory requested or nullptr if it does not exist. */ template - static Factory& getAndCheckFactory(const ProtoMessage& message) { + static Factory* getAndCheckFactory(const ProtoMessage& message, bool is_optional) { Factory* factory = Utility::getFactoryByType(message.typed_config()); if (factory != nullptr) { - return *factory; + return factory; } - return Utility::getAndCheckFactoryByName(message.name()); + return Utility::getAndCheckFactoryByName(message.name(), is_optional); + } + + /** + * Get a Factory from the registry with error checking to ensure the name and the factory are + * valid. + * @param message proto that contains fields 'name' and 'typed_config'. + */ + template + static Factory& getAndCheckFactory(const ProtoMessage& message) { + return *getAndCheckFactory(message, false); } /** diff --git a/source/common/event/dispatcher_impl.h b/source/common/event/dispatcher_impl.h index 5afff9de58761..fe16145e7cd2d 100644 --- a/source/common/event/dispatcher_impl.h +++ b/source/common/event/dispatcher_impl.h @@ -92,6 +92,7 @@ class DispatcherImpl : Logger::Loggable, Buffer::WatermarkFactory& getWatermarkFactory() override { return *buffer_factory_; } void pushTrackedObject(const ScopeTrackedObject* object) override; void popTrackedObject(const ScopeTrackedObject* expected_object) override; + bool trackedObjectStackIsEmpty() const override { return tracked_object_stack_.empty(); } MonotonicTime approximateMonotonicTime() const override; void updateApproximateMonotonicTime() override; void shutdown() override; diff --git a/source/common/http/BUILD b/source/common/http/BUILD index 2dfa803c7d94e..bc2b727a1b3fb 100644 --- a/source/common/http/BUILD +++ b/source/common/http/BUILD @@ -161,11 +161,22 @@ envoy_cc_library( envoy_cc_library( name = "alternate_protocols_cache", - srcs = ["alternate_protocols_cache_impl.cc"], - hdrs = ["alternate_protocols_cache_impl.h"], + srcs = [ + "alternate_protocols_cache_impl.cc", + "alternate_protocols_cache_manager_impl.cc", + ], + hdrs = [ + "alternate_protocols_cache_impl.h", + "alternate_protocols_cache_manager_impl.h", + ], deps = [ "//include/envoy/common:time_interface", + "//include/envoy/event:dispatcher_interface", "//include/envoy/http:alternate_protocols_cache_interface", + "//include/envoy/singleton:manager_interface", + "//include/envoy/thread_local:thread_local_interface", + "//include/envoy/upstream:resource_manager_interface", + "@envoy_api//envoy/config/core/v3:pkg_cc_proto", ], ) @@ -213,6 +224,7 @@ envoy_cc_library( "//include/envoy/matcher:matcher_interface", "//source/common/buffer:watermark_buffer_lib", "//source/common/common:linked_object", + "//source/common/common:scope_tracked_object_stack", "//source/common/common:scope_tracker", "//source/common/grpc:common_lib", "//source/common/http/matching:data_impl_lib", diff --git a/source/common/http/alternate_protocols_cache_impl.cc b/source/common/http/alternate_protocols_cache_impl.cc index cab402de191e1..89d31df615aaf 100644 --- a/source/common/http/alternate_protocols_cache_impl.cc +++ b/source/common/http/alternate_protocols_cache_impl.cc @@ -3,15 +3,6 @@ namespace Envoy { namespace Http { -AlternateProtocolsCacheImpl::AlternateProtocol::AlternateProtocol(absl::string_view alpn, - absl::string_view hostname, - uint32_t port) - : alpn_(alpn), hostname_(hostname), port_(port) {} - -AlternateProtocolsCacheImpl::Origin::Origin(absl::string_view scheme, absl::string_view hostname, - uint32_t port) - : scheme_(scheme), hostname_(hostname), port_(port) {} - AlternateProtocolsCacheImpl::AlternateProtocolsCacheImpl(TimeSource& time_source) : time_source_(time_source) {} @@ -29,19 +20,20 @@ void AlternateProtocolsCacheImpl::setAlternatives(const Origin& origin, } } -OptRef> +OptRef> AlternateProtocolsCacheImpl::findAlternatives(const Origin& origin) { auto entry_it = protocols_.find(origin); if (entry_it == protocols_.end()) { - return makeOptRefFromPtr>( + return makeOptRefFromPtr>( nullptr); } const Entry& entry = entry_it->second; if (time_source_.monotonicTime() > entry.expiration_) { // Expire the entry. + // TODO(RyanTheOptimist): expire entries based on a timer. protocols_.erase(entry_it); - return makeOptRefFromPtr>( + return makeOptRefFromPtr>( nullptr); } return makeOptRef(entry.protocols_); diff --git a/source/common/http/alternate_protocols_cache_impl.h b/source/common/http/alternate_protocols_cache_impl.h index 17f67545c3e15..f72ec506dc2d4 100644 --- a/source/common/http/alternate_protocols_cache_impl.h +++ b/source/common/http/alternate_protocols_cache_impl.h @@ -15,6 +15,7 @@ namespace Envoy { namespace Http { // An implementation of AlternateProtocolsCache. +// See: source/docs/http3_upstream.md class AlternateProtocolsCacheImpl : public AlternateProtocolsCache { public: explicit AlternateProtocolsCacheImpl(TimeSource& time_source); diff --git a/source/common/http/alternate_protocols_cache_manager_impl.cc b/source/common/http/alternate_protocols_cache_manager_impl.cc new file mode 100644 index 0000000000000..b70290b86f6c6 --- /dev/null +++ b/source/common/http/alternate_protocols_cache_manager_impl.cc @@ -0,0 +1,46 @@ +#include "common/http/alternate_protocols_cache_manager_impl.h" + +#include "common/http/alternate_protocols_cache_impl.h" +#include "common/protobuf/protobuf.h" + +#include "absl/container/flat_hash_map.h" + +namespace Envoy { +namespace Http { + +SINGLETON_MANAGER_REGISTRATION(alternate_protocols_cache_manager); + +AlternateProtocolsCacheManagerImpl::AlternateProtocolsCacheManagerImpl(TimeSource& time_source, + ThreadLocal::Instance& tls) + : time_source_(time_source), slot_(tls) { + slot_.set([](Event::Dispatcher& /*dispatcher*/) { return std::make_shared(); }); +} + +AlternateProtocolsCacheSharedPtr AlternateProtocolsCacheManagerImpl::getCache( + const envoy::config::core::v3::AlternateProtocolsCacheOptions& options) { + const auto& existing_cache = (*slot_).caches_.find(options.name()); + if (existing_cache != (*slot_).caches_.end()) { + if (!Protobuf::util::MessageDifferencer::Equivalent(options, existing_cache->second.options_)) { + throw EnvoyException(fmt::format( + "options specified alternate protocols cache '{}' with different settings" + " first '{}' second '{}'", + options.name(), existing_cache->second.options_.DebugString(), options.DebugString())); + } + + return existing_cache->second.cache_; + } + + AlternateProtocolsCacheSharedPtr new_cache = + std::make_shared(time_source_); + (*slot_).caches_.emplace(options.name(), CacheWithOptions{options, new_cache}); + return new_cache; +} + +AlternateProtocolsCacheManagerSharedPtr AlternateProtocolsCacheManagerFactoryImpl::get() { + return singleton_manager_.getTyped( + SINGLETON_MANAGER_REGISTERED_NAME(alternate_protocols_cache_manager), + [this] { return std::make_shared(time_source_, tls_); }); +} + +} // namespace Http +} // namespace Envoy diff --git a/source/common/http/alternate_protocols_cache_manager_impl.h b/source/common/http/alternate_protocols_cache_manager_impl.h new file mode 100644 index 0000000000000..59af34103f652 --- /dev/null +++ b/source/common/http/alternate_protocols_cache_manager_impl.h @@ -0,0 +1,61 @@ +#pragma once + +#include "envoy/config/core/v3/protocol.pb.h" +#include "envoy/http/alternate_protocols_cache.h" +#include "envoy/singleton/instance.h" +#include "envoy/singleton/manager.h" +#include "envoy/thread_local/thread_local.h" + +#include "absl/container/flat_hash_map.h" + +namespace Envoy { +namespace Http { + +class AlternateProtocolsCacheManagerImpl : public AlternateProtocolsCacheManager, + public Singleton::Instance { +public: + AlternateProtocolsCacheManagerImpl(TimeSource& time_source, ThreadLocal::Instance& tls); + + // AlternateProtocolsCacheManager + AlternateProtocolsCacheSharedPtr + getCache(const envoy::config::core::v3::AlternateProtocolsCacheOptions& options) override; + +private: + // Contains a cache and the options associated with it. + struct CacheWithOptions { + CacheWithOptions(const envoy::config::core::v3::AlternateProtocolsCacheOptions& options, + AlternateProtocolsCacheSharedPtr cache) + : options_(options), cache_(cache) {} + + const envoy::config::core::v3::AlternateProtocolsCacheOptions& options_; + AlternateProtocolsCacheSharedPtr cache_; + }; + + // Per-thread state. + struct State : public ThreadLocal::ThreadLocalObject { + // Map from config name to cache for that config. + absl::flat_hash_map caches_; + }; + + TimeSource& time_source_; + + // Thread local state for the cache. + ThreadLocal::TypedSlot slot_; +}; + +class AlternateProtocolsCacheManagerFactoryImpl : public AlternateProtocolsCacheManagerFactory { +public: + AlternateProtocolsCacheManagerFactoryImpl(Singleton::Manager& singleton_manager, + TimeSource& time_source, ThreadLocal::Instance& tls) + : singleton_manager_(singleton_manager), time_source_(time_source), tls_(tls) {} + + AlternateProtocolsCacheManagerSharedPtr get() override; + +private: + Singleton::Manager& singleton_manager_; + TimeSource& time_source_; + ThreadLocal::Instance& tls_; +}; + +} // namespace Http +} // namespace Envoy diff --git a/source/common/http/async_client_impl.h b/source/common/http/async_client_impl.h index 5aea06d97056b..798907b0763be 100644 --- a/source/common/http/async_client_impl.h +++ b/source/common/http/async_client_impl.h @@ -428,6 +428,9 @@ class AsyncStreamImpl : public AsyncClient::Stream, uint32_t decoderBufferLimit() override { return 0; } bool recreateStream(const ResponseHeaderMap*) override { return false; } const ScopeTrackedObject& scope() override { return *this; } + void restoreContextOnContinue(ScopeTrackedObjectStack& tracked_object_stack) override { + tracked_object_stack.add(*this); + } void addUpstreamSocketOptions(const Network::Socket::OptionsSharedPtr&) override {} Network::Socket::OptionsSharedPtr getUpstreamSocketOptions() const override { return {}; } diff --git a/source/common/http/conn_manager_impl.cc b/source/common/http/conn_manager_impl.cc index 285a4f3627738..87fd8073de1fb 100644 --- a/source/common/http/conn_manager_impl.cc +++ b/source/common/http/conn_manager_impl.cc @@ -730,26 +730,20 @@ void ConnectionManagerImpl::ActiveStream::onIdleTimeout() { } else { // TODO(mattklein) this may result in multiple flags. This Ok? filter_manager_.streamInfo().setResponseFlag(StreamInfo::ResponseFlag::StreamIdleTimeout); - sendLocalReply(request_headers_ != nullptr && - Grpc::Common::isGrpcRequestHeaders(*request_headers_), - Http::Code::RequestTimeout, "stream timeout", nullptr, absl::nullopt, + sendLocalReply(Http::Code::RequestTimeout, "stream timeout", nullptr, absl::nullopt, StreamInfo::ResponseCodeDetails::get().StreamIdleTimeout); } } void ConnectionManagerImpl::ActiveStream::onRequestTimeout() { connection_manager_.stats_.named_.downstream_rq_timeout_.inc(); - sendLocalReply(request_headers_ != nullptr && - Grpc::Common::isGrpcRequestHeaders(*request_headers_), - Http::Code::RequestTimeout, "request timeout", nullptr, absl::nullopt, + sendLocalReply(Http::Code::RequestTimeout, "request timeout", nullptr, absl::nullopt, StreamInfo::ResponseCodeDetails::get().RequestOverallTimeout); } void ConnectionManagerImpl::ActiveStream::onRequestHeaderTimeout() { connection_manager_.stats_.named_.downstream_rq_header_timeout_.inc(); - sendLocalReply(request_headers_ != nullptr && - Grpc::Common::isGrpcRequestHeaders(*request_headers_), - Http::Code::RequestTimeout, "request header timeout", nullptr, absl::nullopt, + sendLocalReply(Http::Code::RequestTimeout, "request header timeout", nullptr, absl::nullopt, StreamInfo::ResponseCodeDetails::get().RequestHeaderTimeout); } @@ -757,9 +751,7 @@ void ConnectionManagerImpl::ActiveStream::onStreamMaxDurationReached() { ENVOY_STREAM_LOG(debug, "Stream max duration time reached", *this); connection_manager_.stats_.named_.downstream_rq_max_duration_reached_.inc(); if (Runtime::runtimeFeatureEnabled("envoy.reloadable_features.allow_response_for_timeout")) { - sendLocalReply(request_headers_ != nullptr && - Grpc::Common::isGrpcRequestHeaders(*request_headers_), - Http::Code::RequestTimeout, "downstream duration timeout", nullptr, + sendLocalReply(Http::Code::RequestTimeout, "downstream duration timeout", nullptr, Grpc::Status::WellKnownGrpcStatus::DeadlineExceeded, StreamInfo::ResponseCodeDetails::get().MaxDurationTimeout); } else { @@ -893,8 +885,7 @@ void ConnectionManagerImpl::ActiveStream::decodeHeaders(RequestHeaderMapPtr&& he // overload it is more important to avoid unnecessary allocation than to create the filters. filter_manager_.skipFilterChainCreation(); connection_manager_.stats_.named_.downstream_rq_overload_close_.inc(); - sendLocalReply(Grpc::Common::isGrpcRequestHeaders(*request_headers_), - Http::Code::ServiceUnavailable, "envoy overloaded", nullptr, absl::nullopt, + sendLocalReply(Http::Code::ServiceUnavailable, "envoy overloaded", nullptr, absl::nullopt, StreamInfo::ResponseCodeDetails::get().Overload); return; } @@ -923,7 +914,7 @@ void ConnectionManagerImpl::ActiveStream::decodeHeaders(RequestHeaderMapPtr&& he filter_manager_.streamInfo().protocol(protocol); if (!connection_manager_.config_.http1Settings().accept_http_10_) { // Send "Upgrade Required" if HTTP/1.0 support is not explicitly configured on. - sendLocalReply(false, Code::UpgradeRequired, "", nullptr, absl::nullopt, + sendLocalReply(Code::UpgradeRequired, "", nullptr, absl::nullopt, StreamInfo::ResponseCodeDetails::get().LowVersion); return; } @@ -937,8 +928,8 @@ void ConnectionManagerImpl::ActiveStream::decodeHeaders(RequestHeaderMapPtr&& he if (!request_headers_->Host()) { // Require host header. For HTTP/1.1 Host has already been translated to :authority. - sendLocalReply(Grpc::Common::hasGrpcContentType(*request_headers_), Code::BadRequest, "", - nullptr, absl::nullopt, StreamInfo::ResponseCodeDetails::get().MissingHost); + sendLocalReply(Code::BadRequest, "", nullptr, absl::nullopt, + StreamInfo::ResponseCodeDetails::get().MissingHost); return; } @@ -951,16 +942,16 @@ void ConnectionManagerImpl::ActiveStream::decodeHeaders(RequestHeaderMapPtr&& he // is enabled on the HCM. if ((!HeaderUtility::isConnect(*request_headers_) || request_headers_->Path()) && request_headers_->getPathValue().empty()) { - sendLocalReply(Grpc::Common::hasGrpcContentType(*request_headers_), Code::NotFound, "", nullptr, - absl::nullopt, StreamInfo::ResponseCodeDetails::get().MissingPath); + sendLocalReply(Code::NotFound, "", nullptr, absl::nullopt, + StreamInfo::ResponseCodeDetails::get().MissingPath); return; } // Currently we only support relative paths at the application layer. if (!request_headers_->getPathValue().empty() && request_headers_->getPathValue()[0] != '/') { connection_manager_.stats_.named_.downstream_rq_non_relative_path_.inc(); - sendLocalReply(Grpc::Common::hasGrpcContentType(*request_headers_), Code::NotFound, "", nullptr, - absl::nullopt, StreamInfo::ResponseCodeDetails::get().AbsolutePath); + sendLocalReply(Code::NotFound, "", nullptr, absl::nullopt, + StreamInfo::ResponseCodeDetails::get().AbsolutePath); return; } @@ -973,14 +964,13 @@ void ConnectionManagerImpl::ActiveStream::decodeHeaders(RequestHeaderMapPtr&& he (action == ConnectionManagerUtility::NormalizePathAction::Redirect && Grpc::Common::hasGrpcContentType(*request_headers_))) { connection_manager_.stats_.named_.downstream_rq_failed_path_normalization_.inc(); - sendLocalReply(Grpc::Common::hasGrpcContentType(*request_headers_), Code::BadRequest, "", - nullptr, absl::nullopt, + sendLocalReply(Code::BadRequest, "", nullptr, absl::nullopt, StreamInfo::ResponseCodeDetails::get().PathNormalizationFailed); return; } else if (action == ConnectionManagerUtility::NormalizePathAction::Redirect) { connection_manager_.stats_.named_.downstream_rq_redirected_with_normalized_path_.inc(); sendLocalReply( - false, Code::TemporaryRedirect, "", + Code::TemporaryRedirect, "", [new_path = request_headers_->Path()->value().getStringView()]( Http::ResponseHeaderMap& response_headers) -> void { response_headers.addReferenceKey(Http::Headers::get().Location, new_path); @@ -1010,8 +1000,7 @@ void ConnectionManagerImpl::ActiveStream::decodeHeaders(RequestHeaderMapPtr&& he if (mutate_result.reject_request.has_value()) { const auto& reject_request_params = mutate_result.reject_request.value(); connection_manager_.stats_.named_.downstream_rq_rejected_via_ip_detection_.inc(); - sendLocalReply(Grpc::Common::isGrpcRequestHeaders(*request_headers_), - reject_request_params.response_code, reject_request_params.body, nullptr, + sendLocalReply(reject_request_params.response_code, reject_request_params.body, nullptr, absl::nullopt, StreamInfo::ResponseCodeDetails::get().OriginalIPDetectionFailed); return; @@ -1046,8 +1035,8 @@ void ConnectionManagerImpl::ActiveStream::decodeHeaders(RequestHeaderMapPtr&& he // contains a smuggled HTTP request. state_.saw_connection_close_ = true; connection_manager_.stats_.named_.downstream_rq_ws_on_non_ws_route_.inc(); - sendLocalReply(Grpc::Common::hasGrpcContentType(*request_headers_), Code::Forbidden, "", - nullptr, absl::nullopt, StreamInfo::ResponseCodeDetails::get().UpgradeFailed); + sendLocalReply(Code::Forbidden, "", nullptr, absl::nullopt, + StreamInfo::ResponseCodeDetails::get().UpgradeFailed); return; } // Allow non websocket requests to go through websocket enabled routes. diff --git a/source/common/http/conn_manager_impl.h b/source/common/http/conn_manager_impl.h index 033947b6f729d..ce8c6648a84e7 100644 --- a/source/common/http/conn_manager_impl.h +++ b/source/common/http/conn_manager_impl.h @@ -187,12 +187,11 @@ class ConnectionManagerImpl : Logger::Loggable, const StreamInfo::StreamInfo& streamInfo() const override { return filter_manager_.streamInfo(); } - void sendLocalReply(bool is_grpc_request, Code code, absl::string_view body, + void sendLocalReply(Code code, absl::string_view body, const std::function& modify_headers, const absl::optional grpc_status, absl::string_view details) override { - return filter_manager_.sendLocalReply(is_grpc_request, code, body, modify_headers, - grpc_status, details); + return filter_manager_.sendLocalReply(code, body, modify_headers, grpc_status, details); } // Tracing::TracingConfig diff --git a/source/common/http/conn_pool_grid.cc b/source/common/http/conn_pool_grid.cc index e8e5f53e72b77..12d143717a9d6 100644 --- a/source/common/http/conn_pool_grid.cc +++ b/source/common/http/conn_pool_grid.cc @@ -192,7 +192,7 @@ ConnectivityGrid::ConnectivityGrid( const Network::ConnectionSocket::OptionsSharedPtr& options, const Network::TransportSocketOptionsSharedPtr& transport_socket_options, Upstream::ClusterConnectivityState& state, TimeSource& time_source, - OptRef alternate_protocols, + AlternateProtocolsCacheSharedPtr alternate_protocols, std::chrono::milliseconds next_attempt_duration, ConnectivityOptions connectivity_options) : dispatcher_(dispatcher), random_generator_(random_generator), host_(host), priority_(priority), options_(options), transport_socket_options_(transport_socket_options), @@ -339,7 +339,7 @@ bool ConnectivityGrid::shouldAttemptHttp3() { ENVOY_LOG(trace, "HTTP/3 is broken to host '{}', skipping.", host_->hostname()); return false; } - if (!alternate_protocols_.has_value()) { + if (!alternate_protocols_) { ENVOY_LOG(trace, "No alternate protocols cache. Attempting HTTP/3 to host '{}'.", host_->hostname()); return true; @@ -351,8 +351,8 @@ bool ConnectivityGrid::shouldAttemptHttp3() { } uint32_t port = host_->address()->ip()->port(); // TODO(RyanTheOptimist): Figure out how scheme gets plumbed in here. - AlternateProtocolsCacheImpl::Origin origin("https", host_->hostname(), port); - OptRef> protocols = + AlternateProtocolsCache::Origin origin("https", host_->hostname(), port); + OptRef> protocols = alternate_protocols_->findAlternatives(origin); if (!protocols.has_value()) { ENVOY_LOG(trace, "No alternate protocols available for host '{}', skipping HTTP/3.", @@ -360,7 +360,7 @@ bool ConnectivityGrid::shouldAttemptHttp3() { return false; } - for (const AlternateProtocolsCacheImpl::AlternateProtocol& protocol : protocols.ref()) { + for (const AlternateProtocolsCache::AlternateProtocol& protocol : protocols.ref()) { // TODO(RyanTheOptimist): Handle alternate protocols which change hostname or port. if (!protocol.hostname_.empty() || protocol.port_ != port) { ENVOY_LOG(trace, diff --git a/source/common/http/conn_pool_grid.h b/source/common/http/conn_pool_grid.h index d9391d8087187..e05a2ae68dc8f 100644 --- a/source/common/http/conn_pool_grid.h +++ b/source/common/http/conn_pool_grid.h @@ -132,7 +132,7 @@ class ConnectivityGrid : public ConnectionPool::Instance, const Network::ConnectionSocket::OptionsSharedPtr& options, const Network::TransportSocketOptionsSharedPtr& transport_socket_options, Upstream::ClusterConnectivityState& state, TimeSource& time_source, - OptRef alternate_protocols, + AlternateProtocolsCacheSharedPtr alternate_protocols, std::chrono::milliseconds next_attempt_duration, ConnectivityOptions connectivity_options); ~ConnectivityGrid() override; @@ -192,7 +192,7 @@ class ConnectivityGrid : public ConnectionPool::Instance, TimeSource& time_source_; Http3StatusTracker http3_status_tracker_; // TODO(RyanTheOptimist): Make the alternate_protocols_ member non-optional. - OptRef alternate_protocols_; + AlternateProtocolsCacheSharedPtr alternate_protocols_; // Tracks how many drains are needed before calling drain callbacks. This is // set to the number of pools when the first drain callbacks are added, and diff --git a/source/common/http/filter_manager.cc b/source/common/http/filter_manager.cc index b03a9694986eb..592463ea2aeeb 100644 --- a/source/common/http/filter_manager.cc +++ b/source/common/http/filter_manager.cc @@ -1,9 +1,12 @@ #include "common/http/filter_manager.h" +#include + #include "envoy/http/header_map.h" #include "envoy/matcher/matcher.h" #include "common/common/enum_to_int.h" +#include "common/common/scope_tracked_object_stack.h" #include "common/common/scope_tracker.h" #include "common/http/codes.h" #include "common/http/header_map_impl.h" @@ -53,6 +56,14 @@ void ActiveStreamFilterBase::commonContinue() { return; } + // Set ScopeTrackerScopeState if there's no existing crash context. + ScopeTrackedObjectStack encapsulated_object; + absl::optional state; + if (parent_.dispatcher_.trackedObjectStackIsEmpty()) { + restoreContextOnContinue(encapsulated_object); + state.emplace(&encapsulated_object, parent_.dispatcher_); + } + ENVOY_STREAM_LOG(trace, "continuing filter chain: filter={}", *this, static_cast(this)); ASSERT(!canIterate(), @@ -230,6 +241,11 @@ const ScopeTrackedObject& ActiveStreamFilterBase::scope() { return parent_.filter_manager_callbacks_.scope(); } +void ActiveStreamFilterBase::restoreContextOnContinue( + ScopeTrackedObjectStack& tracked_object_stack) { + parent_.contextOnContinue(tracked_object_stack); +} + Tracing::Config& ActiveStreamFilterBase::tracingConfig() { return parent_.filter_manager_callbacks_.tracingConfig(); } @@ -366,7 +382,7 @@ void ActiveStreamDecoderFilter::sendLocalReply( Code code, absl::string_view body, std::function modify_headers, const absl::optional grpc_status, absl::string_view details) { - parent_.sendLocalReply(is_grpc_request_, code, body, modify_headers, grpc_status, details); + parent_.sendLocalReply(code, body, modify_headers, grpc_status, details); } void ActiveStreamDecoderFilter::encode100ContinueHeaders(ResponseHeaderMapPtr&& headers) { @@ -831,9 +847,8 @@ void FilterManager::onLocalReply(StreamFilterBase::LocalReplyData& data) { state_.under_on_local_reply_ = false; } -// TODO(alyssawilk) update sendLocalReply interface. void FilterManager::sendLocalReply( - bool, Code code, absl::string_view body, + Code code, absl::string_view body, const std::function& modify_headers, const absl::optional grpc_status, absl::string_view details) { ASSERT(!state_.under_on_local_reply_); @@ -1301,6 +1316,11 @@ void FilterManager::setBufferLimit(uint32_t new_limit) { } } +void FilterManager::contextOnContinue(ScopeTrackedObjectStack& tracked_object_stack) { + tracked_object_stack.add(connection_); + tracked_object_stack.add(filter_manager_callbacks_.scope()); +} + bool FilterManager::createFilterChain() { if (state_.created_filter_chain_) { return false; @@ -1522,8 +1542,7 @@ void ActiveStreamEncoderFilter::sendLocalReply( Code code, absl::string_view body, std::function modify_headers, const absl::optional grpc_status, absl::string_view details) { - parent_.sendLocalReply(parent_.state_.is_grpc_request_, code, body, modify_headers, grpc_status, - details); + parent_.sendLocalReply(code, body, modify_headers, grpc_status, details); } Http1StreamEncoderOptionsOptRef ActiveStreamEncoderFilter::http1StreamEncoderOptions() { @@ -1541,8 +1560,6 @@ void ActiveStreamEncoderFilter::responseDataTooLarge() { // In this case, sendLocalReply will either send a response directly to the encoder, or // reset the stream. parent_.sendLocalReply( - parent_.filter_manager_callbacks_.requestHeaders() && - Grpc::Common::isGrpcRequestHeaders(*parent_.filter_manager_callbacks_.requestHeaders()), Http::Code::InternalServerError, CodeUtility::toString(Http::Code::InternalServerError), nullptr, absl::nullopt, StreamInfo::ResponseCodeDetails::get().ResponsePayloadTooLarge); } diff --git a/source/common/http/filter_manager.h b/source/common/http/filter_manager.h index 322e4026adb42..c861d08a228fc 100644 --- a/source/common/http/filter_manager.h +++ b/source/common/http/filter_manager.h @@ -1,5 +1,6 @@ #pragma once +#include #include #include "envoy/common/optref.h" @@ -141,6 +142,7 @@ struct ActiveStreamFilterBase : public virtual StreamFilterCallbacks, Tracing::Span& activeSpan() override; Tracing::Config& tracingConfig() override; const ScopeTrackedObject& scope() override; + void restoreContextOnContinue(ScopeTrackedObjectStack& tracked_object_stack) override; // Functions to set or get iteration state. bool canIterate() { return iteration_state_ == IterationState::Continue; } @@ -829,7 +831,7 @@ class FilterManager : public ScopeTrackedObject, */ void onLocalReply(StreamFilterBase::LocalReplyData& data); - void sendLocalReply(bool is_grpc_request, Code code, absl::string_view body, + void sendLocalReply(Code code, absl::string_view body, const std::function& modify_headers, const absl::optional grpc_status, absl::string_view details); @@ -918,6 +920,8 @@ class FilterManager : public ScopeTrackedObject, return filter_manager_callbacks_.enableInternalRedirectsWithBody(); } + void contextOnContinue(ScopeTrackedObjectStack& tracked_object_stack); + private: // Indicates which filter to start the iteration with. enum class FilterIterationStartState { AlwaysStartFromNext, CanStartFromCurrent }; diff --git a/source/common/http/http1/codec_impl.cc b/source/common/http/http1/codec_impl.cc index ae4dc2a8a39a9..23859ac380a2c 100644 --- a/source/common/http/http1/codec_impl.cc +++ b/source/common/http/http1/codec_impl.cc @@ -1176,17 +1176,8 @@ Status ServerConnectionImpl::sendProtocolError(absl::string_view details) { active_request_.value().response_encoder_.setDetails(details); if (!active_request_.value().response_encoder_.startedResponse()) { - // Note that the correctness of is_grpc_request and is_head_request is best-effort. - // If headers have not been fully parsed they may not be inferred correctly. - bool is_grpc_request = false; - if (absl::holds_alternative(headers_or_trailers_) && - absl::get(headers_or_trailers_) != nullptr) { - is_grpc_request = - Grpc::Common::isGrpcRequestHeaders(*absl::get(headers_or_trailers_)); - } - active_request_->request_decoder_->sendLocalReply(is_grpc_request, error_code_, - CodeUtility::toString(error_code_), nullptr, - absl::nullopt, details); + active_request_->request_decoder_->sendLocalReply( + error_code_, CodeUtility::toString(error_code_), nullptr, absl::nullopt, details); } return okStatus(); } diff --git a/source/common/network/connection_impl.h b/source/common/network/connection_impl.h index bc80f1f92a0fa..17021980febbf 100644 --- a/source/common/network/connection_impl.h +++ b/source/common/network/connection_impl.h @@ -45,9 +45,7 @@ class ConnectionImplUtility { * Implementation of Network::Connection, Network::FilterManagerConnection and * Envoy::ScopeTrackedObject. */ -class ConnectionImpl : public ConnectionImplBase, - public TransportSocketCallbacks, - public ScopeTrackedObject { +class ConnectionImpl : public ConnectionImplBase, public TransportSocketCallbacks { public: ConnectionImpl(Event::Dispatcher& dispatcher, ConnectionSocketPtr&& socket, TransportSocketPtr&& transport_socket, StreamInfo::StreamInfo& stream_info, diff --git a/source/common/quic/envoy_quic_client_session.h b/source/common/quic/envoy_quic_client_session.h index 638d99ffcb809..8aecb34b99382 100644 --- a/source/common/quic/envoy_quic_client_session.h +++ b/source/common/quic/envoy_quic_client_session.h @@ -46,6 +46,9 @@ class EnvoyQuicClientSession : public QuicFilterManagerConnectionImpl, // Network::Connection absl::string_view requestedServerName() const override; + void dumpState(std::ostream&, int) const override { + // TODO(kbaichoo): Implement dumpState for H3. + } // Network::ClientConnection // Set up socket and start handshake. diff --git a/source/common/quic/envoy_quic_server_session.h b/source/common/quic/envoy_quic_server_session.h index 1e4900e5632af..b186cc3b2b53a 100644 --- a/source/common/quic/envoy_quic_server_session.h +++ b/source/common/quic/envoy_quic_server_session.h @@ -1,5 +1,7 @@ #pragma once +#include + #if defined(__GNUC__) #pragma GCC diagnostic push #pragma GCC diagnostic ignored "-Wunused-parameter" @@ -45,6 +47,9 @@ class EnvoyQuicServerSession : public quic::QuicServerSessionBase, // Network::Connection absl::string_view requestedServerName() const override; + void dumpState(std::ostream&, int) const override { + // TODO(kbaichoo): Implement dumpState for H3. + } // Called by QuicHttpServerConnectionImpl before creating data streams. void setHttpConnectionCallbacks(Http::ServerConnectionCallbacks& callbacks) { diff --git a/source/common/router/config_impl.cc b/source/common/router/config_impl.cc index 4994bc5d070e7..8c5ba9b1c67ab 100644 --- a/source/common/router/config_impl.cc +++ b/source/common/router/config_impl.cc @@ -309,6 +309,7 @@ const Tracing::CustomTagMap& RouteTracingImpl::getCustomTags() const { return cu RouteEntryImplBase::RouteEntryImplBase(const VirtualHostImpl& vhost, const envoy::config::route::v3::Route& route, + const OptionalHttpFilters& optional_http_filters, Server::Configuration::ServerFactoryContext& factory_context, ProtobufMessage::ValidationVisitor& validator) : case_sensitive_(PROTOBUF_GET_WRAPPED_OR_DEFAULT(route.match(), case_sensitive, true)), @@ -375,8 +376,8 @@ RouteEntryImplBase::RouteEntryImplBase(const VirtualHostImpl& vhost, route, factory_context.api(), vhost_.globalRouteConfig().maxDirectResponseBodySizeBytes())), per_filter_configs_(route.typed_per_filter_config(), - route.hidden_envoy_deprecated_per_filter_config(), factory_context, - validator), + route.hidden_envoy_deprecated_per_filter_config(), optional_http_filters, + factory_context, validator), route_name_(route.name()), time_source_(factory_context.dispatcher().timeSource()) { if (route.route().has_metadata_match()) { const auto filter_it = route.route().metadata_match().filter_metadata().find( @@ -418,7 +419,8 @@ RouteEntryImplBase::RouteEntryImplBase(const VirtualHostImpl& vhost, for (const auto& cluster : route.route().weighted_clusters().clusters()) { auto cluster_entry = std::make_unique( - this, runtime_key_prefix + "." + cluster.name(), factory_context, validator, cluster); + this, runtime_key_prefix + "." + cluster.name(), factory_context, validator, cluster, + optional_http_filters); weighted_clusters_.emplace_back(std::move(cluster_entry)); total_weight += weighted_clusters_.back()->clusterWeight(); } @@ -1017,7 +1019,8 @@ RouteEntryImplBase::WeightedClusterEntry::WeightedClusterEntry( const RouteEntryImplBase* parent, const std::string& runtime_key, Server::Configuration::ServerFactoryContext& factory_context, ProtobufMessage::ValidationVisitor& validator, - const envoy::config::route::v3::WeightedCluster::ClusterWeight& cluster) + const envoy::config::route::v3::WeightedCluster::ClusterWeight& cluster, + const OptionalHttpFilters& optional_http_filters) : DynamicRouteEntry(parent, cluster.name()), runtime_key_(runtime_key), loader_(factory_context.runtime()), cluster_weight_(PROTOBUF_GET_WRAPPED_REQUIRED(cluster, weight)), @@ -1026,8 +1029,8 @@ RouteEntryImplBase::WeightedClusterEntry::WeightedClusterEntry( response_headers_parser_(HeaderParser::configure(cluster.response_headers_to_add(), cluster.response_headers_to_remove())), per_filter_configs_(cluster.typed_per_filter_config(), - cluster.hidden_envoy_deprecated_per_filter_config(), factory_context, - validator) { + cluster.hidden_envoy_deprecated_per_filter_config(), + optional_http_filters, factory_context, validator) { if (cluster.has_metadata_match()) { const auto filter_it = cluster.metadata_match().filter_metadata().find( Envoy::Config::MetadataFilters::get().ENVOY_LB); @@ -1058,9 +1061,11 @@ RouteEntryImplBase::WeightedClusterEntry::perFilterConfig(const std::string& nam PrefixRouteEntryImpl::PrefixRouteEntryImpl( const VirtualHostImpl& vhost, const envoy::config::route::v3::Route& route, + const OptionalHttpFilters& optional_http_filters, Server::Configuration::ServerFactoryContext& factory_context, ProtobufMessage::ValidationVisitor& validator) - : RouteEntryImplBase(vhost, route, factory_context, validator), prefix_(route.match().prefix()), + : RouteEntryImplBase(vhost, route, optional_http_filters, factory_context, validator), + prefix_(route.match().prefix()), path_matcher_(Matchers::PathMatcher::createPrefix(prefix_, !case_sensitive_)) {} void PrefixRouteEntryImpl::rewritePathHeader(Http::RequestHeaderMap& headers, @@ -1085,9 +1090,11 @@ RouteConstSharedPtr PrefixRouteEntryImpl::matches(const Http::RequestHeaderMap& PathRouteEntryImpl::PathRouteEntryImpl(const VirtualHostImpl& vhost, const envoy::config::route::v3::Route& route, + const OptionalHttpFilters& optional_http_filters, Server::Configuration::ServerFactoryContext& factory_context, ProtobufMessage::ValidationVisitor& validator) - : RouteEntryImplBase(vhost, route, factory_context, validator), path_(route.match().path()), + : RouteEntryImplBase(vhost, route, optional_http_filters, factory_context, validator), + path_(route.match().path()), path_matcher_(Matchers::PathMatcher::createExact(path_, !case_sensitive_)) {} void PathRouteEntryImpl::rewritePathHeader(Http::RequestHeaderMap& headers, @@ -1113,9 +1120,10 @@ RouteConstSharedPtr PathRouteEntryImpl::matches(const Http::RequestHeaderMap& he RegexRouteEntryImpl::RegexRouteEntryImpl( const VirtualHostImpl& vhost, const envoy::config::route::v3::Route& route, + const OptionalHttpFilters& optional_http_filters, Server::Configuration::ServerFactoryContext& factory_context, ProtobufMessage::ValidationVisitor& validator) - : RouteEntryImplBase(vhost, route, factory_context, validator) { + : RouteEntryImplBase(vhost, route, optional_http_filters, factory_context, validator) { // TODO(yangminzhu): Use PathMatcher once hidden_envoy_deprecated_regex is removed. if (route.match().path_specifier_case() == envoy::config::route::v3::RouteMatch::PathSpecifierCase::kHiddenEnvoyDeprecatedRegex) { @@ -1159,9 +1167,10 @@ RouteConstSharedPtr RegexRouteEntryImpl::matches(const Http::RequestHeaderMap& h ConnectRouteEntryImpl::ConnectRouteEntryImpl( const VirtualHostImpl& vhost, const envoy::config::route::v3::Route& route, + const OptionalHttpFilters& optional_http_filters, Server::Configuration::ServerFactoryContext& factory_context, ProtobufMessage::ValidationVisitor& validator) - : RouteEntryImplBase(vhost, route, factory_context, validator) {} + : RouteEntryImplBase(vhost, route, optional_http_filters, factory_context, validator) {} void ConnectRouteEntryImpl::rewritePathHeader(Http::RequestHeaderMap& headers, bool insert_envoy_original_path) const { @@ -1187,7 +1196,7 @@ RouteConstSharedPtr ConnectRouteEntryImpl::matches(const Http::RequestHeaderMap& VirtualHostImpl::VirtualHostImpl( const envoy::config::route::v3::VirtualHost& virtual_host, - const ConfigImpl& global_route_config, + const OptionalHttpFilters& optional_http_filters, const ConfigImpl& global_route_config, Server::Configuration::ServerFactoryContext& factory_context, Stats::Scope& scope, ProtobufMessage::ValidationVisitor& validator, const absl::optional& validation_clusters) @@ -1202,8 +1211,8 @@ VirtualHostImpl::VirtualHostImpl( response_headers_parser_(HeaderParser::configure(virtual_host.response_headers_to_add(), virtual_host.response_headers_to_remove())), per_filter_configs_(virtual_host.typed_per_filter_config(), - virtual_host.hidden_envoy_deprecated_per_filter_config(), factory_context, - validator), + virtual_host.hidden_envoy_deprecated_per_filter_config(), + optional_http_filters, factory_context, validator), retry_shadow_buffer_limit_(PROTOBUF_GET_WRAPPED_OR_DEFAULT( virtual_host, per_request_buffer_limit_bytes, std::numeric_limits::max())), include_attempt_count_in_request_(virtual_host.include_request_attempt_count()), @@ -1236,20 +1245,24 @@ VirtualHostImpl::VirtualHostImpl( for (const auto& route : virtual_host.routes()) { switch (route.match().path_specifier_case()) { case envoy::config::route::v3::RouteMatch::PathSpecifierCase::kPrefix: { - routes_.emplace_back(new PrefixRouteEntryImpl(*this, route, factory_context, validator)); + routes_.emplace_back(new PrefixRouteEntryImpl(*this, route, optional_http_filters, + factory_context, validator)); break; } case envoy::config::route::v3::RouteMatch::PathSpecifierCase::kPath: { - routes_.emplace_back(new PathRouteEntryImpl(*this, route, factory_context, validator)); + routes_.emplace_back( + new PathRouteEntryImpl(*this, route, optional_http_filters, factory_context, validator)); break; } case envoy::config::route::v3::RouteMatch::PathSpecifierCase::kHiddenEnvoyDeprecatedRegex: case envoy::config::route::v3::RouteMatch::PathSpecifierCase::kSafeRegex: { - routes_.emplace_back(new RegexRouteEntryImpl(*this, route, factory_context, validator)); + routes_.emplace_back( + new RegexRouteEntryImpl(*this, route, optional_http_filters, factory_context, validator)); break; } case envoy::config::route::v3::RouteMatch::PathSpecifierCase::kConnectMatcher: { - routes_.emplace_back(new ConnectRouteEntryImpl(*this, route, factory_context, validator)); + routes_.emplace_back(new ConnectRouteEntryImpl(*this, route, optional_http_filters, + factory_context, validator)); break; } default: @@ -1340,6 +1353,7 @@ const VirtualHostImpl* RouteMatcher::findWildcardVirtualHost( } RouteMatcher::RouteMatcher(const envoy::config::route::v3::RouteConfiguration& route_config, + const OptionalHttpFilters& optional_http_filters, const ConfigImpl& global_route_config, Server::Configuration::ServerFactoryContext& factory_context, ProtobufMessage::ValidationVisitor& validator, bool validate_clusters) @@ -1350,9 +1364,9 @@ RouteMatcher::RouteMatcher(const envoy::config::route::v3::RouteConfiguration& r validation_clusters = factory_context.clusterManager().clusters(); } for (const auto& virtual_host_config : route_config.virtual_hosts()) { - VirtualHostSharedPtr virtual_host(new VirtualHostImpl(virtual_host_config, global_route_config, - factory_context, *vhost_scope_, validator, - validation_clusters)); + VirtualHostSharedPtr virtual_host( + new VirtualHostImpl(virtual_host_config, optional_http_filters, global_route_config, + factory_context, *vhost_scope_, validator, validation_clusters)); for (const std::string& domain_name : virtual_host_config.domains()) { const std::string domain = Http::LowerCaseString(domain_name).get(); bool duplicate_found = false; @@ -1506,6 +1520,7 @@ VirtualHostImpl::virtualClusterFromEntries(const Http::HeaderMap& headers) const } ConfigImpl::ConfigImpl(const envoy::config::route::v3::RouteConfiguration& config, + const OptionalHttpFilters& optional_http_filters, Server::Configuration::ServerFactoryContext& factory_context, ProtobufMessage::ValidationVisitor& validator, bool validate_clusters_default) @@ -1516,7 +1531,7 @@ ConfigImpl::ConfigImpl(const envoy::config::route::v3::RouteConfiguration& confi PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, max_direct_response_body_size_bytes, DEFAULT_MAX_DIRECT_RESPONSE_BODY_SIZE_BYTES)) { route_matcher_ = std::make_unique( - config, *this, factory_context, validator, + config, optional_http_filters, *this, factory_context, validator, PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, validate_clusters, validate_clusters_default)); for (const std::string& header : config.internal_only_headers()) { @@ -1536,25 +1551,43 @@ RouteConstSharedPtr ConfigImpl::route(const RouteCallback& cb, return route_matcher_->route(cb, headers, stream_info, random_value); } -namespace { +RouteSpecificFilterConfigConstSharedPtr PerFilterConfigs::createRouteSpecificFilterConfig( + const std::string& name, const ProtobufWkt::Any& typed_config, + const ProtobufWkt::Struct& config, const OptionalHttpFilters& optional_http_filters, + Server::Configuration::ServerFactoryContext& factory_context, + ProtobufMessage::ValidationVisitor& validator) { + bool is_optional = (optional_http_filters.find(name) != optional_http_filters.end()); + auto factory = Envoy::Config::Utility::getAndCheckFactoryByName< + Server::Configuration::NamedHttpFilterConfigFactory>(name, is_optional); + if (factory == nullptr) { + ENVOY_LOG(warn, "Can't find a registered implementation for http filter '{}'", name); + return nullptr; + } -RouteSpecificFilterConfigConstSharedPtr -createRouteSpecificFilterConfig(const std::string& name, const ProtobufWkt::Any& typed_config, - const ProtobufWkt::Struct& config, - Server::Configuration::ServerFactoryContext& factory_context, - ProtobufMessage::ValidationVisitor& validator) { - auto& factory = Envoy::Config::Utility::getAndCheckFactoryByName< - Server::Configuration::NamedHttpFilterConfigFactory>(name); - ProtobufTypes::MessagePtr proto_config = factory.createEmptyRouteConfigProto(); + ProtobufTypes::MessagePtr proto_config = factory->createEmptyRouteConfigProto(); Envoy::Config::Utility::translateOpaqueConfig(typed_config, config, validator, *proto_config); - return factory.createRouteSpecificFilterConfig(*proto_config, factory_context, validator); + auto object = factory->createRouteSpecificFilterConfig(*proto_config, factory_context, validator); + if (object == nullptr) { + if (Runtime::runtimeFeatureEnabled( + "envoy.reloadable_features.check_unsupported_typed_per_filter_config") && + !is_optional) { + throw EnvoyException( + fmt::format("The filter {} doesn't support virtual host-specific configurations", name)); + } else { + ENVOY_LOG(warn, + "The filter {} doesn't support virtual host-specific configurations. Set runtime " + "config `envoy.reloadable_features.check_unsupported_typed_per_filter_config` as " + "true to reject any invalid virtual-host specific configuration.", + name); + } + } + return object; } -} // namespace - PerFilterConfigs::PerFilterConfigs( const Protobuf::Map& typed_configs, const Protobuf::Map& configs, + const OptionalHttpFilters& optional_http_filters, Server::Configuration::ServerFactoryContext& factory_context, ProtobufMessage::ValidationVisitor& validator) { if (!typed_configs.empty() && !configs.empty()) { @@ -1566,22 +1599,11 @@ PerFilterConfigs::PerFilterConfigs( const auto& name = Extensions::HttpFilters::Common::FilterNameUtil::canonicalFilterName(it.first); - auto object = createRouteSpecificFilterConfig( - name, it.second, ProtobufWkt::Struct::default_instance(), factory_context, validator); + auto object = + createRouteSpecificFilterConfig(name, it.second, ProtobufWkt::Struct::default_instance(), + optional_http_filters, factory_context, validator); if (object != nullptr) { configs_[name] = std::move(object); - } else { - if (Runtime::runtimeFeatureEnabled( - "envoy.reloadable_features.check_unsupported_typed_per_filter_config")) { - throw EnvoyException(fmt::format( - "The filter {} doesn't support virtual host-specific configurations", name)); - } else { - ENVOY_LOG(warn, - "The filter {} doesn't support virtual host-specific configurations. Set runtime " - "config `envoy.reloadable_features.check_unsupported_typed_per_filter_config` as " - "true to reject any invalid virtual-host specific configuration.", - name); - } } } @@ -1590,22 +1612,11 @@ PerFilterConfigs::PerFilterConfigs( const auto& name = Extensions::HttpFilters::Common::FilterNameUtil::canonicalFilterName(it.first); - auto object = createRouteSpecificFilterConfig(name, ProtobufWkt::Any::default_instance(), - it.second, factory_context, validator); + auto object = + createRouteSpecificFilterConfig(name, ProtobufWkt::Any::default_instance(), it.second, + optional_http_filters, factory_context, validator); if (object != nullptr) { configs_[name] = std::move(object); - } else { - if (Runtime::runtimeFeatureEnabled( - "envoy.reloadable_features.check_unsupported_typed_per_filter_config")) { - throw EnvoyException(fmt::format( - "The filter {} doesn't support virtual host-specific configurations", name)); - } else { - ENVOY_LOG(warn, - "The filter {} doesn't support virtual host-specific configurations. Set runtime " - "config `envoy.reloadable_features.check_unsupported_typed_per_filter_config` as " - "true to reject any invalid virtual-host specific configuration.", - name); - } } } } diff --git a/source/common/router/config_impl.h b/source/common/router/config_impl.h index b88f1932666d5..7f9abd11c02fc 100644 --- a/source/common/router/config_impl.h +++ b/source/common/router/config_impl.h @@ -72,16 +72,26 @@ class Matchable { virtual bool supportsPathlessHeaders() const { return false; } }; +using OptionalHttpFilters = absl::flat_hash_set; + class PerFilterConfigs : public Logger::Loggable { public: PerFilterConfigs(const Protobuf::Map& typed_configs, const Protobuf::Map& configs, + const OptionalHttpFilters& optional_http_filters, Server::Configuration::ServerFactoryContext& factory_context, ProtobufMessage::ValidationVisitor& validator); const RouteSpecificFilterConfig* get(const std::string& name) const; private: + RouteSpecificFilterConfigConstSharedPtr + createRouteSpecificFilterConfig(const std::string& name, const ProtobufWkt::Any& typed_config, + const ProtobufWkt::Struct& config, + const OptionalHttpFilters& optional_http_filters, + Server::Configuration::ServerFactoryContext& factory_context, + ProtobufMessage::ValidationVisitor& validator); + absl::node_hash_map configs_; }; @@ -177,7 +187,7 @@ class VirtualHostImpl : public VirtualHost { public: VirtualHostImpl( const envoy::config::route::v3::VirtualHost& virtual_host, - const ConfigImpl& global_route_config, + const OptionalHttpFilters& optional_http_filters, const ConfigImpl& global_route_config, Server::Configuration::ServerFactoryContext& factory_context, Stats::Scope& scope, ProtobufMessage::ValidationVisitor& validator, const absl::optional& validation_clusters); @@ -472,6 +482,7 @@ class RouteEntryImplBase : public RouteEntry, * @throw EnvoyException with reason if the route configuration contains any errors */ RouteEntryImplBase(const VirtualHostImpl& vhost, const envoy::config::route::v3::Route& route, + const OptionalHttpFilters& optional_http_filters, Server::Configuration::ServerFactoryContext& factory_context, ProtobufMessage::ValidationVisitor& validator); @@ -741,7 +752,8 @@ class RouteEntryImplBase : public RouteEntry, WeightedClusterEntry(const RouteEntryImplBase* parent, const std::string& rutime_key, Server::Configuration::ServerFactoryContext& factory_context, ProtobufMessage::ValidationVisitor& validator, - const envoy::config::route::v3::WeightedCluster::ClusterWeight& cluster); + const envoy::config::route::v3::WeightedCluster::ClusterWeight& cluster, + const OptionalHttpFilters& optional_http_filters); uint64_t clusterWeight() const { return loader_.snapshot().getInteger(runtime_key_, cluster_weight_); @@ -880,6 +892,7 @@ class RouteEntryImplBase : public RouteEntry, class PrefixRouteEntryImpl : public RouteEntryImplBase { public: PrefixRouteEntryImpl(const VirtualHostImpl& vhost, const envoy::config::route::v3::Route& route, + const OptionalHttpFilters& optional_http_filters, Server::Configuration::ServerFactoryContext& factory_context, ProtobufMessage::ValidationVisitor& validator); @@ -911,6 +924,7 @@ class PrefixRouteEntryImpl : public RouteEntryImplBase { class PathRouteEntryImpl : public RouteEntryImplBase { public: PathRouteEntryImpl(const VirtualHostImpl& vhost, const envoy::config::route::v3::Route& route, + const OptionalHttpFilters& optional_http_filters, Server::Configuration::ServerFactoryContext& factory_context, ProtobufMessage::ValidationVisitor& validator); @@ -942,6 +956,7 @@ class PathRouteEntryImpl : public RouteEntryImplBase { class RegexRouteEntryImpl : public RouteEntryImplBase { public: RegexRouteEntryImpl(const VirtualHostImpl& vhost, const envoy::config::route::v3::Route& route, + const OptionalHttpFilters& optional_http_filters, Server::Configuration::ServerFactoryContext& factory_context, ProtobufMessage::ValidationVisitor& validator); @@ -973,6 +988,7 @@ class RegexRouteEntryImpl : public RouteEntryImplBase { class ConnectRouteEntryImpl : public RouteEntryImplBase { public: ConnectRouteEntryImpl(const VirtualHostImpl& vhost, const envoy::config::route::v3::Route& route, + const OptionalHttpFilters& optional_http_filters, Server::Configuration::ServerFactoryContext& factory_context, ProtobufMessage::ValidationVisitor& validator); @@ -1001,6 +1017,7 @@ class ConnectRouteEntryImpl : public RouteEntryImplBase { class RouteMatcher { public: RouteMatcher(const envoy::config::route::v3::RouteConfiguration& config, + const OptionalHttpFilters& optional_http_filters, const ConfigImpl& global_http_config, Server::Configuration::ServerFactoryContext& factory_context, ProtobufMessage::ValidationVisitor& validator, bool validate_clusters); @@ -1041,6 +1058,7 @@ class RouteMatcher { class ConfigImpl : public Config { public: ConfigImpl(const envoy::config::route::v3::RouteConfiguration& config, + const OptionalHttpFilters& optional_http_filters, Server::Configuration::ServerFactoryContext& factory_context, ProtobufMessage::ValidationVisitor& validator, bool validate_clusters_default); diff --git a/source/common/router/rds_impl.cc b/source/common/router/rds_impl.cc index bce94434d6fb2..cc9c20e2d30e1 100644 --- a/source/common/router/rds_impl.cc +++ b/source/common/router/rds_impl.cc @@ -29,17 +29,24 @@ RouteConfigProviderSharedPtr RouteConfigProviderUtil::create( Server::Configuration::ServerFactoryContext& factory_context, ProtobufMessage::ValidationVisitor& validator, Init::Manager& init_manager, const std::string& stat_prefix, RouteConfigProviderManager& route_config_provider_manager) { + OptionalHttpFilters optional_http_filters; + auto& filters = config.http_filters(); + for (const auto& filter : filters) { + if (filter.is_optional()) { + optional_http_filters.insert(filter.name()); + } + } switch (config.route_specifier_case()) { case envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager:: RouteSpecifierCase::kRouteConfig: return route_config_provider_manager.createStaticRouteConfigProvider( - config.route_config(), factory_context, validator); + config.route_config(), optional_http_filters, factory_context, validator); case envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager:: RouteSpecifierCase::kRds: return route_config_provider_manager.createRdsRouteConfigProvider( // At the creation of a RDS route config provider, the factory_context's initManager is // always valid, though the init manager may go away later when the listener goes away. - config.rds(), factory_context, stat_prefix, init_manager); + config.rds(), optional_http_filters, factory_context, stat_prefix, init_manager); default: NOT_REACHED_GCOVR_EXCL_LINE; } @@ -47,10 +54,11 @@ RouteConfigProviderSharedPtr RouteConfigProviderUtil::create( StaticRouteConfigProviderImpl::StaticRouteConfigProviderImpl( const envoy::config::route::v3::RouteConfiguration& config, + const OptionalHttpFilters& optional_http_filters, Server::Configuration::ServerFactoryContext& factory_context, ProtobufMessage::ValidationVisitor& validator, RouteConfigProviderManagerImpl& route_config_provider_manager) - : config_(new ConfigImpl(config, factory_context, validator, true)), + : config_(new ConfigImpl(config, optional_http_filters, factory_context, validator, true)), route_config_proto_{config}, last_updated_(factory_context.timeSource().systemTime()), route_config_provider_manager_(route_config_provider_manager) { route_config_provider_manager_.static_route_config_providers_.insert(this); @@ -64,7 +72,7 @@ StaticRouteConfigProviderImpl::~StaticRouteConfigProviderImpl() { RdsRouteConfigSubscription::RdsRouteConfigSubscription( const envoy::extensions::filters::network::http_connection_manager::v3::Rds& rds, const uint64_t manager_identifier, Server::Configuration::ServerFactoryContext& factory_context, - const std::string& stat_prefix, + const std::string& stat_prefix, const OptionalHttpFilters& optional_http_filters, Envoy::Router::RouteConfigProviderManagerImpl& route_config_provider_manager) : Envoy::Config::SubscriptionBase( rds.config_source().resource_api_version(), @@ -82,14 +90,15 @@ RdsRouteConfigSubscription::RdsRouteConfigSubscription( local_init_manager_(fmt::format("RDS local-init-manager {}", route_config_name_)), stat_prefix_(stat_prefix), stats_({ALL_RDS_STATS(POOL_COUNTER(*scope_))}), route_config_provider_manager_(route_config_provider_manager), - manager_identifier_(manager_identifier) { + manager_identifier_(manager_identifier), optional_http_filters_(optional_http_filters) { const auto resource_name = getResourceName(); subscription_ = factory_context.clusterManager().subscriptionFactory().subscriptionFromConfigSource( rds.config_source(), Grpc::Common::typeUrl(resource_name), *scope_, *this, resource_decoder_, {}); local_init_manager_.add(local_init_target_); - config_update_info_ = std::make_unique(factory_context); + config_update_info_ = + std::make_unique(factory_context, optional_http_filters_); } RdsRouteConfigSubscription::~RdsRouteConfigSubscription() { @@ -224,15 +233,17 @@ bool RdsRouteConfigSubscription::validateUpdateSize(int num_resources) { RdsRouteConfigProviderImpl::RdsRouteConfigProviderImpl( RdsRouteConfigSubscriptionSharedPtr&& subscription, - Server::Configuration::ServerFactoryContext& factory_context) + Server::Configuration::ServerFactoryContext& factory_context, + const OptionalHttpFilters& optional_http_filters) : subscription_(std::move(subscription)), config_update_info_(subscription_->routeConfigUpdate()), factory_context_(factory_context), validator_(factory_context.messageValidationContext().dynamicValidationVisitor()), - tls_(factory_context.threadLocal()) { + tls_(factory_context.threadLocal()), optional_http_filters_(optional_http_filters) { ConfigConstSharedPtr initial_config; if (config_update_info_->configInfo().has_value()) { - initial_config = std::make_shared(config_update_info_->protobufConfiguration(), - factory_context_, validator_, false); + initial_config = + std::make_shared(config_update_info_->protobufConfiguration(), + optional_http_filters_, factory_context_, validator_, false); } else { initial_config = std::make_shared(); } @@ -289,7 +300,7 @@ void RdsRouteConfigProviderImpl::onConfigUpdate() { void RdsRouteConfigProviderImpl::validateConfig( const envoy::config::route::v3::RouteConfiguration& config) const { // TODO(lizan): consider cache the config here until onConfigUpdate. - ConfigImpl validation_config(config, factory_context_, validator_, false); + ConfigImpl validation_config(config, optional_http_filters_, factory_context_, validator_, false); } // Schedules a VHDS request on the main thread and queues up the callback to use when the VHDS @@ -323,6 +334,7 @@ RouteConfigProviderManagerImpl::RouteConfigProviderManagerImpl(Server::Admin& ad Router::RouteConfigProviderSharedPtr RouteConfigProviderManagerImpl::createRdsRouteConfigProvider( const envoy::extensions::filters::network::http_connection_manager::v3::Rds& rds, + const OptionalHttpFilters& optional_http_filters, Server::Configuration::ServerFactoryContext& factory_context, const std::string& stat_prefix, Init::Manager& init_manager) { // RdsRouteConfigSubscriptions are unique based on their serialized RDS config. @@ -334,10 +346,10 @@ Router::RouteConfigProviderSharedPtr RouteConfigProviderManagerImpl::createRdsRo // around it. However, since this is not a performance critical path we err on the side // of simplicity. RdsRouteConfigSubscriptionSharedPtr subscription(new RdsRouteConfigSubscription( - rds, manager_identifier, factory_context, stat_prefix, *this)); + rds, manager_identifier, factory_context, stat_prefix, optional_http_filters, *this)); init_manager.add(subscription->parent_init_target_); - RdsRouteConfigProviderImplSharedPtr new_provider{ - new RdsRouteConfigProviderImpl(std::move(subscription), factory_context)}; + RdsRouteConfigProviderImplSharedPtr new_provider{new RdsRouteConfigProviderImpl( + std::move(subscription), factory_context, optional_http_filters)}; dynamic_route_config_providers_.insert( {manager_identifier, std::weak_ptr(new_provider)}); return new_provider; @@ -355,10 +367,11 @@ Router::RouteConfigProviderSharedPtr RouteConfigProviderManagerImpl::createRdsRo RouteConfigProviderPtr RouteConfigProviderManagerImpl::createStaticRouteConfigProvider( const envoy::config::route::v3::RouteConfiguration& route_config, + const OptionalHttpFilters& optional_http_filters, Server::Configuration::ServerFactoryContext& factory_context, ProtobufMessage::ValidationVisitor& validator) { - auto provider = std::make_unique(route_config, factory_context, - validator, *this); + auto provider = std::make_unique( + route_config, optional_http_filters, factory_context, validator, *this); static_route_config_providers_.insert(provider.get()); return provider; } diff --git a/source/common/router/rds_impl.h b/source/common/router/rds_impl.h index bbe32506c9398..22182e48d416e 100644 --- a/source/common/router/rds_impl.h +++ b/source/common/router/rds_impl.h @@ -68,6 +68,7 @@ class RouteConfigProviderManagerImpl; class StaticRouteConfigProviderImpl : public RouteConfigProvider { public: StaticRouteConfigProviderImpl(const envoy::config::route::v3::RouteConfiguration& config, + const OptionalHttpFilters& http_filters, Server::Configuration::ServerFactoryContext& factory_context, ProtobufMessage::ValidationVisitor& validator, RouteConfigProviderManagerImpl& route_config_provider_manager); @@ -144,6 +145,7 @@ class RdsRouteConfigSubscription const envoy::extensions::filters::network::http_connection_manager::v3::Rds& rds, const uint64_t manager_identifier, Server::Configuration::ServerFactoryContext& factory_context, const std::string& stat_prefix, + const OptionalHttpFilters& optional_http_filters, RouteConfigProviderManagerImpl& route_config_provider_manager); bool validateUpdateSize(int num_resources); @@ -170,6 +172,7 @@ class RdsRouteConfigSubscription VhdsSubscriptionPtr vhds_subscription_; RouteConfigUpdatePtr config_update_info_; Common::CallbackManager<> update_callback_manager_; + const OptionalHttpFilters optional_http_filters_; friend class RouteConfigProviderManagerImpl; // Access to addUpdateCallback @@ -214,7 +217,8 @@ class RdsRouteConfigProviderImpl : public RouteConfigProvider, }; RdsRouteConfigProviderImpl(RdsRouteConfigSubscriptionSharedPtr&& subscription, - Server::Configuration::ServerFactoryContext& factory_context); + Server::Configuration::ServerFactoryContext& factory_context, + const OptionalHttpFilters& optional_http_filters); RdsRouteConfigSubscriptionSharedPtr subscription_; RouteConfigUpdatePtr& config_update_info_; @@ -225,6 +229,7 @@ class RdsRouteConfigProviderImpl : public RouteConfigProvider, // A flag used to determine if this instance of RdsRouteConfigProviderImpl hasn't been // deallocated. Please also see a comment in requestVirtualHostsUpdate() method implementation. std::shared_ptr still_alive_{std::make_shared(true)}; + const OptionalHttpFilters optional_http_filters_; friend class RouteConfigProviderManagerImpl; }; @@ -241,11 +246,13 @@ class RouteConfigProviderManagerImpl : public RouteConfigProviderManager, // RouteConfigProviderManager RouteConfigProviderSharedPtr createRdsRouteConfigProvider( const envoy::extensions::filters::network::http_connection_manager::v3::Rds& rds, + const OptionalHttpFilters& optional_http_filters, Server::Configuration::ServerFactoryContext& factory_context, const std::string& stat_prefix, Init::Manager& init_manager) override; RouteConfigProviderPtr createStaticRouteConfigProvider(const envoy::config::route::v3::RouteConfiguration& route_config, + const OptionalHttpFilters& optional_http_filters, Server::Configuration::ServerFactoryContext& factory_context, ProtobufMessage::ValidationVisitor& validator) override; diff --git a/source/common/router/route_config_update_receiver_impl.cc b/source/common/router/route_config_update_receiver_impl.cc index bbee2b374772a..b3226de028005 100644 --- a/source/common/router/route_config_update_receiver_impl.cc +++ b/source/common/router/route_config_update_receiver_impl.cc @@ -29,7 +29,7 @@ bool RouteConfigUpdateReceiverImpl::onRdsUpdate( rebuildRouteConfig(rds_virtual_hosts_, *vhds_virtual_hosts_, *route_config_proto_); config_ = std::make_shared( - *route_config_proto_, factory_context_, + *route_config_proto_, optional_http_filters_, factory_context_, factory_context_.messageValidationContext().dynamicValidationVisitor(), false); onUpdateCommon(version_info); @@ -54,7 +54,7 @@ bool RouteConfigUpdateReceiverImpl::onVhdsUpdate( *route_config_after_this_update); auto new_config = std::make_shared( - *route_config_after_this_update, factory_context_, + *route_config_after_this_update, optional_http_filters_, factory_context_, factory_context_.messageValidationContext().dynamicValidationVisitor(), false); // No exception, route_config_after_this_update is valid, can update the state. diff --git a/source/common/router/route_config_update_receiver_impl.h b/source/common/router/route_config_update_receiver_impl.h index b92490d0849c0..4f77bd34156bd 100644 --- a/source/common/router/route_config_update_receiver_impl.h +++ b/source/common/router/route_config_update_receiver_impl.h @@ -18,13 +18,14 @@ namespace Router { class RouteConfigUpdateReceiverImpl : public RouteConfigUpdateReceiver { public: - RouteConfigUpdateReceiverImpl(Server::Configuration::ServerFactoryContext& factory_context) + RouteConfigUpdateReceiverImpl(Server::Configuration::ServerFactoryContext& factory_context, + const OptionalHttpFilters& optional_http_filters) : factory_context_(factory_context), time_source_(factory_context.timeSource()), route_config_proto_(std::make_unique()), last_config_hash_(0ull), last_vhds_config_hash_(0ul), vhds_virtual_hosts_( std::make_unique>()), - vhds_configuration_changed_(true) {} + vhds_configuration_changed_(true), optional_http_filters_(optional_http_filters) {} void initializeRdsVhosts(const envoy::config::route::v3::RouteConfiguration& route_configuration); bool removeVhosts(std::map& vhosts, @@ -75,6 +76,7 @@ class RouteConfigUpdateReceiverImpl : public RouteConfigUpdateReceiver { std::set resource_ids_in_last_update_; bool vhds_configuration_changed_; ConfigConstSharedPtr config_; + const OptionalHttpFilters& optional_http_filters_; }; } // namespace Router diff --git a/source/common/router/scoped_rds.cc b/source/common/router/scoped_rds.cc index 92b02189f4b89..1e7e6d584972f 100644 --- a/source/common/router/scoped_rds.cc +++ b/source/common/router/scoped_rds.cc @@ -43,7 +43,13 @@ ConfigProviderPtr create( ASSERT(config.route_specifier_case() == envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager:: RouteSpecifierCase::kScopedRoutes); - + OptionalHttpFilters optional_http_filters; + auto& filters = config.http_filters(); + for (const auto& filter : filters) { + if (filter.is_optional()) { + optional_http_filters.insert(filter.name()); + } + } switch (config.scoped_routes().config_specifier_case()) { case envoy::extensions::filters::network::http_connection_manager::v3::ScopedRoutes:: ConfigSpecifierCase::kScopedRouteConfigurationsList: { @@ -55,17 +61,17 @@ ConfigProviderPtr create( envoy::config::route::v3::ScopedRouteConfiguration, ProtobufTypes::ConstMessagePtrVector>(scoped_route_list.scoped_route_configurations()), factory_context, - ScopedRoutesConfigProviderManagerOptArg(config.scoped_routes().name(), - config.scoped_routes().rds_config_source(), - config.scoped_routes().scope_key_builder())); + ScopedRoutesConfigProviderManagerOptArg( + config.scoped_routes().name(), config.scoped_routes().rds_config_source(), + config.scoped_routes().scope_key_builder(), optional_http_filters)); } case envoy::extensions::filters::network::http_connection_manager::v3::ScopedRoutes:: ConfigSpecifierCase::kScopedRds: return scoped_routes_config_provider_manager.createXdsConfigProvider( config.scoped_routes().scoped_rds(), factory_context, init_manager, stat_prefix, - ScopedRoutesConfigProviderManagerOptArg(config.scoped_routes().name(), - config.scoped_routes().rds_config_source(), - config.scoped_routes().scope_key_builder())); + ScopedRoutesConfigProviderManagerOptArg( + config.scoped_routes().name(), config.scoped_routes().rds_config_source(), + config.scoped_routes().scope_key_builder(), optional_http_filters)); default: // Proto validation enforces that is not reached. NOT_REACHED_GCOVR_EXCL_LINE; @@ -92,7 +98,8 @@ InlineScopedRoutesConfigProvider::InlineScopedRoutesConfigProvider( ScopedRdsConfigSubscription::ScopedRdsConfigSubscription( const envoy::extensions::filters::network::http_connection_manager::v3::ScopedRds& scoped_rds, - const uint64_t manager_identifier, const std::string& name, + const OptionalHttpFilters& optional_http_filters, const uint64_t manager_identifier, + const std::string& name, const envoy::extensions::filters::network::http_connection_manager::v3::ScopedRoutes:: ScopeKeyBuilder& scope_key_builder, Server::Configuration::ServerFactoryContext& factory_context, const std::string& stat_prefix, @@ -108,7 +115,8 @@ ScopedRdsConfigSubscription::ScopedRdsConfigSubscription( scope_(factory_context.scope().createScope(stat_prefix + "scoped_rds." + name + ".")), stats_({ALL_SCOPED_RDS_STATS(POOL_COUNTER(*scope_), POOL_GAUGE(*scope_))}), scope_key_builder_(scope_key_builder), rds_config_source_(std::move(rds_config_source)), - stat_prefix_(stat_prefix), route_config_provider_manager_(route_config_provider_manager) { + stat_prefix_(stat_prefix), route_config_provider_manager_(route_config_provider_manager), + optional_http_filters_(optional_http_filters) { const auto resource_name = getResourceName(); if (scoped_rds.srds_resources_locator().empty()) { subscription_ = @@ -186,7 +194,8 @@ void ScopedRdsConfigSubscription::RdsRouteConfigProviderHelper::initRdsConfigPro Init::Manager& init_manager) { route_provider_ = std::dynamic_pointer_cast( parent_.route_config_provider_manager_.createRdsRouteConfigProvider( - rds, parent_.factory_context_, parent_.stat_prefix_, init_manager)); + rds, parent_.optional_http_filters_, parent_.factory_context_, parent_.stat_prefix_, + init_manager)); rds_update_callback_handle_ = route_provider_->subscription().addUpdateCallback([this]() { // Subscribe to RDS update. @@ -394,8 +403,9 @@ void ScopedRdsConfigSubscription::onRdsConfigUpdate(const std::string& scope_nam auto new_scoped_route_info = std::make_shared( envoy::config::route::v3::ScopedRouteConfiguration(iter->second->configProto()), std::make_shared( - rds_subscription.routeConfigUpdate()->protobufConfiguration(), factory_context_, - factory_context_.messageValidationContext().dynamicValidationVisitor(), false)); + rds_subscription.routeConfigUpdate()->protobufConfiguration(), optional_http_filters_, + factory_context_, factory_context_.messageValidationContext().dynamicValidationVisitor(), + false)); applyConfigUpdate([new_scoped_route_info](ConfigProvider::ConfigConstSharedPtr config) -> ConfigProvider::ConfigConstSharedPtr { auto* thread_local_scoped_config = @@ -565,9 +575,9 @@ ConfigProviderPtr ScopedRoutesConfigProviderManager::createXdsConfigProvider( const envoy::extensions::filters::network::http_connection_manager::v3::ScopedRds&>( config_source_proto); return std::make_shared( - scoped_rds_config_source, manager_identifier, typed_optarg.scoped_routes_name_, - typed_optarg.scope_key_builder_, factory_context, stat_prefix, - typed_optarg.rds_config_source_, + scoped_rds_config_source, typed_optarg.optional_http_filters_, manager_identifier, + typed_optarg.scoped_routes_name_, typed_optarg.scope_key_builder_, factory_context, + stat_prefix, typed_optarg.rds_config_source_, static_cast(config_provider_manager) .routeConfigProviderPanager(), static_cast(config_provider_manager)); diff --git a/source/common/router/scoped_rds.h b/source/common/router/scoped_rds.h index 05fcf44ef3698..12fde395b134e 100644 --- a/source/common/router/scoped_rds.h +++ b/source/common/router/scoped_rds.h @@ -110,7 +110,8 @@ class ScopedRdsConfigSubscription ScopedRdsConfigSubscription( const envoy::extensions::filters::network::http_connection_manager::v3::ScopedRds& scoped_rds, - const uint64_t manager_identifier, const std::string& name, + const OptionalHttpFilters& optional_http_filters, const uint64_t manager_identifier, + const std::string& name, const envoy::extensions::filters::network::http_connection_manager::v3::ScopedRoutes:: ScopeKeyBuilder& scope_key_builder, Server::Configuration::ServerFactoryContext& factory_context, const std::string& stat_prefix, @@ -238,6 +239,7 @@ class ScopedRdsConfigSubscription absl::flat_hash_map route_provider_by_scope_; // A map of (hash, scope-name), used to detect the key conflict between scopes. absl::flat_hash_map scope_name_by_hash_; + const OptionalHttpFilters optional_http_filters_; }; using ScopedRdsConfigSubscriptionSharedPtr = std::shared_ptr; @@ -313,14 +315,16 @@ class ScopedRoutesConfigProviderManagerOptArg std::string scoped_routes_name, const envoy::config::core::v3::ConfigSource& rds_config_source, const envoy::extensions::filters::network::http_connection_manager::v3::ScopedRoutes:: - ScopeKeyBuilder& scope_key_builder) + ScopeKeyBuilder& scope_key_builder, + const OptionalHttpFilters& optional_http_filters) : scoped_routes_name_(std::move(scoped_routes_name)), rds_config_source_(rds_config_source), - scope_key_builder_(scope_key_builder) {} + scope_key_builder_(scope_key_builder), optional_http_filters_(optional_http_filters) {} const std::string scoped_routes_name_; const envoy::config::core::v3::ConfigSource& rds_config_source_; const envoy::extensions::filters::network::http_connection_manager::v3::ScopedRoutes:: ScopeKeyBuilder& scope_key_builder_; + const OptionalHttpFilters& optional_http_filters_; }; } // namespace Router diff --git a/source/common/runtime/runtime_features.cc b/source/common/runtime/runtime_features.cc index 935961c287696..c44cc2a55ad38 100644 --- a/source/common/runtime/runtime_features.cc +++ b/source/common/runtime/runtime_features.cc @@ -76,6 +76,7 @@ constexpr const char* runtime_features[] = { "envoy.reloadable_features.http2_skip_encoding_empty_trailers", "envoy.reloadable_features.improved_stream_limit_handling", "envoy.reloadable_features.internal_redirects_with_body", + "envoy.reloadable_features.new_tcp_connection_pool", "envoy.reloadable_features.prefer_quic_kernel_bpf_packet_routing", "envoy.reloadable_features.preserve_downstream_scheme", "envoy.reloadable_features.remove_forked_chromium_url", @@ -106,8 +107,6 @@ constexpr const char* runtime_features[] = { constexpr const char* disabled_runtime_features[] = { // v2 is fatal-by-default. "envoy.test_only.broken_in_production.enable_deprecated_v2_api", - // TODO(alyssawilk) flip true after the release. - "envoy.reloadable_features.new_tcp_connection_pool", // TODO(asraa) flip to true in a separate PR to enable the new JSON by default. "envoy.reloadable_features.remove_legacy_json", // Sentinel and test flag. diff --git a/source/common/upstream/BUILD b/source/common/upstream/BUILD index c8f8a219b17a8..e8b3693993742 100644 --- a/source/common/upstream/BUILD +++ b/source/common/upstream/BUILD @@ -74,6 +74,7 @@ envoy_cc_library( "//source/common/config:xds_resource_lib", "//source/common/grpc:async_client_manager_lib", "//source/common/http:async_client_lib", + "//source/common/http:alternate_protocols_cache", "//source/common/http:mixed_conn_pool", "//source/common/http/http1:conn_pool_lib", "//source/common/http/http2:conn_pool_lib", diff --git a/source/common/upstream/cluster_manager_impl.cc b/source/common/upstream/cluster_manager_impl.cc index 84825e941ab29..05e18092e9332 100644 --- a/source/common/upstream/cluster_manager_impl.cc +++ b/source/common/upstream/cluster_manager_impl.cc @@ -11,6 +11,7 @@ #include "envoy/config/bootstrap/v3/bootstrap.pb.h" #include "envoy/config/cluster/v3/cluster.pb.h" #include "envoy/config/core/v3/config_source.pb.h" +#include "envoy/config/core/v3/protocol.pb.h" #include "envoy/event/dispatcher.h" #include "envoy/network/dns.h" #include "envoy/runtime/runtime.h" @@ -1409,6 +1410,8 @@ ClusterManagerImpl::ThreadLocalClusterManagerImpl::ClusterEntry::connPool( hash_key.push_back(uint8_t(protocol)); } + absl::optional + alternate_protocol_options = host->cluster().alternateProtocolsCacheOptions(); Network::Socket::OptionsSharedPtr upstream_options(std::make_shared()); if (context) { // Inherit socket options from downstream connection, if set. @@ -1445,7 +1448,7 @@ ClusterManagerImpl::ThreadLocalClusterManagerImpl::ClusterEntry::connPool( container.pools_->getPool(priority, hash_key, [&]() { return parent_.parent_.factory_.allocateConnPool( parent_.thread_local_dispatcher_, host, priority, upstream_protocols, - !upstream_options->empty() ? upstream_options : nullptr, + alternate_protocol_options, !upstream_options->empty() ? upstream_options : nullptr, have_transport_socket_options ? context->upstreamTransportSocketOptions() : nullptr, parent_.parent_.time_source_, parent_.cluster_manager_state_); }); @@ -1515,19 +1518,25 @@ ClusterManagerPtr ProdClusterManagerFactory::clusterManagerFromProto( Http::ConnectionPool::InstancePtr ProdClusterManagerFactory::allocateConnPool( Event::Dispatcher& dispatcher, HostConstSharedPtr host, ResourcePriority priority, std::vector& protocols, + const absl::optional& + alternate_protocol_options, const Network::ConnectionSocket::OptionsSharedPtr& options, const Network::TransportSocketOptionsSharedPtr& transport_socket_options, TimeSource& source, ClusterConnectivityState& state) { if (protocols.size() == 3 && runtime_.snapshot().featureEnabled("upstream.use_http3", 100)) { ASSERT(contains(protocols, {Http::Protocol::Http11, Http::Protocol::Http2, Http::Protocol::Http3})); + Http::AlternateProtocolsCacheSharedPtr alternate_protocols_cache; + if (alternate_protocol_options.has_value()) { + alternate_protocols_cache = + alternate_protocols_cache_manager_->getCache(alternate_protocol_options.value()); + } #ifdef ENVOY_ENABLE_QUIC // TODO(RyanTheOptimist): Plumb an actual alternate protocols cache. - auto alternate_protocols = makeOptRefFromPtr(nullptr); Envoy::Http::ConnectivityGrid::ConnectivityOptions coptions{protocols}; return std::make_unique( dispatcher, api_.randomGenerator(), host, priority, options, transport_socket_options, - state, source, alternate_protocols, std::chrono::milliseconds(300), coptions); + state, source, alternate_protocols_cache, std::chrono::milliseconds(300), coptions); #else // Should be blocked by configuration checking at an earlier point. NOT_REACHED_GCOVR_EXCL_LINE; diff --git a/source/common/upstream/cluster_manager_impl.h b/source/common/upstream/cluster_manager_impl.h index ec75f0af94677..5034a261c107b 100644 --- a/source/common/upstream/cluster_manager_impl.h +++ b/source/common/upstream/cluster_manager_impl.h @@ -29,6 +29,8 @@ #include "common/common/cleanup.h" #include "common/config/grpc_mux_impl.h" #include "common/config/subscription_factory_impl.h" +#include "common/http/alternate_protocols_cache_impl.h" +#include "common/http/alternate_protocols_cache_manager_impl.h" #include "common/http/async_client_impl.h" #include "common/upstream/load_stats_reporter.h" #include "common/upstream/priority_conn_pool_map.h" @@ -58,7 +60,10 @@ class ProdClusterManagerFactory : public ClusterManagerFactory { router_context_(router_context), admin_(admin), runtime_(runtime), stats_(stats), tls_(tls), dns_resolver_(dns_resolver), ssl_context_manager_(ssl_context_manager), local_info_(local_info), secret_manager_(secret_manager), log_manager_(log_manager), - singleton_manager_(singleton_manager), options_(options) {} + singleton_manager_(singleton_manager), options_(options), + alternate_protocols_cache_manager_factory_(singleton_manager, + main_thread_dispatcher.timeSource(), tls_), + alternate_protocols_cache_manager_(alternate_protocols_cache_manager_factory_.get()) {} // Upstream::ClusterManagerFactory ClusterManagerPtr @@ -66,6 +71,8 @@ class ProdClusterManagerFactory : public ClusterManagerFactory { Http::ConnectionPool::InstancePtr allocateConnPool(Event::Dispatcher& dispatcher, HostConstSharedPtr host, ResourcePriority priority, std::vector& protocol, + const absl::optional& + alternate_protocol_options, const Network::ConnectionSocket::OptionsSharedPtr& options, const Network::TransportSocketOptionsSharedPtr& transport_socket_options, TimeSource& time_source, ClusterConnectivityState& state) override; @@ -101,6 +108,8 @@ class ProdClusterManagerFactory : public ClusterManagerFactory { AccessLog::AccessLogManager& log_manager_; Singleton::Manager& singleton_manager_; const Server::Options& options_; + Http::AlternateProtocolsCacheManagerFactoryImpl alternate_protocols_cache_manager_factory_; + Http::AlternateProtocolsCacheManagerSharedPtr alternate_protocols_cache_manager_; }; // For friend declaration in ClusterManagerInitHelper. diff --git a/source/common/upstream/upstream_impl.h b/source/common/upstream/upstream_impl.h index 28ff4a7b60218..18c8d8ce832da 100644 --- a/source/common/upstream/upstream_impl.h +++ b/source/common/upstream/upstream_impl.h @@ -663,6 +663,11 @@ class ClusterInfoImpl : public ClusterInfo, return http_protocol_options_->upstream_http_protocol_options_; } + const absl::optional& + alternateProtocolsCacheOptions() const override { + return http_protocol_options_->alternate_protocol_cache_options_; + } + absl::optional edsServiceName() const override { return eds_service_name_; } void createNetworkFilterChain(Network::Connection&) const override; diff --git a/source/docs/http3_upstream.md b/source/docs/http3_upstream.md new file mode 100644 index 0000000000000..bd36afaa9640b --- /dev/null +++ b/source/docs/http3_upstream.md @@ -0,0 +1,53 @@ +### Overview + +Support for Upstream HTTP/3 connections is slightly more complex than for HTTP/1 or HTTP/2 +and requires specific configurations to enable it. If HTTP/3 is configured in auto_config, +HTTP/3 connections will only be attempted to servers which +advertise HTTP/3 support either via [HTTP Alternative Services](https://tools.ietf.org/html/rfc7838) +or the [HTTPS DNS resource record](https://datatracker.ietf.org/doc/html/draft-ietf-dnsop-svcb-https-04). +If no such advertisement exists, then HTTP/2 or HTTP/1 will be used instead. Further, +HTTP/3 runs over QUIC (which uses UDP) and not over TCP (which HTTP/1 and HTTP/2 use). +It is not uncommon for network devices to block UDP traffic, and hence block HTTP/3. This +means that upstream HTTP/3 connection attempts might be blocked by the network and should fall +back to using HTTP/2. On such networks, the upstream connection code needs to +track that HTTP/3 connects attempts are not succeeding and avoid making connections +which are doomed to fail. On networks where HTTP/3 is working correctly, however, the +upstream connection code should avoid attempting HTTP/2 unnecessarily. + +### Components + +#### Broken HTTP/3 tracking +The `BrokenHttp3Tracker` is a per-grid object which tracks the state of HTTP/3 for +that grid. When HTTP/3 is marked as broken, it remains broken for 5 minutes after +which point it is no longer broken. If it is marked as broken again, it remains +broken for twice the previous period, up to a maximum of 1 day. If it is instead +marked as confirmed, then the period is reset to the default and the next time +HTTP/3 is marked as broken it will be for the initial 5 minutes. + +#### Alternate Protocols Cache +The `AlternateProtocolsCache` is responsible for tracking servers which advertise HTTP/3. +These advertisements may arrive via HTTP Altnernate Service (alt-svc) or soon via the HTTPS +DNS RR. Currently only advertisements which specify the same hostname and port are stored. + +#### Connectivity Grid +The `ConnectivityGrid` is a `ConnectionPool` which wraps a `Http3ConnPoolImpl` connection pool +(QUIC) and a `HttpConnPoolImplMixed` connection pool (TCP). If HTTP/3 is currently broken, stream +requets will be sent directly to the mixed connection pool. If HTTP/3 is not advertised in the +alternate protocols cache, stream requets will be sent directly to the mixed connection pool. +If, however, HTTP/3 is advertised and working then the request will be sent to the HTTP/3 +connection pool. After 300ms, if this request has not succeeded, then the request will also be +sent to the mixed pool, and whichever succeeds first will be passed back up to the caller. + +### Required configuration + +Specific Envoy configuration is required in order to enable the previously described components. + +#### Auto Cluster Pool + +The "Auto" must be enabled via the Upstream `HttpProtocolOptions` message. The +`upstream_protocol_options` field needs to specify `auto_config`. This config muse specify +all three protocols: HTTP/1, HTTP/2 and HTTP/3. + +#### Alternate Protocols Cache Options + +In addition, the `alternate_protocols_cache_options` field must be specified. diff --git a/source/extensions/clusters/redis/redis_cluster.cc b/source/extensions/clusters/redis/redis_cluster.cc index 297248a6b6a15..822eab1166be6 100644 --- a/source/extensions/clusters/redis/redis_cluster.cc +++ b/source/extensions/clusters/redis/redis_cluster.cc @@ -100,8 +100,8 @@ void RedisCluster::onClusterSlotUpdate(ClusterSlotsPtr&& slots) { new_hosts.emplace_back(new RedisHost(info(), "", slot.primary(), *this, true, time_source_)); all_new_hosts.emplace(slot.primary()->asString()); for (auto const& replica : slot.replicas()) { - new_hosts.emplace_back(new RedisHost(info(), "", replica, *this, false, time_source_)); - all_new_hosts.emplace(replica->asString()); + new_hosts.emplace_back(new RedisHost(info(), "", replica.second, *this, false, time_source_)); + all_new_hosts.emplace(replica.first); } } diff --git a/source/extensions/clusters/redis/redis_cluster_lb.cc b/source/extensions/clusters/redis/redis_cluster_lb.cc index 43bb0f9b32224..25f38f448dae0 100644 --- a/source/extensions/clusters/redis/redis_cluster_lb.cc +++ b/source/extensions/clusters/redis/redis_cluster_lb.cc @@ -6,8 +6,14 @@ namespace Clusters { namespace Redis { bool ClusterSlot::operator==(const Envoy::Extensions::Clusters::Redis::ClusterSlot& rhs) const { - return start_ == rhs.start_ && end_ == rhs.end_ && primary_ == rhs.primary_ && - replicas_ == rhs.replicas_; + if (start_ != rhs.start_ || end_ != rhs.end_ || *primary_ != *rhs.primary_ || + replicas_.size() != rhs.replicas_.size()) { + return false; + } + // The value type is shared_ptr, and the shared_ptr is not same one even for same ip:port. + // so, just compare the key here. + return std::equal(replicas_.begin(), replicas_.end(), rhs.replicas_.begin(), rhs.replicas_.end(), + [](const auto& it1, const auto& it2) { return it1.first == it2.first; }); } // RedisClusterLoadBalancerFactory @@ -43,7 +49,7 @@ bool RedisClusterLoadBalancerFactory::onClusterSlotUpdate(ClusterSlotsPtr&& slot primary_and_replicas->push_back(primary_host->second); for (auto const& replica : slot.replicas()) { - auto replica_host = all_hosts.find(replica->asString()); + auto replica_host = all_hosts.find(replica.first); ASSERT(replica_host != all_hosts.end(), "we expect all address to be found in the updated_hosts"); replicas->push_back(replica_host->second); diff --git a/source/extensions/clusters/redis/redis_cluster_lb.h b/source/extensions/clusters/redis/redis_cluster_lb.h index 561de3b681e53..7ca27407d1612 100644 --- a/source/extensions/clusters/redis/redis_cluster_lb.h +++ b/source/extensions/clusters/redis/redis_cluster_lb.h @@ -18,8 +18,7 @@ #include "extensions/filters/network/common/redis/codec.h" #include "extensions/filters/network/common/redis/supported_commands.h" -#include "absl/container/flat_hash_map.h" -#include "absl/container/flat_hash_set.h" +#include "absl/container/btree_map.h" #include "absl/synchronization/mutex.h" namespace Envoy { @@ -37,11 +36,11 @@ class ClusterSlot { int64_t start() const { return start_; } int64_t end() const { return end_; } Network::Address::InstanceConstSharedPtr primary() const { return primary_; } - const absl::flat_hash_set& replicas() const { + const absl::btree_map& replicas() const { return replicas_; } void addReplica(Network::Address::InstanceConstSharedPtr replica_address) { - replicas_.insert(std::move(replica_address)); + replicas_.emplace(replica_address->asString(), std::move(replica_address)); } bool operator==(const ClusterSlot& rhs) const; @@ -50,7 +49,7 @@ class ClusterSlot { int64_t start_; int64_t end_; Network::Address::InstanceConstSharedPtr primary_; - absl::flat_hash_set replicas_; + absl::btree_map replicas_; }; using ClusterSlotsPtr = std::unique_ptr>; diff --git a/source/extensions/filters/network/http_connection_manager/config.cc b/source/extensions/filters/network/http_connection_manager/config.cc index 684fae1fca17c..67003b945d6c2 100644 --- a/source/extensions/filters/network/http_connection_manager/config.cc +++ b/source/extensions/filters/network/http_connection_manager/config.cc @@ -608,16 +608,22 @@ void HttpConnectionManagerConfig::processFilter( } // Now see if there is a factory that will accept the config. - auto& factory = + auto* factory = Config::Utility::getAndCheckFactory( - proto_config); + proto_config, proto_config.is_optional()); + // null pointer returned only when the filter is optional, then skip all the processes. + if (factory == nullptr) { + ENVOY_LOG(warn, "Didn't find a registered factory for the optional http filter {}", + proto_config.name()); + return; + } ProtobufTypes::MessagePtr message = Config::Utility::translateToFactoryConfig( - proto_config, context_.messageValidationVisitor(), factory); + proto_config, context_.messageValidationVisitor(), *factory); Http::FilterFactoryCb callback = - factory.createFilterFactoryFromProto(*message, stats_prefix_, context_); - dependency_manager.registerFilter(factory.name(), *factory.dependencies()); - bool is_terminal = factory.isTerminalFilterByProto(*message, context_); - Config::Utility::validateTerminalFilters(proto_config.name(), factory.name(), filter_chain_type, + factory->createFilterFactoryFromProto(*message, stats_prefix_, context_); + dependency_manager.registerFilter(factory->name(), *factory->dependencies()); + bool is_terminal = factory->isTerminalFilterByProto(*message, context_); + Config::Utility::validateTerminalFilters(proto_config.name(), factory->name(), filter_chain_type, is_terminal, last_filter_in_current_config); auto filter_config_provider = filter_config_provider_manager_.createStaticFilterConfigProvider( callback, proto_config.name()); diff --git a/source/extensions/transport_sockets/alts/tsi_socket.cc b/source/extensions/transport_sockets/alts/tsi_socket.cc index ff0cd252f11d0..ce2316921ce79 100644 --- a/source/extensions/transport_sockets/alts/tsi_socket.cc +++ b/source/extensions/transport_sockets/alts/tsi_socket.cc @@ -272,7 +272,7 @@ Network::IoResult TsiSocket::repeatProtectAndWrite(Buffer::Instance& buffer, boo uint64_t bytes_to_drain_this_iteration = prev_bytes_to_drain_ > 0 ? prev_bytes_to_drain_ - : std::min(buffer.length(), actual_frame_size_to_use_ - frame_overhead_size_); + : std::min(buffer.length(), actual_frame_size_to_use_ - frame_overhead_size_); // Consumed all data. Exit. if (bytes_to_drain_this_iteration == 0) { break; diff --git a/source/extensions/upstreams/http/config.cc b/source/extensions/upstreams/http/config.cc index 5cb2acc4187f3..fd204bfd75d84 100644 --- a/source/extensions/upstreams/http/config.cc +++ b/source/extensions/upstreams/http/config.cc @@ -111,6 +111,9 @@ ProtocolOptionsConfigImpl::ProtocolOptionsConfigImpl( use_http2_ = true; use_alpn_ = true; use_http3_ = options.auto_config().has_http3_protocol_options(); + if (options.auto_config().has_alternate_protocols_cache_options()) { + alternate_protocol_cache_options_ = options.auto_config().alternate_protocols_cache_options(); + } } } diff --git a/source/extensions/upstreams/http/config.h b/source/extensions/upstreams/http/config.h index 5fcf134c68664..4668c5d1a38a7 100644 --- a/source/extensions/upstreams/http/config.h +++ b/source/extensions/upstreams/http/config.h @@ -8,6 +8,7 @@ #include #include "envoy/config/core/v3/extension.pb.h" +#include "envoy/config/core/v3/protocol.pb.h" #include "envoy/extensions/upstreams/http/v3/http_protocol_options.pb.h" #include "envoy/extensions/upstreams/http/v3/http_protocol_options.pb.validate.h" #include "envoy/http/filter.h" @@ -52,6 +53,8 @@ class ProtocolOptionsConfigImpl : public Upstream::ProtocolOptionsConfig { bool use_http2_{}; bool use_http3_{}; bool use_alpn_{}; + absl::optional + alternate_protocol_cache_options_; }; class ProtocolOptionsConfigFactory : public Server::Configuration::ProtocolOptionsFactory { diff --git a/source/server/api_listener_impl.h b/source/server/api_listener_impl.h index c218e109f4a95..ca792e83ebdde 100644 --- a/source/server/api_listener_impl.h +++ b/source/server/api_listener_impl.h @@ -1,6 +1,7 @@ #pragma once #include +#include #include "envoy/config/core/v3/base.pb.h" #include "envoy/config/listener/v3/listener.pb.h" @@ -144,6 +145,8 @@ class ApiListenerImplBase : public ApiListener, absl::string_view transportFailureReason() const override { return EMPTY_STRING; } bool startSecureTransport() override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; } absl::optional lastRoundTripTime() const override { return {}; }; + // ScopeTrackedObject + void dumpState(std::ostream&, int) const override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; } SyntheticReadCallbacks& parent_; Network::SocketAddressSetterSharedPtr address_provider_; diff --git a/test/common/common/BUILD b/test/common/common/BUILD index da737aa053bd5..0e573f2be0ec8 100644 --- a/test/common/common/BUILD +++ b/test/common/common/BUILD @@ -431,3 +431,12 @@ envoy_cc_test( "//test/test_common:utility_lib", ], ) + +envoy_cc_test( + name = "scope_tracked_object_stack_test", + srcs = ["scope_tracked_object_stack_test.cc"], + deps = [ + "//source/common/common:scope_tracked_object_stack", + "//source/common/common:utility_lib", + ], +) diff --git a/test/common/common/scope_tracked_object_stack_test.cc b/test/common/common/scope_tracked_object_stack_test.cc new file mode 100644 index 0000000000000..984f66f39bcb1 --- /dev/null +++ b/test/common/common/scope_tracked_object_stack_test.cc @@ -0,0 +1,30 @@ +#include + +#include "envoy/common/scope_tracker.h" + +#include "common/common/scope_tracked_object_stack.h" +#include "common/common/utility.h" + +#include "test/test_common/utility.h" + +#include "gtest/gtest.h" + +namespace Envoy { +namespace { + +TEST(OpaqueScopeTrackedObjectTest, ShouldDumpTrackedObjectsInFILO) { + std::array buffer; + OutputBufferStream ostream{buffer.data(), buffer.size()}; + MessageTrackedObject first{"first"}; + MessageTrackedObject second{"second"}; + + ScopeTrackedObjectStack encapsulated_object; + encapsulated_object.add(first); + encapsulated_object.add(second); + encapsulated_object.dumpState(ostream, 0); + + EXPECT_EQ(ostream.contents(), "secondfirst"); +} + +} // namespace +} // namespace Envoy diff --git a/test/common/config/metadata_test.cc b/test/common/config/metadata_test.cc index 9cfae351d9cda..ee3c6b273ebf4 100644 --- a/test/common/config/metadata_test.cc +++ b/test/common/config/metadata_test.cc @@ -51,64 +51,182 @@ TEST(MetadataTest, MetadataValuePath) { class TypedMetadataTest : public testing::Test { public: - TypedMetadataTest() : registered_factory_(foo_factory_) {} + TypedMetadataTest() + : registered_factory_foo_(foo_factory_), registered_factory_bar_(bar_factory_), + registered_factory_baz_(baz_factory_) {} struct Foo : public TypedMetadata::Object { Foo(std::string name) : name_(name) {} std::string name_; }; - struct Bar : public TypedMetadata::Object {}; + struct Qux : public TypedMetadata::Object {}; - class FooFactory : public TypedMetadataFactory { + class FoobarFactory : public TypedMetadataFactory { public: - std::string name() const override { return "foo"; } // Throws EnvoyException (conversion failure) if d is empty. std::unique_ptr parse(const ProtobufWkt::Struct& d) const override { if (d.fields().find("name") != d.fields().end()) { return std::make_unique(d.fields().at("name").string_value()); } - throw EnvoyException("Cannot create a Foo when metadata is empty."); + throw EnvoyException("Cannot create a Foo when Struct metadata is empty."); + } + + std::unique_ptr parse(const ProtobufWkt::Any& d) const override { + if (!(d.type_url().empty())) { + return std::make_unique(d.value()); + } + throw EnvoyException("Cannot create a Foo when Any metadata is empty."); + } + }; + + // Three testing factories with different names + class FooFactory : public FoobarFactory { + public: + std::string name() const override { return "foo"; } + }; + + class BarFactory : public FoobarFactory { + public: + std::string name() const override { return "bar"; } + }; + + class BazFactory : public FoobarFactory { + public: + std::string name() const override { return "baz"; } + using FoobarFactory::parse; + // Override Any parse() to just return nullptr. + std::unique_ptr parse(const ProtobufWkt::Any&) const override { + return nullptr; } }; protected: FooFactory foo_factory_; - Registry::InjectFactory registered_factory_; + BarFactory bar_factory_; + BazFactory baz_factory_; + Registry::InjectFactory registered_factory_foo_; + Registry::InjectFactory registered_factory_bar_; + Registry::InjectFactory registered_factory_baz_; }; -TEST_F(TypedMetadataTest, OkTest) { +// Tests data parsing and retrieving when only Struct field present in the metadata. +TEST_F(TypedMetadataTest, OkTestStruct) { envoy::config::core::v3::Metadata metadata; (*metadata.mutable_filter_metadata())[foo_factory_.name()] = - MessageUtil::keyValueStruct("name", "foo"); + MessageUtil::keyValueStruct("name", "garply"); TypedMetadataImpl typed(metadata); EXPECT_NE(nullptr, typed.get(foo_factory_.name())); - EXPECT_EQ("foo", typed.get(foo_factory_.name())->name_); + EXPECT_EQ("garply", typed.get(foo_factory_.name())->name_); // A duck is a duck. - EXPECT_EQ(nullptr, typed.get(foo_factory_.name())); + EXPECT_EQ(nullptr, typed.get(foo_factory_.name())); +} + +// Tests data parsing and retrieving when only Any field present in the metadata. +TEST_F(TypedMetadataTest, OkTestAny) { + envoy::config::core::v3::Metadata metadata; + ProtobufWkt::Any any; + any.set_type_url("type.googleapis.com/waldo"); + any.set_value("fred"); + (*metadata.mutable_typed_filter_metadata())[bar_factory_.name()] = any; + TypedMetadataImpl typed(metadata); + EXPECT_NE(nullptr, typed.get(bar_factory_.name())); + EXPECT_EQ("fred", typed.get(bar_factory_.name())->name_); +} + +// Tests data parsing and retrieving when only Any field present in the metadata, +// also Any data parsing method just return nullptr. +TEST_F(TypedMetadataTest, OkTestAnyParseReturnNullptr) { + envoy::config::core::v3::Metadata metadata; + ProtobufWkt::Any any; + any.set_type_url("type.googleapis.com/waldo"); + any.set_value("fred"); + (*metadata.mutable_typed_filter_metadata())[baz_factory_.name()] = any; + TypedMetadataImpl typed(metadata); + EXPECT_EQ(nullptr, typed.get(baz_factory_.name())); +} + +// Tests data parsing and retrieving when both Struct and Any field +// present in the metadata, and in the same factory. +TEST_F(TypedMetadataTest, OkTestBothSameFactory) { + envoy::config::core::v3::Metadata metadata; + (*metadata.mutable_filter_metadata())[foo_factory_.name()] = + MessageUtil::keyValueStruct("name", "garply"); + ProtobufWkt::Any any; + any.set_type_url("type.googleapis.com/waldo"); + any.set_value("fred"); + (*metadata.mutable_typed_filter_metadata())[foo_factory_.name()] = any; + TypedMetadataImpl typed(metadata); + EXPECT_NE(nullptr, typed.get(foo_factory_.name())); + // If metadata has both Struct and Any field in same factory, + // only Any field is populated. + EXPECT_EQ("fred", typed.get(foo_factory_.name())->name_); +} + +// Tests data parsing and retrieving when both Struct and Any field +// present in the metadata, but in different factory. +TEST_F(TypedMetadataTest, OkTestBothDifferentFactory) { + envoy::config::core::v3::Metadata metadata; + (*metadata.mutable_filter_metadata())[foo_factory_.name()] = + MessageUtil::keyValueStruct("name", "garply"); + ProtobufWkt::Any any; + any.set_type_url("type.googleapis.com/waldo"); + any.set_value("fred"); + (*metadata.mutable_typed_filter_metadata())[bar_factory_.name()] = any; + + TypedMetadataImpl typed(metadata); + // If metadata has both Struct and Any field in different factory, + // both fields are populated. + EXPECT_NE(nullptr, typed.get(foo_factory_.name())); + EXPECT_EQ("garply", typed.get(foo_factory_.name())->name_); + + EXPECT_NE(nullptr, typed.get(bar_factory_.name())); + EXPECT_EQ("fred", typed.get(bar_factory_.name())->name_); +} + +// Tests data parsing and retrieving when both Struct and Any field +// present in the metadata, and in the same factory, but with the case +// that Any field parse() method returns nullptr. +TEST_F(TypedMetadataTest, OkTestBothSameFactoryAnyParseReturnNullptr) { + envoy::config::core::v3::Metadata metadata; + (*metadata.mutable_filter_metadata())[baz_factory_.name()] = + MessageUtil::keyValueStruct("name", "garply"); + ProtobufWkt::Any any; + any.set_type_url("type.googleapis.com/waldo"); + any.set_value("fred"); + (*metadata.mutable_typed_filter_metadata())[baz_factory_.name()] = any; + + // Since Any Parse() method in BazFactory just return nullptr, + // Struct data is populated. + TypedMetadataImpl typed(metadata); + EXPECT_NE(nullptr, typed.get(baz_factory_.name())); + EXPECT_EQ("garply", typed.get(baz_factory_.name())->name_); } +// Tests data parsing and retrieving when no data present in the metadata. TEST_F(TypedMetadataTest, NoMetadataTest) { envoy::config::core::v3::Metadata metadata; TypedMetadataImpl typed(metadata); + // metadata is empty, nothing is populated. EXPECT_EQ(nullptr, typed.get(foo_factory_.name())); } -TEST_F(TypedMetadataTest, MetadataRefreshTest) { +// Tests data parsing and retrieving when Struct metadata updates. +TEST_F(TypedMetadataTest, StructMetadataRefreshTest) { envoy::config::core::v3::Metadata metadata; (*metadata.mutable_filter_metadata())[foo_factory_.name()] = - MessageUtil::keyValueStruct("name", "foo"); + MessageUtil::keyValueStruct("name", "garply"); TypedMetadataImpl typed(metadata); EXPECT_NE(nullptr, typed.get(foo_factory_.name())); - EXPECT_EQ("foo", typed.get(foo_factory_.name())->name_); + EXPECT_EQ("garply", typed.get(foo_factory_.name())->name_); // Updated. (*metadata.mutable_filter_metadata())[foo_factory_.name()] = - MessageUtil::keyValueStruct("name", "bar"); + MessageUtil::keyValueStruct("name", "plugh"); TypedMetadataImpl typed2(metadata); EXPECT_NE(nullptr, typed2.get(foo_factory_.name())); - EXPECT_EQ("bar", typed2.get(foo_factory_.name())->name_); + EXPECT_EQ("plugh", typed2.get(foo_factory_.name())->name_); // Deleted. (*metadata.mutable_filter_metadata()).erase(foo_factory_.name()); @@ -116,11 +234,47 @@ TEST_F(TypedMetadataTest, MetadataRefreshTest) { EXPECT_EQ(nullptr, typed3.get(foo_factory_.name())); } -TEST_F(TypedMetadataTest, InvalidMetadataTest) { +// Tests data parsing and retrieving when Any metadata updates. +TEST_F(TypedMetadataTest, AnyMetadataRefreshTest) { + envoy::config::core::v3::Metadata metadata; + ProtobufWkt::Any any; + any.set_type_url("type.googleapis.com/waldo"); + any.set_value("fred"); + (*metadata.mutable_typed_filter_metadata())[bar_factory_.name()] = any; + TypedMetadataImpl typed(metadata); + EXPECT_NE(nullptr, typed.get(bar_factory_.name())); + EXPECT_EQ("fred", typed.get(bar_factory_.name())->name_); + + // Updated. + any.set_type_url("type.googleapis.com/xyzzy"); + any.set_value("thud"); + (*metadata.mutable_typed_filter_metadata())[bar_factory_.name()] = any; + TypedMetadataImpl typed2(metadata); + EXPECT_NE(nullptr, typed2.get(bar_factory_.name())); + EXPECT_EQ("thud", typed2.get(bar_factory_.name())->name_); + + // Deleted. + (*metadata.mutable_typed_filter_metadata()).erase(bar_factory_.name()); + TypedMetadataImpl typed3(metadata); + EXPECT_EQ(nullptr, typed3.get(bar_factory_.name())); +} + +// Tests empty Struct metadata parsing case. +TEST_F(TypedMetadataTest, InvalidStructMetadataTest) { envoy::config::core::v3::Metadata metadata; (*metadata.mutable_filter_metadata())[foo_factory_.name()] = ProtobufWkt::Struct(); EXPECT_THROW_WITH_MESSAGE(TypedMetadataImpl typed(metadata), - Envoy::EnvoyException, "Cannot create a Foo when metadata is empty."); + Envoy::EnvoyException, + "Cannot create a Foo when Struct metadata is empty."); +} + +// Tests empty Any metadata parsing case. +TEST_F(TypedMetadataTest, InvalidAnyMetadataTest) { + envoy::config::core::v3::Metadata metadata; + (*metadata.mutable_typed_filter_metadata())[bar_factory_.name()] = ProtobufWkt::Any(); + EXPECT_THROW_WITH_MESSAGE(TypedMetadataImpl typed(metadata), + Envoy::EnvoyException, + "Cannot create a Foo when Any metadata is empty."); } } // namespace diff --git a/test/common/event/dispatcher_impl_test.cc b/test/common/event/dispatcher_impl_test.cc index 4a0ed4e9ef32b..4194c46b6b675 100644 --- a/test/common/event/dispatcher_impl_test.cc +++ b/test/common/event/dispatcher_impl_test.cc @@ -663,15 +663,6 @@ TEST_F(DispatcherImplTest, ShouldDumpNothingIfNoTrackedObjects) { EXPECT_EQ(ostream.contents(), ""); } -class MessageTrackedObject : public ScopeTrackedObject { -public: - MessageTrackedObject(absl::string_view sv) : sv_(sv) {} - void dumpState(std::ostream& os, int /*indent_level*/) const override { os << sv_; } - -private: - absl::string_view sv_; -}; - TEST_F(DispatcherImplTest, ShouldDumpTrackedObjectsInFILO) { std::array buffer; OutputBufferStream ostream{buffer.data(), buffer.size()}; @@ -702,6 +693,35 @@ TEST_F(DispatcherImplTest, ShouldDumpTrackedObjectsInFILO) { EXPECT_EQ(ostream.contents(), "thirdsecondfirst"); } +TEST_F(DispatcherImplTest, TracksIfTrackedObjectStackEmpty) { + // Post on the dispatcher thread. + dispatcher_->post([this]() { + Thread::LockGuard lock(mu_); + + // Initially should be empty + ASSERT_TRUE(dispatcher_->trackedObjectStackIsEmpty()); + + // Add Tracked Object + { + MessageTrackedObject first{"first"}; + ScopeTrackerScopeState first_state{&first, *dispatcher_}; + + EXPECT_FALSE(dispatcher_->trackedObjectStackIsEmpty()); + } + + // Should be empty now + EXPECT_TRUE(dispatcher_->trackedObjectStackIsEmpty()); + + work_finished_ = true; + cv_.notifyOne(); + }); + + Thread::LockGuard lock(mu_); + while (!work_finished_) { + cv_.wait(mu_); + } +} + class TestFatalAction : public Server::Configuration::FatalAction { public: void run(absl::Span /*tracked_objects*/) override { diff --git a/test/common/http/BUILD b/test/common/http/BUILD index 0063b157ced8f..ee91fd614c683 100644 --- a/test/common/http/BUILD +++ b/test/common/http/BUILD @@ -463,6 +463,19 @@ envoy_cc_test( ], ) +envoy_cc_test( + name = "alternate_protocols_cache_manager_test", + srcs = ["alternate_protocols_cache_manager_test.cc"], + deps = [ + ":common_lib", + "//source/common/http:alternate_protocols_cache", + "//source/common/singleton:manager_impl_lib", + "//test/mocks:common_lib", + "//test/mocks/thread_local:thread_local_mocks", + "//test/test_common:simulated_time_system_lib", + ], +) + envoy_proto_library( name = "path_utility_fuzz_proto", srcs = ["path_utility_fuzz.proto"], diff --git a/test/common/http/alternate_protocols_cache_manager_test.cc b/test/common/http/alternate_protocols_cache_manager_test.cc new file mode 100644 index 0000000000000..7eb16ef48c8df --- /dev/null +++ b/test/common/http/alternate_protocols_cache_manager_test.cc @@ -0,0 +1,66 @@ +#include "common/http/alternate_protocols_cache_manager_impl.h" +#include "common/singleton/manager_impl.h" + +#include "test/mocks/thread_local/mocks.h" +#include "test/test_common/simulated_time_system.h" + +#include "gtest/gtest.h" + +namespace Envoy { +namespace Http { + +namespace { +class AlternateProtocolsCacheManagerTest : public testing::Test, + public Event::TestUsingSimulatedTime { +public: + AlternateProtocolsCacheManagerTest() + : factory_(singleton_manager_, simTime(), tls_), manager_(factory_.get()) { + options1_.set_name(name1_); + options1_.mutable_max_entries()->set_value(max_entries1_); + + options2_.set_name(name2_); + options2_.mutable_max_entries()->set_value(max_entries2_); + } + + Singleton::ManagerImpl singleton_manager_{Thread::threadFactoryForTest()}; + testing::NiceMock tls_; + Http::AlternateProtocolsCacheManagerFactoryImpl factory_; + AlternateProtocolsCacheManagerSharedPtr manager_; + const std::string name1_ = "name1"; + const std::string name2_ = "name2"; + const int max_entries1_ = 10; + const int max_entries2_ = 20; + + envoy::config::core::v3::AlternateProtocolsCacheOptions options1_; + envoy::config::core::v3::AlternateProtocolsCacheOptions options2_; +}; + +TEST_F(AlternateProtocolsCacheManagerTest, FactoryGet) { + EXPECT_NE(nullptr, manager_); + EXPECT_EQ(manager_, factory_.get()); +} + +TEST_F(AlternateProtocolsCacheManagerTest, GetCache) { + AlternateProtocolsCacheSharedPtr cache = manager_->getCache(options1_); + EXPECT_NE(nullptr, cache); + EXPECT_EQ(cache, manager_->getCache(options1_)); +} + +TEST_F(AlternateProtocolsCacheManagerTest, GetCacheForDifferentOptions) { + AlternateProtocolsCacheSharedPtr cache1 = manager_->getCache(options1_); + AlternateProtocolsCacheSharedPtr cache2 = manager_->getCache(options2_); + EXPECT_NE(nullptr, cache2); + EXPECT_NE(cache1, cache2); +} + +TEST_F(AlternateProtocolsCacheManagerTest, GetCacheForConflictingOptions) { + AlternateProtocolsCacheSharedPtr cache1 = manager_->getCache(options1_); + options2_.set_name(options1_.name()); + EXPECT_THROW_WITH_REGEX( + manager_->getCache(options2_), EnvoyException, + "options specified alternate protocols cache 'name1' with different settings.*"); +} + +} // namespace +} // namespace Http +} // namespace Envoy diff --git a/test/common/http/conn_pool_grid_test.cc b/test/common/http/conn_pool_grid_test.cc index d2cd280a3bd85..115009c37b56b 100644 --- a/test/common/http/conn_pool_grid_test.cc +++ b/test/common/http/conn_pool_grid_test.cc @@ -1,3 +1,5 @@ +#include "envoy/http/alternate_protocols_cache.h" + #include "common/http/alternate_protocols_cache_impl.h" #include "common/http/conn_pool_grid.h" @@ -98,30 +100,31 @@ class ConnectivityGridTestBase : public Event::TestUsingSimulatedTime, public te public: ConnectivityGridTestBase(bool use_alternate_protocols) : options_({Http::Protocol::Http11, Http::Protocol::Http2, Http::Protocol::Http3}), - alternate_protocols_(simTime()), + alternate_protocols_(maybeCreateAlternateProtocolsCacheImpl(use_alternate_protocols)), grid_(dispatcher_, random_, Upstream::makeTestHost(cluster_, "hostname", "tcp://127.0.0.1:9000", simTime()), Upstream::ResourcePriority::Default, socket_options_, transport_socket_options_, - state_, simTime(), maybeCreateAlternateProtocolsCacheImpl(use_alternate_protocols), - std::chrono::milliseconds(300), options_), + state_, simTime(), alternate_protocols_, std::chrono::milliseconds(300), options_), host_(grid_.host()) { grid_.info_ = &info_; grid_.encoder_ = &encoder_; } - OptRef + AlternateProtocolsCacheSharedPtr maybeCreateAlternateProtocolsCacheImpl(bool use_alternate_protocols) { + AlternateProtocolsCacheSharedPtr cache; if (!use_alternate_protocols) { - return makeOptRefFromPtr(nullptr); + return nullptr; } - return alternate_protocols_; + return std::make_shared(simTime()); } void addHttp3AlternateProtocol() { AlternateProtocolsCacheImpl::Origin origin("https", "hostname", 9000); const std::vector protocols = { {"h3-29", "", origin.port_}}; - alternate_protocols_.setAlternatives(origin, protocols, simTime().monotonicTime() + Seconds(5)); + alternate_protocols_->setAlternatives(origin, protocols, + simTime().monotonicTime() + Seconds(5)); } const Network::ConnectionSocket::OptionsSharedPtr socket_options_; @@ -131,7 +134,7 @@ class ConnectivityGridTestBase : public Event::TestUsingSimulatedTime, public te NiceMock dispatcher_; std::shared_ptr cluster_{new NiceMock()}; NiceMock random_; - AlternateProtocolsCacheImpl alternate_protocols_; + AlternateProtocolsCacheSharedPtr alternate_protocols_; ConnectivityGridForTest grid_; Upstream::HostDescriptionConstSharedPtr host_; @@ -457,7 +460,7 @@ TEST_F(ConnectivityGridTest, NoDrainOnTeardown) { } // Test that when HTTP/3 is broken then the HTTP/3 pool is skipped. -TEST_F(ConnectivityGridTest, SuccessAfterBroken) { +TEST_F(ConnectivityGridWithAlternateProtocolsCacheImplTest, SuccessAfterBroken) { addHttp3AlternateProtocol(); grid_.markHttp3Broken(); EXPECT_EQ(grid_.first(), nullptr); @@ -511,7 +514,7 @@ TEST_F(ConnectivityGridWithAlternateProtocolsCacheImplTest, SuccessWithExpiredHt AlternateProtocolsCacheImpl::Origin origin("https", "hostname", 9000); const std::vector protocols = { {"h3-29", "", origin.port_}}; - alternate_protocols_.setAlternatives(origin, protocols, simTime().monotonicTime() + Seconds(5)); + alternate_protocols_->setAlternatives(origin, protocols, simTime().monotonicTime() + Seconds(5)); simTime().setMonotonicTime(simTime().monotonicTime() + Seconds(10)); EXPECT_EQ(grid_.first(), nullptr); @@ -534,7 +537,7 @@ TEST_F(ConnectivityGridWithAlternateProtocolsCacheImplTest, SuccessWithoutHttp3N AlternateProtocolsCacheImpl::Origin origin("https", "hostname", 9000); const std::vector protocols = { {"h3-29", "otherhostname", origin.port_}}; - alternate_protocols_.setAlternatives(origin, protocols, simTime().monotonicTime() + Seconds(5)); + alternate_protocols_->setAlternatives(origin, protocols, simTime().monotonicTime() + Seconds(5)); EXPECT_EQ(grid_.first(), nullptr); @@ -555,7 +558,7 @@ TEST_F(ConnectivityGridWithAlternateProtocolsCacheImplTest, SuccessWithoutHttp3N AlternateProtocolsCacheImpl::Origin origin("https", "hostname", 9000); const std::vector protocols = { {"h3-29", "", origin.port_ + 1}}; - alternate_protocols_.setAlternatives(origin, protocols, simTime().monotonicTime() + Seconds(5)); + alternate_protocols_->setAlternatives(origin, protocols, simTime().monotonicTime() + Seconds(5)); EXPECT_EQ(grid_.first(), nullptr); @@ -575,7 +578,7 @@ TEST_F(ConnectivityGridWithAlternateProtocolsCacheImplTest, SuccessWithoutHttp3N AlternateProtocolsCacheImpl::Origin origin("https", "hostname", 9000); const std::vector protocols = { {"http/2", "", origin.port_}}; - alternate_protocols_.setAlternatives(origin, protocols, simTime().monotonicTime() + Seconds(5)); + alternate_protocols_->setAlternatives(origin, protocols, simTime().monotonicTime() + Seconds(5)); EXPECT_EQ(grid_.first(), nullptr); diff --git a/test/common/http/filter_manager_test.cc b/test/common/http/filter_manager_test.cc index daf8a95a12ea5..b90a099367efb 100644 --- a/test/common/http/filter_manager_test.cc +++ b/test/common/http/filter_manager_test.cc @@ -536,6 +536,7 @@ TEST_F(FilterManagerTest, MultipleOnLocalReply) { EXPECT_CALL(*decoder_filter, onLocalReply(_)); EXPECT_CALL(*stream_filter, onLocalReply(_)); EXPECT_CALL(*encoder_filter, onLocalReply(_)); + EXPECT_CALL(dispatcher_, trackedObjectStackIsEmpty()); decoder_filter->callbacks_->sendLocalReply(Code::InternalServerError, "body", nullptr, absl::nullopt, "details"); diff --git a/test/common/http/http1/codec_impl_test.cc b/test/common/http/http1/codec_impl_test.cc index a05c13d82a0e6..5131df3481254 100644 --- a/test/common/http/http1/codec_impl_test.cc +++ b/test/common/http/http1/codec_impl_test.cc @@ -158,7 +158,7 @@ void Http1ServerConnectionImplTest::expect400(Protocol p, bool allow_absolute_ur return decoder; })); - EXPECT_CALL(decoder, sendLocalReply(_, Http::Code::BadRequest, "Bad Request", _, _, _)); + EXPECT_CALL(decoder, sendLocalReply(Http::Code::BadRequest, "Bad Request", _, _, _)); auto status = codec_->dispatch(buffer); EXPECT_TRUE(isCodecProtocolError(status)); EXPECT_EQ(p, codec_->protocol()); @@ -258,7 +258,7 @@ void Http1ServerConnectionImplTest::testTrailersExceedLimit(std::string trailer_ EXPECT_TRUE(status.ok()); buffer = Buffer::OwnedImpl(trailer_string); if (enable_trailers) { - EXPECT_CALL(decoder, sendLocalReply(_, _, _, _, _, _)); + EXPECT_CALL(decoder, sendLocalReply(_, _, _, _, _)); status = codec_->dispatch(buffer); EXPECT_TRUE(isCodecProtocolError(status)); EXPECT_EQ(status.message(), error_message); @@ -287,7 +287,7 @@ void Http1ServerConnectionImplTest::testRequestHeadersExceedLimit(std::string he auto status = codec_->dispatch(buffer); EXPECT_TRUE(status.ok()); buffer = Buffer::OwnedImpl(header_string + "\r\n"); - EXPECT_CALL(decoder, sendLocalReply(_, _, _, _, _, _)); + EXPECT_CALL(decoder, sendLocalReply(_, _, _, _, _)); status = codec_->dispatch(buffer); EXPECT_TRUE(isCodecProtocolError(status)); EXPECT_EQ(status.message(), error_message); @@ -344,7 +344,7 @@ void Http1ServerConnectionImplTest::testServerAllowChunkedContentLength(uint32_t } else { EXPECT_CALL(decoder, decodeHeaders_(_, _)).Times(0); EXPECT_CALL(decoder, decodeData(_, _)).Times(0); - EXPECT_CALL(decoder, sendLocalReply(false, Http::Code::BadRequest, "Bad Request", _, _, _)); + EXPECT_CALL(decoder, sendLocalReply(Http::Code::BadRequest, "Bad Request", _, _, _)); } Buffer::OwnedImpl buffer( @@ -400,7 +400,7 @@ TEST_F(Http1ServerConnectionImplTest, IdentityEncodingNoChunked) { EXPECT_CALL(callbacks_, newStream(_, _)).WillOnce(ReturnRef(decoder)); Buffer::OwnedImpl buffer("GET / HTTP/1.1\r\ntransfer-encoding: identity\r\n\r\n"); - EXPECT_CALL(decoder, sendLocalReply(_, _, _, _, _, _)); + EXPECT_CALL(decoder, sendLocalReply(_, _, _, _, _)); auto status = codec_->dispatch(buffer); EXPECT_TRUE(isCodecProtocolError(status)); EXPECT_EQ(status.message(), "http/1.1 protocol error: unsupported transfer encoding"); @@ -415,7 +415,7 @@ TEST_F(Http1ServerConnectionImplTest, UnsupportedEncoding) { EXPECT_CALL(callbacks_, newStream(_, _)).WillOnce(ReturnRef(decoder)); Buffer::OwnedImpl buffer("GET / HTTP/1.1\r\ntransfer-encoding: gzip\r\n\r\n"); - EXPECT_CALL(decoder, sendLocalReply(_, _, _, _, _, _)); + EXPECT_CALL(decoder, sendLocalReply(_, _, _, _, _)); auto status = codec_->dispatch(buffer); EXPECT_TRUE(isCodecProtocolError(status)); EXPECT_EQ(status.message(), "http/1.1 protocol error: unsupported transfer encoding"); @@ -623,7 +623,7 @@ TEST_F(Http1ServerConnectionImplTest, InvalidChunkHeader) { "6\r\nHello \r\n" "invalid\r\nWorl"); - EXPECT_CALL(decoder, sendLocalReply(_, _, _, _, _, _)); + EXPECT_CALL(decoder, sendLocalReply(_, _, _, _, _)); auto status = codec_->dispatch(buffer); EXPECT_TRUE(isCodecProtocolError(status)); EXPECT_EQ(status.message(), "http/1.1 protocol error: HPE_INVALID_CHUNK_SIZE"); @@ -640,7 +640,7 @@ TEST_F(Http1ServerConnectionImplTest, IdentityAndChunkedBody) { Buffer::OwnedImpl buffer("POST / HTTP/1.1\r\ntransfer-encoding: " "identity,chunked\r\n\r\nb\r\nHello World\r\n0\r\n\r\n"); - EXPECT_CALL(decoder, sendLocalReply(_, _, _, _, _, _)); + EXPECT_CALL(decoder, sendLocalReply(_, _, _, _, _)); auto status = codec_->dispatch(buffer); EXPECT_TRUE(isCodecProtocolError(status)); EXPECT_EQ(status.message(), "http/1.1 protocol error: unsupported transfer encoding"); @@ -919,7 +919,7 @@ TEST_F(Http1ServerConnectionImplTest, Http11InvalidTrailerPost) { "body\r\n0\r\n" "badtrailer\r\n\r\n"); - EXPECT_CALL(decoder, sendLocalReply(_, Http::Code::BadRequest, "Bad Request", _, _, _)); + EXPECT_CALL(decoder, sendLocalReply(Http::Code::BadRequest, "Bad Request", _, _, _)); auto status = codec_->dispatch(buffer); EXPECT_TRUE(isCodecProtocolError(status)); } @@ -1003,7 +1003,7 @@ TEST_F(Http1ServerConnectionImplTest, BadRequestNoStream) { return decoder; })); // Check that before any headers are parsed, requests do not look like HEAD or gRPC requests. - EXPECT_CALL(decoder, sendLocalReply(false, _, _, _, _, _)); + EXPECT_CALL(decoder, sendLocalReply(_, _, _, _, _)); Buffer::OwnedImpl buffer("bad"); auto status = codec_->dispatch(buffer); @@ -1019,7 +1019,7 @@ TEST_F(Http1ServerConnectionImplTest, RejectInvalidMethod) { EXPECT_CALL(callbacks_, newStream(_, _)).WillOnce(ReturnRef(decoder)); Buffer::OwnedImpl buffer("BAD / HTTP/1.1\r\nHost: foo\r\n"); - EXPECT_CALL(decoder, sendLocalReply(_, _, _, _, _, _)); + EXPECT_CALL(decoder, sendLocalReply(_, _, _, _, _)); auto status = codec_->dispatch(buffer); EXPECT_TRUE(isCodecProtocolError(status)); } @@ -1035,7 +1035,7 @@ TEST_F(Http1ServerConnectionImplTest, BadRequestStartedStream) { EXPECT_TRUE(status.ok()); Buffer::OwnedImpl buffer2("g"); - EXPECT_CALL(decoder, sendLocalReply(_, _, _, _, _, _)); + EXPECT_CALL(decoder, sendLocalReply(_, _, _, _, _)); status = codec_->dispatch(buffer); EXPECT_TRUE(isCodecProtocolError(status)); } @@ -1127,7 +1127,7 @@ TEST_F(Http1ServerConnectionImplTest, HeaderInvalidCharsRejection) { })); Buffer::OwnedImpl buffer( absl::StrCat("GET / HTTP/1.1\r\nHOST: h.com\r\nfoo: ", std::string(1, 3), "\r\n")); - EXPECT_CALL(decoder, sendLocalReply(_, _, _, _, _, _)); + EXPECT_CALL(decoder, sendLocalReply(_, _, _, _, _)); auto status = codec_->dispatch(buffer); EXPECT_TRUE(isCodecProtocolError(status)); EXPECT_EQ(status.message(), "http/1.1 protocol error: header value contains invalid chars"); @@ -1196,7 +1196,7 @@ TEST_F(Http1ServerConnectionImplTest, HeaderNameWithUnderscoreCauseRequestReject })); Buffer::OwnedImpl buffer(absl::StrCat("GET / HTTP/1.1\r\nHOST: h.com\r\nfoo_bar: bar\r\n\r\n")); - EXPECT_CALL(decoder, sendLocalReply(_, _, _, _, _, _)); + EXPECT_CALL(decoder, sendLocalReply(_, _, _, _, _)); auto status = codec_->dispatch(buffer); EXPECT_TRUE(isCodecProtocolError(status)); EXPECT_EQ(status.message(), "http/1.1 protocol error: header name contains underscores"); @@ -1217,7 +1217,7 @@ TEST_F(Http1ServerConnectionImplTest, HeaderInvalidAuthority) { return decoder; })); Buffer::OwnedImpl buffer(absl::StrCat("GET / HTTP/1.1\r\nHOST: h.\"com\r\n\r\n")); - EXPECT_CALL(decoder, sendLocalReply(_, _, _, _, _, _)); + EXPECT_CALL(decoder, sendLocalReply(_, _, _, _, _)); auto status = codec_->dispatch(buffer); EXPECT_TRUE(isCodecProtocolError(status)); EXPECT_EQ(status.message(), @@ -1240,7 +1240,7 @@ TEST_F(Http1ServerConnectionImplTest, HeaderMutateEmbeddedNul) { Buffer::OwnedImpl buffer( absl::StrCat(example_input.substr(0, n), std::string(1, '\0'), example_input.substr(n))); - EXPECT_CALL(decoder, sendLocalReply(_, _, _, _, _, _)); + EXPECT_CALL(decoder, sendLocalReply(_, _, _, _, _)); auto status = codec_->dispatch(buffer); EXPECT_FALSE(status.ok()); EXPECT_TRUE(isCodecProtocolError(status)); @@ -1847,7 +1847,7 @@ TEST_F(Http1ServerConnectionImplTest, ConnectRequestAbsoluteURLNotallowed) { EXPECT_CALL(callbacks_, newStream(_, _)).WillOnce(ReturnRef(decoder)); Buffer::OwnedImpl buffer("CONNECT http://host:80 HTTP/1.1\r\n\r\n"); - EXPECT_CALL(decoder, sendLocalReply(_, _, _, _, _, _)); + EXPECT_CALL(decoder, sendLocalReply(_, _, _, _, _)); auto status = codec_->dispatch(buffer); EXPECT_TRUE(isCodecProtocolError(status)); } @@ -1876,7 +1876,7 @@ TEST_F(Http1ServerConnectionImplTest, ConnectRequestWithTEChunked) { // Per https://tools.ietf.org/html/rfc7231#section-4.3.6 CONNECT with body has no defined // semantics: Envoy will reject chunked CONNECT requests. - EXPECT_CALL(decoder, sendLocalReply(_, _, _, _, _, _)); + EXPECT_CALL(decoder, sendLocalReply(_, _, _, _, _)); Buffer::OwnedImpl buffer( "CONNECT host:80 HTTP/1.1\r\ntransfer-encoding: chunked\r\n\r\n12345abcd"); auto status = codec_->dispatch(buffer); @@ -1894,7 +1894,7 @@ TEST_F(Http1ServerConnectionImplTest, ConnectRequestWithNonZeroContentLength) { // Make sure we avoid the deferred_end_stream_headers_ optimization for // requests-with-no-body. Buffer::OwnedImpl buffer("CONNECT host:80 HTTP/1.1\r\ncontent-length: 1\r\n\r\nabcd"); - EXPECT_CALL(decoder, sendLocalReply(_, _, _, _, _, _)); + EXPECT_CALL(decoder, sendLocalReply(_, _, _, _, _)); auto status = codec_->dispatch(buffer); EXPECT_TRUE(isCodecProtocolError(status)); EXPECT_EQ(status.message(), "http/1.1 protocol error: unsupported content length"); @@ -2902,7 +2902,7 @@ TEST_F(Http1ServerConnectionImplTest, LargeRequestHeadersSplitRejected) { } // the 60th 1kb header should induce overflow buffer = Buffer::OwnedImpl(fmt::format("big: {}\r\n", long_string)); - EXPECT_CALL(decoder, sendLocalReply(_, _, _, _, _, _)); + EXPECT_CALL(decoder, sendLocalReply(_, _, _, _, _)); status = codec_->dispatch(buffer); EXPECT_TRUE(isCodecProtocolError(status)); EXPECT_EQ(status.message(), "headers size exceeds limit"); @@ -2932,7 +2932,7 @@ TEST_F(Http1ServerConnectionImplTest, LargeRequestHeadersSplitRejectedMaxConfigu } // the 128th 64kb header should induce overflow buffer = Buffer::OwnedImpl(fmt::format("big: {}\r\n", long_string)); - EXPECT_CALL(decoder, sendLocalReply(_, _, _, _, _, _)); + EXPECT_CALL(decoder, sendLocalReply(_, _, _, _, _)); status = codec_->dispatch(buffer); EXPECT_TRUE(isCodecProtocolError(status)); EXPECT_EQ(status.message(), "headers size exceeds limit"); @@ -2962,7 +2962,7 @@ TEST_F(Http1ServerConnectionImplTest, ManyRequestHeadersSplitRejected) { // The final 101th header should induce overflow. buffer = Buffer::OwnedImpl("header101:\r\n\r\n"); - EXPECT_CALL(decoder, sendLocalReply(_, _, _, _, _, _)); + EXPECT_CALL(decoder, sendLocalReply(_, _, _, _, _)); status = codec_->dispatch(buffer); EXPECT_TRUE(isCodecProtocolError(status)); EXPECT_EQ(status.message(), "headers count exceeds limit"); diff --git a/test/common/quic/test_utils.h b/test/common/quic/test_utils.h index 987b5af219a1d..0c54ed2eb4d8a 100644 --- a/test/common/quic/test_utils.h +++ b/test/common/quic/test_utils.h @@ -63,6 +63,7 @@ class MockEnvoyQuicServerConnection : public EnvoyQuicServerConnection { MOCK_METHOD(void, SendConnectionClosePacket, (quic::QuicErrorCode, quic::QuicIetfTransportErrorCodes, const std::string&)); MOCK_METHOD(bool, SendControlFrame, (const quic::QuicFrame& frame)); + MOCK_METHOD(void, dumpState, (std::ostream&, int), (const)); }; class MockEnvoyQuicSession : public quic::QuicSpdySession, public QuicFilterManagerConnectionImpl { @@ -100,6 +101,7 @@ class MockEnvoyQuicSession : public quic::QuicSpdySession, public QuicFilterMana quic::QuicStreamOffset bytes_written)); MOCK_METHOD(void, MaybeSendStopSendingFrame, (quic::QuicStreamId id, quic::QuicRstStreamErrorCode error)); + MOCK_METHOD(void, dumpState, (std::ostream&, int), (const)); absl::string_view requestedServerName() const override { return {GetCryptoStream()->crypto_negotiated_params().sni}; @@ -154,6 +156,7 @@ class MockEnvoyQuicClientSession : public quic::QuicSpdyClientSession, quic::StreamSendingState state, quic::TransmissionType type, absl::optional level)); MOCK_METHOD(bool, ShouldYield, (quic::QuicStreamId id)); + MOCK_METHOD(void, dumpState, (std::ostream&, int), (const)); absl::string_view requestedServerName() const override { return {GetCryptoStream()->crypto_negotiated_params().sni}; diff --git a/test/common/router/config_impl_headermap_benchmark_test.cc b/test/common/router/config_impl_headermap_benchmark_test.cc index 80fa3ae007efc..2e8bd3fcd2fab 100644 --- a/test/common/router/config_impl_headermap_benchmark_test.cc +++ b/test/common/router/config_impl_headermap_benchmark_test.cc @@ -56,8 +56,8 @@ static void manyCountryRoutesLongHeaders(benchmark::State& state) { Api::ApiPtr api(Api::createApiForTest()); NiceMock factory_context; ON_CALL(factory_context, api()).WillByDefault(ReturnRef(*api)); - ConfigImpl config(proto_config, factory_context, ProtobufMessage::getNullValidationVisitor(), - true); + ConfigImpl config(proto_config, OptionalHttpFilters(), factory_context, + ProtobufMessage::getNullValidationVisitor(), true); const auto stream_info = NiceMock(); auto req_headers = Http::TestRequestHeaderMapImpl{{":authority", "www.lyft.com"}, diff --git a/test/common/router/config_impl_speed_test.cc b/test/common/router/config_impl_speed_test.cc index 704aac46644b3..0405138615654 100644 --- a/test/common/router/config_impl_speed_test.cc +++ b/test/common/router/config_impl_speed_test.cc @@ -93,7 +93,7 @@ static void bmRouteTableSize(benchmark::State& state, RouteMatch::PathSpecifierC ON_CALL(factory_context, api()).WillByDefault(ReturnRef(*api)); // Create router config. - ConfigImpl config(genRouteConfig(state, match_type), factory_context, + ConfigImpl config(genRouteConfig(state, match_type), OptionalHttpFilters(), factory_context, ProtobufMessage::getNullValidationVisitor(), true); for (auto _ : state) { // NOLINT diff --git a/test/common/router/config_impl_test.cc b/test/common/router/config_impl_test.cc index c465afe150e6a..5911d90c50dd9 100644 --- a/test/common/router/config_impl_test.cc +++ b/test/common/router/config_impl_test.cc @@ -59,9 +59,10 @@ class TestConfigImpl : public ConfigImpl { public: TestConfigImpl(const envoy::config::route::v3::RouteConfiguration& config, Server::Configuration::ServerFactoryContext& factory_context, - bool validate_clusters_default) - : ConfigImpl(config, factory_context, ProtobufMessage::getNullValidationVisitor(), - validate_clusters_default), + bool validate_clusters_default, + const OptionalHttpFilters& optional_http_filters = OptionalHttpFilters()) + : ConfigImpl(config, optional_http_filters, factory_context, + ProtobufMessage::getNullValidationVisitor(), validate_clusters_default), config_(config) {} void setupRouteConfig(const Http::RequestHeaderMap& headers, uint64_t random_value) const { @@ -5020,6 +5021,11 @@ class BazFactory : public HttpRouteTypedMetadataFactory { } throw EnvoyException("Cannot create a Baz when metadata is empty."); } + + std::unique_ptr + parse(const ProtobufWkt::Any&) const override { + return nullptr; + } }; TEST_F(RouteMatcherTest, WeightedClusters) { @@ -7851,8 +7857,11 @@ class PerFilterConfigsTest : public testing::Test, public ConfigImplTestBase { << "config value does not match expected for source: " << source; } - void checkNoPerFilterConfig(const std::string& yaml) { - const TestConfigImpl config(parseRouteConfigurationFromYaml(yaml), factory_context_, true); + void + checkNoPerFilterConfig(const std::string& yaml, + const OptionalHttpFilters& optional_http_filters = OptionalHttpFilters()) { + const TestConfigImpl config(parseRouteConfigurationFromYaml(yaml), factory_context_, true, + optional_http_filters); const auto route = config.route(genHeaders("www.foo.com", "/", "GET"), 0); const auto* route_entry = route->routeEntry(); @@ -8093,6 +8102,26 @@ TEST_F(PerFilterConfigsTest, DefaultFilterImplementationAnyWithCheckPerVirtualHo "The filter test.default.filter doesn't support virtual host-specific configurations"); } +TEST_F(PerFilterConfigsTest, OptionalDefaultFilterImplementationAnyWithCheckPerVirtualHost) { + const std::string yaml = R"EOF( +virtual_hosts: + - name: bar + domains: ["*"] + routes: + - match: { prefix: "/" } + route: { cluster: baz } + typed_per_filter_config: + test.default.filter: + "@type": type.googleapis.com/google.protobuf.Struct + value: + seconds: 123 +)EOF"; + + OptionalHttpFilters optional_http_filters = {"test.default.filter"}; + factory_context_.cluster_manager_.initializeClusters({"baz"}, {}); + checkNoPerFilterConfig(yaml, optional_http_filters); +} + TEST_F(PerFilterConfigsTest, DefaultFilterImplementationAnyWithCheckPerRoute) { const std::string yaml = R"EOF( virtual_hosts: @@ -8114,6 +8143,108 @@ TEST_F(PerFilterConfigsTest, DefaultFilterImplementationAnyWithCheckPerRoute) { "The filter test.default.filter doesn't support virtual host-specific configurations"); } +TEST_F(PerFilterConfigsTest, OptionalDefaultFilterImplementationAnyWithCheckPerRoute) { + const std::string yaml = R"EOF( +virtual_hosts: + - name: bar + domains: ["*"] + routes: + - match: { prefix: "/" } + route: { cluster: baz } + typed_per_filter_config: + test.default.filter: + "@type": type.googleapis.com/google.protobuf.Struct + value: + seconds: 123 +)EOF"; + + OptionalHttpFilters optional_http_filters = {"test.default.filter"}; + factory_context_.cluster_manager_.initializeClusters({"baz"}, {}); + checkNoPerFilterConfig(yaml, optional_http_filters); +} + +TEST_F(PerFilterConfigsTest, PerVirtualHostWithUnknownFilter) { + const std::string yaml = R"EOF( +virtual_hosts: + - name: bar + domains: ["*"] + routes: + - match: { prefix: "/" } + route: { cluster: baz } + typed_per_filter_config: + filter.unknown: + "@type": type.googleapis.com/google.protobuf.Struct + value: + seconds: 123 +)EOF"; + + EXPECT_THROW_WITH_MESSAGE( + TestConfigImpl config(parseRouteConfigurationFromYaml(yaml), factory_context_, true), + EnvoyException, "Didn't find a registered implementation for name: 'filter.unknown'"); +} + +TEST_F(PerFilterConfigsTest, PerVirtualHostWithOptionalUnknownFilter) { + const std::string yaml = R"EOF( +virtual_hosts: + - name: bar + domains: ["*"] + routes: + - match: { prefix: "/" } + route: { cluster: baz } + typed_per_filter_config: + filter.unknown: + "@type": type.googleapis.com/google.protobuf.Struct + value: + seconds: 123 +)EOF"; + + factory_context_.cluster_manager_.initializeClusters({"baz"}, {}); + OptionalHttpFilters optional_http_filters; + optional_http_filters.insert("filter.unknown"); + checkNoPerFilterConfig(yaml, optional_http_filters); +} + +TEST_F(PerFilterConfigsTest, PerRouteWithUnknownFilter) { + const std::string yaml = R"EOF( +virtual_hosts: + - name: bar + domains: ["*"] + routes: + - match: { prefix: "/" } + route: { cluster: baz } + typed_per_filter_config: + filter.unknown: + "@type": type.googleapis.com/google.protobuf.Struct + value: + seconds: 123 +)EOF"; + + EXPECT_THROW_WITH_MESSAGE( + TestConfigImpl config(parseRouteConfigurationFromYaml(yaml), factory_context_, true), + EnvoyException, "Didn't find a registered implementation for name: 'filter.unknown'"); +} + +TEST_F(PerFilterConfigsTest, PerRouteWithOptionalUnknownFilter) { + const std::string yaml = R"EOF( +virtual_hosts: + - name: bar + domains: ["*"] + routes: + - match: { prefix: "/" } + route: { cluster: baz } + typed_per_filter_config: + filter.unknown: + "@type": type.googleapis.com/google.protobuf.Struct + value: + seconds: 123 +)EOF"; + + factory_context_.cluster_manager_.initializeClusters({"baz"}, {}); + OptionalHttpFilters optional_http_filters; + optional_http_filters.insert("filter.unknown"); + checkNoPerFilterConfig(yaml, optional_http_filters); +} + TEST_F(PerFilterConfigsTest, DEPRECATED_FEATURE_TEST(RouteLocalConfig)) { TestDeprecatedV2Api _deprecated_v2_api; const std::string yaml = R"EOF( diff --git a/test/common/router/rds_impl_test.cc b/test/common/router/rds_impl_test.cc index 58b8fa83424cf..e6fba0e46f5a5 100644 --- a/test/common/router/rds_impl_test.cc +++ b/test/common/router/rds_impl_test.cc @@ -86,8 +86,8 @@ class RdsImplTest : public RdsTestBase { } ~RdsImplTest() override { server_factory_context_.thread_local_.shutdownThread(); } - void setup() { - const std::string config_yaml = R"EOF( + void setup(const std::string& override_config = "") { + std::string config_yaml = R"EOF( rds: config_source: api_config_source: @@ -103,6 +103,9 @@ stat_prefix: foo typed_config: {} )EOF"; + if (!override_config.empty()) { + config_yaml = override_config; + } EXPECT_CALL(outer_init_manager_, add(_)); rds_ = RouteConfigProviderUtil::create( parseHttpConnectionManagerFromYaml(config_yaml), server_factory_context_, @@ -142,6 +145,57 @@ stat_prefix: foo EnvoyException); } +TEST_F(RdsImplTest, RdsAndStaticWithUnknownFilterPerVirtualHostConfig) { + const std::string config_yaml = R"EOF( +route_config: + virtual_hosts: + - name: bar + domains: ["*"] + routes: + - match: { prefix: "/" } + typed_per_filter_config: + filter.unknown: + "@type": type.googleapis.com/google.protobuf.Struct + value: + seconds: 123 +codec_type: auto +stat_prefix: foo +http_filters: +- name: filter.unknown + )EOF"; + + EXPECT_THROW_WITH_MESSAGE( + RouteConfigProviderUtil::create(parseHttpConnectionManagerFromYaml(config_yaml), + server_factory_context_, validation_visitor_, + outer_init_manager_, "foo.", *route_config_provider_manager_), + EnvoyException, "Didn't find a registered implementation for name: 'filter.unknown'"); +} + +TEST_F(RdsImplTest, RdsAndStaticWithOptionalUnknownFilterPerVirtualHostConfig) { + const std::string config_yaml = R"EOF( +route_config: + virtual_hosts: + - name: bar + domains: ["*"] + routes: + - match: { prefix: "/" } + typed_per_filter_config: + filter.unknown: + "@type": type.googleapis.com/google.protobuf.Struct + value: + seconds: 123 +codec_type: auto +stat_prefix: foo +http_filters: +- name: filter.unknown + is_optional: true + )EOF"; + + RouteConfigProviderUtil::create(parseHttpConnectionManagerFromYaml(config_yaml), + server_factory_context_, validation_visitor_, outer_init_manager_, + "foo.", *route_config_provider_manager_); +} + TEST_F(RdsImplTest, DestroyDuringInitialize) { InSequence s; setup(); @@ -239,6 +293,237 @@ TEST_F(RdsImplTest, Basic) { EXPECT_EQ(2UL, scope_.counter("foo.rds.foo_route_config.config_reload").value()); } +// validate there will be exception throw when unknown factory found for per virtualhost typed +// config. +TEST_F(RdsImplTest, UnknownFacotryForPerVirtualHostTypedConfig) { + InSequence s; + + setup(); + + const std::string response1_json = R"EOF( +{ + "version_info": "1", + "resources": [ + { + "@type": "type.googleapis.com/envoy.config.route.v3.RouteConfiguration", + "name": "foo_route_config", + "virtual_hosts": [ + { + "name": "integration", + "domains": [ + "*" + ], + "routes": [ + { + "match": { + "prefix": "/foo" + }, + "route": { + "cluster_header": ":authority" + } + } + ], + "typed_per_filter_config": { + "filter.unknown": { + "@type": "type.googleapis.com/google.protobuf.Struct" + } + } + } + ] + } + ] +} +)EOF"; + auto response1 = + TestUtility::parseYaml(response1_json); + const auto decoded_resources = + TestUtility::decodeResources(response1); + + EXPECT_CALL(init_watcher_, ready()); + EXPECT_THROW_WITH_MESSAGE( + rds_callbacks_->onConfigUpdate(decoded_resources.refvec_, response1.version_info()), + EnvoyException, "Didn't find a registered implementation for name: 'filter.unknown'"); +} + +// validate the optional unknown factory will be ignored for per virtualhost typed config. +TEST_F(RdsImplTest, OptionalUnknownFacotryForPerVirtualHostTypedConfig) { + InSequence s; + const std::string config_yaml = R"EOF( +rds: + config_source: + api_config_source: + api_type: REST + cluster_names: + - foo_cluster + refresh_delay: 1s + route_config_name: foo_route_config +codec_type: auto +stat_prefix: foo +http_filters: +- name: filter.unknown + is_optional: true + )EOF"; + + setup(config_yaml); + + const std::string response1_json = R"EOF( +{ + "version_info": "1", + "resources": [ + { + "@type": "type.googleapis.com/envoy.config.route.v3.RouteConfiguration", + "name": "foo_route_config", + "virtual_hosts": [ + { + "name": "integration", + "domains": [ + "*" + ], + "routes": [ + { + "match": { + "prefix": "/foo" + }, + "route": { + "cluster_header": ":authority" + } + } + ], + "typed_per_filter_config": { + "filter.unknown": { + "@type": "type.googleapis.com/google.protobuf.Struct" + } + } + } + ] + } + ] +} +)EOF"; + auto response1 = + TestUtility::parseYaml(response1_json); + const auto decoded_resources = + TestUtility::decodeResources(response1); + + EXPECT_CALL(init_watcher_, ready()); + rds_callbacks_->onConfigUpdate(decoded_resources.refvec_, response1.version_info()); +} + +// validate there will be exception throw when unknown factory found for per route typed config. +TEST_F(RdsImplTest, UnknownFacotryForPerRouteTypedConfig) { + InSequence s; + + setup(); + + const std::string response1_json = R"EOF( +{ + "version_info": "1", + "resources": [ + { + "@type": "type.googleapis.com/envoy.config.route.v3.RouteConfiguration", + "name": "foo_route_config", + "virtual_hosts": [ + { + "name": "integration", + "domains": [ + "*" + ], + "routes": [ + { + "match": { + "prefix": "/foo" + }, + "route": { + "cluster_header": ":authority" + }, + "typed_per_filter_config": { + "filter.unknown": { + "@type": "type.googleapis.com/google.protobuf.Struct" + } + } + } + ], + } + ] + } + ] +} +)EOF"; + auto response1 = + TestUtility::parseYaml(response1_json); + const auto decoded_resources = + TestUtility::decodeResources(response1); + + EXPECT_CALL(init_watcher_, ready()); + EXPECT_THROW_WITH_MESSAGE( + rds_callbacks_->onConfigUpdate(decoded_resources.refvec_, response1.version_info()), + EnvoyException, "Didn't find a registered implementation for name: 'filter.unknown'"); +} + +// validate the optional unknown factory will be ignored for per route typed config. +TEST_F(RdsImplTest, OptionalUnknownFacotryForPerRouteTypedConfig) { + InSequence s; + const std::string config_yaml = R"EOF( +rds: + config_source: + api_config_source: + api_type: REST + cluster_names: + - foo_cluster + refresh_delay: 1s + route_config_name: foo_route_config +codec_type: auto +stat_prefix: foo +http_filters: +- name: filter.unknown + is_optional: true + )EOF"; + + setup(config_yaml); + + const std::string response1_json = R"EOF( +{ + "version_info": "1", + "resources": [ + { + "@type": "type.googleapis.com/envoy.config.route.v3.RouteConfiguration", + "name": "foo_route_config", + "virtual_hosts": [ + { + "name": "integration", + "domains": [ + "*" + ], + "routes": [ + { + "match": { + "prefix": "/foo" + }, + "route": { + "cluster_header": ":authority" + }, + "typed_per_filter_config": { + "filter.unknown": { + "@type": "type.googleapis.com/google.protobuf.Struct" + } + } + } + ], + } + ] + } + ] +} +)EOF"; + auto response1 = + TestUtility::parseYaml(response1_json); + const auto decoded_resources = + TestUtility::decodeResources(response1); + + EXPECT_CALL(init_watcher_, ready()); + rds_callbacks_->onConfigUpdate(decoded_resources.refvec_, response1.version_info()); +} + // Validate behavior when the config is delivered but it fails PGV validation. TEST_F(RdsImplTest, FailureInvalidConfig) { InSequence s; @@ -412,7 +697,7 @@ TEST_F(RdsRouteConfigSubscriptionTest, CreatesNoopInitManager) { TestUtility::parseYaml( rds_config); const auto route_config_provider = route_config_provider_manager_->createRdsRouteConfigProvider( - rds, server_factory_context_, "stat_prefix", outer_init_manager_); + rds, OptionalHttpFilters(), server_factory_context_, "stat_prefix", outer_init_manager_); RdsRouteConfigSubscription& subscription = (dynamic_cast(route_config_provider.get()))->subscription(); init_watcher_.expectReady(); // The parent_init_target_ will call once. @@ -441,7 +726,7 @@ class RouteConfigProviderManagerImplTest : public RdsTestBase { rds_.set_route_config_name("foo_route_config"); rds_.mutable_config_source()->set_path("foo_path"); provider_ = route_config_provider_manager_->createRdsRouteConfigProvider( - rds_, server_factory_context_, "foo_prefix.", outer_init_manager_); + rds_, OptionalHttpFilters(), server_factory_context_, "foo_prefix.", outer_init_manager_); rds_callbacks_ = server_factory_context_.cluster_manager_.subscription_factory_.callbacks_; } @@ -498,8 +783,8 @@ name: foo server_factory_context_.cluster_manager_.initializeClusters({"baz"}, {}); RouteConfigProviderPtr static_config = route_config_provider_manager_->createStaticRouteConfigProvider( - parseRouteConfigurationFromV3Yaml(config_yaml), server_factory_context_, - validation_visitor_); + parseRouteConfigurationFromV3Yaml(config_yaml), OptionalHttpFilters(), + server_factory_context_, validation_visitor_); message_ptr = server_factory_context_.admin_.config_tracker_.config_tracker_callbacks_["routes"](); const auto& route_config_dump2 = @@ -604,7 +889,7 @@ name: foo_route_config RouteConfigProviderSharedPtr provider2 = route_config_provider_manager_->createRdsRouteConfigProvider( - rds_, server_factory_context_, "foo_prefix", outer_init_manager_); + rds_, OptionalHttpFilters(), server_factory_context_, "foo_prefix", outer_init_manager_); // provider2 should have route config immediately after create EXPECT_TRUE(provider2->configInfo().has_value()); @@ -621,7 +906,7 @@ name: foo_route_config rds2.mutable_config_source()->set_path("bar_path"); RouteConfigProviderSharedPtr provider3 = route_config_provider_manager_->createRdsRouteConfigProvider( - rds2, server_factory_context_, "foo_prefix", outer_init_manager_); + rds2, OptionalHttpFilters(), server_factory_context_, "foo_prefix", outer_init_manager_); EXPECT_NE(provider3, provider_); server_factory_context_.cluster_manager_.subscription_factory_.callbacks_->onConfigUpdate( decoded_resources.refvec_, "provider3"); @@ -659,8 +944,8 @@ TEST_F(RouteConfigProviderManagerImplTest, SameProviderOnTwoInitManager) { Init::ManagerImpl real_init_manager("real"); RouteConfigProviderSharedPtr provider2 = - route_config_provider_manager_->createRdsRouteConfigProvider(rds_, mock_factory_context2, - "foo_prefix", real_init_manager); + route_config_provider_manager_->createRdsRouteConfigProvider( + rds_, OptionalHttpFilters(), mock_factory_context2, "foo_prefix", real_init_manager); EXPECT_FALSE(provider2->configInfo().has_value()); diff --git a/test/common/router/route_fuzz_test.cc b/test/common/router/route_fuzz_test.cc index 17e6b532e36b6..2ab76c31756b0 100644 --- a/test/common/router/route_fuzz_test.cc +++ b/test/common/router/route_fuzz_test.cc @@ -39,7 +39,7 @@ DEFINE_PROTO_FUZZER(const test::common::router::RouteTestCase& input) { static NiceMock factory_context; try { TestUtility::validate(input); - ConfigImpl config(cleanRouteConfig(input.config()), factory_context, + ConfigImpl config(cleanRouteConfig(input.config()), OptionalHttpFilters(), factory_context, ProtobufMessage::getNullValidationVisitor(), true); auto headers = Fuzz::fromHeaders(input.headers()); auto route = config.route(headers, stream_info, input.random_value()); diff --git a/test/common/router/router_ratelimit_test.cc b/test/common/router/router_ratelimit_test.cc index 43a61fdab55f4..7532eeecade74 100644 --- a/test/common/router/router_ratelimit_test.cc +++ b/test/common/router/router_ratelimit_test.cc @@ -80,8 +80,8 @@ class RateLimitConfiguration : public testing::Test { void setupTest(const std::string& yaml) { envoy::config::route::v3::RouteConfiguration route_config; TestUtility::loadFromYaml(yaml, route_config); - config_ = - std::make_unique(route_config, factory_context_, any_validation_visitor_, true); + config_ = std::make_unique(route_config, OptionalHttpFilters(), factory_context_, + any_validation_visitor_, true); stream_info_.downstream_address_provider_->setRemoteAddress(default_remote_address_); } diff --git a/test/common/router/scoped_rds_test.cc b/test/common/router/scoped_rds_test.cc index c96a9c4179342..db6a538785872 100644 --- a/test/common/router/scoped_rds_test.cc +++ b/test/common/router/scoped_rds_test.cc @@ -111,7 +111,7 @@ class ScopedRoutesTestBase : public testing::Test { class ScopedRdsTest : public ScopedRoutesTestBase { protected: - void setup() { + void setup(const OptionalHttpFilters optional_http_filters = OptionalHttpFilters()) { ON_CALL(server_factory_context_.cluster_manager_, adsMux()) .WillByDefault(Return(std::make_shared<::Envoy::Config::NullGrpcMuxImpl>())); @@ -177,9 +177,9 @@ name: foo_scoped_routes TestUtility::loadFromYaml(config_yaml, scoped_routes_config); provider_ = config_provider_manager_->createXdsConfigProvider( scoped_routes_config.scoped_rds(), server_factory_context_, context_init_manager_, "foo.", - ScopedRoutesConfigProviderManagerOptArg(scoped_routes_config.name(), - scoped_routes_config.rds_config_source(), - scoped_routes_config.scope_key_builder())); + ScopedRoutesConfigProviderManagerOptArg( + scoped_routes_config.name(), scoped_routes_config.rds_config_source(), + scoped_routes_config.scope_key_builder(), optional_http_filters)); srds_subscription_ = server_factory_context_.cluster_manager_.subscription_factory_.callbacks_; } @@ -196,9 +196,9 @@ name: foo_scoped_routes // Helper function which pushes an update to given RDS subscription, the start(_) of the // subscription must have been called. - void pushRdsConfig(const std::vector& route_config_names, - const std::string& version) { - const std::string route_config_tmpl = R"EOF( + void pushRdsConfig(const std::vector& route_config_names, const std::string& version, + const std::string& override_config_tmpl = "") { + std::string route_config_tmpl = R"EOF( name: {} virtual_hosts: - name: test @@ -207,6 +207,9 @@ name: foo_scoped_routes - match: {{ prefix: "/" }} route: {{ cluster: bluh }} )EOF"; + if (!override_config_tmpl.empty()) { + route_config_tmpl = override_config_tmpl; + } for (const std::string& name : route_config_names) { const auto route_config = TestUtility::parseYaml( @@ -245,6 +248,77 @@ name: foo_scoped_routes "foo.scoped_rds.foo_scoped_routes.on_demand_scopes", Stats::Gauge::ImportMode::Accumulate)}; }; +// Test an exception will be throw when unknown factory in the per-virtualhost typed config. +TEST_F(ScopedRdsTest, UnknownFactoryForPerVirtualHostTypedConfig) { + setup(); + init_watcher_.expectReady().Times(0); // The onConfigUpdate will simply throw an exception. + const std::string config_yaml = R"EOF( +name: foo_scope +route_configuration_name: foo_routes +key: + fragments: + - string_key: x-foo-key +)EOF"; + + const auto resource = parseScopedRouteConfigurationFromYaml(config_yaml); + const auto decoded_resources = TestUtility::decodeResources({resource}); + + context_init_manager_.initialize(init_watcher_); + EXPECT_NO_THROW(srds_subscription_->onConfigUpdate(decoded_resources.refvec_, "1")); + + std::string route_config_tmpl = R"EOF( + name: {} + virtual_hosts: + - name: test + domains: ["*"] + routes: + - match: {{ prefix: "/" }} + route: {{ cluster: bluh }} + typed_per_filter_config: + filter.unknown: + "@type": type.googleapis.com/google.protobuf.Struct +)EOF"; + + EXPECT_THROW_WITH_MESSAGE(pushRdsConfig({"foo_routes"}, "111", route_config_tmpl), EnvoyException, + "Didn't find a registered implementation for name: 'filter.unknown'"); +} + +// Test ignoring the optional unknown factory in the per-virtualhost typed config. +TEST_F(ScopedRdsTest, OptionalUnknownFactoryForPerVirtualHostTypedConfig) { + OptionalHttpFilters optional_http_filters; + optional_http_filters.insert("filter.unknown"); + setup(optional_http_filters); + init_watcher_.expectReady(); + const std::string config_yaml = R"EOF( +name: foo_scope +route_configuration_name: foo_routes +key: + fragments: + - string_key: x-foo-key +)EOF"; + + const auto resource = parseScopedRouteConfigurationFromYaml(config_yaml); + const auto decoded_resources = TestUtility::decodeResources({resource}); + + context_init_manager_.initialize(init_watcher_); + EXPECT_NO_THROW(srds_subscription_->onConfigUpdate(decoded_resources.refvec_, "1")); + + std::string route_config_tmpl = R"EOF( + name: {} + virtual_hosts: + - name: test + domains: ["*"] + routes: + - match: {{ prefix: "/" }} + route: {{ cluster: bluh }} + typed_per_filter_config: + filter.unknown: + "@type": type.googleapis.com/google.protobuf.Struct +)EOF"; + + pushRdsConfig({"foo_routes"}, "111", route_config_tmpl); +} + // Tests that multiple uniquely named non-conflict resources are allowed in config updates. TEST_F(ScopedRdsTest, MultipleResourcesSotw) { setup(); diff --git a/test/common/router/vhds_test.cc b/test/common/router/vhds_test.cc index 99d431b34f345..e819381f1e2a7 100644 --- a/test/common/router/vhds_test.cc +++ b/test/common/router/vhds_test.cc @@ -76,7 +76,7 @@ name: my_route RouteConfigUpdatePtr makeRouteConfigUpdate(const envoy::config::route::v3::RouteConfiguration& rc) { RouteConfigUpdatePtr config_update_info = - std::make_unique(factory_context_); + std::make_unique(factory_context_, OptionalHttpFilters()); config_update_info->onRdsUpdate(rc, "1"); return config_update_info; } diff --git a/test/common/upstream/cluster_manager_impl_test.cc b/test/common/upstream/cluster_manager_impl_test.cc index de00bb302046c..d15250242ab6b 100644 --- a/test/common/upstream/cluster_manager_impl_test.cc +++ b/test/common/upstream/cluster_manager_impl_test.cc @@ -1603,7 +1603,7 @@ TEST_F(ClusterManagerImplTest, DynamicAddRemove) { EXPECT_EQ(cluster2->info_, cluster_manager_->getThreadLocalCluster("fake_cluster")->info()); EXPECT_EQ(1UL, cluster_manager_->clusters().active_clusters_.size()); Http::ConnectionPool::MockInstance* cp = new Http::ConnectionPool::MockInstance(); - EXPECT_CALL(factory_, allocateConnPool_(_, _, _, _)).WillOnce(Return(cp)); + EXPECT_CALL(factory_, allocateConnPool_(_, _, _, _, _)).WillOnce(Return(cp)); EXPECT_EQ(cp, cluster_manager_->getThreadLocalCluster("fake_cluster") ->httpConnPool(ResourcePriority::Default, Http::Protocol::Http11, nullptr)); @@ -1760,7 +1760,7 @@ TEST_F(ClusterManagerImplTest, CloseHttpConnectionsOnHealthFailure) { })); create(parseBootstrapFromV3Json(json)); - EXPECT_CALL(factory_, allocateConnPool_(_, _, _, _)).WillOnce(Return(cp1)); + EXPECT_CALL(factory_, allocateConnPool_(_, _, _, _, _)).WillOnce(Return(cp1)); cluster_manager_->getThreadLocalCluster("some_cluster") ->httpConnPool(ResourcePriority::Default, Http::Protocol::Http11, nullptr); @@ -1771,7 +1771,7 @@ TEST_F(ClusterManagerImplTest, CloseHttpConnectionsOnHealthFailure) { test_host->healthFlagSet(Host::HealthFlag::FAILED_OUTLIER_CHECK); outlier_detector.runCallbacks(test_host); - EXPECT_CALL(factory_, allocateConnPool_(_, _, _, _)).WillOnce(Return(cp2)); + EXPECT_CALL(factory_, allocateConnPool_(_, _, _, _, _)).WillOnce(Return(cp2)); cluster_manager_->getThreadLocalCluster("some_cluster") ->httpConnPool(ResourcePriority::High, Http::Protocol::Http11, nullptr); } @@ -2034,7 +2034,7 @@ TEST_F(ClusterManagerImplTest, DynamicHostRemove) { EXPECT_CALL(initialized, ready()); cluster_manager_->setInitializedCb([&]() -> void { initialized.ready(); }); - EXPECT_CALL(factory_, allocateConnPool_(_, _, _, _)) + EXPECT_CALL(factory_, allocateConnPool_(_, _, _, _, _)) .Times(4) .WillRepeatedly(ReturnNew()); @@ -2198,7 +2198,7 @@ TEST_F(ClusterManagerImplTest, DynamicHostRemoveWithTls) { EXPECT_CALL(initialized, ready()); cluster_manager_->setInitializedCb([&]() -> void { initialized.ready(); }); - EXPECT_CALL(factory_, allocateConnPool_(_, _, _, _)) + EXPECT_CALL(factory_, allocateConnPool_(_, _, _, _, _)) .Times(4) .WillRepeatedly(ReturnNew()); @@ -2498,7 +2498,7 @@ TEST_F(ClusterManagerImplTest, DynamicHostRemoveDefaultPriority) { dns_callback(Network::DnsResolver::ResolutionStatus::Success, TestUtility::makeDnsResponse({"127.0.0.2"})); - EXPECT_CALL(factory_, allocateConnPool_(_, _, _, _)) + EXPECT_CALL(factory_, allocateConnPool_(_, _, _, _, _)) .WillOnce(ReturnNew()); EXPECT_CALL(factory_, allocateTcpConnPool_(_)) @@ -2585,7 +2585,7 @@ TEST_F(ClusterManagerImplTest, ConnPoolDestroyWithDraining) { TestUtility::makeDnsResponse({"127.0.0.2"})); MockConnPoolWithDestroy* mock_cp = new MockConnPoolWithDestroy(); - EXPECT_CALL(factory_, allocateConnPool_(_, _, _, _)).WillOnce(Return(mock_cp)); + EXPECT_CALL(factory_, allocateConnPool_(_, _, _, _, _)).WillOnce(Return(mock_cp)); MockTcpConnPoolWithDestroy* mock_tcp = new MockTcpConnPoolWithDestroy(); EXPECT_CALL(factory_, allocateTcpConnPool_(_)).WillOnce(Return(mock_tcp)); @@ -3055,7 +3055,7 @@ TEST_F(ClusterManagerImplTest, UpstreamSocketOptionsPassedToConnPool) { Network::SocketOptionFactory::buildIpTransparentOptions(); EXPECT_CALL(context, upstreamSocketOptions()).WillOnce(Return(options_to_return)); - EXPECT_CALL(factory_, allocateConnPool_(_, _, _, _)).WillOnce(Return(to_create)); + EXPECT_CALL(factory_, allocateConnPool_(_, _, _, _, _)).WillOnce(Return(to_create)); Http::ConnectionPool::Instance* cp = cluster_manager_->getThreadLocalCluster("cluster_1") @@ -3078,13 +3078,13 @@ TEST_F(ClusterManagerImplTest, UpstreamSocketOptionsUsedInConnPoolHash) { EXPECT_CALL(context1, upstreamSocketOptions()).WillRepeatedly(Return(options1)); EXPECT_CALL(context2, upstreamSocketOptions()).WillRepeatedly(Return(options2)); - EXPECT_CALL(factory_, allocateConnPool_(_, _, _, _)).WillOnce(Return(to_create1)); + EXPECT_CALL(factory_, allocateConnPool_(_, _, _, _, _)).WillOnce(Return(to_create1)); Http::ConnectionPool::Instance* cp1 = cluster_manager_->getThreadLocalCluster("cluster_1") ->httpConnPool(ResourcePriority::Default, Http::Protocol::Http11, &context1); - EXPECT_CALL(factory_, allocateConnPool_(_, _, _, _)).WillOnce(Return(to_create2)); + EXPECT_CALL(factory_, allocateConnPool_(_, _, _, _, _)).WillOnce(Return(to_create2)); Http::ConnectionPool::Instance* cp2 = cluster_manager_->getThreadLocalCluster("cluster_1") ->httpConnPool(ResourcePriority::Default, Http::Protocol::Http11, &context2); @@ -3112,7 +3112,7 @@ TEST_F(ClusterManagerImplTest, UpstreamSocketOptionsNullIsOkay) { Network::Socket::OptionsSharedPtr options_to_return = nullptr; EXPECT_CALL(context, upstreamSocketOptions()).WillOnce(Return(options_to_return)); - EXPECT_CALL(factory_, allocateConnPool_(_, _, _, _)).WillOnce(Return(to_create)); + EXPECT_CALL(factory_, allocateConnPool_(_, _, _, _, _)).WillOnce(Return(to_create)); Http::ConnectionPool::Instance* cp = cluster_manager_->getThreadLocalCluster("cluster_1") @@ -4028,7 +4028,7 @@ TEST_F(ClusterManagerImplTest, ConnPoolsDrainedOnHostSetChange) { EXPECT_EQ(0, factory_.stats_.counter("cluster_manager.cluster_updated_via_merge").value()); EXPECT_EQ(0, factory_.stats_.counter("cluster_manager.update_merge_cancelled").value()); - EXPECT_CALL(factory_, allocateConnPool_(_, _, _, _)) + EXPECT_CALL(factory_, allocateConnPool_(_, _, _, _, _)) .Times(3) .WillRepeatedly(ReturnNew()); @@ -4136,7 +4136,7 @@ TEST_F(ClusterManagerImplTest, ConnPoolsNotDrainedOnHostSetChange) { 0, HostSetImpl::partitionHosts(hosts_ptr, HostsPerLocalityImpl::empty()), nullptr, hosts, {}, 100); - EXPECT_CALL(factory_, allocateConnPool_(_, _, _, _)) + EXPECT_CALL(factory_, allocateConnPool_(_, _, _, _, _)) .Times(1) .WillRepeatedly(ReturnNew()); @@ -4274,7 +4274,8 @@ TEST_F(ClusterManagerImplTest, ConnectionPoolPerDownstreamConnection) { std::vector conn_pool_vector; for (size_t i = 0; i < 3; ++i) { conn_pool_vector.push_back(new Http::ConnectionPool::MockInstance()); - EXPECT_CALL(factory_, allocateConnPool_(_, _, _, _)).WillOnce(Return(conn_pool_vector.back())); + EXPECT_CALL(factory_, allocateConnPool_(_, _, _, _, _)) + .WillOnce(Return(conn_pool_vector.back())); EXPECT_CALL(downstream_connection, hashKey) .WillOnce(Invoke([i](std::vector& hash_key) { hash_key.push_back(i); })); EXPECT_EQ(conn_pool_vector.back(), @@ -4351,7 +4352,7 @@ TEST_F(PreconnectTest, PreconnectOff) { // With preconnect set to 0, each request for a connection pool will only // allocate that conn pool. initialize(0); - EXPECT_CALL(factory_, allocateConnPool_(_, _, _, _)) + EXPECT_CALL(factory_, allocateConnPool_(_, _, _, _, _)) .Times(1) .WillRepeatedly(ReturnNew()); cluster_manager_->getThreadLocalCluster("cluster_1") @@ -4369,7 +4370,7 @@ TEST_F(PreconnectTest, PreconnectOn) { // preconnecting, so create the pool for both the current connection and the // anticipated one. initialize(1.1); - EXPECT_CALL(factory_, allocateConnPool_(_, _, _, _)) + EXPECT_CALL(factory_, allocateConnPool_(_, _, _, _, _)) .Times(2) .WillRepeatedly(ReturnNew>()); cluster_manager_->getThreadLocalCluster("cluster_1") @@ -4386,7 +4387,7 @@ TEST_F(PreconnectTest, PreconnectHighHttp) { // With preconnect set to 3, the first request will kick off 3 preconnect attempts. initialize(3); int http_preconnect = 0; - EXPECT_CALL(factory_, allocateConnPool_(_, _, _, _)) + EXPECT_CALL(factory_, allocateConnPool_(_, _, _, _, _)) .Times(4) .WillRepeatedly(InvokeWithoutArgs([&]() -> Http::ConnectionPool::Instance* { auto* ret = new NiceMock(); @@ -4426,7 +4427,7 @@ TEST_F(PreconnectTest, PreconnectCappedAt3) { // With preconnect set to 20, no more than 3 connections will be preconnected. initialize(20); int http_preconnect = 0; - EXPECT_CALL(factory_, allocateConnPool_(_, _, _, _)) + EXPECT_CALL(factory_, allocateConnPool_(_, _, _, _, _)) .Times(4) .WillRepeatedly(InvokeWithoutArgs([&]() -> Http::ConnectionPool::Instance* { auto* ret = new NiceMock(); @@ -4455,7 +4456,7 @@ TEST_F(PreconnectTest, PreconnectCappedByMaybePreconnect) { // Set preconnect high, and verify preconnecting stops when maybePreconnect returns false. initialize(20); int http_preconnect_calls = 0; - EXPECT_CALL(factory_, allocateConnPool_(_, _, _, _)) + EXPECT_CALL(factory_, allocateConnPool_(_, _, _, _, _)) .Times(2) .WillRepeatedly(InvokeWithoutArgs([&]() -> Http::ConnectionPool::Instance* { auto* ret = new NiceMock(); diff --git a/test/common/upstream/test_cluster_manager.h b/test/common/upstream/test_cluster_manager.h index f57acc673889f..55b51ee3e01fd 100644 --- a/test/common/upstream/test_cluster_manager.h +++ b/test/common/upstream/test_cluster_manager.h @@ -80,11 +80,13 @@ class TestClusterManagerFactory : public ClusterManagerFactory { Http::ConnectionPool::InstancePtr allocateConnPool(Event::Dispatcher&, HostConstSharedPtr host, ResourcePriority, std::vector&, + const absl::optional& + alternate_protocol_options, const Network::ConnectionSocket::OptionsSharedPtr& options, const Network::TransportSocketOptionsSharedPtr& transport_socket_options, TimeSource&, ClusterConnectivityState& state) override { - return Http::ConnectionPool::InstancePtr{ - allocateConnPool_(host, options, transport_socket_options, state)}; + return Http::ConnectionPool::InstancePtr{allocateConnPool_( + host, alternate_protocol_options, options, transport_socket_options, state)}; } Tcp::ConnectionPool::InstancePtr @@ -118,7 +120,10 @@ class TestClusterManagerFactory : public ClusterManagerFactory { MOCK_METHOD(ClusterManager*, clusterManagerFromProto_, (const envoy::config::bootstrap::v3::Bootstrap& bootstrap)); MOCK_METHOD(Http::ConnectionPool::Instance*, allocateConnPool_, - (HostConstSharedPtr host, Network::ConnectionSocket::OptionsSharedPtr, + (HostConstSharedPtr host, + const absl::optional& + alternate_protocol_options, + Network::ConnectionSocket::OptionsSharedPtr, Network::TransportSocketOptionsSharedPtr, ClusterConnectivityState&)); MOCK_METHOD(Tcp::ConnectionPool::Instance*, allocateTcpConnPool_, (HostConstSharedPtr host)); MOCK_METHOD((std::pair), clusterFromProto_, diff --git a/test/common/upstream/upstream_impl_test.cc b/test/common/upstream/upstream_impl_test.cc index a0ce86ae68ac1..a72407a7981b8 100644 --- a/test/common/upstream/upstream_impl_test.cc +++ b/test/common/upstream/upstream_impl_test.cc @@ -2255,6 +2255,11 @@ class BazFactory : public ClusterTypedMetadataFactory { } throw EnvoyException("Cannot create a Baz when metadata is empty."); } + + std::unique_ptr + parse(const ProtobufWkt::Any&) const override { + return nullptr; + } }; // Cluster metadata and common config retrieval. diff --git a/test/extensions/clusters/redis/redis_cluster_lb_test.cc b/test/extensions/clusters/redis/redis_cluster_lb_test.cc index a0ed75aedf1f9..1b7abc6bb9420 100644 --- a/test/extensions/clusters/redis/redis_cluster_lb_test.cc +++ b/test/extensions/clusters/redis/redis_cluster_lb_test.cc @@ -349,18 +349,30 @@ TEST_F(RedisClusterLoadBalancerTest, ClusterSlotUpdate) { TEST_F(RedisClusterLoadBalancerTest, ClusterSlotNoUpdate) { Upstream::HostVector hosts{Upstream::makeTestHost(info_, "tcp://127.0.0.1:90", simTime()), + Upstream::makeTestHost(info_, "tcp://127.0.0.1:91", simTime()), + Upstream::makeTestHost(info_, "tcp://127.0.0.1:92", simTime()), + Upstream::makeTestHost(info_, "tcp://127.0.0.1:90", simTime()), Upstream::makeTestHost(info_, "tcp://127.0.0.1:91", simTime()), Upstream::makeTestHost(info_, "tcp://127.0.0.1:92", simTime())}; + Upstream::HostVector replicas{Upstream::makeTestHost(info_, "tcp://127.0.0.2:90", simTime()), + Upstream::makeTestHost(info_, "tcp://127.0.0.2:91", simTime()), + Upstream::makeTestHost(info_, "tcp://127.0.0.2:90", simTime()), + Upstream::makeTestHost(info_, "tcp://127.0.0.2:91", simTime())}; ClusterSlotsPtr slots = std::make_unique>(std::vector{ ClusterSlot(0, 1000, hosts[0]->address()), ClusterSlot(1001, 2000, hosts[1]->address()), ClusterSlot(2001, 16383, hosts[2]->address()), }); + + (*slots)[0].addReplica(replicas[0]->address()); + (*slots)[0].addReplica(replicas[1]->address()); Upstream::HostMap all_hosts{ {hosts[0]->address()->asString(), hosts[0]}, {hosts[1]->address()->asString(), hosts[1]}, {hosts[2]->address()->asString(), hosts[2]}, + {replicas[0]->address()->asString(), replicas[0]}, + {replicas[1]->address()->asString(), replicas[1]}, }; // A list of (hash: host_index) pair. @@ -373,10 +385,12 @@ TEST_F(RedisClusterLoadBalancerTest, ClusterSlotNoUpdate) { // Calling cluster slot update without change should not change assignment. std::vector updated_slot{ - ClusterSlot(0, 1000, hosts[0]->address()), - ClusterSlot(1001, 2000, hosts[1]->address()), - ClusterSlot(2001, 16383, hosts[2]->address()), + ClusterSlot(0, 1000, hosts[3]->address()), + ClusterSlot(1001, 2000, hosts[4]->address()), + ClusterSlot(2001, 16383, hosts[5]->address()), }; + updated_slot[0].addReplica(replicas[3]->address()); + updated_slot[0].addReplica(replicas[2]->address()); EXPECT_EQ(false, factory_->onClusterSlotUpdate( std::make_unique>(updated_slot), all_hosts)); validateAssignment(hosts, expected_assignments); diff --git a/test/extensions/filters/http/kill_request/BUILD b/test/extensions/filters/http/kill_request/BUILD index 3243457dd12ce..0d724b113f116 100644 --- a/test/extensions/filters/http/kill_request/BUILD +++ b/test/extensions/filters/http/kill_request/BUILD @@ -53,10 +53,14 @@ envoy_extension_cc_test( envoy_cc_test( name = "crash_integration_test", + size = "large", srcs = ["crash_integration_test.cc"], + shard_count = 2, deps = [ "//source/extensions/filters/http/kill_request:kill_request_config", "//test/integration:http_protocol_integration_lib", + "//test/integration/filters:stop_and_continue_filter_config_proto_cc_proto", + "//test/integration/filters:stop_iteration_and_continue", "@envoy_api//envoy/extensions/filters/http/kill_request/v3:pkg_cc_proto", ], ) diff --git a/test/extensions/filters/http/kill_request/crash_integration_test.cc b/test/extensions/filters/http/kill_request/crash_integration_test.cc index 22406a74b9efb..63da020128495 100644 --- a/test/extensions/filters/http/kill_request/crash_integration_test.cc +++ b/test/extensions/filters/http/kill_request/crash_integration_test.cc @@ -1,5 +1,6 @@ #include "envoy/extensions/filters/http/kill_request/v3/kill_request.pb.h" +#include "test/integration/filters/stop_and_continue_filter_config.pb.h" #include "test/integration/http_protocol_integration.h" #include "gtest/gtest.h" @@ -91,6 +92,145 @@ TEST_P(CrashIntegrationTestAllProtocols, ResponseCrashDumpsTheCorrespondingReque sendRequestAndWaitForResponse(default_request_headers_, 0, kill_response_headers, 1024), "Dumping corresponding downstream request.*UpstreamRequest.*request_headers:"); } + +TEST_P(CrashIntegrationTestAllProtocols, DecodeContinueDoesNotAddTrackedObjectIfExists) { + const std::string request_kill_config = + R"EOF( + name: envoy.filters.http.kill_request + typed_config: + "@type": type.googleapis.com/envoy.extensions.filters.http.kill_request.v3.KillRequest + probability: + numerator: 100 + )EOF"; + config_helper_.addFilter(request_kill_config); + + // This will stop iteration, and continue via a callback. + const std::string stop_and_continue_config = R"EOF( + name: stop-iteration-and-continue-filter + typed_config: + "@type": type.googleapis.com/test.integration.filters.StopAndContinueConfig + installScopeTrackedObject: true + )EOF"; + config_helper_.addFilter(stop_and_continue_config); + + initialize(); + + codec_client_ = makeHttpConnection(makeClientConnection(lookupPort("http"))); + const Http::TestRequestHeaderMapImpl request_headers{{":method", "GET"}, + {":path", "/test"}, + {":scheme", "http"}, + {":authority", "host"}, + {"x-envoy-kill-request", "true"}}; + // We should have the following directly on the tracked object stack and should dump them on + // crash: + // - The filter's custom scope tracked object + EXPECT_DEATH(sendRequestAndWaitForResponse(request_headers, 0, default_response_headers_, 1024), + "StopIterationAndContinue decode_delay_timer"); +} + +TEST_P(CrashIntegrationTestAllProtocols, DecodeContinueAddsCrashContextIfNoneExists) { + const std::string request_kill_config = + R"EOF( + name: envoy.filters.http.kill_request + typed_config: + "@type": type.googleapis.com/envoy.extensions.filters.http.kill_request.v3.KillRequest + probability: + numerator: 100 + )EOF"; + config_helper_.addFilter(request_kill_config); + + // This will stop iteration, and continue via a callback. + const std::string stop_and_continue_config = R"EOF( + name: stop-iteration-and-continue-filter + typed_config: + "@type": type.googleapis.com/test.integration.filters.StopAndContinueConfig + installScopeTrackedObject: false + )EOF"; + config_helper_.addFilter(stop_and_continue_config); + + initialize(); + + codec_client_ = makeHttpConnection(makeClientConnection(lookupPort("http"))); + const Http::TestRequestHeaderMapImpl request_headers{{":method", "GET"}, + {":path", "/test"}, + {":scheme", "http"}, + {":authority", "host"}, + {"x-envoy-kill-request", "true"}}; + // We should should dump the following by the ScopeTrackedObject installed in + // commonContinue: + // - ActiveStream + // - Network::ConnectionImpl + EXPECT_DEATH(sendRequestAndWaitForResponse(request_headers, 0, default_response_headers_, 1024), + "ActiveStream.*.*ConnectionImpl"); +} + +TEST_P(CrashIntegrationTestAllProtocols, EncodeContinueDoesNotAddTrackedObjectIfExists) { + // This will stop iteration, and continue via a callback. + const std::string stop_and_continue_config = R"EOF( + name: stop-iteration-and-continue-filter + typed_config: + "@type": type.googleapis.com/test.integration.filters.StopAndContinueConfig + installScopeTrackedObject: true + )EOF"; + config_helper_.addFilter(stop_and_continue_config); + + const std::string request_kill_config = + R"EOF( + name: envoy.filters.http.kill_request + typed_config: + "@type": type.googleapis.com/envoy.extensions.filters.http.kill_request.v3.KillRequest + probability: + numerator: 100 + direction: RESPONSE + )EOF"; + config_helper_.addFilter(request_kill_config); + + initialize(); + + codec_client_ = makeHttpConnection(makeClientConnection(lookupPort("http"))); + const Http::TestResponseHeaderMapImpl kill_response_headers = {{":status", "200"}, + {"x-envoy-kill-request", "true"}}; + // We should have the following directly on the tracked object stack and should dump them on + // crash: + // - The filter's custom scope tracked object + EXPECT_DEATH(sendRequestAndWaitForResponse(default_request_headers_, 0, kill_response_headers, 0), + "StopIterationAndContinue encode_delay_timer"); +} + +TEST_P(CrashIntegrationTestAllProtocols, EncodeContinueAddsCrashContextIfNoneExists) { + // This will stop iteration, and continue via a callback. + const std::string stop_and_continue_config = R"EOF( + name: stop-iteration-and-continue-filter + typed_config: + "@type": type.googleapis.com/test.integration.filters.StopAndContinueConfig + installScopeTrackedObject: false + )EOF"; + config_helper_.addFilter(stop_and_continue_config); + + const std::string request_kill_config = + R"EOF( + name: envoy.filters.http.kill_request + typed_config: + "@type": type.googleapis.com/envoy.extensions.filters.http.kill_request.v3.KillRequest + probability: + numerator: 100 + direction: RESPONSE + )EOF"; + config_helper_.addFilter(request_kill_config); + + initialize(); + + codec_client_ = makeHttpConnection(makeClientConnection(lookupPort("http"))); + const Http::TestResponseHeaderMapImpl kill_response_headers = {{":status", "200"}, + {"x-envoy-kill-request", "true"}}; + // We should should dump the following by the ScopeTrackedObject installed in + // commonContinue: + // - ActiveStream + // - Network::ConnectionImpl + EXPECT_DEATH(sendRequestAndWaitForResponse(default_request_headers_, 0, kill_response_headers, 0), + "ActiveStream.*.*ConnectionImpl"); +} + #endif #endif #endif 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 7f4b8daec07e4..7d2a11ceb7f0e 100644 --- a/test/extensions/filters/network/http_connection_manager/config_test.cc +++ b/test/extensions/filters/network/http_connection_manager/config_test.cc @@ -1740,6 +1740,33 @@ TEST_F(HttpConnectionManagerConfigTest, UnknownRequestIDExtension) { "Didn't find a registered implementation for type"); } +TEST_F(HttpConnectionManagerConfigTest, UnknownHttpFilterWithException) { + const std::string yaml_string = R"EOF( + stat_prefix: ingress_http + route_config: + name: local_route + http_filters: + - name: envoy.filters.http.unknown + )EOF"; + + EXPECT_THROW_WITH_REGEX( + createHttpConnectionManagerConfig(yaml_string), EnvoyException, + "Didn't find a registered implementation for name: 'envoy.filters.http.unknown"); +} + +TEST_F(HttpConnectionManagerConfigTest, UnknownOptionalHttpFilterWithIgnore) { + const std::string yaml_string = R"EOF( + stat_prefix: ingress_http + route_config: + name: local_route + http_filters: + - name: envoy.filters.http.unknown + is_optional: true + )EOF"; + + createHttpConnectionManagerConfig(yaml_string); +} + TEST_F(HttpConnectionManagerConfigTest, DefaultRequestIDExtension) { const std::string yaml_string = R"EOF( stat_prefix: ingress_http diff --git a/test/integration/alpn_integration_test.cc b/test/integration/alpn_integration_test.cc index 5097515bd60a9..264036a4bc2a8 100644 --- a/test/integration/alpn_integration_test.cc +++ b/test/integration/alpn_integration_test.cc @@ -32,11 +32,10 @@ class AlpnIntegrationTest : public testing::TestWithParamadd_alpn_protocols("h2"); - } else if (upstream_config_.upstream_protocol_ == FakeHttpConnection::Type::HTTP1) { + } else if (upstream_config.upstream_protocol_ == FakeHttpConnection::Type::HTTP1) { tls_context.mutable_common_tls_context()->add_alpn_protocols("http/1.1"); } - if (upstream_config_.upstream_protocol_ != FakeHttpConnection::Type::HTTP3) { + if (upstream_config.upstream_protocol_ != FakeHttpConnection::Type::HTTP3) { auto cfg = std::make_unique( tls_context, factory_context_); static Stats::Scope* upstream_stats_store = new Stats::IsolatedStoreImpl(); @@ -146,7 +147,8 @@ Network::TransportSocketFactoryPtr BaseIntegrationTest::createUpstreamTlsContext void BaseIntegrationTest::createUpstreams() { for (uint32_t i = 0; i < fake_upstreams_count_; ++i) { Network::TransportSocketFactoryPtr factory = - upstream_tls_ ? createUpstreamTlsContext() : Network::Test::createRawBufferSocketFactory(); + upstream_tls_ ? createUpstreamTlsContext(upstreamConfig()) + : Network::Test::createRawBufferSocketFactory(); auto endpoint = upstream_address_fn_(i); if (autonomous_upstream_) { fake_upstreams_.emplace_back(new AutonomousUpstream( diff --git a/test/integration/base_integration_test.h b/test/integration/base_integration_test.h index 94a3a7243e8aa..9f902f958d2dc 100644 --- a/test/integration/base_integration_test.h +++ b/test/integration/base_integration_test.h @@ -307,17 +307,24 @@ class BaseIntegrationTest : protected Logger::Loggable { *dispatcher_, std::move(transport_socket)); } - // Add a fake upstream bound to INADDR_ANY and there is no specified port. - FakeUpstream& addFakeUpstream(FakeHttpConnection::Type type) { + FakeUpstreamConfig configWithType(FakeHttpConnection::Type type) const { FakeUpstreamConfig config = upstream_config_; config.upstream_protocol_ = type; + if (type != FakeHttpConnection::Type::HTTP3) { + config.udp_fake_upstream_ = absl::nullopt; + } + return config; + } + + FakeUpstream& addFakeUpstream(FakeHttpConnection::Type type) { + auto config = configWithType(type); fake_upstreams_.emplace_back(std::make_unique(0, version_, config)); return *fake_upstreams_.back(); } + FakeUpstream& addFakeUpstream(Network::TransportSocketFactoryPtr&& transport_socket_factory, FakeHttpConnection::Type type) { - FakeUpstreamConfig config = upstream_config_; - config.upstream_protocol_ = type; + auto config = configWithType(type); fake_upstreams_.emplace_back( std::make_unique(std::move(transport_socket_factory), 0, version_, config)); return *fake_upstreams_.back(); @@ -394,7 +401,8 @@ class BaseIntegrationTest : protected Logger::Loggable { bool use_lds_{true}; // Use the integration framework's LDS set up. bool upstream_tls_{false}; - Network::TransportSocketFactoryPtr createUpstreamTlsContext(); + Network::TransportSocketFactoryPtr + createUpstreamTlsContext(const FakeUpstreamConfig& upstream_config); testing::NiceMock factory_context_; Extensions::TransportSockets::Tls::ContextManagerImpl context_manager_{timeSystem()}; @@ -437,8 +445,6 @@ class BaseIntegrationTest : protected Logger::Loggable { bool v2_bootstrap_{false}; private: - friend class MixedUpstreamIntegrationTest; - // Configuration for the fake upstream. FakeUpstreamConfig upstream_config_{time_system_}; // True if initialized() has been called. diff --git a/test/integration/fake_upstream.cc b/test/integration/fake_upstream.cc index a4248c52cea39..be91ed958e96e 100644 --- a/test/integration/fake_upstream.cc +++ b/test/integration/fake_upstream.cc @@ -448,18 +448,17 @@ FakeUpstream::FakeUpstream(const std::string& uds_path, const FakeUpstreamConfig : FakeUpstream(Network::Test::createRawBufferSocketFactory(), Network::SocketPtr{new Network::UdsListenSocket( std::make_shared(uds_path))}, - config) { - ENVOY_LOG(info, "starting fake server on unix domain socket {}", uds_path); -} + config) {} static Network::SocketPtr makeTcpListenSocket(const Network::Address::InstanceConstSharedPtr& address) { return std::make_unique(address, nullptr, true); } -static Network::SocketPtr makeTcpListenSocket(uint32_t port, Network::Address::IpVersion version) { - return makeTcpListenSocket(Network::Utility::parseInternetAddress( - Network::Test::getLoopbackAddressString(version), port)); +static Network::Address::InstanceConstSharedPtr makeAddress(uint32_t port, + Network::Address::IpVersion version) { + return Network::Utility::parseInternetAddress(Network::Test::getLoopbackAddressString(version), + port); } static Network::SocketPtr @@ -482,31 +481,19 @@ makeListenSocket(const FakeUpstreamConfig& config, FakeUpstream::FakeUpstream(uint32_t port, Network::Address::IpVersion version, const FakeUpstreamConfig& config) : FakeUpstream(Network::Test::createRawBufferSocketFactory(), - makeTcpListenSocket(port, version), config) { - ASSERT(!config.udp_fake_upstream_.has_value()); - ENVOY_LOG(info, "starting fake server on port {}. Address version is {}", - localAddress()->ip()->port(), Network::Test::addressVersionAsString(version)); -} + makeListenSocket(config, makeAddress(port, version)), config) {} FakeUpstream::FakeUpstream(Network::TransportSocketFactoryPtr&& transport_socket_factory, const Network::Address::InstanceConstSharedPtr& address, const FakeUpstreamConfig& config) : FakeUpstream(std::move(transport_socket_factory), makeListenSocket(config, address), config) { - ENVOY_LOG(info, "starting fake server on socket {}:{}. Address version is {}. UDP={}", - address->ip()->addressAsString(), address->ip()->port(), - Network::Test::addressVersionAsString(address->ip()->version()), - config.udp_fake_upstream_.has_value()); } FakeUpstream::FakeUpstream(Network::TransportSocketFactoryPtr&& transport_socket_factory, uint32_t port, Network::Address::IpVersion version, const FakeUpstreamConfig& config) - : FakeUpstream(std::move(transport_socket_factory), makeTcpListenSocket(port, version), - config) { - ASSERT(!config.udp_fake_upstream_.has_value()); - ENVOY_LOG(info, "starting fake server on port {}. Address version is {}", - localAddress()->ip()->port(), Network::Test::addressVersionAsString(version)); -} + : FakeUpstream(std::move(transport_socket_factory), + makeListenSocket(config, makeAddress(port, version)), config) {} FakeUpstream::FakeUpstream(Network::TransportSocketFactoryPtr&& transport_socket_factory, Network::SocketPtr&& listen_socket, const FakeUpstreamConfig& config) @@ -520,6 +507,8 @@ FakeUpstream::FakeUpstream(Network::TransportSocketFactoryPtr&& transport_socket read_disable_on_new_connection_(true), enable_half_close_(config.enable_half_close_), listener_(*this, http_type_ == FakeHttpConnection::Type::HTTP3), filter_chain_(Network::Test::createEmptyFilterChain(std::move(transport_socket_factory))) { + ENVOY_LOG(info, "starting fake server at {}. UDP={} codec={}", localAddress()->asString(), + config.udp_fake_upstream_.has_value(), FakeHttpConnection::typeToString(http_type_)); if (config.udp_fake_upstream_.has_value() && config.udp_fake_upstream_->max_rx_datagram_size_.has_value()) { listener_.udp_listener_config_.config_.mutable_downstream_socket_config() diff --git a/test/integration/fake_upstream.h b/test/integration/fake_upstream.h index d10c7ce4d8f51..eb3e9ac757095 100644 --- a/test/integration/fake_upstream.h +++ b/test/integration/fake_upstream.h @@ -99,7 +99,7 @@ class FakeStream : public Http::RequestDecoder, return encoder_.http1StreamEncoderOptions(); } void - sendLocalReply(bool is_grpc_request, Http::Code code, absl::string_view body, + sendLocalReply(Http::Code code, absl::string_view body, const std::function& /*modify_headers*/, const absl::optional grpc_status, absl::string_view /*details*/) override { @@ -119,7 +119,7 @@ class FakeStream : public Http::RequestDecoder, [&](Buffer::Instance& data, bool end_stream) -> void { encoder_.encodeData(data, end_stream); }}), - Http::Utility::LocalReplyData({is_grpc_request, code, body, grpc_status, is_head_request})); + Http::Utility::LocalReplyData({false, code, body, grpc_status, is_head_request})); } ABSL_MUST_USE_RESULT @@ -414,6 +414,17 @@ class FakeConnectionBase : public Logger::Loggable { class FakeHttpConnection : public Http::ServerConnectionCallbacks, public FakeConnectionBase { public: enum class Type { HTTP1, HTTP2, HTTP3 }; + static absl::string_view typeToString(Type type) { + switch (type) { + case Type::HTTP1: + return "http1"; + case Type::HTTP2: + return "http2"; + case Type::HTTP3: + return "http3"; + } + return "invalid"; + } FakeHttpConnection(FakeUpstream& fake_upstream, SharedConnectionWrapper& shared_connection, Type type, Event::TestTimeSystem& time_system, uint32_t max_request_headers_kb, diff --git a/test/integration/filters/BUILD b/test/integration/filters/BUILD index 4722b15a3730b..dcc6c91d6782a 100644 --- a/test/integration/filters/BUILD +++ b/test/integration/filters/BUILD @@ -284,17 +284,26 @@ envoy_proto_library( srcs = [":set_is_terminal_filter_config.proto"], ) +envoy_proto_library( + name = "stop_and_continue_filter_config_proto", + srcs = [":stop_and_continue_filter_config.proto"], +) + envoy_cc_test_library( name = "stop_iteration_and_continue", srcs = [ "stop_iteration_and_continue_filter.cc", ], deps = [ + ":stop_and_continue_filter_config_proto_cc_proto", + "//include/envoy/common:scope_tracker_interface", "//include/envoy/http:filter_interface", "//include/envoy/registry", + "//source/common/common:scope_tracker", "//source/common/network:connection_lib", + "//source/extensions/filters/http/common:factory_base_lib", "//source/extensions/filters/http/common:pass_through_filter_lib", - "//test/extensions/filters/http/common:empty_http_filter_config_lib", + "//test/test_common:utility_lib", ], ) diff --git a/test/integration/filters/on_local_reply_filter.cc b/test/integration/filters/on_local_reply_filter.cc index 2bb2923d3c731..c47f053903c65 100644 --- a/test/integration/filters/on_local_reply_filter.cc +++ b/test/integration/filters/on_local_reply_filter.cc @@ -16,11 +16,23 @@ class OnLocalReplyFilter : public Http::PassThroughFilter { if (!request_headers.get(Http::LowerCaseString("reset")).empty()) { reset_ = true; } - decoder_callbacks_->sendLocalReply(Http::Code::BadRequest, "body", nullptr, absl::nullopt, - "details"); + if (!request_headers.get(Http::LowerCaseString("dual-local-reply")).empty()) { + dual_reply_ = true; + } + decoder_callbacks_->sendLocalReply(Http::Code::BadRequest, "original_reply", nullptr, + absl::nullopt, "original_reply"); return Http::FilterHeadersStatus::StopIteration; } + Http::FilterHeadersStatus encodeHeaders(Http::ResponseHeaderMap&, bool) override { + if (dual_reply_) { + decoder_callbacks_->sendLocalReply(Http::Code::BadRequest, "second_reply", nullptr, + absl::nullopt, "second_reply"); + return Http::FilterHeadersStatus::StopIteration; + } + return Http::FilterHeadersStatus::Continue; + } + Http::LocalErrorStatus onLocalReply(const LocalReplyData&) override { if (reset_) { return Http::LocalErrorStatus::ContinueAndResetStream; @@ -29,6 +41,7 @@ class OnLocalReplyFilter : public Http::PassThroughFilter { } bool reset_{}; + bool dual_reply_{}; }; class OnLocalReplyFilterConfig : public Extensions::HttpFilters::Common::EmptyHttpFilterConfig { diff --git a/test/integration/filters/stop_and_continue_filter_config.proto b/test/integration/filters/stop_and_continue_filter_config.proto new file mode 100644 index 0000000000000..5ab6a5d93fc51 --- /dev/null +++ b/test/integration/filters/stop_and_continue_filter_config.proto @@ -0,0 +1,11 @@ +syntax = "proto3"; + +package test.integration.filters; + +import "validate/validate.proto"; + +message StopAndContinueConfig { + // Whether the filter should add tracked object itself to the dispatcher when + // created. + bool install_scope_tracked_object = 1; +} diff --git a/test/integration/filters/stop_iteration_and_continue_filter.cc b/test/integration/filters/stop_iteration_and_continue_filter.cc index c631d8b9e7bc7..3a1f91be901a6 100644 --- a/test/integration/filters/stop_iteration_and_continue_filter.cc +++ b/test/integration/filters/stop_iteration_and_continue_filter.cc @@ -1,29 +1,54 @@ +#include #include +#include "envoy/common/scope_tracker.h" #include "envoy/http/filter.h" #include "envoy/registry/registry.h" #include "envoy/server/filter_config.h" +#include "common/common/scope_tracker.h" + +#include "extensions/filters/http/common/factory_base.h" #include "extensions/filters/http/common/pass_through_filter.h" -#include "test/extensions/filters/http/common/empty_http_filter_config.h" +#include "test/integration/filters/stop_and_continue_filter_config.pb.h" +#include "test/integration/filters/stop_and_continue_filter_config.pb.validate.h" +#include "test/test_common/utility.h" namespace Envoy { // A test filter that does StopIterationNoBuffer on end stream, then continues after a 0ms alarm. +// It can optionally register a ScopeTrackedObject on continuation. class StopIterationAndContinueFilter : public Http::PassThroughFilter { public: + StopIterationAndContinueFilter(bool set_tracked_object) + : set_tracked_object_(set_tracked_object) {} + void setEndStreamAndDecodeTimer() { decode_end_stream_seen_ = true; - decode_delay_timer_ = decoder_callbacks_->dispatcher().createTimer( - [this]() -> void { decoder_callbacks_->continueDecoding(); }); + decode_delay_timer_ = decoder_callbacks_->dispatcher().createTimer([this]() -> void { + absl::optional msg; + absl::optional state; + if (set_tracked_object_) { + msg.emplace("StopIterationAndContinue decode_delay_timer"); + state.emplace(&msg.value(), decoder_callbacks_->dispatcher()); + } + decoder_callbacks_->continueDecoding(); + }); decode_delay_timer_->enableTimer(std::chrono::seconds(0)); } void setEndStreamAndEncodeTimer() { encode_end_stream_seen_ = true; - encode_delay_timer_ = decoder_callbacks_->dispatcher().createTimer( - [this]() -> void { encoder_callbacks_->continueEncoding(); }); + encode_delay_timer_ = decoder_callbacks_->dispatcher().createTimer([this]() -> void { + absl::optional msg; + absl::optional state; + if (set_tracked_object_) { + msg.emplace("StopIterationAndContinue encode_delay_timer"); + state.emplace(&msg.value(), decoder_callbacks_->dispatcher()); + } + encoder_callbacks_->continueEncoding(); + }); encode_delay_timer_->enableTimer(std::chrono::seconds(0)); } @@ -63,25 +88,28 @@ class StopIterationAndContinueFilter : public Http::PassThroughFilter { bool decode_end_stream_seen_{}; Event::TimerPtr encode_delay_timer_; bool encode_end_stream_seen_{}; + bool set_tracked_object_{}; }; -class StopIterationAndContinueFilterConfig - : public Extensions::HttpFilters::Common::EmptyHttpFilterConfig { +class StopIterationAndContinueFilterFactory + : public Extensions::HttpFilters::Common::FactoryBase< + test::integration::filters::StopAndContinueConfig> { public: - StopIterationAndContinueFilterConfig() - : EmptyHttpFilterConfig("stop-iteration-and-continue-filter") {} + StopIterationAndContinueFilterFactory() : FactoryBase("stop-iteration-and-continue-filter") {} - Http::FilterFactoryCb createFilter(const std::string&, - Server::Configuration::FactoryContext&) override { - return [](Http::FilterChainFactoryCallbacks& callbacks) -> void { - callbacks.addStreamFilter(std::make_shared<::Envoy::StopIterationAndContinueFilter>()); +private: + Http::FilterFactoryCb createFilterFactoryFromProtoTyped( + const test::integration::filters::StopAndContinueConfig& proto_config, const std::string&, + Server::Configuration::FactoryContext&) override { + bool set_scope_tacked_object = proto_config.install_scope_tracked_object(); + return [set_scope_tacked_object](Http::FilterChainFactoryCallbacks& callbacks) -> void { + callbacks.addStreamFilter( + std::make_shared<::Envoy::StopIterationAndContinueFilter>(set_scope_tacked_object)); }; } }; -// perform static registration -static Registry::RegisterFactory - register_; +REGISTER_FACTORY(StopIterationAndContinueFilterFactory, + Server::Configuration::NamedHttpFilterConfigFactory); } // namespace Envoy diff --git a/test/integration/hds_integration_test.cc b/test/integration/hds_integration_test.cc index 3ade7ed52e87b..63ae64a0b500c 100644 --- a/test/integration/hds_integration_test.cc +++ b/test/integration/hds_integration_test.cc @@ -62,10 +62,10 @@ class HdsIntegrationTest : public Grpc::VersionedGrpcClientIntegrationParamTest, // Endpoint connections if (tls_hosts_) { - host_upstream_ = - &addFakeUpstream(HttpIntegrationTest::createUpstreamTlsContext(), http_conn_type_); - host2_upstream_ = - &addFakeUpstream(HttpIntegrationTest::createUpstreamTlsContext(), http_conn_type_); + host_upstream_ = &addFakeUpstream( + HttpIntegrationTest::createUpstreamTlsContext(upstreamConfig()), http_conn_type_); + host2_upstream_ = &addFakeUpstream( + HttpIntegrationTest::createUpstreamTlsContext(upstreamConfig()), http_conn_type_); } else { host_upstream_ = &addFakeUpstream(http_conn_type_); host2_upstream_ = &addFakeUpstream(http_conn_type_); diff --git a/test/integration/http_typed_per_filter_config_test.cc b/test/integration/http_typed_per_filter_config_test.cc index 251da64f64e02..27a80a3caa5c5 100644 --- a/test/integration/http_typed_per_filter_config_test.cc +++ b/test/integration/http_typed_per_filter_config_test.cc @@ -11,35 +11,60 @@ class HTTPTypedPerFilterConfigTest : public testing::Test, public HttpIntegratio public: HTTPTypedPerFilterConfigTest() : HttpIntegrationTest(Http::CodecClient::Type::HTTP2, Network::Address::IpVersion::v4) {} - - void initialize() override { - config_helper_.addConfigModifier( - [&](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager& - hcm) { - envoy::extensions::filters::http::health_check::v3::HealthCheck health_check; - health_check.mutable_pass_through_mode()->set_value(false); - - // The http health_check filter doesn't support per filter config. So specify one - // and expect the exception will be raised. - auto* virtual_host = hcm.mutable_route_config()->mutable_virtual_hosts(0); - auto* config = virtual_host->mutable_typed_per_filter_config(); - (*config)["envoy.filters.http.health_check"].PackFrom(health_check); - - auto* filter = hcm.mutable_http_filters()->Add(); - filter->set_name("envoy.filters.http.health_check"); - filter->mutable_typed_config()->PackFrom(health_check); - // keep router the last - auto size = hcm.http_filters_size(); - hcm.mutable_http_filters()->SwapElements(size - 2, size - 1); - }); - HttpIntegrationTest::initialize(); - } }; TEST_F(HTTPTypedPerFilterConfigTest, RejectUnsupportedTypedPerFilterConfig) { + config_helper_.addConfigModifier( + [&](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager& + hcm) { + envoy::extensions::filters::http::health_check::v3::HealthCheck health_check; + health_check.mutable_pass_through_mode()->set_value(false); + + // The http health_check filter doesn't support per filter config. So specify one + // and expect the exception will be raised. + auto* virtual_host = hcm.mutable_route_config()->mutable_virtual_hosts(0); + auto* config = virtual_host->mutable_typed_per_filter_config(); + (*config)["envoy.filters.http.health_check"].PackFrom(health_check); + + auto* filter = hcm.mutable_http_filters()->Add(); + filter->set_name("envoy.filters.http.health_check"); + filter->mutable_typed_config()->PackFrom(health_check); + // keep router the last + auto size = hcm.http_filters_size(); + hcm.mutable_http_filters()->SwapElements(size - 2, size - 1); + }); EXPECT_DEATH(initialize(), "The filter envoy.filters.http.health_check doesn't support virtual " "host-specific configurations"); } +TEST_F(HTTPTypedPerFilterConfigTest, RejectUnknownHttpFilterInTypedPerFilterConfig) { + config_helper_.addConfigModifier( + [&](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager& + hcm) { + auto* virtual_host = hcm.mutable_route_config()->mutable_virtual_hosts(0); + auto* config = virtual_host->mutable_typed_per_filter_config(); + (*config)["filter.unknown"].PackFrom(Envoy::ProtobufWkt::Struct()); + }); + EXPECT_DEATH(initialize(), "Didn't find a registered implementation for name: 'filter.unknown'"); +} + +TEST_F(HTTPTypedPerFilterConfigTest, IgnoreUnknownOptionalHttpFilterInTypedPerFilterConfig) { + config_helper_.addConfigModifier( + [&](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager& + hcm) { + auto* virtual_host = hcm.mutable_route_config()->mutable_virtual_hosts(0); + auto* config = virtual_host->mutable_typed_per_filter_config(); + (*config)["filter.unknown"].PackFrom(Envoy::ProtobufWkt::Struct()); + + auto* filter = hcm.mutable_http_filters()->Add(); + filter->set_name("filter.unknown"); + filter->set_is_optional("true"); + // keep router the last + auto size = hcm.http_filters_size(); + hcm.mutable_http_filters()->SwapElements(size - 2, size - 1); + }); + initialize(); +} + } // namespace } // namespace Envoy diff --git a/test/integration/integration_test.cc b/test/integration/integration_test.cc index c75bb2c497b74..a1ace5f25b9bf 100644 --- a/test/integration/integration_test.cc +++ b/test/integration/integration_test.cc @@ -15,6 +15,7 @@ #include "test/integration/autonomous_upstream.h" #include "test/integration/filters/process_context_filter.h" +#include "test/integration/filters/stop_and_continue_filter_config.pb.h" #include "test/integration/utility.h" #include "test/mocks/http/mocks.h" #include "test/test_common/network_utility.h" @@ -360,7 +361,7 @@ TEST_P(IntegrationTest, EnvoyProxying100ContinueWithDecodeDataPause) { config_helper_.addFilter(R"EOF( name: stop-iteration-and-continue-filter typed_config: - "@type": type.googleapis.com/google.protobuf.Empty + "@type": type.googleapis.com/test.integration.filters.StopAndContinueConfig )EOF"); testEnvoyProxying1xx(true); } diff --git a/test/integration/listener_lds_integration_test.cc b/test/integration/listener_lds_integration_test.cc index 6b9a2fc024bab..e042175fb923f 100644 --- a/test/integration/listener_lds_integration_test.cc +++ b/test/integration/listener_lds_integration_test.cc @@ -267,6 +267,85 @@ TEST_P(ListenerIntegrationTest, RejectsUnsupportedTypedPerFilterConfig) { test_server_->waitForCounterGe("listener_manager.lds.update_rejected", 1); } +TEST_P(ListenerIntegrationTest, RejectsUnknownHttpFilter) { + on_server_init_function_ = [&]() { + createLdsStream(); + envoy::config::listener::v3::Listener listener = + TestUtility::parseYaml(R"EOF( + name: fake_listener + address: + socket_address: + address: 127.0.0.1 + port_value: 0 + filter_chains: + - filters: + - name: http + typed_config: + "@type": type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager + codec_type: HTTP2 + stat_prefix: config_test + route_config: + name: route_config_0 + virtual_hosts: + - name: integration + domains: + - "*" + routes: + - match: + prefix: / + route: + cluster: cluster_0 + http_filters: + - name: filter.unknown + - name: envoy.filters.http.router + )EOF"); + sendLdsResponse({listener}, "2"); + }; + initialize(); + registerTestServerPorts({listener_name_}); + test_server_->waitForCounterGe("listener_manager.lds.update_rejected", 1); +} + +TEST_P(ListenerIntegrationTest, IgnoreUnknownOptionalHttpFilter) { + on_server_init_function_ = [&]() { + createLdsStream(); + envoy::config::listener::v3::Listener listener = + TestUtility::parseYaml(R"EOF( + name: fake_listener + address: + socket_address: + address: 127.0.0.1 + port_value: 0 + filter_chains: + - filters: + - name: http + typed_config: + "@type": type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager + codec_type: HTTP2 + stat_prefix: config_test + route_config: + name: route_config_0 + virtual_hosts: + - name: integration + domains: + - "*" + routes: + - match: + prefix: / + route: + cluster: cluster_0 + http_filters: + - name: filter.unknown + is_optional: true + - name: envoy.filters.http.router + )EOF"); + sendLdsResponse({listener}, "2"); + }; + initialize(); + registerTestServerPorts({listener_name_}); + test_server_->waitForCounterGe("listener_manager.lds.update_rejected", 0); +} + // Tests that a LDS deletion before Server initManager been initialized will not block the Server // from starting. TEST_P(ListenerIntegrationTest, RemoveLastUninitializedListener) { diff --git a/test/integration/multiplexed_integration_test.cc b/test/integration/multiplexed_integration_test.cc index 433790b46d7dd..63a976a738e03 100644 --- a/test/integration/multiplexed_integration_test.cc +++ b/test/integration/multiplexed_integration_test.cc @@ -11,6 +11,7 @@ #include "common/common/random_generator.h" #include "common/http/header_map_impl.h" +#include "test/integration/filters/stop_and_continue_filter_config.pb.h" #include "test/integration/utility.h" #include "test/mocks/http/mocks.h" #include "test/test_common/network_utility.h" @@ -1299,7 +1300,7 @@ TEST_P(Http2IntegrationTest, PauseAndResume) { config_helper_.addFilter(R"EOF( name: stop-iteration-and-continue-filter typed_config: - "@type": type.googleapis.com/google.protobuf.Empty + "@type": type.googleapis.com/test.integration.filters.StopAndContinueConfig )EOF"); initialize(); @@ -1329,7 +1330,7 @@ TEST_P(Http2IntegrationTest, PauseAndResumeHeadersOnly) { config_helper_.addFilter(R"EOF( name: stop-iteration-and-continue-filter typed_config: - "@type": type.googleapis.com/google.protobuf.Empty + "@type": type.googleapis.com/test.integration.filters.StopAndContinueConfig )EOF"); initialize(); @@ -1781,11 +1782,27 @@ TEST_P(Http2IntegrationTest, OnLocalReply) { initialize(); codec_client_ = makeHttpConnection(lookupPort("http")); + // The filter will send a local reply when receiving headers, the client + // should get a complete response. { auto response = codec_client_->makeHeaderOnlyRequest(default_request_headers_); ASSERT_TRUE(response->waitForEndStream()); ASSERT_TRUE(response->complete()); + EXPECT_EQ("original_reply", response->body()); } + // The filter will send a local reply when receiving headers, and interrupt + // that with a second reply sent from the encoder chain. The client will see + // the second response. + { + default_request_headers_.addCopy("dual-local-reply", "yes"); + auto response = codec_client_->makeHeaderOnlyRequest(default_request_headers_); + ASSERT_TRUE(response->waitForEndStream()); + ASSERT_TRUE(response->complete()); + EXPECT_EQ("second_reply", response->body()); + } + // The filter will send a local reply when receiving headers and reset the + // stream onLocalReply. The client will get a reset and no response even if + // dual local replies are on (from the prior request). { default_request_headers_.addCopy("reset", "yes"); auto response = codec_client_->makeHeaderOnlyRequest(default_request_headers_); diff --git a/test/integration/multiplexed_upstream_integration_test.cc b/test/integration/multiplexed_upstream_integration_test.cc index 24690e011e6ca..2c86cba32fb0a 100644 --- a/test/integration/multiplexed_upstream_integration_test.cc +++ b/test/integration/multiplexed_upstream_integration_test.cc @@ -610,16 +610,18 @@ class MixedUpstreamIntegrationTest : public Http2UpstreamIntegrationTest { } void createUpstreams() override { ASSERT_EQ(upstreamProtocol(), FakeHttpConnection::Type::HTTP3); + ASSERT_EQ(fake_upstreams_count_, 1); + ASSERT_FALSE(autonomous_upstream_); + if (use_http2_) { - // Generally we always want to set these fields via accessors, which - // changes both the upstreams and Envoy's configuration at the same time. - // In this particular case, we want to change the upstreams without - // touching config, so edit the raw members directly. - upstream_config_.udp_fake_upstream_ = absl::nullopt; - upstream_config_.upstream_protocol_ = FakeHttpConnection::Type::HTTP2; + auto config = configWithType(FakeHttpConnection::Type::HTTP2); + Network::TransportSocketFactoryPtr factory = createUpstreamTlsContext(config); + addFakeUpstream(std::move(factory), FakeHttpConnection::Type::HTTP2); + } else { + auto config = configWithType(FakeHttpConnection::Type::HTTP3); + Network::TransportSocketFactoryPtr factory = createUpstreamTlsContext(config); + addFakeUpstream(std::move(factory), FakeHttpConnection::Type::HTTP3); } - Http2UpstreamIntegrationTest::createUpstreams(); - upstream_config_.upstream_protocol_ = FakeHttpConnection::Type::HTTP3; } bool use_http2_{false}; diff --git a/test/integration/quic_http_integration_test.cc b/test/integration/quic_http_integration_test.cc index 1653d6e6f36e6..585bbc5beab0b 100644 --- a/test/integration/quic_http_integration_test.cc +++ b/test/integration/quic_http_integration_test.cc @@ -40,6 +40,14 @@ #include "test/config/integration/certs/clientcert_hash.h" #include "extensions/transport_sockets/tls/context_config_impl.h" +#if (defined(__has_feature) && __has_feature(thread_sanitizer)) || defined(ENVOY_CONFIG_COVERAGE) +#define DISABLE_UNDER_TSAN_OR_COVERAGE return +#else +#define DISABLE_UNDER_TSAN_OR_COVERAGE \ + do { \ + } while (0) +#endif + namespace Envoy { namespace Quic { @@ -346,6 +354,7 @@ TEST_P(QuicHttpIntegrationTest, RouterUpstreamResponseBeforeRequestComplete) { TEST_P(QuicHttpIntegrationTest, Retry) { testRetry(); } TEST_P(QuicHttpIntegrationTest, UpstreamReadDisabledOnGiantResponseBody) { + DISABLE_UNDER_TSAN_OR_COVERAGE; config_helper_.addConfigModifier(ConfigHelper::adjustUpstreamTimeoutForTsan); config_helper_.setBufferLimits(/*upstream_buffer_limit=*/1024, /*downstream_buffer_limit=*/1024); testRouterRequestAndResponseWithBody(/*request_size=*/512, /*response_size=*/10 * 1024 * 1024, @@ -354,6 +363,7 @@ TEST_P(QuicHttpIntegrationTest, UpstreamReadDisabledOnGiantResponseBody) { } TEST_P(QuicHttpIntegrationTest, DownstreamReadDisabledOnGiantPost) { + DISABLE_UNDER_TSAN_OR_COVERAGE; config_helper_.addConfigModifier(ConfigHelper::adjustUpstreamTimeoutForTsan); config_helper_.setBufferLimits(/*upstream_buffer_limit=*/1024, /*downstream_buffer_limit=*/1024); testRouterRequestAndResponseWithBody(/*request_size=*/10 * 1024 * 1024, /*response_size=*/1024, @@ -361,6 +371,7 @@ TEST_P(QuicHttpIntegrationTest, DownstreamReadDisabledOnGiantPost) { } TEST_P(QuicHttpIntegrationTest, LargeFlowControlOnAndGiantBody) { + DISABLE_UNDER_TSAN_OR_COVERAGE; config_helper_.addConfigModifier(ConfigHelper::adjustUpstreamTimeoutForTsan); config_helper_.setBufferLimits(/*upstream_buffer_limit=*/128 * 1024, /*downstream_buffer_limit=*/128 * 1024); diff --git a/test/integration/scoped_rds_integration_test.cc b/test/integration/scoped_rds_integration_test.cc index 512853adb3922..0de02704c054c 100644 --- a/test/integration/scoped_rds_integration_test.cc +++ b/test/integration/scoped_rds_integration_test.cc @@ -470,6 +470,109 @@ route_configuration_name: foo_route1 cleanupUpstreamAndDownstream(); } +TEST_P(ScopedRdsIntegrationTest, RejectUnknownHttpFilterInPerFilterTypedConfig) { + const std::string scope_tmpl = R"EOF( +name: {} +route_configuration_name: {} +key: + fragments: + - string_key: {} +)EOF"; + const std::string scope_route = fmt::format(scope_tmpl, "foo_scope1", "foo_route", "foo-route"); + + const std::string route_config_tmpl = R"EOF( + name: {} + virtual_hosts: + - name: integration + domains: ["*"] + routes: + - match: {{ prefix: "/" }} + route: {{ cluster: {} }} + typed_per_filter_config: + filter.unknown: + "@type": type.googleapis.com/google.protobuf.Struct +)EOF"; + + on_server_init_function_ = [&]() { + createScopedRdsStream(); + sendSrdsResponse({scope_route}, {scope_route}, {}, "1"); + createRdsStream("foo_route"); + // CreateRdsStream waits for connection which is fired by RDS subscription. + sendRdsResponse(fmt::format(route_config_tmpl, "foo_route", "cluster_0"), "1"); + }; + initialize(); + registerTestServerPorts({"http"}); + + test_server_->waitForCounterGe("http.config_test.rds.foo_route.update_rejected", 1); + codec_client_ = makeHttpConnection(lookupPort("http")); + auto response = codec_client_->makeHeaderOnlyRequest( + Http::TestRequestHeaderMapImpl{{":method", "GET"}, + {":path", "/meh"}, + {":authority", "host"}, + {":scheme", "http"}, + {"Addr", "x-foo-key=foo-route"}}); + ASSERT_TRUE(response->waitForEndStream()); + verifyResponse(std::move(response), "404", Http::TestResponseHeaderMapImpl{}, ""); + cleanupUpstreamAndDownstream(); +} + +TEST_P(ScopedRdsIntegrationTest, IgnoreUnknownOptionalHttpFilterInPerFilterTypedConfig) { + const std::string scope_tmpl = R"EOF( +name: {} +route_configuration_name: {} +key: + fragments: + - string_key: {} +)EOF"; + const std::string scope_route = fmt::format(scope_tmpl, "foo_scope1", "foo_route", "foo-route"); + + const std::string route_config_tmpl = R"EOF( + name: {} + virtual_hosts: + - name: integration + domains: ["*"] + routes: + - match: {{ prefix: "/" }} + route: {{ cluster: {} }} + typed_per_filter_config: + filter.unknown: + "@type": type.googleapis.com/google.protobuf.Struct +)EOF"; + + on_server_init_function_ = [&]() { + createScopedRdsStream(); + sendSrdsResponse({scope_route}, {scope_route}, {}, "1"); + createRdsStream("foo_route"); + // CreateRdsStream waits for connection which is fired by RDS subscription. + sendRdsResponse(fmt::format(route_config_tmpl, "foo_route", "cluster_0"), "1"); + }; + + config_helper_.addConfigModifier( + [](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager& + http_connection_manager) { + auto* filter = http_connection_manager.mutable_http_filters()->Add(); + filter->set_name("filter.unknown"); + filter->set_is_optional("true"); + // keep router the last + auto size = http_connection_manager.http_filters_size(); + http_connection_manager.mutable_http_filters()->SwapElements(size - 2, size - 1); + }); + + initialize(); + registerTestServerPorts({"http"}); + + test_server_->waitForCounterGe("http.config_test.rds.foo_route.update_attempt", 1); + sendRequestAndVerifyResponse( + Http::TestRequestHeaderMapImpl{{":method", "GET"}, + {":path", "/meh"}, + {":authority", "host"}, + {":scheme", "http"}, + {"Addr", "x-foo-key=foo-route"}}, + 456, Http::TestResponseHeaderMapImpl{{":status", "200"}, {"service", "bluh"}}, 123, + /*cluster_0*/ 0); + cleanupUpstreamAndDownstream(); +} + TEST_P(ScopedRdsIntegrationTest, RejectKeyConflictInDeltaUpdate) { if (!isDelta()) { return; diff --git a/test/integration/ssl_utility.cc b/test/integration/ssl_utility.cc index 0afb4b6d80fb0..0cc1f7de4cfbc 100644 --- a/test/integration/ssl_utility.cc +++ b/test/integration/ssl_utility.cc @@ -1,5 +1,7 @@ #include "test/integration/ssl_utility.h" +#include "envoy/extensions/transport_sockets/quic/v3/quic_transport.pb.h" + #include "common/http/utility.h" #include "common/json/json_loader.h" #include "common/network/utility.h" @@ -96,7 +98,7 @@ createClientSslTransportSocketFactory(const ClientSslTransportOptions& options, } Network::TransportSocketFactoryPtr createUpstreamSslContext(ContextManager& context_manager, - Api::Api& api) { + Api::Api& api, bool use_http3) { envoy::extensions::transport_sockets::tls::v3::DownstreamTlsContext tls_context; ConfigHelper::initializeTls({}, *tls_context.mutable_common_tls_context()); @@ -106,8 +108,18 @@ Network::TransportSocketFactoryPtr createUpstreamSslContext(ContextManager& cont tls_context, mock_factory_ctx); static Stats::Scope* upstream_stats_store = new Stats::TestIsolatedStoreImpl(); - return std::make_unique( - std::move(cfg), context_manager, *upstream_stats_store, std::vector{}); + if (!use_http3) { + return std::make_unique( + std::move(cfg), context_manager, *upstream_stats_store, std::vector{}); + } + envoy::extensions::transport_sockets::quic::v3::QuicDownstreamTransport quic_config; + quic_config.mutable_downstream_tls_context()->MergeFrom(tls_context); + + std::vector server_names; + auto& config_factory = Config::Utility::getAndCheckFactoryByName< + Server::Configuration::DownstreamTransportSocketConfigFactory>( + "envoy.transport_sockets.quic"); + return config_factory.createTransportSocketFactory(quic_config, mock_factory_ctx, server_names); } Network::TransportSocketFactoryPtr createFakeUpstreamSslContext( diff --git a/test/integration/ssl_utility.h b/test/integration/ssl_utility.h index f7a202016aee4..f5a75d35e4028 100644 --- a/test/integration/ssl_utility.h +++ b/test/integration/ssl_utility.h @@ -72,7 +72,7 @@ createClientSslTransportSocketFactory(const ClientSslTransportOptions& options, ContextManager& context_manager, Api::Api& api); Network::TransportSocketFactoryPtr createUpstreamSslContext(ContextManager& context_manager, - Api::Api& api); + Api::Api& api, bool use_http3 = false); Network::TransportSocketFactoryPtr createFakeUpstreamSslContext(const std::string& upstream_cert_name, ContextManager& context_manager, diff --git a/test/integration/tcp_proxy_integration_test.cc b/test/integration/tcp_proxy_integration_test.cc index 438eb3a6ef928..8cb605c65ba3a 100644 --- a/test/integration/tcp_proxy_integration_test.cc +++ b/test/integration/tcp_proxy_integration_test.cc @@ -1320,7 +1320,7 @@ class MysqlIntegrationTest : public TcpProxyIntegrationTest { void createUpstreams() override { for (uint32_t i = 0; i < fake_upstreams_count_; ++i) { Network::TransportSocketFactoryPtr factory = - upstream_tls_ ? createUpstreamTlsContext() + upstream_tls_ ? createUpstreamTlsContext(upstreamConfig()) : Network::Test::createRawBufferSocketFactory(); auto endpoint = upstream_address_fn_(i); fake_upstreams_.emplace_back( diff --git a/test/integration/transport_socket_match_integration_test.cc b/test/integration/transport_socket_match_integration_test.cc index ddef54c3a1704..1bc92bae21f8d 100644 --- a/test/integration/transport_socket_match_integration_test.cc +++ b/test/integration/transport_socket_match_integration_test.cc @@ -119,7 +119,7 @@ name: "tls_socket" auto endpoint = upstream_address_fn_(i); if (isTLSUpstream(i)) { fake_upstreams_.emplace_back(new AutonomousUpstream( - HttpIntegrationTest::createUpstreamTlsContext(), endpoint->ip()->port(), + HttpIntegrationTest::createUpstreamTlsContext(upstreamConfig()), endpoint->ip()->port(), endpoint->ip()->version(), upstreamConfig(), false)); } else { fake_upstreams_.emplace_back(new AutonomousUpstream( diff --git a/test/mocks/event/mocks.h b/test/mocks/event/mocks.h index 31ac32d7b0a6f..36544bd2c1d2f 100644 --- a/test/mocks/event/mocks.h +++ b/test/mocks/event/mocks.h @@ -154,6 +154,7 @@ class MockDispatcher : public Dispatcher { MOCK_METHOD(void, run, (RunType type)); MOCK_METHOD(void, pushTrackedObject, (const ScopeTrackedObject* object)); MOCK_METHOD(void, popTrackedObject, (const ScopeTrackedObject* expected_object)); + MOCK_METHOD(bool, trackedObjectStackIsEmpty, (), (const)); MOCK_METHOD(bool, isThreadSafe, (), (const)); Buffer::WatermarkFactory& getWatermarkFactory() override { return buffer_factory_; } MOCK_METHOD(Thread::ThreadId, getCurrentThreadId, ()); diff --git a/test/mocks/event/wrapped_dispatcher.h b/test/mocks/event/wrapped_dispatcher.h index 867e9c86c9c0b..776f0a5035ec5 100644 --- a/test/mocks/event/wrapped_dispatcher.h +++ b/test/mocks/event/wrapped_dispatcher.h @@ -117,6 +117,8 @@ class WrappedDispatcher : public Dispatcher { return impl_.popTrackedObject(expected_object); } + bool trackedObjectStackIsEmpty() const override { return impl_.trackedObjectStackIsEmpty(); } + MonotonicTime approximateMonotonicTime() const override { return impl_.approximateMonotonicTime(); } diff --git a/test/mocks/http/mocks.h b/test/mocks/http/mocks.h index da5cb0b8a4979..366170ba33b27 100644 --- a/test/mocks/http/mocks.h +++ b/test/mocks/http/mocks.h @@ -107,6 +107,7 @@ class MockFilterManagerCallbacks : public FilterManagerCallbacks { MOCK_METHOD(void, onLocalReply, (Code code)); MOCK_METHOD(Tracing::Config&, tracingConfig, ()); MOCK_METHOD(const ScopeTrackedObject&, scope, ()); + MOCK_METHOD(void, restoreContextOnContinue, (ScopeTrackedObjectStack&)); MOCK_METHOD(bool, enableInternalRedirectsWithBody, (), (const)); ResponseHeaderMapPtr continue_headers_; @@ -213,6 +214,7 @@ class MockStreamDecoderFilterCallbacks : public StreamDecoderFilterCallbacks, MOCK_METHOD(Tracing::Span&, activeSpan, ()); MOCK_METHOD(Tracing::Config&, tracingConfig, ()); MOCK_METHOD(const ScopeTrackedObject&, scope, ()); + MOCK_METHOD(void, restoreContextOnContinue, (ScopeTrackedObjectStack&)); MOCK_METHOD(void, onDecoderFilterAboveWriteBufferHighWatermark, ()); MOCK_METHOD(void, onDecoderFilterBelowWriteBufferLowWatermark, ()); MOCK_METHOD(void, addDownstreamWatermarkCallbacks, (DownstreamWatermarkCallbacks&)); @@ -305,6 +307,7 @@ class MockStreamEncoderFilterCallbacks : public StreamEncoderFilterCallbacks, MOCK_METHOD(void, onEncoderFilterBelowWriteBufferLowWatermark, ()); MOCK_METHOD(void, setEncoderBufferLimit, (uint32_t)); MOCK_METHOD(uint32_t, encoderBufferLimit, ()); + MOCK_METHOD(void, restoreContextOnContinue, (ScopeTrackedObjectStack&)); // Http::StreamEncoderFilterCallbacks MOCK_METHOD(void, addEncodedData, (Buffer::Instance & data, bool streaming)); @@ -346,7 +349,7 @@ class MockStreamDecoderFilter : public StreamDecoderFilter { MOCK_METHOD(void, setDecoderFilterCallbacks, (StreamDecoderFilterCallbacks & callbacks)); MOCK_METHOD(void, decodeComplete, ()); MOCK_METHOD(void, sendLocalReply, - (bool is_grpc_request, Code code, absl::string_view body, + (Code code, absl::string_view body, const std::function& modify_headers, bool is_head_request, const absl::optional grpc_status, absl::string_view details)); diff --git a/test/mocks/http/stream_decoder.h b/test/mocks/http/stream_decoder.h index 3aebc35ea00de..84017eceb6cd1 100644 --- a/test/mocks/http/stream_decoder.h +++ b/test/mocks/http/stream_decoder.h @@ -17,7 +17,7 @@ class MockRequestDecoder : public RequestDecoder { MOCK_METHOD(void, decodeData, (Buffer::Instance & data, bool end_stream)); MOCK_METHOD(void, decodeMetadata_, (MetadataMapPtr & metadata_map)); MOCK_METHOD(void, sendLocalReply, - (bool is_grpc_request, Code code, absl::string_view body, + (Code code, absl::string_view body, const std::function& modify_headers, const absl::optional grpc_status, absl::string_view details)); diff --git a/test/mocks/network/connection.h b/test/mocks/network/connection.h index b0b8486fb0ec4..24fe1ee7a3412 100644 --- a/test/mocks/network/connection.h +++ b/test/mocks/network/connection.h @@ -1,6 +1,7 @@ #pragma once #include +#include #include "envoy/network/connection.h" @@ -83,7 +84,8 @@ class MockConnectionBase { MOCK_METHOD(void, setDelayedCloseTimeout, (std::chrono::milliseconds)); \ MOCK_METHOD(absl::string_view, transportFailureReason, (), (const)); \ MOCK_METHOD(bool, startSecureTransport, ()); \ - MOCK_METHOD(absl::optional, lastRoundTripTime, (), (const)) + MOCK_METHOD(absl::optional, lastRoundTripTime, (), (const)); \ + MOCK_METHOD(void, dumpState, (std::ostream&, int), (const)); class MockConnection : public Connection, public MockConnectionBase { public: diff --git a/test/mocks/router/mocks.h b/test/mocks/router/mocks.h index 13f56a239ef5e..aa06b6e0a6620 100644 --- a/test/mocks/router/mocks.h +++ b/test/mocks/router/mocks.h @@ -510,10 +510,12 @@ class MockRouteConfigProviderManager : public RouteConfigProviderManager { MOCK_METHOD(RouteConfigProviderSharedPtr, createRdsRouteConfigProvider, (const envoy::extensions::filters::network::http_connection_manager::v3::Rds& rds, + const OptionalHttpFilters& optional_http_filters, Server::Configuration::ServerFactoryContext& factory_context, const std::string& stat_prefix, Init::Manager& init_manager)); MOCK_METHOD(RouteConfigProviderPtr, createStaticRouteConfigProvider, (const envoy::config::route::v3::RouteConfiguration& route_config, + const OptionalHttpFilters& optional_http_filters, Server::Configuration::ServerFactoryContext& factory_context, ProtobufMessage::ValidationVisitor& validator)); }; diff --git a/test/mocks/upstream/cluster_info.cc b/test/mocks/upstream/cluster_info.cc index cd0cd7572ec5b..9c698a61ccd2a 100644 --- a/test/mocks/upstream/cluster_info.cc +++ b/test/mocks/upstream/cluster_info.cc @@ -107,6 +107,8 @@ MockClusterInfo::MockClusterInfo() ON_CALL(*this, metadata()).WillByDefault(ReturnRef(metadata_)); ON_CALL(*this, upstreamHttpProtocolOptions()) .WillByDefault(ReturnRef(upstream_http_protocol_options_)); + ON_CALL(*this, alternateProtocolsCacheOptions()) + .WillByDefault(ReturnRef(alternate_protocols_cache_options_)); // Delayed construction of typed_metadata_, to allow for injection of metadata ON_CALL(*this, typedMetadata()) .WillByDefault(Invoke([this]() -> const Envoy::Config::TypedMetadata& { diff --git a/test/mocks/upstream/cluster_info.h b/test/mocks/upstream/cluster_info.h index 640d5bf4f6044..5a3322536313c 100644 --- a/test/mocks/upstream/cluster_info.h +++ b/test/mocks/upstream/cluster_info.h @@ -143,6 +143,8 @@ class MockClusterInfo : public ClusterInfo { MOCK_METHOD(bool, warmHosts, (), (const)); MOCK_METHOD(const absl::optional&, upstreamHttpProtocolOptions, (), (const)); + MOCK_METHOD(const absl::optional&, + alternateProtocolsCacheOptions, (), (const)); MOCK_METHOD(absl::optional, edsServiceName, (), (const)); MOCK_METHOD(void, createNetworkFilterChain, (Network::Connection&), (const)); MOCK_METHOD(std::vector, upstreamHttpProtocol, (absl::optional), @@ -187,6 +189,8 @@ class MockClusterInfo : public ClusterInfo { NiceMock lb_subset_; absl::optional upstream_http_protocol_options_; + absl::optional + alternate_protocols_cache_options_; absl::optional lb_ring_hash_config_; absl::optional lb_maglev_config_; absl::optional lb_original_dst_config_; diff --git a/test/mocks/upstream/cluster_manager_factory.h b/test/mocks/upstream/cluster_manager_factory.h index a542564940504..ffa11b5e7546e 100644 --- a/test/mocks/upstream/cluster_manager_factory.h +++ b/test/mocks/upstream/cluster_manager_factory.h @@ -23,6 +23,8 @@ class MockClusterManagerFactory : public ClusterManagerFactory { MOCK_METHOD(Http::ConnectionPool::InstancePtr, allocateConnPool, (Event::Dispatcher & dispatcher, HostConstSharedPtr host, ResourcePriority priority, std::vector& protocol, + const absl::optional& + alternate_protocol_options, const Network::ConnectionSocket::OptionsSharedPtr& options, const Network::TransportSocketOptionsSharedPtr& transport_socket_options, TimeSource& source, ClusterConnectivityState& state)); diff --git a/test/per_file_coverage.sh b/test/per_file_coverage.sh index 0c2278641e766..ef5e2a6bb6adc 100755 --- a/test/per_file_coverage.sh +++ b/test/per_file_coverage.sh @@ -9,9 +9,8 @@ declare -a KNOWN_LOW_COVERAGE=( "source/common/common:96.3" "source/common/common/posix:94.1" "source/common/crypto:0.0" -"source/common/event:93.4" # Emulated edge events guards don't report LCOV +"source/common/event:94.2" # Emulated edge events guards don't report LCOV "source/common/filesystem/posix:96.2" -"source/common/http/http3:93.9" "source/common/json:90.9" "source/common/network:95.0" # Flaky, `activateFileEvents`, `startSecureTransport` and `ioctl` do not always report LCOV "source/common/protobuf:94.8" @@ -19,8 +18,8 @@ declare -a KNOWN_LOW_COVERAGE=( "source/common/singleton:95.1" "source/common/thread:0.0" # Death tests don't report LCOV "source/common/matcher:93.3" -"source/common/quic:87.4" -"source/common/tracing:94.9" +"source/common/quic:88.4" +"source/common/tracing:95.7" "source/common/watchdog:42.9" # Death tests don't report LCOV "source/exe:94.3" "source/extensions/common/crypto:91.5" @@ -52,10 +51,10 @@ declare -a KNOWN_LOW_COVERAGE=( "source/extensions/tracers:96.4" "source/extensions/tracers/opencensus:91.6" "source/extensions/tracers/xray:94.0" -"source/extensions/transport_sockets:95.6" +"source/extensions/transport_sockets:95.7" "source/extensions/transport_sockets/tls/cert_validator:96.5" "source/extensions/transport_sockets/tls/private_key:76.9" -"source/extensions/transport_sockets/tls:95.0" +"source/extensions/transport_sockets/tls:95.1" "source/extensions/wasm_runtime:50.0" "source/extensions/wasm_runtime/wasmtime:0.0" # Not enabled in coverage build "source/extensions/wasm_runtime/wavm:0.0" # Not enabled in coverage build @@ -63,7 +62,7 @@ declare -a KNOWN_LOW_COVERAGE=( "source/extensions/watchdog/profile_action:85.7" "source/server:94.4" # flaky: be careful adjusting. See https://github.com/envoyproxy/envoy/issues/15239 "source/server/admin:95.7" -"source/server/config_validation:75.6" +"source/server/config_validation:78.2" ) [[ -z "${SRCDIR}" ]] && SRCDIR="${PWD}" diff --git a/test/test_common/utility.h b/test/test_common/utility.h index 4f2d61e4724dc..86d03c5d009a3 100644 --- a/test/test_common/utility.h +++ b/test/test_common/utility.h @@ -1101,6 +1101,16 @@ ApiPtr createApiForTest(Event::TimeSystem& time_system); ApiPtr createApiForTest(Stats::Store& stat_store, Event::TimeSystem& time_system); } // namespace Api +// Useful for testing ScopeTrackedObject order of deletion. +class MessageTrackedObject : public ScopeTrackedObject { +public: + MessageTrackedObject(absl::string_view sv) : sv_(sv) {} + void dumpState(std::ostream& os, int /*indent_level*/) const override { os << sv_; } + +private: + absl::string_view sv_; +}; + MATCHER_P(HeaderMapEqualIgnoreOrder, expected, "") { const bool equal = TestUtility::headerMapEqualIgnoreOrder(*arg, *expected); if (!equal) { diff --git a/test/tools/router_check/router.cc b/test/tools/router_check/router.cc index 73a5d1a0d6334..80441856af305 100644 --- a/test/tools/router_check/router.cc +++ b/test/tools/router_check/router.cc @@ -113,7 +113,8 @@ RouterCheckTool RouterCheckTool::create(const std::string& router_config_file, auto factory_context = std::make_unique>(); auto config = std::make_unique( - route_config, *factory_context, ProtobufMessage::getNullValidationVisitor(), false); + route_config, Router::OptionalHttpFilters(), *factory_context, + ProtobufMessage::getNullValidationVisitor(), false); if (!disable_deprecation_check) { MessageUtil::checkForUnexpectedFields(route_config, ProtobufMessage::getStrictValidationVisitor(), diff --git a/tools/code_format/check_format.py b/tools/code_format/check_format.py index c03697fec7aca..3e24b50f261a6 100755 --- a/tools/code_format/check_format.py +++ b/tools/code_format/check_format.py @@ -323,11 +323,15 @@ def read_file(self, path): # look_path searches for the given executable in all directories in PATH # environment variable. If it cannot be found, empty string is returned. def look_path(self, executable): + if executable is None: + return '' return shutil.which(executable) or '' # path_exists checks whether the given path exists. This function assumes that # the path is absolute and evaluates environment variables. def path_exists(self, executable): + if executable is None: + return False return os.path.exists(os.path.expandvars(executable)) # executable_by_others checks whether the given path has execute permission for diff --git a/tools/deprecate_version/requirements.txt b/tools/deprecate_version/requirements.txt index 68732cef92589..e9024ac6f3bd0 100644 --- a/tools/deprecate_version/requirements.txt +++ b/tools/deprecate_version/requirements.txt @@ -67,9 +67,9 @@ gitdb==4.0.7 \ # via # -r tools/deprecate_version/requirements.txt # gitpython -gitpython==3.1.14 \ - --hash=sha256:3283ae2fba31c913d857e12e5ba5f9a7772bbc064ae2bb09efafa71b0dd4939b \ - --hash=sha256:be27633e7509e58391f10207cd32b2a6cf5b908f92d9cd30da2e514e1137af61 +gitpython==3.1.17 \ + --hash=sha256:29fe82050709760081f588dd50ce83504feddbebdc4da6956d02351552b1c135 \ + --hash=sha256:ee24bdc93dce357630764db659edaf6b8d664d4ff5447ccfeedd2dc5c253f41e # via -r tools/deprecate_version/requirements.txt idna==2.10 \ --hash=sha256:b307872f855b18632ce0c21c5e45be78c0ea7ae4c15c828c20788b26921eb3f6 \ diff --git a/tools/testing/requirements.txt b/tools/testing/requirements.txt index 2d5b619ca3e43..77e56959bd614 100644 --- a/tools/testing/requirements.txt +++ b/tools/testing/requirements.txt @@ -94,9 +94,9 @@ pyparsing==2.4.7 \ # via # -r tools/testing/requirements.txt # packaging -pytest-cov==2.11.1 \ - --hash=sha256:359952d9d39b9f822d9d29324483e7ba04a3a17dd7d05aa6beb7ea01e359e5f7 \ - --hash=sha256:bdb9fdb0b85a7cc825269a4c56b48ccaa5c7e365054b6038772c32ddcdc969da +pytest-cov==2.12.0 \ + --hash=sha256:95d4933dcbbacfa377bb60b29801daa30d90c33981ab2a79e9ab4452c165066e \ + --hash=sha256:8535764137fecce504a49c2b742288e3d34bc09eed298ad65963616cc98fd45e # via -r tools/testing/requirements.txt pytest==6.2.4 \ --hash=sha256:91ef2131a9bd6be8f76f1f08eac5c5317221d6ad1e143ae03894b862e8976890 \