Dependencies by component ref

Signed-off-by: itowlson <[email protected]>

Author: itowlson

Cargo.lock

@@ -70,7 +70,7 @@ pub async fn component_build_configs(manifest_file: impl AsRef<Path>) -> 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 {

crates/build/src/manifest.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,
         }
     }
 }

crates/environments/src/loader.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<LockedApp> {
-        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");
+            }
         }
     }
 }

crates/loader/src/local.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<String>,
+) -> 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);
+    }
+}

crates/manifest/src/normalize.rs

@@ -187,6 +187,13 @@ impl TryFrom<toml::Value> 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<String>,
     },
+    /// `... = { 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<String>,
+    },
 }
 
 /// A Spin component.

crates/manifest/src/schema/v2.rs

@@ -45,6 +45,6 @@ fn run_v1_to_v2_test(input: &Path) -> Result<String, Failed> {
 
 fn run_normalization_test(input: impl AsRef<Path>) -> Result<String, Failed> {
     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"))
 }

crates/manifest/tests/ui.rs

@@ -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 }