Check required variables are potentially resolvable

Adds new dynamism metadata for Providers. Allow for no default value before runtime if Provider is a Dynamic type. PR also includes unit tests

Signed-off-by: Aminu Oluwaseun Joshua <[email protected]>

Author: seun-ja

Cargo.lock

@@ -1,7 +1,7 @@
 pub mod provider;
 mod template;
 
-use std::{borrow::Cow, collections::HashMap, fmt::Debug};
+use std::{borrow::Cow, collections::HashMap, fmt::Debug, vec};
 
 use spin_locked_app::Variable;
 
@@ -11,6 +11,8 @@ pub use provider::Provider;
 use template::Part;
 pub use template::Template;
 
+use crate::provider::ProviderVariableKind;
+
 /// A [`ProviderResolver`] that can be shared.
 pub type SharedPreparedResolver =
     std::sync::Arc<std::sync::OnceLock<std::sync::Arc<PreparedResolver>>>;
@@ -90,6 +92,54 @@ impl ProviderResolver {
         Ok(PreparedResolver { variables })
     }
 
+    /// Ensures that all required variables are resolvable at startup
+    pub async fn ensure_required_variables_resolvable(&self) -> Result<()> {
+        // If a dynamic provider is available, skip validation.
+        if self
+            .providers
+            .iter()
+            .any(|p| p.kind() == ProviderVariableKind::Dynamic)
+        {
+            return Ok(());
+        }
+
+        let mut unvalidated_keys = vec![];
+        for key in self.internal.variables.keys() {
+            // If default value is provided, skip validation.
+            if let Some(value) = self.internal.variables.get(key) {
+                if value.default.is_some() {
+                    continue;
+                }
+            }
+
+            let mut resolved = false;
+
+            for provider in &self.providers {
+                if provider
+                    .get(&Key(key))
+                    .await
+                    .map_err(Error::Provider)?
+                    .is_some()
+                {
+                    resolved = true;
+                    break;
+                }
+            }
+
+            if !resolved {
+                unvalidated_keys.push(key);
+            }
+        }
+
+        if unvalidated_keys.is_empty() {
+            Ok(())
+        } else {
+            Err(Error::Provider(anyhow::anyhow!(
+                "no provider resolved required variable(s): {unvalidated_keys:?}",
+            )))
+        }
+    }
+
     async fn resolve_variable(&self, key: &str) -> Result<String> {
         for provider in &self.providers {
             if let Some(value) = provider.get(&Key(key)).await.map_err(Error::Provider)? {
@@ -310,6 +360,7 @@ mod tests {
     use async_trait::async_trait;
 
     use super::*;
+    use crate::provider::ProviderVariableKind;
 
     #[derive(Debug)]
     struct TestProvider;
@@ -323,6 +374,10 @@ mod tests {
                 _ => Ok(None),
             }
         }
+
+        fn kind(&self) -> ProviderVariableKind {
+            ProviderVariableKind::Static
+        }
     }
 
     async fn test_resolve(template: &str) -> Result<String> {

crates/expressions/src/lib.rs

@@ -9,4 +9,15 @@ use crate::Key;
 pub trait Provider: Debug + Send + Sync {
     /// Returns the value at the given config path, if it exists.
     async fn get(&self, key: &Key) -> anyhow::Result<Option<String>>;
+    fn kind(&self) -> ProviderVariableKind;
+}
+
+/// The dynamism of a Provider.
+#[derive(Clone, Debug, Default, PartialEq)]
+pub enum ProviderVariableKind {
+    /// Variable must be declared on start
+    Static,
+    /// Variable can be made available at runtime
+    #[default]
+    Dynamic,
 }

crates/expressions/src/provider.rs

@@ -0,0 +1,267 @@
+use std::collections::HashMap;
+
+use async_trait::async_trait;
+use spin_expressions::{provider::ProviderVariableKind, Key, Provider, ProviderResolver};
+use spin_locked_app::Variable;
+
+#[derive(Default)]
+struct ResolverTester {
+    providers: Vec<Box<dyn Provider>>,
+    variables: HashMap<String, Variable>,
+}
+
+impl ResolverTester {
+    fn new() -> Self {
+        Self::default()
+    }
+
+    fn with_provider(mut self, provider: impl Provider + 'static) -> Self {
+        self.providers.push(Box::new(provider));
+        self
+    }
+
+    fn with_variable(mut self, key: &str, default: Option<&str>) -> Self {
+        self.variables.insert(
+            key.to_string(),
+            Variable {
+                description: None,
+                default: default.map(ToString::to_string),
+                secret: false,
+            },
+        );
+        self
+    }
+
+    fn make_resolver(self) -> anyhow::Result<ProviderResolver> {
+        let mut provider_resolver = ProviderResolver::new(self.variables)?;
+
+        for provider in self.providers {
+            provider_resolver.add_provider(provider);
+        }
+
+        Ok(provider_resolver)
+    }
+}
+
+#[tokio::test(flavor = "multi_thread")]
+async fn if_single_static_provider_with_no_key_to_resolve_is_valid() -> anyhow::Result<()> {
+    let resolver = ResolverTester::new()
+        .with_provider(StaticProvider::with_variable(
+            "database_host",
+            Some("localhost"),
+        ))
+        .make_resolver()?;
+
+    resolver.ensure_required_variables_resolvable().await?;
+
+    Ok(())
+}
+
+#[tokio::test(flavor = "multi_thread")]
+async fn if_single_static_provider_has_data_for_variable_key_to_resolve_it_succeeds(
+) -> anyhow::Result<()> {
+    let resolver = ResolverTester::new()
+        .with_provider(StaticProvider::with_variable(
+            "database_host",
+            Some("localhost"),
+        ))
+        .with_variable("database_host", None)
+        .make_resolver()?;
+
+    resolver.ensure_required_variables_resolvable().await?;
+
+    Ok(())
+}
+
+#[tokio::test(flavor = "multi_thread")]
+async fn if_there_is_a_single_static_provider_and_it_does_not_contain_a_required_variable_then_validation_fails(
+) -> anyhow::Result<()> {
+    let resolver = ResolverTester::new()
+        .with_provider(StaticProvider::with_variable(
+            "database_host",
+            Some("localhost"),
+        ))
+        .with_variable("api_key", None)
+        .make_resolver()?;
+
+    assert!(resolver
+        .ensure_required_variables_resolvable()
+        .await
+        .is_err());
+
+    Ok(())
+}
+
+#[tokio::test(flavor = "multi_thread")]
+async fn if_there_is_a_dynamic_provider_then_validation_succeeds_even_without_default_value_in_play(
+) -> anyhow::Result<()> {
+    let resolver = ResolverTester::new()
+        .with_provider(DynamicProvider)
+        .with_variable("api_key", None)
+        .make_resolver()?;
+
+    resolver.ensure_required_variables_resolvable().await?;
+
+    Ok(())
+}
+
+#[tokio::test(flavor = "multi_thread")]
+async fn if_there_is_a_dynamic_provider_and_static_provider_but_the_variable_to_be_resolved_is_not_in_play(
+) -> anyhow::Result<()> {
+    let resolver = ResolverTester::new()
+        .with_provider(DynamicProvider)
+        .with_provider(StaticProvider::with_variable(
+            "database_host",
+            Some("localhost"),
+        ))
+        .with_variable("api_key", None)
+        .make_resolver()?;
+
+    resolver.ensure_required_variables_resolvable().await?;
+
+    Ok(())
+}
+
+#[tokio::test(flavor = "multi_thread")]
+async fn if_there_is_a_dynamic_provider_and_a_static_provider_then_validation_succeeds_even_if_a_static_provider_with_the_variable_is_in_play(
+) -> anyhow::Result<()> {
+    let resolver = ResolverTester::new()
+        .with_provider(DynamicProvider)
+        .with_provider(StaticProvider::with_variable(
+            "database_host",
+            Some("localhost"),
+        ))
+        .with_variable("api_key", Some("super-secret-key"))
+        .make_resolver()?;
+
+    resolver.ensure_required_variables_resolvable().await?;
+
+    Ok(())
+}
+
+#[tokio::test(flavor = "multi_thread")]
+async fn if_there_are_two_static_providers_where_one_has_data_is_valid() -> anyhow::Result<()> {
+    let resolver = ResolverTester::new()
+        .with_provider(StaticProvider::with_variable(
+            "database_host",
+            Some("localhost"),
+        ))
+        .with_provider(StaticProvider::with_variable(
+            "api_key",
+            Some("super-secret-key"),
+        ))
+        .with_variable("database_host", None)
+        .make_resolver()?;
+
+    resolver.ensure_required_variables_resolvable().await?;
+
+    Ok(())
+}
+// Ensure that if there are two or more static providers and the first one does not have data for the variable to be resolved,
+// but the second or subsequent one does, then validation still succeeds.
+#[tokio::test(flavor = "multi_thread")]
+async fn if_there_are_two_static_providers_where_first_provider_does_not_have_data_while_second_provider_does(
+) -> anyhow::Result<()> {
+    let resolver = ResolverTester::new()
+        .with_provider(StaticProvider::with_variable(
+            "database_host",
+            Some("localhost"),
+        ))
+        .with_provider(StaticProvider::with_variable(
+            "api_key",
+            Some("super-secret-key"),
+        ))
+        .with_variable("api_key", None)
+        .make_resolver()?;
+
+    resolver.ensure_required_variables_resolvable().await?;
+
+    Ok(())
+}
+
+#[tokio::test(flavor = "multi_thread")]
+async fn if_there_is_two_static_providers_neither_having_data_is_invalid() -> anyhow::Result<()> {
+    let resolver = ResolverTester::new()
+        .with_provider(StaticProvider::with_variable(
+            "database_host",
+            Some("localhost"),
+        ))
+        .with_provider(StaticProvider::with_variable(
+            "api_key",
+            Some("super-secret-key"),
+        ))
+        .with_variable("hello", None)
+        .make_resolver()?;
+
+    assert!(resolver
+        .ensure_required_variables_resolvable()
+        .await
+        .is_err());
+
+    Ok(())
+}
+
+#[tokio::test(flavor = "multi_thread")]
+async fn no_provider_data_available_but_variable_default_value_needed_is_invalid(
+) -> anyhow::Result<()> {
+    let resolver = ResolverTester::new()
+        .with_variable("api_key", None)
+        .make_resolver()?;
+
+    assert!(resolver
+        .ensure_required_variables_resolvable()
+        .await
+        .is_err());
+
+    Ok(())
+}
+
+#[tokio::test(flavor = "multi_thread")]
+async fn no_provider_data_available_but_variable_has_default_value_needed_is_valid(
+) -> anyhow::Result<()> {
+    let resolver = ResolverTester::new()
+        .with_variable("api_key", Some("super-secret-key"))
+        .make_resolver()?;
+
+    resolver.ensure_required_variables_resolvable().await?;
+
+    Ok(())
+}
+
+#[derive(Debug)]
+struct StaticProvider {
+    variables: HashMap<String, Option<String>>,
+}
+
+impl StaticProvider {
+    fn with_variable(key: &str, value: Option<&str>) -> Self {
+        Self {
+            variables: HashMap::from([(key.into(), value.map(|v| v.into()))]),
+        }
+    }
+}
+
+#[async_trait]
+impl Provider for StaticProvider {
+    async fn get(&self, key: &Key) -> anyhow::Result<Option<String>> {
+        Ok(self.variables.get(key.as_str()).cloned().flatten())
+    }
+
+    fn kind(&self) -> ProviderVariableKind {
+        ProviderVariableKind::Static
+    }
+}
+
+#[derive(Debug)]
+struct DynamicProvider;
+
+#[async_trait]
+impl Provider for DynamicProvider {
+    async fn get(&self, _key: &Key) -> anyhow::Result<Option<String>> {
+        panic!("validation should never call get for a dynamic provider")
+    }
+
+    fn kind(&self) -> ProviderVariableKind {
+        ProviderVariableKind::Dynamic
+    }
+}

crates/expressions/tests/validation_test.rs

@@ -85,6 +85,10 @@ impl AppState {
         let template = Template::new(expr)?;
         self.expression_resolver.resolve_template(&template).await
     }
+
+    pub fn expression_resolver(&self) -> &Arc<ExpressionResolver> {
+        &self.expression_resolver
+    }
 }
 
 pub struct InstanceState {