diff --git a/Cargo.lock b/Cargo.lock index 6dae53727..b13a54569 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7763,6 +7763,7 @@ dependencies = [ name = "spin-factors-test" version = "2.7.0-pre0" dependencies = [ + "serde 1.0.197", "spin-app", "spin-factors", "spin-factors-derive", @@ -8689,9 +8690,9 @@ dependencies = [ [[package]] name = "time" -version = "0.3.34" +version = "0.3.36" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c8248b6521bb14bc45b4067159b9b6ad792e2d6d754d6c41fb50e29fefe38749" +checksum = "5dfd88e563464686c916c7e46e623e520ddc6d79fa6641390f2e3fa86e83e885" dependencies = [ "deranged", "itoa", @@ -8713,9 +8714,9 @@ checksum = "ef927ca75afb808a4d64dd374f00a2adf8d0fcff8e7b184af886c3c87ec4a3f3" [[package]] name = "time-macros" -version = "0.2.17" +version = "0.2.18" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7ba3a3ef41e6672a2f0f001392bb5dcd3ff0a9992d618ca761a11c3121547774" +checksum = "3f252a68540fde3a3877aeea552b832b40ab9a69e318efd078774a01ddee1ccf" dependencies = [ "num-conv", "time-core", diff --git a/crates/factor-key-value/src/runtime_config.rs b/crates/factor-key-value/src/runtime_config.rs index 74fddeb54..02f411abd 100644 --- a/crates/factor-key-value/src/runtime_config.rs +++ b/crates/factor-key-value/src/runtime_config.rs @@ -1,7 +1,7 @@ use std::{collections::HashMap, sync::Arc}; use serde::{de::DeserializeOwned, Deserialize}; -use spin_factors::{anyhow, FactorRuntimeConfig}; +use spin_factors::anyhow; use spin_key_value::StoreManager; /// Runtime configuration for all key value stores. @@ -12,10 +12,6 @@ pub struct RuntimeConfig { pub store_configs: HashMap, } -impl FactorRuntimeConfig for RuntimeConfig { - const KEY: &'static str = "key_value_store"; -} - /// Resolves some piece of runtime configuration to a key value store manager. pub trait RuntimeConfigResolver: Send + Sync { /// The type of configuration that this resolver can handle. diff --git a/crates/factor-key-value/tests/test.rs b/crates/factor-key-value/tests/test.rs index fd2e74b48..aed6bb998 100644 --- a/crates/factor-key-value/tests/test.rs +++ b/crates/factor-key-value/tests/test.rs @@ -4,7 +4,9 @@ use spin_factor_key_value::{ }; use spin_factor_key_value_redis::RedisKeyValueStore; use spin_factor_key_value_spin::{SpinKeyValueRuntimeConfig, SpinKeyValueStore}; -use spin_factors::RuntimeFactors; +use spin_factors::{ + Factor, FactorRuntimeConfigSource, RuntimeConfigSourceFinalizer, RuntimeFactors, +}; use spin_factors_test::{toml, TestEnvironment}; use std::collections::HashSet; @@ -41,7 +43,7 @@ async fn default_key_value_works() -> anyhow::Result<()> { source = "does-not-exist.wasm" key_value_stores = ["default"] }); - let state = env.build_instance_state(factors).await?; + let state = env.build_instance_state(factors, ()).await?; assert_eq!( state.key_value.allowed_stores(), @@ -65,15 +67,14 @@ async fn run_test_with_config_and_stores_for_label( key_value: KeyValueFactor::new(test_resolver), }; let labels_clone = labels.clone(); - let mut env = TestEnvironment::default_manifest_extend(toml! { + let env = TestEnvironment::default_manifest_extend(toml! { [component.test-component] source = "does-not-exist.wasm" key_value_stores = labels_clone }); - if let Some(runtime_config) = runtime_config { - env.runtime_config.extend(runtime_config); - } - let state = env.build_instance_state(factors).await?; + let state = env + .build_instance_state(factors, TomlConfig(runtime_config)) + .await?; assert_eq!( labels, state.key_value.allowed_stores().iter().collect::>() @@ -206,13 +207,14 @@ async fn multiple_custom_key_value_uses_first_store() -> anyhow::Result<()> { let factors = TestFactors { key_value: KeyValueFactor::new(test_resolver), }; - let mut env = TestEnvironment::default_manifest_extend(toml! { + let env = TestEnvironment::default_manifest_extend(toml! { [component.test-component] source = "does-not-exist.wasm" key_value_stores = ["custom"] }); - env.runtime_config.extend(runtime_config); - let state = env.build_instance_state(factors).await?; + let state = env + .build_instance_state(factors, TomlConfig(Some(runtime_config))) + .await?; assert_eq!( state.key_value.allowed_stores(), @@ -222,3 +224,32 @@ async fn multiple_custom_key_value_uses_first_store() -> anyhow::Result<()> { assert!(tmp_dir.path().exists()); Ok(()) } + +struct TomlConfig(Option); + +impl TryFrom for TestFactorsRuntimeConfig { + type Error = anyhow::Error; + + fn try_from(value: TomlConfig) -> Result { + Self::from_source(value) + } +} + +impl FactorRuntimeConfigSource> for TomlConfig { + fn get_runtime_config( + &mut self, + ) -> anyhow::Result< + Option< as Factor>::RuntimeConfig>, + > { + let Some(table) = self.0.as_ref().and_then(|t| t.get("key_value_store")) else { + return Ok(None); + }; + Ok(Some(table.clone().try_into()?)) + } +} + +impl RuntimeConfigSourceFinalizer for TomlConfig { + fn finalize(&mut self) -> anyhow::Result<()> { + Ok(()) + } +} diff --git a/crates/factor-llm/tests/factor.rs b/crates/factor-llm/tests/factor.rs index 5337ff25f..a05b68aa3 100644 --- a/crates/factor-llm/tests/factor.rs +++ b/crates/factor-llm/tests/factor.rs @@ -48,7 +48,7 @@ async fn llm_works() -> anyhow::Result<()> { source = "does-not-exist.wasm" ai_models = ["llama2-chat"] }); - let mut state = env.build_instance_state(factors).await?; + let mut state = env.build_instance_state(factors, ()).await?; assert_eq!( &*state.llm.allowed_models, &["llama2-chat".to_owned()] diff --git a/crates/factor-outbound-http/tests/factor_test.rs b/crates/factor-outbound-http/tests/factor_test.rs index e1dc70bbc..d5479ee1c 100644 --- a/crates/factor-outbound-http/tests/factor_test.rs +++ b/crates/factor-outbound-http/tests/factor_test.rs @@ -4,7 +4,7 @@ use anyhow::bail; use http::Request; use spin_factor_outbound_http::OutboundHttpFactor; use spin_factor_outbound_networking::OutboundNetworkingFactor; -use spin_factor_variables::VariablesFactor; +use spin_factor_variables::spin_cli::VariablesFactor; use spin_factor_wasi::{DummyFilesMounter, WasiFactor}; use spin_factors::{anyhow, RuntimeFactors}; use spin_factors_test::{toml, TestEnvironment}; @@ -37,7 +37,7 @@ async fn disallowed_host_fails() -> anyhow::Result<()> { http: OutboundHttpFactor, }; let env = test_env(); - let mut state = env.build_instance_state(factors).await?; + let mut state = env.build_instance_state(factors, ()).await?; let mut wasi_http = OutboundHttpFactor::get_wasi_http_impl(&mut state).unwrap(); let req = Request::get("https://denied.test").body(Default::default())?; diff --git a/crates/factor-outbound-networking/src/lib.rs b/crates/factor-outbound-networking/src/lib.rs index b04262e3f..fbb973947 100644 --- a/crates/factor-outbound-networking/src/lib.rs +++ b/crates/factor-outbound-networking/src/lib.rs @@ -59,7 +59,10 @@ impl Factor for OutboundNetworkingFactor { .get(ctx.app_component().id()) .cloned() .context("missing component allowed hosts")?; - let resolver = builders.get_mut::()?.resolver().clone(); + let resolver = builders + .get_mut::>()? + .expression_resolver() + .clone(); let allowed_hosts_future = async move { let prepared = resolver.prepare().await?; AllowedHostsConfig::parse(&hosts, &prepared) diff --git a/crates/factor-outbound-networking/tests/factor_test.rs b/crates/factor-outbound-networking/tests/factor_test.rs index 0bd409b75..950fd9a3a 100644 --- a/crates/factor-outbound-networking/tests/factor_test.rs +++ b/crates/factor-outbound-networking/tests/factor_test.rs @@ -1,5 +1,5 @@ use spin_factor_outbound_networking::OutboundNetworkingFactor; -use spin_factor_variables::VariablesFactor; +use spin_factor_variables::spin_cli::VariablesFactor; use spin_factor_wasi::{DummyFilesMounter, WasiFactor}; use spin_factors::{anyhow, RuntimeFactors}; use spin_factors_test::{toml, TestEnvironment}; @@ -29,7 +29,7 @@ async fn configures_wasi_socket_addr_check() -> anyhow::Result<()> { }; let env = test_env(); - let mut state = env.build_instance_state(factors).await?; + let mut state = env.build_instance_state(factors, ()).await?; let mut wasi = WasiFactor::get_wasi_impl(&mut state).unwrap(); let network_resource = wasi.instance_network()?; @@ -62,10 +62,13 @@ async fn wasi_factor_is_optional() -> anyhow::Result<()> { networking: OutboundNetworkingFactor, } TestEnvironment::default() - .build_instance_state(WithoutWasi { - variables: VariablesFactor::default(), - networking: OutboundNetworkingFactor, - }) + .build_instance_state( + WithoutWasi { + variables: VariablesFactor::default(), + networking: OutboundNetworkingFactor, + }, + (), + ) .await?; Ok(()) } diff --git a/crates/factor-outbound-pg/tests/factor_test.rs b/crates/factor-outbound-pg/tests/factor_test.rs index 07f47cc0c..89d89f924 100644 --- a/crates/factor-outbound-pg/tests/factor_test.rs +++ b/crates/factor-outbound-pg/tests/factor_test.rs @@ -2,7 +2,7 @@ use anyhow::{bail, Result}; use spin_factor_outbound_networking::OutboundNetworkingFactor; use spin_factor_outbound_pg::client::Client; use spin_factor_outbound_pg::OutboundPgFactor; -use spin_factor_variables::{StaticVariables, VariablesFactor}; +use spin_factor_variables::spin_cli::{StaticVariables, VariablesFactor}; use spin_factors::{anyhow, RuntimeFactors}; use spin_factors_test::{toml, TestEnvironment}; use spin_world::async_trait; @@ -24,7 +24,7 @@ fn factors() -> Result { networking: OutboundNetworkingFactor, pg: OutboundPgFactor::::new(), }; - f.variables.add_provider_type(StaticVariables)?; + f.variables.add_provider_resolver(StaticVariables)?; Ok(f) } @@ -43,7 +43,7 @@ async fn disallowed_host_fails() -> anyhow::Result<()> { [component.test-component] source = "does-not-exist.wasm" }); - let mut state = env.build_instance_state(factors).await?; + let mut state = env.build_instance_state(factors, ()).await?; let res = state .pg @@ -61,7 +61,7 @@ async fn disallowed_host_fails() -> anyhow::Result<()> { async fn allowed_host_succeeds() -> anyhow::Result<()> { let factors = factors()?; let env = test_env(); - let mut state = env.build_instance_state(factors).await?; + let mut state = env.build_instance_state(factors, ()).await?; let res = state .pg @@ -78,7 +78,7 @@ async fn allowed_host_succeeds() -> anyhow::Result<()> { async fn exercise_execute() -> anyhow::Result<()> { let factors = factors()?; let env = test_env(); - let mut state = env.build_instance_state(factors).await?; + let mut state = env.build_instance_state(factors, ()).await?; let connection = state .pg @@ -97,7 +97,7 @@ async fn exercise_execute() -> anyhow::Result<()> { async fn exercise_query() -> anyhow::Result<()> { let factors = factors()?; let env = test_env(); - let mut state = env.build_instance_state(factors).await?; + let mut state = env.build_instance_state(factors, ()).await?; let connection = state .pg diff --git a/crates/factor-outbound-redis/tests/factor_test.rs b/crates/factor-outbound-redis/tests/factor_test.rs index 552a871cc..89b8f64da 100644 --- a/crates/factor-outbound-redis/tests/factor_test.rs +++ b/crates/factor-outbound-redis/tests/factor_test.rs @@ -1,7 +1,7 @@ use anyhow::bail; use spin_factor_outbound_networking::OutboundNetworkingFactor; use spin_factor_outbound_redis::OutboundRedisFactor; -use spin_factor_variables::{StaticVariables, VariablesFactor}; +use spin_factor_variables::spin_cli::{StaticVariables, VariablesFactor}; use spin_factor_wasi::{DummyFilesMounter, WasiFactor}; use spin_factors::{anyhow, RuntimeFactors}; use spin_factors_test::{toml, TestEnvironment}; @@ -28,7 +28,7 @@ fn get_test_factors() -> TestFactors { async fn no_outbound_hosts_fails() -> anyhow::Result<()> { let mut factors = get_test_factors(); - factors.variables.add_provider_type(StaticVariables)?; + factors.variables.add_provider_resolver(StaticVariables)?; let env = TestEnvironment { manifest: toml! { @@ -41,7 +41,7 @@ async fn no_outbound_hosts_fails() -> anyhow::Result<()> { }, ..Default::default() }; - let mut state = env.build_instance_state(factors).await?; + let mut state = env.build_instance_state(factors, ()).await?; let connection = state .redis .open("redis://redis.test:8080".to_string()) diff --git a/crates/factor-sqlite/src/lib.rs b/crates/factor-sqlite/src/lib.rs index 1c270b9a4..10c4b2325 100644 --- a/crates/factor-sqlite/src/lib.rs +++ b/crates/factor-sqlite/src/lib.rs @@ -7,29 +7,29 @@ use std::sync::Arc; use host::InstanceState; use async_trait::async_trait; -use runtime_config::RuntimeConfigResolver; use spin_factors::{anyhow, Factor}; use spin_locked_app::MetadataKey; use spin_world::v1::sqlite as v1; use spin_world::v2::sqlite as v2; -pub struct SqliteFactor { - runtime_config_resolver: Arc, +pub struct SqliteFactor { + default_label_resolver: Arc, } -impl SqliteFactor { +impl SqliteFactor { /// Create a new `SqliteFactor` /// - /// Takes a `runtime_config_resolver` that can resolve a runtime configuration into a connection pool. - pub fn new(runtime_config_resolver: R) -> Self { + /// Takes a `default_label_resolver` for how to handle when a database label doesn't + /// have a corresponding runtime configuration. + pub fn new(default_label_resolver: impl DefaultLabelResolver + 'static) -> Self { Self { - runtime_config_resolver: Arc::new(runtime_config_resolver), + default_label_resolver: Arc::new(default_label_resolver), } } } -impl Factor for SqliteFactor { - type RuntimeConfig = runtime_config::RuntimeConfig; +impl Factor for SqliteFactor { + type RuntimeConfig = runtime_config::RuntimeConfig; type AppState = AppState; type InstanceBuilder = InstanceState; @@ -46,13 +46,10 @@ impl Factor for SqliteFactor { &self, mut ctx: spin_factors::ConfigureAppContext, ) -> anyhow::Result { - let mut connection_pools = HashMap::new(); - if let Some(runtime_config) = ctx.take_runtime_config() { - for (database_label, config) in runtime_config.store_configs { - let pool = self.runtime_config_resolver.get_pool(config)?; - connection_pools.insert(database_label, pool); - } - } + let connection_pools = ctx + .take_runtime_config() + .map(|r| r.pools) + .unwrap_or_default(); let allowed_databases = ctx .app() @@ -70,7 +67,7 @@ impl Factor for SqliteFactor { )) }) .collect::>>()?; - let resolver = self.runtime_config_resolver.clone(); + let resolver = self.default_label_resolver.clone(); let get_connection_pool: host::ConnectionPoolGetter = Arc::new(move |label| { connection_pools .get(label) @@ -139,6 +136,14 @@ fn ensure_allowed_databases_are_configured( pub const ALLOWED_DATABASES_KEY: MetadataKey> = MetadataKey::new("databases"); +/// Resolves a label to a default connection pool. +pub trait DefaultLabelResolver: Send + Sync { + /// If there is no runtime configuration for a given database label, return a default connection pool. + /// + /// If `Option::None` is returned, the database is not allowed. + fn default(&self, label: &str) -> Option>; +} + pub struct AppState { /// A map from component id to a set of allowed database labels. allowed_databases: HashMap>>, diff --git a/crates/factor-sqlite/src/runtime_config.rs b/crates/factor-sqlite/src/runtime_config.rs index 10e6f8e72..ca13c4c02 100644 --- a/crates/factor-sqlite/src/runtime_config.rs +++ b/crates/factor-sqlite/src/runtime_config.rs @@ -3,31 +3,11 @@ pub mod spin; use std::{collections::HashMap, sync::Arc}; -use serde::{de::DeserializeOwned, Deserialize}; -use spin_factors::{anyhow, FactorRuntimeConfig}; - use crate::ConnectionPool; -#[derive(Deserialize)] -#[serde(transparent)] -pub struct RuntimeConfig { - pub store_configs: HashMap, -} - -impl FactorRuntimeConfig for RuntimeConfig { - const KEY: &'static str = "sqlite_database"; -} - -/// Resolves some piece of runtime configuration to a connection pool -pub trait RuntimeConfigResolver: Send + Sync { - type Config: DeserializeOwned; - - /// Get a connection pool for a given config. - /// - fn get_pool(&self, config: Self::Config) -> anyhow::Result>; - - /// If there is no runtime configuration for a given database label, return a default connection pool. - /// - /// If `Option::None` is returned, the database is not allowed. - fn default(&self, label: &str) -> Option>; +/// A runtime configuration for SQLite databases. +/// +/// Maps database labels to connection pools. +pub struct RuntimeConfig { + pub pools: HashMap>, } diff --git a/crates/factor-sqlite/src/runtime_config/spin.rs b/crates/factor-sqlite/src/runtime_config/spin.rs index 003120661..9dc6a23e0 100644 --- a/crates/factor-sqlite/src/runtime_config/spin.rs +++ b/crates/factor-sqlite/src/runtime_config/spin.rs @@ -6,13 +6,14 @@ use std::{ }; use serde::Deserialize; -use spin_factors::anyhow::{self, Context as _}; +use spin_factors::{ + anyhow::{self, Context as _}, + runtime_config::toml::GetTomlValue, +}; use spin_world::v2::sqlite as v2; use tokio::sync::OnceCell; -use crate::{Connection, ConnectionPool, SimpleConnectionPool}; - -use super::RuntimeConfigResolver; +use crate::{Connection, ConnectionPool, DefaultLabelResolver, SimpleConnectionPool}; /// Spin's default handling of the runtime configuration for SQLite databases. /// @@ -48,20 +49,32 @@ impl SpinSqliteRuntimeConfig { local_database_dir, } } -} - -#[derive(Deserialize)] -pub struct RuntimeConfig { - #[serde(rename = "type")] - pub type_: String, - #[serde(flatten)] - pub config: toml::Table, -} -impl RuntimeConfigResolver for SpinSqliteRuntimeConfig { - type Config = RuntimeConfig; + /// Get the runtime configuration for SQLite databases from a TOML table. + /// + /// Expects table to be in the format: + /// ````toml + /// [sqlite_database.$database-label] + /// type = "$database-type" + /// ... extra type specific configuration ... + /// ``` + pub fn config_from_table>( + &self, + table: &T, + ) -> anyhow::Result> { + let Some(table) = table.get("sqlite_database") else { + return Ok(None); + }; + let config: std::collections::HashMap = table.clone().try_into()?; + let pools = config + .into_iter() + .map(|(k, v)| Ok((k, self.get_pool(v)?))) + .collect::>()?; + Ok(Some(super::RuntimeConfig { pools })) + } - fn get_pool(&self, config: RuntimeConfig) -> anyhow::Result> { + /// Get a connection pool for a given runtime configuration. + pub fn get_pool(&self, config: RuntimeConfig) -> anyhow::Result> { let database_kind = config.type_.as_str(); let pool = match database_kind { "spin" => { @@ -76,7 +89,17 @@ impl RuntimeConfigResolver for SpinSqliteRuntimeConfig { }; Ok(Arc::new(pool)) } +} + +#[derive(Deserialize)] +pub struct RuntimeConfig { + #[serde(rename = "type")] + pub type_: String, + #[serde(flatten)] + pub config: toml::Table, +} +impl DefaultLabelResolver for SpinSqliteRuntimeConfig { fn default(&self, label: &str) -> Option> { // Only default the database labeled "default". if label != "default" { diff --git a/crates/factor-sqlite/tests/factor.rs b/crates/factor-sqlite/tests/factor.rs index 6897e015b..690dff009 100644 --- a/crates/factor-sqlite/tests/factor.rs +++ b/crates/factor-sqlite/tests/factor.rs @@ -1,20 +1,21 @@ use std::{collections::HashSet, sync::Arc}; -use factor_sqlite::{runtime_config::spin::RuntimeConfig, SqliteFactor}; +use factor_sqlite::{runtime_config::spin::SpinSqliteRuntimeConfig, SqliteFactor}; use spin_factors::{ anyhow::{self, bail}, - RuntimeFactors, + runtime_config::toml::TomlKeyTracker, + Factor, FactorRuntimeConfigSource, RuntimeConfigSourceFinalizer, RuntimeFactors, }; use spin_factors_test::{toml, TestEnvironment}; #[derive(RuntimeFactors)] struct TestFactors { - sqlite: SqliteFactor, + sqlite: SqliteFactor, } #[tokio::test] async fn sqlite_works() -> anyhow::Result<()> { - let test_resolver = RuntimeConfigResolver::new(Some("default")); + let test_resolver = DefaultLabelResolver::new(Some("default")); let factors = TestFactors { sqlite: SqliteFactor::new(test_resolver), }; @@ -23,7 +24,7 @@ async fn sqlite_works() -> anyhow::Result<()> { source = "does-not-exist.wasm" sqlite_databases = ["default"] }); - let state = env.build_instance_state(factors).await?; + let state = env.build_instance_state(factors, ()).await?; assert_eq!( state.sqlite.allowed_databases(), @@ -35,7 +36,7 @@ async fn sqlite_works() -> anyhow::Result<()> { #[tokio::test] async fn errors_when_non_configured_database_used() -> anyhow::Result<()> { - let test_resolver = RuntimeConfigResolver::new(None); + let test_resolver = DefaultLabelResolver::new(None); let factors = TestFactors { sqlite: SqliteFactor::new(test_resolver), }; @@ -44,7 +45,7 @@ async fn errors_when_non_configured_database_used() -> anyhow::Result<()> { source = "does-not-exist.wasm" sqlite_databases = ["foo"] }); - let Err(err) = env.build_instance_state(factors).await else { + let Err(err) = env.build_instance_state(factors, ()).await else { bail!("Expected build_instance_state to error but it did not"); }; @@ -57,50 +58,83 @@ async fn errors_when_non_configured_database_used() -> anyhow::Result<()> { #[tokio::test] async fn no_error_when_database_is_configured() -> anyhow::Result<()> { - let test_resolver = RuntimeConfigResolver::new(None); + let test_resolver = DefaultLabelResolver::new(None); let factors = TestFactors { sqlite: SqliteFactor::new(test_resolver), }; - let mut env = TestEnvironment::default_manifest_extend(toml! { + let env = TestEnvironment::default_manifest_extend(toml! { [component.test-component] source = "does-not-exist.wasm" sqlite_databases = ["foo"] }); - env.runtime_config = toml! { + let runtime_config = toml! { [sqlite_database.foo] - type = "sqlite" + type = "spin" }; - if let Err(e) = env.build_instance_state(factors).await { + let sqlite_config = SpinSqliteRuntimeConfig::new("/".into(), "/".into()); + if let Err(e) = env + .build_instance_state( + factors, + TomlRuntimeSource::new(&runtime_config, sqlite_config), + ) + .await + { bail!("Expected build_instance_state to succeed but it errored: {e}"); } Ok(()) } -/// Will return an `InvalidConnectionPool` for all runtime configured databases and the supplied default database. -struct RuntimeConfigResolver { - default: Option, +struct TomlRuntimeSource<'a> { + table: TomlKeyTracker<'a>, + sqlite_config: SpinSqliteRuntimeConfig, } -impl RuntimeConfigResolver { - fn new(default: Option<&str>) -> Self { +impl<'a> TomlRuntimeSource<'a> { + fn new(table: &'a toml::Table, sqlite_config: SpinSqliteRuntimeConfig) -> Self { Self { - default: default.map(Into::into), + table: TomlKeyTracker::new(table), + sqlite_config, } } } -impl factor_sqlite::runtime_config::RuntimeConfigResolver for RuntimeConfigResolver { - type Config = RuntimeConfig; +impl FactorRuntimeConfigSource for TomlRuntimeSource<'_> { + fn get_runtime_config( + &mut self, + ) -> anyhow::Result::RuntimeConfig>> { + self.sqlite_config.config_from_table(&self.table) + } +} - fn get_pool( - &self, - config: RuntimeConfig, - ) -> anyhow::Result> { - let _ = config; - Ok(Arc::new(InvalidConnectionPool)) +impl RuntimeConfigSourceFinalizer for TomlRuntimeSource<'_> { + fn finalize(&mut self) -> anyhow::Result<()> { + Ok(self.table.validate_all_keys_used().unwrap()) + } +} + +impl TryFrom> for TestFactorsRuntimeConfig { + type Error = anyhow::Error; + + fn try_from(value: TomlRuntimeSource<'_>) -> Result { + Self::from_source(value) } +} + +/// Will return an `InvalidConnectionPool` for the supplied default database. +struct DefaultLabelResolver { + default: Option, +} + +impl DefaultLabelResolver { + fn new(default: Option<&str>) -> Self { + Self { + default: default.map(Into::into), + } + } +} +impl factor_sqlite::DefaultLabelResolver for DefaultLabelResolver { fn default(&self, label: &str) -> Option> { let Some(default) = &self.default else { return None; diff --git a/crates/factor-variables/src/lib.rs b/crates/factor-variables/src/lib.rs index 22c26022b..5568ec360 100644 --- a/crates/factor-variables/src/lib.rs +++ b/crates/factor-variables/src/lib.rs @@ -1,44 +1,49 @@ pub mod provider; +pub mod spin_cli; -use std::{collections::HashMap, sync::Arc}; +use std::sync::Arc; -use serde::Deserialize; -use spin_expressions::ProviderResolver; +use serde::{de::DeserializeOwned, Deserialize}; +use spin_expressions::ProviderResolver as ExpressionResolver; use spin_factors::{ - anyhow::{self, bail, Context}, - ConfigureAppContext, Factor, FactorRuntimeConfig, InitContext, InstanceBuilders, - PrepareContext, RuntimeFactors, SelfInstanceBuilder, + anyhow, ConfigureAppContext, Factor, InitContext, InstanceBuilders, PrepareContext, + RuntimeFactors, SelfInstanceBuilder, }; use spin_world::{async_trait, v1, v2::variables}; -pub use provider::{MakeVariablesProvider, StaticVariables}; +pub use provider::ProviderResolver; -#[derive(Default)] -pub struct VariablesFactor { - provider_types: HashMap<&'static str, provider::ProviderFromToml>, +/// A factor for providing variables to components. +/// +/// The factor is generic over the type of runtime configuration used to configure the providers. +pub struct VariablesFactor { + provider_resolvers: Vec>>, } -impl VariablesFactor { - pub fn add_provider_type( +impl Default for VariablesFactor { + fn default() -> Self { + Self { + provider_resolvers: Default::default(), + } + } +} + +impl VariablesFactor { + /// Adds a provider resolver to the factor. + /// + /// Each added provider will be called in order with the runtime configuration. This order + /// will be the order in which the providers are called to resolve variables. + pub fn add_provider_resolver>( &mut self, provider_type: T, ) -> anyhow::Result<()> { - if self - .provider_types - .insert( - T::RUNTIME_CONFIG_TYPE, - provider::provider_from_toml_fn(provider_type), - ) - .is_some() - { - bail!("duplicate provider type {:?}", T::RUNTIME_CONFIG_TYPE); - } + self.provider_resolvers.push(Box::new(provider_type)); Ok(()) } } -impl Factor for VariablesFactor { - type RuntimeConfig = RuntimeConfig; +impl Factor for VariablesFactor { + type RuntimeConfig = RuntimeConfig; type AppState = AppState; type InstanceBuilder = InstanceState; @@ -53,29 +58,28 @@ impl Factor for VariablesFactor { mut ctx: ConfigureAppContext, ) -> anyhow::Result { let app = ctx.app(); - let mut resolver = - ProviderResolver::new(app.variables().map(|(key, val)| (key.clone(), val.clone())))?; + let mut expression_resolver = + ExpressionResolver::new(app.variables().map(|(key, val)| (key.clone(), val.clone())))?; for component in app.components() { - resolver.add_component_variables( + expression_resolver.add_component_variables( component.id(), component.config().map(|(k, v)| (k.into(), v.into())), )?; } if let Some(runtime_config) = ctx.take_runtime_config() { - for ProviderConfig { type_, config } in runtime_config.provider_configs { - let provider_maker = self - .provider_types - .get(type_.as_str()) - .with_context(|| format!("unknown variables provider type {type_:?}"))?; - let provider = provider_maker(config)?; - resolver.add_provider(provider); + for config in runtime_config.provider_configs { + for provider_resolver in self.provider_resolvers.iter() { + if let Some(provider) = provider_resolver.resolve_provider(&config)? { + expression_resolver.add_provider(provider); + } + } } } Ok(AppState { - resolver: Arc::new(resolver), + expression_resolver: Arc::new(expression_resolver), }) } @@ -85,44 +89,33 @@ impl Factor for VariablesFactor { _builders: &mut InstanceBuilders, ) -> anyhow::Result { let component_id = ctx.app_component().id().to_string(); - let resolver = ctx.app_state().resolver.clone(); + let expression_resolver = ctx.app_state().expression_resolver.clone(); Ok(InstanceState { component_id, - resolver, + expression_resolver, }) } } +/// The runtime configuration for the variables factor. #[derive(Deserialize)] #[serde(transparent)] -pub struct RuntimeConfig { - provider_configs: Vec, -} - -impl FactorRuntimeConfig for RuntimeConfig { - const KEY: &'static str = "variable_provider"; -} - -#[derive(Deserialize)] -struct ProviderConfig { - #[serde(rename = "type")] - type_: String, - #[serde(flatten)] - config: toml::Table, +pub struct RuntimeConfig { + provider_configs: Vec, } pub struct AppState { - resolver: Arc, + expression_resolver: Arc, } pub struct InstanceState { component_id: String, - resolver: Arc, + expression_resolver: Arc, } impl InstanceState { - pub fn resolver(&self) -> &Arc { - &self.resolver + pub fn expression_resolver(&self) -> &Arc { + &self.expression_resolver } } @@ -132,7 +125,7 @@ impl SelfInstanceBuilder for InstanceState {} impl variables::Host for InstanceState { async fn get(&mut self, key: String) -> Result { let key = spin_expressions::Key::new(&key).map_err(expressions_to_variables_err)?; - self.resolver + self.expression_resolver .resolve(&self.component_id, key) .await .map_err(expressions_to_variables_err) diff --git a/crates/factor-variables/src/provider.rs b/crates/factor-variables/src/provider.rs index f34945f1c..82821fb32 100644 --- a/crates/factor-variables/src/provider.rs +++ b/crates/factor-variables/src/provider.rs @@ -1,30 +1,17 @@ -mod env; -mod statik; - -pub use env::EnvVariables; -pub use statik::StaticVariables; - use serde::de::DeserializeOwned; use spin_expressions::Provider; use spin_factors::anyhow; -pub trait MakeVariablesProvider: 'static { - const RUNTIME_CONFIG_TYPE: &'static str; - +/// A trait for converting a runtime configuration into a variables provider. +pub trait ProviderResolver: 'static { + /// Serialized configuration for the provider. type RuntimeConfig: DeserializeOwned; - type Provider: Provider; - - fn make_provider(&self, runtime_config: Self::RuntimeConfig) -> anyhow::Result; -} - -pub(crate) type ProviderFromToml = Box anyhow::Result>>; -pub(crate) fn provider_from_toml_fn( - provider_type: T, -) -> ProviderFromToml { - Box::new(move |table| { - let runtime_config: T::RuntimeConfig = table.try_into()?; - let provider = provider_type.make_provider(runtime_config)?; - Ok(Box::new(provider)) - }) + /// Create a variables provider from the given runtime configuration. + /// + /// Returns `Ok(None)` if the provider is not applicable to the given configuration. + fn resolve_provider( + &self, + runtime_config: &Self::RuntimeConfig, + ) -> anyhow::Result>>; } diff --git a/crates/factor-variables/src/provider/env.rs b/crates/factor-variables/src/spin_cli/env.rs similarity index 91% rename from crates/factor-variables/src/provider/env.rs rename to crates/factor-variables/src/spin_cli/env.rs index 485c061eb..8db82e1d6 100644 --- a/crates/factor-variables/src/provider/env.rs +++ b/crates/factor-variables/src/spin_cli/env.rs @@ -11,23 +11,28 @@ use spin_factors::anyhow::{self, Context as _}; use spin_world::async_trait; use tracing::{instrument, Level}; -use crate::MakeVariablesProvider; +use crate::ProviderResolver; + +use super::VariableProviderConfiguration; /// Creator of a environment variables provider. pub struct EnvVariables; -impl MakeVariablesProvider for EnvVariables { - const RUNTIME_CONFIG_TYPE: &'static str = "env"; - - type RuntimeConfig = EnvVariablesConfig; - type Provider = EnvVariablesProvider; +impl ProviderResolver for EnvVariables { + type RuntimeConfig = VariableProviderConfiguration; - fn make_provider(&self, runtime_config: Self::RuntimeConfig) -> anyhow::Result { - Ok(EnvVariablesProvider::new( - runtime_config.prefix, + fn resolve_provider( + &self, + runtime_config: &Self::RuntimeConfig, + ) -> anyhow::Result>> { + let VariableProviderConfiguration::Env(runtime_config) = runtime_config else { + return Ok(None); + }; + Ok(Some(Box::new(EnvVariablesProvider::new( + runtime_config.prefix.clone(), |key| std::env::var(key), - runtime_config.dotenv_path, - )) + runtime_config.dotenv_path.clone(), + )))) } } diff --git a/crates/factor-variables/src/spin_cli/mod.rs b/crates/factor-variables/src/spin_cli/mod.rs new file mode 100644 index 000000000..5e6a2bd28 --- /dev/null +++ b/crates/factor-variables/src/spin_cli/mod.rs @@ -0,0 +1,26 @@ +//! The runtime configuration for the variables factor used in the Spin CLI. + +mod env; +mod statik; + +pub use env::EnvVariables; +pub use statik::StaticVariables; + +use serde::Deserialize; +use statik::StaticVariablesProvider; + +/// A runtime configuration used in the Spin CLI for one type of variable provider. +#[derive(Debug, Deserialize)] +#[serde(rename_all = "snake_case", tag = "type")] +pub enum VariableProviderConfiguration { + /// A static provider of variables. + Static(StaticVariablesProvider), + /// An environment variable provider. + Env(env::EnvVariablesConfig), +} + +/// The runtime configuration for the variables factor used in the Spin CLI. +pub type RuntimeConfig = super::RuntimeConfig; + +/// The variables factor used in the Spin CLI. +pub type VariablesFactor = super::VariablesFactor; diff --git a/crates/factor-variables/src/provider/statik.rs b/crates/factor-variables/src/spin_cli/statik.rs similarity index 53% rename from crates/factor-variables/src/provider/statik.rs rename to crates/factor-variables/src/spin_cli/statik.rs index 222c7168e..a34756526 100644 --- a/crates/factor-variables/src/provider/statik.rs +++ b/crates/factor-variables/src/spin_cli/statik.rs @@ -4,24 +4,29 @@ use serde::Deserialize; use spin_expressions::{async_trait::async_trait, Key, Provider}; use spin_factors::anyhow; -use crate::MakeVariablesProvider; +use crate::ProviderResolver; + +use super::VariableProviderConfiguration; /// Creator of a static variables provider. pub struct StaticVariables; -impl MakeVariablesProvider for StaticVariables { - const RUNTIME_CONFIG_TYPE: &'static str = "static"; - - type RuntimeConfig = StaticVariablesProvider; - type Provider = StaticVariablesProvider; - - fn make_provider(&self, runtime_config: Self::RuntimeConfig) -> anyhow::Result { - Ok(runtime_config) +impl ProviderResolver for StaticVariables { + type RuntimeConfig = VariableProviderConfiguration; + + fn resolve_provider( + &self, + runtime_config: &Self::RuntimeConfig, + ) -> anyhow::Result>> { + let VariableProviderConfiguration::Static(config) = runtime_config else { + return Ok(None); + }; + Ok(Some(Box::new(config.clone()) as _)) } } /// A variables provider that reads variables from an static map. -#[derive(Debug, Deserialize)] +#[derive(Debug, Deserialize, Clone)] pub struct StaticVariablesProvider { values: Arc>, } diff --git a/crates/factor-variables/tests/factor_test.rs b/crates/factor-variables/tests/factor_test.rs index e5e11ed16..7557f1882 100644 --- a/crates/factor-variables/tests/factor_test.rs +++ b/crates/factor-variables/tests/factor_test.rs @@ -1,43 +1,77 @@ -use spin_factor_variables::{StaticVariables, VariablesFactor}; -use spin_factors::{anyhow, RuntimeFactors}; +use spin_factor_variables::spin_cli::{ + EnvVariables, StaticVariables, VariableProviderConfiguration, +}; +use spin_factor_variables::VariablesFactor; +use spin_factors::{ + anyhow, Factor, FactorRuntimeConfigSource, RuntimeConfigSourceFinalizer, RuntimeFactors, +}; use spin_factors_test::{toml, TestEnvironment}; +use spin_world::v2::variables::Host; #[derive(RuntimeFactors)] struct TestFactors { - variables: VariablesFactor, + variables: VariablesFactor, } fn test_env() -> TestEnvironment { - let mut env = TestEnvironment::default_manifest_extend(toml! { + TestEnvironment::default_manifest_extend(toml! { [variables] foo = { required = true } [component.test-component] source = "does-not-exist.wasm" variables = { baz = "<{{ foo }}>" } - }); - env.runtime_config = toml! { - [[variable_provider]] - type = "static" - values = { foo = "bar" } - }; - env + }) } #[tokio::test] async fn static_provider_works() -> anyhow::Result<()> { + let runtime_config = toml! { + [[variable_provider]] + type = "static" + values = { foo = "bar" } + }; let mut factors = TestFactors { variables: VariablesFactor::default(), }; - factors.variables.add_provider_type(StaticVariables)?; + factors.variables.add_provider_resolver(StaticVariables)?; + // The env provider will be ignored since there's no configuration for it. + factors.variables.add_provider_resolver(EnvVariables)?; let env = test_env(); - let state = env.build_instance_state(factors).await?; - let val = state - .variables - .resolver() - .resolve("test-component", "baz".try_into().unwrap()) + let mut state = env + .build_instance_state(factors, TomlConfig(runtime_config)) .await?; + let val = state.variables.get("baz".try_into().unwrap()).await?; assert_eq!(val, ""); Ok(()) } + +struct TomlConfig(toml::Table); + +impl TryFrom for TestFactorsRuntimeConfig { + type Error = anyhow::Error; + + fn try_from(value: TomlConfig) -> Result { + Self::from_source(value) + } +} + +impl FactorRuntimeConfigSource> for TomlConfig { + fn get_runtime_config( + &mut self, + ) -> anyhow::Result< + Option< as Factor>::RuntimeConfig>, + > { + let Some(table) = self.0.get("variable_provider") else { + return Ok(None); + }; + Ok(Some(table.clone().try_into()?)) + } +} + +impl RuntimeConfigSourceFinalizer for TomlConfig { + fn finalize(&mut self) -> anyhow::Result<()> { + Ok(()) + } +} diff --git a/crates/factor-wasi/tests/factor_test.rs b/crates/factor-wasi/tests/factor_test.rs index 3f7084336..d6019393f 100644 --- a/crates/factor-wasi/tests/factor_test.rs +++ b/crates/factor-wasi/tests/factor_test.rs @@ -22,7 +22,7 @@ async fn environment_works() -> anyhow::Result<()> { wasi: WasiFactor::new(DummyFilesMounter), }; let env = test_env(); - let mut state = env.build_instance_state(factors).await?; + let mut state = env.build_instance_state(factors, ()).await?; let mut wasi = WasiFactor::get_wasi_impl(&mut state).unwrap(); let val = wasi .get_environment()? diff --git a/crates/factors-derive/src/lib.rs b/crates/factors-derive/src/lib.rs index c65f47485..5b2d38568 100644 --- a/crates/factors-derive/src/lib.rs +++ b/crates/factors-derive/src/lib.rs @@ -27,6 +27,7 @@ fn expand_factors(input: &DeriveInput) -> syn::Result { let app_state_name = format_ident!("{name}AppState"); let builders_name = format_ident!("{name}InstanceBuilders"); let state_name = format_ident!("{name}InstanceState"); + let runtime_config_name = format_ident!("{name}RuntimeConfig"); if !input.generics.params.is_empty() { return Err(Error::new_spanned( @@ -68,7 +69,6 @@ fn expand_factors(input: &DeriveInput) -> syn::Result { let Error = quote!(#factors_path::Error); let Factor = quote!(#factors_path::Factor); let ConfiguredApp = quote!(#factors_path::ConfiguredApp); - let RuntimeConfigTracker = quote!(#factors_path::__internal::RuntimeConfigTracker); let FactorInstanceBuilder = quote!(#factors_path::FactorInstanceBuilder); Ok(quote! { @@ -76,11 +76,23 @@ fn expand_factors(input: &DeriveInput) -> syn::Result { type AppState = #app_state_name; type InstanceBuilders = #builders_name; type InstanceState = #state_name; + type RuntimeConfig = #runtime_config_name; fn init + Send + 'static>( &mut self, linker: &mut #wasmtime::component::Linker, ) -> #Result<()> { + let factor_type_ids = [#( + (stringify!(#factor_types), #TypeId::of::<(<#factor_types as #Factor>::InstanceBuilder, <#factor_types as #Factor>::AppState)>()), + )*]; + + let mut unique = ::std::collections::HashSet::new(); + for (name, type_id) in factor_type_ids { + if !unique.insert(type_id) { + return Err(#Error::DuplicateFactorTypes(name.to_owned())); + } + } + #( #Factor::init::( &mut self.#factor_names, @@ -100,12 +112,11 @@ fn expand_factors(input: &DeriveInput) -> syn::Result { fn configure_app( &self, app: #factors_path::App, - runtime_config: impl #factors_path::RuntimeConfigSource + mut runtime_config: Self::RuntimeConfig, ) -> #Result<#ConfiguredApp> { let mut app_state = #app_state_name { #( #factor_names: None, )* }; - let mut runtime_config_tracker = #RuntimeConfigTracker::new(runtime_config); #( app_state.#factor_names = Some( #Factor::configure_app( @@ -113,12 +124,11 @@ fn expand_factors(input: &DeriveInput) -> syn::Result { #factors_path::ConfigureAppContext::::new( &app, &app_state, - &mut runtime_config_tracker, + &mut runtime_config, )?, ).map_err(#Error::factor_configure_app_error::<#factor_types>)? ); )* - runtime_config_tracker.validate_all_keys_used()?; Ok(#ConfiguredApp::new(app, app_state)) } @@ -175,9 +185,9 @@ fn expand_factors(input: &DeriveInput) -> syn::Result { fn instance_builder_mut( builders: &mut Self::InstanceBuilders, ) -> Option> { - let type_id = #TypeId::of::(); + let type_id = #TypeId::of::<(F::InstanceBuilder, F::AppState)>(); #( - if type_id == #TypeId::of::<#factor_types>() { + if type_id == #TypeId::of::<(<#factor_types as #Factor>::InstanceBuilder, <#factor_types as #Factor>::AppState)>() { return Some( builders.#factor_names.as_mut().map(|builder| { ::downcast_mut(builder).unwrap() @@ -234,5 +244,47 @@ fn expand_factors(input: &DeriveInput) -> syn::Result { self } } + + #vis struct #runtime_config_name { + #( + pub #factor_names: Option<<#factor_types as #Factor>::RuntimeConfig>, + )* + } + + impl #runtime_config_name { + /// Get the runtime configuration from the given source. + pub fn from_source(mut source: T) -> anyhow::Result + where T: #(#factors_path::FactorRuntimeConfigSource<#factor_types> +)* #factors_path::RuntimeConfigSourceFinalizer + { + #( + let #factor_names = >::get_runtime_config(&mut source)?; + )* + source.finalize()?; + Ok(#runtime_config_name { + #( + #factor_names, + )* + }) + } + } + + impl From<()> for #runtime_config_name { + fn from(_: ()) -> Self { + #runtime_config_name { + #( + #factor_names: None, + )* + } + } + } + + #( + impl #factors_path::FactorRuntimeConfigSource<#factor_types> for #runtime_config_name { + fn get_runtime_config(&mut self) -> anyhow::Result::RuntimeConfig>> { + Ok(self.#factor_names.take()) + } + } + )* + }) } diff --git a/crates/factors-test/Cargo.toml b/crates/factors-test/Cargo.toml index 23e606bde..1516b851a 100644 --- a/crates/factors-test/Cargo.toml +++ b/crates/factors-test/Cargo.toml @@ -5,6 +5,7 @@ authors = { workspace = true } edition = { workspace = true } [dependencies] +serde = "1.0" spin-app = { path = "../app" } spin-factors = { path = "../factors" } spin-factors-derive = { path = "../factors-derive", features = ["expander"] } diff --git a/crates/factors-test/src/lib.rs b/crates/factors-test/src/lib.rs index 7e9c78232..13437b0d4 100644 --- a/crates/factors-test/src/lib.rs +++ b/crates/factors-test/src/lib.rs @@ -1,9 +1,8 @@ use spin_app::locked::LockedApp; use spin_factors::{ anyhow::{self, Context}, - serde::de::DeserializeOwned, wasmtime::{component::Linker, Config, Engine}, - App, RuntimeConfigSource, RuntimeFactors, + App, RuntimeFactors, }; use spin_loader::FilesMountStrategy; @@ -13,8 +12,6 @@ pub use toml::toml; pub struct TestEnvironment { /// The `spin.toml` manifest. pub manifest: toml::Table, - /// The runtime config. - pub runtime_config: toml::Table, } impl Default for TestEnvironment { @@ -30,10 +27,7 @@ impl Default for TestEnvironment { [component.empty] source = "does-not-exist.wasm" }; - Self { - manifest, - runtime_config: Default::default(), - } + Self { manifest } } } @@ -53,10 +47,16 @@ impl TestEnvironment { /// Starting from a new _uninitialized_ [`RuntimeFactors`], run through the /// [`Factor`]s' lifecycle(s) to build a [`RuntimeFactors::InstanceState`] /// for the last component defined in the manifest. - pub async fn build_instance_state( - &self, + pub async fn build_instance_state<'a, T, C, E>( + &'a self, mut factors: T, - ) -> anyhow::Result { + runtime_config: C, + ) -> anyhow::Result + where + T: RuntimeFactors, + C: TryInto, + E: Into, + { let mut linker = Self::new_linker::(); factors.init(&mut linker)?; @@ -65,8 +65,8 @@ impl TestEnvironment { .await .context("failed to build locked app")?; let app = App::new("test-app", locked_app); - let runtime_config = TomlRuntimeConfig(&self.runtime_config); - let configured_app = factors.configure_app(app, runtime_config)?; + let configured_app = + factors.configure_app(app, runtime_config.try_into().map_err(|e| e.into())?)?; let component = configured_app.app().components().last().context( @@ -91,20 +91,3 @@ impl TestEnvironment { spin_loader::from_file(&path, FilesMountStrategy::Direct, None).await } } - -/// A [`RuntimeConfigSource`] that reads from a TOML table. -pub struct TomlRuntimeConfig<'a>(&'a toml::Table); - -impl RuntimeConfigSource for TomlRuntimeConfig<'_> { - fn factor_config_keys(&self) -> impl IntoIterator { - self.0.keys().map(|key| key.as_str()) - } - - fn get_factor_config(&self, key: &str) -> anyhow::Result> { - let Some(val) = self.0.get(key) else { - return Ok(None); - }; - let config = val.clone().try_into()?; - Ok(Some(config)) - } -} diff --git a/crates/factors/Cargo.toml b/crates/factors/Cargo.toml index 3efbb2e02..71b97694e 100644 --- a/crates/factors/Cargo.toml +++ b/crates/factors/Cargo.toml @@ -10,6 +10,8 @@ serde = "1.0" spin-app = { path = "../app" } spin-factors-derive = { path = "../factors-derive" } thiserror = "1.0" +# TODO: make this optional and behind a feature flag +toml = "0.8" tracing = { workspace = true } wasmtime = { workspace = true } @@ -28,7 +30,6 @@ spin-factor-variables = { path = "../factor-variables" } spin-factor-wasi = { path = "../factor-wasi" } spin-loader = { path = "../loader" } tokio = { version = "1", features = ["macros", "rt", "sync"] } -toml = "0.8" wasmtime-wasi-http = { workspace = true } [build-dependencies] diff --git a/crates/factors/src/factor.rs b/crates/factors/src/factor.rs index 72c57371f..39be75e59 100644 --- a/crates/factors/src/factor.rs +++ b/crates/factors/src/factor.rs @@ -3,8 +3,8 @@ use std::any::Any; use wasmtime::component::{Linker, ResourceTable}; use crate::{ - prepare::FactorInstanceBuilder, runtime_config::RuntimeConfigTracker, App, Error, - FactorRuntimeConfig, InstanceBuilders, PrepareContext, RuntimeConfigSource, RuntimeFactors, + prepare::FactorInstanceBuilder, runtime_config::FactorRuntimeConfigSource, App, Error, + InstanceBuilders, PrepareContext, RuntimeFactors, }; /// A contained (i.e., "factored") piece of runtime functionality. @@ -13,7 +13,7 @@ pub trait Factor: Any + Sized { /// /// Runtime configuration allows for user-provided customization of the /// factor's behavior on a per-app basis. - type RuntimeConfig: FactorRuntimeConfig; + type RuntimeConfig; /// The application state of this factor. /// @@ -137,12 +137,14 @@ pub struct ConfigureAppContext<'a, T: RuntimeFactors, F: Factor> { impl<'a, T: RuntimeFactors, F: Factor> ConfigureAppContext<'a, T, F> { #[doc(hidden)] - pub fn new( + pub fn new>( app: &'a App, app_state: &'a T::AppState, - runtime_config_tracker: &mut RuntimeConfigTracker, + runtime_config: &mut S, ) -> crate::Result { - let runtime_config = runtime_config_tracker.get_config::()?; + let runtime_config = runtime_config + .get_runtime_config() + .map_err(Error::factor_configure_app_error::)?; Ok(Self { app, app_state, diff --git a/crates/factors/src/lib.rs b/crates/factors/src/lib.rs index 69f5c20b8..ce0666825 100644 --- a/crates/factors/src/lib.rs +++ b/crates/factors/src/lib.rs @@ -1,6 +1,6 @@ mod factor; mod prepare; -mod runtime_config; +pub mod runtime_config; mod runtime_factors; pub use anyhow; @@ -13,7 +13,7 @@ pub use spin_factors_derive::RuntimeFactors; pub use crate::{ factor::{ConfigureAppContext, ConfiguredApp, Factor, FactorInstanceState, InitContext}, prepare::{FactorInstanceBuilder, InstanceBuilders, PrepareContext, SelfInstanceBuilder}, - runtime_config::{FactorRuntimeConfig, RuntimeConfigSource}, + runtime_config::{FactorRuntimeConfigSource, RuntimeConfigSourceFinalizer}, runtime_factors::{RuntimeFactors, RuntimeFactorsInstanceState}, }; @@ -22,6 +22,8 @@ pub type Result = std::result::Result; #[derive(Debug, thiserror::Error)] pub enum Error { + #[error("two or more factors share the same type: {0}")] + DuplicateFactorTypes(String), #[error("factor dependency ordering error: {0}")] DependencyOrderingError(String), #[error("{factor}::InstanceBuilder::build failed: {source}")] @@ -61,7 +63,7 @@ impl Error { Self::NoSuchFactor(std::any::type_name::()) } - fn runtime_config_reused_key(key: impl Into) -> Self { + pub fn runtime_config_reused_key(key: impl Into) -> Self { Self::RuntimeConfigReusedKey { factor: std::any::type_name::(), key: key.into(), @@ -94,8 +96,3 @@ impl Error { Self::FactorBuildError { factor, source } } } - -#[doc(hidden)] -pub mod __internal { - pub use crate::runtime_config::RuntimeConfigTracker; -} diff --git a/crates/factors/src/runtime_config.rs b/crates/factors/src/runtime_config.rs index 67ed29c50..a11fc4583 100644 --- a/crates/factors/src/runtime_config.rs +++ b/crates/factors/src/runtime_config.rs @@ -1,103 +1,27 @@ -use std::collections::HashSet; +pub mod toml; -use serde::de::DeserializeOwned; +use crate::Factor; -use crate::{Error, Factor}; - -pub const NO_RUNTIME_CONFIG: &str = ""; - -/// FactorRuntimeConfig represents an application's runtime configuration. -/// -/// Runtime configuration is partitioned, with each partition being the -/// responsibility of exactly one [`crate::Factor`]. If configuration needs -/// to be shared between Factors, one Factor can be selected as the owner -/// and the others will have a dependency relationship with that owner. -pub trait FactorRuntimeConfig: DeserializeOwned { - /// The key used to identify this runtime configuration in a [`RuntimeConfigSource`]. - const KEY: &'static str; -} - -impl FactorRuntimeConfig for () { - const KEY: &'static str = NO_RUNTIME_CONFIG; -} - -/// The source of runtime configuration for a Factor. -pub trait RuntimeConfigSource { - /// Returns an iterator of factor config keys available in this source. - /// - /// Should only include keys that have been positively provided and that - /// haven't already been parsed by the runtime. A runtime may treat - /// unrecognized keys as a warning or error. - fn factor_config_keys(&self) -> impl IntoIterator; - - /// Returns deserialized runtime config of the given type for the given - /// factor config key. - /// - /// Returns Ok(None) if no configuration is available for the given key. - /// Returns Err if configuration is available but deserialization fails. - fn get_factor_config(&self, key: &str) -> anyhow::Result>; +/// The source of runtime configuration for a particular [`Factor`]. +pub trait FactorRuntimeConfigSource { + /// Get the runtime configuration for the factor. + fn get_runtime_config(&mut self) -> anyhow::Result>; } -impl RuntimeConfigSource for () { - fn get_factor_config( - &self, - _factor_config_key: &str, - ) -> anyhow::Result> { +impl FactorRuntimeConfigSource for () { + fn get_runtime_config(&mut self) -> anyhow::Result::RuntimeConfig>> { Ok(None) } - - fn factor_config_keys(&self) -> impl IntoIterator { - std::iter::empty() - } } -/// Tracks runtime configuration keys used by the runtime. -/// -/// This ensures that the runtime config source does not have any unused keys. -#[doc(hidden)] -pub struct RuntimeConfigTracker { - source: S, - used_keys: HashSet<&'static str>, - unused_keys: HashSet, +/// Run some finalization logic on a [`RuntimeConfigSource`]. +pub trait RuntimeConfigSourceFinalizer { + /// Finalize the runtime config source. + fn finalize(&mut self) -> anyhow::Result<()>; } -impl RuntimeConfigTracker { - #[doc(hidden)] - pub fn new(source: S) -> Self { - let unused_keys = source - .factor_config_keys() - .into_iter() - .map(ToOwned::to_owned) - .collect(); - Self { - source, - used_keys: Default::default(), - unused_keys, - } - } - - #[doc(hidden)] - pub fn validate_all_keys_used(self) -> crate::Result<()> { - if !self.unused_keys.is_empty() { - return Err(Error::RuntimeConfigUnusedKeys { - keys: self.unused_keys.into_iter().collect(), - }); - } +impl RuntimeConfigSourceFinalizer for () { + fn finalize(&mut self) -> anyhow::Result<()> { Ok(()) } - - /// Get the runtime configuration for a factor. - pub(crate) fn get_config(&mut self) -> crate::Result> { - let key = F::RuntimeConfig::KEY; - if key == NO_RUNTIME_CONFIG { - return Ok(None); - } - if !self.used_keys.insert(key) { - return Err(Error::runtime_config_reused_key::(key)); - } - self.unused_keys.remove(key); - self.source - .get_factor_config::(key) - .map_err(Error::RuntimeConfigSource) - } } diff --git a/crates/factors/src/runtime_config/toml.rs b/crates/factors/src/runtime_config/toml.rs new file mode 100644 index 000000000..15e801a47 --- /dev/null +++ b/crates/factors/src/runtime_config/toml.rs @@ -0,0 +1,50 @@ +//! Helpers for reading runtime configuration from a TOML file. + +use std::{cell::RefCell, collections::HashSet}; + +/// A trait for getting a TOML value by key. +pub trait GetTomlValue { + fn get(&self, key: &str) -> Option<&toml::Value>; +} + +/// A helper for tracking which keys have been used in a TOML table. +pub struct TomlKeyTracker<'a> { + unused_keys: RefCell>, + table: &'a toml::Table, +} + +impl<'a> TomlKeyTracker<'a> { + pub fn new(table: &'a toml::Table) -> Self { + Self { + unused_keys: RefCell::new(table.keys().map(String::as_str).collect()), + table, + } + } + + pub fn validate_all_keys_used(&self) -> crate::Result<()> { + if !self.unused_keys.borrow().is_empty() { + return Err(crate::Error::RuntimeConfigUnusedKeys { + keys: self + .unused_keys + .borrow() + .iter() + .map(|s| (*s).to_owned()) + .collect(), + }); + } + Ok(()) + } +} + +impl GetTomlValue for TomlKeyTracker<'_> { + fn get(&self, key: &str) -> Option<&toml::Value> { + self.unused_keys.borrow_mut().remove(key); + self.table.get(key) + } +} + +impl AsRef for TomlKeyTracker<'_> { + fn as_ref(&self) -> &toml::Table { + self.table + } +} diff --git a/crates/factors/src/runtime_factors.rs b/crates/factors/src/runtime_factors.rs index 78154b7eb..124005d32 100644 --- a/crates/factors/src/runtime_factors.rs +++ b/crates/factors/src/runtime_factors.rs @@ -1,6 +1,6 @@ use wasmtime::component::{Linker, ResourceTable}; -use crate::{factor::FactorInstanceState, App, ConfiguredApp, Factor, RuntimeConfigSource}; +use crate::{factor::FactorInstanceState, App, ConfiguredApp, Factor}; /// A collection of `Factor`s that are initialized and configured together. /// @@ -37,6 +37,8 @@ pub trait RuntimeFactors: Sized + 'static { type InstanceState: RuntimeFactorsInstanceState; /// The collection of all the `InstanceBuilder`s of the factors. type InstanceBuilders; + /// The runtime configuration of all the factors. + type RuntimeConfig; /// Initialize the factors with the given linker. /// @@ -51,7 +53,7 @@ pub trait RuntimeFactors: Sized + 'static { fn configure_app( &self, app: App, - runtime_config: impl RuntimeConfigSource, + runtime_config: Self::RuntimeConfig, ) -> crate::Result>; /// Prepare the factors' instance state builders. diff --git a/crates/factors/tests/smoke.rs b/crates/factors/tests/smoke.rs index fb92b509a..205775ca1 100644 --- a/crates/factors/tests/smoke.rs +++ b/crates/factors/tests/smoke.rs @@ -11,15 +11,20 @@ use spin_factor_key_value_redis::RedisKeyValueStore; use spin_factor_key_value_spin::{SpinKeyValueRuntimeConfig, SpinKeyValueStore}; use spin_factor_outbound_http::OutboundHttpFactor; use spin_factor_outbound_networking::OutboundNetworkingFactor; -use spin_factor_variables::{StaticVariables, VariablesFactor}; +use spin_factor_variables::{ + spin_cli::{StaticVariables, VariableProviderConfiguration}, + VariablesFactor, +}; use spin_factor_wasi::{DummyFilesMounter, WasiFactor}; -use spin_factors::{FactorRuntimeConfig, RuntimeConfigSource, RuntimeFactors}; +use spin_factors::{ + Factor, FactorRuntimeConfigSource, RuntimeConfigSourceFinalizer, RuntimeFactors, +}; use wasmtime_wasi_http::WasiHttpView; #[derive(RuntimeFactors)] struct Factors { wasi: WasiFactor, - variables: VariablesFactor, + variables: VariablesFactor, outbound_networking: OutboundNetworkingFactor, outbound_http: OutboundHttpFactor, key_value: KeyValueFactor, @@ -61,7 +66,7 @@ async fn smoke_test_works() -> anyhow::Result<()> { key_value: KeyValueFactor::new(key_value_resolver), }; - factors.variables.add_provider_type(StaticVariables)?; + factors.variables.add_provider_resolver(StaticVariables)?; let locked = spin_loader::from_file( "tests/smoke-app/spin.toml", @@ -74,16 +79,16 @@ async fn smoke_test_works() -> anyhow::Result<()> { let engine = wasmtime::Engine::new(wasmtime::Config::new().async_support(true))?; let mut linker = wasmtime::component::Linker::new(&engine); - factors.init(&mut linker).unwrap(); + factors.init(&mut linker)?; - let configured_app = factors.configure_app(app, TestSource)?; + let configured_app = factors.configure_app(app, TestSource.try_into()?)?; let builders = factors.prepare(&configured_app, "smoke-app")?; let state = factors.build_instance_state(builders)?; assert_eq!( state .variables - .resolver() + .expression_resolver() .resolve("smoke-app", "other".try_into().unwrap()) .await .unwrap(), @@ -140,29 +145,74 @@ async fn smoke_test_works() -> anyhow::Result<()> { struct TestSource; -impl RuntimeConfigSource for TestSource { - fn factor_config_keys(&self) -> impl IntoIterator { - [spin_factor_variables::RuntimeConfig::KEY] +impl TryFrom for FactorsRuntimeConfig { + type Error = anyhow::Error; + + fn try_from(value: TestSource) -> Result { + Self::from_source(value) + } +} + +impl FactorRuntimeConfigSource> for TestSource { + fn get_runtime_config( + &mut self, + ) -> anyhow::Result< + Option< as Factor>::RuntimeConfig>, + > { + let config = toml::toml! { + [other] + type = "redis" + url = "redis://localhost:6379" + }; + + Ok(Some(config.try_into()?)) } +} - fn get_factor_config( - &self, - key: &str, - ) -> anyhow::Result> { - let Some(table) = toml::toml! { +impl FactorRuntimeConfigSource> for TestSource { + fn get_runtime_config( + &mut self, + ) -> anyhow::Result< + Option< as Factor>::RuntimeConfig>, + > { + let config = toml::toml! { [[variable_provider]] type = "static" [variable_provider.values] foo = "bar" - - [key_value_store.other] - type = "redis" - url = "redis://localhost:6379" } - .remove(key) else { - return Ok(None); - }; - let config = table.try_into()?; - Ok(Some(config)) + .remove("variable_provider") + .unwrap(); + Ok(Some(config.try_into()?)) + } +} + +impl FactorRuntimeConfigSource for TestSource { + fn get_runtime_config( + &mut self, + ) -> anyhow::Result::RuntimeConfig>> { + Ok(None) + } +} + +impl FactorRuntimeConfigSource for TestSource { + fn get_runtime_config( + &mut self, + ) -> anyhow::Result::RuntimeConfig>> { + Ok(None) + } +} + +impl FactorRuntimeConfigSource for TestSource { + fn get_runtime_config( + &mut self, + ) -> anyhow::Result::RuntimeConfig>> { + Ok(None) + } +} + +impl RuntimeConfigSourceFinalizer for TestSource { + fn finalize(&mut self) -> anyhow::Result<()> { + Ok(()) } }