Skip to content

Commit 5e7ef37

Browse files
aish-where-yaaish-where-ya
authored
feat(server)!: add validation support for manifests
* Added validation support for manifests Signed-off-by: aish-where-ya <aharpale@cosmonic.com> * Addressed review comments Signed-off-by: aish-where-ya <aharpale@cosmonic.com> * Fixed conflicts Signed-off-by: aish-where-ya <aharpale@cosmonic.com> * Added comments, removed unnecessary code Signed-off-by: aish-where-ya <aharpale@cosmonic.com> --------- Signed-off-by: aish-where-ya <aharpale@cosmonic.com>
1 parent 5d47a37 commit 5e7ef37

File tree

6 files changed

+494
-28
lines changed

6 files changed

+494
-28
lines changed

‎src/model/internal.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use super::LATEST_VERSION;
88

99
/// This struct represents a single manfiest, with its version history. Internally these are stored
1010
/// as an indexmap keyed by version name
11-
#[derive(Debug, Serialize, Deserialize, Default)]
11+
#[derive(Debug, Serialize, Deserialize, Default, Clone)]
1212
pub(crate) struct StoredManifest {
1313
// Ordering matters for how we store a manifest, so we need to use an index map to preserve
1414
// insertion order _and_ have quick access to specific versions

‎src/server/handlers.rs

Lines changed: 163 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,16 @@
1+
use anyhow::anyhow;
12
use async_nats::{jetstream::stream::Stream, Client, Message};
23
use base64::{engine::general_purpose::STANDARD as B64decoder, Engine};
34
use regex::Regex;
45
use serde_json::json;
6+
use std::collections::{HashMap, HashSet};
57
use tokio::sync::OnceCell;
68
use tracing::{debug, error, instrument, log::warn, trace};
79

810
use crate::{
911
model::{
10-
internal::StoredManifest, CapabilityConfig, CapabilityProperties, Properties,
11-
LATEST_VERSION,
12+
internal::StoredManifest, ActorProperties, CapabilityProperties, LinkdefProperty, Manifest,
13+
Properties, Trait, TraitProperty, LATEST_VERSION,
1214
},
1315
publisher::Publisher,
1416
server::StatusType,
@@ -57,31 +59,6 @@ impl<P: Publisher> Handler<P> {
5759
return;
5860
}
5961

60-
// For all components that have JSON config, validate that it can serialize. We need this so
61-
// it doesn't trigger an error when sending a command down the line
62-
for component in manifest.spec.components.iter() {
63-
if let Properties::Capability {
64-
properties:
65-
CapabilityProperties {
66-
config: Some(CapabilityConfig::Json(data)),
67-
..
68-
},
69-
} = &component.properties
70-
{
71-
if let Err(e) = serde_json::to_string(data) {
72-
self.send_error(
73-
msg.reply,
74-
format!(
75-
"Unable to serialize JSON config data for component {}: {e:?}",
76-
component.name,
77-
),
78-
)
79-
.await;
80-
return;
81-
}
82-
}
83-
}
84-
8562
let manifest_name = manifest.metadata.name.trim().to_string();
8663
if !MANIFEST_NAME_REGEX
8764
// SAFETY: We know this is valid Regex
@@ -112,6 +89,11 @@ impl<P: Publisher> Handler<P> {
11289
}
11390
};
11491

92+
if let Some(error_message) = validate_manifest(manifest.clone()).err() {
93+
self.send_error(msg.reply, error_message.to_string()).await;
94+
return;
95+
}
96+
11597
let mut resp = PutModelResponse {
11698
// If we successfully insert, the given manifest version will be the new current version
11799
current_version: manifest.version().to_owned(),
@@ -811,8 +793,162 @@ impl<P: Publisher> Handler<P> {
811793
}
812794
}
813795

796+
// Manifest validation
797+
pub(crate) fn validate_manifest(manifest: Manifest) -> anyhow::Result<()> {
798+
let mut component_details: HashSet<String> = HashSet::new();
799+
800+
// Map of link names to a vector of provider references with that link name
801+
let mut linkdef_map: HashMap<String, Vec<String>> = HashMap::new();
802+
803+
for component in manifest.spec.components.iter() {
804+
// Component name validation : each component (actors or providers) should have a unique name
805+
if !component_details.insert(component.name.clone()) {
806+
return Err(anyhow!(
807+
"Duplicate component name in manifest: {}",
808+
component.name
809+
));
810+
}
811+
// Provider validation :
812+
// Provider config should be serializable [For all components that have JSON config, validate that it can serialize.
813+
// We need this so it doesn't trigger an error when sending a command down the line]
814+
// Providers should have a unique image ref and link name
815+
if let Properties::Capability {
816+
properties:
817+
CapabilityProperties {
818+
image: image_name,
819+
link_name: Some(link),
820+
config: capability_config,
821+
..
822+
},
823+
} = &component.properties
824+
{
825+
if let Some(data) = capability_config {
826+
if let Err(e) = serde_json::to_string(data) {
827+
return Err(anyhow!(
828+
"Unable to serialize JSON config data for component {}: {e:?}",
829+
component.name
830+
));
831+
}
832+
}
833+
834+
if let Some(duplicate_ref) = linkdef_map.get_mut(link) {
835+
if duplicate_ref.contains(image_name) {
836+
return Err(anyhow!(
837+
"Duplicate image reference {} to link name {} in manifest",
838+
image_name,
839+
link
840+
));
841+
} else {
842+
duplicate_ref.push(image_name.to_string());
843+
}
844+
}
845+
linkdef_map.insert(link.to_string(), vec![image_name.to_string()]);
846+
}
847+
848+
// Actor validation : Actors should have a unique name and reference
849+
if let Properties::Actor {
850+
properties: ActorProperties { image: image_name },
851+
} = &component.properties
852+
{
853+
if !component_details.insert(image_name.to_string()) {
854+
return Err(anyhow!(
855+
"Duplicate image reference in manifest: {}",
856+
image_name
857+
));
858+
}
859+
}
860+
861+
// Linkdef validation : A linkdef from a component should have a unique target and reference
862+
let mut linkdef_set: HashSet<String> = HashSet::new();
863+
if let Some(traits_vec) = &component.traits {
864+
for trait_item in traits_vec.iter() {
865+
if let Trait {
866+
// TODO : add trait type validation after custom types are done. See TraitProperty enum.
867+
properties:
868+
TraitProperty::Linkdef(LinkdefProperty {
869+
target: target_name,
870+
..
871+
}),
872+
..
873+
} = &trait_item
874+
{
875+
if !linkdef_set.insert(target_name.to_string()) {
876+
return Err(anyhow!(
877+
"Duplicate target for linkdef in manifest: {}",
878+
target_name
879+
));
880+
}
881+
}
882+
}
883+
}
884+
}
885+
Ok(())
886+
}
887+
814888
#[cfg(test)]
815889
mod test {
890+
use std::io::BufReader;
891+
use std::path::Path;
892+
893+
use super::*;
894+
use anyhow::Result;
895+
use serde_yaml;
896+
897+
pub(crate) fn deserialize_yaml(filepath: impl AsRef<Path>) -> Result<Manifest> {
898+
let file = std::fs::File::open(filepath)?;
899+
let reader = BufReader::new(file);
900+
let yaml_string: Manifest = serde_yaml::from_reader(reader)?;
901+
Ok(yaml_string)
902+
}
903+
904+
#[test]
905+
fn test_manifest_validation() {
906+
let correct_manifest =
907+
deserialize_yaml("./oam/simple1.yaml").expect("Should be able to parse");
908+
909+
assert!(validate_manifest(correct_manifest).is_ok());
910+
911+
let manifest = deserialize_yaml("./test/data/duplicate_component.yaml")
912+
.expect("Should be able to parse");
913+
914+
match validate_manifest(manifest) {
915+
Ok(()) => panic!("Should have detected duplicate component"),
916+
Err(e) => assert!(e
917+
.to_string()
918+
.contains("Duplicate component name in manifest")),
919+
}
920+
921+
let manifest = deserialize_yaml("./test/data/duplicate_imageref1.yaml")
922+
.expect("Should be able to parse");
923+
924+
match validate_manifest(manifest) {
925+
Ok(()) => panic!("Should have detected duplicate image reference for link name in provider properties"),
926+
Err(e) => assert!(e
927+
.to_string()
928+
.contains("Duplicate image reference")),
929+
}
930+
931+
let manifest = deserialize_yaml("./test/data/duplicate_imageref2.yaml")
932+
.expect("Should be able to parse");
933+
934+
match validate_manifest(manifest) {
935+
Ok(()) => panic!("Should have detected duplicate image reference for actor"),
936+
Err(e) => assert!(e
937+
.to_string()
938+
.contains("Duplicate image reference in manifest")),
939+
}
940+
941+
let manifest = deserialize_yaml("./test/data/duplicate_linkdef.yaml")
942+
.expect("Should be able to parse");
943+
944+
match validate_manifest(manifest) {
945+
Ok(()) => panic!("Should have detected duplicate linkdef"),
946+
Err(e) => assert!(e
947+
.to_string()
948+
.contains("Duplicate target for linkdef in manifest")),
949+
}
950+
}
951+
816952
#[tokio::test]
817953
async fn manifest_name_regex_works() {
818954
let regex = super::MANIFEST_NAME_REGEX

‎test/data/duplicate_component.yaml

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
apiVersion: core.oam.dev/v1beta1
2+
kind: Application
3+
metadata:
4+
name: petclinic
5+
annotations:
6+
version: v0.0.1
7+
description: "wasmCloud Pet Clinic Sample"
8+
spec:
9+
components:
10+
- name: ui
11+
type: actor
12+
properties:
13+
image: wasmcloud.azurecr.io/ui:0.3.2
14+
traits:
15+
- type: spreadscaler
16+
properties:
17+
replicas: 1
18+
spread:
19+
- name: uiclinicapp
20+
requirements:
21+
app: petclinic
22+
23+
- name: ui
24+
type: actor
25+
properties:
26+
image: wasmcloud.azurecr.io/customers:0.3.1
27+
traits:
28+
- type: linkdef
29+
properties:
30+
target: postgres
31+
values:
32+
uri: postgres://user:pass@your.db.host.com/petclinic
33+
- type: spreadscaler
34+
properties:
35+
replicas: 1
36+
spread:
37+
- name: customersclinicapp
38+
requirements:
39+
app: petclinic
40+
41+
- name: vets
42+
type: actor
43+
properties:
44+
image: wasmcloud.azurecr.io/vets:0.3.1
45+
traits:
46+
- type: linkdef
47+
properties:
48+
target: postgres
49+
values:
50+
uri: postgres://user:pass@your.db.host.com/petclinic
51+
foo: bar
52+
- type: spreadscaler
53+
properties:
54+
replicas: 1
55+
spread:
56+
- name: vetsclinicapp
57+
requirements:
58+
app: petclinic
59+
60+
- name: vets
61+
type: actor
62+
properties:
63+
image: wasmcloud.azurecr.io/visits:0.3.1
64+
traits:
65+
- type: linkdef
66+
properties:
67+
target: postgres
68+
values:
69+
uri: postgres://user:pass@your.db.host.com/petclinic
70+
71+
- type: spreadscaler
72+
properties:
73+
replicas: 1
74+
spread:
75+
- name: visitsclinicapp
76+
requirements:
77+
app: petclinic
78+
79+
- name: clinicapi
80+
type: actor
81+
properties:
82+
image: wasmcloud.azurecr.io/clinicapi:0.3.1
83+
traits:
84+
- type: spreadscaler
85+
properties:
86+
replicas: 1
87+
spread:
88+
- name: clinicapp
89+
requirements:
90+
app: petclinic
91+
- type: linkdef
92+
properties:
93+
target: httpserver
94+
values:
95+
address: "0.0.0.0:8080"
96+
97+
- name: httpserver
98+
type: capability
99+
properties:
100+
image: wasmcloud.azurecr.io/httpserver:0.16.2
101+
contract: wasmcloud:httpserver
102+
traits:
103+
- type: spreadscaler
104+
properties:
105+
replicas: 1
106+
spread:
107+
- name: httpserverspread
108+
requirements:
109+
app: petclinic
110+
111+
- name: postgres
112+
type: capability
113+
properties:
114+
image: wasmcloud.azurecr.io/sqldb-postgres:0.3.1
115+
contract: wasmcloud:sqldb
116+
traits:
117+
- type: spreadscaler
118+
properties:
119+
replicas: 1
120+
spread:
121+
- name: postgresspread
122+
requirements:
123+
app: petclinic

0 commit comments

Comments
 (0)