diff --git a/hipcheck/src/plugin/manager.rs b/hipcheck/src/plugin/manager.rs index 40d46b47..b772b2fd 100644 --- a/hipcheck/src/plugin/manager.rs +++ b/hipcheck/src/plugin/manager.rs @@ -3,7 +3,7 @@ use crate::{ hc_error, hipcheck::plugin_service_client::PluginServiceClient, - plugin::{HcPluginClient, Plugin, PluginContext}, + plugin::{try_get_bin_for_entrypoint, HcPluginClient, Plugin, PluginContext}, Result, }; use futures::future::join_all; @@ -75,7 +75,21 @@ impl PluginExecutor { // on the cmdline is not already in use, but it is still possible for that // port to become unavailable between our check and the plugin's bind attempt. // Hence the need for subsequent attempts if we get unlucky + log::debug!("Starting plugin '{}'", plugin.name); + + // `entrypoint` is a string that represents a CLI invocation which may contain + // arguments, so we have to split off only the first token + if let Some(bin_path) = try_get_bin_for_entrypoint(&plugin.entrypoint).0 { + if which::which(bin_path).is_err() { + log::warn!( + "Binary '{}' used to spawn {} does not exist, spawn is unlikely to succeed", + bin_path, + plugin.name + ); + } + } + let mut spawn_attempts: usize = 0; while spawn_attempts < self.max_spawn_attempts { // Find free port for process. Don't retry if we fail since this means all @@ -83,14 +97,6 @@ impl PluginExecutor { let port = self.get_available_port()?; let port_str = port.to_string(); - if let Ok(false) = std::fs::exists(&plugin.entrypoint) { - log::warn!( - "entrypoint '{}' for {} does not exist", - plugin.entrypoint, - plugin.name - ) - } - // Spawn plugin process log::debug!("Spawning '{}' on port {}", &plugin.entrypoint, port_str); let Ok(mut proc) = Command::new(&plugin.entrypoint) diff --git a/hipcheck/src/plugin/mod.rs b/hipcheck/src/plugin/mod.rs index 1d72bd7d..e0963fce 100644 --- a/hipcheck/src/plugin/mod.rs +++ b/hipcheck/src/plugin/mod.rs @@ -12,7 +12,9 @@ use crate::error::Result; pub use crate::plugin::{get_plugin_key, manager::*, plugin_id::PluginId, types::*}; pub use arch::{get_current_arch, try_set_arch, Arch}; pub use download_manifest::{ArchiveFormat, DownloadManifest, HashAlgorithm, HashWithDigest}; -pub use plugin_manifest::{PluginManifest, PluginName, PluginPublisher, PluginVersion}; +pub use plugin_manifest::{ + try_get_bin_for_entrypoint, PluginManifest, PluginName, PluginPublisher, PluginVersion, +}; pub use retrieval::retrieve_plugins; use serde_json::Value; use std::collections::HashMap; diff --git a/hipcheck/src/plugin/plugin_manifest.rs b/hipcheck/src/plugin/plugin_manifest.rs index fdbe0d8c..1326ec4a 100644 --- a/hipcheck/src/plugin/plugin_manifest.rs +++ b/hipcheck/src/plugin/plugin_manifest.rs @@ -14,6 +14,7 @@ use crate::{ use kdl::{KdlDocument, KdlNode}; use std::{ collections::HashMap, + ops::Not, path::{Path, PathBuf}, str::FromStr, }; @@ -247,6 +248,14 @@ impl PluginManifest { Self::from_str(read_string(path)?.as_str()) } + pub fn get_entrypoint_for(&self, arch: &Arch) -> Result { + self.entrypoints + .0 + .get(arch) + .map(String::from) + .ok_or(hc_error!("No entrypoint for current arch ({})", arch)) + } + /// Update the directory that an entrypoint is stored in /// /// Returns previous entrypoint @@ -254,13 +263,17 @@ impl PluginManifest { where P: AsRef, { - let current_entrypoint = self + let curr_entrypoint_str = self .entrypoints .0 .remove(arch) - .map(PathBuf::from) .ok_or(hc_error!("No entrypoint for current arch ({})", arch))?; + let (opt_bin_path, args) = try_get_bin_for_entrypoint(&curr_entrypoint_str); + let bin_path = opt_bin_path + .map(PathBuf::from) + .ok_or(hc_error!("Malformed entrypoint string"))?; + fn new_path(new_directory: &Path, current_entrypoint: &Path) -> Result { let entrypoint_filename = new_directory.join(current_entrypoint.file_name().ok_or(hc_error!( @@ -270,9 +283,15 @@ impl PluginManifest { Ok(entrypoint_filename) } - let new_entrypoint = new_path(new_directory.as_ref(), ¤t_entrypoint)?; - self.set_entrypoint(arch.clone(), new_entrypoint.to_string_lossy().to_string()); - Ok(current_entrypoint) + let new_bin_path = new_path(new_directory.as_ref(), &bin_path)?; + let mut new_entrypoint = new_bin_path.to_string_lossy().to_string(); + if args.is_empty().not() { + new_entrypoint.push(' '); + new_entrypoint.push_str(&args); + } + + self.set_entrypoint(arch.clone(), new_entrypoint); + Ok(bin_path) } /// convert a `PluginManifest` to a `KdlDocument` @@ -327,6 +346,11 @@ impl FromStr for PluginManifest { } } +pub fn try_get_bin_for_entrypoint(entrypoint: &str) -> (Option<&str>, String) { + let mut split = entrypoint.split_whitespace(); + (split.next(), split.collect()) +} + #[cfg(test)] mod test { diff --git a/hipcheck/src/plugin/retrieval.rs b/hipcheck/src/plugin/retrieval.rs index 21f054b8..9f13b21f 100644 --- a/hipcheck/src/plugin/retrieval.rs +++ b/hipcheck/src/plugin/retrieval.rs @@ -5,8 +5,8 @@ use crate::{ error::Error, hc_error, plugin::{ - download_manifest::DownloadManifestEntry, ArchiveFormat, DownloadManifest, HashAlgorithm, - HashWithDigest, PluginId, PluginManifest, + download_manifest::DownloadManifestEntry, try_get_bin_for_entrypoint, ArchiveFormat, + DownloadManifest, HashAlgorithm, HashWithDigest, PluginId, PluginManifest, }, policy::policy_file::{ManifestLocation, PolicyPlugin}, util::http::agent::agent, @@ -121,18 +121,27 @@ fn retrieve_local_plugin( let mut plugin_manifest = PluginManifest::from_file(plugin_manifest_path)?; let current_arch = get_current_arch(); - let original_entrypoint = plugin_manifest - .update_entrypoint(¤t_arch, plugin_cache.plugin_download_dir(&plugin_id))?; + let curr_entrypoint = plugin_manifest.get_entrypoint_for(¤t_arch)?; - // @Note - sneaky potential for unexpected behavior if we write local plugin manifest - // to a cache dir that already included a remote download - std::fs::copy( - &original_entrypoint, - plugin_cache - .plugin_download_dir(&plugin_id) + if let Some(curr_bin) = try_get_bin_for_entrypoint(&curr_entrypoint).0 { + // Only do copy if using a path to a binary instead of a PATH-based resolution (i.e. + // `docker _`. We wouldn't want to copy the `docker` binary in this case + if std::fs::exists(curr_bin)? { + let original_entrypoint = plugin_manifest + .update_entrypoint(¤t_arch, plugin_cache.plugin_download_dir(&plugin_id))?; + + let new_entrypoint = plugin_manifest.get_entrypoint_for(¤t_arch)?; // unwrap is safe here, we just updated the entrypoint for current arch - .join(plugin_manifest.get_entrypoint(¤t_arch).unwrap()), - )?; + let new_bin = try_get_bin_for_entrypoint(&new_entrypoint).0.unwrap(); + + // @Note - sneaky potential for unexpected behavior if we write local plugin manifest + // to a cache dir that already included a remote download + std::fs::copy( + &original_entrypoint, + plugin_cache.plugin_download_dir(&plugin_id).join(new_bin), + )?; + } + } let plugin_kdl_path = plugin_cache.plugin_kdl(&plugin_id); write_all(&plugin_kdl_path, &plugin_manifest.to_kdl_formatted_string()).map_err(|e| {