Enable Observability Pipelines via OAuth#520
Open
Jansen-w wants to merge 2 commits into
Open
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4fd71f3bed
ℹ️ 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".
platinummonkey
previously approved these changes
May 20, 2026
Author
|
/remove |
|
View all feedbacks in Devflow UI.
|
amaskara-dd
previously approved these changes
May 22, 2026
Currently `pup obs-pipelines list` (and every other OP subcommand) returns 401 Unauthorized when called with OAuth credentials, because pup's default scope set never requests any `observability_pipelines_*` scope. The CLI side has `pup obs-pipelines …` wired up, and `agents/observability-pipelines.md` already documents that the routes require `observability_pipelines_read`, `observability_pipelines_deploy`, and `observability_pipelines_delete` — this PR closes that gap by adding those scopes to the default consent set. - `observability_pipelines_read` added to both `default_scopes()` and `read_only_scopes()`. - `observability_pipelines_deploy` and `observability_pipelines_delete` added to `default_scopes()` only (write/destructive). - Updated `test_default_scopes()` count from 85 to 88 and added containment assertions for all three new scopes.
The `make_api!` macro sends the OAuth bearer token when one is available and falls back to API-key headers otherwise. Previously the OP commands used `make_api_no_auth!`, which suppressed the bearer header outright because the OP routes did not accept `ValidOAuthAccessToken` on the server side. Server-side support for OAuth on the OP public routes is in flight (DataDog/dd-source#426541 covers the Rapid-side authn list on obs-pipelines-crud; DataDog/dd-go#238110 covers the Route Manager allowlist on rc-api-proxy across staging/prod/gov). Once those land and pup's OAuth client gets the OP scopes added, this change unblocks `pup obs-pipelines` via OAuth without further client work. Pattern copied from the prior attempt in PR DataDog#342, adapted to the current `make_api!` macro (which subsumed the older `make_bearer_client` helper used in that PR).
4fd71f3 to
64006c3
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #521.
Summary
Two changes that, together with in-flight server-side work, will make
pup obs-pipelineswork via OAuth instead of requiringDD_API_KEY+DD_APP_KEY:src/commands/obs_pipelines.rs— switch the OP commands frommake_api_no_auth!tomake_api!, so the OAuth bearer token is sent when one is available (falling back to API-key headers otherwise). Same pattern as PR [NOT READY] Add OAuth support to remaining 6 commands #342, expressed via the macro that subsumed the oldmake_bearer_clienthelper.src/auth/types.rs— addobservability_pipelines_read,observability_pipelines_deploy, andobservability_pipelines_deletetodefault_scopes(), andobservability_pipelines_readtoread_only_scopes(), sopup auth loginrequests them on the consent screen. Updatedtest_default_scopes()count (85 → 88) and added containment assertions.Why this was needed
Today
pup obs-pipelines list(and every other OP subcommand) returns 401 Unauthorized for OAuth-authenticated users. Two reasons:make_api_no_auth!was suppressing the bearer header on every OP call (the comment said "Observability Pipelines does not support OAuth — API key auth only"). API-key headers were the only auth ever sent.observability_pipelines_*scopes, so the token wouldn't have carried them anyway.agents/observability-pipelines.mddocumentsobservability_pipelines_{read,deploy,delete}as required for these endpoints, but none of those scope names appeared anywhere insrc/auth/types.rs. This PR closes both halves of that gap.Test plan
cargo check --bin puppassescargo test --bin pup auth::types::— all 9 tests pass with updated assertionsinvalid_scope(confirming the unsupported-scope hypothesis)pup obs-pipelines listreturns pipelines instead of 401