From 9a474a30a1b9ecbfe1e9d1a5190ee8aef2b29041 Mon Sep 17 00:00:00 2001 From: Eitan Yarmush Date: Thu, 1 Aug 2024 01:18:16 -0400 Subject: [PATCH 1/9] Add the ability to set the hits_addend for a given rate_limit request via a hardcoded dynamic metadata field (#34184) Commit Message: Adds the ability to set the hits_addend for a given rate_limit request via a hardcoded dynamic metadata field: envoy.ratelimit:hits_addend. Additional Description: Risk Level: Low Testing: Added unit test. I have also manually tested this using gloo-edge as the control-plane. Docs Changes: Release Notes: Platform Specific Features: N/A [Optional Runtime guard:] N/A [Optional Fixes #Issue] N/A [Optional Fixes commit #PR or SHA] N/A [Optional Deprecated:] N/A [Optional [API Considerations](https://github.com/envoyproxy/envoy/blob/main/api/review_checklist.md):] N/A --------- Signed-off-by: Eitan Yarmush Signed-off-by: code Co-authored-by: code --- api/envoy/service/ratelimit/v3/rls.proto | 2 + changelogs/current.yaml | 4 ++ .../advanced/well_known_filter_state.rst | 5 ++ .../extensions/filters/http/ratelimit/BUILD | 2 + .../filters/http/ratelimit/ratelimit.cc | 32 +++++++++- .../filters/http/ratelimit/ratelimit_test.cc | 61 +++++++++++++++++++ 6 files changed, 105 insertions(+), 1 deletion(-) diff --git a/api/envoy/service/ratelimit/v3/rls.proto b/api/envoy/service/ratelimit/v3/rls.proto index 7375aceb5c2b..d69a323d88b7 100644 --- a/api/envoy/service/ratelimit/v3/rls.proto +++ b/api/envoy/service/ratelimit/v3/rls.proto @@ -49,6 +49,8 @@ message RateLimitRequest { // Rate limit requests can optionally specify the number of hits a request adds to the matched // limit. If the value is not set in the message, a request increases the matched limit by 1. + // This value can be overridden by setting filter state value ``envoy.ratelimit.hits_addend`` + // to the desired number. Invalid number (< 0) or number will be ignored. uint32 hits_addend = 3; } diff --git a/changelogs/current.yaml b/changelogs/current.yaml index ffe96071d8fc..cf1201c0c0a9 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -115,6 +115,10 @@ new_features: ` to allow overriding TLS certificate selection behavior. An extension can select certificate base on the incoming SNI, in both sync and async mode. +- area: ratelimit + change: | + Added the ability to modify :ref:`hits_addend ` + by setting by setting filter state value ``envoy.ratelimit.hits_addend`` to the desired value. - area: access_log change: | Added new access log command operators ``%START_TIME_LOCAL%`` and ``%EMIT_TIME_LOCAL%``, diff --git a/docs/root/configuration/advanced/well_known_filter_state.rst b/docs/root/configuration/advanced/well_known_filter_state.rst index f238dd207854..edcfc2e321ff 100644 --- a/docs/root/configuration/advanced/well_known_filter_state.rst +++ b/docs/root/configuration/advanced/well_known_filter_state.rst @@ -68,6 +68,11 @@ The following lists the filter state object keys used by the Envoy extensions: ` override on a per-connection basis. Accepts a count of milliseconds number string as a constructor. +``envoy.ratelimit.hits_addend`` + :ref:`Rate Limit Hits Addend + ` override on a per-route basis. + Accepts a number string as a constructor. + Filter state object fields -------------------------- diff --git a/source/extensions/filters/http/ratelimit/BUILD b/source/extensions/filters/http/ratelimit/BUILD index fd4c15c81ace..6d56028db50d 100644 --- a/source/extensions/filters/http/ratelimit/BUILD +++ b/source/extensions/filters/http/ratelimit/BUILD @@ -20,11 +20,13 @@ envoy_cc_library( ":ratelimit_headers_lib", "//envoy/http:codes_interface", "//envoy/ratelimit:ratelimit_interface", + "//envoy/stream_info:uint32_accessor_interface", "//source/common/common:assert_lib", "//source/common/common:empty_string", "//source/common/common:enum_to_int", "//source/common/http:codes_lib", "//source/common/router:config_lib", + "//source/common/stream_info:uint32_accessor_lib", "//source/extensions/filters/common/ratelimit:ratelimit_client_interface", "//source/extensions/filters/common/ratelimit:stat_names_lib", "@envoy_api//envoy/extensions/filters/http/ratelimit/v3:pkg_cc_proto", diff --git a/source/extensions/filters/http/ratelimit/ratelimit.cc b/source/extensions/filters/http/ratelimit/ratelimit.cc index 382029e5f3db..7052f8f793ed 100644 --- a/source/extensions/filters/http/ratelimit/ratelimit.cc +++ b/source/extensions/filters/http/ratelimit/ratelimit.cc @@ -4,6 +4,7 @@ #include #include "envoy/http/codes.h" +#include "envoy/stream_info/stream_info.h" #include "source/common/common/assert.h" #include "source/common/common/enum_to_int.h" @@ -11,6 +12,7 @@ #include "source/common/http/codes.h" #include "source/common/http/header_utility.h" #include "source/common/router/config_impl.h" +#include "source/common/stream_info/uint32_accessor_impl.h" #include "source/extensions/filters/http/ratelimit/ratelimit_headers.h" namespace Envoy { @@ -18,6 +20,26 @@ namespace Extensions { namespace HttpFilters { namespace RateLimitFilter { +namespace { +constexpr absl::string_view HitsAddendFilterStateKey = "envoy.ratelimit.hits_addend"; + +class HitsAddendObjectFactory : public StreamInfo::FilterState::ObjectFactory { +public: + std::string name() const override { return std::string(HitsAddendFilterStateKey); } + std::unique_ptr + createFromBytes(absl::string_view data) const override { + uint32_t hits_addend = 0; + if (absl::SimpleAtoi(data, &hits_addend)) { + return std::make_unique(hits_addend); + } + return nullptr; + } +}; + +REGISTER_FACTORY(HitsAddendObjectFactory, StreamInfo::FilterState::ObjectFactory); + +} // namespace + struct RcDetailsValues { // This request went above the configured limits for the rate limit filter. const std::string RateLimited = "request_rate_limited"; @@ -64,11 +86,19 @@ void Filter::initiateCall(const Http::RequestHeaderMap& headers) { break; } + const StreamInfo::UInt32Accessor* hits_addend_filter_state = + callbacks_->streamInfo().filterState()->getDataReadOnly( + HitsAddendFilterStateKey); + double hits_addend = 0; + if (hits_addend_filter_state != nullptr) { + hits_addend = hits_addend_filter_state->value(); + } + if (!descriptors.empty()) { state_ = State::Calling; initiating_call_ = true; client_->limit(*this, getDomain(), descriptors, callbacks_->activeSpan(), - callbacks_->streamInfo(), 0); + callbacks_->streamInfo(), hits_addend); initiating_call_ = false; } } diff --git a/test/extensions/filters/http/ratelimit/ratelimit_test.cc b/test/extensions/filters/http/ratelimit/ratelimit_test.cc index 8207ba087fab..6caa81e8eb5f 100644 --- a/test/extensions/filters/http/ratelimit/ratelimit_test.cc +++ b/test/extensions/filters/http/ratelimit/ratelimit_test.cc @@ -3,11 +3,13 @@ #include #include "envoy/extensions/filters/http/ratelimit/v3/rate_limit.pb.h" +#include "envoy/stream_info/stream_info.h" #include "source/common/buffer/buffer_impl.h" #include "source/common/common/empty_string.h" #include "source/common/http/context_impl.h" #include "source/common/http/headers.h" +#include "source/common/stream_info/uint32_accessor_impl.h" #include "source/extensions/filters/http/ratelimit/ratelimit.h" #include "test/extensions/filters/common/ratelimit/mocks.h" @@ -257,6 +259,53 @@ TEST_F(HttpRateLimitFilterTest, OkResponse) { 1U, filter_callbacks_.clusterInfo()->statsScope().counterFromStatName(ratelimit_ok_).value()); } +TEST_F(HttpRateLimitFilterTest, OkResponseWithAdditionalHitsAddend) { + setUpTest(filter_config_); + InSequence s; + + filter_callbacks_.stream_info_.filter_state_->setData( + "envoy.ratelimit.hits_addend", std::make_unique(5), + StreamInfo::FilterState::StateType::ReadOnly); + EXPECT_CALL(filter_callbacks_.route_->route_entry_.rate_limit_policy_, getApplicableRateLimit(0)); + + EXPECT_CALL(route_rate_limit_, populateDescriptors(_, _, _, _)) + .WillOnce(SetArgReferee<0>(descriptor_)); + + EXPECT_CALL(filter_callbacks_.route_->virtual_host_.rate_limit_policy_, + getApplicableRateLimit(0)); + + EXPECT_CALL(*client_, limit(_, "foo", + testing::ContainerEq(std::vector{ + {{{"descriptor_key", "descriptor_value"}}}}), + _, _, 5)) + .WillOnce( + WithArgs<0>(Invoke([&](Filters::Common::RateLimit::RequestCallbacks& callbacks) -> void { + request_callbacks_ = &callbacks; + }))); + + request_headers_.addCopy(Http::Headers::get().RequestId, "requestid"); + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + filter_->decodeHeaders(request_headers_, false)); + Http::MetadataMap metadata_map{{"metadata", "metadata"}}; + EXPECT_EQ(Http::FilterMetadataStatus::Continue, filter_->decodeMetadata(metadata_map)); + EXPECT_EQ(Http::FilterDataStatus::StopIterationAndWatermark, filter_->decodeData(data_, false)); + EXPECT_EQ(Http::FilterTrailersStatus::StopIteration, filter_->decodeTrailers(request_trailers_)); + EXPECT_EQ(Http::Filter1xxHeadersStatus::Continue, filter_->encode1xxHeaders(response_headers_)); + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->encodeHeaders(response_headers_, false)); + EXPECT_EQ(Http::FilterDataStatus::Continue, filter_->encodeData(response_data_, false)); + EXPECT_EQ(Http::FilterTrailersStatus::Continue, filter_->encodeTrailers(response_trailers_)); + + EXPECT_CALL(filter_callbacks_, continueDecoding()); + EXPECT_CALL(filter_callbacks_.stream_info_, + setResponseFlag(StreamInfo::CoreResponseFlag::RateLimited)) + .Times(0); + request_callbacks_->complete(Filters::Common::RateLimit::LimitStatus::OK, nullptr, nullptr, + nullptr, "", nullptr); + + EXPECT_EQ( + 1U, filter_callbacks_.clusterInfo()->statsScope().counterFromStatName(ratelimit_ok_).value()); +} + TEST_F(HttpRateLimitFilterTest, OkResponseWithHeaders) { setUpTest(filter_config_); InSequence s; @@ -1667,6 +1716,18 @@ TEST_F(HttpRateLimitFilterTest, StatsWithPrefix) { EXPECT_EQ("request_rate_limited", filter_callbacks_.details()); } +TEST(ObjectFactory, HitsAddend) { + const std::string name = "envoy.ratelimit.hits_addend"; + auto* factory = + Registry::FactoryRegistry::getFactory(name); + ASSERT_NE(nullptr, factory); + EXPECT_EQ(name, factory->name()); + const std::string hits_addend = std::to_string(1234); + auto object = factory->createFromBytes(hits_addend); + ASSERT_NE(nullptr, object); + EXPECT_EQ(hits_addend, object->serializeAsString()); +} + } // namespace } // namespace RateLimitFilter } // namespace HttpFilters From 6e55a3716fe9faa21d25d184f3d1c522de9cb696 Mon Sep 17 00:00:00 2001 From: "dependency-envoy[bot]" <148525496+dependency-envoy[bot]@users.noreply.github.com> Date: Thu, 1 Aug 2024 10:08:05 +0100 Subject: [PATCH 2/9] deps/api: Bump `rules_proto` -> 6.0.2 (#35156) Created by Envoy dependency bot for @phlax Fix #34792 Signed-off-by: dependency-envoy[bot] <148525496+dependency-envoy[bot]@users.noreply.github.com> --------- Signed-off-by: Ryan Northey Co-authored-by: dependency-envoy[bot] <148525496+dependency-envoy[bot]@users.noreply.github.com> Co-authored-by: Ryan Northey From d469f34faa7c4ae046f8e61d1ec16f345d7d4c57 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 1 Aug 2024 11:39:07 +0100 Subject: [PATCH 3/9] build(deps): bump setuptools from 72.0.0 to 72.1.0 in /tools/base (#35492) Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- tools/base/requirements.txt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/base/requirements.txt b/tools/base/requirements.txt index effde79fa116..5608dac5fc67 100644 --- a/tools/base/requirements.txt +++ b/tools/base/requirements.txt @@ -1671,7 +1671,7 @@ zstandard==0.23.0 \ # via envoy-base-utils # The following packages are considered to be unsafe in a requirements file: -setuptools==72.0.0 \ - --hash=sha256:5a0d9c6a2f332881a0153f629d8000118efd33255cfa802757924c53312c76da \ - --hash=sha256:98b4d786a12fadd34eabf69e8d014b84e5fc655981e4ff419994700434ace132 +setuptools==72.1.0 \ + --hash=sha256:5a03e1860cf56bb6ef48ce186b0e557fdba433237481a9a625176c2831be15d1 \ + --hash=sha256:8d243eff56d095e5817f796ede6ae32941278f542e0f941867cc05ae52b162ec # via -r requirements.in From a833cbfae33675b02b860bbb457c7aa89374f944 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 1 Aug 2024 10:39:18 +0000 Subject: [PATCH 4/9] build(deps): bump protobuf from 5.27.2 to 5.27.3 in /tools/base (#35537) Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- tools/base/requirements.txt | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/tools/base/requirements.txt b/tools/base/requirements.txt index 5608dac5fc67..c90fee557dff 100644 --- a/tools/base/requirements.txt +++ b/tools/base/requirements.txt @@ -1019,18 +1019,18 @@ ply==3.11 \ --hash=sha256:00c7c1aaa88358b9c765b6d3000c6eec0ba42abca5351b095321aef446081da3 \ --hash=sha256:096f9b8350b65ebd2fd1346b12452efe5b9607f7482813ffca50c22722a807ce # via -r requirements.in -protobuf==5.27.2 \ - --hash=sha256:0e341109c609749d501986b835f667c6e1e24531096cff9d34ae411595e26505 \ - --hash=sha256:176c12b1f1c880bf7a76d9f7c75822b6a2bc3db2d28baa4d300e8ce4cde7409b \ - --hash=sha256:354d84fac2b0d76062e9b3221f4abbbacdfd2a4d8af36bab0474f3a0bb30ab38 \ - --hash=sha256:4fadd8d83e1992eed0248bc50a4a6361dc31bcccc84388c54c86e530b7f58863 \ - --hash=sha256:54330f07e4949d09614707c48b06d1a22f8ffb5763c159efd5c0928326a91470 \ - --hash=sha256:610e700f02469c4a997e58e328cac6f305f649826853813177e6290416e846c6 \ - --hash=sha256:7fc3add9e6003e026da5fc9e59b131b8f22b428b991ccd53e2af8071687b4fce \ - --hash=sha256:9e8f199bf7f97bd7ecebffcae45ebf9527603549b2b562df0fbc6d4d688f14ca \ - --hash=sha256:a109916aaac42bff84702fb5187f3edadbc7c97fc2c99c5ff81dd15dcce0d1e5 \ - --hash=sha256:b848dbe1d57ed7c191dfc4ea64b8b004a3f9ece4bf4d0d80a367b76df20bf36e \ - --hash=sha256:f3ecdef226b9af856075f28227ff2c90ce3a594d092c39bee5513573f25e2714 +protobuf==5.27.3 \ + --hash=sha256:043853dcb55cc262bf2e116215ad43fa0859caab79bb0b2d31b708f128ece035 \ + --hash=sha256:16ddf3f8c6c41e1e803da7abea17b1793a97ef079a912e42351eabb19b2cffe7 \ + --hash=sha256:68248c60d53f6168f565a8c76dc58ba4fa2ade31c2d1ebdae6d80f969cdc2d4f \ + --hash=sha256:82460903e640f2b7e34ee81a947fdaad89de796d324bcbc38ff5430bcdead82c \ + --hash=sha256:8572c6533e544ebf6899c360e91d6bcbbee2549251643d32c52cf8a5de295ba5 \ + --hash=sha256:a55c48f2a2092d8e213bd143474df33a6ae751b781dd1d1f4d953c128a415b25 \ + --hash=sha256:af7c0b7cfbbb649ad26132e53faa348580f844d9ca46fd3ec7ca48a1ea5db8a1 \ + --hash=sha256:b8a994fb3d1c11156e7d1e427186662b64694a62b55936b2b9348f0a7c6625ce \ + --hash=sha256:c2a105c24f08b1e53d6c7ffe69cb09d0031512f0b72f812dd4005b8112dbe91e \ + --hash=sha256:c84eee2c71ed83704f1afbf1a85c3171eab0fd1ade3b399b3fad0884cbcca8bf \ + --hash=sha256:dcb307cd4ef8fec0cf52cb9105a03d06fbb5275ce6d84a6ae33bc6cf84e0a07b # via # -r requirements.in # envoy-base-utils From fd66419bf21500816066c683db8532bab170ebf6 Mon Sep 17 00:00:00 2001 From: alyssawilk Date: Thu, 1 Aug 2024 09:16:56 -0400 Subject: [PATCH 5/9] exceptions: cleaning up macros (#35482) Signed-off-by: Alyssa Wilk --- envoy/common/exception.h | 11 +++-- .../api_listener_manager.cc | 2 +- source/common/config/datasource.cc | 4 +- .../common/filesystem/inotify/watcher_impl.cc | 2 +- .../common/filesystem/kqueue/watcher_impl.cc | 10 ++--- .../common/filesystem/win32/watcher_impl.cc | 2 +- source/common/filter/config_discovery_impl.h | 8 ++-- .../common/grpc/async_client_manager_impl.cc | 4 +- .../listener_manager/listener_manager_impl.cc | 2 +- source/common/network/resolver_impl.cc | 2 +- source/common/protobuf/visitor.cc | 4 +- source/common/router/config_impl.cc | 4 +- source/common/router/header_parser.cc | 6 +-- source/common/router/rds_impl.cc | 2 +- source/common/runtime/runtime_impl.cc | 10 ++--- source/common/secret/sds_api.cc | 2 +- source/common/stats/tag_producer_impl.cc | 2 +- source/common/tls/ocsp/ocsp.cc | 42 +++++++++---------- .../common/upstream/cluster_factory_impl.cc | 6 +-- .../common/upstream/cluster_manager_impl.cc | 16 +++---- .../upstream/health_discovery_service.cc | 2 +- source/common/upstream/upstream_impl.cc | 14 +++---- .../clusters/dynamic_forward_proxy/cluster.cc | 2 +- .../dns_cache_manager_impl.cc | 2 +- .../filters/http/match_delegate/config.cc | 2 +- .../network/dns_resolver/cares/dns_impl.cc | 2 +- .../transport_sockets/http_11_proxy/config.cc | 2 +- .../internal_upstream/config.cc | 2 +- .../proxy_protocol/config.cc | 2 +- .../transport_sockets/starttls/config.cc | 8 ++-- .../transport_sockets/tap/config.cc | 4 +- .../transport_sockets/tcp_stats/config.cc | 4 +- source/extensions/upstreams/http/config.cc | 8 ++-- source/server/api_listener_impl.cc | 2 +- source/server/configuration_impl.cc | 2 +- source/server/server.cc | 2 +- 36 files changed, 100 insertions(+), 101 deletions(-) diff --git a/envoy/common/exception.h b/envoy/common/exception.h index 327fa906c0fb..0688b440dfbc 100644 --- a/envoy/common/exception.h +++ b/envoy/common/exception.h @@ -57,15 +57,14 @@ class EnvoyException : public std::runtime_error { // the macros above. #define THROW_IF_STATUS_NOT_OK(variable, throw_action) THROW_IF_NOT_OK_REF(variable.status()); -// TODO(alyssawilk) remove in favor of RETURN_IF_NOT_OK -#define RETURN_IF_STATUS_NOT_OK(variable) \ - if (!variable.status().ok()) { \ - return variable.status(); \ +#define RETURN_IF_NOT_OK_REF(variable) \ + if (const absl::Status& temp_status = variable; !temp_status.ok()) { \ + return temp_status; \ } // Make sure this works for functions without calling the functoin twice as well. -#define RETURN_IF_NOT_OK(variable) \ - if (absl::Status temp_status = variable; !temp_status.ok()) { \ +#define RETURN_IF_NOT_OK(status_fn) \ + if (absl::Status temp_status = (status_fn); !temp_status.ok()) { \ return temp_status; \ } diff --git a/mobile/library/common/extensions/listener_managers/api_listener_manager/api_listener_manager.cc b/mobile/library/common/extensions/listener_managers/api_listener_manager/api_listener_manager.cc index 8a4f04320cbe..c89d321179b8 100644 --- a/mobile/library/common/extensions/listener_managers/api_listener_manager/api_listener_manager.cc +++ b/mobile/library/common/extensions/listener_managers/api_listener_manager/api_listener_manager.cc @@ -37,7 +37,7 @@ ApiListenerManagerImpl::addOrUpdateListener(const envoy::config::listener::v3::L } if (!api_listener_ && !added_via_api) { auto listener_or_error = HttpApiListener::create(config, server_, config.name()); - RETURN_IF_STATUS_NOT_OK(listener_or_error); + RETURN_IF_NOT_OK(listener_or_error.status()); api_listener_ = std::move(listener_or_error.value()); return true; } else { diff --git a/source/common/config/datasource.cc b/source/common/config/datasource.cc index 14457b3bff28..c086a2cd9339 100644 --- a/source/common/config/datasource.cc +++ b/source/common/config/datasource.cc @@ -40,7 +40,7 @@ absl::StatusOr readFile(const std::string& path, Api::Api& api, boo } auto file_content_or_error = file_system.fileReadToEnd(path); - RETURN_IF_STATUS_NOT_OK(file_content_or_error); + RETURN_IF_NOT_OK_REF(file_content_or_error.status()); if (!allow_empty && file_content_or_error.value().empty()) { return absl::InvalidArgumentError(fmt::format("file {} is empty", path)); @@ -118,7 +118,7 @@ absl::StatusOr DataSourceProvider::create(const ProtoData Api::Api& api, bool allow_empty, uint64_t max_size) { auto initial_data_or_error = read(source, allow_empty, api, max_size); - RETURN_IF_STATUS_NOT_OK(initial_data_or_error); + RETURN_IF_NOT_OK_REF(initial_data_or_error.status()); // read() only validates the size of the file and does not check the size of inline data. // We check the size of inline data here. diff --git a/source/common/filesystem/inotify/watcher_impl.cc b/source/common/filesystem/inotify/watcher_impl.cc index 549798e00bfc..534cd67f1933 100644 --- a/source/common/filesystem/inotify/watcher_impl.cc +++ b/source/common/filesystem/inotify/watcher_impl.cc @@ -36,7 +36,7 @@ absl::Status WatcherImpl::addWatch(absl::string_view path, uint32_t events, OnCh // Because of general inotify pain, we always watch the directory that the file lives in, // and then synthetically raise per file events. auto result_or_error = file_system_.splitPathFromFilename(path); - RETURN_IF_STATUS_NOT_OK(result_or_error); + RETURN_IF_NOT_OK_REF(result_or_error.status()); const PathSplitResult result = result_or_error.value(); const uint32_t watch_mask = IN_MODIFY | IN_MOVED_TO; diff --git a/source/common/filesystem/kqueue/watcher_impl.cc b/source/common/filesystem/kqueue/watcher_impl.cc index 507cfcd8aab4..5827c19590ae 100644 --- a/source/common/filesystem/kqueue/watcher_impl.cc +++ b/source/common/filesystem/kqueue/watcher_impl.cc @@ -36,7 +36,7 @@ WatcherImpl::~WatcherImpl() { absl::Status WatcherImpl::addWatch(absl::string_view path, uint32_t events, Watcher::OnChangedCb cb) { absl::StatusOr watch_or_error = addWatch(path, events, cb, false); - RETURN_IF_STATUS_NOT_OK(watch_or_error); + RETURN_IF_NOT_OK_REF(watch_or_error.status()); if (watch_or_error.value() == nullptr) { return absl::InvalidArgumentError(absl::StrCat("invalid watch path ", path)); } @@ -56,7 +56,7 @@ absl::StatusOr WatcherImpl::addWatch(absl::string_vie } const auto result_or_error = file_system_.splitPathFromFilename(path); - RETURN_IF_STATUS_NOT_OK(result_or_error); + RETURN_IF_NOT_OK_REF(result_or_error.status()); watch_fd = open(std::string(result_or_error.value().directory_).c_str(), 0); if (watch_fd == -1) { return nullptr; @@ -116,7 +116,7 @@ absl::Status WatcherImpl::onKqueueEvent() { absl::StatusOr pathname_or_error = file_system_.splitPathFromFilename(file->file_); - RETURN_IF_STATUS_NOT_OK(pathname_or_error); + RETURN_IF_NOT_OK_REF(pathname_or_error.status()); PathSplitResult& pathname = pathname_or_error.value(); if (file->watching_dir_) { @@ -129,7 +129,7 @@ absl::Status WatcherImpl::onKqueueEvent() { if (event.fflags & NOTE_WRITE) { // directory was written -- check if the file we're actually watching appeared auto file_or_error = addWatch(file->file_, file->events_, file->callback_, true); - RETURN_IF_STATUS_NOT_OK(file_or_error); + RETURN_IF_NOT_OK_REF(file_or_error.status()); FileWatchPtr new_file = file_or_error.value(); if (new_file != nullptr) { removeWatch(file); @@ -150,7 +150,7 @@ absl::Status WatcherImpl::onKqueueEvent() { removeWatch(file); auto file_or_error = addWatch(file->file_, file->events_, file->callback_, true); - RETURN_IF_STATUS_NOT_OK(file_or_error); + RETURN_IF_NOT_OK_REF(file_or_error.status()); FileWatchPtr new_file = file_or_error.value(); if (new_file == nullptr) { return absl::OkStatus(); diff --git a/source/common/filesystem/win32/watcher_impl.cc b/source/common/filesystem/win32/watcher_impl.cc index 6cb9d00a1bc7..58905d996d22 100644 --- a/source/common/filesystem/win32/watcher_impl.cc +++ b/source/common/filesystem/win32/watcher_impl.cc @@ -56,7 +56,7 @@ absl::Status WatcherImpl::addWatch(absl::string_view path, uint32_t events, OnCh } const absl::StatusOr result_or_error = file_system_.splitPathFromFilename(path); - RETURN_IF_STATUS_NOT_OK(result_or_error); + RETURN_IF_NOT_OK_REF(result_or_error.status()); const PathSplitResult& result = result_or_error.value(); // ReadDirectoryChangesW only has a Unicode version, so we need // to use wide strings here diff --git a/source/common/filter/config_discovery_impl.h b/source/common/filter/config_discovery_impl.h index 1aaec8a0ab3d..643e19eaffe6 100644 --- a/source/common/filter/config_discovery_impl.h +++ b/source/common/filter/config_discovery_impl.h @@ -111,7 +111,7 @@ class DynamicFilterConfigProviderImpl : public DynamicFilterConfigProviderImplBa absl::Status onConfigUpdate(const Protobuf::Message& message, const std::string&, Config::ConfigAppliedCb applied_on_all_threads) override { const absl::StatusOr config_or_error = instantiateFilterFactory(message); - RETURN_IF_STATUS_NOT_OK(config_or_error); + RETURN_IF_NOT_OK_REF(config_or_error.status()); update(config_or_error.value(), applied_on_all_threads); return absl::OkStatus(); } @@ -120,7 +120,7 @@ class DynamicFilterConfigProviderImpl : public DynamicFilterConfigProviderImplBa absl::optional cb; if (default_configuration_) { auto cb_or_error = instantiateFilterFactory(*default_configuration_); - RETURN_IF_STATUS_NOT_OK(cb_or_error); + RETURN_IF_NOT_OK_REF(cb_or_error.status()); cb = cb_or_error.value(); } update(cb, applied_on_all_threads); @@ -225,7 +225,7 @@ class HttpDynamicFilterConfigProviderImpl message.GetTypeName()); absl::StatusOr error_or_factory = factory->createFilterFactoryFromProto(message, getStatPrefix(), factory_context_); - RETURN_IF_STATUS_NOT_OK(error_or_factory); + RETURN_IF_NOT_OK_REF(error_or_factory.status()); return NamedHttpFilterFactoryCb{factory->name(), error_or_factory.value()}; } @@ -257,7 +257,7 @@ class NetworkDynamicFilterConfigProviderImplBase message.GetTypeName()); absl::StatusOr cb_or_error = factory->createFilterFactoryFromProto(message, factory_context_); - RETURN_IF_STATUS_NOT_OK(cb_or_error); + RETURN_IF_NOT_OK_REF(cb_or_error.status()); return cb_or_error.value(); } diff --git a/source/common/grpc/async_client_manager_impl.cc b/source/common/grpc/async_client_manager_impl.cc index 9d3bf65d132c..aabd46907636 100644 --- a/source/common/grpc/async_client_manager_impl.cc +++ b/source/common/grpc/async_client_manager_impl.cc @@ -167,7 +167,7 @@ absl::StatusOr AsyncClientManagerImpl::getOrCreateRawAs } auto factory_or_error = factoryForGrpcService(config_with_hash_key.config(), scope, skip_cluster_check); - RETURN_IF_STATUS_NOT_OK(factory_or_error); + RETURN_IF_NOT_OK_REF(factory_or_error.status()); client = factory_or_error.value()->createUncachedRawAsyncClient(); raw_async_client_cache_->setCache(config_with_hash_key, client); return client; @@ -183,7 +183,7 @@ AsyncClientManagerImpl::getOrCreateRawAsyncClientWithHashKey( } auto factory_or_error = factoryForGrpcService(config_with_hash_key.config(), scope, skip_cluster_check); - RETURN_IF_STATUS_NOT_OK(factory_or_error); + RETURN_IF_NOT_OK_REF(factory_or_error.status()); client = factory_or_error.value()->createUncachedRawAsyncClient(); raw_async_client_cache_->setCache(config_with_hash_key, client); return client; diff --git a/source/common/listener_manager/listener_manager_impl.cc b/source/common/listener_manager/listener_manager_impl.cc index f0841e119d3e..05ad90dcade9 100644 --- a/source/common/listener_manager/listener_manager_impl.cc +++ b/source/common/listener_manager/listener_manager_impl.cc @@ -474,7 +474,7 @@ ListenerManagerImpl::addOrUpdateListener(const envoy::config::listener::v3::List } if (!api_listener_ && !added_via_api) { auto listener_or_error = HttpApiListener::create(config, server_, config.name()); - RETURN_IF_STATUS_NOT_OK(listener_or_error); + RETURN_IF_NOT_OK_REF(listener_or_error.status()); api_listener_ = std::move(listener_or_error.value()); return true; } else { diff --git a/source/common/network/resolver_impl.cc b/source/common/network/resolver_impl.cc index c336580b6697..7f7b5ece9686 100644 --- a/source/common/network/resolver_impl.cc +++ b/source/common/network/resolver_impl.cc @@ -87,7 +87,7 @@ resolveProtoSocketAddress(const envoy::config::core::v3::SocketAddress& socket_a return absl::InvalidArgumentError(fmt::format("Unknown address resolver: {}", resolver_name)); } auto instance_or_error = resolver->resolve(socket_address); - RETURN_IF_STATUS_NOT_OK(instance_or_error); + RETURN_IF_NOT_OK_REF(instance_or_error.status()); return std::move(instance_or_error.value()); } diff --git a/source/common/protobuf/visitor.cc b/source/common/protobuf/visitor.cc index 6f16e6060d9b..f03a84e7f536 100644 --- a/source/common/protobuf/visitor.cc +++ b/source/common/protobuf/visitor.cc @@ -31,11 +31,11 @@ absl::Status traverseMessageWorker(ConstProtoVisitor& visitor, const Protobuf::M RETURN_IF_NOT_OK(MessageUtil::unpackTo(*any_message, *inner_message)); } else if (message.GetTypeName() == "xds.type.v3.TypedStruct") { auto output_or_error = Helper::convertTypedStruct(message); - RETURN_IF_STATUS_NOT_OK(output_or_error); + RETURN_IF_NOT_OK_REF(output_or_error.status()); std::tie(inner_message, target_type_url) = std::move(output_or_error.value()); } else if (message.GetTypeName() == "udpa.type.v1.TypedStruct") { auto output_or_error = Helper::convertTypedStruct(message); - RETURN_IF_STATUS_NOT_OK(output_or_error); + RETURN_IF_NOT_OK_REF(output_or_error.status()); std::tie(inner_message, target_type_url) = std::move(output_or_error.value()); } diff --git a/source/common/router/config_impl.cc b/source/common/router/config_impl.cc index 51bb67a87147..a031d98faa38 100644 --- a/source/common/router/config_impl.cc +++ b/source/common/router/config_impl.cc @@ -1253,7 +1253,7 @@ RouteEntryImplBase::buildPathRewriter(envoy::config::route::v3::Route route, route.route().path_rewrite_policy().typed_config(), validator, factory); absl::StatusOr rewriter = factory.createPathRewriter(*config); - RETURN_IF_STATUS_NOT_OK(rewriter); + RETURN_IF_NOT_OK_REF(rewriter.status()); return rewriter.value(); } @@ -1271,7 +1271,7 @@ RouteEntryImplBase::buildPathMatcher(envoy::config::route::v3::Route route, route.match().path_match_policy().typed_config(), validator, factory); absl::StatusOr matcher = factory.createPathMatcher(*config); - RETURN_IF_STATUS_NOT_OK(matcher); + RETURN_IF_NOT_OK_REF(matcher.status()); return matcher.value(); } diff --git a/source/common/router/header_parser.cc b/source/common/router/header_parser.cc index cd8e9b5ee893..4d33c0ce4580 100644 --- a/source/common/router/header_parser.cc +++ b/source/common/router/header_parser.cc @@ -89,7 +89,7 @@ HeaderParser::configure(const Protobuf::RepeatedPtrField& hea HeaderParserPtr header_parser(new HeaderParser()); for (const auto& header_value_option : headers_to_add) { auto entry_or_error = HeadersToAddEntry::create(header_value_option); - RETURN_IF_STATUS_NOT_OK(entry_or_error); + RETURN_IF_NOT_OK_REF(entry_or_error.status()); header_parser->headers_to_add_.emplace_back( Http::LowerCaseString(header_value_option.header().key()), std::move(entry_or_error.value())); @@ -105,7 +105,7 @@ absl::StatusOr HeaderParser::configure( for (const auto& header_value : headers_to_add) { auto entry_or_error = HeadersToAddEntry::create(header_value, append_action); - RETURN_IF_STATUS_NOT_OK(entry_or_error); + RETURN_IF_NOT_OK_REF(entry_or_error.status()); header_parser->headers_to_add_.emplace_back(Http::LowerCaseString(header_value.key()), std::move(entry_or_error.value())); } @@ -117,7 +117,7 @@ absl::StatusOr HeaderParser::configure(const Protobuf::RepeatedPtrField& headers_to_add, const Protobuf::RepeatedPtrField& headers_to_remove) { auto parser_or_error = configure(headers_to_add); - RETURN_IF_STATUS_NOT_OK(parser_or_error); + RETURN_IF_NOT_OK_REF(parser_or_error.status()); HeaderParserPtr header_parser = std::move(parser_or_error.value()); for (const auto& header : headers_to_remove) { diff --git a/source/common/router/rds_impl.cc b/source/common/router/rds_impl.cc index 78409903f215..43cb301978ea 100644 --- a/source/common/router/rds_impl.cc +++ b/source/common/router/rds_impl.cc @@ -50,7 +50,7 @@ absl::Status RdsRouteConfigSubscription::beforeProviderUpdate( resume_rds); auto subscription_or_error = VhdsSubscription::createVhdsSubscription( config_update_info_, factory_context_, stat_prefix_, route_config_provider_); - RETURN_IF_STATUS_NOT_OK(subscription_or_error); + RETURN_IF_NOT_OK_REF(subscription_or_error.status()); vhds_subscription_ = std::move(subscription_or_error.value()); vhds_subscription_->registerInitTargetWithInitManager( noop_init_manager == nullptr ? local_init_manager_ : *noop_init_manager); diff --git a/source/common/runtime/runtime_impl.cc b/source/common/runtime/runtime_impl.cc index e2a0619393e4..917ef6e655a1 100644 --- a/source/common/runtime/runtime_impl.cc +++ b/source/common/runtime/runtime_impl.cc @@ -439,9 +439,9 @@ absl::Status DiskLayer::walkDirectory(const std::string& path, const std::string Filesystem::Directory directory(path); Filesystem::DirectoryIteratorImpl it = directory.begin(); - RETURN_IF_STATUS_NOT_OK(it); + RETURN_IF_NOT_OK_REF(it.status()); for (; it != directory.end(); ++it) { - RETURN_IF_STATUS_NOT_OK(it); + RETURN_IF_NOT_OK_REF(it.status()); Filesystem::DirectoryEntry entry = *it; std::string full_path = path + "/" + entry.name_; std::string full_prefix; @@ -465,7 +465,7 @@ absl::Status DiskLayer::walkDirectory(const std::string& path, const std::string // Read the file and remove any comments. A comment is a line starting with a '#' character. // Comments are useful for placeholder files with no value. auto file_or_error = api.fileSystem().fileReadToEnd(full_path); - RETURN_IF_STATUS_NOT_OK(file_or_error); + RETURN_IF_NOT_OK_REF(file_or_error.status()); const std::string text_file{file_or_error.value()}; const auto lines = StringUtil::splitToken(text_file, "\n"); @@ -492,7 +492,7 @@ absl::Status DiskLayer::walkDirectory(const std::string& path, const std::string #endif } } - RETURN_IF_STATUS_NOT_OK(it); + RETURN_IF_NOT_OK_REF(it.status()); return absl::OkStatus(); } @@ -721,7 +721,7 @@ absl::Status RtdsSubscription::onConfigRemoved( absl::Status LoaderImpl::loadNewSnapshot() { auto snapshot_or_error = createNewSnapshot(); - RETURN_IF_STATUS_NOT_OK(snapshot_or_error); + RETURN_IF_NOT_OK_REF(snapshot_or_error.status()); std::shared_ptr ptr = std::move(snapshot_or_error.value()); tls_->set([ptr](Event::Dispatcher&) -> ThreadLocal::ThreadLocalObjectSharedPtr { return std::static_pointer_cast(ptr); diff --git a/source/common/secret/sds_api.cc b/source/common/secret/sds_api.cc index a8ab37af3ee6..620bffb4972d 100644 --- a/source/common/secret/sds_api.cc +++ b/source/common/secret/sds_api.cc @@ -126,7 +126,7 @@ absl::Status SdsApi::onConfigUpdate(const std::vectoraddWatch(absl::StrCat(result_or_error.value().directory_, "/"), Filesystem::Watcher::Events::MovedTo, [this](uint32_t) { diff --git a/source/common/stats/tag_producer_impl.cc b/source/common/stats/tag_producer_impl.cc index e01fd800f668..082f98a722bf 100644 --- a/source/common/stats/tag_producer_impl.cc +++ b/source/common/stats/tag_producer_impl.cc @@ -158,7 +158,7 @@ TagProducerImpl::addDefaultExtractors(const envoy::config::metrics::v3::StatsCon for (const auto& desc : Config::TagNames::get().descriptorVec()) { auto extractor_or_error = TagExtractorImplBase::createTagExtractor( desc.name_, desc.regex_, desc.substr_, desc.negative_match_, desc.re_type_); - RETURN_IF_STATUS_NOT_OK(extractor_or_error); + RETURN_IF_NOT_OK_REF(extractor_or_error.status()); addExtractor(std::move(extractor_or_error.value())); } for (const auto& desc : Config::TagNames::get().tokenizedDescriptorVec()) { diff --git a/source/common/tls/ocsp/ocsp.cc b/source/common/tls/ocsp/ocsp.cc index b6b52eda7649..51ef8566a6b1 100644 --- a/source/common/tls/ocsp/ocsp.cc +++ b/source/common/tls/ocsp/ocsp.cc @@ -45,9 +45,9 @@ absl::Status skipResponderId(CBS& cbs) { // (excluding the tag and length fields) auto opt1 = Asn1Utility::getOptional(cbs, CBS_ASN1_CONSTRUCTED | CBS_ASN1_CONTEXT_SPECIFIC | 1); - RETURN_IF_STATUS_NOT_OK(opt1); + RETURN_IF_NOT_OK_REF(opt1.status()); auto opt2 = Asn1Utility::getOptional(cbs, CBS_ASN1_CONSTRUCTED | CBS_ASN1_CONTEXT_SPECIFIC | 2); - RETURN_IF_STATUS_NOT_OK(opt2); + RETURN_IF_NOT_OK_REF(opt2.status()); if (opt1.value() || opt2.value()) { return absl::OkStatus(); @@ -64,11 +64,11 @@ absl::Status skipCertStatus(CBS& cbs) { // unknown [2] IMPLICIT UnknownInfo // } auto opt1 = Asn1Utility::getOptional(cbs, CBS_ASN1_CONTEXT_SPECIFIC | 0); - RETURN_IF_STATUS_NOT_OK(opt1); + RETURN_IF_NOT_OK_REF(opt1.status()); auto opt2 = Asn1Utility::getOptional(cbs, CBS_ASN1_CONSTRUCTED | CBS_ASN1_CONTEXT_SPECIFIC | 1); - RETURN_IF_STATUS_NOT_OK(opt2); + RETURN_IF_NOT_OK_REF(opt2.status()); auto opt3 = Asn1Utility::getOptional(cbs, CBS_ASN1_CONTEXT_SPECIFIC | 2); - RETURN_IF_STATUS_NOT_OK(opt3); + RETURN_IF_NOT_OK_REF(opt3.status()); if (!(opt1.value() || opt2.value() || opt3.value())) { return absl::InvalidArgumentError( @@ -181,14 +181,14 @@ absl::StatusOr> Asn1OcspUtility::parseOcspResponse } auto status_or_error = Asn1OcspUtility::parseResponseStatus(elem); - RETURN_IF_STATUS_NOT_OK(status_or_error); + RETURN_IF_NOT_OK_REF(status_or_error.status()); auto opt = Asn1Utility::getOptional(elem, CBS_ASN1_CONSTRUCTED | CBS_ASN1_CONTEXT_SPECIFIC | 0); - RETURN_IF_STATUS_NOT_OK(opt); + RETURN_IF_NOT_OK_REF(opt.status()); auto maybe_bytes = opt.value(); ResponsePtr resp = nullptr; if (maybe_bytes) { auto resp_or_error = Asn1OcspUtility::parseResponseBytes(maybe_bytes.value()); - RETURN_IF_STATUS_NOT_OK(resp_or_error); + RETURN_IF_NOT_OK_REF(resp_or_error.status()); resp = std::move(resp_or_error.value()); } @@ -243,7 +243,7 @@ absl::StatusOr Asn1OcspUtility::parseResponseBytes(CBS& cbs) { } auto parse_or_error = Asn1Utility::parseOid(elem); - RETURN_IF_STATUS_NOT_OK(parse_or_error); + RETURN_IF_NOT_OK_REF(parse_or_error.status()); auto oid_str = parse_or_error.value(); if (!CBS_get_asn1(&elem, &response, CBS_ASN1_OCTETSTRING)) { return absl::InvalidArgumentError("Expected ASN.1 OCTETSTRING for response"); @@ -271,7 +271,7 @@ Asn1OcspUtility::parseBasicOcspResponse(CBS& cbs) { "OCSP BasicOCSPResponse is not a wellf-formed ASN.1 SEQUENCE"); } auto response_or_error = Asn1OcspUtility::parseResponseData(elem); - RETURN_IF_STATUS_NOT_OK(response_or_error); + RETURN_IF_NOT_OK_REF(response_or_error.status()); // The `signatureAlgorithm` and `signature` are ignored because OCSP // responses are expected to be delivered from a reliable source. // Optional additional certs are ignored. @@ -295,11 +295,11 @@ absl::StatusOr Asn1OcspUtility::parseResponseData(CBS& cbs) { // only support v1, the value of v1 is 0x00 auto version_or_error = Asn1Utility::getOptional(elem, CBS_ASN1_CONTEXT_SPECIFIC | CBS_ASN1_CONSTRUCTED | 0); - RETURN_IF_STATUS_NOT_OK(version_or_error); + RETURN_IF_NOT_OK_REF(version_or_error.status()); auto version_cbs = version_or_error.value(); if (version_cbs.has_value()) { auto version_or_error = Asn1Utility::parseInteger(*version_cbs); - RETURN_IF_STATUS_NOT_OK(version_or_error); + RETURN_IF_NOT_OK_REF(version_or_error.status()); auto version = version_or_error.value(); if (version != "00") { return absl::InvalidArgumentError( @@ -309,10 +309,10 @@ absl::StatusOr Asn1OcspUtility::parseResponseData(CBS& cbs) { auto status = skipResponderId(elem); RETURN_IF_NOT_OK(status); - RETURN_IF_STATUS_NOT_OK(Asn1Utility::skip(elem, CBS_ASN1_GENERALIZEDTIME)); + RETURN_IF_NOT_OK_REF(Asn1Utility::skip(elem, CBS_ASN1_GENERALIZEDTIME).status()); auto responses_or_error = Asn1Utility::parseSequenceOf( elem, [](CBS& cbs) -> absl::StatusOr { return {parseSingleResponse(cbs)}; }); - RETURN_IF_STATUS_NOT_OK(responses_or_error); + RETURN_IF_NOT_OK_REF(responses_or_error.status()); // Extensions currently ignored. return {std::move(responses_or_error.value())}; @@ -332,14 +332,14 @@ absl::StatusOr Asn1OcspUtility::parseSingleResponse(CBS& cbs) { } auto id_or_error = Asn1OcspUtility::parseCertId(elem); - RETURN_IF_STATUS_NOT_OK(id_or_error); + RETURN_IF_NOT_OK_REF(id_or_error.status()); RETURN_IF_NOT_OK(skipCertStatus(elem)); auto this_update_or_error = Asn1Utility::parseGeneralizedTime(elem); - RETURN_IF_STATUS_NOT_OK(this_update_or_error); + RETURN_IF_NOT_OK_REF(this_update_or_error.status()); auto next_update_or_error = Asn1Utility::parseOptional( elem, Asn1Utility::parseGeneralizedTime, CBS_ASN1_CONSTRUCTED | CBS_ASN1_CONTEXT_SPECIFIC | 0); - RETURN_IF_STATUS_NOT_OK(next_update_or_error); + RETURN_IF_NOT_OK_REF(next_update_or_error.status()); // Extensions currently ignored. return SingleResponse{id_or_error.value(), this_update_or_error.value(), @@ -358,11 +358,11 @@ absl::StatusOr Asn1OcspUtility::parseCertId(CBS& cbs) { return absl::InvalidArgumentError("OCSP CertID is not a well-formed ASN.1 SEQUENCE"); } - RETURN_IF_STATUS_NOT_OK(Asn1Utility::skip(elem, CBS_ASN1_SEQUENCE)); - RETURN_IF_STATUS_NOT_OK(Asn1Utility::skip(elem, CBS_ASN1_OCTETSTRING)); - RETURN_IF_STATUS_NOT_OK(Asn1Utility::skip(elem, CBS_ASN1_OCTETSTRING)); + RETURN_IF_NOT_OK_REF(Asn1Utility::skip(elem, CBS_ASN1_SEQUENCE).status()); + RETURN_IF_NOT_OK_REF(Asn1Utility::skip(elem, CBS_ASN1_OCTETSTRING).status()); + RETURN_IF_NOT_OK_REF(Asn1Utility::skip(elem, CBS_ASN1_OCTETSTRING).status()); auto serial_number_or_error = Asn1Utility::parseInteger(elem); - RETURN_IF_STATUS_NOT_OK(serial_number_or_error); + RETURN_IF_NOT_OK_REF(serial_number_or_error.status()); return {serial_number_or_error.value()}; } diff --git a/source/common/upstream/cluster_factory_impl.cc b/source/common/upstream/cluster_factory_impl.cc index 1897c7f3b1bb..a4324b8d472d 100644 --- a/source/common/upstream/cluster_factory_impl.cc +++ b/source/common/upstream/cluster_factory_impl.cc @@ -97,7 +97,7 @@ ClusterFactoryImplBase::create(const envoy::config::cluster::v3::Cluster& cluste absl::StatusOr> status_or_cluster = createClusterImpl(cluster, context); - RETURN_IF_STATUS_NOT_OK(status_or_cluster); + RETURN_IF_NOT_OK_REF(status_or_cluster.status()); std::pair& new_cluster_pair = status_or_cluster.value(); @@ -110,7 +110,7 @@ ClusterFactoryImplBase::create(const envoy::config::cluster::v3::Cluster& cluste } else { auto checker_or_error = HealthCheckerFactory::create(cluster.health_checks()[0], *new_cluster_pair.first, server_context); - RETURN_IF_STATUS_NOT_OK(checker_or_error); + RETURN_IF_NOT_OK_REF(checker_or_error.status()); new_cluster_pair.first->setHealthChecker(checker_or_error.value()); } } @@ -119,7 +119,7 @@ ClusterFactoryImplBase::create(const envoy::config::cluster::v3::Cluster& cluste *new_cluster_pair.first, cluster, server_context.mainThreadDispatcher(), server_context.runtime(), context.outlierEventLogger(), server_context.api().randomGenerator()); - RETURN_IF_STATUS_NOT_OK(detector_or_error); + RETURN_IF_NOT_OK_REF(detector_or_error.status()); new_cluster_pair.first->setOutlierDetector(detector_or_error.value()); return status_or_cluster; diff --git a/source/common/upstream/cluster_manager_impl.cc b/source/common/upstream/cluster_manager_impl.cc index d62f54f9ec29..c0e8f515788b 100644 --- a/source/common/upstream/cluster_manager_impl.cc +++ b/source/common/upstream/cluster_manager_impl.cc @@ -417,7 +417,7 @@ ClusterManagerImpl::initialize(const envoy::config::bootstrap::v3::Bootstrap& bo auto status_or_cluster = loadCluster(cluster, MessageUtil::hash(cluster), "", /*added_via_api=*/false, required_for_ads, active_clusters_); - RETURN_IF_STATUS_NOT_OK(status_or_cluster); + RETURN_IF_NOT_OK_REF(status_or_cluster.status()); } } @@ -437,7 +437,7 @@ ClusterManagerImpl::initialize(const envoy::config::bootstrap::v3::Bootstrap& bo dyn_resources.ads_config(), random_, Envoy::Config::SubscriptionFactory::RetryInitialDelayMs, Envoy::Config::SubscriptionFactory::RetryMaxDelayMs); - RETURN_IF_STATUS_NOT_OK(strategy_or_error); + RETURN_IF_NOT_OK_REF(strategy_or_error.status()); JitteredExponentialBackOffStrategyPtr backoff_strategy = std::move(strategy_or_error.value()); const bool use_eds_cache = @@ -458,12 +458,12 @@ ClusterManagerImpl::initialize(const envoy::config::bootstrap::v3::Bootstrap& bo } auto factory_primary_or_error = Config::Utility::factoryForGrpcApiConfigSource( *async_client_manager_, dyn_resources.ads_config(), *stats_.rootScope(), false, 0); - RETURN_IF_STATUS_NOT_OK(factory_primary_or_error); + RETURN_IF_NOT_OK_REF(factory_primary_or_error.status()); Grpc::AsyncClientFactoryPtr factory_failover = nullptr; if (Runtime::runtimeFeatureEnabled("envoy.restart_features.xds_failover_support")) { auto factory_failover_or_error = Config::Utility::factoryForGrpcApiConfigSource( *async_client_manager_, dyn_resources.ads_config(), *stats_.rootScope(), false, 1); - RETURN_IF_STATUS_NOT_OK(factory_failover_or_error); + RETURN_IF_NOT_OK_REF(factory_failover_or_error.status()); factory_failover = std::move(factory_failover_or_error.value()); } ads_mux_ = factory->create( @@ -489,12 +489,12 @@ ClusterManagerImpl::initialize(const envoy::config::bootstrap::v3::Bootstrap& bo } auto factory_primary_or_error = Config::Utility::factoryForGrpcApiConfigSource( *async_client_manager_, dyn_resources.ads_config(), *stats_.rootScope(), false, 0); - RETURN_IF_STATUS_NOT_OK(factory_primary_or_error); + RETURN_IF_NOT_OK_REF(factory_primary_or_error.status()); Grpc::AsyncClientFactoryPtr factory_failover = nullptr; if (Runtime::runtimeFeatureEnabled("envoy.restart_features.xds_failover_support")) { auto factory_failover_or_error = Config::Utility::factoryForGrpcApiConfigSource( *async_client_manager_, dyn_resources.ads_config(), *stats_.rootScope(), false, 1); - RETURN_IF_STATUS_NOT_OK(factory_failover_or_error); + RETURN_IF_NOT_OK_REF(factory_failover_or_error.status()); factory_failover = std::move(factory_failover_or_error.value()); } ads_mux_ = factory->create( @@ -553,7 +553,7 @@ ClusterManagerImpl::initialize(const envoy::config::bootstrap::v3::Bootstrap& bo if (!dyn_resources.cds_resources_locator().empty()) { auto url_or_error = Config::XdsResourceIdentifier::decodeUrl(dyn_resources.cds_resources_locator()); - RETURN_IF_STATUS_NOT_OK(url_or_error); + RETURN_IF_NOT_OK_REF(url_or_error.status()); cds_resources_locator = std::make_unique(std::move(url_or_error.value())); } @@ -594,7 +594,7 @@ absl::Status ClusterManagerImpl::initializeSecondaryClusters( RETURN_IF_NOT_OK(status); auto factory_or_error = Config::Utility::factoryForGrpcApiConfigSource( *async_client_manager_, load_stats_config, *stats_.rootScope(), false, 0); - RETURN_IF_STATUS_NOT_OK(factory_or_error); + RETURN_IF_NOT_OK_REF(factory_or_error.status()); load_stats_reporter_ = std::make_unique( local_info_, *this, *stats_.rootScope(), factory_or_error.value()->createUncachedRawAsyncClient(), dispatcher_); diff --git a/source/common/upstream/health_discovery_service.cc b/source/common/upstream/health_discovery_service.cc index 1ef4adfc54c6..b1b219263584 100644 --- a/source/common/upstream/health_discovery_service.cc +++ b/source/common/upstream/health_discovery_service.cc @@ -440,7 +440,7 @@ absl::Status HdsCluster::updateHealthchecks( // If it does not, create a new one. auto checker_or_error = Upstream::HealthCheckerFactory::create(health_check, *this, server_context_); - RETURN_IF_STATUS_NOT_OK(checker_or_error); + RETURN_IF_NOT_OK_REF(checker_or_error.status()); auto new_health_checker = checker_or_error.value(); health_checkers_map.insert({health_check, new_health_checker}); health_checkers.push_back(new_health_checker); diff --git a/source/common/upstream/upstream_impl.cc b/source/common/upstream/upstream_impl.cc index f3d2066aed66..f725214c948f 100644 --- a/source/common/upstream/upstream_impl.cc +++ b/source/common/upstream/upstream_impl.cc @@ -124,7 +124,7 @@ parseExtensionProtocolOptions( for (const auto& it : config.typed_extension_protocol_options()) { auto& name = it.first; auto object_or_error = createProtocolOptionsConfig(name, it.second, factory_context); - RETURN_IF_STATUS_NOT_OK(object_or_error); + RETURN_IF_NOT_OK_REF(object_or_error.status()); if (object_or_error.value() != nullptr) { options[name] = std::move(object_or_error.value()); } @@ -235,7 +235,7 @@ parseBindConfig(::Envoy::OptRef bind_ auto address_or_error = ::Envoy::Network::Address::resolveProtoSocketAddress(bind_config->source_address()); - RETURN_IF_STATUS_NOT_OK(address_or_error); + RETURN_IF_NOT_OK_REF(address_or_error.status()); upstream_local_address.address_ = address_or_error.value(); } upstream_local_address.socket_options_ = std::make_shared(); @@ -251,7 +251,7 @@ parseBindConfig(::Envoy::OptRef bind_ UpstreamLocalAddress extra_upstream_local_address; auto address_or_error = ::Envoy::Network::Address::resolveProtoSocketAddress(extra_source_address.address()); - RETURN_IF_STATUS_NOT_OK(address_or_error); + RETURN_IF_NOT_OK_REF(address_or_error.status()); extra_upstream_local_address.address_ = address_or_error.value(); extra_upstream_local_address.socket_options_ = @@ -275,7 +275,7 @@ parseBindConfig(::Envoy::OptRef bind_ UpstreamLocalAddress additional_upstream_local_address; auto address_or_error = ::Envoy::Network::Address::resolveProtoSocketAddress(additional_source_address); - RETURN_IF_STATUS_NOT_OK(address_or_error); + RETURN_IF_NOT_OK_REF(address_or_error.status()); additional_upstream_local_address.address_ = address_or_error.value(); additional_upstream_local_address.socket_options_ = std::make_shared<::Envoy::Network::ConnectionSocket::Options>(); @@ -373,10 +373,10 @@ createUpstreamLocalAddressSelector( envoy::config::core::v3::BindConfig{})), buildClusterSocketOptions(cluster_config, bootstrap_bind_config.value_or( envoy::config::core::v3::BindConfig{}))); - RETURN_IF_STATUS_NOT_OK(config_or_error); + RETURN_IF_NOT_OK_REF(config_or_error.status()); auto selector_or_error = local_address_selector_factory->createLocalAddressSelector( config_or_error.value(), cluster_name); - RETURN_IF_STATUS_NOT_OK(selector_or_error); + RETURN_IF_NOT_OK_REF(selector_or_error.status()); return selector_or_error.value(); } @@ -981,7 +981,7 @@ createOptions(const envoy::config::cluster::v3::Cluster& config, config.protocol_selection() == envoy::config::cluster::v3::Cluster::USE_DOWNSTREAM_PROTOCOL, config.has_http2_protocol_options(), validation_visitor); - RETURN_IF_STATUS_NOT_OK(options_or_error); + RETURN_IF_NOT_OK_REF(options_or_error.status()); return options_or_error.value(); } diff --git a/source/extensions/clusters/dynamic_forward_proxy/cluster.cc b/source/extensions/clusters/dynamic_forward_proxy/cluster.cc index d80ca3e83eac..4ec0a8298ac9 100644 --- a/source/extensions/clusters/dynamic_forward_proxy/cluster.cc +++ b/source/extensions/clusters/dynamic_forward_proxy/cluster.cc @@ -499,7 +499,7 @@ ClusterFactory::createClusterWithConfig( Extensions::Common::DynamicForwardProxy::DnsCacheManagerSharedPtr cache_manager = cache_manager_factory.get(); auto dns_cache_or_error = cache_manager->getCache(proto_config.dns_cache_config()); - RETURN_IF_STATUS_NOT_OK(dns_cache_or_error); + RETURN_IF_NOT_OK_REF(dns_cache_or_error.status()); absl::Status creation_status = absl::OkStatus(); auto new_cluster = std::shared_ptr( diff --git a/source/extensions/common/dynamic_forward_proxy/dns_cache_manager_impl.cc b/source/extensions/common/dynamic_forward_proxy/dns_cache_manager_impl.cc index e15cfe325af1..cb98cfae2b09 100644 --- a/source/extensions/common/dynamic_forward_proxy/dns_cache_manager_impl.cc +++ b/source/extensions/common/dynamic_forward_proxy/dns_cache_manager_impl.cc @@ -27,7 +27,7 @@ absl::StatusOr DnsCacheManagerImpl::getCache( } auto cache_or_status = DnsCacheImpl::createDnsCacheImpl(context_, config); - RETURN_IF_STATUS_NOT_OK(cache_or_status); + RETURN_IF_NOT_OK_REF(cache_or_status.status()); DnsCacheSharedPtr new_cache = std::move(cache_or_status.value()); caches_.emplace(config.name(), ActiveCache{config, new_cache}); return new_cache; diff --git a/source/extensions/filters/http/match_delegate/config.cc b/source/extensions/filters/http/match_delegate/config.cc index e944c7b47bcb..5651c9ca8f40 100644 --- a/source/extensions/filters/http/match_delegate/config.cc +++ b/source/extensions/filters/http/match_delegate/config.cc @@ -312,7 +312,7 @@ absl::StatusOr MatchDelegateConfig::createFilterFa auto message = Config::Utility::translateAnyToFactoryConfig( proto_config.extension_config().typed_config(), validation, factory); auto filter_factory_or_error = factory.createFilterFactoryFromProto(*message, prefix, context); - RETURN_IF_STATUS_NOT_OK(filter_factory_or_error); + RETURN_IF_NOT_OK_REF(filter_factory_or_error.status()); auto filter_factory = filter_factory_or_error.value(); Factory::MatchTreeValidationVisitor validation_visitor(*factory.matchingRequirements()); diff --git a/source/extensions/network/dns_resolver/cares/dns_impl.cc b/source/extensions/network/dns_resolver/cares/dns_impl.cc index 46986aadeb3a..04fa3622e7cf 100644 --- a/source/extensions/network/dns_resolver/cares/dns_impl.cc +++ b/source/extensions/network/dns_resolver/cares/dns_impl.cc @@ -583,7 +583,7 @@ class CaresDnsResolverFactory : public DnsResolverFactory, resolvers.reserve(resolver_addrs.size()); for (const auto& resolver_addr : resolver_addrs) { auto address_or_error = Network::Address::resolveProtoAddress(resolver_addr); - RETURN_IF_STATUS_NOT_OK(address_or_error); + RETURN_IF_NOT_OK_REF(address_or_error.status()); resolvers.push_back(std::move(address_or_error.value())); } } diff --git a/source/extensions/transport_sockets/http_11_proxy/config.cc b/source/extensions/transport_sockets/http_11_proxy/config.cc index a54ead186aac..0e95e6a34743 100644 --- a/source/extensions/transport_sockets/http_11_proxy/config.cc +++ b/source/extensions/transport_sockets/http_11_proxy/config.cc @@ -25,7 +25,7 @@ UpstreamHttp11ConnectSocketConfigFactory::createTransportSocketFactory( outer_config.transport_socket(), context.messageValidationVisitor(), inner_config_factory); auto factory_or_error = inner_config_factory.createTransportSocketFactory(*inner_factory_config, context); - RETURN_IF_STATUS_NOT_OK(factory_or_error); + RETURN_IF_NOT_OK_REF(factory_or_error.status()); return std::make_unique(std::move(factory_or_error.value())); } diff --git a/source/extensions/transport_sockets/internal_upstream/config.cc b/source/extensions/transport_sockets/internal_upstream/config.cc index dfef2a21e662..d8b31db91e9b 100644 --- a/source/extensions/transport_sockets/internal_upstream/config.cc +++ b/source/extensions/transport_sockets/internal_upstream/config.cc @@ -40,7 +40,7 @@ class InternalUpstreamConfigFactory inner_config_factory); auto factory_or_error = inner_config_factory.createTransportSocketFactory(*inner_factory_config, context); - RETURN_IF_STATUS_NOT_OK(factory_or_error); + RETURN_IF_NOT_OK_REF(factory_or_error.status()); return std::make_unique(context, outer_config, std::move(factory_or_error.value())); } diff --git a/source/extensions/transport_sockets/proxy_protocol/config.cc b/source/extensions/transport_sockets/proxy_protocol/config.cc index 05551ad61f28..fcde4faafad4 100644 --- a/source/extensions/transport_sockets/proxy_protocol/config.cc +++ b/source/extensions/transport_sockets/proxy_protocol/config.cc @@ -26,7 +26,7 @@ UpstreamProxyProtocolSocketConfigFactory::createTransportSocketFactory( outer_config.transport_socket(), context.messageValidationVisitor(), inner_config_factory); auto factory_or_error = inner_config_factory.createTransportSocketFactory(*inner_factory_config, context); - RETURN_IF_STATUS_NOT_OK(factory_or_error); + RETURN_IF_NOT_OK_REF(factory_or_error.status()); return std::make_unique( std::move(factory_or_error.value()), outer_config.config(), context.statsScope()); } diff --git a/source/extensions/transport_sockets/starttls/config.cc b/source/extensions/transport_sockets/starttls/config.cc index 7d4552a7a1e6..a2f0722df7e6 100644 --- a/source/extensions/transport_sockets/starttls/config.cc +++ b/source/extensions/transport_sockets/starttls/config.cc @@ -20,11 +20,11 @@ DownstreamStartTlsSocketFactory::createTransportSocketFactory( auto raw_or_error = raw_socket_config_factory.createTransportSocketFactory( outer_config.cleartext_socket_config(), context, server_names); - RETURN_IF_STATUS_NOT_OK(raw_or_error); + RETURN_IF_NOT_OK_REF(raw_or_error.status()); auto factory_or_error = tls_socket_config_factory.createTransportSocketFactory( outer_config.tls_socket_config(), context, server_names); - RETURN_IF_STATUS_NOT_OK(factory_or_error); + RETURN_IF_NOT_OK_REF(factory_or_error.status()); return std::make_unique(std::move(raw_or_error.value()), std::move(factory_or_error.value())); @@ -43,11 +43,11 @@ UpstreamStartTlsSocketFactory::createTransportSocketFactory( auto raw_or_error = raw_socket_config_factory.createTransportSocketFactory( outer_config.cleartext_socket_config(), context); - RETURN_IF_STATUS_NOT_OK(raw_or_error); + RETURN_IF_NOT_OK_REF(raw_or_error.status()); auto factory_or_error = tls_socket_config_factory.createTransportSocketFactory( outer_config.tls_socket_config(), context); - RETURN_IF_STATUS_NOT_OK(factory_or_error); + RETURN_IF_NOT_OK_REF(factory_or_error.status()); return std::make_unique(std::move(raw_or_error.value()), std::move(factory_or_error.value())); diff --git a/source/extensions/transport_sockets/tap/config.cc b/source/extensions/transport_sockets/tap/config.cc index 2bb7c37f22f7..b1417d9beda3 100644 --- a/source/extensions/transport_sockets/tap/config.cc +++ b/source/extensions/transport_sockets/tap/config.cc @@ -47,7 +47,7 @@ UpstreamTapSocketConfigFactory::createTransportSocketFactory( outer_config.transport_socket(), context.messageValidationVisitor(), inner_config_factory); auto factory_or_error = inner_config_factory.createTransportSocketFactory(*inner_factory_config, context); - RETURN_IF_STATUS_NOT_OK(factory_or_error); + RETURN_IF_NOT_OK_REF(factory_or_error.status()); auto& server_context = context.serverFactoryContext(); return std::make_unique( @@ -72,7 +72,7 @@ DownstreamTapSocketConfigFactory::createTransportSocketFactory( outer_config.transport_socket(), context.messageValidationVisitor(), inner_config_factory); auto factory_or_error = inner_config_factory.createTransportSocketFactory(*inner_factory_config, context, server_names); - RETURN_IF_STATUS_NOT_OK(factory_or_error); + RETURN_IF_NOT_OK_REF(factory_or_error.status()); auto& server_context = context.serverFactoryContext(); return std::make_unique( outer_config, diff --git a/source/extensions/transport_sockets/tcp_stats/config.cc b/source/extensions/transport_sockets/tcp_stats/config.cc index f3702c051ad3..938fc6c14e63 100644 --- a/source/extensions/transport_sockets/tcp_stats/config.cc +++ b/source/extensions/transport_sockets/tcp_stats/config.cc @@ -93,7 +93,7 @@ class UpstreamTcpStatsConfigFactory inner_config_factory); auto factory_or_error = inner_config_factory.createTransportSocketFactory(*inner_factory_config, context); - RETURN_IF_STATUS_NOT_OK(factory_or_error); + RETURN_IF_NOT_OK_REF(factory_or_error.status()); return std::make_unique(context, outer_config, std::move(factory_or_error.value())); } @@ -119,7 +119,7 @@ class DownstreamTcpStatsConfigFactory inner_config_factory); auto factory_or_error = inner_config_factory.createTransportSocketFactory( *inner_factory_config, context, server_names); - RETURN_IF_STATUS_NOT_OK(factory_or_error); + RETURN_IF_NOT_OK_REF(factory_or_error.status()); return std::make_unique(context, outer_config, std::move(factory_or_error.value())); } diff --git a/source/extensions/upstreams/http/config.cc b/source/extensions/upstreams/http/config.cc index ce9330bd4661..8acda93047c8 100644 --- a/source/extensions/upstreams/http/config.cc +++ b/source/extensions/upstreams/http/config.cc @@ -189,11 +189,11 @@ ProtocolOptionsConfigImpl::createProtocolOptionsConfig( const envoy::extensions::upstreams::http::v3::HttpProtocolOptions& options, Server::Configuration::ServerFactoryContext& server_context) { auto options_or_error = Http2::Utility::initializeAndValidateOptions(getHttp2Options(options)); - RETURN_IF_STATUS_NOT_OK(options_or_error); + RETURN_IF_NOT_OK_REF(options_or_error.status()); auto cache_options_or_error = getAlternateProtocolsCacheOptions(options, server_context); - RETURN_IF_STATUS_NOT_OK(cache_options_or_error); + RETURN_IF_NOT_OK_REF(cache_options_or_error.status()); auto validator_factory_or_error = createHeaderValidatorFactory(options, server_context); - RETURN_IF_STATUS_NOT_OK(validator_factory_or_error); + RETURN_IF_NOT_OK_REF(validator_factory_or_error.status()); return std::shared_ptr(new ProtocolOptionsConfigImpl( options, options_or_error.value(), std::move(validator_factory_or_error.value()), cache_options_or_error.value(), server_context)); @@ -208,7 +208,7 @@ ProtocolOptionsConfigImpl::createProtocolOptionsConfig( bool use_downstream_protocol, bool use_http2, ProtobufMessage::ValidationVisitor& validation_visitor) { auto options_or_error = Http2::Utility::initializeAndValidateOptions(http2_options); - RETURN_IF_STATUS_NOT_OK(options_or_error); + RETURN_IF_NOT_OK_REF(options_or_error.status()); return std::shared_ptr(new ProtocolOptionsConfigImpl( http1_settings, options_or_error.value(), common_options, upstream_options, use_downstream_protocol, use_http2, validation_visitor)); diff --git a/source/server/api_listener_impl.cc b/source/server/api_listener_impl.cc index a9dfa065d89c..7f5d4e8fe9ad 100644 --- a/source/server/api_listener_impl.cc +++ b/source/server/api_listener_impl.cc @@ -45,7 +45,7 @@ absl::StatusOr> HttpApiListener::create(const envoy::config::listener::v3::Listener& config, Server::Instance& server, const std::string& name) { auto address_or_error = Network::Address::resolveProtoAddress(config.address()); - RETURN_IF_STATUS_NOT_OK(address_or_error); + RETURN_IF_NOT_OK_REF(address_or_error.status()); return std::unique_ptr( new HttpApiListener(std::move(address_or_error.value()), config, server, name)); } diff --git a/source/server/configuration_impl.cc b/source/server/configuration_impl.cc index 883b47531507..8a9f763d4a53 100644 --- a/source/server/configuration_impl.cc +++ b/source/server/configuration_impl.cc @@ -140,7 +140,7 @@ absl::Status MainImpl::initialize(const envoy::config::bootstrap::v3::Bootstrap& ENVOY_LOG(debug, "listener #{}:", i); absl::StatusOr update_or_error = server.listenerManager().addOrUpdateListener(listeners[i], "", false); - RETURN_IF_STATUS_NOT_OK(update_or_error); + RETURN_IF_NOT_OK_REF(update_or_error.status()); } RETURN_IF_NOT_OK(initializeWatchdogs(bootstrap, server)); // This has to happen after ClusterManager initialization, as it depends on config from diff --git a/source/server/server.cc b/source/server/server.cc index 2b43921d973a..a8fbc195549e 100644 --- a/source/server/server.cc +++ b/source/server/server.cc @@ -503,7 +503,7 @@ absl::Status InstanceBase::initializeOrThrow(Network::Address::InstanceConstShar // stats. auto producer_or_error = Stats::TagProducerImpl::createTagProducer(bootstrap_.stats_config(), options_.statsTags()); - RETURN_IF_STATUS_NOT_OK(producer_or_error); + RETURN_IF_NOT_OK_REF(producer_or_error.status()); stats_store_.setTagProducer(std::move(producer_or_error.value())); stats_store_.setStatsMatcher(std::make_unique( bootstrap_.stats_config(), stats_store_.symbolTable(), server_contexts_)); From ec61a3cc7abd77d1dfa5a6e96253492edf7d6bf5 Mon Sep 17 00:00:00 2001 From: "Antonio V. Leonti" <53806445+antoniovleonti@users.noreply.github.com> Date: Thu, 1 Aug 2024 09:33:40 -0400 Subject: [PATCH 6/9] clean up doEndStream (#35506) There are some variables that are only used if check_for_deferred_close is true. Move them to be inside the `if (check_for_deferred_close)` block. Signed-off-by: antoniovleonti --- source/common/http/conn_manager_impl.cc | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/source/common/http/conn_manager_impl.cc b/source/common/http/conn_manager_impl.cc index 2b709f2eebd2..77aa6d7a9490 100644 --- a/source/common/http/conn_manager_impl.cc +++ b/source/common/http/conn_manager_impl.cc @@ -280,17 +280,18 @@ void ConnectionManagerImpl::doEndStream(ActiveStream& stream, bool check_for_def drain_state_ = DrainState::Closing; } - // If HTTP/1.0 has no content length, it is framed by close and won't consider - // the request complete until the FIN is read. Don't delay close in this case. - bool http_10_sans_cl = (codec_->protocol() == Protocol::Http10) && - (!stream.response_headers_ || !stream.response_headers_->ContentLength()); - // We also don't delay-close in the case of HTTP/1.1 where the request is - // fully read, as there's no race condition to avoid. - const bool connection_close = - stream.filter_manager_.streamInfo().shouldDrainConnectionUponCompletion(); - bool request_complete = stream.filter_manager_.remoteDecodeComplete(); - if (check_for_deferred_close) { + // If HTTP/1.0 has no content length, it is framed by close and won't consider + // the request complete until the FIN is read. Don't delay close in this case. + const bool http_10_sans_cl = + (codec_->protocol() == Protocol::Http10) && + (!stream.response_headers_ || !stream.response_headers_->ContentLength()); + // We also don't delay-close in the case of HTTP/1.1 where the request is + // fully read, as there's no race condition to avoid. + const bool connection_close = + stream.filter_manager_.streamInfo().shouldDrainConnectionUponCompletion(); + const bool request_complete = stream.filter_manager_.remoteDecodeComplete(); + // Don't do delay close for HTTP/1.0 or if the request is complete. checkForDeferredClose(connection_close && (request_complete || http_10_sans_cl)); } From ff94c296f27be2f4a6cd71a2d6b0898cec6c2100 Mon Sep 17 00:00:00 2001 From: yanjunxiang-google <78807980+yanjunxiang-google@users.noreply.github.com> Date: Thu, 1 Aug 2024 10:11:07 -0400 Subject: [PATCH 7/9] Adding HTTP service support for Envoy external processing (#35489) This is to address the 1st step, i.e, the API change needed for https://github.com/envoyproxy/envoy/issues/35488. --------- Signed-off-by: Yanjun Xiang --- .../filters/http/ext_proc/v3/ext_proc.proto | 22 +++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/api/envoy/extensions/filters/http/ext_proc/v3/ext_proc.proto b/api/envoy/extensions/filters/http/ext_proc/v3/ext_proc.proto index aeaed7aa2ab7..1d79d39d99b8 100644 --- a/api/envoy/extensions/filters/http/ext_proc/v3/ext_proc.proto +++ b/api/envoy/extensions/filters/http/ext_proc/v3/ext_proc.proto @@ -5,6 +5,7 @@ package envoy.extensions.filters.http.ext_proc.v3; import "envoy/config/common/mutation_rules/v3/mutation_rules.proto"; import "envoy/config/core/v3/base.proto"; import "envoy/config/core/v3/grpc_service.proto"; +import "envoy/config/core/v3/http_service.proto"; import "envoy/extensions/filters/http/ext_proc/v3/processing_mode.proto"; import "envoy/type/matcher/v3/string.proto"; @@ -98,7 +99,7 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE; // ` object in a namespace matching the filter // name. // -// [#next-free-field: 20] +// [#next-free-field: 21] message ExternalProcessor { // Describes the route cache action to be taken when an external processor response // is received in response to request headers. @@ -125,7 +126,18 @@ message ExternalProcessor { // Configuration for the gRPC service that the filter will communicate with. // The filter supports both the "Envoy" and "Google" gRPC clients. - config.core.v3.GrpcService grpc_service = 1 [(validate.rules).message = {required: true}]; + // Only one of ``grpc_service`` or ``http_service`` can be set. + // It is required that one of them must be set. + config.core.v3.GrpcService grpc_service = 1 + [(udpa.annotations.field_migrate).oneof_promotion = "ext_proc_service_type"]; + + // [#not-implemented-hide:] + // Configuration for the HTTP service that the filter will communicate with. + // Only one of ``http_service`` or + // :ref:`grpc_service `. + // can be set. It is required that one of them must be set. + ExtProcHttpService http_service = 20 + [(udpa.annotations.field_migrate).oneof_promotion = "ext_proc_service_type"]; // By default, if the gRPC stream cannot be established, or if it is closed // prematurely with an error, the filter will fail. Specifically, if the @@ -265,6 +277,12 @@ message ExternalProcessor { google.protobuf.Duration deferred_close_timeout = 19; } +// ExtProcHttpService is used for HTTP communication between the filter and the external processing service. +message ExtProcHttpService { + // Sets the HTTP service which the external processing requests must be sent to. + config.core.v3.HttpService http_service = 1; +} + // The MetadataOptions structure defines options for the sending and receiving of // dynamic metadata. Specifically, which namespaces to send to the server, whether // metadata returned by the server may be written, and how that metadata may be written. From 8b9b23b9a1d479b7e11212c865c647f2f78737e8 Mon Sep 17 00:00:00 2001 From: "Vikas Choudhary (vikasc)" Date: Thu, 1 Aug 2024 20:19:58 +0530 Subject: [PATCH 8/9] Remove feature flag upstream_allow_connect_with_2xx (#35521) Signed-off-by: Vikas Choudhary --- changelogs/current.yaml | 3 +++ source/common/router/upstream_codec_filter.cc | 5 +---- source/common/runtime/runtime_features.cc | 1 - 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/changelogs/current.yaml b/changelogs/current.yaml index cf1201c0c0a9..28ea9a0ef938 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -80,6 +80,9 @@ removed_config_or_runtime: - area: stateful_session change: | Removed ``envoy.reloadable_features.stateful_session_encode_ttl_in_cookie`` runtime flag and legacy code paths. +- area: upstream + change: | + Removed runtime flag ``envoy.reloadable_features.upstream_allow_connect_with_2xx`` and legacy code paths. - area: upstream flow control change: | Removed ``envoy.reloadable_features.upstream_wait_for_response_headers_before_disabling_read`` runtime flag diff --git a/source/common/router/upstream_codec_filter.cc b/source/common/router/upstream_codec_filter.cc index 56ac395dd40b..846645a5e000 100644 --- a/source/common/router/upstream_codec_filter.cc +++ b/source/common/router/upstream_codec_filter.cc @@ -148,10 +148,7 @@ void UpstreamCodecFilter::CodecBridge::decodeHeaders(Http::ResponseHeaderMapPtr& filter_.callbacks_->dispatcher().timeSource()); if (filter_.callbacks_->upstreamCallbacks()->pausedForConnect() && - ((Http::Utility::getResponseStatus(*headers) == 200) || - ((Runtime::runtimeFeatureEnabled( - "envoy.reloadable_features.upstream_allow_connect_with_2xx")) && - (Http::CodeUtility::is2xx(Http::Utility::getResponseStatus(*headers)))))) { + ((Http::CodeUtility::is2xx(Http::Utility::getResponseStatus(*headers))))) { filter_.callbacks_->upstreamCallbacks()->setPausedForConnect(false); filter_.callbacks_->continueDecoding(); } diff --git a/source/common/runtime/runtime_features.cc b/source/common/runtime/runtime_features.cc index 55cf4e0e57d7..b2fe1912886b 100644 --- a/source/common/runtime/runtime_features.cc +++ b/source/common/runtime/runtime_features.cc @@ -90,7 +90,6 @@ RUNTIME_GUARD(envoy_reloadable_features_tcp_tunneling_send_downstream_fin_on_ups RUNTIME_GUARD(envoy_reloadable_features_test_feature_true); RUNTIME_GUARD(envoy_reloadable_features_udp_socket_apply_aggregated_read_limit); RUNTIME_GUARD(envoy_reloadable_features_uhv_allow_malformed_url_encoding); -RUNTIME_GUARD(envoy_reloadable_features_upstream_allow_connect_with_2xx); RUNTIME_GUARD(envoy_reloadable_features_upstream_remote_address_use_connection); RUNTIME_GUARD(envoy_reloadable_features_use_http3_header_normalisation); RUNTIME_GUARD(envoy_reloadable_features_use_typed_metadata_in_proxy_protocol_listener); From 0fa9e603085952a80d78559fd8596dafa14cef6b Mon Sep 17 00:00:00 2001 From: Bin Wu <46450037+wu-bin@users.noreply.github.com> Date: Thu, 1 Aug 2024 11:30:25 -0400 Subject: [PATCH 9/9] Ensure every instance of StreamFilter and PassThroughFilter to contain only one copy of StreamFilterBase. (#35341) Change Stream(Decode|Encode)Filter to inherit from StreamFilterBase virtually. Move the default implementation of PassThrough(Decoder|Encode)Filter::onDestroy into StreamFilterBase. --------- Signed-off-by: Bin Wu --- envoy/http/filter.h | 4 ++-- source/extensions/filters/http/common/pass_through_filter.h | 6 +++++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/envoy/http/filter.h b/envoy/http/filter.h index 1caebab6a46c..49d2609a1152 100644 --- a/envoy/http/filter.h +++ b/envoy/http/filter.h @@ -896,7 +896,7 @@ class StreamFilterBase { /** * Stream decoder filter interface. */ -class StreamDecoderFilter : public StreamFilterBase { +class StreamDecoderFilter : public virtual StreamFilterBase { public: /** * Called with decoded headers, optionally indicating end of stream. @@ -1112,7 +1112,7 @@ class StreamEncoderFilterCallbacks : public virtual StreamFilterCallbacks { /** * Stream encoder filter interface. */ -class StreamEncoderFilter : public StreamFilterBase { +class StreamEncoderFilter : public virtual StreamFilterBase { public: /** * Called with supported 1xx headers. diff --git a/source/extensions/filters/http/common/pass_through_filter.h b/source/extensions/filters/http/common/pass_through_filter.h index ad299e47da15..e6291b9e3dd1 100644 --- a/source/extensions/filters/http/common/pass_through_filter.h +++ b/source/extensions/filters/http/common/pass_through_filter.h @@ -62,6 +62,10 @@ class PassThroughEncoderFilter : public virtual StreamEncoderFilter { // A filter which passes all data through with Continue status. class PassThroughFilter : public StreamFilter, public PassThroughDecoderFilter, - public PassThroughEncoderFilter {}; + public PassThroughEncoderFilter { +public: + // Http::StreamFilterBase + void onDestroy() override {} +}; } // namespace Http } // namespace Envoy