Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Runtime Config Independent of Serialization #2685

Merged
merged 13 commits into from
Jul 26, 2024
9 changes: 5 additions & 4 deletions Cargo.lock

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

6 changes: 1 addition & 5 deletions crates/factor-key-value/src/runtime_config.rs
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -12,10 +12,6 @@ pub struct RuntimeConfig<C> {
pub store_configs: HashMap<String, C>,
}

impl<C: DeserializeOwned> FactorRuntimeConfig for RuntimeConfig<C> {
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.
Expand Down
51 changes: 41 additions & 10 deletions crates/factor-key-value/tests/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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(),
Expand All @@ -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::<Vec<_>>()
Expand Down Expand Up @@ -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(),
Expand All @@ -222,3 +224,32 @@ async fn multiple_custom_key_value_uses_first_store() -> anyhow::Result<()> {
assert!(tmp_dir.path().exists());
Ok(())
}

struct TomlConfig(Option<toml::Table>);

impl TryFrom<TomlConfig> for TestFactorsRuntimeConfig {
type Error = anyhow::Error;

fn try_from(value: TomlConfig) -> Result<Self, Self::Error> {
Self::from_source(value)
}
}

impl FactorRuntimeConfigSource<KeyValueFactor<DelegatingRuntimeConfigResolver>> for TomlConfig {
fn get_runtime_config(
&mut self,
) -> anyhow::Result<
Option<<KeyValueFactor<DelegatingRuntimeConfigResolver> 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(())
}
}
2 changes: 1 addition & 1 deletion crates/factor-llm/tests/factor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()]
Expand Down
4 changes: 2 additions & 2 deletions crates/factor-outbound-http/tests/factor_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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())?;
Expand Down
5 changes: 4 additions & 1 deletion crates/factor-outbound-networking/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,10 @@ impl Factor for OutboundNetworkingFactor {
.get(ctx.app_component().id())
.cloned()
.context("missing component allowed hosts")?;
let resolver = builders.get_mut::<VariablesFactor>()?.resolver().clone();
let resolver = builders
.get_mut::<VariablesFactor<()>>()?
.expression_resolver()
.clone();
let allowed_hosts_future = async move {
let prepared = resolver.prepare().await?;
AllowedHostsConfig::parse(&hosts, &prepared)
Expand Down
15 changes: 9 additions & 6 deletions crates/factor-outbound-networking/tests/factor_test.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -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()?;
Expand Down Expand Up @@ -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(())
}
12 changes: 6 additions & 6 deletions crates/factor-outbound-pg/tests/factor_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -24,7 +24,7 @@ fn factors() -> Result<TestFactors> {
networking: OutboundNetworkingFactor,
pg: OutboundPgFactor::<MockClient>::new(),
};
f.variables.add_provider_type(StaticVariables)?;
f.variables.add_provider_resolver(StaticVariables)?;
Ok(f)
}

Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
6 changes: 3 additions & 3 deletions crates/factor-outbound-redis/tests/factor_test.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand All @@ -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! {
Expand All @@ -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())
Expand Down
39 changes: 22 additions & 17 deletions crates/factor-sqlite/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<R> {
runtime_config_resolver: Arc<R>,
pub struct SqliteFactor {
default_label_resolver: Arc<dyn DefaultLabelResolver>,
}

impl<R> SqliteFactor<R> {
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<R: RuntimeConfigResolver + 'static> Factor for SqliteFactor<R> {
type RuntimeConfig = runtime_config::RuntimeConfig<R::Config>;
impl Factor for SqliteFactor {
type RuntimeConfig = runtime_config::RuntimeConfig;
type AppState = AppState;
type InstanceBuilder = InstanceState;

Expand All @@ -46,13 +46,10 @@ impl<R: RuntimeConfigResolver + 'static> Factor for SqliteFactor<R> {
&self,
mut ctx: spin_factors::ConfigureAppContext<T, Self>,
) -> anyhow::Result<Self::AppState> {
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()
Expand All @@ -70,7 +67,7 @@ impl<R: RuntimeConfigResolver + 'static> Factor for SqliteFactor<R> {
))
})
.collect::<anyhow::Result<HashMap<_, _>>>()?;
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)
Expand Down Expand Up @@ -139,6 +136,14 @@ fn ensure_allowed_databases_are_configured(

pub const ALLOWED_DATABASES_KEY: MetadataKey<Vec<String>> = 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<Arc<dyn ConnectionPool>>;
}

pub struct AppState {
/// A map from component id to a set of allowed database labels.
allowed_databases: HashMap<String, Arc<HashSet<String>>>,
Expand Down
Loading
Loading