diff --git a/Cargo.lock b/Cargo.lock index 0ccc6f3d74..5a419aec82 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -9354,10 +9354,13 @@ dependencies = [ "spin-telemetry", "spin-trigger", "spin-world", + "tempfile", "terminal", "tokio", "tokio-rustls 0.26.0", "tracing", + "wasm-compose 0.244.0", + "wasmparser 0.244.0", "wasmtime", "wasmtime-wasi", "wasmtime-wasi-http", @@ -11036,6 +11039,27 @@ dependencies = [ "wat", ] +[[package]] +name = "wasm-compose" +version = "0.244.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "92cda9c76ca8dcac01a8b497860c2cb15cd6f216dc07060517df5abbe82512ac" +dependencies = [ + "anyhow", + "heck 0.5.0", + "im-rc", + "indexmap 2.12.0", + "log", + "petgraph", + "serde", + "serde_derive", + "serde_yaml", + "smallvec", + "wasm-encoder 0.244.0", + "wasmparser 0.244.0", + "wat", +] + [[package]] name = "wasm-encoder" version = "0.41.2" diff --git a/crates/compose/src/lib.rs b/crates/compose/src/lib.rs index eb1b4edcbc..49fa5b2153 100644 --- a/crates/compose/src/lib.rs +++ b/crates/compose/src/lib.rs @@ -27,11 +27,15 @@ use wac_graph::{CompositionGraph, NodeId}; /// dependent component. Finally, the composer will export all exports from the /// dependent component to its dependents. The composer will then encode the /// composition graph into a byte array and return it. -pub async fn compose( +pub async fn compose< + L: ComponentSourceLoader, + Fut: std::future::Future, ComposeError>>, +>( loader: &L, component: &L::Component, + complicator: impl Fn(Vec) -> Fut, ) -> Result, ComposeError> { - Composer::new(loader).compose(component).await + Composer::new(loader).compose(component, complicator).await } /// A Spin component dependency. This abstracts over the metadata associated with the @@ -93,8 +97,10 @@ impl DependencyLike for spin_app::locked::LockedComponentDependency { pub trait ComponentSourceLoader { type Component: ComponentLike; type Dependency: DependencyLike; + type Source; async fn load_component_source(&self, source: &Self::Component) -> anyhow::Result>; async fn load_dependency_source(&self, source: &Self::Dependency) -> anyhow::Result>; + async fn load_source(&self, source: &Self::Source) -> anyhow::Result>; } /// A ComponentSourceLoader that loads component sources from the filesystem. @@ -104,6 +110,7 @@ pub struct ComponentSourceLoaderFs; impl ComponentSourceLoader for ComponentSourceLoaderFs { type Component = spin_app::locked::LockedComponent; type Dependency = spin_app::locked::LockedComponentDependency; + type Source = spin_app::locked::LockedComponentSource; async fn load_component_source(&self, source: &Self::Component) -> anyhow::Result> { Self::load_from_locked_source(&source.source).await @@ -112,6 +119,10 @@ impl ComponentSourceLoader for ComponentSourceLoaderFs { async fn load_dependency_source(&self, source: &Self::Dependency) -> anyhow::Result> { Self::load_from_locked_source(&source.source).await } + + async fn load_source(&self, source: &Self::Source) -> anyhow::Result> { + Self::load_from_locked_source(source).await + } } impl ComponentSourceLoaderFs { @@ -196,39 +207,47 @@ struct Composer<'a, L> { } impl<'a, L: ComponentSourceLoader> Composer<'a, L> { - async fn compose(mut self, component: &L::Component) -> Result, ComposeError> { + async fn compose, ComposeError>>>( + mut self, + component: &L::Component, + complicator: impl Fn(Vec) -> Fut, + ) -> Result, ComposeError> { let source = self .loader .load_component_source(component) .await .map_err(ComposeError::PrepareError)?; - if component.dependencies().len() == 0 { - return Ok(source); - } + let fulfilled_source = if component.dependencies().len() == 0 { + source + } else { + let (world_id, instantiation_id) = self + .register_package(component.id(), None, source) + .map_err(ComposeError::PrepareError)?; - let (world_id, instantiation_id) = self - .register_package(component.id(), None, source) - .map_err(ComposeError::PrepareError)?; + let prepared = self.prepare_dependencies(world_id, component).await?; - let prepared = self.prepare_dependencies(world_id, component).await?; + let arguments = self + .build_instantiation_arguments(world_id, prepared) + .await?; - let arguments = self - .build_instantiation_arguments(world_id, prepared) - .await?; + for (argument_name, argument) in arguments { + self.graph + .set_instantiation_argument(instantiation_id, &argument_name, argument) + .map_err(|e| ComposeError::PrepareError(e.into()))?; + } + + self.export_dependents_exports(world_id, instantiation_id) + .map_err(ComposeError::PrepareError)?; - for (argument_name, argument) in arguments { self.graph - .set_instantiation_argument(instantiation_id, &argument_name, argument) - .map_err(|e| ComposeError::PrepareError(e.into()))?; - } + .encode(Default::default()) + .map_err(|e| ComposeError::EncodeError(e.into()))? + }; - self.export_dependents_exports(world_id, instantiation_id) - .map_err(ComposeError::PrepareError)?; + let with_extras = complicator(fulfilled_source).await?; - self.graph - .encode(Default::default()) - .map_err(|e| ComposeError::EncodeError(e.into())) + Ok(with_extras) } fn new(loader: &'a L) -> Self { diff --git a/crates/environments/src/loader.rs b/crates/environments/src/loader.rs index 56b4aa5e8c..fc3bb41471 100644 --- a/crates/environments/src/loader.rs +++ b/crates/environments/src/loader.rs @@ -146,7 +146,7 @@ impl ApplicationToValidate { let loader = ComponentSourceLoader::new(&self.wasm_loader); - let wasm = spin_compose::compose(&loader, &component).await.with_context(|| format!("Spin needed to compose dependencies for {} as part of target checking, but composition failed", component.id))?; + let wasm = spin_compose::compose(&loader, &component, async |data| Ok(data)).await.with_context(|| format!("Spin needed to compose dependencies for {} as part of target checking, but composition failed", component.id))?; let host_requirements = if component.requires_service_chaining { vec!["local_service_chaining".to_string()] @@ -184,6 +184,7 @@ impl<'a> ComponentSourceLoader<'a> { impl<'a> spin_compose::ComponentSourceLoader for ComponentSourceLoader<'a> { type Component = ComponentSource<'a>; type Dependency = WrappedComponentDependency; + type Source = spin_manifest::schema::v2::ComponentSource; async fn load_component_source(&self, source: &Self::Component) -> anyhow::Result> { let path = self .wasm_loader @@ -209,6 +210,19 @@ impl<'a> spin_compose::ComponentSourceLoader for ComponentSourceLoader<'a> { .with_context(|| format!("componentizing {}", quoted_path(&path)))?; Ok(component.into()) } + + async fn load_source(&self, source: &Self::Source) -> anyhow::Result> { + let path = self + .wasm_loader + .load_component_source("bippety-boppety", source) + .await?; + let bytes = tokio::fs::read(&path) + .await + .with_context(|| format!("reading {}", quoted_path(&path)))?; + let component = spin_componentize::componentize_if_necessary(&bytes) + .with_context(|| format!("componentizing {}", quoted_path(&path)))?; + Ok(component.into()) + } } // This exists only to thwart the orphan rule diff --git a/crates/factors-executor/src/lib.rs b/crates/factors-executor/src/lib.rs index dc68ab504f..b74f13af5d 100644 --- a/crates/factors-executor/src/lib.rs +++ b/crates/factors-executor/src/lib.rs @@ -55,6 +55,7 @@ impl FactorsExecutor { runtime_config: T::RuntimeConfig, component_loader: &impl ComponentLoader, trigger_type: Option<&str>, + complicator: impl Complicator, ) -> anyhow::Result> { let configured_app = self .factors @@ -77,7 +78,7 @@ impl FactorsExecutor { for component in components { let instance_pre = component_loader - .load_instance_pre(&self.core_engine, &component) + .load_instance_pre(&self.core_engine, &component, &complicator) .await?; component_instance_pres.insert(component.id().to_string(), instance_pre); } @@ -116,6 +117,7 @@ pub trait ComponentLoader: Sync { &self, engine: &spin_core::wasmtime::Engine, component: &AppComponent, + complicator: &impl Complicator, ) -> anyhow::Result; /// Loads [`InstancePre`] for the given [`AppComponent`]. @@ -123,12 +125,51 @@ pub trait ComponentLoader: Sync { &self, engine: &spin_core::Engine>, component: &AppComponent, + complicator: &impl Complicator, ) -> anyhow::Result>> { - let component = self.load_component(engine.as_ref(), component).await?; + let component = self + .load_component(engine.as_ref(), component, complicator) + .await?; engine.instantiate_pre(&component) } } +#[async_trait] +pub trait Complicator: Send + Sync { + async fn complicate( + &self, + complications: &HashMap>, + component: Vec, + ) -> anyhow::Result>; +} + +#[async_trait] +impl Complicator for () { + async fn complicate( + &self, + complications: &HashMap>, + component: Vec, + ) -> anyhow::Result> { + if complications.is_empty() { + Ok(component) + } else { + Err(anyhow::anyhow!( + "this trigger should not have complications" + )) + } + } +} + +pub struct Complication { + pub source: spin_app::locked::LockedComponentSource, + pub data: ComplicationData, +} + +pub enum ComplicationData { + InMemory(Vec), + OnDisk(std::path::PathBuf), +} + type InstancePre = spin_core::InstancePre::InstanceState, U>>; @@ -437,7 +478,7 @@ mod tests { let executor = Arc::new(FactorsExecutor::new(engine_builder, env.factors)?); let factors_app = executor - .load_app(app, Default::default(), &DummyComponentLoader, None) + .load_app(app, Default::default(), &DummyComponentLoader, None, ()) .await?; let mut instance_builder = factors_app.prepare("empty")?; @@ -463,6 +504,7 @@ mod tests { &self, engine: &spin_core::wasmtime::Engine, _component: &AppComponent, + _complicator: &impl Complicator, ) -> anyhow::Result { Component::new(engine, "(component)") } diff --git a/crates/loader/src/local.rs b/crates/loader/src/local.rs index 4cc6a23dc6..a9fd455f7d 100644 --- a/crates/loader/src/local.rs +++ b/crates/loader/src/local.rs @@ -129,7 +129,7 @@ impl LocalLoader { drop(sloth_guard); - Ok(LockedApp { + let locked = LockedApp { spin_lock_version: Default::default(), metadata, must_understand, @@ -137,7 +137,11 @@ impl LocalLoader { variables, triggers, components, - }) + }; + + let locked = reassign_extra_components_from_triggers(locked); + + Ok(locked) } // Load the given component into a LockedComponent, ready for execution. @@ -540,6 +544,133 @@ impl LocalLoader { } } +/// We want all component/composition graph information to be in the component, +/// because the component ID is how Spin looks this stuff up. So if a trigger +/// contains a `components` table, e.g. specifying middleware, we want to move +/// that to the component. +/// +/// But it's possible to have two triggers pointing to the same primary component, +/// but with different middleware. In this case, we will synthesise a component +/// for each such trigger, with the same main configuration but with its own +/// "extra" components. +fn reassign_extra_components_from_triggers(mut locked: LockedApp) -> LockedApp { + use std::collections::{HashMap, HashSet}; + + let mut seen = HashSet::new(); + let mut disambiguator = 0; + + // We're going to be inspecting and mutating at the same time. After a while + // I gave up and: + let locked_clone = locked.clone(); + + fn component_id(trigger: &LockedTrigger) -> Option { + trigger + .trigger_config + .get("component") + .and_then(|v| v.as_str()) + .map(|s| s.to_string()) + } + fn set_component_id(app: &mut LockedApp, trigger_id: &str, component_id: &str) { + let trigger = app + .triggers + .iter_mut() + .find(|t| t.id == trigger_id) + .unwrap(); + trigger + .trigger_config + .as_object_mut() + .unwrap() + .insert("component".into(), component_id.into()); + } + fn extra_components(trigger: &LockedTrigger) -> Option<&ValuesMap> { + trigger + .trigger_config + .get("components") + .and_then(|v| v.as_object()) + } + fn has_extra_components(trigger: &LockedTrigger) -> bool { + extra_components(trigger).is_some_and(|xcs| !xcs.is_empty()) + } + fn triggers_referencing<'a>( + all_triggers: &'a [LockedTrigger], + cid: &String, + ) -> Vec<&'a LockedTrigger> { + all_triggers + .iter() + .filter(|t| component_id(t).as_ref() == Some(cid)) + .collect() + } + + let referenced_component_ids: Vec<_> = locked_clone + .triggers + .iter() + .filter_map(component_id) + .collect(); + let cid_to_triggers: HashMap<_, _> = referenced_component_ids + .iter() + .map(|cid| (cid, triggers_referencing(&locked_clone.triggers, cid))) + .collect(); + let needs_splitting = cid_to_triggers + .into_iter() + .filter(|(_, triggers)| { + triggers.len() > 1 && triggers.iter().any(|t| has_extra_components(t)) + }) + .collect::>(); + + for (cid, triggers) in needs_splitting { + for trigger in &triggers { + if !has_extra_components(trigger) { + // It's possible to have e.g. 3 triggers pointing to the same component, + // with only one enriched with middleware. The two unenriched ones can + // continue pointing to the original component. + continue; + } + let mut synthetic_id = format!("{cid}-for-{}", trigger.id); + if seen.contains(&synthetic_id) { + disambiguator += 1; + synthetic_id = format!("{synthetic_id}-d{disambiguator}"); + } + seen.insert(synthetic_id.clone()); + let mut component = locked + .components + .iter() + .find(|c| c.id == **cid) + .unwrap() + .clone(); + component.id = synthetic_id.clone(); + locked.components.push(component); + set_component_id(&mut locked, &trigger.id, &synthetic_id); + } + } + + // Now we have cloned components so that each set of { primary + trigger extras } + // can have its own component, meaning that composition graphs remain uniquely + // identified by component ID. + for trigger in &mut locked.triggers { + if let Some(extras) = extra_components(trigger) { + if let Some(component_id) = component_id(trigger) { + if let Some(component) = locked.components.iter_mut().find(|c| c.id == component_id) + { + component + .metadata + .insert("trigger-extras".into(), extras.clone().into()); + component.metadata.insert( + "resolve-extras-using".into(), + trigger.trigger_type.clone().into(), + ); + trigger + .trigger_config + .as_object_mut() + .unwrap() + .remove("components"); + } + } + } + } + + locked +} + fn explain_file_mount_source_error(e: anyhow::Error, src: &Path) -> anyhow::Error { if let Some(io_error) = e.downcast_ref::() { if io_error.kind() == std::io::ErrorKind::NotFound { @@ -918,11 +1049,13 @@ fn warn_if_component_load_slothful() -> sloth::SlothGuard { mod test { use super::*; - #[tokio::test] - async fn bad_destination_filename_is_explained() -> anyhow::Result<()> { + async fn load_test_case( + testcase_dir: &str, + manifest_file: &str, + ) -> anyhow::Result<(tempfile::TempDir, LockedApp)> { let app_root = PathBuf::from(env!("CARGO_MANIFEST_DIR")) .join("tests") - .join("file-errors"); + .join(testcase_dir); let wd = tempfile::tempdir()?; let loader = LocalLoader::new( &app_root, @@ -930,8 +1063,13 @@ mod test { None, ) .await?; - let err = loader - .load_file(app_root.join("bad.toml")) + let locked_app = loader.load_file(app_root.join(manifest_file)).await; + locked_app.map(|locked| (wd, locked)) + } + + #[tokio::test] + async fn bad_destination_filename_is_explained() -> anyhow::Result<()> { + let err = load_test_case("file-errors", "bad.toml") .await .expect_err("loader should not have succeeded"); let err_ctx = format!("{err:#}"); @@ -941,4 +1079,181 @@ mod test { ); Ok(()) } + + fn trigger_by_route<'a>(locked: &'a LockedApp, route: &str) -> &'a LockedTrigger { + fn route_of(trigger: &LockedTrigger) -> &str { + trigger + .trigger_config + .get("route") + .and_then(|v| v.as_str()) + .unwrap() + } + locked + .triggers + .iter() + .find(|t| route_of(t) == route) + .unwrap() + } + + fn component_for_route<'a>(locked: &'a LockedApp, route: &str) -> &'a LockedComponent { + let component_id = component_id(trigger_by_route(locked, route)); + locked + .components + .iter() + .find(|c| c.id == component_id) + .unwrap() + } + + fn component_id(trigger: &LockedTrigger) -> &str { + trigger + .trigger_config + .get("component") + .and_then(|v| v.as_str()) + .unwrap() + } + + fn component_trigger_extras_for_route<'a>( + locked: &'a LockedApp, + route: &str, + key: &str, + ) -> &'a Vec { + let component = component_for_route(locked, route); + let extras = component + .metadata + .get("trigger-extras") + .expect("should have had trigger-extras"); + extras + .get(key) + .expect("should have had extras for key") + .as_array() + .expect("extras for key should have been an array") + } + + #[tokio::test] + async fn unenriched_lockfile_is_unchanged() { + let (_wd, locked_app) = load_test_case("extra-components", "vanilla.toml") + .await + .unwrap(); + assert_eq!(3, locked_app.triggers.len()); + assert_eq!(2, locked_app.components.len()); + } + + #[tokio::test] + async fn enriched_lockfile_only_one_trigger_per_component_no_changes() { + let (_wd, locked_app) = load_test_case("extra-components", "inoffensive.toml") + .await + .unwrap(); + assert_eq!(2, locked_app.triggers.len()); + assert_eq!("a", component_id(trigger_by_route(&locked_app, "/a"))); + assert_eq!("b", component_id(trigger_by_route(&locked_app, "/b"))); + assert_eq!(5, locked_app.components.len()); + } + + #[tokio::test] + async fn enriched_lockfile_multiple_enriched_triggers_per_component_get_split() { + let (_wd, locked_app) = load_test_case("extra-components", "three-to-one.toml") + .await + .unwrap(); + assert_eq!(4, locked_app.triggers.len()); + // Splitting should resultm in triggers pointing to different IDs, but the same primary source + assert_ne!("a", component_id(trigger_by_route(&locked_app, "/a1"))); + assert!(component_for_route(&locked_app, "/a1") + .source + .content + .source + .as_ref() + .unwrap() + .ends_with("/a.dummy.wasm.txt")); + assert_ne!("a", component_id(trigger_by_route(&locked_app, "/a2"))); + assert!(component_for_route(&locked_app, "/a3") + .source + .content + .source + .as_ref() + .unwrap() + .ends_with("/a.dummy.wasm.txt")); + assert_ne!("a", component_id(trigger_by_route(&locked_app, "/a3"))); + assert!(component_for_route(&locked_app, "/a3") + .source + .content + .source + .as_ref() + .unwrap() + .ends_with("/a.dummy.wasm.txt")); + // Triggers that don't need splitting should be unaffected + assert_eq!("b", component_id(trigger_by_route(&locked_app, "/b"))); + // There should be new components inserted for the split + assert_eq!(8, locked_app.components.len()); + } + + #[tokio::test] + async fn enriched_lockfile_captures_composition_graph_in_split_component() { + let (_wd, locked_app) = load_test_case("extra-components", "three-to-one.toml") + .await + .unwrap(); + assert_eq!(4, locked_app.triggers.len()); + + let a1_mw = component_trigger_extras_for_route(&locked_app, "/a1", "middleware"); + assert_eq!(2, a1_mw.len()); + assert_eq!( + "m1", + a1_mw[0].as_str().expect("a1 mw should have been strings") + ); + assert_eq!( + "m2", + a1_mw[1].as_str().expect("a1 mw should have been strings") + ); + + let a2_mw = component_trigger_extras_for_route(&locked_app, "/a2", "middleware"); + assert_eq!(2, a2_mw.len()); + assert_eq!( + "m2", + a2_mw[0].as_str().expect("a2 mw should have been strings") + ); + assert_eq!( + "m3", + a2_mw[1].as_str().expect("a2 mw should have been strings") + ); + + let a3_mw = component_trigger_extras_for_route(&locked_app, "/a3", "middleware"); + assert_eq!(3, a3_mw.len()); + assert_eq!( + "m3", + a3_mw[0].as_str().expect("a3 mw should have been strings") + ); + assert_eq!( + "m2", + a3_mw[1].as_str().expect("a3 mw should have been strings") + ); + assert_eq!( + "m1", + a3_mw[2].as_str().expect("a3 mw should have been strings") + ); + + // Unsplit things should still get the shunt + let b_mw = component_trigger_extras_for_route(&locked_app, "/b", "middleware"); + assert_eq!(2, b_mw.len()); + assert_eq!( + "m1", + b_mw[0].as_str().expect("b mw should have been strings") + ); + assert_eq!( + "m3", + b_mw[1].as_str().expect("b mw should have been strings") + ); + } + + #[tokio::test] + async fn extras_moved_off_trigger() { + let (_wd, locked_app) = load_test_case("extra-components", "three-to-one.toml") + .await + .unwrap(); + assert_eq!(4, locked_app.triggers.len()); + + for t in &locked_app.triggers { + assert!(t.trigger_config.get("components").is_none()); + } + + println!("{}", serde_json::to_string_pretty(&locked_app).unwrap()); + } } diff --git a/crates/loader/tests/extra-components/a.dummy.wasm.txt b/crates/loader/tests/extra-components/a.dummy.wasm.txt new file mode 100644 index 0000000000..00af6f9155 --- /dev/null +++ b/crates/loader/tests/extra-components/a.dummy.wasm.txt @@ -0,0 +1 @@ +This file needs to exist for manifests to validate, but is never used. \ No newline at end of file diff --git a/crates/loader/tests/extra-components/b.dummy.wasm.txt b/crates/loader/tests/extra-components/b.dummy.wasm.txt new file mode 100644 index 0000000000..00af6f9155 --- /dev/null +++ b/crates/loader/tests/extra-components/b.dummy.wasm.txt @@ -0,0 +1 @@ +This file needs to exist for manifests to validate, but is never used. \ No newline at end of file diff --git a/crates/loader/tests/extra-components/c.dummy.wasm.txt b/crates/loader/tests/extra-components/c.dummy.wasm.txt new file mode 100644 index 0000000000..00af6f9155 --- /dev/null +++ b/crates/loader/tests/extra-components/c.dummy.wasm.txt @@ -0,0 +1 @@ +This file needs to exist for manifests to validate, but is never used. \ No newline at end of file diff --git a/crates/loader/tests/extra-components/inoffensive.toml b/crates/loader/tests/extra-components/inoffensive.toml new file mode 100644 index 0000000000..dfac5253e8 --- /dev/null +++ b/crates/loader/tests/extra-components/inoffensive.toml @@ -0,0 +1,29 @@ +spin_manifest_version = 2 + +[application] +name = "file-errors" + +[[trigger.http]] +route = "/a" +component = "a" +components.middleware = ["m1", "m2"] + +[[trigger.http]] +route = "/b" +component = "b" +components.middleware = ["m1", "m3"] + +[component.a] +source = "a.dummy.wasm.txt" + +[component.b] +source = "b.dummy.wasm.txt" + +[component.m1] +source = "a.dummy.wasm.txt" + +[component.m2] +source = "b.dummy.wasm.txt" + +[component.m3] +source = "c.dummy.wasm.txt" diff --git a/crates/loader/tests/extra-components/three-to-one.toml b/crates/loader/tests/extra-components/three-to-one.toml new file mode 100644 index 0000000000..1c7c39e360 --- /dev/null +++ b/crates/loader/tests/extra-components/three-to-one.toml @@ -0,0 +1,39 @@ +spin_manifest_version = 2 + +[application] +name = "file-errors" + +[[trigger.http]] +route = "/a1" +component = "a" +components.middleware = ["m1", "m2"] + +[[trigger.http]] +route = "/a2" +component = "a" +components.middleware = ["m2", "m3"] + +[[trigger.http]] +route = "/a3" +component = "a" +components.middleware = ["m3", "m2", "m1"] + +[[trigger.http]] +route = "/b" +component = "b" +components.middleware = ["m1", "m3"] + +[component.a] +source = "a.dummy.wasm.txt" + +[component.b] +source = "b.dummy.wasm.txt" + +[component.m1] +source = "a.dummy.wasm.txt" + +[component.m2] +source = "b.dummy.wasm.txt" + +[component.m3] +source = "c.dummy.wasm.txt" diff --git a/crates/loader/tests/extra-components/vanilla.toml b/crates/loader/tests/extra-components/vanilla.toml new file mode 100644 index 0000000000..fd2b4a2018 --- /dev/null +++ b/crates/loader/tests/extra-components/vanilla.toml @@ -0,0 +1,22 @@ +spin_manifest_version = 2 + +[application] +name = "file-errors" + +[[trigger.http]] +route = "/a1" +component = "a" + +[[trigger.http]] +route = "/a2" +component = "a" + +[[trigger.http]] +route = "/b" +component = "b" + +[component.a] +source = "a.dummy.wasm.txt" + +[component.b] +source = "b.dummy.wasm.txt" diff --git a/crates/manifest/src/normalize.rs b/crates/manifest/src/normalize.rs index b421433a50..c7deb303a7 100644 --- a/crates/manifest/src/normalize.rs +++ b/crates/manifest/src/normalize.rs @@ -19,54 +19,49 @@ pub fn normalize_manifest(manifest: &mut AppManifest) -> anyhow::Result<()> { fn normalize_inline_components(manifest: &mut AppManifest) { // Normalize inline components let components = &mut manifest.components; + let mut counter = 1; + + let mut normalize_spec = |spec: &mut ComponentSpec, trigger_id: &str, is_primary: bool| { + if !matches!(spec, ComponentSpec::Inline(_)) { + return; + }; + + let inline_id = { + // Try a "natural" component ID... + let mut id = KebabId::try_from(format!("{trigger_id}-component")); + // ...falling back to a counter-based component ID + if !is_primary || id.is_err() || components.contains_key(id.as_ref().unwrap()) { + id = Ok(loop { + let id = KebabId::try_from(format!("inline-component{counter}")).unwrap(); + if !components.contains_key(&id) { + break id; + } + counter += 1; + }); + } + id.unwrap() + }; + + // Replace the inline component with a reference... + let inline_spec = std::mem::replace(spec, ComponentSpec::Reference(inline_id.clone())); + let ComponentSpec::Inline(component) = inline_spec else { + unreachable!(); + }; + // ...moving the inline component into the top-level components map. + components.insert(inline_id.clone(), *component); + }; for trigger in manifest.triggers.values_mut().flatten() { let trigger_id = &trigger.id; - let component_specs = trigger - .component - .iter_mut() - .chain( - trigger - .components - .values_mut() - .flat_map(|specs| specs.0.iter_mut()), - ) - .collect::>(); - let multiple_components = component_specs.len() > 1; + if let Some(primary_component) = trigger.component.as_mut() { + normalize_spec(primary_component, trigger_id, true); + } - let mut counter = 1; - for spec in component_specs { - if !matches!(spec, ComponentSpec::Inline(_)) { - continue; - }; - - let inline_id = { - // Try a "natural" component ID... - let mut id = KebabId::try_from(format!("{trigger_id}-component")); - // ...falling back to a counter-based component ID - if multiple_components - || id.is_err() - || components.contains_key(id.as_ref().unwrap()) - { - id = Ok(loop { - let id = KebabId::try_from(format!("inline-component{counter}")).unwrap(); - if !components.contains_key(&id) { - break id; - } - counter += 1; - }); - } - id.unwrap() - }; - - // Replace the inline component with a reference... - let inline_spec = std::mem::replace(spec, ComponentSpec::Reference(inline_id.clone())); - let ComponentSpec::Inline(component) = inline_spec else { - unreachable!(); - }; - // ...moving the inline component into the top-level components map. - components.insert(inline_id.clone(), *component); + for complications in trigger.components.values_mut() { + for spec in &mut complications.0 { + normalize_spec(spec, trigger_id, false); + } } } } diff --git a/crates/manifest/src/schema/v2.rs b/crates/manifest/src/schema/v2.rs index 6504798279..3ae1b52ae9 100644 --- a/crates/manifest/src/schema/v2.rs +++ b/crates/manifest/src/schema/v2.rs @@ -132,7 +132,8 @@ pub struct Trigger { /// Learn more: https://spinframework.dev/triggers#triggers-and-components #[serde(default, skip_serializing_if = "Option::is_none")] pub component: Option, - /// Reserved for future use. + /// Additional components used when the trigger occurs. + /// The meaning of entries in this table is trigger-specific. /// /// `components = { ... }` #[serde(default, skip_serializing_if = "Map::is_empty")] @@ -664,11 +665,26 @@ mod one_or_many { D: Deserializer<'de>, { let value = toml::Value::deserialize(deserializer)?; - if let Ok(val) = T::deserialize(value.clone()) { - Ok(vec![val]) + if let Some(arr) = value.as_array() { + arr.iter() + .map(|v| T::deserialize(v.clone())) + .collect::, _>>() + .map_err(serde::de::Error::custom) } else { - Vec::deserialize(value).map_err(serde::de::Error::custom) + T::deserialize(value) + .map(|v| vec![v]) + .map_err(serde::de::Error::custom) } + // the follow is the original code but serde makes complete garbage of it, it treats an array as a sequence of fields to be assigned to a Component a la `Inline(Component { source: Local("middlybiddly"), description: "middlybiddly2", variables: {}, environment: {}...` + // absolutely batshit + // but serde is well-tested, well-used software so am I just doing it wrong + // if let Ok(val) = T::deserialize(value.clone()) { + // println!(" * it's a single"); + // Ok(vec![val]) + // } else { + // println!(" * it's a vec"); + // Vec::deserialize(value).map_err(serde::de::Error::custom) + // } } } diff --git a/crates/oci/src/client.rs b/crates/oci/src/client.rs index 9b08ad5e37..96a490b886 100644 --- a/crates/oci/src/client.rs +++ b/crates/oci/src/client.rs @@ -365,15 +365,25 @@ impl Client { let mut components = Vec::new(); let mut layers = Vec::new(); + let temp_dir = tempfile::tempdir().unwrap(); + let working_dir = temp_dir.path(); + let locked_url = write_locked_app(&locked, working_dir).await.unwrap(); + for mut c in locked.components { - let composed = spin_compose::compose(&ComponentSourceLoaderFs, &c) - .await - .with_context(|| { - format!("failed to resolve dependencies for component {:?}", c.id) - })?; + let extras = c.metadata.get("trigger-extras").and_then(|e| e.as_object()); + + let composed = if extras.is_none_or(|e| e.is_empty()) { + spin_compose::compose(&ComponentSourceLoaderFs, &c, async |a| Ok(a)).await? + } else { + // There are complications: we need to hand off to the trigger + // to do the composition. + precompose_using_trigger(&c, &locked_url, working_dir).await? + }; + let layer = ImageLayer::new(composed, WASM_LAYER_MEDIA_TYPE.to_string(), None); c.source.content = self.content_ref_for_layer(&layer); c.dependencies.clear(); + c.metadata.remove("trigger-extras"); layers.push(layer); c.files = self @@ -382,6 +392,26 @@ impl Client { components.push(c); } + // Copied from `spin up` + async fn write_locked_app( + locked_app: &LockedApp, + working_dir: &Path, + ) -> Result { + let locked_path = working_dir.join("spin.lock"); + let locked_app_contents = + serde_json::to_vec_pretty(&locked_app).context("failed to serialize locked app")?; + tokio::fs::write(&locked_path, locked_app_contents) + .await + .with_context(|| format!("failed to write {}", quoted_path(&locked_path)))?; + let locked_url = Url::from_file_path(&locked_path) + .map_err(|_| { + anyhow::anyhow!("cannot convert to file URL: {}", quoted_path(&locked_path)) + })? + .to_string(); + + Ok(locked_url) + } + Ok((layers, components)) } @@ -907,6 +937,60 @@ fn add_inferred(map: &mut BTreeMap, key: &str, value: Option Result, spin_compose::ComposeError> { + use spin_compose::ComposeError; + + let Some(resolve_extras_using) = c + .metadata + .get("resolve-extras-using") + .and_then(|v| v.as_str()) + else { + return spin_compose::compose(&ComponentSourceLoaderFs, c, async |a| Ok(a)).await; + }; + + let resolver_subcmd = match resolve_extras_using { + "http" | "redis" => vec!["trigger".into(), resolve_extras_using.into()], + _ => vec![format!("trigger-{resolve_extras_using}")], + }; + + let mut cmd = tokio::process::Command::new(std::env::current_exe().unwrap()); + cmd.args(resolver_subcmd) + .args(["--precompose-only", "--precompose-component-id"]) + .arg(&c.id) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::inherit()) + .env("SPIN_PLUGINS_SUPPRESS_COMPATIBILITY_WARNINGS", "1") + .env(SPIN_LOCKED_URL, locked_url) + .env(SPIN_WORKING_DIR, working_dir); + + let child = cmd + .spawn() + .map_err(|e| ComposeError::PrepareError(e.into()))?; + + let trigger_out = child + .wait_with_output() + .await + .map_err(|e| ComposeError::PrepareError(e.into()))?; + + if !trigger_out.status.success() { + return Err(ComposeError::PrepareError(anyhow::anyhow!( + "unable to compose additional components for {} using `{}`", + c.id, + resolve_extras_using + ))); + } + + let complicated = trigger_out.stdout; + Ok(complicated) +} + /// Takes a relative path and turns it into a format that is safe /// for putting into a registry where it might end up on any host. #[cfg(target_os = "windows")] diff --git a/crates/trigger-http/Cargo.toml b/crates/trigger-http/Cargo.toml index ee595bd55b..477e1b678a 100644 --- a/crates/trigger-http/Cargo.toml +++ b/crates/trigger-http/Cargo.toml @@ -24,6 +24,7 @@ serde = { workspace = true } serde_json = { workspace = true } spin-app = { path = "../app" } spin-core = { path = "../core" } +spin-factor-otel = { path = "../factor-otel" } spin-factor-outbound-http = { path = "../factor-outbound-http" } spin-factor-outbound-networking = { path = "../factor-outbound-networking" } spin-factor-wasi = { path = "../factor-wasi" } @@ -33,11 +34,13 @@ spin-http = { path = "../http" } spin-telemetry = { path = "../telemetry" } spin-trigger = { path = "../trigger" } spin-world = { path = "../world" } -spin-factor-otel = { path = "../factor-otel" } +tempfile = { workspace = true } terminal = { path = "../terminal" } tokio = { workspace = true, features = ["full"] } tokio-rustls = { workspace = true } tracing = { workspace = true } +wasm-compose = "0.244" +wasmparser = "0.244" wasmtime = { workspace = true } wasmtime-wasi = { workspace = true } wasmtime-wasi-http = { workspace = true } diff --git a/crates/trigger-http/src/lib.rs b/crates/trigger-http/src/lib.rs index aea876b3ae..99850f66b4 100644 --- a/crates/trigger-http/src/lib.rs +++ b/crates/trigger-http/src/lib.rs @@ -2,6 +2,7 @@ mod headers; mod instrument; +mod middleware; mod outbound_http; mod server; mod spin; @@ -297,6 +298,10 @@ impl Trigger for HttpTrigger { Ok(()) } + fn complicator() -> impl spin_factors_executor::Complicator { + middleware::HttpMiddlewareComplicator + } + fn supported_host_requirements() -> Vec<&'static str> { vec![spin_app::locked::SERVICE_CHAINING_KEY] } diff --git a/crates/trigger-http/src/middleware.rs b/crates/trigger-http/src/middleware.rs new file mode 100644 index 0000000000..4061bb95ef --- /dev/null +++ b/crates/trigger-http/src/middleware.rs @@ -0,0 +1,176 @@ +use anyhow::{bail, Context}; +use wasm_compose::{ + composer::{ComponentComposer, ROOT_COMPONENT_NAME}, + config::{Config, Dependency, Instantiation, InstantiationArg}, +}; + +use std::collections::HashMap; +use std::path::PathBuf; + +use spin_factors_executor::{Complication, ComplicationData, Complicator}; + +#[derive(Default)] +pub(crate) struct HttpMiddlewareComplicator; + +#[spin_core::async_trait] +impl Complicator for HttpMiddlewareComplicator { + async fn complicate( + &self, + complications: &HashMap>, + component: Vec, + ) -> anyhow::Result> { + let Some(middlewares) = complications.get("middleware") else { + return Ok(component); + }; + if complications.len() > 1 { + bail!("the HTTP trigger's only allowed complication is `middleware`"); + } + if middlewares.is_empty() { + return Ok(component); + } + + let middleware_blobs = middlewares.iter().map(|cm| &cm.data); + compose_middlewares(component, middleware_blobs).await + } +} + +async fn compose_middlewares<'a>( + primary: Vec, + middleware_blobs: impl Iterator, +) -> anyhow::Result> { + const MW_NEXT_INBOUND: &str = "wasi:http/handler@0.3.0-rc-2026-01-06"; + const MW_NEXT_OUTBOUND: &str = "wasi:http/handler@0.3.0-rc-2026-01-06"; + + // TODO: I wonder if we can shorten/simplify this (and avoid all the tempfile + // crap) with a sequence of `wac_graph::plug`s now inbound and outbound are the same? + + // `wasm-tools compose` relies on the components it's composing being in + // files, so write all any in-memory blobs to a temp dir. + let temp_dir = tempfile::tempdir().context("creating working dir for middleware")?; + let temp_path = temp_dir.path(); + + let mut mw_blob_paths = write_blobs_to(primary, middleware_blobs, temp_path).await?; + + // We will use the first item in the chain as the composition root. + // This means it does not get mapped in the list of dependencies, but + // is provided directly to the ComponentComposer. So we set it + // aside for now. + let first = mw_blob_paths.remove(0); + let last_index = mw_blob_paths.len() - 1; // points to the end of the composition chain (which is the primary) + + // All blobs except the (already set aside) root are mapped in via dependencies + let dependencies = mw_blob_paths + .iter() + .enumerate() + .map(|(index, mw_path)| { + ( + dep_ref(index), + Dependency { + path: mw_path.clone(), + }, + ) + }) + .collect(); + + let mut config = Config { + skip_validation: true, + dependencies, + ..Default::default() + }; + + // The composition root hooks up to the start of the (remaining) + // pipeline (which we will soon create as inst ref 0). + config.instantiations.insert( + ROOT_COMPONENT_NAME.to_owned(), + Instantiation { + dependency: None, + arguments: [( + MW_NEXT_OUTBOUND.to_owned(), + InstantiationArg { + instance: inst_ref(0), + export: Some(MW_NEXT_INBOUND.to_owned()), + }, + )] + .into(), + }, + ); + + // Go through the remaining items of of the pipeline except for the last. + // For each, create an instantiation (named by index) of the + // middleware at hand with its 'next' import hooked up to the next instance's (named by inst ref) handler export. + // + // The range is deliberately non-inclusive: the last item needs different + // handling, because we do *not* want to fulfil its dependencies. + for index in 0..last_index { + let next_inst_ref = InstantiationArg { + instance: inst_ref(index + 1), + export: Some(MW_NEXT_INBOUND.to_owned()), + }; + let inst = Instantiation { + dependency: Some(dep_ref(index)), + arguments: [(MW_NEXT_OUTBOUND.to_owned(), next_inst_ref)] + .into_iter() + .collect(), + }; + config.instantiations.insert(inst_ref(index), inst); + } + + // Create an instantiation of the primary + // (which is the last thing in the pipeline) with its imports open. + let primary = Instantiation { + dependency: Some(dep_ref(last_index)), + arguments: Default::default(), + }; + config.instantiations.insert(inst_ref(last_index), primary); + + // Run the composition, using the previously set aside first item the composition root. + let composer = ComponentComposer::new(&first, &config); + + composer.compose() +} + +/// The return vector has the written-out paths in chain order: +/// the middlewares in order, followed by the primary. This matters! +async fn write_blobs_to( + primary: Vec, + middleware_blobs: impl Iterator, + temp_path: &std::path::Path, +) -> anyhow::Result> { + let mut mw_blob_paths = vec![]; + + for (mw_index, mw_blob) in middleware_blobs.enumerate() { + let mw_blob_path = match mw_blob { + ComplicationData::InMemory(data) => { + let mw_blob_path = temp_path.join(format!("middleware-blob-idx{mw_index}.wasm")); + tokio::fs::write(&mw_blob_path, data) + .await + .context("writing middleware blob to temp dir")?; + mw_blob_path + } + ComplicationData::OnDisk(path) => path.clone(), + }; + mw_blob_paths.push(mw_blob_path); + } + + let primary_path = temp_path.join("primary.wasm"); + tokio::fs::write(&primary_path, primary) + .await + .context("writing component to temp dir for middleware composition")?; + mw_blob_paths.push(primary_path); + + Ok(mw_blob_paths) +} + +/// The identifier in the composition graph for the index'th item +/// in the 'middlewares + primary' list. The config maps these +/// identifiers to physical files. +fn dep_ref(index: usize) -> String { + format!("mw{index}") +} + +/// The identifier in the composition graph for the instantiation of the +/// index'th item in the 'middlewares + primary' list. This is used when +/// hooking up the imports of one instantiation to the exports of another. +fn inst_ref(index: usize) -> String { + format!("mw{index}inst") +} diff --git a/crates/trigger/src/cli.rs b/crates/trigger/src/cli.rs index 00f8016ddb..c42f51ba5b 100644 --- a/crates/trigger/src/cli.rs +++ b/crates/trigger/src/cli.rs @@ -137,6 +137,11 @@ pub struct FactorsTriggerCommand, B: RuntimeFactorsBuilde #[clap(long = "launch-metadata-only", hide = true)] pub launch_metadata_only: bool, + + #[clap(long = "precompose-only", hide = true)] + pub precompose_only: bool, + #[clap(long = "precompose-component-id", hide = true)] + pub precompose_component_id: Option, } #[cfg(feature = "experimental-wasm-features")] @@ -209,12 +214,38 @@ impl, B: RuntimeFactorsBuilder> FactorsTriggerCommand = TriggerAppBuilder::new(trigger); let config = builder.engine_config(); @@ -373,7 +404,13 @@ impl, B: RuntimeFactorsBuilder> TriggerAppBuilder { let configured_app = { let _sloth_guard = warn_if_wasm_build_slothful(); executor - .load_app(app, runtime_config.into(), loader, Some(T::TYPE)) + .load_app( + app, + runtime_config.into(), + loader, + Some(T::TYPE), + T::complicator(), + ) .await? }; diff --git a/crates/trigger/src/lib.rs b/crates/trigger/src/lib.rs index daefa659b2..264abccb94 100644 --- a/crates/trigger/src/lib.rs +++ b/crates/trigger/src/lib.rs @@ -50,6 +50,12 @@ pub trait Trigger: Sized + Send { Ok(()) } + /// An object which composes extras onto the primary component. + /// TODO: the combination of functions and objects and traits is a bit funny and we may/should be able to streamline it. + fn complicator() -> impl spin_factors_executor::Complicator { + // the do-nothing unit complicator + } + /// Update the [`Linker`] for this trigger. fn add_to_linker( &mut self, diff --git a/crates/trigger/src/loader.rs b/crates/trigger/src/loader.rs index 5f7e6bab74..6164b2e980 100644 --- a/crates/trigger/src/loader.rs +++ b/crates/trigger/src/loader.rs @@ -3,6 +3,7 @@ use spin_common::{ui::quoted_path, url::parse_file_url}; use spin_compose::ComponentSourceLoaderFs; use spin_core::{async_trait, wasmtime, Component}; use spin_factors::{AppComponent, RuntimeFactors}; +use spin_factors_executor::ComplicationData; #[derive(Default)] pub struct ComponentLoader { @@ -65,6 +66,42 @@ impl ComponentLoader { } } } + + pub(crate) async fn load_composed( + &self, + component: &AppComponent<'_>, + complicator: &impl spin_factors_executor::Complicator, + ) -> anyhow::Result> { + let loader = ComponentSourceLoaderFs; + + let empty: serde_json::Map = Default::default(); + let extras = component + .locked + .metadata + .get("trigger-extras") + .and_then(|v| v.as_object()) + .unwrap_or(&empty); + + let complications = load_complications(component.app, extras, &loader).await?; + + let complicate = async |c: Vec| { + complicator + .complicate(&complications, c) + .await + .map_err(spin_compose::ComposeError::PrepareError) + }; + + let composed = spin_compose::compose(&loader, component.locked, complicate) + .await + .with_context(|| { + format!( + "failed to resolve dependencies for component {:?}", + component.locked.id + ) + })?; + + Ok(composed) + } } #[async_trait] @@ -73,6 +110,7 @@ impl spin_factors_executor::ComponentLoader for Comp &self, engine: &wasmtime::Engine, component: &AppComponent, + complicator: &impl spin_factors_executor::Complicator, ) -> anyhow::Result { let source = component .source() @@ -89,16 +127,68 @@ impl spin_factors_executor::ComponentLoader for Comp .with_context(|| format!("error deserializing component from {path:?}")); } - let composed = spin_compose::compose(&ComponentSourceLoaderFs, component.locked) - .await - .with_context(|| { - format!( - "failed to resolve dependencies for component {:?}", - component.locked.id - ) - })?; + let composed = self.load_composed(component, complicator).await?; spin_core::Component::new(engine, composed) .with_context(|| format!("failed to compile component from {}", quoted_path(&path))) } } + +pub(crate) async fn load_complications( + app: &spin_app::App, + extras: &serde_json::Map, + loader: &spin_compose::ComponentSourceLoaderFs, +) -> Result< + std::collections::HashMap>, + anyhow::Error, +> { + use spin_factors_executor::Complication; + use std::collections::HashMap; + + let mut complications = HashMap::with_capacity(extras.len()); + + for (role, role_components) in extras { + let components = role_components + .as_array() + .context("extra components should have been an array")?; + let mut complications_for_role = Vec::with_capacity(components.len()); + + for component_ref in components { + let component_ref = component_ref + .as_str() + .context("middleware should be strings curently")?; + let reffed_component = app + .get_component(component_ref) + .context("no such component")?; + let component_src = reffed_component.source().clone(); + let data = load_complication_data(loader, &component_src).await?; + complications_for_role.push(Complication { + data, + source: component_src, + }); + } + complications.insert(role.clone(), complications_for_role); + } + + Ok(complications) +} + +async fn load_complication_data( + loader: &ComponentSourceLoaderFs, + source: &spin_app::locked::LockedComponentSource, +) -> anyhow::Result { + use spin_compose::ComponentSourceLoader; + + if let Some(path) = source + .content + .source + .as_ref() + .and_then(|url| parse_file_url(url).ok()) + { + Ok(ComplicationData::OnDisk(path)) + } else { + Ok(ComplicationData::InMemory( + loader.load_source(source).await?, + )) + } +}