mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-10-10 03:30:59 +00:00
refactor(server): drop StubEnv newtype for HashMap EnvSource impl
StubEnv was a thin newtype only used by tests but compiled into every build. Implementing EnvSource directly on HashMap<String, String> lets test sites pass a HashMap and removes the type entirely. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
56667c17a8
commit
e7099adf3d
6 changed files with 39 additions and 49 deletions
|
|
@ -80,23 +80,21 @@ impl RealAuthHarness {
|
|||
_ => None,
|
||||
})
|
||||
.expect("auth mode should resolve");
|
||||
let mut secrets = std::collections::HashMap::from([
|
||||
(
|
||||
"SESSION_SECRET".to_string(),
|
||||
TEST_SESSION_SECRET.to_string(),
|
||||
),
|
||||
(
|
||||
"GITHUB_APP_CLIENT_SECRET".to_string(),
|
||||
github_client_secret.clone(),
|
||||
),
|
||||
]);
|
||||
if let Some(token) = dev_token.clone() {
|
||||
secrets.insert("FABRO_DEV_TOKEN".to_string(), token);
|
||||
}
|
||||
let state =
|
||||
create_app_state_with_env_lookup_and_server_secret_env(settings, 5, |_| None, {
|
||||
let mut secrets = std::collections::HashMap::from([
|
||||
(
|
||||
"SESSION_SECRET".to_string(),
|
||||
TEST_SESSION_SECRET.to_string(),
|
||||
),
|
||||
(
|
||||
"GITHUB_APP_CLIENT_SECRET".to_string(),
|
||||
github_client_secret.clone(),
|
||||
),
|
||||
]);
|
||||
if let Some(token) = dev_token.clone() {
|
||||
secrets.insert("FABRO_DEV_TOKEN".to_string(), token);
|
||||
}
|
||||
secrets
|
||||
});
|
||||
create_app_state_with_env_lookup_and_server_secret_env(settings, 5, |_| None, &secrets);
|
||||
let github_base = github_base_url(&twin.base_url);
|
||||
let router = build_router_with_options(
|
||||
state,
|
||||
|
|
|
|||
|
|
@ -1949,6 +1949,7 @@ async fn wait_for_shutdown(mut shutdown_rx: watch::Receiver<bool>) {
|
|||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use std::collections::HashMap;
|
||||
use std::io;
|
||||
use std::sync::atomic::AtomicBool;
|
||||
use std::sync::{Arc, Mutex};
|
||||
|
|
@ -1965,7 +1966,6 @@ mod tests {
|
|||
classify_object_store_validation_error, detect_canonical_url, install_object_store_lookup,
|
||||
lock_unpoisoned, resolve_install_object_store_state, token_is_valid,
|
||||
};
|
||||
use crate::server_secrets::StubEnv;
|
||||
|
||||
#[test]
|
||||
fn token_validation_accepts_any_matching_source() {
|
||||
|
|
@ -2090,7 +2090,8 @@ AWS_SESSION_TOKEN=ambient-session\n\
|
|||
AWS_WEB_IDENTITY_TOKEN_FILE=/tmp/fabro-web-identity-token\n",
|
||||
)
|
||||
.unwrap();
|
||||
let server_secrets = ServerSecrets::load(env_path.clone(), &StubEnv::default()).unwrap();
|
||||
let server_secrets =
|
||||
ServerSecrets::load(env_path.clone(), &HashMap::<String, String>::new()).unwrap();
|
||||
let manual_credentials =
|
||||
InstallAwsCredentialPair::new("submitted-access", "submitted-secret");
|
||||
|
||||
|
|
|
|||
|
|
@ -125,7 +125,7 @@ use crate::jwt_auth::{
|
|||
use crate::run_files::{FilesInFlight, list_run_files, new_files_in_flight};
|
||||
use crate::run_selector::{ResolveRunError, resolve_run_by_selector};
|
||||
use crate::server_secrets::{
|
||||
LlmClientResult, ProviderCredentials, ServerSecrets, StubEnv, auth_issue_message,
|
||||
LlmClientResult, ProviderCredentials, ServerSecrets, auth_issue_message,
|
||||
};
|
||||
use crate::spawn_env::{apply_render_graph_env, apply_worker_env};
|
||||
use crate::{
|
||||
|
|
@ -2495,7 +2495,7 @@ pub fn create_app_state_with_env_lookup(
|
|||
settings,
|
||||
max_concurrent_runs,
|
||||
env_lookup,
|
||||
HashMap::new(),
|
||||
&HashMap::new(),
|
||||
)
|
||||
}
|
||||
|
||||
|
|
@ -2504,7 +2504,7 @@ pub fn create_app_state_with_env_lookup_and_server_secret_env(
|
|||
settings: SettingsLayer,
|
||||
max_concurrent_runs: usize,
|
||||
env_lookup: impl Fn(&str) -> Option<String> + Send + Sync + 'static,
|
||||
server_secret_env: HashMap<String, String>,
|
||||
server_secret_env: &HashMap<String, String>,
|
||||
) -> Arc<AppState> {
|
||||
let (store, artifact_store) = test_store_bundle();
|
||||
let env_lookup: EnvLookup = Arc::new(env_lookup);
|
||||
|
|
@ -2553,7 +2553,7 @@ pub(crate) fn create_test_app_state_with_session_key(
|
|||
store,
|
||||
artifact_store,
|
||||
vault_path,
|
||||
server_secrets: load_test_server_secrets(server_env_path, HashMap::new()),
|
||||
server_secrets: load_test_server_secrets(server_env_path, &HashMap::new()),
|
||||
local_daemon_mode,
|
||||
env_lookup,
|
||||
http_client: Some(fabro_http::test_http_client().expect("test HTTP client should build")),
|
||||
|
|
@ -2589,7 +2589,7 @@ fn default_test_app_state_config(
|
|||
store,
|
||||
artifact_store,
|
||||
vault_path,
|
||||
server_secrets: load_test_server_secrets(server_env_path, HashMap::new()),
|
||||
server_secrets: load_test_server_secrets(server_env_path, &HashMap::new()),
|
||||
local_daemon_mode: false,
|
||||
env_lookup,
|
||||
http_client: Some(fabro_http::test_http_client().expect("test HTTP client should build")),
|
||||
|
|
@ -2646,8 +2646,8 @@ fn default_env_lookup() -> EnvLookup {
|
|||
Arc::new(|name| std::env::var(name).ok())
|
||||
}
|
||||
|
||||
fn load_test_server_secrets(path: PathBuf, env: HashMap<String, String>) -> ServerSecrets {
|
||||
ServerSecrets::load(path, &StubEnv(env)).expect("test server secrets should load")
|
||||
fn load_test_server_secrets(path: PathBuf, env: &HashMap<String, String>) -> ServerSecrets {
|
||||
ServerSecrets::load(path, env).expect("test server secrets should load")
|
||||
}
|
||||
|
||||
pub(crate) fn build_app_state(config: AppStateConfig) -> anyhow::Result<Arc<AppState>> {
|
||||
|
|
@ -7480,7 +7480,7 @@ mod tests {
|
|||
SettingsLayer::default(),
|
||||
5,
|
||||
|_| None,
|
||||
HashMap::from([(WEBHOOK_SECRET_ENV.to_string(), secret)]),
|
||||
&HashMap::from([(WEBHOOK_SECRET_ENV.to_string(), secret)]),
|
||||
);
|
||||
build_router_with_options(
|
||||
state,
|
||||
|
|
@ -7892,10 +7892,7 @@ type = "http"
|
|||
|
||||
let secrets = ServerSecrets::load(
|
||||
dir.path().join("server.env"),
|
||||
&StubEnv(HashMap::from([(
|
||||
"SESSION_SECRET".to_string(),
|
||||
"env-value".to_string(),
|
||||
)])),
|
||||
&HashMap::from([("SESSION_SECRET".to_string(), "env-value".to_string())]),
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
|
|
@ -7977,13 +7974,14 @@ allowed_usernames = ["octocat"]
|
|||
.write(&runtime_directory)
|
||||
.unwrap();
|
||||
|
||||
let server_secret_env = dev_token
|
||||
.map(|token| HashMap::from([("FABRO_DEV_TOKEN".to_string(), token)]))
|
||||
.unwrap_or_default();
|
||||
create_app_state_with_env_lookup_and_server_secret_env(
|
||||
settings,
|
||||
5,
|
||||
|_| None,
|
||||
dev_token
|
||||
.map(|token| HashMap::from([("FABRO_DEV_TOKEN".to_string(), token)]))
|
||||
.unwrap_or_default(),
|
||||
&server_secret_env,
|
||||
)
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -23,12 +23,9 @@ impl EnvSource for ProcessEnv {
|
|||
}
|
||||
}
|
||||
|
||||
#[derive(Clone, Debug, Default)]
|
||||
pub(crate) struct StubEnv(pub(crate) HashMap<String, String>);
|
||||
|
||||
impl EnvSource for StubEnv {
|
||||
impl EnvSource for HashMap<String, String> {
|
||||
fn snapshot(&self) -> HashMap<String, String> {
|
||||
self.0.clone()
|
||||
self.clone()
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -172,7 +169,7 @@ mod tests {
|
|||
use fabro_vault::{SecretType, Vault};
|
||||
use tokio::sync::RwLock as AsyncRwLock;
|
||||
|
||||
use super::{ProviderCredentials, ServerSecrets, StubEnv};
|
||||
use super::{ProviderCredentials, ServerSecrets};
|
||||
use crate::server_secrets::Provider;
|
||||
|
||||
#[tokio::test]
|
||||
|
|
@ -234,10 +231,7 @@ mod tests {
|
|||
|
||||
let secrets = ServerSecrets::load(
|
||||
env_path,
|
||||
&StubEnv(HashMap::from([(
|
||||
"SESSION_SECRET".to_string(),
|
||||
"env-value".to_string(),
|
||||
)])),
|
||||
&HashMap::from([("SESSION_SECRET".to_string(), "env-value".to_string())]),
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
|
|
@ -254,7 +248,7 @@ mod tests {
|
|||
let env_path = dir.path().join("server.env");
|
||||
let mut env = HashMap::from([("SESSION_SECRET".to_string(), "before".to_string())]);
|
||||
|
||||
let secrets = ServerSecrets::load(env_path, &StubEnv(env.clone())).unwrap();
|
||||
let secrets = ServerSecrets::load(env_path, &env.clone()).unwrap();
|
||||
env.insert("SESSION_SECRET".to_string(), "after".to_string());
|
||||
|
||||
assert_eq!(secrets.get("SESSION_SECRET").as_deref(), Some("before"));
|
||||
|
|
|
|||
|
|
@ -59,7 +59,6 @@ mod tests {
|
|||
use fabro_types::settings::ServerSettings as ResolvedServerSettings;
|
||||
|
||||
use super::{resolve_startup, validate_startup};
|
||||
use crate::server_secrets::StubEnv;
|
||||
|
||||
fn resolved_settings(auth_methods: &[&str]) -> ResolvedServerSettings {
|
||||
let settings = parse_settings_layer(&format!(
|
||||
|
|
@ -82,7 +81,7 @@ methods = [{}]
|
|||
#[test]
|
||||
fn validate_startup_matches_resolve_startup() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let env = StubEnv(HashMap::from([
|
||||
let env = HashMap::from([
|
||||
(
|
||||
"SESSION_SECRET".to_string(),
|
||||
"0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef".to_string(),
|
||||
|
|
@ -92,7 +91,7 @@ methods = [{}]
|
|||
"fabro_dev_abababababababababababababababababababababababababababababababab"
|
||||
.to_string(),
|
||||
),
|
||||
]));
|
||||
]);
|
||||
let settings = resolved_settings(&["dev-token"]);
|
||||
|
||||
assert!(validate_startup(dir.path().join("server.env").as_path(), &env, &settings).is_ok());
|
||||
|
|
@ -102,7 +101,7 @@ methods = [{}]
|
|||
#[test]
|
||||
fn validate_startup_and_resolve_startup_share_errors() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let env = StubEnv::default();
|
||||
let env: HashMap<String, String> = HashMap::new();
|
||||
let settings = resolved_settings(&["dev-token"]);
|
||||
|
||||
let validate_err =
|
||||
|
|
|
|||
|
|
@ -150,7 +150,7 @@ async fn github_webhook_spec_route_is_routable_when_webhook_secret_is_present()
|
|||
test_settings(),
|
||||
5,
|
||||
|_| None,
|
||||
std::collections::HashMap::from([("GITHUB_APP_WEBHOOK_SECRET".to_string(), secret)]),
|
||||
&std::collections::HashMap::from([("GITHUB_APP_WEBHOOK_SECRET".to_string(), secret)]),
|
||||
),
|
||||
AuthMode::Disabled,
|
||||
);
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue