Skip to content

Commit 89663c8

Browse files
committed
Validate against target environments during build
Signed-off-by: itowlson <ivan.towlson@fermyon.com>
1 parent 4223521 commit 89663c8

File tree

23 files changed

+2499
-301
lines changed

23 files changed

+2499
-301
lines changed

Cargo.lock

Lines changed: 220 additions & 65 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ spin-app = { path = "crates/app" }
5555
spin-build = { path = "crates/build" }
5656
spin-common = { path = "crates/common" }
5757
spin-doctor = { path = "crates/doctor" }
58+
spin-environments = { path = "crates/environments" }
5859
spin-factor-outbound-networking = { path = "crates/factor-outbound-networking" }
5960
spin-http = { path = "crates/http" }
6061
spin-loader = { path = "crates/loader" }

crates/build/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ edition = { workspace = true }
88
anyhow = { workspace = true }
99
serde = { workspace = true }
1010
spin-common = { path = "../common" }
11+
spin-environments = { path = "../environments" }
1112
spin-manifest = { path = "../manifest" }
1213
subprocess = "0.2"
1314
terminal = { path = "../terminal" }

crates/build/src/lib.rs

Lines changed: 89 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -16,31 +16,87 @@ use subprocess::{Exec, Redirection};
1616
use crate::manifest::component_build_configs;
1717

