diff --git a/AGENTS.md b/AGENTS.md index 33e6ba4..2107411 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -23,7 +23,7 @@ internal/catalog/ # catalog types, normalisation, diff engine, renderers, internal/puppetserver/ # mTLS client for POST /puppet/v4/catalog + /status/v1/simple internal/impact/ # path -> selector rules, repo index, reverse lookup, # estate fact table, renderers -internal/pdbmux/ # unauthenticated fact client for pdbmux +internal/puppetdb/ # unauthenticated fact client for PuppetDB internal/config/ # XDG config file + env var loading packaging/nfpm.yaml # RPM spec (envsubst-templated) scripts/build-rpm.sh # completions + nfpm packaging @@ -40,13 +40,14 @@ Adding a binary: create `cmd//`. The Makefile, `scripts/build-rpm.sh`, `options.prefer_requested_environment: true`, facts and synthesised trusted facts in the body. Auth is client mTLS against the estate Puppet CA; the certname must be allowed in the puppetserver `auth.conf`. -- Facts: `GET /pdb/query/v4/nodes//facts` on pdbmux, falling back to - `/pdb/query/v4/factsets/`. No auth, no client cert. Real PuppetDB - answers the factsets route with an *array*; pdbmux currently answers with a - bare object. The fallback accepts both — do not narrow it to one. -- Estate-wide facts: one AST query against `/pdb/query/v4/facts` - (`pdbmux.Client.FactsByName`). The estate is ~150 nodes, so `catalog-impact` - pulls the whole table once and filters in Go rather than querying per host. +- Facts: `GET /pdb/query/v4/facts` with the AST query + `["=","certname",""]`. No auth, no client cert. The response is a + flat array of `{certname, environment, name, value}` records; a record for + another node is never folded into the result. +- Estate-wide facts: the same endpoint and record shape, with an `extract` + query selecting the wanted fact names (`puppetdb.Client.FactsByName`). The + estate is ~150 nodes, so `catalog-impact` pulls the whole table once and + filters in Go rather than querying per host. - The two services have different auth models and stay separate clients. ## Gotchas @@ -104,7 +105,7 @@ make test # go test -v -race ./... ``` Everything is stubbed with `httptest` (a TLS server for puppetserver, a plain -one for pdbmux) — no live estate access is required. Renderer output is +one for PuppetDB) — no live estate access is required. Renderer output is covered by golden files under `internal/catalog/testdata`; regenerate with `go test ./internal/catalog -update`. diff --git a/README.md b/README.md index f70229d..b7beb59 100644 --- a/README.md +++ b/README.md @@ -8,7 +8,7 @@ CLI tooling for the Puppet estate: affect, and prints their hostnames. Catalogs are compiled over the puppetserver API (`POST /puppet/v4/catalog`) -with facts read from `pdbmux`. Nothing is compiled locally and nothing is +with facts read from PuppetDB. Nothing is compiled locally and nothing is persisted on the server. ## Install @@ -37,7 +37,7 @@ catalog-diff web01.example.net --branch benvin/motd-change --testonly | `--testonly` | compile the test environment only; result via exit code | | `--json` | machine-readable output | | `--server` | puppetserver base URL (default `https://puppet.k8s.syd1.au.unkin.net:8140`) | -| `--pdb` | pdbmux base URL (default `https://pdbmux.k8s.syd1.au.unkin.net`) | +| `--puppetdb` | PuppetDB base URL (default `https://pdbmux.k8s.syd1.au.unkin.net`) | | `--cert` / `--key` / `--cacert` | client mTLS material | | `--timeout` | per-compile timeout (default 10m; must be positive) | | `--retry` | retries for transient compile or transport failures (default 2, range 0-20; a failed compiler-side PuppetDB query counts, a manifest error does not) | @@ -71,7 +71,7 @@ Config file: `$XDG_CONFIG_HOME/puppet-catalog-tools/config.yaml` ```yaml server_url: https://puppet.k8s.syd1.au.unkin.net:8140 -pdb_url: https://pdbmux.k8s.syd1.au.unkin.net +puppetdb_url: https://pdbmux.k8s.syd1.au.unkin.net cert: /etc/puppetlabs/puppet/ssl/certs/catalog-diff.pem key: /etc/puppetlabs/puppet/ssl/private_keys/catalog-diff.pem cacert: /etc/puppetlabs/puppet/ssl/certs/ca.pem @@ -81,15 +81,15 @@ base_environment: develop | Env var | Config key | |---|---| | `CATALOG_DIFF_SERVER` | `server_url` | -| `CATALOG_DIFF_PDB` | `pdb_url` | +| `CATALOG_DIFF_PUPPETDB` | `puppetdb_url` | | `CATALOG_DIFF_CERT` | `cert` | | `CATALOG_DIFF_KEY` | `key` | | `CATALOG_DIFF_CACERT` | `cacert` | | `CATALOG_DIFF_BASE_ENVIRONMENT` | `base_environment` | `catalog-diff` needs a client certificate whose certname is allowed to -`POST /puppet/v4/catalog` in the puppetserver `auth.conf`. pdbmux needs no -credentials. +`POST /puppet/v4/catalog` in the puppetserver `auth.conf`. PuppetDB is queried +without credentials. ## Branch to environment name @@ -146,17 +146,17 @@ git diff --name-only origin/develop... | paste -sd, | xargs catalog-impact --fil | `-b, --branch` | branch under test (default: the repo's current branch) | | `--base` | base ref to diff against (default `origin/develop`) | | `--files` | explicit changed-file list, bypassing git entirely | -| `--pdb` | pdbmux base URL (default `https://pdbmux.k8s.syd1.au.unkin.net`) | +| `--puppetdb` | PuppetDB base URL (default `https://pdbmux.k8s.syd1.au.unkin.net`) | | `--json` | structured output with per-host reasons | | `--explain` | human-readable: for each host, which changed files hit it and why | | `--limit` | cap the number of hosts emitted (0 = no cap) | -| `--timeout` | pdbmux request timeout (default 1m) | +| `--timeout` | PuppetDB request timeout (default 1m) | -`--pdb` defaults to the shared configuration described under -[Configuration](#configuration), so `pdb_url` and `CATALOG_DIFF_PDB` apply to -both binaries. No credentials are needed. +`--puppetdb` defaults to the shared configuration described under +[Configuration](#configuration), so `puppetdb_url` and `CATALOG_DIFF_PUPPETDB` +apply to both binaries. No credentials are needed. -Facts for the whole estate are fetched from pdbmux in a single query and every +Facts for the whole estate are fetched from PuppetDB in a single query and every selector is evaluated in memory, so the cost is one HTTP request regardless of how many files changed. diff --git a/cmd/catalog-diff/main.go b/cmd/catalog-diff/main.go index 11c1c66..c56f85b 100644 --- a/cmd/catalog-diff/main.go +++ b/cmd/catalog-diff/main.go @@ -16,7 +16,7 @@ import ( "git.unkin.net/unkin/puppet-catalog-tools/internal/catalog" "git.unkin.net/unkin/puppet-catalog-tools/internal/config" - "git.unkin.net/unkin/puppet-catalog-tools/internal/pdbmux" + "git.unkin.net/unkin/puppet-catalog-tools/internal/puppetdb" "git.unkin.net/unkin/puppet-catalog-tools/internal/puppetserver" ) @@ -56,7 +56,7 @@ type options struct { testonly bool jsonOut bool server string - pdb string + puppetdb string cert string key string cacert string @@ -125,7 +125,7 @@ func main() { f.BoolVar(&opts.testonly, "testonly", false, "compile the test environment only; no output, result via exit code") f.BoolVar(&opts.jsonOut, "json", false, "machine-readable output") f.StringVar(&opts.server, "server", cfg.ServerURL, "puppetserver base URL") - f.StringVar(&opts.pdb, "pdb", cfg.PDBURL, "pdbmux base URL") + f.StringVar(&opts.puppetdb, "puppetdb", cfg.PuppetDBURL, "PuppetDB base URL") f.StringVar(&opts.cert, "cert", cfg.Cert, "client certificate PEM (CATALOG_DIFF_CERT)") f.StringVar(&opts.key, "key", cfg.Key, "client private key PEM (CATALOG_DIFF_KEY)") f.StringVar(&opts.cacert, "cacert", cfg.CACert, "CA bundle PEM used to verify puppetserver (CATALOG_DIFF_CACERT)") @@ -177,7 +177,7 @@ func configCmd() *cobra.Command { } fmt.Printf("config file : %s\n", config.Path()) fmt.Printf("server_url : %s\n", cfg.ServerURL) - fmt.Printf("pdb_url : %s\n", cfg.PDBURL) + fmt.Printf("puppetdb_url : %s\n", cfg.PuppetDBURL) fmt.Printf("cert : %s\n", cfg.Cert) fmt.Printf("key : %s\n", cfg.Key) fmt.Printf("cacert : %s\n", cfg.CACert) @@ -253,7 +253,7 @@ func run(ctx context.Context, out io.Writer, hostname string, opts *options) err } server := puppetserver.New(opts.server, tlsCfg, opts.timeout) - facts := pdbmux.New(opts.pdb, opts.timeout) + facts := puppetdb.New(opts.puppetdb, opts.timeout) facts.Warn = func(msg string) { fmt.Fprintln(os.Stderr, "warning:", msg) } healthCtx, cancel := context.WithTimeout(ctx, 30*time.Second) diff --git a/cmd/catalog-diff/main_test.go b/cmd/catalog-diff/main_test.go index 748fbcd..76bd51a 100644 --- a/cmd/catalog-diff/main_test.go +++ b/cmd/catalog-diff/main_test.go @@ -27,7 +27,7 @@ func catalogFor(environment, content string) string { ]}` } -// testServers stands up a fake puppetserver (TLS) and pdbmux (plain), and +// testServers stands up a fake puppetserver (TLS) and PuppetDB (plain), and // returns options wired to them with the server's own cert as the CA bundle. func testServers(t *testing.T, compile http.HandlerFunc) *options { t.Helper() @@ -53,11 +53,11 @@ func testServers(t *testing.T, compile http.HandlerFunc) *options { } return &options{ - base: "develop", - server: puppet.URL, - pdb: pdb.URL, - cacert: caPath, - timeout: 10 * time.Second, + base: "develop", + server: puppet.URL, + puppetdb: pdb.URL, + cacert: caPath, + timeout: 10 * time.Second, } } diff --git a/cmd/catalog-impact/main.go b/cmd/catalog-impact/main.go index f9302d7..bdf4adc 100644 --- a/cmd/catalog-impact/main.go +++ b/cmd/catalog-impact/main.go @@ -13,7 +13,7 @@ import ( "git.unkin.net/unkin/puppet-catalog-tools/internal/config" "git.unkin.net/unkin/puppet-catalog-tools/internal/impact" - "git.unkin.net/unkin/puppet-catalog-tools/internal/pdbmux" + "git.unkin.net/unkin/puppet-catalog-tools/internal/puppetdb" ) var version = "dev" @@ -39,15 +39,15 @@ and a class reference that is not a literal name — a selector, an interpolated name, a variable — is reported rather than silently dropped.` type options struct { - repo string - branch string - base string - files []string - pdb string - jsonOut bool - explain bool - limit int - timeout time.Duration + repo string + branch string + base string + files []string + puppetdb string + jsonOut bool + explain bool + limit int + timeout time.Duration } func main() { @@ -76,11 +76,11 @@ func main() { f.StringVarP(&opts.branch, "branch", "b", "", "branch under test (default: the repo's current branch)") f.StringVar(&opts.base, "base", "origin/develop", "base ref to diff against") f.StringSliceVar(&opts.files, "files", nil, "explicit changed-file list, bypassing git") - f.StringVar(&opts.pdb, "pdb", cfg.PDBURL, "pdbmux base URL") + f.StringVar(&opts.puppetdb, "puppetdb", cfg.PuppetDBURL, "PuppetDB base URL") f.BoolVar(&opts.jsonOut, "json", false, "emit structured output with per-host reasons") f.BoolVar(&opts.explain, "explain", false, "human-readable: for each host, which changed files hit it and why") f.IntVar(&opts.limit, "limit", 0, "cap the number of hosts emitted (0 = no cap)") - f.DurationVar(&opts.timeout, "timeout", time.Minute, "pdbmux request timeout") + f.DurationVar(&opts.timeout, "timeout", time.Minute, "PuppetDB request timeout") if err := root.Execute(); err != nil { fmt.Fprintln(os.Stderr, "error:", err) @@ -119,7 +119,7 @@ func run(ctx context.Context, out io.Writer, opts *options) error { return fmt.Errorf("indexing %s: %w", opts.repo, err) } - nodes, nodeWarnings, err := impact.LoadNodes(ctx, pdbmux.New(opts.pdb, opts.timeout)) + nodes, nodeWarnings, err := impact.LoadNodes(ctx, puppetdb.New(opts.puppetdb, opts.timeout)) if err != nil { return err } diff --git a/cmd/catalog-impact/main_test.go b/cmd/catalog-impact/main_test.go index 29dbcdb..3723b19 100644 --- a/cmd/catalog-impact/main_test.go +++ b/cmd/catalog-impact/main_test.go @@ -29,7 +29,7 @@ const estateBody = `[ {"certname":"k8s1.example.net","name":"os","value":{"name":"AlmaLinux","release":{"major":"9","minor":"8"}}} ]` -func fakePDB(t *testing.T) string { +func fakePuppetDB(t *testing.T) string { t.Helper() srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.Header().Set("Content-Type", "application/json") @@ -42,11 +42,11 @@ func fakePDB(t *testing.T) string { func TestRunWithExplicitFileList(t *testing.T) { var out bytes.Buffer opts := &options{ - repo: "../../internal/impact/testdata/repo", - base: "origin/develop", - files: []string{"modules/jellyfin/manifests/install.pp"}, - pdb: fakePDB(t), - timeout: 5 * time.Second, + repo: "../../internal/impact/testdata/repo", + base: "origin/develop", + files: []string{"modules/jellyfin/manifests/install.pp"}, + puppetdb: fakePuppetDB(t), + timeout: 5 * time.Second, } if err := run(context.Background(), &out, opts); err != nil { t.Fatalf("run: %v", err) @@ -59,12 +59,12 @@ func TestRunWithExplicitFileList(t *testing.T) { func TestRunExplain(t *testing.T) { var out bytes.Buffer opts := &options{ - repo: "../../internal/impact/testdata/repo", - base: "origin/develop", - files: []string{"hieradata/common.yaml"}, - pdb: fakePDB(t), - explain: true, - timeout: 5 * time.Second, + repo: "../../internal/impact/testdata/repo", + base: "origin/develop", + files: []string{"hieradata/common.yaml"}, + puppetdb: fakePuppetDB(t), + explain: true, + timeout: 5 * time.Second, } if err := run(context.Background(), &out, opts); err != nil { t.Fatalf("run: %v", err) diff --git a/internal/config/config.go b/internal/config/config.go index 586bb36..05e93c6 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -18,8 +18,8 @@ const ( // DefaultServerURL is the k8s OpenVoxServer compiler pool. The legacy VM // masters behind puppet.query.consul remain reachable via --server. DefaultServerURL = "https://puppet.k8s.syd1.au.unkin.net:8140" - // DefaultPDBURL is pdbmux, the PuppetDB-compatible merging proxy. - DefaultPDBURL = "https://pdbmux.k8s.syd1.au.unkin.net" + // DefaultPuppetDBURL is the PuppetDB facts are read from. + DefaultPuppetDBURL = "https://pdbmux.k8s.syd1.au.unkin.net" // DefaultBaseEnvironment is the environment branches are diffed against. DefaultBaseEnvironment = "develop" ) @@ -27,7 +27,7 @@ const ( // Config holds every configurable value. Fields map 1:1 to config file keys. type Config struct { ServerURL string `yaml:"server_url"` - PDBURL string `yaml:"pdb_url"` + PuppetDBURL string `yaml:"puppetdb_url"` Cert string `yaml:"cert"` Key string `yaml:"key"` CACert string `yaml:"cacert"` @@ -38,7 +38,7 @@ type Config struct { func Default() Config { return Config{ ServerURL: DefaultServerURL, - PDBURL: DefaultPDBURL, + PuppetDBURL: DefaultPuppetDBURL, BaseEnvironment: DefaultBaseEnvironment, } } @@ -73,7 +73,7 @@ func Load() (Config, error) { env := map[string]*string{ "CATALOG_DIFF_SERVER": &cfg.ServerURL, - "CATALOG_DIFF_PDB": &cfg.PDBURL, + "CATALOG_DIFF_PUPPETDB": &cfg.PuppetDBURL, "CATALOG_DIFF_CERT": &cfg.Cert, "CATALOG_DIFF_KEY": &cfg.Key, "CATALOG_DIFF_CACERT": &cfg.CACert, @@ -103,8 +103,9 @@ func WriteDefault() (string, error) { return "", err } header := "# puppet-catalog-tools configuration\n" + - "# Overridable with CATALOG_DIFF_SERVER, CATALOG_DIFF_PDB, CATALOG_DIFF_CERT,\n" + - "# CATALOG_DIFF_KEY, CATALOG_DIFF_CACERT, CATALOG_DIFF_BASE_ENVIRONMENT.\n\n" + "# Overridable with CATALOG_DIFF_SERVER, CATALOG_DIFF_PUPPETDB,\n" + + "# CATALOG_DIFF_CERT, CATALOG_DIFF_KEY, CATALOG_DIFF_CACERT,\n" + + "# CATALOG_DIFF_BASE_ENVIRONMENT.\n\n" if err := os.WriteFile(path, append([]byte(header), data...), 0o644); err != nil { return "", fmt.Errorf("writing config: %w", err) } diff --git a/internal/config/config_test.go b/internal/config/config_test.go index dae3ad0..9f1ea22 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -13,7 +13,7 @@ func TestLoadDefaults(t *testing.T) { if err != nil { t.Fatalf("Load: %v", err) } - if cfg.ServerURL != DefaultServerURL || cfg.PDBURL != DefaultPDBURL { + if cfg.ServerURL != DefaultServerURL || cfg.PuppetDBURL != DefaultPuppetDBURL { t.Errorf("unexpected defaults: %+v", cfg) } if cfg.BaseEnvironment != DefaultBaseEnvironment { @@ -28,7 +28,7 @@ func TestLoadPrecedence(t *testing.T) { if err := os.MkdirAll(filepath.Join(dir, appName), 0o755); err != nil { t.Fatal(err) } - body := "server_url: https://file.example:8140\npdb_url: https://file.example\ncert: /file/cert.pem\n" + body := "server_url: https://file.example:8140\npuppetdb_url: https://file.example\ncert: /file/cert.pem\n" if err := os.WriteFile(Path(), []byte(body), 0o644); err != nil { t.Fatal(err) } @@ -56,8 +56,8 @@ func TestLoadPrecedence(t *testing.T) { if cfg.Cert != "/env/cert.pem" { t.Errorf("cert = %q", cfg.Cert) } - if cfg.PDBURL != "https://file.example" { - t.Errorf("unset env clobbered file value: %q", cfg.PDBURL) + if cfg.PuppetDBURL != "https://file.example" { + t.Errorf("unset env clobbered file value: %q", cfg.PuppetDBURL) } } diff --git a/internal/impact/impact_test.go b/internal/impact/impact_test.go index 33103b9..8685f4d 100644 --- a/internal/impact/impact_test.go +++ b/internal/impact/impact_test.go @@ -8,7 +8,7 @@ import ( "strings" "testing" - "git.unkin.net/unkin/puppet-catalog-tools/internal/pdbmux" + "git.unkin.net/unkin/puppet-catalog-tools/internal/puppetdb" ) func testAnalyzer(t *testing.T) *Analyzer { @@ -204,19 +204,19 @@ func TestRenderExplainNamesFileRuleAndHost(t *testing.T) { } } -// stubFacts serves a canned fact table the way pdbmux would. +// stubFacts serves a canned fact table the way PuppetDB would. type stubFacts struct { - records []pdbmux.FactRecord + records []puppetdb.FactRecord asked []string } -func (s *stubFacts) FactsByName(_ context.Context, names []string) ([]pdbmux.FactRecord, error) { +func (s *stubFacts) FactsByName(_ context.Context, names []string) ([]puppetdb.FactRecord, error) { s.asked = names return s.records, nil } func TestLoadNodesFoldsStructuredOS(t *testing.T) { - src := &stubFacts{records: []pdbmux.FactRecord{ + src := &stubFacts{records: []puppetdb.FactRecord{ {Certname: "b.example.net", Name: "enc_role", Value: "roles::infra::k8s::node"}, {Certname: "b.example.net", Name: "enc_role_tier1", Value: "infra"}, {Certname: "b.example.net", Name: "enc_role_tier2", Value: "k8s"}, @@ -261,7 +261,7 @@ func TestLoadNodesRejectsEmptyEstate(t *testing.T) { func TestLoadNodesReportsRecordsWithNoCertname(t *testing.T) { // A record with no certname cannot be attributed to a host. Dropping it in // silence hides a host, so it has to be reported. - src := &stubFacts{records: []pdbmux.FactRecord{ + src := &stubFacts{records: []puppetdb.FactRecord{ {Certname: "a.example.net", Name: "enc_role", Value: "roles::base"}, {Name: "enc_role", Value: "roles::base"}, {Name: "enc_role", Value: "roles::web"}, diff --git a/internal/impact/node.go b/internal/impact/node.go index 17922a9..7d68073 100644 --- a/internal/impact/node.go +++ b/internal/impact/node.go @@ -8,7 +8,7 @@ import ( "sort" "strings" - "git.unkin.net/unkin/puppet-catalog-tools/internal/pdbmux" + "git.unkin.net/unkin/puppet-catalog-tools/internal/puppetdb" ) // FactNames are the facts every selector is evaluated against. They are @@ -53,7 +53,7 @@ func (n Node) Tiers() []string { // FactSource supplies the estate-wide fact table. type FactSource interface { - FactsByName(ctx context.Context, names []string) ([]pdbmux.FactRecord, error) + FactsByName(ctx context.Context, names []string) ([]puppetdb.FactRecord, error) } // LoadNodes fetches FactNames for every node and folds them into a sorted @@ -87,7 +87,7 @@ func LoadNodes(ctx context.Context, src FactSource) ([]Node, []string, error) { } sort.Slice(nodes, func(i, j int) bool { return nodes[i].Certname < nodes[j].Certname }) if len(nodes) == 0 { - return nil, anonymousWarnings(anonymous), fmt.Errorf("pdbmux returned no nodes") + return nil, anonymousWarnings(anonymous), fmt.Errorf("puppetdb returned no nodes") } return nodes, anonymousWarnings(anonymous), nil } diff --git a/internal/pdbmux/pdbmux.go b/internal/pdbmux/pdbmux.go deleted file mode 100644 index a6e7762..0000000 --- a/internal/pdbmux/pdbmux.go +++ /dev/null @@ -1,220 +0,0 @@ -// Package pdbmux reads node facts from pdbmux, the estate's PuppetDB -// compatible merging proxy. It is plain HTTPS, GET-only and unauthenticated — -// deliberately a separate client from the mTLS puppetserver one. -package pdbmux - -import ( - "bytes" - "context" - "encoding/json" - "errors" - "fmt" - "io" - "net/http" - "net/url" - "strings" - "time" -) - -// ErrNodeNotFound is returned when pdbmux knows nothing about a certname. -var ErrNodeNotFound = errors.New("node not found in pdbmux") - -// Client queries pdbmux. -type Client struct { - BaseURL string - HTTP *http.Client - - // Warn, when set, receives a message for every fact record that could not - // be decoded and was therefore dropped. - Warn func(string) -} - -// New returns a client for baseURL. -func New(baseURL string, timeout time.Duration) *Client { - return &Client{ - BaseURL: strings.TrimRight(baseURL, "/"), - HTTP: &http.Client{Timeout: timeout}, - } -} - -type factRecord struct { - Name string `json:"name"` - Value json.RawMessage `json:"value"` -} - -type factset struct { - Certname string `json:"certname"` - Facts struct { - Data []factRecord `json:"data"` - } `json:"facts"` -} - -// Facts returns the node's facts as the values map a catalog request needs. -// It reads /pdb/query/v4/nodes//facts and falls back to -// /pdb/query/v4/factsets/, which returns the same data nested in -// either an array of factsets or a single bare factset object. -func (c *Client) Facts(ctx context.Context, certname string) (map[string]any, error) { - body, status, err := c.get(ctx, "/pdb/query/v4/nodes/"+url.PathEscape(certname)+"/facts") - if err != nil { - return nil, err - } - if status == http.StatusOK { - var records []factRecord - if err := json.Unmarshal(body, &records); err != nil { - return nil, fmt.Errorf("parsing facts for %s: %w", certname, err) - } - if len(records) > 0 { - return c.fold(certname, records) - } - } else if status != http.StatusNotFound { - return nil, fmt.Errorf("pdbmux returned %d for %s facts: %s", status, certname, snippet(body)) - } - - body, status, err = c.get(ctx, "/pdb/query/v4/factsets/"+url.PathEscape(certname)) - if err != nil { - return nil, err - } - if status == http.StatusNotFound { - return nil, fmt.Errorf("%w: %s", ErrNodeNotFound, certname) - } - if status != http.StatusOK { - return nil, fmt.Errorf("pdbmux returned %d for %s factset: %s", status, certname, snippet(body)) - } - - records, err := decodeFactsets(body, certname) - if err != nil { - return nil, fmt.Errorf("parsing factset for %s: %w", certname, err) - } - if len(records) == 0 { - return nil, fmt.Errorf("%w: %s", ErrNodeNotFound, certname) - } - return c.fold(certname, records) -} - -// decodeFactsets reads the two shapes /factsets/ comes back as: real -// PuppetDB returns an array of factsets, pdbmux currently returns a single -// bare object. -func decodeFactsets(body []byte, certname string) ([]factRecord, error) { - if bytes.HasPrefix(bytes.TrimLeft(body, " \t\r\n"), []byte("[")) { - var sets []factset - if err := json.Unmarshal(body, &sets); err != nil { - return nil, err - } - for _, fs := range sets { - if fs.Certname == certname { - return fs.Facts.Data, nil - } - } - if len(sets) == 1 { - return sets[0].Facts.Data, nil - } - if len(sets) == 0 { - return nil, nil - } - return nil, fmt.Errorf("%d factsets returned, none for the requested certname", len(sets)) - } - - var fs factset - if err := json.Unmarshal(body, &fs); err != nil { - return nil, err - } - return fs.Facts.Data, nil -} - -func (c *Client) get(ctx context.Context, path string) ([]byte, int, error) { - req, err := http.NewRequestWithContext(ctx, http.MethodGet, c.BaseURL+path, nil) - if err != nil { - return nil, 0, err - } - req.Header.Set("Accept", "application/json") - - resp, err := c.HTTP.Do(req) - if err != nil { - return nil, 0, fmt.Errorf("pdbmux request failed: %w", err) - } - defer func() { _ = resp.Body.Close() }() - - body, err := io.ReadAll(resp.Body) - if err != nil { - return nil, resp.StatusCode, fmt.Errorf("reading pdbmux response: %w", err) - } - return body, resp.StatusCode, nil -} - -// fold turns fact records into the flat name -> value map the catalog API -// wants. Structured facts keep their nested shape. An undecodable record is -// dropped and reported: one bad fact must not cost the whole set. A response -// in which nothing decodes is a failed fetch, not an empty fact set. -func (c *Client) fold(certname string, records []factRecord) (map[string]any, error) { - values := make(map[string]any, len(records)) - var skipped []string - for _, r := range records { - if r.Name == "" { - continue - } - var v any - if len(r.Value) == 0 || json.Unmarshal(r.Value, &v) != nil { - skipped = append(skipped, r.Name) - continue - } - values[r.Name] = v - } - if len(values) == 0 { - return nil, fmt.Errorf("no usable facts for %s: all %d records were undecodable", certname, len(records)) - } - if len(skipped) > 0 && c.Warn != nil { - c.Warn(fmt.Sprintf("skipped %d undecodable fact(s) for %s: %s", - len(skipped), certname, strings.Join(skipped, ", "))) - } - return values, nil -} - -func snippet(body []byte) string { - s := strings.TrimSpace(string(body)) - if len(s) > 200 { - return s[:200] + "..." - } - return s -} - -// FactRecord is one (certname, fact) pair from an estate-wide fact query. -type FactRecord struct { - Certname string `json:"certname"` - Name string `json:"name"` - Value any `json:"value"` -} - -// FactsByName returns the named facts for every node pdbmux knows about, in a -// single query. Tools that evaluate selectors over the whole estate want the -// table once rather than a request per host. -func (c *Client) FactsByName(ctx context.Context, names []string) ([]FactRecord, error) { - if len(names) == 0 { - return nil, nil - } - clauses := make([]any, 0, len(names)+1) - clauses = append(clauses, "or") - for _, n := range names { - clauses = append(clauses, []any{"=", "name", n}) - } - query := []any{"extract", []string{"certname", "name", "value"}, clauses} - - encoded, err := json.Marshal(query) - if err != nil { - return nil, fmt.Errorf("encoding fact query: %w", err) - } - path := "/pdb/query/v4/facts?query=" + url.QueryEscape(string(encoded)) - - body, status, err := c.get(ctx, path) - if err != nil { - return nil, err - } - if status != http.StatusOK { - return nil, fmt.Errorf("pdbmux returned %d for fact query: %s", status, snippet(body)) - } - - var records []FactRecord - if err := json.Unmarshal(body, &records); err != nil { - return nil, fmt.Errorf("parsing fact query response: %w", err) - } - return records, nil -} diff --git a/internal/puppetdb/puppetdb.go b/internal/puppetdb/puppetdb.go new file mode 100644 index 0000000..284fa0d --- /dev/null +++ b/internal/puppetdb/puppetdb.go @@ -0,0 +1,260 @@ +// Package puppetdb reads node facts from PuppetDB's query API. It is GET-only +// and unauthenticated — deliberately a separate client from the mTLS +// puppetserver one. +package puppetdb + +import ( + "bytes" + "context" + "encoding/json" + "errors" + "fmt" + "io" + "net/http" + "net/url" + "strings" + "time" +) + +// ErrNodeNotFound is returned when PuppetDB knows no facts for a certname. +var ErrNodeNotFound = errors.New("node not found in PuppetDB") + +// Client queries PuppetDB. +type Client struct { + BaseURL string + HTTP *http.Client + + // Warn, when set, receives a message for every fact record that could not + // be decoded and was therefore dropped. + Warn func(string) +} + +// New returns a client for baseURL. +func New(baseURL string, timeout time.Duration) *Client { + return &Client{ + BaseURL: strings.TrimRight(baseURL, "/"), + HTTP: &http.Client{Timeout: timeout}, + } +} + +// factRecord is one fact as the facts endpoint returns it. Value stays raw so +// that a single undecodable fact can be dropped without costing the rest. +type factRecord struct { + Certname string `json:"certname"` + Name string `json:"name"` + Value json.RawMessage `json:"value"` +} + +// FactRecord is one (certname, fact) pair with its value decoded. +type FactRecord struct { + Certname string + Name string + Value any +} + +// Facts returns the node's facts as the values map a catalog request needs. +func (c *Client) Facts(ctx context.Context, certname string) (map[string]any, error) { + body, status, err := c.queryFacts(ctx, []string{"=", "certname", certname}, certname) + if err != nil { + return nil, err + } + if status == http.StatusNotFound { + return nil, fmt.Errorf("%w: %s", ErrNodeNotFound, certname) + } + if status != http.StatusOK { + return nil, fmt.Errorf("puppetdb returned %d for %s facts: %s", status, certname, snippet(body)) + } + + records, err := decodeFacts(body, certname) + if err != nil { + return nil, err + } + if len(records) == 0 { + return nil, fmt.Errorf("%w: %s", ErrNodeNotFound, certname) + } + return c.fold(certname, records) +} + +// FactsByName returns the named facts for every node PuppetDB knows about, in +// a single query. Tools that evaluate selectors over the whole estate want the +// table once rather than a request per host. +func (c *Client) FactsByName(ctx context.Context, names []string) ([]FactRecord, error) { + if len(names) == 0 { + return nil, nil + } + clauses := make([]any, 0, len(names)+1) + clauses = append(clauses, "or") + for _, n := range names { + clauses = append(clauses, []any{"=", "name", n}) + } + query := []any{"extract", []string{"certname", "name", "value"}, clauses} + + body, status, err := c.queryFacts(ctx, query, "the estate") + if err != nil { + return nil, err + } + if status != http.StatusOK { + return nil, fmt.Errorf("puppetdb returned %d for fact query: %s", status, snippet(body)) + } + + records, err := decodeRecords(body, "the estate") + if err != nil { + return nil, err + } + + out := make([]FactRecord, 0, len(records)) + var skipped []string + for _, r := range records { + v, ok := decodeValue(r) + if !ok { + skipped = append(skipped, r.Name) + continue + } + out = append(out, FactRecord{Certname: r.Certname, Name: r.Name, Value: v}) + } + if len(skipped) > 0 && c.Warn != nil { + c.Warn(fmt.Sprintf("skipped %d undecodable fact(s) in the estate fact table: %s", + len(skipped), strings.Join(skipped, ", "))) + } + return out, nil +} + +// queryFacts runs an AST query against the facts endpoint. subject names what +// was asked for so a failure says so. +func (c *Client) queryFacts(ctx context.Context, query any, subject string) ([]byte, int, error) { + encoded, err := json.Marshal(query) + if err != nil { + return nil, 0, fmt.Errorf("building facts query for %s: %w", subject, err) + } + return c.get(ctx, "/pdb/query/v4/facts", string(encoded)) +} + +// decodeRecords reads the flat array of fact records the facts endpoint +// answers with. +func decodeRecords(body []byte, subject string) ([]factRecord, error) { + body = bytes.TrimPrefix(body, []byte("\xef\xbb\xbf")) + + var records []factRecord + if err := json.Unmarshal(body, &records); err != nil { + return nil, fmt.Errorf("parsing facts for %s: %w", subject, err) + } + return records, nil +} + +// decodeValue reports a record's value and whether it decoded at all. +func decodeValue(r factRecord) (any, bool) { + if len(r.Value) == 0 { + return nil, false + } + var v any + if json.Unmarshal(r.Value, &v) != nil { + return nil, false + } + return v, true +} + +// decodeFacts reads the fact records a single-node query returns. A record +// belonging to another node is never substituted for the one that was asked +// for. A record that carries no certname of its own cannot be contradicting +// the query that named one, so it is kept. +func decodeFacts(body []byte, certname string) ([]factRecord, error) { + records, err := decodeRecords(body, certname) + if err != nil { + return nil, err + } + + kept := make([]factRecord, 0, len(records)) + for _, r := range records { + if r.Certname == "" || sameCertname(r.Certname, certname) { + kept = append(kept, r) + } + } + if len(kept) == 0 && len(records) > 0 { + return nil, fmt.Errorf("no facts for %s: puppetdb returned %s", + certname, strings.Join(certnames(records), ", ")) + } + return kept, nil +} + +// sameCertname compares certnames case-insensitively: they are DNS names, so +// folding can only absorb a certname that was normalised for case, never mask +// a different node. +func sameCertname(got, want string) bool { + return got != "" && strings.EqualFold(got, want) +} + +// certnames lists the distinct nodes a response spoke for, in the order they +// appeared, so a mismatch error names what actually came back. +func certnames(records []factRecord) []string { + seen := make(map[string]bool, len(records)) + names := make([]string, 0, len(records)) + for _, r := range records { + name := r.Certname + if name == "" { + name = "(unnamed)" + } + if seen[name] { + continue + } + seen[name] = true + names = append(names, name) + } + return names +} + +func (c *Client) get(ctx context.Context, path, query string) ([]byte, int, error) { + target := c.BaseURL + path + "?" + url.Values{"query": {query}}.Encode() + req, err := http.NewRequestWithContext(ctx, http.MethodGet, target, nil) + if err != nil { + return nil, 0, err + } + req.Header.Set("Accept", "application/json") + + resp, err := c.HTTP.Do(req) + if err != nil { + return nil, 0, fmt.Errorf("puppetdb request failed: %w", err) + } + defer func() { _ = resp.Body.Close() }() + + body, err := io.ReadAll(resp.Body) + if err != nil { + return nil, resp.StatusCode, fmt.Errorf("reading puppetdb response: %w", err) + } + return body, resp.StatusCode, nil +} + +// fold turns fact records into the flat name -> value map the catalog API +// wants. Structured facts keep their nested shape. An undecodable record is +// dropped and reported: one bad fact must not cost the whole set. A response +// in which nothing decodes is a failed fetch, not an empty fact set. +func (c *Client) fold(certname string, records []factRecord) (map[string]any, error) { + values := make(map[string]any, len(records)) + var skipped []string + for _, r := range records { + if r.Name == "" { + continue + } + v, ok := decodeValue(r) + if !ok { + skipped = append(skipped, r.Name) + continue + } + values[r.Name] = v + } + if len(values) == 0 { + return nil, fmt.Errorf("no usable facts for %s: all %d records were undecodable", certname, len(records)) + } + if len(skipped) > 0 && c.Warn != nil { + c.Warn(fmt.Sprintf("skipped %d undecodable fact(s) for %s: %s", + len(skipped), certname, strings.Join(skipped, ", "))) + } + return values, nil +} + +func snippet(body []byte) string { + s := strings.TrimSpace(string(body)) + if len(s) > 200 { + return s[:200] + "..." + } + return s +} diff --git a/internal/pdbmux/pdbmux_test.go b/internal/puppetdb/puppetdb_test.go similarity index 52% rename from internal/pdbmux/pdbmux_test.go rename to internal/puppetdb/puppetdb_test.go index aefbf5a..37224c4 100644 --- a/internal/pdbmux/pdbmux_test.go +++ b/internal/puppetdb/puppetdb_test.go @@ -1,7 +1,8 @@ -package pdbmux +package puppetdb import ( "context" + "encoding/json" "errors" "net/http" "net/http/httptest" @@ -10,36 +11,14 @@ import ( "time" ) -const nodeFactsBody = `[ +// The shape PuppetDB answers /pdb/query/v4/facts with: a flat array of fact +// records, each naming the node it belongs to. +const factsBody = `[ {"certname":"host.example.net","environment":"develop","name":"id","value":"root"}, {"certname":"host.example.net","environment":"develop","name":"os","value":{"family":"RedHat","release":{"major":"9"}}}, {"certname":"host.example.net","environment":"develop","name":"processorcount","value":4} ]` -// The shape real PuppetDB returns: /factsets/ is a query endpoint -// and answers with an array. -const factsetBody = `[{ - "certname":"host.example.net", - "environment":"develop", - "timestamp":"2026-09-12T05:26:11.807Z", - "facts":{"data":[ - {"name":"id","value":"root"}, - {"name":"os","value":{"family":"RedHat","release":{"major":"9"}}}, - {"name":"processorcount","value":4} - ]} -}]` - -// The shape pdbmux itself currently returns for the same route. -const bareFactsetBody = `{ - "certname":"host.example.net", - "environment":"develop", - "facts":{"data":[ - {"name":"id","value":"root"}, - {"name":"os","value":{"family":"RedHat","release":{"major":"9"}}}, - {"name":"processorcount","value":4} - ]} -}` - func assertFacts(t *testing.T, facts map[string]any) { t.Helper() if facts["id"] != "root" { @@ -61,61 +40,98 @@ func assertFacts(t *testing.T, facts map[string]any) { } } -func TestFactsFromNodesEndpoint(t *testing.T) { - var paths []string +// factsServer answers the query route with body and fails the test on any +// other path. +func factsServer(t *testing.T, body string) *httptest.Server { + t.Helper() srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - paths = append(paths, r.URL.Path) - w.Header().Set("Content-Type", "application/json") - _, _ = w.Write([]byte(nodeFactsBody)) - })) - defer srv.Close() - - facts, err := New(srv.URL, 5*time.Second).Facts(context.Background(), "host.example.net") - if err != nil { - t.Fatalf("Facts: %v", err) - } - assertFacts(t, facts) - if len(paths) != 1 || paths[0] != "/pdb/query/v4/nodes/host.example.net/facts" { - t.Errorf("unexpected requests: %v", paths) - } -} - -func TestFactsFallsBackToFactsets(t *testing.T) { - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - switch r.URL.Path { - case "/pdb/query/v4/nodes/host.example.net/facts": - w.WriteHeader(http.StatusNotFound) - _, _ = w.Write([]byte(`{"error":"No information is known about node host.example.net"}`)) - case "/pdb/query/v4/factsets/host.example.net": - _, _ = w.Write([]byte(factsetBody)) - default: + if r.URL.Path != "/pdb/query/v4/facts" { t.Errorf("unexpected path %s", r.URL.Path) - } - })) - defer srv.Close() - - facts, err := New(srv.URL, 5*time.Second).Facts(context.Background(), "host.example.net") - if err != nil { - t.Fatalf("Facts: %v", err) - } - assertFacts(t, facts) -} - -func TestFactsEmptyArrayFallsBack(t *testing.T) { - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.URL.Path == "/pdb/query/v4/factsets/host.example.net" { - _, _ = w.Write([]byte(factsetBody)) + w.WriteHeader(http.StatusNotFound) return } + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(body)) + })) + t.Cleanup(srv.Close) + return srv +} + +func TestFactsQueriesByCertname(t *testing.T) { + var paths, queries []string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + paths = append(paths, r.URL.Path) + queries = append(queries, r.URL.Query().Get("query")) + _, _ = w.Write([]byte(factsBody)) + })) + defer srv.Close() + + facts, err := New(srv.URL, 5*time.Second).Facts(context.Background(), "host.example.net") + if err != nil { + t.Fatalf("Facts: %v", err) + } + assertFacts(t, facts) + if len(paths) != 1 || paths[0] != "/pdb/query/v4/facts" { + t.Errorf("unexpected requests: %v", paths) + } + if len(queries) != 1 || queries[0] != `["=","certname","host.example.net"]` { + t.Errorf("unexpected query: %v", queries) + } +} + +// A certname with characters the AST would otherwise have to escape must +// survive both JSON encoding and query-string encoding intact. +func TestFactsEscapesTheCertnameInTheQuery(t *testing.T) { + var got string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + got = r.URL.Query().Get("query") _, _ = w.Write([]byte(`[]`)) })) defer srv.Close() + const certname = `we"ird &host` + _, _ = New(srv.URL, 5*time.Second).Facts(context.Background(), certname) + + var ast []string + if err := json.Unmarshal([]byte(got), &ast); err != nil { + t.Fatalf("query %q is not valid JSON: %v", got, err) + } + if len(ast) != 3 || ast[0] != "=" || ast[1] != "certname" || ast[2] != certname { + t.Errorf("query = %q, decoded %#v", got, ast) + } +} + +// The response shape as PuppetDB really returns it, structured facts and all. +func TestFactsDecodesARealResponse(t *testing.T) { + srv := factsServer(t, `[ + {"certname":"host.example.net","environment":"develop","name":"id","value":"root"}, + {"certname":"host.example.net","environment":"develop","name":"processorcount","value":4}, + {"certname":"host.example.net","environment":"develop","name":"os","value":{"name":"AlmaLinux","distro":{"id":"AlmaLinux","release":{"full":"9.7","major":"9","minor":"7"}},"family":"RedHat","release":{"full":"9.7","major":"9","minor":"7"},"selinux":{"enabled":false},"architecture":"x86_64"}} + ]`) + facts, err := New(srv.URL, 5*time.Second).Facts(context.Background(), "host.example.net") if err != nil { t.Fatalf("Facts: %v", err) } assertFacts(t, facts) + os := facts["os"].(map[string]any) + if os["architecture"] != "x86_64" { + t.Errorf("os.architecture = %v", os["architecture"]) + } + selinux, ok := os["selinux"].(map[string]any) + if !ok || selinux["enabled"] != false { + t.Errorf("nested os.selinux lost: %#v", os["selinux"]) + } +} + +// An unknown node is not an HTTP error: the query simply matches nothing. +func TestFactsEmptyResultIsAMissingNode(t *testing.T) { + srv := factsServer(t, `[]`) + + _, err := New(srv.URL, 5*time.Second).Facts(context.Background(), "nope.example.net") + if !errors.Is(err, ErrNodeNotFound) { + t.Fatalf("err = %v, want ErrNodeNotFound", err) + } } func TestFactsMissingNode(t *testing.T) { @@ -148,10 +164,7 @@ func TestFactsServerError(t *testing.T) { } func TestFactsBadJSON(t *testing.T) { - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - _, _ = w.Write([]byte(`not json`)) - })) - defer srv.Close() + srv := factsServer(t, `not json`) if _, err := New(srv.URL, 5*time.Second).Facts(context.Background(), "host.example.net"); err == nil { t.Fatal("expected a parse error") @@ -161,7 +174,7 @@ func TestFactsBadJSON(t *testing.T) { func TestFactsContextCancelled(t *testing.T) { srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { time.Sleep(200 * time.Millisecond) - _, _ = w.Write([]byte(nodeFactsBody)) + _, _ = w.Write([]byte(factsBody)) })) defer srv.Close() @@ -172,83 +185,107 @@ func TestFactsContextCancelled(t *testing.T) { } } -// factsetServer answers the nodes route with a 404 and the factsets route with -// body, which is the path the fallback takes. -func factsetServer(t *testing.T, body string) *httptest.Server { +// assertNamesBoth checks the mismatch error names the node asked for and the +// ones that came back, and that it is not dressed up as a parse failure. +func assertNamesBoth(t *testing.T, err error, want string, got ...string) { t.Helper() - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.URL.Path == "/pdb/query/v4/factsets/host.example.net" { - _, _ = w.Write([]byte(body)) - return - } - w.WriteHeader(http.StatusNotFound) - _, _ = w.Write([]byte(`{"error":"No information is known about node host.example.net"}`)) - })) - t.Cleanup(srv.Close) - return srv -} - -func TestFactsFallsBackToBareFactsetObject(t *testing.T) { - srv := factsetServer(t, bareFactsetBody) - - facts, err := New(srv.URL, 5*time.Second).Facts(context.Background(), "host.example.net") - if err != nil { - t.Fatalf("Facts: %v", err) + if !strings.Contains(err.Error(), want) { + t.Errorf("error does not name the requested certname %s: %v", want, err) + } + for _, g := range got { + if !strings.Contains(err.Error(), g) { + t.Errorf("error does not name the returned certname %s: %v", g, err) + } + } + if strings.Contains(err.Error(), "parsing facts") { + t.Errorf("a certname mismatch is not a parse failure: %v", err) } - assertFacts(t, facts) } -func TestFactsPicksTheRequestedCertnameFromAFactsetArray(t *testing.T) { - srv := factsetServer(t, `[ - {"certname":"other.example.net","facts":{"data":[{"name":"id","value":"nobody"}]}}, - {"certname":"host.example.net","facts":{"data":[ - {"name":"id","value":"root"}, - {"name":"os","value":{"family":"RedHat","release":{"major":"9"}}}, - {"name":"processorcount","value":4} - ]}} +func TestFactsRejectsRecordsForAnotherNode(t *testing.T) { + srv := factsServer(t, `[ + {"certname":"totally-different-host.example.net","name":"id","value":"nobody"}, + {"certname":"totally-different-host.example.net","name":"os","value":{"family":"Debian"}} ]`) facts, err := New(srv.URL, 5*time.Second).Facts(context.Background(), "host.example.net") - if err != nil { - t.Fatalf("Facts: %v", err) + if err == nil { + t.Fatalf("another node's facts were served as ours: %#v", facts) } - assertFacts(t, facts) + assertNamesBoth(t, err, "host.example.net", "totally-different-host.example.net") } -func TestFactsRejectsAFactsetArrayWithNoMatch(t *testing.T) { - srv := factsetServer(t, `[ - {"certname":"a.example.net","facts":{"data":[{"name":"id","value":"a"}]}}, - {"certname":"b.example.net","facts":{"data":[{"name":"id","value":"b"}]}} +func TestFactsRejectsAMixedResponseNamingEveryNode(t *testing.T) { + srv := factsServer(t, `[ + {"certname":"a.example.net","name":"id","value":"a"}, + {"certname":"b.example.net","name":"id","value":"b"} ]`) _, err := New(srv.URL, 5*time.Second).Facts(context.Background(), "host.example.net") if err == nil { - t.Fatal("expected an error when no factset matches the certname") - } - if !strings.Contains(err.Error(), "none for the requested certname") { - t.Errorf("unhelpful error: %v", err) + t.Fatal("expected an error when no record matches the certname") } + assertNamesBoth(t, err, "host.example.net", "a.example.net", "b.example.net") } -func TestFactsEmptyFactsetArrayIsAMissingNode(t *testing.T) { - srv := factsetServer(t, `[]`) +func TestFactsDropsAForeignRecordFromAnOtherwiseMatchingResponse(t *testing.T) { + srv := factsServer(t, `[ + {"certname":"host.example.net","name":"id","value":"root"}, + {"certname":"other.example.net","name":"id","value":"nobody"}, + {"certname":"host.example.net","name":"os","value":{"family":"RedHat","release":{"major":"9"}}}, + {"certname":"host.example.net","name":"processorcount","value":4} + ]`) - _, err := New(srv.URL, 5*time.Second).Facts(context.Background(), "host.example.net") - if !errors.Is(err, ErrNodeNotFound) { - t.Fatalf("err = %v, want ErrNodeNotFound", err) + facts, err := New(srv.URL, 5*time.Second).Facts(context.Background(), "host.example.net") + if err != nil { + t.Fatalf("Facts: %v", err) } + assertFacts(t, facts) +} + +// The query already names the node, so a record that asserts no certname of +// its own cannot be contradicting it. +func TestFactsAcceptsRecordsWithNoCertname(t *testing.T) { + srv := factsServer(t, `[ + {"name":"id","value":"root"}, + {"name":"os","value":{"family":"RedHat","release":{"major":"9"}}}, + {"name":"processorcount","value":4} + ]`) + + facts, err := New(srv.URL, 5*time.Second).Facts(context.Background(), "host.example.net") + if err != nil { + t.Fatalf("Facts: %v", err) + } + assertFacts(t, facts) +} + +func TestFactsMatchesCertnameCaseInsensitively(t *testing.T) { + srv := factsServer(t, factsBody) + + facts, err := New(srv.URL, 5*time.Second).Facts(context.Background(), "HOST.example.net") + if err != nil { + t.Fatalf("Facts: %v", err) + } + assertFacts(t, facts) +} + +func TestFactsToleratesALeadingBOM(t *testing.T) { + srv := factsServer(t, "\ufeff"+factsBody) + + facts, err := New(srv.URL, 5*time.Second).Facts(context.Background(), "host.example.net") + if err != nil { + t.Fatalf("Facts: %v", err) + } + assertFacts(t, facts) } func TestFactsSkipsUndecodableRecords(t *testing.T) { - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - _, _ = w.Write([]byte(`[ - {"name":"id","value":"root"}, - {"name":"broken"}, - {"name":"os","value":{"family":"RedHat","release":{"major":"9"}}}, - {"name":"processorcount","value":4} - ]`)) - })) - defer srv.Close() + srv := factsServer(t, `[ + {"certname":"host.example.net","name":"id","value":"root"}, + {"certname":"host.example.net","name":"broken"}, + {"certname":"host.example.net","name":"os","value":{"family":"RedHat","release":{"major":"9"}}}, + {"certname":"host.example.net","name":"processorcount","value":4} + ]`) var warnings []string c := New(srv.URL, 5*time.Second) @@ -268,10 +305,7 @@ func TestFactsSkipsUndecodableRecords(t *testing.T) { } func TestFactsWithNothingDecodableIsAnError(t *testing.T) { - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - _, _ = w.Write([]byte(`[{"name":"a"},{"name":"b"}]`)) - })) - defer srv.Close() + srv := factsServer(t, `[{"name":"a"},{"name":"b"}]`) _, err := New(srv.URL, 5*time.Second).Facts(context.Background(), "host.example.net") if err == nil { @@ -283,8 +317,9 @@ func TestFactsWithNothingDecodableIsAnError(t *testing.T) { } func TestFactsByName(t *testing.T) { - var gotQuery string + var gotQuery, gotPath string srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotPath = r.URL.Path gotQuery = r.URL.Query().Get("query") w.Header().Set("Content-Type", "application/json") _, _ = w.Write([]byte(`[ @@ -298,6 +333,9 @@ func TestFactsByName(t *testing.T) { if err != nil { t.Fatalf("FactsByName: %v", err) } + if gotPath != "/pdb/query/v4/facts" { + t.Errorf("path = %s", gotPath) + } if gotQuery != `["extract",["certname","name","value"],["or",["=","name","country"],["=","name","os"]]]` { t.Errorf("query = %s", gotQuery) } @@ -336,3 +374,29 @@ func TestFactsByNameServerError(t *testing.T) { t.Fatal("expected an error for a 500") } } + +func TestFactsByNameSkipsUndecodableRecords(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte("\ufeff" + `[ + {"certname":"a.example.net","name":"country","value":"au"}, + {"certname":"a.example.net","name":"broken"} + ]`)) + })) + defer srv.Close() + + var warnings []string + c := New(srv.URL, 5*time.Second) + c.Warn = func(msg string) { warnings = append(warnings, msg) } + + records, err := c.FactsByName(context.Background(), []string{"country", "broken"}) + if err != nil { + t.Fatalf("FactsByName: %v", err) + } + if len(records) != 1 || records[0].Name != "country" { + t.Fatalf("records = %#v", records) + } + if len(warnings) != 1 || !strings.Contains(warnings[0], "broken") { + t.Errorf("the skipped fact must be reported, got %v", warnings) + } +} diff --git a/internal/puppetserver/puppetserver.go b/internal/puppetserver/puppetserver.go index 7a2720c..f8f47e0 100644 --- a/internal/puppetserver/puppetserver.go +++ b/internal/puppetserver/puppetserver.go @@ -1,5 +1,5 @@ // Package puppetserver compiles catalogs against an OpenVoxServer/Puppet -// Server 8 compiler over client mTLS. It is a separate client from pdbmux: +// Server 8 compiler over client mTLS. It is a separate client from puppetdb: // different endpoint, different auth model. package puppetserver