From cb87ba673e26def3409a26cda94bf12d182da513 Mon Sep 17 00:00:00 2001 From: Stephen Rosenthal Date: Tue, 12 May 2026 15:35:04 -0400 Subject: [PATCH 1/2] feat(apm): re-add service remapping CRUD commands Resurrects #439 (reverted in #454). The original revert was because pup's `default_scopes()` requested `apm_service_renaming_write` before that scope was on pup's server-side DCR client allowlist, breaking every `pup auth login` with `invalid_scope`. The allowlist is now in place, verified end-to-end against app.datadoghq.com with this build: - `pup auth login` succeeds with `apm_service_renaming_write` in the token - `pup apm service-remapping list` returns the rules - `pup apm service-remapping delete 1` returns 404 (scope passed; rule not found), confirming the write scope is honored Test-count assertion bumped 84 -> 85 to match current main. Adds back: - `pup apm service-remapping {list,create,get,update,delete}` commands - `apm_service_renaming_write` in `default_scopes()` - `raw_put` helper in `client.rs` --- src/auth/types.rs | 4 +- src/client.rs | 27 ++++ src/commands/apm.rs | 337 ++++++++++++++++++++++++++++++++++++++++++++ src/main.rs | 105 ++++++++++++++ 4 files changed, 472 insertions(+), 1 deletion(-) diff --git a/src/auth/types.rs b/src/auth/types.rs index 8c1c9dfb..c3d90679 100644 --- a/src/auth/types.rs +++ b/src/auth/types.rs @@ -105,6 +105,7 @@ pub fn default_scopes() -> Vec<&'static str> { // APM "apm_read", "apm_service_catalog_read", + "apm_service_renaming_write", // Audit "audit_logs_read", // AWS @@ -270,7 +271,7 @@ mod tests { #[test] fn test_default_scopes() { let scopes = default_scopes(); - assert_eq!(scopes.len(), 84); + assert_eq!(scopes.len(), 85); assert!(scopes.contains(&"dashboards_read")); assert!(scopes.contains(&"monitors_read")); assert!(scopes.contains(&"logs_read_data")); @@ -283,6 +284,7 @@ mod tests { assert!(scopes.contains(&"ci_visibility_read")); assert!(scopes.contains(&"teams_read")); assert!(scopes.contains(&"apm_service_catalog_read")); + assert!(scopes.contains(&"apm_service_renaming_write")); assert!(scopes.contains(&"status_pages_settings_read")); assert!(scopes.contains(&"on_call_read")); assert!(scopes.contains(&"on_call_write")); diff --git a/src/client.rs b/src/client.rs index 0bc58677..a2e69c03 100644 --- a/src/client.rs +++ b/src/client.rs @@ -1009,6 +1009,33 @@ pub async fn raw_post_jsonapi( parse_response_json(resp).await } +pub async fn raw_put( + cfg: &Config, + path: &str, + body: serde_json::Value, +) -> anyhow::Result { + let url = format!("{}{}", cfg.api_base_url(), path); + let client = reqwest::Client::new(); + let req = client.put(&url); + let req = apply_auth(req, cfg, "PUT", path)?; + let resp = req + .header("Content-Type", "application/json") + .header("Accept", "application/json") + .header("User-Agent", useragent::get()) + .json(&body) + .send() + .await?; + if resp.status() == reqwest::StatusCode::NO_CONTENT { + return Ok(serde_json::Value::Null); + } + if !resp.status().is_success() { + let status = resp.status(); + let body = resp.text().await.unwrap_or_default(); + anyhow::bail!("PUT {url} failed (HTTP {status}): {body}"); + } + Ok(resp.json().await?) +} + /// Like `raw_post`, but returns the parsed JSON body even on non-2xx responses. /// Callers are responsible for inspecting the body for errors. pub async fn raw_post_lenient( diff --git a/src/commands/apm.rs b/src/commands/apm.rs index f0f185e5..4fe973cc 100644 --- a/src/commands/apm.rs +++ b/src/commands/apm.rs @@ -100,6 +100,67 @@ pub async fn troubleshooting_list( formatter::output(cfg, &data) } +pub async fn service_remapping_list(cfg: &Config) -> Result<()> { + let data = client::raw_get(cfg, "/api/v2/service-naming-rules", &[]).await?; + formatter::output(cfg, &data) +} + +pub async fn service_remapping_create( + cfg: &Config, + name: String, + filter: String, + rule_type: i64, + value: String, +) -> Result<()> { + let body = serde_json::json!({ + "data": { + "type": "rule", + "attributes": { + "name": name, + "filter": filter, + "rule_type": rule_type, + "rewrite_tag_rules": [{"destination_tag_name": "service", "value": value}] + } + } + }); + let data = client::raw_post(cfg, "/api/v2/service-naming-rules", body).await?; + formatter::output(cfg, &data) +} + +pub async fn service_remapping_get(cfg: &Config, id: String) -> Result<()> { + let data = client::raw_get(cfg, &format!("/api/v2/service-naming-rules/{id}"), &[]).await?; + formatter::output(cfg, &data) +} + +pub async fn service_remapping_update( + cfg: &Config, + id: String, + name: String, + filter: String, + rule_type: i64, + value: String, + version: i64, +) -> Result<()> { + let body = serde_json::json!({ + "data": { + "type": "rule", + "attributes": { + "name": name, + "filter": filter, + "rule_type": rule_type, + "rewrite_tag_rules": [{"destination_tag_name": "service", "value": value}], + "version": version + } + } + }); + let data = client::raw_put(cfg, &format!("/api/v2/service-naming-rules/{id}"), body).await?; + formatter::output(cfg, &data) +} + +pub async fn service_remapping_delete(cfg: &Config, id: String, version: i64) -> Result<()> { + client::raw_delete(cfg, &format!("/api/v2/service-naming-rules/{id}/{version}")).await +} + pub async fn service_config_get( cfg: &Config, service_name: String, @@ -384,4 +445,280 @@ mod tests { mock.assert_async().await; cleanup_env(); } + + #[tokio::test] + async fn test_service_remapping_list() { + let _lock = lock_env().await; + let mut server = mockito::Server::new_async().await; + let cfg = test_config(&server.url()); + + let mock = server + .mock("GET", "/api/v2/service-naming-rules") + .with_status(200) + .with_header("content-type", "application/json") + .with_body(r#"{"data": []}"#) + .create_async() + .await; + + let result = super::service_remapping_list(&cfg).await; + assert!( + result.is_ok(), + "service_remapping_list failed: {:?}", + result.err() + ); + mock.assert_async().await; + cleanup_env(); + } + + #[tokio::test] + async fn test_service_remapping_list_api_error() { + let _lock = lock_env().await; + let mut server = mockito::Server::new_async().await; + let cfg = test_config(&server.url()); + + server + .mock("GET", "/api/v2/service-naming-rules") + .with_status(403) + .with_header("content-type", "application/json") + .with_body(r#"{"errors": ["Forbidden"]}"#) + .create_async() + .await; + + let result = super::service_remapping_list(&cfg).await; + assert!(result.is_err(), "expected error on 403"); + cleanup_env(); + } + + #[tokio::test] + async fn test_service_remapping_create() { + let _lock = lock_env().await; + let mut server = mockito::Server::new_async().await; + let cfg = test_config(&server.url()); + + let mock = server + .mock("POST", "/api/v2/service-naming-rules") + .with_status(201) + .with_header("content-type", "application/json") + .with_body(r#"{"data": {"id": "abc123"}}"#) + .create_async() + .await; + + let result = super::service_remapping_create( + &cfg, + "my-rule".into(), + "service:my-svc".into(), + 0, + "new-name".into(), + ) + .await; + assert!( + result.is_ok(), + "service_remapping_create failed: {:?}", + result.err() + ); + mock.assert_async().await; + cleanup_env(); + } + + #[tokio::test] + async fn test_service_remapping_create_api_error() { + let _lock = lock_env().await; + let mut server = mockito::Server::new_async().await; + let cfg = test_config(&server.url()); + + server + .mock("POST", "/api/v2/service-naming-rules") + .with_status(422) + .with_header("content-type", "application/json") + .with_body(r#"{"errors": ["Invalid rule_type"]}"#) + .create_async() + .await; + + let result = super::service_remapping_create( + &cfg, + "my-rule".into(), + "service:my-svc".into(), + 99, + "new-name".into(), + ) + .await; + assert!(result.is_err(), "expected error on 422"); + cleanup_env(); + } + + #[tokio::test] + async fn test_service_remapping_get() { + let _lock = lock_env().await; + let mut server = mockito::Server::new_async().await; + let cfg = test_config(&server.url()); + + let mock = server + .mock("GET", "/api/v2/service-naming-rules/abc123") + .with_status(200) + .with_header("content-type", "application/json") + .with_body(r#"{"data": {"id": "abc123"}}"#) + .create_async() + .await; + + let result = super::service_remapping_get(&cfg, "abc123".into()).await; + assert!( + result.is_ok(), + "service_remapping_get failed: {:?}", + result.err() + ); + mock.assert_async().await; + cleanup_env(); + } + + #[tokio::test] + async fn test_service_remapping_get_not_found() { + let _lock = lock_env().await; + let mut server = mockito::Server::new_async().await; + let cfg = test_config(&server.url()); + + server + .mock("GET", "/api/v2/service-naming-rules/missing") + .with_status(404) + .with_header("content-type", "application/json") + .with_body(r#"{"errors": ["Not found"]}"#) + .create_async() + .await; + + let result = super::service_remapping_get(&cfg, "missing".into()).await; + assert!(result.is_err(), "expected error on 404"); + cleanup_env(); + } + + #[tokio::test] + async fn test_service_remapping_update() { + let _lock = lock_env().await; + let mut server = mockito::Server::new_async().await; + let cfg = test_config(&server.url()); + + let mock = server + .mock("PUT", "/api/v2/service-naming-rules/abc123") + .with_status(200) + .with_header("content-type", "application/json") + .with_body(r#"{"data": {"id": "abc123"}}"#) + .create_async() + .await; + + let result = super::service_remapping_update( + &cfg, + "abc123".into(), + "updated-rule".into(), + "service:my-svc".into(), + 0, + "new-name".into(), + 2, + ) + .await; + assert!( + result.is_ok(), + "service_remapping_update failed: {:?}", + result.err() + ); + mock.assert_async().await; + cleanup_env(); + } + + #[tokio::test] + async fn test_service_remapping_update_conflict() { + let _lock = lock_env().await; + let mut server = mockito::Server::new_async().await; + let cfg = test_config(&server.url()); + + server + .mock("PUT", "/api/v2/service-naming-rules/abc123") + .with_status(409) + .with_header("content-type", "application/json") + .with_body(r#"{"errors": ["Conflict: stale version"]}"#) + .create_async() + .await; + + let result = super::service_remapping_update( + &cfg, + "abc123".into(), + "updated-rule".into(), + "service:my-svc".into(), + 0, + "new-name".into(), + 1, + ) + .await; + assert!(result.is_err(), "expected error on 409"); + cleanup_env(); + } + + #[tokio::test] + async fn test_service_remapping_delete() { + let _lock = lock_env().await; + let mut server = mockito::Server::new_async().await; + let cfg = test_config(&server.url()); + + let mock = server + .mock("DELETE", "/api/v2/service-naming-rules/abc123/2") + .with_status(204) + .create_async() + .await; + + let result = super::service_remapping_delete(&cfg, "abc123".into(), 2).await; + assert!( + result.is_ok(), + "service_remapping_delete failed: {:?}", + result.err() + ); + mock.assert_async().await; + cleanup_env(); + } + + #[tokio::test] + async fn test_service_remapping_delete_not_found() { + let _lock = lock_env().await; + let mut server = mockito::Server::new_async().await; + let cfg = test_config(&server.url()); + + server + .mock("DELETE", "/api/v2/service-naming-rules/missing/1") + .with_status(404) + .with_header("content-type", "application/json") + .with_body(r#"{"errors": ["Not found"]}"#) + .create_async() + .await; + + let result = super::service_remapping_delete(&cfg, "missing".into(), 1).await; + assert!(result.is_err(), "expected error on 404"); + cleanup_env(); + } + + #[tokio::test] + async fn test_service_remapping_update_204_no_content() { + let _lock = lock_env().await; + let mut server = mockito::Server::new_async().await; + let cfg = test_config(&server.url()); + + let mock = server + .mock("PUT", "/api/v2/service-naming-rules/abc123") + .with_status(204) + .create_async() + .await; + + let result = super::service_remapping_update( + &cfg, + "abc123".into(), + "updated-rule".into(), + "service:my-svc".into(), + 0, + "new-name".into(), + 2, + ) + .await; + assert!( + result.is_ok(), + "204 No Content should not be an error: {:?}", + result.err() + ); + mock.assert_async().await; + cleanup_env(); + } } diff --git a/src/main.rs b/src/main.rs index 3ab8a2b1..16f75a2e 100644 --- a/src/main.rs +++ b/src/main.rs @@ -7866,6 +7866,12 @@ enum ApmActions { #[command(subcommand)] action: ApmTroubleshootingActions, }, + /// Manage APM service remapping rules + #[command(name = "service-remapping")] + ServiceRemapping { + #[command(subcommand)] + action: ApmServiceRemappingActions, + }, /// View APM service instance configuration #[command(name = "service-config")] ServiceConfig { @@ -7987,6 +7993,70 @@ enum ApmTroubleshootingActions { }, } +#[derive(Subcommand)] +enum ApmServiceRemappingActions { + /// List all service remapping rules + List, + /// Create a service remapping rule + Create { + #[arg(long, help = "Rule name")] + name: String, + #[arg( + long, + help = "Filter query (e.g. 'service:my-svc', 'peer.service:*.shopify.com')" + )] + filter: String, + #[arg( + long, + value_name = "TYPE", + help = "Entity type: 0 = instrumented service, 1 = inferred entity (from outbound calls)" + )] + rule_type: i64, + #[arg( + long, + help = "New service name — static string or template (e.g. 'new-name', '{{service|^(.+?)\\..*$}}')" + )] + value: String, + }, + /// Get a service remapping rule by ID + Get { + #[arg(help = "Rule ID")] + id: String, + }, + /// Update a service remapping rule + Update { + #[arg(help = "Rule ID")] + id: String, + #[arg(long, help = "Rule name")] + name: String, + #[arg( + long, + help = "Filter query (e.g. 'service:my-svc', 'peer.service:*.shopify.com')" + )] + filter: String, + #[arg( + long, + value_name = "TYPE", + help = "Entity type: 0 = instrumented service, 1 = inferred entity" + )] + rule_type: i64, + #[arg( + long, + help = "New service name — static string or template (e.g. 'new-name', '{{service|^(.+?)\\..*$}}')" + )] + value: String, + #[arg(long, help = "Current rule version (from list/get output)")] + version: i64, + }, + /// Delete a service remapping rule + Delete { + #[arg(help = "Rule ID")] + id: String, + #[arg(help = "Rule version (from list output)")] + version: i64, + }, +} + #[derive(Subcommand)] enum ApmServiceConfigActions { /// Get service instance configuration. @@ -13405,6 +13475,41 @@ async fn main_inner() -> anyhow::Result<()> { commands::apm::troubleshooting_list(&cfg, hostname, timeframe).await?; } }, + ApmActions::ServiceRemapping { action } => match action { + ApmServiceRemappingActions::List => { + commands::apm::service_remapping_list(&cfg).await?; + } + ApmServiceRemappingActions::Create { + name, + filter, + rule_type, + value, + } => { + commands::apm::service_remapping_create( + &cfg, name, filter, rule_type, value, + ) + .await?; + } + ApmServiceRemappingActions::Get { id } => { + commands::apm::service_remapping_get(&cfg, id).await?; + } + ApmServiceRemappingActions::Update { + id, + name, + filter, + rule_type, + value, + version, + } => { + commands::apm::service_remapping_update( + &cfg, id, name, filter, rule_type, value, version, + ) + .await?; + } + ApmServiceRemappingActions::Delete { id, version } => { + commands::apm::service_remapping_delete(&cfg, id, version).await?; + } + }, ApmActions::ServiceConfig { action } => match action { ApmServiceConfigActions::Get { service_name, From 1be03a2f5f783f6f70196efe346b659dbfa83d0b Mon Sep 17 00:00:00 2001 From: Stephen Rosenthal Date: Tue, 12 May 2026 15:49:25 -0400 Subject: [PATCH 2/2] fix(client): route raw_put success body through parse_response_json Restored from #439 verbatim, but main has since moved every other raw_* helper onto parse_response_json (deep-stack JSON parsing via serde_stacker to bypass serde_json's 128-level recursion cap). Match that pattern so service remapping update doesn't re-introduce the pre-hardening failure mode for deeply nested PUT responses. --- src/client.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client.rs b/src/client.rs index a2e69c03..104db34c 100644 --- a/src/client.rs +++ b/src/client.rs @@ -1033,7 +1033,7 @@ pub async fn raw_put( let body = resp.text().await.unwrap_or_default(); anyhow::bail!("PUT {url} failed (HTTP {status}): {body}"); } - Ok(resp.json().await?) + parse_response_json(resp).await } /// Like `raw_post`, but returns the parsed JSON body even on non-2xx responses.