Accept match value positionally so -pm <value> works
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/pre-commit Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful

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 <value> and
  a bare `-p <value>` 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.
This commit is contained in:
2026-07-05 17:02:25 +10:00
parent e070357d3f
commit 8696097a6a
3 changed files with 52 additions and 6 deletions
+6 -4
View File
@@ -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 -R # show all nodes with role fact
./node-lookup -n <hostname> # lookup a specific node ./node-lookup -n <hostname> # lookup a specific node
./node-lookup -F <fact_name> # filter by fact name ./node-lookup -F <fact_name> # filter by fact name
./node-lookup -m <value> # exact value match (-m) ./node-lookup -R -m <value> # exact value match (-m)
./node-lookup -pm <value> # partial/regex match (-p -m combined) ./node-lookup -R -pm <value> # partial/regex match (-p -m combined)
./node-lookup -im <value> # inverse exact match (-i -m combined) ./node-lookup -R -im <value> # inverse exact match (-i -m combined)
./node-lookup -ipm <value> # inverse partial match (-i -p -m combined) ./node-lookup -R -ipm <value> # inverse partial match (-i -p -m combined)
./node-lookup -R -p <value> # value may also be given positionally
./node-lookup -R -1 # node names only ./node-lookup -R -1 # node names only
./node-lookup -R -2 # values only ./node-lookup -R -2 # values only
./node-lookup -R -C # count occurrences ./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()`. - **`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. - **`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. - **`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. - **`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). - **Output modes**: JSON (`-j`), count (`-C`), Ansible YAML (`-A`), node-only (`-1`), value-only (`-2`), default (node + value).
+23 -2
View File
@@ -236,6 +236,21 @@ func stdinReader(f *os.File) (*bufio.Reader, bool) {
return r, true 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) { func allFactsForNode(puppetDBURL, node string) ([]fact, error) {
query, _ := json.Marshal([]interface{}{"=", "certname", node}) query, _ := json.Marshal([]interface{}{"=", "certname", node})
return queryPuppetDB(puppetDBURL, string(query)) return queryPuppetDB(puppetDBURL, string(query))
@@ -389,8 +404,14 @@ func main() {
) )
rootCmd := &cobra.Command{ rootCmd := &cobra.Command{
Use: appName, Use: appName + " [value]",
Short: "Query PuppetDB for nodes.", 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 { PersistentPreRunE: func(cmd *cobra.Command, args []string) error {
if cmd.Flags().Changed("url") { if cmd.Flags().Changed("url") {
cfg.PuppetDBURL = puppetDBURL cfg.PuppetDBURL = puppetDBURL
@@ -398,7 +419,7 @@ func main() {
return nil return nil
}, },
RunE: func(cmd *cobra.Command, args []string) error { 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, SilenceUsage: true,
} }
+23
View File
@@ -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 ------------------------------------------------- // ---- valueString / valueAny -------------------------------------------------
func TestValueString_String(t *testing.T) { func TestValueString_String(t *testing.T) {