diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index be1d8420..a79e0c2b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -66,7 +66,7 @@ jobs: submodules: true - uses: ./.github/actions/install-rust - name: Run tests - run: cargo run -- exec -m simulation,walltime,memory --skip-upload --warmup-time 0s --max-rounds 5 -- ls -la + run: cargo run -- exec -m simulation,walltime,memory --warmup-time 0s --max-rounds 5 -- sleep 1 macos-basic-run-test: runs-on: macos-latest diff --git a/Cargo.lock b/Cargo.lock index d606e039..1431888a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1350,7 +1350,7 @@ checksum = "0cc23270f6e1808e30a928bdc84dea0b9b4136a8bc82338574f23baf47bbd280" [[package]] name = "gql_client" version = "1.0.7" -source = "git+https://github.com/CodSpeedHQ/gql-client-rs#83610cc89083cf3b18d7b7e539e76e82121b6ebb" +source = "git+https://github.com/CodSpeedHQ/gql-client-rs?rev=617a889e1d9089b5aa36679f1a5aa13ad01993ae#617a889e1d9089b5aa36679f1a5aa13ad01993ae" dependencies = [ "log", "reqwest 0.11.27", @@ -3983,6 +3983,16 @@ version = "1.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0fda2ff0d084019ba4d7c6f371c95d8fd75ce3524c3cb8fb653a3023f6323e64" +[[package]] +name = "signal-hook-registry" +version = "1.4.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c4db69cba1110affc0e9f7bcd48bbf87b3f4fc7c61fc9155afd4c469eb3d6c1b" +dependencies = [ + "errno", + "libc", +] + [[package]] name = "simd-adler32" version = "0.3.7" @@ -4477,6 +4487,7 @@ dependencies = [ "libc", "mio", "pin-project-lite", + "signal-hook-registry", "socket2 0.6.1", "tokio-macros", "windows-sys 0.61.2", diff --git a/Cargo.toml b/Cargo.toml index 8ffabc4e..bc7e0924 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -30,7 +30,7 @@ serde = { workspace = true } serde_json = { workspace = true, features = ["preserve_order"] } url = "2.4.1" sha256 = "1.4.0" -tokio = { version = "1", features = ["macros", "rt"] } +tokio = { version = "1", features = ["macros", "rt", "signal"] } tokio-tar = { package = "astral-tokio-tar", version = "0.6.0" } tokio-util = "0.7.16" md5 = "0.7.0" @@ -43,7 +43,7 @@ simplelog = { version = "0.12.1", default-features = false, features = [ tempfile = { workspace = true } git2 = "0.20.4" nestify = "0.3.3" -gql_client = { git = "https://github.com/CodSpeedHQ/gql-client-rs" } +gql_client = { git = "https://github.com/CodSpeedHQ/gql-client-rs", rev = "617a889e1d9089b5aa36679f1a5aa13ad01993ae" } serde_yaml = "0.9.34" sysinfo = { version = "0.33.1", features = ["serde"] } indicatif = "0.17.8" diff --git a/src/api_client.rs b/src/api_client.rs index c4a40730..ce9ef6fc 100644 --- a/src/api_client.rs +++ b/src/api_client.rs @@ -3,7 +3,6 @@ use std::fmt::Display; use crate::executor::ExecutorName; use crate::prelude::*; use crate::run_environment::RepositoryProvider; -use crate::{cli::Cli, config::CodSpeedConfig}; use console::style; use gql_client::{Client as GQLClient, ClientConfig}; use nestify::nest; @@ -12,36 +11,62 @@ use serde::{Deserialize, Serialize}; pub struct CodSpeedAPIClient { gql_client: GQLClient, unauthenticated_gql_client: GQLClient, + api_url: String, + /// The token this client authenticates with. Exposed so downstream + /// consumers (the uploader's `Authorization` header, the executor's + /// `CODSPEED_OAUTH_TOKEN` env injection) don't have to thread the + /// token separately from the client. + token: Option, } -impl TryFrom<(&Cli, &CodSpeedConfig)> for CodSpeedAPIClient { - type Error = Error; - fn try_from((args, codspeed_config): (&Cli, &CodSpeedConfig)) -> Result { - Ok(Self { - gql_client: build_gql_api_client(codspeed_config, args.api_url.clone(), true), - unauthenticated_gql_client: build_gql_api_client( - codspeed_config, - args.api_url.clone(), - false, - ), - }) +impl CodSpeedAPIClient { + /// Build a client authenticated with `token` (when `Some`). + /// + /// The CLI resolves the effective token at construction time, so + /// callers downstream (the uploader, the executor's env injection, + /// every GraphQL caller) just consume it from the client through + /// [`Self::token`] and don't have to thread the token separately. + pub fn new(token: Option, api_url: String) -> Self { + Self { + gql_client: build_gql_api_client(token.as_deref(), api_url.clone()), + unauthenticated_gql_client: build_gql_api_client(None, api_url.clone()), + api_url, + token, + } + } + + /// Returns a client that uses `token` for authentication, regardless of + /// the token this client was built with. + pub fn with_token(&self, token: String) -> Self { + Self::new(Some(token), self.api_url.clone()) + } + + /// The token this client currently authenticates with, if any. + /// + /// Note: this is not necessarily the token the client was built with — + /// in CI with OIDC, [`Self::set_token`] is called before each upload to + /// rotate the credentials. See [`crate::run_environment::RunEnvironmentProvider::refresh_token`]. + pub fn token(&self) -> Option<&str> { + self.token.as_deref() + } + + /// Replace the token this client uses for authenticated GraphQL + /// requests and that the uploader pulls for its `Authorization` + /// header. The single mutation point for the credentials. + pub fn set_token(&mut self, token: Option) { + self.gql_client = build_gql_api_client(token.as_deref(), self.api_url.clone()); + self.token = token; } } -fn build_gql_api_client( - codspeed_config: &CodSpeedConfig, - api_url: String, - with_auth: bool, -) -> GQLClient { - let headers = if with_auth && codspeed_config.auth.token.is_some() { - let mut headers = std::collections::HashMap::new(); - headers.insert( - "Authorization".to_string(), - codspeed_config.auth.token.clone().unwrap(), - ); - headers - } else { - Default::default() +fn build_gql_api_client(token: Option<&str>, api_url: String) -> GQLClient { + let headers = match token { + Some(token) => { + let mut headers = std::collections::HashMap::new(); + headers.insert("Authorization".to_string(), token.to_owned()); + headers + } + None => Default::default(), }; GQLClient::new_with_config(ClientConfig { @@ -283,47 +308,141 @@ nest! { } } -#[derive(Serialize, Clone)] +nest! { + #[derive(Debug, Deserialize, Serialize, Clone)]* + #[serde(rename_all = "camelCase")]* + pub struct SessionPayload { + pub user: Option, + } +} + +#[derive(Debug, Deserialize, Serialize)] #[serde(rename_all = "camelCase")] -pub struct GetRepositoryVars { +struct SessionData { + session: SessionPayload, +} + +/// Outcome of [`CodSpeedAPIClient::session`]. The CLI distinguishes +/// "no/expired token" from any other error so it can render a clear message. +pub enum SessionError { + /// Token is missing or no longer valid. + Unauthenticated, + /// Anything else (network, server error, etc). + Other(anyhow::Error), +} + +#[derive(Debug, Deserialize, Serialize, Clone)] +#[serde(rename_all = "camelCase")] +pub struct RepositoryOverviewPayload { pub owner: String, pub name: String, pub provider: RepositoryProvider, + pub has_write_access: bool, } -nest! { - #[derive(Debug, Deserialize, Serialize, Clone)]* - #[serde(rename_all = "camelCase")]* - struct GetRepositoryData { - repository_overview: Option, - user: Option, - } +#[derive(Serialize, Clone)] +#[serde(rename_all = "camelCase")] +pub struct SessionAndRepositoryOverviewVars { + pub owner: String, + pub name: String, + pub provider: Option, } -nest! { - #[derive(Debug, Deserialize, Serialize)]* - #[serde(rename_all = "camelCase")]* - struct CurrentUserData { - user: Option, - } +#[derive(Debug, Deserialize, Serialize)] +#[serde(rename_all = "camelCase")] +struct SessionAndRepositoryOverviewData { + session: SessionPayload, + repository_overview: Option, +} + +pub struct SessionAndRepositoryOverview { + pub session: SessionPayload, + pub repository_overview: Option, +} + +/// Outcome of [`CodSpeedAPIClient::session_and_repository_overview`]. A +/// missing repository is folded into the success path (`repository_overview` +/// becomes `None`); only a missing/expired token or a transport-level +/// failure surfaces here. +pub enum SessionAndRepositoryOverviewError { + Unauthenticated, + Other(anyhow::Error), } impl CodSpeedAPIClient { - pub async fn get_current_user(&self) -> Result> { + /// Introspect the token currently configured on this client. + /// + /// Returns the linked user (when applicable). Used to verify a token's + /// validity without conflating it with repository-level access checks — + /// those are done with [`Self::session_and_repository_overview`]. + pub async fn session(&self) -> std::result::Result { let response = self .gql_client - .query_unwrap::(include_str!("queries/CurrentUser.gql")) + .query_unwrap::(include_str!("queries/Session.gql")) .await; match response { - Ok(data) => Ok(data.user), - Err(err) => bail!("Failed to get current user: {err}"), + Ok(data) => Ok(data.session), + Err(err) if err.contains_error_code("UNAUTHENTICATED") => { + Err(SessionError::Unauthenticated) + } + Err(err) => Err(SessionError::Other(anyhow!( + "Failed to validate token: {err}" + ))), + } + } + + /// Validate the token and look up a candidate repository in one + /// round-trip. Used by `auth status` (with a detected git remote) and + /// the up-front check in `run`/`exec`. + /// + /// `repositoryOverview` is nullable in the schema, so a missing + /// repository surfaces as `repository_overview: None` on the success + /// path. The server still returns a `REPOSITORY_NOT_FOUND` error in + /// that case to avoid leaking existence info, but the partial-data + /// payload carries the `session` field — we deserialize it from the + /// error and treat the call as successful for the session's purposes. + pub async fn session_and_repository_overview( + &self, + vars: SessionAndRepositoryOverviewVars, + ) -> std::result::Result { + let response = self + .gql_client + .query_with_vars_unwrap::< + SessionAndRepositoryOverviewData, + SessionAndRepositoryOverviewVars, + >( + include_str!("queries/SessionAndRepositoryOverview.gql"), + vars, + ) + .await; + match response { + Ok(data) => Ok(SessionAndRepositoryOverview { + session: data.session, + repository_overview: data.repository_overview, + }), + Err(err) if err.contains_error_code("UNAUTHENTICATED") => { + Err(SessionAndRepositoryOverviewError::Unauthenticated) + } + Err(err) if err.contains_error_code("REPOSITORY_NOT_FOUND") => { + match err.data::() { + Some(Ok(data)) => Ok(SessionAndRepositoryOverview { + session: data.session, + repository_overview: None, + }), + Some(Err(decode_err)) => Err(SessionAndRepositoryOverviewError::Other( + anyhow!("Failed to deserialize partial response data: {decode_err}"), + )), + None => Err(SessionAndRepositoryOverviewError::Other(anyhow!( + "Server returned REPOSITORY_NOT_FOUND without partial data: {err}" + ))), + } + } + Err(err) => Err(SessionAndRepositoryOverviewError::Other(anyhow!( + "Failed to validate token and repository: {err}" + ))), } } @@ -423,38 +542,6 @@ impl CodSpeedAPIClient { Err(err) => bail!("Failed to get or create project repository: {err}"), } } - - /// Check if a repository exists in CodSpeed. - /// Returns Some(payload) if the repository exists, None otherwise. - pub async fn get_repository( - &self, - vars: GetRepositoryVars, - ) -> Result> { - let response = self - .gql_client - .query_with_vars_unwrap::( - include_str!("queries/GetRepository.gql"), - vars.clone(), - ) - .await; - match response { - Ok(response) => { - if response.user.is_none() { - bail!( - "Your session has expired, please login again using `codspeed auth login`" - ); - } - Ok(response.repository_overview) - } - Err(err) if err.contains_error_code("REPOSITORY_NOT_FOUND") => Ok(None), - Err(err) if err.contains_error_code("UNAUTHENTICATED") => { - bail!("Your session has expired, please login again using `codspeed auth login`") - } - Err(err) => { - bail!("Failed to get repository: {err}") - } - } - } } impl CodSpeedAPIClient { @@ -467,7 +554,6 @@ impl CodSpeedAPIClient { /// Create a test API client with a custom URL for use in tests #[cfg(test)] pub fn create_test_client_with_url(api_url: String) -> Self { - let codspeed_config = CodSpeedConfig::default(); - Self::try_from((&Cli::test_with_url(api_url), &codspeed_config)).unwrap() + Self::new(None, api_url) } } diff --git a/src/cli/auth.rs b/src/cli/auth.rs index b41339a2..af8c9869 100644 --- a/src/cli/auth.rs +++ b/src/cli/auth.rs @@ -1,7 +1,10 @@ use std::io::Read; use std::time::Duration; -use crate::api_client::{CodSpeedAPIClient, GetRepositoryVars}; +use crate::api_client::{ + CodSpeedAPIClient, RepositoryOverviewPayload, SessionAndRepositoryOverviewError, + SessionAndRepositoryOverviewVars, SessionError, SessionPayload, +}; use crate::cli::run::helpers::{ ParsedRepository, find_repository_root, parse_repository_from_remote, }; @@ -103,6 +106,18 @@ async fn login( token }; + // Validate the token before persisting + let api_client_with_token = api_client.with_token(token.clone()); + api_client_with_token + .session() + .await + .map_err(|err| match err { + SessionError::Unauthenticated => { + anyhow!("Invalid token. The token is either malformed or has expired.") + } + SessionError::Other(err) => err, + })?; + let mut config = CodSpeedConfig::load_with_override(config_name, None)?; config.auth.token = Some(token); config.persist(config_name)?; @@ -123,94 +138,160 @@ fn detect_repository() -> Option { parse_repository_from_remote(url).ok() } +/// Outcome of resolving the auth status, before rendering. +struct AuthStatus { + session: Option, + /// `Some(parsed)` when we detected a git remote and tried to look it up; + /// the inner `Option` is `None` if the repo + /// is not on CodSpeed (or we don't have a token to verify it with). + detected_repository: Option<(ParsedRepository, Option)>, +} + pub async fn status(api_client: &CodSpeedAPIClient) -> Result<()> { let config = CodSpeedConfig::load_with_override(None, None)?; let has_token = config.auth.token.is_some(); - let detected_repo = detect_repository(); + let parsed = detect_repository(); - // 1. Check token validity - let current_user = if has_token { - api_client.get_current_user().await.ok().flatten() + let auth_status = if has_token { + resolve_auth_status(api_client, parsed).await? } else { - None + AuthStatus { + session: None, + detected_repository: parsed.map(|p| (p, None)), + } }; - let token_valid = current_user.is_some(); info!("{}", style("Authentication").bold()); - if let Some(user) = current_user { + print_authentication_section(has_token, auth_status.session.as_ref()); + info!(""); + + info!("{}", style("Repository").bold()); + let local_runs_fallback = print_repository_section(&auth_status.detected_repository, has_token); + + if local_runs_fallback { + warn!( + "Runs will be uploaded to a {} CodSpeed project not associated with any repository.", + crate::cli::exec::DEFAULT_REPOSITORY_NAME + ); + } + + Ok(()) +} + +/// Resolve the session and (when a git remote is detected) the repository overview. +async fn resolve_auth_status( + api_client: &CodSpeedAPIClient, + parsed: Option, +) -> Result { + let Some(parsed) = parsed else { + let session = match api_client.session().await { + Ok(payload) => Some(payload), + Err(SessionError::Unauthenticated) => None, + Err(SessionError::Other(err)) => return Err(err), + }; + return Ok(AuthStatus { + session, + detected_repository: None, + }); + }; + + let combined = api_client + .session_and_repository_overview(SessionAndRepositoryOverviewVars { + owner: parsed.owner.clone(), + name: parsed.name.clone(), + provider: Some(parsed.provider.clone()), + }) + .await; + + match combined { + Ok(payload) => Ok(AuthStatus { + session: Some(payload.session), + detected_repository: Some((parsed, payload.repository_overview)), + }), + Err(SessionAndRepositoryOverviewError::Unauthenticated) => Ok(AuthStatus { + session: None, + detected_repository: Some((parsed, None)), + }), + Err(SessionAndRepositoryOverviewError::Other(err)) => Err(err), + } +} + +fn print_authentication_section(has_token: bool, session: Option<&SessionPayload>) { + let Some(session) = session else { + if has_token { + info!( + " {} Token expired (run {} to re-authenticate)", + cross_mark(), + style("codspeed auth login").cyan() + ); + } else { + info!( + " {} Not logged in (run {} to authenticate)", + cross_mark(), + style("codspeed auth login").cyan() + ); + } + return; + }; + + if let Some(user) = &session.user { info!( " {} Logged in as {} ({})", check_mark(), style(&user.login).bold(), user.provider ); - } else if has_token { - info!( - " {} Token expired (run {} to re-authenticate)", - cross_mark(), - style("codspeed auth login").cyan() - ); } else { - info!( - " {} Not logged in (run {} to authenticate)", - cross_mark(), - style("codspeed auth login").cyan() - ); + // Repo-bound, non-user token (automation today, repo tokens later). + info!(" {} Authenticated", check_mark()); } - info!(""); +} - // 2. If token is valid and we detected a repo, check repository existence - info!("{}", style("Repository").bold()); - let local_runs_fallback = match detected_repo { - Some(parsed) => { - let label = parsed.provider.to_string(); - if token_valid { - let repo_exists = api_client - .get_repository(GetRepositoryVars { - owner: parsed.owner.clone(), - name: parsed.name.clone(), - provider: parsed.provider.clone(), - }) - .await - .ok() - .flatten() - .is_some(); - if repo_exists { - info!( - " {} {}/{} ({})", - check_mark(), - parsed.owner, - parsed.name, - label - ); - false - } else { - info!( - " {} {}/{} ({}, not enabled on CodSpeed)", - cross_mark(), - parsed.owner, - parsed.name, - label - ); - true - } - } else { - info!(" {}/{} ({})", parsed.owner, parsed.name, label); - false - } - } - None => { - info!(" Not inside a git repository"); - true +/// Render the repository section. Returns whether the local-runs fallback +/// warning should be printed. +fn print_repository_section( + detected_repository: &Option<(ParsedRepository, Option)>, + has_token: bool, +) -> bool { + let Some((parsed, overview)) = detected_repository else { + info!(" Not inside a git repository"); + return true; + }; + + let label = parsed.provider.to_string(); + + let Some(overview) = overview else { + if has_token { + info!( + " {} {}/{} ({}, not enabled on CodSpeed)", + cross_mark(), + parsed.owner, + parsed.name, + label + ); + return true; } + info!(" {}/{} ({})", parsed.owner, parsed.name, label); + return false; }; - if local_runs_fallback { - warn!( - "Runs will be uploaded to a {} CodSpeed project not associated with any repository.", - crate::cli::exec::DEFAULT_REPOSITORY_NAME + if overview.has_write_access { + info!( + " {} {}/{} ({})", + check_mark(), + overview.owner, + overview.name, + label ); + false + } else { + info!( + " {} {}/{} ({}, token not authorized for this repository)", + cross_mark(), + overview.owner, + overview.name, + label + ); + false } - - Ok(()) } diff --git a/src/cli/exec/mod.rs b/src/cli/exec/mod.rs index 8719128d..a51349b9 100644 --- a/src/cli/exec/mod.rs +++ b/src/cli/exec/mod.rs @@ -1,6 +1,5 @@ use super::ExecAndRunSharedArgs; use crate::api_client::CodSpeedAPIClient; -use crate::config::CodSpeedConfig; use crate::executor; use crate::executor::config::{self, OrchestratorConfig, RepositoryOverride}; use crate::instruments::Instruments; @@ -67,7 +66,6 @@ fn build_orchestrator_config( Ok(OrchestratorConfig { upload_url, - token: args.shared.token, repository_override: args .shared .repository @@ -95,8 +93,7 @@ fn build_orchestrator_config( pub async fn run( args: ExecArgs, - api_client: &CodSpeedAPIClient, - codspeed_config: &CodSpeedConfig, + api_client: &mut CodSpeedAPIClient, project_config: Option<&ProjectConfig>, setup_cache_dir: Option<&Path>, ) -> Result<()> { @@ -113,7 +110,7 @@ pub async fn run( PollResultsOptions::new(false, base_run_id), )?; - execute_config(config, api_client, codspeed_config, setup_cache_dir).await + execute_config(config, api_client, setup_cache_dir).await } /// Core execution logic shared by `codspeed exec` and `codspeed run` with config targets. @@ -122,8 +119,7 @@ pub async fn run( /// by the orchestrator when exec targets are present. pub async fn execute_config( mut config: OrchestratorConfig, - api_client: &CodSpeedAPIClient, - codspeed_config: &CodSpeedConfig, + api_client: &mut CodSpeedAPIClient, setup_cache_dir: Option<&Path>, ) -> Result<()> { // Resolve exec target binary paths so memtrack can discover statically linked @@ -160,7 +156,7 @@ pub async fn execute_config( ); } - let orchestrator = executor::Orchestrator::new(config, codspeed_config, api_client).await?; + let orchestrator = executor::Orchestrator::new(config, api_client).await?; if !orchestrator.is_local() { super::show_banner(); diff --git a/src/cli/mod.rs b/src/cli/mod.rs index 15ba60db..6f652234 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -123,9 +123,25 @@ pub async fn run() -> Result<()> { let cli = Cli::parse(); // Important: keep this after the Cli::parse() because the function can exit the process by itself, skipping the drop of the CursorGuard let _cursor_guard = CursorGuard::new(); - let codspeed_config = - CodSpeedConfig::load_with_override(cli.config_name.as_deref(), cli.oauth_token.as_deref())?; - let api_client = CodSpeedAPIClient::try_from((&cli, &codspeed_config))?; + if *IS_TTY { + // Ctrl+C terminates the process before `CursorGuard::drop` runs, + // so we restore the cursor explicitly, then re-raise SIGINT with + // the default disposition so the parent shell sees the expected + // signal-terminated status. + tokio::spawn(async { + if tokio::signal::ctrl_c().await.is_ok() { + drop(_cursor_guard); // explicitly drop to restore cursor before re-raising + } + // Safety: resetting SIGINT to SIG_DFL and raising it are + // async-signal-safe and have no Rust-level invariants to break. + unsafe { + libc::signal(libc::SIGINT, libc::SIG_DFL); + libc::raise(libc::SIGINT); + } + }); + } + + let mut api_client = build_api_client(&cli)?; // Discover project configuration file let discovered_config = DiscoveredProjectConfig::discover_and_load( @@ -152,8 +168,7 @@ pub async fn run() -> Result<()> { args.shared.experimental.warn_if_active(); run::run( *args, - &api_client, - &codspeed_config, + &mut api_client, discovered_config.as_ref(), setup_cache_dir, ) @@ -163,8 +178,7 @@ pub async fn run() -> Result<()> { args.shared.experimental.warn_if_active(); exec::run( *args, - &api_client, - &codspeed_config, + &mut api_client, discovered_config.as_ref().map(|d| &d.config), setup_cache_dir, ) @@ -180,17 +194,37 @@ pub async fn run() -> Result<()> { Ok(()) } -impl Cli { - /// Create a test CLI instance with a custom API URL for use in tests - #[cfg(test)] - pub fn test_with_url(api_url: String) -> Self { - Self { - api_url, - oauth_token: None, - config_name: None, - config: None, - setup_cache_dir: None, - command: Commands::Setup(setup::SetupArgs::default()), +/// Build the api client for this invocation, resolving the auth token +/// from the most specific source available. This is the single source +/// of truth for token resolution; the result lives on the returned +/// client and every downstream consumer (GraphQL queries, upload +/// `Authorization` header, executor env injection) reads it from there. +/// +/// Priority (most specific first): +/// 1. `--token` / `CODSPEED_TOKEN` — run/exec-level override +/// 2. `--oauth-token` / `CODSPEED_OAUTH_TOKEN` and the persisted CLI +/// token — both live on disk and are loaded together by +/// [`CodSpeedConfig::load_with_override`]. +/// +/// The CLI config file is only read when no explicit token was passed, +/// so an invocation like `codspeed run --token ` never touches the +/// user's `~/.config/codspeed/`. +fn build_api_client(cli: &Cli) -> Result { + let explicit = match &cli.command { + Commands::Run(args) => args.shared.token.clone(), + Commands::Exec(args) => args.shared.token.clone(), + _ => None, + }; + let token = match explicit { + Some(token) => Some(token), + None => { + CodSpeedConfig::load_with_override( + cli.config_name.as_deref(), + cli.oauth_token.as_deref(), + )? + .auth + .token } - } + }; + Ok(CodSpeedAPIClient::new(token, cli.api_url.clone())) } diff --git a/src/cli/run/mod.rs b/src/cli/run/mod.rs index fa0b6a93..9b0edd62 100644 --- a/src/cli/run/mod.rs +++ b/src/cli/run/mod.rs @@ -1,6 +1,5 @@ use super::ExecAndRunSharedArgs; use crate::api_client::CodSpeedAPIClient; -use crate::config::CodSpeedConfig; use crate::executor; use crate::executor::config::{self, OrchestratorConfig, RepositoryOverride}; use crate::instruments::Instruments; @@ -101,7 +100,6 @@ fn build_orchestrator_config( Ok(OrchestratorConfig { upload_url, - token: args.shared.token, repository_override: args .shared .repository @@ -142,8 +140,7 @@ enum RunTarget<'a> { pub async fn run( args: RunArgs, - api_client: &CodSpeedAPIClient, - codspeed_config: &CodSpeedConfig, + api_client: &mut CodSpeedAPIClient, discovered_config: Option<&DiscoveredProjectConfig>, setup_cache_dir: Option<&Path>, ) -> Result<()> { @@ -188,8 +185,7 @@ pub async fn run( poll_opts, )?; - let orchestrator = - executor::Orchestrator::new(config, codspeed_config, api_client).await?; + let orchestrator = executor::Orchestrator::new(config, api_client).await?; if !orchestrator.is_local() { super::show_banner(); @@ -244,8 +240,7 @@ pub async fn run( PollResultsOptions::new(false, base_run_id), )?; config.working_directory = resolved_working_directory; - super::exec::execute_config(config, api_client, codspeed_config, setup_cache_dir) - .await?; + super::exec::execute_config(config, api_client, setup_cache_dir).await?; } } diff --git a/src/executor/config.rs b/src/executor/config.rs index 057fd2fd..0816704b 100644 --- a/src/executor/config.rs +++ b/src/executor/config.rs @@ -51,7 +51,6 @@ pub enum SimulationTool { #[derive(Debug, Clone)] pub struct OrchestratorConfig { pub upload_url: Url, - pub token: Option, pub repository_override: Option, pub working_directory: Option, @@ -91,7 +90,6 @@ pub struct OrchestratorConfig { /// `skip_upload`, `repository_override`) live on [`OrchestratorConfig`]. #[derive(Debug, Clone)] pub struct ExecutorConfig { - pub token: Option, pub working_directory: Option, pub command: String, @@ -144,10 +142,6 @@ impl RepositoryOverride { pub const DEFAULT_UPLOAD_URL: &str = "https://api.codspeed.io/upload"; impl OrchestratorConfig { - pub fn set_token(&mut self, token: Option) { - self.token = token; - } - /// Compute the total number of executor runs that will be performed. /// /// All `Exec` targets are combined into a single invocation, while each @@ -177,7 +171,6 @@ impl OrchestratorConfig { enable_introspection: bool, ) -> ExecutorConfig { ExecutorConfig { - token: self.token.clone(), working_directory: self.working_directory.clone(), command, instruments: self.instruments.clone(), @@ -195,19 +188,12 @@ impl OrchestratorConfig { } } -impl ExecutorConfig { - pub fn set_token(&mut self, token: Option) { - self.token = token; - } -} - #[cfg(test)] impl OrchestratorConfig { /// Constructs a new `OrchestratorConfig` with default values for testing purposes pub fn test() -> Self { Self { upload_url: Url::parse(DEFAULT_UPLOAD_URL).unwrap(), - token: None, repository_override: None, working_directory: None, targets: vec![BenchmarkTarget::Entrypoint { diff --git a/src/executor/mod.rs b/src/executor/mod.rs index 353a1669..3123fa15 100644 --- a/src/executor/mod.rs +++ b/src/executor/mod.rs @@ -17,7 +17,7 @@ use crate::prelude::*; use crate::runner_mode::RunnerMode; use crate::system::SystemInfo; use async_trait::async_trait; -pub use config::{BenchmarkTarget, ExecutorConfig, OrchestratorConfig}; +pub use config::{BenchmarkTarget, ExecutorConfig}; pub use execution_context::ExecutionContext; pub use interfaces::ExecutorName; pub use orchestrator::Orchestrator; diff --git a/src/executor/orchestrator.rs b/src/executor/orchestrator.rs index aa6d8558..5840368f 100644 --- a/src/executor/orchestrator.rs +++ b/src/executor/orchestrator.rs @@ -3,7 +3,6 @@ use crate::api_client::CodSpeedAPIClient; use crate::binary_installer::ensure_binary_installed; use crate::cli::exec::multi_targets; use crate::cli::run::logger::Logger; -use crate::config::CodSpeedConfig; use crate::executor::config::BenchmarkTarget; use crate::executor::config::OrchestratorConfig; use crate::executor::helpers::profile_folder::create_profile_folder; @@ -36,23 +35,11 @@ impl Orchestrator { self.provider.get_run_environment() == RunEnvironment::Local } - pub async fn new( - mut config: OrchestratorConfig, - codspeed_config: &CodSpeedConfig, - api_client: &CodSpeedAPIClient, - ) -> Result { + pub async fn new(config: OrchestratorConfig, api_client: &CodSpeedAPIClient) -> Result { let provider = run_environment::get_provider(&config, api_client).await?; let system_info = SystemInfo::new()?; let logger = Logger::new(provider.as_ref())?; - if provider.get_run_environment() == RunEnvironment::Local { - if codspeed_config.auth.token.is_none() { - bail!("You have to authenticate the CLI first. Run `codspeed auth login`."); - } - debug!("Using the token from the CodSpeed configuration file"); - config.set_token(codspeed_config.auth.token.clone()); - } - #[allow(deprecated)] if config.modes.contains(&RunnerMode::Instrumentation) { warn!( @@ -82,7 +69,7 @@ impl Orchestrator { pub async fn execute( &self, setup_cache_dir: Option<&Path>, - api_client: &CodSpeedAPIClient, + api_client: &mut CodSpeedAPIClient, ) -> Result<()> { // Build (command, label, uses_exec_harness) tuples while we still know the target type let mut command_labels: Vec<(String, String, bool)> = vec![]; @@ -215,13 +202,13 @@ impl Orchestrator { async fn upload_and_poll( &self, mut completed_runs: Vec<(ExecutionContext, ExecutorName)>, - api_client: &CodSpeedAPIClient, + api_client: &mut CodSpeedAPIClient, ) -> Result<()> { let skip_upload = self.config.skip_upload; if !skip_upload { start_group!("Uploading results"); - let last_upload_result = self.upload_all(&mut completed_runs).await?; + let last_upload_result = self.upload_all(&mut completed_runs, api_client).await?; end_group!(); if self.is_local() { @@ -258,22 +245,28 @@ impl Orchestrator { pub async fn upload_all( &self, completed_runs: &mut [(ExecutionContext, ExecutorName)], + api_client: &mut CodSpeedAPIClient, ) -> Result { let mut last_upload_result: Option = None; let total_runs = completed_runs.len(); for (run_part_index, (ctx, executor_name)) in completed_runs.iter_mut().enumerate() { - if !self.is_local() { - // OIDC tokens can expire quickly, so refresh just before each upload - self.provider.set_oidc_token(&mut ctx.config).await?; - } + // OIDC tokens can expire quickly, so refresh just before each upload + self.provider.set_oidc_token(api_client).await?; if total_runs > 1 { info!("Uploading results {}/{total_runs}", run_part_index + 1); } let run_part_suffix = Self::build_run_part_suffix(executor_name, run_part_index, total_runs); - let upload_result = upload(self, ctx, executor_name.clone(), run_part_suffix).await?; + let upload_result = upload( + self, + api_client, + ctx, + executor_name.clone(), + run_part_suffix, + ) + .await?; last_upload_result = Some(upload_result); } info!("Performance data uploaded"); diff --git a/src/executor/tests.rs b/src/executor/tests.rs index 9fd6135b..59a5fc9f 100644 --- a/src/executor/tests.rs +++ b/src/executor/tests.rs @@ -123,13 +123,6 @@ fi pub async fn create_test_setup(config: ExecutorConfig) -> (ExecutionContext, TempDir) { let temp_dir = TempDir::new().unwrap(); - let mut config = config; - - // Provide a test token so authentication doesn't fail - if config.token.is_none() { - config.token = Some("test-token".to_string()); - } - let profile_folder = temp_dir.path().to_path_buf(); let execution_context = ExecutionContext::new(config, profile_folder); diff --git a/src/queries/CurrentUser.gql b/src/queries/CurrentUser.gql deleted file mode 100644 index 2a09c7e3..00000000 --- a/src/queries/CurrentUser.gql +++ /dev/null @@ -1,6 +0,0 @@ -query CurrentUser { - user { - login - provider - } -} diff --git a/src/queries/Session.gql b/src/queries/Session.gql new file mode 100644 index 00000000..168046d9 --- /dev/null +++ b/src/queries/Session.gql @@ -0,0 +1,8 @@ +query Session { + session { + user { + login + provider + } + } +} diff --git a/src/queries/GetRepository.gql b/src/queries/SessionAndRepositoryOverview.gql similarity index 51% rename from src/queries/GetRepository.gql rename to src/queries/SessionAndRepositoryOverview.gql index b39e6f70..b1d6f76e 100644 --- a/src/queries/GetRepository.gql +++ b/src/queries/SessionAndRepositoryOverview.gql @@ -1,17 +1,26 @@ -query Repository( +query SessionAndRepositoryOverview( $owner: String! $name: String! $provider: RepositoryProvider ) { - user { - id + session { + user { + login + provider + } } repositoryOverview(owner: $owner, name: $name, provider: $provider) { ... on Repository { - id + owner + name + provider + hasWriteAccess } ... on AvailableRepository { - id + owner + name + provider + hasWriteAccess } } } diff --git a/src/run_environment/buildkite/provider.rs b/src/run_environment/buildkite/provider.rs index 6d1ab4bd..d7c16d61 100644 --- a/src/run_environment/buildkite/provider.rs +++ b/src/run_environment/buildkite/provider.rs @@ -3,10 +3,11 @@ use std::env; use async_trait::async_trait; use simplelog::SharedLogger; +use crate::api_client::CodSpeedAPIClient; use crate::cli::run::helpers::{ GitRemote, find_repository_root, get_env_variable, parse_git_remote, }; -use crate::executor::config::{ExecutorConfig, OrchestratorConfig}; +use crate::executor::config::OrchestratorConfig; use crate::prelude::*; use crate::run_environment::interfaces::{RepositoryProvider, RunEnvironmentMetadata, RunEvent}; use crate::run_environment::provider::{RunEnvironmentDetector, RunEnvironmentProvider}; @@ -55,10 +56,6 @@ pub fn get_ref() -> Result { impl TryFrom<&OrchestratorConfig> for BuildkiteProvider { type Error = Error; fn try_from(config: &OrchestratorConfig) -> Result { - if config.token.is_none() { - bail!("Token authentication is required for Buildkite"); - } - if config.repository_override.is_some() { bail!("Specifying owner and repository from CLI is not supported for Buildkite"); } @@ -152,14 +149,15 @@ impl RunEnvironmentProvider for BuildkiteProvider { None } - /// For now, we do not support OIDC tokens for Buildkite - /// - /// If we want to in the future, we can implement it using the Buildkite Agent CLI. - /// - /// Docs: - /// - https://buildkite.com/docs/agent/v3/cli-oidc - /// - https://buildkite.com/docs/pipelines/security/oidc - async fn set_oidc_token(&self, _config: &mut ExecutorConfig) -> Result<()> { + /// Buildkite requires a static `CODSPEED_TOKEN`. We don't yet support + /// OIDC tokens here (could be added via the Buildkite Agent CLI: + /// , + /// ), so this + /// just enforces token presence. + fn check_oidc_configuration(&mut self, api_client: &CodSpeedAPIClient) -> Result<()> { + if api_client.token().is_none() { + bail!("Token authentication is required for Buildkite"); + } Ok(()) } } @@ -200,7 +198,6 @@ mod tests { ], || { let config = OrchestratorConfig { - token: Some("token".into()), ..OrchestratorConfig::test() }; let provider = BuildkiteProvider::try_from(&config).unwrap(); @@ -239,7 +236,6 @@ mod tests { ], || { let config = OrchestratorConfig { - token: Some("token".into()), ..OrchestratorConfig::test() }; let provider = BuildkiteProvider::try_from(&config).unwrap(); @@ -278,7 +274,6 @@ mod tests { ], || { let config = OrchestratorConfig { - token: Some("token".into()), ..OrchestratorConfig::test() }; let provider = BuildkiteProvider::try_from(&config).unwrap(); diff --git a/src/run_environment/github_actions/provider.rs b/src/run_environment/github_actions/provider.rs index c8a024e7..27afc683 100644 --- a/src/run_environment/github_actions/provider.rs +++ b/src/run_environment/github_actions/provider.rs @@ -9,8 +9,9 @@ use simplelog::SharedLogger; use std::collections::BTreeMap; use std::{env, fs}; +use crate::api_client::CodSpeedAPIClient; use crate::cli::run::helpers::{find_repository_root, get_env_variable}; -use crate::executor::config::{ExecutorConfig, OrchestratorConfig}; +use crate::executor::config::OrchestratorConfig; use crate::prelude::*; use crate::request_client::OIDC_CLIENT; use crate::run_environment::interfaces::{ @@ -288,9 +289,9 @@ impl RunEnvironmentProvider for GitHubActionsProvider { /// - https://docs.github.com/en/actions/how-tos/secure-your-work/security-harden-deployments/oidc-with-reusable-workflows /// - https://docs.github.com/en/actions/concepts/security/openid-connect /// - https://docs.github.com/en/actions/reference/security/oidc#methods-for-requesting-the-oidc-token - fn check_oidc_configuration(&mut self, config: &OrchestratorConfig) -> Result<()> { + fn check_oidc_configuration(&mut self, api_client: &CodSpeedAPIClient) -> Result<()> { // Check if a static token is already set - if config.token.is_some() { + if api_client.token().is_some() { announcement!( "You can now authenticate your CI workflows using OpenID Connect (OIDC) tokens instead of `CODSPEED_TOKEN` secrets.\n\ This makes integrating and authenticating jobs safer and simpler.\n\ @@ -343,7 +344,7 @@ impl RunEnvironmentProvider for GitHubActionsProvider { /// /// All the validation has already been performed in `check_oidc_configuration`. /// So if the oidc_config is None, we simply return. - async fn set_oidc_token(&self, config: &mut ExecutorConfig) -> Result<()> { + async fn set_oidc_token(&self, api_client: &mut CodSpeedAPIClient) -> Result<()> { if let Some(oidc_config) = &self.oidc_config { let request_url = format!( "{}&audience={}", @@ -370,7 +371,7 @@ impl RunEnvironmentProvider for GitHubActionsProvider { if token.is_some() { debug!("Successfully retrieved OIDC token for authentication."); - config.set_token(token); + api_client.set_token(token); } else if self.is_repository_private { bail!( "Unable to retrieve OIDC token for authentication. \n\ @@ -436,7 +437,6 @@ mod tests { ], || { let config = OrchestratorConfig { - token: Some("token".into()), ..OrchestratorConfig::test() }; let github_actions_provider = GitHubActionsProvider::try_from(&config).unwrap(); @@ -494,7 +494,6 @@ mod tests { ], || { let config = OrchestratorConfig { - token: Some("token".into()), ..OrchestratorConfig::test() }; let github_actions_provider = GitHubActionsProvider::try_from(&config).unwrap(); @@ -550,7 +549,6 @@ mod tests { ], || { let config = OrchestratorConfig { - token: Some("token".into()), ..OrchestratorConfig::test() }; let github_actions_provider = GitHubActionsProvider::try_from(&config).unwrap(); @@ -633,7 +631,6 @@ mod tests { ], || { let config = OrchestratorConfig { - token: Some("token".into()), ..OrchestratorConfig::test() }; let github_actions_provider = GitHubActionsProvider::try_from(&config).unwrap(); diff --git a/src/run_environment/gitlab_ci/provider.rs b/src/run_environment/gitlab_ci/provider.rs index 3fc3c280..57271fc3 100644 --- a/src/run_environment/gitlab_ci/provider.rs +++ b/src/run_environment/gitlab_ci/provider.rs @@ -4,7 +4,7 @@ use std::collections::BTreeMap; use std::env; use crate::cli::run::helpers::get_env_variable; -use crate::executor::config::{ExecutorConfig, OrchestratorConfig}; +use crate::executor::config::OrchestratorConfig; use crate::prelude::*; use crate::run_environment::interfaces::{ GlData, RepositoryProvider, RunEnvironment, RunEnvironmentMetadata, RunEvent, Sender, @@ -178,18 +178,6 @@ impl RunEnvironmentProvider for GitLabCIProvider { metadata: BTreeMap::new(), }) } - - /// For GitLab CI, OIDC tokens must be pre-generated and passed via env variable. - /// - /// In [our documentation](https://codspeed.io/docs/integrations/ci/gitlab-ci#openid-connect-oidc-authentication), we ask - /// user to create a variable named `CODSPEED_TOKEN` with the OIDC token. So there is nothing to do here. - /// - /// See: - /// - https://docs.gitlab.com/integration/openid_connect_provider/ - /// - https://docs.gitlab.com/ci/secrets/id_token_authentication/ - async fn set_oidc_token(&self, _config: &mut ExecutorConfig) -> Result<()> { - Ok(()) - } } #[cfg(test)] @@ -225,7 +213,6 @@ mod tests { ], || { let config = OrchestratorConfig { - token: Some("token".into()), ..OrchestratorConfig::test() }; let gitlab_ci_provider = GitLabCIProvider::try_from(&config).unwrap(); @@ -272,7 +259,6 @@ mod tests { ], || { let config = OrchestratorConfig { - token: Some("token".into()), ..OrchestratorConfig::test() }; let gitlab_ci_provider = GitLabCIProvider::try_from(&config).unwrap(); @@ -319,7 +305,6 @@ mod tests { ], || { let config = OrchestratorConfig { - token: Some("token".into()), ..OrchestratorConfig::test() }; let gitlab_ci_provider = GitLabCIProvider::try_from(&config).unwrap(); diff --git a/src/run_environment/local/provider.rs b/src/run_environment/local/provider.rs index ad838e87..2097a30f 100644 --- a/src/run_environment/local/provider.rs +++ b/src/run_environment/local/provider.rs @@ -5,7 +5,8 @@ use uuid::Uuid; use crate::api_client::{ CodSpeedAPIClient, GetOrCreateProjectRepositoryPayload, GetOrCreateProjectRepositoryVars, - GetRepositoryPayload, GetRepositoryVars, + SessionAndRepositoryOverview, SessionAndRepositoryOverviewError, + SessionAndRepositoryOverviewVars, }; use crate::cli::run::helpers::{find_repository_root, parse_repository_from_remote}; use crate::executor::config::OrchestratorConfig; @@ -22,6 +23,14 @@ use std::collections::BTreeMap; static FAKE_COMMIT_REF: &str = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; +/// Prints user-facing messages emitted before the logger is initialized. +/// We do this because we initialize the logger after creating the provider. +/// It is error-prone, a better solution would be to setup a minimal logger that gets flushed once +/// the actual logger is initialized, but it is good enough for now. +fn log_pre_init(message: &str) { + eprintln!("{message}"); +} + #[derive(Debug)] pub struct LocalProvider { repository_provider: RepositoryProvider, @@ -51,6 +60,8 @@ struct ResolvedRepository { } impl LocalProvider { + /// Create a new `LocalProvider`, resolving repository information from git and/or the API as needed. + /// Since the logger is not setup yet, we use `log_pre_init` for any user-facing messages during resolution. pub async fn new( config: &OrchestratorConfig, api_client: &impl LocalProviderApiClient, @@ -63,10 +74,10 @@ impl LocalProvider { .map(|ctx| ctx.root_path.clone()) .unwrap_or_else(|| current_dir.to_string_lossy().to_string()); - let resolved = if !config.skip_upload { - Self::resolve_repository(config, api_client, git_context.as_ref()).await? - } else { + let resolved = if config.skip_upload { Self::dummy_resolved_repository(git_context.as_ref()) + } else { + Self::resolve_repository(config, api_client, git_context.as_ref()).await? }; let expected_run_parts_count = config.expected_run_parts_count(); @@ -109,16 +120,15 @@ impl LocalProvider { } } - /// Resolve repository information from override, git remote, or API fallback + /// Resolve repository information from override, git remote, or API fallback. /// - /// When there is no explicit repository override, this flow also makes sure the user is logged in with a valid token - /// 1. Repo found - /// a. Logged in: user is set, repositoryOverview is set — repo found - /// b. NOT logged in: user is null, repositoryOverview is set => bails with "session expired" + /// For the override and git-remote paths, this also validates the token + /// up front (auth + `CREATE_LOCAL_RUN` access on the target repo) so an + /// invalid or mis-scoped token surfaces before any benchmark runs rather + /// than as a 401 from the upload endpoint. /// - /// 2. REPOSITORY_NOT_FOUND => falls through to `get_or_create_project_repository` - /// a. Logged in: `get_or_create_project_repository` succeeds - /// b. NOT logged in: `get_or_create_project_repository` fails, bail with "session expired" + /// The project-repository fallback skips the explicit auth check and + /// relies on `get_or_create_project_repository` to surface auth errors. async fn resolve_repository( config: &OrchestratorConfig, api_client: &impl LocalProviderApiClient, @@ -126,43 +136,80 @@ impl LocalProvider { ) -> Result { // Priority 1: Use explicit repository override if let Some(repo_override) = &config.repository_override { - return Self::resolve_from_override(repo_override, git_context); + return Self::resolve_from_override(api_client, repo_override, git_context).await; } // Priority 2: Try to use git remote if repository exists in CodSpeed if let Some(ctx) = git_context { if let Some(resolved) = - Self::try_resolve_from_codspeed_repository(api_client, ctx).await? + Self::try_resolve_from_detected_repository(api_client, ctx).await? { return Ok(resolved); } + } else { + log_pre_init(&format!( + "Not inside a git repository, the run will be uploaded to a {} CodSpeed project not associated with any repository.", + crate::cli::exec::DEFAULT_REPOSITORY_NAME + )); } // Priority 3: Fallback to project repository Self::resolve_as_project_repository(api_client).await } - /// Resolve repository from explicit override configuration - fn resolve_from_override( + /// Resolve repository from explicit override configuration, validating + /// the token has `CREATE_LOCAL_RUN` access on the target repo. + async fn resolve_from_override( + api_client: &impl LocalProviderApiClient, repo_override: &RepositoryOverride, git_context: Option<&GitContext>, ) -> Result { + let overview = Self::fetch_repository_overview( + api_client, + &repo_override.owner, + &repo_override.repository, + Some(&repo_override.repository_provider), + ) + .await?; + + let Some(overview) = overview else { + bail!( + "Explicitly requested repository {}/{} not found on CodSpeed. Is the repository enabled on CodSpeed?\n\ + Check out https://codspeed.io/docs for instructions on how to enable CodSpeed. + You can also run without the repository override to automatically detect repositories, and fallback to an isolated \ + CodSpeed run if needed.", + repo_override.owner, + repo_override.repository, + ); + }; + + if !overview.has_write_access { + bail!( + "You are not authorized to create runs on {}/{}.\n\ + Ask a repository to give you write access to the repository. + You can also run without the repository override to automatically fallback to an isolated CodSpeed run.", + overview.owner, + overview.name, + ); + } + let (ref_, head_ref) = git_context .map(|ctx| Self::get_git_ref_info(&ctx.root_path)) .transpose()? .unwrap_or_else(|| (FAKE_COMMIT_REF.to_string(), None)); Ok(ResolvedRepository { - provider: repo_override.repository_provider.clone(), - owner: repo_override.owner.clone(), - name: repo_override.repository.clone(), + provider: overview.provider, + owner: overview.owner, + name: overview.name, ref_, head_ref, }) } - /// Try to resolve repository from git remote, validating it exists in CodSpeed - async fn try_resolve_from_codspeed_repository( + /// Try to resolve repository from git remote, validating it exists in + /// CodSpeed and that the token has `CREATE_LOCAL_RUN` access. + async fn try_resolve_from_detected_repository( api_client: &impl LocalProviderApiClient, git_context: &GitContext, ) -> Result> { @@ -175,38 +222,64 @@ impl LocalProvider { let parsed = parse_repository_from_remote(remote.url().unwrap())?; let (provider, owner, name) = (parsed.provider, parsed.owner, parsed.name); - // Check if repository exists in CodSpeed - // Note: we only check existence here, we don't check that - // - the provider is properly setup - // - the provider has access to the repository - // - // If the repo exists, but these two conditions are not satisfied, the upload will fail - // later on, but by checking repository existence here we catch most of the cases where the - // user would run their benchmarks, but fail to upload afterwards. - let exists = api_client - .get_repository(GetRepositoryVars { - owner: owner.clone(), - name: name.clone(), - provider: provider.clone(), - }) - .await? - .is_some(); + let Some(overview) = + Self::fetch_repository_overview(api_client, &owner, &name, Some(&provider)).await? + else { + log_pre_init(&format!( + "Repostory {owner}/{name} not found on CodSpeed.\n\ + Check out https://codspeed.io/docs for instructions on how to enable CodSpeed.\n\ + Falling back to an isolated CodSpeed run.", + )); + return Ok(None); + }; - if !exists { + if !overview.has_write_access { + log_pre_init(&format!( + "You are not authorized to create runs on {}/{}.\n\ + To fix this, ask a repository admin to give you write access to the repository.\n\ + Falling back to an isolated CodSpeed run.", + overview.owner, overview.name, + )); return Ok(None); } let (ref_, head_ref) = Self::get_git_ref_info(&git_context.root_path)?; Ok(Some(ResolvedRepository { - provider, - owner, - name, + provider: overview.provider, + owner: overview.owner, + name: overview.name, ref_, head_ref, })) } + /// Run the combined session+repository query, mapping the auth error to + /// the standard "re-authenticate" message and folding "repo not found" + /// into `Ok(None)`. + async fn fetch_repository_overview( + api_client: &impl LocalProviderApiClient, + owner: &str, + name: &str, + provider: Option<&RepositoryProvider>, + ) -> Result> { + let payload = api_client + .session_and_repository_overview(SessionAndRepositoryOverviewVars { + owner: owner.to_string(), + name: name.to_string(), + provider: provider.cloned(), + }) + .await + .map_err(|err| match err { + SessionAndRepositoryOverviewError::Unauthenticated => { + anyhow!("Invalid token. Run `codspeed auth login` to re-authenticate.") + } + SessionAndRepositoryOverviewError::Other(err) => err, + })?; + + Ok(payload.repository_overview) + } + /// Resolve repository by creating/getting a project repository async fn resolve_as_project_repository( api_client: &impl LocalProviderApiClient, @@ -317,8 +390,10 @@ impl RunEnvironmentProvider for LocalProvider { #[async_trait(?Send)] pub trait LocalProviderApiClient { - async fn get_repository(&self, vars: GetRepositoryVars) - -> Result>; + async fn session_and_repository_overview( + &self, + vars: SessionAndRepositoryOverviewVars, + ) -> std::result::Result; async fn get_or_create_project_repository( &self, @@ -328,11 +403,11 @@ pub trait LocalProviderApiClient { #[async_trait(?Send)] impl LocalProviderApiClient for CodSpeedAPIClient { - async fn get_repository( + async fn session_and_repository_overview( &self, - vars: GetRepositoryVars, - ) -> Result> { - self.get_repository(vars).await + vars: SessionAndRepositoryOverviewVars, + ) -> std::result::Result { + self.session_and_repository_overview(vars).await } async fn get_or_create_project_repository( @@ -382,8 +457,14 @@ mod tests { format!("{}/", dir.to_string_lossy()) } - /// A mock API client that returns a found repository for `get_repository` - /// and a fixed project repository for `get_or_create_project_repository`. + use crate::api_client::{RepositoryOverviewPayload, SessionPayload}; + + fn fake_session() -> SessionPayload { + SessionPayload { user: None } + } + + /// A mock API client that returns a found repository (with write access) + /// from `session_and_repository_overview`. struct MockApiClientRepoFound; impl MockApiClientRepoFound { @@ -394,13 +475,20 @@ mod tests { #[async_trait(?Send)] impl LocalProviderApiClient for MockApiClientRepoFound { - async fn get_repository( + async fn session_and_repository_overview( &self, - _vars: GetRepositoryVars, - ) -> Result> { - Ok(Some(GetRepositoryPayload { - id: "my-repo-id".into(), - })) + vars: SessionAndRepositoryOverviewVars, + ) -> std::result::Result + { + Ok(SessionAndRepositoryOverview { + session: fake_session(), + repository_overview: Some(RepositoryOverviewPayload { + owner: vars.owner, + name: vars.name, + provider: vars.provider.unwrap_or(RepositoryProvider::GitHub), + has_write_access: true, + }), + }) } async fn get_or_create_project_repository( @@ -411,8 +499,8 @@ mod tests { } } - /// A mock API client that returns no repository for `get_repository`, - /// falling back to a fixed project repository. + /// A mock API client that returns no repository, falling back to a fixed + /// project repository. struct MockApiClientRepoNotFound; impl MockApiClientRepoNotFound { @@ -423,11 +511,15 @@ mod tests { #[async_trait(?Send)] impl LocalProviderApiClient for MockApiClientRepoNotFound { - async fn get_repository( + async fn session_and_repository_overview( &self, - _vars: GetRepositoryVars, - ) -> Result> { - Ok(None) + _vars: SessionAndRepositoryOverviewVars, + ) -> std::result::Result + { + Ok(SessionAndRepositoryOverview { + session: fake_session(), + repository_overview: None, + }) } async fn get_or_create_project_repository( diff --git a/src/run_environment/mod.rs b/src/run_environment/mod.rs index e5075296..ddebd46f 100644 --- a/src/run_environment/mod.rs +++ b/src/run_environment/mod.rs @@ -44,7 +44,7 @@ pub async fn get_provider( } }; - provider.check_oidc_configuration(config)?; + provider.check_oidc_configuration(api_client)?; Ok(provider) } diff --git a/src/run_environment/provider.rs b/src/run_environment/provider.rs index ae142412..b71f0e3a 100644 --- a/src/run_environment/provider.rs +++ b/src/run_environment/provider.rs @@ -4,7 +4,8 @@ use serde_json::Value; use simplelog::SharedLogger; use std::collections::BTreeMap; -use crate::executor::{ExecutorConfig, ExecutorName, OrchestratorConfig}; +use crate::api_client::CodSpeedAPIClient; +use crate::executor::{ExecutorConfig, ExecutorName}; use crate::prelude::*; use crate::system::SystemInfo; use crate::upload::{ @@ -87,20 +88,15 @@ pub trait RunEnvironmentProvider { } /// Check the OIDC configuration for the current run environment, if supported. - fn check_oidc_configuration(&mut self, _config: &OrchestratorConfig) -> Result<()> { + fn check_oidc_configuration(&mut self, _api_client: &CodSpeedAPIClient) -> Result<()> { Ok(()) } - /// Handle an OIDC token for the current run environment, if supported. + /// Request an OIDC token for the current run environment, if supported. + /// The requested token will be set to the `api_client` to be used to subsequent requests. /// - /// Updates the config if necessary. - /// - /// Depending on the provider, this may involve requesting the token, - /// warning the user about potential misconfigurations, or other necessary steps. - /// - /// Warning: OIDC tokens are typically short-lived. This method must be called - /// just before the upload step to ensure the token is valid during the upload. - async fn set_oidc_token(&self, _config: &mut ExecutorConfig) -> Result<()> { + /// For providers that do not support OIDC, or if the provider detects that OIDC is not in use, this is a no-op. + async fn set_oidc_token(&self, _api_client: &mut CodSpeedAPIClient) -> Result<()> { Ok(()) } @@ -111,6 +107,7 @@ pub trait RunEnvironmentProvider { async fn get_upload_metadata( &self, config: &ExecutorConfig, + api_client: &CodSpeedAPIClient, system_info: &SystemInfo, profile_archive: &ProfileArchive, executor_name: ExecutorName, @@ -131,7 +128,7 @@ pub trait RunEnvironmentProvider { Ok(UploadMetadata { version: Some(LATEST_UPLOAD_METADATA_VERSION), - tokenless: config.token.is_none(), + tokenless: api_client.token().is_none(), repository_provider: self.get_repository_provider(), run_environment_metadata, profile_md5: profile_archive.hash.clone(), diff --git a/src/upload/uploader.rs b/src/upload/uploader.rs index 434fca05..6983eadf 100644 --- a/src/upload/uploader.rs +++ b/src/upload/uploader.rs @@ -1,5 +1,5 @@ +use crate::api_client::CodSpeedAPIClient; use crate::executor::ExecutionContext; -use crate::executor::ExecutorConfig; use crate::executor::ExecutorName; use crate::executor::Orchestrator; use crate::run_environment::RunEnvironment; @@ -123,14 +123,14 @@ async fn create_profile_archive( async fn retrieve_upload_data( orchestrator: &Orchestrator, - config: &ExecutorConfig, + api_client: &CodSpeedAPIClient, upload_metadata: &UploadMetadata, ) -> Result { let mut upload_request = REQUEST_CLIENT .post(orchestrator.config.upload_url.clone()) .json(&upload_metadata); - if !upload_metadata.tokenless { - upload_request = upload_request.header("Authorization", config.token.clone().unwrap()); + if let Some(token) = api_client.token() { + upload_request = upload_request.header("Authorization", token.to_owned()); } let response = upload_request.send().await; @@ -250,6 +250,7 @@ pub struct UploadResult { pub async fn upload( orchestrator: &Orchestrator, + api_client: &CodSpeedAPIClient, execution_context: &ExecutionContext, executor_name: ExecutorName, run_part_suffix: BTreeMap, @@ -266,6 +267,7 @@ pub async fn upload( .provider .get_upload_metadata( &execution_context.config, + api_client, &orchestrator.system_info, &profile_archive, executor_name, @@ -279,8 +281,7 @@ pub async fn upload( } debug!("Preparing upload..."); - let upload_data = - retrieve_upload_data(orchestrator, &execution_context.config, &upload_metadata).await?; + let upload_data = retrieve_upload_data(orchestrator, api_client, &upload_metadata).await?; debug!("runId: {}", upload_data.run_id); debug!( @@ -303,18 +304,17 @@ mod tests { use url::Url; use super::*; - use crate::config::CodSpeedConfig; use std::path::PathBuf; // TODO: remove the ignore when implementing network mocking #[ignore] #[tokio::test] async fn test_upload() { - use crate::executor::OrchestratorConfig; + use crate::executor::ExecutorConfig; + use crate::executor::config::OrchestratorConfig; let orchestrator_config = OrchestratorConfig { upload_url: Url::parse("change me").unwrap(), - token: Some("change me".into()), profile_folder: Some(PathBuf::from(format!( "{}/src/uploader/samples/adrien-python-test", env!("CARGO_MANIFEST_DIR") @@ -327,7 +327,6 @@ mod tests { )); let executor_config = ExecutorConfig { command: "pytest tests/ --codspeed".into(), - token: Some("change me".into()), ..ExecutorConfig::test() }; async_with_vars( @@ -359,17 +358,16 @@ mod tests { ("VERSION", Some("0.1.0")), ], async { - let codspeed_config = CodSpeedConfig::default(); let api_client = CodSpeedAPIClient::create_test_client(); - let orchestrator = - Orchestrator::new(orchestrator_config, &codspeed_config, &api_client) - .await - .expect("Failed to create Orchestrator for test"); + let orchestrator = Orchestrator::new(orchestrator_config, &api_client) + .await + .expect("Failed to create Orchestrator for test"); let execution_context = ExecutionContext::new(executor_config, profile_folder); let run_part_suffix = BTreeMap::from([("executor".to_string(), Value::from("valgrind"))]); upload( &orchestrator, + &api_client, &execution_context, ExecutorName::Valgrind, run_part_suffix,