Skip to content

Commit

Permalink
Merge branch 'main' into allow-upstream-half-close
Browse files Browse the repository at this point in the history
  • Loading branch information
yanavlasov committed Aug 26, 2024
2 parents c02ac47 + bd5bec9 commit d965ff6
Show file tree
Hide file tree
Showing 127 changed files with 1,136 additions and 617 deletions.
1 change: 1 addition & 0 deletions .clang-tidy
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ CheckOptions:
|Returns(Default)?WorkerId$|
|^sched_getaffinity$|
|^shutdownThread_$|
|^envoy_dynamic_module(.*)$|
|TEST|
|^use_count$)
- key: readability-identifier-naming.ParameterCase
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/codeql-daily.yml
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ jobs:

# Initializes the CodeQL tools for scanning.
- name: Initialize CodeQL
uses: github/codeql-action/init@f0f3afee809481da311ca3a6ff1ff51d81dbeb24 # codeql-bundle-v3.26.4
uses: github/codeql-action/init@2c779ab0d087cd7fe7b826087247c2c81f27bfa6 # codeql-bundle-v3.26.5
# Override language selection by uncommenting this and choosing your languages
with:
languages: cpp
Expand Down Expand Up @@ -68,4 +68,4 @@ jobs:
git clean -xdf
- name: Perform CodeQL Analysis
uses: github/codeql-action/analyze@f0f3afee809481da311ca3a6ff1ff51d81dbeb24 # codeql-bundle-v3.26.4
uses: github/codeql-action/analyze@2c779ab0d087cd7fe7b826087247c2c81f27bfa6 # codeql-bundle-v3.26.5
4 changes: 2 additions & 2 deletions .github/workflows/codeql-push.yml
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ jobs:

- name: Initialize CodeQL
if: ${{ env.BUILD_TARGETS != '' }}
uses: github/codeql-action/init@f0f3afee809481da311ca3a6ff1ff51d81dbeb24 # codeql-bundle-v3.26.4
uses: github/codeql-action/init@2c779ab0d087cd7fe7b826087247c2c81f27bfa6 # codeql-bundle-v3.26.5
with:
languages: cpp

Expand Down Expand Up @@ -109,4 +109,4 @@ jobs:
- name: Perform CodeQL Analysis
if: ${{ env.BUILD_TARGETS != '' }}
uses: github/codeql-action/analyze@f0f3afee809481da311ca3a6ff1ff51d81dbeb24 # codeql-bundle-v3.26.4
uses: github/codeql-action/analyze@2c779ab0d087cd7fe7b826087247c2c81f27bfa6 # codeql-bundle-v3.26.5
2 changes: 1 addition & 1 deletion .github/workflows/scorecard.yml
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,6 @@ jobs:
retention-days: 5

