diff --git a/crates/trigger/src/locked.rs b/crates/trigger/src/locked.rs index 2a46874e3..1e10abe46 100644 --- a/crates/trigger/src/locked.rs +++ b/crates/trigger/src/locked.rs @@ -193,13 +193,8 @@ fn content_ref_path(path: &Path) -> Result { } fn file_uri(path: &Path) -> Result { - let path = path.canonicalize()?; - let url = if path.is_dir() { - url::Url::from_directory_path(&path) - } else { - url::Url::from_file_path(&path) - } - .map_err(|_| anyhow!("Could not construct file URL for {path:?}"))?; + let url = url::Url::from_file_path(path) + .map_err(|_| anyhow!("Could not construct file URL for {path:?}"))?; Ok(url.to_string()) } @@ -258,10 +253,13 @@ mod tests { assert_eq!(locked.triggers[0].trigger_config["route"], "/"); let component = &locked.components[0]; + let source = component.source.content.source.as_deref().unwrap(); assert!(source.ends_with("test-source.wasm")); + let mount = component.files[0].content.source.as_deref().unwrap(); - assert!(mount.ends_with('/')); + let mount_path = url::Url::try_from(mount).unwrap().to_file_path().unwrap(); + assert!(mount_path.is_dir(), "{mount:?} is not a dir"); } #[tokio::test] diff --git a/src/commands/up.rs b/src/commands/up.rs index d15e9e543..400a8fc79 100644 --- a/src/commands/up.rs +++ b/src/commands/up.rs @@ -9,7 +9,7 @@ use clap::{CommandFactory, Parser}; use reqwest::Url; use spin_app::locked::LockedApp; use spin_loader::bindle::{deprecation::print_bindle_deprecation, BindleConnectionInfo}; -use spin_manifest::{Application, ApplicationOrigin, ApplicationTrigger}; +use spin_manifest::ApplicationTrigger; use spin_oci::OciLoader; use spin_trigger::cli::{SPIN_LOCKED_URL, SPIN_STATE_DIR, SPIN_WORKING_DIR}; use tempfile::TempDir; @@ -51,7 +51,7 @@ pub struct UpCommand { )] pub file_source: Option, - /// The application to run. This interprets the applicaiton as a bindle ID. + /// The application to run. This interprets the application as a bindle ID. /// This option is deprecated; use OCI registries and `--from` where possible. #[clap( hide = true, @@ -155,6 +155,101 @@ impl UpCommand { }) } + async fn run_inner(self) -> Result<()> { + let app_source = self.resolve_app_source(); + + if app_source == AppSource::None { + return self + .run_trigger(trigger_command(HELP_ARGS_ONLY_TRIGGER_TYPE), None) + .await; + } + + let working_dir_holder = match &self.tmp { + None => WorkingDirectory::Temporary(tempfile::tempdir()?), + Some(d) => WorkingDirectory::Given(d.to_owned()), + }; + let working_dir = working_dir_holder.path().canonicalize()?; + + let mut locked_app = match &app_source { + AppSource::None => bail!("Internal error - should have shown help"), + AppSource::File(path) => self.prepare_app_from_file(path, &working_dir).await?, + AppSource::Bindle(id) => self.prepare_app_from_bindle(id, &working_dir).await?, + AppSource::OciRegistry(oci) => self.prepare_app_from_oci(oci, &working_dir).await?, + AppSource::Unresolvable(err) => bail!("{err}"), + }; + + let trigger_cmd = trigger_command_from_locked_app(&locked_app)?; + + if self.help { + return self.run_trigger(trigger_cmd, None).await; + } + + self.update_locked_app(&mut locked_app); + + let state_dir = self.prepare_state_dir(&app_source); + + let run_opts = RunTriggerOpts { + locked_app, + working_dir, + state_dir, + }; + + self.run_trigger(trigger_cmd, Some(run_opts)).await + } + + async fn run_trigger( + self, + trigger_cmd: Vec, + opts: Option, + ) -> Result<(), anyhow::Error> { + // The docs for `current_exe` warn that this may be insecure because it could be executed + // via hard-link. I think it should be fine as long as we aren't `setuid`ing this binary. + let mut cmd = std::process::Command::new(std::env::current_exe().unwrap()); + cmd.args(&trigger_cmd); + + if let Some(RunTriggerOpts { + locked_app, + working_dir, + state_dir, + }) = opts + { + let locked_url = self.write_locked_app(&locked_app, &working_dir).await?; + + cmd.env(SPIN_LOCKED_URL, locked_url) + .env(SPIN_WORKING_DIR, &working_dir) + .args(&self.trigger_args); + + if let Some(state_dir) = state_dir { + cmd.env(SPIN_STATE_DIR, state_dir); + } + } else { + cmd.arg("--help-args-only"); + } + + tracing::trace!("Running trigger executor: {:?}", cmd); + + let mut child = cmd.spawn().context("Failed to execute trigger")?; + + // Terminate trigger executor if `spin up` itself receives a termination signal + #[cfg(not(windows))] + { + // https://github.com/nix-rust/nix/issues/656 + let pid = nix::unistd::Pid::from_raw(child.id() as i32); + ctrlc::set_handler(move || { + if let Err(err) = nix::sys::signal::kill(pid, nix::sys::signal::SIGTERM) { + tracing::warn!("Failed to kill trigger handler process: {:?}", err) + } + })?; + } + + let status = child.wait()?; + if status.success() { + Ok(()) + } else { + bail!(status); + } + } + fn resolve_app_source(&self) -> AppSource { match ( &self.app_source, @@ -167,7 +262,7 @@ impl UpCommand { (None, Some(file), None, None) => Self::infer_file_source(file.to_owned()), (None, None, Some(id), None) => AppSource::Bindle(id.to_owned()), (None, None, None, Some(reference)) => AppSource::OciRegistry(reference.to_owned()), - _ => AppSource::unresolvable("More than one application was specified"), + _ => AppSource::unresolvable("More than one application source was specified"), } } @@ -186,14 +281,19 @@ impl UpCommand { } } - fn trigger_args_look_file_like(&self) -> bool { - // Heuristic for the user typing `spin up foo` instead of `spin up -f foo` - in the - // first case `foo` gets interpreted as a trigger arg which is probably not what the - // user intended. - !self.trigger_args.is_empty() && !self.trigger_args[0].to_string_lossy().starts_with('-') + fn infer_source(source: &str) -> AppSource { + let path = PathBuf::from(source); + if path.exists() { + Self::infer_file_source(path) + } else if spin_oci::is_probably_oci_reference(source) { + AppSource::OciRegistry(source.to_owned()) + } else { + AppSource::Unresolvable(format!("File or directory '{source}' not found. If you meant to load from a registry, use the `--from-registry` option.")) + } } - fn infer_file_source(path: PathBuf) -> AppSource { + fn infer_file_source(path: impl Into) -> AppSource { + let path = path.into(); if path.is_file() { AppSource::File(path) } else if path.is_dir() { @@ -214,110 +314,11 @@ impl UpCommand { } } - fn infer_source(source: &str) -> AppSource { - let path = PathBuf::from(source); - if path.exists() { - Self::infer_file_source(path) - } else if spin_oci::is_probably_oci_reference(source) { - AppSource::OciRegistry(source.to_owned()) - } else { - AppSource::Unresolvable(format!("File or directory '{source}' not found. If you meant to load from a registry, use the `--from-registry` option.")) - } - } - - async fn run_inner(self) -> Result<()> { - let app_source = self.resolve_app_source(); - - if app_source == AppSource::None { - return self - .run_trigger( - trigger_command(HELP_ARGS_ONLY_TRIGGER_TYPE), - TriggerExecOpts::NoApp, - ) - .await; - } - - if matches!(app_source, AppSource::Bindle(_)) { - print_bindle_deprecation(); - } - - let working_dir_holder = match &self.tmp { - None => WorkingDirectory::Temporary(tempfile::tempdir()?), - Some(d) => WorkingDirectory::Given(d.to_owned()), - }; - let working_dir = working_dir_holder.path().canonicalize()?; - - let (trigger_cmd, exec_opts) = match app_source { - AppSource::None => bail!("Internal error - should have shown help"), - AppSource::File(app) => self.prepare_app_from_file(&app, working_dir).await?, - AppSource::Bindle(bindle) => self.prepare_app_from_bindle(&bindle, working_dir).await?, - AppSource::OciRegistry(oci) => self.prepare_app_from_oci(&oci, working_dir).await?, - AppSource::Unresolvable(err) => bail!(err), - }; - - self.run_trigger(trigger_cmd, exec_opts).await - } - - async fn run_trigger( - self, - trigger_type: Vec, - exec_opts: TriggerExecOpts, - ) -> Result<(), anyhow::Error> { - // The docs for `current_exe` warn that this may be insecure because it could be executed - // via hard-link. I think it should be fine as long as we aren't `setuid`ing this binary. - let mut cmd = std::process::Command::new(std::env::current_exe().unwrap()); - cmd.args(&trigger_type); - - match exec_opts { - TriggerExecOpts::NoApp => { - cmd.arg("--help-args-only"); - } - TriggerExecOpts::Local { app, working_dir } => { - let locked_app = spin_trigger::locked::build_locked_app(app.clone(), &working_dir)?; - - let state_dir = match app.info.origin { - ApplicationOrigin::File(f) => f.parent().unwrap().join(".spin"), - _ => panic!("Expected application origin file for local app"), - }; - - let locked_url = self.write_locked_app(&locked_app, &working_dir).await?; - cmd.env(SPIN_LOCKED_URL, locked_url) - .env(SPIN_WORKING_DIR, &working_dir) - .env(SPIN_STATE_DIR, state_dir) - .args(&self.trigger_args); - } - TriggerExecOpts::Remote { - locked_url, - working_dir, - } => { - cmd.env(SPIN_LOCKED_URL, locked_url) - .env(SPIN_WORKING_DIR, &working_dir) - .args(&self.trigger_args); - } - } - - tracing::trace!("Running trigger executor: {:?}", cmd); - - let mut child = cmd.spawn().context("Failed to execute trigger")?; - - // Terminate trigger executor if `spin up` itself receives a termination signal - #[cfg(not(windows))] - { - // https://github.com/nix-rust/nix/issues/656 - let pid = nix::unistd::Pid::from_raw(child.id() as i32); - ctrlc::set_handler(move || { - if let Err(err) = nix::sys::signal::kill(pid, nix::sys::signal::SIGTERM) { - tracing::warn!("Failed to kill trigger handler process: {:?}", err) - } - })?; - } - - let status = child.wait()?; - if status.success() { - Ok(()) - } else { - bail!(status); - } + fn trigger_args_look_file_like(&self) -> bool { + // Heuristic for the user typing `spin up foo` instead of `spin up -f foo` - in the + // first case `foo` gets interpreted as a trigger arg which is probably not what the + // user intended. + !self.trigger_args.is_empty() && !self.trigger_args[0].to_string_lossy().starts_with('-') } async fn write_locked_app( @@ -349,101 +350,74 @@ impl UpCommand { }) } - // Prepares the application for trigger execution returning the trigger command - // to execute and the URL of the locked application. - async fn prepare_locked_app( - &self, - mut locked_app: LockedApp, - working_dir: PathBuf, - ) -> Result<(Vec, TriggerExecOpts)> { - // Apply --env to component environments - if !self.env.is_empty() { - for component in locked_app.components.iter_mut() { - component.env.extend(self.env.iter().cloned()); - } - } - - let trigger_command = trigger_command_from_locked_app(&locked_app)?; - let locked_url = self.write_locked_app(&locked_app, &working_dir).await?; - - let exec_opts = if self.help { - TriggerExecOpts::NoApp - } else { - TriggerExecOpts::Remote { - locked_url, - working_dir, - } - }; - - Ok((trigger_command, exec_opts)) - } - async fn prepare_app_from_file( &self, - manifest_file: &Path, - working_dir: PathBuf, - ) -> Result<(Vec, TriggerExecOpts)> { + manifest_path: &Path, + working_dir: &Path, + ) -> Result { let bindle_connection = self.bindle_connection(); let asset_dst = if self.direct_mounts { None } else { - Some(&working_dir) + Some(working_dir) }; - let mut app = spin_loader::from_file(manifest_file, asset_dst, &bindle_connection).await?; + let app = spin_loader::from_file(manifest_path, asset_dst, &bindle_connection).await?; - // Apply --env to component environments - if !self.env.is_empty() { - for component in app.components.iter_mut() { - component.wasm.environment.extend(self.env.iter().cloned()); - } - } - - let command = trigger_command_from_app(&app)?; - - let exec_opts = if self.help { - TriggerExecOpts::NoApp - } else { - TriggerExecOpts::Local { app, working_dir } - }; - - Ok((command, exec_opts)) + spin_trigger::locked::build_locked_app(app, working_dir) } - async fn prepare_app_from_oci( - &self, - reference: &str, - working_dir: PathBuf, - ) -> Result<(Vec, TriggerExecOpts)> { + async fn prepare_app_from_oci(&self, reference: &str, working_dir: &Path) -> Result { let mut client = spin_oci::Client::new(self.insecure, None) .await .context("cannot create registry client")?; - let locked_app = OciLoader::new(&working_dir) + OciLoader::new(working_dir) .load_app(&mut client, reference) - .await?; - self.prepare_locked_app(locked_app, working_dir).await + .await } async fn prepare_app_from_bindle( &self, bindle_id: &str, - working_dir: PathBuf, - ) -> Result<(Vec, TriggerExecOpts)> { - let app = match &self.server { - Some(server) => { - assert!(!self.direct_mounts); - spin_loader::from_bindle(bindle_id, server, &working_dir).await? - } - None => bail!("Loading from a bindle requires a Bindle server URL"), + working_dir: &Path, + ) -> Result { + print_bindle_deprecation(); + assert!(!self.direct_mounts); + + let Some(server) = &self.server else { + bail!("Loading from a bindle requires a Bindle server URL"); }; - let locked_app = spin_trigger::locked::build_locked_app(app, &working_dir)?; - self.prepare_locked_app(locked_app, working_dir).await + let app = spin_loader::from_bindle(bindle_id, server, working_dir).await?; + + spin_trigger::locked::build_locked_app(app, working_dir) + } + + fn prepare_state_dir(&self, source: &AppSource) -> Option { + match source { + AppSource::File(path) => Some(path.parent().unwrap().join(".spin")), + _ => None, + } + } + + fn update_locked_app(&self, locked_app: &mut LockedApp) { + // Apply --env to component environments + if !self.env.is_empty() { + for component in locked_app.components.iter_mut() { + component.env.extend(self.env.iter().cloned()); + } + } } } +struct RunTriggerOpts { + locked_app: LockedApp, + working_dir: PathBuf, + state_dir: Option, +} + enum WorkingDirectory { Given(PathBuf), Temporary(TempDir), @@ -500,33 +474,10 @@ fn resolve_trigger_plugin(trigger_type: &str) -> Result { } } -#[allow(clippy::large_enum_variant)] // The large variant is the common case and really this is equivalent to an Option -enum TriggerExecOpts { - NoApp, - Local { - app: Application, - working_dir: PathBuf, - }, - Remote { - locked_url: String, - working_dir: PathBuf, - }, -} - fn trigger_command(trigger_type: &str) -> Vec { vec!["trigger".to_owned(), trigger_type.to_owned()] } -fn trigger_command_from_app(app: &Application) -> Result> { - match &app.info.trigger { - ApplicationTrigger::Http(_) => Ok(trigger_command("http")), - ApplicationTrigger::Redis(_) => Ok(trigger_command("redis")), - ApplicationTrigger::External(cfg) => { - resolve_trigger_plugin(cfg.trigger_type()).map(|p| vec![p]) - } - } -} - fn trigger_command_from_locked_app(locked_app: &LockedApp) -> Result> { let trigger_metadata = locked_app .metadata @@ -556,8 +507,8 @@ enum AppSource { } impl AppSource { - fn unresolvable(message: impl AsRef) -> Self { - Self::Unresolvable(message.as_ref().to_owned()) + fn unresolvable(message: impl Into) -> Self { + Self::Unresolvable(message.into()) } } diff --git a/tests/integration.rs b/tests/integration.rs index 4ae5daac1..d9d13ee35 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -845,6 +845,8 @@ mod integration_tests { .collect::>() .join(" "), ); + + cmd.env("RUST_LOG", "spin_cli=warn"); if let Some(envs) = envs { for (k, v) in envs { cmd.env(k, v); @@ -852,10 +854,11 @@ mod integration_tests { } let output = cmd.output()?; + println!("STDOUT:\n{}", String::from_utf8_lossy(&output.stdout)); + println!("STDERR:\n{}", String::from_utf8_lossy(&output.stderr)); + let code = output.status.code().expect("should have status code"); if code != 0 { - println!("{:#?}", std::str::from_utf8(&output.stderr)?); - println!("{:#?}", std::str::from_utf8(&output.stdout)?); panic!("command `{:?}` exited with code {}", cmd, code); }