Skip to content

Add OAuth support to workflows commands#523

Merged
platinummonkey merged 4 commits into
DataDog:mainfrom
srosenthal-dd:stephen.rosenthal/oauth-remaining-commands
May 22, 2026
Merged

Add OAuth support to workflows commands#523
platinummonkey merged 4 commits into
DataDog:mainfrom
srosenthal-dd:stephen.rosenthal/oauth-remaining-commands

Conversation

@srosenthal-dd
Copy link
Copy Markdown
Member

@srosenthal-dd srosenthal-dd commented May 21, 2026

Summary

Pup's workflow commands now use OAuth bearer when available, falling back to DD_API_KEY + DD_APP_KEY. The Datadog Workflow Automation API has accepted OAuth on the public workflow routes for a while; the pup-side helpers and pre-flight guards were the only thing still treating workflows as API-key-only.

Changes

  • src/commands/workflows.rs: workflow CRUD and instance call sites flipped to make_api! (OAuth-with-fallback). The action-connections helper stays on make_api_no_auth! for now -- see "What's not in scope".
  • src/main.rs: drop the top-level "API keys required" pre-flight for the Workflows subcommand and the matching guard on Run. Update the AUTHENTICATION doc-comments to reflect what works via OAuth.
  • src/auth/types.rs: add workflows_read, workflows_run, workflows_write to default_scopes() and workflows_read to read_only_scopes().

What's not in scope

pup workflows connections * (the action-connections API) stays API-key-only. That surface still only accepts API+App key auth server-side; a separate server-side change to add OAuth there is in flight but out of scope here.

End-to-end validation (prod)

Real HTTP probes against api.datadoghq.com with an OAuth-only token carrying workflows_read, workflows_run, workflows_write:

Probe Required perm HTTP What it proves
workflows get <bogus> workflows_read 404 "workflow not found" OAuth admit + perm check pass
workflows instances list <bogus> workflows_read 200 (empty data) full successful round-trip via OAuth
workflows instances get <bogus> <bogus> workflows_read 404 "instance not found" OAuth healthy
workflows instances cancel <bogus> <bogus> workflows_run 404 "instance not found" Run scope works via OAuth
workflows delete <bogus> workflows_write 404 "workflow not found" Write scope works via OAuth
workflows connections get <bogus> (connections API) 401 Unauthorized as expected -- the documented non-OAuth surface

workflows run and workflows {create,update} weren't probed because they would trigger or mutate real workflows; their underlying scopes (workflows_run and workflows_write) are validated by cancel and delete respectively.

Test plan

  • `cargo build --release --bin pup` -- clean
  • `cargo test --bin pup auth::types::` -- 9/9 pass
  • End-to-end probe matrix above

https://datadoghq.atlassian.net/browse/DAL-551

The WorkflowAutomationAPI surface (/api/v2/workflows/{...}, including
instances list/get/cancel) already accepts OAuth bearer tokens
server-side in dd-source domains/workflow/apps/apis/
wf-orchestration-api/main.go. The pup side was the only blocker:
the helper used make_api_no_auth! and a top-level pre-flight bailed
the whole subcommand if API keys weren't set.

Changes:
- workflows.rs: WorkflowAutomationAPI calls now go through make_api!
  (3 sites). The ActionConnectionAPI helper at the bottom stays on
  make_api_no_auth! since the /api/v2/actions/connections/* routes
  do not yet accept OAuth.
- main.rs: move the validate_api_and_app_keys() guard from the
  top-level Workflows arm down to the Run arm only. `workflows run`
  hits the API-trigger endpoint, which is API-key-only.
- main.rs: update the AUTHENTICATION doc comment to reflect what
  works via OAuth and what still requires API+App keys.
- auth/types.rs: add workflows_read + workflows_write to
  default_scopes(), and workflows_read to read_only_scopes().
  Test count 85 -> 87, plus containment asserts.
The /api/v2/workflows/{id}/instances POST and the
.../instances/{id}/cancel PUT routes both attach defaultAuthnPerms
in domains/workflow/apps/apis/wf-orchestration-api/main.go, which
includes ValidOAuthAccessToken. The historical API-key-only
guidance in pup pre-dated that change and is stale.

- Add workflows_run to default_scopes() (not read_only_scopes).
- Drop the API-key-only pre-flight from the workflows run dispatch.
- Update the Workflows / Run doc-comments to stop claiming OAuth is
  unsupported, and drop the (now incorrect) section header
  'API trigger only -- requires DD_API_KEY + DD_APP_KEY' in
  commands/workflows.rs.

workflows.rs's WorkflowAutomationAPI helpers are already on
make_api!, so cancel and run will use the OAuth bearer when present
and fall back to DD_API_KEY+DD_APP_KEY otherwise. The
ActionConnectionAPI helper stays on make_api_no_auth! until
dd-source#426542 lands.

Requires: workflows_run added to the datadog-pup-cli OAuth client
allowlist per DC, otherwise pup auth login fails with invalid_scope.
@datadog-official

This comment has been minimized.

Several PRs in flight all add/remove scopes in default_scopes(),
each one bumping the hardcoded count. Every collision turns into
a trivial 3-way merge resolve. The containment asserts below give
us coverage for the scopes this PR cares about; dropping the count
assertion costs nothing meaningful.
Comment thread src/auth/types.rs
#[test]
fn test_default_scopes() {
let scopes = default_scopes();
assert_eq!(scopes.len(), 85);
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dropped the assert_eq!(scopes.len(), N); assertion that used to live here.

Several PRs in flight all add or remove scopes in default_scopes(), and each one bumps that hardcoded count. The result was every concurrent PR landing turned into a trivial 3-way merge resolution against the count.

The containment asserts below still cover the scopes this PR cares about. If we ever want a non-numeric sanity check (no duplicates, alphabetical, etc.) that doesn't rot on every scope change, we can add it separately.

@srosenthal-dd srosenthal-dd marked this pull request as ready for review May 21, 2026 22:26
@srosenthal-dd srosenthal-dd requested a review from a team as a code owner May 21, 2026 22:26
Removing the top-level validate_api_and_app_keys() pre-flight left
the Workflows arm as `=> { match action { ... } }`, which cargo fmt
prefers to collapse to `=> match action { ... }`. Pure formatting,
no semantic change.

CI 'Check, Test & Coverage' was failing on cargo fmt --check; this
fixes it. The AI annotation that claimed this was a missing-match-
arm compilation error was a misclassification.
@platinummonkey platinummonkey merged commit 41026ad into DataDog:main May 22, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants