diff --git a/Cargo.lock b/Cargo.lock index 906f6fc198..f52bf16996 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7655,11 +7655,13 @@ dependencies = [ "http 1.1.0", "http-body-util", "hyper 1.4.1", + "reqwest 0.11.27", "rustls 0.23.7", "spin-factor-outbound-networking", "spin-factor-variables", "spin-factors", "spin-factors-test", + "spin-telemetry", "spin-world", "terminal", "tokio", diff --git a/crates/factor-outbound-http/Cargo.toml b/crates/factor-outbound-http/Cargo.toml index cee32e4830..e62c6c52ff 100644 --- a/crates/factor-outbound-http/Cargo.toml +++ b/crates/factor-outbound-http/Cargo.toml @@ -9,9 +9,11 @@ anyhow = "1.0" http = "1.1.0" http-body-util = "0.1" hyper = "1.4.1" +reqwest = { version = "0.11", features = ["gzip"] } rustls = "0.23" spin-factor-outbound-networking = { path = "../factor-outbound-networking" } spin-factors = { path = "../factors" } +spin-telemetry = { path = "../telemetry" } spin-world = { path = "../world" } terminal = { path = "../terminal" } tokio = { version = "1", features = ["macros", "rt"] } diff --git a/crates/factor-outbound-http/src/lib.rs b/crates/factor-outbound-http/src/lib.rs index b0c18baf8a..739be2ab9a 100644 --- a/crates/factor-outbound-http/src/lib.rs +++ b/crates/factor-outbound-http/src/lib.rs @@ -71,6 +71,7 @@ impl Factor for OutboundHttpFactor { component_tls_configs, self_request_origin: None, request_interceptor: None, + spin_http_client: None, }) } } @@ -81,6 +82,8 @@ pub struct InstanceState { component_tls_configs: ComponentTlsConfigs, self_request_origin: Option, request_interceptor: Option>, + // Connection-pooling client for 'fermyon:spin/http' interface + spin_http_client: Option, } impl InstanceState { @@ -156,6 +159,12 @@ impl SelfRequestOrigin { } } +impl std::fmt::Display for SelfRequestOrigin { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}://{}", self.scheme, self.authority) + } +} + /// An outbound HTTP request interceptor to be used with /// [`InstanceState::set_request_interceptor`]. pub trait OutboundHttpInterceptor: Send + Sync { diff --git a/crates/factor-outbound-http/src/spin.rs b/crates/factor-outbound-http/src/spin.rs index c9dd6f1862..633df727d9 100644 --- a/crates/factor-outbound-http/src/spin.rs +++ b/crates/factor-outbound-http/src/spin.rs @@ -1,24 +1,84 @@ use spin_world::{ async_trait, - v1::http, - v1::http_types::{self, HttpError, Request, Response}, + v1::{ + http as spin_http, + http_types::{self, HttpError, Method, Request, Response}, + }, }; +use tracing::{field::Empty, instrument, Level, Span}; #[async_trait] -impl http::Host for crate::InstanceState { +impl spin_http::Host for crate::InstanceState { + #[instrument(name = "spin_outbound_http.send_request", skip_all, err(level = Level::INFO), + fields(otel.kind = "client", url.full = Empty, http.request.method = Empty, + http.response.status_code = Empty, otel.name = Empty, server.address = Empty, server.port = Empty))] async fn send_request(&mut self, req: Request) -> Result { - // FIXME(lann): This is all just a stub to test allowed_outbound_hosts - match self.allowed_hosts.check_url(&req.uri, "https").await { - Ok(true) => (), - _ => { + let span = Span::current(); + record_request_fields(&span, &req); + + let uri = req.uri; + tracing::trace!("Sending outbound HTTP to {uri:?}"); + + let abs_url = if !uri.starts_with('/') { + // Absolute URI + let is_allowed = self + .allowed_hosts + .check_url(&uri, "https") + .await + .unwrap_or(false); + if !is_allowed { + return Err(HttpError::DestinationNotAllowed); + } + uri + } else { + // Relative URI ("self" request) + let is_allowed = self + .allowed_hosts + .check_relative_url(&["http", "https"]) + .await + .unwrap_or(false); + if !is_allowed { return Err(HttpError::DestinationNotAllowed); } + + let Some(origin) = &self.self_request_origin else { + tracing::error!( + "Couldn't handle outbound HTTP request to relative URI; no origin set" + ); + return Err(HttpError::InvalidUrl); + }; + format!("{origin}{uri}") + }; + let req_url = reqwest::Url::parse(&abs_url).map_err(|_| HttpError::InvalidUrl)?; + + if !req.params.is_empty() { + tracing::warn!("HTTP params field is deprecated"); } - Ok(Response { - status: 200, - headers: None, - body: Some(b"test response".into()), - }) + + // Allow reuse of Client's internal connection pool for multiple requests + // in a single component execution + let client = self.spin_http_client.get_or_insert_with(Default::default); + + let mut req = { + let mut builder = client.request(reqwest_method(req.method), req_url); + for (key, val) in req.headers { + builder = builder.header(key, val); + } + builder + .body(req.body.unwrap_or_default()) + .build() + .map_err(|err| { + tracing::error!("Error building outbound request: {err}"); + HttpError::RuntimeError + })? + }; + spin_telemetry::inject_trace_context(req.headers_mut()); + + let resp = client.execute(req).await.map_err(log_reqwest_error)?; + + tracing::trace!("Returning response from outbound request to {abs_url}"); + span.record("http.response.status_code", resp.status().as_u16()); + response_from_reqwest(resp).await } } @@ -27,3 +87,93 @@ impl http_types::Host for crate::InstanceState { Ok(err) } } + +fn record_request_fields(span: &Span, req: &Request) { + let method = match req.method { + Method::Get => "GET", + Method::Post => "POST", + Method::Put => "PUT", + Method::Delete => "DELETE", + Method::Patch => "PATCH", + Method::Head => "HEAD", + Method::Options => "OPTIONS", + }; + span.record("otel.name", method) + .record("http.request.method", method) + .record("url.full", req.uri.clone()); + if let Ok(uri) = req.uri.parse::() { + if let Some(authority) = uri.authority() { + span.record("server.address", authority.host()); + if let Some(port) = authority.port() { + span.record("server.port", port.as_u16()); + } + } + } +} + +fn reqwest_method(m: Method) -> reqwest::Method { + match m { + Method::Get => reqwest::Method::GET, + Method::Post => reqwest::Method::POST, + Method::Put => reqwest::Method::PUT, + Method::Delete => reqwest::Method::DELETE, + Method::Patch => reqwest::Method::PATCH, + Method::Head => reqwest::Method::HEAD, + Method::Options => reqwest::Method::OPTIONS, + } +} + +fn log_reqwest_error(err: reqwest::Error) -> HttpError { + let error_desc = if err.is_timeout() { + "timeout error" + } else if err.is_connect() { + "connection error" + } else if err.is_body() || err.is_decode() { + "message body error" + } else if err.is_request() { + "request error" + } else { + "error" + }; + tracing::warn!( + "Outbound HTTP {}: URL {}, error detail {:?}", + error_desc, + err.url() + .map(|u| u.to_string()) + .unwrap_or_else(|| "".to_owned()), + err + ); + HttpError::RuntimeError +} + +async fn response_from_reqwest(res: reqwest::Response) -> Result { + let status = res.status().as_u16(); + + let headers = res + .headers() + .into_iter() + .map(|(key, val)| { + Ok(( + key.to_string(), + val.to_str() + .map_err(|_| { + tracing::error!("Non-ascii response header {key} = {val:?}"); + HttpError::RuntimeError + })? + .to_string(), + )) + }) + .collect::, _>>()?; + + let body = res + .bytes() + .await + .map_err(|_| HttpError::RuntimeError)? + .to_vec(); + + Ok(Response { + status, + headers: Some(headers), + body: Some(body), + }) +} diff --git a/crates/factor-outbound-http/src/wasi.rs b/crates/factor-outbound-http/src/wasi.rs index e63f40b532..8d49bad2ab 100644 --- a/crates/factor-outbound-http/src/wasi.rs +++ b/crates/factor-outbound-http/src/wasi.rs @@ -86,6 +86,21 @@ impl<'a> WasiHttpView for WasiHttpImplInner<'a> { mut request: Request, mut config: wasmtime_wasi_http::types::OutgoingRequestConfig, ) -> wasmtime_wasi_http::HttpResult { + // wasmtime-wasi-http fills in scheme and authority for relative URLs + // (e.g. https://:443/), which makes them hard to reason about. + // Undo that here. + let uri = request.uri_mut(); + if uri + .authority() + .is_some_and(|authority| authority.host().is_empty()) + { + let mut builder = http::uri::Builder::new(); + if let Some(paq) = uri.path_and_query() { + builder = builder.path_and_query(paq.clone()); + } + *uri = builder.build().unwrap(); + } + if let Some(interceptor) = &self.state.request_interceptor { match interceptor.intercept(&mut request, &mut config) { InterceptOutcome::Continue => (), @@ -149,6 +164,7 @@ async fn send_request_impl( config.use_tls = origin.use_tls(); request.headers_mut().insert(HOST, origin.host_header()); + spin_telemetry::inject_trace_context(&mut request); let path_and_query = request.uri().path_and_query().cloned(); *request.uri_mut() = origin.into_uri(path_and_query); diff --git a/examples/spin-timer/Cargo.lock b/examples/spin-timer/Cargo.lock index b9d18f476f..425b5c4b74 100644 --- a/examples/spin-timer/Cargo.lock +++ b/examples/spin-timer/Cargo.lock @@ -58,12 +58,6 @@ dependencies = [ "memchr", ] -[[package]] -name = "aliasable" -version = "0.1.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "250f629c0161ad8107cf89319e990051fae62832fd343083bea452d93e2205fd" - [[package]] name = "allocator-api2" version = "0.2.16" @@ -3923,31 +3917,6 @@ version = "6.6.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e2355d85b9a3786f481747ced0e0ff2ba35213a1f9bd406ed906554d7af805a1" -[[package]] -name = "ouroboros" -version = "0.18.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "97b7be5a8a3462b752f4be3ff2b2bf2f7f1d00834902e46be2a4d68b87b0573c" -dependencies = [ - "aliasable", - "ouroboros_macro", - "static_assertions", -] - -[[package]] -name = "ouroboros_macro" -version = "0.18.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b645dcde5f119c2c454a92d0dfa271a2a3b205da92e4292a68ead4bdbfde1f33" -dependencies = [ - "heck 0.4.1", - "itertools 0.12.1", - "proc-macro2", - "proc-macro2-diagnostics", - "quote", - "syn 2.0.48", -] - [[package]] name = "outbound-http" version = "2.8.0-pre0" @@ -4470,19 +4439,6 @@ dependencies = [ "unicode-ident", ] -[[package]] -name = "proc-macro2-diagnostics" -version = "0.10.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "af066a9c399a26e020ada66a034357a868728e72cd426f3adcd35f80d88d88c8" -dependencies = [ - "proc-macro2", - "quote", - "syn 2.0.48", - "version_check", - "yansi", -] - [[package]] name = "prost" version = "0.12.6" @@ -5643,10 +5599,8 @@ version = "2.8.0-pre0" dependencies = [ "anyhow", "async-trait", - "ouroboros", "serde 1.0.203", "serde_json", - "spin-core", "spin-locked-app", "spin-serde", "thiserror", @@ -5683,20 +5637,8 @@ version = "2.8.0-pre0" dependencies = [ "anyhow", "async-trait", - "bytes", - "cap-primitives", - "cap-std", - "http 1.1.0", - "io-extras", - "rustix 0.37.27", - "spin-telemetry", - "system-interface", - "tokio", "tracing", - "wasi-common", "wasmtime", - "wasmtime-wasi", - "wasmtime-wasi-http", ] [[package]] @@ -5814,7 +5756,6 @@ dependencies = [ "itertools 0.10.5", "lazy_static 1.4.0", "mime_guess", - "outbound-http", "path-absolutize", "regex", "reqwest 0.11.24", @@ -5844,7 +5785,6 @@ version = "2.8.0-pre0" dependencies = [ "anyhow", "async-trait", - "ouroboros", "serde 1.0.203", "serde_json", "spin-serde", @@ -6032,6 +5972,7 @@ dependencies = [ name = "spin-world" version = "2.8.0-pre0" dependencies = [ + "async-trait", "wasmtime", ] @@ -7132,33 +7073,6 @@ version = "0.11.0+wasi-snapshot-preview1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9c8d87e72b64a3b4db28d11ce29237c246188f4f51057d65a7eab63b7987e423" -[[package]] -name = "wasi-common" -version = "22.0.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b86fd41e1e26ff6af9451c6a332a5ce5f5283ca51e87d875cdd9a05305598ee3" -dependencies = [ - "anyhow", - "bitflags 2.4.2", - "cap-fs-ext", - "cap-rand", - "cap-std", - "cap-time-ext", - "fs-set-times", - "io-extras", - "io-lifetimes 2.0.3", - "log", - "once_cell", - "rustix 0.38.31", - "system-interface", - "thiserror", - "tokio", - "tracing", - "wasmtime", - "wiggle", - "windows-sys 0.52.0", -] - [[package]] name = "wasite" version = "0.1.0" @@ -8218,12 +8132,6 @@ dependencies = [ "linked-hash-map", ] -[[package]] -name = "yansi" -version = "1.0.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cfe53a6657fd280eaa890a3bc59152892ffa3e30101319d168b781ed6529b049" - [[package]] name = "zbus" version = "3.15.2" diff --git a/tests/integration.rs b/tests/integration.rs index ee869aeef4..2c4b8ad04c 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -72,12 +72,12 @@ mod integration_tests { let spin = env.runtime_mut(); assert_spin_request( spin, - Request::new(Method::Get, "/test/hello"), + Request::new(Method::Get, "/hello"), Response::new_with_body(200, "I'm a teapot"), )?; assert_spin_request( spin, - Request::new(Method::Get, "/test/hello/wildcards/should/be/handled"), + Request::new(Method::Get, "/hello/wildcards/should/be/handled"), Response::new_with_body(200, "I'm a teapot"), )?; assert_spin_request( @@ -87,7 +87,7 @@ mod integration_tests { )?; assert_spin_request( spin, - Request::new(Method::Get, "/test/hello/test-placement"), + Request::new(Method::Get, "/hello/test-placement"), Response::new_with_body(200, "text for test"), ) }, @@ -183,7 +183,7 @@ mod integration_tests { let spin = env.runtime_mut(); assert_spin_request( spin, - Request::new(Method::Get, "/test/hello"), + Request::new(Method::Get, "/hello"), Response::new_with_body(200, "Hello, Fermyon!\n"), )?; @@ -368,13 +368,13 @@ Caused by: let spin = env.runtime_mut(); assert_spin_request( spin, - Request::new(Method::Get, "/test/outbound-allowed"), + Request::new(Method::Get, "/outbound-allowed"), Response::new_with_body(200, "Hello, Fermyon!\n"), )?; assert_spin_request( spin, - Request::new(Method::Get, "/test/outbound-not-allowed"), + Request::new(Method::Get, "/outbound-not-allowed"), Response::new_with_body( 500, "Error::UnexpectedError(\"ErrorCode::HttpRequestDenied\")", @@ -421,14 +421,14 @@ Caused by: Response::new_with_body(expected_status, expected_body), ) }; - ensure_success("/test/hello", 200, "I'm a teapot")?; + ensure_success("/hello", 200, "I'm a teapot")?; ensure_success( - "/test/hello/wildcards/should/be/handled", + "/hello/wildcards/should/be/handled", 200, "I'm a teapot", )?; ensure_success("/thisshouldfail", 404, "")?; - ensure_success("/test/hello/test-placement", 200, "text for test")?; + ensure_success("/hello/test-placement", 200, "text for test")?; Ok(()) }, )?; @@ -1255,14 +1255,14 @@ route = "/..." let spin = env.runtime_mut(); assert_spin_request( spin, - Request::full(Method::Get, "/base/echo", &[], Some("Echo...")), + Request::full(Method::Get, "/echo", &[], Some("Echo...")), Response::new_with_body(200, "Echo..."), )?; assert_spin_request( spin, Request::full( Method::Get, - "/base/assert-headers?k=v", + "/assert-headers?k=v", &[("X-Custom-Foo", "bar")], Some(r#"{"x-custom-foo": "bar"}"#), ), @@ -1288,16 +1288,16 @@ route = "/..." let spin = env.runtime_mut(); assert_spin_request( spin, - Request::full(Method::Get, "/base/echo", &[], Some("Echo...")), + Request::full(Method::Get, "/echo", &[], Some("Echo...")), Response::new_with_body(200, "Echo..."), )?; assert_spin_request( spin, Request::full( Method::Get, - "/base/assert-args?x=y", + "/assert-args?x=y", &[], - Some(r#"["/base/assert-args", "x=y"]"#), + Some(r#"["/assert-args", "x=y"]"#), ), Response::new(200), )?; @@ -1305,7 +1305,7 @@ route = "/..." spin, Request::full( Method::Get, - "/base/assert-env", + "/assert-env", &[("X-Custom-Foo", "bar")], Some(r#"{"HTTP_X_CUSTOM_FOO": "bar"}"#), ), @@ -1464,7 +1464,7 @@ route = "/..." spin, Request::full( Method::Get, - "/test/outbound-allowed/hello", + "/outbound-allowed/hello", &[("Host", "google.com")], Some(""), ), diff --git a/tests/runtime-tests/tests/llm/spin.toml b/tests/runtime-tests/tests/llm/spin.toml index 9a1e18e1f9..bee13ef4a6 100644 --- a/tests/runtime-tests/tests/llm/spin.toml +++ b/tests/runtime-tests/tests/llm/spin.toml @@ -2,7 +2,7 @@ spin_manifest_version = "1" authors = ["Ryan Levick "] description = "" name = "ai" -trigger = { type = "http", base = "/" } +trigger = { type = "http" } version = "0.1.0" [[component]] diff --git a/tests/test-components/components/internal-http-middle/src/lib.rs b/tests/test-components/components/internal-http-middle/src/lib.rs index 32160ab55d..6095422f01 100644 --- a/tests/test-components/components/internal-http-middle/src/lib.rs +++ b/tests/test-components/components/internal-http-middle/src/lib.rs @@ -16,10 +16,6 @@ async fn handle_middle_impl(req: Request) -> Result { .header("spin-path-info") .and_then(|v| v.as_str()); let inbound_rel_path = ensure_some!(inbound_rel_path); - let inbound_base = req - .header("spin-base-path") - .and_then(|v| v.as_str()); - ensure_eq!("/", ensure_some!(inbound_base)); let out_req = spin_sdk::http::Request::builder() .uri("https://back.spin.internal/hello/from/middle") diff --git a/tests/test-components/components/outbound-http/src/lib.rs b/tests/test-components/components/outbound-http/src/lib.rs index 5799ac6674..d9155ab843 100644 --- a/tests/test-components/components/outbound-http/src/lib.rs +++ b/tests/test-components/components/outbound-http/src/lib.rs @@ -10,7 +10,7 @@ async fn send_outbound(_req: Request) -> Result { let mut res: http::Response = spin_sdk::http::send( http::Request::builder() .method("GET") - .uri("/test/hello") + .uri("/hello") .body(())?, ) .await?; diff --git a/tests/testcases/http-smoke-test/spin.toml b/tests/testcases/http-smoke-test/spin.toml index b12a0e28d6..c0340f9e2c 100644 --- a/tests/testcases/http-smoke-test/spin.toml +++ b/tests/testcases/http-smoke-test/spin.toml @@ -2,7 +2,7 @@ spin_version = "1" authors = ["Fermyon Engineering "] description = "A simple application that returns hello and goodbye." name = "head-rust-sdk-http" -trigger = { type = "http", base = "/test" } +trigger = { type = "http" } version = "1.0.0" [variables] diff --git a/tests/testcases/key-value/spin.toml b/tests/testcases/key-value/spin.toml index d241467176..3202eb21d3 100644 --- a/tests/testcases/key-value/spin.toml +++ b/tests/testcases/key-value/spin.toml @@ -2,7 +2,7 @@ spin_version = "1" authors = ["Fermyon Engineering "] description = "A simple application that exercises key/value storage." name = "key-value" -trigger = { type = "http", base = "/test" } +trigger = { type = "http" } version = "1.0.0" [[component]] diff --git a/tests/testcases/otel-smoke-test/spin.toml b/tests/testcases/otel-smoke-test/spin.toml index c5911038b5..a4eb09f671 100644 --- a/tests/testcases/otel-smoke-test/spin.toml +++ b/tests/testcases/otel-smoke-test/spin.toml @@ -2,7 +2,7 @@ spin_version = "1" authors = ["Fermyon Engineering "] description = "A simple application that returns hello and goodbye." name = "head-rust-sdk-http" -trigger = { type = "http", base = "/test" } +trigger = { type = "http" } version = "1.0.0" [[component]] diff --git a/tests/testcases/outbound-http-to-same-app/spin.toml b/tests/testcases/outbound-http-to-same-app/spin.toml index 90e517f550..8f241d7e63 100644 --- a/tests/testcases/outbound-http-to-same-app/spin.toml +++ b/tests/testcases/outbound-http-to-same-app/spin.toml @@ -2,7 +2,7 @@ spin_version = "1" authors = ["Fermyon Engineering "] description = "An application that demonstates a component making an outbound http request to another component in the same application." name = "local-outbound-http" -trigger = { type = "http", base = "/test" } +trigger = { type = "http" } version = "1.0.0" [[component]] diff --git a/tests/testcases/simple-test/spin.toml b/tests/testcases/simple-test/spin.toml index e069317e11..26fd2fa707 100644 --- a/tests/testcases/simple-test/spin.toml +++ b/tests/testcases/simple-test/spin.toml @@ -2,7 +2,7 @@ spin_version = "1" authors = ["Fermyon Engineering "] description = "A simple application that returns hello and goodbye." name = "spin-hello-world" -trigger = { type = "http", base = "/test" } +trigger = { type = "http" } version = "1.0.0" [variables] diff --git a/tests/testcases/spin-inbound-http/spin.toml b/tests/testcases/spin-inbound-http/spin.toml index 5c018b71e6..a97853d344 100644 --- a/tests/testcases/spin-inbound-http/spin.toml +++ b/tests/testcases/spin-inbound-http/spin.toml @@ -6,9 +6,6 @@ description = "Test using the spin inbound-http interface." name = "spin-inbound-http" version = "1.0.0" -[application.trigger.http] -base = "/base" - [[trigger.http]] route = "/..." [trigger.http.component] diff --git a/tests/testcases/wagi-http/spin.toml b/tests/testcases/wagi-http/spin.toml index 871d681b4a..5e3f028127 100644 --- a/tests/testcases/wagi-http/spin.toml +++ b/tests/testcases/wagi-http/spin.toml @@ -6,9 +6,6 @@ description = "Test using WAGI HTTP." name = "wagi-http" version = "1.0.0" -[application.trigger.http] -base = "/base" - [[trigger.http]] route = "/..." executor = { type = "wagi" }