- name: "Upload to code-scanning"
uses: github/codeql-action/upload-sarif@f0f3afee809481da311ca3a6ff1ff51d81dbeb24 # v3.26.4
uses: github/codeql-action/upload-sarif@2c779ab0d087cd7fe7b826087247c2c81f27bfa6 # v3.26.5
with:
sarif_file: results.sarif
26 changes: 13 additions & 13 deletions CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ extensions/filters/common/original_src @klarose @mattklein123
# cdn_loop extension
/*/extensions/filters/http/cdn_loop @justin-mp @penguingao @alyssawilk
# external processing filter
/*/extensions/filters/http/ext_proc @gbrail @stevenzzzz @tyxia @mattklein123 @htuch @yanavlasov
/*/extensions/filters/common/mutation_rules @gbrail @tyxia @mattklein123 @htuch @yanavlasov
/*/extensions/filters/http/ext_proc @gbrail @stevenzzzz @tyxia @mattklein123 @yanavlasov
/*/extensions/filters/common/mutation_rules @gbrail @tyxia @mattklein123 @yanavlasov
# jwt_authn http filter extension
/*/extensions/filters/http/jwt_authn @taoxuy @lizan @tyxia @yanavlasov
# grpc_field_extraction http filter extension
Expand Down Expand Up @@ -68,7 +68,7 @@ extensions/filters/common/original_src @klarose @mattklein123
# tracers.skywalking extension
/*/extensions/tracers/skywalking @wbpcode @Shikugawa
# tracers.opentelemetry extension
/*/extensions/tracers/opentelemetry @alexanderellis @htuch
/*/extensions/tracers/opentelemetry @alexanderellis @yanavlasov
# quic extension
/*/extensions/quic/ @alyssawilk @danzh2010 @mattklein123 @mpwarres @wu-bin @ggreenway
# UDP packet writer
Expand Down Expand Up @@ -128,8 +128,8 @@ extensions/filters/common/original_src @klarose @mattklein123
/*/extensions/filters/http/connect_grpc_bridge @jchadwick-buf @mattklein123
/*/extensions/filters/common/original_src @klarose @mattklein123
/*/extensions/filters/listener/tls_inspector @ggreenway @KBaichoo
/*/extensions/grpc_credentials/example @wozz @htuch
/*/extensions/grpc_credentials/file_based_metadata @wozz @htuch
/*/extensions/grpc_credentials/example @wozz @yanavlasov
/*/extensions/grpc_credentials/file_based_metadata @wozz @yanavlasov
/*/extensions/internal_redirect @alyssawilk @penguingao
/*/extensions/stat_sinks/dog_statsd @taiki45 @jmarantz
/*/extensions/stat_sinks/graphite_statsd @vaccarium @mattklein123
Expand All @@ -138,22 +138,22 @@ extensions/filters/common/original_src @klarose @mattklein123
/*/extensions/stat_sinks/open_telemetry @ohadvano @mattklein123
# webassembly stat-sink extensions
/*/extensions/stat_sinks/wasm @mpwarres @lizan @UNOWNED
/*/extensions/resource_monitors/injected_resource @eziskind @htuch
/*/extensions/resource_monitors/common @eziskind @htuch @nezdolik
/*/extensions/resource_monitors/fixed_heap @eziskind @htuch @nezdolik
/*/extensions/resource_monitors/injected_resource @eziskind @yanavlasov
/*/extensions/resource_monitors/common @eziskind @yanavlasov @nezdolik
/*/extensions/resource_monitors/fixed_heap @eziskind @yanavlasov @nezdolik
/*/extensions/resource_monitors/downstream_connections @nezdolik @mattklein123
/*/extensions/retry/priority @alyssawilk @mattklein123
/*/extensions/retry/priority/previous_priorities @alyssawilk @mattklein123
/*/extensions/retry/host @alyssawilk @mattklein123
/*/extensions/filters/network/http_connection_manager @alyssawilk @mattklein123
/*/extensions/filters/network/tcp_proxy @alyssawilk @zuercher @ggreenway
/*/extensions/filters/network/echo @htuch @alyssawilk
/*/extensions/filters/network/echo @yanavlasov @alyssawilk
/*/extensions/filters/udp/dns_filter @mattklein123 @yanjunxiang-google
/*/extensions/filters/network/direct_response @kyessenov @zuercher
/*/extensions/filters/udp/udp_proxy @mattklein123 @danzh2010
/*/extensions/clusters/aggregate @yxue @mattklein123
# support for on-demand VHDS requests
/*/extensions/filters/http/on_demand @dmitri-d @htuch @kyessenov
/*/extensions/filters/http/on_demand @dmitri-d @yanavlasov @kyessenov
/*/extensions/filters/network/connection_limit @mattklein123 @alyssawilk @delong-coder
/*/extensions/filters/http/aws_request_signing @derekargueta @suniltheta @mattklein123 @marcomagdy @nbaws
/*/extensions/filters/http/aws_lambda @suniltheta @mattklein123 @marcomagdy @nbaws
Expand All @@ -170,7 +170,7 @@ extensions/filters/http/oauth2 @derekargueta @mattklein123
/*/extensions/filters/http/local_ratelimit @mattklein123 @wbpcode
/*/extensions/filters/common/local_ratelimit @mattklein123 @wbpcode
# HTTP Kill Request
/*/extensions/filters/http/kill_request @qqustc @htuch
/*/extensions/filters/http/kill_request @qqustc @yanavlasov
# Rate limit expression descriptor
/*/extensions/rate_limit_descriptors/expr @kyessenov @cpakulski
# hash input matcher
Expand Down Expand Up @@ -223,7 +223,7 @@ extensions/filters/http/oauth2 @derekargueta @mattklein123
# Key Value store
/*/extensions/key_value @alyssawilk @ryantheoptimist
# Config Validators
/*/extensions/config/validators/minimum_clusters @adisuissa @htuch
/*/extensions/config/validators/minimum_clusters @adisuissa @yanavlasov
# File system based extensions
/*/extensions/common/async_files @mattklein123 @ravenblackx
/*/extensions/filters/http/file_system_buffer @mattklein123 @ravenblackx
Expand Down Expand Up @@ -335,7 +335,7 @@ extensions/filters/http/oauth2 @derekargueta @mattklein123
# String matching extensions
/*/extensions/string_matcher/ @ggreenway @cpakulski
# Header mutation
/*/extensions/filters/http/header_mutation @wbpcode @htuch @soulxu
/*/extensions/filters/http/header_mutation @wbpcode @yanavlasov @soulxu
# Health checkers
/*/extensions/health_checkers/grpc @zuercher @botengyao
/*/extensions/health_checkers/http @zuercher @botengyao
Expand Down
3 changes: 1 addition & 2 deletions OWNERS.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@ routing PRs, questions, etc. to the right place.
* Matt Klein ([mattklein123](https://github.com/mattklein123)) ([email protected])
* Catch-all, "all the things", and generally trying to make himself obsolete as fast as
possible.
* Harvey Tuch ([htuch](https://github.com/htuch)) ([email protected])
* xDS APIs, configuration and control plane.
* Alyssa Wilk ([alyssawilk](https://github.com/alyssawilk)) ([email protected])
* HTTP, flow control, cluster manager, load balancing, and core networking (listeners,
connections, etc.), Envoy Mobile.
Expand Down Expand Up @@ -107,6 +105,7 @@ without further review.
* Rafal Augustyniak ([Augustyniak](https://github.com/Augustyniak)) ([email protected])
* Snow Pettersen ([snowp](https://github.com/snowp)) ([email protected])
* Lizan Zhou ([lizan](https://github.com/lizan)) ([email protected])
* Harvey Tuch ([htuch](https://github.com/htuch)) ([email protected])

# Friends of Envoy

Expand Down
1 change: 0 additions & 1 deletion SECURITY-INSIGHTS.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ project-lifecycle:
core-maintainers: # from https://github.com/envoyproxy/envoy/blob/main/OWNERS.md
# Senior maintainers
- github:mattklein123
- github:htuch
- github:alyssawilk
- github:zuercher
- github:lizan
Expand Down
6 changes: 3 additions & 3 deletions api/bazel/repository_locations.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -179,12 +179,12 @@ REPOSITORY_LOCATIONS_SPEC = dict(
project_name = "envoy_toolshed",
project_desc = "Tooling, libraries, runners and checkers for Envoy proxy's CI",
project_url = "https://github.com/envoyproxy/toolshed",
version = "0.1.4",
sha256 = "7ddfd251a89518b97c4eb8064a7d37454bbd998bf29e4cd3ad8f44227b5ca7b3",
version = "0.1.11",
sha256 = "f868812bff7ae372e4b53d565ee75a999d33e09b2980cc0c3dfa40684f85bbda",
strip_prefix = "toolshed-bazel-v{version}/bazel",
urls = ["https://github.com/envoyproxy/toolshed/archive/bazel-v{version}.tar.gz"],
use_category = ["build"],
release_date = "2024-07-22",
release_date = "2024-08-24",
cpe = "N/A",
license = "Apache-2.0",
license_url = "https://github.com/envoyproxy/envoy/blob/bazel-v{version}/LICENSE",
Expand Down
13 changes: 12 additions & 1 deletion api/envoy/config/cluster/v3/cluster.proto
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ message ClusterCollection {
}

// Configuration for a single upstream cluster.
// [#next-free-field: 58]
// [#next-free-field: 59]
message Cluster {
option (udpa.annotations.versioning).previous_message_type = "envoy.api.v2.Cluster";

Expand Down Expand Up @@ -956,6 +956,17 @@ message Cluster {
google.protobuf.Duration dns_refresh_rate = 16
[(validate.rules).duration = {gt {nanos: 1000000}}];

// DNS jitter can be optionally specified if the cluster type is either
// :ref:`STRICT_DNS<envoy_v3_api_enum_value_config.cluster.v3.Cluster.DiscoveryType.STRICT_DNS>`,
// or :ref:`LOGICAL_DNS<envoy_v3_api_enum_value_config.cluster.v3.Cluster.DiscoveryType.LOGICAL_DNS>`.
// DNS jitter causes the cluster to refresh DNS entries later by a random amount of time to avoid a
// stampede of DNS requests. This value sets the upper bound (exclusive) for the random amount.
// There will be no jitter if this value is omitted. For cluster types other than
// :ref:`STRICT_DNS<envoy_v3_api_enum_value_config.cluster.v3.Cluster.DiscoveryType.STRICT_DNS>`
// and :ref:`LOGICAL_DNS<envoy_v3_api_enum_value_config.cluster.v3.Cluster.DiscoveryType.LOGICAL_DNS>`
// this setting is ignored.
google.protobuf.Duration dns_jitter = 58;

// If the DNS failure refresh rate is specified and the cluster type is either
// :ref:`STRICT_DNS<envoy_v3_api_enum_value_config.cluster.v3.Cluster.DiscoveryType.STRICT_DNS>`,
// or :ref:`LOGICAL_DNS<envoy_v3_api_enum_value_config.cluster.v3.Cluster.DiscoveryType.LOGICAL_DNS>`,
Expand Down
28 changes: 28 additions & 0 deletions api/envoy/config/core/v3/socket_cmsg_headers.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
syntax = "proto3";

package envoy.config.core.v3;

import "google/protobuf/wrappers.proto";

import "udpa/annotations/status.proto";

option java_package = "io.envoyproxy.envoy.config.core.v3";
option java_outer_classname = "SocketCmsgHeadersProto";
option java_multiple_files = true;
option go_package = "github.com/envoyproxy/go-control-plane/envoy/config/core/v3;corev3";
option (udpa.annotations.file_status).package_version_status = ACTIVE;

// [#protodoc-title: Socket CMSG headers]

// Configuration for socket cmsg headers.
// See `:ref:CMSG <https://man7.org/linux/man-pages/man3/cmsg.3.html>`_ for further information.
message SocketCmsgHeaders {
// cmsg level. Default is unset.
google.protobuf.UInt32Value level = 1;

// cmsg type. Default is unset.
google.protobuf.UInt32Value type = 2;

// Expected size of cmsg value. Default is zero.
uint32 expected_size = 3;
}
10 changes: 9 additions & 1 deletion api/envoy/config/listener/v3/quic_config.proto
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package envoy.config.listener.v3;
import "envoy/config/core/v3/base.proto";
import "envoy/config/core/v3/extension.proto";
import "envoy/config/core/v3/protocol.proto";
import "envoy/config/core/v3/socket_cmsg_headers.proto";

import "google/protobuf/duration.proto";
import "google/protobuf/wrappers.proto";
Expand All @@ -24,7 +25,7 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE;
// [#protodoc-title: QUIC listener config]

// Configuration specific to the UDP QUIC listener.
// [#next-free-field: 12]
// [#next-free-field: 13]
message QuicProtocolOptions {
option (udpa.annotations.versioning).previous_message_type =
"envoy.api.v2.listener.QuicProtocolOptions";
Expand Down Expand Up @@ -86,4 +87,11 @@ message QuicProtocolOptions {
// If not specified, no debug visitor will be attached to connections.
// [#extension-category: envoy.quic.connection_debug_visitor]
core.v3.TypedExtensionConfig connection_debug_visitor_config = 11;

// Configure a type of UDP cmsg to pass to listener filters via QuicReceivedPacket.
// Both level and type must be specified for cmsg to be saved.
// Cmsg may be truncated or omitted if expected size is not set.
// If not specified, no cmsg will be saved to QuicReceivedPacket.
repeated core.v3.SocketCmsgHeaders save_cmsg_config = 12
[(validate.rules).repeated = {max_items: 1}];
}
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE;
// 1. the extracted field names/values will be wrapped in be ``field<StringValue
// or MapValue>`` -> ``values<ListValue of StringValue or StructValue>``, which will be added in the dynamic ``metadata<google.protobuf.Struct>``.
//
// 2. if the field value is empty, an empty ``<ListValue>`` will be set.
// 2. if the field value is empty, an empty ``Value`` will be set.
//
// Performance
// -----------
Expand Down
12 changes: 12 additions & 0 deletions changelogs/current.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,13 @@ removed_config_or_runtime:
Removed ``envoy.reloadable_features.normalize_host_for_preresolve_dfp_dns`` runtime flag and legacy code paths.
new_features:
- area: dns
change: |
for the :ref:`strict DNS <arch_overview_service_discovery_types_strict_dns>` and :ref:`logical DNS
<arch_overview_service_discovery_types_logical_dns>` cluster types,
the new :ref:`dns_jitter <envoy_v3_api_field_config.cluster.v3.Cluster.dns_jitter>` field, if
provided, will causes the cluster to refresh DNS entries later by a random amount of time as to
avoid stampedes of DNS requests. This field sets the upper bound (exclusive) for the random amount.
- area: redis
change: |
Added support for publish.
Expand Down Expand Up @@ -224,6 +231,11 @@ new_features:
change: |
Added new access log command operators ``%START_TIME_LOCAL%`` and ``%EMIT_TIME_LOCAL%``,
similar to ``%START_TIME%`` and ``%EMIT_TIME%``, but use local time zone.
- area: quic
change: |
Added QUIC protocol option :ref:`save_cmsg_config
<envoy_v3_api_field_config.listener.v3.QuicProtocolOptions.save_cmsg_config>` to optionally specify a CMSG header type to be
propagated from the first packet on the connection to QuicListenerFilter.
- area: dns
change: |
Prefer using IPv6 address when addresses from both families are available.
Expand Down
6 changes: 4 additions & 2 deletions contrib/vcl/source/vcl_io_handle.cc
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,8 @@ Api::IoCallUint64Result VclIoHandle::sendmsg(const Buffer::RawSlice* slices, uin
}

Api::IoCallUint64Result VclIoHandle::recvmsg(Buffer::RawSlice* slices, const uint64_t num_slice,
uint32_t self_port, RecvMsgOutput& output) {
uint32_t self_port, const UdpSaveCmsgConfig&,
RecvMsgOutput& output) {
if (!VCL_SH_VALID(sh_)) {
return vclCallResultToIoCallResult(VPPCOM_EBADFD);
}
Expand Down Expand Up @@ -373,7 +374,8 @@ Api::IoCallUint64Result VclIoHandle::recvmsg(Buffer::RawSlice* slices, const uin
return vclCallResultToIoCallResult(result);
}

Api::IoCallUint64Result VclIoHandle::recvmmsg(RawSliceArrays&, uint32_t, RecvMsgOutput&) {
Api::IoCallUint64Result VclIoHandle::recvmmsg(RawSliceArrays&, uint32_t, const UdpSaveCmsgConfig&,
RecvMsgOutput&) {
PANIC("not implemented");
}

Expand Down
4 changes: 3 additions & 1 deletion contrib/vcl/source/vcl_io_handle.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,10 @@ class VclIoHandle : public Envoy::Network::IoHandle,
const Envoy::Network::Address::Ip* self_ip,
const Envoy::Network::Address::Instance& peer_address) override;
Api::IoCallUint64Result recvmsg(Buffer::RawSlice* slices, const uint64_t num_slice,
uint32_t self_port, RecvMsgOutput& output) override;
uint32_t self_port, const UdpSaveCmsgConfig& save_cmsg_config,
RecvMsgOutput& output) override;
Api::IoCallUint64Result recvmmsg(RawSliceArrays& slices, uint32_t self_port,
const UdpSaveCmsgConfig& save_cmsg_config,
RecvMsgOutput& output) override;
absl::optional<std::chrono::milliseconds> lastRoundTripTime() override;
absl::optional<uint64_t> congestionWindowInBytes() const override;
Expand Down
2 changes: 1 addition & 1 deletion docs/root/_static/searchtools.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ const _displayItem = (item, searchTerms) => {
let linkEl = listItem.appendChild(document.createElement("a"));
linkEl.href = linkUrl + anchor;
linkEl.dataset.score = score;
linkEl.innerHTML = title;
linkEl.innerText = title;
// <ENVOY>
const apiVersion = _renderApiVersionLabel(linkUrl);
if (apiVersion !== "") {
Expand Down
1 change: 1 addition & 0 deletions docs/root/api-v3/common_messages/common_messages.rst
Original file line number Diff line number Diff line change
Expand Up @@ -41,3 +41,4 @@ Common messages
../config/core/v3/substitution_format_string.proto
../config/core/v3/udp_socket_config.proto
../extensions/filters/common/set_filter_state/v3/value.proto
../config/core/v3/socket_cmsg_headers.proto
1 change: 1 addition & 0 deletions docs/root/configuration/http/http_conn_man/stats.rst
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ statistics:
``downstream_rq_timeout``, Counter, Total requests closed due to a timeout on the request path
``downstream_rq_overload_close``, Counter, Total requests closed due to Envoy overload
``downstream_rq_redirected_with_normalized_path``, Counter, Total requests redirected due to different original and normalized URL paths. This action is configured by setting the :ref:`path_with_escaped_slashes_action <envoy_v3_api_field_extensions.filters.network.http_connection_manager.v3.HttpConnectionManager.path_with_escaped_slashes_action>` config option.
``downstream_rq_too_many_premature_resets``, Counter, Total number of connections closed due to too many premature request resets on the connection.
``rs_too_large``, Counter, Total response errors due to buffering an overly large body

.. _config_http_conn_man_stats_per_ua:
Expand Down
5 changes: 4 additions & 1 deletion envoy/config/grpc_mux.h
Original file line number Diff line number Diff line change
Expand Up @@ -134,8 +134,11 @@ template <class ResponseProto> class GrpcStreamCallbacks {
/**
* For the GrpcStream to prompt the context to take appropriate action in response to
* failure to establish the gRPC stream.
* @param next_attempt_may_send_initial_resource_version a flag indicating whether the
* next reconnection attempt will be to the same source that was previously successful
* or not (used to pass primary/failover reconnection information to the GrpcMux).
*/
virtual void onEstablishmentFailure() PURE;
virtual void onEstablishmentFailure(bool next_attempt_may_send_initial_resource_version) PURE;

/**
* For the GrpcStream to pass received protos to the context.
Expand Down
1 change: 1 addition & 0 deletions envoy/network/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ envoy_cc_library(
":address_interface",
"//envoy/api:io_error_interface",
"//envoy/api:os_sys_calls_interface",
"//envoy/buffer:buffer_interface",
"//envoy/event:file_event_interface",
"//source/common/common:assert_lib",
],
Expand Down
Loading

0 comments on commit d965ff6

Please sign in to comment.