Merge branch 'main' into metadata-matcher

Signed-off-by: Vikas Choudhary <[email protected]>

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;
 // <arch_overview_advanced_filter_state_sharing>` 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 <envoy_v3_api_field_extensions.filters.http.ext_proc.v3.ExternalProcessor.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.

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;
 }
 

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
@@ -119,6 +122,10 @@ new_features:
   change: |
     Added dynamic metadata matcher support :ref:`Dynamic metadata input <extension_envoy.matching.inputs.dynamic_metadata>`
     and :ref:`Dynamic metadata input matcher <extension_envoy.matching.matchers.metadata_matcher>`.
+- area: ratelimit
+  change: |
+    Added the ability to modify :ref:`hits_addend <envoy_v3_api_field_service.ratelimit.v3.RateLimitRequest.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%``,

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:
   <envoy_v3_api_field_extensions.filters.network.tcp_proxy.v3.TcpProxy.idle_timeout>` 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
+  <envoy_v3_api_field_service.ratelimit.v3.RateLimitRequest.hits_addend>` override on a per-route basis.
+  Accepts a number string as a constructor.
+
 Filter state object fields
 --------------------------
 

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;                                                                            \
   }
 

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.

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 {

source/common/config/datasource.cc

@@ -40,7 +40,7 @@ absl::StatusOr<std::string> 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<DataSourceProviderPtr> 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.

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;

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<FileWatchPtr> 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::FileWatchPtr> 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<PathSplitResult> 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();

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<PathSplitResult> 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

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<FactoryCb> 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<FactoryCb> 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<Http::FilterFactoryCb> 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<Network::FilterFactoryCb> 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();
   }
 

source/common/grpc/async_client_manager_impl.cc

@@ -167,7 +167,7 @@ absl::StatusOr<RawAsyncClientSharedPtr> 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;

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));
   }

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 {

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());
 }
 

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<xds::type::v3::TypedStruct>(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<udpa::type::v1::TypedStruct>(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());
     }
 

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<PathRewriterSharedPtr> 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<PathMatcherSharedPtr> matcher = factory.createPathMatcher(*config);
-  RETURN_IF_STATUS_NOT_OK(matcher);
+  RETURN_IF_NOT_OK_REF(matcher.status());
 
   return matcher.value();
 }

source/common/router/header_parser.cc

@@ -89,7 +89,7 @@ HeaderParser::configure(const Protobuf::RepeatedPtrField<HeaderValueOption>& 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<HeaderParserPtr> 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<HeaderParserPtr>
 HeaderParser::configure(const Protobuf::RepeatedPtrField<HeaderValueOption>& headers_to_add,
                         const Protobuf::RepeatedPtrField<std::string>& 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) {