Skip to content

Commit

Permalink
feat(server): ensure deployed apps find shared components
Browse files Browse the repository at this point in the history 
Signed-off-by: Brooks Townsend <[email protected]>
  • Loading branch information
@brooksmtownsend
brooksmtownsend committed Sep 25, 2024
1 parent 4c9883e commit 78702c1
Show file tree
Hide file tree
Showing 2 changed files with 108 additions and 66 deletions.
46 changes: 46 additions & 0 deletions crates/wadm-types/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,52 @@ impl Manifest {
self.spec.components.iter()
}

/// Helper function to find shared components that are missing from the given list of
/// deployed applications
pub fn missing_shared_components(&self, deployed_apps: &Vec<&Manifest>) -> Vec<&Component> {
self.spec
.components
.iter()
.filter_map(|shared_component| {
match &shared_component.properties {
Properties::Capability {
properties:
CapabilityProperties {
image: None,
application: Some(shared_app),
..
},
}
| Properties::Component {
properties:
ComponentProperties {
image: None,
application: Some(shared_app),
..
},
} => {
if deployed_apps.iter().filter(|a| a.shared()).any(|m| {
m.metadata.name == shared_app.name
&& m.components().any(|c| {
c.name == shared_app.component
// This compares just the enum variant, not the actual properties
// For example, if we reference a shared component that's a capability,
// we want to make sure the deployed component is a capability.
&& std::mem::discriminant(&c.properties)
== std::mem::discriminant(&shared_component.properties)
})
}) {
None
} else {
Some(shared_component)
}
}
_ => None,
}
})
.collect()
}

