diff --git a/AGENTS.md b/AGENTS.md index 8da86bf..3783807 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -35,7 +35,9 @@ Makefile and to `scripts/build-rpm.sh`, and add its files to 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. + `/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. - The two services have different auth models and stay separate clients. ## Gotchas @@ -58,6 +60,12 @@ Makefile and to `scripts/build-rpm.sh`, and add its files to preserved). The rule lives in one function, `catalog.BranchToEnvironment`. - Exit codes are a contract: 0 same, 1 compile failed, 2 differences, 3 transport/auth/mismatch/usage. `--testonly` never returns 2. +- Numeric flags are validated up front (`validate`). A negative `--retry` used + to skip the compile loop and leave a nil error inside `exitError`; that type + now prints safely whatever it holds. +- The two compiles share a cancellable context: the first failure abandons its + sibling. An error that is only that cancellation is not the cause and is not + what gets reported. - Normalisation carries a pile of false-positive fixes (classes and exported resources skipped, `before`/`require` dropped, `notify`/`subscribe` sorted, File titles keyed by `path`, datatype-only changes suppressed). Removing one @@ -74,7 +82,8 @@ one for pdbmux) — no live estate access is required. Renderer output is covered by golden files under `internal/catalog/testdata`; regenerate with `go test ./internal/catalog -update`. -`internal/puppetserver/testdata/catalog_response.json` is a trimmed capture of -a real OpenVoxServer 8.8 response, kept for its shape: the `catalog` envelope -and resources that omit `parameters`, `file` and `line`. Sensitive values are -replaced. +`internal/puppetserver/testdata/catalog_response.json` is a synthetic +OpenVoxServer 8.8 response, kept for its shape: the `catalog` envelope, a +string `version`, a null `code_id`, and resources that omit `parameters`, +`file` and `line`, plus the exported and `sensitive_parameters` cases. This +repo is public — hostnames, class names and paths in fixtures stay fake. diff --git a/README.md b/README.md index 90d7eb3..d67ec4e 100644 --- a/README.md +++ b/README.md @@ -35,8 +35,8 @@ catalog-diff web01.example.net --branch benvin/motd-change --testonly | `--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`) | | `--cert` / `--key` / `--cacert` | client mTLS material | -| `--timeout` | per-compile timeout (default 10m) | -| `--retry` | retries for transient compile or transport failures (default 2; a failed compiler-side PuppetDB query counts, a manifest error does not) | +| `--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) | | `--include-classes` | diff `Class` resources (ignored by default; they manage nothing) | | `--ignore` | ignore resources matching `Type[Title]` (glob), repeatable | | `--ignore-attr` | ignore a parameter on every resource, repeatable | diff --git a/internal/catalog/diff_test.go b/internal/catalog/diff_test.go index e2b0c0e..3f4fd1c 100644 --- a/internal/catalog/diff_test.go +++ b/internal/catalog/diff_test.go @@ -309,15 +309,15 @@ func TestDiffNoChanges(t *testing.T) { } } -// Real resources from a live OpenVoxServer 8.8 catalog. Class, stage and -// anchor resources carry no "parameters" key, and resources with no manifest -// location carry neither "file" nor "line". +// Resources shaped like an OpenVoxServer 8.8 catalog. Class, stage and anchor +// resources carry no "parameters" key, and resources with no manifest location +// carry neither "file" nor "line". const optionalFieldCatalog = `{ - "name":"prodnxsr0009.main.unkin.net","environment":"develop","version":"979c188", + "name":"web01.example.test","environment":"develop","version":"0123abc", "resources":[ {"type":"Class","title":"Settings","tags":["class","settings"],"exported":false,"kind":"unknown"}, - {"type":"Anchor","title":"selinux::end","tags":["anchor","selinux::end"],"file":"init.pp","line":111,"exported":false,"kind":"compilable_type"}, - {"type":"File","title":"/opt/puppetlabs/puppet/cache/puppet-selinux/modules/frr_local.loaded","tags":["file"],"file":"module.pp","line":177,"exported":false,"kind":"compilable_type"}, + {"type":"Anchor","title":"examplemod::end","tags":["anchor","examplemod::end"],"file":"init.pp","line":111,"exported":false,"kind":"compilable_type"}, + {"type":"File","title":"/opt/puppetlabs/puppet/cache/examplemod/modules/example_local.loaded","tags":["file"],"file":"module.pp","line":177,"exported":false,"kind":"compilable_type"}, {"type":"Stage","title":"main","tags":["stage"],"exported":false,"kind":"compilable_type","parameters":{"name":"main"}} ] }` @@ -325,7 +325,7 @@ const optionalFieldCatalog = `{ func TestNormalizeAcceptsMissingParameters(t *testing.T) { res := normalized(t, optionalFieldCatalog, Options{IncludeClasses: true}) - for _, key := range []string{"Class[Settings]", "Anchor[selinux::end]"} { + for _, key := range []string{"Class[Settings]", "Anchor[examplemod::end]"} { r, ok := res[key] if !ok { t.Fatalf("%s dropped: %v", key, res) @@ -376,7 +376,7 @@ func TestNormalizeKeepsMissingSourceDistinctFromLineZero(t *testing.T) { t.Errorf("an absent line must stay absent, got %d", *settings.Line) } - anchor := res["Anchor[selinux::end]"] + anchor := res["Anchor[examplemod::end]"] if !anchor.HasSource() || anchor.File != "init.pp" || *anchor.Line != 111 { t.Errorf("manifest location lost: %+v", anchor) } diff --git a/internal/puppetserver/puppetserver_test.go b/internal/puppetserver/puppetserver_test.go index f05a4dd..b108373 100644 --- a/internal/puppetserver/puppetserver_test.go +++ b/internal/puppetserver/puppetserver_test.go @@ -270,8 +270,8 @@ func TestTransactionUUIDUnique(t *testing.T) { } } -// wrappedResponse is a trimmed capture of a real OpenVoxServer 8.8 response: -// the catalog sits under a single "catalog" key. +// wrappedResponse is a synthetic OpenVoxServer 8.8 response shaped like the +// real thing: the catalog sits under a single "catalog" key. func wrappedResponse(t *testing.T) []byte { t.Helper() raw, err := os.ReadFile(filepath.Join("testdata", "catalog_response.json")) @@ -288,7 +288,7 @@ func compileFixture(t *testing.T, body []byte, environment string) (*catalog.Cat })) defer srv.Close() return tlsClient(t, srv).Compile(context.Background(), CompileRequest{ - Certname: "prodnxsr0009.main.unkin.net", Environment: environment, + Certname: "web01.example.test", Environment: environment, }) } @@ -297,14 +297,14 @@ func TestCompileUnwrapsServerResponse(t *testing.T) { if err != nil { t.Fatalf("Compile: %v", err) } - if cat.Environment != "develop" || cat.Name != "prodnxsr0009.main.unkin.net" { + if cat.Environment != "develop" || cat.Name != "web01.example.test" { t.Fatalf("envelope not unwrapped: %+v", cat) } if len(cat.Resources) != 9 || len(cat.Classes) == 0 || len(cat.Tags) == 0 { t.Fatalf("catalog body lost: %d resources, %d classes, %d tags", len(cat.Resources), len(cat.Classes), len(cat.Tags)) } - if string(cat.Version) != `"979c188c3437099863e0b53c57942ef7ed6523a3"` { + if string(cat.Version) != `"0123456789abcdef0123456789abcdef01234567"` { t.Errorf("version = %s, want the string form the server sends", cat.Version) } } @@ -346,7 +346,7 @@ func TestCompileDecodesOptionalResourceFields(t *testing.T) { t.Errorf("a resource without file/line must report no source: %+v", settings) } - anchor := byKey["Anchor[selinux::end]"] + anchor := byKey["Anchor[examplemod::end]"] if anchor.Parameters != nil { t.Errorf("anchor parameters should be absent: %+v", anchor.Parameters) } @@ -354,17 +354,17 @@ func TestCompileDecodesOptionalResourceFields(t *testing.T) { t.Errorf("anchor manifest location lost: %+v", anchor) } - pkg := byKey["Package[openvox-agent]"] - if pkg.Params()["ensure"] != "7.37.2" || !pkg.HasSource() { + pkg := byKey["Package[example-agent]"] + if pkg.Params()["ensure"] != "1.2.3" || !pkg.HasSource() { t.Errorf("package decoded wrong: %+v", pkg) } - key := byKey["File[/etc/dns-updater/key]"] + key := byKey["File[/etc/example/secret.key]"] if len(key.SensitiveParameters) != 1 || key.SensitiveParameters[0] != "content" { t.Errorf("per-resource sensitive_parameters lost: %+v", key) } - member := byKey["Haproxy::Balancermember[prodnxsr0009.main.unkin.net_9443]"] + member := byKey["Haproxy::Balancermember[web01.example.test_9443]"] if !member.Exported { t.Errorf("exported flag lost: %+v", member) } @@ -396,10 +396,10 @@ func TestCompileErrorTransientClassification(t *testing.T) { const pdbFailure = "Internal Server Error: org.jruby.exceptions.RuntimeError: (PreformattedError) Evaluation Error: " + "Error while evaluating a Resource Statement, Evaluation Error: Error while evaluating a Function Call, " + "Failed to execute '/pdb/query/v4' on at least 1 of the following 'server_urls': https://puppetdb.example.net " + - "(file: /etc/puppetlabs/code/environments/develop/site/profiles/manifests/consul/client.pp, line: 31, column: 29) " + + "(file: /etc/puppetlabs/code/environments/develop/site/profiles/manifests/app/client.pp, line: 31, column: 29) " + "on node host.example.net" const missingClass = "Internal Server Error: org.jruby.exceptions.RuntimeError: (Error) " + - "Could not find class roles::infra::incus::node for host.example.net on node host.example.net" + "Could not find class roles::app::demo::node for host.example.net on node host.example.net" tests := []struct { name string diff --git a/internal/puppetserver/testdata/catalog_response.json b/internal/puppetserver/testdata/catalog_response.json index 3a7d82b..827d488 100644 --- a/internal/puppetserver/testdata/catalog_response.json +++ b/internal/puppetserver/testdata/catalog_response.json @@ -2,12 +2,12 @@ "catalog": { "tags": [ "settings", - "roles::infra::incus::node" + "roles::app::demo::node" ], - "name": "prodnxsr0009.main.unkin.net", - "version": "979c188c3437099863e0b53c57942ef7ed6523a3", + "name": "web01.example.test", + "version": "0123456789abcdef0123456789abcdef01234567", "code_id": null, - "catalog_uuid": "8d7f423e-5bf1-4b64-97b4-4a20c67ec59f", + "catalog_uuid": "00000000-0000-4000-8000-0000000000aa", "catalog_format": 2, "environment": "develop", "resources": [ @@ -35,129 +35,124 @@ }, { "type": "Class", - "title": "Profiles::Puppet::Agent", + "title": "Profiles::App::Agent", "tags": [ "class", - "profiles::puppet::agent", + "profiles::app::agent", "profiles", - "puppet", + "app", "agent", "profiles::base", "base", - "roles::infra::incus::node", + "roles::app::demo::node", "roles", - "infra", - "incus", + "demo", "node" ], "exported": false, "kind": "unknown", "parameters": { - "version": "7.37.2", - "openvox_enable": true + "version": "1.2.3", + "manage_service": true } }, { "type": "Anchor", - "title": "selinux::end", + "title": "examplemod::end", "tags": [ "anchor", - "selinux::end", - "selinux", + "examplemod::end", + "examplemod", "end", "class", - "selinux::module", + "examplemod::module", "module", - "frr_local", - "profiles::selinux::frr", + "example_local", + "profiles::app::module", "profiles", - "frr", + "app", "profiles::base", "base", - "roles::infra::incus::node", + "roles::app::demo::node", "roles", - "infra", - "incus", + "demo", "node" ], - "file": "/etc/puppetlabs/code/environments/develop/external_modules/selinux/manifests/init.pp", + "file": "/etc/puppetlabs/code/environments/develop/external_modules/examplemod/manifests/init.pp", "line": 111, "exported": false, "kind": "compilable_type" }, { "type": "File", - "title": "/opt/puppetlabs/puppet/cache/puppet-selinux/modules/frr_local.loaded", + "title": "/opt/puppetlabs/puppet/cache/examplemod/modules/example_local.loaded", "tags": [ "file", - "selinux::module", - "selinux", + "examplemod::module", + "examplemod", "module", - "frr_local", + "example_local", "class", - "profiles::selinux::frr", + "profiles::app::module", "profiles", - "frr", + "app", "profiles::base", "base", - "roles::infra::incus::node", + "roles::app::demo::node", "roles", - "infra", - "incus", + "demo", "node" ], - "file": "/etc/puppetlabs/code/environments/develop/external_modules/selinux/manifests/module.pp", + "file": "/etc/puppetlabs/code/environments/develop/external_modules/examplemod/manifests/module.pp", "line": 177, "exported": false, "kind": "compilable_type" }, { "type": "Package", - "title": "openvox-agent", + "title": "example-agent", "tags": [ "package", - "openvox-agent", + "example-agent", "class", - "profiles::puppet::agent", + "profiles::app::agent", "profiles", - "puppet", + "app", "agent", "profiles::base", "base", - "roles::infra::incus::node", + "roles::app::demo::node", "roles", - "infra", - "incus", + "demo", "node" ], - "file": "/etc/puppetlabs/code/environments/develop/site/profiles/manifests/puppet/agent.pp", + "file": "/etc/puppetlabs/code/environments/develop/site/profiles/manifests/app/agent.pp", "line": 58, "exported": false, "kind": "compilable_type", "parameters": { - "ensure": "7.37.2", - "require": "Yumrepo[openvox]" + "ensure": "1.2.3", + "require": "Yumrepo[example]" } }, { "type": "Service", - "title": "puppet", + "title": "example-agent", "tags": [ "service", - "puppet", + "example-agent", "class", - "profiles::puppet::agent", + "profiles::app::agent", "profiles", "agent", "profiles::base", "base", - "roles::infra::incus::node", + "roles::app::demo::node", "roles", - "infra", - "incus", + "demo", "node" ], - "file": "/etc/puppetlabs/code/environments/develop/site/profiles/manifests/puppet/agent.pp", + "file": "/etc/puppetlabs/code/environments/develop/site/profiles/manifests/app/agent.pp", "line": 81, "exported": false, "kind": "compilable_type", @@ -165,29 +160,28 @@ "ensure": "running", "enable": true, "hasrestart": true, - "require": "Package[openvox-agent]" + "require": "Package[example-agent]" } }, { "type": "File", - "title": "/etc/dns-updater/key", + "title": "/etc/example/secret.key", "tags": [ "file", "class", - "profiles::dns::updater", + "profiles::app::secret", "profiles", - "dns", - "updater", - "profiles::dns::base", + "app", + "secret", + "profiles::app::base", "base", "profiles::base", - "roles::infra::incus::node", + "roles::app::demo::node", "roles", - "infra", - "incus", + "demo", "node" ], - "file": "/etc/puppetlabs/code/environments/develop/site/profiles/manifests/dns/updater.pp", + "file": "/etc/puppetlabs/code/environments/develop/site/profiles/manifests/app/secret.pp", "line": 97, "exported": false, "kind": "compilable_type", @@ -205,41 +199,40 @@ }, { "type": "Haproxy::Balancermember", - "title": "prodnxsr0009.main.unkin.net_9443", + "title": "web01.example.test_9443", "tags": [ - "be_ceph_dashboard_au-syd1-prod", + "be_example_dashboard_test", "haproxy::balancermember", "haproxy", "balancermember", - "prodnxsr0009.main.unkin.net_9443", - "profiles::haproxy::balancemember", + "web01.example.test_9443", + "profiles::app::balancermember", "profiles", - "balancemember", + "balancermember", "class", - "profiles::ceph::dashboard", - "ceph", + "profiles::app::dashboard", + "app", "dashboard", "profiles::base", "base", - "roles::infra::incus::node", + "roles::app::demo::node", "roles", - "infra", - "incus", + "demo", "node" ], - "file": "/etc/puppetlabs/code/environments/develop/site/profiles/manifests/haproxy/balancemember.pp", + "file": "/etc/puppetlabs/code/environments/develop/site/profiles/manifests/app/balancermember.pp", "line": 12, "exported": true, "kind": "defined_type", "parameters": { - "listening_service": "be_ceph_dashboard", + "listening_service": "be_example_dashboard", "ports": [ 9443 ], - "server_names": "prodnxsr0009", - "ipaddresses": "198.18.23.9", + "server_names": "web01", + "ipaddresses": "192.0.2.9", "options": [ - "cookie prodnxsr0009", + "cookie web01", "ssl", "verify none", "check", @@ -247,7 +240,7 @@ "rise 3", "fall 2" ], - "tag": "be_ceph_dashboard_au-syd1-prod" + "tag": "be_example_dashboard_test" } } ], @@ -263,7 +256,7 @@ ], "classes": [ "settings", - "profiles::puppet::agent" + "profiles::app::agent" ] } }