From 3c94860d4eb8678b41a8ad7ade2d7082ef24dd4a Mon Sep 17 00:00:00 2001 From: itowlson Date: Fri, 12 Sep 2025 09:06:17 +1200 Subject: [PATCH] Dependencies by component ref Signed-off-by: itowlson --- Cargo.lock | 1 + crates/build/src/manifest.rs | 2 +- crates/environments/src/loader.rs | 1 + crates/loader/src/local.rs | 5 +- crates/manifest/src/normalize.rs | 275 +++++++++++++++++++++++++++++- crates/manifest/src/schema/v2.rs | 23 +++ crates/manifest/tests/ui.rs | 2 +- crates/serde/Cargo.toml | 3 + crates/serde/src/id.rs | 13 ++ 9 files changed, 320 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1897e8b49a..066a7dfd49 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -9050,6 +9050,7 @@ version = "3.5.0-pre0" dependencies = [ "anyhow", "base64 0.22.1", + "indexmap 2.7.1", "schemars", "semver", "serde", diff --git a/crates/build/src/manifest.rs b/crates/build/src/manifest.rs index db570e4145..a27b067dde 100644 --- a/crates/build/src/manifest.rs +++ b/crates/build/src/manifest.rs @@ -70,7 +70,7 @@ pub async fn component_build_configs(manifest_file: impl AsRef) -> Result< let manifest = spin_manifest::manifest_from_file(&manifest_file); match manifest { Ok(mut manifest) => { - spin_manifest::normalize::normalize_manifest(&mut manifest); + spin_manifest::normalize::normalize_manifest(&mut manifest)?; let components = build_configs_from_manifest(&manifest); let deployment_targets = deployment_targets_from_manifest(&manifest); Ok(ManifestBuildInfo::Loadable { diff --git a/crates/environments/src/loader.rs b/crates/environments/src/loader.rs index fa5639775a..1de68e551e 100644 --- a/crates/environments/src/loader.rs +++ b/crates/environments/src/loader.rs @@ -272,6 +272,7 @@ impl spin_compose::DependencyLike for WrappedComponentDependency { spin_manifest::schema::v2::ComponentDependency::Package { export, .. } => export, spin_manifest::schema::v2::ComponentDependency::Local { export, .. } => export, spin_manifest::schema::v2::ComponentDependency::HTTP { export, .. } => export, + spin_manifest::schema::v2::ComponentDependency::AppComponent { export, .. } => export, } } } diff --git a/crates/loader/src/local.rs b/crates/loader/src/local.rs index abcd9015e6..4cc6a23dc6 100644 --- a/crates/loader/src/local.rs +++ b/crates/loader/src/local.rs @@ -73,7 +73,7 @@ impl LocalLoader { // Load the given manifest into a LockedApp, ready for execution. pub(crate) async fn load_manifest(&self, mut manifest: AppManifest) -> Result { - spin_manifest::normalize::normalize_manifest(&mut manifest); + spin_manifest::normalize::normalize_manifest(&mut manifest)?; manifest.validate_dependencies()?; @@ -867,6 +867,9 @@ impl WasmLoader { let content = self.load_http_source(&url, &digest).await?; Ok((content, export)) } + v2::ComponentDependency::AppComponent { .. } => { + panic!("Internal error: component ID dependency was not resolved to a source"); + } } } } diff --git a/crates/manifest/src/normalize.rs b/crates/manifest/src/normalize.rs index 2182c95515..acb14c7de6 100644 --- a/crates/manifest/src/normalize.rs +++ b/crates/manifest/src/normalize.rs @@ -1,16 +1,19 @@ //! Manifest normalization functions. -use std::collections::HashSet; +use std::{collections::HashSet, path::PathBuf}; use crate::schema::v2::{AppManifest, ComponentSpec, KebabId}; +use anyhow::Context; /// Normalizes some optional [`AppManifest`] features into a canonical form: /// - Inline components in trigger configs are moved into top-level /// components and replaced with a reference. /// - Any triggers without an ID are assigned a generated ID. -pub fn normalize_manifest(manifest: &mut AppManifest) { +pub fn normalize_manifest(manifest: &mut AppManifest) -> anyhow::Result<()> { normalize_trigger_ids(manifest); normalize_inline_components(manifest); + normalize_dependency_component_refs(manifest)?; + Ok(()) } fn normalize_inline_components(manifest: &mut AppManifest) { @@ -103,3 +106,271 @@ fn normalize_trigger_ids(manifest: &mut AppManifest) { } } } + +use crate::schema::v2::{Component, ComponentDependency, ComponentSource}; + +fn normalize_dependency_component_refs(manifest: &mut AppManifest) -> anyhow::Result<()> { + // `clone` a snapshot, because we are about to mutate collection elements, + // and the borrow checker gets mad at us if we try to index into the collection + // while that's happening. + let components = manifest.components.clone(); + + for (depender_id, component) in &mut manifest.components { + for dependency in component.dependencies.inner.values_mut() { + if let ComponentDependency::AppComponent { + component: depended_on_id, + export, + } = dependency + { + let depended_on = components + .get(depended_on_id) + .with_context(|| format!("dependency ID {depended_on_id} does not exist"))?; + ensure_is_acceptable_dependency(depended_on, depended_on_id, depender_id)?; + *dependency = component_source_to_dependency(&depended_on.source, export.clone()); + } + } + } + + Ok(()) +} + +fn component_source_to_dependency( + source: &ComponentSource, + export: Option, +) -> ComponentDependency { + match source { + ComponentSource::Local(path) => ComponentDependency::Local { + path: PathBuf::from(path), + export, + }, + ComponentSource::Remote { url, digest } => ComponentDependency::HTTP { + url: url.clone(), + digest: digest.clone(), + export, + }, + ComponentSource::Registry { + registry, + package, + version, + } => ComponentDependency::Package { + version: version.clone(), + registry: registry.as_ref().map(|r| r.to_string()), + package: Some(package.to_string()), + export, + }, + } +} + +/// If a dependency has things like files or KV stores or network access... +/// those won't apply when it's composed, and that's likely to be surprising, +/// and developers hate surprises. +fn ensure_is_acceptable_dependency( + component: &Component, + depended_on_id: &KebabId, + depender_id: &KebabId, +) -> anyhow::Result<()> { + let mut surprises = vec![]; + + // Explicitly discard fields we don't need to check (do *not* .. them away). This + // way, the compiler will give us a heads up if a new field is added so we can + // decide whether or not we need to check it. + #[allow(deprecated)] + let Component { + source: _, + description: _, + variables, + environment, + files, + exclude_files: _, + allowed_http_hosts, + allowed_outbound_hosts, + key_value_stores, + sqlite_databases, + ai_models, + build: _, + tool: _, + dependencies_inherit_configuration: _, + dependencies, + } = component; + + if !ai_models.is_empty() { + surprises.push("ai_models"); + } + if !allowed_http_hosts.is_empty() { + surprises.push("allowed_http_hosts"); + } + if !allowed_outbound_hosts.is_empty() { + surprises.push("allowed_outbound_hosts"); + } + if !dependencies.inner.is_empty() { + surprises.push("dependencies"); + } + if !environment.is_empty() { + surprises.push("environment"); + } + if !files.is_empty() { + surprises.push("files"); + } + if !key_value_stores.is_empty() { + surprises.push("key_value_stores"); + } + if !sqlite_databases.is_empty() { + surprises.push("sqlite_databases"); + } + if !variables.is_empty() { + surprises.push("variables"); + } + + if surprises.is_empty() { + Ok(()) + } else { + anyhow::bail!("Dependencies may not have their own resources or permissions. Component {depended_on_id} cannot be used as a dependency of {depender_id} because it specifies: {}", surprises.join(", ")); + } +} + +#[cfg(test)] +mod test { + use super::*; + + use serde::Deserialize; + use toml::toml; + + fn package_name(name: &str) -> spin_serde::DependencyName { + let dpn = spin_serde::DependencyPackageName::try_from(name.to_string()).unwrap(); + spin_serde::DependencyName::Package(dpn) + } + + #[test] + fn can_resolve_dependency_on_file_source() { + let mut manifest = AppManifest::deserialize(toml! { + spin_manifest_version = 2 + + [application] + name = "dummy" + + [[trigger.dummy]] + component = "a" + + [component.a] + source = "a.wasm" + [component.a.dependencies] + "b:b" = { component = "b" } + + [component.b] + source = "b.wasm" + }) + .unwrap(); + + normalize_manifest(&mut manifest).unwrap(); + + let dep = manifest + .components + .get("a") + .unwrap() + .dependencies + .inner + .get(&package_name("b:b")) + .unwrap(); + + let ComponentDependency::Local { path, export } = dep else { + panic!("should have normalised to local dep"); + }; + + assert_eq!(&PathBuf::from("b.wasm"), path); + assert_eq!(&None, export); + } + + #[test] + fn can_resolve_dependency_on_http_source() { + let mut manifest = AppManifest::deserialize(toml! { + spin_manifest_version = 2 + + [application] + name = "dummy" + + [[trigger.dummy]] + component = "a" + + [component.a] + source = "a.wasm" + [component.a.dependencies] + "b:b" = { component = "b", export = "c:d/e" } + + [component.b] + source = { url = "http://example.com/b.wasm", digest = "12345" } + }) + .unwrap(); + + normalize_manifest(&mut manifest).unwrap(); + + let dep = manifest + .components + .get("a") + .unwrap() + .dependencies + .inner + .get(&package_name("b:b")) + .unwrap(); + + let ComponentDependency::HTTP { + url, + digest, + export, + } = dep + else { + panic!("should have normalised to HTTP dep"); + }; + + assert_eq!("http://example.com/b.wasm", url); + assert_eq!("12345", digest); + assert_eq!("c:d/e", export.as_ref().unwrap()); + } + + #[test] + fn can_resolve_dependency_on_package() { + let mut manifest = AppManifest::deserialize(toml! { + spin_manifest_version = 2 + + [application] + name = "dummy" + + [[trigger.dummy]] + component = "a" + + [component.a] + source = "a.wasm" + [component.a.dependencies] + "b:b" = { component = "b" } + + [component.b] + source = { package = "bb:bb", version = "1.2.3", registry = "reginalds-registry.reg" } + }) + .unwrap(); + + normalize_manifest(&mut manifest).unwrap(); + + let dep = manifest + .components + .get("a") + .unwrap() + .dependencies + .inner + .get(&package_name("b:b")) + .unwrap(); + + let ComponentDependency::Package { + version, + registry, + package, + export, + } = dep + else { + panic!("should have normalised to HTTP dep"); + }; + + assert_eq!("1.2.3", version); + assert_eq!("reginalds-registry.reg", registry.as_ref().unwrap()); + assert_eq!("bb:bb", package.as_ref().unwrap()); + assert_eq!(&None, export); + } +} diff --git a/crates/manifest/src/schema/v2.rs b/crates/manifest/src/schema/v2.rs index 238139ced0..0c5a43b4f1 100644 --- a/crates/manifest/src/schema/v2.rs +++ b/crates/manifest/src/schema/v2.rs @@ -187,6 +187,13 @@ impl TryFrom for ComponentSpec { /// /// Example: `"my:dependency" = { path = "path/to/component.wasm", export = "my-export" }` /// +/// - A component in the application. The referenced component binary is composed: additional +/// configuration such as files, networking, storage, etc. are ignored. This is intended +/// primarily as a convenience for including dependencies in the manifest so that they +/// can be built using `spin build`. +/// +/// Example: `"my:dependency" = { component = "my-dependency", export = "my-export" }` +/// /// - A package from an HTTP URL. /// /// Example: `"my:import" = { url = "https://example.com/component.wasm", sha256 = "sha256:..." }` @@ -268,6 +275,22 @@ pub enum ComponentDependency { /// Learn more: https://spinframework.dev/writing-apps#dependencies-from-a-url export: Option, }, + /// `... = { component = "my-dependency" }` + #[schemars(description = "")] // schema docs are on the parent + AppComponent { + /// The ID of the component which implements the dependency. + /// + /// Example: `"my:dep/import" = { component = "my-dependency" }` + /// + /// Learn more: https://spinframework.dev/writing-apps#using-component-dependencies + component: KebabId, + /// The name of the export in the package. If omitted, this defaults to the name of the import. + /// + /// Example: `"my:dep/import" = { export = "your:impl/export", component = "my-dependency" }` + /// + /// Learn more: https://spinframework.dev/writing-apps#using-component-dependencies + export: Option, + }, } /// A Spin component. diff --git a/crates/manifest/tests/ui.rs b/crates/manifest/tests/ui.rs index 99b2504b8e..b0be99e722 100644 --- a/crates/manifest/tests/ui.rs +++ b/crates/manifest/tests/ui.rs @@ -45,6 +45,6 @@ fn run_v1_to_v2_test(input: &Path) -> Result { fn run_normalization_test(input: impl AsRef) -> Result { let mut manifest = spin_manifest::manifest_from_file(input)?; - normalize_manifest(&mut manifest); + normalize_manifest(&mut manifest)?; Ok(toml::to_string(&manifest).expect("serialization should work")) } diff --git a/crates/serde/Cargo.toml b/crates/serde/Cargo.toml index a6a60eb17f..1c6616ad43 100644 --- a/crates/serde/Cargo.toml +++ b/crates/serde/Cargo.toml @@ -11,3 +11,6 @@ schemars = { version = "0.8.21", features = ["indexmap2", "semver"] } semver = { workspace = true, features = ["serde"] } serde = { workspace = true } wasm-pkg-common = { workspace = true } + +[dev-dependencies] +indexmap = { workspace = true } diff --git a/crates/serde/src/id.rs b/crates/serde/src/id.rs index 4e8d12e70e..a577e9658a 100644 --- a/crates/serde/src/id.rs +++ b/crates/serde/src/id.rs @@ -76,6 +76,19 @@ impl TryFrom for Id } } +#[cfg(test)] +impl indexmap::Equivalent> for &str { + fn equivalent(&self, key: &Id) -> bool { + key.as_ref() == *self + } +} + +impl std::borrow::Borrow for Id { + fn borrow(&self) -> &str { + self.as_ref() + } +} + const fn wrong_delim() -> Option { match DELIM { '_' => Some('-'),