From fb67c30654412125ed1c778cfb245e6d3f84fa7f Mon Sep 17 00:00:00 2001 From: Marc Brinkmann Date: Sat, 30 Dec 2023 03:54:44 +0100 Subject: [PATCH 1/6] Add `RoutingTable` type to factor out routing code --- src/reverse_proxy.rs | 71 +++++++++++++++++++++++++++++++++++++------- 1 file changed, 61 insertions(+), 10 deletions(-) diff --git a/src/reverse_proxy.rs b/src/reverse_proxy.rs index 319c720..7e929f2 100644 --- a/src/reverse_proxy.rs +++ b/src/reverse_proxy.rs @@ -20,15 +20,68 @@ use tracing::{trace, warn}; pub(crate) struct ReverseProxy { client: reqwest::Client, - containers: RwLock>, + routing_table: RwLock, } -#[derive(Debug)] +#[derive(Clone, Debug)] pub(crate) struct PublishedContainer { host_addr: SocketAddr, image_location: ImageLocation, } +#[derive(Debug, Default)] +pub(crate) struct RoutingTable { + path_maps: HashMap, + domain_maps: HashMap, +} + +impl RoutingTable { + #[inline(always)] + fn get_path_route(&self, image_location: &ImageLocation) -> Option<&PublishedContainer> { + self.path_maps.get(image_location) + } +} + +#[derive(Debug, Hash, Eq, PartialEq)] +struct Domain(String); + +impl Domain { + fn new(raw: &str) -> Option { + let domain_name = raw.to_lowercase(); + if !domain_name.contains('.') { + return None; + } + + Some(Self(domain_name)) + } +} + +impl PartialEq for Domain { + fn eq(&self, other: &String) -> bool { + other.to_lowercase() == self.0 + } +} + +impl RoutingTable { + fn from_containers(containers: impl IntoIterator) -> Self { + let mut path_maps = HashMap::new(); + let mut domain_maps = HashMap::new(); + + for container in containers { + if let Some(domain) = Domain::new(&container.image_location.repository()) { + domain_maps.insert(domain, container.clone()); + } + + path_maps.insert(container.image_location.clone(), container); + } + + Self { + path_maps, + domain_maps, + } + } +} + #[derive(Debug)] enum AppError { NoSuchContainer, @@ -86,7 +139,7 @@ impl ReverseProxy { pub(crate) fn new() -> Arc { Arc::new(ReverseProxy { client: reqwest::Client::new(), - containers: RwLock::new(HashMap::new()), + routing_table: RwLock::new(Default::default()), }) } @@ -102,12 +155,10 @@ impl ReverseProxy { &self, containers: impl Iterator, ) { - let mut new_mapping = containers - .map(|pc| (pc.image_location.clone(), pc)) - .collect(); + let mut routing_table = RoutingTable::from_containers(containers); - let mut guard = self.containers.write().await; - mem::swap(&mut *guard, &mut new_mapping); + let mut guard = self.routing_table.write().await; + mem::swap(&mut *guard, &mut routing_table); } } @@ -136,10 +187,10 @@ async fn reverse_proxy( // TODO: Return better error (404?). let dest_addr = rp - .containers + .routing_table .read() .await - .get(&image_location) + .get_path_route(&image_location) .ok_or(AppError::NoSuchContainer)? .host_addr; let base_url = format!("http://{dest_addr}"); From bed47373b7eac58d6be1a556191755580e61c759 Mon Sep 17 00:00:00 2001 From: Marc Brinkmann Date: Sat, 30 Dec 2023 16:56:36 +0100 Subject: [PATCH 2/6] Write new routing function that also routes domains --- src/reverse_proxy.rs | 134 ++++++++++++++++++++++++++----------------- 1 file changed, 80 insertions(+), 54 deletions(-) diff --git a/src/reverse_proxy.rs b/src/reverse_proxy.rs index 7e929f2..e22ccf1 100644 --- a/src/reverse_proxy.rs +++ b/src/reverse_proxy.rs @@ -3,21 +3,26 @@ use std::{ fmt::{self, Display}, mem, net::SocketAddr, + str::FromStr, sync::Arc, }; use axum::{ body::Body, extract::{Request, State}, - http::StatusCode, + http::{ + uri::{Authority, Scheme}, + StatusCode, Uri, + }, response::{IntoResponse, Response}, - routing::any, Router, }; use itertools::Itertools; use tokio::sync::RwLock; use tracing::{trace, warn}; +use crate::registry::storage::ImageLocation; + pub(crate) struct ReverseProxy { client: reqwest::Client, routing_table: RwLock, @@ -40,6 +45,11 @@ impl RoutingTable { fn get_path_route(&self, image_location: &ImageLocation) -> Option<&PublishedContainer> { self.path_maps.get(image_location) } + + #[inline(always)] + fn get_domain_route(&self, domain: &Domain) -> Option<&PublishedContainer> { + self.domain_maps.get(domain) + } } #[derive(Debug, Hash, Eq, PartialEq)] @@ -144,11 +154,7 @@ impl ReverseProxy { } pub(crate) fn make_router(self: Arc) -> Router { - Router::new() - .route("/:repository/:image", any(reverse_proxy)) - .route("/:repository/:image/", any(reverse_proxy)) - .route("/:repository/:image/*remainder", any(reverse_proxy)) - .with_state(self) + Router::new().fallback(route_request).with_state(self) } pub(crate) async fn update_containers( @@ -162,63 +168,85 @@ impl ReverseProxy { } } -async fn reverse_proxy( +async fn route_request( State(rp): State>, request: Request, -) -> Result, AppError> { - // Determine rewritten URL. +) -> Result { let req_uri = request.uri(); - let mut segments = req_uri - .path() - .split('/') - .filter(|segment| !segment.is_empty()); - - let image_location = ImageLocation::new( - segments - .next() - .ok_or(AppError::AssertionFailed("repository segment disappeared"))? - .to_owned(), - segments - .next() - .ok_or(AppError::AssertionFailed("image segment disappeared"))? - .to_owned(), - ); + let opt_domain = req_uri.host().and_then(Domain::new); + let routing_table = rp.routing_table.read().await; + + let dest_uri = if let Some(pc) = + opt_domain.and_then(|domain| routing_table.get_domain_route(&domain)) + { + // We only need to swap the protocol and domain and we're good to go. + let mut parts = req_uri.clone().into_parts(); + parts.scheme = Some(Scheme::HTTP); + parts.authority = Some(Authority::from_str(&pc.host_addr.to_string()).map_err(|_| { + anyhow::anyhow!("failed to convert container address into `Authority`") + })?); + Some( + Uri::from_parts(parts) + .map_err(|_| anyhow::anyhow!("did not expect invalid uri parts"))? + .to_string(), + ) + } else { + // Reconstruct image location from path segments, keeping remainder intact. + let mut segments = req_uri + .path() + .split('/') + .filter(|segment| !segment.is_empty()); + + let image_location = ImageLocation::new( + segments + .next() + .ok_or(AppError::AssertionFailed("repository segment disappeared"))? + .to_owned(), + segments + .next() + .ok_or(AppError::AssertionFailed("image segment disappeared"))? + .to_owned(), + ); + + if let Some(pc) = routing_table.get_path_route(&image_location) { + let container_addr = pc.host_addr; + + // Now create the path, format is: '' / repository / image / ... + // Need to skip the first three. + let cleaned_path = segments.join("/"); + + let mut dest_path_and_query = cleaned_path; + + if req_uri.path().ends_with('/') { + dest_path_and_query.push('/'); + } - // TODO: Return better error (404?). - let dest_addr = rp - .routing_table - .read() - .await - .get_path_route(&image_location) - .ok_or(AppError::NoSuchContainer)? - .host_addr; - let base_url = format!("http://{dest_addr}"); - - // Format is: '' / repository / image / ... - // Need to skip the first three. - let cleaned_path = segments.join("/"); - - let mut dest_path_and_query = cleaned_path; - - if req_uri.path().ends_with('/') { - dest_path_and_query.push('/'); - } + if let Some(query) = req_uri.query() { + dest_path_and_query.push('?'); + dest_path_and_query += query; + } - if let Some(query) = req_uri.query() { - dest_path_and_query.push('?'); - dest_path_and_query += query; - } + // Reassemble + Some(format!("http://{container_addr}/{dest_path_and_query}")) + } else { + None + } + }; - let dest_uri = format!("{base_url}/{dest_path_and_query}"); - trace!(%dest_uri, "reverse proxying"); + // Release lock. + drop(routing_table); + + // TODO: Return better error (404?). + let dest = dest_uri.ok_or(AppError::NoSuchContainer)?; + trace!(%dest, "reverse proxying"); // Note: `reqwest` and `axum` currently use different versions of `http` let method = request.method().to_string().parse().map_err(|_| { AppError::AssertionFailed("method http version mismatch workaround failed") })?; - let response = rp.client.request(method, &dest_uri).send().await; + let response = rp.client.request(method, &dest).send().await; match response { Ok(response) => { @@ -238,7 +266,7 @@ async fn reverse_proxy( .map_err(|_| AppError::AssertionFailed("should not fail to construct response"))?) } Err(err) => { - warn!(%err, %dest_uri, "failed request"); + warn!(%err, %dest, "failed request"); Ok(Response::builder() .status(500) .body(Body::empty()) @@ -264,5 +292,3 @@ mod hop_by_hop { ]; } use hop_by_hop::HOP_BY_HOP; - -use crate::registry::storage::ImageLocation; From 1f9b6d7a070724a5d599cfb96852fa728b6c0eeb Mon Sep 17 00:00:00 2001 From: Marc Brinkmann Date: Sat, 30 Dec 2023 18:21:49 +0100 Subject: [PATCH 3/6] Fix shell script issues due to idiotic shell syntax --- quick-test.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/quick-test.sh b/quick-test.sh index 4a750ac..2c57940 100755 --- a/quick-test.sh +++ b/quick-test.sh @@ -2,7 +2,7 @@ export REGISTRY_ADDR=127.0.0.1:3000 -if [ $PODMAN_IS_REMOTE == "true" ]; then +if [ "x$PODMAN_IS_REMOTE" == "xtrue" ]; then export REGISTRY_ADDR=$(dig +short $(hostname)):3000 fi From 56fb86c9aad4ca57ee7dd34518f0eddc93f7a541 Mon Sep 17 00:00:00 2001 From: Marc Brinkmann Date: Sat, 30 Dec 2023 18:22:02 +0100 Subject: [PATCH 4/6] Refactor routing code --- src/reverse_proxy.rs | 126 +++++++++++++++++++++---------------------- 1 file changed, 63 insertions(+), 63 deletions(-) diff --git a/src/reverse_proxy.rs b/src/reverse_proxy.rs index e22ccf1..b814194 100644 --- a/src/reverse_proxy.rs +++ b/src/reverse_proxy.rs @@ -90,6 +90,52 @@ impl RoutingTable { domain_maps, } } + + // TODO: Consider return a `Uri`` instead. + fn get_destination_uri_from_request(&self, request: &Request) -> Option { + let req_uri = request.uri(); + + // First, attempt to match a domain. + let opt_domain = req_uri.host().and_then(Domain::new); + if let Some(pc) = opt_domain.and_then(|domain| self.get_domain_route(&domain)) { + // We only need to swap the protocol and domain and we're good to go. + let mut parts = req_uri.clone().into_parts(); + parts.scheme = Some(Scheme::HTTP); + parts.authority = Some( + Authority::from_str(&pc.host_addr.to_string()) + .expect("SocketAddr should never fail to convert to Authority"), + ); + return Some( + Uri::from_parts(parts) + .expect("should not have invalidated Uri") + .to_string(), + ); + } + + // Matching a domain did not succeed, let's try with a path. + // Reconstruct image location from path segments, keeping remainder intact. + if let Some((image_location, remainder)) = split_path_base_url(req_uri) { + if let Some(pc) = self.get_path_route(&image_location) { + let container_addr = pc.host_addr; + + let mut dest_path_and_query = remainder; + + if req_uri.path().ends_with('/') { + dest_path_and_query.push('/'); + } + + if let Some(query) = req_uri.query() { + dest_path_and_query.push('?'); + dest_path_and_query += query; + } + + // Reassemble + return Some(format!("http://{container_addr}/{dest_path_and_query}")); + } + } + + None + } } #[derive(Debug)] @@ -168,75 +214,29 @@ impl ReverseProxy { } } +fn split_path_base_url(uri: &Uri) -> Option<(ImageLocation, String)> { + // Reconstruct image location from path segments, keeping remainder intact. + let mut segments = uri.path().split('/').filter(|segment| !segment.is_empty()); + + let image_location = + ImageLocation::new(segments.next()?.to_owned(), segments.next()?.to_owned()); + + // Now create the path, format is: '' / repository / image / ... + // Need to skip the first three. + let remainder = segments.join("/"); + + Some((image_location, remainder)) +} + async fn route_request( State(rp): State>, request: Request, ) -> Result { - let req_uri = request.uri(); - - let opt_domain = req_uri.host().and_then(Domain::new); - let routing_table = rp.routing_table.read().await; - - let dest_uri = if let Some(pc) = - opt_domain.and_then(|domain| routing_table.get_domain_route(&domain)) - { - // We only need to swap the protocol and domain and we're good to go. - let mut parts = req_uri.clone().into_parts(); - parts.scheme = Some(Scheme::HTTP); - parts.authority = Some(Authority::from_str(&pc.host_addr.to_string()).map_err(|_| { - anyhow::anyhow!("failed to convert container address into `Authority`") - })?); - Some( - Uri::from_parts(parts) - .map_err(|_| anyhow::anyhow!("did not expect invalid uri parts"))? - .to_string(), - ) - } else { - // Reconstruct image location from path segments, keeping remainder intact. - let mut segments = req_uri - .path() - .split('/') - .filter(|segment| !segment.is_empty()); - - let image_location = ImageLocation::new( - segments - .next() - .ok_or(AppError::AssertionFailed("repository segment disappeared"))? - .to_owned(), - segments - .next() - .ok_or(AppError::AssertionFailed("image segment disappeared"))? - .to_owned(), - ); - - if let Some(pc) = routing_table.get_path_route(&image_location) { - let container_addr = pc.host_addr; - - // Now create the path, format is: '' / repository / image / ... - // Need to skip the first three. - let cleaned_path = segments.join("/"); - - let mut dest_path_and_query = cleaned_path; - - if req_uri.path().ends_with('/') { - dest_path_and_query.push('/'); - } - - if let Some(query) = req_uri.query() { - dest_path_and_query.push('?'); - dest_path_and_query += query; - } - - // Reassemble - Some(format!("http://{container_addr}/{dest_path_and_query}")) - } else { - None - } + let dest_uri = { + let routing_table = rp.routing_table.read().await; + routing_table.get_destination_uri_from_request(&request) }; - // Release lock. - drop(routing_table); - // TODO: Return better error (404?). let dest = dest_uri.ok_or(AppError::NoSuchContainer)?; trace!(%dest, "reverse proxying"); From 8edf8e87b99561fc46b85b2ba3cffe8a2c02215f Mon Sep 17 00:00:00 2001 From: Marc Brinkmann Date: Sat, 30 Dec 2023 19:54:50 +0100 Subject: [PATCH 5/6] Return a `Uri` when parsing reverse proxy request --- src/reverse_proxy.rs | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/src/reverse_proxy.rs b/src/reverse_proxy.rs index b814194..50db4f4 100644 --- a/src/reverse_proxy.rs +++ b/src/reverse_proxy.rs @@ -11,7 +11,7 @@ use axum::{ body::Body, extract::{Request, State}, http::{ - uri::{Authority, Scheme}, + uri::{Authority, Parts, PathAndQuery, Scheme}, StatusCode, Uri, }, response::{IntoResponse, Response}, @@ -92,7 +92,7 @@ impl RoutingTable { } // TODO: Consider return a `Uri`` instead. - fn get_destination_uri_from_request(&self, request: &Request) -> Option { + fn get_destination_uri_from_request(&self, request: &Request) -> Option { let req_uri = request.uri(); // First, attempt to match a domain. @@ -105,11 +105,7 @@ impl RoutingTable { Authority::from_str(&pc.host_addr.to_string()) .expect("SocketAddr should never fail to convert to Authority"), ); - return Some( - Uri::from_parts(parts) - .expect("should not have invalidated Uri") - .to_string(), - ); + return Some(Uri::from_parts(parts).expect("should not have invalidated Uri")); } // Matching a domain did not succeed, let's try with a path. @@ -129,8 +125,12 @@ impl RoutingTable { dest_path_and_query += query; } - // Reassemble - return Some(format!("http://{container_addr}/{dest_path_and_query}")); + let mut parts = Parts::default(); + parts.scheme = Some(Scheme::HTTP); + parts.authority = Some(Authority::from_str(&container_addr.to_string()).unwrap()); + parts.path_and_query = Some(PathAndQuery::from_str(&dest_path_and_query).unwrap()); + + return Some(Uri::from_parts(parts).unwrap()); } } @@ -237,7 +237,6 @@ async fn route_request( routing_table.get_destination_uri_from_request(&request) }; - // TODO: Return better error (404?). let dest = dest_uri.ok_or(AppError::NoSuchContainer)?; trace!(%dest, "reverse proxying"); @@ -246,7 +245,7 @@ async fn route_request( request.method().to_string().parse().map_err(|_| { AppError::AssertionFailed("method http version mismatch workaround failed") })?; - let response = rp.client.request(method, &dest).send().await; + let response = rp.client.request(method, dest.to_string()).send().await; match response { Ok(response) => { From ce0ca4c9f3f7d34f11e07cf877e0d53be6deaaea Mon Sep 17 00:00:00 2001 From: Marc Brinkmann Date: Sat, 30 Dec 2023 22:06:02 +0100 Subject: [PATCH 6/6] Use `Host` header instead of URL to find host --- src/reverse_proxy.rs | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/src/reverse_proxy.rs b/src/reverse_proxy.rs index 50db4f4..8415e3f 100644 --- a/src/reverse_proxy.rs +++ b/src/reverse_proxy.rs @@ -3,7 +3,7 @@ use std::{ fmt::{self, Display}, mem, net::SocketAddr, - str::FromStr, + str::{self, FromStr}, sync::Arc, }; @@ -11,6 +11,7 @@ use axum::{ body::Body, extract::{Request, State}, http::{ + header::HOST, uri::{Authority, Parts, PathAndQuery, Scheme}, StatusCode, Uri, }, @@ -96,7 +97,22 @@ impl RoutingTable { let req_uri = request.uri(); // First, attempt to match a domain. - let opt_domain = req_uri.host().and_then(Domain::new); + let opt_domain = if let Some(host_header) = request + .headers() + .get(HOST) + .and_then(|h| str::from_utf8(h.as_bytes()).ok()) + { + let candidate = if let Some(colon) = host_header.rfind(':') { + &host_header[..colon] + } else { + host_header + }; + + Domain::new(candidate) + } else { + None + }; + if let Some(pc) = opt_domain.and_then(|domain| self.get_domain_route(&domain)) { // We only need to swap the protocol and domain and we're good to go. let mut parts = req_uri.clone().into_parts();