/// Returns only the WebAssembly components in the manifest
pub fn wasm_components(&self) -> impl Iterator<Item = &Component> {
self.components()
Expand Down
128 changes: 62 additions & 66 deletions crates/wadm/src/server/handlers.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,7 @@ impl<P: Publisher> Handler<P> {
self.send_error(
msg.reply,
format!(
"Manifest name {} contains invalid characters. Manifest names can only contain alphanumeric characters, dashes, and underscores.",
manifest_name
"Manifest name {manifest_name} contains invalid characters. Manifest names can only contain alphanumeric characters, dashes, and underscores.",
),
)
.await;
Expand All @@ -90,66 +89,48 @@ impl<P: Publisher> Handler<P> {
}
};

if let Some(error_message) = validate_manifest(manifest.clone()).await.err() {
if let Some(error_message) = validate_manifest(&manifest).await.err() {
self.send_error(msg.reply, error_message.to_string()).await;
return;
}

let all_manifests: Vec<Manifest> = self
let all_stored_manifests = self
.store
.list(account_id, lattice_id)
.await
.unwrap_or_default()
.into_iter()
.filter(|manifest| manifest.get_current().shared())
// TODO: Do we need to just get deployed here? I assume that would be a warning if not deployed
.map(|manifest| manifest.get_current().clone())
.unwrap_or_default();
let deployed_shared_apps: Vec<&Manifest> = all_stored_manifests
.iter()
// Only keep deployed, shared applications
.filter(|manifest| {
manifest.deployed_version().is_some() && manifest.get_current().shared()
})
.map(|manifest| manifest.get_current())
.collect();

// TODO: We are also going to have to ensure that links don't clobber other links. Specifically
// this relates to the uniqueness properties of a source/target/interface/etc for a link.

// Ensure that all shared components are present in other (deployed?) manifests that are
// marked as shared applications
let mut shared_components = manifest
.components()
// Find all shared components
.filter_map(|c| match &c.properties {
Properties::Capability {
properties:
CapabilityProperties {
image: None,
application: Some(manifest_properties),
..
},
} => Some(manifest_properties),
Properties::Component {
properties:
ComponentProperties {
image: None,
application: Some(manifest_properties),
..
},
} => Some(manifest_properties),
_ => None,
});

// Ensure all shared components point to a component that is deployed in another manifest
// TODO: would be better to find matching bad shared components
if !shared_components.all(|properties| {
// TODO: ensure that the type of the property is preserved, e.g. component -> component
// TODO: ensure that the manifest is deployed
all_manifests.iter().any(|m| {
m.metadata.name == properties.name
&& m.components().any(|c| c.name == properties.component)
})
}) {
self.send_error(msg.reply, "shared component not found".to_string())
.await;
return;
}
// Explicitly drop to let the compiler know we're done with the manifest borrow
drop(shared_components);
// TODO: Consider basic link collision validation, e.g. with link name for same source/ns/package/interface

// NOTE(brooksmtownsend): You can put an application with missing shared components, because
// the time where you truly need them is when you deploy the application. This can cause a bit
// of friction when it comes to deploy, but it avoids the frustrating race condition where you
// - Put the application looking for a deployed shared component
// - Undeploy the application with the shared component
// - Deploy the new application looking for the shared component (error)
let missing_shared_components = manifest.missing_shared_components(&deployed_shared_apps);
let message = if missing_shared_components.is_empty() {
format!(
"Successfully put manifest {} {}",
manifest_name,
current_manifests.current_version().to_owned()
)
} else {
format!(
"Successfully put manifest {} {}, but some shared components are not deployed: {:?}",
manifest_name,
current_manifests.current_version().to_owned(),
missing_shared_components
)
};

let incoming_version = manifest.version().to_owned();
if !current_manifests.add_version(manifest) {
Expand All @@ -171,11 +152,7 @@ impl<P: Publisher> Handler<P> {
},
name: manifest_name.clone(),
total_versions: current_manifests.count(),
message: format!(
"Successfully put manifest {} {}",
manifest_name,
current_manifests.current_version()
),
message,
};

trace!(total_manifests = %resp.total_versions, "Storing manifests");
Expand Down Expand Up @@ -438,6 +415,7 @@ impl<P: Publisher> Handler<P> {
}
}
};
// TODO: if shared and deployed, make sure that no other shared apps are using it
let reply_data = if let Some(version) = req.version {
match self.store.get(account_id, lattice_id, name).await {
Ok(Some((mut current, current_revision))) => {
Expand Down Expand Up @@ -730,6 +708,23 @@ impl<P: Publisher> Handler<P> {
}
}

// TODO: If this app is shared, or the previous version was, I need to make sure
// that deploying this version doesn't break a relationship

let missing_shared_components = staged_model.missing_shared_components(
&stored_models
.iter()
.filter(|a| a.deployed_version().is_some() && a.get_current().shared())
.map(|a| a.get_current())
.collect(),
);

// Ensure all shared components point to a valid component that is deployed in another application
if !missing_shared_components.is_empty() {
self.send_error(msg.reply, format!("Application contains shared components that are not deployed in other applications: {:?}", missing_shared_components)).await;
return;
}

if !manifests.deploy(req.version.clone()) {
trace!("Requested version does not exist");
self.send_reply(
Expand Down Expand Up @@ -858,6 +853,7 @@ impl<P: Publisher> Handler<P> {
return;
}
};
// TODO: if shared, make sure that no other shared apps are using it

let reply = if manifests.undeploy() {
trace!("Manifest undeployed. Storing updated manifest");
Expand Down Expand Up @@ -1023,7 +1019,7 @@ fn summary_from_manifest_status(manifest: StoredManifest, status: Status) -> Mod
}

// Manifest validation
pub(crate) async fn validate_manifest(manifest: Manifest) -> anyhow::Result<()> {
pub(crate) async fn validate_manifest(manifest: &Manifest) -> anyhow::Result<()> {
let failures = wadm_types::validation::validate_manifest(&manifest).await?;
for failure in failures {
if matches!(
Expand Down Expand Up @@ -1059,12 +1055,12 @@ mod test {
let correct_manifest = deserialize_yaml("../../tests/fixtures/manifests/simple.yaml")
.expect("Should be able to parse");

assert!(validate_manifest(correct_manifest).await.is_ok());
assert!(validate_manifest(&correct_manifest).await.is_ok());

let manifest = deserialize_yaml("../../tests/fixtures/manifests/incorrect_component.yaml")
.expect("Should be able to parse");

match validate_manifest(manifest).await {
match validate_manifest(&manifest).await {
Ok(()) => panic!("Should have detected incorrect component"),
Err(e) => {
assert!(e
Expand All @@ -1076,7 +1072,7 @@ mod test {
let manifest = deserialize_yaml("../../tests/fixtures/manifests/duplicate_component.yaml")
.expect("Should be able to parse");

match validate_manifest(manifest).await {
match validate_manifest(&manifest).await {
Ok(()) => panic!("Should have detected duplicate component"),
Err(e) => assert!(e
.to_string()
Expand All @@ -1086,7 +1082,7 @@ mod test {
let manifest = deserialize_yaml("../../tests/fixtures/manifests/duplicate_id1.yaml")
.expect("Should be able to parse");

match validate_manifest(manifest).await {
match validate_manifest(&manifest).await {
Ok(()) => {
panic!("Should have detected duplicate component ID in provider properties")
}
Expand All @@ -1098,7 +1094,7 @@ mod test {
let manifest = deserialize_yaml("../../tests/fixtures/manifests/duplicate_id2.yaml")
.expect("Should be able to parse");

match validate_manifest(manifest).await {
match validate_manifest(&manifest).await {
Ok(()) => panic!("Should have detected duplicate component ID in component properties"),
Err(e) => assert!(e
.to_string()
Expand All @@ -1109,7 +1105,7 @@ mod test {
deserialize_yaml("../../tests/fixtures/manifests/missing_capability_component.yaml")
.expect("Should be able to parse");

match validate_manifest(manifest).await {
match validate_manifest(&manifest).await {
Ok(()) => panic!("Should have detected missing capability component"),
Err(e) => assert!(e
.to_string()
Expand All @@ -1122,7 +1118,7 @@ mod test {
#[tokio::test]
async fn manifest_name_long_image_ref() -> Result<()> {
validate_manifest(
deserialize_yaml("../../tests/fixtures/manifests/long_image_refs.yaml")
&deserialize_yaml("../../tests/fixtures/manifests/long_image_refs.yaml")
.context("failed to deserialize YAML")?,
)
.await
Expand Down

0 comments on commit 78702c1

Please sign in to comment.