-
Notifications
You must be signed in to change notification settings - Fork 37
Feat: Add api_token field to client configuration #514
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feat: Add api_token field to client configuration #514
Conversation
5f0213b to
daca788
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Assuming token values do not change during runtime, I would suggest reading the env var once at startup so that invalid values (env var does not exist) for api_key are caught when the config is deserialized.
A helper function can be used with serde's deserialize_with attribute for this. Here is one that can be added to utils.rs if you all want to do this.
// utils.rs
/// Serde helper to deserialize value from environment variable.
pub fn from_env<'de, D>(deserializer: D) -> Result<Option<String>, D::Error>
where
D: Deserializer<'de>,
{
let env_name: Option<String> = Option::deserialize(deserializer)?;
if let Some(env_name) = env_name {
let value = std::env::var(&env_name)
.map_err(|_| serde::de::Error::custom(format!("env var `{env_name}` not found")))?;
Ok(Some(value))
} else {
Ok(None)
}
}
#[cfg(test)]
mod tests {
use serde::Deserialize;
use serde_json::json;
use super::from_env;
#[derive(Debug, Deserialize)]
pub struct Config {
#[serde(default, deserialize_with = "from_env")]
pub api_token: Option<String>,
}
#[test]
fn test_from_env() -> Result<(), Box<dyn std::error::Error>> {
// Test no value
let config: Config = serde_json::from_value(json!({}))?;
assert_eq!(config.api_token, None);
// Test invalid value
let config: Result<Config, serde_json::error::Error> = serde_json::from_value(json!({
"api_token": "DOES_NOT_EXIST"
}));
assert!(config.is_err_and(|err| err.to_string() == "env var `DOES_NOT_EXIST` not found"));
// Test valid value
unsafe {
std::env::set_var("CLIENT_API_TOKEN", "token");
}
let config: Config = serde_json::from_value(json!({
"api_token": "CLIENT_API_TOKEN"
}))?;
assert_eq!(config.api_token, Some("token".into()));
Ok(())
}
}To use with ServiceConfig, just import crate::utils::from_env and add the attribute, e.g.
pub struct ServiceConfig {
// ...
#[serde(default, deserialize_with = "from_env")]
pub api_token: Option<String>,
}HttpClient::inject_api_token() would then just use self.api_token instead of reading via std::env::var().
…om env vars Signed-off-by: Rob Geada <[email protected]>
daca788 to
6712b22
Compare
Signed-off-by: Rob Geada <[email protected]>
6712b22 to
f975ce9
Compare
|
@declark1 thanks for the review- I've incorporated your changes and re-verified that it all works as expected |
declark1
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Description
Introduces
api_tokenas a field to the client config struct:If set, the orchestrator will look for an environment variable matching the value, e.g.,
$MODEL_TOKEN. If it exists, it is injected as a bearer token in the HTTP request. In the example above,Authorization: Bearer $MODEL_TOKENwill be sent as a header in all requests to that client.This enables:
kube-rbac-proxy)Verification
Manual testing confirmed that this works as expected