Skip to content

Commit

Permalink
fea(updater): ✨ pre-auth to hosts to avoid multiple auth queries (#249)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
xaf authored Dec 1, 2023
1 parent 105e802 commit 4155dfe
Show file tree
Hide file tree
Showing 4 changed files with 88 additions and 12 deletions.
2 changes: 2 additions & 0 deletions src/internal/config/config_value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions src/internal/config/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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),
Expand Down
86 changes: 74 additions & 12 deletions src/internal/git/updater.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -207,7 +208,7 @@ pub fn report_update_error() {
}
}

pub fn trigger_background_update(skip_paths: Vec<PathBuf>) -> bool {
pub fn trigger_background_update(skip_paths: HashSet<PathBuf>) -> bool {
let mut command = Command::new(current_exe());
command.arg("--update-and-log-on-error");
command.stdin(std::process::Stdio::null());
Expand Down Expand Up @@ -240,7 +241,7 @@ pub fn update(
force_update: bool,
allow_background_update: bool,
force_sync: Vec<PathBuf>,
) -> (Vec<PathBuf>, Vec<PathBuf>) {
) -> (HashSet<PathBuf>, HashSet<PathBuf>) {
// Get the configuration
let config = global_config();

Expand All @@ -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<PathBuf> =
let skip_update_path: HashSet<PathBuf> =
if let Some(skip_update_path) = std::env::var_os("OMNI_SKIP_UPDATE_PATH") {
skip_update_path
.to_str()
Expand All @@ -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::<Vec<_>>();

// Override allow_background_update if the configuration does not allow it
let allow_background_update = if !config.path_repo_updates.background_updates {
false
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -452,6 +512,8 @@ pub fn update(
}
}
}

ensure_newline();
}
}

Expand All @@ -464,7 +526,7 @@ pub fn update(
None
}
})
.collect::<Vec<_>>();
.collect::<HashSet<_>>();

let errored_paths = updates_per_path
.iter()
Expand All @@ -475,10 +537,10 @@ pub fn update(
None
}
})
.collect::<Vec<_>>();
.chain(failed_early_auth.iter().map(PathBuf::from))
.collect::<HashSet<_>>();

// 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)* |
Expand All @@ -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
Expand Down

0 comments on commit 4155dfe

Please sign in to comment.