upsteam protocol: add the ability to decide what protocol to use for …

…upstream requests (#697) Description: this PR adds the ability to decide what protocol to use for upstream requests. Note that this will be unnecessary once ALPN works for upstream requests (#502) Risk Level: low Testing: updated unit tests Docs Changes: updated docs Fixes #679 Signed-off-by: Michael Rebello <[email protected]> Co-authored-by: Michael Rebello <[email protected]>
envoyproxy · Feb 21, 2020 · 92f9bf2 · 92f9bf2
1 parent b91d67c
commit 92f9bf2
Show file tree

Hide file tree

Showing 28 changed files with 419 additions and 32 deletions.
diff --git a/docs/root/api/http.rst b/docs/root/api/http.rst
@@ -19,6 +19,7 @@ a previously created :ref:`Envoy instance <api_starting_envoy>`.
 
   val request = RequestBuilder(RequestMethod.POST, "https", "api.envoyproxy.io", "/foo")
     .addRetryPolicy(RetryPolicy(...))
+    .addUpstreamHttpProtocol(UpstreamRequestProtocol.HTTP2)
     .addHeader("x-custom-header", "foobar")
     ...
     .build()
@@ -27,6 +28,7 @@ a previously created :ref:`Envoy instance <api_starting_envoy>`.
 
   let request = RequestBuilder(method: .post, scheme: "https", authority: "api.envoyproxy.io", path: "/foo")
     .addRetryPolicy(RetryPolicy(...))
+    .addUpstreamHttpProtocol(.http2)
     .addHeader(name: "x-custom-header", value: "foobar")
     ...
     .build()

diff --git a/examples/java/hello_world/MainActivity.java b/examples/java/hello_world/MainActivity.java
@@ -74,6 +74,9 @@ protected void onDestroy() {
   }
 
   private void makeRequest() {
+    // Note: this request will use an http/1.1 stream for the upstream request.
+    // The Kotlin example uses h2. This is done on purpose to test both paths in end-to-end tests
+    // in CI.
     Request request =
         new RequestBuilder(RequestMethod.GET, REQUEST_SCHEME, REQUEST_AUTHORITY, REQUEST_PATH)
             .build();

diff --git a/examples/kotlin/hello_world/MainActivity.kt b/examples/kotlin/hello_world/MainActivity.kt
@@ -13,6 +13,7 @@ import io.envoyproxy.envoymobile.Envoy
 import io.envoyproxy.envoymobile.RequestBuilder
 import io.envoyproxy.envoymobile.RequestMethod
 import io.envoyproxy.envoymobile.ResponseHandler
+import io.envoyproxy.envoymobile.UpstreamHttpProtocol
 
 import io.envoyproxy.envoymobile.shared.Failure
 import io.envoyproxy.envoymobile.shared.ResponseRecyclerViewAdapter
@@ -73,7 +74,11 @@ class MainActivity : Activity() {
   }
 
   private fun makeRequest() {
+    // Note: this request will use an h2 stream for the upstream request.
+    // The Java example uses http/1.1. This is done on purpose to test both paths in end-to-end
+    // tests in CI.
     val request = RequestBuilder(RequestMethod.GET, REQUEST_SCHEME, REQUEST_AUTHORITY, REQUEST_PATH)
+        .addUpstreamHttpProtocol(UpstreamHttpProtocol.HTTP2)
         .build()
     val responseHeaders = HashMap<String, List<String>>()
     val responseStatus = AtomicInteger()

diff --git a/examples/objective-c/hello_world/ViewController.m b/examples/objective-c/hello_world/ViewController.m
@@ -66,6 +66,9 @@ - (void)performRequest {
   NSLog(@"Starting request to '%@'", _REQUEST_PATH);
 
   int requestID = self.requestCount;
+  // Note: this request will use an http/1.1 stream for the upstream request.
+  // The Swift example uses h2. This is done on purpose to test both paths in end-to-end tests
+  // in CI.
   RequestBuilder *builder = [[RequestBuilder alloc] initWithMethod:RequestMethodGet
                                                             scheme:_REQUEST_SCHEME
                                                          authority:_REQUEST_AUTHORITY

diff --git a/examples/swift/hello_world/ViewController.swift b/examples/swift/hello_world/ViewController.swift
@@ -47,9 +47,14 @@ final class ViewController: UITableViewController {
     NSLog("Starting request to '\(kRequestPath)'")
 
     let requestID = self.requestCount
+    // Note: this request will use an h2 stream for the upstream request.
+    // The Objective-C example uses http/1.1. This is done on purpose to test both paths in
+    // end-to-end tests in CI.
     let request = RequestBuilder(method: .get, scheme: kRequestScheme,
                                  authority: kRequestAuthority,
-                                 path: kRequestPath).build()
+                                 path: kRequestPath)
+        .addUpstreamHttpProtocol(.http2)
+        .build()
     let handler = ResponseHandler()
       .onHeaders { [weak self] headers, statusCode, _ in
         NSLog("Response status (\(requestID)): \(statusCode)\n\(headers)")

diff --git a/library/common/config_template.cc b/library/common/config_template.cc
@@ -96,6 +96,48 @@ const char* config_template = R"(
           dns_refresh_rate: {{ dns_refresh_rate_seconds }}s
     transport_socket: *base_transport_socket
     upstream_connection_options: *upstream_opts
+  - name: base_h2
+    http2_protocol_options: {}
+    connect_timeout: {{ connect_timeout_seconds }}s
+    lb_policy: CLUSTER_PROVIDED
+    cluster_type:
+      name: envoy.clusters.dynamic_forward_proxy
+      typed_config:
+        "@type": type.googleapis.com/envoy.extensions.clusters.dynamic_forward_proxy.v3.ClusterConfig
+        dns_cache_config:
+          name: dynamic_forward_proxy_cache_config
+          dns_lookup_family: AUTO
+          dns_refresh_rate: {{ dns_refresh_rate_seconds }}s
+    transport_socket: *base_transport_socket
+    upstream_connection_options: *upstream_opts
+  - name: base_wlan_h2
+    http2_protocol_options: {}
+    connect_timeout: {{ connect_timeout_seconds }}s
+    lb_policy: CLUSTER_PROVIDED
+    cluster_type:
+      name: envoy.clusters.dynamic_forward_proxy
+      typed_config:
+        "@type": type.googleapis.com/envoy.extensions.clusters.dynamic_forward_proxy.v3.ClusterConfig
+        dns_cache_config:
+          name: dynamic_forward_proxy_cache_config
+          dns_lookup_family: AUTO
+          dns_refresh_rate: {{ dns_refresh_rate_seconds }}s
+    transport_socket: *base_transport_socket
+    upstream_connection_options: *upstream_opts
+  - name: base_wwan_h2
+    http2_protocol_options: {}
+    connect_timeout: {{ connect_timeout_seconds }}s
+    lb_policy: CLUSTER_PROVIDED
+    cluster_type:
+      name: envoy.clusters.dynamic_forward_proxy
+      typed_config:
+        "@type": type.googleapis.com/envoy.extensions.clusters.dynamic_forward_proxy.v3.ClusterConfig
+        dns_cache_config:
+          name: dynamic_forward_proxy_cache_config
+          dns_lookup_family: AUTO
+          dns_refresh_rate: {{ dns_refresh_rate_seconds }}s
+    transport_socket: *base_transport_socket
+    upstream_connection_options: *upstream_opts
   - name: stats
     connect_timeout: {{ connect_timeout_seconds }}s
     dns_refresh_rate: {{ dns_refresh_rate_seconds }}s

diff --git a/library/common/http/dispatcher.cc b/library/common/http/dispatcher.cc
@@ -321,7 +321,7 @@ envoy_status_t Dispatcher::sendHeaders(envoy_stream_t stream, envoy_headers head
       RequestHeaderMapPtr internal_headers = Utility::toRequestHeaders(headers);
       ENVOY_LOG(debug, "[S{}] request headers for stream (end_stream={}):\n{}", stream, end_stream,
                 *internal_headers);
-      attachPreferredNetwork(*internal_headers);
+      setDestinationCluster(*internal_headers);
       direct_stream->request_decoder_->decodeHeaders(std::move(internal_headers), end_stream);
       direct_stream->closeLocal(end_stream);
     }
@@ -478,19 +478,41 @@ const LowerCaseString ClusterHeader{"x-envoy-mobile-cluster"};
 const std::string BaseCluster = "base";
 const std::string BaseWlanCluster = "base_wlan";
 const std::string BaseWwanCluster = "base_wwan";
+const LowerCaseString H2UpstreamHeader{"x-envoy-mobile-upstream-protocol"};
+const std::string H2Suffix = "_h2";
+const std::string BaseClusterH2 = BaseCluster + H2Suffix;
+const std::string BaseWlanClusterH2 = BaseWlanCluster + H2Suffix;
+const std::string BaseWwanClusterH2 = BaseWwanCluster + H2Suffix;
 } // namespace
 
-void Dispatcher::attachPreferredNetwork(HeaderMap& headers) {
+void Dispatcher::setDestinationCluster(HeaderMap& headers) {
+
+  // Determine upstream protocol. Use http2 if selected for explicitly, otherwise (any other value,
+  // absence of value) select http1.
+  // TODO(junr03): once http3 is available this would probably benefit from some sort of struct that
+  // maps to appropriate cluster names. However, with only two options (http1/2) this suffices.
+  bool use_h2_cluster{};
+  const HeaderEntry* entry = headers.get(H2UpstreamHeader);
+  if (entry) {
+    if (entry->value() == "http2") {
+      use_h2_cluster = true;
+    } else {
+      ASSERT(entry->value() == "http1",
+             fmt::format("using unsupported protocol version {}", entry->value().getStringView()));
+    }
+    headers.remove(H2UpstreamHeader);
+  }
+
   switch (preferred_network_.load()) {
   case ENVOY_NET_WLAN:
-    headers.addReference(ClusterHeader, BaseWlanCluster);
+    headers.addReference(ClusterHeader, use_h2_cluster ? BaseWlanClusterH2 : BaseWlanCluster);
     break;
   case ENVOY_NET_WWAN:
-    headers.addReference(ClusterHeader, BaseWwanCluster);
+    headers.addReference(ClusterHeader, use_h2_cluster ? BaseWwanClusterH2 : BaseWwanCluster);
     break;
   case ENVOY_NET_GENERIC:
   default:
-    headers.addReference(ClusterHeader, BaseCluster);
+    headers.addReference(ClusterHeader, use_h2_cluster ? BaseClusterH2 : BaseCluster);
   }
 }
 

diff --git a/library/common/http/dispatcher.h b/library/common/http/dispatcher.h
@@ -195,7 +195,7 @@ class Dispatcher : public Logger::Loggable<Logger::Id::http> {
   void post(Event::PostCb callback);
   DirectStreamSharedPtr getStream(envoy_stream_t stream_handle);
   void cleanup(envoy_stream_t stream_handle);
-  void attachPreferredNetwork(HeaderMap& headers);
+  void setDestinationCluster(HeaderMap& headers);
 
   Thread::MutexBasicLockable ready_lock_;
   std::list<Event::PostCb> init_queue_ GUARDED_BY(ready_lock_);

diff --git a/library/kotlin/src/io/envoyproxy/envoymobile/BUILD b/library/kotlin/src/io/envoyproxy/envoymobile/BUILD
@@ -52,6 +52,7 @@ envoy_mobile_kt_library(
         "RetryPolicy.kt",
         "RetryPolicyMapper.kt",
         "StreamEmitter.kt",
+        "UpstreamHttpProtocol.kt",
     ],
     visibility = ["//visibility:public"],
     deps = [

diff --git a/library/kotlin/src/io/envoyproxy/envoymobile/GRPCRequestBuilder.kt b/library/kotlin/src/io/envoyproxy/envoymobile/GRPCRequestBuilder.kt
@@ -15,7 +15,7 @@ class GRPCRequestBuilder(
       path = path)
 
   init {
-    underlyingBuilder.addHeader("content-type", "application/grpc")
+    underlyingBuilder.addHeader("content-type", "application/grpc").addUpstreamHttpProtocol(UpstreamHttpProtocol.HTTP2)
   }
 
   /**

diff --git a/library/kotlin/src/io/envoyproxy/envoymobile/Request.kt b/library/kotlin/src/io/envoyproxy/envoymobile/Request.kt
@@ -14,7 +14,8 @@ data class Request internal constructor(
     val authority: String,
     val path: String,
     val headers: Map<String, List<String>>,
-    val retryPolicy: RetryPolicy?
+    val retryPolicy: RetryPolicy?,
+    val upstreamHttpProtocol: UpstreamHttpProtocol?
 ) {
 
   /**
@@ -27,5 +28,6 @@ data class Request internal constructor(
     return RequestBuilder(method, scheme, authority, path)
         .setHeaders(headers)
         .addRetryPolicy(retryPolicy)
+        .addUpstreamHttpProtocol(upstreamHttpProtocol)
   }
 }
diff --git a/library/kotlin/src/io/envoyproxy/envoymobile/RequestBuilder.kt b/library/kotlin/src/io/envoyproxy/envoymobile/RequestBuilder.kt
@@ -22,16 +22,8 @@ class RequestBuilder(
   // Retry policy to use for this request.
   private var retryPolicy: RetryPolicy? = null
 
-  /**
-   * Add a retry policy to use for this request.
-   *
-   * @param retryPolicy the {@link io.envoyproxy.envoymobile.RetryPolicy} for this request.
-   * @return this builder.
-   */
-  fun addRetryPolicy(retryPolicy: RetryPolicy?): RequestBuilder {
-    this.retryPolicy = retryPolicy
-    return this
-  }
+  // The protocol version to use for upstream requests.
+  private var upstreamHttpProtocol: UpstreamHttpProtocol? = null
 
   /**
    * Append a value to the header key.
@@ -77,6 +69,28 @@ class RequestBuilder(
     return this
   }
 
+  /**
+   * Add a retry policy to use for this request.
+   *
+   * @param retryPolicy the {@link io.envoyproxy.envoymobile.RetryPolicy} for this request.
+   * @return this builder.
+   */
+  fun addRetryPolicy(retryPolicy: RetryPolicy?): RequestBuilder {
+    this.retryPolicy = retryPolicy
+    return this
+  }
+
+  /**
+   * Add an HTTP protocol hint for this request.
+   *
+   * @param upstreamHttpProtocol the {@link io.envoyproxy.envoymobile.UpstreamHttpProtocol} for this request.
+   * @return this builder.
+   */
+  fun addUpstreamHttpProtocol(upstreamHttpProtocol: UpstreamHttpProtocol?): RequestBuilder {
+    this.upstreamHttpProtocol = upstreamHttpProtocol
+    return this
+  }
+
   /**
    * Creates the {@link io.envoyproxy.envoymobile.Request} object using the data set in the builder.
    *
@@ -89,7 +103,8 @@ class RequestBuilder(
         authority,
         path,
         headers,
-        retryPolicy
+        retryPolicy,
+        upstreamHttpProtocol
     )
   }
 

diff --git a/library/kotlin/src/io/envoyproxy/envoymobile/RequestMapper.kt b/library/kotlin/src/io/envoyproxy/envoymobile/RequestMapper.kt
@@ -4,13 +4,17 @@ internal fun Request.outboundHeaders(): Map<String, List<String>> {
   val retryPolicyHeaders = retryPolicy?.outboundHeaders() ?: emptyMap()
 
   val result = mutableMapOf<String, List<String>>()
-  result.putAll(headers.filter { entry -> !entry.key.startsWith(":") })
+  result.putAll(headers.filter { entry -> !entry.key.startsWith(":") && !entry.key.startsWith("x-envoy-mobile")})
   result.putAll(retryPolicyHeaders)
   result[":method"] = listOf(method.stringValue())
   result[":scheme"] = listOf(scheme)
   result[":authority"] = listOf(authority)
   result[":path"] = listOf(path)
 
+  if (upstreamHttpProtocol != null) {
+    result["x-envoy-mobile-upstream-protocol"] = listOf(upstreamHttpProtocol.stringValue)
+  }
+
   return result
 }
 

diff --git a/library/kotlin/src/io/envoyproxy/envoymobile/UpstreamHttpProtocol.kt b/library/kotlin/src/io/envoyproxy/envoymobile/UpstreamHttpProtocol.kt
@@ -0,0 +1,9 @@
+package io.envoyproxy.envoymobile
+
+/**
+ * Available upstream HTTP protocols.
+ */
+enum class UpstreamHttpProtocol(internal val stringValue: String) {
+  HTTP1("http1"),
+  HTTP2("http2"),
+}
diff --git a/library/kotlin/test/io/envoyproxy/envoymobile/GRPCRequestBuilderTest.kt b/library/kotlin/test/io/envoyproxy/envoymobile/GRPCRequestBuilderTest.kt
@@ -51,4 +51,13 @@ class GRPCRequestBuilderTest {
 
     assertThat(request.headers.containsKey("grpc-timeout")).isFalse()
   }
+
+  @Test
+  fun `h2 header is present`() {
+    val headers = GRPCRequestBuilder("/pb.api.v1.Foo/GetBar", "foo.bar.com", false)
+        .build()
+        .outboundHeaders()
+
+    assertThat(headers["x-envoy-mobile-upstream-protocol"]).containsExactly("http2")
+  }
 }
diff --git a/library/kotlin/test/io/envoyproxy/envoymobile/RequestBuilderTest.kt b/library/kotlin/test/io/envoyproxy/envoymobile/RequestBuilderTest.kt
@@ -24,6 +24,25 @@ class RequestBuilderTest {
     assertThat(request.retryPolicy).isNull()
   }
 
+  @Test
+  fun `adding upstream http protocol should have hint present in request`() {
+
+    val retryPolicy = RetryPolicy(maxRetryCount = 23, retryOn = listOf(RetryRule.STATUS_5XX, RetryRule.CONNECT_FAILURE), perRetryTimeoutMS = 1234)
+    val request = RequestBuilder(method = RequestMethod.POST, scheme = "https", authority = "api.foo.com", path = "foo")
+        .addUpstreamHttpProtocol(UpstreamHttpProtocol.HTTP2)
+        .build()
+
+    assertThat(request.upstreamHttpProtocol).isEqualTo(UpstreamHttpProtocol.HTTP2)
+  }
+
+  @Test
+  fun `not adding upstream http protocol should have null upstream protocol in request`() {
+    val request = RequestBuilder(method = RequestMethod.POST, scheme = "https", authority = "api.foo.com", path = "foo")
+        .build()
+
+    assertThat(request.upstreamHttpProtocol).isNull()
+  }
+
   @Test
   fun `adding new headers should append to the list of header keys`() {
     val request = RequestBuilder(method = RequestMethod.POST, scheme = "https", authority = "api.foo.com", path = "foo")
-Original file line number
+Diff line change
@@ Expand Up / @@ -15,7 +15,7 @@ class GRPCRequestBuilder( @@
           path = path)
       init {
-        underlyingBuilder.addHeader("content-type", "application/grpc")
+        underlyingBuilder.addHeader("content-type", "application/grpc").addUpstreamHttpProtocol(UpstreamHttpProtocol.HTTP2)
       }
       /**
@@ Expand Down @@