pup: remove stale OAuth-excluded endpoints for fleet, cost/billing, and CCM #762
+40
−181
There are no files selected for viewing
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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", | ||
|
|
@@ -133,162 +139,6 @@ static OAUTH_EXCLUDED_ENDPOINTS: &[EndpointRequirement] = &[ | |
| path: "/api/unstable/ddsql-editor/tools/table-data", | ||
| method: "POST", | ||
| }, | ||
| // 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 AGENTS.md reference: AGENTS.md:L21-L22 Useful? React with 👍 / 👎. |
||
| // No OAuth scope is declared for Continuous Profiler endpoints; force API-key auth. | ||
| EndpointRequirement { | ||
|
|
@@ -818,10 +668,11 @@ mod tests { | |
|
|
||
| #[test] | ||
| fn test_prefix_matching_with_id() { | ||
| // Trailing "/" in the pattern should match paths with IDs. | ||
| // Uses the test fixture entry (permanently excluded) as the example. | ||
| assert!(requires_api_key_fallback( | ||
| "GET", | ||
| "/api/v2/test-oauth-excluded/some-id" | ||
| )); | ||
| } | ||
|
|
||
|
|
@@ -834,11 +685,6 @@ mod tests { | |
| )); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_no_fallback_for_notebooks() { | ||
| assert!(!requires_api_key_fallback("GET", "/api/v1/notebooks")); | ||
|
|
@@ -847,12 +693,27 @@ mod tests { | |
| } | ||
|
|
||
| #[test] | ||
| 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 +877,15 @@ mod tests { | |
|
|
||
| #[test] | ||
| fn test_other_oauth_excluded_endpoints_still_require_both_keys() { | ||
| // 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/api/v2/test-oauth-excluded/some-id"); | ||
|
|
||
| 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")); | ||
|
|
||
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When an OAuth-authenticated user with API/application keys runs
pup obs-pipelines diff,src/commands/obs_pipelines.rscallsraw_getfor/api/v2/obs-pipelines/pipelines/{id}; removing this endpoint group makesapply_authsend 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 👍 / 👎.