Skip to content

Commit de55b8a

Browse files
authored
Merge pull request #56 from cachix/perf/onepassword-batch-fetch
perf(onepassword): cache auth and batch fetch secrets
2 parents af8d223 + ee0a592 commit de55b8a

File tree

7 files changed

+298
-39
lines changed

7 files changed

+298
-39
lines changed

CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## [Unreleased]
99

10+
### Changed
11+
- OnePassword provider: Significant performance improvement by caching authentication status
12+
and using batch fetching with parallel threads. Reduces CLI calls from 2N sequential to
13+
~2 sequential + N parallel for N secrets.
14+
1015
## [0.6.2] - 2026-01-27
1116

1217
### Added

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ trybuild = "1.0"
3535
insta = "1.34"
3636
linkme = "0.3"
3737
secrecy = { version = "0.10.3", features = ["serde"] }
38+
once_cell = "1.21"
3839
google-cloud-secretmanager-v1 = "1.2"
3940
tokio = { version = "1", features = ["rt"] }
4041
secretspec-derive = { version = "0.6.2", path = "./secretspec-derive" }

secretspec/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ url.workspace = true
3434
whoami = { workspace = true, optional = true }
3535
linkme.workspace = true
3636
secrecy.workspace = true
37+
once_cell.workspace = true
3738
google-cloud-secretmanager-v1 = { workspace = true, optional = true }
3839
tokio = { workspace = true, optional = true }
3940

secretspec/src/provider/mod.rs

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -270,6 +270,42 @@ pub trait Provider: Send + Sync {
270270
self.name()
271271
)))
272272
}
273+
274+
/// Retrieves multiple secrets from the provider in a single batch operation.
275+
///
276+
/// This method allows providers to optimize fetching multiple secrets at once,
277+
/// which can significantly improve performance for providers with high latency
278+
/// per request (like cloud-based secret managers).
279+
///
280+
/// # Arguments
281+
///
282+
/// * `project` - The project namespace for the secrets
283+
/// * `keys` - A slice of secret keys to retrieve
284+
/// * `profile` - The profile context (e.g., "default", "production")
285+
///
286+
/// # Returns
287+
///
288+
/// A HashMap where keys are the secret names and values are the secret values.
289+
/// Secrets that don't exist are not included in the result.
290+
///
291+
/// # Default Implementation
292+
///
293+
/// The default implementation calls `get()` for each key sequentially.
294+
/// Providers should override this for better performance when possible.
295+
fn get_batch(
296+
&self,
297+
project: &str,
298+
keys: &[&str],
299+
profile: &str,
300+
) -> Result<HashMap<String, SecretString>> {
301+
let mut results = HashMap::new();
302+
for key in keys {
303+
if let Some(value) = self.get(project, key, profile)? {
304+
results.insert((*key).to_string(), value);
305+
}
306+
}
307+
Ok(results)
308+
}
273309
}
274310

275311
impl TryFrom<String> for Box<dyn Provider> {

secretspec/src/provider/onepassword.rs

Lines changed: 165 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
use crate::provider::Provider;
22
use crate::{Result, SecretSpecError};
3+
use once_cell::sync::OnceCell;
34
use secrecy::{ExposeSecret, SecretString};
45
use serde::{Deserialize, Serialize};
6+
use std::collections::HashMap;
57
use std::process::Command;
68
use url::Url;
79

@@ -240,6 +242,8 @@ pub struct OnePasswordProvider {
240242
config: OnePasswordConfig,
241243
/// The OnePassword CLI command to use (either "op" or a custom path).
242244
op_command: String,
245+
/// Cached authentication status to avoid repeated `op whoami` calls.
246+
auth_verified: OnceCell<bool>,
243247
}
244248

245249
crate::register_provider! {
@@ -265,7 +269,11 @@ impl OnePasswordProvider {
265269
"op".to_string()
266270
}
267271
});
268-
Self { config, op_command }
272+
Self {
273+
config,
274+
op_command,
275+
auth_verified: OnceCell::new(),
276+
}
269277
}
270278

271279
/// Executes a OnePassword CLI command with proper error handling.
@@ -369,7 +377,7 @@ impl OnePasswordProvider {
369377
.map_err(|e| SecretSpecError::ProviderOperationFailed(e.to_string()))
370378
}
371379

372-
/// Checks if the user is authenticated with OnePassword.
380+
/// Checks if the user is authenticated with OnePassword (uncached).
373381
///
374382
/// Uses the `op whoami` command to verify authentication status.
375383
/// This is non-intrusive and doesn't require any permissions.
@@ -391,6 +399,28 @@ impl OnePasswordProvider {
391399
}
392400
}
393401

