From 5cbe4c87136701ab643658044a13a8cdde9580cd Mon Sep 17 00:00:00 2001 From: unkin-agent Date: Sat, 12 Sep 2026 17:35:09 +1000 Subject: [PATCH 1/2] stop serving another node's factset Also strip a leading BOM and stop reporting a certname mismatch as a parse error. --- internal/pdbmux/pdbmux.go | 52 ++++++++++++----- internal/pdbmux/pdbmux_test.go | 103 ++++++++++++++++++++++++++++++++- 2 files changed, 140 insertions(+), 15 deletions(-) diff --git a/internal/pdbmux/pdbmux.go b/internal/pdbmux/pdbmux.go index ef43d6b..c99b042 100644 --- a/internal/pdbmux/pdbmux.go +++ b/internal/pdbmux/pdbmux.go @@ -83,7 +83,7 @@ func (c *Client) Facts(ctx context.Context, certname string) (map[string]any, er records, err := decodeFactsets(body, certname) if err != nil { - return nil, fmt.Errorf("parsing factset for %s: %w", certname, err) + return nil, err } if len(records) == 0 { return nil, fmt.Errorf("%w: %s", ErrNodeNotFound, certname) @@ -93,34 +93,60 @@ func (c *Client) Facts(ctx context.Context, certname string) (map[string]any, er // decodeFactsets reads the two shapes /factsets/ comes back as: real // PuppetDB returns an array of factsets, pdbmux currently returns a single -// bare object. +// bare object. A factset belonging to another node is never substituted for +// the one that was asked for. func decodeFactsets(body []byte, certname string) ([]factRecord, error) { + body = bytes.TrimPrefix(body, []byte("\xef\xbb\xbf")) + 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 + return nil, fmt.Errorf("parsing factset for %s: %w", certname, err) } if len(sets) == 0 { return nil, nil } - return nil, fmt.Errorf("%d factsets returned, none for the requested certname", len(sets)) + for _, fs := range sets { + if sameCertname(fs.Certname, certname) { + return fs.Facts.Data, nil + } + } + if len(sets) == 1 && sets[0].Certname == "" { + return sets[0].Facts.Data, nil + } + return nil, fmt.Errorf("no factset for %s: pdbmux returned %s", + certname, strings.Join(certnames(sets), ", ")) } var fs factset if err := json.Unmarshal(body, &fs); err != nil { - return nil, err + return nil, fmt.Errorf("parsing factset for %s: %w", certname, err) + } + if fs.Certname != "" && !sameCertname(fs.Certname, certname) { + return nil, fmt.Errorf("no factset for %s: pdbmux returned %s", certname, fs.Certname) } return fs.Facts.Data, nil } +// sameCertname compares certnames case-insensitively: they are DNS names, and +// PuppetDB matches the per-node route exactly, so folding can only absorb a +// backend that normalised case, never mask a different node. +func sameCertname(got, want string) bool { + return got != "" && strings.EqualFold(got, want) +} + +func certnames(sets []factset) []string { + names := make([]string, 0, len(sets)) + for _, fs := range sets { + if fs.Certname == "" { + names = append(names, "(unnamed)") + continue + } + names = append(names, fs.Certname) + } + return names +} + 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 { diff --git a/internal/pdbmux/pdbmux_test.go b/internal/pdbmux/pdbmux_test.go index d8149fe..888315a 100644 --- a/internal/pdbmux/pdbmux_test.go +++ b/internal/pdbmux/pdbmux_test.go @@ -225,8 +225,107 @@ func TestFactsRejectsAFactsetArrayWithNoMatch(t *testing.T) { 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) + assertNamesBoth(t, err, "host.example.net", "a.example.net", "b.example.net") +} + +// 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() + 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 factset") { + t.Errorf("a certname mismatch is not a parse failure: %v", err) + } +} + +func TestFactsRejectsASoleFactsetForAnotherNode(t *testing.T) { + srv := factsetServer(t, `[{ + "certname":"totally-different-host.example.net", + "facts":{"data":[ + {"name":"id","value":"nobody"}, + {"name":"os","value":{"family":"Debian"}} + ]} + }]`) + + facts, err := New(srv.URL, 5*time.Second).Facts(context.Background(), "host.example.net") + if err == nil { + t.Fatalf("another node's facts were served as ours: %#v", facts) + } + assertNamesBoth(t, err, "host.example.net", "totally-different-host.example.net") +} + +func TestFactsRejectsABareFactsetForAnotherNode(t *testing.T) { + srv := factsetServer(t, `{ + "certname":"totally-different-host.example.net", + "facts":{"data":[{"name":"id","value":"nobody"}]} + }`) + + facts, err := New(srv.URL, 5*time.Second).Facts(context.Background(), "host.example.net") + if err == nil { + t.Fatalf("another node's facts were served as ours: %#v", facts) + } + assertNamesBoth(t, err, "host.example.net", "totally-different-host.example.net") +} + +// The per-node route already names the node, so a factset that asserts no +// certname of its own cannot be contradicting it. +func TestFactsAcceptsAFactsetWithNoCertname(t *testing.T) { + body := `{"facts":{"data":[ + {"name":"id","value":"root"}, + {"name":"os","value":{"family":"RedHat","release":{"major":"9"}}}, + {"name":"processorcount","value":4} + ]}}` + + for name, resp := range map[string]string{"object": body, "array": "[" + body + "]"} { + t.Run(name, func(t *testing.T) { + srv := factsetServer(t, resp) + 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 := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if strings.HasPrefix(r.URL.Path, "/pdb/query/v4/factsets/") { + _, _ = w.Write([]byte(factsetBody)) + return + } + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(`{"error":"unknown node"}`)) + })) + 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 TestFactsToleratesALeadingBOM(t *testing.T) { + for name, resp := range map[string]string{ + "array": "\ufeff" + factsetBody, + "object": "\ufeff" + bareFactsetBody, + } { + t.Run(name, func(t *testing.T) { + srv := factsetServer(t, resp) + 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) + }) } } From 3f14554faed70d10359bb5f5f9f38c9a3c44d667 Mon Sep 17 00:00:00 2001 From: unkin-agent Date: Sat, 12 Sep 2026 17:49:57 +1000 Subject: [PATCH 2/2] read facts from PuppetDB's query API - rename internal/pdbmux to internal/puppetdb - fetch facts with one GET /pdb/query/v4/facts AST query per node - drop the per-node fallback route and its two response shapes - rename --pdb to --puppetdb, CATALOG_DIFF_PDB to CATALOG_DIFF_PUPPETDB and pdb_url to puppetdb_url - describe the fact source generically in the README and AGENTS.md --- AGENTS.md | 12 +- README.md | 12 +- cmd/catalog-diff/main.go | 10 +- cmd/catalog-diff/main_test.go | 12 +- internal/config/config.go | 15 +- internal/config/config_test.go | 8 +- internal/pdbmux/pdbmux.go | 204 ----------- internal/puppetdb/puppetdb.go | 182 ++++++++++ .../puppetdb_test.go} | 339 +++++++----------- internal/puppetserver/puppetserver.go | 2 +- 10 files changed, 355 insertions(+), 441 deletions(-) delete mode 100644 internal/pdbmux/pdbmux.go create mode 100644 internal/puppetdb/puppetdb.go rename internal/{pdbmux/pdbmux_test.go => puppetdb/puppetdb_test.go} (53%) diff --git a/AGENTS.md b/AGENTS.md index 3783807..caa3e4a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -17,7 +17,7 @@ cmd/catalog-diff/ # the CLI (flags, exit codes, orchestration) internal/catalog/ # catalog types, normalisation, diff engine, renderers, # branch.go (BranchToEnvironment) internal/puppetserver/ # mTLS client for POST /puppet/v4/catalog + /status/v1/simple -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 @@ -34,10 +34,10 @@ Makefile and to `scripts/build-rpm.sh`, and add its files to `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. +- 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. - The two services have different auth models and stay separate clients. ## Gotchas @@ -78,7 +78,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 d67ec4e..d39bd41 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ CLI tooling for the Puppet estate. Currently one binary: the environment built from the branch under test, then diffs the two. 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 @@ -33,7 +33,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) | @@ -67,7 +67,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 @@ -77,15 +77,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 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/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/pdbmux/pdbmux.go b/internal/pdbmux/pdbmux.go deleted file mode 100644 index c99b042..0000000 --- a/internal/pdbmux/pdbmux.go +++ /dev/null @@ -1,204 +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, 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. A factset belonging to another node is never substituted for -// the one that was asked for. -func decodeFactsets(body []byte, certname string) ([]factRecord, error) { - body = bytes.TrimPrefix(body, []byte("\xef\xbb\xbf")) - - if bytes.HasPrefix(bytes.TrimLeft(body, " \t\r\n"), []byte("[")) { - var sets []factset - if err := json.Unmarshal(body, &sets); err != nil { - return nil, fmt.Errorf("parsing factset for %s: %w", certname, err) - } - if len(sets) == 0 { - return nil, nil - } - for _, fs := range sets { - if sameCertname(fs.Certname, certname) { - return fs.Facts.Data, nil - } - } - if len(sets) == 1 && sets[0].Certname == "" { - return sets[0].Facts.Data, nil - } - return nil, fmt.Errorf("no factset for %s: pdbmux returned %s", - certname, strings.Join(certnames(sets), ", ")) - } - - var fs factset - if err := json.Unmarshal(body, &fs); err != nil { - return nil, fmt.Errorf("parsing factset for %s: %w", certname, err) - } - if fs.Certname != "" && !sameCertname(fs.Certname, certname) { - return nil, fmt.Errorf("no factset for %s: pdbmux returned %s", certname, fs.Certname) - } - return fs.Facts.Data, nil -} - -// sameCertname compares certnames case-insensitively: they are DNS names, and -// PuppetDB matches the per-node route exactly, so folding can only absorb a -// backend that normalised case, never mask a different node. -func sameCertname(got, want string) bool { - return got != "" && strings.EqualFold(got, want) -} - -func certnames(sets []factset) []string { - names := make([]string, 0, len(sets)) - for _, fs := range sets { - if fs.Certname == "" { - names = append(names, "(unnamed)") - continue - } - names = append(names, fs.Certname) - } - return names -} - -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 -} diff --git a/internal/puppetdb/puppetdb.go b/internal/puppetdb/puppetdb.go new file mode 100644 index 0000000..b3812a1 --- /dev/null +++ b/internal/puppetdb/puppetdb.go @@ -0,0 +1,182 @@ +// 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}, + } +} + +type factRecord struct { + Certname string `json:"certname"` + Name string `json:"name"` + Value json.RawMessage `json:"value"` +} + +// Facts returns the node's facts as the values map a catalog request needs. +// It reads /pdb/query/v4/facts with an AST query selecting the certname, which +// answers with a flat array of fact records. +func (c *Client) Facts(ctx context.Context, certname string) (map[string]any, error) { + query, err := json.Marshal([]string{"=", "certname", certname}) + if err != nil { + return nil, fmt.Errorf("building facts query for %s: %w", certname, err) + } + + body, status, err := c.get(ctx, "/pdb/query/v4/facts", string(query)) + 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) +} + +// decodeFacts reads the fact records the 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) { + 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", certname, 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 + } + 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 +} diff --git a/internal/pdbmux/pdbmux_test.go b/internal/puppetdb/puppetdb_test.go similarity index 53% rename from internal/pdbmux/pdbmux_test.go rename to internal/puppetdb/puppetdb_test.go index 888315a..7d9b450 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,62 +185,6 @@ 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 { - 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) - } - 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} - ]}} - ]`) - - 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 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"}]}} - ]`) - - _, 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") - } - assertNamesBoth(t, err, "host.example.net", "a.example.net", "b.example.net") -} - // 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) { @@ -240,19 +197,16 @@ func assertNamesBoth(t *testing.T, err error, want string, got ...string) { t.Errorf("error does not name the returned certname %s: %v", g, err) } } - if strings.Contains(err.Error(), "parsing factset") { + if strings.Contains(err.Error(), "parsing facts") { t.Errorf("a certname mismatch is not a parse failure: %v", err) } } -func TestFactsRejectsASoleFactsetForAnotherNode(t *testing.T) { - srv := factsetServer(t, `[{ - "certname":"totally-different-host.example.net", - "facts":{"data":[ - {"name":"id","value":"nobody"}, - {"name":"os","value":{"family":"Debian"}} - ]} - }]`) +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 { @@ -261,50 +215,52 @@ func TestFactsRejectsASoleFactsetForAnotherNode(t *testing.T) { assertNamesBoth(t, err, "host.example.net", "totally-different-host.example.net") } -func TestFactsRejectsABareFactsetForAnotherNode(t *testing.T) { - srv := factsetServer(t, `{ - "certname":"totally-different-host.example.net", - "facts":{"data":[{"name":"id","value":"nobody"}]} - }`) +func TestFactsRejectsAMixedResponseNamingEveryNode(t *testing.T) { + srv := factsServer(t, `[ + {"certname":"a.example.net","name":"id","value":"a"}, + {"certname":"b.example.net","name":"id","value":"b"} + ]`) - facts, err := New(srv.URL, 5*time.Second).Facts(context.Background(), "host.example.net") + _, err := New(srv.URL, 5*time.Second).Facts(context.Background(), "host.example.net") if err == nil { - t.Fatalf("another node's facts were served as ours: %#v", facts) + t.Fatal("expected an error when no record matches the certname") } - assertNamesBoth(t, err, "host.example.net", "totally-different-host.example.net") + assertNamesBoth(t, err, "host.example.net", "a.example.net", "b.example.net") } -// The per-node route already names the node, so a factset that asserts no -// certname of its own cannot be contradicting it. -func TestFactsAcceptsAFactsetWithNoCertname(t *testing.T) { - body := `{"facts":{"data":[ +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} + ]`) + + 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} - ]}}` + ]`) - for name, resp := range map[string]string{"object": body, "array": "[" + body + "]"} { - t.Run(name, func(t *testing.T) { - srv := factsetServer(t, resp) - 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) - }) + 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 := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if strings.HasPrefix(r.URL.Path, "/pdb/query/v4/factsets/") { - _, _ = w.Write([]byte(factsetBody)) - return - } - w.WriteHeader(http.StatusNotFound) - _, _ = w.Write([]byte(`{"error":"unknown node"}`)) - })) - defer srv.Close() + srv := factsServer(t, factsBody) facts, err := New(srv.URL, 5*time.Second).Facts(context.Background(), "HOST.example.net") if err != nil { @@ -314,40 +270,22 @@ func TestFactsMatchesCertnameCaseInsensitively(t *testing.T) { } func TestFactsToleratesALeadingBOM(t *testing.T) { - for name, resp := range map[string]string{ - "array": "\ufeff" + factsetBody, - "object": "\ufeff" + bareFactsetBody, - } { - t.Run(name, func(t *testing.T) { - srv := factsetServer(t, resp) - 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) - }) - } -} + srv := factsServer(t, "\ufeff"+factsBody) -func TestFactsEmptyFactsetArrayIsAMissingNode(t *testing.T) { - srv := factsetServer(t, `[]`) - - _, 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) } 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) @@ -367,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 { 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