From 12cc792f96351a4dc259046f5c35364ff204fc08 Mon Sep 17 00:00:00 2001 From: Ryan Levick Date: Fri, 16 Aug 2024 18:31:46 +0200 Subject: [PATCH 1/2] Integrate SQLite factor into trigger2 Signed-off-by: Ryan Levick --- Cargo.lock | 38 +++++----- crates/factor-sqlite/Cargo.toml | 2 +- crates/factor-sqlite/src/lib.rs | 4 +- .../factor-sqlite/src/runtime_config/spin.rs | 14 ++-- crates/factor-sqlite/tests/factor_test.rs | 21 +++--- crates/runtime-config/Cargo.toml | 1 + crates/runtime-config/src/lib.rs | 70 ++++++++++++++++--- crates/trigger2/Cargo.toml | 1 + crates/trigger2/src/cli.rs | 3 +- crates/trigger2/src/factors.rs | 8 ++- 10 files changed, 112 insertions(+), 50 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4b5828045f..e773786f20 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2498,24 +2498,6 @@ dependencies = [ "tracing", ] -[[package]] -name = "factor-sqlite" -version = "2.7.0-pre0" -dependencies = [ - "async-trait", - "serde 1.0.197", - "spin-factors", - "spin-factors-test", - "spin-locked-app", - "spin-sqlite", - "spin-sqlite-inproc", - "spin-sqlite-libsql", - "spin-world", - "table", - "tokio", - "toml 0.8.14", -] - [[package]] name = "fallible-iterator" version = "0.2.0" @@ -7787,6 +7769,24 @@ dependencies = [ "tracing", ] +[[package]] +name = "spin-factor-sqlite" +version = "2.7.0-pre0" +dependencies = [ + "async-trait", + "serde 1.0.197", + "spin-factors", + "spin-factors-test", + "spin-locked-app", + "spin-sqlite", + "spin-sqlite-inproc", + "spin-sqlite-libsql", + "spin-world", + "table", + "tokio", + "toml 0.8.14", +] + [[package]] name = "spin-factor-variables" version = "2.7.0-pre0" @@ -8164,6 +8164,7 @@ dependencies = [ "spin-factor-key-value-spin", "spin-factor-outbound-http", "spin-factor-outbound-networking", + "spin-factor-sqlite", "spin-factor-wasi", "spin-factors", "toml 0.8.14", @@ -8471,6 +8472,7 @@ dependencies = [ "spin-factor-key-value", "spin-factor-outbound-http", "spin-factor-outbound-networking", + "spin-factor-sqlite", "spin-factor-wasi", "spin-factors", "spin-factors-executor", diff --git a/crates/factor-sqlite/Cargo.toml b/crates/factor-sqlite/Cargo.toml index 24442bd33c..3b45dcf3ca 100644 --- a/crates/factor-sqlite/Cargo.toml +++ b/crates/factor-sqlite/Cargo.toml @@ -1,5 +1,5 @@ [package] -name = "factor-sqlite" +name = "spin-factor-sqlite" version.workspace = true authors.workspace = true edition.workspace = true diff --git a/crates/factor-sqlite/src/lib.rs b/crates/factor-sqlite/src/lib.rs index e9a80e646b..4d90a1c64a 100644 --- a/crates/factor-sqlite/src/lib.rs +++ b/crates/factor-sqlite/src/lib.rs @@ -12,6 +12,8 @@ use spin_locked_app::MetadataKey; use spin_world::v1::sqlite as v1; use spin_world::v2::sqlite as v2; +pub use runtime_config::RuntimeConfig; + pub struct SqliteFactor { default_label_resolver: Arc, } @@ -29,7 +31,7 @@ impl SqliteFactor { } impl Factor for SqliteFactor { - type RuntimeConfig = runtime_config::RuntimeConfig; + type RuntimeConfig = RuntimeConfig; type AppState = AppState; type InstanceBuilder = InstanceState; diff --git a/crates/factor-sqlite/src/runtime_config/spin.rs b/crates/factor-sqlite/src/runtime_config/spin.rs index bf14cf04b0..3bc2452f0b 100644 --- a/crates/factor-sqlite/src/runtime_config/spin.rs +++ b/crates/factor-sqlite/src/runtime_config/spin.rs @@ -15,16 +15,16 @@ use tokio::sync::OnceCell; use crate::{Connection, ConnectionCreator, DefaultLabelResolver}; -/// Spin's default handling of the runtime configuration for SQLite databases. +/// Spin's default resolution of runtime configuration for SQLite databases. /// -/// This type implements the [`RuntimeConfigResolver`] trait and provides a way to -/// opt into the default behavior of Spin's SQLite database handling. -pub struct SpinSqliteRuntimeConfig { +/// This type implements how Spin CLI's SQLite implementation is configured +/// through the runtime config toml as well as the behavior of the "default" label. +pub struct RuntimeConfigResolver { default_database_dir: PathBuf, local_database_dir: PathBuf, } -impl SpinSqliteRuntimeConfig { +impl RuntimeConfigResolver { /// Create a new `SpinSqliteRuntimeConfig` /// /// This takes as arguments: @@ -59,7 +59,7 @@ impl SpinSqliteRuntimeConfig { /// type = "$database-type" /// ... extra type specific configuration ... /// ``` - pub fn config_from_table( + pub fn resolve_from_toml( &self, table: &T, ) -> anyhow::Result> { @@ -106,7 +106,7 @@ pub struct RuntimeConfig { pub config: toml::Table, } -impl DefaultLabelResolver for SpinSqliteRuntimeConfig { +impl DefaultLabelResolver for RuntimeConfigResolver { fn default(&self, label: &str) -> Option> { // Only default the database labeled "default". if label != "default" { diff --git a/crates/factor-sqlite/tests/factor_test.rs b/crates/factor-sqlite/tests/factor_test.rs index 8fb73c3c49..5b9972c9e7 100644 --- a/crates/factor-sqlite/tests/factor_test.rs +++ b/crates/factor-sqlite/tests/factor_test.rs @@ -1,6 +1,6 @@ use std::{collections::HashSet, sync::Arc}; -use factor_sqlite::{runtime_config::spin::SpinSqliteRuntimeConfig, SqliteFactor}; +use spin_factor_sqlite::{runtime_config::spin::RuntimeConfigResolver, SqliteFactor}; use spin_factors::{ anyhow::{self, bail, Context}, runtime_config::toml::TomlKeyTracker, @@ -66,7 +66,7 @@ async fn no_error_when_database_is_configured() -> anyhow::Result<()> { [sqlite_database.foo] type = "spin" }; - let sqlite_config = SpinSqliteRuntimeConfig::new("/".into(), "/".into()); + let sqlite_config = RuntimeConfigResolver::new("/".into(), "/".into()); let env = TestEnvironment::new(factors) .extend_manifest(toml! { [component.test-component] @@ -82,14 +82,14 @@ async fn no_error_when_database_is_configured() -> anyhow::Result<()> { struct TomlRuntimeSource<'a> { table: TomlKeyTracker<'a>, - sqlite_config: SpinSqliteRuntimeConfig, + runtime_config_resolver: RuntimeConfigResolver, } impl<'a> TomlRuntimeSource<'a> { - fn new(table: &'a toml::Table, sqlite_config: SpinSqliteRuntimeConfig) -> Self { + fn new(table: &'a toml::Table, runtime_config_resolver: RuntimeConfigResolver) -> Self { Self { table: TomlKeyTracker::new(table), - sqlite_config, + runtime_config_resolver, } } } @@ -98,7 +98,7 @@ impl FactorRuntimeConfigSource for TomlRuntimeSource<'_> { fn get_runtime_config( &mut self, ) -> anyhow::Result::RuntimeConfig>> { - self.sqlite_config.config_from_table(&self.table) + self.runtime_config_resolver.resolve_from_toml(&self.table) } } @@ -129,8 +129,8 @@ impl DefaultLabelResolver { } } -impl factor_sqlite::DefaultLabelResolver for DefaultLabelResolver { - fn default(&self, label: &str) -> Option> { +impl spin_factor_sqlite::DefaultLabelResolver for DefaultLabelResolver { + fn default(&self, label: &str) -> Option> { let Some(default) = &self.default else { return None; }; @@ -142,10 +142,11 @@ impl factor_sqlite::DefaultLabelResolver for DefaultLabelResolver { struct InvalidConnectionCreator; #[async_trait::async_trait] -impl factor_sqlite::ConnectionCreator for InvalidConnectionCreator { +impl spin_factor_sqlite::ConnectionCreator for InvalidConnectionCreator { async fn create_connection( &self, - ) -> Result, spin_world::v2::sqlite::Error> { + ) -> Result, spin_world::v2::sqlite::Error> + { Err(spin_world::v2::sqlite::Error::InvalidConnection) } } diff --git a/crates/runtime-config/Cargo.toml b/crates/runtime-config/Cargo.toml index a5b66b5f3c..7a52c51280 100644 --- a/crates/runtime-config/Cargo.toml +++ b/crates/runtime-config/Cargo.toml @@ -17,6 +17,7 @@ spin-factor-key-value-redis = { path = "../factor-key-value-redis" } spin-factor-key-value-azure = { path = "../factor-key-value-azure" } spin-factor-outbound-http = { path = "../factor-outbound-http" } spin-factor-outbound-networking = { path = "../factor-outbound-networking" } +spin-factor-sqlite = { path = "../factor-sqlite" } spin-factor-wasi = { path = "../factor-wasi" } toml = "0.8" diff --git a/crates/runtime-config/src/lib.rs b/crates/runtime-config/src/lib.rs index eeeb026006..0ab315d4bd 100644 --- a/crates/runtime-config/src/lib.rs +++ b/crates/runtime-config/src/lib.rs @@ -6,6 +6,8 @@ use spin_factor_key_value::{DefaultLabelResolver as _, KeyValueFactor}; use spin_factor_outbound_http::OutboundHttpFactor; use spin_factor_outbound_networking::runtime_config::spin::SpinTlsRuntimeConfig; use spin_factor_outbound_networking::OutboundNetworkingFactor; +use spin_factor_sqlite::runtime_config::spin as sqlite; +use spin_factor_sqlite::SqliteFactor; use spin_factor_wasi::WasiFactor; use spin_factors::{ runtime_config::toml::TomlKeyTracker, FactorRuntimeConfigSource, RuntimeConfigSourceFinalizer, @@ -17,12 +19,13 @@ pub const DEFAULT_STATE_DIR: &str = ".spin"; /// A runtime configuration which has been resolved from a runtime config source. /// /// Includes other pieces of configuration that are used to resolve the runtime configuration. -#[derive(Default)] pub struct ResolvedRuntimeConfig { /// The resolved runtime configuration. pub runtime_config: T, /// The resolver used to resolve key-value stores from runtime configuration. pub key_value_resolver: key_value::RuntimeConfigResolver, + /// The resolver used to resolve sqlite databases from runtime configuration. + pub sqlite_resolver: sqlite::RuntimeConfigResolver, } impl ResolvedRuntimeConfig @@ -32,9 +35,12 @@ where { /// Creates a new resolved runtime configuration from a runtime config source TOML file. pub fn from_file(runtime_config_path: &Path, state_dir: Option<&str>) -> anyhow::Result { - let key_value_resolver = - key_value_resolver(PathBuf::from(state_dir.unwrap_or(DEFAULT_STATE_DIR))); let tls_resolver = SpinTlsRuntimeConfig::new(runtime_config_path); + let key_value_config_resolver = + key_value_config_resolver(PathBuf::from(state_dir.unwrap_or(DEFAULT_STATE_DIR))); + + let sqlite_config_resolver = + sqlite_config_resolver(state_dir).context("failed to resolve sqlite runtime config")?; let file = std::fs::read_to_string(runtime_config_path).with_context(|| { format!( @@ -48,14 +54,19 @@ where runtime_config_path.display() ) })?; - let runtime_config: T = - TomlRuntimeConfigSource::new(&toml, &key_value_resolver, &tls_resolver) - .try_into() - .map_err(Into::into)?; + let runtime_config: T = TomlRuntimeConfigSource::new( + &toml, + &key_value_config_resolver, + &tls_resolver, + &sqlite_config_resolver, + ) + .try_into() + .map_err(Into::into)?; Ok(Self { runtime_config, - key_value_resolver, + key_value_resolver: key_value_config_resolver, + sqlite_resolver: sqlite_config_resolver, }) } @@ -81,11 +92,23 @@ where } } +impl ResolvedRuntimeConfig { + pub fn default(state_dir: Option<&str>) -> Self { + Self { + sqlite_resolver: sqlite_config_resolver(state_dir) + .expect("failed to resolve sqlite runtime config"), + key_value_resolver: Default::default(), + runtime_config: Default::default(), + } + } +} + /// The TOML based runtime configuration source Spin CLI. pub struct TomlRuntimeConfigSource<'a> { table: TomlKeyTracker<'a>, key_value: &'a key_value::RuntimeConfigResolver, tls: &'a SpinTlsRuntimeConfig, + sqlite: &'a sqlite::RuntimeConfigResolver, } impl<'a> TomlRuntimeConfigSource<'a> { @@ -93,11 +116,13 @@ impl<'a> TomlRuntimeConfigSource<'a> { table: &'a toml::Table, key_value: &'a key_value::RuntimeConfigResolver, tls: &'a SpinTlsRuntimeConfig, + sqlite: &'a sqlite::RuntimeConfigResolver, ) -> Self { Self { table: TomlKeyTracker::new(table), key_value, tls, + sqlite, } } } @@ -131,6 +156,12 @@ impl FactorRuntimeConfigSource for TomlRuntimeConfigSource<' } } +impl FactorRuntimeConfigSource for TomlRuntimeConfigSource<'_> { + fn get_runtime_config(&mut self) -> anyhow::Result> { + self.sqlite.resolve_from_toml(self.table.as_ref()) + } +} + impl RuntimeConfigSourceFinalizer for TomlRuntimeConfigSource<'_> { fn finalize(&mut self) -> anyhow::Result<()> { Ok(self.table.validate_all_keys_used()?) @@ -140,10 +171,12 @@ impl RuntimeConfigSourceFinalizer for TomlRuntimeConfigSource<'_> { const DEFAULT_KEY_VALUE_STORE_FILENAME: &str = "sqlite_key_value.db"; const DEFAULT_KEY_VALUE_STORE_LABEL: &str = "default"; -/// The key-value runtime configuration resolver used by the trigger. +/// The key-value runtime configuration resolver. /// /// Takes a base path for the local store. -pub fn key_value_resolver(local_store_base_path: PathBuf) -> key_value::RuntimeConfigResolver { +pub fn key_value_config_resolver( + local_store_base_path: PathBuf, +) -> key_value::RuntimeConfigResolver { let mut key_value = key_value::RuntimeConfigResolver::new(); // Register the supported store types. @@ -173,3 +206,20 @@ pub fn key_value_resolver(local_store_base_path: PathBuf) -> key_value::RuntimeC key_value } + +/// The sqlite runtime configuration resolver. +/// +/// Takes a base path to the state directory. +fn sqlite_config_resolver( + state_dir: Option<&str>, +) -> anyhow::Result { + let default_database_dir = PathBuf::from(state_dir.unwrap_or(DEFAULT_STATE_DIR)); + let default_database_dir = std::path::absolute(default_database_dir) + .context("failed to make default database directory absolute")?; + let local_database_dir = + std::env::current_dir().context("failed to get current working directory")?; + Ok(sqlite::RuntimeConfigResolver::new( + default_database_dir, + local_database_dir, + )) +} diff --git a/crates/trigger2/Cargo.toml b/crates/trigger2/Cargo.toml index 1342cc02ad..656f510354 100644 --- a/crates/trigger2/Cargo.toml +++ b/crates/trigger2/Cargo.toml @@ -24,6 +24,7 @@ spin-factor-outbound-http = { path = "../factor-outbound-http" } spin-factor-outbound-networking = { path = "../factor-outbound-networking" } spin-factor-wasi = { path = "../factor-wasi" } spin-factor-key-value = { path = "../factor-key-value" } +spin-factor-sqlite = { path = "../factor-sqlite" } spin-factors = { path = "../factors" } spin-factors-executor = { path = "../factors-executor" } spin-telemetry = { path = "../telemetry" } diff --git a/crates/trigger2/src/cli.rs b/crates/trigger2/src/cli.rs index 0c72480b31..321f1d56dd 100644 --- a/crates/trigger2/src/cli.rs +++ b/crates/trigger2/src/cli.rs @@ -199,7 +199,7 @@ impl FactorsTriggerCommand { self.state_dir.as_deref(), )? } - None => ResolvedRuntimeConfig::default(), + None => ResolvedRuntimeConfig::default(self.state_dir.as_deref()), }; runtime_config @@ -210,6 +210,7 @@ impl FactorsTriggerCommand { working_dir, self.allow_transient_write, runtime_config.key_value_resolver, + runtime_config.sqlite_resolver, ); // TODO: move these into Factor methods/constructors diff --git a/crates/trigger2/src/factors.rs b/crates/trigger2/src/factors.rs index aa1a0f3155..9734718948 100644 --- a/crates/trigger2/src/factors.rs +++ b/crates/trigger2/src/factors.rs @@ -1,8 +1,9 @@ use std::path::PathBuf; -use spin_factor_key_value::{DefaultLabelResolver, KeyValueFactor}; +use spin_factor_key_value::KeyValueFactor; use spin_factor_outbound_http::OutboundHttpFactor; use spin_factor_outbound_networking::OutboundNetworkingFactor; +use spin_factor_sqlite::SqliteFactor; use spin_factor_wasi::{spin::SpinFilesMounter, WasiFactor}; use spin_factors::RuntimeFactors; use spin_runtime_config::TomlRuntimeConfigSource; @@ -13,13 +14,15 @@ pub struct TriggerFactors { pub key_value: KeyValueFactor, pub outbound_networking: OutboundNetworkingFactor, pub outbound_http: OutboundHttpFactor, + pub sqlite: SqliteFactor, } impl TriggerFactors { pub fn new( working_dir: impl Into, allow_transient_writes: bool, - default_key_value_label_resolver: impl DefaultLabelResolver + 'static, + default_key_value_label_resolver: impl spin_factor_key_value::DefaultLabelResolver + 'static, + default_sqlite_label_resolver: impl spin_factor_sqlite::DefaultLabelResolver + 'static, ) -> Self { let files_mounter = SpinFilesMounter::new(working_dir, allow_transient_writes); Self { @@ -27,6 +30,7 @@ impl TriggerFactors { key_value: KeyValueFactor::new(default_key_value_label_resolver), outbound_networking: OutboundNetworkingFactor, outbound_http: OutboundHttpFactor, + sqlite: SqliteFactor::new(default_sqlite_label_resolver), } } } From 3cafa2a82621c9967deac6861231675c06f6b5a3 Mon Sep 17 00:00:00 2001 From: Ryan Levick Date: Mon, 19 Aug 2024 13:39:08 +0200 Subject: [PATCH 2/2] Don't require absolute paths in sqlite runtime config resolver Signed-off-by: Ryan Levick --- crates/factor-sqlite/src/runtime_config/spin.rs | 13 +------------ crates/runtime-config/src/lib.rs | 2 -- 2 files changed, 1 insertion(+), 14 deletions(-) diff --git a/crates/factor-sqlite/src/runtime_config/spin.rs b/crates/factor-sqlite/src/runtime_config/spin.rs index 3bc2452f0b..a98387df13 100644 --- a/crates/factor-sqlite/src/runtime_config/spin.rs +++ b/crates/factor-sqlite/src/runtime_config/spin.rs @@ -30,21 +30,10 @@ impl RuntimeConfigResolver { /// This takes as arguments: /// * the directory to use as the default location for SQLite databases. /// Usually this will be the path to the `.spin` state directory. - /// * the *absolute* path to the directory from which relative paths to + /// * the path to the directory from which relative paths to /// local SQLite databases are resolved. (this should most likely be the /// path to the runtime-config file or the current working dir). - /// - /// Panics if either `default_database_dir` or `local_database_dir` are not - /// absolute paths. pub fn new(default_database_dir: PathBuf, local_database_dir: PathBuf) -> Self { - assert!( - default_database_dir.is_absolute(), - "default_database_dir must be an absolute path" - ); - assert!( - local_database_dir.is_absolute(), - "local_database_dir must be an absolute path" - ); Self { default_database_dir, local_database_dir, diff --git a/crates/runtime-config/src/lib.rs b/crates/runtime-config/src/lib.rs index 0ab315d4bd..12f40c7505 100644 --- a/crates/runtime-config/src/lib.rs +++ b/crates/runtime-config/src/lib.rs @@ -214,8 +214,6 @@ fn sqlite_config_resolver( state_dir: Option<&str>, ) -> anyhow::Result { let default_database_dir = PathBuf::from(state_dir.unwrap_or(DEFAULT_STATE_DIR)); - let default_database_dir = std::path::absolute(default_database_dir) - .context("failed to make default database directory absolute")?; let local_database_dir = std::env::current_dir().context("failed to get current working directory")?; Ok(sqlite::RuntimeConfigResolver::new(