Merge pull request #2418 from itowlson/private-endpoints

itowlson · web-flow · commit 96342e28defb · 2024-04-05T08:11:52.000+13:00
Private endpoints
diff --git 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<HttpExecutorType>,
 }
 
+/// 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<T: Into<String>> From<T> 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
@@ -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<Item = (&'a str, &'a str)>,
+        component_routes: impl IntoIterator<Item = (&'a str, &'a HttpTriggerRouteConfig)>,
     ) -> Result<(Self, Vec<DuplicateRoute>)> {
         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::<Result<Vec<_>>>()?;
 
         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
@@ -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<String>) -> &mut Self {
+    pub fn http_spin_trigger(&mut self, route: impl Into<HttpTriggerRouteConfig>) -> &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<String>,
+        route: impl Into<HttpTriggerRouteConfig>,
         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
@@ -30,7 +30,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::{
@@ -120,7 +120,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)?;
 
@@ -258,14 +258,19 @@ 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
                             .execute(
                                 self.engine.clone(),
                                 component_id,
                                 &self.base,
-                                &trigger.route,
+                                raw_route,
                                 req,
                                 addr,
                             )
@@ -280,7 +285,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
@@ -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]
