From 788dec12bec206b40ef6226a90e508691cf9312c Mon Sep 17 00:00:00 2001 From: Kate Goldenring Date: Thu, 12 Sep 2024 09:53:38 -0700 Subject: [PATCH] Add spin up component flag to run a subset of app components Signed-off-by: Kate Goldenring --- Cargo.lock | 2 + Cargo.toml | 2 + crates/factor-outbound-networking/src/lib.rs | 3 +- src/commands/up.rs | 276 ++++++++++++++++++- src/commands/up/app_source.rs | 6 +- tests/integration.rs | 7 +- 6 files changed, 269 insertions(+), 27 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 62cf5ab301..9a64364e80 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7006,6 +7006,7 @@ dependencies = [ "futures", "glob", "hex", + "http 1.1.0", "http-body-util", "hyper 1.4.1", "hyper-util", @@ -7032,6 +7033,7 @@ dependencies = [ "spin-common", "spin-doctor", "spin-expressions", + "spin-factor-outbound-networking", "spin-http", "spin-loader", "spin-locked-app", diff --git a/Cargo.toml b/Cargo.toml index 78a8ae3d51..3410690242 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -28,6 +28,7 @@ dialoguer = "0.10" dirs = { workspace = true } futures = { workspace = true } glob = { workspace = true } +http = { workspace = true } indicatif = "0.17" is-terminal = "0.4" itertools = { workspace = true } @@ -59,6 +60,7 @@ spin-build = { path = "crates/build" } spin-common = { path = "crates/common" } spin-doctor = { path = "crates/doctor" } spin-expressions = { path = "crates/expressions" } +spin-factor-outbound-networking = { path = "crates/factor-outbound-networking" } spin-http = { path = "crates/http" } spin-loader = { path = "crates/loader" } spin-locked-app = { path = "crates/locked-app" } diff --git a/crates/factor-outbound-networking/src/lib.rs b/crates/factor-outbound-networking/src/lib.rs index 25d8085c13..7c83677e27 100644 --- a/crates/factor-outbound-networking/src/lib.rs +++ b/crates/factor-outbound-networking/src/lib.rs @@ -1,8 +1,6 @@ mod config; pub mod runtime_config; -use std::{collections::HashMap, sync::Arc}; - use futures_util::{ future::{BoxFuture, Shared}, FutureExt, @@ -14,6 +12,7 @@ use spin_factors::{ anyhow::{self, Context}, ConfigureAppContext, Error, Factor, FactorInstanceBuilder, PrepareContext, RuntimeFactors, }; +use std::{collections::HashMap, sync::Arc}; pub use config::{ allowed_outbound_hosts, is_service_chaining_host, parse_service_chaining_target, diff --git a/src/commands/up.rs b/src/commands/up.rs index 87f5d61cf3..7fb5f4c56b 100644 --- a/src/commands/up.rs +++ b/src/commands/up.rs @@ -1,18 +1,19 @@ mod app_source; use std::{ - collections::HashMap, + collections::{HashMap, HashSet}, ffi::OsString, fmt::Debug, path::{Path, PathBuf}, process::Stdio, }; -use anyhow::{anyhow, bail, Context, Result}; +use anyhow::{anyhow, bail, ensure, Context, Result}; 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_loader::FilesMountStrategy; use spin_oci::OciLoader; use spin_trigger::cli::{LaunchMetadata, SPIN_LOCAL_APP_DIR, SPIN_LOCKED_URL, SPIN_WORKING_DIR}; @@ -113,6 +114,10 @@ pub struct UpCommand { #[clap(long, takes_value = false, env = ALWAYS_BUILD_ENV)] pub build: bool, + /// [Experimental] Component ID to run. This can be specified multiple times. The default is all components. + #[clap(hide = true, short = 'c', long = "component-id")] + pub components: Vec, + /// All other args, to be passed through to the trigger #[clap(hide = true)] pub trigger_args: Vec, @@ -164,13 +169,12 @@ impl UpCommand { .context("Could not canonicalize working directory")?; let resolved_app_source = self.resolve_app_source(&app_source, &working_dir).await?; - - let trigger_cmds = trigger_command_for_resolved_app_source(&resolved_app_source) - .with_context(|| format!("Couldn't find trigger executor for {app_source}"))?; - - let is_multi = trigger_cmds.len() > 1; - if self.help { + let trigger_cmds = + trigger_commands_for_trigger_types(resolved_app_source.trigger_types()) + .with_context(|| format!("Couldn't find trigger executor for {app_source}"))?; + + let is_multi = trigger_cmds.len() > 1; if is_multi { // For now, only common flags are allowed on multi-trigger apps. let mut child = self @@ -189,10 +193,25 @@ impl UpCommand { if self.build { app_source.build().await?; } - let mut locked_app = self .load_resolved_app_source(resolved_app_source, &working_dir) - .await?; + .await + .context("Failed to load application")?; + if !self.components.is_empty() { + retain_components(&mut locked_app, &self.components)?; + } + + let trigger_types: HashSet<&str> = locked_app + .triggers + .iter() + .map(|t| t.trigger_type.as_ref()) + .collect(); + + ensure!(!trigger_types.is_empty(), "No triggers in app"); + + let trigger_cmds = trigger_commands_for_trigger_types(trigger_types.into_iter().collect()) + .with_context(|| format!("Couldn't find trigger executor for {app_source}"))?; + let is_multi = trigger_cmds.len() > 1; self.update_locked_app(&mut locked_app); let locked_url = self.write_locked_app(&locked_app, &working_dir).await?; @@ -630,11 +649,8 @@ fn trigger_command(trigger_type: &str) -> Vec { vec!["trigger".to_owned(), trigger_type.to_owned()] } -fn trigger_command_for_resolved_app_source( - resolved: &ResolvedAppSource, -) -> Result>> { - let trigger_type = resolved.trigger_types()?; - trigger_type +fn trigger_commands_for_trigger_types(trigger_types: Vec<&str>) -> Result>> { + trigger_types .iter() .map(|&t| match t { "http" | "redis" => Ok(trigger_command(t)), @@ -646,6 +662,86 @@ fn trigger_command_for_resolved_app_source( .collect() } +/// 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(locked_app: &mut LockedApp, retained_components: &[String]) -> 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; @@ -658,6 +754,156 @@ 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"); diff --git a/src/commands/up/app_source.rs b/src/commands/up/app_source.rs index 4c7ab656f6..7969106992 100644 --- a/src/commands/up/app_source.rs +++ b/src/commands/up/app_source.rs @@ -3,7 +3,6 @@ use std::{ path::{Path, PathBuf}, }; -use anyhow::ensure; use spin_common::ui::quoted_path; use spin_locked_app::locked::LockedApp; use spin_manifest::schema::v2::AppManifest; @@ -99,7 +98,7 @@ pub enum ResolvedAppSource { } impl ResolvedAppSource { - pub fn trigger_types(&self) -> anyhow::Result> { + pub fn trigger_types(&self) -> Vec<&str> { let types = match self { ResolvedAppSource::File { manifest, .. } => manifest .triggers @@ -114,7 +113,6 @@ impl ResolvedAppSource { ResolvedAppSource::BareWasm { .. } => ["http"].into_iter().collect(), }; - ensure!(!types.is_empty(), "no triggers in app"); - Ok(types.into_iter().collect()) + types.into_iter().collect() } } diff --git a/tests/integration.rs b/tests/integration.rs index 6cbf43cfc6..4b14a1e8a9 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -348,12 +348,7 @@ mod integration_tests { }, )?; - let expected = r#"Error: Couldn't find trigger executor for local app "spin.toml" - -Caused by: - no triggers in app -"#; - + let expected = "Error: No triggers in app\n"; assert_eq!(env.runtime_mut().stderr(), expected); Ok(())