diff --git a/Cargo.lock b/Cargo.lock index a376eb5a..0924f3ca 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3553,7 +3553,7 @@ dependencies = [ "serde_yaml", "serial_test", "sha2", - "thiserror 2.0.6", + "thiserror 2.0.7", "tokio", "tracing", "tracing-futures", @@ -3603,7 +3603,7 @@ dependencies = [ "nkeys", "serde_json", "serde_yaml", - "thiserror 2.0.6", + "thiserror 2.0.7", "tokio", "wadm-types", ] diff --git a/crates/wadm-types/src/validation.rs b/crates/wadm-types/src/validation.rs index ae58570c..e69ba4a1 100644 --- a/crates/wadm-types/src/validation.rs +++ b/crates/wadm-types/src/validation.rs @@ -345,6 +345,7 @@ pub async fn validate_manifest(manifest: &Manifest) -> Result Vec Vec { + let mut failures = Vec::new(); + let mut link_config_names = HashSet::new(); + for link_trait in manifest.links() { + if let TraitProperty::Link(LinkProperty { + target, + source, + .. + }) = &link_trait.properties { + for config in target.config.iter() { + // Check if config name is unique + if !link_config_names.insert(( + config.name.clone(), + )) { + failures.push(ValidationFailure::new( + ValidationFailureLevel::Error, + format!( + "Duplicate link config name found: '{}'", + config.name + ), + )); + } + } + + if let Some(source) = source { + for config in source.config.iter() { + // Check if config name is unique + if !link_config_names.insert(( + config.name.clone(), + )) { + failures.push(ValidationFailure::new( + ValidationFailureLevel::Error, + format!( + "Duplicate link config name found: '{}'", + config.name + ), + )); + } + } + } + } + }; + 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. diff --git a/tests/fixtures/manifests/duplicate_link_config_names.wadm.yaml b/tests/fixtures/manifests/duplicate_link_config_names.wadm.yaml new file mode 100644 index 00000000..45116c13 --- /dev/null +++ b/tests/fixtures/manifests/duplicate_link_config_names.wadm.yaml @@ -0,0 +1,78 @@ +apiVersion: core.oam.dev/v1beta1 +kind: Application +metadata: + name: my-example-app + annotations: + description: "This is my app" +spec: + components: + - name: userinfo1 + type: component + properties: + image: wasmcloud.azurecr.io/fake:1 + traits: + - type: link + properties: + namespace: wasi + package: keyvalue + interfaces: [atomics, store] + target: + name: kvredis + config: + - name: redis-url + + - name: userinfo2 + type: component + properties: + image: wasmcloud.azurecr.io/fake:1 + traits: + - type: link + properties: + namespace: wasi + package: keyvalue + interfaces: [atomics, store] + target: + name: kvredis + config: + - name: redis-url + + - name: webcap1 + type: capability + properties: + id: httpserver1 + image: wasmcloud.azurecr.io/httpserver:0.13.1 + traits: + - type: link + properties: + namespace: wasi + package: http + interfaces: ["incoming-handler"] + target: + name: userinfo1 + source: + config: + - name: default-port + - name: alternate-port + - name: alternate-port + + - name: webcap2 + type: capability + properties: + id: httpserver2 + image: wasmcloud.azurecr.io/httpserver:0.14.1 + traits: + - type: link + properties: + target: + name: userinfo2 + namespace: wasi + package: http + interfaces: ["incoming-handler"] + source: + config: + - name: default-port + + - name: kvredis + type: capability + properties: + image: ghcr.io/wasmcloud/keyvalue-redis:0.28.1 diff --git a/tests/validation.rs b/tests/validation.rs index b048ef33..a7335d5c 100644 --- a/tests/validation.rs +++ b/tests/validation.rs @@ -118,3 +118,26 @@ async fn validate_policy() -> Result<()> { assert!(failures.valid(), "manifest is valid"); Ok(()) } + +/// Ensure that we can detect duplicated link config names +#[tokio::test] +async fn validate_link_config_names() -> Result<()> { + let (_manifest, failures) = + validate_manifest_file("./tests/fixtures/manifests/duplicate_link_config_names.wadm.yaml") + .await + .context("failed to validate manifest")?; + let expected_errors = 3; + assert!( + !failures.is_empty() + && failures + .iter() + .all(|f| f.level == ValidationFailureLevel::Error) + && failures.len() == expected_errors, + "expected {} errors because manifest contains {} duplicated link config names, instead {} errors were found", expected_errors, expected_errors, failures.len().to_string() + ); + assert!( + !failures.valid(), + "manifest should be invalid (duplicated link config names lead to a dead loop)" + ); + Ok(()) +}