node-lookup: auto-qualify short node names to .main.unkin.net (#19)
ci/woodpecker/tag/release Pipeline was successful
ci/woodpecker/tag/release Pipeline was successful
Short `-n` node names silently returned nothing: `node-lookup -R -n ausyd1nxvm2120` found nothing while `-n ausyd1nxvm2120.main.unkin.net` worked, because the PuppetDB `certname` filter needs a FQDN. This auto-qualifies a dotless name before the lookup. ## Changes - Add `qualifyNode()` pure helper: a dotless name gets `.<domain>` appended; a name already containing a dot (any domain, incl. `*.k8s.syd1.au.unkin.net`) is left unchanged; a single trailing dot is stripped first; empty input is preserved (same "no node" behavior as today). - Apply normalization to the `-n` value and to stdin-sourced node names in `run()`, so both entry points behave consistently. - Make the domain configurable: config key `domain`, `NODE_LOOKUP_DOMAIN` env var, and `--domain` flag, all defaulting to `main.unkin.net`. - Surface `domain` in `config show` / `config init` output and document the new env var/flag/behavior in AGENTS.md. - Add table-driven `qualifyNode` tests (short name appended, FQDN unchanged, multi-label other-domain FQDN unchanged, trailing-dot handling, empty input, custom domain) and a `NODE_LOOKUP_DOMAIN` env-override test. Companion tools `pburl`/`pblastreport` take already-qualified hostnames (typically piped from `node-lookup`) via `puppet.ReadHosts` and do not share the `-n` code path, so they are intentionally left out to keep this PR atomic. ## Validation - `gofmt -l .` clean, `go vet ./...` clean - `go test -race ./...` pass - `make build` builds all three binaries Reviewed-on: #19 Co-authored-by: unkin-agent <unkin-agent@unkin.net> Co-committed-by: unkin-agent <unkin-agent@unkin.net>
This commit was merged in pull request #19.
This commit is contained in:
@@ -138,10 +138,12 @@ Show the active configuration (after all overrides applied):
|
|||||||
| `NODE_LOOKUP_URL` | `puppetdb_url` | PuppetDB facts endpoint |
|
| `NODE_LOOKUP_URL` | `puppetdb_url` | PuppetDB facts endpoint |
|
||||||
| `NODE_LOOKUP_ROLE_FACT` | `role_fact` | Fact name used by `-R` flag |
|
| `NODE_LOOKUP_ROLE_FACT` | `role_fact` | Fact name used by `-R` flag |
|
||||||
| `NODE_LOOKUP_PUPPETBOARD_URL` | `puppetboard_url` | Puppetboard base URL (pburl / pblastreport) |
|
| `NODE_LOOKUP_PUPPETBOARD_URL` | `puppetboard_url` | Puppetboard base URL (pburl / pblastreport) |
|
||||||
|
| `NODE_LOOKUP_DOMAIN` | `domain` | Domain appended to short (dotless) `-n` node names (default `main.unkin.net`) |
|
||||||
|
|
||||||
### CLI flag
|
### CLI flags
|
||||||
|
|
||||||
`--url <url>` overrides the PuppetDB URL for a single invocation (highest precedence).
|
`--url <url>` overrides the PuppetDB URL for a single invocation (highest precedence).
|
||||||
|
`--domain <domain>` overrides the auto-qualify domain for a single invocation.
|
||||||
|
|
||||||
## Code Patterns
|
## Code Patterns
|
||||||
|
|
||||||
@@ -152,6 +154,7 @@ Show the active configuration (after all overrides applied):
|
|||||||
- **`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). `-j` and `-A` share `factsByHost()`, so both attach the queried fact(s) per host — as an object under the host (`-j`) or as inventory host vars (`-A`).
|
- **Output modes**: JSON (`-j`), count (`-C`), Ansible YAML (`-A`), node-only (`-1`), value-only (`-2`), default (node + value). `-j` and `-A` share `factsByHost()`, so both attach the queried fact(s) per host — as an object under the host (`-j`) or as inventory host vars (`-A`).
|
||||||
|
- **Short node names / `qualifyNode()`**: a `-n` value (and stdin-sourced node names) with no dot is auto-qualified to `<name>.<domain>` (domain defaults to `main.unkin.net`, overridable via `--domain`/`NODE_LOOKUP_DOMAIN`), so `-n ausyd1nxvm2120` resolves the same as its FQDN. A name that already contains a dot (any domain) is left unchanged; a single trailing dot is stripped; empty input is preserved.
|
||||||
- **Stdin support**: `stdinReader()` reads node names from stdin only when it is a real pipe/redirect carrying data (and no `-n` given). Terminals, `/dev/null`, and empty/closed pipes fall through to a normal query — so running without a TTY (e.g. invoked by an agent or CI) behaves like an interactive run instead of consuming empty input.
|
- **Stdin support**: `stdinReader()` reads node names from stdin only when it is a real pipe/redirect carrying data (and no `-n` given). Terminals, `/dev/null`, and empty/closed pipes fall through to a normal query — so running without a TTY (e.g. invoked by an agent or CI) behaves like an interactive run instead of consuming empty input.
|
||||||
- **SIGPIPE handling**: `signal.Ignore(syscall.SIGPIPE)` so pipes to `head` etc. work cleanly.
|
- **SIGPIPE handling**: `signal.Ignore(syscall.SIGPIPE)` so pipes to `head` etc. work cleanly.
|
||||||
|
|
||||||
|
|||||||
@@ -22,6 +22,7 @@ const (
|
|||||||
defaultPuppetDBURL = "http://puppetdbapi.service.consul:8080/pdb/query/v4/facts"
|
defaultPuppetDBURL = "http://puppetdbapi.service.consul:8080/pdb/query/v4/facts"
|
||||||
defaultRoleFact = "enc_role"
|
defaultRoleFact = "enc_role"
|
||||||
defaultPuppetboardURL = "https://puppetboard.k8s.syd1.au.unkin.net"
|
defaultPuppetboardURL = "https://puppetboard.k8s.syd1.au.unkin.net"
|
||||||
|
defaultDomain = "main.unkin.net"
|
||||||
configFileName = "config.yaml"
|
configFileName = "config.yaml"
|
||||||
appName = "node-lookup"
|
appName = "node-lookup"
|
||||||
)
|
)
|
||||||
@@ -37,6 +38,8 @@ type config struct {
|
|||||||
// the shared config file also configures the companion tools (pburl,
|
// the shared config file also configures the companion tools (pburl,
|
||||||
// pblastreport) that read this same file.
|
// pblastreport) that read this same file.
|
||||||
PuppetboardURL string `yaml:"puppetboard_url"`
|
PuppetboardURL string `yaml:"puppetboard_url"`
|
||||||
|
// Domain is appended to a short (dotless) -n node name to form its FQDN.
|
||||||
|
Domain string `yaml:"domain"`
|
||||||
}
|
}
|
||||||
|
|
||||||
func defaultConfig() config {
|
func defaultConfig() config {
|
||||||
@@ -44,6 +47,7 @@ func defaultConfig() config {
|
|||||||
PuppetDBURL: defaultPuppetDBURL,
|
PuppetDBURL: defaultPuppetDBURL,
|
||||||
RoleFact: defaultRoleFact,
|
RoleFact: defaultRoleFact,
|
||||||
PuppetboardURL: defaultPuppetboardURL,
|
PuppetboardURL: defaultPuppetboardURL,
|
||||||
|
Domain: defaultDomain,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -87,6 +91,9 @@ func loadConfig() (config, error) {
|
|||||||
if v := os.Getenv("NODE_LOOKUP_PUPPETBOARD_URL"); v != "" {
|
if v := os.Getenv("NODE_LOOKUP_PUPPETBOARD_URL"); v != "" {
|
||||||
cfg.PuppetboardURL = v
|
cfg.PuppetboardURL = v
|
||||||
}
|
}
|
||||||
|
if v := os.Getenv("NODE_LOOKUP_DOMAIN"); v != "" {
|
||||||
|
cfg.Domain = v
|
||||||
|
}
|
||||||
|
|
||||||
return cfg, nil
|
return cfg, nil
|
||||||
}
|
}
|
||||||
@@ -105,7 +112,7 @@ func writeDefaultConfig() error {
|
|||||||
|
|
||||||
cfg := defaultConfig()
|
cfg := defaultConfig()
|
||||||
data, _ := yaml.Marshal(cfg)
|
data, _ := yaml.Marshal(cfg)
|
||||||
header := []byte("# node-lookup configuration\n# Fields can be overridden with env vars: NODE_LOOKUP_URL, NODE_LOOKUP_ROLE_FACT, NODE_LOOKUP_PUPPETBOARD_URL\n# puppetboard_url is used by the companion tools (pburl, pblastreport).\n\n")
|
header := []byte("# node-lookup configuration\n# Fields can be overridden with env vars: NODE_LOOKUP_URL, NODE_LOOKUP_ROLE_FACT, NODE_LOOKUP_PUPPETBOARD_URL, NODE_LOOKUP_DOMAIN\n# puppetboard_url is used by the companion tools (pburl, pblastreport).\n# domain is appended to short (dotless) -n node names to form their FQDN.\n\n")
|
||||||
if err := os.WriteFile(path, append(header, data...), 0o644); err != nil {
|
if err := os.WriteFile(path, append(header, data...), 0o644); err != nil {
|
||||||
return fmt.Errorf("writing config: %w", err)
|
return fmt.Errorf("writing config: %w", err)
|
||||||
}
|
}
|
||||||
@@ -144,6 +151,21 @@ func nameFilter(names []string) []interface{} {
|
|||||||
return or
|
return or
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// qualifyNode auto-qualifies a short (dotless) node name by appending
|
||||||
|
// ".<domain>", so `-n ausyd1nxvm2120` resolves the same as its FQDN. A name
|
||||||
|
// that already contains a dot is treated as already-qualified (including names
|
||||||
|
// in other domains like *.k8s.syd1.au.unkin.net) and returned unchanged. A
|
||||||
|
// single trailing dot is stripped first, so a dotless name with a trailing dot
|
||||||
|
// is still qualified. Empty input is returned unchanged to preserve the
|
||||||
|
// existing "no node given" behavior.
|
||||||
|
func qualifyNode(name, domain string) string {
|
||||||
|
name = strings.TrimSuffix(name, ".")
|
||||||
|
if name == "" || strings.Contains(name, ".") {
|
||||||
|
return name
|
||||||
|
}
|
||||||
|
return name + "." + domain
|
||||||
|
}
|
||||||
|
|
||||||
func buildQuery(node, factName, match, roleFact string, showRole, partial, inverse bool) string {
|
func buildQuery(node, factName, match, roleFact string, showRole, partial, inverse bool) string {
|
||||||
type filter = []interface{}
|
type filter = []interface{}
|
||||||
var filters []filter
|
var filters []filter
|
||||||
@@ -318,6 +340,8 @@ func allFactsForNode(puppetDBURL, node string) ([]fact, error) {
|
|||||||
func run(cfg config, nodeName, factName, match string, showRole, partial, inverse, nodeOnly, valueOnly, count, ansible, jsonMode, allFacts bool) error {
|
func run(cfg config, nodeName, factName, match string, showRole, partial, inverse, nodeOnly, valueOnly, count, ansible, jsonMode, allFacts bool) error {
|
||||||
signal.Ignore(syscall.SIGPIPE)
|
signal.Ignore(syscall.SIGPIPE)
|
||||||
|
|
||||||
|
nodeName = qualifyNode(nodeName, cfg.Domain)
|
||||||
|
|
||||||
if allFacts {
|
if allFacts {
|
||||||
if nodeName == "" {
|
if nodeName == "" {
|
||||||
return fmt.Errorf("-a requires -n")
|
return fmt.Errorf("-a requires -n")
|
||||||
@@ -365,7 +389,7 @@ func run(cfg config, nodeName, factName, match string, showRole, partial, invers
|
|||||||
if len(fields) == 0 {
|
if len(fields) == 0 {
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
if err := doQuery(fields[0]); err != nil {
|
if err := doQuery(qualifyNode(fields[0], cfg.Domain)); err != nil {
|
||||||
fmt.Fprintln(os.Stderr, "error:", err)
|
fmt.Fprintln(os.Stderr, "error:", err)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -446,6 +470,7 @@ func main() {
|
|||||||
jsonMode bool
|
jsonMode bool
|
||||||
allFacts bool
|
allFacts bool
|
||||||
puppetDBURL string
|
puppetDBURL string
|
||||||
|
domain string
|
||||||
)
|
)
|
||||||
|
|
||||||
rootCmd := &cobra.Command{
|
rootCmd := &cobra.Command{
|
||||||
@@ -461,6 +486,9 @@ func main() {
|
|||||||
if cmd.Flags().Changed("url") {
|
if cmd.Flags().Changed("url") {
|
||||||
cfg.PuppetDBURL = puppetDBURL
|
cfg.PuppetDBURL = puppetDBURL
|
||||||
}
|
}
|
||||||
|
if cmd.Flags().Changed("domain") {
|
||||||
|
cfg.Domain = domain
|
||||||
|
}
|
||||||
return nil
|
return nil
|
||||||
},
|
},
|
||||||
RunE: func(cmd *cobra.Command, args []string) error {
|
RunE: func(cmd *cobra.Command, args []string) error {
|
||||||
@@ -482,6 +510,7 @@ func main() {
|
|||||||
f.BoolVarP(&ansible, "ansible", "A", false, "Output as Ansible inventory")
|
f.BoolVarP(&ansible, "ansible", "A", false, "Output as Ansible inventory")
|
||||||
f.BoolVarP(&jsonMode, "json", "j", false, "Emit valid JSON for all output")
|
f.BoolVarP(&jsonMode, "json", "j", false, "Emit valid JSON for all output")
|
||||||
f.BoolVarP(&allFacts, "all", "a", false, "Show all facts for a node (requires -n)")
|
f.BoolVarP(&allFacts, "all", "a", false, "Show all facts for a node (requires -n)")
|
||||||
|
f.StringVar(&domain, "domain", cfg.Domain, "Domain appended to short (dotless) -n node names (overrides config and NODE_LOOKUP_DOMAIN)")
|
||||||
rootCmd.PersistentFlags().StringVar(&puppetDBURL, "url", cfg.PuppetDBURL, "PuppetDB facts URL (overrides config and NODE_LOOKUP_URL)")
|
rootCmd.PersistentFlags().StringVar(&puppetDBURL, "url", cfg.PuppetDBURL, "PuppetDB facts URL (overrides config and NODE_LOOKUP_URL)")
|
||||||
|
|
||||||
configCmd := &cobra.Command{
|
configCmd := &cobra.Command{
|
||||||
@@ -506,6 +535,7 @@ func main() {
|
|||||||
fmt.Printf("puppetdb_url : %s\n", cfg.PuppetDBURL)
|
fmt.Printf("puppetdb_url : %s\n", cfg.PuppetDBURL)
|
||||||
fmt.Printf("role_fact : %s\n", cfg.RoleFact)
|
fmt.Printf("role_fact : %s\n", cfg.RoleFact)
|
||||||
fmt.Printf("puppetboard_url: %s\n", cfg.PuppetboardURL)
|
fmt.Printf("puppetboard_url: %s\n", cfg.PuppetboardURL)
|
||||||
|
fmt.Printf("domain : %s\n", cfg.Domain)
|
||||||
return nil
|
return nil
|
||||||
},
|
},
|
||||||
SilenceUsage: true,
|
SilenceUsage: true,
|
||||||
|
|||||||
@@ -167,6 +167,35 @@ func TestSplitFactNames(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestQualifyNode(t *testing.T) {
|
||||||
|
const domain = "main.unkin.net"
|
||||||
|
cases := []struct {
|
||||||
|
name string
|
||||||
|
in string
|
||||||
|
want string
|
||||||
|
}{
|
||||||
|
{"short name appends domain", "ausyd1nxvm2120", "ausyd1nxvm2120.main.unkin.net"},
|
||||||
|
{"fqdn in default domain unchanged", "ausyd1nxvm2120.main.unkin.net", "ausyd1nxvm2120.main.unkin.net"},
|
||||||
|
{"multi-label fqdn other domain unchanged", "foo.k8s.syd1.au.unkin.net", "foo.k8s.syd1.au.unkin.net"},
|
||||||
|
{"short name with trailing dot qualified", "ausyd1nxvm2120.", "ausyd1nxvm2120.main.unkin.net"},
|
||||||
|
{"fqdn with trailing dot stripped", "foo.main.unkin.net.", "foo.main.unkin.net"},
|
||||||
|
{"empty unchanged", "", ""},
|
||||||
|
}
|
||||||
|
for _, tc := range cases {
|
||||||
|
t.Run(tc.name, func(t *testing.T) {
|
||||||
|
if got := qualifyNode(tc.in, domain); got != tc.want {
|
||||||
|
t.Fatalf("qualifyNode(%q, %q) = %q, want %q", tc.in, domain, got, tc.want)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestQualifyNode_CustomDomain(t *testing.T) {
|
||||||
|
if got := qualifyNode("host1", "example.com"); got != "host1.example.com" {
|
||||||
|
t.Fatalf("qualifyNode with custom domain = %q, want host1.example.com", got)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestBuildQuery_SingleFact_NoOr(t *testing.T) {
|
func TestBuildQuery_SingleFact_NoOr(t *testing.T) {
|
||||||
q := buildQuery("", "ipaddress", "", "enc_role", false, false, false)
|
q := buildQuery("", "ipaddress", "", "enc_role", false, false, false)
|
||||||
if strings.Contains(q, `"or"`) {
|
if strings.Contains(q, `"or"`) {
|
||||||
@@ -397,6 +426,22 @@ func TestLoadConfig_Defaults(t *testing.T) {
|
|||||||
if cfg.RoleFact != defaultRoleFact {
|
if cfg.RoleFact != defaultRoleFact {
|
||||||
t.Fatalf("expected default role fact, got %s", cfg.RoleFact)
|
t.Fatalf("expected default role fact, got %s", cfg.RoleFact)
|
||||||
}
|
}
|
||||||
|
if cfg.Domain != defaultDomain {
|
||||||
|
t.Fatalf("expected default domain, got %s", cfg.Domain)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestLoadConfig_DomainEnvOverride(t *testing.T) {
|
||||||
|
t.Setenv("XDG_CONFIG_HOME", t.TempDir())
|
||||||
|
t.Setenv("NODE_LOOKUP_DOMAIN", "example.com")
|
||||||
|
|
||||||
|
cfg, err := loadConfig()
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
if cfg.Domain != "example.com" {
|
||||||
|
t.Fatalf("domain env override failed: %s", cfg.Domain)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestLoadConfig_EnvOverride(t *testing.T) {
|
func TestLoadConfig_EnvOverride(t *testing.T) {
|
||||||
|
|||||||
Reference in New Issue
Block a user