Move validation code to client, serialize policy config data

Signed-off-by: Dan Norris <protochron@users.noreply.github.com>

.github/workflows/test.yml

@@ -48,4 +48,4 @@ jobs:
       # Run all tests
       - name: Run tests
         run: |
-          cargo test -- --nocapture
+          cargo test --workspace -- --nocapture

crates/wadm-types/src/lib.rs

@@ -34,7 +34,10 @@ pub const LINK_TRAIT: &str = "link";
 /// for a manifest
 pub const LATEST_VERSION: &str = "latest";
 
-pub const SECRET_TYPE: &str = "v1.secret.wasmcloud.dev";
+/// The type and version of the secret reference stored as configuration. This is meant as an
+/// internal marker of the format of the serialized secret and should not be referenced by
+/// anything else but wadm. It is intended to help with schema upgrades in the future.
+pub const SECRET_TYPE: &str = "secret-reference.wasmcloud.dev/v1alpha1";
 
 /// An OAM manifest
 #[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq, utoipa::ToSchema, JsonSchema)]
@@ -171,6 +174,38 @@ pub struct Component {
     pub traits: Option<Vec<Trait>>,
 }
 
+impl Component {
+    fn secrets(&self) -> Vec<SecretProperty> {
+        let mut secrets = Vec::new();
+        if let Some(traits) = self.traits.as_ref() {
+            let l: Vec<SecretProperty> = traits
+                .iter()
+                .filter_map(|t| {
+                    if let TraitProperty::Link(link) = &t.properties {
+                        let mut tgt_iter = link.target.secrets.clone();
+                        if let Some(src) = &link.source {
+                            tgt_iter.extend(src.secrets.clone());
+                        }
+                        Some(tgt_iter)
+                    } else {
+                        None
+                    }
+                })
+                .flatten()
+                .collect();
+            secrets.extend(l);
+        };
+
+        match &self.properties {
+            Properties::Component { properties } => {
+                secrets.extend(properties.secrets.clone());
+            }
+            Properties::Capability { properties } => secrets.extend(properties.secrets.clone()),
+        };
+        secrets
+    }
+}
+
 /// Properties that can be defined for a component
 #[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq, JsonSchema)]
 #[serde(tag = "type")]
@@ -885,7 +920,7 @@ mod test {
     }
 
     #[test]
