Skip to content

Commit 92d3e78

Browse files
jaluna0claude
andcommitted
feat(synthetics): accept OAuth2 bearer for tests run
`synthetics tests run` was the only synthetics command that could not authenticate with an OAuth2 session (`pup auth login`). Its `build_auth_headers` hard-required DD_API_KEY + DD_APP_KEY and never read `cfg.access_token`, unlike every sibling command which routes through `make_dd_client`/`apply_auth` and prefers a bearer token. The Datadog CI endpoints this command calls (`synthetics/ci/tunnel`, `synthetics/tests/trigger/ci`, `synthetics/ci/batch/{id}`) already accept OAuth2 bearer tokens with the `synthetics_write`/`synthetics_read` scopes, which pup requests by default — so this was a client-side gap, not a backend limitation. - Prefer OAuth2 bearer, fall back to API + app keys, mirroring `client::apply_auth`. - Clearer error when no auth is configured. - Add positive/negative tests for the three auth paths. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 6347a25 commit 92d3e78

1 file changed

Lines changed: 57 additions & 16 deletions

File tree

src/commands/synthetics.rs

Lines changed: 57 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -29,23 +29,29 @@ fn synthetics_intake_base_url(cfg: &Config) -> String {
2929
#[cfg(not(target_arch = "wasm32"))]
3030
fn build_auth_headers(cfg: &Config) -> anyhow::Result<reqwest::header::HeaderMap> {
3131
use reqwest::header::{HeaderMap, HeaderName, HeaderValue};
32-
let api_key = cfg
33-
.api_key
34-
.as_ref()
35-
.ok_or_else(|| anyhow::anyhow!("DD_API_KEY is required for 'synthetics tests run'"))?;
36-
let app_key = cfg
37-
.app_key
38-
.as_ref()
39-
.ok_or_else(|| anyhow::anyhow!("DD_APP_KEY is required for 'synthetics tests run'"))?;
4032
let mut headers = HeaderMap::new();
41-
headers.insert(
42-
HeaderName::from_static("dd-api-key"),
43-
HeaderValue::from_str(api_key)?,
44-
);
45-
headers.insert(
46-
HeaderName::from_static("dd-application-key"),
47-
HeaderValue::from_str(app_key)?,
48-
);
33+
34+
// Prefer OAuth2 bearer token, falling back to API + app keys.
35+
if let Some(token) = cfg.access_token.as_ref() {
36+
headers.insert(
37+
reqwest::header::AUTHORIZATION,
38+
HeaderValue::from_str(&format!("Bearer {token}"))?,
39+
);
40+
} else if let (Some(api_key), Some(app_key)) = (cfg.api_key.as_ref(), cfg.app_key.as_ref()) {
41+
headers.insert(
42+
HeaderName::from_static("dd-api-key"),
43+
HeaderValue::from_str(api_key)?,
44+
);
45+
headers.insert(
46+
HeaderName::from_static("dd-application-key"),
47+
HeaderValue::from_str(app_key)?,
48+
);
49+
} else {
50+
anyhow::bail!(
51+
"'synthetics tests run' requires authentication: run 'pup auth login' or set DD_API_KEY and DD_APP_KEY"
52+
);
53+
}
54+
4955
headers.insert(
5056
reqwest::header::USER_AGENT,
5157
HeaderValue::from_str(&crate::useragent::get())?,
@@ -959,6 +965,41 @@ mod tests {
959965
cleanup_env();
960966
}
961967

968+
#[cfg(not(target_arch = "wasm32"))]
969+
#[test]
970+
fn test_build_auth_headers_prefers_bearer() {
971+
let mut cfg = test_config("http://unused.local");
972+
cfg.access_token = Some("tok-123".into());
973+
let headers = super::build_auth_headers(&cfg).expect("bearer headers");
974+
assert_eq!(headers.get("authorization").unwrap(), "Bearer tok-123");
975+
assert!(headers.get("dd-api-key").is_none());
976+
assert!(headers.get("dd-application-key").is_none());
977+
}
978+
979+
#[cfg(not(target_arch = "wasm32"))]
980+
#[test]
981+
fn test_build_auth_headers_falls_back_to_api_keys() {
982+
let mut cfg = test_config("http://unused.local");
983+
cfg.access_token = None;
984+
cfg.api_key = Some("api-1".into());
985+
cfg.app_key = Some("app-1".into());
986+
let headers = super::build_auth_headers(&cfg).expect("api-key headers");
987+
assert_eq!(headers.get("dd-api-key").unwrap(), "api-1");
988+
assert_eq!(headers.get("dd-application-key").unwrap(), "app-1");
989+
assert!(headers.get("authorization").is_none());
990+
}
991+
992+
#[cfg(not(target_arch = "wasm32"))]
993+
#[test]
994+
fn test_build_auth_headers_requires_some_auth() {
995+
let mut cfg = test_config("http://unused.local");
996+
cfg.access_token = None;
997+
cfg.api_key = None;
998+
cfg.app_key = None;
999+
let err = super::build_auth_headers(&cfg).expect_err("should require auth");
1000+
assert!(err.to_string().contains("requires authentication"));
1001+
}
1002+
9621003
async fn server_mock_delete(s: &mut mockito::Server) -> mockito::Mock {
9631004
s.mock("DELETE", mockito::Matcher::Any)
9641005
.with_status(204)

0 commit comments

Comments
 (0)