diff --git a/agents/service-catalog.md b/agents/service-catalog.md index bf0183a6..72ef76f4 100644 --- a/agents/service-catalog.md +++ b/agents/service-catalog.md @@ -11,8 +11,8 @@ Help users understand and maintain Datadog's service and software catalog. Use t - Default to `pup idp kinds` and `pup idp entities query` when a read needs connected service context: ownership, on-call, systems, code, dependencies, health, work, operations, or security. UEG can return selected service and dependency context in one bounded request. Load the `dd-idp` skill for its schema-first workflow and DSL guidance when available. - Use `pup service-catalog list|get` for the legacy typed service registry. - Use `pup idp assist` for a fast curated single-service summary, metadata gaps, and suggested next actions; use `owner` for convenient owner/on-call resolution. These trade graph fidelity for a narrower opinionated result. -- Treat `find` as a simple legacy service-name lookup. Use `deps` as a convenient one-hour UEG runtime service-to-service summary, and use `entities query` for another lookback or broader relation families, counts, pagination, and traversal. -- Use `pup software-catalog entities|kinds|relations` for Catalog inventory and explicit Catalog mutations. +- Treat `find` as a simple paginated literal service-name lookup. Explicit `kind:` and `ref:` queries remain compatibility paths; use `entities query` for non-service kinds, another lookback, broader relation families, selected fields, counts, pagination, and traversal. Use `deps` as a convenient one-hour UEG runtime service-to-service summary. +- Use `pup software-catalog entities|kinds|relations` for Catalog inventory. Only `entities` and `kinds` expose mutations; `relations` is read-only. - Use `pup idp register` to preserve the familiar file-oriented workflow across v1, v2, v2.1, v2.2, and v3 Catalog definitions. It sends raw YAML or JSON to the Catalog entity API and accepts multi-document YAML. Do not use nonexistent `pup services` or `pup catalog` commands. @@ -113,7 +113,7 @@ Use valid JSON payloads and confirm the target org and intended mutation: pup software-catalog entities upsert --file entity.json pup software-catalog entities delete pup software-catalog kinds upsert --file kind.json -pup software-catalog kinds delete +pup software-catalog kinds delete ``` Never delete as cleanup or guess an entity identifier. Read the entity first, show the exact target, and require explicit user approval. diff --git a/docs/EXAMPLES.md b/docs/EXAMPLES.md index cde28d2c..bc3d703e 100644 --- a/docs/EXAMPLES.md +++ b/docs/EXAMPLES.md @@ -1058,10 +1058,15 @@ pup idp assist payments-api ### Find Entities ```bash -# Search services by name (fuzzy match) +# Search services by a literal name substring pup idp find payments -# Use kind: prefix to search other entity types +# Bound the page and continue with the returned cursor +pup idp find payments --limit 5 +pup idp find payments --limit 5 --cursor '' + +# Existing explicit queries remain compatible; prefer `idp entities query` +# for new non-service or advanced graph workflows pup idp find "kind:team AND name:backend" ``` diff --git a/skills/dd-idp/SKILL.md b/skills/dd-idp/SKILL.md index 18056485..500ef5d4 100644 --- a/skills/dd-idp/SKILL.md +++ b/skills/dd-idp/SKILL.md @@ -16,7 +16,7 @@ Use Pup's read-only Unified Entity Graph (UEG) to discover software, ownership, - Default to `pup idp kinds` and `pup idp entities query` when the answer needs connected context across services, teams, systems, repositories, dependencies, work, operations, or security kinds. - Use `pup idp assist` when the user values a fast, curated single-service summary, metadata gaps, and suggested next actions over graph fidelity; use `owner` for convenient owner/on-call resolution. -- Treat `find` as a simple legacy service-name lookup. Use `deps` as a convenient one-hour UEG runtime service-to-service dependency summary, and `entities query` for another lookback or broader relation families, counts, pagination, and traversal. +- Treat `find` as a simple paginated literal service-name lookup. Explicit `kind:` and `ref:` queries remain compatibility paths; use `entities query` for non-service kinds, another lookback, broader relation families, selected fields, counts, pagination, and traversal. Use `deps` as a convenient one-hour UEG runtime service-to-service dependency summary. - Use product commands such as `pup incidents`, `pup slos`, `pup monitors`, `pup logs`, `pup traces`, or `pup security` when the user needs deeper or current telemetry. - Use `pup service-catalog` for the legacy typed service registry and `pup software-catalog` for Catalog entity/kind reads and writes. Do not use graph queries for mutations. diff --git a/src/commands/idp/entity_query.rs b/src/commands/idp/entity_query.rs index b8b0826b..35521e0d 100644 --- a/src/commands/idp/entity_query.rs +++ b/src/commands/idp/entity_query.rs @@ -14,8 +14,8 @@ use crate::config::Config; use crate::formatter::{self, Metadata}; use crate::raw_client; -const ENTITIES_PATH: &str = "/api/v2/idp/entity_graph/entities"; -const MAX_PAGE_LIMIT: usize = 100; +pub(super) const ENTITIES_PATH: &str = "/api/v2/idp/entity_graph/entities"; +pub(super) const MAX_PAGE_LIMIT: usize = 100; const MAX_RELATION_LIMIT: usize = 100; #[derive(Debug, Clone)] @@ -112,21 +112,7 @@ fn normalize_options(options: EntityQueryOptions) -> Result or ref:\"ref::\"; quoted kind filters like kind:\"service\" are invalid" - ) - })?; - if has_semantic_top_level_or(&query) { - bail!( - "top-level OR is invalid because the entity graph cannot determine one result kind; keep kind: or ref:\"ref::\" in the shared scope, for example kind:service AND (owner:idp OR team:idp)" - ); - } - if free_text_pattern().is_match(&query) { - bail!( - "free_text is not an entity field; use a real field such as name:*text*, or set --free-text-match to partial or fuzzy" - ); - } + let kind = validate_query_scope(&query)?; validate_limit("limit", options.limit, MAX_PAGE_LIMIT)?; validate_limit("relation-limit", options.relation_limit, MAX_RELATION_LIMIT)?; @@ -165,7 +151,26 @@ fn normalize_options(options: EntityQueryOptions) -> Result Result<()> { +pub(super) fn validate_query_scope(query: &str) -> Result { + let kind = infer_kind(query).ok_or_else(|| { + anyhow::anyhow!( + "query must include kind: or ref:\"ref::\"; quoted kind filters like kind:\"service\" are invalid" + ) + })?; + if has_semantic_top_level_or(query) { + bail!( + "top-level OR is invalid because the entity graph cannot determine one result kind; keep kind: or ref:\"ref::\" in the shared scope, for example kind:service AND (owner:idp OR team:idp)" + ); + } + if free_text_pattern().is_match(query) { + bail!( + "free_text is not an entity field; use a real field such as name:*text*, or set --free-text-match to partial or fuzzy" + ); + } + Ok(kind) +} + +pub(super) fn validate_limit(name: &str, value: usize, maximum: usize) -> Result<()> { if value == 0 || value > maximum { bail!("--{name} must be between 1 and {maximum}, got {value}"); } @@ -488,7 +493,7 @@ pub(super) fn parse_relationship_data(value: &Value) -> Vec .unwrap_or_default() } -fn raw_response_metadata(raw: &Value) -> (Option, bool, Option) { +pub(super) fn raw_response_metadata(raw: &Value) -> (Option, bool, Option) { let count = raw.get("data").and_then(Value::as_array).map(Vec::len); let cursor = raw .pointer("/meta/page/next_cursor") diff --git a/src/commands/idp/mod.rs b/src/commands/idp/mod.rs index 4eaa48bd..a83aed59 100644 --- a/src/commands/idp/mod.rs +++ b/src/commands/idp/mod.rs @@ -1,4 +1,4 @@ -use anyhow::{Context, Result}; +use anyhow::{bail, Context, Result}; use serde::Serialize; use crate::config::Config; @@ -516,25 +516,78 @@ pub async fn assist(cfg: &Config, entity: &str) -> Result<()> { ) } +fn find_query(query: &str) -> Result { + let query = query.trim(); + if query.is_empty() { + bail!("search query cannot be empty"); + } + if query.contains("kind:") || query.contains("ref:") { + entity_query::validate_query_scope(query)?; + return Ok(query.to_string()); + } + Ok(format!( + "kind:service AND name:*{}*", + escape_ueg_glob_literal(query) + )) +} + +fn escape_ueg_glob_literal(value: &str) -> String { + let mut escaped = String::with_capacity(value.len()); + for character in value.chars() { + if character.is_whitespace() + || matches!( + character, + '\\' | '+' + | '-' + | '=' + | '&' + | '|' + | '>' + | '<' + | '!' + | '(' + | ')' + | '{' + | '}' + | '[' + | ']' + | '^' + | '"' + | '~' + | '*' + | '?' + | ':' + ) + { + escaped.push('\\'); + } + escaped.push(character); + } + escaped +} + /// Find entities matching a query. -pub async fn find(cfg: &Config, query: &str) -> Result<()> { - // The UEG API requires kind in the query. If the user didn't specify one, default to service. - let full_query = if query.contains("kind:") { - query.to_string() - } else { - format!("kind:service AND name:*{query}*") - }; - let encoded = util_ext::percent_encode(&full_query); - let path = format!("/api/v2/idp/entity_graph/entities?query={encoded}&page%5Blimit%5D=10"); - let data = raw_client::raw_get(cfg, &path, &[]).await?; +pub async fn find(cfg: &Config, query: &str, limit: usize, cursor: Option<&str>) -> Result<()> { + entity_query::validate_limit("limit", limit, entity_query::MAX_PAGE_LIMIT)?; + let full_query = find_query(query)?; + let limit = limit.to_string(); + let mut params = vec![ + ("query", full_query.as_str()), + ("page[limit]", limit.as_str()), + ]; + if let Some(cursor) = cursor.filter(|cursor| !cursor.trim().is_empty()) { + params.push(("page[cursor]", cursor)); + } + let data = raw_client::raw_get(cfg, entity_query::ENTITIES_PATH, ¶ms).await?; + let (count, truncated, next_action) = entity_query::raw_response_metadata(&data); let meta = formatter::Metadata { - count: data.get("data").and_then(|d| d.as_array()).map(|a| a.len()), - truncated: false, + count, + truncated, command: Some(format!("idp find {query}")), - next_action: Some( - "Use `pup idp assist ` for full context on a specific entity".into(), - ), + next_action: next_action.or_else(|| { + Some("Use `pup idp assist ` for full context on a specific entity".into()) + }), }; formatter::format_and_print( @@ -1059,4 +1112,114 @@ mod tests { mock.assert_async().await; crate::test_support::cleanup_env(); } + + #[test] + fn test_find_query_defaults_to_service_name_search() { + assert_eq!( + find_query(" catalog ").unwrap(), + "kind:service AND name:*catalog*" + ); + } + + #[test] + fn test_find_query_escapes_literal_text_from_ueg_syntax() { + assert_eq!( + find_query("catalog OR api").unwrap(), + r"kind:service AND name:*catalog\ OR\ api*" + ); + assert_eq!( + find_query(r#"payments:(api)*\v2"#).unwrap(), + r#"kind:service AND name:*payments\:\(api\)\*\\v2*"# + ); + } + + #[test] + fn test_find_query_preserves_explicit_kind_or_ref() { + assert_eq!( + find_query("kind:team AND name:*platform*").unwrap(), + "kind:team AND name:*platform*" + ); + assert_eq!( + find_query(r#"ref:"ref:service:catalog-http""#).unwrap(), + r#"ref:"ref:service:catalog-http""# + ); + } + + #[test] + fn test_find_query_rejects_empty_or_invalid_scope() { + assert!(find_query(" ").unwrap_err().to_string().contains("empty")); + assert!(find_query(r#"kind:"service""#) + .unwrap_err() + .to_string() + .contains("query must include kind:")); + assert!(find_query("kind:service OR kind:team") + .unwrap_err() + .to_string() + .contains("top-level OR is invalid")); + assert!(find_query("kind:service AND free_text:catalog") + .unwrap_err() + .to_string() + .contains("free_text is not an entity field")); + } + + #[tokio::test] + async fn test_find_sends_limit_and_cursor_to_ueg() { + let _guard = crate::test_support::lock_env().await; + let mut server = mockito::Server::new_async().await; + let mock = server + .mock("GET", entity_query::ENTITIES_PATH) + .match_query(Matcher::AllOf(vec![ + Matcher::UrlEncoded("query".into(), "kind:service AND name:*catalog*".into()), + Matcher::UrlEncoded("page[limit]".into(), "5".into()), + Matcher::UrlEncoded("page[cursor]".into(), "next-page".into()), + ])) + .with_status(200) + .with_header("content-type", "application/json") + .with_body(r#"{"data":[],"meta":{"page":{"next_cursor":"another-page"}}}"#) + .create_async() + .await; + let cfg = crate::test_support::test_config(&server.url()); + + find(&cfg, "catalog", 5, Some("next-page")).await.unwrap(); + + mock.assert_async().await; + crate::test_support::cleanup_env(); + } + + #[tokio::test] + async fn test_find_escapes_literal_text_before_request() { + let _guard = crate::test_support::lock_env().await; + let mut server = mockito::Server::new_async().await; + let mock = server + .mock("GET", entity_query::ENTITIES_PATH) + .match_query(Matcher::AllOf(vec![ + Matcher::UrlEncoded( + "query".into(), + r"kind:service AND name:*catalog\ OR\ api*".into(), + ), + Matcher::UrlEncoded("page[limit]".into(), "10".into()), + ])) + .with_status(200) + .with_header("content-type", "application/json") + .with_body(r#"{"data":[]}"#) + .create_async() + .await; + let cfg = crate::test_support::test_config(&server.url()); + + find(&cfg, "catalog OR api", 10, None).await.unwrap(); + + mock.assert_async().await; + crate::test_support::cleanup_env(); + } + + #[tokio::test] + async fn test_find_rejects_invalid_limit_before_request() { + let cfg = crate::test_support::test_config("http://unused.local"); + + let error = find(&cfg, "catalog", 0, None).await.unwrap_err(); + + assert!(error + .to_string() + .contains("--limit must be between 1 and 100")); + } } diff --git a/src/main.rs b/src/main.rs index d1b42e1c..a036b17c 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1547,7 +1547,7 @@ enum Commands { /// • Discover entity kinds and inspect their live query schemas /// • Query entities and traverse declared relationships /// • Get an opinionated service summary (assist) - /// • Run a quick legacy service lookup (find) + /// • Run a quick UEG service lookup (find) /// • Resolve service ownership and on-call (owner) /// • Show runtime service dependencies from UEG (deps) /// • Register Catalog entities from YAML or JSON (register) @@ -5347,24 +5347,31 @@ enum IdpActions { /// Entity name (e.g. "catalog-http", "payment-service") entity: String, }, - /// Run a quick legacy service lookup by name or query + /// Run a quick UEG service-name lookup /// - /// Simple text defaults to a bounded wildcard service-name lookup. - /// Use `idp entities query` for arbitrary kinds, selected fields and - /// relations, explicit pagination, or schema-driven queries. + /// Simple text is treated literally and defaults to a bounded wildcard + /// service-name lookup. Explicit `kind:` and concrete `ref:` queries remain + /// supported for compatibility. Prefer `idp entities query` for non-service + /// kinds, selected fields, relations, or schema-driven queries. /// /// QUERY SYNTAX: - /// Simple text searches by name. Prefix with kind: to filter by type. - /// Use AND to combine filters. + /// Simple text searches service names. Existing explicit UEG queries are + /// passed through after scope validation. /// /// EXAMPLES: /// pup idp find "catalog" - /// pup idp find "kind:service AND name:payment" - /// pup idp find "kind:service AND owner:platform" + /// pup idp find "catalog" --limit 5 + /// pup idp find "catalog" --limit 5 --cursor #[command(verbatim_doc_comment)] Find { - /// Search query (e.g. "catalog", "kind:service AND name:payment") + /// Literal service-name text or a compatibility UEG query query: String, + /// Maximum entities in this page (1-100) + #[arg(long, default_value_t = 10)] + limit: usize, + /// Cursor returned by the previous page + #[arg(long)] + cursor: Option, }, /// Resolve ownership, team details, and on-call context /// @@ -14555,9 +14562,13 @@ async fn main_inner() -> anyhow::Result<()> { cfg.validate_auth()?; commands::idp::assist(&cfg, &entity).await?; } - IdpActions::Find { query } => { + IdpActions::Find { + query, + limit, + cursor, + } => { cfg.validate_auth()?; - commands::idp::find(&cfg, &query).await?; + commands::idp::find(&cfg, &query, limit, cursor.as_deref()).await?; } IdpActions::Owner { entity } => { cfg.validate_auth()?; diff --git a/src/test_commands.rs b/src/test_commands.rs index 71414a28..683dfcce 100644 --- a/src/test_commands.rs +++ b/src/test_commands.rs @@ -868,6 +868,33 @@ fn test_idp_entity_graph_commands_parse() { } _ => panic!("expected IdpKindsActions::List"), } + + let find = crate::Cli::try_parse_from([ + "pup", + "idp", + "find", + "catalog", + "--limit", + "5", + "--cursor", + "next-page", + ]) + .expect("IDP find should parse pagination options"); + match find.command { + crate::Commands::Idp { + action: + crate::IdpActions::Find { + query, + limit, + cursor, + }, + } => { + assert_eq!(query, "catalog"); + assert_eq!(limit, 5); + assert_eq!(cursor.as_deref(), Some("next-page")); + } + _ => panic!("expected IdpActions::Find"), + } } #[test] @@ -888,7 +915,14 @@ fn test_idp_convenience_help_routes_connected_context_to_entity_graph() { .clone() .render_long_help() .to_string(); + let find_help = idp + .find_subcommand("find") + .expect("idp find command should exist") + .clone() + .render_long_help() + .to_string(); let deps_help = deps_help.split_whitespace().collect::>().join(" "); + let find_help = find_help.split_whitespace().collect::>().join(" "); assert!(assist_help.contains("idp entities query")); assert!(!assist_help.contains("flagship")); @@ -896,6 +930,9 @@ fn test_idp_convenience_help_routes_connected_context_to_entity_graph() { assert!(deps_help.contains("past hour")); assert!(deps_help.contains("broader unified graph")); assert!(!deps_help.contains("env=prod")); + assert!(find_help.contains("service-name lookup")); + assert!(find_help.contains("supported for compatibility")); + assert!(find_help.contains("idp entities query")); } #[test]