1818
/// If present, run the build command of each component.
19-
pub async fn build(manifest_file: &Path, component_ids: &[String]) -> Result<()> {
20-
let (components, manifest_err) =
21-
component_build_configs(manifest_file)
22-
.await
23-
.with_context(|| {
24-
format!(
25-
"Cannot read manifest file from {}",
26-
quoted_path(manifest_file)
27-
)
28-
})?;
19+
pub async fn build(
20+
manifest_file: &Path,
21+
component_ids: &[String],
22+
target_checks: TargetChecking,
23+
cache_root: Option<PathBuf>,
24+
) -> Result<()> {
25+
let build_info = component_build_configs(manifest_file)
26+
.await
27+
.with_context(|| {
28+
format!(
29+
"Cannot read manifest file from {}",
30+
quoted_path(manifest_file)
31+
)
32+
})?;
2933
let app_dir = parent_dir(manifest_file)?;
3034

31-
let build_result = build_components(component_ids, components, app_dir);
35+
let build_result = build_components(component_ids, build_info.components(), &app_dir);
3236

33-
if let Some(e) = manifest_err {
37+
// Emit any required warnings now, so that they don't bury any errors.
38+
if let Some(e) = build_info.load_error() {
39+
// The manifest had errors. We managed to attempt a build anyway, but we want to
40+
// let the user know about them.
3441
terminal::warn!("The manifest has errors not related to the Wasm component build. Error details:\n{e:#}");
42+
// Checking deployment targets requires a healthy manifest (because trigger types etc.),
43+
// if any of these were specified, warn they are being skipped.
44+
let should_have_checked_targets =
45+
target_checks.check() && build_info.has_deployment_targets();
46+
if should_have_checked_targets {
47+
terminal::warn!(
48+
"The manifest error(s) prevented Spin from checking the deployment targets."
49+
);
50+
}
51+
}
52+
53+
// If the build failed, exit with an error at this point.
54+
build_result?;
55+
56+
let Some(manifest) = build_info.manifest() else {
57+
// We can't proceed to checking (because that needs a full healthy manifest), and we've
58+
// already emitted any necessary warning, so quit.
59+
return Ok(());
60+
};
61+
62+
if target_checks.check() {
63+
let application = spin_environments::ApplicationToValidate::new(
64+
manifest.clone(),
65+
manifest_file.parent().unwrap(),
66+
)
67+
.await
68+
.context("unable to load application for checking against deployment targets")?;
69+
let target_validation = spin_environments::validate_application_against_environment_ids(
70+
&application,
71+
build_info.deployment_targets(),
72+
cache_root.clone(),
73+
&app_dir,
74+
)
75+
.await
76+
.context("unable to check if the application is compatible with deployment targets")?;
77+
78+
if !target_validation.is_ok() {
79+
for error in target_validation.errors() {
80+
terminal::error!("{error}");
81+
}
82+
anyhow::bail!("All components built successfully, but one or more was incompatible with one or more of the deployment targets.");
83+
}
3584
}
3685

37-
build_result
86+
Ok(())
87+
}
88+
89+
/// Run all component build commands, using the default options (build all
90+
/// components, perform target checking). We run a "default build" in several
91+
/// places and this centralises the logic of what such a "default build" means.
92+
pub async fn build_default(manifest_file: &Path, cache_root: Option<PathBuf>) -> Result<()> {
93+
build(manifest_file, &[], TargetChecking::Check, cache_root).await
3894
}
3995

4096
fn build_components(
4197
component_ids: &[String],
4298
components: Vec<ComponentBuildInfo>,
43-
app_dir: PathBuf,
99+
app_dir: &Path,
44100
) -> Result<(), anyhow::Error> {
45101
let components_to_build = if component_ids.is_empty() {
46102
components
@@ -70,7 +126,7 @@ fn build_components(
70126

71127
components_to_build
72128
.into_iter()
73-
.map(|c| build_component(c, &app_dir))
129+
.map(|c| build_component(c, app_dir))
74130
.collect::<Result<Vec<_>, _>>()?;
75131

76132
terminal::step!("Finished", "building all Spin components");
@@ -159,6 +215,21 @@ fn construct_workdir(app_dir: &Path, workdir: Option<impl AsRef<Path>>) -> Resul
159215
Ok(cwd)
160216
}
161217

218+
/// Specifies target environment checking behaviour
219+
pub enum TargetChecking {
220+
/// The build should check that all components are compatible with all target environments.
221+
Check,
222+
/// The build should not check target environments.
223+
Skip,
224+
}
225+
226+
impl TargetChecking {
227+
/// Should the build check target environments?
228+
fn check(&self) -> bool {
229+
matches!(self, Self::Check)
230+
}
231+
}
232+
162233
#[cfg(test)]
163234
mod tests {
164235
use super::*;
@@ -171,6 +242,8 @@ mod tests {
171242
#[tokio::test]
172243
async fn can_load_even_if_trigger_invalid() {
173244
let bad_trigger_file = test_data_root().join("bad_trigger.toml");
174-
build(&bad_trigger_file, &[]).await.unwrap();
245+
build(&bad_trigger_file, &[], TargetChecking::Skip, None)
246+
.await
247+
.unwrap();
175248
}
176249
}

crates/build/src/manifest.rs

Lines changed: 112 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4,37 +4,120 @@ use std::{collections::BTreeMap, path::Path};
44

55
use spin_manifest::{schema::v2, ManifestVersion};
66

7+
#[allow(clippy::large_enum_variant)] // only ever constructed once
8+
pub enum ManifestBuildInfo {
9+
Loadable {
10+
components: Vec<ComponentBuildInfo>,
11+
deployment_targets: Vec<spin_manifest::schema::v2::TargetEnvironmentRef>,
12+
manifest: spin_manifest::schema::v2::AppManifest,
13+
},
14+
Unloadable {
15+
components: Vec<ComponentBuildInfo>,
16+
has_deployment_targets: bool,
17+
load_error: spin_manifest::Error,
18+
},
19+
}
20+
21+
impl ManifestBuildInfo {
22+
pub fn components(&self) -> Vec<ComponentBuildInfo> {
23+
match self {
24+
Self::Loadable { components, .. } => components.clone(),
25+
Self::Unloadable { components, .. } => components.clone(),
26+
}
27+
}
28+
29+
pub fn load_error(&self) -> Option<&spin_manifest::Error> {
30+
match self {
31+
Self::Loadable { .. } => None,
32+
Self::Unloadable { load_error, .. } => Some(load_error),
33+
}
34+
}
35+
36+
pub fn deployment_targets(&self) -> &[spin_manifest::schema::v2::TargetEnvironmentRef] {
37+
match self {
38+
Self::Loadable {
39+
deployment_targets, ..
40+
} => deployment_targets,
41+
Self::Unloadable { .. } => &[],
42+
}
43+
}
44+
45+
pub fn has_deployment_targets(&self) -> bool {
46+
match self {
47+
Self::Loadable {
48+
deployment_targets, ..
49+
} => !deployment_targets.is_empty(),
50+
Self::Unloadable {
51+
has_deployment_targets,
52+
..
53+
} => *has_deployment_targets,
54+
}
55+
}
56+
57+
pub fn manifest(&self) -> Option<&spin_manifest::schema::v2::AppManifest> {
58+
match self {
59+
Self::Loadable { manifest, .. } => Some(manifest),
60+
Self::Unloadable { .. } => None,
61+
}
62+
}
63+
}
64+
765
/// Returns a map of component IDs to [`v2::ComponentBuildConfig`]s for the
866
/// given (v1 or v2) manifest path. If the manifest cannot be loaded, the
967
/// function attempts fallback: if fallback succeeds, result is Ok but the load error
1068
/// is also returned via the second part of the return value tuple.
11-
pub async fn component_build_configs(
12-
manifest_file: impl AsRef<Path>,
13-
) -> Result<(Vec<ComponentBuildInfo>, Option<spin_manifest::Error>)> {
69+
pub async fn component_build_configs(manifest_file: impl AsRef<Path>) -> Result<ManifestBuildInfo> {
1470
let manifest = spin_manifest::manifest_from_file(&manifest_file);
1571
match manifest {
16-
Ok(manifest) => Ok((build_configs_from_manifest(manifest), None)),
17-
Err(e) => fallback_load_build_configs(&manifest_file)
18-
.await
19-
.map(|bc| (bc, Some(e))),
72+
Ok(mut manifest) => {
73+
spin_manifest::normalize::normalize_manifest(&mut manifest);
74+
let components = build_configs_from_manifest(&manifest);
75+
let deployment_targets = deployment_targets_from_manifest(&manifest);
76+
Ok(ManifestBuildInfo::Loadable {
77+
components,
78+
deployment_targets,
79+
manifest,
80+
})
81+
}
82+
Err(load_error) => {
83+
// The manifest didn't load, but the problem might not be build-affecting.
84+
// Try to fall back by parsing out only the bits we need. And if something
85+
// goes wrong with the fallback, give up and return the original manifest load
86+
// error.
87+
let Ok(components) = fallback_load_build_configs(&manifest_file).await else {
88+
return Err(load_error.into());
89+
};
90+
let Ok(has_deployment_targets) = has_deployment_targets(&manifest_file).await else {
91+
return Err(load_error.into());
92+
};
93+
Ok(ManifestBuildInfo::Unloadable {
94+
components,
95+
has_deployment_targets,
96+
load_error,
97+
})
98+
}
2099
}
21100
}
22101

23102
fn build_configs_from_manifest(
24-
mut manifest: spin_manifest::schema::v2::AppManifest,
103+
manifest: &spin_manifest::schema::v2::AppManifest,
25104
) -> Vec<ComponentBuildInfo> {
26-
spin_manifest::normalize::normalize_manifest(&mut manifest);
27-
28105
manifest
29106
.components
30-
.into_iter()
107+
.iter()
31108
.map(|(id, c)| ComponentBuildInfo {
32109
id: id.to_string(),
33-
build: c.build,
110+
build: c.build.clone(),
34111
})
35112
.collect()
36113
}
37114

115+
fn deployment_targets_from_manifest(
116+
manifest: &spin_manifest::schema::v2::AppManifest,
117+
) -> Vec<spin_manifest::schema::v2::TargetEnvironmentRef> {
118+
manifest.application.targets.clone()
119+
}
120+
38121
async fn fallback_load_build_configs(
39122
manifest_file: impl AsRef<Path>,
40123
) -> Result<Vec<ComponentBuildInfo>> {
@@ -57,7 +140,23 @@ async fn fallback_load_build_configs(
57140
})
58141
}
59142

60-
#[derive(Deserialize)]
143+
async fn has_deployment_targets(manifest_file: impl AsRef<Path>) -> Result<bool> {
144+
let manifest_text = tokio::fs::read_to_string(manifest_file).await?;
145+
Ok(match ManifestVersion::detect(&manifest_text)? {
146+
ManifestVersion::V1 => false,
147+
ManifestVersion::V2 => {
148+
let table: toml::value::Table = toml::from_str(&manifest_text)?;
149+
table
150+
.get("application")
151+
.and_then(|a| a.as_table())
152+
.and_then(|t| t.get("targets"))
153+
.and_then(|arr| arr.as_array())
154+
.is_some_and(|arr| !arr.is_empty())
155+
}
156+
})
157+
}
158+
159+
#[derive(Clone, Deserialize)]
61160
pub struct ComponentBuildInfo {
62161
#[serde(default)]
63162
pub id: String,

0 commit comments

Comments
 (0)