diff --git a/proxy/src/outbound.rs b/proxy/src/outbound.rs index 334601df4..b4269b32b 100644 --- a/proxy/src/outbound.rs +++ b/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 = bind::BindProtocol, B>; @@ -87,12 +88,20 @@ where fn recognize(&self, req: &Self::Request) -> Option> { let proto = bind::Protocol::detect(req); - let local = req.uri().authority_part().map(|authority| { - FullyQualifiedAuthority::normalize( - authority, - &self.default_namespace) - - }); + // The request URI and Host: header have not yet been normalized + // by `NormalizeUri`, as we need to know whether the request will + // be routed by Host/authority or by SO_ORIGINAL_DST, in order to + // 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 diff --git a/proxy/src/transparency/h1.rs b/proxy/src/transparency/h1.rs index f0b869d8f..f189e4ce0 100644 --- a/proxy/src/transparency/h1.rs +++ b/proxy/src/transparency/h1.rs @@ -46,7 +46,7 @@ pub fn normalize_our_view_of_uri(req: &mut http::Request) { /// Returns an Authority from a request's Host header. pub fn authority_from_host(req: &http::Request) -> Option { - req.headers().get(HOST).cloned() + req.headers().get(HOST) .and_then(|host| { host.to_str().ok() .and_then(|s| { diff --git a/proxy/tests/discovery.rs b/proxy/tests/discovery.rs index cad094499..2888b261f 100644 --- a/proxy/tests/discovery.rs +++ b/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();