mirror of
https://github.com/aaif-goose/goose.git
synced 2026-07-03 14:10:03 +02:00
Add support for optional api_key configuration for declarative openai-engine providers (#9202)
Signed-off-by: Douwe Osinga <douwe@squareup.com> Co-authored-by: Douwe Osinga <douwe@squareup.com>
This commit is contained in:
@@ -674,7 +674,7 @@ mod tests {
|
||||
assert_eq!(config.name, "llama_swap");
|
||||
assert_eq!(config.display_name, "Llama Swap");
|
||||
assert!(matches!(config.engine, ProviderEngine::OpenAI));
|
||||
assert_eq!(config.api_key_env, "");
|
||||
assert_eq!(config.api_key_env, "LLAMA_SWAP_API_KEY");
|
||||
assert!(!config.requires_auth);
|
||||
assert!(config.skip_canonical_filtering);
|
||||
assert_eq!(config.dynamic_models, Some(true));
|
||||
|
||||
@@ -3,7 +3,7 @@
|
||||
"engine": "openai",
|
||||
"display_name": "Llama Swap",
|
||||
"description": "Local proxy that hot-swaps llama.cpp (and other) inference backends on demand via an OpenAI-compatible API.",
|
||||
"api_key_env": "",
|
||||
"api_key_env": "LLAMA_SWAP_API_KEY",
|
||||
"base_url": "${LLAMA_SWAP_HOST}/v1/chat/completions",
|
||||
"env_vars": [
|
||||
{
|
||||
|
||||
@@ -3,7 +3,7 @@
|
||||
"engine": "openai",
|
||||
"display_name": "LM Studio",
|
||||
"description": "Run local models with LM Studio",
|
||||
"api_key_env": "",
|
||||
"api_key_env": "LMSTUDIO_API_KEY",
|
||||
"base_url": "${LMSTUDIO_HOST}/v1/chat/completions",
|
||||
"env_vars": [
|
||||
{
|
||||
|
||||
@@ -3,7 +3,7 @@
|
||||
"engine": "openai",
|
||||
"display_name": "oMLX",
|
||||
"description": "Run local models with oMLX",
|
||||
"api_key_env": "",
|
||||
"api_key_env": "OMLX_API_KEY",
|
||||
"base_url": "${OMLX_HOST}/v1/chat/completions",
|
||||
"env_vars": [
|
||||
{
|
||||
|
||||
@@ -810,7 +810,7 @@ pub fn declarative_inventory_identity(
|
||||
.public_inputs
|
||||
.insert("headers".to_string(), serialize_string_map(headers)?);
|
||||
}
|
||||
if config.requires_auth && !config.api_key_env.is_empty() {
|
||||
if !config.api_key_env.is_empty() {
|
||||
if let Some(value) = config_secret_value(global, &config.api_key_env) {
|
||||
identity
|
||||
.secret_inputs
|
||||
|
||||
@@ -298,6 +298,53 @@ impl OpenAiProvider {
|
||||
}
|
||||
}
|
||||
|
||||
/// Resolve the API key from a declarative provider config.
|
||||
///
|
||||
/// Returns `Some(key)` if a key is found, `None` if the key is optional/missing,
|
||||
/// or an error if the key is required but missing/unreadable.
|
||||
///
|
||||
/// The `get_secret` closure is used to look up the secret by key name. This allows
|
||||
/// testing without depending on `Config::global()`.
|
||||
pub fn resolve_api_key(
|
||||
config: &DeclarativeProviderConfig,
|
||||
get_secret: &dyn Fn(&str) -> Result<String, crate::config::ConfigError>,
|
||||
) -> Result<Option<String>> {
|
||||
if config.api_key_env.is_empty() {
|
||||
return Ok(None);
|
||||
}
|
||||
|
||||
match get_secret(&config.api_key_env) {
|
||||
Ok(key) => Ok(Some(key)),
|
||||
Err(e) => {
|
||||
use crate::config::ConfigError;
|
||||
match e {
|
||||
ConfigError::NotFound(_) => {
|
||||
if config.requires_auth {
|
||||
anyhow::bail!(
|
||||
"Required API key {} is not set. Configure it via `goose configure` or set the {} environment variable.",
|
||||
config.api_key_env,
|
||||
config.api_key_env
|
||||
);
|
||||
}
|
||||
Ok(None)
|
||||
}
|
||||
other => {
|
||||
if config.requires_auth {
|
||||
anyhow::bail!("Failed to read {}: {}", config.api_key_env, other);
|
||||
} else {
|
||||
tracing::warn!(
|
||||
"Failed to read optional API key {}: {}. Proceeding without authentication.",
|
||||
config.api_key_env,
|
||||
other
|
||||
);
|
||||
Ok(None)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
pub fn from_custom_config(
|
||||
model: ModelConfig,
|
||||
config: DeclarativeProviderConfig,
|
||||
@@ -323,22 +370,7 @@ impl OpenAiProvider {
|
||||
}
|
||||
|
||||
let global_config = crate::config::Config::global();
|
||||
|
||||
let api_key: Option<String> = if config.requires_auth && !config.api_key_env.is_empty() {
|
||||
Some(global_config.get_secret::<String>(&config.api_key_env).map_err(|e| {
|
||||
use crate::config::ConfigError;
|
||||
match e {
|
||||
ConfigError::NotFound(_) => anyhow::anyhow!(
|
||||
"Required API key {} is not set. Configure it via `goose configure` or set the {} environment variable.",
|
||||
config.api_key_env,
|
||||
config.api_key_env
|
||||
),
|
||||
other => anyhow::anyhow!("Failed to read {}: {}", config.api_key_env, other),
|
||||
}
|
||||
})?)
|
||||
} else {
|
||||
None
|
||||
};
|
||||
let api_key = Self::resolve_api_key(&config, &|key| global_config.get_secret(key))?;
|
||||
|
||||
let url = url::Url::parse(&config.base_url)
|
||||
.map_err(|e| anyhow::anyhow!("Invalid base URL '{}': {}", config.base_url, e))?;
|
||||
@@ -1225,4 +1257,83 @@ mod tests {
|
||||
"error message should mention dynamic_models: false; got: {msg}"
|
||||
);
|
||||
}
|
||||
|
||||
// ── resolve_api_key tests ──────────────────────────────────────────────
|
||||
|
||||
fn config_with_key(api_key_env: &str, requires_auth: bool) -> DeclarativeProviderConfig {
|
||||
let mut config = base_declarative_config(vec![], None);
|
||||
config.api_key_env = api_key_env.to_string();
|
||||
config.requires_auth = requires_auth;
|
||||
config
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resolve_api_key_empty_env_returns_none() {
|
||||
let config = config_with_key("", true);
|
||||
assert_eq!(
|
||||
OpenAiProvider::resolve_api_key(&config, &|_| unreachable!()).unwrap(),
|
||||
None
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resolve_api_key_missing_with_requires_auth_bails() {
|
||||
let config = config_with_key("MY_KEY", true);
|
||||
let err = OpenAiProvider::resolve_api_key(&config, &|_| {
|
||||
Err(crate::config::ConfigError::NotFound("x".into()))
|
||||
})
|
||||
.unwrap_err()
|
||||
.to_string();
|
||||
assert!(
|
||||
err.contains("MY_KEY"),
|
||||
"error should mention the key name; got: {err}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resolve_api_key_missing_without_requires_auth_returns_none() {
|
||||
let config = config_with_key("MY_KEY", false);
|
||||
assert_eq!(
|
||||
OpenAiProvider::resolve_api_key(&config, &|_| Err(
|
||||
crate::config::ConfigError::NotFound("x".into())
|
||||
))
|
||||
.unwrap(),
|
||||
None
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resolve_api_key_present_returns_value() {
|
||||
let config = config_with_key("MY_KEY", true);
|
||||
assert_eq!(
|
||||
OpenAiProvider::resolve_api_key(&config, &|_| Ok("secret".into())).unwrap(),
|
||||
Some("secret".to_string())
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resolve_api_key_other_error_bails_when_required() {
|
||||
let config = config_with_key("MY_KEY", true);
|
||||
let err = OpenAiProvider::resolve_api_key(&config, &|_| {
|
||||
Err(crate::config::ConfigError::KeyringError("ring fail".into()))
|
||||
})
|
||||
.unwrap_err()
|
||||
.to_string();
|
||||
assert!(
|
||||
err.contains("MY_KEY"),
|
||||
"error should mention the key name; got: {err}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resolve_api_key_other_error_warns_and_returns_none_when_optional() {
|
||||
let config = config_with_key("MY_KEY", false);
|
||||
assert_eq!(
|
||||
OpenAiProvider::resolve_api_key(&config, &|_| Err(
|
||||
crate::config::ConfigError::KeyringError("ring fail".into())
|
||||
))
|
||||
.unwrap(),
|
||||
None
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -186,8 +186,14 @@ impl ProviderRegistry {
|
||||
.collect();
|
||||
|
||||
let mut config_keys = if provider_type == ProviderType::Declarative {
|
||||
if config.requires_auth && !config.api_key_env.is_empty() {
|
||||
vec![ConfigKey::new(&config.api_key_env, true, true, None, true)]
|
||||
if !config.api_key_env.is_empty() {
|
||||
vec![ConfigKey::new(
|
||||
&config.api_key_env,
|
||||
config.requires_auth,
|
||||
true,
|
||||
None,
|
||||
true,
|
||||
)]
|
||||
} else {
|
||||
Vec::new()
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user