From 6a876bef3ccd7136ff5ba28b4dbbcc371dc61336 Mon Sep 17 00:00:00 2001 From: Kate Goldenring Date: Wed, 15 Jan 2025 15:03:28 -0800 Subject: [PATCH 1/2] cleanup: use component filtering utils from Spin crates Signed-off-by: Kate Goldenring --- containerd-shim-spin/src/engine.rs | 24 ++- containerd-shim-spin/src/main.rs | 1 - containerd-shim-spin/src/retain.rs | 241 ----------------------------- 3 files changed, 16 insertions(+), 250 deletions(-) delete mode 100644 containerd-shim-spin/src/retain.rs diff --git a/containerd-shim-spin/src/engine.rs b/containerd-shim-spin/src/engine.rs index 191107a6..9fd76506 100644 --- a/containerd-shim-spin/src/engine.rs +++ b/containerd-shim-spin/src/engine.rs @@ -13,6 +13,7 @@ use containerd_shim_wasm::{ use futures::future; use log::info; use spin_app::locked::LockedApp; +use spin_factor_outbound_networking::validate_service_chaining_for_components; use spin_trigger::cli::NoCliArgs; use spin_trigger_http::HttpTrigger; use spin_trigger_redis::RedisTrigger; @@ -156,14 +157,21 @@ impl SpinEngine { let cache = initialize_cache().await?; let app_source = Source::from_ctx(ctx, &cache).await?; let mut locked_app = app_source.to_locked_app(&cache).await?; - let components_to_execute = env::var(constants::SPIN_COMPONENTS_TO_RETAIN_ENV) - .ok() - .map(|s| s.split(',').map(|s| s.to_string()).collect::>()); - if let Some(components) = components_to_execute { - if let Err(e) = crate::retain::retain_components(&mut locked_app, &components) { - println!("Error with selective deployment: {:?}", e); - return Err(e); - } + if let Ok(components_env) = env::var(constants::SPIN_COMPONENTS_TO_RETAIN_ENV) { + let components = components_env + .split(',') + .filter(|s| !s.is_empty()) + .collect::>(); + locked_app = spin_app::retain_components( + locked_app, + &components, + &[&validate_service_chaining_for_components], + ) + .with_context(|| { + format!( + "failed to resolve application with only [{components:?}] components retained by configured environment variable {}", constants::SPIN_COMPONENTS_TO_RETAIN_ENV + ) + })?; } configure_application_variables_from_environment_variables(&locked_app)?; let trigger_cmds = get_supported_triggers(&locked_app) diff --git a/containerd-shim-spin/src/main.rs b/containerd-shim-spin/src/main.rs index 51ca64a4..75b5915c 100644 --- a/containerd-shim-spin/src/main.rs +++ b/containerd-shim-spin/src/main.rs @@ -6,7 +6,6 @@ use containerd_shim_wasm::{ mod constants; mod engine; -mod retain; mod source; mod trigger; mod utils; diff --git a/containerd-shim-spin/src/retain.rs b/containerd-shim-spin/src/retain.rs deleted file mode 100644 index ba5cc45b..00000000 --- a/containerd-shim-spin/src/retain.rs +++ /dev/null @@ -1,241 +0,0 @@ -//! This module contains the logic for modifying a locked app to only contain a subset of its components - -use std::collections::HashSet; - -use anyhow::{bail, Context, Result}; -use spin_app::locked::LockedApp; -use spin_factor_outbound_networking::{allowed_outbound_hosts, parse_service_chaining_target}; - -/// 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_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 super::*; - - 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, spin_loader::FilesMountStrategy::Direct, None).await - } - - #[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() - ); - } -} From a22b85108183cd755e56147f654cd9f31f75d0f2 Mon Sep 17 00:00:00 2001 From: Kate Goldenring Date: Thu, 16 Jan 2025 10:40:00 -0800 Subject: [PATCH 2/2] Update CHANGELOG Signed-off-by: Kate Goldenring --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 885c1533..4cf834c4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), ### Changed +- Use component filtering utils from Spin crate, removing redundant `retain` module ([#213](https://github.com/spinkube/containerd-shim-spin/pull/213)) - Bump Spin dependencies to v3.1.2 ([#263](https://github.com/spinkube/containerd-shim-spin/pull/263)) - Updated the minimum required Rust version to 1.81