Merge pull request #2688 from fermyon/ungeneric-variables

Make `VariablesFactor` non-generic

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::{

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::<VariablesFactor<()>>()?
+            .get_mut::<VariablesFactor>()?
             .expression_resolver()
             .clone();
         let allowed_hosts_future = async move {

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};

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;

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};

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<String, variables::Error> {
+        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<variables::Error> {
+        Ok(error)
+    }
+}
+
+#[async_trait]
+impl v1::config::Host for InstanceState {
+    async fn get_config(&mut self, key: String) -> Result<String, v1::config::Error> {
+        <Self as variables::Host>::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<v1::config::Error> {
+        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}")),
+    }
+}

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<C> {
-    provider_resolvers: Vec<Box<dyn ProviderResolver<RuntimeConfig = C>>>,
-}
-
-impl<C> Default for VariablesFactor<C> {
-    fn default() -> Self {
-        Self {
-            provider_resolvers: Default::default(),
-        }
-    }
-}
-
-impl<C> VariablesFactor<C> {
-    /// 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<T: ProviderResolver<RuntimeConfig = C>>(
-        &mut self,
-        provider_type: T,
-    ) -> anyhow::Result<()> {
-        self.provider_resolvers.push(Box::new(provider_type));
-        Ok(())
-    }
+#[derive(Default)]
+pub struct VariablesFactor {
+    _priv: (),
 }
 
-impl<C: DeserializeOwned + 'static> Factor for VariablesFactor<C> {
-    type RuntimeConfig = RuntimeConfig<C>;
+impl Factor for VariablesFactor {
+    type RuntimeConfig = RuntimeConfig;
     type AppState = AppState;
     type InstanceBuilder = InstanceState;
 
@@ -68,14 +43,8 @@ impl<C: DeserializeOwned + 'static> Factor for VariablesFactor<C> {
             )?;
         }
 
-        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<C: DeserializeOwned + 'static> Factor for VariablesFactor<C> {
     }
 }
 
-/// The runtime configuration for the variables factor.
-#[derive(Deserialize)]
-#[serde(transparent)]
-pub struct RuntimeConfig<C> {
-    provider_configs: Vec<C>,
-}
-
 pub struct AppState {
     expression_resolver: Arc<ExpressionResolver>,
 }
@@ -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<String, variables::Error> {
-        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<variables::Error> {
-        Ok(error)
-    }
-}
-
-#[async_trait]
-impl v1::config::Host for InstanceState {
-    async fn get_config(&mut self, key: String) -> Result<String, v1::config::Error> {
-        <Self as variables::Host>::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<v1::config::Error> {
-        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}")),
-    }
-}

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<Box<dyn Provider>>,
+}
+
+impl IntoIterator for RuntimeConfig {
+    type Item = Box<dyn Provider>;
+    type IntoIter = std::vec::IntoIter<Box<dyn Provider>>;
+
+    fn into_iter(self) -> Self::IntoIter {
+        self.providers.into_iter()
+    }
+}

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<Option<Box<dyn Provider>>> {
-        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,22 +29,33 @@ const DEFAULT_ENV_PREFIX: &str = "SPIN_VARIABLE";
 
 type EnvFetcherFn = Box<dyn Fn(&str) -> Result<String, VarError> + Send + Sync>;
 
-/// A config Provider that uses environment variables.
+/// A [`Provider`] that uses environment variables.
 pub struct EnvVariablesProvider {
     prefix: Option<String>,
     env_fetcher: EnvFetcherFn,
     dotenv_path: Option<PathBuf>,
     dotenv_cache: OnceLock<HashMap<String, String>>,
 }
 
+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<impl Into<String>>,
         env_fetcher: impl Fn(&str) -> Result<String, VarError> + 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}");

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<RuntimeConfig> {
+    // 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<VariableProviderConfiguration> = 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<VariableProviderConfiguration>;
-
-/// The variables factor used in the Spin CLI.
-pub type VariablesFactor = super::VariablesFactor<VariableProviderConfiguration>;
+impl VariableProviderConfiguration {
+    /// Returns the provider for the configuration.
+    pub fn into_provider(self) -> Box<dyn Provider> {
+        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,
+            )),
+        }
+    }
+}