-    fn t_deprecated_fields_not_set() {
+    fn test_deprecated_fields_not_set() {
         let manifest = deserialize_yaml("./oam/simple2.yaml").expect("Should be able to parse");
         // Validate component traits
         let traits = manifest

crates/wadm-types/src/validation.rs

@@ -1,15 +1,18 @@
 //! Logic for model ([`Manifest`]) validation
 //!
 
-use std::collections::HashMap;
+use std::collections::{HashMap, HashSet};
 use std::path::Path;
 use std::sync::OnceLock;
 
 use anyhow::{Context as _, Result};
 use regex::Regex;
 use serde::{Deserialize, Serialize};
 
-use crate::{LinkProperty, Manifest, TraitProperty, LATEST_VERSION};
+use crate::{
+    CapabilityProperties, ComponentProperties, LinkProperty, Manifest, Properties, Trait,
+    TraitProperty, LATEST_VERSION,
+};
 
 /// A namespace -> package -> interface lookup
 type KnownInterfaceLookup = HashMap<String, HashMap<String, HashMap<String, ()>>>;
@@ -298,6 +301,7 @@ pub async fn validate_manifest_bytes(
 /// - unsupported interfaces (i.e. typos, etc)
 /// - unknown packages under known namespaces
 /// - "dangling" links (missing components)
+/// - secrets mapped to unknown policies
 ///
 /// Since `[ValidationFailure]` implements `ValidationOutput`, you can call `valid()` and other
 /// trait methods on it:
@@ -325,11 +329,114 @@ pub async fn validate_manifest(manifest: &Manifest) -> Result<Vec<ValidationFail
             .into_iter()
             .cloned(),
     );
+    failures.extend(core_validation(manifest));
     failures.extend(check_misnamed_interfaces(manifest));
     failures.extend(check_dangling_links(manifest));
+    failures.extend(check_secrets_mapped_to_policies(manifest));
     Ok(failures)
 }
 
+fn core_validation(manifest: &Manifest) -> Vec<ValidationFailure> {
+    let mut failures = Vec::new();
+    let mut name_registry: HashSet<String> = HashSet::new();
+    let mut id_registry: HashSet<String> = HashSet::new();
+    let mut required_capability_components: HashSet<String> = HashSet::new();
+
+    for label in manifest.metadata.labels.iter() {
+        if !valid_oam_label(label) {
+            failures.push(ValidationFailure::new(
+                ValidationFailureLevel::Error,
+                format!("Invalid OAM label: {:?}", label),
+            ));
+        }
+    }
+
+    for annotation in manifest.metadata.annotations.iter() {
+        if !valid_oam_label(annotation) {
+            failures.push(ValidationFailure::new(
+                ValidationFailureLevel::Error,
+                format!("Invalid OAM annotation: {:?}", annotation),
+            ));
+        }
+    }
+
+    for component in manifest.spec.components.iter() {
+        // Component name validation : each component (components or providers) should have a unique name
+        if !name_registry.insert(component.name.clone()) {
+            failures.push(ValidationFailure::new(
+                ValidationFailureLevel::Error,
+                format!("Duplicate component name in manifest: {}", component.name),
+            ));
+        }
+        // Provider validation :
+        // Provider config should be serializable [For all components that have JSON config, validate that it can serialize.
+        // We need this so it doesn't trigger an error when sending a command down the line]
+        // Providers should have a unique image ref and link name
+        if let Properties::Capability {
+            properties:
+                CapabilityProperties {
+                    id: Some(component_id),
+                    config: _capability_config,
+                    ..
+                },
+        } = &component.properties
+        {
+            if !id_registry.insert(component_id.to_string()) {
+                failures.push(ValidationFailure::new(
+                    ValidationFailureLevel::Error,
+                    format!(
+                        "Duplicate component identifier in manifest: {}",
+                        component_id
+                    ),
+                ));
+            }
+        }
+
+        // Component validation : Components should have a unique identifier per manifest
+        if let Properties::Component {
+            properties: ComponentProperties { id: Some(id), .. },
+        } = &component.properties
+        {
+            if !id_registry.insert(id.to_string()) {
+                failures.push(ValidationFailure::new(
+                    ValidationFailureLevel::Error,
+                    format!("Duplicate component identifier in manifest: {}", id),
+                ));
+            }
+        }
+
+        // Linkdef validation : A linkdef from a component should have a unique target and reference
+        if let Some(traits_vec) = &component.traits {
+            for trait_item in traits_vec.iter() {
+                if let Trait {
+                    // TODO : add trait type validation after custom types are done. See TraitProperty enum.
+                    properties: TraitProperty::Link(LinkProperty { target, .. }),
+                    ..
+                } = &trait_item
+                {
+                    // Multiple components{ with type != 'capability'} can declare the same target, so we don't need to check for duplicates on insert
+                    required_capability_components.insert(target.name.to_string());
+                }
+            }
+        }
+    }
+
+    let missing_capability_components = required_capability_components
+        .difference(&name_registry)
+        .collect::<Vec<&String>>();
+
+    if !missing_capability_components.is_empty() {
+        failures.push(ValidationFailure::new(
+            ValidationFailureLevel::Error,
+            format!(
+                "The following capability component(s) are missing from the manifest: {:?}",
+                missing_capability_components
+            ),
+        ));
+    };
+    failures
+}
+
 /// Check for misnamed host-supported interfaces in the manifest
 fn check_misnamed_interfaces(manifest: &Manifest) -> Vec<ValidationFailure> {
     let mut failures = Vec::new();
@@ -415,6 +522,69 @@ fn check_dangling_links(manifest: &Manifest) -> Vec<ValidationFailure> {
     failures
 }
 
+fn check_secrets_mapped_to_policies(manifest: &Manifest) -> Vec<ValidationFailure> {
+    let policies = manifest.policy_lookup();
+    let mut failures = Vec::new();
+    for c in manifest.components() {
+        for secret in c.secrets() {
+            if !policies.contains_key(&secret.source.policy) {
+                failures.push(ValidationFailure::new(
+                    ValidationFailureLevel::Error,
+                    format!(
+                        "secret '{}' is mapped to unknown policy '{}'",
+                        secret.name, secret.source.policy
+                    ),
+                ))
+            }
+        }
+    }
+    failures
+}
+
+/// This function validates that a key/value pair is a valid OAM label. It's using fairly
+/// basic validation rules to ensure that the manifest isn't doing anything horribly wrong. Keeping
+/// this function free of regex is intentional to keep this code functional but simple.
+///
+/// See <https://github.com/oam-dev/spec/blob/master/metadata.md#metadata> for details
+fn valid_oam_label(label: (&String, &String)) -> bool {
+    let (key, _) = label;
+    match key.split_once('/') {
+        Some((prefix, name)) => is_valid_dns_subdomain(prefix) && is_valid_label_name(name),
+        None => is_valid_label_name(key),
+    }
+}
+
+fn is_valid_dns_subdomain(s: &str) -> bool {
+    if s.is_empty() || s.len() > 253 {
+        return false;
+    }
+
+    s.split('.').all(|part| {
+        // Ensure each part is non-empty, <= 63 characters, starts with an alphabetic character,
+        // ends with an alphanumeric character, and contains only alphanumeric characters or hyphens
+        !part.is_empty()
+            && part.len() <= 63
+            && part.starts_with(|c: char| c.is_ascii_alphabetic())
+            && part.ends_with(|c: char| c.is_ascii_alphanumeric())
+            && part.chars().all(|c| c.is_ascii_alphanumeric() || c == '-')
+    })
+}
+
+// Ensure each name is non-empty, <= 63 characters, starts with an alphanumeric character,
+// ends with an alphanumeric character, and contains only alphanumeric characters, hyphens,
+// underscores, or periods
+fn is_valid_label_name(name: &str) -> bool {
+    if name.is_empty() || name.len() > 63 {
+        return false;
+    }
+
+    name.starts_with(|c: char| c.is_ascii_alphanumeric())
+        && name.ends_with(|c: char| c.is_ascii_alphanumeric())
+        && name
+            .chars()
+            .all(|c| c.is_ascii_alphanumeric() || c == '-' || c == '_' || c == '.')
+}
+
 #[cfg(test)]
 mod tests {
     use super::is_valid_manifest_name;

crates/wadm/src/scaler/manager.rs

@@ -944,6 +944,8 @@ fn secrets_to_scalers<S: SecretSource + Send + Sync + Clone>(
         .iter()
         .map(|s| {
             let name = compute_secret_id(model_name, None, &s.name);
+            println!("{:?}", s.source.policy);
+            println!("{:?}", policies);
             let policy = *policies.get(&s.source.policy).unwrap();
             (
                 SecretScaler::new(

crates/wadm/src/scaler/secretscaler.rs

@@ -5,7 +5,7 @@ use std::{
     hash::{DefaultHasher, Hash, Hasher},
 };
 use tokio::sync::RwLock;
-use tracing::{debug, error, instrument, trace};
+use tracing::{debug, error, info, instrument, trace};
 use wadm_types::{
     api::{StatusInfo, StatusType},
     Policy, SecretSourceProperty, TraitProperty,
@@ -106,8 +106,13 @@ impl<S: SecretSource + Send + Sync + Clone> Scaler for SecretScaler<S> {
             // If configuration is out of sync, we put the configuration
             (Ok(_config), scaler_config) => {
                 debug!(self.secret_name, "Putting secret");
-                let mut cfg: HashMap<String, String> = scaler_config.try_into()?;
-                cfg.extend(self.policy.properties.clone());
+
+                let mut cfg: HashMap<String, String> = scaler_config.clone().try_into()?;
+                let mut policy = self.policy.properties.clone();
+                policy.insert("type".to_string(), self.policy.policy_type.clone());
+                let policy_json = serde_json::to_string(&policy)?;
+                cfg.insert("policy".to_string(), policy_json);
+
                 *self.status.write().await = StatusInfo::reconciling("Secret out of sync");
                 Ok(vec![Command::PutConfig(PutConfig {
                     config_name: self.secret_name.clone(),