From b98c996201b7fc198cc746b7d78a97d8afcb64d2 Mon Sep 17 00:00:00 2001 From: Kate Goldenring Date: Tue, 3 Dec 2024 15:21:10 -0800 Subject: [PATCH 1/2] fix: enable configuring log_dir from runtime config Signed-off-by: Kate Goldenring --- crates/runtime-config/src/lib.rs | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/crates/runtime-config/src/lib.rs b/crates/runtime-config/src/lib.rs index fd0687361a..6d2f91236d 100644 --- a/crates/runtime-config/src/lib.rs +++ b/crates/runtime-config/src/lib.rs @@ -129,27 +129,29 @@ where let runtime_config_dir = runtime_config_path .and_then(Path::parent) .map(ToOwned::to_owned); + let state_dir = toml_resolver.state_dir()?; let tls_resolver = runtime_config_dir.clone().map(SpinTlsRuntimeConfig::new); - let key_value_config_resolver = - key_value_config_resolver(runtime_config_dir, toml_resolver.state_dir()?); - let sqlite_config_resolver = sqlite_config_resolver(toml_resolver.state_dir()?) + let key_value_resolver = key_value_config_resolver(runtime_config_dir, state_dir.clone()); + let sqlite_resolver = sqlite_config_resolver(state_dir.clone()) .context("failed to resolve sqlite runtime config")?; + let toml = toml_resolver.toml(); + let log_dir = toml_resolver.log_dir()?; let source = TomlRuntimeConfigSource::new( - toml_resolver.clone(), - &key_value_config_resolver, + toml_resolver, + &key_value_resolver, tls_resolver.as_ref(), - &sqlite_config_resolver, + &sqlite_resolver, ); let runtime_config: T = source.try_into().map_err(Into::into)?; Ok(Self { runtime_config, - key_value_resolver: key_value_config_resolver, - sqlite_resolver: sqlite_config_resolver, - state_dir: toml_resolver.state_dir()?, - log_dir: toml_resolver.log_dir()?, - toml: toml_resolver.toml(), + key_value_resolver, + sqlite_resolver, + state_dir, + log_dir, + toml, }) } From af6ad600c781adb5603de30597542a41ae27fc4e Mon Sep 17 00:00:00 2001 From: Kate Goldenring Date: Wed, 4 Dec 2024 12:12:31 -0800 Subject: [PATCH 2/2] Add tests to runtime config to confirm fields are resolved Signed-off-by: Kate Goldenring --- crates/runtime-config/src/lib.rs | 41 ++++++++++++++++++++++++++++---- 1 file changed, 36 insertions(+), 5 deletions(-) diff --git a/crates/runtime-config/src/lib.rs b/crates/runtime-config/src/lib.rs index 6d2f91236d..68d4d11370 100644 --- a/crates/runtime-config/src/lib.rs +++ b/crates/runtime-config/src/lib.rs @@ -143,6 +143,9 @@ where tls_resolver.as_ref(), &sqlite_resolver, ); + // Note: all valid fields in the runtime config must have been referenced at + // this point or the finalizer will fail due to `validate_all_keys_used` + // not passing. let runtime_config: T = source.try_into().map_err(Into::into)?; Ok(Self { @@ -460,12 +463,11 @@ mod tests { fn resolve_toml( toml: toml::Table, path: impl AsRef, - ) -> ResolvedRuntimeConfig { + ) -> anyhow::Result> { ResolvedRuntimeConfig::::new( toml_resolver(&toml), Some(path.as_ref()), ) - .unwrap() } }; } @@ -501,13 +503,16 @@ mod tests { type = "spin" }; assert_eq!( - resolve_toml(toml, ".").runtime_config.configured_labels(), + resolve_toml(toml, ".") + .unwrap() + .runtime_config + .configured_labels(), vec!["default", "foo"] ); // Test that the default label is added with an empty toml config. let toml = toml::Table::new(); - let runtime_config = resolve_toml(toml, "config.toml").runtime_config; + let runtime_config = resolve_toml(toml, "config.toml").unwrap().runtime_config; assert_eq!(runtime_config.configured_labels(), vec!["default"]); } @@ -526,7 +531,7 @@ mod tests { [key_value_store.foo] type = "spin" }; - let runtime_config = resolve_toml(toml, "config.toml").runtime_config; + let runtime_config = resolve_toml(toml, "config.toml").unwrap().runtime_config; assert!(["default", "foo"] .iter() .all(|label| runtime_config.has_store_manager(label))); @@ -564,6 +569,7 @@ mod tests { }) .runtime_config( resolve_toml(runtime_config, tmp_dir.path().join("runtime-config.toml")) + .unwrap() .runtime_config, )?; let mut state = env.build_instance_state().await?; @@ -589,4 +595,29 @@ mod tests { UserProvidedPath::Default, ) } + + #[test] + fn dirs_are_resolved() { + define_test_factor!(sqlite: SqliteFactor); + + let toml = toml::toml! { + state_dir = "/foo" + log_dir = "/bar" + }; + resolve_toml(toml, "config.toml").unwrap(); + } + + #[test] + fn fails_to_resolve_with_unused_key() { + define_test_factor!(sqlite: SqliteFactor); + + let toml = toml::toml! { + baz = "/baz" + }; + // assert returns an error with value "unused runtime config key(s): local_app_dir" + let Err(e) = resolve_toml(toml, "config.toml") else { + panic!("Should not be able to resolve unknown key"); + }; + assert_eq!(e.to_string(), "unused runtime config key(s): baz"); + } }