From e67a6558bd7c44d39bd5a801630d75e3f5784a87 Mon Sep 17 00:00:00 2001 From: Justin Bacher <92jbach@gmail.com> Date: Mon, 20 Jan 2025 03:09:30 -0500 Subject: [PATCH 1/8] Working concurrency Just need to get elevations working correctly and clean things up --- Cargo.lock | 110 ++++- app/Cargo.toml | 5 + app/src/commands/apply.rs | 477 +++++++++++----------- app/src/commands/contexts.rs | 4 +- app/src/commands/gen_completions.rs | 4 +- app/src/commands/mod.rs | 2 +- app/src/commands/version.rs | 4 +- app/src/config/mod.rs | 4 +- app/src/main.rs | 27 +- app/src/utils/mod.rs | 23 ++ lib/Cargo.toml | 3 + lib/src/actions/command/run.rs | 2 +- lib/src/actions/directory/copy.rs | 12 +- lib/src/actions/mod.rs | 4 +- lib/src/actions/package/mod.rs | 4 +- lib/src/actions/package/providers/paru.rs | 2 +- lib/src/atoms/command/exec.rs | 120 ++++-- lib/src/atoms/directory/create.rs | 10 +- lib/src/atoms/directory/remove.rs | 10 +- lib/src/atoms/file/chmod.rs | 11 +- lib/src/atoms/file/chown.rs | 4 +- lib/src/atoms/file/contents.rs | 10 +- lib/src/atoms/file/copy.rs | 14 +- lib/src/atoms/file/create.rs | 10 +- lib/src/atoms/file/decrypt.rs | 10 +- lib/src/atoms/file/link.rs | 12 +- lib/src/atoms/file/remove.rs | 10 +- lib/src/atoms/file/unarchive.rs | 4 +- lib/src/atoms/git/clone.rs | 4 +- lib/src/atoms/http/download.rs | 10 +- lib/src/atoms/mod.rs | 15 +- lib/src/lib.rs | 2 +- lib/src/manifests/mod.rs | 17 + lib/src/steps/finalizers/mod.rs | 2 +- lib/src/steps/mod.rs | 88 ++-- lib/src/utilities/mod.rs | 8 +- lib/src/utilities/password_manager.rs | 68 +++ 37 files changed, 707 insertions(+), 419 deletions(-) create mode 100644 app/src/utils/mod.rs create mode 100644 lib/src/utilities/password_manager.rs diff --git a/Cargo.lock b/Cargo.lock index 78ddce21..d55fef08 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1,6 +1,6 @@ # This file is automatically @generated by Cargo. # It is not intended for manual editing. -version = 3 +version = 4 [[package]] name = "addr2line" @@ -205,9 +205,9 @@ dependencies = [ [[package]] name = "async-trait" -version = "0.1.81" +version = "0.1.85" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6e0c28dcc82d7c8ead5cb13beb15405b57b8546e93215673ff8ca0349a028107" +checksum = "3f934833b4b7233644e5848f235df3f57ed8c80f1528a26c3dfa13d2147fa056" dependencies = [ "proc-macro2", "quote", @@ -495,12 +495,12 @@ dependencies = [ "crossterm", "strum", "strum_macros", - "unicode-width", + "unicode-width 0.1.13", ] [[package]] name = "comtrya" -version = "0.9.0" +version = "0.9.1" dependencies = [ "anyhow", "assert_cmd", @@ -509,26 +509,32 @@ dependencies = [ "colored", "comfy-table", "comtrya-lib", + "crossbeam-queue", "dirs-next", "petgraph", "predicates", "rhai", + "rpassword", "serde", "serde_yml", "strip-ansi-escapes", "tempfile", + "tokio", "tracing", "tracing-journald", "tracing-subscriber", "update-informer", + "yapp", + "zeroize", ] [[package]] name = "comtrya-lib" -version = "0.9.0" +version = "0.9.1" dependencies = [ "age", "anyhow", + "async-trait", "dirs-next", "file-owner", "file_diff", @@ -546,6 +552,7 @@ dependencies = [ "regex", "reqwest 0.12.9", "rhai", + "rpassword", "schemars", "serde", "serde_json", @@ -562,6 +569,20 @@ dependencies = [ "walkdir", "which", "whoami", + "zeroize", +] + +[[package]] +name = "console" +version = "0.15.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ea3c6ecd8059b57859df5c69830340ed3c41d30e3da0c1cbed90a96ac853041b" +dependencies = [ + "encode_unicode", + "libc", + "once_cell", + "unicode-width 0.2.0", + "windows-sys 0.59.0", ] [[package]] @@ -629,9 +650,9 @@ dependencies = [ [[package]] name = "crossbeam-channel" -version = "0.5.13" +version = "0.5.14" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "33480d6946193aa8033910124896ca395333cae7e2d1113d1fef6c3272217df2" +checksum = "06ba6d68e24814cb8de6bb986db8222d3a027d15872cabc0d18817bc3c0e4471" dependencies = [ "crossbeam-utils", ] @@ -655,6 +676,15 @@ dependencies = [ "crossbeam-utils", ] +[[package]] +name = "crossbeam-queue" +version = "0.3.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0f58bbc28f91df819d0aa2a2c00cd19754769c2fad90579b3592b1c9ba7a3115" +dependencies = [ + "crossbeam-utils", +] + [[package]] name = "crossbeam-utils" version = "0.8.20" @@ -873,6 +903,12 @@ version = "1.13.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "60b1af1c220855b6ceac025d3f6ecdd2b7c4894bfe9cd9bda4fbb4bc7c0d4cf0" +[[package]] +name = "encode_unicode" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "34aa73646ffb006b8f5147f3dc182bd4bcb190227ce861fc4a4844bf8e3cb2c0" + [[package]] name = "encoding_rs" version = "0.8.34" @@ -2683,9 +2719,9 @@ checksum = "bbd2bcb4c963f2ddae06a2efc7e9f3591312473c50c6685e1f298068316e66fe" [[package]] name = "libc" -version = "0.2.158" +version = "0.2.169" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d8adc4bb1803a324070e64a98ae98f38934d91957a99cfb3a43dcbc01bc56439" +checksum = "b5aba8db14291edd000dfcc4d620c7ebfb122c613afb886ca8803fa4e128a20a" [[package]] name = "libm" @@ -3578,6 +3614,27 @@ dependencies = [ "windows-sys 0.52.0", ] +[[package]] +name = "rpassword" +version = "7.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "80472be3c897911d0137b2d2b9055faf6eeac5b14e324073d83bc17b191d7e3f" +dependencies = [ + "libc", + "rtoolbox", + "windows-sys 0.48.0", +] + +[[package]] +name = "rtoolbox" +version = "0.0.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c247d24e63230cdb56463ae328478bd5eac8b8faa8c69461a77e8e323afac90e" +dependencies = [ + "libc", + "windows-sys 0.48.0", +] + [[package]] name = "rust-embed" version = "8.5.0" @@ -4451,19 +4508,33 @@ checksum = "1f3ccbac311fea05f86f61904b462b55fb3df8837a366dfc601a0161d0532f20" [[package]] name = "tokio" -version = "1.41.0" +version = "1.43.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "145f3413504347a2be84393cc8a7d2fb4d863b375909ea59f2158261aa258bbb" +checksum = "3d61fa4ffa3de412bfea335c6ecff681de2b609ba3c77ef3e00e521813a9ed9e" dependencies = [ "backtrace", "bytes", "libc", "mio", + "parking_lot", "pin-project-lite", + "signal-hook-registry", "socket2", + "tokio-macros", "windows-sys 0.52.0", ] +[[package]] +name = "tokio-macros" +version = "2.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6e06d43f1345a3bcd39f6a56dbb7dcab2ba47e68e8ac134855e7e2bdbaf8cab8" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.90", +] + [[package]] name = "tokio-rustls" version = "0.24.1" @@ -4857,6 +4928,12 @@ version = "0.1.13" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0336d538f7abc86d282a4189614dfaa90810dfc2c6f6427eaf88e16311dd225d" +[[package]] +name = "unicode-width" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1fc81956842c57dac11422a97c3b8195a1ff727f06e85c84ed2e8aa277c9a0fd" + [[package]] name = "universal-hash" version = "0.5.1" @@ -5398,6 +5475,15 @@ version = "1.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "cfe53a6657fd280eaa890a3bc59152892ffa3e30101319d168b781ed6529b049" +[[package]] +name = "yapp" +version = "0.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6760b1caf07f5ead23a94c360a2aee290518a89edfd9b32b459b47b5e248294d" +dependencies = [ + "console", +] + [[package]] name = "zerocopy" version = "0.7.35" diff --git a/app/Cargo.toml b/app/Cargo.toml index 4c653760..83221e7f 100644 --- a/app/Cargo.toml +++ b/app/Cargo.toml @@ -23,6 +23,11 @@ update-informer = "1.1" dirs-next = "2.0" serde = { version = "1.0", features = ["derive"] } serde_yml = "0" +tokio = { version = "1.43.0", features = ["full"] } +crossbeam-queue = "0.3.12" +yapp = "0.4.1" +zeroize = { version = "1.8.1", features = ["derive", "std"] } +rpassword = "7.3.1" [dev-dependencies] assert_cmd = "2.0" diff --git a/app/src/commands/apply.rs b/app/src/commands/apply.rs index 09124f04..6b37d243 100644 --- a/app/src/commands/apply.rs +++ b/app/src/commands/apply.rs @@ -1,17 +1,32 @@ -use super::ComtryaCommand; -use crate::Runtime; +use std::{ + collections::HashMap, + ops::Deref, + path::PathBuf, + sync::{Arc, RwLock}, + thread::available_parallelism, +}; + +use anyhow::{anyhow, Context, Result}; use clap::Parser; use comfy_table::{Cell, ContentArrangement, Table}; -use comtrya_lib::contexts::to_rhai; -use comtrya_lib::manifests::{load, Manifest}; -use core::panic; -use petgraph::{visit::DfsPostOrder, Graph}; +use petgraph::{ + stable_graph::{NodeIndex, StableDiGraph}, + Direction::*, +}; use rhai::Engine; -use std::path::PathBuf; -use std::{collections::HashMap, ops::Deref}; -use tracing::{debug, error, info, instrument, span, trace, warn}; +use tokio::task::JoinSet; +use tracing::{debug, error, info, info_span, instrument, trace, warn}; -#[derive(Parser, Debug)] +use super::ComtryaCommand; +use crate::{utils::ZipLongest, Runtime}; +use comtrya_lib::{ + actions::Actions::{CommandRun, PackageInstall, PackageRepository}, + contexts::to_rhai, + manifests::{load, Manifest}, + utilities::{get_privilege_provider, password_manager::PasswordManager}, +}; + +#[derive(Parser, Debug, Clone)] pub(crate) struct Apply { /// Run a subset of your manifests, comma separated list. /// This should be a list of manifest names. No paths. @@ -27,42 +42,95 @@ pub(crate) struct Apply { pub label: Option, } +#[derive(Debug)] +struct DependenceyGraph { + graph: StableDiGraph, ()>, + name_to_idx: HashMap, NodeIndex>, + should_ask_for_pass: bool, +} + +impl DependenceyGraph { + pub fn new(manifests: HashMap) -> Self { + let mut this = Self { + graph: StableDiGraph::new(), + name_to_idx: HashMap::new(), + should_ask_for_pass: false, + }; + + let mut first_package_install: Option = None; + + for (_, manifest) in manifests.iter() { + let from_index = this.add_manifest(manifest); + + + for (dependant, action) in manifest.depends.iter().zip_longest(manifest.actions.iter()) + { + if let Some(dependency_manifest) = dependant.and_then(|dep| manifests.get(dep)) { + let to_index = this.add_manifest(dependency_manifest); + this.graph.add_edge(from_index, to_index, ()); + } + + if !this.should_ask_for_pass { + this.should_ask_for_pass = match action { + Some(CommandRun(cva)) => cva.action.privileged, + Some(PackageInstall(_)) | Some(PackageRepository(_)) => { + if let Some(index) = first_package_install { + this.graph.add_edge(from_index, index, ()); + } else { + first_package_install = Some(from_index); + } + true + }, + _ => false, + }; + } + } + } + + this + } + + pub fn add_manifest(&mut self, manifest: &Manifest) -> NodeIndex { + if let Some(&idx) = self.name_to_idx.get(&manifest.name) { + idx + } else { + let m = Arc::new(manifest.to_owned()); + let idx = self.graph.add_node(m); + self.name_to_idx.insert(manifest.name.to_owned(), idx); + idx + } + } +} + impl Apply { fn manifest_path(&self, runtime: &Runtime) -> anyhow::Result { for manifest in &self.manifests { if manifest.contains(std::path::MAIN_SEPARATOR) { - return Err(anyhow::anyhow!( + return Err(anyhow!( "Found a path, expected only names in the manifests list!" )); } } - let first_manifest_path = runtime.config.manifest_paths.first().ok_or_else(|| { - anyhow::anyhow!( - "No manifest paths found in config file, please add at least one path to your manifests" - ) - })?; - - let manifest_path = match crate::manifests::resolve(first_manifest_path) { - Some(path) => path, - None => { - return Err(anyhow::anyhow!( - "Manifest location, {:?}, could be resolved", - first_manifest_path - )) - } - }; + let first_manifest_path = runtime.config.manifest_paths.first().context( + "No manifest paths found in config file, please add at least one path to your manifests" + )?; + + let manifest_path = crate::manifests::resolve(first_manifest_path).context(format!( + "Manifest location, {first_manifest_path:?}, could be resolved", + ))?; + + trace!(manifests = self.manifests.join(",").deref()); - trace!(manifests = self.manifests.join(",").deref(),); Ok(manifest_path) } #[instrument(skip(self, runtime))] - pub fn status(&self, runtime: &Runtime) -> anyhow::Result<()> { + pub async fn status(&self, runtime: &Runtime) -> anyhow::Result<()> { let contexts = &runtime.contexts; - let manifest_path = self.manifest_path(&runtime)?; + let manifest_path = self.manifest_path(runtime)?; - println!("Load manifests from path: {:#?}", manifest_path); + println!("Load manifests from path: {manifest_path:#?}",); let manifests = load(manifest_path, contexts); @@ -74,243 +142,186 @@ impl Apply { for (name, manifest) in manifests.iter() { table.add_row(vec![ - Cell::new(format!("{name}")), + Cell::new(name), Cell::new(format!("{}", manifest.actions.len())), ]); } + println!("{table}"); + Ok(()) } -} - -impl ComtryaCommand for Apply { - #[instrument(skip(self, runtime))] - fn execute(&self, runtime: &Runtime) -> anyhow::Result<()> { - let contexts = &runtime.contexts; - let manifest_path = self.manifest_path(&runtime)?; - let manifests = load(manifest_path, contexts); - - // Build DAG - let mut dag: Graph = Graph::new(); - - let manifest_root = Manifest { - r#where: None, - root_dir: None, - dag_index: None, - name: None, - depends: vec![], - actions: vec![], - ..Default::default() - }; - let root_index = dag.add_node(manifest_root); - - let manifests: HashMap = manifests - .into_iter() - .map(|(name, mut manifest)| { - let abc = dag.add_node(manifest.clone()); - - manifest.dag_index = Some(abc); - dag.add_edge(root_index, abc, 0); - - (name, manifest) - }) - .collect(); - - for (name, manifest) in manifests.iter() { - manifest.depends.iter().for_each(|dependency| { - let (local_dependency_prefix, _) = name.rsplit_once('.').unwrap_or((name, "")); - - let resolved_dependency_name = - dependency.replace("./", format!("{}.", local_dependency_prefix).as_str()); - - let m1 = match manifests.get(&resolved_dependency_name) { - Some(manifest) => manifest, - None => { - error!( - message = "Unresolved dependency", - dependency = resolved_dependency_name.as_str() - ); - - return; - } - }; - - trace!( - message = "Dependency Registered", - from = name.as_str(), - to = m1.name.as_deref().unwrap_or("cannot extract name"), + #[instrument(skip(self, runtime, manifest))] + async fn process_manifest( + &self, + runtime: &Arc, + manifest: &Arc, + ) -> Result<()> { + if let Some(label) = self.label.as_ref() { + if !manifest.labels.contains(label) { + info!( + message = "Skipping manifest, label not found", + label = label.as_str() ); - - if let (Some(from), Some(to)) = (manifest.dag_index, m1.dag_index) { - dag.add_edge(from, to, 0); - } else { - error!(message = "Cannot add dependency, missing dag index"); - } - }); + return Ok(()); + } } - let clone_m = self.manifests.clone(); - - let run_manifests = if self.manifests.is_empty() { - // No manifests specified on command line, so run everything - vec![String::from("")] - } else { - // Run subset - manifests - .keys() - .filter(|z| clone_m.contains(z)) - .cloned() - .collect::>() - }; - - let dry_run = self.dry_run; - - let engine = Engine::new(); - let mut scope = to_rhai(contexts); - - run_manifests.iter().for_each(|manifest| { - let start = if manifest.eq(&String::from("")) { - root_index - } else if let Some(dag_index) = manifests - .get(manifest) - .and_then(|manifest| manifest.dag_index) + if let Some(where_condition) = &manifest.r#where { + let engine = Engine::new(); + let mut scope = to_rhai(&runtime.contexts); + + match engine.eval_with_scope::(&mut scope, where_condition) { - dag_index - } else { - // FIXME: Don't panic here. Find a better way to handle this. - panic!("Cannot find manifest in DAG"); + Ok(result) => { + debug!("Result of 'where' condition '{where_condition}' -> '{result}'",); + } + Err(err) => { + warn!("'where' condition '{where_condition}' failed: {err}"); + info!("Skipping manifest, because 'where' conditions were false!"); + return Ok(()); + } }; + } - let mut dfs = DfsPostOrder::new(&dag, start); - - while let Some(visited) = dfs.next(&dag) { - if dag.node_weight(visited).is_none() { - info!( - message = "Skipping manifest, not found in DAG", - index = visited.index() - ); + for action in manifest.actions.iter() { + let span_action = info_span!("", %action).entered(); + let action = action.inner_ref(); + + let mut steps = match action.plan(manifest, &runtime.contexts) { + Ok(steps) => steps + .into_iter() + .filter(|step| step.do_initializers_allow_us_to_run()) + .filter(|step| match step.atom.plan() { + Ok(outcome) => outcome.should_run, + Err(_) => false, + }) + .peekable(), + Err(err) => { + error!("Failed Processing: {manifest}. Action failed to get plan: {err:?}",); + return Ok(()); } + }; - // .unwrap() is safe here, because we just checked that the node exists - let m1 = dag.node_weight(visited).unwrap(); + if steps.peek().is_none() { + info!("nothing to be done to reconcile action"); + span_action.exit(); + return Ok(()); + } - // Root manifest, nothing to do. - if m1.name.is_none() { - continue; - } + if self.dry_run { + return Ok(()); + } - let span_manifest = span!( - tracing::Level::INFO, - "", - manifest = m1.name.as_deref().unwrap_or("Cannot extract name"), - ) - .entered(); - - let mut successful = true; - - if let Some(label) = self.label.as_ref() { - if !m1.labels.contains(label) { - info!( - message = "Skipping manifest, label not found", - label = label.as_str() - ); - continue; - } + for mut step in steps { + let password_manager = runtime.password_manager.clone(); + if let Err(err) = step.atom.execute(password_manager).await { + debug!("Atom failed to execute: {:?}", err); + break; } - if let Some(where_condition) = &m1.r#where { - let where_result = - match engine.eval_with_scope::(&mut scope, where_condition) { - Ok(result) => { - debug!( - "Result of 'where' condition '{}' -> '{}'", - where_condition, result - ); - - result - } - Err(err) => { - warn!("'where' condition '{}' failed: {}", where_condition, err); - false - } - }; - - if !where_result { - info!("Skip manifest, because 'where' conditions were false!"); - span_manifest.exit(); - continue; - } + if !step.do_finalizers_allow_us_to_continue() { + debug!("Finalizers won't allow us to continue with this action"); + break; } + } + info!("{}", action.summarize()); + span_action.exit(); + } - for action in m1.actions.iter() { - let span_action = span!(tracing::Level::INFO, "", %action).entered(); + info!("Completed: {manifest}",); + Ok(()) + } +} - let action = action.inner_ref(); +impl ComtryaCommand for Apply { + #[instrument(skip(self, runtime))] + async fn execute(&self, runtime: &mut Runtime) -> Result<()> { + let ready_queue = Arc::new(crossbeam_queue::SegQueue::new()); + let done_queue = Arc::new(crossbeam_queue::SegQueue::new()); + let manifest_manager = DependenceyGraph::new(load( + self.manifest_path(&runtime.clone())?, + &runtime.contexts, + )); + + let contexts = runtime.clone().contexts; + if manifest_manager.should_ask_for_pass { + let mut password_manager = + PasswordManager::new(get_privilege_provider(&contexts).as_deref())?; + password_manager.prompt("Please enter password:").await?; + runtime.password_manager = Some(password_manager); + } - let plan = match action.plan(m1, contexts) { - Ok(steps) => steps, - Err(err) => { - info!("Action failed to get plan: {:?}", err); - successful = false; - continue; - } - }; + for idx in manifest_manager.graph.externals(Outgoing) { + if let Some(manifest) = manifest_manager.graph.node_weight(idx) { + ready_queue.push(manifest.clone()); + } + } - let mut steps = plan - .into_iter() - .filter(|step| step.do_initializers_allow_us_to_run()) - .filter(|step| match step.atom.plan() { - Ok(outcome) => outcome.should_run, - Err(_) => false, - }) - .peekable(); - - if steps.peek().is_none() { - info!("nothing to be done to reconcile action"); - span_action.exit(); - continue; + let shared_self = Arc::new(self.clone()); + let mut workers = JoinSet::new(); + let thread_runtime = Arc::new(runtime.clone()); + + for _ in 0..available_parallelism().map_or(1, |p| p.get() / 2) { + // Every runner needs it's own reference + let thread_runtime = Arc::clone(&thread_runtime); + let thread_ready_queue = Arc::clone(&ready_queue); + let thread_done_queue = Arc::clone(&done_queue); + let thread_self = Arc::clone(&shared_self); + + workers.spawn(async move { + tokio::task::spawn_blocking(move || { + while let Some(manifest) = thread_ready_queue.pop() { + if let Err(e) = tokio::runtime::Handle::current().block_on(thread_self + .process_manifest(&thread_runtime, &manifest)) + { + error!("Unable to process manifest: '{manifest}'. {e}") + } else { + thread_done_queue.push(manifest.clone()); + } } + }).await?; + Ok::<_, anyhow::Error>(()) + }); + } - for mut step in steps { - if dry_run { - continue; - } + let shared_graph = Arc::new(RwLock::new(manifest_manager.graph)); - match step.atom.execute() { - Ok(_) => (), - Err(err) => { - debug!("Atom failed to execute: {:?}", err); - successful = false; - break; + tokio::spawn(async move { + if let Some(completed) = done_queue.pop() { + if let Some(&idx) = manifest_manager.name_to_idx.get(&completed.name) { + { + match shared_graph.write() { + Ok(mut guard) => { + guard.remove_node(idx); + } + Err(e) => { + error!("Failed to acquire write lock: {e}"); } } + } - if !step.do_finalizers_allow_us_to_continue() { - debug!("Finalizers won't allow us to continue with this action"); - successful = false; - break; + match shared_graph.read() { + Ok(graph) => { + for idx in graph.externals(Outgoing) { + if let Some(manifest) = graph.node_weight(idx) { + ready_queue.push(Arc::clone(manifest)); + } + } + } + Err(e) => { + error!("Failed to acquire read lock: {e}"); } } - info!("{}", action.summarize()); - span_action.exit(); - } - - if dry_run { - span_manifest.exit(); - continue; - } - - if !successful { - error!("Failed"); - span_manifest.exit(); - break; } - - info!("Completed"); - span_manifest.exit(); } - }); + // println!("{:?}", shared_graph.read().unwrap()); + drop(ready_queue); + }) + .await?; + + workers.join_all().await; Ok(()) } diff --git a/app/src/commands/contexts.rs b/app/src/commands/contexts.rs index f0f6943d..6d8e7f19 100644 --- a/app/src/commands/contexts.rs +++ b/app/src/commands/contexts.rs @@ -5,7 +5,7 @@ use comfy_table::{presets::NOTHING, Attribute, Cell, ContentArrangement, Table}; use clap::Parser; -#[derive(Parser, Debug)] +#[derive(Parser, Debug, Clone)] #[command()] pub(crate) struct Contexts { /// Show the values of the contexts @@ -14,7 +14,7 @@ pub(crate) struct Contexts { } impl ComtryaCommand for Contexts { - fn execute(&self, runtime: &Runtime) -> anyhow::Result<()> { + async fn execute(&self, runtime: &mut Runtime) -> anyhow::Result<()> { for (name, context) in runtime.contexts.iter() { println!("{}", name.to_string().underline().bold()); diff --git a/app/src/commands/gen_completions.rs b/app/src/commands/gen_completions.rs index c755231d..53606c67 100644 --- a/app/src/commands/gen_completions.rs +++ b/app/src/commands/gen_completions.rs @@ -8,7 +8,7 @@ use clap_complete::{generate, Generator, Shell}; use crate::GlobalArgs; -#[derive(Parser, Debug)] +#[derive(Parser, Debug, Clone)] #[command(arg_required_else_help = true)] pub(crate) struct GenCompletions { /// If provided, outputs the completion file for given shell @@ -21,7 +21,7 @@ fn print_completions(gen: G, cmd: &mut Command) { } impl ComtryaCommand for GenCompletions { - fn execute(&self, _runtime: &Runtime) -> anyhow::Result<()> { + async fn execute(&self, _runtime: &mut Runtime) -> anyhow::Result<()> { print_completions(self.shell, &mut GlobalArgs::command()); Ok(()) diff --git a/app/src/commands/mod.rs b/app/src/commands/mod.rs index ccd4646b..cdf7df3f 100644 --- a/app/src/commands/mod.rs +++ b/app/src/commands/mod.rs @@ -13,5 +13,5 @@ pub(crate) use gen_completions::GenCompletions; use crate::Runtime; pub trait ComtryaCommand { - fn execute(&self, runtime: &Runtime) -> anyhow::Result<()>; + async fn execute(&self, runtime: &mut Runtime) -> anyhow::Result<()>; } diff --git a/app/src/commands/version.rs b/app/src/commands/version.rs index e5fa2a8c..9f559292 100644 --- a/app/src/commands/version.rs +++ b/app/src/commands/version.rs @@ -3,12 +3,12 @@ use crate::Runtime; use clap::Parser; -#[derive(Parser, Debug)] +#[derive(Parser, Debug, Clone)] #[command()] pub(crate) struct Version {} impl ComtryaCommand for Version { - fn execute(&self, _: &Runtime) -> anyhow::Result<()> { + async fn execute(&self, _: &mut Runtime) -> anyhow::Result<()> { const VERSION: Option<&'static str> = option_env!("CARGO_PKG_VERSION"); println!("{}", VERSION.unwrap_or("unknown")); diff --git a/app/src/config/mod.rs b/app/src/config/mod.rs index c7a72854..d0814f2d 100644 --- a/app/src/config/mod.rs +++ b/app/src/config/mod.rs @@ -10,7 +10,7 @@ use std::{ use tracing::{trace, warn}; -#[derive(Parser, Debug, Default)] +#[derive(Parser, Debug, Default, Clone)] #[command(version, about, name="comtrya", long_about = None)] pub struct GlobalArgs { #[arg(short = 'd', long)] @@ -32,7 +32,7 @@ pub struct GlobalArgs { pub command: Commands, } -#[derive(Debug, Subcommand)] +#[derive(Debug, Subcommand, Clone)] pub enum Commands { /// Apply manifests #[clap(aliases = &["do", "run"])] diff --git a/app/src/main.rs b/app/src/main.rs index 038f49c9..e4dcc7ab 100644 --- a/app/src/main.rs +++ b/app/src/main.rs @@ -15,22 +15,25 @@ use tracing_subscriber::{fmt::writer::MakeWriterExt, layer::SubscriberExt, FmtSu mod commands; mod config; +mod utils; +use comtrya_lib::utilities::password_manager::PasswordManager; use config::Config; -#[derive(Debug)] +#[derive(Debug, Clone)] pub struct Runtime { pub(crate) args: GlobalArgs, pub(crate) config: Config, pub(crate) contexts: Contexts, + pub(crate) password_manager: Option, } -pub(crate) fn execute(runtime: Runtime) -> anyhow::Result<()> { - match &runtime.args.command { - Commands::Apply(apply) => apply.execute(&runtime), - Commands::Status(apply) => apply.status(&runtime), - Commands::Version(version) => version.execute(&runtime), - Commands::Contexts(contexts) => contexts.execute(&runtime), - Commands::GenCompletions(gen_completions) => gen_completions.execute(&runtime), +pub(crate) async fn execute(runtime: &mut Runtime) -> anyhow::Result<()> { + match runtime.clone().args.command { + Commands::Apply(apply) => apply.execute(runtime).await, + Commands::Status(apply) => apply.status(runtime).await, + Commands::Version(version) => version.execute(runtime).await, + Commands::Contexts(contexts) => contexts.execute(runtime).await, + Commands::GenCompletions(gen_completions) => gen_completions.execute(runtime).await, } } @@ -59,7 +62,8 @@ fn configure_tracing(args: &GlobalArgs) { .expect("Unable to set a global subscriber"); } -fn main() -> anyhow::Result<()> { +#[tokio::main] +async fn main() -> anyhow::Result<()> { let args = GlobalArgs::parse(); configure_tracing(&args); @@ -77,13 +81,14 @@ fn main() -> anyhow::Result<()> { // Run Context Providers let contexts = build_contexts(&config); - let runtime = Runtime { + let mut runtime = Runtime { args, config, contexts, + password_manager: None, }; - execute(runtime)?; + execute(&mut runtime).await?; Ok(()) } diff --git a/app/src/utils/mod.rs b/app/src/utils/mod.rs new file mode 100644 index 00000000..d5f0df41 --- /dev/null +++ b/app/src/utils/mod.rs @@ -0,0 +1,23 @@ +use std::iter::from_fn; + +pub trait ZipLongest: Iterator { + fn zip_longest(self, other: I) -> impl Iterator, Option)> + where + Self: Sized, + I: IntoIterator; +} + +impl> ZipLongest for I { + fn zip_longest(self, other: J) -> impl Iterator, Option)> + where + J: IntoIterator, + { + let mut a = self.fuse(); + let mut b = other.into_iter().fuse(); + + from_fn(move || match (a.next(), b.next()) { + (None, None) => None, + (a, b) => Some((a, b)), + }) + } +} diff --git a/lib/Cargo.toml b/lib/Cargo.toml index c8106988..11606a96 100644 --- a/lib/Cargo.toml +++ b/lib/Cargo.toml @@ -42,6 +42,9 @@ flate2 = "1.0.34" file-owner = "0.1.2" gix = { version = "0.68.0", features = ["blocking-http-transport-reqwest-rust-tls", "blocking-network-client"] } gix-protocol = "0.46.1" +rpassword = "7.3.1" +zeroize = { version = "1.8.1", features = ["derive", "std"] } +async-trait = "0.1.85" [target.'cfg(unix)'.dependencies] uzers = "0.12" diff --git a/lib/src/actions/command/run.rs b/lib/src/actions/command/run.rs index 63c32c36..f0152153 100644 --- a/lib/src/actions/command/run.rs +++ b/lib/src/actions/command/run.rs @@ -43,7 +43,7 @@ impl Action for RunCommand { use crate::atoms::command::Exec; let privilege_provider = - utilities::get_privilege_provider(&contexts).unwrap_or_else(|| "sudo".to_string()); + utilities::get_privilege_provider(contexts).unwrap_or_else(|| "sudo".to_string()); Ok(vec![Step { atom: Box::new(Exec { diff --git a/lib/src/actions/directory/copy.rs b/lib/src/actions/directory/copy.rs index f8917c72..e47c78fd 100644 --- a/lib/src/actions/directory/copy.rs +++ b/lib/src/actions/directory/copy.rs @@ -25,15 +25,15 @@ impl Action for DirectoryCopy { fn plan(&self, manifest: &Manifest, _context: &Contexts) -> anyhow::Result> { let from: String = self.resolve(manifest, &self.from).display().to_string(); - Ok(vec![Step { - atom: Box::new(Exec { + Ok(vec![Step::new( + Exec { command: String::from("Xcopy"), arguments: vec!["/E".to_string(), "/I".to_string(), from, self.to.clone()], ..Default::default() - }), - initializers: vec![], - finalizers: vec![], - }]) + }, + vec![], + vec![], + )]) } } diff --git a/lib/src/actions/mod.rs b/lib/src/actions/mod.rs index 7a6042ff..31955eee 100644 --- a/lib/src/actions/mod.rs +++ b/lib/src/actions/mod.rs @@ -1,5 +1,5 @@ mod binary; -mod command; +pub mod command; mod directory; mod file; mod git; @@ -238,7 +238,7 @@ impl From for ActionError { } } -pub trait Action { +pub trait Action: Send + Sync { fn summarize(&self) -> String { warn!("need to define action summarize"); "not found action summarize".to_string() diff --git a/lib/src/actions/package/mod.rs b/lib/src/actions/package/mod.rs index dc64a4ff..75235acc 100644 --- a/lib/src/actions/package/mod.rs +++ b/lib/src/actions/package/mod.rs @@ -77,7 +77,7 @@ impl From<&Package> for PackageVariant { list: package.list.clone(), provider: package.provider.clone(), extra_args: package.extra_args.clone(), - file: package.file.clone(), + file: package.file, }; }; @@ -91,7 +91,7 @@ impl From<&Package> for PackageVariant { list: package.list.clone(), provider: variant.provider.clone(), extra_args: variant.extra_args.clone(), - file: package.file.clone(), + file: package.file, }; if variant.name.is_some() { diff --git a/lib/src/actions/package/providers/paru.rs b/lib/src/actions/package/providers/paru.rs index 61c913c8..c3245342 100644 --- a/lib/src/actions/package/providers/paru.rs +++ b/lib/src/actions/package/providers/paru.rs @@ -32,7 +32,7 @@ impl PackageProvider for Paru { fn bootstrap(&self, contexts: &Contexts) -> Vec { let privilege_provider = - utilities::get_privilege_provider(&contexts).unwrap_or_else(|| String::from("sudo")); + utilities::get_privilege_provider(contexts).unwrap_or_else(|| String::from("sudo")); vec![ Step { diff --git a/lib/src/atoms/command/exec.rs b/lib/src/atoms/command/exec.rs index 57d8124c..58fa9149 100644 --- a/lib/src/atoms/command/exec.rs +++ b/lib/src/atoms/command/exec.rs @@ -1,9 +1,19 @@ -use crate::atoms::Outcome; +use std::{ + io::{BufRead, BufReader, Write}, + process::Stdio, + sync::{Arc, RwLock}, +}; + +use anyhow::{anyhow, Result}; +use tracing::debug; use super::super::Atom; +use crate::atoms::Outcome; use crate::utilities; -use anyhow::anyhow; -use tracing::debug; +use crate::utilities::password_manager::PasswordManager; +use tokio::process::Command; +use tokio::time::Duration; +use tokio::io::AsyncWriteExt; #[derive(Default)] pub struct Exec { @@ -53,7 +63,7 @@ impl Exec { } } - fn elevate(&mut self) -> anyhow::Result<()> { + async fn elevate(&mut self, password_manager: &Option) -> Result<()> { tracing::info!( "Privilege elevation required to run `{} {}`. Validating privileges ...", &self.command, @@ -62,20 +72,26 @@ impl Exec { let privilege_provider = utilities::get_binary_path(&self.privilege_provider)?; - match std::process::Command::new(privilege_provider) - .stdin(std::process::Stdio::inherit()) - .stdout(std::process::Stdio::inherit()) - .stderr(std::process::Stdio::inherit()) - .arg("--validate") - .output() - { - Ok(std::process::Output { status, .. }) if status.success() => Ok(()), + let mut child = Command::new(privilege_provider) + .args(["-S", "--validate"]) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn()?; + + if let Some(mut stdin) = child.stdin.take() { + if let Some(secret) = password_manager.as_ref().and_then(|pm| pm.secret.clone()) { + stdin.write_all(secret.as_bytes()).await?; + stdin.write_all(b"\n").await?; + } + } + match child.wait_with_output().await { + Ok(std::process::Output { status, .. }) if status.success() => Ok(()), Ok(std::process::Output { stderr, .. }) => Err(anyhow!( "Command requires privilege escalation, but couldn't elevate privileges: {}", String::from_utf8(stderr)? )), - Err(err) => Err(anyhow!( "Command requires privilege escalation, but couldn't elevate privileges: {}", err @@ -96,6 +112,7 @@ impl std::fmt::Display for Exec { } } +#[async_trait::async_trait] impl Atom for Exec { fn plan(&self) -> anyhow::Result { Ok(Outcome { @@ -110,16 +127,18 @@ impl Atom for Exec { }) } - fn execute(&mut self) -> anyhow::Result<()> { + async fn execute(&mut self, password_manager: Option) -> anyhow::Result<()> { let (command, arguments) = self.elevate_if_required(); + println!("{}", self); let command = utilities::get_binary_path(&command) - .or_else(|_| Err(anyhow!("Command `{}` not found in path", command)))?; + .map_err(|_| anyhow!("Command `{}` not found in path", command))?; // If we require root, we need to use sudo with inherited IO // to ensure the user can respond if prompted for a password if command.eq("doas") || command.eq("sudo") || command.eq("run0") { - match self.elevate() { + println!("elevating {}", command); + match self.elevate(&password_manager).await { Ok(_) => (), Err(err) => { return Err(anyhow!(err)); @@ -127,16 +146,66 @@ impl Atom for Exec { } } - match std::process::Command::new(&command) + let mut child = std::process::Command::new(&command) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) .envs(self.environment.clone()) .args(&arguments) - .current_dir(&self.working_dir.clone().unwrap_or_else(|| { + .current_dir(self.working_dir.clone().unwrap_or_else(|| { std::env::current_dir() .map(|current_dir| current_dir.display().to_string()) - .expect("Failed to get current directory") + .unwrap_or_else(|_| String::from(".")) })) - .output() - { + .spawn()?; + + let secret = Arc::new(password_manager + .and_then(|pm| pm.secret.clone()) + .map_or(String::new(), |s| format!("{}\n", s.as_str()))); + let stdin = Arc::new(RwLock::new(child.stdin.take().unwrap())); + let stdout = child.stdout.take().unwrap(); + let stderr = child.stderr.take().unwrap(); + + let secret1 = secret.clone(); + let stdin1 = stdin.clone(); + + let stdout_handle = tokio::spawn(async move { + let reader = BufReader::new(stdout); + for line in reader.lines().map_while(Result::ok) { + if line.to_lowercase().contains("password") { + let stdin = stdin1.clone(); + let secret = secret1.clone(); + tokio::spawn(async move { + tokio::time::sleep(Duration::from_millis(100)).await; + if let Ok(mut stdin) = stdin.write() { + stdin.write_all(secret.as_bytes()).unwrap(); + stdin.flush().unwrap(); + } + }); + } + } + }); + + tokio::spawn(async move { + let reader = BufReader::new(stderr); + for line in reader.lines().map_while(Result::ok) { + if line.to_lowercase().contains("password") { + let stdin = stdin.clone(); + let secret = secret.clone(); + tokio::spawn(async move { + tokio::time::sleep(Duration::from_millis(100)).await; + if let Ok(mut stdin) = stdin.write() { + stdin.write_all(secret.as_bytes()).unwrap(); + stdin.flush().unwrap(); + } + }); + } + } + }).await?; + + let _ = stdout_handle.await; + + match child.wait_with_output() { Ok(output) if output.status.success() => { self.status.stdout = String::from_utf8(output.stdout)?; self.status.stderr = String::from_utf8(output.stderr)?; @@ -265,9 +334,12 @@ mod tests { ); } - #[test] - fn error_propagation() { + #[tokio::test] + async fn error_propagation() { let mut command_run = new_run_command(String::from("non-existant-command")); - command_run.execute().expect_err("Command should fail"); + command_run + .execute(None) + .await + .expect_err("Command should fail"); } } diff --git a/lib/src/atoms/directory/create.rs b/lib/src/atoms/directory/create.rs index a572fd46..0a1fbfe1 100644 --- a/lib/src/atoms/directory/create.rs +++ b/lib/src/atoms/directory/create.rs @@ -1,4 +1,5 @@ use crate::atoms::Outcome; +use crate::utilities::password_manager::PasswordManager; use super::super::Atom; use std::path::PathBuf; @@ -17,6 +18,7 @@ impl std::fmt::Display for Create { } } +#[async_trait::async_trait] impl Atom for Create { fn plan(&self) -> anyhow::Result { Ok(Outcome { @@ -25,7 +27,7 @@ impl Atom for Create { }) } - fn execute(&mut self) -> anyhow::Result<()> { + async fn execute(&mut self, _: Option) -> anyhow::Result<()> { std::fs::create_dir_all(&self.path)?; Ok(()) @@ -51,8 +53,8 @@ mod tests { assert_eq!(false, atom.plan().unwrap().should_run); } - #[test] - fn it_can_execute() { + #[tokio::test] + async fn it_can_execute() { let temp_dir = match tempfile::tempdir() { std::result::Result::Ok(dir) => dir, std::result::Result::Err(_) => { @@ -67,7 +69,7 @@ mod tests { assert_eq!(false, temp_dir.path().join("create-me").exists()); - assert_eq!(true, atom.execute().is_ok()); + assert_eq!(true, atom.execute(None).await.is_ok()); assert_eq!(false, atom.plan().unwrap().should_run); assert_eq!(true, temp_dir.path().join("create-me").is_dir()); diff --git a/lib/src/atoms/directory/remove.rs b/lib/src/atoms/directory/remove.rs index 933b1af5..b27eb0d2 100644 --- a/lib/src/atoms/directory/remove.rs +++ b/lib/src/atoms/directory/remove.rs @@ -1,3 +1,4 @@ +use crate::utilities::password_manager::PasswordManager; use std::path::PathBuf; use tracing::error; @@ -18,6 +19,7 @@ impl std::fmt::Display for Remove { } } +#[async_trait::async_trait] impl Atom for Remove { fn plan(&self) -> anyhow::Result { let path_to_dir = PathBuf::from(&self.target); @@ -53,7 +55,7 @@ impl Atom for Remove { }) } - fn execute(&mut self) -> anyhow::Result<()> { + async fn execute(&mut self, _: Option) -> anyhow::Result<()> { std::fs::remove_dir(&self.target)?; Ok(()) } @@ -91,8 +93,8 @@ mod tests { assert_eq!(false, atom.plan().unwrap().should_run) } - #[test] - fn it_can_execute() { + #[tokio::test] + async fn it_can_execute() { let temp_dir = match tempfile::tempdir() { std::result::Result::Ok(dir) => dir, std::result::Result::Err(_) => { @@ -110,7 +112,7 @@ mod tests { }; // Deletes dir - assert_eq!(true, atom.execute().is_ok()); + assert_eq!(true, atom.execute(None).await.is_ok()); // Dir is deleted and dont exists assert_eq!(false, temp_dir.path().exists()) } diff --git a/lib/src/atoms/file/chmod.rs b/lib/src/atoms/file/chmod.rs index 2fb040b8..ddf040e0 100644 --- a/lib/src/atoms/file/chmod.rs +++ b/lib/src/atoms/file/chmod.rs @@ -1,4 +1,5 @@ use crate::atoms::Outcome; +use crate::utilities::password_manager::PasswordManager; use super::super::Atom; use super::FileAtom; @@ -30,6 +31,7 @@ impl std::fmt::Display for Chmod { use {std::os::unix::prelude::PermissionsExt, tracing::error}; #[cfg(unix)] +#[async_trait::async_trait] impl Atom for Chmod { fn plan(&self) -> anyhow::Result { // If the file doesn't exist, assume it's because @@ -67,7 +69,7 @@ impl Atom for Chmod { }) } - fn execute(&mut self) -> anyhow::Result<()> { + async fn execute(&mut self, _: Option) -> anyhow::Result<()> { std::fs::set_permissions( self.path.as_path(), std::fs::Permissions::from_mode(self.mode), @@ -78,6 +80,7 @@ impl Atom for Chmod { } #[cfg(not(unix))] +#[async_trait] impl Atom for Chmod { fn plan(&self) -> anyhow::Result { // Never run @@ -140,8 +143,8 @@ mod tests { assert_eq!(true, file_chmod.plan().unwrap().should_run); } - #[test] - fn it_can_execute() { + #[tokio::test] + async fn it_can_execute() { let temp_dir = match tempfile::tempdir() { std::result::Result::Ok(dir) => dir, std::result::Result::Err(_) => { @@ -180,7 +183,7 @@ mod tests { }; assert_eq!(true, file_chmod.plan().unwrap().should_run); - assert_eq!(true, file_chmod.execute().is_ok()); + assert_eq!(true, file_chmod.execute(None).await.is_ok()); assert_eq!(false, file_chmod.plan().unwrap().should_run); } } diff --git a/lib/src/atoms/file/chown.rs b/lib/src/atoms/file/chown.rs index d57dd957..41a8d361 100644 --- a/lib/src/atoms/file/chown.rs +++ b/lib/src/atoms/file/chown.rs @@ -1,4 +1,5 @@ use crate::atoms::Outcome; +use crate::utilities::password_manager::PasswordManager; use super::super::Atom; use super::FileAtom; @@ -39,6 +40,7 @@ use std::os::unix::prelude::MetadataExt; use file_owner::PathExt; #[cfg(unix)] +#[async_trait::async_trait] impl Atom for Chown { fn plan(&self) -> anyhow::Result { // If the file doesn't exist, assume it's because @@ -122,7 +124,7 @@ impl Atom for Chown { }) } - fn execute(&mut self) -> anyhow::Result<()> { + async fn execute(&mut self, _: Option) -> anyhow::Result<()> { if !self.owner.is_empty() { self.path.set_owner(self.owner.as_str())?; } diff --git a/lib/src/atoms/file/contents.rs b/lib/src/atoms/file/contents.rs index aa6eab0b..bb9c3d43 100644 --- a/lib/src/atoms/file/contents.rs +++ b/lib/src/atoms/file/contents.rs @@ -1,4 +1,5 @@ use crate::atoms::Outcome; +use crate::utilities::password_manager::PasswordManager; use super::super::Atom; use super::FileAtom; @@ -26,6 +27,7 @@ impl std::fmt::Display for SetContents { } } +#[async_trait::async_trait] impl Atom for SetContents { fn plan(&self) -> anyhow::Result { // If the file doesn't exist, assume it's because @@ -58,7 +60,7 @@ impl Atom for SetContents { }) } - fn execute(&mut self) -> anyhow::Result<()> { + async fn execute(&mut self, _: Option) -> anyhow::Result<()> { std::fs::write(&self.path, &self.contents)?; Ok(()) @@ -70,8 +72,8 @@ mod tests { use super::*; use pretty_assertions::assert_eq; - #[test] - fn it_can() { + #[tokio::test] + async fn it_can() { let file = match tempfile::NamedTempFile::new() { std::result::Result::Ok(file) => file, std::result::Result::Err(_) => { @@ -93,7 +95,7 @@ mod tests { }; assert_eq!(true, file_contents.plan().unwrap().should_run); - assert_eq!(true, file_contents.execute().is_ok()); + assert_eq!(true, file_contents.execute(None).await.is_ok()); assert_eq!(false, file_contents.plan().unwrap().should_run); } } diff --git a/lib/src/atoms/file/copy.rs b/lib/src/atoms/file/copy.rs index 36e9af13..909d85fd 100644 --- a/lib/src/atoms/file/copy.rs +++ b/lib/src/atoms/file/copy.rs @@ -1,4 +1,5 @@ use crate::atoms::Outcome; +use crate::utilities::password_manager::PasswordManager; use super::super::Atom; use super::FileAtom; @@ -28,6 +29,7 @@ impl std::fmt::Display for Copy { } } +#[async_trait::async_trait] impl Atom for Copy { fn plan(&self) -> anyhow::Result { if !self.to.is_file() { @@ -48,7 +50,7 @@ impl Atom for Copy { }); } - fn execute(&mut self) -> anyhow::Result<()> { + async fn execute(&mut self, _: Option) -> anyhow::Result<()> { std::fs::copy(&self.from, &self.to)?; Ok(()) @@ -103,8 +105,8 @@ mod tests { assert_eq!(false, file_copy.plan().unwrap().should_run); } - #[test] - fn it_can_execute() { + #[tokio::test] + async fn it_can_execute() { use std::io::Write; let to_file = match tempfile::NamedTempFile::new() { @@ -138,12 +140,12 @@ mod tests { }; assert_eq!(true, file_copy.plan().unwrap().should_run); - assert_eq!(true, file_copy.execute().is_ok()); + assert_eq!(true, file_copy.execute(None).await.is_ok()); assert_eq!(false, file_copy.plan().unwrap().should_run); } - #[test] - fn it_wont_destroy_directories() { + #[tokio::test] + async fn it_wont_destroy_directories() { let to = match tempfile::TempDir::new() { std::result::Result::Ok(dir) => dir, std::result::Result::Err(_) => { diff --git a/lib/src/atoms/file/create.rs b/lib/src/atoms/file/create.rs index 50456aec..12ed5f44 100644 --- a/lib/src/atoms/file/create.rs +++ b/lib/src/atoms/file/create.rs @@ -2,6 +2,7 @@ use crate::atoms::Outcome; use super::super::Atom; use super::FileAtom; +use crate::utilities::password_manager::PasswordManager; use std::path::PathBuf; pub struct Create { @@ -20,6 +21,7 @@ impl std::fmt::Display for Create { } } +#[async_trait::async_trait] impl Atom for Create { fn plan(&self) -> anyhow::Result { Ok(Outcome { @@ -28,7 +30,7 @@ impl Atom for Create { }) } - fn execute(&mut self) -> anyhow::Result<()> { + async fn execute(&mut self, _: Option) -> anyhow::Result<()> { std::fs::File::create(&self.path)?; Ok(()) @@ -63,8 +65,8 @@ mod tests { assert_eq!(false, file_create.plan().unwrap().should_run); } - #[test] - fn it_can_execute() { + #[tokio::test] + async fn it_can_execute() { let temp_dir = match tempfile::tempdir() { std::result::Result::Ok(dir) => dir, std::result::Result::Err(_) => { @@ -77,7 +79,7 @@ mod tests { path: temp_dir.path().join("create-me"), }; - assert_eq!(true, file_create.execute().is_ok()); + assert_eq!(true, file_create.execute(None).await.is_ok()); assert_eq!(false, file_create.plan().unwrap().should_run); } } diff --git a/lib/src/atoms/file/decrypt.rs b/lib/src/atoms/file/decrypt.rs index 29f13647..ae7ff311 100644 --- a/lib/src/atoms/file/decrypt.rs +++ b/lib/src/atoms/file/decrypt.rs @@ -2,6 +2,7 @@ use crate::atoms::Outcome; use super::super::Atom; use super::FileAtom; +use crate::utilities::password_manager::PasswordManager; use age::armor::ArmoredReader; use age::secrecy::Secret; use std::io::Read; @@ -30,6 +31,7 @@ impl std::fmt::Display for Decrypt { } } +#[async_trait::async_trait] impl Atom for Decrypt { fn plan(&self) -> anyhow::Result { // If the file doesn't exist, assume it's because @@ -62,7 +64,7 @@ impl Atom for Decrypt { } } - fn execute(&mut self) -> anyhow::Result<()> { + async fn execute(&mut self, _: Option) -> anyhow::Result<()> { let decrypted_content = decrypt(&self.passphrase, &self.encrypted_content)?; std::fs::write(&self.path, decrypted_content)?; @@ -127,8 +129,8 @@ mod tests { Ok(()) } - #[test] - fn it_can_execute() -> anyhow::Result<()> { + #[tokio::test] + async fn it_can_execute() -> anyhow::Result<()> { // encrypt and write to file let passphrase = "Teal'c".to_string(); let content = b"Shol'va"; @@ -147,7 +149,7 @@ mod tests { // plan, execute assert_eq!(true, decrypt.plan().unwrap().should_run); - assert_eq!(true, decrypt.execute().is_ok()); + assert_eq!(true, decrypt.execute(None).await.is_ok()); Ok(()) } diff --git a/lib/src/atoms/file/link.rs b/lib/src/atoms/file/link.rs index 8b1c9a0e..45087d26 100644 --- a/lib/src/atoms/file/link.rs +++ b/lib/src/atoms/file/link.rs @@ -2,6 +2,7 @@ use crate::atoms::Outcome; use super::super::Atom; use super::FileAtom; +use crate::utilities::password_manager::PasswordManager; use std::path::PathBuf; use tracing::{error, warn}; @@ -27,6 +28,7 @@ impl std::fmt::Display for Link { } } +#[async_trait::async_trait] impl Atom for Link { fn plan(&self) -> anyhow::Result { // First, ensure source exists and can be linked to @@ -83,14 +85,14 @@ impl Atom for Link { } #[cfg(unix)] - fn execute(&mut self) -> anyhow::Result<()> { + async fn execute(&mut self, _: Option) -> anyhow::Result<()> { std::os::unix::fs::symlink(&self.source, &self.target)?; Ok(()) } #[cfg(windows)] - fn execute(&mut self) -> anyhow::Result<()> { + async fn execute(&mut self) -> anyhow::Result<()> { if self.target.is_dir() { std::os::windows::fs::symlink_dir(&self.source, &self.target)?; } else { @@ -106,8 +108,8 @@ mod tests { use super::*; use pretty_assertions::assert_eq; - #[test] - fn it_can() { + #[tokio::test] + async fn it_can() { let from_dir = match tempfile::tempdir() { Ok(dir) => dir, Err(_) => { @@ -129,7 +131,7 @@ mod tests { source: to_file.path().to_path_buf(), }; assert_eq!(true, atom.plan().unwrap().should_run); - assert_eq!(true, atom.execute().is_ok()); + assert_eq!(true, atom.execute(None).await.is_ok()); assert_eq!(false, atom.plan().unwrap().should_run); } } diff --git a/lib/src/atoms/file/remove.rs b/lib/src/atoms/file/remove.rs index 24fcd45f..f3bda375 100644 --- a/lib/src/atoms/file/remove.rs +++ b/lib/src/atoms/file/remove.rs @@ -3,6 +3,7 @@ use std::path::PathBuf; use tracing::error; use crate::atoms::{Atom, Outcome}; +use crate::utilities::password_manager::PasswordManager; use super::FileAtom; @@ -22,6 +23,7 @@ impl std::fmt::Display for Remove { } } +#[async_trait::async_trait] impl Atom for Remove { fn plan(&self) -> anyhow::Result { if !self.target.is_file() { @@ -71,7 +73,7 @@ impl Atom for Remove { }) } - fn execute(&mut self) -> anyhow::Result<()> { + async fn execute(&mut self, _: Option) -> anyhow::Result<()> { std::fs::remove_file(&self.target)?; Ok(()) } @@ -99,8 +101,8 @@ mod tests { assert_eq!(true, file_remove.plan().unwrap().should_run) } - #[test] - fn it_can_execute() { + #[tokio::test] + async fn it_can_execute() { let target_file = match tempfile::NamedTempFile::new() { std::result::Result::Ok(file) => file, std::result::Result::Err(_) => { @@ -114,7 +116,7 @@ mod tests { }; assert_eq!(true, file_remove.plan().unwrap().should_run); - assert_eq!(true, file_remove.execute().is_ok()); + assert_eq!(true, file_remove.execute(None).await.is_ok()); assert_eq!(false, file_remove.plan().unwrap().should_run) } } diff --git a/lib/src/atoms/file/unarchive.rs b/lib/src/atoms/file/unarchive.rs index 5c2132a0..43b11a5f 100644 --- a/lib/src/atoms/file/unarchive.rs +++ b/lib/src/atoms/file/unarchive.rs @@ -4,6 +4,7 @@ use flate2::read::GzDecoder; use tar::Archive; use crate::atoms::{Atom, Outcome}; +use crate::utilities::password_manager::PasswordManager; use super::FileAtom; @@ -19,6 +20,7 @@ impl FileAtom for Unarchive { } } +#[async_trait::async_trait] impl Atom for Unarchive { // Determine if this atom needs to run fn plan(&self) -> anyhow::Result { @@ -42,7 +44,7 @@ impl Atom for Unarchive { } // Apply new to old - fn execute(&mut self) -> anyhow::Result<()> { + async fn execute(&mut self, _: Option) -> anyhow::Result<()> { let tar_gz = File::open(&self.origin)?; let tar = GzDecoder::new(tar_gz); let mut archive = Archive::new(tar); diff --git a/lib/src/atoms/git/clone.rs b/lib/src/atoms/git/clone.rs index f6da3367..8713b4f2 100644 --- a/lib/src/atoms/git/clone.rs +++ b/lib/src/atoms/git/clone.rs @@ -1,5 +1,6 @@ use super::super::Atom; use crate::atoms::Outcome; +use crate::utilities::password_manager::PasswordManager; use gix::interrupt; use gix::{progress::Discard, Url}; use std::path::PathBuf; @@ -22,6 +23,7 @@ impl std::fmt::Display for Clone { } } +#[async_trait::async_trait] impl Atom for Clone { #[instrument(name = "git.clone.plan", level = "info", skip(self))] fn plan(&self) -> anyhow::Result { @@ -32,7 +34,7 @@ impl Atom for Clone { } #[instrument(name = "git.clone.execute", level = "info", skip(self))] - fn execute(&mut self) -> anyhow::Result<()> { + async fn execute(&mut self, _: Option) -> anyhow::Result<()> { unsafe { interrupt::init_handler(1, || {})?; }; diff --git a/lib/src/atoms/http/download.rs b/lib/src/atoms/http/download.rs index 03a43728..0f74bd06 100644 --- a/lib/src/atoms/http/download.rs +++ b/lib/src/atoms/http/download.rs @@ -1,4 +1,5 @@ use crate::atoms::Outcome; +use crate::utilities::password_manager::PasswordManager; use super::super::Atom; use std::io::Write; @@ -15,6 +16,7 @@ impl std::fmt::Display for Download { } } +#[async_trait::async_trait] impl Atom for Download { fn plan(&self) -> anyhow::Result { // Initial implementation will return false if the local file @@ -27,7 +29,7 @@ impl Atom for Download { }) } - fn execute(&mut self) -> anyhow::Result<()> { + async fn execute(&mut self, _: Option) -> anyhow::Result<()> { let response = reqwest::blocking::get(&self.url)?; let mut file = File::create(&self.to)?; @@ -45,8 +47,8 @@ mod tests { use pretty_assertions::assert_eq; use tempfile::tempdir; - #[test] - fn it_can() { + #[tokio::test] + async fn it_can() { let tmpdir = tempdir().unwrap(); let to_file = tmpdir.path().join("download"); @@ -57,7 +59,7 @@ mod tests { assert_eq!(true, atom.plan().unwrap().should_run); - let result = atom.execute(); + let result = atom.execute(PasswordManager::new(None).ok()).await; assert_eq!(true, result.is_ok()); assert_eq!(false, atom.plan().unwrap().should_run); } diff --git a/lib/src/atoms/mod.rs b/lib/src/atoms/mod.rs index 7a0e2494..6d24cd2b 100644 --- a/lib/src/atoms/mod.rs +++ b/lib/src/atoms/mod.rs @@ -4,6 +4,8 @@ pub mod file; pub mod git; pub mod http; +use crate::utilities::password_manager::PasswordManager; + pub enum SideEffect {} pub struct Outcome { @@ -11,16 +13,12 @@ pub struct Outcome { pub should_run: bool, } -pub trait Atom: std::fmt::Display { - // Determine if this atom needs to run +#[async_trait::async_trait] +pub trait Atom: std::fmt::Display + Send + Sync { fn plan(&self) -> anyhow::Result; - // Apply new to old - fn execute(&mut self) -> anyhow::Result<()>; + async fn execute(&mut self, _: Option) -> anyhow::Result<()>; - // These methods allow for finalizers to query the outcome of the Atom. - // We'll provide default implementations to allow Atoms to opt in to - // the queries that make sense for them fn output_string(&self) -> String { String::from("") } @@ -36,6 +34,7 @@ pub trait Atom: std::fmt::Display { pub struct Echo(pub &'static str); +#[async_trait::async_trait] impl Atom for Echo { fn plan(&self) -> anyhow::Result { Ok(Outcome { @@ -44,7 +43,7 @@ impl Atom for Echo { }) } - fn execute(&mut self) -> anyhow::Result<()> { + async fn execute(&mut self, _: Option) -> anyhow::Result<()> { Ok(()) } diff --git a/lib/src/lib.rs b/lib/src/lib.rs index bd9b2587..ada12c85 100644 --- a/lib/src/lib.rs +++ b/lib/src/lib.rs @@ -5,5 +5,5 @@ pub mod contexts; pub mod manifests; pub mod steps; pub mod tera_functions; -mod utilities; +pub mod utilities; pub mod values; diff --git a/lib/src/manifests/mod.rs b/lib/src/manifests/mod.rs index 753bd1e7..9554e7cb 100644 --- a/lib/src/manifests/mod.rs +++ b/lib/src/manifests/mod.rs @@ -7,6 +7,7 @@ pub use providers::register_providers; pub use providers::ManifestProvider; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; +use std::fmt; use std::path::{Path, PathBuf}; use tracing::error; @@ -35,6 +36,22 @@ pub struct Manifest { pub dag_index: Option>, } +impl fmt::Display for Manifest { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!( + f, + "{}", + self.name.as_deref().unwrap_or("Cannot extract name") + ) + } +} + +impl AsRef for Manifest { + fn as_ref(&self) -> &Manifest { + self + } +} + pub fn resolve(uri: &String) -> Option { let manifest_directory = register_providers() .into_iter() diff --git a/lib/src/steps/finalizers/mod.rs b/lib/src/steps/finalizers/mod.rs index c0ee2b92..1ac2800c 100644 --- a/lib/src/steps/finalizers/mod.rs +++ b/lib/src/steps/finalizers/mod.rs @@ -14,7 +14,7 @@ pub enum FlowControl { /// Finalizers allow us to store data within the manifests KV store, /// or to end the execution of atoms for the action -pub trait Finalizer { +pub trait Finalizer: Send + Sync { fn finalize(&self, atom: &dyn Atom) -> anyhow::Result; } diff --git a/lib/src/steps/mod.rs b/lib/src/steps/mod.rs index ae2e80ec..d89c211d 100644 --- a/lib/src/steps/mod.rs +++ b/lib/src/steps/mod.rs @@ -16,70 +16,42 @@ impl Step { self.initializers .iter() .fold(true, |_, flow_control| match flow_control { - initializers::FlowControl::Ensure(i) => { - i.initialize().unwrap_or_else(|err| { + initializers::FlowControl::Ensure(i) => i.initialize().unwrap_or_else(|err| { + error!("Failed to run initializer: {}", err.to_string()); + false + }), + + initializers::FlowControl::SkipIf(i) => match i.initialize() { + Ok(true) => false, + Ok(false) => true, + Err(err) => { error!("Failed to run initializer: {}", err.to_string()); - - // On an error, we can't really determine if this Atom should - // run; so lets play it safe and filter it out too false - }) - } - - initializers::FlowControl::SkipIf(i) => { - match i.initialize() { - Ok(true) => { - // Returning false because we should Skip if true, so false - // will filter this out of the atom list - false - } - Ok(false) => true, - Err(err) => { - error!("Failed to run initializer: {}", err.to_string()); - - // On an error, we can't really determine if this Atom should - // run; so lets play it safe and filter it out too - false - } } - } + }, }) } - pub fn do_finalizers_allow_us_to_continue(&self) -> bool { + pub fn do_finalizers_allow_us_to_continue(&mut self) -> bool { self.finalizers .iter() .fold(true, |_, flow_control| match flow_control { - finalizers::FlowControl::StopIf(i) => { - match i.finalize(self.atom.as_ref()) { - Ok(true) => { - // Returning false because we should Skip if true, so false - // will filter this out of the atom list - false - } - Ok(false) => true, - Err(err) => { - error!("Failed to run finalizers: {}", err.to_string()); - - // On an error, we can't really determine if this Atom should - // run; so lets play it safe and filter it out too - false - } + finalizers::FlowControl::StopIf(i) => match i.finalize(self.atom.as_ref()) { + Ok(true) => false, + Ok(false) => true, + Err(err) => { + error!("Failed to run finalizers: {}", err.to_string()); + false } - } - FlowControl::Ensure(i) => { - match i.finalize(self.atom.as_ref()) { - Ok(true) => true, - Ok(false) => false, - Err(err) => { - error!("Failed to run finalizers: {}", err.to_string()); - - // On an error, we can't really determine if this Atom should - // run; so lets play it safe and filter it out too - false - } + }, + FlowControl::Ensure(i) => match i.finalize(self.atom.as_ref()) { + Ok(true) => true, + Ok(false) => false, + Err(err) => { + error!("Failed to run finalizers: {}", err.to_string()); + false } - } + }, }) } } @@ -175,7 +147,7 @@ mod tests { #[test] fn finalizers_can_control_execution() { - let step = Step { + let mut step = Step { atom: Box::new(EchoAtom("hello-world")), initializers: vec![], finalizers: vec![FinalizerFlowControl::StopIf(Box::new(EchoFinalizer(false)))], @@ -183,7 +155,7 @@ mod tests { assert_eq!(true, step.do_finalizers_allow_us_to_continue()); - let step = Step { + let mut step = Step { atom: Box::new(EchoAtom("hello-world")), initializers: vec![], finalizers: vec![FinalizerFlowControl::StopIf(Box::new(EchoFinalizer(true)))], @@ -191,7 +163,7 @@ mod tests { assert_eq!(false, step.do_finalizers_allow_us_to_continue()); - let step = Step { + let mut step = Step { atom: Box::new(EchoAtom("hello-world")), initializers: vec![], finalizers: vec![ @@ -205,7 +177,7 @@ mod tests { #[test] fn finalizers_that_error_block_execution() { - let step = Step { + let mut step = Step { atom: Box::new(EchoAtom("hello-world")), initializers: vec![], finalizers: vec![FinalizerFlowControl::StopIf(Box::new(ErrorFinalizer()))], @@ -213,7 +185,7 @@ mod tests { assert_eq!(false, step.do_finalizers_allow_us_to_continue()); - let step = Step { + let mut step = Step { atom: Box::new(EchoAtom("hello-world")), initializers: vec![], finalizers: vec![ diff --git a/lib/src/utilities/mod.rs b/lib/src/utilities/mod.rs index 6b935c24..f7cdbe10 100644 --- a/lib/src/utilities/mod.rs +++ b/lib/src/utilities/mod.rs @@ -1,10 +1,10 @@ +pub mod password_manager; + use crate::contexts::Contexts; -use which; +use which::which; pub fn get_binary_path(binary: &str) -> Result { - let binary = which::which(String::from(binary))? - .to_string_lossy() - .to_string(); + let binary = which(String::from(binary))?.to_string_lossy().to_string(); Ok(binary) } diff --git a/lib/src/utilities/password_manager.rs b/lib/src/utilities/password_manager.rs new file mode 100644 index 00000000..931fd9aa --- /dev/null +++ b/lib/src/utilities/password_manager.rs @@ -0,0 +1,68 @@ +use std::{process::Stdio, sync::Arc}; + +use anyhow::Result; +use rpassword::prompt_password; +use tokio::{ + io::AsyncWriteExt, + process::Command, + time::{interval, Duration}, +}; +use tracing::error; +use zeroize::{Zeroize, ZeroizeOnDrop, Zeroizing}; + +#[derive(Debug, Zeroize, ZeroizeOnDrop, Clone)] +pub struct PasswordManager { + privilege_provider: String, + #[cfg(unix)] + pub secret: Option>, + #[cfg(windows)] + token: String, +} + +impl PasswordManager { + pub fn new(package_providor: Option<&str>) -> Result { + let this = Self { + privilege_provider: package_providor.unwrap_or_default().to_string(), + #[cfg(unix)] + secret: None, + #[cfg(windows)] + token: String::new(), + }; + + Ok(this) + } + + #[cfg(target_os = "linux")] + pub async fn prompt(&mut self, prompt: &str) -> Result<()> { + self.secret = Some(Zeroizing::new(prompt_password(prompt)?)); + self.keep_elevated().await; + Ok(()) + } + + async fn keep_elevated(&self) { + let shared_self = Arc::new(self.clone()); + let mut wait = interval(Duration::from_secs(60 * 10)); + tokio::spawn(async move { + let this = Arc::clone(&shared_self); + let mut command = Command::new(this.privilege_provider.clone()) + .arg("-S") // Read password from stdin + .arg("-v") // Validate and update timestamp + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .expect("Failed to refresh sudo"); + + if let (Some(mut stdin), Some(secret)) = (command.stdin.take(), this.secret.clone()) { + stdin + .write_all(format!("{}\n", secret.as_str()).as_bytes()) + .await + .unwrap(); + } else { + error!("Unable to elevate privilege") + } + + wait.tick().await; + }); + } +} From 847c61a884c153c76deab26ee2acb9f0b4a5bb00 Mon Sep 17 00:00:00 2001 From: Justin Bacher <92jbach@gmail.com> Date: Sat, 25 Jan 2025 13:54:35 -0500 Subject: [PATCH 2/8] using barriers to manage depends --- app/Cargo.toml | 6 +- app/src/commands/apply.rs | 369 ++++++++-------------- app/src/utils/dependency_graph.rs | 178 +++++++++++ app/src/utils/mod.rs | 28 ++ lib/Cargo.toml | 1 + lib/src/actions/mod.rs | 47 ++- lib/src/actions/package/install.rs | 2 +- lib/src/actions/package/providers/paru.rs | 3 +- lib/src/atoms/command/exec.rs | 168 ++++------ lib/src/manifests/mod.rs | 99 +++++- lib/src/steps/finalizers/mod.rs | 4 +- lib/src/steps/initializers/mod.rs | 4 +- lib/src/steps/mod.rs | 18 +- 13 files changed, 581 insertions(+), 346 deletions(-) create mode 100644 app/src/utils/dependency_graph.rs diff --git a/app/Cargo.toml b/app/Cargo.toml index 83221e7f..31ce0e13 100644 --- a/app/Cargo.toml +++ b/app/Cargo.toml @@ -15,6 +15,7 @@ comfy-table = "7" comtrya-lib = { path = "../lib", version = "0.9.1" } petgraph = "0.6" rhai = { version = "1.19", features = ["serde"] } +backoff = { version = "0.4", features = ["tokio"] } strip-ansi-escapes = "0.2" tracing = "0.1" tracing-journald = "0.3.0" @@ -24,10 +25,7 @@ dirs-next = "2.0" serde = { version = "1.0", features = ["derive"] } serde_yml = "0" tokio = { version = "1.43.0", features = ["full"] } -crossbeam-queue = "0.3.12" -yapp = "0.4.1" -zeroize = { version = "1.8.1", features = ["derive", "std"] } -rpassword = "7.3.1" +scopeguard = "1.2.0" [dev-dependencies] assert_cmd = "2.0" diff --git a/app/src/commands/apply.rs b/app/src/commands/apply.rs index 6b37d243..0c79c0b6 100644 --- a/app/src/commands/apply.rs +++ b/app/src/commands/apply.rs @@ -1,30 +1,25 @@ use std::{ collections::HashMap, - ops::Deref, path::PathBuf, - sync::{Arc, RwLock}, - thread::available_parallelism, + sync::{ + atomic::{AtomicBool, Ordering}, + Arc, + }, }; use anyhow::{anyhow, Context, Result}; use clap::Parser; use comfy_table::{Cell, ContentArrangement, Table}; -use petgraph::{ - stable_graph::{NodeIndex, StableDiGraph}, - Direction::*, +use petgraph::{graph::NodeIndex, Direction::*}; +use tokio::{ + sync::{Barrier, RwLock}, + task::JoinSet, }; -use rhai::Engine; -use tokio::task::JoinSet; -use tracing::{debug, error, info, info_span, instrument, trace, warn}; +use tracing::{debug, info, instrument, trace}; use super::ComtryaCommand; -use crate::{utils::ZipLongest, Runtime}; -use comtrya_lib::{ - actions::Actions::{CommandRun, PackageInstall, PackageRepository}, - contexts::to_rhai, - manifests::{load, Manifest}, - utilities::{get_privilege_provider, password_manager::PasswordManager}, -}; +use crate::{utils::DependencyGraph, Runtime}; +use comtrya_lib::manifests::load; #[derive(Parser, Debug, Clone)] pub(crate) struct Apply { @@ -42,66 +37,6 @@ pub(crate) struct Apply { pub label: Option, } -#[derive(Debug)] -struct DependenceyGraph { - graph: StableDiGraph, ()>, - name_to_idx: HashMap, NodeIndex>, - should_ask_for_pass: bool, -} - -impl DependenceyGraph { - pub fn new(manifests: HashMap) -> Self { - let mut this = Self { - graph: StableDiGraph::new(), - name_to_idx: HashMap::new(), - should_ask_for_pass: false, - }; - - let mut first_package_install: Option = None; - - for (_, manifest) in manifests.iter() { - let from_index = this.add_manifest(manifest); - - - for (dependant, action) in manifest.depends.iter().zip_longest(manifest.actions.iter()) - { - if let Some(dependency_manifest) = dependant.and_then(|dep| manifests.get(dep)) { - let to_index = this.add_manifest(dependency_manifest); - this.graph.add_edge(from_index, to_index, ()); - } - - if !this.should_ask_for_pass { - this.should_ask_for_pass = match action { - Some(CommandRun(cva)) => cva.action.privileged, - Some(PackageInstall(_)) | Some(PackageRepository(_)) => { - if let Some(index) = first_package_install { - this.graph.add_edge(from_index, index, ()); - } else { - first_package_install = Some(from_index); - } - true - }, - _ => false, - }; - } - } - } - - this - } - - pub fn add_manifest(&mut self, manifest: &Manifest) -> NodeIndex { - if let Some(&idx) = self.name_to_idx.get(&manifest.name) { - idx - } else { - let m = Arc::new(manifest.to_owned()); - let idx = self.graph.add_node(m); - self.name_to_idx.insert(manifest.name.to_owned(), idx); - idx - } - } -} - impl Apply { fn manifest_path(&self, runtime: &Runtime) -> anyhow::Result { for manifest in &self.manifests { @@ -120,7 +55,7 @@ impl Apply { "Manifest location, {first_manifest_path:?}, could be resolved", ))?; - trace!(manifests = self.manifests.join(",").deref()); + trace!(manifests = self.manifests.join(",")); Ok(manifest_path) } @@ -151,178 +86,150 @@ impl Apply { Ok(()) } +} - #[instrument(skip(self, runtime, manifest))] - async fn process_manifest( - &self, - runtime: &Arc, - manifest: &Arc, - ) -> Result<()> { - if let Some(label) = self.label.as_ref() { - if !manifest.labels.contains(label) { - info!( - message = "Skipping manifest, label not found", - label = label.as_str() - ); - return Ok(()); - } - } +impl ComtryaCommand for Apply { + // #[instrument(skip(self, runtime))] + async fn execute(&self, runtime: &mut Runtime) -> Result<()> { + let contexts = runtime.contexts.clone(); + let manifest_path = self.manifest_path(runtime)?; - if let Some(where_condition) = &manifest.r#where { - let engine = Engine::new(); - let mut scope = to_rhai(&runtime.contexts); - - match engine.eval_with_scope::(&mut scope, where_condition) - { - Ok(result) => { - debug!("Result of 'where' condition '{where_condition}' -> '{result}'",); - } - Err(err) => { - warn!("'where' condition '{where_condition}' failed: {err}"); - info!("Skipping manifest, because 'where' conditions were false!"); - return Ok(()); - } - }; - } + debug!("Load manifests from path: {manifest_path:#?}"); - for action in manifest.actions.iter() { - let span_action = info_span!("", %action).entered(); - let action = action.inner_ref(); - - let mut steps = match action.plan(manifest, &runtime.contexts) { - Ok(steps) => steps - .into_iter() - .filter(|step| step.do_initializers_allow_us_to_run()) - .filter(|step| match step.atom.plan() { - Ok(outcome) => outcome.should_run, - Err(_) => false, - }) - .peekable(), - Err(err) => { - error!("Failed Processing: {manifest}. Action failed to get plan: {err:?}",); - return Ok(()); - } - }; + let manifests = load(manifest_path, &contexts); - if steps.peek().is_none() { - info!("nothing to be done to reconcile action"); - span_action.exit(); - return Ok(()); - } + let manifest_manager = Arc::new(RwLock::new( + DependencyGraph::new(manifests, &contexts, runtime).await?, + )); - if self.dry_run { - return Ok(()); - } + let mut barrier_map: HashMap, Arc> = HashMap::new(); + for node in manifest_manager.read().await.graph.node_indices() { + barrier_map.insert( + node, + Arc::new(Barrier::new( + manifest_manager + .read() + .await + .graph + .neighbors_directed(node, Incoming) + .count() + .max(1), + )), + ); + } - for mut step in steps { - let password_manager = runtime.password_manager.clone(); - if let Err(err) = step.atom.execute(password_manager).await { - debug!("Atom failed to execute: {:?}", err); - break; - } + let ordered_manifests = manifest_manager.read().await.get_ordered_manifests(); + let mut workers = JoinSet::>::new(); + let dry_run = Arc::new(AtomicBool::new(self.dry_run)); + let password_manager = &runtime.password_manager; + let barrier_map = Arc::new(barrier_map); + for manifest in ordered_manifests { + // Need to clone all these because they'll be in their own threads + let dry_run = Arc::clone(&dry_run); + let label = self.label.clone(); + let contexts = contexts.clone(); + let password_manager = password_manager.clone(); + let barrier_map = Arc::clone(&barrier_map); + let manager = Arc::clone(&manifest_manager); + let barrier = Arc::clone( + barrier_map + .get(manager.read().await.get_node_from_manifest(&manifest).await) + .unwrap(), + ); - if !step.do_finalizers_allow_us_to_continue() { - debug!("Finalizers won't allow us to continue with this action"); - break; + workers.spawn(async move { + let manifest = Arc::clone(&manifest); + let manifest_guard = manifest.lock().await; + let manifest_manager = Arc::clone(&manager); + + let dependents: Vec<_> = manifest_manager + .read() + .await + .graph + .neighbors_directed( + *manifest_manager + .read() + .await + .get_node_from_manifest(&manifest) + .await, + petgraph::Outgoing, + ) + .collect(); + + for dep in dependents.iter() { + debug!( + "Dependencies for manifest {}: {:?}", + manifest_guard, + manifest_manager + .read() + .await + .graph + .node_weight(*dep) + .unwrap() + .lock() + .await + .get_name() + ); } - } - info!("{}", action.summarize()); - span_action.exit(); - } - - info!("Completed: {manifest}",); - Ok(()) - } -} -impl ComtryaCommand for Apply { - #[instrument(skip(self, runtime))] - async fn execute(&self, runtime: &mut Runtime) -> Result<()> { - let ready_queue = Arc::new(crossbeam_queue::SegQueue::new()); - let done_queue = Arc::new(crossbeam_queue::SegQueue::new()); - let manifest_manager = DependenceyGraph::new(load( - self.manifest_path(&runtime.clone())?, - &runtime.contexts, - )); + info!( + "Worker for manifest {:?} waiting on dependencies", + &manifest_guard.get_name() + ); - let contexts = runtime.clone().contexts; - if manifest_manager.should_ask_for_pass { - let mut password_manager = - PasswordManager::new(get_privilege_provider(&contexts).as_deref())?; - password_manager.prompt("Please enter password:").await?; - runtime.password_manager = Some(password_manager); - } + // let is_leader = barrier.wait().await.is_leader(); + for &dependent in &dependents { + let dependent_barrier = barrier_map.get(&dependent).unwrap(); + dependent_barrier.wait().await; + } - for idx in manifest_manager.graph.externals(Outgoing) { - if let Some(manifest) = manifest_manager.graph.node_weight(idx) { - ready_queue.push(manifest.clone()); - } - } + info!( + "Worker for manifest {:?} executing manifest", + &manifest_guard.get_name() + ); - let shared_self = Arc::new(self.clone()); - let mut workers = JoinSet::new(); - let thread_runtime = Arc::new(runtime.clone()); + manifest_guard + .execute( + dry_run.load(Ordering::Relaxed), + label.clone(), + &contexts, + password_manager.clone(), + ) + .await?; - for _ in 0..available_parallelism().map_or(1, |p| p.get() / 2) { - // Every runner needs it's own reference - let thread_runtime = Arc::clone(&thread_runtime); - let thread_ready_queue = Arc::clone(&ready_queue); - let thread_done_queue = Arc::clone(&done_queue); - let thread_self = Arc::clone(&shared_self); + info!( + "Worker for manifest {:?} finished executing", + &manifest_guard.get_name() + ); - workers.spawn(async move { - tokio::task::spawn_blocking(move || { - while let Some(manifest) = thread_ready_queue.pop() { - if let Err(e) = tokio::runtime::Handle::current().block_on(thread_self - .process_manifest(&thread_runtime, &manifest)) - { - error!("Unable to process manifest: '{manifest}'. {e}") - } else { - thread_done_queue.push(manifest.clone()); - } - } - }).await?; - Ok::<_, anyhow::Error>(()) + let has_dependees = manifest_manager + .read() + .await + .graph + .neighbors_directed( + *manifest_manager + .read() + .await + .get_node_from_manifest(&manifest) + .await, + Outgoing, + ) + .peekable() + .peek() + .is_none(); + + if has_dependees { + barrier.wait().await; + } + info!( + "Worker for manifest {:?} FINISHED executing", + &manifest_guard.get_name() + ); + Ok(()) }); } - let shared_graph = Arc::new(RwLock::new(manifest_manager.graph)); - - tokio::spawn(async move { - if let Some(completed) = done_queue.pop() { - if let Some(&idx) = manifest_manager.name_to_idx.get(&completed.name) { - { - match shared_graph.write() { - Ok(mut guard) => { - guard.remove_node(idx); - } - Err(e) => { - error!("Failed to acquire write lock: {e}"); - } - } - } - - match shared_graph.read() { - Ok(graph) => { - for idx in graph.externals(Outgoing) { - if let Some(manifest) = graph.node_weight(idx) { - ready_queue.push(Arc::clone(manifest)); - } - } - } - Err(e) => { - error!("Failed to acquire read lock: {e}"); - } - } - } - } - // println!("{:?}", shared_graph.read().unwrap()); - drop(ready_queue); - }) - .await?; - workers.join_all().await; - Ok(()) } } diff --git a/app/src/utils/dependency_graph.rs b/app/src/utils/dependency_graph.rs new file mode 100644 index 00000000..64d81f64 --- /dev/null +++ b/app/src/utils/dependency_graph.rs @@ -0,0 +1,178 @@ +use std::{collections::HashMap, iter::from_fn, ops::Deref, sync::Arc}; + +use anyhow::Result; +use petgraph::{ + graph::{DiGraph, NodeIndex}, + visit::Topo, + // Direction::*, +}; +use tokio::sync::Mutex; + +use super::ZipLongest; +use crate::Runtime; +use comtrya_lib::{ + actions::Actions::*, + contexts::Contexts, + manifests::Manifest, + utilities::{get_privilege_provider, password_manager::PasswordManager}, +}; + +#[derive(Debug)] +pub struct DependencyGraph { + pub(crate) graph: DiGraph>, ()>, + pub(crate) name_to_idx: HashMap, +} + +impl DependencyGraph { + pub async fn new( + manifests: HashMap, + contexts: &Contexts, + runtime: &mut Runtime, + ) -> Result { + let mut this = Self { + graph: DiGraph::new(), + name_to_idx: HashMap::new(), + }; + + let mut first_package_install: Option = None; + + let mut should_ask_for_pass = false; + for (_, manifest) in manifests.iter() { + let from_index = this.add_manifest(manifest).await; + + for (dependant, action) in manifest.depends.iter().zip_longest(manifest.actions.iter()) + { + if let Some(dependency_manifest) = dependant.and_then(|dep| manifests.get(dep)) { + let to_index = this.add_manifest(dependency_manifest).await; + this.graph.add_edge(from_index, to_index, ()); + } + + should_ask_for_pass = match action { + Some(PackageInstall(_)) | Some(PackageRepository(_)) => { + if let Some(index) = first_package_install { + first_package_install = first_package_install.or(Some(index)); + this.graph.add_edge(from_index, index, ()); + } + true + } + Some(CommandRun(cva)) => cva.action.privileged, + _ => false, + }; + } + } + + if should_ask_for_pass { + let mut password_manager = + PasswordManager::new(get_privilege_provider(contexts).as_deref())?; + password_manager.prompt("Please enter password:").await?; + runtime.password_manager = Some(password_manager); + } + + Ok(this) + } + + fn get_ordered_nodes(&self) -> impl Iterator + '_ { + let mut topo = Topo::new(&self.graph); + + from_fn(move || topo.next(&self.graph)) + } + + pub fn get_ordered_manifests(&self) -> Vec>> { + self.get_ordered_nodes() + .flat_map(|idx| self.graph.node_weight(idx)) + .cloned() + .collect::>() + } + + pub async fn add_manifest(&mut self, manifest: impl AsRef) -> NodeIndex { + let manifest = manifest.as_ref(); + *self + .name_to_idx + .entry(manifest.get_name()) + .or_insert_with(|| self.graph.add_node(Arc::new(Mutex::new(manifest.clone())))) + } + + #[allow(dead_code)] + async fn get_node_index(&self, manifest: M) -> &NodeIndex + where + M: AsRef> + Deref>, + { + self.name_to_idx + .get(&manifest.lock().await.get_name()) + .unwrap() + } + + // #[allow(dead_code)] + // async fn get_next_nodes( + // &self, + // manifest: Arc>, + // ) -> impl Iterator>> { + // let futures: Vec<&Arc>> = Vec::new(); + // self.graph + // .neighbors_directed(*self.get_node_index(manifest).await, Outgoing) + // .map(|idx| self.graph.node_weight(idx).unwrap()) + // .filter(|m| self.are_dependencies_completed(*self.get_node_index(Arc::clone(m)))) + // } + + // #[allow(dead_code)] + // pub async fn get_ready_dependencies( + // &mut self, + // manifest: Arc>, + // ) -> Vec<&Arc>> { + // manifest + // .lock() + // .await + // .depends + // .iter() + // .map(|dep| self.name_to_idx.get(dep).unwrap()) + // .map(|idx| self.graph.node_weight(*idx).unwrap()) + // .filter(|m| m.blocking_lock().state == ManifestState::Completed) + // .collect::>() + // } + + // #[allow(dead_code)] + // async fn dependencies_completed(&self, node: NodeIndex) -> bool { + // self.graph + // .neighbors_directed(node, Incoming) + // .flat_map(|idx| self.graph.node_weight(idx)) + // .all(|dep| dep.blocking_lock().state == ManifestState::Completed) + // } + + pub async fn get_node_from_manifest(&self, manifest: &Arc>) -> &NodeIndex { + // let join_set = JoinSet::new(); + self.name_to_idx + .get(&manifest.lock().await.get_name()) + .unwrap() + } + + // #[allow(dead_code)] + // pub fn are_dependencies_completed(&self, idx: NodeIndex) -> bool { + // self.graph.neighbors_directed(idx, Outgoing).all(|idx| { + // self.graph.node_weight(idx).unwrap().blocking_lock().state == ManifestState::Completed + // }) + // } + + // #[allow(dead_code)] + // pub fn get_ready_manifests(&self) -> impl Iterator>> { + // self.graph.node_weights().filter(|m| { + // m.blocking_lock().state == ManifestState::Pending + // && self.are_dependencies_completed( + // *self.name_to_idx.get(&m.blocking_lock().get_name()).unwrap(), + // ) + // }) + // } + + // #[allow(dead_code)] + // pub fn all_manifests_completed(&self) -> bool { + // self.graph + // .node_weights() + // .all(|m| m.blocking_lock().state == ManifestState::Completed) + // } + + // #[allow(dead_code)] + // pub fn all_manifests_queued(&self) -> bool { + // self.graph + // .node_weights() + // .all(|m| m.blocking_lock().state != ManifestState::Pending) + // } +} diff --git a/app/src/utils/mod.rs b/app/src/utils/mod.rs index d5f0df41..6f6f954d 100644 --- a/app/src/utils/mod.rs +++ b/app/src/utils/mod.rs @@ -1,4 +1,9 @@ +mod dependency_graph; +use anyhow::{anyhow, Result}; +use backoff::{future::retry, ExponentialBackoff}; +pub use dependency_graph::DependencyGraph; use std::iter::from_fn; +use tokio::time::{timeout, Duration}; pub trait ZipLongest: Iterator { fn zip_longest(self, other: I) -> impl Iterator, Option)> @@ -21,3 +26,26 @@ impl> ZipLongest for I { }) } } + +#[allow(dead_code)] +pub async fn try_acquire_lock(operation: impl Fn() -> Result) -> Result +where + T: Send + 'static, +{ + let backoff = ExponentialBackoff { + initial_interval: Duration::from_millis(10), + max_interval: Duration::from_secs(1), + max_elapsed_time: Some(Duration::from_secs(5)), + ..Default::default() + }; + + retry(backoff, || async { + match timeout(Duration::from_millis(100), async { operation() }).await { + Ok(result) => result.map_err(backoff::Error::Permanent), + Err(_) => Err(backoff::Error::Permanent(anyhow!( + "Lock acquisition timeout" + ))), + } + }) + .await +} diff --git a/lib/Cargo.toml b/lib/Cargo.toml index 11606a96..e8e30510 100644 --- a/lib/Cargo.toml +++ b/lib/Cargo.toml @@ -45,6 +45,7 @@ gix-protocol = "0.46.1" rpassword = "7.3.1" zeroize = { version = "1.8.1", features = ["derive", "std"] } async-trait = "0.1.85" +crossbeam-deque = "0.8.6" [target.'cfg(unix)'.dependencies] uzers = "0.12" diff --git a/lib/src/actions/mod.rs b/lib/src/actions/mod.rs index 31955eee..95354025 100644 --- a/lib/src/actions/mod.rs +++ b/lib/src/actions/mod.rs @@ -8,9 +8,9 @@ mod macos; mod package; mod user; -use crate::contexts::Contexts; use crate::manifests::Manifest; use crate::steps::Step; +use crate::{contexts::Contexts, utilities::password_manager::PasswordManager}; use anyhow::anyhow; use binary::BinaryGitHub; use command::run::RunCommand; @@ -29,7 +29,7 @@ use rhai::Engine; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use std::fmt::Display; -use tracing::{error, warn}; +use tracing::{debug, error, info, instrument, warn}; use user::add::UserAdd; use self::user::add_group::UserAddGroup; @@ -238,12 +238,55 @@ impl From for ActionError { } } +#[async_trait::async_trait] pub trait Action: Send + Sync { fn summarize(&self) -> String { warn!("need to define action summarize"); "not found action summarize".to_string() } fn plan(&self, manifest: &Manifest, context: &Contexts) -> anyhow::Result>; + + #[instrument(skip_all)] + async fn execute( + &self, + dry_run: bool, + action: &Actions, + manifest: &Manifest, + contexts: &Contexts, + password_manager: Option, + ) -> anyhow::Result<()> { + let steps: Vec<_> = match self.plan(manifest, contexts) { + Ok(steps) => steps + .into_iter() + .filter(|step| step.do_initializers_allow_us_to_run()) + .filter(|step| match step.atom.plan() { + Ok(outcome) => outcome.should_run, + Err(_) => false, + }) + .collect(), + Err(err) => { + error!("Failed Processing: {action}. Action failed to get plan: {err:?}",); + return Ok(()); + } + }; + + if steps.is_empty() { + info!("nothing to be done to reconcile action"); + return Ok(()); + } + + if dry_run { + return Ok(()); + } + + for mut step in steps { + if let Err(e) = step.execute(password_manager.clone()).await { + debug!("{e}"); + } + } + info!("{}", self.summarize()); + Ok(()) + } } #[cfg(test)] diff --git a/lib/src/actions/package/install.rs b/lib/src/actions/package/install.rs index 079d43f6..b028eb16 100644 --- a/lib/src/actions/package/install.rs +++ b/lib/src/actions/package/install.rs @@ -33,7 +33,7 @@ impl Action for PackageInstall { // If the provider isn't available, see if we can bootstrap it if !provider.available() { - if provider.bootstrap(&context).is_empty() { + if provider.bootstrap(context).is_empty() { return Err(anyhow!( "Package Provider, {}, isn't available. Skipping action", provider.name() diff --git a/lib/src/actions/package/providers/paru.rs b/lib/src/actions/package/providers/paru.rs index c3245342..3a925c5b 100644 --- a/lib/src/actions/package/providers/paru.rs +++ b/lib/src/actions/package/providers/paru.rs @@ -32,7 +32,7 @@ impl PackageProvider for Paru { fn bootstrap(&self, contexts: &Contexts) -> Vec { let privilege_provider = - utilities::get_privilege_provider(contexts).unwrap_or_else(|| String::from("sudo")); + utilities::get_privilege_provider(contexts).unwrap_or( String::from("sudo")); vec![ Step { @@ -137,7 +137,6 @@ impl PackageProvider for Paru { String::from("--noconfirm"), String::from("--noprogressbar"), String::from("--skipreview"), - String::from("--sudoloop"), String::from("--useask"), ], package.extra_args.clone(), diff --git a/lib/src/atoms/command/exec.rs b/lib/src/atoms/command/exec.rs index 58fa9149..7cbbf9d0 100644 --- a/lib/src/atoms/command/exec.rs +++ b/lib/src/atoms/command/exec.rs @@ -1,19 +1,19 @@ -use std::{ - io::{BufRead, BufReader, Write}, - process::Stdio, - sync::{Arc, RwLock}, -}; +use std::{process::Stdio, sync::Arc}; use anyhow::{anyhow, Result}; -use tracing::debug; +use tracing::{debug, trace}; use super::super::Atom; use crate::atoms::Outcome; use crate::utilities; use crate::utilities::password_manager::PasswordManager; use tokio::process::Command; -use tokio::time::Duration; -use tokio::io::AsyncWriteExt; +use tokio::{ + io::{AsyncBufReadExt, AsyncWriteExt, BufReader}, + sync::RwLock, + task::JoinSet, + time::{sleep, Duration}, +}; #[derive(Default)] pub struct Exec { @@ -42,7 +42,7 @@ pub fn new_run_command(command: String) -> Exec { } impl Exec { - fn elevate_if_required(&self) -> (String, Vec) { + fn elevate_if_required(&self) -> (bool, String, Vec) { // Depending on the priviledged flag and who who the current user is // we can determine if we need to prepend sudo to the command @@ -50,54 +50,19 @@ impl Exec { match (self.privileged, whoami::username().as_str()) { // Hasn't requested priviledged, so never try to elevate - (false, _) => (self.command.clone(), self.arguments.clone()), + (false, _) => (false, self.command.clone(), self.arguments.clone()), // Requested priviledged, but is already root - (true, "root") => (self.command.clone(), self.arguments.clone()), + (true, "root") => (true, self.command.clone(), self.arguments.clone()), // Requested priviledged, but is not root (true, _) => ( + true, privilege_provider, [vec![self.command.clone()], self.arguments.clone()].concat(), ), } } - - async fn elevate(&mut self, password_manager: &Option) -> Result<()> { - tracing::info!( - "Privilege elevation required to run `{} {}`. Validating privileges ...", - &self.command, - &self.arguments.join(" ") - ); - - let privilege_provider = utilities::get_binary_path(&self.privilege_provider)?; - - let mut child = Command::new(privilege_provider) - .args(["-S", "--validate"]) - .stdin(Stdio::piped()) - .stdout(Stdio::piped()) - .stderr(Stdio::piped()) - .spawn()?; - - if let Some(mut stdin) = child.stdin.take() { - if let Some(secret) = password_manager.as_ref().and_then(|pm| pm.secret.clone()) { - stdin.write_all(secret.as_bytes()).await?; - stdin.write_all(b"\n").await?; - } - } - - match child.wait_with_output().await { - Ok(std::process::Output { status, .. }) if status.success() => Ok(()), - Ok(std::process::Output { stderr, .. }) => Err(anyhow!( - "Command requires privilege escalation, but couldn't elevate privileges: {}", - String::from_utf8(stderr)? - )), - Err(err) => Err(anyhow!( - "Command requires privilege escalation, but couldn't elevate privileges: {}", - err - )), - } - } } impl std::fmt::Display for Exec { @@ -128,25 +93,18 @@ impl Atom for Exec { } async fn execute(&mut self, password_manager: Option) -> anyhow::Result<()> { - let (command, arguments) = self.elevate_if_required(); - println!("{}", self); + let (elevated, command, mut arguments) = self.elevate_if_required(); let command = utilities::get_binary_path(&command) .map_err(|_| anyhow!("Command `{}` not found in path", command))?; // If we require root, we need to use sudo with inherited IO // to ensure the user can respond if prompted for a password - if command.eq("doas") || command.eq("sudo") || command.eq("run0") { - println!("elevating {}", command); - match self.elevate(&password_manager).await { - Ok(_) => (), - Err(err) => { - return Err(anyhow!(err)); - } - } + if elevated && command.eq("sudo") { + arguments.insert(0, String::from("-S")); } - let mut child = std::process::Command::new(&command) + let mut child = Command::new(&command) .stdin(Stdio::piped()) .stdout(Stdio::piped()) .stderr(Stdio::piped()) @@ -159,53 +117,65 @@ impl Atom for Exec { })) .spawn()?; - let secret = Arc::new(password_manager - .and_then(|pm| pm.secret.clone()) - .map_or(String::new(), |s| format!("{}\n", s.as_str()))); + let secret = Arc::new( + password_manager + .and_then(|pm| pm.secret.clone()) + .map_or(String::new(), |s| format!("{}\n", s.as_str())), + ); let stdin = Arc::new(RwLock::new(child.stdin.take().unwrap())); let stdout = child.stdout.take().unwrap(); let stderr = child.stderr.take().unwrap(); - let secret1 = secret.clone(); - let stdin1 = stdin.clone(); - - let stdout_handle = tokio::spawn(async move { - let reader = BufReader::new(stdout); - for line in reader.lines().map_while(Result::ok) { - if line.to_lowercase().contains("password") { - let stdin = stdin1.clone(); - let secret = secret1.clone(); - tokio::spawn(async move { - tokio::time::sleep(Duration::from_millis(100)).await; - if let Ok(mut stdin) = stdin.write() { - stdin.write_all(secret.as_bytes()).unwrap(); - stdin.flush().unwrap(); - } - }); + let secret1 = Arc::clone(&secret); + let stdin1 = Arc::clone(&stdin); + + let mut watchers = JoinSet::>::new(); + + watchers.spawn(async move { + let stdin = Arc::clone(&stdin1); + let secret = secret1.clone(); + let reader = &mut BufReader::new(stdout); + + while let Ok(line) = reader.lines().next_line().await { + if let Some(line) = line { + trace!("{line}"); + if line.to_lowercase().contains("password") { + let mut stdin = stdin.write().await; + stdin.write_all(secret.as_bytes()).await.unwrap(); + stdin.flush().await.unwrap(); + sleep(Duration::from_millis(100)).await; + } + } else { + break; } } + Ok(()) }); - tokio::spawn(async move { - let reader = BufReader::new(stderr); - for line in reader.lines().map_while(Result::ok) { - if line.to_lowercase().contains("password") { - let stdin = stdin.clone(); - let secret = secret.clone(); - tokio::spawn(async move { - tokio::time::sleep(Duration::from_millis(100)).await; - if let Ok(mut stdin) = stdin.write() { - stdin.write_all(secret.as_bytes()).unwrap(); - stdin.flush().unwrap(); - } - }); + watchers.spawn(async move { + let stdin = Arc::clone(&stdin); + let secret = secret.clone(); + let reader = &mut BufReader::new(stderr); + + while let Ok(line) = reader.lines().next_line().await { + if let Some(line) = line { + trace!("{line}"); + if line.to_lowercase().contains("password") { + sleep(Duration::from_millis(100)).await; + let mut stdin = stdin.write().await; + stdin.write_all(secret.as_bytes()).await.unwrap(); + stdin.flush().await.unwrap(); + } + } else { + break; } } - }).await?; + Ok(()) + }); - let _ = stdout_handle.await; + watchers.join_all().await; - match child.wait_with_output() { + match child.wait_with_output().await { Ok(output) if output.status.success() => { self.status.stdout = String::from_utf8(output.stdout)?; self.status.stderr = String::from_utf8(output.stderr)?; @@ -273,7 +243,7 @@ mod tests { fn elevate() { let mut command_run = new_run_command(String::from("echo")); command_run.arguments = vec![String::from("Hello, world!")]; - let (command, args) = command_run.elevate_if_required(); + let (_, command, args) = command_run.elevate_if_required(); assert_eq!(String::from("echo"), command); assert_eq!(vec![String::from("Hello, world!")], args); @@ -282,7 +252,7 @@ mod tests { command_run.arguments = vec![String::from("Hello, world!")]; command_run.privileged = true; command_run.privilege_provider = Privilege::Sudo.to_string(); - let (command, args) = command_run.elevate_if_required(); + let (_, command, args) = command_run.elevate_if_required(); assert_eq!(String::from("sudo"), command); assert_eq!( @@ -295,7 +265,7 @@ mod tests { fn elevate_doas() { let mut command_run = new_run_command(String::from("echo")); command_run.arguments = vec![String::from("Hello, world!")]; - let (command, args) = command_run.elevate_if_required(); + let (_, command, args) = command_run.elevate_if_required(); assert_eq!(String::from("echo"), command); assert_eq!(vec![String::from("Hello, world!")], args); @@ -304,7 +274,7 @@ mod tests { command_run.arguments = vec![String::from("Hello, world!")]; command_run.privileged = true; command_run.privilege_provider = Privilege::Doas.to_string(); - let (command, args) = command_run.elevate_if_required(); + let (_, command, args) = command_run.elevate_if_required(); assert_eq!(String::from("doas"), command); assert_eq!( @@ -316,7 +286,7 @@ mod tests { fn elevate_run0() { let mut command_run = new_run_command(String::from("echo")); command_run.arguments = vec![String::from("Hello, world!")]; - let (command, args) = command_run.elevate_if_required(); + let (_, command, args) = command_run.elevate_if_required(); assert_eq!(String::from("echo"), command); assert_eq!(vec![String::from("Hello, world!")], args); @@ -325,7 +295,7 @@ mod tests { command_run.arguments = vec![String::from("Hello, world!")]; command_run.privileged = true; command_run.privilege_provider = Privilege::Run0.to_string(); - let (command, args) = command_run.elevate_if_required(); + let (_, command, args) = command_run.elevate_if_required(); assert_eq!(String::from("run0"), command); assert_eq!( diff --git a/lib/src/manifests/mod.rs b/lib/src/manifests/mod.rs index 9554e7cb..420143cb 100644 --- a/lib/src/manifests/mod.rs +++ b/lib/src/manifests/mod.rs @@ -1,16 +1,52 @@ mod load; pub use load::load; +use tokio::sync::Barrier; mod providers; use crate::actions::Actions; +use crate::contexts::{to_rhai, Contexts}; +use crate::utilities::password_manager::PasswordManager; +use anyhow::{Error, Result}; use petgraph::prelude::*; pub use providers::register_providers; pub use providers::ManifestProvider; +use rhai::Engine; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use std::fmt; +use std::mem::discriminant; use std::path::{Path, PathBuf}; -use tracing::error; +use std::sync::Arc; +use tracing::{debug, error, info, instrument}; + +#[derive(Debug, Clone, Default)] +pub enum ManifestState { + #[default] + Pending, + Working, + Completed, + Failed(Arc), +} + +impl fmt::Display for ManifestState { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!( + f, + "Manifest {}", + match self { + Self::Pending => String::from("Pending"), + Self::Working => String::from("Working"), + Self::Completed => String::from("Completed"), + Self::Failed(e) => format!("Failed {}", e), + } + ) + } +} +impl PartialEq for ManifestState { + fn eq(&self, other: &Self) -> bool { + discriminant(self) == discriminant(other) + } +} #[derive(JsonSchema, Clone, Debug, Default, Serialize, Deserialize)] #[serde(deny_unknown_fields)] pub struct Manifest { @@ -33,7 +69,13 @@ pub struct Manifest { pub root_dir: Option, #[serde(skip)] - pub dag_index: Option>, + pub dag_index: Option, + + #[serde(skip)] + pub dependency_barrier: Option>, + + #[serde(skip)] + pub state: ManifestState, } impl fmt::Display for Manifest { @@ -52,6 +94,59 @@ impl AsRef for Manifest { } } +impl Manifest { + pub fn get_name(&self) -> String { + self.name + .as_deref() + .unwrap_or("Cannot extract name") + .to_string() + } + + #[instrument(skip(self, dry_run, label, contexts, password_manager))] + pub async fn execute( + &self, + dry_run: bool, + label: Option, + contexts: &Contexts, + password_manager: Option, + ) -> Result<()> { + if let Some(label) = label { + if !self.labels.contains(&label) { + info!( + message = "Skipping manifest, label not found", + label = label.as_str() + ); + return Ok(()); + } + } + + if let Some(where_condition) = &self.r#where { + let result = { + let engine = Engine::new(); + let mut scope = to_rhai(contexts); + engine + .eval_with_scope::(&mut scope, where_condition) + .unwrap() + }; + + debug!("Result of 'where' condition '{where_condition}' -> '{result}'"); + if !result { + info!("Skipping manifest, because 'where' conditions were false!"); + return Ok(()); + } + } + + for action in self.actions.iter() { + let act = action.inner_ref(); + act.execute(dry_run, action, self, contexts, password_manager.clone()) + .await?; + } + + info!("Completed: {self}",); + Ok(()) + } +} + pub fn resolve(uri: &String) -> Option { let manifest_directory = register_providers() .into_iter() diff --git a/lib/src/steps/finalizers/mod.rs b/lib/src/steps/finalizers/mod.rs index 1ac2800c..806ec363 100644 --- a/lib/src/steps/finalizers/mod.rs +++ b/lib/src/steps/finalizers/mod.rs @@ -8,8 +8,8 @@ pub use output_contains::OutputContains; #[allow(dead_code)] pub enum FlowControl { - Ensure(Box), - StopIf(Box), + Ensure(Box), + StopIf(Box), } /// Finalizers allow us to store data within the manifests KV store, diff --git a/lib/src/steps/initializers/mod.rs b/lib/src/steps/initializers/mod.rs index 91368fb3..69184ab7 100644 --- a/lib/src/steps/initializers/mod.rs +++ b/lib/src/steps/initializers/mod.rs @@ -9,8 +9,8 @@ pub use file_exists::FileExists; #[allow(dead_code)] pub enum FlowControl { - Ensure(Box), - SkipIf(Box), + Ensure(Box), + SkipIf(Box), } /// Initializers allow us to modify or skip the execution of an atom diff --git a/lib/src/steps/mod.rs b/lib/src/steps/mod.rs index d89c211d..50cffde1 100644 --- a/lib/src/steps/mod.rs +++ b/lib/src/steps/mod.rs @@ -1,12 +1,14 @@ use crate::atoms::Atom; use crate::steps::finalizers::FlowControl; +use crate::utilities::password_manager::PasswordManager; +use anyhow::{anyhow, Result}; use tracing::error; pub mod finalizers; pub mod initializers; pub struct Step { - pub atom: Box, + pub atom: Box, pub initializers: Vec, pub finalizers: Vec, } @@ -54,6 +56,20 @@ impl Step { }, }) } + + pub async fn execute(&mut self, password_manager: Option) -> Result<()> { + let result = self.atom.execute(password_manager.clone()).await; + if let Err(err) = result { + return Err(anyhow!("Atom failed to execute: {:?}", err)); + } + + if !self.do_finalizers_allow_us_to_continue() { + return Err(anyhow!( + "Finalizers won't allow us to continue with this action" + )); + } + Ok(()) + } } #[cfg(test)] From 3d9f1356d479cc5bdd0eef9c5a855e3ff169b53d Mon Sep 17 00:00:00 2001 From: Justin Bacher <92jbach@gmail.com> Date: Mon, 27 Jan 2025 00:50:14 -0500 Subject: [PATCH 3/8] =?UTF-8?q?Near-final=20version=E2=80=94just=20ironing?= =?UTF-8?q?=20out=20stdio=20parsing.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app/Cargo.toml | 2 - app/src/commands/apply.rs | 148 ++++-------------------------- app/src/utils/dependency_graph.rs | 148 +++++++++++------------------- lib/Cargo.toml | 6 +- lib/src/atoms/command/exec.rs | 57 +++++++----- 5 files changed, 113 insertions(+), 248 deletions(-) diff --git a/app/Cargo.toml b/app/Cargo.toml index 31ce0e13..415975d7 100644 --- a/app/Cargo.toml +++ b/app/Cargo.toml @@ -15,7 +15,6 @@ comfy-table = "7" comtrya-lib = { path = "../lib", version = "0.9.1" } petgraph = "0.6" rhai = { version = "1.19", features = ["serde"] } -backoff = { version = "0.4", features = ["tokio"] } strip-ansi-escapes = "0.2" tracing = "0.1" tracing-journald = "0.3.0" @@ -25,7 +24,6 @@ dirs-next = "2.0" serde = { version = "1.0", features = ["derive"] } serde_yml = "0" tokio = { version = "1.43.0", features = ["full"] } -scopeguard = "1.2.0" [dev-dependencies] assert_cmd = "2.0" diff --git a/app/src/commands/apply.rs b/app/src/commands/apply.rs index 0c79c0b6..e6b7b2f0 100644 --- a/app/src/commands/apply.rs +++ b/app/src/commands/apply.rs @@ -1,21 +1,13 @@ use std::{ - collections::HashMap, path::PathBuf, - sync::{ - atomic::{AtomicBool, Ordering}, - Arc, - }, + sync::{Arc, LazyLock}, }; use anyhow::{anyhow, Context, Result}; use clap::Parser; use comfy_table::{Cell, ContentArrangement, Table}; -use petgraph::{graph::NodeIndex, Direction::*}; -use tokio::{ - sync::{Barrier, RwLock}, - task::JoinSet, -}; -use tracing::{debug, info, instrument, trace}; +use tokio::task::JoinSet; +use tracing::{debug, instrument, trace}; use super::ComtryaCommand; use crate::{utils::DependencyGraph, Runtime}; @@ -89,142 +81,42 @@ impl Apply { } impl ComtryaCommand for Apply { - // #[instrument(skip(self, runtime))] + #[instrument(skip_all)] async fn execute(&self, runtime: &mut Runtime) -> Result<()> { let contexts = runtime.contexts.clone(); let manifest_path = self.manifest_path(runtime)?; - debug!("Load manifests from path: {manifest_path:#?}"); - let manifests = load(manifest_path, &contexts); - let manifest_manager = Arc::new(RwLock::new( - DependencyGraph::new(manifests, &contexts, runtime).await?, - )); - - let mut barrier_map: HashMap, Arc> = HashMap::new(); - for node in manifest_manager.read().await.graph.node_indices() { - barrier_map.insert( - node, - Arc::new(Barrier::new( - manifest_manager - .read() - .await - .graph - .neighbors_directed(node, Incoming) - .count() - .max(1), - )), - ); - } + let manifest_manager = { + let graph = DependencyGraph::new(manifests, &contexts, runtime).await?; + Arc::new(LazyLock::new(|| graph)) + }; - let ordered_manifests = manifest_manager.read().await.get_ordered_manifests(); let mut workers = JoinSet::>::new(); - let dry_run = Arc::new(AtomicBool::new(self.dry_run)); + let dry_run = self.dry_run; let password_manager = &runtime.password_manager; - let barrier_map = Arc::new(barrier_map); - for manifest in ordered_manifests { + + for manifest in manifest_manager.get_ordered_manifests() { // Need to clone all these because they'll be in their own threads - let dry_run = Arc::clone(&dry_run); let label = self.label.clone(); let contexts = contexts.clone(); let password_manager = password_manager.clone(); - let barrier_map = Arc::clone(&barrier_map); - let manager = Arc::clone(&manifest_manager); - let barrier = Arc::clone( - barrier_map - .get(manager.read().await.get_node_from_manifest(&manifest).await) - .unwrap(), - ); + let manifest_manager = Arc::clone(&manifest_manager); workers.spawn(async move { - let manifest = Arc::clone(&manifest); - let manifest_guard = manifest.lock().await; - let manifest_manager = Arc::clone(&manager); - - let dependents: Vec<_> = manifest_manager - .read() - .await - .graph - .neighbors_directed( - *manifest_manager - .read() - .await - .get_node_from_manifest(&manifest) - .await, - petgraph::Outgoing, - ) - .collect(); - - for dep in dependents.iter() { - debug!( - "Dependencies for manifest {}: {:?}", - manifest_guard, - manifest_manager - .read() - .await - .graph - .node_weight(*dep) - .unwrap() - .lock() - .await - .get_name() - ); + for successor in manifest_manager.get_successors(&manifest).await { + manifest_manager.get_barrier(successor).wait().await; } - info!( - "Worker for manifest {:?} waiting on dependencies", - &manifest_guard.get_name() - ); - - // let is_leader = barrier.wait().await.is_leader(); - for &dependent in &dependents { - let dependent_barrier = barrier_map.get(&dependent).unwrap(); - dependent_barrier.wait().await; - } - - info!( - "Worker for manifest {:?} executing manifest", - &manifest_guard.get_name() - ); - - manifest_guard - .execute( - dry_run.load(Ordering::Relaxed), - label.clone(), - &contexts, - password_manager.clone(), - ) + manifest + .execute(dry_run, label.clone(), &contexts, password_manager.clone()) .await?; - info!( - "Worker for manifest {:?} finished executing", - &manifest_guard.get_name() - ); - - let has_dependees = manifest_manager - .read() - .await - .graph - .neighbors_directed( - *manifest_manager - .read() - .await - .get_node_from_manifest(&manifest) - .await, - Outgoing, - ) - .peekable() - .peek() - .is_none(); - - if has_dependees { - barrier.wait().await; - } - info!( - "Worker for manifest {:?} FINISHED executing", - &manifest_guard.get_name() - ); + manifest_manager + .get_barrier(*manifest_manager.get_node_from_manifest(&manifest).await) + .wait() + .await; Ok(()) }); } diff --git a/app/src/utils/dependency_graph.rs b/app/src/utils/dependency_graph.rs index 64d81f64..30dfced9 100644 --- a/app/src/utils/dependency_graph.rs +++ b/app/src/utils/dependency_graph.rs @@ -1,12 +1,17 @@ -use std::{collections::HashMap, iter::from_fn, ops::Deref, sync::Arc}; +use std::{ + collections::HashMap, + iter::from_fn, + ops::Deref, + sync::{Arc, LazyLock}, +}; use anyhow::Result; use petgraph::{ graph::{DiGraph, NodeIndex}, - visit::Topo, - // Direction::*, + visit::{Reversed, Topo}, + Direction::{Incoming, Outgoing}, }; -use tokio::sync::Mutex; +use tokio::sync::Barrier; use super::ZipLongest; use crate::Runtime; @@ -17,10 +22,12 @@ use comtrya_lib::{ utilities::{get_privilege_provider, password_manager::PasswordManager}, }; +type LockedManifest = Arc Manifest + Send>>>; #[derive(Debug)] pub struct DependencyGraph { - pub(crate) graph: DiGraph>, ()>, + pub(crate) graph: DiGraph, pub(crate) name_to_idx: HashMap, + pub(crate) barrier_map: HashMap>, } impl DependencyGraph { @@ -32,18 +39,19 @@ impl DependencyGraph { let mut this = Self { graph: DiGraph::new(), name_to_idx: HashMap::new(), + barrier_map: HashMap::new(), }; let mut first_package_install: Option = None; let mut should_ask_for_pass = false; for (_, manifest) in manifests.iter() { - let from_index = this.add_manifest(manifest).await; + let to_index = this.add_manifest(manifest.clone()).await; for (dependant, action) in manifest.depends.iter().zip_longest(manifest.actions.iter()) { if let Some(dependency_manifest) = dependant.and_then(|dep| manifests.get(dep)) { - let to_index = this.add_manifest(dependency_manifest).await; + let from_index = this.add_manifest(dependency_manifest.clone()).await; this.graph.add_edge(from_index, to_index, ()); } @@ -51,7 +59,7 @@ impl DependencyGraph { Some(PackageInstall(_)) | Some(PackageRepository(_)) => { if let Some(index) = first_package_install { first_package_install = first_package_install.or(Some(index)); - this.graph.add_edge(from_index, index, ()); + this.graph.add_edge(index, to_index, ()); } true } @@ -61,6 +69,20 @@ impl DependencyGraph { } } + this.barrier_map.extend( + this.graph + .node_indices() + .map(|node| { + ( + node, + Arc::new(Barrier::new( + this.graph.neighbors_directed(node, Outgoing).count() + 1, + )), + ) + }) + .collect::>(), + ); + if should_ask_for_pass { let mut password_manager = PasswordManager::new(get_privilege_provider(contexts).as_deref())?; @@ -72,107 +94,47 @@ impl DependencyGraph { } fn get_ordered_nodes(&self) -> impl Iterator + '_ { - let mut topo = Topo::new(&self.graph); + let graph = Reversed(&self.graph); + let mut topo = Topo::new(&graph); - from_fn(move || topo.next(&self.graph)) + from_fn(move || topo.next(&graph)) } - pub fn get_ordered_manifests(&self) -> Vec>> { + pub fn get_ordered_manifests(&self) -> Vec { self.get_ordered_nodes() .flat_map(|idx| self.graph.node_weight(idx)) .cloned() .collect::>() } - pub async fn add_manifest(&mut self, manifest: impl AsRef) -> NodeIndex { - let manifest = manifest.as_ref(); - *self - .name_to_idx - .entry(manifest.get_name()) - .or_insert_with(|| self.graph.add_node(Arc::new(Mutex::new(manifest.clone())))) + pub async fn add_manifest(&mut self, manifest: Manifest) -> NodeIndex { + *self.name_to_idx.entry(manifest.get_name()).or_insert( + self.graph + .add_node(Arc::new(LazyLock::new(Box::new(|| manifest)))), + ) + } + + pub fn get_barrier(&self, index: NodeIndex) -> &Arc { + self.barrier_map.get(&index).unwrap() } #[allow(dead_code)] - async fn get_node_index(&self, manifest: M) -> &NodeIndex + pub async fn get_node_index(&self, manifest: M) -> &NodeIndex where - M: AsRef> + Deref>, + M: AsRef> + Deref>, { - self.name_to_idx - .get(&manifest.lock().await.get_name()) - .unwrap() + self.name_to_idx.get(&manifest.get_name()).unwrap() } - // #[allow(dead_code)] - // async fn get_next_nodes( - // &self, - // manifest: Arc>, - // ) -> impl Iterator>> { - // let futures: Vec<&Arc>> = Vec::new(); - // self.graph - // .neighbors_directed(*self.get_node_index(manifest).await, Outgoing) - // .map(|idx| self.graph.node_weight(idx).unwrap()) - // .filter(|m| self.are_dependencies_completed(*self.get_node_index(Arc::clone(m)))) - // } - - // #[allow(dead_code)] - // pub async fn get_ready_dependencies( - // &mut self, - // manifest: Arc>, - // ) -> Vec<&Arc>> { - // manifest - // .lock() - // .await - // .depends - // .iter() - // .map(|dep| self.name_to_idx.get(dep).unwrap()) - // .map(|idx| self.graph.node_weight(*idx).unwrap()) - // .filter(|m| m.blocking_lock().state == ManifestState::Completed) - // .collect::>() - // } - - // #[allow(dead_code)] - // async fn dependencies_completed(&self, node: NodeIndex) -> bool { - // self.graph - // .neighbors_directed(node, Incoming) - // .flat_map(|idx| self.graph.node_weight(idx)) - // .all(|dep| dep.blocking_lock().state == ManifestState::Completed) - // } - - pub async fn get_node_from_manifest(&self, manifest: &Arc>) -> &NodeIndex { - // let join_set = JoinSet::new(); - self.name_to_idx - .get(&manifest.lock().await.get_name()) - .unwrap() + #[allow(dead_code)] + pub async fn get_successors(&self, manifest: &LockedManifest) -> Vec { + self.graph + .neighbors_directed(*self.get_node_from_manifest(manifest).await, Incoming) + .collect() } - // #[allow(dead_code)] - // pub fn are_dependencies_completed(&self, idx: NodeIndex) -> bool { - // self.graph.neighbors_directed(idx, Outgoing).all(|idx| { - // self.graph.node_weight(idx).unwrap().blocking_lock().state == ManifestState::Completed - // }) - // } - - // #[allow(dead_code)] - // pub fn get_ready_manifests(&self) -> impl Iterator>> { - // self.graph.node_weights().filter(|m| { - // m.blocking_lock().state == ManifestState::Pending - // && self.are_dependencies_completed( - // *self.name_to_idx.get(&m.blocking_lock().get_name()).unwrap(), - // ) - // }) - // } - - // #[allow(dead_code)] - // pub fn all_manifests_completed(&self) -> bool { - // self.graph - // .node_weights() - // .all(|m| m.blocking_lock().state == ManifestState::Completed) - // } - - // #[allow(dead_code)] - // pub fn all_manifests_queued(&self) -> bool { - // self.graph - // .node_weights() - // .all(|m| m.blocking_lock().state != ManifestState::Pending) - // } + pub async fn get_node_from_manifest(&self, manifest: &LockedManifest) -> &NodeIndex { + // let join_set = JoinSet::new(); + self.name_to_idx.get(&manifest.get_name()).unwrap() + } } diff --git a/lib/Cargo.toml b/lib/Cargo.toml index e8e30510..28b1dd0e 100644 --- a/lib/Cargo.toml +++ b/lib/Cargo.toml @@ -40,12 +40,14 @@ whoami = "1.5" tar = "0.4.42" flate2 = "1.0.34" file-owner = "0.1.2" -gix = { version = "0.68.0", features = ["blocking-http-transport-reqwest-rust-tls", "blocking-network-client"] } +gix = { version = "0.68.0", features = [ + "blocking-http-transport-reqwest-rust-tls", + "blocking-network-client", +] } gix-protocol = "0.46.1" rpassword = "7.3.1" zeroize = { version = "1.8.1", features = ["derive", "std"] } async-trait = "0.1.85" -crossbeam-deque = "0.8.6" [target.'cfg(unix)'.dependencies] uzers = "0.12" diff --git a/lib/src/atoms/command/exec.rs b/lib/src/atoms/command/exec.rs index 7cbbf9d0..02f8d87d 100644 --- a/lib/src/atoms/command/exec.rs +++ b/lib/src/atoms/command/exec.rs @@ -1,7 +1,7 @@ use std::{process::Stdio, sync::Arc}; use anyhow::{anyhow, Result}; -use tracing::{debug, trace}; +use tracing::{debug, error, trace}; use super::super::Atom; use crate::atoms::Outcome; @@ -134,19 +134,24 @@ impl Atom for Exec { watchers.spawn(async move { let stdin = Arc::clone(&stdin1); let secret = secret1.clone(); - let reader = &mut BufReader::new(stdout); - - while let Ok(line) = reader.lines().next_line().await { - if let Some(line) = line { - trace!("{line}"); - if line.to_lowercase().contains("password") { - let mut stdin = stdin.write().await; - stdin.write_all(secret.as_bytes()).await.unwrap(); - stdin.flush().await.unwrap(); - sleep(Duration::from_millis(100)).await; + + let mut lines = BufReader::new(stdout).lines(); + loop { + match lines.next_line().await { + Ok(Some(line)) => { + trace!("{line}"); + if line.to_lowercase().contains("password") { + let mut stdin = stdin.write().await; + stdin.write_all(secret.as_bytes()).await.unwrap(); + stdin.flush().await.unwrap(); + sleep(Duration::from_millis(100)).await; + } + } + Ok(None) => break, + Err(e) => { + error!("Error while reading stdout. {e}"); + break; } - } else { - break; } } Ok(()) @@ -157,17 +162,23 @@ impl Atom for Exec { let secret = secret.clone(); let reader = &mut BufReader::new(stderr); - while let Ok(line) = reader.lines().next_line().await { - if let Some(line) = line { - trace!("{line}"); - if line.to_lowercase().contains("password") { - sleep(Duration::from_millis(100)).await; - let mut stdin = stdin.write().await; - stdin.write_all(secret.as_bytes()).await.unwrap(); - stdin.flush().await.unwrap(); + let mut lines = reader.lines(); + loop { + match lines.next_line().await { + Ok(Some(line)) => { + trace!("{line}"); + if line.to_lowercase().contains("password") { + let mut stdin = stdin.write().await; + stdin.write_all(secret.as_bytes()).await.unwrap(); + stdin.flush().await.unwrap(); + sleep(Duration::from_millis(100)).await; + } + } + Ok(None) => break, + Err(e) => { + error!("Error while reading stdout. {e}"); + break; } - } else { - break; } } Ok(()) From 635ccc38e5e86581bf5f352c2ff92ec25ec2edb1 Mon Sep 17 00:00:00 2001 From: Justin Bacher <92jbach@gmail.com> Date: Fri, 31 Jan 2025 00:11:20 -0500 Subject: [PATCH 4/8] reverted to original dependency lookups & corrected dependency waiting reverted to original dependency lookups & corrected dependency waiting --- app/src/commands/apply.rs | 46 +++++--- app/src/main.rs | 8 +- app/src/utils/dependency_graph.rs | 123 ++++++++++------------ app/src/utils/mod.rs | 26 ----- lib/Cargo.toml | 1 + lib/src/actions/mod.rs | 6 +- lib/src/actions/package/providers/paru.rs | 21 ++-- lib/src/atoms/command/exec.rs | 60 ++++++----- lib/src/manifests/mod.rs | 91 ++++++++-------- lib/src/utilities/password_manager.rs | 1 + 10 files changed, 188 insertions(+), 195 deletions(-) diff --git a/app/src/commands/apply.rs b/app/src/commands/apply.rs index e6b7b2f0..3091c8e8 100644 --- a/app/src/commands/apply.rs +++ b/app/src/commands/apply.rs @@ -7,7 +7,7 @@ use anyhow::{anyhow, Context, Result}; use clap::Parser; use comfy_table::{Cell, ContentArrangement, Table}; use tokio::task::JoinSet; -use tracing::{debug, instrument, trace}; +use tracing::{debug, error, instrument, trace}; use super::ComtryaCommand; use crate::{utils::DependencyGraph, Runtime}; @@ -84,19 +84,19 @@ impl ComtryaCommand for Apply { #[instrument(skip_all)] async fn execute(&self, runtime: &mut Runtime) -> Result<()> { let contexts = runtime.contexts.clone(); - let manifest_path = self.manifest_path(runtime)?; - debug!("Load manifests from path: {manifest_path:#?}"); + let manifest_path = self + .manifest_path(runtime) + .inspect(|path| debug!("Load manifests from path: {path:#?}"))?; let manifests = load(manifest_path, &contexts); - let manifest_manager = { let graph = DependencyGraph::new(manifests, &contexts, runtime).await?; Arc::new(LazyLock::new(|| graph)) }; - let mut workers = JoinSet::>::new(); let dry_run = self.dry_run; let password_manager = &runtime.password_manager; + println!("Total: {}", manifest_manager.get_ordered_manifests().len()); for manifest in manifest_manager.get_ordered_manifests() { // Need to clone all these because they'll be in their own threads let label = self.label.clone(); @@ -106,22 +106,40 @@ impl ComtryaCommand for Apply { workers.spawn(async move { for successor in manifest_manager.get_successors(&manifest).await { - manifest_manager.get_barrier(successor).wait().await; + // May need to remove this unwrap and handle the option instead + if !successor.barrier.as_ref().unwrap().wait(true).await { + return Err(anyhow!( + "Debug unable to run manifest {manifest:?} due dependency failure." + )); + } } - manifest - .execute(dry_run, label.clone(), &contexts, password_manager.clone()) - .await?; + let exec_result = manifest + .execute( + dry_run, + label.clone(), + &contexts.clone(), + password_manager.clone(), + ) + .await; - manifest_manager - .get_barrier(*manifest_manager.get_node_from_manifest(&manifest).await) - .wait() + manifest + .barrier + .as_ref() + .unwrap() + .wait(exec_result.is_ok()) .await; - Ok(()) + + exec_result.context("Manifest {manifest:?} failed. {e}") }); } - workers.join_all().await; + while let Some(result) = workers.join_next().await { + if let Err(e) = result { + error!("{e}"); + } + } + Ok(()) } } diff --git a/app/src/main.rs b/app/src/main.rs index e4dcc7ab..179bae8a 100644 --- a/app/src/main.rs +++ b/app/src/main.rs @@ -28,13 +28,19 @@ pub struct Runtime { } pub(crate) async fn execute(runtime: &mut Runtime) -> anyhow::Result<()> { - match runtime.clone().args.command { + let result = match runtime.clone().args.command { Commands::Apply(apply) => apply.execute(runtime).await, Commands::Status(apply) => apply.status(runtime).await, Commands::Version(version) => version.execute(runtime).await, Commands::Contexts(contexts) => contexts.execute(runtime).await, Commands::GenCompletions(gen_completions) => gen_completions.execute(runtime).await, + }; + + if let Err(e) = result { + println!("{e}"); } + + Ok(()) } fn configure_tracing(args: &GlobalArgs) { diff --git a/app/src/utils/dependency_graph.rs b/app/src/utils/dependency_graph.rs index 30dfced9..74b5de59 100644 --- a/app/src/utils/dependency_graph.rs +++ b/app/src/utils/dependency_graph.rs @@ -1,7 +1,6 @@ use std::{ collections::HashMap, iter::from_fn, - ops::Deref, sync::{Arc, LazyLock}, }; @@ -9,91 +8,94 @@ use anyhow::Result; use petgraph::{ graph::{DiGraph, NodeIndex}, visit::{Reversed, Topo}, - Direction::{Incoming, Outgoing}, + Direction::*, }; -use tokio::sync::Barrier; +use tracing::{error, trace}; -use super::ZipLongest; use crate::Runtime; use comtrya_lib::{ - actions::Actions::*, + actions::Actions, contexts::Contexts, - manifests::Manifest, + manifests::{DependencyBarrier, Manifest}, utilities::{get_privilege_provider, password_manager::PasswordManager}, }; type LockedManifest = Arc Manifest + Send>>>; + #[derive(Debug)] pub struct DependencyGraph { pub(crate) graph: DiGraph, pub(crate) name_to_idx: HashMap, - pub(crate) barrier_map: HashMap>, } impl DependencyGraph { pub async fn new( - manifests: HashMap, + mut manifests: HashMap, contexts: &Contexts, runtime: &mut Runtime, ) -> Result { let mut this = Self { graph: DiGraph::new(), name_to_idx: HashMap::new(), - barrier_map: HashMap::new(), }; + let mut should_ask_for_pass = false; + let mut dependency_map = Vec::new(); - let mut first_package_install: Option = None; + for (_, manifest) in manifests.iter_mut() { + manifest.barrier = Some(DependencyBarrier::new(manifest.depends.len() + 1)); + let node = this.add_manifest(manifest.clone()).await; + this.name_to_idx.insert(manifest.get_name(), node); + } - let mut should_ask_for_pass = false; - for (_, manifest) in manifests.iter() { - let to_index = this.add_manifest(manifest.clone()).await; - - for (dependant, action) in manifest.depends.iter().zip_longest(manifest.actions.iter()) - { - if let Some(dependency_manifest) = dependant.and_then(|dep| manifests.get(dep)) { - let from_index = this.add_manifest(dependency_manifest.clone()).await; - this.graph.add_edge(from_index, to_index, ()); - } - - should_ask_for_pass = match action { - Some(PackageInstall(_)) | Some(PackageRepository(_)) => { - if let Some(index) = first_package_install { - first_package_install = first_package_install.or(Some(index)); - this.graph.add_edge(index, to_index, ()); - } - true - } - Some(CommandRun(cva)) => cva.action.privileged, - _ => false, + for (node, manifest) in this.graph.node_indices().map(|n| (n, &this.graph[n])) { + manifest.depends.iter().for_each(|dependency_name| { + let name = manifest.get_name(); + + let dep_prefix = name.rsplit_once('.').map(|(n, _)| n).unwrap_or(&name); + let dependency_name = dependency_name.replace("./", &format!("{dep_prefix}.")); + + let Some(dependency_manifest) = manifests.get(&dependency_name) else { + return error!( + message = "Unresolved dependency", + dependency = dependency_name + ); }; + + trace!( + message = "Dependency Registered", + from = name, + to = dependency_manifest.get_name() + ); + + dependency_map.push((node, this.name_to_idx[&dependency_manifest.get_name()])); + }); + + if !should_ask_for_pass { + should_ask_for_pass = manifest.actions.iter().any(|action| match action { + Actions::CommandRun(cva) => cva.action.privileged, + Actions::PackageInstall(_) | Actions::PackageRepository(_) => true, + _ => false, + }); } } - this.barrier_map.extend( - this.graph - .node_indices() - .map(|node| { - ( - node, - Arc::new(Barrier::new( - this.graph.neighbors_directed(node, Outgoing).count() + 1, - )), - ) - }) - .collect::>(), - ); + for (from, to) in dependency_map { + this.graph.add_edge(from, to, ()); + } if should_ask_for_pass { + debug!("Should be prompting for password. Asking now"); + let mut password_manager = PasswordManager::new(get_privilege_provider(contexts).as_deref())?; - password_manager.prompt("Please enter password:").await?; + password_manager.prompt("Please enter password:")?; runtime.password_manager = Some(password_manager); } Ok(this) } - fn get_ordered_nodes(&self) -> impl Iterator + '_ { + fn ordered_nodes(&self) -> impl Iterator + '_ { let graph = Reversed(&self.graph); let mut topo = Topo::new(&graph); @@ -101,40 +103,29 @@ impl DependencyGraph { } pub fn get_ordered_manifests(&self) -> Vec { - self.get_ordered_nodes() + self.ordered_nodes() .flat_map(|idx| self.graph.node_weight(idx)) .cloned() .collect::>() } pub async fn add_manifest(&mut self, manifest: Manifest) -> NodeIndex { - *self.name_to_idx.entry(manifest.get_name()).or_insert( + let idx = self.name_to_idx.entry(manifest.get_name()).or_insert( self.graph .add_node(Arc::new(LazyLock::new(Box::new(|| manifest)))), - ) - } - - pub fn get_barrier(&self, index: NodeIndex) -> &Arc { - self.barrier_map.get(&index).unwrap() - } - - #[allow(dead_code)] - pub async fn get_node_index(&self, manifest: M) -> &NodeIndex - where - M: AsRef> + Deref>, - { - self.name_to_idx.get(&manifest.get_name()).unwrap() + ); + *idx } - #[allow(dead_code)] - pub async fn get_successors(&self, manifest: &LockedManifest) -> Vec { + pub async fn get_successors(&self, manifest: &LockedManifest) -> Vec { self.graph - .neighbors_directed(*self.get_node_from_manifest(manifest).await, Incoming) + .neighbors_directed(self.get_node_from_manifest(manifest).await, Incoming) + .map(|node| Arc::clone(&self.graph[node].clone())) .collect() } - pub async fn get_node_from_manifest(&self, manifest: &LockedManifest) -> &NodeIndex { + pub async fn get_node_from_manifest(&self, manifest: &LockedManifest) -> NodeIndex { // let join_set = JoinSet::new(); - self.name_to_idx.get(&manifest.get_name()).unwrap() + *self.name_to_idx.get(&manifest.get_name()).unwrap() } } diff --git a/app/src/utils/mod.rs b/app/src/utils/mod.rs index 6f6f954d..c6309144 100644 --- a/app/src/utils/mod.rs +++ b/app/src/utils/mod.rs @@ -1,9 +1,6 @@ mod dependency_graph; -use anyhow::{anyhow, Result}; -use backoff::{future::retry, ExponentialBackoff}; pub use dependency_graph::DependencyGraph; use std::iter::from_fn; -use tokio::time::{timeout, Duration}; pub trait ZipLongest: Iterator { fn zip_longest(self, other: I) -> impl Iterator, Option)> @@ -26,26 +23,3 @@ impl> ZipLongest for I { }) } } - -#[allow(dead_code)] -pub async fn try_acquire_lock(operation: impl Fn() -> Result) -> Result -where - T: Send + 'static, -{ - let backoff = ExponentialBackoff { - initial_interval: Duration::from_millis(10), - max_interval: Duration::from_secs(1), - max_elapsed_time: Some(Duration::from_secs(5)), - ..Default::default() - }; - - retry(backoff, || async { - match timeout(Duration::from_millis(100), async { operation() }).await { - Ok(result) => result.map_err(backoff::Error::Permanent), - Err(_) => Err(backoff::Error::Permanent(anyhow!( - "Lock acquisition timeout" - ))), - } - }) - .await -} diff --git a/lib/Cargo.toml b/lib/Cargo.toml index 28b1dd0e..cc59bcc0 100644 --- a/lib/Cargo.toml +++ b/lib/Cargo.toml @@ -48,6 +48,7 @@ gix-protocol = "0.46.1" rpassword = "7.3.1" zeroize = { version = "1.8.1", features = ["derive", "std"] } async-trait = "0.1.85" +fnv_rs = "0.4.3" [target.'cfg(unix)'.dependencies] uzers = "0.12" diff --git a/lib/src/actions/mod.rs b/lib/src/actions/mod.rs index 95354025..b9db97f0 100644 --- a/lib/src/actions/mod.rs +++ b/lib/src/actions/mod.rs @@ -29,7 +29,7 @@ use rhai::Engine; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use std::fmt::Display; -use tracing::{debug, error, info, instrument, warn}; +use tracing::{error, info, instrument, warn}; use user::add::UserAdd; use self::user::add_group::UserAddGroup; @@ -280,9 +280,7 @@ pub trait Action: Send + Sync { } for mut step in steps { - if let Err(e) = step.execute(password_manager.clone()).await { - debug!("{e}"); - } + step.execute(password_manager.clone()).await?; } info!("{}", self.summarize()); Ok(()) diff --git a/lib/src/actions/package/providers/paru.rs b/lib/src/actions/package/providers/paru.rs index 3a925c5b..b85994ba 100644 --- a/lib/src/actions/package/providers/paru.rs +++ b/lib/src/actions/package/providers/paru.rs @@ -7,6 +7,7 @@ use crate::steps::Step; use crate::utilities; use serde::{Deserialize, Serialize}; use std::collections::HashSet; +use std::iter::once; use std::process::Command; use tracing::warn; use tracing::{debug, trace}; @@ -21,18 +22,14 @@ impl PackageProvider for Paru { } fn available(&self) -> bool { - match which("paru") { - Ok(_) => true, - Err(_) => { - warn!(message = "paru not available"); - false - } - } + which("paru") + .inspect_err(|_| warn!(message = "paru not available")) + .is_ok() } fn bootstrap(&self, contexts: &Contexts) -> Vec { let privilege_provider = - utilities::get_privilege_provider(contexts).unwrap_or( String::from("sudo")); + utilities::get_privilege_provider(contexts).unwrap_or(String::from("sudo")); vec![ Step { @@ -46,7 +43,7 @@ impl PackageProvider for Paru { String::from("git"), ], privileged: true, - privilege_provider: privilege_provider.clone(), + privilege_provider, ..Default::default() }), initializers: vec![], @@ -93,11 +90,7 @@ impl PackageProvider for Paru { fn query(&self, package: &PackageVariant) -> anyhow::Result> { let requested_already_installed: HashSet = String::from_utf8( Command::new("pacman") - .args( - vec![String::from("-Qq")] - .into_iter() - .chain(package.packages().into_iter()), - ) + .args(once(String::from("-Qq")).chain(package.packages())) .output()? .stdout, )? diff --git a/lib/src/atoms/command/exec.rs b/lib/src/atoms/command/exec.rs index 02f8d87d..1490a2c6 100644 --- a/lib/src/atoms/command/exec.rs +++ b/lib/src/atoms/command/exec.rs @@ -1,19 +1,19 @@ use std::{process::Stdio, sync::Arc}; use anyhow::{anyhow, Result}; -use tracing::{debug, error, trace}; - -use super::super::Atom; -use crate::atoms::Outcome; -use crate::utilities; -use crate::utilities::password_manager::PasswordManager; -use tokio::process::Command; use tokio::{ io::{AsyncBufReadExt, AsyncWriteExt, BufReader}, + process::Command, sync::RwLock, task::JoinSet, time::{sleep, Duration}, }; +use tracing::{debug, error, trace}; + +use super::super::Atom; +use crate::atoms::Outcome; +use crate::utilities; +use crate::utilities::password_manager::PasswordManager; #[derive(Default)] pub struct Exec { @@ -96,11 +96,12 @@ impl Atom for Exec { let (elevated, command, mut arguments) = self.elevate_if_required(); let command = utilities::get_binary_path(&command) - .map_err(|_| anyhow!("Command `{}` not found in path", command))?; + .map_err(|_| anyhow!("Command `{command}` not found in path"))?; // If we require root, we need to use sudo with inherited IO // to ensure the user can respond if prompted for a password - if elevated && command.eq("sudo") { + + if elevated && command.eq("echo") { arguments.insert(0, String::from("-S")); } @@ -120,20 +121,22 @@ impl Atom for Exec { let secret = Arc::new( password_manager .and_then(|pm| pm.secret.clone()) - .map_or(String::new(), |s| format!("{}\n", s.as_str())), + .map(|s| format!("{}\n", s.as_str())), ); - let stdin = Arc::new(RwLock::new(child.stdin.take().unwrap())); - let stdout = child.stdout.take().unwrap(); - let stderr = child.stderr.take().unwrap(); + let stdin = Arc::new(RwLock::new( + child.stdin.take().context("Error executing command")?, + )); + let stdout = child.stdout.take().context("Error executing command")?; + let stderr = child.stderr.take().context("Error executing command")?; - let secret1 = Arc::clone(&secret); - let stdin1 = Arc::clone(&stdin); + let secret_out = Arc::clone(&secret); + let stdin_out = Arc::clone(&stdin); let mut watchers = JoinSet::>::new(); watchers.spawn(async move { - let stdin = Arc::clone(&stdin1); - let secret = secret1.clone(); + let stdin = Arc::clone(&stdin_out); + let secret = secret_out.clone(); let mut lines = BufReader::new(stdout).lines(); loop { @@ -141,10 +144,12 @@ impl Atom for Exec { Ok(Some(line)) => { trace!("{line}"); if line.to_lowercase().contains("password") { - let mut stdin = stdin.write().await; - stdin.write_all(secret.as_bytes()).await.unwrap(); - stdin.flush().await.unwrap(); - sleep(Duration::from_millis(100)).await; + if let Some(pass) = secret.as_deref() { + let mut stdin = stdin.write().await; + stdin.write_all(pass.as_bytes()).await.unwrap(); + stdin.flush().await.unwrap(); + sleep(Duration::from_millis(100)).await; + } } } Ok(None) => break, @@ -160,18 +165,19 @@ impl Atom for Exec { watchers.spawn(async move { let stdin = Arc::clone(&stdin); let secret = secret.clone(); - let reader = &mut BufReader::new(stderr); - let mut lines = reader.lines(); + let mut lines = BufReader::new(stderr).lines(); loop { match lines.next_line().await { Ok(Some(line)) => { trace!("{line}"); if line.to_lowercase().contains("password") { - let mut stdin = stdin.write().await; - stdin.write_all(secret.as_bytes()).await.unwrap(); - stdin.flush().await.unwrap(); - sleep(Duration::from_millis(100)).await; + if let Some(pass) = secret.as_deref() { + let mut stdin = stdin.write().await; + stdin.write_all(pass.as_bytes()).await.unwrap(); + stdin.flush().await.unwrap(); + sleep(Duration::from_millis(100)).await; + } } } Ok(None) => break, diff --git a/lib/src/manifests/mod.rs b/lib/src/manifests/mod.rs index 420143cb..15accf4a 100644 --- a/lib/src/manifests/mod.rs +++ b/lib/src/manifests/mod.rs @@ -1,52 +1,49 @@ mod load; pub use load::load; -use tokio::sync::Barrier; mod providers; use crate::actions::Actions; use crate::contexts::{to_rhai, Contexts}; use crate::utilities::password_manager::PasswordManager; -use anyhow::{Error, Result}; -use petgraph::prelude::*; +use ::std::hash::Hasher; +use anyhow::{anyhow, Result}; +use fnv_rs::{Fnv64, FnvHasher}; pub use providers::register_providers; pub use providers::ManifestProvider; use rhai::Engine; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use std::fmt; -use std::mem::discriminant; use std::path::{Path, PathBuf}; -use std::sync::Arc; +use std::sync::{ + atomic::{AtomicBool, Ordering::SeqCst}, + Arc, +}; +use tokio::sync::Barrier; use tracing::{debug, error, info, instrument}; -#[derive(Debug, Clone, Default)] -pub enum ManifestState { - #[default] - Pending, - Working, - Completed, - Failed(Arc), +#[derive(Debug, Clone)] +pub struct DependencyBarrier { + barrier: Arc, + can_continue: Arc, } -impl fmt::Display for ManifestState { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!( - f, - "Manifest {}", - match self { - Self::Pending => String::from("Pending"), - Self::Working => String::from("Working"), - Self::Completed => String::from("Completed"), - Self::Failed(e) => format!("Failed {}", e), - } - ) +impl DependencyBarrier { + pub fn new(n: usize) -> Self { + Self { + barrier: Arc::new(Barrier::new(n)), + can_continue: Arc::new(AtomicBool::new(true)), + } } -} -impl PartialEq for ManifestState { - fn eq(&self, other: &Self) -> bool { - discriminant(self) == discriminant(other) + pub async fn wait(&self, result: bool) -> bool { + let new_can_continue = result & self.can_continue.load(SeqCst); + self.can_continue.store(new_can_continue, SeqCst); + self.barrier.wait().await; + + new_can_continue } } + #[derive(JsonSchema, Clone, Debug, Default, Serialize, Deserialize)] #[serde(deny_unknown_fields)] pub struct Manifest { @@ -69,13 +66,10 @@ pub struct Manifest { pub root_dir: Option, #[serde(skip)] - pub dag_index: Option, - - #[serde(skip)] - pub dependency_barrier: Option>, + pub barrier: Option, #[serde(skip)] - pub state: ManifestState, + pub dependencies: Vec, } impl fmt::Display for Manifest { @@ -102,14 +96,24 @@ impl Manifest { .to_string() } - #[instrument(skip(self, dry_run, label, contexts, password_manager))] + pub fn id(&self) -> u64 { + let mut hasher = Fnv64::new(); + hasher.update(self.get_name().as_bytes()); + if let Some(root_dir) = &self.root_dir { + hasher.update(root_dir.to_string_lossy().as_bytes()); + } + hasher.finish() + } + + #[instrument(skip_all)] pub async fn execute( &self, dry_run: bool, label: Option, contexts: &Contexts, password_manager: Option, - ) -> Result<()> { + ) -> Result<()> +where { if let Some(label) = label { if !self.labels.contains(&label) { info!( @@ -121,12 +125,12 @@ impl Manifest { } if let Some(where_condition) = &self.r#where { - let result = { - let engine = Engine::new(); - let mut scope = to_rhai(contexts); - engine - .eval_with_scope::(&mut scope, where_condition) - .unwrap() + let Ok(result) = + Engine::new().eval_with_scope::(&mut to_rhai(contexts), where_condition) + else { + return Err(anyhow!( + "Unable to evaluate 'where' condition '{where_condition}'" + )); }; debug!("Result of 'where' condition '{where_condition}' -> '{result}'"); @@ -137,8 +141,9 @@ impl Manifest { } for action in self.actions.iter() { - let act = action.inner_ref(); - act.execute(dry_run, action, self, contexts, password_manager.clone()) + action + .inner_ref() + .execute(dry_run, action, self, contexts, password_manager.clone()) .await?; } diff --git a/lib/src/utilities/password_manager.rs b/lib/src/utilities/password_manager.rs index 931fd9aa..4f75aee5 100644 --- a/lib/src/utilities/password_manager.rs +++ b/lib/src/utilities/password_manager.rs @@ -65,4 +65,5 @@ impl PasswordManager { wait.tick().await; }); } + } } From 7ca6e66370a7036601a1bc0d059f4f016f09b256 Mon Sep 17 00:00:00 2001 From: JustinBacher <92jbach@gmail.com> Date: Tue, 4 Feb 2025 04:03:20 -0500 Subject: [PATCH 5/8] fix for arch package managers asking for pass --- lib/src/utilities/password_manager.rs | 70 +++++++++++++-------------- 1 file changed, 34 insertions(+), 36 deletions(-) diff --git a/lib/src/utilities/password_manager.rs b/lib/src/utilities/password_manager.rs index 4f75aee5..1a2e7f09 100644 --- a/lib/src/utilities/password_manager.rs +++ b/lib/src/utilities/password_manager.rs @@ -1,13 +1,11 @@ -use std::{process::Stdio, sync::Arc}; +use std::{ + io::Write, + process::{Command, Stdio}, +}; -use anyhow::Result; +use anyhow::{anyhow, Context, Result}; use rpassword::prompt_password; -use tokio::{ - io::AsyncWriteExt, - process::Command, - time::{interval, Duration}, -}; -use tracing::error; +use tracing::warn; use zeroize::{Zeroize, ZeroizeOnDrop, Zeroizing}; #[derive(Debug, Zeroize, ZeroizeOnDrop, Clone)] @@ -32,38 +30,38 @@ impl PasswordManager { Ok(this) } - #[cfg(target_os = "linux")] - pub async fn prompt(&mut self, prompt: &str) -> Result<()> { - self.secret = Some(Zeroizing::new(prompt_password(prompt)?)); - self.keep_elevated().await; - Ok(()) - } + #[cfg(any(target_os = "linux", target_os = "macos"))] + pub fn prompt(&mut self, prompt: &str) -> Result<()> { + let attempts = 3; - async fn keep_elevated(&self) { - let shared_self = Arc::new(self.clone()); - let mut wait = interval(Duration::from_secs(60 * 10)); - tokio::spawn(async move { - let this = Arc::clone(&shared_self); - let mut command = Command::new(this.privilege_provider.clone()) - .arg("-S") // Read password from stdin - .arg("-v") // Validate and update timestamp - .stdin(Stdio::piped()) - .stdout(Stdio::piped()) - .stderr(Stdio::piped()) - .spawn() - .expect("Failed to refresh sudo"); + for attempt in 1..=attempts { + let secret = Zeroizing::new(prompt_password(prompt)?); - if let (Some(mut stdin), Some(secret)) = (command.stdin.take(), this.secret.clone()) { - stdin - .write_all(format!("{}\n", secret.as_str()).as_bytes()) - .await - .unwrap(); - } else { - error!("Unable to elevate privilege") + if !self.try_password(&secret)? { + warn!("Incorrect Password. Try again! (attempt: {attempt}/{attempts})"); + continue; } - wait.tick().await; - }); + self.secret = Some(secret); + return Ok(()); + } + + Err(anyhow!("Too many incorrect attempts. Access denied.")) } + + #[cfg(any(target_os = "linux", target_os = "macos"))] + fn try_password(&self, secret: &Zeroizing) -> Result { + let mut pass_cmd = Command::new("sudo") + .arg("-Sv") + .stdin(Stdio::piped()) + .spawn()?; + + pass_cmd + .stdin + .take() + .context("Error occured while attempting pasword verificaton")? + .write_all(format!("{}\n", secret.as_str()).as_bytes())?; + + Ok(pass_cmd.wait()?.success()) } } From 7a344c3b188dffe27a0375ce67ce7d1d20878d97 Mon Sep 17 00:00:00 2001 From: JustinBacher <92jbach@gmail.com> Date: Tue, 4 Feb 2025 22:30:16 -0500 Subject: [PATCH 6/8] cleaner way of ensuring package manager events are ran in succession --- app/src/commands/apply.rs | 69 +++++++------ app/src/config/mod.rs | 2 +- app/src/main.rs | 9 +- app/src/utils/dependency_graph.rs | 39 +++---- app/src/utils/mod.rs | 24 +---- lib/src/actions/command/run.rs | 4 + lib/src/actions/file/link.rs | 31 +++--- lib/src/actions/file/mod.rs | 22 ++-- lib/src/actions/group/add.rs | 6 +- lib/src/actions/mod.rs | 119 ++++++++++++++-------- lib/src/actions/package/install.rs | 45 +++++--- lib/src/actions/package/mod.rs | 3 + lib/src/actions/package/providers/paru.rs | 2 + lib/src/actions/package/repository.rs | 26 ++++- lib/src/actions/user/add.rs | 4 +- lib/src/actions/user/add_group.rs | 2 +- lib/src/actions/user/mod.rs | 42 +++----- lib/src/atoms/command/exec.rs | 4 +- lib/src/manifests/mod.rs | 19 ++-- lib/src/utilities/password_manager.rs | 10 +- 20 files changed, 259 insertions(+), 223 deletions(-) diff --git a/app/src/commands/apply.rs b/app/src/commands/apply.rs index 3091c8e8..6b9ed89a 100644 --- a/app/src/commands/apply.rs +++ b/app/src/commands/apply.rs @@ -7,7 +7,7 @@ use anyhow::{anyhow, Context, Result}; use clap::Parser; use comfy_table::{Cell, ContentArrangement, Table}; use tokio::task::JoinSet; -use tracing::{debug, error, instrument, trace}; +use tracing::{debug, instrument, trace}; use super::ComtryaCommand; use crate::{utils::DependencyGraph, Runtime}; @@ -52,7 +52,7 @@ impl Apply { Ok(manifest_path) } - #[instrument(skip(self, runtime))] + #[instrument(skip_all)] pub async fn status(&self, runtime: &Runtime) -> anyhow::Result<()> { let contexts = &runtime.contexts; let manifest_path = self.manifest_path(runtime)?; @@ -83,61 +83,66 @@ impl Apply { impl ComtryaCommand for Apply { #[instrument(skip_all)] async fn execute(&self, runtime: &mut Runtime) -> Result<()> { - let contexts = runtime.contexts.clone(); let manifest_path = self .manifest_path(runtime) - .inspect(|path| debug!("Load manifests from path: {path:#?}"))?; + .inspect(|path| debug!("Load manifests from path: {:#?}", path))?; + + let contexts = runtime.contexts.clone(); let manifests = load(manifest_path, &contexts); let manifest_manager = { + // Can't have async in closure let graph = DependencyGraph::new(manifests, &contexts, runtime).await?; Arc::new(LazyLock::new(|| graph)) }; + let mut workers = JoinSet::>::new(); let dry_run = self.dry_run; let password_manager = &runtime.password_manager; - println!("Total: {}", manifest_manager.get_ordered_manifests().len()); for manifest in manifest_manager.get_ordered_manifests() { + let name = manifest.get_name(); + // Need to clone all these because they'll be in their own threads let label = self.label.clone(); let contexts = contexts.clone(); - let password_manager = password_manager.clone(); + let pm = password_manager.clone(); let manifest_manager = Arc::clone(&manifest_manager); workers.spawn(async move { - for successor in manifest_manager.get_successors(&manifest).await { - // May need to remove this unwrap and handle the option instead - if !successor.barrier.as_ref().unwrap().wait(true).await { - return Err(anyhow!( - "Debug unable to run manifest {manifest:?} due dependency failure." - )); - } - } - - let exec_result = manifest - .execute( - dry_run, - label.clone(), - &contexts.clone(), - password_manager.clone(), - ) - .await; - + // Wait on current manifest's barrier. If a dependency + // fails it propugates the failure upward as false. manifest .barrier .as_ref() - .unwrap() - .wait(exec_result.is_ok()) - .await; + .with_context(|| format!("Cannot lock manifest '{}' for execution", name))? + .wait(true) + .await + .then_some(()) + .context(format!("Skipping manifest '{}' Dependancy(s) failed", name))?; + + let result = manifest.execute(dry_run, label, &contexts, pm).await; + + // Inform successors (dependants) of pass or fail + for successor in manifest_manager + .get_successors(&manifest) + .await + // should never be None but can't hurt to be careful + .context(format!("Cannot resolve dependants for manifest '{}'", name))? + { + successor + .barrier + .as_ref() + .context(format!("Cannot mark manifest '{}' completed", name))? + .wait(result.is_ok()) + .await; + } - exec_result.context("Manifest {manifest:?} failed. {e}") + Ok(()) }); } - while let Some(result) = workers.join_next().await { - if let Err(e) = result { - error!("{e}"); - } + while let Some(Err(error)) = workers.join_next().await { + eprintln!("{error}"); } Ok(()) diff --git a/app/src/config/mod.rs b/app/src/config/mod.rs index 6df34595..65ba71d8 100644 --- a/app/src/config/mod.rs +++ b/app/src/config/mod.rs @@ -81,7 +81,7 @@ where } pub(crate) fn load_config(args: &GlobalArgs) -> Result { - match lib_config(&args) { + match lib_config(args) { Ok(config) => match args.manifest_directory.clone() { Some(manifest_path) => Ok(Config { manifest_paths: vec![manifest_path], diff --git a/app/src/main.rs b/app/src/main.rs index 179bae8a..09185091 100644 --- a/app/src/main.rs +++ b/app/src/main.rs @@ -10,7 +10,6 @@ use comtrya_lib::manifests; use clap::Parser; use tracing::{error, Level}; -#[allow(unused_imports)] use tracing_subscriber::{fmt::writer::MakeWriterExt, layer::SubscriberExt, FmtSubscriber}; mod commands; @@ -28,16 +27,14 @@ pub struct Runtime { } pub(crate) async fn execute(runtime: &mut Runtime) -> anyhow::Result<()> { - let result = match runtime.clone().args.command { + if let Err(e) = match runtime.clone().args.command { Commands::Apply(apply) => apply.execute(runtime).await, Commands::Status(apply) => apply.status(runtime).await, Commands::Version(version) => version.execute(runtime).await, Commands::Contexts(contexts) => contexts.execute(runtime).await, Commands::GenCompletions(gen_completions) => gen_completions.execute(runtime).await, - }; - - if let Err(e) = result { - println!("{e}"); + } { + eprintln!("{e}"); } Ok(()) diff --git a/app/src/utils/dependency_graph.rs b/app/src/utils/dependency_graph.rs index 74b5de59..d72b1c5f 100644 --- a/app/src/utils/dependency_graph.rs +++ b/app/src/utils/dependency_graph.rs @@ -14,7 +14,6 @@ use tracing::{error, trace}; use crate::Runtime; use comtrya_lib::{ - actions::Actions, contexts::Contexts, manifests::{DependencyBarrier, Manifest}, utilities::{get_privilege_provider, password_manager::PasswordManager}, @@ -41,7 +40,7 @@ impl DependencyGraph { let mut should_ask_for_pass = false; let mut dependency_map = Vec::new(); - for (_, manifest) in manifests.iter_mut() { + for manifest in manifests.values_mut() { manifest.barrier = Some(DependencyBarrier::new(manifest.depends.len() + 1)); let node = this.add_manifest(manifest.clone()).await; this.name_to_idx.insert(manifest.get_name(), node); @@ -54,7 +53,7 @@ impl DependencyGraph { let dep_prefix = name.rsplit_once('.').map(|(n, _)| n).unwrap_or(&name); let dependency_name = dependency_name.replace("./", &format!("{dep_prefix}.")); - let Some(dependency_manifest) = manifests.get(&dependency_name) else { + let Some(dependency) = manifests.get(&dependency_name) else { return error!( message = "Unresolved dependency", dependency = dependency_name @@ -64,28 +63,20 @@ impl DependencyGraph { trace!( message = "Dependency Registered", from = name, - to = dependency_manifest.get_name() + to = dependency.get_name() ); - dependency_map.push((node, this.name_to_idx[&dependency_manifest.get_name()])); + dependency_map.push((node, this.name_to_idx[&dependency.get_name()])); }); - if !should_ask_for_pass { - should_ask_for_pass = manifest.actions.iter().any(|action| match action { - Actions::CommandRun(cva) => cva.action.privileged, - Actions::PackageInstall(_) | Actions::PackageRepository(_) => true, - _ => false, - }); - } + should_ask_for_pass |= manifest.actions.iter().any(|action| action.is_privileged()); } - for (from, to) in dependency_map { - this.graph.add_edge(from, to, ()); + for (from, to) in &dependency_map { + this.graph.add_edge(*from, *to, ()); } if should_ask_for_pass { - debug!("Should be prompting for password. Asking now"); - let mut password_manager = PasswordManager::new(get_privilege_provider(contexts).as_deref())?; password_manager.prompt("Please enter password:")?; @@ -117,15 +108,17 @@ impl DependencyGraph { *idx } - pub async fn get_successors(&self, manifest: &LockedManifest) -> Vec { - self.graph - .neighbors_directed(self.get_node_from_manifest(manifest).await, Incoming) + pub async fn get_successors(&self, manifest: &LockedManifest) -> Option> { + let manifests = self + .graph + .neighbors_directed(self.get_node_from_manifest(manifest).await?, Incoming) .map(|node| Arc::clone(&self.graph[node].clone())) - .collect() + .collect(); + + Some(manifests) } - pub async fn get_node_from_manifest(&self, manifest: &LockedManifest) -> NodeIndex { - // let join_set = JoinSet::new(); - *self.name_to_idx.get(&manifest.get_name()).unwrap() + pub async fn get_node_from_manifest(&self, manifest: &LockedManifest) -> Option { + self.name_to_idx.get(&manifest.get_name()).copied() } } diff --git a/app/src/utils/mod.rs b/app/src/utils/mod.rs index c6309144..a6893948 100644 --- a/app/src/utils/mod.rs +++ b/app/src/utils/mod.rs @@ -1,25 +1,3 @@ mod dependency_graph; -pub use dependency_graph::DependencyGraph; -use std::iter::from_fn; - -pub trait ZipLongest: Iterator { - fn zip_longest(self, other: I) -> impl Iterator, Option)> - where - Self: Sized, - I: IntoIterator; -} -impl> ZipLongest for I { - fn zip_longest(self, other: J) -> impl Iterator, Option)> - where - J: IntoIterator, - { - let mut a = self.fuse(); - let mut b = other.into_iter().fuse(); - - from_fn(move || match (a.next(), b.next()) { - (None, None) => None, - (a, b) => Some((a, b)), - }) - } -} +pub use dependency_graph::DependencyGraph; diff --git a/lib/src/actions/command/run.rs b/lib/src/actions/command/run.rs index f0152153..082cf4b2 100644 --- a/lib/src/actions/command/run.rs +++ b/lib/src/actions/command/run.rs @@ -62,6 +62,10 @@ impl Action for RunCommand { ))], }]) } + + fn is_privileged(&self) -> bool { + self.privileged + } } #[cfg(test)] diff --git a/lib/src/actions/file/link.rs b/lib/src/actions/file/link.rs index 9a36aec4..fe79fe5a 100644 --- a/lib/src/actions/file/link.rs +++ b/lib/src/actions/file/link.rs @@ -92,22 +92,19 @@ impl FileLink { }]; if let Ok(paths) = std::fs::read_dir(from) { - paths.for_each(|path| { - if let Ok(path) = path { - let p = path.path(); - - if let Some(file_name) = p.file_name() { - steps.push(Step { - atom: Box::new(Link { - source: p.clone(), - target: to.join(file_name), - }), - initializers: vec![Ensure(Box::new(FileExists(p.clone())))], - finalizers: vec![], - }) - } - } - }) + steps.extend(paths.filter_map(|path| { + let p = path.ok()?.path(); + let file_name = p.file_name()?; + + Some(Step { + atom: Box::new(Link { + source: p.clone(), + target: to.join(file_name), + }), + initializers: vec![Ensure(Box::new(FileExists(p.clone())))], + finalizers: vec![], + }) + })) } steps @@ -209,7 +206,6 @@ mod tests { actions: vec![], depends: vec![], name: None, - dag_index: None, ..Default::default() }; @@ -265,7 +261,6 @@ mod tests { actions: vec![], depends: vec![], name: None, - dag_index: None, ..Default::default() }; diff --git a/lib/src/actions/file/mod.rs b/lib/src/actions/file/mod.rs index 09a8fd6a..3044eec8 100644 --- a/lib/src/actions/file/mod.rs +++ b/lib/src/actions/file/mod.rs @@ -7,7 +7,7 @@ pub mod unarchive; use crate::actions::Action; use crate::manifests::Manifest; -use anyhow::{anyhow, Result}; +use anyhow::{anyhow, Context, Result}; use normpath::PathExt; use serde::{de::Error, Deserialize, Deserializer}; use std::path::PathBuf; @@ -16,20 +16,16 @@ pub trait FileAction: Action { fn resolve(&self, manifest: &Manifest, path: &str) -> anyhow::Result { Ok(manifest .root_dir - .clone() - .ok_or_else(|| anyhow!("Failed because manifest has no root_dir"))? + .as_ref() + .context("Failed because manifest has no root_dir")? .join("files") .join(path) .normalize() - .map_err(|e| { - anyhow!( - "Resolution of {} failed in manifest {} because {}", - path.to_string(), - manifest - .name - .as_ref() - .unwrap_or(&"cannot extract manifest name".to_string()), - e.to_string() + .with_context(|| { + format!( + "Resolution of {} failed in manifest {}", + path, + manifest.get_name() ) })? .as_path() @@ -45,7 +41,7 @@ pub trait FileAction: Action { .join("files") .join(path); - std::fs::read(file_path.clone()).map_err(|e| match e.kind() { + std::fs::read(&*file_path).map_err(|e| match e.kind() { ErrorKind::NotFound => anyhow!( "Failed because {} was not found", file_path.to_string_lossy() diff --git a/lib/src/actions/group/add.rs b/lib/src/actions/group/add.rs index c30b5fb3..f2e751f3 100644 --- a/lib/src/actions/group/add.rs +++ b/lib/src/actions/group/add.rs @@ -20,8 +20,12 @@ impl Action for GroupAdd { let mut atoms: Vec = vec![]; - atoms.append(&mut provider.add_group(&variant, &contexts)); + atoms.append(&mut provider.add_group(&variant, contexts)); Ok(atoms) } + + fn is_privileged(&self) -> bool { + true + } } diff --git a/lib/src/actions/mod.rs b/lib/src/actions/mod.rs index b9db97f0..edcddad9 100644 --- a/lib/src/actions/mod.rs +++ b/lib/src/actions/mod.rs @@ -69,31 +69,21 @@ where let mut scope = crate::contexts::to_rhai(context); let variant = self.variants.iter().find(|variant| { - if variant.condition.is_none() { - return false; - } - - // .unwrap() is safe here because we checked for None above - let condition = variant.condition.clone().unwrap(); - match engine.eval_with_scope::(&mut scope, condition.as_str()) { - Ok(b) => b, - Err(error) => { - error!("Failed execution condition for action: {}", error); - false - } - } + variant.condition.as_ref().is_some_and(|condition| { + engine + .eval_with_scope::(&mut scope, condition) + .inspect_err(|e| error!("Failed execution condition: {}", e)) + .unwrap_or(false) + }) }); if let Some(variant) = variant { return variant.action.plan(manifest, context); } - if self.condition.is_none() { + let Some(condition) = self.condition.as_ref() else { return self.action.plan(manifest, context); - } - - // .unwrap() is safe here because we checked for None above - let condition = self.condition.as_ref().unwrap(); + }; match engine.eval_with_scope::(&mut scope, condition.as_str()) { Ok(true) => self.action.plan(manifest, context), @@ -101,6 +91,10 @@ where Err(error) => Err(anyhow!("Failed execution condition for action: {}", error)), } } + + fn is_privileged(&self) -> bool { + self.action.is_privileged() + } } #[derive(JsonSchema, Clone, Debug, Serialize, Deserialize)] @@ -169,24 +163,60 @@ pub enum Actions { impl Actions { pub fn inner_ref(&self) -> &dyn Action { match self { - Actions::BinaryGitHub(a) => a, - Actions::CommandRun(a) => a, - Actions::DirectoryCopy(a) => a, - Actions::DirectoryCreate(a) => a, - Actions::FileCopy(a) => a, - Actions::FileChown(a) => a, - Actions::FileDownload(a) => a, - Actions::FileLink(a) => a, - Actions::FileUnarchive(a) => a, - Actions::GitClone(a) => a, - Actions::GroupAdd(a) => a, - Actions::MacOSDefault(a) => a, - Actions::PackageInstall(a) => a, - Actions::PackageRepository(a) => a, - Actions::UserAdd(a) => a, - Actions::UserAddGroup(a) => a, - Actions::FileRemove(a) => a, - Actions::DirectoryRemove(a) => a, + Self::BinaryGitHub(a) => a, + Self::CommandRun(a) => a, + Self::DirectoryCopy(a) => a, + Self::DirectoryCreate(a) => a, + Self::FileCopy(a) => a, + Self::FileChown(a) => a, + Self::FileDownload(a) => a, + Self::FileLink(a) => a, + Self::FileUnarchive(a) => a, + Self::GitClone(a) => a, + Self::GroupAdd(a) => a, + Self::MacOSDefault(a) => a, + Self::PackageInstall(a) => a, + Self::PackageRepository(a) => a, + Self::UserAdd(a) => a, + Self::UserAddGroup(a) => a, + Self::FileRemove(a) => a, + Self::DirectoryRemove(a) => a, + } + } + + pub fn is_privileged(&self) -> bool { + self.inner_ref().is_privileged() + } + + pub async fn execute( + &self, + dry_run: bool, + manifest: &Manifest, + contexts: &Contexts, + pm: Option, + ) -> anyhow::Result<()> { + // Need this to ensure if execute is called on an action with it's own + // implementaion of execute, it uses that implementation instead of default. + // for some reason the dyn lookup doesn't resolve correctly. + match self { + Self::BinaryGitHub(a) => a.execute(dry_run, self, manifest, contexts, pm).await, + Self::CommandRun(a) => a.execute(dry_run, self, manifest, contexts, pm).await, + Self::DirectoryCopy(a) => a.execute(dry_run, self, manifest, contexts, pm).await, + Self::DirectoryCreate(a) => a.execute(dry_run, self, manifest, contexts, pm).await, + Self::FileCopy(a) => a.execute(dry_run, self, manifest, contexts, pm).await, + Self::FileChown(a) => a.execute(dry_run, self, manifest, contexts, pm).await, + Self::FileDownload(a) => a.execute(dry_run, self, manifest, contexts, pm).await, + Self::FileLink(a) => a.execute(dry_run, self, manifest, contexts, pm).await, + Self::FileUnarchive(a) => a.execute(dry_run, self, manifest, contexts, pm).await, + Self::GitClone(a) => a.execute(dry_run, self, manifest, contexts, pm).await, + Self::GroupAdd(a) => a.execute(dry_run, self, manifest, contexts, pm).await, + Self::MacOSDefault(a) => a.execute(dry_run, self, manifest, contexts, pm).await, + Self::PackageInstall(a) => a.execute(dry_run, self, manifest, contexts, pm).await, + Self::PackageRepository(a) => a.execute(dry_run, self, manifest, contexts, pm).await, + Self::UserAdd(a) => a.execute(dry_run, self, manifest, contexts, pm).await, + Self::UserAddGroup(a) => a.execute(dry_run, self, manifest, contexts, pm).await, + Self::FileRemove(a) => a.execute(dry_run, self, manifest, contexts, pm).await, + Self::DirectoryRemove(a) => a.execute(dry_run, self, manifest, contexts, pm).await, } } } @@ -232,8 +262,8 @@ pub struct ActionError { impl From for ActionError { fn from(e: E) -> Self { - ActionError { - message: format!("{}", e), + Self { + message: format!("{e}"), } } } @@ -244,6 +274,7 @@ pub trait Action: Send + Sync { warn!("need to define action summarize"); "not found action summarize".to_string() } + fn plan(&self, manifest: &Manifest, context: &Contexts) -> anyhow::Result>; #[instrument(skip_all)] @@ -255,7 +286,7 @@ pub trait Action: Send + Sync { contexts: &Contexts, password_manager: Option, ) -> anyhow::Result<()> { - let steps: Vec<_> = match self.plan(manifest, contexts) { + let steps: Vec = match self.plan(manifest, contexts) { Ok(steps) => steps .into_iter() .filter(|step| step.do_initializers_allow_us_to_run()) @@ -265,8 +296,8 @@ pub trait Action: Send + Sync { }) .collect(), Err(err) => { - error!("Failed Processing: {action}. Action failed to get plan: {err:?}",); - return Ok(()); + error!("Failed Processing: {action}. Action failed to get plan: {err:?}"); + return Err(err); } }; @@ -282,9 +313,15 @@ pub trait Action: Send + Sync { for mut step in steps { step.execute(password_manager.clone()).await?; } + info!("{}", self.summarize()); + Ok(()) } + + fn is_privileged(&self) -> bool { + false + } } #[cfg(test)] diff --git a/lib/src/actions/package/install.rs b/lib/src/actions/package/install.rs index b028eb16..cae029da 100644 --- a/lib/src/actions/package/install.rs +++ b/lib/src/actions/package/install.rs @@ -1,17 +1,21 @@ use super::providers::PackageProviders; use super::Package; use super::PackageVariant; +use super::PACKAGE_LOCK; use crate::actions::Action; +use crate::actions::Actions; use crate::contexts::Contexts; use crate::manifests::Manifest; use crate::steps::Step; +use crate::utilities::password_manager::PasswordManager; use anyhow::anyhow; use std::ops::Deref; use tracing::debug; -use tracing::span; +use tracing::info_span; pub type PackageInstall = Package; +#[async_trait::async_trait] impl Action for PackageInstall { fn summarize(&self) -> String { "Installing packages".to_string() @@ -21,19 +25,13 @@ impl Action for PackageInstall { let variant: PackageVariant = self.into(); let box_provider = variant.provider.clone().get_provider(); let provider = box_provider.deref(); - - let span = span!( - tracing::Level::INFO, - "package.install", - provider = provider.name() - ) - .entered(); - + let span = info_span!("package.install", provider = provider.name()).entered(); let mut atoms: Vec = vec![]; // If the provider isn't available, see if we can bootstrap it if !provider.available() { - if provider.bootstrap(context).is_empty() { + let mut bootstrap = provider.bootstrap(context); + if bootstrap.is_empty() { return Err(anyhow!( "Package Provider, {}, isn't available. Skipping action", provider.name() @@ -48,22 +46,39 @@ impl Action for PackageInstall { } _ => { return Err(anyhow!( - "Package Provider, {}, isn't capabale of local file installs. Skipping action.", - provider.name() - )); + "Package Provider, {}, isn't capabale of local file installs. Skipping action.", + provider.name() + )); } } } - atoms.append(&mut provider.bootstrap(&context)); + atoms.append(&mut bootstrap); } - atoms.append(&mut provider.install(&variant, &context)?); + atoms.append(&mut provider.install(&variant, context)?); span.exit(); Ok(atoms) } + + async fn execute( + &self, + dry_run: bool, + action: &Actions, + manifest: &Manifest, + contexts: &Contexts, + password_manager: Option, + ) -> anyhow::Result<()> { + // Limit concurrent package installs to run exclusively of each-other + let _permit = PACKAGE_LOCK.acquire().await?; + Action::execute(self, dry_run, action, manifest, contexts, password_manager).await + } + + fn is_privileged(&self) -> bool { + true + } } #[cfg(test)] diff --git a/lib/src/actions/package/mod.rs b/lib/src/actions/package/mod.rs index 75235acc..87b2d551 100644 --- a/lib/src/actions/package/mod.rs +++ b/lib/src/actions/package/mod.rs @@ -8,8 +8,11 @@ pub(crate) use repository::PackageRepository; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use std::collections::HashMap; +use tokio::sync::Semaphore; use tracing::debug; +static PACKAGE_LOCK: Semaphore = Semaphore::const_new(1); + #[derive(JsonSchema, Clone, Debug, Default, Serialize, Deserialize)] #[serde(rename = "package.install")] pub struct Package { diff --git a/lib/src/actions/package/providers/paru.rs b/lib/src/actions/package/providers/paru.rs index b85994ba..92e47a94 100644 --- a/lib/src/actions/package/providers/paru.rs +++ b/lib/src/actions/package/providers/paru.rs @@ -125,6 +125,8 @@ impl PackageProvider for Paru { arguments: [ vec![ String::from("-Sq"), + String::from("--sudoflags"), + String::from("-S"), String::from("--batchinstall"), String::from("--needed"), String::from("--noconfirm"), diff --git a/lib/src/actions/package/repository.rs b/lib/src/actions/package/repository.rs index b838a8c7..8aaf2e4b 100644 --- a/lib/src/actions/package/repository.rs +++ b/lib/src/actions/package/repository.rs @@ -1,8 +1,10 @@ use super::providers::PackageProviders; -use crate::actions::Action; +use super::PACKAGE_LOCK; +use crate::actions::{Action, Actions}; use crate::contexts::Contexts; use crate::manifests::Manifest; use crate::steps::Step; +use crate::utilities::password_manager::PasswordManager; use anyhow::anyhow; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; @@ -28,6 +30,7 @@ pub struct RepositoryKey { pub fingerprint: Option, } +#[async_trait::async_trait] impl Action for PackageRepository { fn summarize(&self) -> String { format!("Adding repository {}", self.name) @@ -48,24 +51,39 @@ impl Action for PackageRepository { // If the provider isn't available, see if we can bootstrap it if !provider.available() { - if provider.bootstrap(&context).is_empty() { + let mut bootstrap = provider.bootstrap(context); + + if bootstrap.is_empty() { return Err(anyhow!( "Package Provider, {}, isn't available. Skipping action", provider.name() )); } - atoms.append(&mut provider.bootstrap(&context)); + atoms.append(&mut bootstrap); } if !provider.has_repository(self) { - atoms.append(&mut provider.add_repository(self, &context)?); + atoms.append(&mut provider.add_repository(self, context)?); } span.exit(); Ok(atoms) } + + async fn execute( + &self, + dry_run: bool, + action: &Actions, + manifest: &Manifest, + contexts: &Contexts, + password_manager: Option, + ) -> anyhow::Result<()> { + // Limit concurrent package repository execs to run exclusively of each-other + let _permit = PACKAGE_LOCK.acquire().await?; + Action::execute(self, dry_run, action, manifest, contexts, password_manager).await + } } // #[cfg(test)] diff --git a/lib/src/actions/user/add.rs b/lib/src/actions/user/add.rs index 14152073..d03348ea 100644 --- a/lib/src/actions/user/add.rs +++ b/lib/src/actions/user/add.rs @@ -30,11 +30,11 @@ impl Action for UserAdd { #[cfg(unix)] match uzers::get_user_by_name(&variant.username) { Some(_user) => debug!(message = "User already exists", username = ?variant.username), - None => atoms.append(&mut provider.add_user(&variant, &context)?), + None => atoms.append(&mut provider.add_user(&variant, context)?), } #[cfg(not(unix))] - atoms.append(&mut provider.add_user(&variant, &context)?); + atoms.append(&mut provider.add_user(&variant, context)?); Ok(atoms) } diff --git a/lib/src/actions/user/add_group.rs b/lib/src/actions/user/add_group.rs index 4158b5a8..2c0bdd9b 100644 --- a/lib/src/actions/user/add_group.rs +++ b/lib/src/actions/user/add_group.rs @@ -35,7 +35,7 @@ impl Action for UserAddGroup { let mut atoms: Vec = vec![]; - atoms.append(&mut provider.add_to_group(self, &context)?); + atoms.append(&mut provider.add_to_group(self, context)?); Ok(atoms) } diff --git a/lib/src/actions/user/mod.rs b/lib/src/actions/user/mod.rs index 7ed14848..5dc38f31 100644 --- a/lib/src/actions/user/mod.rs +++ b/lib/src/actions/user/mod.rs @@ -55,39 +55,23 @@ pub struct UserVariant { impl From<&User> for UserVariant { fn from(user: &User) -> Self { - let os = os_info::get(); - - // Check for variant configuration for this OS - let variant = user.variants.get(&os.os_type()); - - // No variant overlays - if variant.is_none() { - return UserVariant { - provider: user.provider.clone(), - username: user.username.clone(), - home_dir: user.home_dir.clone(), - fullname: user.fullname.clone(), - shell: user.shell.clone(), - group: user.group.clone(), - }; + let user = user.clone(); + let mut user_variant = UserVariant { + provider: user.provider, + username: user.username, + home_dir: user.home_dir, + fullname: user.fullname, + shell: user.shell, + group: user.group, }; - // .unwrap() is safe here because we checked for None above - let variant = variant.unwrap(); - - debug!(message = "Built Variant", variant = ?variant); - - let mut user = UserVariant { - provider: user.provider.clone(), - username: user.username.clone(), - home_dir: user.home_dir.clone(), - fullname: user.fullname.clone(), - shell: user.shell.clone(), - group: user.group.clone(), + let Some(variant) = user.variants.get(&os_info::get().os_type()) else { + return user_variant; }; - user.provider = variant.provider.clone(); + debug!(message = "Built Variant", variant = ?variant); - user + user_variant.provider = variant.provider.clone(); + user_variant } } diff --git a/lib/src/atoms/command/exec.rs b/lib/src/atoms/command/exec.rs index 1490a2c6..81ee8331 100644 --- a/lib/src/atoms/command/exec.rs +++ b/lib/src/atoms/command/exec.rs @@ -1,6 +1,6 @@ use std::{process::Stdio, sync::Arc}; -use anyhow::{anyhow, Result}; +use anyhow::{anyhow, Context, Result}; use tokio::{ io::{AsyncBufReadExt, AsyncWriteExt, BufReader}, process::Command, @@ -145,6 +145,7 @@ impl Atom for Exec { trace!("{line}"); if line.to_lowercase().contains("password") { if let Some(pass) = secret.as_deref() { + println!("Printing password: {}", pass); let mut stdin = stdin.write().await; stdin.write_all(pass.as_bytes()).await.unwrap(); stdin.flush().await.unwrap(); @@ -173,6 +174,7 @@ impl Atom for Exec { trace!("{line}"); if line.to_lowercase().contains("password") { if let Some(pass) = secret.as_deref() { + println!("Printing password: {}", pass); let mut stdin = stdin.write().await; stdin.write_all(pass.as_bytes()).await.unwrap(); stdin.flush().await.unwrap(); diff --git a/lib/src/manifests/mod.rs b/lib/src/manifests/mod.rs index 15accf4a..7520474f 100644 --- a/lib/src/manifests/mod.rs +++ b/lib/src/manifests/mod.rs @@ -36,8 +36,7 @@ impl DependencyBarrier { } pub async fn wait(&self, result: bool) -> bool { - let new_can_continue = result & self.can_continue.load(SeqCst); - self.can_continue.store(new_can_continue, SeqCst); + let new_can_continue = result & self.can_continue.fetch_and(result, SeqCst); self.barrier.wait().await; new_can_continue @@ -125,15 +124,12 @@ where { } if let Some(where_condition) = &self.r#where { - let Ok(result) = - Engine::new().eval_with_scope::(&mut to_rhai(contexts), where_condition) - else { - return Err(anyhow!( - "Unable to evaluate 'where' condition '{where_condition}'" - )); - }; + let result = Engine::new() + .eval_with_scope::(&mut to_rhai(contexts), where_condition) + .map_err(|_| anyhow!("Unable to evaluate 'where' condition '{where_condition}'"))?; debug!("Result of 'where' condition '{where_condition}' -> '{result}'"); + if !result { info!("Skipping manifest, because 'where' conditions were false!"); return Ok(()); @@ -142,12 +138,11 @@ where { for action in self.actions.iter() { action - .inner_ref() - .execute(dry_run, action, self, contexts, password_manager.clone()) + .execute(dry_run, self, contexts, password_manager.clone()) .await?; } - info!("Completed: {self}",); + info!("Completed: {self}"); Ok(()) } } diff --git a/lib/src/utilities/password_manager.rs b/lib/src/utilities/password_manager.rs index 1a2e7f09..c755a02d 100644 --- a/lib/src/utilities/password_manager.rs +++ b/lib/src/utilities/password_manager.rs @@ -54,6 +54,8 @@ impl PasswordManager { let mut pass_cmd = Command::new("sudo") .arg("-Sv") .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) .spawn()?; pass_cmd @@ -62,6 +64,12 @@ impl PasswordManager { .context("Error occured while attempting pasword verificaton")? .write_all(format!("{}\n", secret.as_str()).as_bytes())?; - Ok(pass_cmd.wait()?.success()) + let output = pass_cmd.wait_with_output()?; + println!( + "{}, {}", + unsafe { String::from_utf8_unchecked(output.stdout) }, + unsafe { String::from_utf8_unchecked(output.stderr) }, + ); + Ok(output.status.success()) } } From 79f304aede7965612d9e2e3798dcfc88bfe2f3b5 Mon Sep 17 00:00:00 2001 From: JustinBacher <92jbach@gmail.com> Date: Mon, 17 Feb 2025 18:50:01 -0500 Subject: [PATCH 7/8] fix: git clone and file download migrated to work with async --- lib/Cargo.toml | 3 +-- lib/src/atoms/git/clone.rs | 27 ++++++++++++++++++++++++--- lib/src/atoms/http/download.rs | 4 ++-- 3 files changed, 27 insertions(+), 7 deletions(-) diff --git a/lib/Cargo.toml b/lib/Cargo.toml index cc59bcc0..55abadc0 100644 --- a/lib/Cargo.toml +++ b/lib/Cargo.toml @@ -20,7 +20,6 @@ petgraph = "0.6" rand = "0.8" regex = "1.11" reqwest = { version = "0.12", default-features = false, features = [ - "blocking", "rustls-tls", ] } rhai = { version = "1.19", features = ["serde"] } @@ -41,7 +40,7 @@ tar = "0.4.42" flate2 = "1.0.34" file-owner = "0.1.2" gix = { version = "0.68.0", features = [ - "blocking-http-transport-reqwest-rust-tls", + "blocking-http-transport-reqwest-native-tls", "blocking-network-client", ] } gix-protocol = "0.46.1" diff --git a/lib/src/atoms/git/clone.rs b/lib/src/atoms/git/clone.rs index 8713b4f2..585fe619 100644 --- a/lib/src/atoms/git/clone.rs +++ b/lib/src/atoms/git/clone.rs @@ -4,6 +4,7 @@ use crate::utilities::password_manager::PasswordManager; use gix::interrupt; use gix::{progress::Discard, Url}; use std::path::PathBuf; +use tokio::task; use tracing::instrument; #[derive(Default)] @@ -41,9 +42,17 @@ impl Atom for Clone { std::fs::create_dir_all(&self.directory)?; - let mut prepare_clone = gix::prepare_clone(self.repository.clone(), &self.directory)?; - let (mut prepare_checkout, _) = prepare_clone - .fetch_then_checkout(gix::progress::Discard, &interrupt::IS_INTERRUPTED)?; + let repo = self.repository.clone(); + let directory = self.directory.clone(); + + // I wonder if this will cause tokio to freak out + let (mut prepare_checkout, _) = task::spawn_blocking(move || { + let mut prepare_clone = gix::prepare_clone(repo, &directory).unwrap(); + prepare_clone + .fetch_then_checkout(gix::progress::Discard, &interrupt::IS_INTERRUPTED) + .unwrap() + }) + .await?; let (repo, _) = prepare_checkout.main_worktree(Discard, &interrupt::IS_INTERRUPTED)?; @@ -53,4 +62,16 @@ impl Atom for Clone { Ok(()) } + + fn output_string(&self) -> String { + String::from("") + } + + fn error_message(&self) -> String { + String::from("") + } + + fn status_code(&self) -> i32 { + 0 + } } diff --git a/lib/src/atoms/http/download.rs b/lib/src/atoms/http/download.rs index 0f74bd06..5abed6cf 100644 --- a/lib/src/atoms/http/download.rs +++ b/lib/src/atoms/http/download.rs @@ -30,11 +30,11 @@ impl Atom for Download { } async fn execute(&mut self, _: Option) -> anyhow::Result<()> { - let response = reqwest::blocking::get(&self.url)?; + let response = reqwest::get(&self.url).await?; let mut file = File::create(&self.to)?; - let content = response.bytes()?; + let content = response.bytes().await?; file.write_all(&content)?; Ok(()) From 868b6ff87359b610f4725d897c12bb1c6d81247f Mon Sep 17 00:00:00 2001 From: JustinBacher <92jbach@gmail.com> Date: Mon, 17 Feb 2025 19:19:22 -0500 Subject: [PATCH 8/8] fix: applying cargo clippy changes --- app/src/config/mod.rs | 8 ++--- lib/src/actions/file/copy.rs | 2 +- lib/src/actions/file/download.rs | 2 +- lib/src/actions/group/providers/freebsd.rs | 2 +- lib/src/actions/group/providers/linux.rs | 2 +- lib/src/actions/group/providers/macos.rs | 2 +- lib/src/actions/package/providers/aptitude.rs | 13 ++++---- lib/src/actions/package/providers/bsdpkg.rs | 4 +-- lib/src/actions/package/providers/dnf.rs | 6 ++-- lib/src/actions/package/providers/macports.rs | 2 +- lib/src/actions/package/providers/pkgin.rs | 2 +- .../actions/package/providers/snapcraft.rs | 4 +-- lib/src/actions/package/providers/xbps.rs | 2 +- lib/src/actions/package/providers/yay.rs | 2 +- lib/src/actions/package/providers/zypper.rs | 2 +- lib/src/actions/user/providers/freebsd.rs | 6 ++-- lib/src/actions/user/providers/linux.rs | 6 ++-- lib/src/actions/user/providers/macos.rs | 6 ++-- lib/src/atoms/file/copy.rs | 4 +-- lib/src/contexts/privilege.rs | 17 ++++------- lib/src/contexts/variable_include/mod.rs | 2 +- lib/src/contexts/variables.rs | 2 +- lib/src/manifests/load.rs | 2 +- lib/src/values/mod.rs | 30 +++++++++++-------- 24 files changed, 62 insertions(+), 68 deletions(-) diff --git a/app/src/config/mod.rs b/app/src/config/mod.rs index 65ba71d8..e98de007 100644 --- a/app/src/config/mod.rs +++ b/app/src/config/mod.rs @@ -113,7 +113,7 @@ pub(crate) fn load_config(args: &GlobalArgs) -> Result { /// Exits if the user specified an invalid config file path /// returns errors if file read fails or yaml content is not successfully deserialized pub fn lib_config(args: &GlobalArgs) -> anyhow::Result { - let mut config = match find_configs(&args) { + let mut config = match find_configs(args) { Some(config_path) => { let yaml = std::fs::read_to_string(&config_path) .with_context(|| "Found Comtrya.yaml, but was unable to read the contents.")?; @@ -153,8 +153,8 @@ pub fn lib_config(args: &GlobalArgs) -> anyhow::Result { } }; - let mut defines_iterator = args.defines.iter(); - while let Some(pair) = defines_iterator.next() { + let defines_iterator = args.defines.iter(); + for pair in defines_iterator { config.variables.insert(pair.0.clone(), pair.1.clone()); } @@ -265,6 +265,6 @@ mod tests { }; let result = lib_config(&args); - assert!(!result.is_err()); + assert!(result.is_ok()); } } diff --git a/lib/src/actions/file/copy.rs b/lib/src/actions/file/copy.rs index a311db42..54646aba 100644 --- a/lib/src/actions/file/copy.rs +++ b/lib/src/actions/file/copy.rs @@ -9,7 +9,7 @@ use anyhow::anyhow; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use std::error::Error as StdError; -use std::{path::PathBuf, u32}; +use std::path::PathBuf; use tera::Tera; #[derive(JsonSchema, Clone, Debug, Default, PartialEq, Eq, Serialize, Deserialize)] diff --git a/lib/src/actions/file/download.rs b/lib/src/actions/file/download.rs index 7e80ae2b..c4fdf52c 100644 --- a/lib/src/actions/file/download.rs +++ b/lib/src/actions/file/download.rs @@ -6,7 +6,7 @@ use crate::steps::Step; use crate::{actions::Action, contexts::Contexts}; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; -use std::{path::PathBuf, u32}; +use std::path::PathBuf; #[derive(JsonSchema, Clone, Debug, Default, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename = "file.download")] diff --git a/lib/src/actions/group/providers/freebsd.rs b/lib/src/actions/group/providers/freebsd.rs index 0866bf5b..818719c3 100644 --- a/lib/src/actions/group/providers/freebsd.rs +++ b/lib/src/actions/group/providers/freebsd.rs @@ -16,7 +16,7 @@ impl GroupProvider for FreeBSDGroupProvider { } let privilege_provider = - utilities::get_privilege_provider(&contexts).unwrap_or_else(|| "sudo".to_string()); + utilities::get_privilege_provider(contexts).unwrap_or_else(|| "sudo".to_string()); vec![Step { atom: Box::new(Exec { diff --git a/lib/src/actions/group/providers/linux.rs b/lib/src/actions/group/providers/linux.rs index 8de80a1c..653fb369 100644 --- a/lib/src/actions/group/providers/linux.rs +++ b/lib/src/actions/group/providers/linux.rs @@ -25,7 +25,7 @@ impl GroupProvider for LinuxGroupProvider { } let privilege_provider = - utilities::get_privilege_provider(&contexts).unwrap_or_else(|| "sudo".to_string()); + utilities::get_privilege_provider(contexts).unwrap_or_else(|| "sudo".to_string()); vec![Step { atom: Box::new(Exec { diff --git a/lib/src/actions/group/providers/macos.rs b/lib/src/actions/group/providers/macos.rs index 6016e31a..6d828a69 100644 --- a/lib/src/actions/group/providers/macos.rs +++ b/lib/src/actions/group/providers/macos.rs @@ -34,7 +34,7 @@ impl GroupProvider for MacOsGroupProvider { ]; let privilege_provider = - utilities::get_privilege_provider(&contexts).unwrap_or_else(|| "sudo".to_string()); + utilities::get_privilege_provider(contexts).unwrap_or_else(|| "sudo".to_string()); vec![Step { atom: Box::new(Exec { diff --git a/lib/src/actions/package/providers/aptitude.rs b/lib/src/actions/package/providers/aptitude.rs index eca694a9..7f1932f2 100644 --- a/lib/src/actions/package/providers/aptitude.rs +++ b/lib/src/actions/package/providers/aptitude.rs @@ -38,7 +38,7 @@ impl PackageProvider for Aptitude { fn bootstrap(&self, contexts: &Contexts) -> Vec { let privilege_provider = - utilities::get_privilege_provider(&contexts).unwrap_or_else(|| "sudo".to_string()); + utilities::get_privilege_provider(contexts).unwrap_or_else(|| "sudo".to_string()); vec![Step { atom: Box::new(Exec { @@ -74,7 +74,7 @@ impl PackageProvider for Aptitude { let mut signed_by = String::from(""); let privilege_provider = - utilities::get_privilege_provider(&contexts).unwrap_or_else(|| "sudo".to_string()); + utilities::get_privilege_provider(contexts).unwrap_or_else(|| "sudo".to_string()); if repository.key.is_some() { // .unwrap() is safe here because we checked for key.is_some() above @@ -143,7 +143,7 @@ impl PackageProvider for Aptitude { fn install(&self, package: &PackageVariant, contexts: &Contexts) -> anyhow::Result> { let privilege_provider = - utilities::get_privilege_provider(&contexts).unwrap_or_else(|| "sudo".to_string()); + utilities::get_privilege_provider(contexts).unwrap_or_else(|| "sudo".to_string()); Ok(vec![Step { atom: Box::new(Exec { @@ -246,16 +246,13 @@ mod test { &contexts, ); - let steps = match steps { - Ok(s) => s, - Err(_) => vec![], - }; + let steps = steps.unwrap_or_default(); if let Some(step) = steps.first() { let exec = step.atom.to_string(); assert!(exec.contains(" /usr/share/keyrings/")); } else { - assert!(false); + panic!("No steps found"); } } } diff --git a/lib/src/actions/package/providers/bsdpkg.rs b/lib/src/actions/package/providers/bsdpkg.rs index a9d28de4..2f250c1b 100644 --- a/lib/src/actions/package/providers/bsdpkg.rs +++ b/lib/src/actions/package/providers/bsdpkg.rs @@ -36,7 +36,7 @@ impl PackageProvider for BsdPkg { #[instrument(name = "bootstrap", level = "info", skip(self))] fn bootstrap(&self, contexts: &Contexts) -> Vec { let privilege_provider = - utilities::get_privilege_provider(&contexts).unwrap_or_else(|| "sudo".to_string()); + utilities::get_privilege_provider(contexts).unwrap_or_else(|| "sudo".to_string()); vec![Step { atom: Box::new(Exec { @@ -72,7 +72,7 @@ impl PackageProvider for BsdPkg { fn install(&self, package: &PackageVariant, contexts: &Contexts) -> anyhow::Result> { let privilege_provider = - utilities::get_privilege_provider(&contexts).unwrap_or_else(|| "sudo".to_string()); + utilities::get_privilege_provider(contexts).unwrap_or_else(|| "sudo".to_string()); if package.file { return Ok(vec![Step { diff --git a/lib/src/actions/package/providers/dnf.rs b/lib/src/actions/package/providers/dnf.rs index a8ecc54f..1ef078af 100644 --- a/lib/src/actions/package/providers/dnf.rs +++ b/lib/src/actions/package/providers/dnf.rs @@ -29,7 +29,7 @@ impl PackageProvider for Dnf { fn bootstrap(&self, contexts: &Contexts) -> Vec { let privilege_provider = - utilities::get_privilege_provider(&contexts).unwrap_or_else(|| "sudo".to_string()); + utilities::get_privilege_provider(contexts).unwrap_or_else(|| "sudo".to_string()); vec![Step { atom: Box::new(Exec { @@ -60,7 +60,7 @@ impl PackageProvider for Dnf { let mut steps: Vec = vec![]; let privilege_provider = - utilities::get_privilege_provider(&contexts).unwrap_or_else(|| "sudo".to_string()); + utilities::get_privilege_provider(contexts).unwrap_or_else(|| "sudo".to_string()); if repository.key.is_some() { // .unwrap() is safe here because we checked for key presence above @@ -122,7 +122,7 @@ impl PackageProvider for Dnf { fn install(&self, package: &PackageVariant, contexts: &Contexts) -> anyhow::Result> { let privilege_provider = - utilities::get_privilege_provider(&contexts).unwrap_or_else(|| "sudo".to_string()); + utilities::get_privilege_provider(contexts).unwrap_or_else(|| "sudo".to_string()); Ok(vec![Step { atom: Box::new(Exec { diff --git a/lib/src/actions/package/providers/macports.rs b/lib/src/actions/package/providers/macports.rs index 7b8f6df2..a874c814 100644 --- a/lib/src/actions/package/providers/macports.rs +++ b/lib/src/actions/package/providers/macports.rs @@ -58,7 +58,7 @@ impl PackageProvider for Macports { }; let privilege_provider = - utilities::get_privilege_provider(&contexts).unwrap_or_else(|| "sudo".to_string()); + utilities::get_privilege_provider(contexts).unwrap_or_else(|| "sudo".to_string()); Ok(vec![Step { atom: Box::new(Exec { diff --git a/lib/src/actions/package/providers/pkgin.rs b/lib/src/actions/package/providers/pkgin.rs index e4c8de76..1796517e 100644 --- a/lib/src/actions/package/providers/pkgin.rs +++ b/lib/src/actions/package/providers/pkgin.rs @@ -54,7 +54,7 @@ impl PackageProvider for Pkgin { fn install(&self, package: &PackageVariant, contexts: &Contexts) -> anyhow::Result> { let privilege_provider = - utilities::get_privilege_provider(&contexts).unwrap_or_else(|| "sudo".to_string()); + utilities::get_privilege_provider(contexts).unwrap_or_else(|| "sudo".to_string()); Ok(vec![ Step { diff --git a/lib/src/actions/package/providers/snapcraft.rs b/lib/src/actions/package/providers/snapcraft.rs index b3bc199f..e94f4d1f 100644 --- a/lib/src/actions/package/providers/snapcraft.rs +++ b/lib/src/actions/package/providers/snapcraft.rs @@ -28,7 +28,7 @@ impl PackageProvider for Snapcraft { fn bootstrap(&self, contexts: &Contexts) -> Vec { let privilege_provider = - utilities::get_privilege_provider(&contexts).unwrap_or_else(|| "sudo".to_string()); + utilities::get_privilege_provider(contexts).unwrap_or_else(|| "sudo".to_string()); vec![Step { atom: Box::new(Exec { @@ -65,7 +65,7 @@ impl PackageProvider for Snapcraft { fn install(&self, package: &PackageVariant, contexts: &Contexts) -> anyhow::Result> { let privilege_provider = - utilities::get_privilege_provider(&contexts).unwrap_or_else(|| "sudo".to_string()); + utilities::get_privilege_provider(contexts).unwrap_or_else(|| "sudo".to_string()); Ok(vec![Step { atom: Box::new(Exec { command: String::from("snap"), diff --git a/lib/src/actions/package/providers/xbps.rs b/lib/src/actions/package/providers/xbps.rs index 7e5dab78..577d4f96 100644 --- a/lib/src/actions/package/providers/xbps.rs +++ b/lib/src/actions/package/providers/xbps.rs @@ -99,7 +99,7 @@ impl PackageProvider for Xbps { } let privilege_provider = - utilities::get_privilege_provider(&contexts).unwrap_or_else(|| "sudo".to_string()); + utilities::get_privilege_provider(contexts).unwrap_or_else(|| "sudo".to_string()); Ok(vec![Step { atom: Box::new(Exec { diff --git a/lib/src/actions/package/providers/yay.rs b/lib/src/actions/package/providers/yay.rs index 8662b3a0..d34b1a0d 100644 --- a/lib/src/actions/package/providers/yay.rs +++ b/lib/src/actions/package/providers/yay.rs @@ -32,7 +32,7 @@ impl PackageProvider for Yay { fn bootstrap(&self, contexts: &Contexts) -> Vec { let privilege_provider = - utilities::get_privilege_provider(&contexts).unwrap_or_else(|| "sudo".to_string()); + utilities::get_privilege_provider(contexts).unwrap_or_else(|| "sudo".to_string()); vec![ Step { diff --git a/lib/src/actions/package/providers/zypper.rs b/lib/src/actions/package/providers/zypper.rs index 06018408..c7c7b134 100644 --- a/lib/src/actions/package/providers/zypper.rs +++ b/lib/src/actions/package/providers/zypper.rs @@ -48,7 +48,7 @@ impl PackageProvider for Zypper { fn install(&self, package: &PackageVariant, contexts: &Contexts) -> anyhow::Result> { let privilege_provider = - utilities::get_privilege_provider(&contexts).unwrap_or_else(|| "sudo".to_string()); + utilities::get_privilege_provider(contexts).unwrap_or_else(|| "sudo".to_string()); Ok(vec![Step { atom: Box::new(Exec { diff --git a/lib/src/actions/user/providers/freebsd.rs b/lib/src/actions/user/providers/freebsd.rs index bd0ef4bb..a41b5196 100644 --- a/lib/src/actions/user/providers/freebsd.rs +++ b/lib/src/actions/user/providers/freebsd.rs @@ -43,7 +43,7 @@ impl UserProvider for FreeBSDUserProvider { args.push(String::from("random")); let privilege_provider = - utilities::get_privilege_provider(&contexts).unwrap_or_else(|| "sudo".to_string()); + utilities::get_privilege_provider(contexts).unwrap_or_else(|| "sudo".to_string()); let mut steps: Vec = vec![Step { atom: Box::new(Exec { @@ -66,7 +66,7 @@ impl UserProvider for FreeBSDUserProvider { group: user.group.clone(), provider: user.provider.clone(), }; - for group in self.add_to_group(&user_groups, &contexts)? { + for group in self.add_to_group(&user_groups, contexts)? { steps.push(group); } } @@ -88,7 +88,7 @@ impl UserProvider for FreeBSDUserProvider { } let privilege_provider = - utilities::get_privilege_provider(&contexts).unwrap_or_else(|| "sudo".to_string()); + utilities::get_privilege_provider(contexts).unwrap_or_else(|| "sudo".to_string()); for group in user.group.iter() { steps.push(Step { diff --git a/lib/src/actions/user/providers/linux.rs b/lib/src/actions/user/providers/linux.rs index df3756e0..fc27ad54 100644 --- a/lib/src/actions/user/providers/linux.rs +++ b/lib/src/actions/user/providers/linux.rs @@ -48,7 +48,7 @@ impl UserProvider for LinuxUserProvider { } let privilege_provider = - utilities::get_privilege_provider(&contexts).unwrap_or_else(|| "sudo".to_string()); + utilities::get_privilege_provider(contexts).unwrap_or_else(|| "sudo".to_string()); let mut steps: Vec = vec![Step { atom: Box::new(Exec { @@ -68,7 +68,7 @@ impl UserProvider for LinuxUserProvider { group: user.group.clone(), provider: user.provider.clone(), }; - for group in self.add_to_group(&user_groups, &contexts)? { + for group in self.add_to_group(&user_groups, contexts)? { steps.push(group); } } @@ -96,7 +96,7 @@ impl UserProvider for LinuxUserProvider { } let privilege_provider = - utilities::get_privilege_provider(&contexts).unwrap_or_else(|| "sudo".to_string()); + utilities::get_privilege_provider(contexts).unwrap_or_else(|| "sudo".to_string()); let mut steps: Vec = vec![]; diff --git a/lib/src/actions/user/providers/macos.rs b/lib/src/actions/user/providers/macos.rs index 3a575646..82ef9e19 100644 --- a/lib/src/actions/user/providers/macos.rs +++ b/lib/src/actions/user/providers/macos.rs @@ -46,7 +46,7 @@ impl UserProvider for MacOSUserProvider { } let privilege_provider = - utilities::get_privilege_provider(&contexts).unwrap_or_else(|| "sudo".to_string()); + utilities::get_privilege_provider(contexts).unwrap_or_else(|| "sudo".to_string()); let mut steps: Vec = vec![Step { atom: Box::new(Exec { @@ -66,7 +66,7 @@ impl UserProvider for MacOSUserProvider { group: user.group.clone(), provider: user.provider.clone(), }; - for group in self.add_to_group(&user_group, &contexts)? { + for group in self.add_to_group(&user_group, contexts)? { steps.push(group); } } @@ -94,7 +94,7 @@ impl UserProvider for MacOSUserProvider { } let privilege_provider = - utilities::get_privilege_provider(&contexts).unwrap_or_else(|| "sudo".to_string()); + utilities::get_privilege_provider(contexts).unwrap_or_else(|| "sudo".to_string()); let mut steps: Vec = vec![]; diff --git a/lib/src/atoms/file/copy.rs b/lib/src/atoms/file/copy.rs index 909d85fd..d92206c0 100644 --- a/lib/src/atoms/file/copy.rs +++ b/lib/src/atoms/file/copy.rs @@ -41,13 +41,13 @@ impl Atom for Copy { }); } - return Ok(Outcome { + Ok(Outcome { side_effects: vec![], should_run: !diff( &self.from.display().to_string(), &self.to.display().to_string(), ), - }); + }) } async fn execute(&mut self, _: Option) -> anyhow::Result<()> { diff --git a/lib/src/contexts/privilege.rs b/lib/src/contexts/privilege.rs index ed3e9763..927fff0d 100644 --- a/lib/src/contexts/privilege.rs +++ b/lib/src/contexts/privilege.rs @@ -2,9 +2,10 @@ use crate::config::Config; use crate::contexts::{Context, ContextProvider}; use serde::{Deserialize, Serialize}; use std::fmt::Display; -#[derive(Debug, Serialize, Deserialize, Clone)] +#[derive(Debug, Serialize, Deserialize, Clone, Default)] pub enum Privilege { #[serde(alias = "sudo")] + #[default] Sudo, #[serde(alias = "doas")] @@ -14,12 +15,6 @@ pub enum Privilege { Run0, } -impl Default for Privilege { - fn default() -> Self { - Privilege::Sudo - } -} - impl Display for Privilege { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { let str = match self { @@ -35,18 +30,16 @@ pub struct PrivilegeContextProvider<'a> { pub config: &'a Config, } -impl<'a> ContextProvider for PrivilegeContextProvider<'a> { +impl ContextProvider for PrivilegeContextProvider<'_> { fn get_prefix(&self) -> String { "privilege".to_string() } fn get_contexts(&self) -> anyhow::Result> { - let mut contexts = vec![]; - - contexts.push(Context::KeyValueContext( + let contexts = vec![Context::KeyValueContext( "privilege".to_string(), self.config.privilege.to_string().into(), - )); + )]; Ok(contexts) } diff --git a/lib/src/contexts/variable_include/mod.rs b/lib/src/contexts/variable_include/mod.rs index b8088210..e1c7ed62 100644 --- a/lib/src/contexts/variable_include/mod.rs +++ b/lib/src/contexts/variable_include/mod.rs @@ -12,7 +12,7 @@ pub struct VariableIncludeContextProvider<'a> { pub config: &'a Config, } -impl<'a> ContextProvider for VariableIncludeContextProvider<'a> { +impl ContextProvider for VariableIncludeContextProvider<'_> { fn get_prefix(&self) -> String { String::from("include_variables") } diff --git a/lib/src/contexts/variables.rs b/lib/src/contexts/variables.rs index 0869d926..60ff955b 100644 --- a/lib/src/contexts/variables.rs +++ b/lib/src/contexts/variables.rs @@ -9,7 +9,7 @@ pub struct VariablesContextProvider<'a> { pub config: &'a Config, } -impl<'a> ContextProvider for VariablesContextProvider<'a> { +impl ContextProvider for VariablesContextProvider<'_> { fn get_prefix(&self) -> String { String::from("variables") } diff --git a/lib/src/manifests/load.rs b/lib/src/manifests/load.rs index 8052f011..2a5637b9 100644 --- a/lib/src/manifests/load.rs +++ b/lib/src/manifests/load.rs @@ -24,7 +24,7 @@ pub fn load(manifest_path: PathBuf, contexts: &Contexts) -> HashMap> From> for Value { } } -impl ToString for Value { - fn to_string(&self) -> String { - match self { - Value::Null => "null".to_string(), - Value::String(string) => string.to_owned(), - Value::Number(number) => number.to_string(), - Value::List(list) => list - .iter() - .map(|value| value.to_string()) - .collect::>() - .join(","), - } +impl Display for Value { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> std::fmt::Result { + write!( + f, + "{}", + match self { + Value::Null => "null".to_string(), + Value::String(string) => string.to_owned(), + Value::Number(number) => number.to_string(), + Value::List(list) => list + .iter() + .map(|value| value.to_string()) + .collect::>() + .join(","), + } + ) } }