Fix outbound HTTP/1 requests not using Destinations (#555)

Commit 569d6939a7 introduced a regression that caused the proxy to stop using the Destination service for outbound HTTP/1 requests with no authority in the request URI but a valid authority in the `Host:` header. 

The bug is due to some code in `Outbound::recognize` which assumed that a request had already been passed through `normalize_our_view_of_uri`. This was valid at one point while I was writing #492, as URIs were normalized prior to `recognize` and a request `Extension` was used to mark that they had been rewritten, and the host header and request URI could be assumed to be in agreement, but after merging #514 into the dev branch for #492, this behaviour changed and I forgot to update the logic in `recognize`.

I've fixed the issue by adding the logic for routing on `Host:` headers back into `Outbound::recognize`.

@seanmonstar added a test in `discovery.rs`, `outbound_http1_asks_controller_about_host`, which should exercise this case. I've added a couple more unit tests in that file to try and ensure we cover more of the different cases that can occur here.

Fixes #552

proxy/src/outbound.rs

@@ -20,6 +20,7 @@ use control::discovery::Bind as BindTrait;
 use ctx;
 use fully_qualified_authority::{FullyQualifiedAuthority, NamedAddress};
 use timeout::Timeout;
+use transparency::h1;
 
 type BindProtocol<B> = bind::BindProtocol<Arc<ctx::Proxy>, B>;
 
@@ -87,12 +88,20 @@ where
     fn recognize(&self, req: &Self::Request) -> Option<Reuse<Self::Key>> {
         let proto = bind::Protocol::detect(req);
 
-        let local = req.uri().authority_part().map(|authority| {
+        // The request URI and Host: header have not yet been normalized
-            FullyQualifiedAuthority::normalize(
+        // by `NormalizeUri`, as we need to know whether the request will
-                authority,
+        // be routed by Host/authority or by SO_ORIGINAL_DST, in order to
-                &self.default_namespace)
+        // determine whether the service is reusable.
-
+        let local = req.uri().authority_part()
-        });
+            .cloned()
+        // Therefore, we need to check the host header as well as the URI
+        // for a valid authority, before we fall back to SO_ORIGINAL_DST.
+            .or_else(|| h1::authority_from_host(req))
+            .map(|authority| {
+                FullyQualifiedAuthority::normalize(
+                    &authority,
+                    &self.default_namespace)
+            });
 
         // If we can't fully qualify the authority as a local service,
         // and there is no original dst, then we have nothing! In that

proxy/src/transparency/h1.rs

@@ -46,7 +46,7 @@ pub fn normalize_our_view_of_uri<B>(req: &mut http::Request<B>) {
 
 /// Returns an Authority from a request's Host header.
 pub fn authority_from_host<B>(req: &http::Request<B>) -> Option<Authority> {
-    req.headers().get(HOST).cloned()
+    req.headers().get(HOST)
         .and_then(|host| {
              host.to_str().ok()
                 .and_then(|s| {

proxy/tests/discovery.rs

@@ -18,6 +18,21 @@ fn outbound_asks_controller_api() {
     assert_eq!(client.get("/bye"), "bye");
 }
 
+#[test]
+fn outbound_asks_controller_about_authority() {
+    let _ = env_logger::try_init();
+
+    let srv = server::new().route("/", "hello").route("/bye", "bye").run();
+    let ctrl = controller::new()
+        .destination("disco.test.svc.cluster.local", srv.addr)
+        .run();
+    let proxy = proxy::new().controller(ctrl).run();
+    let client = client::new(proxy.outbound, "disco.test.svc.cluster.local");
+
+    assert_eq!(client.get("/"), "hello");
+    assert_eq!(client.get("/bye"), "bye");
+}
+
 #[test]
 fn outbound_reconnects_if_controller_stream_ends() {
     let _ = env_logger::try_init();
@@ -33,6 +48,50 @@ fn outbound_reconnects_if_controller_stream_ends() {
     assert_eq!(client.get("/recon"), "nect");
 }
 
+#[test]
+fn outbound_http1_asks_controller_about_host() {
+    let _ = env_logger::try_init();
+
+    let srv = server::http1()
+        .route("/", "hello")
+        .route("/bye", "bye")
+        .run();
+    let ctrl = controller::new()
+        .destination("disco.test.svc.cluster.local", srv.addr)
+        .run();
+    let proxy = proxy::new()
+        .controller(ctrl)
+        // don't set srv as outbound(), so that SO_ORIGINAL_DST isn't
+        // used as a backup
+        .run();
+    let client = client::http1(proxy.outbound, "disco.test.svc.cluster.local");
+
+    assert_eq!(client.get("/"), "hello");
+    assert_eq!(client.get("/bye"), "bye");
+}
+
+#[test]
+fn outbound_http1_asks_controller_api() {
+    let _ = env_logger::try_init();
+
+    let srv = server::http1()
+        .route("/", "hello")
+        .route("/bye", "bye")
+        .run();
+    let ctrl = controller::new()
+        .destination("disco.test.svc.cluster.local", srv.addr)
+        .run();
+    let proxy = proxy::new()
+        .controller(ctrl)
+        // don't set srv as outbound(), so that SO_ORIGINAL_DST isn't
+        // used as a backup
+        .run();
+    let client = client::http1(proxy.outbound, "disco.test.svc.cluster.local");
+
+    assert_eq!(client.get("/"), "hello");
+    assert_eq!(client.get("/bye"), "bye");
+}
+
 #[test]
 fn outbound_updates_newer_services() {
     let _ = env_logger::try_init();