Skip to content

Commit ee4911e

Browse files
ref: make api_client the single source of truth for the auth token
Token state used to live in two places: the api_client carried it for GraphQL Authorization headers, while OrchestratorConfig.token (and its clone in ExecutorConfig.token) carried it for upload Authorization headers. The two were filled from different paths and could drift — notably in CI with OIDC, where set_oidc_token mutated config but the api_client kept its original token. Centralize on the api_client: - CodSpeedAPIClient now exposes token() and set_token() and is the single mutation point for the credentials. - RunEnvironmentProvider::set_oidc_token(&mut ExecutorConfig) becomes refresh_token(&mut CodSpeedAPIClient). GHA implements it; other providers inherit the no-op default. - The Buildkite "token required" check moves from try_from(config) to check_oidc_configuration(api_client) — token presence is the api_client's authority now. - Token resolution happens once at CLI entry in build_api_client(&cli): --token / CODSPEED_TOKEN takes precedence; the persisted CLI config is only loaded as fallback. - token field deleted from OrchestratorConfig and ExecutorConfig. The uploader's Authorization header and the tokenless metadata flag both read from api_client.token(). - &mut CodSpeedAPIClient is plumbed through cli/run/exec → Orchestrator → upload_all so refresh_token can mutate in place. Refs COD-2628 Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 9c19a9a commit ee4911e

14 files changed

Lines changed: 146 additions & 176 deletions

File tree

src/api_client.rs

Lines changed: 46 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ use std::fmt::Display;
33
use crate::executor::ExecutorName;
44
use crate::prelude::*;
55
use crate::run_environment::RepositoryProvider;
6-
use crate::{cli::Cli, config::CodSpeedConfig};
76
use console::style;
87
use gql_client::{Client as GQLClient, ClientConfig};
98
use nestify::nest;
@@ -12,36 +11,56 @@ use serde::{Deserialize, Serialize};
1211
pub struct CodSpeedAPIClient {
1312
gql_client: GQLClient,
1413
unauthenticated_gql_client: GQLClient,
14+
api_url: String,
15+
/// The token this client authenticates with. Exposed so downstream
16+
/// consumers (the uploader's `Authorization` header, the executor's
17+
/// `CODSPEED_OAUTH_TOKEN` env injection) don't have to thread the
18+
/// token separately from the client.
19+
token: Option<String>,
1520
}
1621

