From 103ebb2393b1c1049b78068c292bef91cb58fce0 Mon Sep 17 00:00:00 2001 From: Ben Vincent Date: Sun, 5 Jul 2026 17:16:07 +1000 Subject: [PATCH] Support comma-separated -F for querying multiple facts `node-lookup -jF ipaddress,enc_role` returned `{}` because it queried a single fact literally named "ipaddress,enc_role". Requesting several facts per host is a natural need (e.g. pairing an address with its role). - Split -F on commas (splitFactNames) and match any of them via an "or" over ["=","name",] clauses (nameFilter); a single name keeps the plain "=" form. - Key JSON output by each result's real fact name so all requested facts appear under the host (previously keyed by the raw -F string). - Update the -F flag help and add unit tests (split, single vs multi query shape, multi-fact JSON). --- AGENTS.md | 4 ++- main.go | 46 +++++++++++++++++++++++++++------ main_test.go | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 113 insertions(+), 9 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 7f73dfa..8eada4b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -59,6 +59,7 @@ installed. To load ad-hoc in the current shell, e.g. zsh: ./node-lookup -R # show all nodes with role fact ./node-lookup -n # lookup a specific node ./node-lookup -F # filter by fact name +./node-lookup -jF ipaddress,enc_role # several facts at once (comma-separated) ./node-lookup -R -m # exact value match (-m) ./node-lookup -R -pm # partial/regex match (-p -m combined) ./node-lookup -R -im # inverse exact match (-i -m combined) @@ -111,6 +112,7 @@ Show the active configuration (after all overrides applied): - **`loadConfig()`**: reads config file → applies env vars → returns `config` struct. Called once at startup in `main()`. - **`buildQuery()`**: returns a PuppetDB PQL-compatible JSON array string. Uses `roleFact` from config (not hardcoded). Match modifiers: `-p` (partial/regex, uses `~` op), `-i` (inverse, wraps with `not`), composable. +- **Multiple facts**: `-F` accepts a comma-separated list (`ipaddress,enc_role`). `splitFactNames()`/`nameFilter()` turn several names into an `or` over `["=","name",]` clauses; JSON output keys each value by the fact's real name so all requested facts appear per host. - **Match value / `matchValue()`**: the value to match comes from `-m/--match` or, if that is empty, an optional positional argument. The positional fallback exists because pflag does not attach a space-separated value to a string flag grouped with a bool flag, so in `-pm k8s` the `k8s` arrives as a positional. `-m` still wins when both are given. - **`queryPuppetDB(url, query)`**: takes the URL as a parameter — never reads globals. - **`processResults()`**: iterates facts, returns sorted `"certname value"` strings. JSON string values are unquoted; other JSON types rendered as compact JSON. @@ -138,5 +140,5 @@ mode (default, `-1`, `-2`, `-C`, `-j`, `-A`, `-a`). PuppetDB is stubbed with - `-1`, `-2`, `-C`, and `-A` all require `-R` or `-F`; the tool exits with an error otherwise. - `-C` (count) with stdin reads all lines as pre-fetched `"node value"` output for counting — it does **not** query PuppetDB per line. -- JSON output (`-j`) builds `{ hostname: { factname: value } }` where the fact key is the `-F` value, the `role_fact` config value (if `-R`), or `"value"` as fallback. +- JSON output (`-j`) builds `{ hostname: { factname: value } }` keyed by each result's actual fact name (so `-F ipaddress,enc_role` yields both per host); it falls back to the `-F` value, the `role_fact` config value (if `-R`), or `"value"` only when a result carries no name. - `config init` fails if the config file already exists (will not overwrite). diff --git a/main.go b/main.go index b12419e..e1cdd98 100644 --- a/main.go +++ b/main.go @@ -110,6 +110,31 @@ type fact struct { Value json.RawMessage `json:"value"` } +// splitFactNames splits a comma-separated -F value into trimmed, non-empty +// names, so `-F ipaddress,enc_role` queries both facts. +func splitFactNames(factName string) []string { + var names []string + for _, n := range strings.Split(factName, ",") { + if n = strings.TrimSpace(n); n != "" { + names = append(names, n) + } + } + return names +} + +// nameFilter returns a PQL filter matching any of the given fact names: a plain +// equality for one name, an "or" over per-name equalities for several. +func nameFilter(names []string) []interface{} { + if len(names) == 1 { + return []interface{}{"=", "name", names[0]} + } + or := []interface{}{"or"} + for _, n := range names { + or = append(or, []interface{}{"=", "name", n}) + } + return or +} + func buildQuery(node, factName, match, roleFact string, showRole, partial, inverse bool) string { type filter = []interface{} var filters []filter @@ -117,8 +142,8 @@ func buildQuery(node, factName, match, roleFact string, showRole, partial, inver if node != "" { filters = append(filters, filter{"=", "certname", node}) } - if factName != "" { - filters = append(filters, filter{"=", "name", factName}) + if names := splitFactNames(factName); len(names) > 0 { + filters = append(filters, nameFilter(names)) } else if showRole { filters = append(filters, filter{"=", "name", roleFact}) } @@ -326,12 +351,17 @@ func run(cfg config, nodeName, factName, match string, showRole, partial, invers if _, ok := hostFactMap[f.Certname]; !ok { hostFactMap[f.Certname] = map[string]interface{}{} } - key := factName + // Key by the fact's actual name so multiple -F facts (e.g. + // ipaddress,enc_role) each appear under the host. Fall back to the + // computed key only if the result carries no name. + key := f.Name if key == "" { - if showRole { - key = cfg.RoleFact - } else { - key = "value" + if key = factName; key == "" { + if showRole { + key = cfg.RoleFact + } else { + key = "value" + } } } hostFactMap[f.Certname][key] = valueAny(f.Value) @@ -426,7 +456,7 @@ func main() { f := rootCmd.Flags() f.StringVarP(&nodeName, "node", "n", "", "Node name") - f.StringVarP(&factName, "fact", "F", "", "Fact name") + f.StringVarP(&factName, "fact", "F", "", "Fact name (comma-separated for several, e.g. -F ipaddress,enc_role)") f.BoolVarP(&showRole, "role", "R", false, "Show role fact ("+defaultRoleFact+" by default)") f.StringVarP(&match, "match", "m", "", "Value to match (use with -p and/or -i)") f.BoolVarP(&partial, "partial", "p", false, "Partial/regex match modifier (combine with -m)") diff --git a/main_test.go b/main_test.go index 2cc8814..125b643 100644 --- a/main_test.go +++ b/main_test.go @@ -142,6 +142,78 @@ func TestBuildQuery_ValidJSON(t *testing.T) { } } +// ---- multi-fact -F (comma-separated) ---------------------------------------- + +func TestSplitFactNames(t *testing.T) { + cases := map[string][]string{ + "ipaddress": {"ipaddress"}, + "ipaddress,enc_role": {"ipaddress", "enc_role"}, + "ipaddress, enc_role ": {"ipaddress", "enc_role"}, // trims spaces + "a,,b,": {"a", "b"}, // drops empties + "": nil, + } + for in, want := range cases { + got := splitFactNames(in) + if len(got) != len(want) { + t.Fatalf("splitFactNames(%q) = %v, want %v", in, got, want) + } + for i := range want { + if got[i] != want[i] { + t.Fatalf("splitFactNames(%q) = %v, want %v", in, got, want) + } + } + } +} + +func TestBuildQuery_SingleFact_NoOr(t *testing.T) { + q := buildQuery("", "ipaddress", "", "enc_role", false, false, false) + if strings.Contains(q, `"or"`) { + t.Fatalf("single fact should not use 'or': %s", q) + } + if !strings.Contains(q, "ipaddress") { + t.Fatalf("expected fact name in query: %s", q) + } +} + +func TestBuildQuery_MultiFact_UsesOr(t *testing.T) { + q := buildQuery("host1", "ipaddress,enc_role", "", "enc_role", false, false, false) + if !strings.Contains(q, `"or"`) { + t.Fatalf("expected 'or' over fact names: %s", q) + } + if !strings.Contains(q, "ipaddress") || !strings.Contains(q, "enc_role") { + t.Fatalf("expected both fact names: %s", q) + } + // Must remain valid PQL JSON, combined under "and" with the certname filter. + var v []interface{} + if err := json.Unmarshal([]byte(q), &v); err != nil { + t.Fatalf("query is not valid JSON: %v (%s)", err, q) + } + if v[0] != "and" { + t.Fatalf("expected top-level 'and', got %v", v[0]) + } +} + +func TestRun_JSON_MultipleFacts(t *testing.T) { + // Two facts returned for one host must both appear, keyed by their real name. + facts := []fact{ + {Certname: "hosta", Name: "ipaddress", Value: rawJSON("198.18.0.1")}, + {Certname: "hosta", Name: "enc_role", Value: rawJSON("roles::dns")}, + } + out := runToString(t, facts, func(a *runArgs) { + a.showRole = false + a.factName = "ipaddress,enc_role" + a.jsonMode = true + }) + var parsed map[string]map[string]interface{} + if err := json.Unmarshal([]byte(out), &parsed); err != nil { + t.Fatalf("output is not valid JSON: %v (%s)", err, out) + } + host := parsed["hosta"] + if host["ipaddress"] != "198.18.0.1" || host["enc_role"] != "roles::dns" { + t.Fatalf("expected both facts under host, got: %v", host) + } +} + // ---- matchValue (positional fallback for `-pm value`) ----------------------- func TestMatchValue_FlagWins(t *testing.T) {