Fixups and tidying

Signed-off-by: itowlson <[email protected]>

crates/loader/src/local.rs

@@ -12,6 +12,7 @@ use spin_locked_app::{
     values::{ValuesMap, ValuesMapBuilder},
 };
 use spin_manifest::schema::v2::{self, AppManifest, KebabId, WasiFilesMount};
+use spin_outbound_networking::SERVICE_CHAINING_DOMAIN_SUFFIX;
 use tokio::sync::Semaphore;
 
 use crate::{cache::Cache, FilesMountStrategy};
@@ -559,8 +560,10 @@ fn is_chaining_host(pattern: &str) -> bool {
     };
 
     match allowed.host() {
-        HostConfig::List(hosts) => hosts.iter().any(|h| h.ends_with(".spin.internal")),
-        HostConfig::AnySubdomain(domain) => domain == ".spin.internal",
+        HostConfig::List(hosts) => hosts
+            .iter()
+            .any(|h| h.ends_with(SERVICE_CHAINING_DOMAIN_SUFFIX)),
+        HostConfig::AnySubdomain(domain) => domain == SERVICE_CHAINING_DOMAIN_SUFFIX,
         _ => false,
     }
 }

crates/locked-app/src/locked.rs

@@ -33,11 +33,11 @@ pub enum MustUnderstand {
     HostRequirements,
 }
 
-/// TODO:
+/// Features or capabilities the application requires the host to support.
 #[derive(Clone, Debug, Serialize, Deserialize)]
 #[serde(rename_all = "snake_case")]
 pub enum HostRequirement {
-    /// TODO:
+    /// The application requires local service chaining.
     LocalServiceChaining,
 }
 
@@ -56,19 +56,19 @@ pub struct LockedApp {
     #[serde(
         default,
         skip_serializing_if = "ValuesMap::is_empty",
-        deserialize_with = "i_cant_even"
+        deserialize_with = "deserialize_host_requirements"
     )]
     pub host_requirements: ValuesMap,
     /// Custom config variables
-    #[serde(default, skip_serializing_if = "Vec::is_empty")]
+    #[serde(default, skip_serializing_if = "LockedMap::is_empty")]
     pub variables: LockedMap<Variable>,
     /// Application triggers
     pub triggers: Vec<LockedTrigger>,
     /// Application components
     pub components: Vec<LockedComponent>,
 }
 
