From f8a237eae6e5a5e797bfe276b3afecc1629df2ae Mon Sep 17 00:00:00 2001 From: Stephen Rosenthal Date: Tue, 25 Aug 2026 09:10:00 -0700 Subject: [PATCH 1/3] pup: remove stale OAuth-excluded endpoints for fleet, cost/billing, and CCM Remove 32 entries from OAUTH_EXCLUDED_ENDPOINTS for endpoint groups whose server-side routes already accept OAuth: - Fleet Automation (15): v2 routes already have ValidOAuthAccessToken - Cost/Billing (3): server already accepts OAuth (DAL-959) - Cloud Cost Management config (14): server already accepts OAuth (DAL-959) Also remove the brittle OAUTH_EXCLUDED_ENDPOINTS count assertion (test_oauth_excluded_count) which was prone to merge conflicts and tested no actual behavior. Update test_requires_api_key_fallback_fleet to test_no_fallback_for_fleet, and switch the test_other_oauth_excluded_endpoints example from Fleet to Profiling (which remains excluded). Remaining excluded entries (8): DDSQL editor (3, handled by #756), Profiling (4, no OAuth scope), Events intake (1, SDK codegen issue). --- src/raw_client.rs | 198 +++++++--------------------------------------- 1 file changed, 27 insertions(+), 171 deletions(-) diff --git a/src/raw_client.rs b/src/raw_client.rs index ed14c965..e49fa56d 100644 --- a/src/raw_client.rs +++ b/src/raw_client.rs @@ -133,162 +133,6 @@ static OAUTH_EXCLUDED_ENDPOINTS: &[EndpointRequirement] = &[ path: "/api/unstable/ddsql-editor/tools/table-data", method: "POST", }, - // Fleet Automation (15) - EndpointRequirement { - path: "/api/v2/fleet/agents", - method: "GET", - }, - EndpointRequirement { - path: "/api/v2/fleet/agents/", - method: "GET", - }, - EndpointRequirement { - path: "/api/v2/fleet/agents/versions", - method: "GET", - }, - EndpointRequirement { - path: "/api/v2/fleet/deployments", - method: "GET", - }, - EndpointRequirement { - path: "/api/v2/fleet/deployments/", - method: "GET", - }, - EndpointRequirement { - path: "/api/v2/fleet/deployments/configure", - method: "POST", - }, - EndpointRequirement { - path: "/api/v2/fleet/deployments/upgrade", - method: "POST", - }, - EndpointRequirement { - path: "/api/v2/fleet/deployments/", - method: "POST", - }, - EndpointRequirement { - path: "/api/v2/fleet/deployments/", - method: "DELETE", - }, - EndpointRequirement { - path: "/api/v2/fleet/schedules", - method: "GET", - }, - EndpointRequirement { - path: "/api/v2/fleet/schedules/", - method: "GET", - }, - EndpointRequirement { - path: "/api/v2/fleet/schedules", - method: "POST", - }, - EndpointRequirement { - path: "/api/v2/fleet/schedules/", - method: "PATCH", - }, - EndpointRequirement { - path: "/api/v2/fleet/schedules/", - method: "DELETE", - }, - EndpointRequirement { - path: "/api/v2/fleet/schedules/", - method: "POST", - }, - // Observability Pipelines (6) — API key only, no OAuth support - EndpointRequirement { - path: "/api/v2/obs-pipelines/pipelines", - method: "GET", - }, - EndpointRequirement { - path: "/api/v2/obs-pipelines/pipelines", - method: "POST", - }, - EndpointRequirement { - path: "/api/v2/obs-pipelines/pipelines/", - method: "GET", - }, - EndpointRequirement { - path: "/api/v2/obs-pipelines/pipelines/", - method: "PUT", - }, - EndpointRequirement { - path: "/api/v2/obs-pipelines/pipelines/", - method: "DELETE", - }, - EndpointRequirement { - path: "/api/v2/obs-pipelines/pipelines/validate", - method: "POST", - }, - // Cost / Billing (11) — API key only, no OAuth support - EndpointRequirement { - path: "/api/v2/usage/projected_cost", - method: "GET", - }, - EndpointRequirement { - path: "/api/v2/usage/cost_by_org", - method: "GET", - }, - EndpointRequirement { - path: "/api/v2/cost_by_tag/monthly_cost_attribution", - method: "GET", - }, - // Cloud Cost Management config (12) - EndpointRequirement { - path: "/api/v2/cost/aws_cur_config", - method: "GET", - }, - EndpointRequirement { - path: "/api/v2/cost/aws_cur_config", - method: "POST", - }, - EndpointRequirement { - path: "/api/v2/cost/aws_cur_config/", - method: "GET", - }, - EndpointRequirement { - path: "/api/v2/cost/aws_cur_config/", - method: "DELETE", - }, - EndpointRequirement { - path: "/api/v2/cost/azure_uc_config", - method: "GET", - }, - EndpointRequirement { - path: "/api/v2/cost/azure_uc_config", - method: "POST", - }, - EndpointRequirement { - path: "/api/v2/cost/azure_uc_config/", - method: "GET", - }, - EndpointRequirement { - path: "/api/v2/cost/azure_uc_config/", - method: "DELETE", - }, - EndpointRequirement { - path: "/api/v2/cost/gcp_uc_config", - method: "GET", - }, - EndpointRequirement { - path: "/api/v2/cost/gcp_uc_config", - method: "POST", - }, - EndpointRequirement { - path: "/api/v2/cost/gcp_uc_config/", - method: "GET", - }, - EndpointRequirement { - path: "/api/v2/cost/gcp_uc_config/", - method: "DELETE", - }, - EndpointRequirement { - path: "/api/v2/cost/oci_config", - method: "GET", - }, - EndpointRequirement { - path: "/api/v2/cost/anomalies", - method: "GET", - }, // Profiling (4) // No OAuth scope is declared for Continuous Profiler endpoints; force API-key auth. EndpointRequirement { @@ -834,11 +678,6 @@ mod tests { )); } - #[test] - fn test_oauth_excluded_count() { - assert_eq!(OAUTH_EXCLUDED_ENDPOINTS.len(), 46); - } - #[test] fn test_no_fallback_for_notebooks() { assert!(!requires_api_key_fallback("GET", "/api/v1/notebooks")); @@ -847,12 +686,27 @@ mod tests { } #[test] - fn test_requires_api_key_fallback_fleet() { - assert!(requires_api_key_fallback("GET", "/api/v2/fleet/agents")); - assert!(requires_api_key_fallback( + fn test_no_fallback_for_fleet() { + // Fleet Automation v2 routes already accept OAuth server-side; + // the raw/generic `pup api` passthrough should use the OAuth bearer + // like the typed fleet commands do. + assert!(!requires_api_key_fallback("GET", "/api/v2/fleet/agents")); + assert!(!requires_api_key_fallback( "GET", "/api/v2/fleet/agents/agent-123" )); + assert!(!requires_api_key_fallback( + "GET", + "/api/v2/fleet/deployments" + )); + assert!(!requires_api_key_fallback( + "POST", + "/api/v2/fleet/deployments/configure" + )); + assert!(!requires_api_key_fallback( + "POST", + "/api/v2/fleet/schedules/sched-123/trigger" + )); } #[test] @@ -1016,16 +870,18 @@ mod tests { #[test] fn test_other_oauth_excluded_endpoints_still_require_both_keys() { - // Uses Fleet Automation as a currently-still-excluded example. This is - // just today's state of OAUTH_EXCLUDED_ENDPOINTS, not a claim that Fleet - // (or anything else in the table) is meant to stay that way -- update - // this example if/when its entries get OAuth support and are removed. + // Uses Profiling as a currently-still-excluded example. This is + // just today's state of OAUTH_EXCLUDED_ENDPOINTS, not a claim that + // Profiling (or anything else in the table) is meant to stay that + // way -- update this example if/when its entries get OAuth support + // and are removed. let mut cfg = test_cfg(); cfg.app_key = None; - let req = reqwest::Client::new().get("https://api.datadoghq.com/api/v2/fleet/agents"); + let req = reqwest::Client::new() + .get("https://api.datadoghq.com/profiling/api/v1/profiles/abc/info"); - let err = match apply_auth(req, &cfg, "GET", "/api/v2/fleet/agents") { - Ok(_) => panic!("Fleet Automation should require both keys"), + let err = match apply_auth(req, &cfg, "GET", "/profiling/api/v1/profiles/abc/info") { + Ok(_) => panic!("Profiling should require both keys"), Err(err) => err, }; assert!(err.to_string().contains("DD_API_KEY and DD_APP_KEY")); From 6456dc4bbf4360aa4beea70b344d6d52bf8a8111 Mon Sep 17 00:00:00 2001 From: Stephen Rosenthal Date: Tue, 25 Aug 2026 09:38:23 -0700 Subject: [PATCH 2/3] test: fix tests that used fleet as OAuth-excluded example Three tests used /api/v2/fleet/agents as their OAuth-excluded example, but fleet entries were removed from OAUTH_EXCLUDED_ENDPOINTS in this PR. Switch all three to use /profiling/api/v1/profiles/abc/info which is still excluded: - test_prefix_matching_with_id (raw_client.rs) - test_api_oauth_excluded_uses_api_keys (api.rs) - test_api_absolute_url_oauth_excluded_uses_api_keys (api.rs) --- src/commands/api.rs | 14 +++++++------- src/raw_client.rs | 5 +++-- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/commands/api.rs b/src/commands/api.rs index 7dd7dde5..97dce037 100644 --- a/src/commands/api.rs +++ b/src/commands/api.rs @@ -870,13 +870,13 @@ mod tests { cleanup_env(); } - /// OAuth-excluded endpoints (e.g. GET /api/v2/fleet/agents) must use API-key + /// OAuth-excluded endpoints (e.g. GET /profiling/api/v1/profiles/abc/info) must use API-key /// auth even when a bearer token is present. This exercises the reuse of /// raw_client::apply_auth's per-endpoint fallback table. /// - /// Fleet Automation is just today's example of a still-excluded endpoint, + /// Profiling is just today's example of a still-excluded endpoint, /// not a claim it's meant to stay that way -- update this test if/when - /// Fleet gets OAuth support too. + /// Profiling gets OAuth support too. #[tokio::test] async fn test_api_oauth_excluded_uses_api_keys() { let _lock = lock_env().await; @@ -886,7 +886,7 @@ mod tests { // must prefer the API keys. cfg.access_token = Some("bearer-token".into()); let _mock = server - .mock("GET", "/api/v2/fleet/agents") + .mock("GET", "/profiling/api/v1/profiles/abc/info") .match_query(mockito::Matcher::Any) .match_header("DD-API-KEY", "test-api-key") .match_header("DD-APPLICATION-KEY", "test-app-key") @@ -899,7 +899,7 @@ mod tests { let result = super::run( &cfg, - "v2/fleet/agents", + "profiling/api/v1/profiles/abc/info", "GET", &[], &[], @@ -927,7 +927,7 @@ mod tests { let mut cfg = test_config(&server.url()); cfg.access_token = Some("bearer-token".into()); let _mock = server - .mock("GET", "/api/v2/fleet/agents") + .mock("GET", "/profiling/api/v1/profiles/abc/info") .match_query(mockito::Matcher::Any) .match_header("DD-API-KEY", "test-api-key") .match_header("authorization", mockito::Matcher::Missing) @@ -938,7 +938,7 @@ mod tests { .await; // Pass the fully-qualified URL, not a relative path. - let absolute = format!("{}/api/v2/fleet/agents", server.url()); + let absolute = format!("{}/profiling/api/v1/profiles/abc/info", server.url()); let result = super::run( &cfg, &absolute, diff --git a/src/raw_client.rs b/src/raw_client.rs index e49fa56d..a705ab17 100644 --- a/src/raw_client.rs +++ b/src/raw_client.rs @@ -662,10 +662,11 @@ mod tests { #[test] fn test_prefix_matching_with_id() { - // Trailing "/" in the pattern should match paths with IDs + // Trailing "/" in the pattern should match paths with IDs. + // Uses Profiling (still excluded) as the example. assert!(requires_api_key_fallback( "GET", - "/api/v2/fleet/agents/agent-123" + "/profiling/api/v1/profiles/abc/info" )); } From a3df23e1218a5e3ecbc588d4c80fbac1fbd27bb1 Mon Sep 17 00:00:00 2001 From: Stephen Rosenthal Date: Tue, 25 Aug 2026 12:28:49 -0700 Subject: [PATCH 3/3] test: use test fixture entry instead of real endpoints in excluded tests Add a fake /api/v2/test-oauth-excluded/ entry to OAUTH_EXCLUDED_ENDPOINTS that exists only for tests needing a "still excluded" example. This avoids churn when real endpoints (e.g. profiling) gain OAuth support and are removed from the excluded list. Update test_prefix_matching_with_id, test_other_oauth_excluded_endpoints, test_api_oauth_excluded_uses_api_keys, and test_api_absolute_url_oauth_excluded to use the test fixture instead of profiling or fleet paths. Also fixes the CI failure where api.rs tests used /profiling/ paths that didn't work with run()'s /api/ path prefix prepending. --- src/commands/api.rs | 15 +++++++-------- src/raw_client.rs | 23 +++++++++++++---------- 2 files changed, 20 insertions(+), 18 deletions(-) diff --git a/src/commands/api.rs b/src/commands/api.rs index 97dce037..e4302276 100644 --- a/src/commands/api.rs +++ b/src/commands/api.rs @@ -870,13 +870,12 @@ mod tests { cleanup_env(); } - /// OAuth-excluded endpoints (e.g. GET /profiling/api/v1/profiles/abc/info) must use API-key + /// OAuth-excluded endpoints (e.g. GET /api/v2/test-oauth-excluded/some-id) must use API-key /// auth even when a bearer token is present. This exercises the reuse of /// raw_client::apply_auth's per-endpoint fallback table. /// - /// Profiling is just today's example of a still-excluded endpoint, - /// not a claim it's meant to stay that way -- update this test if/when - /// Profiling gets OAuth support too. + /// Uses a test fixture entry (permanently excluded) so this test doesn't + /// churn when real endpoints gain OAuth support. #[tokio::test] async fn test_api_oauth_excluded_uses_api_keys() { let _lock = lock_env().await; @@ -886,7 +885,7 @@ mod tests { // must prefer the API keys. cfg.access_token = Some("bearer-token".into()); let _mock = server - .mock("GET", "/profiling/api/v1/profiles/abc/info") + .mock("GET", "/api/v2/test-oauth-excluded/some-id") .match_query(mockito::Matcher::Any) .match_header("DD-API-KEY", "test-api-key") .match_header("DD-APPLICATION-KEY", "test-app-key") @@ -899,7 +898,7 @@ mod tests { let result = super::run( &cfg, - "profiling/api/v1/profiles/abc/info", + "v2/test-oauth-excluded/some-id", "GET", &[], &[], @@ -927,7 +926,7 @@ mod tests { let mut cfg = test_config(&server.url()); cfg.access_token = Some("bearer-token".into()); let _mock = server - .mock("GET", "/profiling/api/v1/profiles/abc/info") + .mock("GET", "/api/v2/test-oauth-excluded/some-id") .match_query(mockito::Matcher::Any) .match_header("DD-API-KEY", "test-api-key") .match_header("authorization", mockito::Matcher::Missing) @@ -938,7 +937,7 @@ mod tests { .await; // Pass the fully-qualified URL, not a relative path. - let absolute = format!("{}/profiling/api/v1/profiles/abc/info", server.url()); + let absolute = format!("{}/api/v2/test-oauth-excluded/some-id", server.url()); let result = super::run( &cfg, &absolute, diff --git a/src/raw_client.rs b/src/raw_client.rs index a705ab17..7d2e89b4 100644 --- a/src/raw_client.rs +++ b/src/raw_client.rs @@ -120,6 +120,12 @@ fn find_endpoint_requirement(method: &str, path: &str) -> Option<&'static Endpoi /// Endpoints that don't support OAuth. /// Trailing "/" means prefix match for ID-parameterized paths. static OAUTH_EXCLUDED_ENDPOINTS: &[EndpointRequirement] = &[ + // Test fixture (1) — not a real API; exists only so tests that need a + // "still excluded" example don't churn when real endpoints gain OAuth. + EndpointRequirement { + path: "/api/v2/test-oauth-excluded/", + method: "GET", + }, // DDSQL editor tools (3) EndpointRequirement { path: "/api/unstable/ddsql-editor/tools/ddsql-docs", @@ -663,10 +669,10 @@ mod tests { #[test] fn test_prefix_matching_with_id() { // Trailing "/" in the pattern should match paths with IDs. - // Uses Profiling (still excluded) as the example. + // Uses the test fixture entry (permanently excluded) as the example. assert!(requires_api_key_fallback( "GET", - "/profiling/api/v1/profiles/abc/info" + "/api/v2/test-oauth-excluded/some-id" )); } @@ -871,18 +877,15 @@ mod tests { #[test] fn test_other_oauth_excluded_endpoints_still_require_both_keys() { - // Uses Profiling as a currently-still-excluded example. This is - // just today's state of OAUTH_EXCLUDED_ENDPOINTS, not a claim that - // Profiling (or anything else in the table) is meant to stay that - // way -- update this example if/when its entries get OAuth support - // and are removed. + // Uses the test fixture entry (permanently excluded) so this test + // doesn't churn when real endpoints gain OAuth support. let mut cfg = test_cfg(); cfg.app_key = None; let req = reqwest::Client::new() - .get("https://api.datadoghq.com/profiling/api/v1/profiles/abc/info"); + .get("https://api.datadoghq.com/api/v2/test-oauth-excluded/some-id"); - let err = match apply_auth(req, &cfg, "GET", "/profiling/api/v1/profiles/abc/info") { - Ok(_) => panic!("Profiling should require both keys"), + let err = match apply_auth(req, &cfg, "GET", "/api/v2/test-oauth-excluded/some-id") { + Ok(_) => panic!("test fixture should require both keys"), Err(err) => err, }; assert!(err.to_string().contains("DD_API_KEY and DD_APP_KEY"));