vishalpowar · vishalpowar · Aug 3, 2018 · Aug 2, 2018 · Aug 2, 2018 · Aug 2, 2018
diff --git a/OWNERS.md b/OWNERS.md
@@ -14,6 +14,11 @@ routing PRs, questions, etc. to the right place.
 * Alyssa Wilk ([alyssawilk](https://github.com/alyssawilk)) (alyssar@google.com)
   * HTTP, flow control, cluster manager, load balancing, and core networking (listeners,
     connections, etc.).
+* Stephan Zuercher ([zuercher](https://github.com/zuercher)) (stephan@turbinelabs.io)
+  * Load balancing, upstream clusters and cluster manager, logging, complex HTTP routing
+    (metadata, etc.), and OSX build.
+* Greg Greenway ([ggreenway](https://github.com/ggreenway)) (ggreenway@apple.com)
+  * TCP proxy, TLS, logging, and core networking (listeners, connections, etc.).
 
 # Maintainers
 
@@ -25,13 +30,10 @@ routing PRs, questions, etc. to the right place.
   * Base server (watchdog, workers, startup, stack trace handling, etc.).
 * Daniel Hochman ([danielhochman](https://github.com/danielhochman)) (dhochman@lyft.com)
   * Redis, Python, configuration/operational questions.
-* Stephan Zuercher ([zuercher](https://github.com/zuercher)) (stephan@turbinelabs.io)
-  * Load balancing, upstream clusters and cluster manager, logging, complex HTTP routing
-    (metadata, etc.), and OSX build.
-* Greg Greenway ([ggreenway](https://github.com/ggreenway)) (ggreenway@apple.com)
-  * TCP proxy, TLS, logging, and core networking (listeners, connections, etc.).
 * Lizan Zhou ([lizan](https://github.com/lizan)) (zlizan@google.com)
   * gRPC, gRPC/JSON transcoding, and core networking (transport socket abstractions).
+* Dhi Aurrahman ([dio](https://github.com/dio)) (dio@tetrate.io)
+  * Lua, access logging, and general miscellany.
 
 # Emeritus maintainers
 
@@ -50,4 +52,4 @@ matter expert reviews. Feel free to loop them in as needed.
 * John Millikin ([jmillikin-stripe](https://github.com/jmillikin-stripe)) (jmillikin@stripe.com)
   * Bazel/build.
 * Joshua Marantz ([jmarantz](https://github.com/jmarantz)) (jmarantz@google.com)
-  * abseil and performance work.
+  * abseil and performance work.
diff --git a/api/envoy/api/v2/core/config_source.proto b/api/envoy/api/v2/core/config_source.proto
@@ -44,6 +44,10 @@ message ApiConfigSource {
 
   // For REST APIs, the delay between successive polls.
   google.protobuf.Duration refresh_delay = 3 [(gogoproto.stdduration) = true];
+
+  // For REST APIs, the request timeout. If not set, a default value of 1s will be used.
+  google.protobuf.Duration request_timeout = 5
+      [(validate.rules).duration.gt.seconds = 0, (gogoproto.stdduration) = true];
 }
 
 // Aggregated Discovery Service (ADS) options. This is currently empty, but when

diff --git a/bazel/repository_locations.bzl b/bazel/repository_locations.bzl
@@ -5,7 +5,7 @@ REPOSITORY_LOCATIONS = dict(
         remote = "https://github.com/google/boringssl",
     ),
     com_google_absl = dict(
-        commit = "92020a042c0cd46979db9f6f0cb32783dc07765e",  # 2018-06-08
+        commit = "92e07e5590752d6b8e67f7f2f86c6286561e8cea",  # 2018-08-01
         remote = "https://github.com/abseil/abseil-cpp",
     ),
     com_github_apache_thrift = dict(

diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst
@@ -48,6 +48,7 @@ Version history
   :ref:`use_data_plane_proto<envoy_api_field_config.ratelimit.v2.RateLimitServiceConfig.use_data_plane_proto>`
   boolean flag in the ratelimit configuration.
   Support for the legacy proto :repo:`source/common/ratelimit/ratelimit.proto` is deprecated and will be removed at the start of the 1.9.0 release cycle.
+* rest-api: added ability to set the :ref:`request timeout <envoy_api_field_core.ApiConfigSource.request_timeout>` for REST API requests.
 * router: added ability to set request/response headers at the :ref:`envoy_api_msg_route.Route` level.
 * tracing: added support for configuration of :ref:`tracing sampling
   <envoy_api_field_config.filter.network.http_connection_manager.v2.HttpConnectionManager.tracing>`.

diff --git a/include/envoy/server/BUILD b/include/envoy/server/BUILD
@@ -175,9 +175,14 @@ envoy_cc_library(
     name = "transport_socket_config_interface",
     hdrs = ["transport_socket_config.h"],
     deps = [
+        "//include/envoy/event:dispatcher_interface",
+        "//include/envoy/local_info:local_info_interface",
         "//include/envoy/network:transport_socket_interface",
+        "//include/envoy/runtime:runtime_interface",
         "//include/envoy/secret:secret_manager_interface",
         "//include/envoy/ssl:context_manager_interface",
+        "//include/envoy/stats:stats_interface",
+        "//include/envoy/upstream:cluster_manager_interface",
         "//source/common/protobuf",
     ],
 )

diff --git a/include/envoy/server/transport_socket_config.h b/include/envoy/server/transport_socket_config.h
@@ -2,9 +2,14 @@
 
 #include <string>
 
+#include "envoy/event/dispatcher.h"
+#include "envoy/local_info/local_info.h"
 #include "envoy/network/transport_socket.h"
+#include "envoy/runtime/runtime.h"
 #include "envoy/secret/secret_manager.h"
 #include "envoy/ssl/context_manager.h"
+#include "envoy/stats/stats.h"
+#include "envoy/upstream/cluster_manager.h"
 
 #include "common/protobuf/protobuf.h"
 
@@ -33,6 +38,31 @@ class TransportSocketFactoryContext {
    * Return the instance of secret manager.
    */
   virtual Secret::SecretManager& secretManager() PURE;
+
+  /**
+   * @return the instance of ClusterManager.
+   */
+  virtual Upstream::ClusterManager& clusterManager() PURE;
+
+  /**
+   * @return information about the local environment the server is running in.
+   */
+  virtual const LocalInfo::LocalInfo& localInfo() PURE;
+
+  /**
+   * @return Event::Dispatcher& the main thread's dispatcher.
+   */
+  virtual Event::Dispatcher& dispatcher() PURE;
+
+  /**
+   * @return RandomGenerator& the random generator for the server.
+   */
+  virtual Envoy::Runtime::RandomGenerator& random() PURE;
+
+  /**
+   * @return the server-wide stats store.
+   */
+  virtual Stats::Store& stats() PURE;
 };
 
 class TransportSocketConfigFactory {

diff --git a/include/envoy/ssl/context_config.h b/include/envoy/ssl/context_config.h
@@ -127,6 +127,8 @@ class ClientContextConfig : public virtual ContextConfig {
   virtual bool allowRenegotiation() const PURE;
 };
 
+typedef std::unique_ptr<ClientContextConfig> ClientContextConfigPtr;
+
 class ServerContextConfig : public virtual ContextConfig {
 public:
   struct SessionTicketKey {
@@ -148,5 +150,7 @@ class ServerContextConfig : public virtual ContextConfig {
   virtual const std::vector<SessionTicketKey>& sessionTicketKeys() const PURE;
 };
 
+typedef std::unique_ptr<ServerContextConfig> ServerContextConfigPtr;
+
 } // namespace Ssl
 } // namespace Envoy
diff --git a/include/envoy/tcp/conn_pool.h b/include/envoy/tcp/conn_pool.h
@@ -58,8 +58,7 @@ class UpstreamCallbacks : public Network::ConnectionCallbacks {
 };
 
 /*
- * ConnectionData wraps a ClientConnection allocated to a caller. Open ClientConnections are
- * released back to the pool for re-use when their containing ConnectionData is destroyed.
+ * ConnectionData wraps a ClientConnection allocated to a caller.
  */
 class ConnectionData {
 public:
@@ -77,9 +76,13 @@ class ConnectionData {
    * @param callback the UpstreamCallbacks to invoke for upstream data
    */
   virtual void addUpstreamCallbacks(ConnectionPool::UpstreamCallbacks& callback) PURE;
-};
 
-typedef std::unique_ptr<ConnectionData> ConnectionDataPtr;
+  /**
+   * Release the connection after use. The connection should be closed first only if it is
+   * not viable for future use.
+   */
+  virtual void release() PURE;
+};
 
 /**
  * Pool callbacks invoked in the context of a newConnection() call, either synchronously or
@@ -99,17 +102,14 @@ class Callbacks {
                              Upstream::HostDescriptionConstSharedPtr host) PURE;
 
   /**
-   * Called when a connection is available to process a request/response. Connections may be
-   * released back to the pool for re-use by resetting the ConnectionDataPtr. If the connection is
-   * no longer viable for reuse (e.g. due to some kind of protocol error), the underlying
-   * ClientConnection should be closed to prevent its reuse.
-   *
+   * Called when a connection is available to process a request/response. Recipients of connections
+   * must release the connection after use. They should only close the underlying ClientConnection
+   * if it is no longer viable for future requests.
    * @param conn supplies the connection data to use.
    * @param host supplies the description of the host that will carry the request. For logical
    *             connection pools the description may be different each time this is called.
    */
-  virtual void onPoolReady(ConnectionDataPtr&& conn,
-                           Upstream::HostDescriptionConstSharedPtr host) PURE;
+  virtual void onPoolReady(ConnectionData& conn, Upstream::HostDescriptionConstSharedPtr host) PURE;
 };
 
 /**

diff --git a/source/common/common/assert.h b/source/common/common/assert.h
@@ -29,11 +29,19 @@ namespace Envoy {
   } while (false)
 
 #ifndef NDEBUG
-#define ASSERT(X) RELEASE_ASSERT(X, "")
+#define _ASSERT_ORIGINAL(X) RELEASE_ASSERT(X, "")
+#define _ASSERT_VERBOSE(X, Y) RELEASE_ASSERT(X, Y)
+#define _ASSERT_SELECTOR(_1, _2, ASSERT_MACRO, ...) ASSERT_MACRO
+
+// If ASSERT is called with one argument, the ASSERT_SELECTOR will return
+// _ASSERT_ORIGINAL and this will call _ASSERT_ORIGINAL(__VA_ARGS__).
+// If ASSERT is called with two arguments, ASSERT_SELECTOR will return
+// _ASSERT_VERBOSE, and this will call _ASSERT_VERBOSE,(__VA_ARGS__)
+#define ASSERT(...) _ASSERT_SELECTOR(__VA_ARGS__, _ASSERT_VERBOSE, _ASSERT_ORIGINAL)(__VA_ARGS__)
 #else
 // This non-implementation ensures that its argument is a valid expression that can be statically
 // casted to a bool, but the expression is never evaluated and will be compiled away.
-#define ASSERT(X)                                                                                  \
+#define ASSERT(X, ...)                                                                             \
   do {                                                                                             \
     constexpr bool __assert_dummy_variable = false && static_cast<bool>(X);                        \
     (void)__assert_dummy_variable;                                                                 \

diff --git a/source/common/config/http_subscription_impl.h b/source/common/config/http_subscription_impl.h
@@ -33,8 +33,10 @@ class HttpSubscriptionImpl : public Http::RestApiFetcher,
   HttpSubscriptionImpl(const envoy::api::v2::core::Node& node, Upstream::ClusterManager& cm,
                        const std::string& remote_cluster_name, Event::Dispatcher& dispatcher,
                        Runtime::RandomGenerator& random, std::chrono::milliseconds refresh_interval,
+                       std::chrono::milliseconds request_timeout,
                        const Protobuf::MethodDescriptor& service_method, SubscriptionStats stats)
-      : Http::RestApiFetcher(cm, remote_cluster_name, dispatcher, random, refresh_interval),
+      : Http::RestApiFetcher(cm, remote_cluster_name, dispatcher, random, refresh_interval,
+                             request_timeout),
         stats_(stats) {
     request_.mutable_node()->CopyFrom(node);
     ASSERT(service_method.options().HasExtension(google::api::http));

diff --git a/source/common/config/subscription_factory.h b/source/common/config/subscription_factory.h
@@ -60,6 +60,7 @@ class SubscriptionFactory {
         result.reset(new HttpSubscriptionImpl<ResourceType>(
             node, cm, api_config_source.cluster_names()[0], dispatcher, random,
             Utility::apiConfigSourceRefreshDelay(api_config_source),
+            Utility::apiConfigSourceRequestTimeout(api_config_source),
             *Protobuf::DescriptorPool::generated_pool()->FindMethodByName(rest_method), stats));
         break;
       case envoy::api::v2::core::ApiConfigSource::GRPC: {

diff --git a/source/common/config/utility.cc b/source/common/config/utility.cc
@@ -159,6 +159,12 @@ std::chrono::milliseconds Utility::apiConfigSourceRefreshDelay(
       DurationUtil::durationToMilliseconds(api_config_source.refresh_delay()));
 }
 
+std::chrono::milliseconds Utility::apiConfigSourceRequestTimeout(
+    const envoy::api::v2::core::ApiConfigSource& api_config_source) {
+  return std::chrono::milliseconds(
+      PROTOBUF_GET_MS_OR_DEFAULT(api_config_source, request_timeout, 1000));
+}
+
 void Utility::translateEdsConfig(const Json::Object& json_config,
                                  envoy::api::v2::core::ConfigSource& eds_config) {
   translateApiConfigSource(json_config.getObject("cluster")->getString("name"),

diff --git a/source/common/config/utility.h b/source/common/config/utility.h
@@ -74,6 +74,14 @@ class Utility {
   static std::chrono::milliseconds
   apiConfigSourceRefreshDelay(const envoy::api::v2::core::ApiConfigSource& api_config_source);
 
+  /**
+   * Extract request_timeout as a std::chrono::milliseconds from
+   * envoy::api::v2::core::ApiConfigSource. If request_timeout isn't set in the config source, a
+   * default value of 1s will be returned.
+   */
+  static std::chrono::milliseconds
+  apiConfigSourceRequestTimeout(const envoy::api::v2::core::ApiConfigSource& api_config_source);
+
   /**
    * Populate an envoy::api::v2::core::ApiConfigSource.
    * @param cluster supplies the cluster name for the ApiConfigSource.

diff --git a/source/common/http/BUILD b/source/common/http/BUILD
@@ -215,6 +215,7 @@ envoy_cc_library(
         "//include/envoy/runtime:runtime_interface",
         "//include/envoy/upstream:cluster_manager_interface",
         "//source/common/common:enum_to_int",
+        "//source/common/config:utility_lib",
     ],
 )
 

diff --git a/source/common/http/rest_api_fetcher.cc b/source/common/http/rest_api_fetcher.cc
@@ -5,17 +5,33 @@
 #include <string>
 
 #include "common/common/enum_to_int.h"
+#include "common/config/utility.h"
 #include "common/http/message_impl.h"
 #include "common/http/utility.h"
 
 namespace Envoy {
 namespace Http {
 
+RestApiFetcher::RestApiFetcher(Upstream::ClusterManager& cm,
+                               const envoy::api::v2::core::ApiConfigSource& api_config_source,
+                               Event::Dispatcher& dispatcher, Runtime::RandomGenerator& random)
+    : RestApiFetcher(cm, api_config_source.cluster_names()[0], dispatcher, random,
+                     Config::Utility::apiConfigSourceRefreshDelay(api_config_source),
+                     Config::Utility::apiConfigSourceRequestTimeout(api_config_source)) {
+  UNREFERENCED_PARAMETER(api_config_source);
+  // The ApiType must be REST_LEGACY for xDS implementations that call this constructor.
+  ASSERT(api_config_source.api_type() == envoy::api::v2::core::ApiConfigSource::REST_LEGACY);
+  // TODO(htuch): Add support for multiple clusters, #1170.
+  ASSERT(api_config_source.cluster_names().size() == 1);
+  ASSERT(api_config_source.has_refresh_delay());
+}
+
 RestApiFetcher::RestApiFetcher(Upstream::ClusterManager& cm, const std::string& remote_cluster_name,
                                Event::Dispatcher& dispatcher, Runtime::RandomGenerator& random,
-                               std::chrono::milliseconds refresh_interval)
+                               std::chrono::milliseconds refresh_interval,
+                               std::chrono::milliseconds request_timeout)
     : remote_cluster_name_(remote_cluster_name), cm_(cm), random_(random),
-      refresh_interval_(refresh_interval),
+      refresh_interval_(refresh_interval), request_timeout_(request_timeout),
       refresh_timer_(dispatcher.createTimer([this]() -> void { refresh(); })) {}
 
 RestApiFetcher::~RestApiFetcher() {
@@ -51,10 +67,9 @@ void RestApiFetcher::refresh() {
   MessagePtr message(new RequestMessageImpl());
   createRequest(*message);
   message->headers().insertHost().value(remote_cluster_name_);
-  active_request_ =
-      cm_.httpAsyncClientForCluster(remote_cluster_name_)
-          .send(std::move(message), *this,
-                absl::optional<std::chrono::milliseconds>(std::chrono::milliseconds(1000)));
+  active_request_ = cm_.httpAsyncClientForCluster(remote_cluster_name_)
+                        .send(std::move(message), *this,
+                              absl::optional<std::chrono::milliseconds>(request_timeout_));
 }
 
 void RestApiFetcher::requestComplete() {

diff --git a/source/common/http/rest_api_fetcher.h b/source/common/http/rest_api_fetcher.h
@@ -16,9 +16,13 @@ namespace Http {
  */
 class RestApiFetcher : public Http::AsyncClient::Callbacks {
 protected:
+  RestApiFetcher(Upstream::ClusterManager& cm,
+                 const envoy::api::v2::core::ApiConfigSource& api_config_source,
+                 Event::Dispatcher& dispatcher, Runtime::RandomGenerator& random);
   RestApiFetcher(Upstream::ClusterManager& cm, const std::string& remote_cluster_name,
                  Event::Dispatcher& dispatcher, Runtime::RandomGenerator& random,
-                 std::chrono::milliseconds refresh_interval);
+                 std::chrono::milliseconds refresh_interval,
+                 std::chrono::milliseconds request_timeout);
   ~RestApiFetcher();
 
   /**
@@ -63,6 +67,7 @@ class RestApiFetcher : public Http::AsyncClient::Callbacks {
 
   Runtime::RandomGenerator& random_;
   const std::chrono::milliseconds refresh_interval_;
+  const std::chrono::milliseconds request_timeout_;
   Event::TimerPtr refresh_timer_;
   Http::AsyncClient::Request* active_request_{};
 };

diff --git a/source/common/http/websocket/ws_handler_impl.cc b/source/common/http/websocket/ws_handler_impl.cc
@@ -131,8 +131,7 @@ void WsHandlerImpl::onConnectionSuccess() {
   // the connection pool. The current approach is a stop gap solution, where
   // we put the onus on the user to tell us if a route (and corresponding upstream)
   // is supposed to allow websocket upgrades or not.
-  Http1::ClientConnectionImpl upstream_http(upstream_conn_data_->connection(),
-                                            http_conn_callbacks_);
+  Http1::ClientConnectionImpl upstream_http(*upstream_connection_, http_conn_callbacks_);
   Http1::RequestStreamEncoderImpl upstream_request = Http1::RequestStreamEncoderImpl(upstream_http);
   upstream_request.encodeHeaders(request_headers_, false);
   ASSERT(state_ == ConnectState::PreConnect);

diff --git a/source/common/network/connection_impl.cc b/source/common/network/connection_impl.cc
@@ -504,7 +504,9 @@ void ConnectionImpl::onWriteReady() {
 }
 
 void ConnectionImpl::setConnectionStats(const ConnectionStats& stats) {
-  ASSERT(!connection_stats_);
+  ASSERT(!connection_stats_,
+         "Two network filters are attempting to set connection stats. This indicates an issue "
+         "with the configured filter chain.");
   connection_stats_.reset(new ConnectionStats(stats));
 }
 

diff --git a/source/common/router/rds_subscription.cc b/source/common/router/rds_subscription.cc
@@ -17,20 +17,10 @@ RdsSubscription::RdsSubscription(
     const envoy::config::filter::network::http_connection_manager::v2::Rds& rds,
     Upstream::ClusterManager& cm, Event::Dispatcher& dispatcher, Runtime::RandomGenerator& random,
     const LocalInfo::LocalInfo& local_info, const Stats::Scope& scope)
-    : RestApiFetcher(cm, rds.config_source().api_config_source().cluster_names()[0], dispatcher,
-                     random,
-                     Envoy::Config::Utility::apiConfigSourceRefreshDelay(
-                         rds.config_source().api_config_source())),
+    : RestApiFetcher(cm, rds.config_source().api_config_source(), dispatcher, random),
       local_info_(local_info), stats_(stats), scope_(scope) {
-  const auto& api_config_source = rds.config_source().api_config_source();
-  UNREFERENCED_PARAMETER(api_config_source);
-  // If we are building an RdsSubscription, the ConfigSource should be REST_LEGACY.
-  ASSERT(api_config_source.api_type() == envoy::api::v2::core::ApiConfigSource::REST_LEGACY);
-  // TODO(htuch): Add support for multiple clusters, #1170.
-  ASSERT(api_config_source.cluster_names().size() == 1);
-  ASSERT(api_config_source.has_refresh_delay());
-  Envoy::Config::Utility::checkClusterAndLocalInfo("rds", api_config_source.cluster_names()[0], cm,
-                                                   local_info);
+  Envoy::Config::Utility::checkClusterAndLocalInfo(
+      "rds", rds.config_source().api_config_source().cluster_names()[0], cm, local_info);
 }
 
 void RdsSubscription::createRequest(Http::Message& request) {