402+
/// Ensures the user is authenticated, caching the result for subsequent calls.
403+
///
404+
/// This method only calls `op whoami` once per provider instance, significantly
405+
/// improving performance when checking multiple secrets.
406+
///
407+
/// # Returns
408+
///
409+
/// * `Ok(())` - User is authenticated
410+
/// * `Err(_)` - User is not authenticated or command failed
411+
fn ensure_authenticated(&self) -> Result<()> {
412+
let is_authenticated = self.auth_verified.get_or_try_init(|| self.whoami())?;
413+
414+
if *is_authenticated {
415+
Ok(())
416+
} else {
417+
Err(SecretSpecError::ProviderOperationFailed(
418+
"OnePassword authentication required. Please run 'eval $(op signin)' first."
419+
.to_string(),
420+
))
421+
}
422+
}
423+
394424
/// Determines the vault name to use.
395425
///
396426
/// # Arguments
@@ -607,13 +637,8 @@ impl Provider for OnePasswordProvider {
607637
/// * `Ok(None)` - No secret found with the given key
608638
/// * `Err(_)` - Authentication or retrieval error
609639
fn get(&self, project: &str, key: &str, profile: &str) -> Result<Option<SecretString>> {
610-
// Check authentication status first
611-
if !self.whoami()? {
612-
return Err(SecretSpecError::ProviderOperationFailed(
613-
"OnePassword authentication required. Please run 'eval $(op signin)' first."
614-
.to_string(),
615-
));
616-
}
640+
// Check authentication status first (cached)
641+
self.ensure_authenticated()?;
617642

618643
let vault = self.get_vault_name(profile);
619644
let item_name = self.format_item_name(project, key, profile);
@@ -671,13 +696,8 @@ impl Provider for OnePasswordProvider {
671696
/// - Item creation/update failures
672697
/// - Temporary file creation errors
673698
fn set(&self, project: &str, key: &str, value: &SecretString, profile: &str) -> Result<()> {
674-
// Check authentication status first
675-
if !self.whoami()? {
676-
return Err(SecretSpecError::ProviderOperationFailed(
677-
"OnePassword authentication required. Please run 'eval $(op signin)' first."
678-
.to_string(),
679-
));
680-
}
699+
// Check authentication status first (cached)
700+
self.ensure_authenticated()?;
681701

682702
let vault = self.get_vault_name(profile);
683703
let item_name = self.format_item_name(project, key, profile);
@@ -710,6 +730,135 @@ impl Provider for OnePasswordProvider {
710730

711731
Ok(())
712732
}
733+
734+
/// Retrieves multiple secrets from OnePassword in a single batch operation.
735+
///
736+
/// This optimized implementation:
737+
/// 1. Authenticates once (cached)
738+
/// 2. Lists all items in the vault once to identify which secrets exist
739+
/// 3. Fetches only the items that exist, using parallel threads
740+
///
741+
/// This significantly improves performance compared to fetching secrets one-by-one,
742+
/// especially when checking many secrets.
743+
fn get_batch(
744+
&self,
745+
project: &str,
746+
keys: &[&str],
747+
profile: &str,
748+
) -> Result<HashMap<String, SecretString>> {
749+
use std::thread;
750+
751+
if keys.is_empty() {
752+
return Ok(HashMap::new());
753+
}
754+
755+
// Check authentication status first (cached)
756+
self.ensure_authenticated()?;
757+
758+
let vault = self.get_vault_name(profile);
759+
760+
// List all items in the vault once
761+
let args = vec!["item", "list", "--vault", &vault, "--format", "json"];
762+
let output = self.execute_op_command(&args, None)?;
763+
764+
#[derive(Deserialize)]
765+
struct ListItem {
766+
id: String,
767+
title: String,
768+
}
769+
770+
let items: Vec<ListItem> = serde_json::from_str(&output).unwrap_or_default();
771+
772+
// Build a map of item titles to IDs for quick lookup
773+
let item_map: HashMap<String, String> = items
774+
.into_iter()
775+
.map(|item| (item.title, item.id))
776+
.collect();
777+
778+
// Find which keys exist and need to be fetched
779+
let keys_to_fetch: Vec<(&str, String)> = keys
780+
.iter()
781+
.filter_map(|key| {
782+
let item_name = self.format_item_name(project, key, profile);
783+
item_map.get(&item_name).map(|id| (*key, id.clone()))
784+
})
785+
.collect();
786+
787+
// Fetch items in parallel using threads
788+
let vault_clone = vault.clone();
789+
let op_command = self.op_command.clone();
790+
let service_token = self.config.service_account_token.clone();
791+
let account = self.config.account.clone();
792+
793+
let handles: Vec<_> = keys_to_fetch
794+
.into_iter()
795+
.map(|(key, item_id)| {
796+
let vault = vault_clone.clone();
797+
let op_cmd = op_command.clone();
798+
let token = service_token.clone();
799+
let acct = account.clone();
800+
let key_owned = key.to_string();
801+
802+
thread::spawn(move || {
803+
let mut cmd = Command::new(&op_cmd);
804+
805+
if let Some(ref t) = token {
806+
cmd.env("OP_SERVICE_ACCOUNT_TOKEN", t);
807+
}
808+
if let Some(ref a) = acct {
809+
cmd.arg("--account").arg(a);
810+
}
811+
812+
cmd.args([
813+
"item", "get", &item_id, "--vault", &vault, "--format", "json",
814+
]);
815+
816+
match cmd.output() {
817+
Ok(output) if output.status.success() => {
818+
let stdout = String::from_utf8_lossy(&output.stdout);
819+
// Parse the item and extract value
820+
if let Ok(item) = serde_json::from_str::<OnePasswordItem>(&stdout) {
821+
// Look for "value" field first
822+
for field in &item.fields {
823+
if field.label.as_deref() == Some("value") {
824+
if let Some(ref v) = field.value {
825+
return Some((
826+
key_owned,
827+
SecretString::new(v.clone().into()),
828+
));
829+
}
830+
}
831+
}
832+
// Fallback: look for password/concealed field
833+
for field in &item.fields {
834+
if field.field_type == "CONCEALED" || field.id == "password" {
835+
if let Some(ref v) = field.value {
836+
return Some((
837+
key_owned,
838+
SecretString::new(v.clone().into()),
839+
));
840+
}
841+
}
842+
}
843+
}
844+
None
845+
}
846+
_ => None,
847+
}
848+
})
849+
})
850+
.collect();
851+
852+
// Collect results from all threads
853+
let mut results = HashMap::new();
854+
for handle in handles {
855+
if let Ok(Some((key, value))) = handle.join() {
856+
results.insert(key, value);
857+
}
858+
}
859+
860+
Ok(results)
861+
}
713862
}
714863

715864
impl Default for OnePasswordProvider {

0 commit comments

Comments
 (0)