-fn i_cant_even<'de, D>(deserializer: D) -> Result<ValuesMap, D::Error>
+fn deserialize_host_requirements<'de, D>(deserializer: D) -> Result<ValuesMap, D::Error>
 where
     D: serde::Deserializer<'de>,
 {
@@ -84,15 +84,17 @@ where
         where
             A: serde::de::MapAccess<'de>,
         {
+            use serde::de::Error;
+
             let mut hr = ValuesMapBuilder::new();
 
             while let Some(key) = map.next_key::<String>()? {
                 let value: serde_json::Value = map.next_value()?;
                 if value.as_str() == Some(HOST_REQ_OPTIONAL) {
                     continue;
                 }
-                // TODO: something better with errors, but A::Error is not helpful
-                hr.serializable(key, value).unwrap();
+
+                hr.serializable(key, value).map_err(A::Error::custom)?;
             }
 
             Ok(hr.build())

crates/outbound-networking/Cargo.toml

@@ -6,6 +6,7 @@ edition.workspace = true
 
 [dependencies]
 anyhow = "1.0"
+http = "1.0.0"
 ipnet = "2.9.0"
 spin-expressions = { path = "../expressions" } 
 spin-locked-app = { path = "../locked-app" }

crates/outbound-networking/src/lib.rs

@@ -5,6 +5,9 @@ use spin_locked_app::MetadataKey;
 
 pub const ALLOWED_HOSTS_KEY: MetadataKey<Vec<String>> = MetadataKey::new("allowed_outbound_hosts");
 
+pub const SERVICE_CHAINING_DOMAIN: &str = "spin.internal";
+pub const SERVICE_CHAINING_DOMAIN_SUFFIX: &str = ".spin.internal";
+
 /// Checks address against allowed hosts
 ///
 /// Emits several warnings
@@ -433,6 +436,27 @@ impl std::fmt::Display for OutboundUrl {
     }
 }
 
+pub fn is_service_chaining_host(host: &str) -> bool {
+    parse_service_chaining_host(host).is_some()
+}
+
+pub fn parse_service_chaining_target(url: &http::Uri) -> Option<String> {
+    let host = url.authority().map(|a| a.host().trim())?;
+    parse_service_chaining_host(host)
+}
+
+fn parse_service_chaining_host(host: &str) -> Option<String> {
+    let (host, _) = host.rsplit_once(':').unwrap_or((host, ""));
+
+    let (first, rest) = host.split_once('.')?;
+
+    if rest == SERVICE_CHAINING_DOMAIN {
+        Some(first.to_owned())
+    } else {
+        None
+    }
+}
+
 #[cfg(test)]
 mod test {
     impl AllowedHostConfig {

crates/trigger-http/src/handler.rs

@@ -1,6 +1,6 @@
 use std::{net::SocketAddr, str, str::FromStr};
 
-use crate::{Body, HttpExecutor, HttpInstance, HttpTrigger, Store};
+use crate::{Body, ChainedRequestHandler, HttpExecutor, HttpInstance, HttpTrigger, Store};
 use anyhow::bail;
 use anyhow::{anyhow, Context, Result};
 use futures::TryFutureExt;
@@ -26,7 +26,7 @@ pub struct HttpHandlerExecutor;
 impl HttpExecutor for HttpHandlerExecutor {
     async fn execute(
         &self,
-        engine: &Arc<TriggerAppEngine<HttpTrigger>>,
+        engine: Arc<TriggerAppEngine<HttpTrigger>>,
         component_id: &str,
         base: &str,
         raw_route: &str,
@@ -43,7 +43,7 @@ impl HttpExecutor for HttpHandlerExecutor {
             unreachable!()
         };
 
-        set_http_origin_from_request(&mut store, engine, self, &req);
+        set_http_origin_from_request(&mut store, engine.clone(), self, &req);
 
         let resp = match HandlerType::from_exports(instance.exports(&mut store)) {
             Some(HandlerType::Wasi) => {
@@ -347,7 +347,7 @@ impl HandlerType {
 
 fn set_http_origin_from_request(
     store: &mut Store,
-    engine: &Arc<TriggerAppEngine<HttpTrigger>>,
+    engine: Arc<TriggerAppEngine<HttpTrigger>>,
     handler: &HttpHandlerExecutor,
     req: &Request<Body>,
 ) {
@@ -366,9 +366,13 @@ fn set_http_origin_from_request(
                 store.as_mut().data_mut().as_mut().allowed_hosts =
                     outbound_http_data.allowed_hosts.clone();
             }
+
+            let chained_request_handler = ChainedRequestHandler {
+                engine: engine.clone(),
+                executor: handler.clone(),
+            };
             store.as_mut().data_mut().as_mut().origin = Some(origin);
-            store.as_mut().data_mut().as_mut().engine = Some(engine.clone());
-            store.as_mut().data_mut().as_mut().handler = Some(handler.clone())
+            store.as_mut().data_mut().as_mut().chained_handler = Some(chained_request_handler);
         }
     }
 }

crates/trigger-http/src/lib.rs

@@ -32,7 +32,9 @@ use spin_http::{
     config::{HttpExecutorType, HttpTriggerConfig},
     routes::{RoutePattern, Router},
 };
-use spin_outbound_networking::{AllowedHostsConfig, OutboundUrl};
+use spin_outbound_networking::{
+    is_service_chaining_host, parse_service_chaining_target, AllowedHostsConfig, OutboundUrl,
+};
 use spin_trigger::{TriggerAppEngine, TriggerExecutor, TriggerInstancePre};
 use tokio::{
     io::{AsyncRead, AsyncWrite},
@@ -253,7 +255,7 @@ impl HttpTrigger {
                     HttpExecutorType::Http => {
                         HttpHandlerExecutor
                             .execute(
-                                &self.engine,
+                                self.engine.clone(),
                                 component_id,
                                 &self.base,
                                 &trigger.route,
@@ -268,7 +270,7 @@ impl HttpTrigger {
                         };
                         executor
                             .execute(
-                                &self.engine,
+                                self.engine.clone(),
                                 component_id,
                                 &self.base,
                                 &trigger.route,
@@ -419,8 +421,7 @@ fn strip_forbidden_headers(req: &mut Request<Body>) {
     let headers = req.headers_mut();
     if let Some(host_header) = headers.get("Host") {
         if let Ok(host) = host_header.to_str() {
-            let (host, _) = host.rsplit_once(':').unwrap_or((host, ""));
-            if host.ends_with(".spin.internal") {
+            if is_service_chaining_host(host) {
                 headers.remove("Host");
             }
         }
@@ -484,7 +485,7 @@ pub(crate) fn compute_default_headers<'a>(
 pub(crate) trait HttpExecutor: Clone + Send + Sync + 'static {
     async fn execute(
         &self,
-        engine: &Arc<TriggerAppEngine<HttpTrigger>>,
+        engine: Arc<TriggerAppEngine<HttpTrigger>>,
         component_id: &str,
         base: &str,
         raw_route: &str,
@@ -493,11 +494,16 @@ pub(crate) trait HttpExecutor: Clone + Send + Sync + 'static {
     ) -> Result<Response<Body>>;
 }
 
+#[derive(Clone)]
+struct ChainedRequestHandler {
+    engine: Arc<TriggerAppEngine<HttpTrigger>>,
+    executor: HttpHandlerExecutor,
+}
+
 #[derive(Default)]
 pub struct HttpRuntimeData {
     origin: Option<String>,
-    engine: Option<Arc<TriggerAppEngine<HttpTrigger>>>,
-    handler: Option<HttpHandlerExecutor>,
+    chained_handler: Option<ChainedRequestHandler>,
     /// The hosts this app is allowed to make outbound requests to
     allowed_hosts: AllowedHostsConfig,
 }
@@ -509,35 +515,41 @@ impl HttpRuntimeData {
         component_id: String,
     ) -> wasmtime::Result<
         wasmtime::component::Resource<wasmtime_wasi_http::types::HostFutureIncomingResponse>,
-    >
-    where
-        Self: Sized,
-    {
+    > {
         use wasmtime_wasi_http::types::HostFutureIncomingResponse;
         use wasmtime_wasi_http::types::IncomingResponseInternal;
 
-        let this = data.as_mut();
+        let this = data.as_ref();
 
-        let engine = this.engine.clone().ok_or(wasmtime::Error::msg(
+        let chained_handler = this.chained_handler.clone().ok_or(wasmtime::Error::msg(
             "Internal error: internal request chaining not prepared (engine not assigned)",
         ))?;
-        let handler = this.handler.clone().ok_or(wasmtime::Error::msg(
-            "Internal error: internal request chaining not prepared (handler not assigned)",
-        ))?;
+
+        let engine = chained_handler.engine;
+        let handler = chained_handler.executor;
 
         let base = "/";
         let raw_route = "/...";
 
         let client_addr = std::net::SocketAddr::from_str("0.0.0.0:0")?;
 
         let between_bytes_timeout = request.between_bytes_timeout;
+        // The IncomingResponseInternal type formally needs a "worker" join handle, but
+        // in a chained use case it doesn't seem to do anything.
         let worker = Arc::new(wasmtime_wasi::preview2::spawn(async {}));
 
         let req = request.request;
 
         let resp_fut = async move {
             match handler
-                .execute(&engine, &component_id, base, raw_route, req, client_addr)
+                .execute(
+                    engine.clone(),
+                    &component_id,
+                    base,
+                    raw_route,
+                    req,
+                    client_addr,
+                )
                 .await
             {
                 Ok(resp) => Ok(Ok(IncomingResponseInternal {
@@ -555,15 +567,7 @@ impl HttpRuntimeData {
 }
 
 fn parse_chaining_target(request: &wasmtime_wasi_http::types::OutgoingRequest) -> Option<String> {
-    let host = request.request.uri().authority().map(|a| a.host().trim())?;
-
-    let (first, rest) = host.split_once('.')?;
-
-    if rest == "spin.internal" {
-        Some(first.to_owned())
-    } else {
-        None
-    }
+    parse_service_chaining_target(request.request.uri())
 }
 
 impl OutboundWasiHttpHandler for HttpRuntimeData {

crates/trigger-http/src/wagi.rs

@@ -21,7 +21,7 @@ pub struct WagiHttpExecutor {
 impl HttpExecutor for WagiHttpExecutor {
     async fn execute(
         &self,
-        engine: &Arc<TriggerAppEngine<HttpTrigger>>,
+        engine: Arc<TriggerAppEngine<HttpTrigger>>,
         component: &str,
         base: &str,
         raw_route: &str,