pup: remove stale OAuth-excluded endpoints for fleet, cost/billing, and CCM - #762
pup: remove stale OAuth-excluded endpoints for fleet, cost/billing, and CCM#762srosenthal-dd wants to merge 3 commits into
Conversation
…nd 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 DataDog#756), Profiling (4, no OAuth scope), Events intake (1, SDK codegen issue).
This comment has been minimized.
This comment has been minimized.
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)
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.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a3df23e121
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| path: "/api/v2/cost/anomalies", | ||
| method: "GET", | ||
| }, | ||
| // Profiling (4) |
There was a problem hiding this comment.
When an OAuth-authenticated user with API/application keys runs pup obs-pipelines diff, src/commands/obs_pipelines.rs calls raw_get for /api/v2/obs-pipelines/pipelines/{id}; removing this endpoint group makes apply_auth send the bearer token instead of the required keys, causing the request to be rejected. The documented rollout covers Fleet, Cost/Billing, and CCM—not Observability Pipelines, whose six entries were explicitly marked API-key-only—so restore those exclusions and their behavioral coverage. This unrelated deletion also exceeds the stated scope of the change.
AGENTS.md reference: AGENTS.md:L21-L27
Useful? React with 👍 / 👎.
| path: "/api/v2/cost/anomalies", | ||
| method: "GET", | ||
| }, | ||
| // Profiling (4) |
There was a problem hiding this comment.
The commit removes the Cost/Billing and Cloud Cost Management entries, but the only new no-fallback assertions exercise Fleet; a repo-wide search finds no test proving that representative /api/v2/usage/* and /api/v2/cost/* requests now attach an OAuth bearer rather than falling back to API/application keys. Add header-level happy-path coverage for these endpoint groups, along with the applicable missing-auth failure case, so method/path mistakes in this security-sensitive table cannot pass unnoticed.
AGENTS.md reference: AGENTS.md:L21-L22
Useful? React with 👍 / 👎.
2 participants
Summary
Remove 32 stale entries from
OAUTH_EXCLUDED_ENDPOINTS(src/raw_client.rs) whose server-side routes already accept OAuth: Fleet Automation v2 routes (15, since 2026-06-04), Cost/Billing (3, DAL-959), Cloud Cost Management config (14, DAL-959). This table only affectsraw_get/raw_post(pup apipassthrough and hand-written commands) -- no live pup command usesraw_get/raw_postfor any of these paths today, so this only changespup apipassthrough behavior.Note: this doesn't close pup's fleet OAuth gap.
fleet.rs's commands call/api/unstable/fleet/*, not the/api/v2/fleet/*paths covered here, and that surface still deliberately excludes OAuth (DAL-509, separate issue).Also removes the brittle
test_oauth_excluded_countassertion.Remaining excluded entries (8)