Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion crates/build/src/manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
1 change: 1 addition & 0 deletions crates/environments/src/loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
}
}
Expand Down
5 changes: 4 additions & 1 deletion crates/loader/src/local.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()?;

Expand Down Expand Up @@ -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");
}
}
}
}
Expand Down
275 changes: 273 additions & 2 deletions crates/manifest/src/normalize.rs
Original file line number Diff line number Diff line change
@@ -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) {
Expand Down Expand Up @@ -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(", "));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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(", "));
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);
}
}
23 changes: 23 additions & 0 deletions crates/manifest/src/schema/v2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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:..." }`
Expand Down Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion crates/manifest/tests/ui.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
}
3 changes: 3 additions & 0 deletions crates/serde/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Loading
Loading