Skip to content

Validate target environments #2806

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
285 changes: 220 additions & 65 deletions Cargo.lock

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ spin-app = { path = "crates/app" }
spin-build = { path = "crates/build" }
spin-common = { path = "crates/common" }
spin-doctor = { path = "crates/doctor" }
spin-environments = { path = "crates/environments" }
spin-factor-outbound-networking = { path = "crates/factor-outbound-networking" }
spin-http = { path = "crates/http" }
spin-loader = { path = "crates/loader" }
Expand Down
1 change: 1 addition & 0 deletions crates/build/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ edition = { workspace = true }
anyhow = { workspace = true }
serde = { workspace = true }
spin-common = { path = "../common" }
spin-environments = { path = "../environments" }
spin-manifest = { path = "../manifest" }
subprocess = "0.2"
terminal = { path = "../terminal" }
Expand Down
105 changes: 89 additions & 16 deletions crates/build/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,31 +16,87 @@ use subprocess::{Exec, Redirection};
use crate::manifest::component_build_configs;

/// If present, run the build command of each component.
pub async fn build(manifest_file: &Path, component_ids: &[String]) -> Result<()> {
let (components, manifest_err) =
component_build_configs(manifest_file)
.await
.with_context(|| {
format!(
"Cannot read manifest file from {}",
quoted_path(manifest_file)
)
})?;
pub async fn build(
manifest_file: &Path,
component_ids: &[String],
target_checks: TargetChecking,
cache_root: Option<PathBuf>,
) -> Result<()> {
let build_info = component_build_configs(manifest_file)
.await
.with_context(|| {
format!(
"Cannot read manifest file from {}",
quoted_path(manifest_file)
)
})?;
let app_dir = parent_dir(manifest_file)?;

let build_result = build_components(component_ids, components, app_dir);
let build_result = build_components(component_ids, build_info.components(), &app_dir);

if let Some(e) = manifest_err {
// Emit any required warnings now, so that they don't bury any errors.
if let Some(e) = build_info.load_error() {
// The manifest had errors. We managed to attempt a build anyway, but we want to
// let the user know about them.
terminal::warn!("The manifest has errors not related to the Wasm component build. Error details:\n{e:#}");
// Checking deployment targets requires a healthy manifest (because trigger types etc.),
// if any of these were specified, warn they are being skipped.
let should_have_checked_targets =
target_checks.check() && build_info.has_deployment_targets();
if should_have_checked_targets {
terminal::warn!(
"The manifest error(s) prevented Spin from checking the deployment targets."
);
}
}

// If the build failed, exit with an error at this point.
build_result?;

let Some(manifest) = build_info.manifest() else {
// We can't proceed to checking (because that needs a full healthy manifest), and we've
// already emitted any necessary warning, so quit.
return Ok(());
};

if target_checks.check() {
let application = spin_environments::ApplicationToValidate::new(
manifest.clone(),
manifest_file.parent().unwrap(),
)
.await
.context("unable to load application for checking against deployment targets")?;
let target_validation = spin_environments::validate_application_against_environment_ids(
&application,
build_info.deployment_targets(),
cache_root.clone(),
&app_dir,
)
.await
.context("unable to check if the application is compatible with deployment targets")?;

if !target_validation.is_ok() {
for error in target_validation.errors() {
terminal::error!("{error}");
}
anyhow::bail!("All components built successfully, but one or more was incompatible with one or more of the deployment targets.");
}
}

build_result
Ok(())
}

/// Run all component build commands, using the default options (build all
/// components, perform target checking). We run a "default build" in several
/// places and this centralises the logic of what such a "default build" means.
pub async fn build_default(manifest_file: &Path, cache_root: Option<PathBuf>) -> Result<()> {
build(manifest_file, &[], TargetChecking::Check, cache_root).await
}

fn build_components(
component_ids: &[String],
components: Vec<ComponentBuildInfo>,
app_dir: PathBuf,
app_dir: &Path,
) -> Result<(), anyhow::Error> {
let components_to_build = if component_ids.is_empty() {
components
Expand Down Expand Up @@ -70,7 +126,7 @@ fn build_components(

components_to_build
.into_iter()
.map(|c| build_component(c, &app_dir))
.map(|c| build_component(c, app_dir))
.collect::<Result<Vec<_>, _>>()?;

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

/// Specifies target environment checking behaviour
pub enum TargetChecking {
/// The build should check that all components are compatible with all target environments.
Check,
/// The build should not check target environments.
Skip,
}

impl TargetChecking {
/// Should the build check target environments?
fn check(&self) -> bool {
matches!(self, Self::Check)
}
}

#[cfg(test)]
mod tests {
use super::*;
Expand All @@ -171,6 +242,8 @@ mod tests {
#[tokio::test]
async fn can_load_even_if_trigger_invalid() {
let bad_trigger_file = test_data_root().join("bad_trigger.toml");
build(&bad_trigger_file, &[]).await.unwrap();
build(&bad_trigger_file, &[], TargetChecking::Skip, None)
.await
.unwrap();
}
}
125 changes: 112 additions & 13 deletions crates/build/src/manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,37 +4,120 @@ use std::{collections::BTreeMap, path::Path};

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

#[allow(clippy::large_enum_variant)] // only ever constructed once
pub enum ManifestBuildInfo {
Loadable {
components: Vec<ComponentBuildInfo>,
deployment_targets: Vec<spin_manifest::schema::v2::TargetEnvironmentRef>,
manifest: spin_manifest::schema::v2::AppManifest,
},
Unloadable {
components: Vec<ComponentBuildInfo>,
has_deployment_targets: bool,
load_error: spin_manifest::Error,
},
}

impl ManifestBuildInfo {
pub fn components(&self) -> Vec<ComponentBuildInfo> {
match self {
Self::Loadable { components, .. } => components.clone(),
Self::Unloadable { components, .. } => components.clone(),
}
}

pub fn load_error(&self) -> Option<&spin_manifest::Error> {
match self {
Self::Loadable { .. } => None,
Self::Unloadable { load_error, .. } => Some(load_error),
}
}

pub fn deployment_targets(&self) -> &[spin_manifest::schema::v2::TargetEnvironmentRef] {
match self {
Self::Loadable {
deployment_targets, ..
} => deployment_targets,
Self::Unloadable { .. } => &[],
}
}

pub fn has_deployment_targets(&self) -> bool {
match self {
Self::Loadable {
deployment_targets, ..
} => !deployment_targets.is_empty(),
Self::Unloadable {
has_deployment_targets,
..
} => *has_deployment_targets,
}
}

pub fn manifest(&self) -> Option<&spin_manifest::schema::v2::AppManifest> {
match self {
Self::Loadable { manifest, .. } => Some(manifest),
Self::Unloadable { .. } => None,
}
}
}

/// Returns a map of component IDs to [`v2::ComponentBuildConfig`]s for the
/// given (v1 or v2) manifest path. If the manifest cannot be loaded, the
/// function attempts fallback: if fallback succeeds, result is Ok but the load error
/// is also returned via the second part of the return value tuple.
pub async fn component_build_configs(
manifest_file: impl AsRef<Path>,
) -> Result<(Vec<ComponentBuildInfo>, Option<spin_manifest::Error>)> {
pub async fn component_build_configs(manifest_file: impl AsRef<Path>) -> Result<ManifestBuildInfo> {
let manifest = spin_manifest::manifest_from_file(&manifest_file);
match manifest {
Ok(manifest) => Ok((build_configs_from_manifest(manifest), None)),
Err(e) => fallback_load_build_configs(&manifest_file)
.await
.map(|bc| (bc, Some(e))),
Ok(mut manifest) => {
spin_manifest::normalize::normalize_manifest(&mut manifest);
let components = build_configs_from_manifest(&manifest);
let deployment_targets = deployment_targets_from_manifest(&manifest);
Ok(ManifestBuildInfo::Loadable {
components,
deployment_targets,
manifest,
})
}
Err(load_error) => {
// The manifest didn't load, but the problem might not be build-affecting.
// Try to fall back by parsing out only the bits we need. And if something
// goes wrong with the fallback, give up and return the original manifest load
// error.
let Ok(components) = fallback_load_build_configs(&manifest_file).await else {
return Err(load_error.into());
};
let Ok(has_deployment_targets) = has_deployment_targets(&manifest_file).await else {
return Err(load_error.into());
};
Ok(ManifestBuildInfo::Unloadable {
components,
has_deployment_targets,
load_error,
})
}
}
}

fn build_configs_from_manifest(
mut manifest: spin_manifest::schema::v2::AppManifest,
manifest: &spin_manifest::schema::v2::AppManifest,
) -> Vec<ComponentBuildInfo> {
spin_manifest::normalize::normalize_manifest(&mut manifest);

manifest
.components
.into_iter()
.iter()
.map(|(id, c)| ComponentBuildInfo {
id: id.to_string(),
build: c.build,
build: c.build.clone(),
})
.collect()
}

fn deployment_targets_from_manifest(
manifest: &spin_manifest::schema::v2::AppManifest,
) -> Vec<spin_manifest::schema::v2::TargetEnvironmentRef> {
manifest.application.targets.clone()
}

async fn fallback_load_build_configs(
manifest_file: impl AsRef<Path>,
) -> Result<Vec<ComponentBuildInfo>> {
Expand All @@ -57,7 +140,23 @@ async fn fallback_load_build_configs(
})
}

#[derive(Deserialize)]
async fn has_deployment_targets(manifest_file: impl AsRef<Path>) -> Result<bool> {
let manifest_text = tokio::fs::read_to_string(manifest_file).await?;
Ok(match ManifestVersion::detect(&manifest_text)? {
ManifestVersion::V1 => false,
ManifestVersion::V2 => {
let table: toml::value::Table = toml::from_str(&manifest_text)?;
table
.get("application")
.and_then(|a| a.as_table())
.and_then(|t| t.get("targets"))
.and_then(|arr| arr.as_array())
.is_some_and(|arr| !arr.is_empty())
}
})
}

#[derive(Clone, Deserialize)]
pub struct ComponentBuildInfo {
#[serde(default)]
pub id: String,
Expand Down
Loading
Loading