feat(finalize): support multiple manifest extensions and add hook tests

- Remove underscore suffix from FINALIZE_SHELL_ENVPREFIX constant
- Split FINALIZE_PLUGIN_MANIFEST into name + extensions for flexible
  manifest discovery
- Add ManifestNotFound error variant for clearer plugin loading errors
- Add debug logging for plugin download operations
- Add integration tests for finalize hook execution
- Remove obsolete clarification.md
This commit is contained in:
Catty Steve
2026-04-21 21:40:19 +08:00
parent 0dd4a157ef
commit 7b3b71a4b3
12 changed files with 498 additions and 1234 deletions
-1211
View File
File diff suppressed because it is too large Load Diff
+3 -2
View File
@@ -3,6 +3,7 @@ pub const BOOTSTRAP_SCRIPT_PATH: &str = "/bootstrap.sh";
pub const BAKE_SCRIPT_PATH: &str = "bake.sh";
pub const PIPELINE_SKIP_ERRORCODE: u8 = 233;
pub const TRIVIAL_SCRIPT_NAME: &str = "BAKE_TRIVIAL";
pub const FINALIZE_PLUGIN_MANIFEST: &str = "manifest.yml";
pub const FINALIZE_SHELL_ENVPREFIX: &str = "HBW_ARG_";
pub const FINALIZE_PLUGIN_MANIFEST: &str = "manifest";
pub const FINALIZE_PLUGIN_MANIFEST_EXTENSION: &[&str] = &[".yml", ".yaml"];
pub const FINALIZE_SHELL_ENVPREFIX: &str = "HBW_ARG";
pub const FINALIZE_STANDALONE_PLUGIN_PATH: &str = "/tmp/baker/plugin";
+47 -3
View File
@@ -4,6 +4,7 @@ use std::process::Stdio;
use tokio::io::AsyncReadExt;
use tokio::process::Command;
use tokio::sync::mpsc;
use tokio::time::timeout;
use super::pm::PackageManager;
use super::types::{
@@ -66,6 +67,10 @@ impl super::types::Engine {
ctx.working_dir.join(script_path)
};
if !abs_path.exists() {
return Err(ExecutionError::ScriptNotFound(abs_path))
}
let mut command = Command::new(ctx.shell.as_str());
command.arg("-c");
command.arg(abs_path);
@@ -210,9 +215,48 @@ impl super::types::Engine {
}
});
let status = child.wait().await.map_err(|e| {
ExecutionError::IoError(format!("Failed to wait for process: {}", e))
})?;
let status = if let Some(timeout_duration) = ctx.timeout {
match timeout(timeout_duration, child.wait()).await {
Ok(Ok(status)) => status,
Ok(Err(e)) => {
return Err(ExecutionError::IoError(format!(
"Failed to wait for process: {}",
e
)))
}
Err(_) => {
log::warn!(
"Task {} timed out after {:?}, killing process",
ctx.task_id,
timeout_duration
);
if let Some(tx) = event_tx {
let _ = tx
.send(ExecutionEvent::TaskFailed {
task_id: ctx.task_id.clone(),
error: "Timeout".to_string(),
})
.await;
}
let _ = child.kill().await;
drop(stdout_tx);
drop(stderr_tx);
let _ = stdout_handle.await;
let _ = stderr_handle.await;
while let Some(data) = stdout_rx.recv().await {
stdout_buf.push_str(&data);
}
while let Some(data) = stderr_rx.recv().await {
stderr_buf.push_str(&data);
}
return Err(ExecutionError::Timeout);
}
}
} else {
child.wait().await.map_err(|e| {
ExecutionError::IoError(format!("Failed to wait for process: {}", e))
})?
};
drop(stdout_tx);
drop(stderr_tx);
+9 -4
View File
@@ -37,11 +37,13 @@ pub async fn finalize(
};
if ctx.standalone {
// In standalone mode, finalize should download plugin by itself
log::debug!("Downloading {} plugins", finalize.plugin.len());
let plugin_path = PathBuf::from(FINALIZE_STANDALONE_PLUGIN_PATH);
if plugin_path.exists() {
std::fs::remove_dir_all(&plugin_path).map_err(FinalizeError::IoError)?;
}
// if plugin_path.exists() {
// std::fs::remove_dir_all(&plugin_path).map_err(FinalizeError::IoError)?;
// }
for (name, data) in finalize.plugin.iter_mut() {
log::info!("Downloading plugin {}", name);
let (source, config) = data.iter_mut().next().unwrap_or_else(|| {
panic!(
"Internal Error: Plugin {} has no source configured, this should have been caught by validation!",
@@ -58,6 +60,7 @@ pub async fn finalize(
}
// Register Plugins
log::info!("Registering {} plugins", finalize.plugin.len());
let registered_plugins = plugin::register(finalize.plugin)?;
// Early hook
@@ -81,5 +84,7 @@ pub async fn finalize(
)
.await?;
todo!();
// todo!();
// For testing purpose only
Ok(())
}
+72
View File
@@ -300,3 +300,75 @@ impl FinalizeConfig {
}
}
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn test_deserialize_hooks_full() {
let yaml = r#"
version: "0.0.1"
hooks:
early:
- name: "test-early"
command: "echo early"
timeout: 10
late:
- name: "test-late"
command: "plugin:test-plugin"
working_dir: "/tmp"
"#;
let config: FinalizeConfig = serde_yaml::from_str(yaml).unwrap();
assert!(config.hooks.is_some());
let hooks = config.hooks.unwrap();
let early = hooks.early.as_ref().unwrap();
let late = hooks.late.as_ref().unwrap();
assert_eq!(early.len(), 1);
assert_eq!(late.len(), 1);
assert_eq!(early[0].command, "echo early");
assert_eq!(late[0].command, "plugin:test-plugin");
assert_eq!(late[0].working_dir, Some("/tmp".to_string()));
}
#[test]
fn test_deserialize_hooks_missing() {
let yaml = r#"
version: "0.0.1"
"#;
let config: FinalizeConfig = serde_yaml::from_str(yaml).unwrap();
assert!(config.hooks.is_none());
}
#[test]
fn test_deserialize_hooks_timeout_float() {
let yaml = r#"
version: "0.0.1"
hooks:
early:
- name: "float-timeout"
command: "echo ok"
timeout: 10.5
"#;
let config: FinalizeConfig = serde_yaml::from_str(yaml).unwrap();
let hooks = config.hooks.unwrap();
let early = hooks.early.unwrap();
assert_eq!(early[0].timeout_ms, 10500);
}
#[test]
fn test_deserialize_hooks_timeout_string() {
let yaml = r#"
version: "0.0.1"
hooks:
early:
- name: "string-timeout"
command: "echo ok"
timeout: "30s"
"#;
let config: FinalizeConfig = serde_yaml::from_str(yaml).unwrap();
let hooks = config.hooks.unwrap();
let early = hooks.early.unwrap();
assert_eq!(early[0].timeout_ms, 30000);
}
}
+4 -1
View File
@@ -73,5 +73,8 @@ pub enum PluginError {
name: String,
url: String,
reason: String,
}
},
#[error("Plugin manifest {0}/manifest.yml not found.")]
ManifestNotFound(PathBuf),
}
+32 -8
View File
@@ -1,10 +1,10 @@
use crate::constant::FINALIZE_PLUGIN_MANIFEST;
use crate::constant::{FINALIZE_PLUGIN_MANIFEST, FINALIZE_PLUGIN_MANIFEST_EXTENSION};
use crate::finalize::RawPluginMap;
use crate::finalize::plugin::metadata::{PluginMetadata, PluginType};
use crate::{EventSender, ExecutionContext, finalize::error::PluginError};
use async_trait::async_trait;
use std::collections::HashMap;
use std::path::PathBuf;
use std::path::{PathBuf, Path};
mod dylib;
mod internal;
@@ -91,13 +91,20 @@ fn read_manifest(path: Option<PathBuf>, name: &str) -> Result<PluginMetadata, Pl
name: name.to_string(),
reason: "Plugin not present in filesystem.".to_string(),
})?;
let manifest_content =
std::fs::read_to_string(path.join(FINALIZE_PLUGIN_MANIFEST)).map_err(|e| {
PluginError::PluginUnavailable {
name: name.to_string(),
reason: format!("Failed to read plugin manifest: {}", e),
// Extract content from manifest.yml or manifest.yaml
let manifest_content = 'load: {
for ext in FINALIZE_PLUGIN_MANIFEST_EXTENSION {
match load_manifest(&path, name, ext) {
Ok(content) => break 'load content,
Err(PluginError::ManifestNotFound(_)) => {},
Err(e) => return Err(e),
}
})?;
}
return Err(PluginError::ManifestNotFound(path));
};
log::debug!("[plugin:{}] Parsing manifest content", name);
let mut metadata: PluginMetadata = serde_yaml::from_str(manifest_content.as_str()).map_err(|e| PluginError::PluginUnavailable {
name: name.to_string(),
reason: format!("Failed to parse plugin manifest: {}", e),
@@ -106,6 +113,21 @@ fn read_manifest(path: Option<PathBuf>, name: &str) -> Result<PluginMetadata, Pl
Ok(metadata)
}
fn load_manifest(path: &Path, name: &str, suffix: &str) -> Result<String, PluginError> {
let manifest_path = path.join(format!("{}{}", FINALIZE_PLUGIN_MANIFEST,suffix));
log::debug!("[plugin:{}] Reading manifest from {}", name, manifest_path.display());
if !manifest_path.exists() {
return Err(PluginError::ManifestNotFound(manifest_path));
}
std::fs::read_to_string(&manifest_path).map_err(|e| {
PluginError::PluginUnavailable {
name: name.to_string(),
reason: format!("Failed to read plugin manifest from {}: {}", manifest_path.display(), e.to_string()),
}
})
}
/// Determines the plugin type based on the manifest type or file extension.
///
/// The function first returns `original` if it's not [`PluginType::Unknown`].
@@ -183,11 +205,13 @@ pub fn register(external_plugins: RawPluginMap) -> Result<PluginMap, PluginError
// Register internal plugin
for i in internal::get_internal_plugin() {
log::info!("Registering internal plugin {}", i.name);
plugins.insert(i.name.to_string(), FinalizePlugin::new(i.entry));
}
// Register external plugin
for (name, data) in external_plugins.into_iter() {
log::info!("Registering external plugin {}", name);
let (_source, config) = data.into_iter().next().unwrap_or_else(|| {
panic!(
"Internal Error: Plugin {} has no source configured, this should have been caught by validation!",
@@ -38,6 +38,7 @@ pub async fn hook(
let plugin_name = hook_cmd.command.strip_prefix("plugin:").expect(
"Internal error: command starts with \"plugin:\" but failed to strip prefix",
);
log::info!("Calling plugin hook {}: {}", index, plugin_name);
call_named_plugin(
plugins,
plugin_name,
+3 -2
View File
@@ -1,6 +1,6 @@
use crate::types::time::deserialize_duration_ms;
use serde::{Deserialize, Serialize};
use std::collections::HashMap;
use crate::types::time::deserialize_duration_ms;
#[derive(Debug, Deserialize, Serialize, Clone)]
pub struct CustomCommand {
@@ -13,7 +13,8 @@ pub struct CustomCommand {
pub working_dir: Option<String>,
#[serde(
default = "default_timeout_ms",
deserialize_with = "deserialize_duration_ms"
deserialize_with = "deserialize_duration_ms",
rename = "timeout"
)]
pub timeout_ms: u64,
#[serde(default)]
+5 -3
View File
@@ -27,6 +27,7 @@ fn create_context() -> ExecutionContext {
shell: "bash".to_string(),
dry_run: false,
privileged: false,
standalone: false,
}
}
@@ -145,8 +146,8 @@ async fn test_execute_sets_ws_taskid() {
let result = executor.execute(cmd, &ctx, &None).await.unwrap();
assert!(result.stdout.contains("WS_TASKID=task-abc-123"));
assert!(result.stdout.contains("WS_PIPELINE=pipeline-xyz"));
assert!(result.stdout.contains("HBW_TASKID=task-abc-123"));
assert!(result.stdout.contains("HBW_PIPELINE=pipeline-xyz"));
}
#[tokio::test]
@@ -159,7 +160,7 @@ async fn test_execute_sets_ws_user_info() {
let result = executor.execute(cmd, &ctx, &None).await.unwrap();
assert!(result.stdout.contains("WS_USERNAME=testuser"));
assert!(result.stdout.contains("HBW_USERNAME=testuser"));
}
#[tokio::test]
@@ -174,6 +175,7 @@ async fn test_execute_respects_working_directory() {
timeout: Some(Duration::from_secs(30)),
cgroup_path: None,
shell: "bash".to_string(),
standalone: false,
..Default::default()
};
@@ -0,0 +1,179 @@
//! Integration tests for finalize hook execution
//!
//! These tests verify that shell hooks and plugin hooks execute correctly
//! through the finalize stage hook executor.
use std::collections::HashMap;
use std::time::Duration;
use workshop_baker::engine::{ExecutionContext, ExecutionEvent};
use workshop_baker::finalize::stage::hook::hook;
use workshop_baker::finalize::plugin::{PluginMap};
use workshop_baker::types::command::CustomCommand;
fn create_test_context() -> ExecutionContext {
ExecutionContext {
task_id: "test-hook-1".to_string(),
pipeline_name: "test-pipeline".to_string(),
username: "testuser".to_string(),
working_dir: std::env::temp_dir(),
env_vars: HashMap::new(),
timeout: Some(Duration::from_secs(30)),
cgroup_path: None,
shell: "bash".to_string(),
dry_run: false,
privileged: false,
standalone: false,
}
}
fn create_test_event_channel() -> (Option<tokio::sync::mpsc::Sender<ExecutionEvent>>, tokio::sync::mpsc::Receiver<ExecutionEvent>) {
let (tx, rx) = tokio::sync::mpsc::channel(64);
(Some(tx), rx)
}
#[tokio::test]
async fn test_shell_hook_success() {
let ctx = create_test_context();
let (event_tx, _event_rx) = create_test_event_channel();
let plugins = PluginMap::new();
let cmd = CustomCommand {
name: "echo-test".to_string(),
command: "echo 'hook-ok'".to_string(),
environment: None,
working_dir: None,
timeout_ms: 5000,
argument: serde_yaml::Value::Null,
};
let result = hook(Some(&vec![cmd]), &ctx, event_tx, &plugins).await;
assert!(result.is_ok(), "Shell hook should succeed: {:?}", result);
}
#[tokio::test]
async fn test_shell_hook_failure_propagates() {
let ctx = create_test_context();
let (event_tx, _event_rx) = create_test_event_channel();
let plugins = PluginMap::new();
let cmd = CustomCommand {
name: "false-cmd".to_string(),
command: "false".to_string(),
environment: None,
working_dir: None,
timeout_ms: 5000,
argument: serde_yaml::Value::Null,
};
let result = hook(Some(&vec![cmd]), &ctx, event_tx, &plugins).await;
assert!(result.is_err(), "Shell hook failure should propagate");
}
#[tokio::test]
async fn test_hook_timeout_kills_long_command() {
let ctx = create_test_context();
let (event_tx, _event_rx) = create_test_event_channel();
let plugins = PluginMap::new();
let cmd = CustomCommand {
name: "sleep-forever".to_string(),
command: "sleep 10".to_string(),
environment: None,
working_dir: None,
timeout_ms: 500, // 0.5 seconds
argument: serde_yaml::Value::Null,
};
let result = hook(Some(&vec![cmd]), &ctx, event_tx, &plugins).await;
assert!(result.is_err(), "Hook should fail due to timeout");
}
#[tokio::test]
async fn test_hook_working_dir_applied() {
let ctx = create_test_context();
let (event_tx, _event_rx) = create_test_event_channel();
let plugins = PluginMap::new();
let cmd = CustomCommand {
name: "pwd-check".to_string(),
command: "pwd".to_string(),
environment: None,
working_dir: Some("/tmp".to_string()),
timeout_ms: 5000,
argument: serde_yaml::Value::Null,
};
// We verify by checking the hook returns Ok — the working_dir is passed to
// Engine::execute_command_with_delta. Full stdout verification would require
// mocking the Engine or using a more intrusive test setup.
let result = hook(Some(&vec![cmd]), &ctx, event_tx, &plugins).await;
assert!(result.is_ok(), "Hook with working_dir should succeed");
}
#[tokio::test]
async fn test_no_hooks_returns_ok() {
let ctx = create_test_context();
let (event_tx, _event_rx) = create_test_event_channel();
let plugins = PluginMap::new();
let result = hook(None, &ctx, event_tx, &plugins).await;
assert!(result.is_ok(), "No hooks should return Ok immediately");
}
#[tokio::test]
async fn test_multiple_hooks_execute_sequentially() {
let ctx = create_test_context();
let (event_tx, _event_rx) = create_test_event_channel();
let plugins = PluginMap::new();
let cmds = vec![
CustomCommand {
name: "hook-1".to_string(),
command: "true".to_string(),
environment: None,
working_dir: None,
timeout_ms: 5000,
argument: serde_yaml::Value::Null,
},
CustomCommand {
name: "hook-2".to_string(),
command: "true".to_string(),
environment: None,
working_dir: None,
timeout_ms: 5000,
argument: serde_yaml::Value::Null,
},
];
let result = hook(Some(&cmds), &ctx, event_tx, &plugins).await;
assert!(result.is_ok(), "Multiple hooks should execute sequentially and succeed");
}
#[tokio::test]
async fn test_first_failure_stops_remaining_hooks() {
let ctx = create_test_context();
let (event_tx, _event_rx) = create_test_event_channel();
let plugins = PluginMap::new();
let cmds = vec![
CustomCommand {
name: "failing-hook".to_string(),
command: "false".to_string(),
environment: None,
working_dir: None,
timeout_ms: 5000,
argument: serde_yaml::Value::Null,
},
CustomCommand {
name: "never-reached".to_string(),
command: "true".to_string(),
environment: None,
working_dir: None,
timeout_ms: 5000,
argument: serde_yaml::Value::Null,
},
];
let result = hook(Some(&cmds), &ctx, event_tx, &plugins).await;
assert!(result.is_err(), "First failing hook should stop execution");
}
@@ -0,0 +1,143 @@
//! Integration tests for the full finalize pipeline
//!
//! These tests verify the complete finalize workflow from YAML parsing
//! through hook execution, using real files and the public `finalize()` entry point.
use std::collections::HashMap;
use std::time::Duration;
use workshop_baker::engine::{ExecutionContext, ExecutionEvent};
use workshop_baker::finalize::finalize;
use workshop_baker::cli::Cli;
fn create_test_context() -> ExecutionContext {
ExecutionContext {
task_id: "test-finalize-1".to_string(),
pipeline_name: "test-pipeline".to_string(),
username: "testuser".to_string(),
working_dir: std::env::temp_dir(),
env_vars: HashMap::new(),
timeout: Some(Duration::from_secs(30)),
cgroup_path: None,
shell: "bash".to_string(),
dry_run: false,
privileged: false,
standalone: false,
}
}
fn create_test_cli() -> Cli {
Cli {
username: "testuser".to_string(),
pipeline: "test-pipeline".to_string(),
build_id: "test-build-1".to_string(),
dry_run: false,
command: None,
}
}
fn create_test_event_channel() -> (Option<tokio::sync::mpsc::Sender<ExecutionEvent>>, tokio::sync::mpsc::Receiver<ExecutionEvent>) {
let (tx, rx) = tokio::sync::mpsc::channel(64);
(Some(tx), rx)
}
#[tokio::test]
async fn test_full_finalize_with_early_late_hooks() {
let temp_dir = std::env::temp_dir();
let finalize_path = temp_dir.join("test_finalize.yml");
let yaml_content = r#"
version: "0.0.1"
hooks:
early:
- name: "early-echo"
command: "echo early-hook-ok"
timeout: 5
late:
- name: "late-echo"
command: "echo late-hook-ok"
timeout: 5
"#;
std::fs::write(&finalize_path, yaml_content).expect("Failed to write test finalize.yml");
let cli = create_test_cli();
let mut ctx = create_test_context();
let (event_tx, mut event_rx) = create_test_event_channel();
// Spawn an event consumer so the channel does not block
let consumer_handle = tokio::spawn(async move {
while let Some(_event) = event_rx.recv().await {
// Events are consumed but not validated in this test
}
});
let result = finalize(&finalize_path, &cli, &mut ctx, event_tx).await;
let _ = consumer_handle.await;
std::fs::remove_file(&finalize_path).ok();
assert!(result.is_ok(), "Full finalize pipeline should succeed: {:?}", result);
}
#[tokio::test]
async fn test_full_finalize_with_empty_hooks() {
let temp_dir = std::env::temp_dir();
let finalize_path = temp_dir.join("test_finalize_empty.yml");
let yaml_content = r#"
version: "0.0.1"
"#;
std::fs::write(&finalize_path, yaml_content).expect("Failed to write test finalize.yml");
let cli = create_test_cli();
let mut ctx = create_test_context();
let (event_tx, mut event_rx) = create_test_event_channel();
let consumer_handle = tokio::spawn(async move {
while let Some(_event) = event_rx.recv().await {}
});
let result = finalize(&finalize_path, &cli, &mut ctx, event_tx).await;
let _ = consumer_handle.await;
std::fs::remove_file(&finalize_path).ok();
assert!(result.is_ok(), "Finalize with empty hooks should succeed: {:?}", result);
}
#[tokio::test]
async fn test_full_finalize_with_failing_early_hook() {
let temp_dir = std::env::temp_dir();
let finalize_path = temp_dir.join("test_finalize_fail.yml");
let yaml_content = r#"
version: "0.0.1"
hooks:
early:
- name: "failing-hook"
command: "false"
timeout: 5
late:
- name: "never-reached"
command: "echo should-not-run"
timeout: 5
"#;
std::fs::write(&finalize_path, yaml_content).expect("Failed to write test finalize.yml");
let cli = create_test_cli();
let mut ctx = create_test_context();
let (event_tx, mut event_rx) = create_test_event_channel();
let consumer_handle = tokio::spawn(async move {
while let Some(_event) = event_rx.recv().await {}
});
let result = finalize(&finalize_path, &cli, &mut ctx, event_tx).await;
let _ = consumer_handle.await;
std::fs::remove_file(&finalize_path).ok();
assert!(result.is_err(), "Finalize with failing early hook should return an error");
}