Merge pull request #2892 from kate-goldenring/component-filtering-move

fix: move component filtering methods to crates

Author: itowlson

Cargo.lock

@@ -5,6 +5,12 @@ authors = { workspace = true }
 edition = { workspace = true }
 
 [dependencies]
+anyhow = { workspace = true }
 serde = { workspace = true }
 serde_json = { workspace = true }
 spin-locked-app = { path = "../locked-app" }
+
+[dev-dependencies]
+toml = { workspace = true }
+spin-factors-test = { path = "../factors-test" }
+tokio = { workspace = true }

crates/app/Cargo.toml

@@ -6,6 +6,8 @@
 
 #![deny(missing_docs)]
 
+use std::collections::HashSet;
+
 use serde::Deserialize;
 use serde_json::Value;
 use spin_locked_app::MetadataExt;
@@ -27,6 +29,10 @@ pub const APP_DESCRIPTION_KEY: MetadataKey = MetadataKey::new("description");
 /// MetadataKey for extracting the OCI image digest.
 pub const OCI_IMAGE_DIGEST_KEY: MetadataKey = MetadataKey::new("oci_image_digest");
 
+/// Validation function type for ensuring that applications meet requirements
+/// even with components filtered out.
+pub type ValidatorFn = dyn Fn(&App, &[&str]) -> anyhow::Result<()>;
+
 /// An `App` holds loaded configuration for a Spin application.
 #[derive(Debug, Clone)]
 pub struct App {
@@ -160,6 +166,49 @@ impl App {
     pub fn ensure_needs_only(&self, supported: &[&str]) -> std::result::Result<(), String> {
         self.locked.ensure_needs_only(supported)
     }
+
+    /// Scrubs the locked app to only contain the given list of components
+    /// Introspects the LockedApp to find and selectively retain the triggers that correspond to those components
+    fn retain_components(
+        mut self,
+        retained_components: &[&str],
+        validators: &[&ValidatorFn],
+    ) -> Result<LockedApp> {
+        self.validate_retained_components_exist(retained_components)?;
+        for validator in validators {
+            validator(&self, retained_components).map_err(Error::ValidationError)?;
+        }
+        let (component_ids, trigger_ids): (HashSet<String>, HashSet<String>) = self
+            .triggers()
+            .filter_map(|t| match t.component() {
+                Ok(comp) if retained_components.contains(&comp.id()) => {
+                    Some((comp.id().to_owned(), t.id().to_owned()))
+                }
+                _ => None,
+            })
+            .collect();
+        self.locked
+            .components
+            .retain(|c| component_ids.contains(&c.id));
+        self.locked.triggers.retain(|t| trigger_ids.contains(&t.id));
+        Ok(self.locked)
+    }
+
+    /// Validates that all components specified to be retained actually exist in the app
+    fn validate_retained_components_exist(&self, retained_components: &[&str]) -> Result<()> {
+        let app_components = self
+            .components()
+            .map(|c| c.id().to_string())
+            .collect::<HashSet<_>>();
+        for c in retained_components {
+            if !app_components.contains(*c) {
+                return Err(Error::ValidationError(anyhow::anyhow!(
+                    "Specified component \"{c}\" not found in application"
+                )));
+            }
+        }
+        Ok(())
+    }
 }
 
 /// An `AppComponent` holds configuration for a Spin application component.
@@ -266,3 +315,49 @@ impl<'a> AppTrigger<'a> {
 struct CommonTriggerConfig {
     component: Option<String>,
 }
+
+/// Scrubs the locked app to only contain the given list of components
+/// Introspects the LockedApp to find and selectively retain the triggers that correspond to those components
+pub fn retain_components(
+    locked: LockedApp,
+    components: &[&str],
+    validators: &[&ValidatorFn],
+) -> Result<LockedApp> {
+    App::new("unused", locked).retain_components(components, validators)
+}
+
+#[cfg(test)]
+mod test {
+    use spin_factors_test::build_locked_app;
+
+    use super::*;
+
+    fn does_nothing_validator(_: &App, _: &[&str]) -> anyhow::Result<()> {
+        Ok(())
+    }
+
+    #[tokio::test]
+    async fn test_retain_components_filtering_for_only_component_works() {
+        let manifest = toml::toml! {
+            spin_manifest_version = 2
+
+            [application]
+            name = "test-app"
+
+            [[trigger.test-trigger]]
+            component = "empty"
+
+            [component.empty]
+            source = "does-not-exist.wasm"
+        };
+        let mut locked_app = build_locked_app(&manifest).await.unwrap();
+        locked_app = retain_components(locked_app, &["empty"], &[&does_nothing_validator]).unwrap();
+        let components = locked_app
+            .components
+            .iter()
+            .map(|c| c.id.to_string())
+            .collect::<HashSet<_>>();
+        assert!(components.contains("empty"));
+        assert!(components.len() == 1);
+    }
+}

crates/app/src/lib.rs

@@ -1,7 +1,7 @@
 use std::ops::Range;
 
 use anyhow::{bail, ensure, Context};
-use spin_factors::AppComponent;
+use spin_factors::{App, AppComponent};
 use spin_locked_app::MetadataKey;
 
 const ALLOWED_HOSTS_KEY: MetadataKey<Vec<String>> = MetadataKey::new("allowed_outbound_hosts");
@@ -34,6 +34,46 @@ pub fn allowed_outbound_hosts(component: &AppComponent) -> anyhow::Result<Vec<St
     Ok(allowed_hosts)
 }
 
+/// Validates that all service chaining of an app will be satisfied by the
+/// supplied subset of components.
+///
+/// This does a best effort look up of components that are
+/// allowed to be accessed through service chaining and will error early if a
+/// component is configured to to chain to another component that is not
+/// retained. All wildcard service chaining is disallowed and all templated URLs
+/// are ignored.
+pub fn validate_service_chaining_for_components(
+    app: &App,
+    retained_components: &[&str],
+) -> anyhow::Result<()> {
+    app
+        .triggers().try_for_each(|t| {
+            let Ok(component) = t.component() else  { return Ok(()) };
+            if retained_components.contains(&component.id()) {
+            let allowed_hosts = allowed_outbound_hosts(&component).context("failed to get allowed hosts")?;
+            for host in allowed_hosts {
+                // Templated URLs are not yet resolved at this point, so ignore unresolvable URIs
+                if let Ok(uri) = host.parse::<http::Uri>() {
+                    if let Some(chaining_target) = parse_service_chaining_target(&uri) {
+                        if !retained_components.contains(&chaining_target.as_ref()) {
+                            if chaining_target == "*" {
+                                return  Err(anyhow::anyhow!("Selected component '{}' cannot use wildcard service chaining: allowed_outbound_hosts = [\"http://*.spin.internal\"]", component.id()));
+                            }
+                            return  Err(anyhow::anyhow!(
+                                "Selected component '{}' cannot use service chaining to unselected component: allowed_outbound_hosts = [\"http://{}.spin.internal\"]",
+                                component.id(), chaining_target
+                            ));
+                        }
+                    }
+                }
+            }
+        }
+        anyhow::Ok(())
+    })?;
+
+    Ok(())
+}
+
 /// An address is a url-like string that contains a host, a port, and an optional scheme
 #[derive(Eq, Debug, Clone)]
 pub struct AllowedHostConfig {
@@ -818,4 +858,90 @@ mod test {
             AllowedHostsConfig::parse(&["*://127.0.0.1/24:63551"], &dummy_resolver()).unwrap();
         assert!(allowed.allows(&OutboundUrl::parse("tcp://127.0.0.1:63551", "tcp").unwrap()));
     }
+
+    #[tokio::test]
+    async fn validate_service_chaining_for_components_fails() {
+        let manifest = toml::toml! {
+            spin_manifest_version = 2
+
+            [application]
+            name = "test-app"
+
+            [[trigger.test-trigger]]
+            component = "empty"
+
+            [component.empty]
+            source = "does-not-exist.wasm"
+            allowed_outbound_hosts = ["http://another.spin.internal"]
+
+            [[trigger.another-trigger]]
+            component = "another"
+
+            [component.another]
+            source = "does-not-exist.wasm"
+
+            [[trigger.third-trigger]]
+            component = "third"
+
+            [component.third]
+            source = "does-not-exist.wasm"
+            allowed_outbound_hosts = ["http://*.spin.internal"]
+        };
+        let locked_app = spin_factors_test::build_locked_app(&manifest)
+            .await
+            .expect("could not build locked app");
+        let app = App::new("unused", locked_app);
+        let Err(e) = validate_service_chaining_for_components(&app, &["empty"]) else {
+            panic!("Expected service chaining to non-retained component error");
+        };
+        assert_eq!(
+            e.to_string(),
+            "Selected component 'empty' cannot use service chaining to unselected component: allowed_outbound_hosts = [\"http://another.spin.internal\"]"
+        );
+        let Err(e) = validate_service_chaining_for_components(&app, &["third", "another"]) else {
+            panic!("Expected wildcard service chaining error");
+        };
+        assert_eq!(
+            e.to_string(),
+            "Selected component 'third' cannot use wildcard service chaining: allowed_outbound_hosts = [\"http://*.spin.internal\"]"
+        );
+        assert!(validate_service_chaining_for_components(&app, &["another"]).is_ok());
+    }
+
+    #[tokio::test]
+    async fn validate_service_chaining_for_components_with_templated_host_passes() {
+        let manifest = toml::toml! {
+            spin_manifest_version = 2
+
+            [application]
+            name = "test-app"
+
+            [variables]
+            host = { default = "test" }
+
+            [[trigger.test-trigger]]
+            component = "empty"
+
+            [component.empty]
+            source = "does-not-exist.wasm"
+
+            [[trigger.another-trigger]]
+            component = "another"
+
+            [component.another]
+            source = "does-not-exist.wasm"
+
+            [[trigger.third-trigger]]
+            component = "third"
+
+            [component.third]
+            source = "does-not-exist.wasm"
+            allowed_outbound_hosts = ["http://{{ host }}.spin.internal"]
+        };
+        let locked_app = spin_factors_test::build_locked_app(&manifest)
+            .await
+            .expect("could not build locked app");
+        let app = App::new("unused", locked_app);
+        assert!(validate_service_chaining_for_components(&app, &["empty", "third"]).is_ok());
+    }
 }

crates/factor-outbound-networking/src/config.rs

@@ -16,7 +16,8 @@ use std::{collections::HashMap, sync::Arc};
 
 pub use config::{
     allowed_outbound_hosts, is_service_chaining_host, parse_service_chaining_target,
-    AllowedHostConfig, AllowedHostsConfig, HostConfig, OutboundUrl, SERVICE_CHAINING_DOMAIN_SUFFIX,
+    validate_service_chaining_for_components, AllowedHostConfig, AllowedHostsConfig, HostConfig,
+    OutboundUrl, SERVICE_CHAINING_DOMAIN_SUFFIX,
 };
 
 pub use runtime_config::ComponentTlsConfigs;

crates/factor-outbound-networking/src/lib.rs

@@ -91,10 +91,14 @@ impl<T: RuntimeFactors> TestEnvironment<T> {
     }
 
     pub async fn build_locked_app(&self) -> anyhow::Result<LockedApp> {
-        let toml_str = toml::to_string(&self.manifest).context("failed serializing manifest")?;
-        let dir = tempfile::tempdir().context("failed creating tempdir")?;
-        let path = dir.path().join("spin.toml");
-        std::fs::write(&path, toml_str).context("failed writing manifest")?;
-        spin_loader::from_file(&path, FilesMountStrategy::Direct, None).await
+        build_locked_app(&self.manifest).await
     }
 }
+
+pub async fn build_locked_app(manifest: &toml::Table) -> anyhow::Result<LockedApp> {
+    let toml_str = toml::to_string(manifest).context("failed serializing manifest")?;
+    let dir = tempfile::tempdir().context("failed creating tempdir")?;
+    let path = dir.path().join("spin.toml");
+    std::fs::write(&path, toml_str).context("failed writing manifest")?;
+    spin_loader::from_file(&path, FilesMountStrategy::Direct, None).await
+}