Skip to content

Commit

Permalink
fix: move component filtering methods to crates
Browse files Browse the repository at this point in the history
Signed-off-by: Kate Goldenring <[email protected]>
  • Loading branch information
kate-goldenring committed Oct 22, 2024
1 parent 2a9bf7c commit b01be64
Show file tree
Hide file tree
Showing 8 changed files with 348 additions and 370 deletions.
4 changes: 4 additions & 0 deletions Cargo.lock

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

6 changes: 6 additions & 0 deletions crates/app/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
100 changes: 100 additions & 0 deletions crates/app/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@

#![deny(missing_docs)]

use std::collections::HashSet;

use serde::Deserialize;
use serde_json::Value;
use spin_locked_app::MetadataExt;
Expand All @@ -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 {
Expand Down Expand Up @@ -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<LockedApp> {
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<String>, HashSet<String>) = 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::<HashSet<_>>();
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.
Expand Down Expand Up @@ -266,3 +315,54 @@ impl<'a> AppTrigger<'a> {
struct CommonTriggerConfig {
component: Option<String>,
}

/// 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<LockedApp> {
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::<HashSet<_>>();
assert!(components.contains("empty"));
assert!(components.len() == 1);
}
}
135 changes: 134 additions & 1 deletion crates/factor-outbound-networking/src/config.rs
Original file line number Diff line number Diff line change
@@ -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<Vec<String>> = MetadataKey::new("allowed_outbound_hosts");
Expand Down Expand Up @@ -34,6 +34,46 @@ pub fn allowed_outbound_hosts(component: &AppComponent) -> anyhow::Result<Vec<St
Ok(allowed_hosts)
}

/// Validates that all service chaining of an app will be satisfied by the
/// supplied subset of 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.
pub fn validate_service_chaining_for_components(
app: &App,
retained_components: &[String],
) -> 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::<http::Uri>() {
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 {
Expand Down Expand Up @@ -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());
}
}
3 changes: 2 additions & 1 deletion crates/factor-outbound-networking/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
14 changes: 9 additions & 5 deletions crates/factors-test/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,10 +91,14 @@ impl<T: RuntimeFactors> TestEnvironment<T> {
}

pub async fn build_locked_app(&self) -> anyhow::Result<LockedApp> {
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<LockedApp> {
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
}
Loading

0 comments on commit b01be64

Please sign in to comment.