From 654d07b31d97aba4ae0d5ffbc2e45d247b482bf9 Mon Sep 17 00:00:00 2001 From: itowlson Date: Tue, 11 Nov 2025 13:13:43 +1300 Subject: [PATCH 1/3] HTTP middleware Signed-off-by: itowlson --- Cargo.lock | 24 ++ crates/compose/src/lib.rs | 63 ++-- crates/environments/src/loader.rs | 16 +- crates/factors-executor/src/lib.rs | 48 ++- crates/loader/src/local.rs | 329 +++++++++++++++++- .../tests/extra-components/a.dummy.wasm.txt | 1 + .../tests/extra-components/b.dummy.wasm.txt | 1 + .../tests/extra-components/c.dummy.wasm.txt | 1 + .../tests/extra-components/inoffensive.toml | 29 ++ .../tests/extra-components/three-to-one.toml | 39 +++ .../tests/extra-components/vanilla.toml | 22 ++ crates/manifest/src/normalize.rs | 126 ++++--- crates/manifest/src/schema/v2.rs | 21 +- crates/oci/src/client.rs | 99 +++++- crates/trigger-http/Cargo.toml | 5 +- crates/trigger-http/src/lib.rs | 5 + crates/trigger-http/src/middleware.rs | 176 ++++++++++ crates/trigger/src/cli.rs | 60 +++- crates/trigger/src/lib.rs | 6 + crates/trigger/src/loader.rs | 82 ++++- 20 files changed, 1070 insertions(+), 83 deletions(-) create mode 100644 crates/loader/tests/extra-components/a.dummy.wasm.txt create mode 100644 crates/loader/tests/extra-components/b.dummy.wasm.txt create mode 100644 crates/loader/tests/extra-components/c.dummy.wasm.txt create mode 100644 crates/loader/tests/extra-components/inoffensive.toml create mode 100644 crates/loader/tests/extra-components/three-to-one.toml create mode 100644 crates/loader/tests/extra-components/vanilla.toml create mode 100644 crates/trigger-http/src/middleware.rs 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..6efc5407f6 100644 --- a/crates/manifest/src/normalize.rs +++ b/crates/manifest/src/normalize.rs @@ -19,55 +19,95 @@ 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); + } } + + // 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; + + // 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); + // } } } diff --git a/crates/manifest/src/schema/v2.rs b/crates/manifest/src/schema/v2.rs index 6504798279..3a809b0f8a 100644 --- a/crates/manifest/src/schema/v2.rs +++ b/crates/manifest/src/schema/v2.rs @@ -664,11 +664,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..fbfc1a3526 100644 --- a/crates/oci/src/client.rs +++ b/crates/oci/src/client.rs @@ -365,8 +365,15 @@ 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) + let complicate = + |data: Vec| compose_trigger_extras(&c, &locked_url, working_dir, data); + + let composed = spin_compose::compose(&ComponentSourceLoaderFs, &c, complicate) .await .with_context(|| { format!("failed to resolve dependencies for component {:?}", c.id) @@ -374,6 +381,7 @@ impl Client { 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 +390,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 +935,75 @@ 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 Result::<_, ComposeError>::Ok(data); + }; + + 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(["--resolve-extras-only", "--resolve-extras-component-id"]) + .arg(&c.id) + .stdin(std::process::Stdio::piped()) + .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 mut child = cmd + .spawn() + .map_err(|e| ComposeError::PrepareError(e.into()))?; + + use tokio::io::AsyncWriteExt; + + let mut input = child.stdin.take().unwrap(); + input + .write_all(&data) + .await + .map_err(|e| ComposeError::PrepareError(e.into()))?; + input + .flush() + .await + .map_err(|e| ComposeError::PrepareError(e.into()))?; + drop(input); + + 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..9d7667d447 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 = "resolve-extras-only", hide = true)] + pub resolve_extras_only: bool, + #[clap(long = "resolve-extras-component-id", hide = true)] + pub resolve_extras_component_id: Option, } #[cfg(feature = "experimental-wasm-features")] @@ -215,6 +220,53 @@ impl, B: RuntimeFactorsBuilder> FactorsTriggerCommand = TriggerAppBuilder::new(trigger); let config = builder.engine_config(); @@ -373,7 +425,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..e9052c7883 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 { @@ -73,6 +74,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,7 +91,26 @@ impl spin_factors_executor::ComponentLoader for Comp .with_context(|| format!("error deserializing component from {path:?}")); } - let composed = spin_compose::compose(&ComponentSourceLoaderFs, component.locked) + 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!( @@ -102,3 +123,62 @@ impl spin_factors_executor::ComponentLoader for Comp .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?, + )) + } +} From 816226ba30bda8e0d789ff15b38dba405b839050 Mon Sep 17 00:00:00 2001 From: itowlson Date: Tue, 17 Feb 2026 13:02:01 +1300 Subject: [PATCH 2/3] Interim commit for transfer Signed-off-by: itowlson --- crates/manifest/src/normalize.rs | 45 ------------ crates/manifest/src/schema/v2.rs | 3 +- crates/oci/src/client.rs | 116 ++++++++++++++++++++++++------- crates/trigger/src/cli.rs | 50 +++---------- crates/trigger/src/loader.rs | 82 +++++++++++++++------- 5 files changed, 161 insertions(+), 135 deletions(-) diff --git a/crates/manifest/src/normalize.rs b/crates/manifest/src/normalize.rs index 6efc5407f6..c7deb303a7 100644 --- a/crates/manifest/src/normalize.rs +++ b/crates/manifest/src/normalize.rs @@ -63,51 +63,6 @@ fn normalize_inline_components(manifest: &mut AppManifest) { normalize_spec(spec, trigger_id, false); } } - - // 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; - - // 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); - // } } } diff --git a/crates/manifest/src/schema/v2.rs b/crates/manifest/src/schema/v2.rs index 3a809b0f8a..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")] diff --git a/crates/oci/src/client.rs b/crates/oci/src/client.rs index fbfc1a3526..69f46e9e95 100644 --- a/crates/oci/src/client.rs +++ b/crates/oci/src/client.rs @@ -370,14 +370,26 @@ impl Client { let locked_url = write_locked_app(&locked, working_dir).await.unwrap(); for mut c in locked.components { - let complicate = - |data: Vec| compose_trigger_extras(&c, &locked_url, working_dir, data); + // TODO: what if, instead of having a separate complicate that + // called the trigger, we had the trigger do the full compose + // (in the presence of extras)? The trigger already has logic + // for that... + 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 { + compose_trigger_extras_2(&c, &locked_url, working_dir).await? + }; - let composed = spin_compose::compose(&ComponentSourceLoaderFs, &c, complicate) - .await - .with_context(|| { - format!("failed to resolve dependencies for component {:?}", c.id) - })?; + // let complicate = + // |data: Vec| compose_trigger_extras(&c, &locked_url, working_dir, data); + + // let composed = spin_compose::compose(&ComponentSourceLoaderFs, &c, complicate) + // .await + // .with_context(|| { + // format!("failed to resolve dependencies for component {:?}", c.id) + // })?; let layer = ImageLayer::new(composed, WASM_LAYER_MEDIA_TYPE.to_string(), None); c.source.content = self.content_ref_for_layer(&layer); c.dependencies.clear(); @@ -938,11 +950,76 @@ 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 Result::<_, ComposeError>::Ok(data); +// }; + +// 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(["--resolve-extras-only", "--resolve-extras-component-id"]) +// .arg(&c.id) +// // .stdin(std::process::Stdio::piped()) +// .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()))?; + +// // use tokio::io::AsyncWriteExt; + +// // let mut input = child.stdin.take().unwrap(); +// // input +// // .write_all(&data) +// // .await +// // .map_err(|e| ComposeError::PrepareError(e.into()))?; +// // input +// // .flush() +// // .await +// // .map_err(|e| ComposeError::PrepareError(e.into()))?; +// // drop(input); + +// 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) +// } + +async fn compose_trigger_extras_2( c: &LockedComponent, locked_url: &str, working_dir: &Path, - data: Vec, ) -> Result, spin_compose::ComposeError> { use spin_compose::ComposeError; @@ -951,7 +1028,8 @@ async fn compose_trigger_extras( .get("resolve-extras-using") .and_then(|v| v.as_str()) else { - return Result::<_, ComposeError>::Ok(data); + return spin_compose::compose(&ComponentSourceLoaderFs, &c, async |a| Ok(a)).await; + // return Result::<_, ComposeError>::Ok(data); }; let resolver_subcmd = match resolve_extras_using { @@ -961,32 +1039,18 @@ async fn compose_trigger_extras( let mut cmd = tokio::process::Command::new(std::env::current_exe().unwrap()); cmd.args(resolver_subcmd) - .args(["--resolve-extras-only", "--resolve-extras-component-id"]) + .args(["--precompose-only", "--precompose-component-id"]) .arg(&c.id) - .stdin(std::process::Stdio::piped()) .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 mut child = cmd + let child = cmd .spawn() .map_err(|e| ComposeError::PrepareError(e.into()))?; - use tokio::io::AsyncWriteExt; - - let mut input = child.stdin.take().unwrap(); - input - .write_all(&data) - .await - .map_err(|e| ComposeError::PrepareError(e.into()))?; - input - .flush() - .await - .map_err(|e| ComposeError::PrepareError(e.into()))?; - drop(input); - let trigger_out = child .wait_with_output() .await diff --git a/crates/trigger/src/cli.rs b/crates/trigger/src/cli.rs index 9d7667d447..a8e7ab6f07 100644 --- a/crates/trigger/src/cli.rs +++ b/crates/trigger/src/cli.rs @@ -138,10 +138,10 @@ pub struct FactorsTriggerCommand, B: RuntimeFactorsBuilde #[clap(long = "launch-metadata-only", hide = true)] pub launch_metadata_only: bool, - #[clap(long = "resolve-extras-only", hide = true)] - pub resolve_extras_only: bool, - #[clap(long = "resolve-extras-component-id", hide = true)] - pub resolve_extras_component_id: Option, + #[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")] @@ -219,54 +219,26 @@ impl, B: RuntimeFactorsBuilder> FactorsTriggerCommand = TriggerAppBuilder::new(trigger); let config = builder.engine_config(); diff --git a/crates/trigger/src/loader.rs b/crates/trigger/src/loader.rs index e9052c7883..d70c9db09b 100644 --- a/crates/trigger/src/loader.rs +++ b/crates/trigger/src/loader.rs @@ -66,31 +66,8 @@ impl ComponentLoader { } } } -} - -#[async_trait] -impl spin_factors_executor::ComponentLoader for ComponentLoader { - async fn load_component( - &self, - engine: &wasmtime::Engine, - component: &AppComponent, - complicator: &impl spin_factors_executor::Complicator, - ) -> anyhow::Result { - let source = component - .source() - .content - .source - .as_ref() - .context("LockedComponentSource missing source field")?; - let path = parse_file_url(source)?; - - #[cfg(feature = "unsafe-aot-compilation")] - if self.aot_compilation_enabled { - return self - .load_precompiled_component(engine, &path) - .with_context(|| format!("error deserializing component from {path:?}")); - } + pub(crate) async fn load_full(&self, component: &AppComponent<'_>, complicator: &impl spin_factors_executor::Complicator) -> anyhow::Result> { let loader = ComponentSourceLoaderFs; let empty: serde_json::Map = Default::default(); @@ -119,6 +96,63 @@ impl spin_factors_executor::ComponentLoader for Comp ) })?; + Ok(composed) + } +} + +#[async_trait] +impl spin_factors_executor::ComponentLoader for ComponentLoader { + async fn load_component( + &self, + engine: &wasmtime::Engine, + component: &AppComponent, + complicator: &impl spin_factors_executor::Complicator, + ) -> anyhow::Result { + let source = component + .source() + .content + .source + .as_ref() + .context("LockedComponentSource missing source field")?; + let path = parse_file_url(source)?; + + #[cfg(feature = "unsafe-aot-compilation")] + if self.aot_compilation_enabled { + return self + .load_precompiled_component(engine, &path) + .with_context(|| format!("error deserializing component from {path:?}")); + } + + let composed = self.load_full(component, complicator).await?; + + // 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 + // ) + // })?; + spin_core::Component::new(engine, composed) .with_context(|| format!("failed to compile component from {}", quoted_path(&path))) } From 867c0be687b86d6aaa3a9cab20bb29dfb73ea610 Mon Sep 17 00:00:00 2001 From: itowlson Date: Tue, 17 Feb 2026 14:49:54 +1300 Subject: [PATCH 3/3] Tidying because IT WORKS Signed-off-by: itowlson --- crates/oci/src/client.rs | 89 +++--------------------------------- crates/trigger/src/cli.rs | 31 ++++++++----- crates/trigger/src/loader.rs | 36 +++------------ 3 files changed, 31 insertions(+), 125 deletions(-) diff --git a/crates/oci/src/client.rs b/crates/oci/src/client.rs index 69f46e9e95..96a490b886 100644 --- a/crates/oci/src/client.rs +++ b/crates/oci/src/client.rs @@ -370,26 +370,16 @@ impl Client { let locked_url = write_locked_app(&locked, working_dir).await.unwrap(); for mut c in locked.components { - // TODO: what if, instead of having a separate complicate that - // called the trigger, we had the trigger do the full compose - // (in the presence of extras)? The trigger already has logic - // for that... - let extras = c.metadata.get("trigger-extras").and_then(|e| e.as_object()); + 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 { - compose_trigger_extras_2(&c, &locked_url, working_dir).await? + // 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 complicate = - // |data: Vec| compose_trigger_extras(&c, &locked_url, working_dir, data); - - // let composed = spin_compose::compose(&ComponentSourceLoaderFs, &c, complicate) - // .await - // .with_context(|| { - // format!("failed to resolve dependencies for component {:?}", c.id) - // })?; let layer = ImageLayer::new(composed, WASM_LAYER_MEDIA_TYPE.to_string(), None); c.source.content = self.content_ref_for_layer(&layer); c.dependencies.clear(); @@ -950,73 +940,7 @@ 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 Result::<_, ComposeError>::Ok(data); -// }; - -// 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(["--resolve-extras-only", "--resolve-extras-component-id"]) -// .arg(&c.id) -// // .stdin(std::process::Stdio::piped()) -// .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()))?; - -// // use tokio::io::AsyncWriteExt; - -// // let mut input = child.stdin.take().unwrap(); -// // input -// // .write_all(&data) -// // .await -// // .map_err(|e| ComposeError::PrepareError(e.into()))?; -// // input -// // .flush() -// // .await -// // .map_err(|e| ComposeError::PrepareError(e.into()))?; -// // drop(input); - -// 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) -// } - -async fn compose_trigger_extras_2( +async fn precompose_using_trigger( c: &LockedComponent, locked_url: &str, working_dir: &Path, @@ -1028,8 +952,7 @@ async fn compose_trigger_extras_2( .get("resolve-extras-using") .and_then(|v| v.as_str()) else { - return spin_compose::compose(&ComponentSourceLoaderFs, &c, async |a| Ok(a)).await; - // return Result::<_, ComposeError>::Ok(data); + return spin_compose::compose(&ComponentSourceLoaderFs, c, async |a| Ok(a)).await; }; let resolver_subcmd = match resolve_extras_using { diff --git a/crates/trigger/src/cli.rs b/crates/trigger/src/cli.rs index a8e7ab6f07..c42f51ba5b 100644 --- a/crates/trigger/src/cli.rs +++ b/crates/trigger/src/cli.rs @@ -214,29 +214,36 @@ impl, B: RuntimeFactorsBuilder> FactorsTriggerCommand = TriggerAppBuilder::new(trigger); diff --git a/crates/trigger/src/loader.rs b/crates/trigger/src/loader.rs index d70c9db09b..6164b2e980 100644 --- a/crates/trigger/src/loader.rs +++ b/crates/trigger/src/loader.rs @@ -67,7 +67,11 @@ impl ComponentLoader { } } - pub(crate) async fn load_full(&self, component: &AppComponent<'_>, complicator: &impl spin_factors_executor::Complicator) -> anyhow::Result> { + 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(); @@ -123,35 +127,7 @@ impl spin_factors_executor::ComponentLoader for Comp .with_context(|| format!("error deserializing component from {path:?}")); } - let composed = self.load_full(component, complicator).await?; - - // 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 - // ) - // })?; + 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)))