diff --git a/crates/factor-outbound-http/tests/factor_test.rs b/crates/factor-outbound-http/tests/factor_test.rs index 90394f3a0..9abee5f57 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::spin_cli::VariablesFactor; +use spin_factor_variables::VariablesFactor; use spin_factors::{anyhow, RuntimeFactors}; use spin_factors_test::{toml, TestEnvironment}; use wasmtime_wasi_http::{ diff --git a/crates/factor-outbound-networking/src/lib.rs b/crates/factor-outbound-networking/src/lib.rs index fbb973947..9d7f04b2d 100644 --- a/crates/factor-outbound-networking/src/lib.rs +++ b/crates/factor-outbound-networking/src/lib.rs @@ -60,7 +60,7 @@ impl Factor for OutboundNetworkingFactor { .cloned() .context("missing component allowed hosts")?; let resolver = builders - .get_mut::>()? + .get_mut::()? .expression_resolver() .clone(); let allowed_hosts_future = async move { diff --git a/crates/factor-outbound-networking/tests/factor_test.rs b/crates/factor-outbound-networking/tests/factor_test.rs index 8d85af094..693ac6600 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::spin_cli::VariablesFactor; +use spin_factor_variables::VariablesFactor; use spin_factor_wasi::{DummyFilesMounter, WasiFactor}; use spin_factors::{anyhow, RuntimeFactors}; use spin_factors_test::{toml, TestEnvironment}; diff --git a/crates/factor-outbound-pg/tests/factor_test.rs b/crates/factor-outbound-pg/tests/factor_test.rs index 77fa257bb..e189b9d2a 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::spin_cli::VariablesFactor; +use spin_factor_variables::VariablesFactor; use spin_factors::{anyhow, RuntimeFactors}; use spin_factors_test::{toml, TestEnvironment}; use spin_world::async_trait; diff --git a/crates/factor-outbound-redis/tests/factor_test.rs b/crates/factor-outbound-redis/tests/factor_test.rs index dc6578a33..14c1ee24f 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::spin_cli::VariablesFactor; +use spin_factor_variables::VariablesFactor; use spin_factors::{anyhow, RuntimeFactors}; use spin_factors_test::{toml, TestEnvironment}; use spin_world::v2::redis::{Error, HostConnection}; diff --git a/crates/factor-variables/src/host.rs b/crates/factor-variables/src/host.rs new file mode 100644 index 000000000..aa1d70f34 --- /dev/null +++ b/crates/factor-variables/src/host.rs @@ -0,0 +1,46 @@ +use spin_factors::anyhow; +use spin_world::{async_trait, v1, v2::variables}; + +use crate::InstanceState; + +#[async_trait] +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.expression_resolver + .resolve(&self.component_id, key) + .await + .map_err(expressions_to_variables_err) + } + + fn convert_error(&mut self, error: variables::Error) -> anyhow::Result { + Ok(error) + } +} + +#[async_trait] +impl v1::config::Host for InstanceState { + async fn get_config(&mut self, key: String) -> Result { + ::get(self, key) + .await + .map_err(|err| match err { + variables::Error::InvalidName(msg) => v1::config::Error::InvalidKey(msg), + variables::Error::Undefined(msg) => v1::config::Error::Provider(msg), + other => v1::config::Error::Other(format!("{other}")), + }) + } + + fn convert_error(&mut self, err: v1::config::Error) -> anyhow::Result { + Ok(err) + } +} + +fn expressions_to_variables_err(err: spin_expressions::Error) -> variables::Error { + use spin_expressions::Error; + match err { + Error::InvalidName(msg) => variables::Error::InvalidName(msg), + Error::Undefined(msg) => variables::Error::Undefined(msg), + Error::Provider(err) => variables::Error::Provider(err.to_string()), + other => variables::Error::Other(format!("{other}")), + } +} diff --git a/crates/factor-variables/src/lib.rs b/crates/factor-variables/src/lib.rs index 5568ec360..9ea892a2b 100644 --- a/crates/factor-variables/src/lib.rs +++ b/crates/factor-variables/src/lib.rs @@ -1,49 +1,24 @@ -pub mod provider; +mod host; +pub mod runtime_config; pub mod spin_cli; use std::sync::Arc; -use serde::{de::DeserializeOwned, Deserialize}; +use runtime_config::RuntimeConfig; use spin_expressions::ProviderResolver as ExpressionResolver; use spin_factors::{ anyhow, ConfigureAppContext, Factor, InitContext, InstanceBuilders, PrepareContext, RuntimeFactors, SelfInstanceBuilder, }; -use spin_world::{async_trait, v1, v2::variables}; - -pub use provider::ProviderResolver; /// 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 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<()> { - self.provider_resolvers.push(Box::new(provider_type)); - Ok(()) - } +#[derive(Default)] +pub struct VariablesFactor { + _priv: (), } -impl Factor for VariablesFactor { - type RuntimeConfig = RuntimeConfig; +impl Factor for VariablesFactor { + type RuntimeConfig = RuntimeConfig; type AppState = AppState; type InstanceBuilder = InstanceState; @@ -68,14 +43,8 @@ impl Factor for VariablesFactor { )?; } - if let Some(runtime_config) = ctx.take_runtime_config() { - 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); - } - } - } + for provider in ctx.take_runtime_config().unwrap_or_default() { + expression_resolver.add_provider(provider); } Ok(AppState { @@ -97,13 +66,6 @@ impl Factor for VariablesFactor { } } -/// The runtime configuration for the variables factor. -#[derive(Deserialize)] -#[serde(transparent)] -pub struct RuntimeConfig { - provider_configs: Vec, -} - pub struct AppState { expression_resolver: Arc, } @@ -120,45 +82,3 @@ impl InstanceState { } impl SelfInstanceBuilder for InstanceState {} - -#[async_trait] -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.expression_resolver - .resolve(&self.component_id, key) - .await - .map_err(expressions_to_variables_err) - } - - fn convert_error(&mut self, error: variables::Error) -> anyhow::Result { - Ok(error) - } -} - -#[async_trait] -impl v1::config::Host for InstanceState { - async fn get_config(&mut self, key: String) -> Result { - ::get(self, key) - .await - .map_err(|err| match err { - variables::Error::InvalidName(msg) => v1::config::Error::InvalidKey(msg), - variables::Error::Undefined(msg) => v1::config::Error::Provider(msg), - other => v1::config::Error::Other(format!("{other}")), - }) - } - - fn convert_error(&mut self, err: v1::config::Error) -> anyhow::Result { - Ok(err) - } -} - -fn expressions_to_variables_err(err: spin_expressions::Error) -> variables::Error { - use spin_expressions::Error; - match err { - Error::InvalidName(msg) => variables::Error::InvalidName(msg), - Error::Undefined(msg) => variables::Error::Undefined(msg), - Error::Provider(err) => variables::Error::Provider(err.to_string()), - other => variables::Error::Other(format!("{other}")), - } -} diff --git a/crates/factor-variables/src/provider.rs b/crates/factor-variables/src/provider.rs deleted file mode 100644 index 82821fb32..000000000 --- a/crates/factor-variables/src/provider.rs +++ /dev/null @@ -1,17 +0,0 @@ -use serde::de::DeserializeOwned; -use spin_expressions::Provider; -use spin_factors::anyhow; - -/// A trait for converting a runtime configuration into a variables provider. -pub trait ProviderResolver: 'static { - /// Serialized configuration for the provider. - type RuntimeConfig: DeserializeOwned; - - /// 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/runtime_config.rs b/crates/factor-variables/src/runtime_config.rs new file mode 100644 index 000000000..aaa7eb8da --- /dev/null +++ b/crates/factor-variables/src/runtime_config.rs @@ -0,0 +1,16 @@ +use spin_expressions::Provider; + +/// The runtime configuration for the variables factor. +#[derive(Default)] +pub struct RuntimeConfig { + pub providers: Vec>, +} + +impl IntoIterator for RuntimeConfig { + type Item = Box; + type IntoIter = std::vec::IntoIter>; + + fn into_iter(self) -> Self::IntoIter { + self.providers.into_iter() + } +} diff --git a/crates/factor-variables/src/spin_cli/env.rs b/crates/factor-variables/src/spin_cli/env.rs index 779f54d85..781140a1b 100644 --- a/crates/factor-variables/src/spin_cli/env.rs +++ b/crates/factor-variables/src/spin_cli/env.rs @@ -11,31 +11,6 @@ use spin_factors::anyhow::{self, Context as _}; use spin_world::async_trait; use tracing::{instrument, Level}; -use crate::ProviderResolver; - -use super::VariableProviderConfiguration; - -/// Creator of a environment variables provider. -pub struct EnvVariables; - -impl ProviderResolver for EnvVariables { - type RuntimeConfig = VariableProviderConfiguration; - - 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.clone(), - )))) - } -} - /// Configuration for the environment variables provider. #[derive(Debug, Default, Deserialize)] #[serde(deny_unknown_fields)] @@ -54,7 +29,7 @@ const DEFAULT_ENV_PREFIX: &str = "SPIN_VARIABLE"; type EnvFetcherFn = Box Result + Send + Sync>; -/// A config Provider that uses environment variables. +/// A [`Provider`] that uses environment variables. pub struct EnvVariablesProvider { prefix: Option, env_fetcher: EnvFetcherFn, @@ -62,14 +37,25 @@ pub struct EnvVariablesProvider { dotenv_cache: OnceLock>, } +impl Default for EnvVariablesProvider { + fn default() -> Self { + Self { + prefix: None, + env_fetcher: Box::new(|s| std::env::var(s)), + dotenv_path: Some(".env".into()), + dotenv_cache: Default::default(), + } + } +} + impl EnvVariablesProvider { /// Creates a new EnvProvider. /// /// * `prefix` - The string prefix to use to distinguish an environment variable that should be used. - /// If not set, the default prefix is used. + /// If not set, the default prefix is used. /// * `env_fetcher` - The function to use to fetch an environment variable. /// * `dotenv_path` - The path to the .env file to load environment variables from. If not set, - /// no .env file is loaded. + /// no .env file is loaded. pub fn new( prefix: Option>, env_fetcher: impl Fn(&str) -> Result + Send + Sync + 'static, @@ -88,7 +74,7 @@ impl EnvVariablesProvider { let prefix = self .prefix .clone() - .unwrap_or(DEFAULT_ENV_PREFIX.to_string()); + .unwrap_or_else(|| DEFAULT_ENV_PREFIX.to_string()); let upper_key = key.as_ref().to_ascii_uppercase(); let env_key = format!("{prefix}_{upper_key}"); diff --git a/crates/factor-variables/src/spin_cli/mod.rs b/crates/factor-variables/src/spin_cli/mod.rs index 5e6a2bd28..2b6a48c48 100644 --- a/crates/factor-variables/src/spin_cli/mod.rs +++ b/crates/factor-variables/src/spin_cli/mod.rs @@ -3,11 +3,31 @@ mod env; mod statik; -pub use env::EnvVariables; -pub use statik::StaticVariables; +pub use env::*; +pub use statik::*; use serde::Deserialize; -use statik::StaticVariablesProvider; +use spin_expressions::Provider; +use spin_factors::anyhow; + +use crate::runtime_config::RuntimeConfig; + +/// Resolves a runtime configuration for the variables factor from a TOML table. +pub fn runtime_config_from_toml(table: &toml::Table) -> anyhow::Result { + // Always include the environment variable provider. + let mut providers = vec![Box::new(EnvVariablesProvider::default()) as _]; + let Some(array) = table.get("variable_provider") else { + return Ok(RuntimeConfig { providers }); + }; + + let provider_configs: Vec = array.clone().try_into()?; + providers.extend( + provider_configs + .into_iter() + .map(VariableProviderConfiguration::into_provider), + ); + Ok(RuntimeConfig { providers }) +} /// A runtime configuration used in the Spin CLI for one type of variable provider. #[derive(Debug, Deserialize)] @@ -16,11 +36,19 @@ pub enum VariableProviderConfiguration { /// A static provider of variables. Static(StaticVariablesProvider), /// An environment variable provider. - Env(env::EnvVariablesConfig), + 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; +impl VariableProviderConfiguration { + /// Returns the provider for the configuration. + pub fn into_provider(self) -> Box { + match self { + VariableProviderConfiguration::Static(provider) => Box::new(provider), + VariableProviderConfiguration::Env(config) => Box::new(env::EnvVariablesProvider::new( + config.prefix, + |s| std::env::var(s), + config.dotenv_path, + )), + } + } +} diff --git a/crates/factor-variables/src/spin_cli/statik.rs b/crates/factor-variables/src/spin_cli/statik.rs index a34756526..d596c3e56 100644 --- a/crates/factor-variables/src/spin_cli/statik.rs +++ b/crates/factor-variables/src/spin_cli/statik.rs @@ -4,28 +4,7 @@ use serde::Deserialize; use spin_expressions::{async_trait::async_trait, Key, Provider}; use spin_factors::anyhow; -use crate::ProviderResolver; - -use super::VariableProviderConfiguration; - -/// Creator of a static variables provider. -pub struct StaticVariables; - -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. +/// A [`Provider`] that reads variables from an static map. #[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 cc2735574..1ce403000 100644 --- a/crates/factor-variables/tests/factor_test.rs +++ b/crates/factor-variables/tests/factor_test.rs @@ -1,7 +1,4 @@ -use spin_factor_variables::spin_cli::{ - EnvVariables, StaticVariables, VariableProviderConfiguration, -}; -use spin_factor_variables::VariablesFactor; +use spin_factor_variables::{spin_cli, VariablesFactor}; use spin_factors::{ anyhow, Factor, FactorRuntimeConfigSource, RuntimeConfigSourceFinalizer, RuntimeFactors, }; @@ -10,18 +7,14 @@ use spin_world::v2::variables::Host; #[derive(RuntimeFactors)] struct TestFactors { - variables: VariablesFactor, + variables: VariablesFactor, } -#[tokio::test] +#[tokio::test(flavor = "multi_thread")] async fn static_provider_works() -> anyhow::Result<()> { - let mut factors = TestFactors { + let factors = TestFactors { variables: VariablesFactor::default(), }; - 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 = TestEnvironment::new(factors) .extend_manifest(toml! { [variables] @@ -31,7 +24,7 @@ async fn static_provider_works() -> anyhow::Result<()> { source = "does-not-exist.wasm" variables = { baz = "<{{ foo }}>" } }) - .runtime_config(TomlConfig(toml! { + .runtime_config(TomlConfig::new(toml! { [[variable_provider]] type = "static" values = { foo = "bar" } @@ -43,7 +36,15 @@ async fn static_provider_works() -> anyhow::Result<()> { Ok(()) } -struct TomlConfig(toml::Table); +struct TomlConfig { + table: toml::Table, +} + +impl TomlConfig { + fn new(table: toml::Table) -> Self { + Self { table } + } +} impl TryFrom for TestFactorsRuntimeConfig { type Error = anyhow::Error; @@ -53,16 +54,11 @@ impl TryFrom for TestFactorsRuntimeConfig { } } -impl FactorRuntimeConfigSource> for TomlConfig { +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()?)) + ) -> anyhow::Result::RuntimeConfig>> { + spin_cli::runtime_config_from_toml(&self.table).map(Some) } } diff --git a/crates/factors/tests/smoke.rs b/crates/factors/tests/smoke.rs index 205775ca1..6beb96f50 100644 --- a/crates/factors/tests/smoke.rs +++ b/crates/factors/tests/smoke.rs @@ -11,10 +11,7 @@ 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::{ - spin_cli::{StaticVariables, VariableProviderConfiguration}, - VariablesFactor, -}; +use spin_factor_variables::VariablesFactor; use spin_factor_wasi::{DummyFilesMounter, WasiFactor}; use spin_factors::{ Factor, FactorRuntimeConfigSource, RuntimeConfigSourceFinalizer, RuntimeFactors, @@ -24,7 +21,7 @@ use wasmtime_wasi_http::WasiHttpView; #[derive(RuntimeFactors)] struct Factors { wasi: WasiFactor, - variables: VariablesFactor, + variables: VariablesFactor, outbound_networking: OutboundNetworkingFactor, outbound_http: OutboundHttpFactor, key_value: KeyValueFactor, @@ -66,8 +63,6 @@ async fn smoke_test_works() -> anyhow::Result<()> { key_value: KeyValueFactor::new(key_value_resolver), }; - factors.variables.add_provider_resolver(StaticVariables)?; - let locked = spin_loader::from_file( "tests/smoke-app/spin.toml", spin_loader::FilesMountStrategy::Direct, @@ -169,21 +164,17 @@ impl FactorRuntimeConfigSource> } } -impl FactorRuntimeConfigSource> for TestSource { +impl FactorRuntimeConfigSource for TestSource { fn get_runtime_config( &mut self, - ) -> anyhow::Result< - Option< as Factor>::RuntimeConfig>, - > { - let config = toml::toml! { + ) -> anyhow::Result::RuntimeConfig>> { + spin_factor_variables::spin_cli::runtime_config_from_toml(&toml::toml! { [[variable_provider]] type = "static" [variable_provider.values] foo = "bar" - } - .remove("variable_provider") - .unwrap(); - Ok(Some(config.try_into()?)) + }) + .map(Some) } }