Skip to content

Commit bb28eb0

Browse files
committed
more util test coverage and remove duplicate env read from docker_init.rs
1 parent 9782d22 commit bb28eb0

7 files changed

Lines changed: 371 additions & 22 deletions

File tree

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.

crates/cli/src/docker_init.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -344,7 +344,6 @@ fn create_signer_service_local(
344344
let mut envs = IndexMap::from([
345345
get_env_val(CONFIG_ENV, CONFIG_DEFAULT),
346346
get_env_same(JWTS_ENV),
347-
get_env_same(JWTS_ENV),
348347
get_env_same(ADMIN_JWT_ENV),
349348
get_env_val(SIGNER_TLS_CERTIFICATES_PATH_ENV, SIGNER_TLS_CERTIFICATES_PATH_DEFAULT),
350349
]);

crates/common/Cargo.toml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,3 +52,6 @@ tree_hash.workspace = true
5252
tree_hash_derive.workspace = true
5353
unicode-normalization.workspace = true
5454
url.workspace = true
55+
56+
[dev-dependencies]
57+
tempfile.workspace = true

crates/common/src/commit/request.rs

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -318,6 +318,29 @@ mod tests {
318318
let _: SignedProxyDelegationEcdsa = serde_json::from_str(data).unwrap();
319319
}
320320

321+
#[test]
322+
fn test_reload_request_jwt_secrets_present() {
323+
let data = r#"{"jwt_secrets": "module_a=secret1,module_b=secret2"}"#;
324+
let req: ReloadRequest = serde_json::from_str(data).unwrap();
325+
let secrets = req.jwt_secrets.expect("should have secrets");
326+
assert_eq!(secrets.get(&ModuleId("module_a".into())), Some(&"secret1".to_string()));
327+
assert_eq!(secrets.get(&ModuleId("module_b".into())), Some(&"secret2".to_string()));
328+
}
329+
330+
#[test]
331+
fn test_reload_request_jwt_secrets_absent() {
332+
let data = r#"{}"#;
333+
let req: ReloadRequest = serde_json::from_str(data).unwrap();
334+
assert!(req.jwt_secrets.is_none());
335+
}
336+
337+
#[test]
338+
fn test_reload_request_jwt_secrets_invalid_format() {
339+
// Missing '=' separator — decode_string_to_map should fail
340+
let data = r#"{"jwt_secrets": "bad_value_no_equals"}"#;
341+
assert!(serde_json::from_str::<ReloadRequest>(data).is_err());
342+
}
343+
321344
#[test]
322345
fn test_decode_response_proxy_map() {
323346
let data = r#"{

crates/common/src/config/pbs.rs

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -183,19 +183,16 @@ impl PbsConfig {
183183
}
184184

185185
if let Some(rpc_url) = &self.rpc_url {
186-
// TODO: remove this once we support chain ids for custom chains
187-
if !matches!(chain, Chain::Custom { .. }) {
188-
let provider = ProviderBuilder::new().connect_http(rpc_url.clone());
189-
let chain_id = provider.get_chain_id().await?;
190-
let chain_id_big = U256::from(chain_id);
191-
ensure!(
192-
chain_id_big == chain.id(),
193-
"Rpc url is for the wrong chain, expected: {} ({:?}) got {}",
194-
chain.id(),
195-
chain,
196-
chain_id_big
197-
);
198-
}
186+
let provider = ProviderBuilder::new().connect_http(rpc_url.clone());
187+
let chain_id = provider.get_chain_id().await?;
188+
let chain_id_big = U256::from(chain_id);
189+
ensure!(
190+
chain_id_big == chain.id(),
191+
"Rpc url is for the wrong chain, expected: {} ({:?}) got {}",
192+
chain.id(),
193+
chain,
194+
chain_id_big
195+
);
199196
}
200197

201198
ensure!(

crates/common/src/config/utils.rs

Lines changed: 184 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -98,20 +98,68 @@ pub fn decode_string_to_map(raw: &str) -> Result<HashMap<ModuleId, String>> {
9898

9999
#[cfg(test)]
100100
mod tests {
101+
use std::sync::Mutex;
102+
101103
use super::*;
102104
use crate::utils::TestRandomSeed;
103105

104-
/// TODO: This was only used by the old JWT loader, can it be removed now?
106+
// Serializes all tests that read/write environment variables.
107+
// std::env::set_var is unsafe (Rust 1.81+) because mutating `environ`
108+
// while another thread reads it is UB at the OS level. Holding this
109+
// lock ensures our Rust threads don't race each other.
110+
static ENV_LOCK: Mutex<()> = Mutex::new(());
111+
112+
/// Sets or removes env vars for the duration of `f`, then restores the
113+
/// original values. Pass `Some("val")` to set, `None` to ensure absent.
114+
fn with_env<R>(vars: &[(&str, Option<&str>)], f: impl FnOnce() -> R) -> R {
115+
let _guard = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner());
116+
let saved: Vec<(&str, Option<String>)> =
117+
vars.iter().map(|(k, _)| (*k, std::env::var(k).ok())).collect();
118+
for (k, v) in vars {
119+
match v {
120+
Some(val) => unsafe { std::env::set_var(k, val) },
121+
None => unsafe { std::env::remove_var(k) },
122+
}
123+
}
124+
let result = f();
125+
for (k, old) in &saved {
126+
match old {
127+
Some(v) => unsafe { std::env::set_var(k, v) },
128+
None => unsafe { std::env::remove_var(k) },
129+
}
130+
}
131+
result
132+
}
133+
134+
// Minimal TOML-deserializable type used by load_from_file / load_file_from_env
135+
// tests.
136+
#[derive(serde::Deserialize, Debug, PartialEq)]
137+
struct TestConfig {
138+
value: String,
139+
}
140+
141+
// ── decode_string_to_map ─────────────────────────────────────────────────
142+
105143
#[test]
106-
fn test_decode_string_to_map() {
107-
let raw = " KEY=VALUE , KEY2=value2 ";
144+
fn test_decode_string_to_map_single_pair() {
145+
let map = decode_string_to_map("ONLY=ONE").unwrap();
146+
assert_eq!(map.len(), 1);
147+
assert_eq!(map.get(&ModuleId("ONLY".into())), Some(&"ONE".to_string()));
148+
}
108149

109-
let map = decode_string_to_map(raw).unwrap();
150+
#[test]
151+
fn test_decode_string_to_map_empty_string() {
152+
// An empty string yields one token with no `=`, which is invalid.
153+
assert!(decode_string_to_map("").is_err());
154+
}
110155

111-
assert_eq!(map.get(&ModuleId("KEY".into())), Some(&"VALUE".to_string()));
112-
assert_eq!(map.get(&ModuleId("KEY2".into())), Some(&"value2".to_string()));
156+
#[test]
157+
fn test_decode_string_to_map_malformed_no_equals() {
158+
assert!(decode_string_to_map("KEYONLY").is_err());
113159
}
114160

161+
// ── remove_duplicate_keys ────────────────────────────────────────────────
162+
115163
#[test]
116164
fn test_remove_duplicate_keys() {
117165
let key1 = BlsPublicKey::test_random();
@@ -123,4 +171,134 @@ mod tests {
123171
assert!(unique_keys.contains(&key1));
124172
assert!(unique_keys.contains(&key2));
125173
}
174+
175+
// ── load_env_var ─────────────────────────────────────────────────────────
176+
177+
#[test]
178+
fn test_load_env_var_present() {
179+
with_env(&[("CB_TEST_LOAD_ENV_VAR", Some("hello"))], || {
180+
assert_eq!(load_env_var("CB_TEST_LOAD_ENV_VAR").unwrap(), "hello");
181+
});
182+
}
183+
184+
#[test]
185+
fn test_load_env_var_absent() {
186+
with_env(&[("CB_TEST_LOAD_ENV_VAR_ABSENT", None)], || {
187+
let err = load_env_var("CB_TEST_LOAD_ENV_VAR_ABSENT").unwrap_err();
188+
assert!(err.to_string().contains("CB_TEST_LOAD_ENV_VAR_ABSENT"));
189+
});
190+
}
191+
192+
// ── load_optional_env_var ────────────────────────────────────────────────
193+
194+
#[test]
195+
fn test_load_optional_env_var_present() {
196+
with_env(&[("CB_TEST_OPT_VAR", Some("world"))], || {
197+
assert_eq!(load_optional_env_var("CB_TEST_OPT_VAR"), Some("world".to_string()));
198+
});
199+
}
200+
201+
#[test]
202+
fn test_load_optional_env_var_absent() {
203+
with_env(&[("CB_TEST_OPT_VAR_ABSENT", None)], || {
204+
assert_eq!(load_optional_env_var("CB_TEST_OPT_VAR_ABSENT"), None);
205+
});
206+
}
207+
208+
// ── load_from_file ───────────────────────────────────────────────────────
209+
210+
#[test]
211+
fn test_load_from_file_valid() {
212+
use std::io::Write as _;
213+
let mut file = tempfile::NamedTempFile::new().unwrap();
214+
file.write_all(b"value = \"hello\"").unwrap();
215+
let path = file.path().to_path_buf();
216+
217+
let (config, returned_path): (TestConfig, _) = load_from_file(&path).unwrap();
218+
assert_eq!(config.value, "hello");
219+
assert_eq!(returned_path, path);
220+
}
221+
222+
#[test]
223+
fn test_load_from_file_missing() {
224+
let result: eyre::Result<(TestConfig, _)> =
225+
load_from_file("/nonexistent/cb_test_path/file.toml");
226+
assert!(result.is_err());
227+
}
228+
229+
#[test]
230+
fn test_load_from_file_invalid_toml() {
231+
use std::io::Write as _;
232+
let mut file = tempfile::NamedTempFile::new().unwrap();
233+
file.write_all(b"not valid toml !!!{{").unwrap();
234+
235+
let result: eyre::Result<(TestConfig, _)> = load_from_file(file.path());
236+
assert!(result.is_err());
237+
}
238+
239+
// ── load_file_from_env ───────────────────────────────────────────────────
240+
241+
#[test]
242+
fn test_load_file_from_env_ok() {
243+
use std::io::Write as _;
244+
let mut file = tempfile::NamedTempFile::new().unwrap();
245+
file.write_all(b"value = \"from_env\"").unwrap();
246+
let path = file.path().to_str().unwrap().to_owned();
247+
248+
with_env(&[("CB_TEST_FILE_ENV", Some(&path))], || {
249+
let (config, _): (TestConfig, _) = load_file_from_env("CB_TEST_FILE_ENV").unwrap();
250+
assert_eq!(config.value, "from_env");
251+
});
252+
}
253+
254+
#[test]
255+
fn test_load_file_from_env_var_not_set() {
256+
with_env(&[("CB_TEST_FILE_ENV_ABSENT", None)], || {
257+
let result: eyre::Result<(TestConfig, _)> =
258+
load_file_from_env("CB_TEST_FILE_ENV_ABSENT");
259+
assert!(result.is_err());
260+
assert!(result.unwrap_err().to_string().contains("CB_TEST_FILE_ENV_ABSENT"));
261+
});
262+
}
263+
264+
// ── load_jwt_secrets ─────────────────────────────────────────────────────
265+
266+
#[test]
267+
fn test_load_jwt_secrets_ok() {
268+
with_env(
269+
&[
270+
(ADMIN_JWT_ENV, Some("admin_secret")),
271+
(JWTS_ENV, Some("MODULE1=secret1,MODULE2=secret2")),
272+
],
273+
|| {
274+
let (admin_jwt, secrets) = load_jwt_secrets().unwrap();
275+
assert_eq!(admin_jwt, "admin_secret");
276+
assert_eq!(secrets.get(&ModuleId("MODULE1".into())), Some(&"secret1".to_string()));
277+
assert_eq!(secrets.get(&ModuleId("MODULE2".into())), Some(&"secret2".to_string()));
278+
},
279+
);
280+
}
281+
282+
#[test]
283+
fn test_load_jwt_secrets_missing_admin_jwt() {
284+
with_env(&[(ADMIN_JWT_ENV, None), (JWTS_ENV, Some("MODULE1=secret1"))], || {
285+
let err = load_jwt_secrets().unwrap_err();
286+
assert!(err.to_string().contains(ADMIN_JWT_ENV));
287+
});
288+
}
289+
290+
#[test]
291+
fn test_load_jwt_secrets_missing_jwts() {
292+
with_env(&[(ADMIN_JWT_ENV, Some("admin_secret")), (JWTS_ENV, None)], || {
293+
let err = load_jwt_secrets().unwrap_err();
294+
assert!(err.to_string().contains(JWTS_ENV));
295+
});
296+
}
297+
298+
#[test]
299+
fn test_load_jwt_secrets_malformed_jwts() {
300+
with_env(&[(ADMIN_JWT_ENV, Some("admin_secret")), (JWTS_ENV, Some("MALFORMED"))], || {
301+
assert!(load_jwt_secrets().is_err());
302+
});
303+
}
126304
}

0 commit comments

Comments
 (0)