diff --git a/api/envoy/config/cluster/v3/cluster.proto b/api/envoy/config/cluster/v3/cluster.proto index be7710815b70..7eb53d84c4f8 100644 --- a/api/envoy/config/cluster/v3/cluster.proto +++ b/api/envoy/config/cluster/v3/cluster.proto @@ -19,6 +19,7 @@ import "google/protobuf/struct.proto"; import "google/protobuf/wrappers.proto"; import "envoy/annotations/deprecation.proto"; +import "udpa/annotations/security.proto"; import "udpa/annotations/status.proto"; import "udpa/annotations/versioning.proto"; import "validate/validate.proto"; @@ -584,7 +585,8 @@ message Cluster { // Soft limit on size of the cluster’s connections read and write buffers. If // unspecified, an implementation defined default is applied (1MiB). - google.protobuf.UInt32Value per_connection_buffer_limit_bytes = 5; + google.protobuf.UInt32Value per_connection_buffer_limit_bytes = 5 + [(udpa.annotations.security).configure_for_untrusted_upstream = true]; // The :ref:`load balancer type ` to use // when picking a host in the cluster. @@ -635,7 +637,8 @@ message Cluster { // supports prior knowledge for upstream connections. Even if TLS is used // with ALPN, `http2_protocol_options` must be specified. As an aside this allows HTTP/2 // connections to happen over plain text. - core.v3.Http2ProtocolOptions http2_protocol_options = 14; + core.v3.Http2ProtocolOptions http2_protocol_options = 14 + [(udpa.annotations.security).configure_for_untrusted_upstream = true]; // The extension_protocol_options field is used to provide extension-specific protocol options // for upstream connections. The key should match the extension filter name, such as diff --git a/api/envoy/config/cluster/v4alpha/cluster.proto b/api/envoy/config/cluster/v4alpha/cluster.proto index 2b044b2c6437..eab2f2d80fcb 100644 --- a/api/envoy/config/cluster/v4alpha/cluster.proto +++ b/api/envoy/config/cluster/v4alpha/cluster.proto @@ -19,6 +19,7 @@ import "google/protobuf/struct.proto"; import "google/protobuf/wrappers.proto"; import "envoy/annotations/deprecation.proto"; +import "udpa/annotations/security.proto"; import "udpa/annotations/status.proto"; import "udpa/annotations/versioning.proto"; import "validate/validate.proto"; @@ -585,7 +586,8 @@ message Cluster { // Soft limit on size of the cluster’s connections read and write buffers. If // unspecified, an implementation defined default is applied (1MiB). - google.protobuf.UInt32Value per_connection_buffer_limit_bytes = 5; + google.protobuf.UInt32Value per_connection_buffer_limit_bytes = 5 + [(udpa.annotations.security).configure_for_untrusted_upstream = true]; // The :ref:`load balancer type ` to use // when picking a host in the cluster. @@ -636,7 +638,8 @@ message Cluster { // supports prior knowledge for upstream connections. Even if TLS is used // with ALPN, `http2_protocol_options` must be specified. As an aside this allows HTTP/2 // connections to happen over plain text. - core.v4alpha.Http2ProtocolOptions http2_protocol_options = 14; + core.v4alpha.Http2ProtocolOptions http2_protocol_options = 14 + [(udpa.annotations.security).configure_for_untrusted_upstream = true]; // The extension_protocol_options field is used to provide extension-specific protocol options // for upstream connections. The key should match the extension filter name, such as 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 b46e63076d7a..355eaba01e93 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 @@ -19,6 +19,7 @@ import "google/protobuf/struct.proto"; import "google/protobuf/wrappers.proto"; import "envoy/annotations/deprecation.proto"; +import "udpa/annotations/security.proto"; import "udpa/annotations/status.proto"; import "udpa/annotations/versioning.proto"; import "validate/validate.proto"; @@ -281,13 +282,15 @@ message HttpConnectionManager { // Additional settings for HTTP requests handled by the connection manager. These will be // applicable to both HTTP1 and HTTP2 requests. - config.core.v3.HttpProtocolOptions common_http_protocol_options = 35; + config.core.v3.HttpProtocolOptions common_http_protocol_options = 35 + [(udpa.annotations.security).configure_for_untrusted_downstream = true]; // Additional HTTP/1 settings that are passed to the HTTP/1 codec. config.core.v3.Http1ProtocolOptions http_protocol_options = 8; // Additional HTTP/2 settings that are passed directly to the HTTP/2 codec. - config.core.v3.Http2ProtocolOptions http2_protocol_options = 9; + config.core.v3.Http2ProtocolOptions http2_protocol_options = 9 + [(udpa.annotations.security).configure_for_untrusted_downstream = true]; // An optional override that the connection manager will write to the server // header in responses. If not set, the default is *envoy*. @@ -332,13 +335,15 @@ message HttpConnectionManager { // // A value of 0 will completely disable the connection manager stream idle // timeout, although per-route idle timeout overrides will continue to apply. - google.protobuf.Duration stream_idle_timeout = 24; + google.protobuf.Duration stream_idle_timeout = 24 + [(udpa.annotations.security).configure_for_untrusted_downstream = true]; // The amount of time that Envoy will wait for the entire request to be received. // The timer is activated when the request is initiated, and is disarmed when the last byte of the // request is sent upstream (i.e. all decoding filters have processed the request), OR when the // response is initiated. If not specified or set to 0, this timeout is disabled. - google.protobuf.Duration request_timeout = 28; + google.protobuf.Duration request_timeout = 28 + [(udpa.annotations.security).configure_for_untrusted_downstream = true]; // The time that Envoy will wait between sending an HTTP/2 “shutdown // notification” (GOAWAY frame with max stream ID) and a final GOAWAY frame. @@ -394,7 +399,8 @@ message HttpConnectionManager { // :ref:`config_http_conn_man_headers_x-forwarded-for`, // :ref:`config_http_conn_man_headers_x-envoy-internal`, and // :ref:`config_http_conn_man_headers_x-envoy-external-address` for more information. - google.protobuf.BoolValue use_remote_address = 14; + google.protobuf.BoolValue use_remote_address = 14 + [(udpa.annotations.security).configure_for_untrusted_downstream = true]; // The number of additional ingress proxy hops from the right side of the // :ref:`config_http_conn_man_headers_x-forwarded-for` HTTP header to trust when 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 482bb5ed95e9..f5e6619dee33 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 @@ -19,6 +19,7 @@ import "google/protobuf/struct.proto"; import "google/protobuf/wrappers.proto"; import "envoy/annotations/deprecation.proto"; +import "udpa/annotations/security.proto"; import "udpa/annotations/status.proto"; import "udpa/annotations/versioning.proto"; import "validate/validate.proto"; @@ -281,13 +282,15 @@ message HttpConnectionManager { // Additional settings for HTTP requests handled by the connection manager. These will be // applicable to both HTTP1 and HTTP2 requests. - config.core.v4alpha.HttpProtocolOptions common_http_protocol_options = 35; + config.core.v4alpha.HttpProtocolOptions common_http_protocol_options = 35 + [(udpa.annotations.security).configure_for_untrusted_downstream = true]; // Additional HTTP/1 settings that are passed to the HTTP/1 codec. config.core.v4alpha.Http1ProtocolOptions http_protocol_options = 8; // Additional HTTP/2 settings that are passed directly to the HTTP/2 codec. - config.core.v4alpha.Http2ProtocolOptions http2_protocol_options = 9; + config.core.v4alpha.Http2ProtocolOptions http2_protocol_options = 9 + [(udpa.annotations.security).configure_for_untrusted_downstream = true]; // An optional override that the connection manager will write to the server // header in responses. If not set, the default is *envoy*. @@ -332,13 +335,15 @@ message HttpConnectionManager { // // A value of 0 will completely disable the connection manager stream idle // timeout, although per-route idle timeout overrides will continue to apply. - google.protobuf.Duration stream_idle_timeout = 24; + google.protobuf.Duration stream_idle_timeout = 24 + [(udpa.annotations.security).configure_for_untrusted_downstream = true]; // The amount of time that Envoy will wait for the entire request to be received. // The timer is activated when the request is initiated, and is disarmed when the last byte of the // request is sent upstream (i.e. all decoding filters have processed the request), OR when the // response is initiated. If not specified or set to 0, this timeout is disabled. - google.protobuf.Duration request_timeout = 28; + google.protobuf.Duration request_timeout = 28 + [(udpa.annotations.security).configure_for_untrusted_downstream = true]; // The time that Envoy will wait between sending an HTTP/2 “shutdown // notification” (GOAWAY frame with max stream ID) and a final GOAWAY frame. @@ -394,7 +399,8 @@ message HttpConnectionManager { // :ref:`config_http_conn_man_headers_x-forwarded-for`, // :ref:`config_http_conn_man_headers_x-envoy-internal`, and // :ref:`config_http_conn_man_headers_x-envoy-external-address` for more information. - google.protobuf.BoolValue use_remote_address = 14; + google.protobuf.BoolValue use_remote_address = 14 + [(udpa.annotations.security).configure_for_untrusted_downstream = true]; // The number of additional ingress proxy hops from the right side of the // :ref:`config_http_conn_man_headers_x-forwarded-for` HTTP header to trust when diff --git a/docs/BUILD b/docs/BUILD index d190c0a59a0a..ead7bddb9a7f 100644 --- a/docs/BUILD +++ b/docs/BUILD @@ -1,3 +1,3 @@ licenses(["notice"]) # Apache 2 -exports_files(["edge_defaults_manifest.yaml"]) +exports_files(["protodoc_manifest.yaml"]) diff --git a/docs/edge_defaults_manifest.yaml b/docs/edge_defaults_manifest.yaml deleted file mode 100644 index b5072c26a32b..000000000000 --- a/docs/edge_defaults_manifest.yaml +++ /dev/null @@ -1,21 +0,0 @@ -envoy.config.bootstrap.v3.Bootstrap.overload_manager: - refresh_interval: 0.25s - resource_monitors: - - name: "envoy.resource_monitors.fixed_heap" - typed_config: - "@type": type.googleapis.com/envoy.config.resource_monitor.fixed_heap.v2alpha.FixedHeapConfig - # TODO: Tune for your system. - max_heap_size_bytes: 2147483648 # 2 GiB - actions: - - name: "envoy.overload_actions.shrink_heap" - triggers: - - name: "envoy.resource_monitors.fixed_heap" - threshold: - value: 0.95 - - name: "envoy.overload_actions.stop_accepting_requests" - triggers: - - name: "envoy.resource_monitors.fixed_heap" - threshold: - value: 0.98 - -envoy.config.listener.v3.Listener.per_connection_buffer_limit_bytes: 32768 # 32 KiB diff --git a/docs/protodoc_manifest.yaml b/docs/protodoc_manifest.yaml new file mode 100644 index 000000000000..2e2afff3264d --- /dev/null +++ b/docs/protodoc_manifest.yaml @@ -0,0 +1,51 @@ +fields: + envoy.config.bootstrap.v3.Bootstrap.overload_manager: + edge_config: + example: + refresh_interval: 0.25s + resource_monitors: + - name: "envoy.resource_monitors.fixed_heap" + typed_config: + "@type": type.googleapis.com/envoy.config.resource_monitor.fixed_heap.v2alpha.FixedHeapConfig + max_heap_size_bytes: 1073741824 + actions: + - name: "envoy.overload_actions.shrink_heap" + triggers: + - name: "envoy.resource_monitors.fixed_heap" + threshold: + value: 0.90 + - name: "envoy.overload_actions.stop_accepting_requests" + triggers: + - name: "envoy.resource_monitors.fixed_heap" + threshold: + value: 0.95 + envoy.config.cluster.v3.Cluster.per_connection_buffer_limit_bytes: + edge_config: { example: 32768 } + envoy.config.cluster.v3.Cluster.http2_protocol_options: + edge_config: + example: + initial_stream_window_size: 65536 # 64 KiB + initial_connection_window_size: 1048576 # 1 MiB + envoy.config.listener.v3.Listener.per_connection_buffer_limit_bytes: + edge_config: { example: 32768 } + envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager.common_http_protocol_options: + edge_config: + example: + idle_timeout: 900s # 15 mins + headers_with_underscores_action: REJECT_REQUEST + envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager.http2_protocol_options: + edge_config: + example: + max_concurrent_streams: 100 + initial_stream_window_size: 65536 # 64 KiB + initial_connection_window_size: 1048576 # 1 MiB + envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager.stream_idle_timeout: + edge_config: + example: 300s # 5 mins + envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager.request_timeout: + edge_config: + note: > + This timeout is not compatible with streaming requests. + example: 300s # 5 mins + envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager.use_remote_address: + edge_config: { example: true } diff --git a/docs/root/version_history/v1.12.4.rst b/docs/root/version_history/v1.12.4.rst new file mode 100644 index 000000000000..1635bbb5f000 --- /dev/null +++ b/docs/root/version_history/v1.12.4.rst @@ -0,0 +1,8 @@ +1.12.4 (June 8, 2020) +===================== + +Changes +------- + +* http: added :ref:`headers_with_underscores_action setting ` to control how client requests with header names containing underscore characters are handled. The options are to allow such headers, reject request or drop headers. The default is to allow headers, preserving existing behavior. +* http: fixed CVE-2020-11080 by rejecting HTTP/2 SETTINGS frames with too many parameters. diff --git a/docs/root/version_history/v1.13.2.rst b/docs/root/version_history/v1.13.2.rst new file mode 100644 index 000000000000..641bbaa451d4 --- /dev/null +++ b/docs/root/version_history/v1.13.2.rst @@ -0,0 +1,8 @@ +1.13.2 (June 8, 2020) +===================== + +Changes +------- + +* http: added :ref:`headers_with_underscores_action setting ` to control how client requests with header names containing underscore characters are handled. The options are to allow such headers, reject request or drop headers. The default is to allow headers, preserving existing behavior. +* http: fixed CVE-2020-11080 by rejecting HTTP/2 SETTINGS frames with too many parameters. diff --git a/docs/root/version_history/v1.14.2.rst b/docs/root/version_history/v1.14.2.rst new file mode 100644 index 000000000000..18bdf0bfce9d --- /dev/null +++ b/docs/root/version_history/v1.14.2.rst @@ -0,0 +1,7 @@ +1.14.2 (June 8, 2020) +===================== + +Changes +------- + +* http: fixed CVE-2020-11080 by rejecting HTTP/2 SETTINGS frames with too many parameters. diff --git a/docs/root/version_history/version_history.rst b/docs/root/version_history/version_history.rst index 6451336bffe7..527dec86ca8d 100644 --- a/docs/root/version_history/version_history.rst +++ b/docs/root/version_history/version_history.rst @@ -7,10 +7,13 @@ Version history :titlesonly: current + v1.14.2 v1.14.1 v1.14.0 + v1.13.2 v1.13.1 v1.13.0 + v1.12.4 v1.12.3 v1.12.2 v1.12.1 diff --git a/generated_api_shadow/envoy/config/cluster/v3/cluster.proto b/generated_api_shadow/envoy/config/cluster/v3/cluster.proto index f512cbcc9d22..8140007f68af 100644 --- a/generated_api_shadow/envoy/config/cluster/v3/cluster.proto +++ b/generated_api_shadow/envoy/config/cluster/v3/cluster.proto @@ -20,6 +20,7 @@ import "google/protobuf/struct.proto"; import "google/protobuf/wrappers.proto"; import "envoy/annotations/deprecation.proto"; +import "udpa/annotations/security.proto"; import "udpa/annotations/status.proto"; import "udpa/annotations/versioning.proto"; import "validate/validate.proto"; @@ -582,7 +583,8 @@ message Cluster { // Soft limit on size of the cluster’s connections read and write buffers. If // unspecified, an implementation defined default is applied (1MiB). - google.protobuf.UInt32Value per_connection_buffer_limit_bytes = 5; + google.protobuf.UInt32Value per_connection_buffer_limit_bytes = 5 + [(udpa.annotations.security).configure_for_untrusted_upstream = true]; // The :ref:`load balancer type ` to use // when picking a host in the cluster. @@ -633,7 +635,8 @@ message Cluster { // supports prior knowledge for upstream connections. Even if TLS is used // with ALPN, `http2_protocol_options` must be specified. As an aside this allows HTTP/2 // connections to happen over plain text. - core.v3.Http2ProtocolOptions http2_protocol_options = 14; + core.v3.Http2ProtocolOptions http2_protocol_options = 14 + [(udpa.annotations.security).configure_for_untrusted_upstream = true]; // The extension_protocol_options field is used to provide extension-specific protocol options // for upstream connections. The key should match the extension filter name, such as diff --git a/generated_api_shadow/envoy/config/cluster/v4alpha/cluster.proto b/generated_api_shadow/envoy/config/cluster/v4alpha/cluster.proto index 2b044b2c6437..eab2f2d80fcb 100644 --- a/generated_api_shadow/envoy/config/cluster/v4alpha/cluster.proto +++ b/generated_api_shadow/envoy/config/cluster/v4alpha/cluster.proto @@ -19,6 +19,7 @@ import "google/protobuf/struct.proto"; import "google/protobuf/wrappers.proto"; import "envoy/annotations/deprecation.proto"; +import "udpa/annotations/security.proto"; import "udpa/annotations/status.proto"; import "udpa/annotations/versioning.proto"; import "validate/validate.proto"; @@ -585,7 +586,8 @@ message Cluster { // Soft limit on size of the cluster’s connections read and write buffers. If // unspecified, an implementation defined default is applied (1MiB). - google.protobuf.UInt32Value per_connection_buffer_limit_bytes = 5; + google.protobuf.UInt32Value per_connection_buffer_limit_bytes = 5 + [(udpa.annotations.security).configure_for_untrusted_upstream = true]; // The :ref:`load balancer type ` to use // when picking a host in the cluster. @@ -636,7 +638,8 @@ message Cluster { // supports prior knowledge for upstream connections. Even if TLS is used // with ALPN, `http2_protocol_options` must be specified. As an aside this allows HTTP/2 // connections to happen over plain text. - core.v4alpha.Http2ProtocolOptions http2_protocol_options = 14; + core.v4alpha.Http2ProtocolOptions http2_protocol_options = 14 + [(udpa.annotations.security).configure_for_untrusted_upstream = true]; // The extension_protocol_options field is used to provide extension-specific protocol options // for upstream connections. The key should match the extension filter name, such as 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 8efa78bf0eb9..1362850f0530 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 @@ -19,6 +19,7 @@ import "google/protobuf/struct.proto"; import "google/protobuf/wrappers.proto"; import "envoy/annotations/deprecation.proto"; +import "udpa/annotations/security.proto"; import "udpa/annotations/status.proto"; import "udpa/annotations/versioning.proto"; import "validate/validate.proto"; @@ -283,13 +284,15 @@ message HttpConnectionManager { // Additional settings for HTTP requests handled by the connection manager. These will be // applicable to both HTTP1 and HTTP2 requests. - config.core.v3.HttpProtocolOptions common_http_protocol_options = 35; + config.core.v3.HttpProtocolOptions common_http_protocol_options = 35 + [(udpa.annotations.security).configure_for_untrusted_downstream = true]; // Additional HTTP/1 settings that are passed to the HTTP/1 codec. config.core.v3.Http1ProtocolOptions http_protocol_options = 8; // Additional HTTP/2 settings that are passed directly to the HTTP/2 codec. - config.core.v3.Http2ProtocolOptions http2_protocol_options = 9; + config.core.v3.Http2ProtocolOptions http2_protocol_options = 9 + [(udpa.annotations.security).configure_for_untrusted_downstream = true]; // An optional override that the connection manager will write to the server // header in responses. If not set, the default is *envoy*. @@ -334,13 +337,15 @@ message HttpConnectionManager { // // A value of 0 will completely disable the connection manager stream idle // timeout, although per-route idle timeout overrides will continue to apply. - google.protobuf.Duration stream_idle_timeout = 24; + google.protobuf.Duration stream_idle_timeout = 24 + [(udpa.annotations.security).configure_for_untrusted_downstream = true]; // The amount of time that Envoy will wait for the entire request to be received. // The timer is activated when the request is initiated, and is disarmed when the last byte of the // request is sent upstream (i.e. all decoding filters have processed the request), OR when the // response is initiated. If not specified or set to 0, this timeout is disabled. - google.protobuf.Duration request_timeout = 28; + google.protobuf.Duration request_timeout = 28 + [(udpa.annotations.security).configure_for_untrusted_downstream = true]; // The time that Envoy will wait between sending an HTTP/2 “shutdown // notification” (GOAWAY frame with max stream ID) and a final GOAWAY frame. @@ -396,7 +401,8 @@ message HttpConnectionManager { // :ref:`config_http_conn_man_headers_x-forwarded-for`, // :ref:`config_http_conn_man_headers_x-envoy-internal`, and // :ref:`config_http_conn_man_headers_x-envoy-external-address` for more information. - google.protobuf.BoolValue use_remote_address = 14; + google.protobuf.BoolValue use_remote_address = 14 + [(udpa.annotations.security).configure_for_untrusted_downstream = true]; // The number of additional ingress proxy hops from the right side of the // :ref:`config_http_conn_man_headers_x-forwarded-for` HTTP header to trust when 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 482bb5ed95e9..f5e6619dee33 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 @@ -19,6 +19,7 @@ import "google/protobuf/struct.proto"; import "google/protobuf/wrappers.proto"; import "envoy/annotations/deprecation.proto"; +import "udpa/annotations/security.proto"; import "udpa/annotations/status.proto"; import "udpa/annotations/versioning.proto"; import "validate/validate.proto"; @@ -281,13 +282,15 @@ message HttpConnectionManager { // Additional settings for HTTP requests handled by the connection manager. These will be // applicable to both HTTP1 and HTTP2 requests. - config.core.v4alpha.HttpProtocolOptions common_http_protocol_options = 35; + config.core.v4alpha.HttpProtocolOptions common_http_protocol_options = 35 + [(udpa.annotations.security).configure_for_untrusted_downstream = true]; // Additional HTTP/1 settings that are passed to the HTTP/1 codec. config.core.v4alpha.Http1ProtocolOptions http_protocol_options = 8; // Additional HTTP/2 settings that are passed directly to the HTTP/2 codec. - config.core.v4alpha.Http2ProtocolOptions http2_protocol_options = 9; + config.core.v4alpha.Http2ProtocolOptions http2_protocol_options = 9 + [(udpa.annotations.security).configure_for_untrusted_downstream = true]; // An optional override that the connection manager will write to the server // header in responses. If not set, the default is *envoy*. @@ -332,13 +335,15 @@ message HttpConnectionManager { // // A value of 0 will completely disable the connection manager stream idle // timeout, although per-route idle timeout overrides will continue to apply. - google.protobuf.Duration stream_idle_timeout = 24; + google.protobuf.Duration stream_idle_timeout = 24 + [(udpa.annotations.security).configure_for_untrusted_downstream = true]; // The amount of time that Envoy will wait for the entire request to be received. // The timer is activated when the request is initiated, and is disarmed when the last byte of the // request is sent upstream (i.e. all decoding filters have processed the request), OR when the // response is initiated. If not specified or set to 0, this timeout is disabled. - google.protobuf.Duration request_timeout = 28; + google.protobuf.Duration request_timeout = 28 + [(udpa.annotations.security).configure_for_untrusted_downstream = true]; // The time that Envoy will wait between sending an HTTP/2 “shutdown // notification” (GOAWAY frame with max stream ID) and a final GOAWAY frame. @@ -394,7 +399,8 @@ message HttpConnectionManager { // :ref:`config_http_conn_man_headers_x-forwarded-for`, // :ref:`config_http_conn_man_headers_x-envoy-internal`, and // :ref:`config_http_conn_man_headers_x-envoy-external-address` for more information. - google.protobuf.BoolValue use_remote_address = 14; + google.protobuf.BoolValue use_remote_address = 14 + [(udpa.annotations.security).configure_for_untrusted_downstream = true]; // The number of additional ingress proxy hops from the right side of the // :ref:`config_http_conn_man_headers_x-forwarded-for` HTTP header to trust when diff --git a/include/envoy/network/address.h b/include/envoy/network/address.h index 243c877045d6..136b10f3cf3e 100644 --- a/include/envoy/network/address.h +++ b/include/envoy/network/address.h @@ -116,7 +116,6 @@ class Pipe { }; enum class Type { Ip, Pipe }; -enum class SocketType { Stream, Datagram }; /** * Interface for all network addresses. diff --git a/include/envoy/network/listener.h b/include/envoy/network/listener.h index 2f511eb99a77..ba8f27918cb7 100644 --- a/include/envoy/network/listener.h +++ b/include/envoy/network/listener.h @@ -37,7 +37,7 @@ class ListenSocketFactory { /** * @return the type of the socket getListenSocket() returns. */ - virtual Address::SocketType socketType() const PURE; + virtual Socket::Type socketType() const PURE; /** * @return the listening address of the socket getListenSocket() returns. Before getListenSocket() diff --git a/include/envoy/network/socket.h b/include/envoy/network/socket.h index d2d87928e98e..07c09474038b 100644 --- a/include/envoy/network/socket.h +++ b/include/envoy/network/socket.h @@ -49,6 +49,11 @@ class Socket { public: virtual ~Socket() = default; + /** + * Type of sockets supported. See man 2 socket for more details + */ + enum class Type { Stream, Datagram }; + /** * @return the local address of the socket. */ @@ -76,7 +81,7 @@ class Socket { /** * @return the type (stream or datagram) of the socket. */ - virtual Address::SocketType socketType() const PURE; + virtual Socket::Type socketType() const PURE; /** * @return the type (IP or pipe) of addresses used by the socket (subset of socket domain) @@ -234,7 +239,7 @@ class SocketInterface { * @param version IP version if address type is IP * @return @ref Network::IoHandlePtr that wraps the underlying socket file descriptor */ - virtual IoHandlePtr socket(Address::SocketType type, Address::Type addr_type, + virtual IoHandlePtr socket(Socket::Type type, Address::Type addr_type, Address::IpVersion version) PURE; /** @@ -244,7 +249,7 @@ class SocketInterface { * @param addr address that is gleaned for address type and version if needed * @return @ref Network::IoHandlePtr that wraps the underlying socket file descriptor */ - virtual IoHandlePtr socket(Address::SocketType socket_type, + virtual IoHandlePtr socket(Socket::Type socket_type, const Address::InstanceConstSharedPtr addr) PURE; /** diff --git a/include/envoy/server/BUILD b/include/envoy/server/BUILD index 8803acb34cc5..dd72215ae18e 100644 --- a/include/envoy/server/BUILD +++ b/include/envoy/server/BUILD @@ -154,8 +154,8 @@ envoy_cc_library( ) envoy_cc_library( - name = "filter_config_interface", - hdrs = ["filter_config.h"], + name = "factory_context_interface", + hdrs = ["factory_context.h"], deps = [ ":admin_interface", ":drain_manager_interface", @@ -173,7 +173,6 @@ envoy_cc_library( "//include/envoy/network:drain_decision_interface", "//include/envoy/runtime:runtime_interface", "//include/envoy/server:overload_manager_interface", - "//include/envoy/server:transport_socket_config_interface", "//include/envoy/singleton:manager_interface", "//include/envoy/thread_local:thread_local_interface", "//include/envoy/tracing:http_tracer_interface", @@ -185,6 +184,30 @@ envoy_cc_library( ], ) +envoy_cc_library( + name = "filter_config_interface", + hdrs = ["filter_config.h"], + deps = [ + ":drain_manager_interface", + ":factory_context_interface", + ":lifecycle_notifier_interface", + ":process_context_interface", + "//include/envoy/access_log:access_log_interface", + "//include/envoy/config:typed_config_interface", + "//include/envoy/http:codes_interface", + "//include/envoy/http:filter_interface", + "//include/envoy/server:overload_manager_interface", + "//include/envoy/server:transport_socket_config_interface", + "//include/envoy/singleton:manager_interface", + "//include/envoy/thread_local:thread_local_interface", + "//include/envoy/tracing:http_tracer_interface", + "//include/envoy/upstream:cluster_manager_interface", + "//source/common/common:assert_lib", + "//source/common/common:macros", + "//source/common/protobuf", + ], +) + envoy_cc_library( name = "lifecycle_notifier_interface", hdrs = ["lifecycle_notifier.h"], @@ -220,6 +243,7 @@ envoy_cc_library( name = "transport_socket_config_interface", hdrs = ["transport_socket_config.h"], deps = [ + ":factory_context_interface", "//include/envoy/config:typed_config_interface", "//include/envoy/event:dispatcher_interface", "//include/envoy/init:manager_interface", @@ -298,7 +322,7 @@ envoy_cc_library( name = "bootstrap_extension_config_interface", hdrs = ["bootstrap_extension_config.h"], deps = [ - ":filter_config_interface", + ":factory_context_interface", "//include/envoy/config:typed_config_interface", ], ) diff --git a/include/envoy/server/bootstrap_extension_config.h b/include/envoy/server/bootstrap_extension_config.h index 9b0d6e043396..7eaf4dcb2530 100644 --- a/include/envoy/server/bootstrap_extension_config.h +++ b/include/envoy/server/bootstrap_extension_config.h @@ -2,7 +2,7 @@ #include -#include "envoy/server/filter_config.h" +#include "envoy/server/factory_context.h" #include "common/protobuf/protobuf.h" diff --git a/include/envoy/server/factory_context.h b/include/envoy/server/factory_context.h new file mode 100644 index 000000000000..245499464e1d --- /dev/null +++ b/include/envoy/server/factory_context.h @@ -0,0 +1,265 @@ +#pragma once + +#include + +#include "envoy/access_log/access_log.h" +#include "envoy/config/core/v3/base.pb.h" +#include "envoy/config/typed_config.h" +#include "envoy/grpc/context.h" +#include "envoy/http/codes.h" +#include "envoy/http/context.h" +#include "envoy/http/filter.h" +#include "envoy/init/manager.h" +#include "envoy/network/drain_decision.h" +#include "envoy/network/filter.h" +#include "envoy/runtime/runtime.h" +#include "envoy/server/admin.h" +#include "envoy/server/drain_manager.h" +#include "envoy/server/lifecycle_notifier.h" +#include "envoy/server/overload_manager.h" +#include "envoy/server/process_context.h" +#include "envoy/singleton/manager.h" +#include "envoy/stats/scope.h" +#include "envoy/thread_local/thread_local.h" +#include "envoy/tracing/http_tracer.h" +#include "envoy/upstream/cluster_manager.h" + +#include "common/common/assert.h" +#include "common/common/macros.h" +#include "common/protobuf/protobuf.h" + +namespace Envoy { +namespace Server { +namespace Configuration { + +/** + * Common interface for downstream and upstream network filters. + */ +class CommonFactoryContext { +public: + virtual ~CommonFactoryContext() = default; + + /** + * @return Upstream::ClusterManager& singleton for use by the entire server. + */ + virtual Upstream::ClusterManager& clusterManager() PURE; + + /** + * @return Event::Dispatcher& the main thread's dispatcher. This dispatcher should be used + * for all singleton processing. + */ + virtual Event::Dispatcher& dispatcher() PURE; + + /** + * @return information about the local environment the server is running in. + */ + virtual const LocalInfo::LocalInfo& localInfo() const PURE; + + /** + * @return ProtobufMessage::ValidationContext& validation visitor for xDS and static configuration + * messages. + */ + virtual ProtobufMessage::ValidationContext& messageValidationContext() PURE; + + /** + * @return RandomGenerator& the random generator for the server. + */ + virtual Envoy::Runtime::RandomGenerator& random() PURE; + + /** + * @return Runtime::Loader& the singleton runtime loader for the server. + */ + virtual Envoy::Runtime::Loader& runtime() PURE; + + /** + * @return Stats::Scope& the filter's stats scope. + */ + virtual Stats::Scope& scope() PURE; + + /** + * @return Singleton::Manager& the server-wide singleton manager. + */ + virtual Singleton::Manager& singletonManager() PURE; + + /** + * @return ThreadLocal::SlotAllocator& the thread local storage engine for the server. This is + * used to allow runtime lockless updates to configuration, etc. across multiple threads. + */ + virtual ThreadLocal::SlotAllocator& threadLocal() PURE; + + /** + * @return Server::Admin& the server's global admin HTTP endpoint. + */ + virtual Server::Admin& admin() PURE; + + /** + * @return TimeSource& a reference to the time source. + */ + virtual TimeSource& timeSource() PURE; + + /** + * @return Api::Api& a reference to the api object. + */ + virtual Api::Api& api() PURE; +}; + +/** + * ServerFactoryContext is an specialization of common interface for downstream and upstream network + * filters. The implementation guarantees the lifetime is no shorter than server. It could be used + * across listeners. + */ +class ServerFactoryContext : public virtual CommonFactoryContext { +public: + ~ServerFactoryContext() override = default; + + /** + * @return the server-wide grpc context. + */ + virtual Grpc::Context& grpcContext() PURE; + + /** + * @return DrainManager& the server-wide drain manager. + */ + virtual Envoy::Server::DrainManager& drainManager() PURE; + + /** + * @return the server's init manager. This can be used for extensions that need to initialize + * after cluster manager init but before the server starts listening. All extensions + * should register themselves during configuration load. initialize() will be called on + * each registered target after cluster manager init but before the server starts + * listening. Once all targets have initialized and invoked their callbacks, the server + * will start listening. + */ + virtual Init::Manager& initManager() PURE; + + /** + * @return ServerLifecycleNotifier& the lifecycle notifier for the server. + */ + virtual ServerLifecycleNotifier& lifecycleNotifier() PURE; +}; + +/** + * Context passed to network and HTTP filters to access server resources. + * TODO(mattklein123): When we lock down visibility of the rest of the code, filters should only + * access the rest of the server via interfaces exposed here. + */ +class FactoryContext : public virtual CommonFactoryContext { +public: + ~FactoryContext() override = default; + + /** + * @return ServerFactoryContext which lifetime is no shorter than the server. + */ + virtual ServerFactoryContext& getServerFactoryContext() const PURE; + + /** + * @return TransportSocketFactoryContext which lifetime is no shorter than the server. + */ + virtual TransportSocketFactoryContext& getTransportSocketFactoryContext() const PURE; + + /** + * @return AccessLogManager for use by the entire server. + */ + virtual AccessLog::AccessLogManager& accessLogManager() PURE; + + /** + * @return envoy::config::core::v3::TrafficDirection the direction of the traffic relative to + * the local proxy. + */ + virtual envoy::config::core::v3::TrafficDirection direction() const PURE; + + /** + * @return const Network::DrainDecision& a drain decision that filters can use to determine if + * they should be doing graceful closes on connections when possible. + */ + virtual const Network::DrainDecision& drainDecision() PURE; + + /** + * @return whether external healthchecks are currently failed or not. + */ + virtual bool healthCheckFailed() PURE; + + /** + * @return the server's init manager. This can be used for extensions that need to initialize + * after cluster manager init but before the server starts listening. All extensions + * should register themselves during configuration load. initialize() will be called on + * each registered target after cluster manager init but before the server starts + * listening. Once all targets have initialized and invoked their callbacks, the server + * will start listening. + */ + virtual Init::Manager& initManager() PURE; + + /** + * @return ServerLifecycleNotifier& the lifecycle notifier for the server. + */ + virtual ServerLifecycleNotifier& lifecycleNotifier() PURE; + + /** + * @return Stats::Scope& the listener's stats scope. + */ + virtual Stats::Scope& listenerScope() PURE; + + /** + * @return const envoy::config::core::v3::Metadata& the config metadata associated with this + * listener. + */ + virtual const envoy::config::core::v3::Metadata& listenerMetadata() const PURE; + + /** + * @return OverloadManager& the overload manager for the server. + */ + virtual OverloadManager& overloadManager() PURE; + + /** + * @return Http::Context& a reference to the http context. + */ + virtual Http::Context& httpContext() PURE; + + /** + * @return Grpc::Context& a reference to the grpc context. + */ + virtual Grpc::Context& grpcContext() PURE; + + /** + * @return ProcessContextOptRef an optional reference to the + * process context. Will be unset when running in validation mode. + */ + virtual ProcessContextOptRef processContext() PURE; + + /** + * @return ProtobufMessage::ValidationVisitor& validation visitor for filter configuration + * messages. + */ + virtual ProtobufMessage::ValidationVisitor& messageValidationVisitor() PURE; +}; + +/** + * An implementation of FactoryContext. The life time is no shorter than the created filter chains. + * The life time is no longer than the owning listener. It should be used to create + * NetworkFilterChain. + */ +class FilterChainFactoryContext : public virtual FactoryContext { +public: + /** + * Set the flag that all attached filter chains will be destroyed. + */ + virtual void startDraining() PURE; +}; + +using FilterChainFactoryContextPtr = std::unique_ptr; + +/** + * An implementation of FactoryContext. The life time should cover the lifetime of the filter chains + * and connections. It can be used to create ListenerFilterChain. + */ +class ListenerFactoryContext : public virtual FactoryContext { +public: + /** + * Give access to the listener configuration + */ + virtual const Network::ListenerConfig& listenerConfig() const PURE; +}; + +} // namespace Configuration +} // namespace Server +} // namespace Envoy diff --git a/include/envoy/server/filter_config.h b/include/envoy/server/filter_config.h index 75d8dd24d371..343e6c87ad28 100644 --- a/include/envoy/server/filter_config.h +++ b/include/envoy/server/filter_config.h @@ -2,28 +2,12 @@ #include -#include "envoy/access_log/access_log.h" -#include "envoy/config/core/v3/base.pb.h" #include "envoy/config/typed_config.h" -#include "envoy/grpc/context.h" -#include "envoy/http/codes.h" -#include "envoy/http/context.h" #include "envoy/http/filter.h" #include "envoy/init/manager.h" -#include "envoy/network/drain_decision.h" #include "envoy/network/filter.h" -#include "envoy/runtime/runtime.h" -#include "envoy/server/admin.h" #include "envoy/server/drain_manager.h" -#include "envoy/server/lifecycle_notifier.h" -#include "envoy/server/overload_manager.h" -#include "envoy/server/process_context.h" -#include "envoy/server/transport_socket_config.h" -#include "envoy/singleton/manager.h" -#include "envoy/stats/scope.h" -#include "envoy/thread_local/thread_local.h" -#include "envoy/tracing/http_tracer.h" -#include "envoy/upstream/cluster_manager.h" +#include "envoy/server/factory_context.h" #include "common/common/assert.h" #include "common/common/macros.h" @@ -33,219 +17,6 @@ namespace Envoy { namespace Server { namespace Configuration { -/** - * Common interface for downstream and upstream network filters. - */ -class CommonFactoryContext { -public: - virtual ~CommonFactoryContext() = default; - - /** - * @return Upstream::ClusterManager& singleton for use by the entire server. - */ - virtual Upstream::ClusterManager& clusterManager() PURE; - - /** - * @return Event::Dispatcher& the main thread's dispatcher. This dispatcher should be used - * for all singleton processing. - */ - virtual Event::Dispatcher& dispatcher() PURE; - - /** - * @return information about the local environment the server is running in. - */ - virtual const LocalInfo::LocalInfo& localInfo() const PURE; - - /** - * @return ProtobufMessage::ValidationContext& validation visitor for xDS and static configuration - * messages. - */ - virtual ProtobufMessage::ValidationContext& messageValidationContext() PURE; - - /** - * @return RandomGenerator& the random generator for the server. - */ - virtual Envoy::Runtime::RandomGenerator& random() PURE; - - /** - * @return Runtime::Loader& the singleton runtime loader for the server. - */ - virtual Envoy::Runtime::Loader& runtime() PURE; - - /** - * @return Stats::Scope& the filter's stats scope. - */ - virtual Stats::Scope& scope() PURE; - - /** - * @return Singleton::Manager& the server-wide singleton manager. - */ - virtual Singleton::Manager& singletonManager() PURE; - - /** - * @return ThreadLocal::SlotAllocator& the thread local storage engine for the server. This is - * used to allow runtime lockless updates to configuration, etc. across multiple threads. - */ - virtual ThreadLocal::SlotAllocator& threadLocal() PURE; - - /** - * @return Server::Admin& the server's global admin HTTP endpoint. - */ - virtual Server::Admin& admin() PURE; - - /** - * @return TimeSource& a reference to the time source. - */ - virtual TimeSource& timeSource() PURE; - - /** - * @return Api::Api& a reference to the api object. - */ - virtual Api::Api& api() PURE; -}; - -/** - * ServerFactoryContext is an specialization of common interface for downstream and upstream network - * filters. The implementation guarantees the lifetime is no shorter than server. It could be used - * across listeners. - */ -class ServerFactoryContext : public virtual CommonFactoryContext { -public: - ~ServerFactoryContext() override = default; - - /** - * @return the server-wide grpc context. - */ - virtual Grpc::Context& grpcContext() PURE; - - /** - * @return DrainManager& the server-wide drain manager. - */ - virtual Envoy::Server::DrainManager& drainManager() PURE; -}; - -/** - * Context passed to network and HTTP filters to access server resources. - * TODO(mattklein123): When we lock down visibility of the rest of the code, filters should only - * access the rest of the server via interfaces exposed here. - */ -class FactoryContext : public virtual CommonFactoryContext { -public: - ~FactoryContext() override = default; - - /** - * @return ServerFactoryContext which lifetime is no shorter than the server. - */ - virtual ServerFactoryContext& getServerFactoryContext() const PURE; - - /** - * @return TransportSocketFactoryContext which lifetime is no shorter than the server. - */ - virtual TransportSocketFactoryContext& getTransportSocketFactoryContext() const PURE; - - /** - * @return AccessLogManager for use by the entire server. - */ - virtual AccessLog::AccessLogManager& accessLogManager() PURE; - - /** - * @return envoy::config::core::v3::TrafficDirection the direction of the traffic relative to - * the local proxy. - */ - virtual envoy::config::core::v3::TrafficDirection direction() const PURE; - - /** - * @return const Network::DrainDecision& a drain decision that filters can use to determine if - * they should be doing graceful closes on connections when possible. - */ - virtual const Network::DrainDecision& drainDecision() PURE; - - /** - * @return whether external healthchecks are currently failed or not. - */ - virtual bool healthCheckFailed() PURE; - - /** - * @return the server's init manager. This can be used for extensions that need to initialize - * after cluster manager init but before the server starts listening. All extensions - * should register themselves during configuration load. initialize() will be called on - * each registered target after cluster manager init but before the server starts - * listening. Once all targets have initialized and invoked their callbacks, the server - * will start listening. - */ - virtual Init::Manager& initManager() PURE; - - /** - * @return ServerLifecycleNotifier& the lifecycle notifier for the server. - */ - virtual ServerLifecycleNotifier& lifecycleNotifier() PURE; - - /** - * @return Stats::Scope& the listener's stats scope. - */ - virtual Stats::Scope& listenerScope() PURE; - - /** - * @return const envoy::config::core::v3::Metadata& the config metadata associated with this - * listener. - */ - virtual const envoy::config::core::v3::Metadata& listenerMetadata() const PURE; - - /** - * @return OverloadManager& the overload manager for the server. - */ - virtual OverloadManager& overloadManager() PURE; - - /** - * @return Http::Context& a reference to the http context. - */ - virtual Http::Context& httpContext() PURE; - - /** - * @return Grpc::Context& a reference to the grpc context. - */ - virtual Grpc::Context& grpcContext() PURE; - - /** - * @return ProcessContextOptRef an optional reference to the - * process context. Will be unset when running in validation mode. - */ - virtual ProcessContextOptRef processContext() PURE; - - /** - * @return ProtobufMessage::ValidationVisitor& validation visitor for filter configuration - * messages. - */ - virtual ProtobufMessage::ValidationVisitor& messageValidationVisitor() PURE; -}; - -/** - * An implementation of FactoryContext. The life time is no shorter than the created filter chains. - * The life time is no longer than the owning listener. It should be used to create - * NetworkFilterChain. - */ -class FilterChainFactoryContext : public virtual FactoryContext { -public: - /** - * Set the flag that all attached filter chains will be destroyed. - */ - virtual void startDraining() PURE; -}; - -using FilterChainFactoryContextPtr = std::unique_ptr; - -/** - * An implementation of FactoryContext. The life time should cover the lifetime of the filter chains - * and connections. It can be used to create ListenerFilterChain. - */ -class ListenerFactoryContext : public virtual FactoryContext { -public: - /** - * Give access to the listener configuration - */ - virtual const Network::ListenerConfig& listenerConfig() const PURE; -}; - /** * Common interface for listener filters and UDP listener filters */ diff --git a/include/envoy/server/listener_manager.h b/include/envoy/server/listener_manager.h index 57a7e97549a2..956a89264ac4 100644 --- a/include/envoy/server/listener_manager.h +++ b/include/envoy/server/listener_manager.h @@ -71,7 +71,7 @@ class ListenerComponentFactory { */ virtual Network::SocketSharedPtr createListenSocket(Network::Address::InstanceConstSharedPtr address, - Network::Address::SocketType socket_type, + Network::Socket::Type socket_type, const Network::Socket::OptionsSharedPtr& options, const ListenSocketCreationParams& params) PURE; diff --git a/include/envoy/server/transport_socket_config.h b/include/envoy/server/transport_socket_config.h index a3dd4d5dac6d..ac3337738017 100644 --- a/include/envoy/server/transport_socket_config.h +++ b/include/envoy/server/transport_socket_config.h @@ -9,6 +9,7 @@ #include "envoy/network/transport_socket.h" #include "envoy/runtime/runtime.h" #include "envoy/secret/secret_manager.h" +#include "envoy/server/factory_context.h" #include "envoy/singleton/manager.h" #include "envoy/ssl/context_manager.h" #include "envoy/stats/scope.h" @@ -74,10 +75,9 @@ class TransportSocketFactoryContext { virtual Stats::Store& stats() PURE; /** - * @return a pointer pointing to the instance of an init manager, or nullptr - * if not set. + * @return a reference to the instance of an init manager. */ - virtual Init::Manager* initManager() PURE; + virtual Init::Manager& initManager() PURE; /** * @return the server's singleton manager. diff --git a/include/envoy/thread/thread.h b/include/envoy/thread/thread.h index 70452ca5d29a..8633c03e1ebe 100644 --- a/include/envoy/thread/thread.h +++ b/include/envoy/thread/thread.h @@ -9,6 +9,9 @@ #include "common/common/thread_annotations.h" +#include "absl/strings/string_view.h" +#include "absl/types/optional.h" + namespace Envoy { namespace Thread { @@ -37,13 +40,25 @@ class Thread { virtual ~Thread() = default; /** - * Join on thread exit. + * @return the name of the thread. + */ + virtual std::string name() const PURE; + + /** + * Blocks until the thread exits. */ virtual void join() PURE; }; using ThreadPtr = std::unique_ptr; +// Options specified during thread creation. +struct Options { + std::string name_; // A name supplied for the thread. On Linux this is limited to 15 chars. +}; + +using OptionsOptConstRef = const absl::optional&; + /** * Interface providing a mechanism for creating threads. */ @@ -52,10 +67,13 @@ class ThreadFactory { virtual ~ThreadFactory() = default; /** - * Create a thread. + * Creates a thread, immediately starting the thread_routine. + * * @param thread_routine supplies the function to invoke in the thread. + * @param options supplies options specified on thread creation. */ - virtual ThreadPtr createThread(std::function thread_routine) PURE; + virtual ThreadPtr createThread(std::function thread_routine, + OptionsOptConstRef options = absl::nullopt) PURE; /** * Return the current system thread ID diff --git a/source/common/access_log/access_log_manager_impl.cc b/source/common/access_log/access_log_manager_impl.cc index f173904d2dff..055e602bdcfb 100644 --- a/source/common/access_log/access_log_manager_impl.cc +++ b/source/common/access_log/access_log_manager_impl.cc @@ -203,7 +203,8 @@ void AccessLogFileImpl::write(absl::string_view data) { } void AccessLogFileImpl::createFlushStructures() { - flush_thread_ = thread_factory_.createThread([this]() -> void { flushThreadFunc(); }); + flush_thread_ = thread_factory_.createThread([this]() -> void { flushThreadFunc(); }, + Thread::Options{"AccessLogFlush"}); flush_timer_->enableTimer(flush_interval_msec_); } diff --git a/source/common/common/posix/thread_impl.cc b/source/common/common/posix/thread_impl.cc index 324230ade176..359af8245ed9 100644 --- a/source/common/common/posix/thread_impl.cc +++ b/source/common/common/posix/thread_impl.cc @@ -1,6 +1,8 @@ #include "common/common/assert.h" #include "common/common/thread_impl.h" +#include "absl/strings/str_cat.h" + #if defined(__linux__) #include #endif @@ -24,26 +26,99 @@ int64_t getCurrentThreadId() { } // namespace -ThreadImplPosix::ThreadImplPosix(std::function thread_routine) - : thread_routine_(std::move(thread_routine)) { - RELEASE_ASSERT(Logger::Registry::initialized(), ""); - const int rc = pthread_create( - &thread_handle_, nullptr, - [](void* arg) -> void* { - static_cast(arg)->thread_routine_(); - return nullptr; - }, - this); - RELEASE_ASSERT(rc == 0, ""); -} +// See https://www.man7.org/linux/man-pages/man3/pthread_setname_np.3.html. +// The maximum thread name is 16 bytes including the terminating nul byte, +// so we need to truncate the string_view to 15 bytes. +#define PTHREAD_MAX_THREADNAME_LEN_INCLUDING_NULL_BYTE 16 -void ThreadImplPosix::join() { - const int rc = pthread_join(thread_handle_, nullptr); - RELEASE_ASSERT(rc == 0, ""); -} +/** + * Wrapper for a pthread thread. We don't use std::thread because it eats exceptions and leads to + * unusable stack traces. + */ +class ThreadImplPosix : public Thread { +public: + ThreadImplPosix(std::function thread_routine, OptionsOptConstRef options) + : thread_routine_(std::move(thread_routine)) { + if (options) { + name_ = options->name_.substr(0, PTHREAD_MAX_THREADNAME_LEN_INCLUDING_NULL_BYTE - 1); + } + RELEASE_ASSERT(Logger::Registry::initialized(), ""); + const int rc = pthread_create( + &thread_handle_, nullptr, + [](void* arg) -> void* { + static_cast(arg)->thread_routine_(); + return nullptr; + }, + this); + RELEASE_ASSERT(rc == 0, ""); + +#ifdef __linux__ + // If the name was not specified, get it from the OS. If the name was + // specified, write it into the thread, and assert that the OS sees it the + // same way. + if (name_.empty()) { + getNameFromOS(name_); + } else { + const int set_name_rc = pthread_setname_np(thread_handle_, name_.c_str()); + if (set_name_rc != 0) { + ENVOY_LOG_MISC(trace, "Error {} setting name `{}'", set_name_rc, name_); + } else { + // When compiling in debug mode, read back the thread-name from the OS, + // and verify it's what we asked for. This ensures the truncation is as + // expected, and that the OS will actually retain all the bytes of the + // name we expect. + // + // Note that the system-call to read the thread name may fail in case + // the thread exits after the call to set the name above, and before the + // call to get the name, so we can only do the assert if that call + // succeeded. + std::string check_name; + ASSERT(!getNameFromOS(check_name) || check_name == name_, + absl::StrCat("configured name=", name_, " os name=", check_name)); + } + } +#endif + } + + ~ThreadImplPosix() override { ASSERT(joined_); } + + std::string name() const override { return name_; } + + // Thread::Thread + void join() override { + ASSERT(!joined_); + joined_ = true; + const int rc = pthread_join(thread_handle_, nullptr); + RELEASE_ASSERT(rc == 0, ""); + } + +private: +#ifdef __linux__ + // Attempts to get the name from the operating system, returning true and + // updating 'name' if successful. Note that during normal operation this + // may fail, if the thread exits prior to the system call. + bool getNameFromOS(std::string& name) { + // Verify that the name got written into the thread as expected. + char buf[PTHREAD_MAX_THREADNAME_LEN_INCLUDING_NULL_BYTE]; + const int get_name_rc = pthread_getname_np(thread_handle_, buf, sizeof(buf)); + if (get_name_rc != 0) { + ENVOY_LOG_MISC(trace, "Error {} getting name", get_name_rc); + return false; + } + name = buf; + return true; + } +#endif + + std::function thread_routine_; + pthread_t thread_handle_; + std::string name_; + bool joined_{false}; +}; -ThreadPtr ThreadFactoryImplPosix::createThread(std::function thread_routine) { - return std::make_unique(thread_routine); +ThreadPtr ThreadFactoryImplPosix::createThread(std::function thread_routine, + OptionsOptConstRef options) { + return std::make_unique(thread_routine, options); } ThreadId ThreadFactoryImplPosix::currentThreadId() { return ThreadId(getCurrentThreadId()); } diff --git a/source/common/common/posix/thread_impl.h b/source/common/common/posix/thread_impl.h index 81c81d3be3fc..9b373ecaceb6 100644 --- a/source/common/common/posix/thread_impl.h +++ b/source/common/common/posix/thread_impl.h @@ -9,29 +9,13 @@ namespace Envoy { namespace Thread { -/** - * Wrapper for a pthread thread. We don't use std::thread because it eats exceptions and leads to - * unusable stack traces. - */ -class ThreadImplPosix : public Thread { -public: - ThreadImplPosix(std::function thread_routine); - - // Thread::Thread - void join() override; - -private: - std::function thread_routine_; - pthread_t thread_handle_; -}; - /** * Implementation of ThreadFactory */ class ThreadFactoryImplPosix : public ThreadFactory { public: // Thread::ThreadFactory - ThreadPtr createThread(std::function thread_routine) override; + ThreadPtr createThread(std::function thread_routine, OptionsOptConstRef options) override; ThreadId currentThreadId() override; }; diff --git a/source/common/common/win32/thread_impl.cc b/source/common/common/win32/thread_impl.cc index 1d3eca968957..8f26d63e0eb3 100644 --- a/source/common/common/win32/thread_impl.cc +++ b/source/common/common/win32/thread_impl.cc @@ -6,8 +6,14 @@ namespace Envoy { namespace Thread { -ThreadImplWin32::ThreadImplWin32(std::function thread_routine) +ThreadImplWin32::ThreadImplWin32(std::function thread_routine, OptionsOptConstRef options) : thread_routine_(thread_routine) { + if (options) { + name_ = options->name_; + // TODO(jmarantz): set the thread name for task manager, etc, or pull the + // auto-generated name from the OS if options is not present. + } + RELEASE_ASSERT(Logger::Registry::initialized(), ""); thread_handle_ = reinterpret_cast(::_beginthreadex( nullptr, 0, @@ -26,8 +32,9 @@ void ThreadImplWin32::join() { RELEASE_ASSERT(rc == WAIT_OBJECT_0, ""); } -ThreadPtr ThreadFactoryImplWin32::createThread(std::function thread_routine) { - return std::make_unique(thread_routine); +ThreadPtr ThreadFactoryImplWin32::createThread(std::function thread_routine, + OptionsOptConstRef options) { + return std::make_unique(thread_routine, options); } ThreadId ThreadFactoryImplWin32::currentThreadId() { diff --git a/source/common/common/win32/thread_impl.h b/source/common/common/win32/thread_impl.h index 8b5d0fe37e15..87be085291c8 100644 --- a/source/common/common/win32/thread_impl.h +++ b/source/common/common/win32/thread_impl.h @@ -14,11 +14,12 @@ namespace Thread { */ class ThreadImplWin32 : public Thread { public: - ThreadImplWin32(std::function thread_routine); + ThreadImplWin32(std::function thread_routine, OptionsOptConstRef options); ~ThreadImplWin32(); // Thread::Thread void join() override; + std::string name() const override { return name_; } // Needed for WatcherImpl for the QueueUserAPC callback context HANDLE handle() const { return thread_handle_; } @@ -26,6 +27,7 @@ class ThreadImplWin32 : public Thread { private: std::function thread_routine_; HANDLE thread_handle_; + std::string name_; }; /** @@ -34,7 +36,7 @@ class ThreadImplWin32 : public Thread { class ThreadFactoryImplWin32 : public ThreadFactory { public: // Thread::ThreadFactory - ThreadPtr createThread(std::function thread_routine) override; + ThreadPtr createThread(std::function thread_routine, OptionsOptConstRef options) override; ThreadId currentThreadId() override; }; diff --git a/source/common/filesystem/win32/watcher_impl.cc b/source/common/filesystem/win32/watcher_impl.cc index 5bc400639109..80531f78d54e 100644 --- a/source/common/filesystem/win32/watcher_impl.cc +++ b/source/common/filesystem/win32/watcher_impl.cc @@ -31,7 +31,10 @@ WatcherImpl::WatcherImpl(Event::Dispatcher& dispatcher, Api::Api& api) thread_exit_event_ = ::CreateEvent(nullptr, false, false, nullptr); ASSERT(thread_exit_event_ != NULL); keep_watching_ = true; - watch_thread_ = thread_factory_.createThread([this]() -> void { watchLoop(); }); + + // See comments in WorkerImpl::start for the naming convention. + Thread::Options options{absl::StrCat("wat:", dispatcher.name())}; + watch_thread_ = thread_factory_.createThread([this]() -> void { watchLoop(); }, options); } WatcherImpl::~WatcherImpl() { diff --git a/source/common/grpc/google_async_client_impl.cc b/source/common/grpc/google_async_client_impl.cc index 18c4936e5e25..ff7774034ac9 100644 --- a/source/common/grpc/google_async_client_impl.cc +++ b/source/common/grpc/google_async_client_impl.cc @@ -21,7 +21,8 @@ static constexpr int DefaultBufferLimitBytes = 1024 * 1024; } GoogleAsyncClientThreadLocal::GoogleAsyncClientThreadLocal(Api::Api& api) - : completion_thread_(api.threadFactory().createThread([this] { completionThread(); })) {} + : completion_thread_(api.threadFactory().createThread([this] { completionThread(); }, + Thread::Options{"GrpcGoogClient"})) {} GoogleAsyncClientThreadLocal::~GoogleAsyncClientThreadLocal() { // Force streams to shutdown and invoke TryCancel() to start the drain of diff --git a/source/common/network/listen_socket_impl.cc b/source/common/network/listen_socket_impl.cc index 1dc095bb299d..43e342d340b6 100644 --- a/source/common/network/listen_socket_impl.cc +++ b/source/common/network/listen_socket_impl.cc @@ -46,8 +46,7 @@ void ListenSocketImpl::setupSocket(const Network::Socket::OptionsSharedPtr& opti } template <> -void NetworkListenSocket< - NetworkSocketTrait>::setPrebindSocketOptions() { +void NetworkListenSocket>::setPrebindSocketOptions() { // On Windows, SO_REUSEADDR does not restrict subsequent bind calls when there is a listener as on // Linux and later BSD socket stacks #ifndef WIN32 @@ -58,11 +57,10 @@ void NetworkListenSocket< } template <> -void NetworkListenSocket< - NetworkSocketTrait>::setPrebindSocketOptions() {} +void NetworkListenSocket>::setPrebindSocketOptions() {} UdsListenSocket::UdsListenSocket(const Address::InstanceConstSharedPtr& address) - : ListenSocketImpl(SocketInterfaceSingleton::get().socket(Address::SocketType::Stream, address), + : ListenSocketImpl(SocketInterfaceSingleton::get().socket(Socket::Type::Stream, address), address) { RELEASE_ASSERT(io_handle_->fd() != -1, ""); bind(local_address_); diff --git a/source/common/network/listen_socket_impl.h b/source/common/network/listen_socket_impl.h index 99b6a6e499e3..a77ccefbd78a 100644 --- a/source/common/network/listen_socket_impl.h +++ b/source/common/network/listen_socket_impl.h @@ -29,14 +29,14 @@ class ListenSocketImpl : public SocketImpl { /** * Wraps a unix socket. */ -template struct NetworkSocketTrait {}; +template struct NetworkSocketTrait {}; -template <> struct NetworkSocketTrait { - static constexpr Address::SocketType type = Address::SocketType::Stream; +template <> struct NetworkSocketTrait { + static constexpr Socket::Type type = Socket::Type::Stream; }; -template <> struct NetworkSocketTrait { - static constexpr Address::SocketType type = Address::SocketType::Datagram; +template <> struct NetworkSocketTrait { + static constexpr Socket::Type type = Socket::Type::Datagram; }; template class NetworkListenSocket : public ListenSocketImpl { @@ -58,23 +58,23 @@ template class NetworkListenSocket : public ListenSocketImpl { setListenSocketOptions(options); } - Address::SocketType socketType() const override { return T::type; } + Socket::Type socketType() const override { return T::type; } protected: void setPrebindSocketOptions(); }; -using TcpListenSocket = NetworkListenSocket>; +using TcpListenSocket = NetworkListenSocket>; using TcpListenSocketPtr = std::unique_ptr; -using UdpListenSocket = NetworkListenSocket>; +using UdpListenSocket = NetworkListenSocket>; using UdpListenSocketPtr = std::unique_ptr; class UdsListenSocket : public ListenSocketImpl { public: UdsListenSocket(const Address::InstanceConstSharedPtr& address); UdsListenSocket(IoHandlePtr&& io_handle, const Address::InstanceConstSharedPtr& address); - Address::SocketType socketType() const override { return Address::SocketType::Stream; } + Socket::Type socketType() const override { return Socket::Type::Stream; } }; class ConnectionSocketImpl : public SocketImpl, public ConnectionSocket { @@ -85,8 +85,7 @@ class ConnectionSocketImpl : public SocketImpl, public ConnectionSocket { : SocketImpl(std::move(io_handle), local_address), remote_address_(remote_address), direct_remote_address_(remote_address) {} - ConnectionSocketImpl(Address::SocketType type, - const Address::InstanceConstSharedPtr& local_address, + ConnectionSocketImpl(Socket::Type type, const Address::InstanceConstSharedPtr& local_address, const Address::InstanceConstSharedPtr& remote_address) : SocketImpl(type, local_address), remote_address_(remote_address), direct_remote_address_(remote_address) { @@ -94,7 +93,7 @@ class ConnectionSocketImpl : public SocketImpl, public ConnectionSocket { } // Network::Socket - Address::SocketType socketType() const override { return Address::SocketType::Stream; } + Socket::Type socketType() const override { return Socket::Type::Stream; } // Network::ConnectionSocket const Address::InstanceConstSharedPtr& remoteAddress() const override { return remote_address_; } @@ -152,9 +151,9 @@ class ClientSocketImpl : public ConnectionSocketImpl { public: ClientSocketImpl(const Address::InstanceConstSharedPtr& remote_address, const OptionsSharedPtr& options) - : ConnectionSocketImpl(Network::SocketInterfaceSingleton::get().socket( - Address::SocketType::Stream, remote_address), - nullptr, remote_address) { + : ConnectionSocketImpl( + Network::SocketInterfaceSingleton::get().socket(Socket::Type::Stream, remote_address), + nullptr, remote_address) { if (options) { addOptions(options); } diff --git a/source/common/network/socket_impl.cc b/source/common/network/socket_impl.cc index 4e0200c381d2..1c0e89439e68 100644 --- a/source/common/network/socket_impl.cc +++ b/source/common/network/socket_impl.cc @@ -10,11 +10,10 @@ namespace Envoy { namespace Network { -SocketImpl::SocketImpl(Address::SocketType type, Address::Type addr_type, - Address::IpVersion version) +SocketImpl::SocketImpl(Socket::Type type, Address::Type addr_type, Address::IpVersion version) : io_handle_(SocketInterfaceSingleton::get().socket(type, addr_type, version)) {} -SocketImpl::SocketImpl(Address::SocketType sock_type, const Address::InstanceConstSharedPtr addr) +SocketImpl::SocketImpl(Socket::Type sock_type, const Address::InstanceConstSharedPtr addr) : io_handle_(SocketInterfaceSingleton::get().socket(sock_type, addr)), sock_type_(sock_type), addr_type_(addr->type()) {} diff --git a/source/common/network/socket_impl.h b/source/common/network/socket_impl.h index e4e14798f67c..cb84bfb1c699 100644 --- a/source/common/network/socket_impl.h +++ b/source/common/network/socket_impl.h @@ -9,8 +9,8 @@ namespace Network { class SocketImpl : public virtual Socket { public: - SocketImpl(Address::SocketType type, Address::Type addr_type, Address::IpVersion version); - SocketImpl(Address::SocketType socket_type, const Address::InstanceConstSharedPtr addr); + SocketImpl(Socket::Type type, Address::Type addr_type, Address::IpVersion version); + SocketImpl(Socket::Type socket_type, const Address::InstanceConstSharedPtr addr); // Network::Socket const Address::InstanceConstSharedPtr& localAddress() const override { return local_address_; } @@ -50,7 +50,7 @@ class SocketImpl : public virtual Socket { Api::SysCallIntResult setBlockingForTest(bool blocking) override; const OptionsSharedPtr& options() const override { return options_; } - Address::SocketType socketType() const override { return sock_type_; } + Socket::Type socketType() const override { return sock_type_; } Address::Type addressType() const override { return addr_type_; } protected: @@ -59,7 +59,7 @@ class SocketImpl : public virtual Socket { const IoHandlePtr io_handle_; Address::InstanceConstSharedPtr local_address_; OptionsSharedPtr options_; - Address::SocketType sock_type_; + Socket::Type sock_type_; Address::Type addr_type_; }; diff --git a/source/common/network/socket_interface_impl.cc b/source/common/network/socket_interface_impl.cc index 1e9481e2368d..a682e2d23406 100644 --- a/source/common/network/socket_interface_impl.cc +++ b/source/common/network/socket_interface_impl.cc @@ -10,7 +10,7 @@ namespace Envoy { namespace Network { -IoHandlePtr SocketInterfaceImpl::socket(Address::SocketType socket_type, Address::Type addr_type, +IoHandlePtr SocketInterfaceImpl::socket(Socket::Type socket_type, Address::Type addr_type, Address::IpVersion version) { #if defined(__APPLE__) || defined(WIN32) int flags = 0; @@ -18,7 +18,7 @@ IoHandlePtr SocketInterfaceImpl::socket(Address::SocketType socket_type, Address int flags = SOCK_NONBLOCK; #endif - if (socket_type == Address::SocketType::Stream) { + if (socket_type == Socket::Type::Stream) { flags |= SOCK_STREAM; } else { flags |= SOCK_DGRAM; @@ -51,7 +51,7 @@ IoHandlePtr SocketInterfaceImpl::socket(Address::SocketType socket_type, Address return io_handle; } -IoHandlePtr SocketInterfaceImpl::socket(Address::SocketType socket_type, +IoHandlePtr SocketInterfaceImpl::socket(Socket::Type socket_type, const Address::InstanceConstSharedPtr addr) { Address::IpVersion ip_version = addr->ip() ? addr->ip()->version() : Address::IpVersion::v4; IoHandlePtr io_handle = SocketInterfaceImpl::socket(socket_type, addr->type(), ip_version); diff --git a/source/common/network/socket_interface_impl.h b/source/common/network/socket_interface_impl.h index 190a2765031c..ddef0528d659 100644 --- a/source/common/network/socket_interface_impl.h +++ b/source/common/network/socket_interface_impl.h @@ -10,10 +10,9 @@ namespace Network { class SocketInterfaceImpl : public SocketInterface { public: - IoHandlePtr socket(Address::SocketType socket_type, Address::Type addr_type, + IoHandlePtr socket(Socket::Type socket_type, Address::Type addr_type, Address::IpVersion version) override; - IoHandlePtr socket(Address::SocketType socket_type, - const Address::InstanceConstSharedPtr addr) override; + IoHandlePtr socket(Socket::Type socket_type, const Address::InstanceConstSharedPtr addr) override; bool ipFamilySupported(int domain) override; Address::InstanceConstSharedPtr addressFromFd(os_fd_t fd) override; Address::InstanceConstSharedPtr peerAddressFromFd(os_fd_t fd) override; diff --git a/source/common/network/utility.cc b/source/common/network/utility.cc index 5c334879bdb5..107b50304eac 100644 --- a/source/common/network/utility.cc +++ b/source/common/network/utility.cc @@ -472,22 +472,22 @@ void Utility::addressToProtobufAddress(const Address::Instance& address, } } -Address::SocketType +Socket::Type Utility::protobufAddressSocketType(const envoy::config::core::v3::Address& proto_address) { switch (proto_address.address_case()) { case envoy::config::core::v3::Address::AddressCase::kSocketAddress: { const auto protocol = proto_address.socket_address().protocol(); switch (protocol) { case envoy::config::core::v3::SocketAddress::TCP: - return Address::SocketType::Stream; + return Socket::Type::Stream; case envoy::config::core::v3::SocketAddress::UDP: - return Address::SocketType::Datagram; + return Socket::Type::Datagram; default: NOT_REACHED_GCOVR_EXCL_LINE; } } case envoy::config::core::v3::Address::AddressCase::kPipe: - return Address::SocketType::Stream; + return Socket::Type::Stream; default: NOT_REACHED_GCOVR_EXCL_LINE; } diff --git a/source/common/network/utility.h b/source/common/network/utility.h index fd4b925aea9a..be64071e9ea6 100644 --- a/source/common/network/utility.h +++ b/source/common/network/utility.h @@ -286,7 +286,7 @@ class Utility { * @param proto_address the address protobuf * @return socket type */ - static Address::SocketType + static Socket::Type protobufAddressSocketType(const envoy::config::core::v3::Address& proto_address); /** diff --git a/source/common/secret/sds_api.h b/source/common/secret/sds_api.h index 0ca7c93f24aa..d0173f8ae470 100644 --- a/source/common/secret/sds_api.h +++ b/source/common/secret/sds_api.h @@ -118,7 +118,7 @@ class TlsCertificateSdsApi : public SdsApi, public TlsCertificateConfigProvider sds_config, sds_config_name, secret_provider_context.clusterManager().subscriptionFactory(), secret_provider_context.dispatcher().timeSource(), secret_provider_context.messageValidationVisitor(), secret_provider_context.stats(), - *secret_provider_context.initManager(), destructor_cb, secret_provider_context.dispatcher(), + secret_provider_context.initManager(), destructor_cb, secret_provider_context.dispatcher(), secret_provider_context.api()); } @@ -179,7 +179,7 @@ class CertificateValidationContextSdsApi : public SdsApi, sds_config, sds_config_name, secret_provider_context.clusterManager().subscriptionFactory(), secret_provider_context.dispatcher().timeSource(), secret_provider_context.messageValidationVisitor(), secret_provider_context.stats(), - *secret_provider_context.initManager(), destructor_cb, secret_provider_context.dispatcher(), + secret_provider_context.initManager(), destructor_cb, secret_provider_context.dispatcher(), secret_provider_context.api()); } CertificateValidationContextSdsApi(const envoy::config::core::v3::ConfigSource& sds_config, @@ -250,7 +250,7 @@ class TlsSessionTicketKeysSdsApi : public SdsApi, public TlsSessionTicketKeysCon sds_config, sds_config_name, secret_provider_context.clusterManager().subscriptionFactory(), secret_provider_context.dispatcher().timeSource(), secret_provider_context.messageValidationVisitor(), secret_provider_context.stats(), - *secret_provider_context.initManager(), destructor_cb, secret_provider_context.dispatcher(), + secret_provider_context.initManager(), destructor_cb, secret_provider_context.dispatcher(), secret_provider_context.api()); } @@ -321,7 +321,7 @@ class GenericSecretSdsApi : public SdsApi, public GenericSecretConfigProvider { sds_config, sds_config_name, secret_provider_context.clusterManager().subscriptionFactory(), secret_provider_context.dispatcher().timeSource(), secret_provider_context.messageValidationVisitor(), secret_provider_context.stats(), - *secret_provider_context.initManager(), destructor_cb, secret_provider_context.dispatcher(), + secret_provider_context.initManager(), destructor_cb, secret_provider_context.dispatcher(), secret_provider_context.api()); } diff --git a/source/common/secret/secret_manager_impl.h b/source/common/secret/secret_manager_impl.h index 002bed3decb2..d7be0e8b6b54 100644 --- a/source/common/secret/secret_manager_impl.h +++ b/source/common/secret/secret_manager_impl.h @@ -88,7 +88,6 @@ class SecretManagerImpl : public SecretManager { std::function unregister_secret_provider = [map_key, this]() { removeDynamicSecretProvider(map_key); }; - ASSERT(secret_provider_context.initManager() != nullptr); secret_provider = SecretType::create(secret_provider_context, sds_config_source, config_name, unregister_secret_provider); dynamic_secret_providers_[map_key] = secret_provider; diff --git a/source/common/stats/allocator_impl.cc b/source/common/stats/allocator_impl.cc index 4f41b208e4ad..04dd31eca9c7 100644 --- a/source/common/stats/allocator_impl.cc +++ b/source/common/stats/allocator_impl.cc @@ -264,7 +264,7 @@ CounterSharedPtr AllocatorImpl::makeCounter(StatName name, StatName tag_extracte if (iter != counters_.end()) { return CounterSharedPtr(*iter); } - auto counter = CounterSharedPtr(new CounterImpl(name, *this, tag_extracted_name, stat_name_tags)); + auto counter = CounterSharedPtr(makeCounterInternal(name, tag_extracted_name, stat_name_tags)); counters_.insert(counter.get()); return counter; } @@ -308,5 +308,10 @@ bool AllocatorImpl::isMutexLockedForTest() { return !locked; } +Counter* AllocatorImpl::makeCounterInternal(StatName name, StatName tag_extracted_name, + const StatNameTagVector& stat_name_tags) { + return new CounterImpl(name, *this, tag_extracted_name, stat_name_tags); +} + } // namespace Stats } // namespace Envoy diff --git a/source/common/stats/allocator_impl.h b/source/common/stats/allocator_impl.h index 02e926529358..ddee00c39559 100644 --- a/source/common/stats/allocator_impl.h +++ b/source/common/stats/allocator_impl.h @@ -47,11 +47,16 @@ class AllocatorImpl : public Allocator { */ bool isMutexLockedForTest(); +protected: + virtual Counter* makeCounterInternal(StatName name, StatName tag_extracted_name, + const StatNameTagVector& stat_name_tags); + private: template friend class StatsSharedImpl; friend class CounterImpl; friend class GaugeImpl; friend class TextReadoutImpl; + friend class NotifyingAllocatorImpl; struct HeapStatHash { using is_transparent = void; // NOLINT(readability-identifier-naming) diff --git a/source/extensions/filters/udp/udp_proxy/udp_proxy_filter.h b/source/extensions/filters/udp/udp_proxy/udp_proxy_filter.h index dc16c2748082..8456d96089b3 100644 --- a/source/extensions/filters/udp/udp_proxy/udp_proxy_filter.h +++ b/source/extensions/filters/udp/udp_proxy/udp_proxy_filter.h @@ -222,7 +222,7 @@ class UdpProxyFilter : public Network::UdpListenerReadFilter, virtual Network::IoHandlePtr createIoHandle(const Upstream::HostConstSharedPtr& host) { // Virtual so this can be overridden in unit tests. - return Network::SocketInterfaceSingleton::get().socket(Network::Address::SocketType::Datagram, + return Network::SocketInterfaceSingleton::get().socket(Network::Socket::Type::Datagram, host->address()); } diff --git a/source/extensions/quic_listeners/quiche/envoy_quic_utils.cc b/source/extensions/quic_listeners/quiche/envoy_quic_utils.cc index 9e0ee82de167..6fa268c53cc0 100644 --- a/source/extensions/quic_listeners/quiche/envoy_quic_utils.cc +++ b/source/extensions/quic_listeners/quiche/envoy_quic_utils.cc @@ -95,7 +95,7 @@ createConnectionSocket(Network::Address::InstanceConstSharedPtr& peer_addr, Network::Address::InstanceConstSharedPtr& local_addr, const Network::ConnectionSocket::OptionsSharedPtr& options) { auto connection_socket = std::make_unique( - Network::Address::SocketType::Datagram, local_addr, peer_addr); + Network::Socket::Type::Datagram, local_addr, peer_addr); connection_socket->addOptions(Network::SocketOptionFactory::buildIpPacketInfoOptions()); connection_socket->addOptions(Network::SocketOptionFactory::buildRxQueueOverFlowOptions()); if (options != nullptr) { diff --git a/source/extensions/stat_sinks/common/statsd/statsd.cc b/source/extensions/stat_sinks/common/statsd/statsd.cc index 40d83274de5a..c502cb6e02ca 100644 --- a/source/extensions/stat_sinks/common/statsd/statsd.cc +++ b/source/extensions/stat_sinks/common/statsd/statsd.cc @@ -29,7 +29,7 @@ namespace Statsd { UdpStatsdSink::WriterImpl::WriterImpl(UdpStatsdSink& parent) : parent_(parent), io_handle_(Network::SocketInterfaceSingleton::get().socket( - Network::Address::SocketType::Datagram, parent_.server_address_)) {} + Network::Socket::Type::Datagram, parent_.server_address_)) {} void UdpStatsdSink::WriterImpl::write(const std::string& message) { // TODO(mattklein123): We can avoid this const_cast pattern by having a constant variant of diff --git a/source/extensions/tracers/xray/daemon_broker.cc b/source/extensions/tracers/xray/daemon_broker.cc index 6e5f1622e7aa..99e7a9bee4d3 100644 --- a/source/extensions/tracers/xray/daemon_broker.cc +++ b/source/extensions/tracers/xray/daemon_broker.cc @@ -30,8 +30,8 @@ std::string createHeader(const std::string& format, uint32_t version) { DaemonBrokerImpl::DaemonBrokerImpl(const std::string& daemon_endpoint) : address_(Network::Utility::parseInternetAddressAndPort(daemon_endpoint, false /*v6only*/)), - io_handle_(Network::SocketInterfaceSingleton::get().socket( - Network::Address::SocketType::Datagram, address_)) {} + io_handle_(Network::SocketInterfaceSingleton::get().socket(Network::Socket::Type::Datagram, + address_)) {} void DaemonBrokerImpl::send(const std::string& data) const { auto& logger = Logger::Registry::getLog(Logger::Id::tracing); diff --git a/source/server/admin/admin.h b/source/server/admin/admin.h index 74449114bae9..439cacf0013e 100644 --- a/source/server/admin/admin.h +++ b/source/server/admin/admin.h @@ -292,7 +292,7 @@ class AdminImpl : public Admin, AdminListenSocketFactory(Network::SocketSharedPtr socket) : socket_(socket) {} // Network::ListenSocketFactory - Network::Address::SocketType socketType() const override { return socket_->socketType(); } + Network::Socket::Type socketType() const override { return socket_->socketType(); } const Network::Address::InstanceConstSharedPtr& localAddress() const override { return socket_->localAddress(); diff --git a/source/server/config_validation/server.h b/source/server/config_validation/server.h index 70bb29bf180b..14682097f400 100644 --- a/source/server/config_validation/server.h +++ b/source/server/config_validation/server.h @@ -141,7 +141,7 @@ class ValidationInstance final : Logger::Loggable, return ProdListenerComponentFactory::createUdpListenerFilterFactoryList_(filters, context); } Network::SocketSharedPtr createListenSocket(Network::Address::InstanceConstSharedPtr, - Network::Address::SocketType, + Network::Socket::Type, const Network::Socket::OptionsSharedPtr&, const ListenSocketCreationParams&) override { // Returned sockets are not currently used so we can return nothing here safely vs. a diff --git a/source/server/connection_handler_impl.cc b/source/server/connection_handler_impl.cc index 1b2d03794eb9..708eef2e47ef 100644 --- a/source/server/connection_handler_impl.cc +++ b/source/server/connection_handler_impl.cc @@ -30,7 +30,7 @@ void ConnectionHandlerImpl::decNumConnections() { void ConnectionHandlerImpl::addListener(absl::optional overridden_listener, Network::ListenerConfig& config) { ActiveListenerDetails details; - if (config.listenSocketFactory().socketType() == Network::Address::SocketType::Stream) { + if (config.listenSocketFactory().socketType() == Network::Socket::Type::Stream) { if (overridden_listener.has_value()) { for (auto& listener : listeners_) { if (listener.second.listener_->listenerTag() == overridden_listener) { diff --git a/source/server/guarddog_impl.cc b/source/server/guarddog_impl.cc index d05e84f6ff6e..ad66b59a5299 100644 --- a/source/server/guarddog_impl.cc +++ b/source/server/guarddog_impl.cc @@ -142,8 +142,10 @@ void GuardDogImpl::stopWatching(WatchDogSharedPtr wd) { void GuardDogImpl::start(Api::Api& api) { Thread::LockGuard guard(mutex_); + // See comments in WorkerImpl::start for the naming convention. + Thread::Options options{absl::StrCat("dog:", dispatcher_->name())}; thread_ = api.threadFactory().createThread( - [this]() -> void { dispatcher_->run(Event::Dispatcher::RunType::RunUntilExit); }); + [this]() -> void { dispatcher_->run(Event::Dispatcher::RunType::RunUntilExit); }, options); loop_timer_->enableTimer(std::chrono::milliseconds(0)); } diff --git a/source/server/listener_impl.cc b/source/server/listener_impl.cc index 4b36fc4a20df..9a31da2cb033 100644 --- a/source/server/listener_impl.cc +++ b/source/server/listener_impl.cc @@ -53,7 +53,7 @@ bool needTlsInspector(const envoy::config::listener::v3::Listener& config) { ListenSocketFactoryImpl::ListenSocketFactoryImpl(ListenerComponentFactory& factory, Network::Address::InstanceConstSharedPtr address, - Network::Address::SocketType socket_type, + Network::Socket::Type socket_type, const Network::Socket::OptionsSharedPtr& options, bool bind_to_port, const std::string& listener_name, bool reuse_port) @@ -62,7 +62,7 @@ ListenSocketFactoryImpl::ListenSocketFactoryImpl(ListenerComponentFactory& facto bool create_socket = false; if (local_address_->type() == Network::Address::Type::Ip) { - if (socket_type_ == Network::Address::SocketType::Datagram) { + if (socket_type_ == Network::Socket::Type::Datagram) { ASSERT(reuse_port_ == true); } @@ -260,7 +260,7 @@ ListenerImpl::ListenerImpl(const envoy::config::listener::v3::Listener& config, createListenerFilterFactories(socket_type); validateFilterChains(socket_type); buildFilterChains(); - if (socket_type == Network::Address::SocketType::Datagram) { + if (socket_type == Network::Socket::Type::Datagram) { return; } buildSocketOptions(); @@ -330,9 +330,9 @@ void ListenerImpl::buildAccessLog() { } } -void ListenerImpl::buildUdpListenerFactory(Network::Address::SocketType socket_type, +void ListenerImpl::buildUdpListenerFactory(Network::Socket::Type socket_type, uint32_t concurrency) { - if (socket_type == Network::Address::SocketType::Datagram) { + if (socket_type == Network::Socket::Type::Datagram) { auto udp_config = config_.udp_listener_config(); if (udp_config.udp_listener_name().empty()) { udp_config.set_udp_listener_name(UdpListenerNames::get().RawUdp); @@ -350,7 +350,7 @@ void ListenerImpl::buildUdpListenerFactory(Network::Address::SocketType socket_t } } -void ListenerImpl::buildListenSocketOptions(Network::Address::SocketType socket_type) { +void ListenerImpl::buildListenSocketOptions(Network::Socket::Type socket_type) { if (PROTOBUF_GET_WRAPPED_OR_DEFAULT(config_, transparent, false)) { addListenSocketOptions(Network::SocketOptionFactory::buildIpTransparentOptions()); } @@ -364,7 +364,7 @@ void ListenerImpl::buildListenSocketOptions(Network::Address::SocketType socket_ addListenSocketOptions( Network::SocketOptionFactory::buildLiteralOptions(config_.socket_options())); } - if (socket_type == Network::Address::SocketType::Datagram) { + if (socket_type == Network::Socket::Type::Datagram) { // Needed for recvmsg to return destination address in IP header. addListenSocketOptions(Network::SocketOptionFactory::buildIpPacketInfoOptions()); // Needed to return receive buffer overflown indicator. @@ -372,10 +372,10 @@ void ListenerImpl::buildListenSocketOptions(Network::Address::SocketType socket_ } } -void ListenerImpl::createListenerFilterFactories(Network::Address::SocketType socket_type) { +void ListenerImpl::createListenerFilterFactories(Network::Socket::Type socket_type) { if (!config_.listener_filters().empty()) { switch (socket_type) { - case Network::Address::SocketType::Datagram: + case Network::Socket::Type::Datagram: if (config_.listener_filters().size() > 1) { // Currently supports only 1 UDP listener filter. throw EnvoyException(fmt::format( @@ -385,7 +385,7 @@ void ListenerImpl::createListenerFilterFactories(Network::Address::SocketType so udp_listener_filter_factories_ = parent_.factory_.createUdpListenerFilterFactoryList( config_.listener_filters(), *listener_factory_context_); break; - case Network::Address::SocketType::Stream: + case Network::Socket::Type::Stream: listener_filter_factories_ = parent_.factory_.createListenerFilterFactoryList( config_.listener_filters(), *listener_factory_context_); break; @@ -395,8 +395,8 @@ void ListenerImpl::createListenerFilterFactories(Network::Address::SocketType so } } -void ListenerImpl::validateFilterChains(Network::Address::SocketType socket_type) { - if (config_.filter_chains().empty() && (socket_type == Network::Address::SocketType::Stream || +void ListenerImpl::validateFilterChains(Network::Socket::Type socket_type) { + if (config_.filter_chains().empty() && (socket_type == Network::Socket::Type::Stream || !udp_listener_factory_->isTransportConnectionless())) { // If we got here, this is a tcp listener or connection-oriented udp listener, so ensure there // is a filter chain specified @@ -637,9 +637,9 @@ bool ListenerImpl::supportUpdateFilterChain(const envoy::config::listener::v3::L // Currently we only support TCP filter chain update. if (Network::Utility::protobufAddressSocketType(config_.address()) != - Network::Address::SocketType::Stream || + Network::Socket::Type::Stream || Network::Utility::protobufAddressSocketType(config.address()) != - Network::Address::SocketType::Stream) { + Network::Socket::Type::Stream) { return false; } diff --git a/source/server/listener_impl.h b/source/server/listener_impl.h index 6160c0f4f87c..328e744a29a7 100644 --- a/source/server/listener_impl.h +++ b/source/server/listener_impl.h @@ -40,12 +40,12 @@ class ListenSocketFactoryImpl : public Network::ListenSocketFactory, public: ListenSocketFactoryImpl(ListenerComponentFactory& factory, Network::Address::InstanceConstSharedPtr address, - Network::Address::SocketType socket_type, + Network::Socket::Type socket_type, const Network::Socket::OptionsSharedPtr& options, bool bind_to_port, const std::string& listener_name, bool reuse_port); // Network::ListenSocketFactory - Network::Address::SocketType socketType() const override { return socket_type_; } + Network::Socket::Type socketType() const override { return socket_type_; } const Network::Address::InstanceConstSharedPtr& localAddress() const override { return local_address_; } @@ -73,7 +73,7 @@ class ListenSocketFactoryImpl : public Network::ListenSocketFactory, // Initially, its port number might be 0. Once a socket is created, its port // will be set to the binding port. Network::Address::InstanceConstSharedPtr local_address_; - Network::Address::SocketType socket_type_; + Network::Socket::Type socket_type_; const Network::Socket::OptionsSharedPtr options_; bool bind_to_port_; const std::string& listener_name_; @@ -337,10 +337,10 @@ class ListenerImpl final : public Network::ListenerConfig, uint32_t concurrency); // Helpers for constructor. void buildAccessLog(); - void buildUdpListenerFactory(Network::Address::SocketType socket_type, uint32_t concurrency); - void buildListenSocketOptions(Network::Address::SocketType socket_type); - void createListenerFilterFactories(Network::Address::SocketType socket_type); - void validateFilterChains(Network::Address::SocketType socket_type); + void buildUdpListenerFactory(Network::Socket::Type socket_type, uint32_t concurrency); + void buildListenSocketOptions(Network::Socket::Type socket_type); + void createListenerFilterFactories(Network::Socket::Type socket_type); + void validateFilterChains(Network::Socket::Type socket_type); void buildFilterChains(); void buildSocketOptions(); void buildOriginalDstListenerFilter(); diff --git a/source/server/listener_manager_impl.cc b/source/server/listener_manager_impl.cc index c81fd1da6225..9aac1a14d953 100644 --- a/source/server/listener_manager_impl.cc +++ b/source/server/listener_manager_impl.cc @@ -39,11 +39,11 @@ namespace Envoy { namespace Server { namespace { -std::string toString(Network::Address::SocketType socket_type) { +std::string toString(Network::Socket::Type socket_type) { switch (socket_type) { - case Network::Address::SocketType::Stream: + case Network::Socket::Type::Stream: return "SocketType::Stream"; - case Network::Address::SocketType::Datagram: + case Network::Socket::Type::Datagram: return "SocketType::Datagram"; } NOT_REACHED_GCOVR_EXCL_LINE; @@ -188,17 +188,17 @@ Network::ListenerFilterMatcherSharedPtr ProdListenerComponentFactory::createList } Network::SocketSharedPtr ProdListenerComponentFactory::createListenSocket( - Network::Address::InstanceConstSharedPtr address, Network::Address::SocketType socket_type, + Network::Address::InstanceConstSharedPtr address, Network::Socket::Type socket_type, const Network::Socket::OptionsSharedPtr& options, const ListenSocketCreationParams& params) { ASSERT(address->type() == Network::Address::Type::Ip || address->type() == Network::Address::Type::Pipe); - ASSERT(socket_type == Network::Address::SocketType::Stream || - socket_type == Network::Address::SocketType::Datagram); + ASSERT(socket_type == Network::Socket::Type::Stream || + socket_type == Network::Socket::Type::Datagram); // For each listener config we share a single socket among all threaded listeners. // First we try to get the socket from our parent if applicable. if (address->type() == Network::Address::Type::Pipe) { - if (socket_type != Network::Address::SocketType::Stream) { + if (socket_type != Network::Socket::Type::Stream) { // This could be implemented in the future, since Unix domain sockets // support SOCK_DGRAM, but there would need to be a way to specify it in // envoy.api.v2.core.Pipe. @@ -215,7 +215,7 @@ Network::SocketSharedPtr ProdListenerComponentFactory::createListenSocket( return std::make_shared(address); } - const std::string scheme = (socket_type == Network::Address::SocketType::Stream) + const std::string scheme = (socket_type == Network::Socket::Type::Stream) ? std::string(Network::Utility::TCP_SCHEME) : std::string(Network::Utility::UDP_SCHEME); const std::string addr = absl::StrCat(scheme, address->asString()); @@ -225,7 +225,7 @@ Network::SocketSharedPtr ProdListenerComponentFactory::createListenSocket( if (fd != -1) { ENVOY_LOG(debug, "obtained socket for address {} from parent", addr); Network::IoHandlePtr io_handle = std::make_unique(fd); - if (socket_type == Network::Address::SocketType::Stream) { + if (socket_type == Network::Socket::Type::Stream) { return std::make_shared(std::move(io_handle), address, options); } else { return std::make_shared(std::move(io_handle), address, options); @@ -233,7 +233,7 @@ Network::SocketSharedPtr ProdListenerComponentFactory::createListenSocket( } } - if (socket_type == Network::Address::SocketType::Stream) { + if (socket_type == Network::Socket::Type::Stream) { return std::make_shared(address, options, params.bind_to_port); } else { return std::make_shared(address, options, params.bind_to_port); @@ -491,13 +491,13 @@ bool ListenerManagerImpl::addOrUpdateListenerInternal( draining_listen_socket_factory = existing_draining_listener->listener_->getSocketFactory(); } - Network::Address::SocketType socket_type = + Network::Socket::Type socket_type = Network::Utility::protobufAddressSocketType(config.address()); new_listener->setSocketFactory( draining_listen_socket_factory ? draining_listen_socket_factory : createListenSocketFactory(config.address(), *new_listener, - (socket_type == Network::Address::SocketType::Datagram) || + (socket_type == Network::Socket::Type::Datagram) || config.reuse_port())); if (workers_started_) { new_listener->debugLog("add warming listener"); @@ -983,8 +983,7 @@ ListenerFilterChainFactoryBuilder::buildFilterChainInternal( Network::ListenSocketFactorySharedPtr ListenerManagerImpl::createListenSocketFactory( const envoy::config::core::v3::Address& proto_address, ListenerImpl& listener, bool reuse_port) { - Network::Address::SocketType socket_type = - Network::Utility::protobufAddressSocketType(proto_address); + Network::Socket::Type socket_type = Network::Utility::protobufAddressSocketType(proto_address); return std::make_shared( factory_, listener.address(), socket_type, listener.listenSocketOptions(), listener.bindToPort(), listener.name(), reuse_port); diff --git a/source/server/listener_manager_impl.h b/source/server/listener_manager_impl.h index fa278a2d3806..8a734350103b 100644 --- a/source/server/listener_manager_impl.h +++ b/source/server/listener_manager_impl.h @@ -89,7 +89,7 @@ class ProdListenerComponentFactory : public ListenerComponentFactory, } Network::SocketSharedPtr createListenSocket(Network::Address::InstanceConstSharedPtr address, - Network::Address::SocketType socket_type, + Network::Socket::Type socket_type, const Network::Socket::OptionsSharedPtr& options, const ListenSocketCreationParams& params) override; diff --git a/source/server/server.h b/source/server/server.h index 2ec00d266696..e6014f038645 100644 --- a/source/server/server.h +++ b/source/server/server.h @@ -171,19 +171,20 @@ class ServerFactoryContextImpl : public Configuration::ServerFactoryContext, Api::Api& api() override { return server_.api(); } Grpc::Context& grpcContext() override { return server_.grpcContext(); } Envoy::Server::DrainManager& drainManager() override { return server_.drainManager(); } + ServerLifecycleNotifier& lifecycleNotifier() override { return server_.lifecycleNotifier(); } // Configuration::TransportSocketFactoryContext Ssl::ContextManager& sslContextManager() override { return server_.sslContextManager(); } Secret::SecretManager& secretManager() override { return server_.secretManager(); } Stats::Store& stats() override { return server_.stats(); } - Init::Manager* initManager() override { return &server_.initManager(); } + Init::Manager& initManager() override { return server_.initManager(); } ProtobufMessage::ValidationVisitor& messageValidationVisitor() override { // Server has two message validation visitors, one for static and // other for dynamic configuration. Choose the dynamic validation // visitor if server's init manager indicates that the server is // in the Initialized state, as this state is engaged right after // the static configuration (e.g., bootstrap) has been completed. - return initManager()->state() == Init::Manager::State::Initialized + return initManager().state() == Init::Manager::State::Initialized ? server_.messageValidationContext().dynamicValidationVisitor() : server_.messageValidationContext().staticValidationVisitor(); } diff --git a/source/server/transport_socket_config_impl.h b/source/server/transport_socket_config_impl.h index 9e5bb4639e92..6a7a8ec17613 100644 --- a/source/server/transport_socket_config_impl.h +++ b/source/server/transport_socket_config_impl.h @@ -41,7 +41,10 @@ class TransportSocketFactoryContextImpl : public TransportSocketFactoryContext { Event::Dispatcher& dispatcher() override { return dispatcher_; } Envoy::Runtime::RandomGenerator& random() override { return random_; } Stats::Store& stats() override { return stats_; } - Init::Manager* initManager() override { return init_manager_; } + Init::Manager& initManager() override { + ASSERT(init_manager_ != nullptr); + return *init_manager_; + } Singleton::Manager& singletonManager() override { return singleton_manager_; } ThreadLocal::SlotAllocator& threadLocal() override { return tls_; } ProtobufMessage::ValidationVisitor& messageValidationVisitor() override { diff --git a/source/server/worker_impl.cc b/source/server/worker_impl.cc index 17e02486e5f8..54ef058ea6e5 100644 --- a/source/server/worker_impl.cc +++ b/source/server/worker_impl.cc @@ -81,8 +81,20 @@ void WorkerImpl::removeFilterChains(uint64_t listener_tag, void WorkerImpl::start(GuardDog& guard_dog) { ASSERT(!thread_); - thread_ = - api_.threadFactory().createThread([this, &guard_dog]() -> void { threadRoutine(guard_dog); }); + + // In posix, thread names are limited to 15 characters, so contrive to make + // sure all interesting data fits there. The naming occurs in + // ListenerManagerImpl's constructor: absl::StrCat("worker_", i). Let's say we + // have 9999 threads. We'd need, so we need 7 bytes for "worker_", 4 bytes + // for the thread index, leaving us 4 bytes left to distinguish between the + // two threads used per dispatcher. We'll call this one "dsp:" and the + // one allocated in guarddog_impl.cc "dog:". + // + // TODO(jmarantz): consider refactoring how this naming works so this naming + // architecture is centralized, resulting in clearer names. + Thread::Options options{absl::StrCat("wrk:", dispatcher_->name())}; + thread_ = api_.threadFactory().createThread( + [this, &guard_dog]() -> void { threadRoutine(guard_dog); }, options); } void WorkerImpl::initializeStats(Stats::Scope& scope) { dispatcher_->initializeStats(scope); } diff --git a/test/common/common/thread_test.cc b/test/common/common/thread_test.cc index 431d4be38f32..9dac043921f7 100644 --- a/test/common/common/thread_test.cc +++ b/test/common/common/thread_test.cc @@ -5,7 +5,9 @@ #include "test/test_common/thread_factory_for_test.h" +#include "absl/strings/str_cat.h" #include "absl/synchronization/notification.h" +#include "gmock/gmock.h" #include "gtest/gtest.h" namespace Envoy { @@ -27,12 +29,15 @@ TEST_F(ThreadAsyncPtrTest, DeleteOnDestruct) { // On thread1, we will lazily instantiate the string as "thread1". However // in the creation function we will block on a sync-point. - auto thread1 = thread_factory_.createThread([&str, &sync]() { - str.get([&sync]() -> std::string* { - sync.syncPoint("creator"); - return new std::string("thread1"); - }); - }); + auto thread1 = thread_factory_.createThread( + [&str, &sync]() { + str.get([&sync]() -> std::string* { + sync.syncPoint("creator"); + return new std::string("thread1"); + }); + }, + Options{"thread1"}); + EXPECT_EQ("thread1", thread1->name()); sync.barrierOn("creator"); @@ -40,7 +45,9 @@ TEST_F(ThreadAsyncPtrTest, DeleteOnDestruct) { // string as "thread2", but that allocator will never run because // the allocator on thread1 has already locked the AtomicPtr's mutex. auto thread2 = thread_factory_.createThread( - [&str]() { str.get([]() -> std::string* { return new std::string("thread2"); }); }); + [&str]() { str.get([]() -> std::string* { return new std::string("thread2"); }); }, + Options{"thread2"}); + EXPECT_EQ("thread2", thread2->name()); // Now let thread1's initializer finish. sync.signal("creator"); @@ -68,21 +75,25 @@ TEST_F(ThreadAsyncPtrTest, DoNotDelete) { // On thread1, we will lazily instantiate the string as "thread1". However // in the creation function we will block on a sync-point. - auto thread1 = thread_factory_.createThread([&str, &sync, &thread1_str]() { - str.get([&sync, &thread1_str]() -> const std::string* { - sync.syncPoint("creator"); - return &thread1_str; - }); - }); + auto thread1 = thread_factory_.createThread( + [&str, &sync, &thread1_str]() { + str.get([&sync, &thread1_str]() -> const std::string* { + sync.syncPoint("creator"); + return &thread1_str; + }); + }, + Options{"thread1"}); sync.barrierOn("creator"); // Now spawn a separate thread that will attempt to lazy-initialize the // string as "thread2", but that allocator will never run because // the allocator on thread1 has already locked the AtomicPtr's mutex. - auto thread2 = thread_factory_.createThread([&str, &thread2_str]() { - str.get([&thread2_str]() -> const std::string* { return &thread2_str; }); - }); + auto thread2 = thread_factory_.createThread( + [&str, &thread2_str]() { + str.get([&thread2_str]() -> const std::string* { return &thread2_str; }); + }, + Options{"thread2"}); // Now let thread1's initializer finish. sync.signal("creator"); @@ -113,7 +124,9 @@ TEST_F(ThreadAsyncPtrTest, ThreadSpammer) { }; std::vector threads; for (uint32_t i = 0; i < num_threads; ++i) { - threads.emplace_back(thread_factory_.createThread(thread_fn)); + std::string name = absl::StrCat("thread", i); + threads.emplace_back(thread_factory_.createThread(thread_fn, Options{name})); + EXPECT_EQ(name, threads.back()->name()); } EXPECT_EQ(0, calls); go.Notify(); @@ -190,6 +203,49 @@ TEST_F(ThreadAsyncPtrTest, ManagedAlloc) { } } +TEST_F(ThreadAsyncPtrTest, TruncateWait) { + absl::Notification notify; + auto thread = thread_factory_.createThread([¬ify]() { notify.WaitForNotification(); }, + Options{"this name is way too long for posix"}); + notify.Notify(); + + // To make this test work on multiple platforms, just assume the first 10 characters + // are retained. + EXPECT_THAT(thread->name(), testing::StartsWith("this name ")); + thread->join(); +} + +TEST_F(ThreadAsyncPtrTest, TruncateNoWait) { + auto thread = + thread_factory_.createThread([]() {}, Options{"this name is way too long for posix"}); + + // In general, across platforms, just assume the first 10 characters are + // retained. + EXPECT_THAT(thread->name(), testing::StartsWith("this name ")); + + // On Linux we can check for 15 exactly. +#ifdef __linux__ + EXPECT_EQ("this name is wa", thread->name()) << "truncated to 15 chars"; +#endif + + thread->join(); +} + +TEST_F(ThreadAsyncPtrTest, NameNotSpecifiedWait) { + absl::Notification notify; + auto thread = thread_factory_.createThread([¬ify]() { notify.WaitForNotification(); }); + notify.Notify(); + + // For linux builds, the thread name defaults to the name of the + // binary. However the name of the binary is different depending on whether + // this is a coverage test or not. Currently, this population does not occur + // for Mac or Windows. +#ifdef __linux__ + EXPECT_FALSE(thread->name().empty()); +#endif + thread->join(); +} + } // namespace } // namespace Thread } // namespace Envoy diff --git a/test/common/http/BUILD b/test/common/http/BUILD index fc43ab1ff66f..41160fb6a407 100644 --- a/test/common/http/BUILD +++ b/test/common/http/BUILD @@ -342,6 +342,7 @@ envoy_cc_test( envoy_proto_library( name = "utility_fuzz_proto", srcs = ["utility_fuzz.proto"], + deps = ["@envoy_api//envoy/config/core/v3:pkg"], ) envoy_cc_fuzz_test( diff --git a/test/common/http/utility_fuzz.proto b/test/common/http/utility_fuzz.proto index 2f023d29911f..50bb1a3c911b 100644 --- a/test/common/http/utility_fuzz.proto +++ b/test/common/http/utility_fuzz.proto @@ -3,6 +3,7 @@ syntax = "proto3"; package test.common.http; import "validate/validate.proto"; +import "envoy/config/core/v3/protocol.proto"; // Structured input for utility_fuzz_test. @@ -44,5 +45,6 @@ message UtilityTestCase { [(validate.rules).string = {well_known_regex: HTTP_HEADER_VALUE, strict: false}]; CookieValue make_set_cookie_value = 10; string parse_authority_string = 11; + envoy.config.core.v3.Http2ProtocolOptions initialize_and_validate = 12; } } diff --git a/test/common/http/utility_fuzz_test.cc b/test/common/http/utility_fuzz_test.cc index 4e62e7c2edc1..e81c10e4ae97 100644 --- a/test/common/http/utility_fuzz_test.cc +++ b/test/common/http/utility_fuzz_test.cc @@ -10,6 +10,12 @@ namespace Fuzz { namespace { DEFINE_PROTO_FUZZER(const test::common::http::UtilityTestCase& input) { + try { + TestUtility::validate(input); + } catch (ProtoValidationException& e) { + ENVOY_LOG_MISC(debug, "ProtoValidationException: {}", e.what()); + return; + } switch (input.utility_selector_case()) { case test::common::http::UtilityTestCase::kParseQueryString: { Http::Utility::parseQueryString(input.parse_query_string()); @@ -71,6 +77,11 @@ DEFINE_PROTO_FUZZER(const test::common::http::UtilityTestCase& input) { Http::Utility::parseAuthority(authority_string); break; } + case test::common::http::UtilityTestCase::kInitializeAndValidate: { + const auto& options = input.initialize_and_validate(); + Http2::Utility::initializeAndValidateOptions(options); + break; + } default: // Nothing to do. diff --git a/test/common/network/addr_family_aware_socket_option_impl_test.cc b/test/common/network/addr_family_aware_socket_option_impl_test.cc index dc874f2c3b41..4e264e30f405 100644 --- a/test/common/network/addr_family_aware_socket_option_impl_test.cc +++ b/test/common/network/addr_family_aware_socket_option_impl_test.cc @@ -48,7 +48,7 @@ TEST_F(AddrFamilyAwareSocketOptionImplTest, SetOptionFailure) { TEST_F(AddrFamilyAwareSocketOptionImplTest, SetOptionSuccess) { Address::Ipv4Instance address("1.2.3.4", 5678); IoHandlePtr io_handle = Network::SocketInterfaceSingleton::get().socket( - Address::SocketType::Stream, std::make_shared(address)); + Socket::Type::Stream, std::make_shared(address)); EXPECT_CALL(testing::Const(socket_), ioHandle()).WillRepeatedly(testing::ReturnRef(*io_handle)); AddrFamilyAwareSocketOptionImpl socket_option{ @@ -64,7 +64,7 @@ TEST_F(AddrFamilyAwareSocketOptionImplTest, SetOptionSuccess) { TEST_F(AddrFamilyAwareSocketOptionImplTest, V4EmptyOptionNames) { Address::Ipv4Instance address("1.2.3.4", 5678); IoHandlePtr io_handle = Network::SocketInterfaceSingleton::get().socket( - Address::SocketType::Stream, std::make_shared(address)); + Socket::Type::Stream, std::make_shared(address)); EXPECT_CALL(testing::Const(socket_), ioHandle()).WillRepeatedly(testing::ReturnRef(*io_handle)); AddrFamilyAwareSocketOptionImpl socket_option{ envoy::config::core::v3::SocketOption::STATE_PREBIND, {}, {}, 1}; @@ -78,7 +78,7 @@ TEST_F(AddrFamilyAwareSocketOptionImplTest, V4EmptyOptionNames) { TEST_F(AddrFamilyAwareSocketOptionImplTest, V6EmptyOptionNames) { Address::Ipv6Instance address("::1:2:3:4", 5678); IoHandlePtr io_handle = Network::SocketInterfaceSingleton::get().socket( - Address::SocketType::Stream, std::make_shared(address)); + Socket::Type::Stream, std::make_shared(address)); EXPECT_CALL(testing::Const(socket_), ioHandle()).WillRepeatedly(testing::ReturnRef(*io_handle)); AddrFamilyAwareSocketOptionImpl socket_option{ envoy::config::core::v3::SocketOption::STATE_PREBIND, {}, {}, 1}; @@ -93,7 +93,7 @@ TEST_F(AddrFamilyAwareSocketOptionImplTest, V6EmptyOptionNames) { TEST_F(AddrFamilyAwareSocketOptionImplTest, V4IgnoreV6) { Address::Ipv4Instance address("1.2.3.4", 5678); IoHandlePtr io_handle = Network::SocketInterfaceSingleton::get().socket( - Address::SocketType::Stream, std::make_shared(address)); + Socket::Type::Stream, std::make_shared(address)); EXPECT_CALL(testing::Const(socket_), ioHandle()).WillRepeatedly(testing::ReturnRef(*io_handle)); AddrFamilyAwareSocketOptionImpl socket_option{ @@ -107,7 +107,7 @@ TEST_F(AddrFamilyAwareSocketOptionImplTest, V4IgnoreV6) { TEST_F(AddrFamilyAwareSocketOptionImplTest, V6Only) { Address::Ipv6Instance address("::1:2:3:4", 5678); IoHandlePtr io_handle = Network::SocketInterfaceSingleton::get().socket( - Address::SocketType::Stream, std::make_shared(address)); + Socket::Type::Stream, std::make_shared(address)); EXPECT_CALL(testing::Const(socket_), ioHandle()).WillRepeatedly(testing::ReturnRef(*io_handle)); AddrFamilyAwareSocketOptionImpl socket_option{ @@ -124,7 +124,7 @@ TEST_F(AddrFamilyAwareSocketOptionImplTest, V6Only) { TEST_F(AddrFamilyAwareSocketOptionImplTest, V6OnlyV4Fallback) { Address::Ipv6Instance address("::1:2:3:4", 5678); IoHandlePtr io_handle = Network::SocketInterfaceSingleton::get().socket( - Address::SocketType::Stream, std::make_shared(address)); + Socket::Type::Stream, std::make_shared(address)); EXPECT_CALL(testing::Const(socket_), ioHandle()).WillRepeatedly(testing::ReturnRef(*io_handle)); AddrFamilyAwareSocketOptionImpl socket_option{ @@ -141,7 +141,7 @@ TEST_F(AddrFamilyAwareSocketOptionImplTest, V6OnlyV4Fallback) { TEST_F(AddrFamilyAwareSocketOptionImplTest, V6Precedence) { Address::Ipv6Instance address("::1:2:3:4", 5678); IoHandlePtr io_handle = Network::SocketInterfaceSingleton::get().socket( - Address::SocketType::Stream, std::make_shared(address)); + Socket::Type::Stream, std::make_shared(address)); EXPECT_CALL(testing::Const(socket_), ioHandle()).WillRepeatedly(testing::ReturnRef(*io_handle)); AddrFamilyAwareSocketOptionImpl socket_option{ @@ -155,7 +155,7 @@ TEST_F(AddrFamilyAwareSocketOptionImplTest, V6Precedence) { TEST_F(AddrFamilyAwareSocketOptionImplTest, V4GetSocketOptionName) { Address::Ipv4Instance address("1.2.3.4", 5678); IoHandlePtr io_handle = Network::SocketInterfaceSingleton::get().socket( - Address::SocketType::Stream, std::make_shared(address)); + Socket::Type::Stream, std::make_shared(address)); EXPECT_CALL(testing::Const(socket_), ioHandle()).WillRepeatedly(testing::ReturnRef(*io_handle)); AddrFamilyAwareSocketOptionImpl socket_option{ @@ -171,7 +171,7 @@ TEST_F(AddrFamilyAwareSocketOptionImplTest, V4GetSocketOptionName) { TEST_F(AddrFamilyAwareSocketOptionImplTest, V6GetSocketOptionName) { Address::Ipv6Instance address("2::1", 5678); IoHandlePtr io_handle = Network::SocketInterfaceSingleton::get().socket( - Address::SocketType::Stream, std::make_shared(address)); + Socket::Type::Stream, std::make_shared(address)); EXPECT_CALL(testing::Const(socket_), ioHandle()).WillRepeatedly(testing::ReturnRef(*io_handle)); AddrFamilyAwareSocketOptionImpl socket_option{ diff --git a/test/common/network/address_impl_test.cc b/test/common/network/address_impl_test.cc index e8c2a1b33fc3..3935004acf74 100644 --- a/test/common/network/address_impl_test.cc +++ b/test/common/network/address_impl_test.cc @@ -43,13 +43,13 @@ void testSocketBindAndConnect(Network::Address::IpVersion ip_version, bool v6onl ASSERT_NE(addr_port, nullptr); if (addr_port->ip()->port() == 0) { - addr_port = Network::Test::findOrCheckFreePort(addr_port, SocketType::Stream); + addr_port = Network::Test::findOrCheckFreePort(addr_port, Socket::Type::Stream); } ASSERT_NE(addr_port, nullptr); ASSERT_NE(addr_port->ip(), nullptr); // Create a socket on which we'll listen for connections from clients. - SocketImpl sock(SocketType::Stream, addr_port); + SocketImpl sock(Socket::Type::Stream, addr_port); ASSERT_GE(sock.ioHandle().fd(), 0) << addr_port->asString(); // Check that IPv6 sockets accept IPv6 connections only. @@ -71,7 +71,7 @@ void testSocketBindAndConnect(Network::Address::IpVersion ip_version, bool v6onl auto client_connect = [](Address::InstanceConstSharedPtr addr_port) { // Create a client socket and connect to the server. - SocketImpl client_sock(SocketType::Stream, addr_port); + SocketImpl client_sock(Socket::Type::Stream, addr_port); ASSERT_GE(client_sock.ioHandle().fd(), 0) << addr_port->asString(); @@ -326,7 +326,7 @@ TEST(PipeInstanceTest, BasicPermission) { const mode_t mode = 0777; PipeInstance pipe(path, mode); InstanceConstSharedPtr address = std::make_shared(pipe); - SocketImpl sock(SocketType::Stream, address); + SocketImpl sock(Socket::Type::Stream, address); ASSERT_GE(sock.ioHandle().fd(), 0) << pipe.asString(); @@ -353,7 +353,7 @@ TEST(PipeInstanceTest, PermissionFail) { const mode_t mode = 0777; PipeInstance pipe(path, mode); InstanceConstSharedPtr address = std::make_shared(pipe); - SocketImpl sock(SocketType::Stream, address); + SocketImpl sock(Socket::Type::Stream, address); ASSERT_GE(sock.ioHandle().fd(), 0) << pipe.asString(); @@ -425,7 +425,7 @@ TEST(PipeInstanceTest, UnlinksExistingFile) { const auto bind_uds_socket = [](const std::string& path) { PipeInstance pipe(path); InstanceConstSharedPtr address = std::make_shared(pipe); - SocketImpl sock(SocketType::Stream, address); + SocketImpl sock(Socket::Type::Stream, address); ASSERT_GE(sock.ioHandle().fd(), 0) << pipe.asString(); diff --git a/test/common/network/listen_socket_impl_test.cc b/test/common/network/listen_socket_impl_test.cc index 4b2d98442d75..ae595d3ced28 100644 --- a/test/common/network/listen_socket_impl_test.cc +++ b/test/common/network/listen_socket_impl_test.cc @@ -20,7 +20,7 @@ namespace Envoy { namespace Network { namespace { -template +template class ListenSocketImplTest : public testing::TestWithParam { protected: ListenSocketImplTest() : version_(GetParam()) {} @@ -44,7 +44,7 @@ class ListenSocketImplTest : public testing::TestWithParam { while (true) { ++loop_number; - auto addr_fd = Network::Test::bindFreeLoopbackPort(version_, Address::SocketType::Stream); + auto addr_fd = Network::Test::bindFreeLoopbackPort(version_, Socket::Type::Stream); auto addr = addr_fd.first; SocketPtr& sock = addr_fd.second; EXPECT_TRUE(SOCKET_VALID(sock->ioHandle().fd())); @@ -82,7 +82,7 @@ class ListenSocketImplTest : public testing::TestWithParam { // TODO (conqerAtapple): This is unfortunate. We should be able to templatize this // instead of if block. auto os_sys_calls = Api::OsSysCallsSingleton::get(); - if (NetworkSocketTrait::type == Address::SocketType::Stream) { + if (NetworkSocketTrait::type == Socket::Type::Stream) { EXPECT_EQ(0, socket1->listen(0).rc_); } @@ -125,8 +125,8 @@ class ListenSocketImplTest : public testing::TestWithParam { } }; -using ListenSocketImplTestTcp = ListenSocketImplTest; -using ListenSocketImplTestUdp = ListenSocketImplTest; +using ListenSocketImplTestTcp = ListenSocketImplTest; +using ListenSocketImplTestUdp = ListenSocketImplTest; INSTANTIATE_TEST_SUITE_P(IpVersions, ListenSocketImplTestTcp, testing::ValuesIn(TestEnvironment::getIpVersionsForTest()), @@ -146,7 +146,7 @@ class TestListenSocket : public ListenSocketImpl { public: TestListenSocket(Address::InstanceConstSharedPtr address) : ListenSocketImpl(std::make_unique(), address) {} - Address::SocketType socketType() const override { return Address::SocketType::Stream; } + Socket::Type socketType() const override { return Socket::Type::Stream; } }; TEST_P(ListenSocketImplTestTcp, SetLocalAddress) { diff --git a/test/common/network/listener_impl_test_base.h b/test/common/network/listener_impl_test_base.h index 19884ba46982..8f3ee21b8727 100644 --- a/test/common/network/listener_impl_test_base.h +++ b/test/common/network/listener_impl_test_base.h @@ -20,7 +20,7 @@ class ListenerImplTestBase : public testing::TestWithParam { ListenerImplTestBase() : version_(GetParam()), alt_address_(Network::Test::findOrCheckFreePort( - Network::Test::getCanonicalLoopbackAddress(version_), Address::SocketType::Stream)), + Network::Test::getCanonicalLoopbackAddress(version_), Socket::Type::Stream)), api_(Api::createApiForTest()), dispatcher_(api_->allocateDispatcher("test_thread")) {} Event::DispatcherImpl& dispatcherImpl() { diff --git a/test/common/network/utility_test.cc b/test/common/network/utility_test.cc index 49e99f755fa2..8039f7c873b5 100644 --- a/test/common/network/utility_test.cc +++ b/test/common/network/utility_test.cc @@ -350,24 +350,24 @@ TEST(NetworkUtility, ProtobufAddressSocketType) { { envoy::config::core::v3::Address proto_address; proto_address.mutable_socket_address(); - EXPECT_EQ(Address::SocketType::Stream, Utility::protobufAddressSocketType(proto_address)); + EXPECT_EQ(Socket::Type::Stream, Utility::protobufAddressSocketType(proto_address)); } { envoy::config::core::v3::Address proto_address; proto_address.mutable_socket_address()->set_protocol( envoy::config::core::v3::SocketAddress::TCP); - EXPECT_EQ(Address::SocketType::Stream, Utility::protobufAddressSocketType(proto_address)); + EXPECT_EQ(Socket::Type::Stream, Utility::protobufAddressSocketType(proto_address)); } { envoy::config::core::v3::Address proto_address; proto_address.mutable_socket_address()->set_protocol( envoy::config::core::v3::SocketAddress::UDP); - EXPECT_EQ(Address::SocketType::Datagram, Utility::protobufAddressSocketType(proto_address)); + EXPECT_EQ(Socket::Type::Datagram, Utility::protobufAddressSocketType(proto_address)); } { envoy::config::core::v3::Address proto_address; proto_address.mutable_pipe(); - EXPECT_EQ(Address::SocketType::Stream, Utility::protobufAddressSocketType(proto_address)); + EXPECT_EQ(Socket::Type::Stream, Utility::protobufAddressSocketType(proto_address)); } } diff --git a/test/common/secret/secret_manager_impl_test.cc b/test/common/secret/secret_manager_impl_test.cc index 4ee5fe108806..3790fbdf7c95 100644 --- a/test/common/secret/secret_manager_impl_test.cc +++ b/test/common/secret/secret_manager_impl_test.cc @@ -267,7 +267,7 @@ TEST_F(SecretManagerImplTest, DeduplicateDynamicTlsCertificateSecretProvider) { init_target_handle = target.createHandle("test"); })); EXPECT_CALL(secret_context, stats()).WillRepeatedly(ReturnRef(stats)); - EXPECT_CALL(secret_context, initManager()).WillRepeatedly(Return(&init_manager)); + EXPECT_CALL(secret_context, initManager()).WillRepeatedly(ReturnRef(init_manager)); EXPECT_CALL(secret_context, dispatcher()).WillRepeatedly(ReturnRef(dispatcher)); EXPECT_CALL(secret_context, localInfo()).WillRepeatedly(ReturnRef(local_info)); @@ -350,7 +350,7 @@ TEST_F(SecretManagerImplTest, SdsDynamicSecretUpdateSuccess) { init_target_handle = target.createHandle("test"); })); EXPECT_CALL(secret_context, stats()).WillOnce(ReturnRef(stats)); - EXPECT_CALL(secret_context, initManager()).WillRepeatedly(Return(&init_manager)); + EXPECT_CALL(secret_context, initManager()).WillRepeatedly(ReturnRef(init_manager)); EXPECT_CALL(secret_context, dispatcher()).WillRepeatedly(ReturnRef(*dispatcher_)); EXPECT_CALL(secret_context, localInfo()).WillOnce(ReturnRef(local_info)); EXPECT_CALL(secret_context, api()).WillRepeatedly(ReturnRef(*api_)); @@ -400,7 +400,7 @@ TEST_F(SecretManagerImplTest, SdsDynamicGenericSecret) { EXPECT_CALL(secret_context, dispatcher()).WillRepeatedly(ReturnRef(*dispatcher_)); EXPECT_CALL(secret_context, messageValidationVisitor()).WillOnce(ReturnRef(validation_visitor)); EXPECT_CALL(secret_context, stats()).WillOnce(ReturnRef(stats)); - EXPECT_CALL(secret_context, initManager()).WillRepeatedly(Return(&init_manager)); + EXPECT_CALL(secret_context, initManager()).WillRepeatedly(ReturnRef(init_manager)); EXPECT_CALL(secret_context, localInfo()).WillOnce(ReturnRef(local_info)); EXPECT_CALL(secret_context, api()).WillRepeatedly(ReturnRef(*api_)); EXPECT_CALL(init_manager, add(_)) @@ -450,7 +450,7 @@ TEST_F(SecretManagerImplTest, ConfigDumpHandler) { init_target_handle = target.createHandle("test"); })); EXPECT_CALL(secret_context, stats()).WillRepeatedly(ReturnRef(stats)); - EXPECT_CALL(secret_context, initManager()).WillRepeatedly(Return(&init_manager)); + EXPECT_CALL(secret_context, initManager()).WillRepeatedly(ReturnRef(init_manager)); EXPECT_CALL(secret_context, dispatcher()).WillRepeatedly(ReturnRef(dispatcher)); EXPECT_CALL(secret_context, localInfo()).WillRepeatedly(ReturnRef(local_info)); @@ -708,7 +708,7 @@ TEST_F(SecretManagerImplTest, ConfigDumpHandlerWarmingSecrets) { init_target_handle = target.createHandle("test"); })); EXPECT_CALL(secret_context, stats()).WillRepeatedly(ReturnRef(stats)); - EXPECT_CALL(secret_context, initManager()).WillRepeatedly(Return(&init_manager)); + EXPECT_CALL(secret_context, initManager()).WillRepeatedly(ReturnRef(init_manager)); EXPECT_CALL(secret_context, dispatcher()).WillRepeatedly(ReturnRef(dispatcher)); EXPECT_CALL(secret_context, localInfo()).WillRepeatedly(ReturnRef(local_info)); @@ -841,7 +841,7 @@ TEST_F(SecretManagerImplTest, ConfigDumpHandlerStaticSecrets) { init_target_handle = target.createHandle("test"); })); EXPECT_CALL(secret_context, stats()).WillRepeatedly(ReturnRef(stats)); - EXPECT_CALL(secret_context, initManager()).WillRepeatedly(Return(&init_manager)); + EXPECT_CALL(secret_context, initManager()).WillRepeatedly(ReturnRef(init_manager)); EXPECT_CALL(secret_context, dispatcher()).WillRepeatedly(ReturnRef(dispatcher)); EXPECT_CALL(secret_context, localInfo()).WillRepeatedly(ReturnRef(local_info)); @@ -913,7 +913,7 @@ TEST_F(SecretManagerImplTest, ConfigDumpHandlerStaticValidationContext) { init_target_handle = target.createHandle("test"); })); EXPECT_CALL(secret_context, stats()).WillRepeatedly(ReturnRef(stats)); - EXPECT_CALL(secret_context, initManager()).WillRepeatedly(Return(&init_manager)); + EXPECT_CALL(secret_context, initManager()).WillRepeatedly(ReturnRef(init_manager)); EXPECT_CALL(secret_context, dispatcher()).WillRepeatedly(ReturnRef(dispatcher)); EXPECT_CALL(secret_context, localInfo()).WillRepeatedly(ReturnRef(local_info)); @@ -958,7 +958,7 @@ TEST_F(SecretManagerImplTest, ConfigDumpHandlerStaticSessionTicketsContext) { init_target_handle = target.createHandle("test"); })); EXPECT_CALL(secret_context, stats()).WillRepeatedly(ReturnRef(stats)); - EXPECT_CALL(secret_context, initManager()).WillRepeatedly(Return(&init_manager)); + EXPECT_CALL(secret_context, initManager()).WillRepeatedly(ReturnRef(init_manager)); EXPECT_CALL(secret_context, dispatcher()).WillRepeatedly(ReturnRef(dispatcher)); EXPECT_CALL(secret_context, localInfo()).WillRepeatedly(ReturnRef(local_info)); diff --git a/test/config/utility.cc b/test/config/utility.cc index afd05d26bd50..cfbe5a92bf11 100644 --- a/test/config/utility.cc +++ b/test/config/utility.cc @@ -1003,6 +1003,8 @@ void EdsHelper::setEds(const std::vector& cluster_load_assignments, IntegrationTestServerStats& server_stats) { + // Make sure the last version has been accepted before setting a new one. + server_stats.waitForCounterGe("cluster.cluster_0.update_success", update_successes_); setEds(cluster_load_assignments); // Make sure Envoy has consumed the update now that it is running. ++update_successes_; diff --git a/test/extensions/common/proxy_protocol/proxy_protocol_regression_test.cc b/test/extensions/common/proxy_protocol/proxy_protocol_regression_test.cc index 701df21eb901..e51fbb8b6fa3 100644 --- a/test/extensions/common/proxy_protocol/proxy_protocol_regression_test.cc +++ b/test/extensions/common/proxy_protocol/proxy_protocol_regression_test.cc @@ -47,8 +47,7 @@ class ProxyProtocolRegressionTest : public testing::TestWithParamlocalAddress())); EXPECT_CALL(socket_factory_, getListenSocket()).WillOnce(Return(socket_)); connection_handler_->addListener(absl::nullopt, *this); diff --git a/test/extensions/filters/http/buffer/buffer_filter_integration_test.cc b/test/extensions/filters/http/buffer/buffer_filter_integration_test.cc index d5fbb21fa0f4..c61b6e175368 100644 --- a/test/extensions/filters/http/buffer/buffer_filter_integration_test.cc +++ b/test/extensions/filters/http/buffer/buffer_filter_integration_test.cc @@ -102,7 +102,7 @@ TEST_P(BufferIntegrationTest, RouterRequestBufferLimitExceeded) { // the response is read. config_helper_.addConfigModifier( [](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager& - hcm) { hcm.mutable_delayed_close_timeout()->set_seconds(1000 * 1000); }); + hcm) { hcm.mutable_delayed_close_timeout()->set_seconds(2000 * 1000); }); config_helper_.addFilter(ConfigHelper::smallBufferFilter()); initialize(); diff --git a/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc b/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc index 74dc142304e2..75d52d3b4235 100644 --- a/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc +++ b/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc @@ -60,8 +60,7 @@ class ProxyProtocolTest : public testing::TestWithParamlocalAddress())); EXPECT_CALL(socket_factory_, getListenSocket()).WillOnce(Return(socket_)); connection_handler_->addListener(absl::nullopt, *this); @@ -1001,8 +1000,7 @@ class WildcardProxyProtocolTest : public testing::TestWithParamlocalAddress()->ip()->port())), connection_handler_(new Server::ConnectionHandlerImpl(*dispatcher_)), name_("proxy"), filter_chain_(Network::Test::createEmptyFilterChainWithRawBufferSockets()) { - EXPECT_CALL(socket_factory_, socketType()) - .WillOnce(Return(Network::Address::SocketType::Stream)); + EXPECT_CALL(socket_factory_, socketType()).WillOnce(Return(Network::Socket::Type::Stream)); EXPECT_CALL(socket_factory_, localAddress()).WillOnce(ReturnRef(socket_->localAddress())); EXPECT_CALL(socket_factory_, getListenSocket()).WillOnce(Return(socket_)); connection_handler_->addListener(absl::nullopt, *this); diff --git a/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc b/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc index c3836ac38163..ff4598da2b3a 100644 --- a/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc +++ b/test/extensions/quic_listeners/quiche/active_quic_listener_test.cc @@ -68,8 +68,8 @@ class ActiveQuicListenerFactoryPeer { class ActiveQuicListenerTest : public testing::TestWithParam { protected: - using Socket = Network::NetworkListenSocket< - Network::NetworkSocketTrait>; + using Socket = + Network::NetworkListenSocket>; ActiveQuicListenerTest() : version_(GetParam()), api_(Api::createApiForTest(simulated_time_system_)), diff --git a/test/extensions/quic_listeners/quiche/envoy_quic_dispatcher_test.cc b/test/extensions/quic_listeners/quiche/envoy_quic_dispatcher_test.cc index 6162165935db..f6357feb98be 100644 --- a/test/extensions/quic_listeners/quiche/envoy_quic_dispatcher_test.cc +++ b/test/extensions/quic_listeners/quiche/envoy_quic_dispatcher_test.cc @@ -51,7 +51,7 @@ class EnvoyQuicDispatcherTest : public testing::TestWithParamallocateDispatcher("test_thread")), listen_socket_(std::make_unique>>( + Network::NetworkSocketTrait>>( Network::Test::getCanonicalLoopbackAddress(version_), nullptr, /*bind*/ true)), connection_helper_(*dispatcher_), crypto_config_(quic::QuicCryptoServerConfig::TESTING, quic::QuicRandom::GetInstance(), diff --git a/test/extensions/quic_listeners/quiche/platform/quic_platform_test.cc b/test/extensions/quic_listeners/quiche/platform/quic_platform_test.cc index dc8ecf64a852..916195b9dc13 100644 --- a/test/extensions/quic_listeners/quiche/platform/quic_platform_test.cc +++ b/test/extensions/quic_listeners/quiche/platform/quic_platform_test.cc @@ -670,7 +670,7 @@ TEST_F(QuicPlatformTest, PickUnsedPort) { Envoy::Network::Test::getCanonicalLoopbackAddress(ip_version); Envoy::Network::Address::InstanceConstSharedPtr addr_with_port = Envoy::Network::Utility::getAddressWithPort(*addr, port); - Envoy::Network::SocketImpl sock(Envoy::Network::Address::SocketType::Datagram, addr_with_port); + Envoy::Network::SocketImpl sock(Envoy::Network::Socket::Type::Datagram, addr_with_port); // binding of given port should success. EXPECT_EQ(0, sock.bind(addr_with_port).rc_); } diff --git a/test/extensions/quic_listeners/quiche/platform/quic_port_utils_impl.cc b/test/extensions/quic_listeners/quiche/platform/quic_port_utils_impl.cc index de3f39a001a7..ab902f546073 100644 --- a/test/extensions/quic_listeners/quiche/platform/quic_port_utils_impl.cc +++ b/test/extensions/quic_listeners/quiche/platform/quic_port_utils_impl.cc @@ -32,8 +32,8 @@ int QuicPickServerPortForTestsOrDieImpl() { fmt::format("{}:{}", Envoy::Network::Test::getAnyAddressUrlString(ip_version), /*port*/ 0), /*v6only*/ false); ASSERT(addr_port != nullptr); - addr_port = Envoy::Network::Test::findOrCheckFreePort( - addr_port, Envoy::Network::Address::SocketType::Datagram); + addr_port = + Envoy::Network::Test::findOrCheckFreePort(addr_port, Envoy::Network::Socket::Type::Datagram); if (addr_port != nullptr && addr_port->ip() != nullptr) { // Find a port. return addr_port->ip()->port(); diff --git a/test/extensions/stats_sinks/common/statsd/udp_statsd_test.cc b/test/extensions/stats_sinks/common/statsd/udp_statsd_test.cc index afd7ede41339..9bad9d78a873 100644 --- a/test/extensions/stats_sinks/common/statsd/udp_statsd_test.cc +++ b/test/extensions/stats_sinks/common/statsd/udp_statsd_test.cc @@ -52,7 +52,7 @@ TEST(UdpOverUdsStatsdSinkTest, InitWithPipeAddress) { sink.flush(snapshot); // Start the server. - Network::SocketImpl sock(Network::Address::SocketType::Datagram, uds_address); + Network::SocketImpl sock(Network::Socket::Type::Datagram, uds_address); RELEASE_ASSERT(sock.setBlockingForTest(false).rc_ != -1, ""); sock.bind(uds_address); diff --git a/test/extensions/transport_sockets/tls/context_impl_test.cc b/test/extensions/transport_sockets/tls/context_impl_test.cc index cb790eff3b99..468e22f0bd26 100644 --- a/test/extensions/transport_sockets/tls/context_impl_test.cc +++ b/test/extensions/transport_sockets/tls/context_impl_test.cc @@ -593,7 +593,7 @@ TEST_F(SslServerContextImplTicketTest, TicketKeySdsNotReady) { // EXPECT_CALL(factory_context_, random()).WillOnce(ReturnRef(random)); EXPECT_CALL(factory_context_, stats()).WillOnce(ReturnRef(stats)); EXPECT_CALL(factory_context_, clusterManager()).WillOnce(ReturnRef(cluster_manager)); - EXPECT_CALL(factory_context_, initManager()).WillRepeatedly(Return(&init_manager)); + EXPECT_CALL(factory_context_, initManager()).WillRepeatedly(ReturnRef(init_manager)); auto* sds_secret_configs = tls_context.mutable_session_ticket_keys_sds_secret_config(); sds_secret_configs->set_name("abc.com"); sds_secret_configs->mutable_sds_config(); @@ -1001,7 +1001,7 @@ TEST_F(ClientContextConfigImplTest, SecretNotReady) { NiceMock dispatcher; EXPECT_CALL(factory_context_, localInfo()).WillOnce(ReturnRef(local_info)); EXPECT_CALL(factory_context_, stats()).WillOnce(ReturnRef(stats)); - EXPECT_CALL(factory_context_, initManager()).WillRepeatedly(Return(&init_manager)); + EXPECT_CALL(factory_context_, initManager()).WillRepeatedly(ReturnRef(init_manager)); EXPECT_CALL(factory_context_, dispatcher()).WillRepeatedly(ReturnRef(dispatcher)); auto sds_secret_configs = tls_context.mutable_common_tls_context()->mutable_tls_certificate_sds_secret_configs()->Add(); @@ -1033,7 +1033,7 @@ TEST_F(ClientContextConfigImplTest, ValidationContextNotReady) { NiceMock dispatcher; EXPECT_CALL(factory_context_, localInfo()).WillOnce(ReturnRef(local_info)); EXPECT_CALL(factory_context_, stats()).WillOnce(ReturnRef(stats)); - EXPECT_CALL(factory_context_, initManager()).WillRepeatedly(Return(&init_manager)); + EXPECT_CALL(factory_context_, initManager()).WillRepeatedly(ReturnRef(init_manager)); EXPECT_CALL(factory_context_, dispatcher()).WillRepeatedly(ReturnRef(dispatcher)); auto sds_secret_configs = tls_context.mutable_common_tls_context()->mutable_validation_context_sds_secret_config(); @@ -1339,7 +1339,7 @@ TEST_F(ServerContextConfigImplTest, SecretNotReady) { NiceMock dispatcher; EXPECT_CALL(factory_context_, localInfo()).WillOnce(ReturnRef(local_info)); EXPECT_CALL(factory_context_, stats()).WillOnce(ReturnRef(stats)); - EXPECT_CALL(factory_context_, initManager()).WillRepeatedly(Return(&init_manager)); + EXPECT_CALL(factory_context_, initManager()).WillRepeatedly(ReturnRef(init_manager)); EXPECT_CALL(factory_context_, dispatcher()).WillRepeatedly(ReturnRef(dispatcher)); auto sds_secret_configs = tls_context.mutable_common_tls_context()->mutable_tls_certificate_sds_secret_configs()->Add(); @@ -1371,7 +1371,7 @@ TEST_F(ServerContextConfigImplTest, ValidationContextNotReady) { NiceMock dispatcher; EXPECT_CALL(factory_context_, localInfo()).WillOnce(ReturnRef(local_info)); EXPECT_CALL(factory_context_, stats()).WillOnce(ReturnRef(stats)); - EXPECT_CALL(factory_context_, initManager()).WillRepeatedly(Return(&init_manager)); + EXPECT_CALL(factory_context_, initManager()).WillRepeatedly(ReturnRef(init_manager)); EXPECT_CALL(factory_context_, dispatcher()).WillRepeatedly(ReturnRef(dispatcher)); auto sds_secret_configs = tls_context.mutable_common_tls_context()->mutable_validation_context_sds_secret_config(); diff --git a/test/extensions/transport_sockets/tls/ssl_socket_test.cc b/test/extensions/transport_sockets/tls/ssl_socket_test.cc index 49b98320ff9d..aa2c429a4ed0 100644 --- a/test/extensions/transport_sockets/tls/ssl_socket_test.cc +++ b/test/extensions/transport_sockets/tls/ssl_socket_test.cc @@ -4211,7 +4211,7 @@ TEST_P(SslSocketTest, DownstreamNotReadySslSocket) { EXPECT_CALL(factory_context, dispatcher()).WillRepeatedly(ReturnRef(dispatcher)); EXPECT_CALL(factory_context, localInfo()).WillOnce(ReturnRef(local_info)); EXPECT_CALL(factory_context, stats()).WillOnce(ReturnRef(stats_store)); - EXPECT_CALL(factory_context, initManager()).WillRepeatedly(Return(&init_manager)); + EXPECT_CALL(factory_context, initManager()).WillRepeatedly(ReturnRef(init_manager)); envoy::extensions::transport_sockets::tls::v3::DownstreamTlsContext tls_context; auto sds_secret_configs = @@ -4246,7 +4246,7 @@ TEST_P(SslSocketTest, UpstreamNotReadySslSocket) { NiceMock dispatcher; EXPECT_CALL(factory_context, localInfo()).WillOnce(ReturnRef(local_info)); EXPECT_CALL(factory_context, stats()).WillOnce(ReturnRef(stats_store)); - EXPECT_CALL(factory_context, initManager()).WillRepeatedly(Return(&init_manager)); + EXPECT_CALL(factory_context, initManager()).WillRepeatedly(ReturnRef(init_manager)); EXPECT_CALL(factory_context, dispatcher()).WillRepeatedly(ReturnRef(dispatcher)); envoy::extensions::transport_sockets::tls::v3::UpstreamTlsContext tls_context; diff --git a/test/integration/cds_integration_test.cc b/test/integration/cds_integration_test.cc index 8a28eeb652c8..1e1790bfd8fa 100644 --- a/test/integration/cds_integration_test.cc +++ b/test/integration/cds_integration_test.cc @@ -147,6 +147,7 @@ INSTANTIATE_TEST_SUITE_P(IpVersionsClientTypeDelta, CdsIntegrationTest, TEST_P(CdsIntegrationTest, CdsClusterUpDownUp) { // Calls our initialize(), which includes establishing a listener, route, and cluster. testRouterHeaderOnlyRequestAndResponse(nullptr, UpstreamIndex1, "/cluster1"); + test_server_->waitForCounterGe("cluster_manager.cluster_added", 1); // Tell Envoy that cluster_1 is gone. EXPECT_TRUE(compareDiscoveryRequest(Config::TypeUrl::get().Cluster, "55", {}, {}, {})); diff --git a/test/integration/fake_upstream.h b/test/integration/fake_upstream.h index ec2bb436b291..d93536f6a925 100644 --- a/test/integration/fake_upstream.h +++ b/test/integration/fake_upstream.h @@ -643,7 +643,7 @@ class FakeUpstream : Logger::Loggable, FakeListenSocketFactory(Network::SocketSharedPtr socket) : socket_(socket) {} // Network::ListenSocketFactory - Network::Address::SocketType socketType() const override { return socket_->socketType(); } + Network::Socket::Type socketType() const override { return socket_->socketType(); } const Network::Address::InstanceConstSharedPtr& localAddress() const override { return socket_->localAddress(); diff --git a/test/integration/integration.cc b/test/integration/integration.cc index 5ba1f7e746ab..73e9aa8a4b10 100644 --- a/test/integration/integration.cc +++ b/test/integration/integration.cc @@ -457,7 +457,7 @@ void BaseIntegrationTest::createGeneratedApiTestServer( test_server_ = IntegrationTestServer::create( bootstrap_path, version_, on_server_ready_function_, on_server_init_function_, deterministic_, timeSystem(), *api_, defer_listener_finalization_, process_object_, validator_config, - concurrency_, drain_time_); + concurrency_, drain_time_, use_real_stats_); if (config_helper_.bootstrap().static_resources().listeners_size() > 0 && !defer_listener_finalization_) { diff --git a/test/integration/integration.h b/test/integration/integration.h index 3ec31c3eff66..31ee89af7127 100644 --- a/test/integration/integration.h +++ b/test/integration/integration.h @@ -447,6 +447,10 @@ class BaseIntegrationTest : protected Logger::Loggable { bool use_lds_{true}; // Use the integration framework's LDS set up. Grpc::SotwOrDelta sotw_or_delta_{Grpc::SotwOrDelta::Sotw}; + // By default the test server will use custom stats to notify on increment. + // This override exists for tests measuring stats memory. + bool use_real_stats_{}; + private: // The type for the Envoy-to-backend connection FakeHttpConnection::Type upstream_protocol_{FakeHttpConnection::Type::HTTP1}; diff --git a/test/integration/server.cc b/test/integration/server.cc index ec9469b919c1..fa3e14af7602 100644 --- a/test/integration/server.cc +++ b/test/integration/server.cc @@ -58,9 +58,9 @@ IntegrationTestServerPtr IntegrationTestServer::create( std::function on_server_init_function, bool deterministic, Event::TestTimeSystem& time_system, Api::Api& api, bool defer_listener_finalization, ProcessObjectOptRef process_object, Server::FieldValidationConfig validation_config, - uint32_t concurrency, std::chrono::seconds drain_time) { + uint32_t concurrency, std::chrono::seconds drain_time, bool use_real_stats) { IntegrationTestServerPtr server{ - std::make_unique(time_system, api, config_path)}; + std::make_unique(time_system, api, config_path, use_real_stats)}; if (server_ready_function != nullptr) { server->setOnServerReadyCb(server_ready_function); } @@ -182,6 +182,16 @@ void IntegrationTestServer::threadRoutine(const Network::Address::IpVersion vers lock, *this, std::move(random_generator), process_object); } +IntegrationTestServerImpl::IntegrationTestServerImpl(Event::TestTimeSystem& time_system, + Api::Api& api, const std::string& config_path, + bool use_real_stats) + : IntegrationTestServer(time_system, api, config_path), + symbol_table_(Stats::SymbolTableCreator::makeSymbolTable()) { + stats_allocator_ = + (use_real_stats ? std::make_unique(*symbol_table_) + : std::make_unique(*symbol_table_)); +} + void IntegrationTestServerImpl::createAndRunEnvoyServer( OptionsImpl& options, Event::TimeSystem& time_system, Network::Address::InstanceConstSharedPtr local_address, ListenerHooks& hooks, @@ -189,11 +199,9 @@ void IntegrationTestServerImpl::createAndRunEnvoyServer( Runtime::RandomGeneratorPtr&& random_generator, ProcessObjectOptRef process_object) { { Init::ManagerImpl init_manager{"Server"}; - Stats::SymbolTablePtr symbol_table = Stats::SymbolTableCreator::makeSymbolTable(); Server::HotRestartNopImpl restarter; ThreadLocal::InstanceImpl tls; - Stats::AllocatorImpl stats_allocator(*symbol_table); - Stats::ThreadLocalStoreImpl stat_store(stats_allocator); + Stats::ThreadLocalStoreImpl stat_store(*stats_allocator_); std::unique_ptr process_context; if (process_object.has_value()) { process_context = std::make_unique(process_object->get()); diff --git a/test/integration/server.h b/test/integration/server.h index 531a5574e2cb..5862cd1f1bbf 100644 --- a/test/integration/server.h +++ b/test/integration/server.h @@ -15,6 +15,7 @@ #include "common/common/lock_guard.h" #include "common/common/logger.h" #include "common/common/thread.h" +#include "common/stats/allocator_impl.h" #include "server/drain_manager_impl.h" #include "server/listener_hooks.h" @@ -152,6 +153,98 @@ class TestScopeWrapper : public Scope { ScopePtr wrapped_scope_; }; +// A counter which signals on a condition variable when it is incremented. +class NotifyingCounter : public Stats::Counter { +public: + NotifyingCounter(Stats::Counter* counter, absl::Mutex& mutex, absl::CondVar& condvar) + : counter_(counter), mutex_(mutex), condvar_(condvar) {} + + std::string name() const override { return counter_->name(); } + StatName statName() const override { return counter_->statName(); } + TagVector tags() const override { return counter_->tags(); } + std::string tagExtractedName() const override { return counter_->tagExtractedName(); } + void iterateTagStatNames(const TagStatNameIterFn& fn) const override { + counter_->iterateTagStatNames(fn); + } + void add(uint64_t amount) override { + counter_->add(amount); + absl::MutexLock l(&mutex_); + condvar_.Signal(); + } + void inc() override { add(1); } + uint64_t latch() override { return counter_->latch(); } + void reset() override { return counter_->reset(); } + uint64_t value() const override { return counter_->value(); } + void incRefCount() override { counter_->incRefCount(); } + bool decRefCount() override { return counter_->decRefCount(); } + uint32_t use_count() const override { return counter_->use_count(); } + StatName tagExtractedStatName() const override { return counter_->tagExtractedStatName(); } + bool used() const override { return counter_->used(); } + SymbolTable& symbolTable() override { return counter_->symbolTable(); } + const SymbolTable& constSymbolTable() const override { return counter_->constSymbolTable(); } + +private: + std::unique_ptr counter_; + absl::Mutex& mutex_; + absl::CondVar& condvar_; +}; + +// A stats allocator which creates NotifyingCounters rather than regular CounterImpls. +class NotifyingAllocatorImpl : public Stats::AllocatorImpl { +public: + using Stats::AllocatorImpl::AllocatorImpl; + + virtual void waitForCounterFromStringEq(const std::string& name, uint64_t value) { + absl::MutexLock l(&mutex_); + ENVOY_LOG_MISC(trace, "waiting for {} to be {}", name, value); + while (getCounterLockHeld(name) == nullptr || getCounterLockHeld(name)->value() != value) { + condvar_.Wait(&mutex_); + } + ENVOY_LOG_MISC(trace, "done waiting for {} to be {}", name, value); + } + + virtual void waitForCounterFromStringGe(const std::string& name, uint64_t value) { + absl::MutexLock l(&mutex_); + ENVOY_LOG_MISC(trace, "waiting for {} to be {}", name, value); + while (getCounterLockHeld(name) == nullptr || getCounterLockHeld(name)->value() < value) { + condvar_.Wait(&mutex_); + } + ENVOY_LOG_MISC(trace, "done waiting for {} to be {}", name, value); + } + +protected: + Stats::Counter* makeCounterInternal(StatName name, StatName tag_extracted_name, + const StatNameTagVector& stat_name_tags) override { + Stats::Counter* counter = new NotifyingCounter( + Stats::AllocatorImpl::makeCounterInternal(name, tag_extracted_name, stat_name_tags), mutex_, + condvar_); + { + absl::MutexLock l(&mutex_); + // Allow getting the counter directly from the allocator, since it's harder to + // signal when the counter has been added to a given stats store. + counters_.emplace(counter->name(), counter); + if (counter->name() == "cluster_manager.cluster_removed") { + } + condvar_.Signal(); + } + return counter; + } + + virtual Stats::Counter* getCounterLockHeld(const std::string& name) + EXCLUSIVE_LOCKS_REQUIRED(mutex_) { + auto it = counters_.find(name); + if (it != counters_.end()) { + return it->second; + } + return nullptr; + } + +private: + absl::flat_hash_map counters_; + absl::Mutex mutex_; + absl::CondVar condvar_; +}; + /** * This is a variant of the isolated store that has locking across all operations so that it can * be used during the integration tests. @@ -275,7 +368,8 @@ class IntegrationTestServer : public Logger::Loggable, bool defer_listener_finalization = false, ProcessObjectOptRef process_object = absl::nullopt, Server::FieldValidationConfig validation_config = Server::FieldValidationConfig(), - uint32_t concurrency = 1, std::chrono::seconds drain_time = std::chrono::seconds(1)); + uint32_t concurrency = 1, std::chrono::seconds drain_time = std::chrono::seconds(1), + bool use_real_stats = false); // Note that the derived class is responsible for tearing down the server in its // destructor. ~IntegrationTestServer() override; @@ -301,11 +395,11 @@ class IntegrationTestServer : public Logger::Loggable, std::chrono::seconds drain_time); void waitForCounterEq(const std::string& name, uint64_t value) override { - TestUtility::waitForCounterEq(statStore(), name, value, time_system_); + notifyingStatsAllocator().waitForCounterFromStringEq(name, value); } void waitForCounterGe(const std::string& name, uint64_t value) override { - TestUtility::waitForCounterGe(statStore(), name, value, time_system_); + notifyingStatsAllocator().waitForCounterFromStringGe(name, value); } void waitForGaugeGe(const std::string& name, uint64_t value) override { @@ -351,6 +445,7 @@ class IntegrationTestServer : public Logger::Loggable, virtual Server::Instance& server() PURE; virtual Stats::Store& statStore() PURE; virtual Network::Address::InstanceConstSharedPtr adminAddress() PURE; + virtual Stats::NotifyingAllocatorImpl& notifyingStatsAllocator() PURE; void useAdminInterfaceToQuit(bool use) { use_admin_interface_to_quit_ = use; } bool useAdminInterfaceToQuit() { return use_admin_interface_to_quit_; } @@ -404,8 +499,7 @@ class IntegrationTestServer : public Logger::Loggable, class IntegrationTestServerImpl : public IntegrationTestServer { public: IntegrationTestServerImpl(Event::TestTimeSystem& time_system, Api::Api& api, - const std::string& config_path) - : IntegrationTestServer(time_system, api, config_path) {} + const std::string& config_path, bool real_stats = false); ~IntegrationTestServerImpl() override; @@ -419,6 +513,13 @@ class IntegrationTestServerImpl : public IntegrationTestServer { } Network::Address::InstanceConstSharedPtr adminAddress() override { return admin_address_; } + Stats::NotifyingAllocatorImpl& notifyingStatsAllocator() override { + auto* ret = dynamic_cast(stats_allocator_.get()); + RELEASE_ASSERT(ret != nullptr, + "notifyingStatsAllocator() is not created when real_stats is true"); + return *ret; + } + private: void createAndRunEnvoyServer(OptionsImpl& options, Event::TimeSystem& time_system, Network::Address::InstanceConstSharedPtr local_address, @@ -433,6 +534,8 @@ class IntegrationTestServerImpl : public IntegrationTestServer { Stats::Store* stat_store_{}; Network::Address::InstanceConstSharedPtr admin_address_; absl::Notification server_gone_; + Stats::SymbolTablePtr symbol_table_; + std::unique_ptr stats_allocator_; }; } // namespace Envoy diff --git a/test/integration/stats_integration_test.cc b/test/integration/stats_integration_test.cc index 914657507162..43985c20fead 100644 --- a/test/integration/stats_integration_test.cc +++ b/test/integration/stats_integration_test.cc @@ -144,7 +144,9 @@ TEST_P(StatsIntegrationTest, WithTagSpecifierWithFixedValue) { class ClusterMemoryTestHelper : public BaseIntegrationTest { public: ClusterMemoryTestHelper() - : BaseIntegrationTest(testing::TestWithParam::GetParam()) {} + : BaseIntegrationTest(testing::TestWithParam::GetParam()) { + use_real_stats_ = true; + } static size_t computeMemoryDelta(int initial_num_clusters, int initial_num_hosts, int final_num_clusters, int final_num_hosts, bool allow_stats) { diff --git a/test/integration/xds_integration_test.cc b/test/integration/xds_integration_test.cc index 833e2d73f65c..b76f8c476b4a 100644 --- a/test/integration/xds_integration_test.cc +++ b/test/integration/xds_integration_test.cc @@ -196,6 +196,7 @@ TEST_P(LdsInplaceUpdateTcpProxyIntegrationTest, ReloadConfigDeletingFilterChain) TEST_P(LdsInplaceUpdateTcpProxyIntegrationTest, ReloadConfigAddingFilterChain) { setUpstreamCount(2); initialize(); + test_server_->waitForCounterGe("listener_manager.listener_create_success", 1); std::string response_0; auto client_conn_0 = createConnectionAndWrite("alpn0", "hello", response_0); @@ -368,6 +369,7 @@ TEST_P(LdsInplaceUpdateHttpIntegrationTest, ReloadConfigDeletingFilterChain) { // chain 2. TEST_P(LdsInplaceUpdateHttpIntegrationTest, ReloadConfigAddingFilterChain) { initialize(); + test_server_->waitForCounterGe("listener_manager.listener_create_success", 1); auto codec_client_0 = createHttpCodec("alpn0"); Cleanup cleanup0([c0 = codec_client_0.get()]() { c0->close(); }); diff --git a/test/mocks/network/mocks.h b/test/mocks/network/mocks.h index c967b2b18b5b..e4e58290ed58 100644 --- a/test/mocks/network/mocks.h +++ b/test/mocks/network/mocks.h @@ -219,17 +219,16 @@ class MockListenSocket : public Socket { MOCK_METHOD(void, setLocalAddress, (const Address::InstanceConstSharedPtr&)); MOCK_METHOD(IoHandle&, ioHandle, ()); MOCK_METHOD(const IoHandle&, ioHandle, (), (const)); - MOCK_METHOD(Address::SocketType, socketType, (), (const)); + MOCK_METHOD(Socket::Type, socketType, (), (const)); MOCK_METHOD(Address::Type, addressType, (), (const)); MOCK_METHOD(void, close, ()); MOCK_METHOD(bool, isOpen, (), (const)); MOCK_METHOD(void, addOption_, (const Socket::OptionConstSharedPtr& option)); MOCK_METHOD(void, addOptions_, (const Socket::OptionsSharedPtr& options)); MOCK_METHOD(const OptionsSharedPtr&, options, (), (const)); - MOCK_METHOD(IoHandlePtr, socket, (Address::SocketType, Address::Type, Address::IpVersion), + MOCK_METHOD(IoHandlePtr, socket, (Socket::Type, Address::Type, Address::IpVersion), (const)); + MOCK_METHOD(IoHandlePtr, socketForAddrPtr, (Socket::Type, const Address::InstanceConstSharedPtr), (const)); - MOCK_METHOD(IoHandlePtr, socketForAddrPtr, - (Address::SocketType, const Address::InstanceConstSharedPtr), (const)); MOCK_METHOD(Api::SysCallIntResult, bind, (const Address::InstanceConstSharedPtr)); MOCK_METHOD(Api::SysCallIntResult, connect, (const Address::InstanceConstSharedPtr)); MOCK_METHOD(Api::SysCallIntResult, listen, (int)); @@ -281,14 +280,13 @@ class MockConnectionSocket : public ConnectionSocket { MOCK_METHOD(const Network::ConnectionSocket::OptionsSharedPtr&, options, (), (const)); MOCK_METHOD(IoHandle&, ioHandle, ()); MOCK_METHOD(const IoHandle&, ioHandle, (), (const)); - MOCK_METHOD(Address::SocketType, socketType, (), (const)); + MOCK_METHOD(Socket::Type, socketType, (), (const)); MOCK_METHOD(Address::Type, addressType, (), (const)); MOCK_METHOD(void, close, ()); MOCK_METHOD(bool, isOpen, (), (const)); - MOCK_METHOD(IoHandlePtr, socket, (Address::SocketType, Address::Type, Address::IpVersion), + MOCK_METHOD(IoHandlePtr, socket, (Socket::Type, Address::Type, Address::IpVersion), (const)); + MOCK_METHOD(IoHandlePtr, socketForAddrPtr, (Socket::Type, const Address::InstanceConstSharedPtr), (const)); - MOCK_METHOD(IoHandlePtr, socketForAddrPtr, - (Address::SocketType, const Address::InstanceConstSharedPtr), (const)); MOCK_METHOD(Api::SysCallIntResult, bind, (const Address::InstanceConstSharedPtr)); MOCK_METHOD(Api::SysCallIntResult, connect, (const Address::InstanceConstSharedPtr)); MOCK_METHOD(Api::SysCallIntResult, listen, (int)); @@ -318,7 +316,7 @@ class MockListenSocketFactory : public ListenSocketFactory { public: MockListenSocketFactory() = default; - MOCK_METHOD(Network::Address::SocketType, socketType, (), (const)); + MOCK_METHOD(Network::Socket::Type, socketType, (), (const)); MOCK_METHOD(const Network::Address::InstanceConstSharedPtr&, localAddress, (), (const)); MOCK_METHOD(Network::SocketSharedPtr, getListenSocket, ()); MOCK_METHOD(SocketOptRef, sharedSocket, (), (const)); @@ -419,7 +417,7 @@ class MockResolvedAddress : public Address::Instance { MOCK_METHOD(Api::SysCallIntResult, connect, (os_fd_t), (const)); MOCK_METHOD(Address::Ip*, ip, (), (const)); MOCK_METHOD(Address::Pipe*, pipe, (), (const)); - MOCK_METHOD(IoHandlePtr, socket, (Address::SocketType), (const)); + MOCK_METHOD(IoHandlePtr, socket, (Socket::Type), (const)); MOCK_METHOD(Address::Type, type, (), (const)); MOCK_METHOD(sockaddr*, sockAddr, (), (const)); MOCK_METHOD(socklen_t, sockAddrLen, (), (const)); diff --git a/test/mocks/server/mocks.cc b/test/mocks/server/mocks.cc index 0ea123678448..e5b7fb43c63b 100644 --- a/test/mocks/server/mocks.cc +++ b/test/mocks/server/mocks.cc @@ -99,8 +99,7 @@ MockOverloadManager::~MockOverloadManager() = default; MockListenerComponentFactory::MockListenerComponentFactory() : socket_(std::make_shared>()) { ON_CALL(*this, createListenSocket(_, _, _, _)) - .WillByDefault(Invoke([&](Network::Address::InstanceConstSharedPtr, - Network::Address::SocketType, + .WillByDefault(Invoke([&](Network::Address::InstanceConstSharedPtr, Network::Socket::Type, const Network::Socket::OptionsSharedPtr& options, const ListenSocketCreationParams&) -> Network::SocketSharedPtr { if (!Network::Socket::applyOptions(options, *socket_, diff --git a/test/mocks/server/mocks.h b/test/mocks/server/mocks.h index 1fb58dc9ea0b..ac53f79c51ff 100644 --- a/test/mocks/server/mocks.h +++ b/test/mocks/server/mocks.h @@ -273,8 +273,7 @@ class MockListenerComponentFactory : public ListenerComponentFactory { (const Protobuf::RepeatedPtrField&, Configuration::ListenerFactoryContext& context)); MOCK_METHOD(Network::SocketSharedPtr, createListenSocket, - (Network::Address::InstanceConstSharedPtr address, - Network::Address::SocketType socket_type, + (Network::Address::InstanceConstSharedPtr address, Network::Socket::Type socket_type, const Network::Socket::OptionsSharedPtr& options, const ListenSocketCreationParams& params)); MOCK_METHOD(DrainManager*, createDrainManager_, @@ -515,6 +514,8 @@ class MockServerFactoryContext : public virtual ServerFactoryContext { MOCK_METHOD(Api::Api&, api, ()); Grpc::Context& grpcContext() override { return grpc_context_; } MOCK_METHOD(Server::DrainManager&, drainManager, ()); + MOCK_METHOD(Init::Manager&, initManager, ()); + MOCK_METHOD(ServerLifecycleNotifier&, lifecycleNotifier, ()); testing::NiceMock cluster_manager_; testing::NiceMock dispatcher_; @@ -604,7 +605,7 @@ class MockTransportSocketFactoryContext : public TransportSocketFactoryContext { MOCK_METHOD(Event::Dispatcher&, dispatcher, ()); MOCK_METHOD(Envoy::Runtime::RandomGenerator&, random, ()); MOCK_METHOD(Stats::Store&, stats, ()); - MOCK_METHOD(Init::Manager*, initManager, ()); + MOCK_METHOD(Init::Manager&, initManager, ()); MOCK_METHOD(Singleton::Manager&, singletonManager, ()); MOCK_METHOD(ThreadLocal::SlotAllocator&, threadLocal, ()); MOCK_METHOD(ProtobufMessage::ValidationVisitor&, messageValidationVisitor, ()); diff --git a/test/server/connection_handler_test.cc b/test/server/connection_handler_test.cc index 247810612f14..65703ba56dca 100644 --- a/test/server/connection_handler_test.cc +++ b/test/server/connection_handler_test.cc @@ -50,8 +50,7 @@ class ConnectionHandlerTest : public testing::Test, protected Logger::Loggable> filter_chain_manager = nullptr) @@ -126,7 +125,7 @@ class ConnectionHandlerTest : public testing::Test, protected Logger::Loggable> overridden_filter_chain_manager = @@ -142,7 +141,7 @@ class ConnectionHandlerTest : public testing::Test, protected Logger::Loggablesocket_)); - if (socket_type == Network::Address::SocketType::Stream) { + if (socket_type == Network::Socket::Type::Stream) { EXPECT_CALL(dispatcher_, createListener_(_, _, _)) .WillOnce(Invoke([listener, listener_callbacks](Network::SocketSharedPtr&&, Network::ListenerCallbacks& cb, @@ -649,7 +648,7 @@ TEST_F(ConnectionHandlerTest, ContinueOnListenerFilterTimeout) { auto listener = new NiceMock(); TestListener* test_listener = addListener(1, true, false, "test_listener", listener, &listener_callbacks, nullptr, nullptr, - Network::Address::SocketType::Stream, std::chrono::milliseconds(15000), true); + Network::Socket::Type::Stream, std::chrono::milliseconds(15000), true); EXPECT_CALL(*socket_factory_, localAddress()).WillRepeatedly(ReturnRef(local_address_)); handler_->addListener(absl::nullopt, *test_listener); @@ -733,7 +732,7 @@ TEST_F(ConnectionHandlerTest, ListenerFilterDisabledTimeout) { auto listener = new NiceMock(); TestListener* test_listener = addListener(1, true, false, "test_listener", listener, &listener_callbacks, nullptr, nullptr, - Network::Address::SocketType::Stream, std::chrono::milliseconds()); + Network::Socket::Type::Stream, std::chrono::milliseconds()); EXPECT_CALL(*socket_factory_, localAddress()).WillRepeatedly(ReturnRef(local_address_)); handler_->addListener(absl::nullopt, *test_listener); @@ -803,7 +802,7 @@ TEST_F(ConnectionHandlerTest, UdpListenerNoFilterThrowsException) { auto listener = new NiceMock(); TestListener* test_listener = addListener(1, true, false, "test_listener", listener, nullptr, nullptr, nullptr, - Network::Address::SocketType::Datagram, std::chrono::milliseconds()); + Network::Socket::Type::Datagram, std::chrono::milliseconds()); EXPECT_CALL(factory_, createUdpListenerFilterChain(_, _)) .WillOnce(Invoke([&](Network::UdpListenerFilterManager&, Network::UdpReadFilterCallbacks&) -> bool { return true; })); @@ -838,7 +837,7 @@ TEST_F(ConnectionHandlerTest, TcpListenerInplaceUpdate) { std::make_shared>(); TestListener* new_test_listener = addListener(new_listener_tag, true, false, "test_listener", /* Network::Listener */ nullptr, - &new_listener_callbacks, nullptr, nullptr, Network::Address::SocketType::Stream, + &new_listener_callbacks, nullptr, nullptr, Network::Socket::Type::Stream, std::chrono::milliseconds(15000), false, overridden_filter_chain_manager); handler_->addListener(old_listener_tag, *new_test_listener); ASSERT_EQ(new_listener_callbacks, nullptr) diff --git a/test/server/filter_chain_benchmark_test.cc b/test/server/filter_chain_benchmark_test.cc index 600855709f2d..18cb7a5caf67 100644 --- a/test/server/filter_chain_benchmark_test.cc +++ b/test/server/filter_chain_benchmark_test.cc @@ -89,9 +89,7 @@ class MockConnectionSocket : public Network::ConnectionSocket { // Dummy method void close() override {} bool isOpen() const override { return false; } - Network::Address::SocketType socketType() const override { - return Network::Address::SocketType::Stream; - } + Network::Socket::Type socketType() const override { return Network::Socket::Type::Stream; } Network::Address::Type addressType() const override { return local_address_->type(); } void setLocalAddress(const Network::Address::InstanceConstSharedPtr&) override {} void restoreLocalAddress(const Network::Address::InstanceConstSharedPtr&) override {} diff --git a/test/server/listener_manager_impl_test.cc b/test/server/listener_manager_impl_test.cc index 6d3f7d887ac3..a9c169fd5b55 100644 --- a/test/server/listener_manager_impl_test.cc +++ b/test/server/listener_manager_impl_test.cc @@ -336,13 +336,12 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, UdpAddress) { EXPECT_CALL(server_.random_, uuid()); EXPECT_CALL(*worker_, addListener(_, _, _)); EXPECT_CALL(listener_factory_, - createListenSocket(_, Network::Address::SocketType::Datagram, _, {{true, false}})) - .WillOnce( - Invoke([this](const Network::Address::InstanceConstSharedPtr&, - Network::Address::SocketType, const Network::Socket::OptionsSharedPtr&, - const ListenSocketCreationParams&) -> Network::SocketSharedPtr { - return listener_factory_.socket_; - })); + createListenSocket(_, Network::Socket::Type::Datagram, _, {{true, false}})) + .WillOnce(Invoke([this](const Network::Address::InstanceConstSharedPtr&, + Network::Socket::Type, const Network::Socket::OptionsSharedPtr&, + const ListenSocketCreationParams&) -> Network::SocketSharedPtr { + return listener_factory_.socket_; + })); EXPECT_CALL(*listener_factory_.socket_, setSocketOption(_, _, _, _)).Times(testing::AtLeast(1)); EXPECT_CALL(os_sys_calls_, close(_)).WillRepeatedly(Return(Api::SysCallIntResult{0, errno})); manager_->addOrUpdateListener(listener_proto, "", true); @@ -1370,7 +1369,7 @@ name: foo EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, ListenSocketCreationParams(false))) .WillOnce(Invoke([this, &syscall_result, &real_listener_factory]( const Network::Address::InstanceConstSharedPtr& address, - Network::Address::SocketType socket_type, + Network::Socket::Type socket_type, const Network::Socket::OptionsSharedPtr& options, const ListenSocketCreationParams& params) -> Network::SocketSharedPtr { EXPECT_CALL(server_, hotRestart).Times(0); @@ -1407,7 +1406,7 @@ reuse_port: true EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, {{true, false}})) .WillOnce(Invoke([this, &syscall_result, &real_listener_factory]( const Network::Address::InstanceConstSharedPtr& address, - Network::Address::SocketType socket_type, + Network::Socket::Type socket_type, const Network::Socket::OptionsSharedPtr& options, const ListenSocketCreationParams& params) -> Network::SocketSharedPtr { EXPECT_CALL(server_, hotRestart).Times(0); @@ -1423,7 +1422,7 @@ TEST_F(ListenerManagerImplTest, NotSupportedDatagramUds) { ProdListenerComponentFactory real_listener_factory(server_); EXPECT_THROW_WITH_MESSAGE(real_listener_factory.createListenSocket( std::make_shared("/foo"), - Network::Address::SocketType::Datagram, nullptr, {true}), + Network::Socket::Type::Datagram, nullptr, {true}), EnvoyException, "socket type SocketType::Datagram not supported for pipes"); } @@ -3647,7 +3646,7 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, TransparentFreebindListenerDisabl )EOF", Network::Address::IpVersion::v4); EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, {true})) - .WillOnce(Invoke([&](Network::Address::InstanceConstSharedPtr, Network::Address::SocketType, + .WillOnce(Invoke([&](Network::Address::InstanceConstSharedPtr, Network::Socket::Type, const Network::Socket::OptionsSharedPtr& options, const ListenSocketCreationParams&) -> Network::SocketSharedPtr { EXPECT_EQ(options, nullptr); diff --git a/test/server/listener_manager_impl_test.h b/test/server/listener_manager_impl_test.h index bd979c98ed14..d1f3256fd4a4 100644 --- a/test/server/listener_manager_impl_test.h +++ b/test/server/listener_manager_impl_test.h @@ -203,8 +203,7 @@ class ListenerManagerImplTest : public testing::Test { ListenSocketCreationParams expected_creation_params = {true, true}) { EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, expected_creation_params)) .WillOnce(Invoke([this, expected_num_options, &expected_state]( - const Network::Address::InstanceConstSharedPtr&, - Network::Address::SocketType, + const Network::Address::InstanceConstSharedPtr&, Network::Socket::Type, const Network::Socket::OptionsSharedPtr& options, const ListenSocketCreationParams&) -> Network::SocketSharedPtr { EXPECT_NE(options.get(), nullptr); diff --git a/test/test_common/environment.cc b/test/test_common/environment.cc index 45a6e9b883ba..c303a88aea7b 100644 --- a/test/test_common/environment.cc +++ b/test/test_common/environment.cc @@ -243,10 +243,12 @@ std::vector TestEnvironment::getIpVersionsForTest() if (TestEnvironment::shouldRunTestForIpVersion(version)) { parameters.push_back(version); if (!Network::Test::supportsIpVersion(version)) { - ENVOY_LOG_TO_LOGGER(Logger::Registry::getLog(Logger::Id::testing), warn, - "Testing with IP{} addresses may not be supported on this machine. If " - "testing fails, set the environment variable ENVOY_IP_TEST_VERSIONS.", - Network::Test::addressVersionAsString(version)); + const auto version_string = Network::Test::addressVersionAsString(version); + ENVOY_LOG_TO_LOGGER( + Logger::Registry::getLog(Logger::Id::testing), warn, + "Testing with IP{} addresses may not be supported on this machine. If " + "testing fails, set the environment variable ENVOY_IP_TEST_VERSIONS to 'v{}only'.", + version_string, version_string); } } } diff --git a/test/test_common/network_utility.cc b/test/test_common/network_utility.cc index 4eb091d78be5..5936b88c69c3 100644 --- a/test/test_common/network_utility.cc +++ b/test/test_common/network_utility.cc @@ -20,7 +20,7 @@ namespace Network { namespace Test { Address::InstanceConstSharedPtr findOrCheckFreePort(Address::InstanceConstSharedPtr addr_port, - Address::SocketType type) { + Socket::Type type) { if (addr_port == nullptr || addr_port->type() != Address::Type::Ip) { ADD_FAILURE() << "Not an internet address: " << (addr_port == nullptr ? "nullptr" : addr_port->asString()); @@ -34,7 +34,7 @@ Address::InstanceConstSharedPtr findOrCheckFreePort(Address::InstanceConstShared const char* failing_fn = nullptr; if (result.rc_ != 0) { failing_fn = "bind"; - } else if (type == Address::SocketType::Stream) { + } else if (type == Socket::Type::Stream) { // Try listening on the port also, if the type is TCP. result = sock.listen(1); if (result.rc_ != 0) { @@ -58,7 +58,7 @@ Address::InstanceConstSharedPtr findOrCheckFreePort(Address::InstanceConstShared } Address::InstanceConstSharedPtr findOrCheckFreePort(const std::string& addr_port, - Address::SocketType type) { + Socket::Type type) { auto instance = Utility::parseInternetAddressAndPort(addr_port); if (instance != nullptr) { instance = findOrCheckFreePort(instance, type); @@ -68,35 +68,35 @@ Address::InstanceConstSharedPtr findOrCheckFreePort(const std::string& addr_port return instance; } -const std::string getLoopbackAddressUrlString(const Address::IpVersion version) { +std::string getLoopbackAddressUrlString(const Address::IpVersion version) { if (version == Address::IpVersion::v6) { return std::string("[::1]"); } return std::string("127.0.0.1"); } -const std::string getLoopbackAddressString(const Address::IpVersion version) { +std::string getLoopbackAddressString(const Address::IpVersion version) { if (version == Address::IpVersion::v6) { return std::string("::1"); } return std::string("127.0.0.1"); } -const std::string getAnyAddressUrlString(const Address::IpVersion version) { +std::string getAnyAddressUrlString(const Address::IpVersion version) { if (version == Address::IpVersion::v6) { return std::string("[::]"); } return std::string("0.0.0.0"); } -const std::string getAnyAddressString(const Address::IpVersion version) { +std::string getAnyAddressString(const Address::IpVersion version) { if (version == Address::IpVersion::v6) { return std::string("::"); } return std::string("0.0.0.0"); } -const std::string addressVersionAsString(const Address::IpVersion version) { +std::string addressVersionAsString(const Address::IpVersion version) { if (version == Address::IpVersion::v4) { return std::string("v4"); } @@ -159,7 +159,7 @@ std::string ipVersionToDnsFamily(Network::Address::IpVersion version) { } std::pair -bindFreeLoopbackPort(Address::IpVersion version, Address::SocketType type) { +bindFreeLoopbackPort(Address::IpVersion version, Socket::Type type) { Address::InstanceConstSharedPtr addr = getCanonicalLoopbackAddress(version); SocketPtr sock = std::make_unique(type, addr); Api::SysCallIntResult result = sock->bind(addr); diff --git a/test/test_common/network_utility.h b/test/test_common/network_utility.h index 76c01a87cc62..36fa1868ea84 100644 --- a/test/test_common/network_utility.h +++ b/test/test_common/network_utility.h @@ -25,7 +25,7 @@ namespace Test { * listening, else nullptr if the address and port are not free. */ Address::InstanceConstSharedPtr findOrCheckFreePort(Address::InstanceConstSharedPtr addr_port, - Address::SocketType type); + Socket::Type type); /** * As above, but addr_port is specified as a string. For example: @@ -35,14 +35,14 @@ Address::InstanceConstSharedPtr findOrCheckFreePort(Address::InstanceConstShared * - [::]:45678 Check whether a specific port on all local IPv6 addresses is free. */ Address::InstanceConstSharedPtr findOrCheckFreePort(const std::string& addr_port, - Address::SocketType type); + Socket::Type type); /** * Get a URL ready IP loopback address as a string. * @param version IP address version of loopback address. * @return std::string URL ready loopback address as a string. */ -const std::string getLoopbackAddressUrlString(const Address::IpVersion version); +std::string getLoopbackAddressUrlString(const Address::IpVersion version); /** * Get a IP loopback address as a string. There are no square brackets around IPv6 addresses, this @@ -50,28 +50,28 @@ const std::string getLoopbackAddressUrlString(const Address::IpVersion version); * @param version IP address version of loopback address. * @return std::string loopback address as a string. */ -const std::string getLoopbackAddressString(const Address::IpVersion version); +std::string getLoopbackAddressString(const Address::IpVersion version); /** * Get a URL ready IP any address as a string. * @param version IP address version of any address. * @return std::string URL ready any address as a string. */ -const std::string getAnyAddressUrlString(const Address::IpVersion version); +std::string getAnyAddressUrlString(const Address::IpVersion version); /** * Get an IP any address as a string. * @param version IP address version of any address. * @return std::string any address as a string. */ -const std::string getAnyAddressString(const Address::IpVersion version); +std::string getAnyAddressString(const Address::IpVersion version); /** * Return a string version of enum IpVersion version. * @param version IP address version. * @return std::string string version of IpVersion. */ -const std::string addressVersionAsString(const Address::IpVersion version); +std::string addressVersionAsString(const Address::IpVersion version); /** * Returns a loopback address for the specified IP version (127.0.0.1 for IPv4 and ::1 for IPv6). @@ -115,7 +115,7 @@ std::string ipVersionToDnsFamily(Network::Address::IpVersion version); * @returns the address and the fd of the socket bound to that address. */ std::pair -bindFreeLoopbackPort(Address::IpVersion version, Address::SocketType type); +bindFreeLoopbackPort(Address::IpVersion version, Socket::Type type); /** * Create a transport socket for testing purposes. diff --git a/test/test_common/network_utility_test.cc b/test/test_common/network_utility_test.cc index b95e29a94617..dc40f1d28f2f 100644 --- a/test/test_common/network_utility_test.cc +++ b/test/test_common/network_utility_test.cc @@ -35,7 +35,7 @@ TEST_P(NetworkUtilityTest, DISABLED_ValidateBindFreeLoopbackPort) { std::map seen; const size_t kLimit = 50; for (size_t n = 0; n < kLimit; ++n) { - auto addr_fd = Network::Test::bindFreeLoopbackPort(version_, Address::SocketType::Stream); + auto addr_fd = Network::Test::bindFreeLoopbackPort(version_, Socket::Type::Stream); addr_fd.second->close(); auto addr = addr_fd.first->asString(); auto search = seen.find(addr); diff --git a/tools/protodoc/BUILD b/tools/protodoc/BUILD index 256316f2a18c..51bb3a9fbde9 100644 --- a/tools/protodoc/BUILD +++ b/tools/protodoc/BUILD @@ -1,8 +1,11 @@ load("@rules_python//python:defs.bzl", "py_binary") load("@protodoc_pip3//:requirements.bzl", "requirement") +load("//bazel:envoy_build_system.bzl", "envoy_package", "envoy_proto_library") licenses(["notice"]) # Apache 2 +envoy_package() + py_binary( name = "generate_empty", srcs = ["generate_empty.py"], @@ -10,12 +13,18 @@ py_binary( deps = [":protodoc"], ) +envoy_proto_library( + name = "manifest_proto", + srcs = ["manifest.proto"], +) + py_binary( name = "protodoc", srcs = ["protodoc.py"], - data = ["//docs:edge_defaults_manifest.yaml"], + data = ["//docs:protodoc_manifest.yaml"], visibility = ["//visibility:public"], deps = [ + ":manifest_proto_py_proto", "//tools/api_proto_plugin", "//tools/config_validation:validate_fragment", "@com_envoyproxy_protoc_gen_validate//validate:validate_py", diff --git a/tools/protodoc/manifest.proto b/tools/protodoc/manifest.proto new file mode 100644 index 000000000000..4757c76a8c10 --- /dev/null +++ b/tools/protodoc/manifest.proto @@ -0,0 +1,29 @@ +syntax = "proto3"; + +package tools.protodoc; + +import "google/protobuf/struct.proto"; + +// Additional structure information consumed by protodoc when generating +// documentation for a field. +message Description { + message EdgeConfiguration { + // Example secure edge default for the field. + google.protobuf.Value example = 1; + + // Additional note to include in the configuration warning. + string note = 2; + } + + // Additional information for when this field is used in edge deployments. + EdgeConfiguration edge_config = 1; + + // TODO: add additional information here to reflect things like Envoy + // implementation status. +} + +message Manifest { + // Map from fully qualified field name to additional information to be used in + // protodoc generation. + map fields = 1; +} diff --git a/tools/protodoc/protodoc.py b/tools/protodoc/protodoc.py index 750ca3cd78d1..c96f7cafa6c9 100755 --- a/tools/protodoc/protodoc.py +++ b/tools/protodoc/protodoc.py @@ -12,6 +12,7 @@ import string import sys +from google.protobuf import json_format from bazel_tools.tools.python.runfiles import runfiles import yaml @@ -27,6 +28,7 @@ from tools.api_proto_plugin import visitor from tools.config_validation import validate_fragment +from tools.protodoc import manifest_pb2 from udpa.annotations import security_pb2 from udpa.annotations import status_pb2 from validate import validate_pb2 @@ -401,7 +403,7 @@ def FormatAnchor(label): return '.. _%s:\n\n' % label -def FormatSecurityOptions(security_option, field, type_context, edge_default_yaml): +def FormatSecurityOptions(security_option, field, type_context, edge_config): sections = [] if security_option.configure_for_untrusted_downstream: @@ -410,10 +412,13 @@ def FormatSecurityOptions(security_option, field, type_context, edge_default_yam if security_option.configure_for_untrusted_upstream: sections.append( Indent(4, 'This field should be configured in the presence of untrusted *upstreams*.')) + if edge_config.note: + sections.append(Indent(4, edge_config.note)) - validate_fragment.ValidateFragment(field.type_name[1:], edge_default_yaml) + example_dict = json_format.MessageToDict(edge_config.example) + validate_fragment.ValidateFragment(field.type_name[1:], example_dict) field_name = type_context.name.split('.')[-1] - example = {field_name: edge_default_yaml} + example = {field_name: example_dict} sections.append( Indent(4, 'Example configuration for untrusted environments:\n\n') + Indent(4, '.. code-block:: yaml\n\n') + @@ -423,14 +428,14 @@ def FormatSecurityOptions(security_option, field, type_context, edge_default_yam return '.. attention::\n' + '\n\n'.join(sections) -def FormatFieldAsDefinitionListItem(outer_type_context, type_context, field, - edge_defaults_manifest): +def FormatFieldAsDefinitionListItem(outer_type_context, type_context, field, protodoc_manifest): """Format a FieldDescriptorProto as RST definition list item. Args: outer_type_context: contextual information for enclosing message. type_context: contextual information for message/enum/field. field: FieldDescriptorProto. + protodoc_manifest: tools.protodoc.Manifest for proto. Returns: RST formatted definition list item. @@ -479,11 +484,12 @@ def FormatFieldAsDefinitionListItem(outer_type_context, type_context, field, # If there is a udpa.annotations.security option, include it after the comment. if field.options.HasExtension(security_pb2.security): - edge_default_yaml = edge_defaults_manifest.get(type_context.name) - if not edge_default_yaml: - raise ProtodocError('Missing edge default YAML example for %s' % type_context.name) + manifest_description = protodoc_manifest.fields.get(type_context.name) + if not manifest_description: + raise ProtodocError('Missing protodoc manifest YAML for %s' % type_context.name) formatted_security_options = FormatSecurityOptions( - field.options.Extensions[security_pb2.security], field, type_context, edge_default_yaml) + field.options.Extensions[security_pb2.security], field, type_context, + manifest_description.edge_config) else: formatted_security_options = '' @@ -493,12 +499,13 @@ def FormatFieldAsDefinitionListItem(outer_type_context, type_context, field, Indent, 2), comment + formatted_oneof_comment) + formatted_security_options -def FormatMessageAsDefinitionList(type_context, msg, edge_defaults_manifest): +def FormatMessageAsDefinitionList(type_context, msg, protodoc_manifest): """Format a DescriptorProto as RST definition list. Args: type_context: contextual information for message/enum/field. msg: DescriptorProto. + protodoc_manifest: tools.protodoc.Manifest for proto. Returns: RST formatted definition list item. @@ -518,7 +525,7 @@ def FormatMessageAsDefinitionList(type_context, msg, edge_defaults_manifest): type_context.oneof_names[index] = oneof_decl.name return '\n'.join( FormatFieldAsDefinitionListItem(type_context, type_context.ExtendField(index, field.name), - field, edge_defaults_manifest) + field, protodoc_manifest) for index, field in enumerate(msg.field)) + '\n' @@ -574,8 +581,12 @@ class RstFormatVisitor(visitor.Visitor): def __init__(self): r = runfiles.Create() - with open(r.Rlocation('envoy/docs/edge_defaults_manifest.yaml'), 'r') as f: - self.edge_defaults_manifest = yaml.load(f.read()) + with open(r.Rlocation('envoy/docs/protodoc_manifest.yaml'), 'r') as f: + # Load as YAML, emit as JSON and then parse as proto to provide type + # checking. + protodoc_manifest_untyped = yaml.load(f.read()) + self.protodoc_manifest = manifest_pb2.Manifest() + json_format.Parse(json.dumps(protodoc_manifest_untyped), self.protodoc_manifest) def VisitEnum(self, enum_proto, type_context): normal_enum_type = NormalizeTypeContextName(type_context.name) @@ -606,7 +617,7 @@ def VisitMessage(self, msg_proto, type_context, nested_msgs, nested_enums): return anchor + header + proto_link + formatted_leading_comment + FormatMessageAsJson( type_context, msg_proto) + FormatMessageAsDefinitionList( type_context, msg_proto, - self.edge_defaults_manifest) + '\n'.join(nested_msgs) + '\n' + '\n'.join(nested_enums) + self.protodoc_manifest) + '\n'.join(nested_msgs) + '\n' + '\n'.join(nested_enums) def VisitFile(self, file_proto, type_context, services, msgs, enums): has_messages = True