17-
impl TryFrom<(&Cli, &CodSpeedConfig)> for CodSpeedAPIClient {
18-
type Error = Error;
19-
fn try_from((args, codspeed_config): (&Cli, &CodSpeedConfig)) -> Result<Self> {
20-
Ok(Self {
21-
gql_client: build_gql_api_client(codspeed_config, args.api_url.clone(), true),
22-
unauthenticated_gql_client: build_gql_api_client(
23-
codspeed_config,
24-
args.api_url.clone(),
25-
false,
26-
),
27-
})
22+
impl CodSpeedAPIClient {
23+
/// Build a client authenticated with `token` (when `Some`).
24+
///
25+
/// The CLI resolves the effective token at construction time, so
26+
/// callers downstream (the uploader, the executor's env injection,
27+
/// every GraphQL caller) just consume it from the client through
28+
/// [`Self::token`] and don't have to thread the token separately.
29+
pub fn new(token: Option<String>, api_url: String) -> Self {
30+
Self {
31+
gql_client: build_gql_api_client(token.as_deref(), api_url.clone()),
32+
unauthenticated_gql_client: build_gql_api_client(None, api_url.clone()),
33+
api_url,
34+
token,
35+
}
36+
}
37+
38+
/// The token this client currently authenticates with, if any.
39+
///
40+
/// Note: this is not necessarily the token the client was built with —
41+
/// in CI with OIDC, [`Self::set_token`] is called before each upload to
42+
/// rotate the credentials. See [`crate::run_environment::RunEnvironmentProvider::refresh_token`].
43+
pub fn token(&self) -> Option<&str> {
44+
self.token.as_deref()
45+
}
46+
47+
/// Replace the token this client uses for authenticated GraphQL
48+
/// requests and that the uploader pulls for its `Authorization`
49+
/// header. The single mutation point for the credentials.
50+
pub fn set_token(&mut self, token: Option<String>) {
51+
self.gql_client = build_gql_api_client(token.as_deref(), self.api_url.clone());
52+
self.token = token;
2853
}
2954
}
3055

31-
fn build_gql_api_client(
32-
codspeed_config: &CodSpeedConfig,
33-
api_url: String,
34-
with_auth: bool,
35-
) -> GQLClient {
36-
let headers = if with_auth && codspeed_config.auth.token.is_some() {
37-
let mut headers = std::collections::HashMap::new();
38-
headers.insert(
39-
"Authorization".to_string(),
40-
codspeed_config.auth.token.clone().unwrap(),
41-
);
42-
headers
43-
} else {
44-
Default::default()
56+
fn build_gql_api_client(token: Option<&str>, api_url: String) -> GQLClient {
57+
let headers = match token {
58+
Some(token) => {
59+
let mut headers = std::collections::HashMap::new();
60+
headers.insert("Authorization".to_string(), token.to_owned());
61+
headers
62+
}
63+
None => Default::default(),
4564
};
4665

4766
GQLClient::new_with_config(ClientConfig {
@@ -467,7 +486,6 @@ impl CodSpeedAPIClient {
467486
/// Create a test API client with a custom URL for use in tests
468487
#[cfg(test)]
469488
pub fn create_test_client_with_url(api_url: String) -> Self {
470-
let codspeed_config = CodSpeedConfig::default();
471-
Self::try_from((&Cli::test_with_url(api_url), &codspeed_config)).unwrap()
489+
Self::new(None, api_url)
472490
}
473491
}

src/cli/exec/mod.rs

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
use super::ExecAndRunSharedArgs;
22
use crate::api_client::CodSpeedAPIClient;
3-
use crate::config::CodSpeedConfig;
43
use crate::executor;
54
use crate::executor::config::{self, OrchestratorConfig, RepositoryOverride};
65
use crate::instruments::Instruments;
@@ -67,7 +66,6 @@ fn build_orchestrator_config(
6766

6867
Ok(OrchestratorConfig {
6968
upload_url,
70-
token: args.shared.token,
7169
repository_override: args
7270
.shared
7371
.repository
@@ -95,8 +93,7 @@ fn build_orchestrator_config(
9593

9694
pub async fn run(
9795
args: ExecArgs,
98-
api_client: &CodSpeedAPIClient,
99-
codspeed_config: &CodSpeedConfig,
96+
api_client: &mut CodSpeedAPIClient,
10097
project_config: Option<&ProjectConfig>,
10198
setup_cache_dir: Option<&Path>,
10299
) -> Result<()> {
@@ -113,7 +110,7 @@ pub async fn run(
113110
PollResultsOptions::new(false, base_run_id),
114111
)?;
115112

116-
execute_config(config, api_client, codspeed_config, setup_cache_dir).await
113+
execute_config(config, api_client, setup_cache_dir).await
117114
}
118115

119116
/// Core execution logic shared by `codspeed exec` and `codspeed run` with config targets.
@@ -122,8 +119,7 @@ pub async fn run(
122119
/// by the orchestrator when exec targets are present.
123120
pub async fn execute_config(
124121
mut config: OrchestratorConfig,
125-
api_client: &CodSpeedAPIClient,
126-
codspeed_config: &CodSpeedConfig,
122+
api_client: &mut CodSpeedAPIClient,
127123
setup_cache_dir: Option<&Path>,
128124
) -> Result<()> {
129125
// Resolve exec target binary paths so memtrack can discover statically linked
@@ -160,7 +156,7 @@ pub async fn execute_config(
160156
);
161157
}
162158

163-
let orchestrator = executor::Orchestrator::new(config, codspeed_config, api_client).await?;
159+
let orchestrator = executor::Orchestrator::new(config, api_client).await?;
164160

165161
if !orchestrator.is_local() {
166162
super::show_banner();

src/cli/mod.rs

Lines changed: 36 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -123,9 +123,8 @@ pub async fn run() -> Result<()> {
123123
let cli = Cli::parse();
124124
// Important: keep this after the Cli::parse() because the function can exit the process by itself, skipping the drop of the CursorGuard
125125
let _cursor_guard = CursorGuard::new();
126-
let codspeed_config =
127-
CodSpeedConfig::load_with_override(cli.config_name.as_deref(), cli.oauth_token.as_deref())?;
128-
let api_client = CodSpeedAPIClient::try_from((&cli, &codspeed_config))?;
126+
127+
let mut api_client = build_api_client(&cli)?;
129128

130129
// Discover project configuration file
131130
let discovered_config = DiscoveredProjectConfig::discover_and_load(
@@ -152,8 +151,7 @@ pub async fn run() -> Result<()> {
152151
args.shared.experimental.warn_if_active();
153152
run::run(
154153
*args,
155-
&api_client,
156-
&codspeed_config,
154+
&mut api_client,
157155
discovered_config.as_ref(),
158156
setup_cache_dir,
159157
)
@@ -163,8 +161,7 @@ pub async fn run() -> Result<()> {
163161
args.shared.experimental.warn_if_active();
164162
exec::run(
165163
*args,
166-
&api_client,
167-
&codspeed_config,
164+
&mut api_client,
168165
discovered_config.as_ref().map(|d| &d.config),
169166
setup_cache_dir,
170167
)
@@ -180,17 +177,37 @@ pub async fn run() -> Result<()> {
180177
Ok(())
181178
}
182179

183-
impl Cli {
184-
/// Create a test CLI instance with a custom API URL for use in tests
185-
#[cfg(test)]
186-
pub fn test_with_url(api_url: String) -> Self {
187-
Self {
188-
api_url,
189-
oauth_token: None,
190-
config_name: None,
191-
config: None,
192-
setup_cache_dir: None,
193-
command: Commands::Setup(setup::SetupArgs::default()),
180+
/// Build the api client for this invocation, resolving the auth token
181+
/// from the most specific source available. This is the single source
182+
/// of truth for token resolution; the result lives on the returned
183+
/// client and every downstream consumer (GraphQL queries, upload
184+
/// `Authorization` header, executor env injection) reads it from there.
185+
///
186+
/// Priority (most specific first):
187+
/// 1. `--token` / `CODSPEED_TOKEN` — run/exec-level override
188+
/// 2. `--oauth-token` / `CODSPEED_OAUTH_TOKEN` and the persisted CLI
189+
/// token — both live on disk and are loaded together by
190+
/// [`CodSpeedConfig::load_with_override`].
191+
///
192+
/// The CLI config file is only read when no explicit token was passed,
193+
/// so an invocation like `codspeed run --token <X>` never touches the
194+
/// user's `~/.config/codspeed/`.
195+
fn build_api_client(cli: &Cli) -> Result<CodSpeedAPIClient> {
196+
let explicit = match &cli.command {
197+
Commands::Run(args) => args.shared.token.clone(),
198+
Commands::Exec(args) => args.shared.token.clone(),
199+
_ => None,
200+
};
201+
let token = match explicit {
202+
Some(token) => Some(token),
203+
None => {
204+
CodSpeedConfig::load_with_override(
205+
cli.config_name.as_deref(),
206+
cli.oauth_token.as_deref(),
207+
)?
208+
.auth
209+
.token
194210
}
195-
}
211+
};
212+
Ok(CodSpeedAPIClient::new(token, cli.api_url.clone()))
196213
}

src/cli/run/mod.rs

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
use super::ExecAndRunSharedArgs;
22
use crate::api_client::CodSpeedAPIClient;
3-
use crate::config::CodSpeedConfig;
43
use crate::executor;
54
use crate::executor::config::{self, OrchestratorConfig, RepositoryOverride};
65
use crate::instruments::Instruments;
@@ -101,7 +100,6 @@ fn build_orchestrator_config(
101100

102101
Ok(OrchestratorConfig {
103102
upload_url,
104-
token: args.shared.token,
105103
repository_override: args
106104
.shared
107105
.repository
@@ -142,8 +140,7 @@ enum RunTarget<'a> {
142140

143141
pub async fn run(
144142
args: RunArgs,
145-
api_client: &CodSpeedAPIClient,
146-
codspeed_config: &CodSpeedConfig,
143+
api_client: &mut CodSpeedAPIClient,
147144
discovered_config: Option<&DiscoveredProjectConfig>,
148145
setup_cache_dir: Option<&Path>,
149146
) -> Result<()> {
@@ -188,8 +185,7 @@ pub async fn run(
188185
poll_opts,
189186
)?;
190187

191-
let orchestrator =
192-
executor::Orchestrator::new(config, codspeed_config, api_client).await?;
188+
let orchestrator = executor::Orchestrator::new(config, api_client).await?;
193189

194190
if !orchestrator.is_local() {
195191
super::show_banner();
@@ -244,8 +240,7 @@ pub async fn run(
244240
PollResultsOptions::new(false, base_run_id),
245241
)?;
246242
config.working_directory = resolved_working_directory;
247-
super::exec::execute_config(config, api_client, codspeed_config, setup_cache_dir)
248-
.await?;
243+
super::exec::execute_config(config, api_client, setup_cache_dir).await?;
249244
}
250245
}
251246

src/executor/config.rs

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,6 @@ pub enum SimulationTool {
5151
#[derive(Debug, Clone)]
5252
pub struct OrchestratorConfig {
5353
pub upload_url: Url,
54-
pub token: Option<String>,
5554
pub repository_override: Option<RepositoryOverride>,
5655
pub working_directory: Option<String>,
5756

@@ -91,7 +90,6 @@ pub struct OrchestratorConfig {
9190
/// `skip_upload`, `repository_override`) live on [`OrchestratorConfig`].
9291
#[derive(Debug, Clone)]
9392
pub struct ExecutorConfig {
94-
pub token: Option<String>,
9593
pub working_directory: Option<String>,
9694
pub command: String,
9795

@@ -144,10 +142,6 @@ impl RepositoryOverride {
144142
pub const DEFAULT_UPLOAD_URL: &str = "https://api.codspeed.io/upload";
145143

146144
impl OrchestratorConfig {
147-
pub fn set_token(&mut self, token: Option<String>) {
148-
self.token = token;
149-
}
150-
151145
/// Compute the total number of executor runs that will be performed.
152146
///
153147
/// All `Exec` targets are combined into a single invocation, while each
@@ -177,7 +171,6 @@ impl OrchestratorConfig {
177171
enable_introspection: bool,
178172
) -> ExecutorConfig {
179173
ExecutorConfig {
180-
token: self.token.clone(),
181174
working_directory: self.working_directory.clone(),
182175
command,
183176
instruments: self.instruments.clone(),
@@ -195,19 +188,12 @@ impl OrchestratorConfig {
195188
}
196189
}
197190

198-
impl ExecutorConfig {
199-
pub fn set_token(&mut self, token: Option<String>) {
200-
self.token = token;
201-
}
202-
}
203-
204191
#[cfg(test)]
205192
impl OrchestratorConfig {
206193
/// Constructs a new `OrchestratorConfig` with default values for testing purposes
207194
pub fn test() -> Self {
208195
Self {
209196
upload_url: Url::parse(DEFAULT_UPLOAD_URL).unwrap(),
210-
token: None,
211197
repository_override: None,
212198
working_directory: None,
213199
targets: vec![BenchmarkTarget::Entrypoint {

src/executor/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ use crate::prelude::*;
1717
use crate::runner_mode::RunnerMode;
1818
use crate::system::SystemInfo;
1919
use async_trait::async_trait;
20-
pub use config::{BenchmarkTarget, ExecutorConfig, OrchestratorConfig};
20+
pub use config::{BenchmarkTarget, ExecutorConfig};
2121
pub use execution_context::ExecutionContext;
2222
pub use interfaces::ExecutorName;
2323
pub use orchestrator::Orchestrator;

0 commit comments

Comments
 (0)