From 8696097a6a18a5e7fef6e0777fce19c517f299f7 Mon Sep 17 00:00:00 2001 From: Ben Vincent Date: Sun, 5 Jul 2026 17:02:25 +1000 Subject: [PATCH] Accept match value positionally so -pm works pflag does not attach a space-separated value to a string flag that is grouped with a bool flag, so `node-lookup -R -pm k8s` parsed -p and left `k8s` as a stray positional, failing with "unknown command k8s". Only `-pm=k8s` or the un-grouped `-p -m k8s` worked, which is surprising. - Allow one positional argument (cobra.MaximumNArgs(1)) and fall back to it for the match value when -m is empty (matchValue()), so -pm/-im/-ipm and a bare `-p ` all work. -m still wins when both are given. - Add matchValue unit tests. - Document the positional value and the pflag grouping quirk in AGENTS.md. --- AGENTS.md | 10 ++++++---- main.go | 25 +++++++++++++++++++++++-- main_test.go | 23 +++++++++++++++++++++++ 3 files changed, 52 insertions(+), 6 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index cf90293..7f73dfa 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -59,10 +59,11 @@ 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 -m # exact value match (-m) -./node-lookup -pm # partial/regex match (-p -m combined) -./node-lookup -im # inverse exact match (-i -m combined) -./node-lookup -ipm # inverse partial match (-i -p -m combined) +./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) +./node-lookup -R -ipm # inverse partial match (-i -p -m combined) +./node-lookup -R -p # value may also be given positionally ./node-lookup -R -1 # node names only ./node-lookup -R -2 # values only ./node-lookup -R -C # count occurrences @@ -110,6 +111,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. +- **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. - **Output modes**: JSON (`-j`), count (`-C`), Ansible YAML (`-A`), node-only (`-1`), value-only (`-2`), default (node + value). diff --git a/main.go b/main.go index 80ca754..b12419e 100644 --- a/main.go +++ b/main.go @@ -236,6 +236,21 @@ func stdinReader(f *os.File) (*bufio.Reader, bool) { return r, true } +// matchValue resolves the value to match against. The -m/--match flag wins; if +// it is empty, the (optional) positional argument is used instead. The +// positional fallback exists so combined shorthands like `-pm k8s` work — pflag +// leaves the space-separated `k8s` as a positional rather than attaching it to +// the grouped -m flag. +func matchValue(flagMatch string, args []string) string { + if flagMatch != "" { + return flagMatch + } + if len(args) > 0 { + return args[0] + } + return "" +} + func allFactsForNode(puppetDBURL, node string) ([]fact, error) { query, _ := json.Marshal([]interface{}{"=", "certname", node}) return queryPuppetDB(puppetDBURL, string(query)) @@ -389,8 +404,14 @@ func main() { ) rootCmd := &cobra.Command{ - Use: appName, + Use: appName + " [value]", Short: "Query PuppetDB for nodes.", + // Accept an optional positional match value in addition to -m. This makes + // combined shorthands like `-pm k8s` work: pflag does not attach a + // space-separated value to a string flag grouped with a bool flag (only + // `-pm=k8s` or `-p -m k8s` do), so `k8s` arrives here as a positional + // argument instead. Falling back to it keeps the ergonomic form working. + Args: cobra.MaximumNArgs(1), PersistentPreRunE: func(cmd *cobra.Command, args []string) error { if cmd.Flags().Changed("url") { cfg.PuppetDBURL = puppetDBURL @@ -398,7 +419,7 @@ func main() { return nil }, RunE: func(cmd *cobra.Command, args []string) error { - return run(cfg, nodeName, factName, match, showRole, partial, inverse, nodeOnly, valueOnly, count, ansible, jsonMode, allFacts) + return run(cfg, nodeName, factName, matchValue(match, args), showRole, partial, inverse, nodeOnly, valueOnly, count, ansible, jsonMode, allFacts) }, SilenceUsage: true, } diff --git a/main_test.go b/main_test.go index a691d9b..2cc8814 100644 --- a/main_test.go +++ b/main_test.go @@ -142,6 +142,29 @@ func TestBuildQuery_ValidJSON(t *testing.T) { } } +// ---- matchValue (positional fallback for `-pm value`) ----------------------- + +func TestMatchValue_FlagWins(t *testing.T) { + // An explicit -m value takes precedence over any positional arg. + if got := matchValue("flagval", []string{"posval"}); got != "flagval" { + t.Fatalf("expected flag value to win, got %q", got) + } +} + +func TestMatchValue_PositionalFallback(t *testing.T) { + // This is the `-pm k8s` case: pflag leaves k8s as a positional because the + // grouped -m flag does not attach the space-separated value. + if got := matchValue("", []string{"k8s"}); got != "k8s" { + t.Fatalf("expected positional fallback, got %q", got) + } +} + +func TestMatchValue_NoneGiven(t *testing.T) { + if got := matchValue("", nil); got != "" { + t.Fatalf("expected empty, got %q", got) + } +} + // ---- valueString / valueAny ------------------------------------------------- func TestValueString_String(t *testing.T) {