From b01be64cd5ddf62ae08fdb0fdc4c1bad842dfb75 Mon Sep 17 00:00:00 2001 From: Kate Goldenring Date: Mon, 21 Oct 2024 09:32:00 -0700 Subject: [PATCH 1/2] fix: move component filtering methods to crates Signed-off-by: Kate Goldenring --- Cargo.lock | 4 + crates/app/Cargo.toml | 6 + crates/app/src/lib.rs | 100 ++++++++ .../factor-outbound-networking/src/config.rs | 135 +++++++++- crates/factor-outbound-networking/src/lib.rs | 3 +- crates/factors-test/src/lib.rs | 14 +- examples/spin-timer/Cargo.lock | 214 ++++++---------- src/commands/up.rs | 242 +----------------- 8 files changed, 348 insertions(+), 370 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index dcb490ed1f..0b9ad06f7f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6877,9 +6877,13 @@ dependencies = [ name = "spin-app" version = "2.8.0-pre0" dependencies = [ + "anyhow", "serde 1.0.210", "serde_json", + "spin-factors-test", "spin-locked-app", + "tokio", + "toml 0.8.19", ] [[package]] diff --git a/crates/app/Cargo.toml b/crates/app/Cargo.toml index 887b65eefc..94bbfa7c27 100644 --- a/crates/app/Cargo.toml +++ b/crates/app/Cargo.toml @@ -5,6 +5,12 @@ authors = { workspace = true } edition = { workspace = true } [dependencies] +anyhow = { workspace = true } serde = { workspace = true } serde_json = { workspace = true } spin-locked-app = { path = "../locked-app" } + +[dev-dependencies] +toml = { workspace = true } +spin-factors-test = { path = "../factors-test" } +tokio = { workspace = true } \ No newline at end of file diff --git a/crates/app/src/lib.rs b/crates/app/src/lib.rs index 8f09561e8e..b626e2c603 100644 --- a/crates/app/src/lib.rs +++ b/crates/app/src/lib.rs @@ -6,6 +6,8 @@ #![deny(missing_docs)] +use std::collections::HashSet; + use serde::Deserialize; use serde_json::Value; use spin_locked_app::MetadataExt; @@ -27,6 +29,10 @@ pub const APP_DESCRIPTION_KEY: MetadataKey = MetadataKey::new("description"); /// MetadataKey for extracting the OCI image digest. pub const OCI_IMAGE_DIGEST_KEY: MetadataKey = MetadataKey::new("oci_image_digest"); +/// Validation function type for ensuring that applications meet requirements +/// even with components filtered out. +pub type ValidatorFn = dyn Fn(&App, &[String]) -> anyhow::Result<()>; + /// An `App` holds loaded configuration for a Spin application. #[derive(Debug, Clone)] pub struct App { @@ -160,6 +166,49 @@ impl App { pub fn ensure_needs_only(&self, supported: &[&str]) -> std::result::Result<(), String> { self.locked.ensure_needs_only(supported) } + + /// Scrubs the locked app to only contain the given list of components + /// Introspects the LockedApp to find and selectively retain the triggers that correspond to those components + fn retain_components( + mut self, + retained_components: &[String], + validators: &[&ValidatorFn], + ) -> Result { + self.validate_retained_components_exist(retained_components)?; + for validator in validators { + validator(&self, retained_components).map_err(Error::ValidationError)?; + } + let (component_ids, trigger_ids): (HashSet, HashSet) = self + .triggers() + .filter_map(|t| match t.component() { + Ok(comp) if retained_components.contains(&comp.id().to_string()) => { + Some((comp.id().to_owned(), t.id().to_owned())) + } + _ => None, + }) + .collect(); + self.locked + .components + .retain(|c| component_ids.contains(&c.id)); + self.locked.triggers.retain(|t| trigger_ids.contains(&t.id)); + Ok(self.locked) + } + + /// Validates that all components specified to be retained actually exist in the app + fn validate_retained_components_exist(&self, retained_components: &[String]) -> Result<()> { + let app_components = self + .components() + .map(|c| c.id().to_string()) + .collect::>(); + for c in retained_components { + if !app_components.contains(c) { + return Err(Error::ValidationError(anyhow::anyhow!( + "Specified component \"{c}\" not found in application" + ))); + } + } + Ok(()) + } } /// An `AppComponent` holds configuration for a Spin application component. @@ -266,3 +315,54 @@ impl<'a> AppTrigger<'a> { struct CommonTriggerConfig { component: Option, } + +/// Scrubs the locked app to only contain the given list of components +/// Introspects the LockedApp to find and selectively retain the triggers that correspond to those components +pub fn retain_components( + locked: LockedApp, + components: &[String], + validators: &[&ValidatorFn], +) -> Result { + App::new("unused", locked).retain_components(components, validators) +} + +#[cfg(test)] +mod test { + use spin_factors_test::build_locked_app; + + use super::*; + + fn does_nothing_validator(_: &App, _: &[String]) -> anyhow::Result<()> { + Ok(()) + } + + #[tokio::test] + async fn test_retain_components_filtering_for_only_component_works() { + let manifest = toml::toml! { + spin_manifest_version = 2 + + [application] + name = "test-app" + + [[trigger.test-trigger]] + component = "empty" + + [component.empty] + source = "does-not-exist.wasm" + }; + let mut locked_app = build_locked_app(&manifest).await.unwrap(); + locked_app = retain_components( + locked_app, + &["empty".to_string()], + &[&does_nothing_validator], + ) + .unwrap(); + let components = locked_app + .components + .iter() + .map(|c| c.id.to_string()) + .collect::>(); + assert!(components.contains("empty")); + assert!(components.len() == 1); + } +} diff --git a/crates/factor-outbound-networking/src/config.rs b/crates/factor-outbound-networking/src/config.rs index e1dc4c3120..52999becae 100644 --- a/crates/factor-outbound-networking/src/config.rs +++ b/crates/factor-outbound-networking/src/config.rs @@ -1,7 +1,7 @@ use std::ops::Range; use anyhow::{bail, ensure, Context}; -use spin_factors::AppComponent; +use spin_factors::{App, AppComponent}; use spin_locked_app::MetadataKey; const ALLOWED_HOSTS_KEY: MetadataKey> = MetadataKey::new("allowed_outbound_hosts"); @@ -34,6 +34,46 @@ pub fn allowed_outbound_hosts(component: &AppComponent) -> anyhow::Result anyhow::Result<()> { + app + .triggers().try_for_each(|t| { + let Ok(component) = t.component() else { return Ok(()) }; + if retained_components.contains(&component.id().to_string()) { + let allowed_hosts = allowed_outbound_hosts(&component).context("failed to get allowed hosts")?; + for host in allowed_hosts { + // Templated URLs are not yet resolved at this point, so ignore unresolvable URIs + if let Ok(uri) = host.parse::() { + if let Some(chaining_target) = parse_service_chaining_target(&uri) { + if !retained_components.contains(&chaining_target) { + if chaining_target == "*" { + return Err(anyhow::anyhow!("Selected component '{}' cannot use wildcard service chaining: allowed_outbound_hosts = [\"http://*.spin.internal\"]", component.id())); + } + return Err(anyhow::anyhow!( + "Selected component '{}' cannot use service chaining to unselected component: allowed_outbound_hosts = [\"http://{}.spin.internal\"]", + component.id(), chaining_target + )); + } + } + } + } + } + anyhow::Ok(()) + })?; + + Ok(()) +} + /// An address is a url-like string that contains a host, a port, and an optional scheme #[derive(Eq, Debug, Clone)] pub struct AllowedHostConfig { @@ -818,4 +858,97 @@ mod test { AllowedHostsConfig::parse(&["*://127.0.0.1/24:63551"], &dummy_resolver()).unwrap(); assert!(allowed.allows(&OutboundUrl::parse("tcp://127.0.0.1:63551", "tcp").unwrap())); } + + #[tokio::test] + async fn validate_service_chaining_for_components_fails() { + let manifest = toml::toml! { + spin_manifest_version = 2 + + [application] + name = "test-app" + + [[trigger.test-trigger]] + component = "empty" + + [component.empty] + source = "does-not-exist.wasm" + allowed_outbound_hosts = ["http://another.spin.internal"] + + [[trigger.another-trigger]] + component = "another" + + [component.another] + source = "does-not-exist.wasm" + + [[trigger.third-trigger]] + component = "third" + + [component.third] + source = "does-not-exist.wasm" + allowed_outbound_hosts = ["http://*.spin.internal"] + }; + let locked_app = spin_factors_test::build_locked_app(&manifest) + .await + .expect("could not build locked app"); + let app = App::new("unused", locked_app); + let Err(e) = validate_service_chaining_for_components(&app, &["empty".to_string()]) else { + panic!("Expected service chaining to non-retained component error"); + }; + assert_eq!( + e.to_string(), + "Selected component 'empty' cannot use service chaining to unselected component: allowed_outbound_hosts = [\"http://another.spin.internal\"]" + ); + let Err(e) = validate_service_chaining_for_components( + &app, + &["third".to_string(), "another".to_string()], + ) else { + panic!("Expected wildcard service chaining error"); + }; + assert_eq!( + e.to_string(), + "Selected component 'third' cannot use wildcard service chaining: allowed_outbound_hosts = [\"http://*.spin.internal\"]" + ); + assert!(validate_service_chaining_for_components(&app, &["another".to_string()]).is_ok()); + } + + #[tokio::test] + async fn validate_service_chaining_for_components_with_templated_host_passes() { + let manifest = toml::toml! { + spin_manifest_version = 2 + + [application] + name = "test-app" + + [variables] + host = { default = "test" } + + [[trigger.test-trigger]] + component = "empty" + + [component.empty] + source = "does-not-exist.wasm" + + [[trigger.another-trigger]] + component = "another" + + [component.another] + source = "does-not-exist.wasm" + + [[trigger.third-trigger]] + component = "third" + + [component.third] + source = "does-not-exist.wasm" + allowed_outbound_hosts = ["http://{{ host }}.spin.internal"] + }; + let locked_app = spin_factors_test::build_locked_app(&manifest) + .await + .expect("could not build locked app"); + let app = App::new("unused", locked_app); + assert!(validate_service_chaining_for_components( + &app, + &["empty".to_string(), "third".to_string()] + ) + .is_ok()); + } } diff --git a/crates/factor-outbound-networking/src/lib.rs b/crates/factor-outbound-networking/src/lib.rs index 8dc1a58de5..c7378e3fb3 100644 --- a/crates/factor-outbound-networking/src/lib.rs +++ b/crates/factor-outbound-networking/src/lib.rs @@ -16,7 +16,8 @@ use std::{collections::HashMap, sync::Arc}; pub use config::{ allowed_outbound_hosts, is_service_chaining_host, parse_service_chaining_target, - AllowedHostConfig, AllowedHostsConfig, HostConfig, OutboundUrl, SERVICE_CHAINING_DOMAIN_SUFFIX, + validate_service_chaining_for_components, AllowedHostConfig, AllowedHostsConfig, HostConfig, + OutboundUrl, SERVICE_CHAINING_DOMAIN_SUFFIX, }; pub use runtime_config::ComponentTlsConfigs; diff --git a/crates/factors-test/src/lib.rs b/crates/factors-test/src/lib.rs index 344153fbee..9eae7915a3 100644 --- a/crates/factors-test/src/lib.rs +++ b/crates/factors-test/src/lib.rs @@ -91,10 +91,14 @@ impl TestEnvironment { } pub async fn build_locked_app(&self) -> anyhow::Result { - let toml_str = toml::to_string(&self.manifest).context("failed serializing manifest")?; - let dir = tempfile::tempdir().context("failed creating tempdir")?; - let path = dir.path().join("spin.toml"); - std::fs::write(&path, toml_str).context("failed writing manifest")?; - spin_loader::from_file(&path, FilesMountStrategy::Direct, None).await + build_locked_app(&self.manifest).await } } + +pub async fn build_locked_app(manifest: &toml::Table) -> anyhow::Result { + let toml_str = toml::to_string(manifest).context("failed serializing manifest")?; + let dir = tempfile::tempdir().context("failed creating tempdir")?; + let path = dir.path().join("spin.toml"); + std::fs::write(&path, toml_str).context("failed writing manifest")?; + spin_loader::from_file(&path, FilesMountStrategy::Direct, None).await +} diff --git a/examples/spin-timer/Cargo.lock b/examples/spin-timer/Cargo.lock index 41c4e14a63..edbb76b5f5 100644 --- a/examples/spin-timer/Cargo.lock +++ b/examples/spin-timer/Cargo.lock @@ -759,18 +759,18 @@ dependencies = [ [[package]] name = "cranelift-bforest" -version = "0.112.0" +version = "0.112.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ad5264b5d315c515e0845dcd2cc1697ea0018d739d58b47477f8455842583568" +checksum = "7b765ed4349e66bedd9b88c7691da42e24c7f62067a6be17ddffa949367b6e17" dependencies = [ "cranelift-entity", ] [[package]] name = "cranelift-bitset" -version = "0.112.0" +version = "0.112.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6c2797648025a7b2e32ec49fb2f71655fed74453cd41e209c6e39fd3107654f8" +checksum = "9eaa2aece6237198afd32bff57699e08d4dccb8d3902c214fc1e6ba907247ca4" dependencies = [ "serde", "serde_derive", @@ -778,9 +778,9 @@ dependencies = [ [[package]] name = "cranelift-codegen" -version = "0.112.0" +version = "0.112.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "548a3af0d36a36bab5c6a3bb8684816d501fd012c3328beb0f57dbbcb364c479" +checksum = "351824439e59d42f0e4fa5aac1d13deded155120043565769e55cd4ad3ca8ed9" dependencies = [ "bumpalo", "cranelift-bforest", @@ -801,33 +801,33 @@ dependencies = [ [[package]] name = "cranelift-codegen-meta" -version = "0.112.0" +version = "0.112.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9001ad2a4893d3505be514d3b55acc6d7efecba4bcc9ab6a7c4d422765c84202" +checksum = "5a0ce0273d7a493ef8f31f606849a4e931c19187a4923f5f87fc1f2b13109981" dependencies = [ "cranelift-codegen-shared", ] [[package]] name = "cranelift-codegen-shared" -version = "0.112.0" +version = "0.112.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "df4b34c22fdfd5d95287ae0cc766e962a976754f0cf7daa4bfa5c6af55c5fb6b" +checksum = "0f72016ac35579051913f4f07f6b36c509ed69412d852fd44c8e1d7b7fa6d92a" [[package]] name = "cranelift-control" -version = "0.112.0" +version = "0.112.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a4d78c20a5ba56200e691e0a62d15ffd18ffc781064443acbadce1f7dc847917" +checksum = "db28951d21512c4fd0554ef179bfb11e4eb6815062957a9173824eee5de0c46c" dependencies = [ "arbitrary", ] [[package]] name = "cranelift-entity" -version = "0.112.0" +version = "0.112.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "67e9d6c799b0775d43211d983b5f9230ea604063003cb6d492daf8dcac51da9b" +checksum = "14ebe592a2f81af9237cf9be29dd3854ecb72108cfffa59e85ef12389bf939e3" dependencies = [ "cranelift-bitset", "serde", @@ -836,9 +836,9 @@ dependencies = [ [[package]] name = "cranelift-frontend" -version = "0.112.0" +version = "0.112.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7c1bd2fdbe0c0c10fcee7826c00ea0e7b2a0c4e95e6a879d88e11c006587560f" +checksum = "4437db9d60c7053ac91ded0802740c2ccf123ee6d6898dd906c34f8c530cd119" dependencies = [ "cranelift-codegen", "log", @@ -848,15 +848,15 @@ dependencies = [ [[package]] name = "cranelift-isle" -version = "0.112.0" +version = "0.112.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e12b357f51e34f8e271977a5f422940aa985943d14ee8d49f66c6459ef458511" +checksum = "230cb33572b9926e210f2ca28145f2bc87f389e1456560932168e2591feb65c1" [[package]] name = "cranelift-native" -version = "0.112.0" +version = "0.112.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "da80e271413343c8ca2ca3375360a8d486355063bf96547db9714f2ac4580629" +checksum = "364524ac7aef7070b1141478724abebeec297d4ea1e87ad8b8986465e91146d9" dependencies = [ "cranelift-codegen", "libc", @@ -865,9 +865,9 @@ dependencies = [ [[package]] name = "cranelift-wasm" -version = "0.112.0" +version = "0.112.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "aa9276bbb4bbf05ba98dba1d07a506acc9ac1e15a500530399ff8aee70860118" +checksum = "0572cbd9d136a62c0f39837b6bce3b0978b96b8586794042bec0c214668fd6f5" dependencies = [ "cranelift-codegen", "cranelift-entity", @@ -2562,12 +2562,6 @@ dependencies = [ "tonic", ] -[[package]] -name = "opentelemetry-semantic-conventions" -version = "0.25.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9b8e442487022a943e2315740e443dc5ee95fd541c18f509a5a6251b408a9f95" - [[package]] name = "opentelemetry_sdk" version = "0.25.0" @@ -3746,11 +3740,9 @@ name = "spin-app" version = "2.8.0-pre0" dependencies = [ "anyhow", - "async-trait", "serde", "serde_json", "spin-locked-app", - "thiserror", ] [[package]] @@ -3787,7 +3779,6 @@ dependencies = [ "indexmap 2.2.6", "semver", "spin-app", - "spin-componentize", "spin-serde", "thiserror", "wac-graph", @@ -3809,7 +3800,6 @@ version = "2.8.0-pre0" dependencies = [ "anyhow", "async-trait", - "serde", "spin-locked-app", "thiserror", ] @@ -3824,8 +3814,8 @@ dependencies = [ "spin-core", "spin-factors", "spin-locked-app", + "spin-resource-table", "spin-world", - "table", "tokio", "toml", "tracing", @@ -3863,7 +3853,6 @@ dependencies = [ "spin-factors", "spin-telemetry", "spin-world", - "terminal", "tokio", "tokio-rustls 0.26.0", "tracing", @@ -3881,8 +3870,8 @@ dependencies = [ "spin-core", "spin-factor-outbound-networking", "spin-factors", + "spin-resource-table", "spin-world", - "table", "tokio", "tracing", ] @@ -3892,16 +3881,12 @@ name = "spin-factor-outbound-mysql" version = "2.8.0-pre0" dependencies = [ "anyhow", - "flate2", "mysql_async", - "mysql_common", - "spin-app", "spin-core", - "spin-expressions", "spin-factor-outbound-networking", "spin-factors", + "spin-resource-table", "spin-world", - "table", "tokio", "tracing", "url", @@ -3926,7 +3911,6 @@ dependencies = [ "spin-locked-app", "spin-manifest", "spin-serde", - "terminal", "tracing", "url", "urlencoding", @@ -3943,8 +3927,8 @@ dependencies = [ "spin-core", "spin-factor-outbound-networking", "spin-factors", + "spin-resource-table", "spin-world", - "table", "tokio", "tokio-postgres", "tracing", @@ -3959,8 +3943,8 @@ dependencies = [ "spin-core", "spin-factor-outbound-networking", "spin-factors", + "spin-resource-table", "spin-world", - "table", "tracing", ] @@ -3969,13 +3953,11 @@ name = "spin-factor-sqlite" version = "2.8.0-pre0" dependencies = [ "async-trait", - "serde", "spin-factors", "spin-locked-app", + "spin-resource-table", "spin-world", - "table", "tokio", - "toml", "tracing", ] @@ -3983,18 +3965,10 @@ dependencies = [ name = "spin-factor-variables" version = "2.8.0-pre0" dependencies = [ - "azure_core", - "azure_identity", - "azure_security_keyvault", - "dotenvy", - "serde", "spin-expressions", "spin-factors", "spin-world", - "tokio", - "toml", "tracing", - "vaultrs", ] [[package]] @@ -4003,7 +3977,6 @@ version = "2.8.0-pre0" dependencies = [ "async-trait", "bytes", - "cap-primitives", "spin-common", "spin-factors", "tokio", @@ -4021,7 +3994,6 @@ dependencies = [ "spin-factors-derive", "thiserror", "toml", - "tracing", "wasmtime", ] @@ -4055,8 +4027,6 @@ dependencies = [ "serde", "spin-core", "spin-factor-key-value", - "tokio", - "url", ] [[package]] @@ -4068,7 +4038,6 @@ dependencies = [ "serde", "spin-core", "spin-factor-key-value", - "spin-world", "tokio", "url", ] @@ -4127,6 +4096,10 @@ dependencies = [ "wasm-pkg-common", ] +[[package]] +name = "spin-resource-table" +version = "2.8.0-pre0" + [[package]] name = "spin-runtime-config" version = "2.8.0-pre0" @@ -4150,6 +4123,7 @@ dependencies = [ "spin-key-value-spin", "spin-sqlite", "spin-trigger", + "spin-variables", "toml", ] @@ -4174,7 +4148,6 @@ dependencies = [ "spin-factors", "spin-factors-executor", "spin-runtime-config", - "spin-telemetry", "spin-trigger", "terminal", "tracing", @@ -4195,16 +4168,11 @@ dependencies = [ name = "spin-sqlite" version = "2.8.0-pre0" dependencies = [ - "async-trait", "serde", "spin-factor-sqlite", "spin-factors", - "spin-locked-app", "spin-sqlite-inproc", "spin-sqlite-libsql", - "spin-world", - "table", - "tokio", "toml", ] @@ -4214,7 +4182,6 @@ version = "2.8.0-pre0" dependencies = [ "anyhow", "async-trait", - "rand 0.8.5", "rusqlite", "spin-factor-sqlite", "spin-world", @@ -4228,10 +4195,8 @@ dependencies = [ "anyhow", "async-trait", "libsql", - "rusqlite", "spin-factor-sqlite", "spin-world", - "sqlparser", "tokio", ] @@ -4244,14 +4209,11 @@ dependencies = [ "http 1.1.0", "opentelemetry", "opentelemetry-otlp", - "opentelemetry-semantic-conventions", "opentelemetry_sdk", "terminal", "tracing", - "tracing-appender", "tracing-opentelemetry", "tracing-subscriber", - "url", ] [[package]] @@ -4277,10 +4239,27 @@ dependencies = [ "spin-factors-executor", "spin-telemetry", "tokio", - "toml", "tracing", ] +[[package]] +name = "spin-variables" +version = "2.8.0-pre0" +dependencies = [ + "azure_core", + "azure_identity", + "azure_security_keyvault", + "dotenvy", + "serde", + "spin-expressions", + "spin-factor-variables", + "spin-factors", + "spin-world", + "tokio", + "tracing", + "vaultrs", +] + [[package]] name = "spin-world" version = "2.8.0-pre0" @@ -4295,15 +4274,6 @@ version = "0.3.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3b9b39299b249ad65f3b7e96443bad61c02ca5cd3589f46cb6d610a0fd6c0d6a" -[[package]] -name = "sqlparser" -version = "0.51.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5fe11944a61da0da3f592e19a45ebe5ab92dc14a779907ff1f08fbb797bfefc7" -dependencies = [ - "log", -] - [[package]] name = "stable_deref_trait" version = "1.2.0" @@ -4456,10 +4426,6 @@ dependencies = [ "winx", ] -[[package]] -name = "table" -version = "2.8.0-pre0" - [[package]] name = "target-lexicon" version = "0.12.16" @@ -4492,7 +4458,6 @@ dependencies = [ name = "terminal" version = "2.8.0-pre0" dependencies = [ - "atty", "termcolor", ] @@ -4822,18 +4787,6 @@ dependencies = [ "tracing-core", ] -[[package]] -name = "tracing-appender" -version = "0.2.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3566e8ce28cc0a3fe42519fc80e6b4c943cc4c8cef275620eb8dac2d3d4e06cf" -dependencies = [ - "crossbeam-channel", - "thiserror", - "time", - "tracing-subscriber", -] - [[package]] name = "tracing-attributes" version = "0.1.27" @@ -4908,7 +4861,6 @@ version = "0.1.0" dependencies = [ "anyhow", "clap", - "futures", "serde", "spin-factors", "spin-runtime-factors", @@ -5320,9 +5272,9 @@ dependencies = [ [[package]] name = "wasmtime" -version = "25.0.0" +version = "25.0.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9e025f6280f91611a59f38057e0a4e72fbc08a2a4e6ed753a0d1970ac634a997" +checksum = "ef01f9cb9636ed42a7ec5a09d785c0643590199dc7372dc22c7e2ba7a31a97d4" dependencies = [ "addr2line 0.22.0", "anyhow", @@ -5376,18 +5328,18 @@ dependencies = [ [[package]] name = "wasmtime-asm-macros" -version = "25.0.0" +version = "25.0.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2977f9d1d1228154598e8d1cc5d55c4aa744297e9a3523b258e20d6ba0cbc3c9" +checksum = "ba5b20797419d6baf2296db2354f864e8bb3447cacca9d151ce7700ae08b4460" dependencies = [ "cfg-if", ] [[package]] name = "wasmtime-cache" -version = "25.0.0" +version = "25.0.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "97d80a94087214484c427095fdb28448643f16d4b4223d98e21f48df87844125" +checksum = "272d5939e989c5b54e3fa83ef420e4a6dba3995c3065626066428b2f73ad1e06" dependencies = [ "anyhow", "base64 0.21.7", @@ -5405,9 +5357,9 @@ dependencies = [ [[package]] name = "wasmtime-component-macro" -version = "25.0.0" +version = "25.0.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "65b4bc589d7839d8dbfc4f4a0ea3380b11062ae26ff77c3a133c202fc4b21a31" +checksum = "26593c4b18c76ca3c3fbdd813d6692256537b639b851d8a6fe827e3d6966fc01" dependencies = [ "anyhow", "proc-macro2", @@ -5420,15 +5372,15 @@ dependencies = [ [[package]] name = "wasmtime-component-util" -version = "25.0.0" +version = "25.0.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8553d3720625ad4e65a9c71e215566361fcefc4e4001f17e7c669c503c33e6f6" +checksum = "a2ed562fbb0cbed20a56c369c8de146c1de06a48c19e26ed9aa45f073514ee60" [[package]] name = "wasmtime-cranelift" -version = "25.0.0" +version = "25.0.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1b1b81791925aa182f0816562b8b41b9546077ba3a789ca18454a3ffe083963a" +checksum = "f389b789cbcb53a8499131182135dea21d7d97ad77e7fb66830f69479ef0e68c" dependencies = [ "anyhow", "cfg-if", @@ -5451,9 +5403,9 @@ dependencies = [ [[package]] name = "wasmtime-environ" -version = "25.0.0" +version = "25.0.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fe742ef5ee9ce201e513ee8da472eaf198e760499a730853622fc85a61cfb1eb" +checksum = "84b72debe8899f19bedf66f7071310f06ef62de943a1369ba9b373613e77dd3d" dependencies = [ "anyhow", "cpp_demangle", @@ -5478,9 +5430,9 @@ dependencies = [ [[package]] name = "wasmtime-fiber" -version = "25.0.0" +version = "25.0.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2be377649da32af7b3eadd3ab5c89d645bdf0f5af9fe4fc59da457fbe4a87cdd" +checksum = "92b8d4d504266ee598204f9e69cea8714499cc7c5aeddaa9b3f76aaace8b0680" dependencies = [ "anyhow", "cc", @@ -5493,9 +5445,9 @@ dependencies = [ [[package]] name = "wasmtime-jit-debug" -version = "25.0.0" +version = "25.0.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "109dcbe0367eeda5467ea2950ff81899dab3ee362220eadcae0691d336122d29" +checksum = "48ed7f0bbb9da3252c252b05fcd5fd42672db161e6276aa96e92059500247d8c" dependencies = [ "object 0.36.0", "once_cell", @@ -5505,9 +5457,9 @@ dependencies = [ [[package]] name = "wasmtime-jit-icache-coherence" -version = "25.0.0" +version = "25.0.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a67e6379ff6f5eb316e4fe2baaf360c7871082006fc31addf3cf58011edb855c" +checksum = "1d930bc1325bc0448be6a11754156d770f56f6c3a61f440e9567f36cd2ea3065" dependencies = [ "anyhow", "cfg-if", @@ -5517,15 +5469,15 @@ dependencies = [ [[package]] name = "wasmtime-slab" -version = "25.0.0" +version = "25.0.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7e1daff42dc6660aa4aead9586a1c41e498a1c15674784589aeb5c5090d09930" +checksum = "055a181b8d03998511294faea14798df436503f14d7fd20edcf7370ec583e80a" [[package]] name = "wasmtime-types" -version = "25.0.0" +version = "25.0.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "24adc06abbf23bf9abbdc4b4a3bb743436a60a2a76dfabb2e49bf5237d0dadcc" +checksum = "c8340d976673ac3fdacac781f2afdc4933920c1adc738c3409e825dab3955399" dependencies = [ "anyhow", "cranelift-entity", @@ -5537,9 +5489,9 @@ dependencies = [ [[package]] name = "wasmtime-versioned-export-macros" -version = "25.0.0" +version = "25.0.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "467bf568f44048477d865a7bb42a1876acd1e2d3de77b42307f5d8e0126fc241" +checksum = "a4b0c1f76891f778db9602ee3fbb4eb7e9a3f511847d1fb1b69eddbcea28303c" dependencies = [ "proc-macro2", "quote", @@ -5602,9 +5554,9 @@ dependencies = [ [[package]] name = "wasmtime-winch" -version = "25.0.0" +version = "25.0.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4e8fdcd0682324b16fac3f3dd12eb4325d175e849b771aeda6edcb3065c85a4a" +checksum = "a702ff5eff3b37c11453ec8b54ec444bb9f2c689c7a7af382766c52df86b1e9b" dependencies = [ "anyhow", "cranelift-codegen", @@ -5619,9 +5571,9 @@ dependencies = [ [[package]] name = "wasmtime-wit-bindgen" -version = "25.0.0" +version = "25.0.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "eb8a4c5f38371e9dc1718421b03bc8737696587af5e1b233ea515ba5a111d106" +checksum = "b2fca2cbb5bb390f65d4434c19bf8d9873dfc60f10802918ebcd6f819a38d703" dependencies = [ "anyhow", "heck", @@ -5781,9 +5733,9 @@ checksum = "712e227841d057c1ee1cd2fb22fa7e5a5461ae8e48fa2ca79ec42cfc1931183f" [[package]] name = "winch-codegen" -version = "0.23.0" +version = "0.23.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b160fca5249410873830548ba7b1d956d8bf2afe72ced5e78266622d07de1303" +checksum = "d716f7c87db8ea79f1dc69f7344354b6256451bccca422ac4c3e0d607d144532" dependencies = [ "anyhow", "cranelift-codegen", diff --git a/src/commands/up.rs b/src/commands/up.rs index 275b54a266..5b6597cc02 100644 --- a/src/commands/up.rs +++ b/src/commands/up.rs @@ -13,7 +13,7 @@ use clap::{CommandFactory, Parser}; use reqwest::Url; use spin_app::locked::LockedApp; use spin_common::ui::quoted_path; -use spin_factor_outbound_networking::{allowed_outbound_hosts, parse_service_chaining_target}; +use spin_factor_outbound_networking::validate_service_chaining_for_components; use spin_loader::FilesMountStrategy; use spin_oci::OciLoader; use spin_trigger::cli::{LaunchMetadata, SPIN_LOCAL_APP_DIR, SPIN_LOCKED_URL, SPIN_WORKING_DIR}; @@ -197,8 +197,16 @@ impl UpCommand { .load_resolved_app_source(resolved_app_source, &working_dir) .await .context("Failed to load application")?; + if !self.components.is_empty() { - retain_components(&mut locked_app, &self.components)?; + locked_app = spin_app::retain_components( + locked_app, + &self.components, + &[&validate_service_chaining_for_components], + ) + .context( + "failed to resolve application with only components selected with --component", + )?; } let trigger_types: HashSet<&str> = locked_app @@ -668,86 +676,6 @@ fn trigger_commands_for_trigger_types(trigger_types: Vec<&str>) -> Result Result<()> { - // Create a temporary app to access parsed component and trigger information - let tmp_app = spin_app::App::new("tmp", locked_app.clone()); - validate_retained_components_exist(&tmp_app, retained_components)?; - validate_retained_components_service_chaining(&tmp_app, retained_components)?; - let (component_ids, trigger_ids): (HashSet, HashSet) = tmp_app - .triggers() - .filter_map(|t| match t.component() { - Ok(comp) if retained_components.contains(&comp.id().to_string()) => { - Some((comp.id().to_owned(), t.id().to_owned())) - } - _ => None, - }) - .collect(); - locked_app - .components - .retain(|c| component_ids.contains(&c.id)); - locked_app.triggers.retain(|t| trigger_ids.contains(&t.id)); - Ok(()) -} - -/// Validates that all service chaining of an app will be satisfied by the -/// retained components. -/// -/// This does a best effort look up of components that are -/// allowed to be accessed through service chaining and will error early if a -/// component is configured to to chain to another component that is not -/// retained. All wildcard service chaining is disallowed and all templated URLs -/// are ignored. -fn validate_retained_components_service_chaining( - app: &spin_app::App, - retained_components: &[String], -) -> Result<()> { - app - .triggers().try_for_each(|t| { - let Ok(component) = t.component() else { return Ok(()) }; - if retained_components.contains(&component.id().to_string()) { - let allowed_hosts = allowed_outbound_hosts(&component).context("failed to get allowed hosts")?; - for host in allowed_hosts { - // Templated URLs are not yet resolved at this point, so ignore unresolvable URIs - if let Ok(uri) = host.parse::() { - if let Some(chaining_target) = parse_service_chaining_target(&uri) { - if !retained_components.contains(&chaining_target) { - if chaining_target == "*" { - bail!("Component selected with '--component {}' cannot use wildcard service chaining: allowed_outbound_hosts = [\"http://*.spin.internal\"]", component.id()); - } - bail!( - "Component selected with '--component {}' cannot use service chaining to unselected component: allowed_outbound_hosts = [\"http://{}.spin.internal\"]", - component.id(), chaining_target - ); - } - } - } - } - } - anyhow::Ok(()) - })?; - - Ok(()) -} - -/// Validates that all components specified to be retained actually exist in the app -fn validate_retained_components_exist( - app: &spin_app::App, - retained_components: &[String], -) -> Result<()> { - let app_components = app - .components() - .map(|c| c.id().to_string()) - .collect::>(); - for c in retained_components { - if !app_components.contains(c) { - bail!("Specified component \"{c}\" not found in application"); - } - } - Ok(()) -} - #[cfg(test)] mod test { use crate::commands::up::app_source::AppSource; @@ -760,156 +688,6 @@ mod test { format!("{repo_base}/{path}") } - #[tokio::test] - async fn test_retain_components_filtering_for_only_component_works() { - let manifest = toml::toml! { - spin_manifest_version = 2 - - [application] - name = "test-app" - - [[trigger.test-trigger]] - component = "empty" - - [component.empty] - source = "does-not-exist.wasm" - }; - let mut locked_app = build_locked_app(&manifest).await.unwrap(); - retain_components(&mut locked_app, &["empty".to_string()]).unwrap(); - let components = locked_app - .components - .iter() - .map(|c| c.id.to_string()) - .collect::>(); - assert!(components.contains("empty")); - assert!(components.len() == 1); - } - - #[tokio::test] - async fn test_retain_components_filtering_for_non_existent_component_fails() { - let manifest = toml::toml! { - spin_manifest_version = 2 - - [application] - name = "test-app" - - [[trigger.test-trigger]] - component = "empty" - - [component.empty] - source = "does-not-exist.wasm" - }; - let mut locked_app = build_locked_app(&manifest).await.unwrap(); - let Err(e) = retain_components(&mut locked_app, &["dne".to_string()]) else { - panic!("Expected component not found error"); - }; - assert_eq!( - e.to_string(), - "Specified component \"dne\" not found in application" - ); - assert!(retain_components(&mut locked_app, &["dne".to_string()]).is_err()); - } - - #[tokio::test] - async fn test_retain_components_app_with_service_chaining_fails() { - let manifest = toml::toml! { - spin_manifest_version = 2 - - [application] - name = "test-app" - - [[trigger.test-trigger]] - component = "empty" - - [component.empty] - source = "does-not-exist.wasm" - allowed_outbound_hosts = ["http://another.spin.internal"] - - [[trigger.another-trigger]] - component = "another" - - [component.another] - source = "does-not-exist.wasm" - - [[trigger.third-trigger]] - component = "third" - - [component.third] - source = "does-not-exist.wasm" - allowed_outbound_hosts = ["http://*.spin.internal"] - }; - let mut locked_app = build_locked_app(&manifest) - .await - .expect("could not build locked app"); - let Err(e) = retain_components(&mut locked_app, &["empty".to_string()]) else { - panic!("Expected service chaining to non-retained component error"); - }; - assert_eq!( - e.to_string(), - "Component selected with '--component empty' cannot use service chaining to unselected component: allowed_outbound_hosts = [\"http://another.spin.internal\"]" - ); - let Err(e) = retain_components( - &mut locked_app, - &["third".to_string(), "another".to_string()], - ) else { - panic!("Expected wildcard service chaining error"); - }; - assert_eq!( - e.to_string(), - "Component selected with '--component third' cannot use wildcard service chaining: allowed_outbound_hosts = [\"http://*.spin.internal\"]" - ); - assert!(retain_components(&mut locked_app, &["another".to_string()]).is_ok()); - } - - #[tokio::test] - async fn test_retain_components_app_with_templated_host_passes() { - let manifest = toml::toml! { - spin_manifest_version = 2 - - [application] - name = "test-app" - - [variables] - host = { default = "test" } - - [[trigger.test-trigger]] - component = "empty" - - [component.empty] - source = "does-not-exist.wasm" - - [[trigger.another-trigger]] - component = "another" - - [component.another] - source = "does-not-exist.wasm" - - [[trigger.third-trigger]] - component = "third" - - [component.third] - source = "does-not-exist.wasm" - allowed_outbound_hosts = ["http://{{ host }}.spin.internal"] - }; - let mut locked_app = build_locked_app(&manifest) - .await - .expect("could not build locked app"); - assert!( - retain_components(&mut locked_app, &["empty".to_string(), "third".to_string()]).is_ok() - ); - } - - // Duplicate from crates/factors-test/src/lib.rs - pub async fn build_locked_app( - manifest: &toml::map::Map, - ) -> anyhow::Result { - let toml_str = toml::to_string(manifest).context("failed serializing manifest")?; - let dir = tempfile::tempdir().context("failed creating tempdir")?; - let path = dir.path().join("spin.toml"); - std::fs::write(&path, toml_str).context("failed writing manifest")?; - spin_loader::from_file(&path, FilesMountStrategy::Direct, None).await - } - #[test] fn can_infer_files() { let file = repo_path("examples/http-rust/spin.toml"); From 785cbd243bedf5592bee16d87961d8b7e03b0715 Mon Sep 17 00:00:00 2001 From: Kate Goldenring Date: Wed, 23 Oct 2024 14:10:29 -0700 Subject: [PATCH 2/2] Update ValidatorFn to use string slices Signed-off-by: Kate Goldenring --- crates/app/src/lib.rs | 21 +++++++------------ .../factor-outbound-networking/src/config.rs | 21 +++++++------------ src/commands/up.rs | 6 +++++- 3 files changed, 20 insertions(+), 28 deletions(-) diff --git a/crates/app/src/lib.rs b/crates/app/src/lib.rs index b626e2c603..806894bfa1 100644 --- a/crates/app/src/lib.rs +++ b/crates/app/src/lib.rs @@ -31,7 +31,7 @@ pub const OCI_IMAGE_DIGEST_KEY: MetadataKey = MetadataKey::new("oci_image_digest /// Validation function type for ensuring that applications meet requirements /// even with components filtered out. -pub type ValidatorFn = dyn Fn(&App, &[String]) -> anyhow::Result<()>; +pub type ValidatorFn = dyn Fn(&App, &[&str]) -> anyhow::Result<()>; /// An `App` holds loaded configuration for a Spin application. #[derive(Debug, Clone)] @@ -171,7 +171,7 @@ impl App { /// Introspects the LockedApp to find and selectively retain the triggers that correspond to those components fn retain_components( mut self, - retained_components: &[String], + retained_components: &[&str], validators: &[&ValidatorFn], ) -> Result { self.validate_retained_components_exist(retained_components)?; @@ -181,7 +181,7 @@ impl App { let (component_ids, trigger_ids): (HashSet, HashSet) = self .triggers() .filter_map(|t| match t.component() { - Ok(comp) if retained_components.contains(&comp.id().to_string()) => { + Ok(comp) if retained_components.contains(&comp.id()) => { Some((comp.id().to_owned(), t.id().to_owned())) } _ => None, @@ -195,13 +195,13 @@ impl App { } /// Validates that all components specified to be retained actually exist in the app - fn validate_retained_components_exist(&self, retained_components: &[String]) -> Result<()> { + fn validate_retained_components_exist(&self, retained_components: &[&str]) -> Result<()> { let app_components = self .components() .map(|c| c.id().to_string()) .collect::>(); for c in retained_components { - if !app_components.contains(c) { + if !app_components.contains(*c) { return Err(Error::ValidationError(anyhow::anyhow!( "Specified component \"{c}\" not found in application" ))); @@ -320,7 +320,7 @@ struct CommonTriggerConfig { /// Introspects the LockedApp to find and selectively retain the triggers that correspond to those components pub fn retain_components( locked: LockedApp, - components: &[String], + components: &[&str], validators: &[&ValidatorFn], ) -> Result { App::new("unused", locked).retain_components(components, validators) @@ -332,7 +332,7 @@ mod test { use super::*; - fn does_nothing_validator(_: &App, _: &[String]) -> anyhow::Result<()> { + fn does_nothing_validator(_: &App, _: &[&str]) -> anyhow::Result<()> { Ok(()) } @@ -351,12 +351,7 @@ mod test { source = "does-not-exist.wasm" }; let mut locked_app = build_locked_app(&manifest).await.unwrap(); - locked_app = retain_components( - locked_app, - &["empty".to_string()], - &[&does_nothing_validator], - ) - .unwrap(); + locked_app = retain_components(locked_app, &["empty"], &[&does_nothing_validator]).unwrap(); let components = locked_app .components .iter() diff --git a/crates/factor-outbound-networking/src/config.rs b/crates/factor-outbound-networking/src/config.rs index 52999becae..bd7ef01547 100644 --- a/crates/factor-outbound-networking/src/config.rs +++ b/crates/factor-outbound-networking/src/config.rs @@ -44,18 +44,18 @@ pub fn allowed_outbound_hosts(component: &AppComponent) -> anyhow::Result anyhow::Result<()> { app .triggers().try_for_each(|t| { let Ok(component) = t.component() else { return Ok(()) }; - if retained_components.contains(&component.id().to_string()) { + if retained_components.contains(&component.id()) { let allowed_hosts = allowed_outbound_hosts(&component).context("failed to get allowed hosts")?; for host in allowed_hosts { // Templated URLs are not yet resolved at this point, so ignore unresolvable URIs if let Ok(uri) = host.parse::() { if let Some(chaining_target) = parse_service_chaining_target(&uri) { - if !retained_components.contains(&chaining_target) { + if !retained_components.contains(&chaining_target.as_ref()) { if chaining_target == "*" { return Err(anyhow::anyhow!("Selected component '{}' cannot use wildcard service chaining: allowed_outbound_hosts = [\"http://*.spin.internal\"]", component.id())); } @@ -891,24 +891,21 @@ mod test { .await .expect("could not build locked app"); let app = App::new("unused", locked_app); - let Err(e) = validate_service_chaining_for_components(&app, &["empty".to_string()]) else { + let Err(e) = validate_service_chaining_for_components(&app, &["empty"]) else { panic!("Expected service chaining to non-retained component error"); }; assert_eq!( e.to_string(), "Selected component 'empty' cannot use service chaining to unselected component: allowed_outbound_hosts = [\"http://another.spin.internal\"]" ); - let Err(e) = validate_service_chaining_for_components( - &app, - &["third".to_string(), "another".to_string()], - ) else { + let Err(e) = validate_service_chaining_for_components(&app, &["third", "another"]) else { panic!("Expected wildcard service chaining error"); }; assert_eq!( e.to_string(), "Selected component 'third' cannot use wildcard service chaining: allowed_outbound_hosts = [\"http://*.spin.internal\"]" ); - assert!(validate_service_chaining_for_components(&app, &["another".to_string()]).is_ok()); + assert!(validate_service_chaining_for_components(&app, &["another"]).is_ok()); } #[tokio::test] @@ -945,10 +942,6 @@ mod test { .await .expect("could not build locked app"); let app = App::new("unused", locked_app); - assert!(validate_service_chaining_for_components( - &app, - &["empty".to_string(), "third".to_string()] - ) - .is_ok()); + assert!(validate_service_chaining_for_components(&app, &["empty", "third"]).is_ok()); } } diff --git a/src/commands/up.rs b/src/commands/up.rs index 5b6597cc02..b812aa1838 100644 --- a/src/commands/up.rs +++ b/src/commands/up.rs @@ -201,7 +201,11 @@ impl UpCommand { if !self.components.is_empty() { locked_app = spin_app::retain_components( locked_app, - &self.components, + &self + .components + .iter() + .map(|s| s.as_str()) + .collect::>(), &[&validate_service_chaining_for_components], ) .context(