From 4155dfe14e1ecb4dab437b93693feafdad648198 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Beamonte?= Date: Fri, 1 Dec 2023 00:23:39 -0500 Subject: [PATCH] =?UTF-8?q?fea(updater):=20=E2=9C=A8=20pre-auth=20to=20hos?= =?UTF-8?q?ts=20to=20avoid=20multiple=20auth=20queries=20(#249)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When doing updates, multiple `git pull` are being done in parallel, which can be bothersome if using a password on the ssh key or a ssh agent with authentication, since with multiple parallel queries, the user will have to give multiple approvals. This adds a pre-auth step when doing updates that will run a `ls-remote` on each individual host found in the repositories to trigger the authentication process. Failed auth will lead to skipping updates for repositories in that host. --- src/internal/config/config_value.rs | 2 + src/internal/config/parser.rs | 6 ++ src/internal/git/updater.rs | 86 ++++++++++++++++--- .../010250-path_repo_updates/010250-self.md | 6 ++ 4 files changed, 88 insertions(+), 12 deletions(-) diff --git a/src/internal/config/config_value.rs b/src/internal/config/config_value.rs index 2573f639..0f341eab 100644 --- a/src/internal/config/config_value.rs +++ b/src/internal/config/config_value.rs @@ -150,6 +150,8 @@ path: path_repo_updates: enabled: true self_update: ask + pre_auth: true + pre_auth_timeout: 120 background_updates: true background_updates_timeout: 3600 # 1 hour interval: 43200 # 12 hours diff --git a/src/internal/config/parser.rs b/src/internal/config/parser.rs index 5f89b8a4..52f3ba6a 100644 --- a/src/internal/config/parser.rs +++ b/src/internal/config/parser.rs @@ -690,6 +690,8 @@ impl PathEntryConfig { pub struct PathRepoUpdatesConfig { pub enabled: bool, pub self_update: PathRepoUpdatesSelfUpdateEnum, + pub pre_auth: bool, + pub pre_auth_timeout: u64, pub background_updates: bool, pub background_updates_timeout: u64, pub interval: u64, @@ -729,6 +731,10 @@ impl PathRepoUpdatesConfig { }, (None, None) => PathRepoUpdatesSelfUpdateEnum::Ask, }, + pre_auth: config_value.get_as_bool("pre_auth").unwrap_or(true), + pre_auth_timeout: config_value + .get_as_unsigned_integer("pre_auth_timeout") + .unwrap_or(120), background_updates: config_value .get_as_bool("background_updates") .unwrap_or(true), diff --git a/src/internal/git/updater.rs b/src/internal/git/updater.rs index 26e74f67..a8ee4845 100644 --- a/src/internal/git/updater.rs +++ b/src/internal/git/updater.rs @@ -24,6 +24,7 @@ use crate::internal::config::up::utils::RunConfig; use crate::internal::config::up::utils::SpinnerProgressHandler; use crate::internal::env::current_exe; use crate::internal::env::shell_is_interactive; +use crate::internal::git::full_git_url_parse; use crate::internal::git::path_entry_config; use crate::internal::git_env; use crate::internal::self_update; @@ -207,7 +208,7 @@ pub fn report_update_error() { } } -pub fn trigger_background_update(skip_paths: Vec) -> bool { +pub fn trigger_background_update(skip_paths: HashSet) -> bool { let mut command = Command::new(current_exe()); command.arg("--update-and-log-on-error"); command.stdin(std::process::Stdio::null()); @@ -240,7 +241,7 @@ pub fn update( force_update: bool, allow_background_update: bool, force_sync: Vec, -) -> (Vec, Vec) { +) -> (HashSet, HashSet) { // Get the configuration let config = global_config(); @@ -249,7 +250,7 @@ pub fn update( // Check if OMNI_SKIP_UPDATE_PATH is set, in which case we // can parse it into a list of paths to skip - let skip_update_path: Vec = + let skip_update_path: HashSet = if let Some(skip_update_path) = std::env::var_os("OMNI_SKIP_UPDATE_PATH") { skip_update_path .to_str() @@ -258,25 +259,86 @@ pub fn update( .map(PathBuf::from) .collect() } else { - vec![] + HashSet::new() }; // Nothing to do if nothing is in the omnipath and we don't // check for omni updates if omnipath_entries.is_empty() && config.path_repo_updates.self_update.do_not_check() { - return (vec![], vec![]); + return (HashSet::new(), HashSet::new()); } + // Nothing to do if we don't need to update if !force_update && !should_update() { - return (vec![], vec![]); + return (HashSet::new(), HashSet::new()); } self_update(); + // Nothing more to do if nothing is in the omnipath if omnipath_entries.is_empty() { - return (vec![], vec![]); + return (HashSet::new(), HashSet::new()); } + // Make sure we run git fetch --dry-run at least once per host + // to trigger ssh agent authentication if needed + // TODO: disable that if no agent is setup for the given host + let mut failed_early_auth = HashSet::new(); + if !config.path_repo_updates.pre_auth { + let mut auth_hosts = HashMap::new(); + for path_entry in &omnipath_entries { + let git_env = git_env(&path_entry.as_string()); + let repo_id = git_env.id(); + if repo_id.is_none() { + continue; + } + let repo_id = repo_id.unwrap(); + let repo_root = git_env.root().unwrap().to_string(); + + if let Ok(git_url) = full_git_url_parse(&repo_id) { + if let Some(host) = git_url.host { + let key = (host.clone(), git_url.scheme.to_string()); + + if let Some(succeeded) = auth_hosts.get(&key) { + if !succeeded { + failed_early_auth.insert(repo_root.clone()); + } + continue; + } + + // Check using git ls-remote + let mut cmd = TokioCommand::new("git"); + cmd.arg("ls-remote"); + cmd.current_dir(&repo_root); + cmd.stdout(std::process::Stdio::piped()); + cmd.stderr(std::process::Stdio::piped()); + + let result = run_command_with_handler( + &mut cmd, + |_stdout, _stderr| { + // Do nothing + }, + RunConfig::new().with_timeout(config.path_repo_updates.pre_auth_timeout), + ); + + auth_hosts.insert(key, result.is_ok()); + if result.is_err() { + omni_error!(format!("failed to authenticate to {}", host.light_cyan())); + failed_early_auth.insert(repo_root); + } + } + } + } + } + + // Add the paths that failed early authentication + // to the list of paths to skip + let skip_update_path = skip_update_path + .iter() + .cloned() + .chain(failed_early_auth.iter().map(PathBuf::from)) + .collect::>(); + // Override allow_background_update if the configuration does not allow it let allow_background_update = if !config.path_repo_updates.background_updates { false @@ -386,8 +448,6 @@ pub fn update( } } - // multiprogress.clear().unwrap(); - if !results.is_empty() { let current_exe = std::env::current_exe(); if current_exe.is_err() { @@ -452,6 +512,8 @@ pub fn update( } } } + + ensure_newline(); } } @@ -464,7 +526,7 @@ pub fn update( None } }) - .collect::>(); + .collect::>(); let errored_paths = updates_per_path .iter() @@ -475,10 +537,10 @@ pub fn update( None } }) - .collect::>(); + .chain(failed_early_auth.iter().map(PathBuf::from)) + .collect::>(); // If we need to update in the background, let's do that now - ensure_newline(); if allow_background_update && count_left_to_update > 0 { trigger_background_update( skip_update_path diff --git a/website/contents/reference/01-configuration/0102-parameters/010250-path_repo_updates/010250-self.md b/website/contents/reference/01-configuration/0102-parameters/010250-path_repo_updates/010250-self.md index a1eafd40..6aa0b1e4 100644 --- a/website/contents/reference/01-configuration/0102-parameters/010250-path_repo_updates/010250-self.md +++ b/website/contents/reference/01-configuration/0102-parameters/010250-path_repo_updates/010250-self.md @@ -13,6 +13,8 @@ Configuration for the automated updates of the repositories in omni path. |------------|----------------|---------------------------------------------------| | `enabled` | boolean | whether or not automated updates are enabled *(default: true)* | | `self_update` | enum: `true`, `false`, `ask`, `nocheck` | whether to update omni if a new release is found (`false` will check for release but only show a message, `true` will automatically install any new release, `ask` will ask the user and `nocheck` will entirely skip checking for updates | +| `pre_auth` | boolean | whether or not to allow pre-auth before updates; pre-auth allows to trigger the authorization process for hosts before updates happen concurrently, avoiding asking multiple authorizations for the same host, at the cost of one extra `git` call per host *(default: true)* | +| `pre_auth_timeout` | integer | the number of seconds after which a pre-auth timeouts *(default: 120)* | | `background_updates` | boolean | whether or not to allow background updates of the repositories *(default: true)* | | `background_updates_timeout` | integer | the number of seconds after which a background update timeouts *(default: 3600)* | | `interval` | integer | the number of seconds to wait between two updates of the repositories *(default: 43200)* | @@ -26,6 +28,10 @@ Configuration for the automated updates of the repositories in omni path. path_repo_updates: enabled: true self_update: ask + pre_auth: true + pre_auth_timeout: 120 # 2 minutes + background_updates: true + background_updates_timeout: 3600 # 1 hour interval: 43200 # 12 hours ref_type: "branch" # branch or tag ref_match: null # regex or null