variables: Refactor Provider::kind into ::may_resolve

Signed-off-by: Lann Martin <[email protected]>

Author: lann

crates/expressions/src/lib.rs

@@ -11,8 +11,6 @@ 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>>>;
@@ -92,50 +90,25 @@ impl ProviderResolver {
         Ok(PreparedResolver { variables })
     }
 
-    /// Ensures that all required variables are resolvable at startup
+    /// Ensures that all required variables are not unresolvable
     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);
+        let mut unresolvable_keys = vec![];
+        for key in self.internal.required_variables() {
+            let key = Key::new(key)?;
+            let resolvable = self
+                .providers
+                .iter()
+                .any(|provider| provider.may_resolve(&key));
+            if !resolvable {
+                unresolvable_keys.push(key);
             }
         }
 
-        if unvalidated_keys.is_empty() {
+        if unresolvable_keys.is_empty() {
             Ok(())
         } else {
             Err(Error::Provider(anyhow::anyhow!(
-                "no provider resolved required variable(s): {unvalidated_keys:?}",
+                "no provider resolved required variable(s): {unresolvable_keys:?}",
             )))
         }
     }
@@ -248,6 +221,12 @@ impl Resolver {
         })?;
         Ok(template)
     }
+
+    fn required_variables(&self) -> impl Iterator<Item = &str> {
+        self.variables
+            .iter()
+            .filter_map(|(name, variable)| variable.default.is_none().then_some(name.as_str()))
+    }
 }
 
 /// A resolver who has resolved all variables.
@@ -360,7 +339,6 @@ mod tests {
     use async_trait::async_trait;
 
     use super::*;
-    use crate::provider::ProviderVariableKind;
 
     #[derive(Debug)]
     struct TestProvider;
@@ -375,8 +353,8 @@ mod tests {
             }
         }
 
-        fn kind(&self) -> ProviderVariableKind {
-            ProviderVariableKind::Static
+        fn may_resolve(&self, key: &Key) -> bool {
+            key.as_ref() == "required"
         }
     }
 

crates/expressions/src/provider.rs

@@ -9,15 +9,13 @@ 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,
+    /// Returns true if the given key _might_ be resolvable by this Provider.
+    ///
+    /// Dynamic resolvers will typically return true unconditionally, which is
+    /// the default implementation.
+    fn may_resolve(&self, key: &Key) -> bool {
+        let _ = key;
+        true
+    }
 }

crates/expressions/tests/validation_test.rs

@@ -1,7 +1,7 @@
 use std::collections::HashMap;
 
 use async_trait::async_trait;
-use spin_expressions::{provider::ProviderVariableKind, Key, Provider, ProviderResolver};
+use spin_expressions::{Key, Provider, ProviderResolver};
 use spin_locked_app::Variable;
 
 #[derive(Default)]
@@ -247,8 +247,8 @@ impl Provider for StaticProvider {
         Ok(self.variables.get(key.as_str()).cloned().flatten())
     }
 
-    fn kind(&self) -> ProviderVariableKind {
-        ProviderVariableKind::Static
+    fn may_resolve(&self, key: &Key) -> bool {
+        self.variables.contains_key(key.as_str())
     }
 }
 
@@ -260,8 +260,4 @@ 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/factor-variables/tests/factor_test.rs

@@ -1,4 +1,4 @@
-use spin_expressions::{provider::ProviderVariableKind, Key, Provider};
+use spin_expressions::{Key, Provider};
 use spin_factor_variables::{runtime_config::RuntimeConfig, VariablesFactor};
 use spin_factors::{anyhow, RuntimeFactors};
 use spin_factors_test::{toml, TestEnvironment};
@@ -47,7 +47,7 @@ impl Provider for MockProvider {
         }
     }
 
-    fn kind(&self) -> ProviderVariableKind {
-        ProviderVariableKind::Static
+    fn may_resolve(&self, key: &Key) -> bool {
+        key.as_ref() == "foo"
     }
 }

crates/variables-azure/src/lib.rs

@@ -4,7 +4,7 @@ use anyhow::Context as _;
 use azure_core::{auth::TokenCredential, Url};
 use azure_security_keyvault::SecretClient;
 use serde::Deserialize;
-use spin_expressions::{provider::ProviderVariableKind, Key, Provider};
+use spin_expressions::{Key, Provider};
 use spin_factors::anyhow;
 use spin_world::async_trait;
 use tracing::{instrument, Level};
@@ -151,10 +151,6 @@ impl Provider for AzureKeyVaultProvider {
             .context("Failed to read variable from Azure Key Vault")?;
         Ok(Some(secret.value))
     }
-
-    fn kind(&self) -> ProviderVariableKind {
-        ProviderVariableKind::Dynamic
-    }
 }
 
 impl From<AzureAuthorityHost> for Url {

crates/variables-env/src/lib.rs

@@ -6,7 +6,7 @@ use std::{
 };
 
 use serde::Deserialize;
-use spin_expressions::{provider::ProviderVariableKind, Key, Provider};
+use spin_expressions::{Key, Provider};
 use spin_factors::anyhow::{self, Context as _};
 use spin_world::async_trait;
 use tracing::{instrument, Level};
@@ -133,8 +133,8 @@ impl Provider for EnvVariablesProvider {
         tokio::task::block_in_place(|| self.get_sync(key))
     }
 
-    fn kind(&self) -> ProviderVariableKind {
-        ProviderVariableKind::Static
+    fn may_resolve(&self, key: &Key) -> bool {
+        matches!(self.get_sync(key), Ok(Some(_)))
     }
 }
 

crates/variables-static/src/lib.rs

@@ -1,7 +1,7 @@
 use std::{collections::HashMap, hash::Hash, sync::Arc};
 
 use serde::Deserialize;
-use spin_expressions::{async_trait::async_trait, provider::ProviderVariableKind, Key, Provider};
+use spin_expressions::{async_trait::async_trait, Key, Provider};
 use spin_factors::anyhow;
 
 pub use source::*;
@@ -19,8 +19,8 @@ impl Provider for StaticVariablesProvider {
         Ok(self.values.get(key.as_str()).cloned())
     }
 
-    fn kind(&self) -> ProviderVariableKind {
-        ProviderVariableKind::Static
+    fn may_resolve(&self, key: &Key) -> bool {
+        self.values.contains_key(key.as_str())
     }
 }
 

crates/variables-vault/src/lib.rs

@@ -1,5 +1,5 @@
 use serde::{Deserialize, Serialize};
-use spin_expressions::{async_trait::async_trait, provider::ProviderVariableKind};
+use spin_expressions::async_trait::async_trait;
 use spin_factors::anyhow::{self, Context as _};
 use tracing::{instrument, Level};
 use vaultrs::{
@@ -52,8 +52,4 @@ impl Provider for VaultVariablesProvider {
             Err(e) => Err(e).context("Failed to check Vault for config"),
         }
     }
-
-    fn kind(&self) -> ProviderVariableKind {
-        ProviderVariableKind::Dynamic
-    }
 }