From 9fbc065c472565da2493eb542fb618564e77c029 Mon Sep 17 00:00:00 2001 From: itowlson Date: Wed, 3 Apr 2024 12:55:43 +1300 Subject: [PATCH] Private endpoints Signed-off-by: itowlson --- crates/http/src/config.rs | 22 ++++- crates/http/src/routes.rs | 85 ++++++++++++++----- crates/testing/src/lib.rs | 8 +- crates/trigger-http/src/lib.rs | 13 ++- .../tests/internal-http/spin.toml | 2 +- 5 files changed, 101 insertions(+), 29 deletions(-) diff --git a/crates/http/src/config.rs b/crates/http/src/config.rs index 9fb1db5482..f34aaf3b0c 100644 --- a/crates/http/src/config.rs +++ b/crates/http/src/config.rs @@ -7,12 +7,32 @@ pub struct HttpTriggerConfig { /// Component ID to invoke pub component: String, /// HTTP route the component will be invoked for - pub route: String, + pub route: HttpTriggerRouteConfig, /// The HTTP executor the component requires #[serde(default)] pub executor: Option, } +/// An HTTP trigger route +#[derive(Clone, Debug, Deserialize, Serialize)] +#[serde(untagged)] +pub enum HttpTriggerRouteConfig { + Route(String), + IsRoutable(bool), +} + +impl Default for HttpTriggerRouteConfig { + fn default() -> Self { + Self::Route(Default::default()) + } +} + +impl> From for HttpTriggerRouteConfig { + fn from(value: T) -> Self { + Self::Route(value.into()) + } +} + /// The executor for the HTTP component. /// The component can either implement the Spin HTTP interface, /// the `wasi-http` interface, or the Wagi CGI interface. diff --git a/crates/http/src/routes.rs b/crates/http/src/routes.rs index 8b0fe2422f..d8d859738f 100644 --- a/crates/http/src/routes.rs +++ b/crates/http/src/routes.rs @@ -7,6 +7,8 @@ use http::Uri; use indexmap::IndexMap; use std::{borrow::Cow, fmt}; +use crate::config::HttpTriggerRouteConfig; + /// Router for the HTTP trigger. #[derive(Clone, Debug)] pub struct Router { @@ -15,6 +17,7 @@ pub struct Router { } /// A detected duplicate route. +#[derive(Debug)] // Needed to call `expect_err` on `Router::build` pub struct DuplicateRoute { /// The duplicated route pattern. pub route: RoutePattern, @@ -28,14 +31,23 @@ impl Router { /// Builds a router based on application configuration. pub fn build<'a>( base: &str, - component_routes: impl IntoIterator, + component_routes: impl IntoIterator, ) -> Result<(Self, Vec)> { let mut routes = IndexMap::new(); let mut duplicates = vec![]; - let routes_iter = component_routes.into_iter().map(|(component_id, route)| { - (RoutePattern::from(base, route), component_id.to_string()) - }); + let routes_iter = component_routes + .into_iter() + .filter_map(|(component_id, route)| { + match route { + HttpTriggerRouteConfig::Route(r) => { + Some(Ok((RoutePattern::from(base, r), component_id.to_string()))) + } + HttpTriggerRouteConfig::IsRoutable(false) => None, + HttpTriggerRouteConfig::IsRoutable(true) => Some(Err(anyhow!("route must be a string pattern or 'false': component '{component_id}' has route = 'true'"))), + } + }) + .collect::>>()?; for (route, component_id) in routes_iter { let replaced = routes.insert(route.clone(), component_id.clone()); @@ -417,10 +429,10 @@ mod route_tests { let (routes, duplicates) = Router::build( "/", vec![ - ("/", "/"), - ("/foo", "/foo"), - ("/bar", "/bar"), - ("/whee/...", "/whee/..."), + ("/", &"/".into()), + ("/foo", &"/foo".into()), + ("/bar", &"/bar".into()), + ("/whee/...", &"/whee/...".into()), ], ) .unwrap(); @@ -434,10 +446,10 @@ mod route_tests { let (routes, _) = Router::build( "/", vec![ - ("/", "/"), - ("/foo", "/foo"), - ("/bar", "/bar"), - ("/whee/...", "/whee/..."), + ("/", &"/".into()), + ("/foo", &"/foo".into()), + ("/bar", &"/bar".into()), + ("/whee/...", &"/whee/...".into()), ], ) .unwrap(); @@ -453,10 +465,10 @@ mod route_tests { let (routes, duplicates) = Router::build( "/", vec![ - ("/", "/"), - ("first /foo", "/foo"), - ("second /foo", "/foo"), - ("/whee/...", "/whee/..."), + ("/", &"/".into()), + ("first /foo", &"/foo".into()), + ("second /foo", &"/foo".into()), + ("/whee/...", &"/whee/...".into()), ], ) .unwrap(); @@ -470,10 +482,10 @@ mod route_tests { let (routes, duplicates) = Router::build( "/", vec![ - ("/", "/"), - ("first /foo", "/foo"), - ("second /foo", "/foo"), - ("/whee/...", "/whee/..."), + ("/", &"/".into()), + ("first /foo", &"/foo".into()), + ("second /foo", &"/foo".into()), + ("/whee/...", &"/whee/...".into()), ], ) .unwrap(); @@ -482,4 +494,37 @@ mod route_tests { assert_eq!("first /foo", duplicates[0].replaced_id); assert_eq!("second /foo", duplicates[0].effective_id); } + + #[test] + fn unroutable_routes_are_skipped() { + let (routes, _) = Router::build( + "/", + vec![ + ("/", &"/".into()), + ("/foo", &"/foo".into()), + ("private", &HttpTriggerRouteConfig::IsRoutable(false)), + ("/whee/...", &"/whee/...".into()), + ], + ) + .unwrap(); + + assert_eq!(3, routes.routes.len()); + assert!(!routes.routes.values().any(|c| c == "private")); + } + + #[test] + fn unroutable_routes_have_to_be_unroutable_thats_just_common_sense() { + let e = Router::build( + "/", + vec![ + ("/", &"/".into()), + ("/foo", &"/foo".into()), + ("bad component", &HttpTriggerRouteConfig::IsRoutable(true)), + ("/whee/...", &"/whee/...".into()), + ], + ) + .expect_err("should not have accepted a 'route = true'"); + + assert!(e.to_string().contains("bad component")); + } } diff --git a/crates/testing/src/lib.rs b/crates/testing/src/lib.rs index 0b815740db..48b56bf589 100644 --- a/crates/testing/src/lib.rs +++ b/crates/testing/src/lib.rs @@ -16,7 +16,9 @@ use spin_app::{ AppComponent, Loader, }; use spin_core::{Component, StoreBuilder}; -use spin_http::config::{HttpExecutorType, HttpTriggerConfig, WagiTriggerConfig}; +use spin_http::config::{ + HttpExecutorType, HttpTriggerConfig, HttpTriggerRouteConfig, WagiTriggerConfig, +}; use spin_trigger::{HostComponentInitData, RuntimeConfig, TriggerExecutor, TriggerExecutorBuilder}; use tokio::fs; @@ -68,7 +70,7 @@ impl HttpTestConfig { self.module_path(Path::new(TEST_PROGRAM_PATH).join(name)) } - pub fn http_spin_trigger(&mut self, route: impl Into) -> &mut Self { + pub fn http_spin_trigger(&mut self, route: impl Into) -> &mut Self { self.http_trigger_config = HttpTriggerConfig { component: "test-component".to_string(), route: route.into(), @@ -79,7 +81,7 @@ impl HttpTestConfig { pub fn http_wagi_trigger( &mut self, - route: impl Into, + route: impl Into, wagi_config: WagiTriggerConfig, ) -> &mut Self { self.http_trigger_config = HttpTriggerConfig { diff --git a/crates/trigger-http/src/lib.rs b/crates/trigger-http/src/lib.rs index e4db3fbd1b..9b2fa2525b 100644 --- a/crates/trigger-http/src/lib.rs +++ b/crates/trigger-http/src/lib.rs @@ -29,7 +29,7 @@ use spin_core::{Engine, OutboundWasiHttpHandler}; use spin_http::{ app_info::AppInfo, body, - config::{HttpExecutorType, HttpTriggerConfig}, + config::{HttpExecutorType, HttpTriggerConfig, HttpTriggerRouteConfig}, routes::{RoutePattern, Router}, }; use spin_outbound_networking::{ @@ -119,7 +119,7 @@ impl TriggerExecutor for HttpTrigger { let component_routes = engine .trigger_configs() - .map(|(_, config)| (config.component.as_str(), config.route.as_str())); + .map(|(_, config)| (config.component.as_str(), &config.route)); let (router, duplicate_routes) = Router::build(&base, component_routes)?; @@ -257,6 +257,11 @@ impl HttpTrigger { let executor = trigger.executor.as_ref().unwrap_or(&HttpExecutorType::Http); + let raw_route = match &trigger.route { + HttpTriggerRouteConfig::Route(r) => r.as_str(), + HttpTriggerRouteConfig::IsRoutable(_) => "/...", + }; + let res = match executor { HttpExecutorType::Http => { HttpHandlerExecutor @@ -264,7 +269,7 @@ impl HttpTrigger { self.engine.clone(), component_id, &self.base, - &trigger.route, + raw_route, req, addr, ) @@ -279,7 +284,7 @@ impl HttpTrigger { self.engine.clone(), component_id, &self.base, - &trigger.route, + raw_route, req, addr, ) diff --git a/tests/runtime-tests/tests/internal-http/spin.toml b/tests/runtime-tests/tests/internal-http/spin.toml index 2c99c65a8b..aff1c9f541 100644 --- a/tests/runtime-tests/tests/internal-http/spin.toml +++ b/tests/runtime-tests/tests/internal-http/spin.toml @@ -14,7 +14,7 @@ source = "%{source=internal-http-front}" allowed_outbound_hosts = ["http://middle.spin.internal"] [[trigger.http]] -route = "/middle/..." +route = false component = "middle" [component.middle]