Skip to content

fix: correct the v4 catalog envelope and compiler edge wire shapes, a… - #1

Merged
alvagante merged 2 commits into
feat/piace-implementationfrom
devel
Aug 25, 2026
Merged

fix: correct the v4 catalog envelope and compiler edge wire shapes, a…#1
alvagante merged 2 commits into
feat/piace-implementationfrom
devel

Conversation

@alvagante

Copy link
Copy Markdown
Member

…dd --debug

Two wire-shape assumptions were wrong against a real compiler, each making a successful compilation surface as a PIACE failure.

v4 response envelope. POST /puppet/v4/catalog returns {"catalog": <document>}; POST /puppet/v3/catalog/:node returns the document directly. PIACE read both unwrapped, so a v4 body decoded into wireCatalog with every field absent and was reported as "malformed or unparseable compiler catalog response" against a compiler whose own log recorded a successful compile. catalogDocument now unwraps, keyed on the API that served the response — never target.Candidate.CatalogAPI, which is v4 on the permitted v4-to-v3 fallback path while the response in hand came from v3. No shape sniffing: that would accept either envelope from either endpoint and mask a compiler returning the wrong one.

Sources: puppetserver's compiler.rb returns { catalog: catalog } and master_core.clj's v4-catalog-fn encodes it verbatim; openvox's api/docs/http_catalog.md shows the bare v3 document.

Edge vertex form. A compiler serializes each edge vertex as a Type[title] reference string (Puppet::Relationship#to_data_hash calls source.to_s/target.to_s), not the {type, title} object of PuppetDB's wire format v8 — that object form is produced by the PuppetDB terminus, which converts the strings itself in munge_edges. resourceSpecWire now accepts both, splitting the string with a faithful port of the terminus's own resource_ref_to_hash regex: type stops at the first bracket, title runs greedily to the last, so a composite title such as File[/etc/foo[bar]] survives. Using that exact regex is what makes a candidate identity line up with the PuppetDB baseline's stored source_type/source_title, which the same function produced. A non-matching reference is a reported normalization diagnostic; the Ruby original yields {nil, nil} there, which this package's contract forbids.

Both doc.go files stated the wrong shape outright and are corrected in place, citing the compiler and terminus sources rather than the previous "documented but unverified" framing.

The test fixtures encoded both bugs: internal/compiler served one unwrapped shape from both endpoints, and cmd/piace's compilerCatalog built edge vertices as objects no compiler emits. Both are corrected, with regressions pinning each envelope in both directions, the fallback path's v3 shape, composite titles, mixed vertex forms, and unparseable references. One acceptance test was using the catalogs map to inject a 200 {"error":...} body; that moved to an explicit rawBodies hatch, since a real semantic rejection arrives non-2xx.

--debug. Both failures were diagnosable from one fact PIACE never showed: the response body's top-level keys. An Observer hook on transport.Client.Do — the single chokepoint every compiler and PuppetDB request passes through — feeds two options on all three subcommands:

--debug method, URL, status, duration, body sizes, content
type, and the response's top-level JSON member
names, to stderr. Member names, not values, so it
stays inside requirements.md 3.5 and is safe for a
CI log.
--debug-dump-dir DIR verbatim request/response bodies, 0600 files in a
0700 directory, never a console. Documented in
code and README as a deliberate, operator-requested
bypass of the redaction boundary rather than as
something safe.

Observation is inert: an acceptance test asserts --debug changes neither stdout nor the exit code.

…dd --debug

Two wire-shape assumptions were wrong against a real compiler, each
making a successful compilation surface as a PIACE failure.

v4 response envelope. `POST /puppet/v4/catalog` returns
`{"catalog": <document>}`; `POST /puppet/v3/catalog/:node` returns the
document directly. PIACE read both unwrapped, so a v4 body decoded into
wireCatalog with every field absent and was reported as "malformed or
unparseable compiler catalog response" against a compiler whose own log
recorded a successful compile. catalogDocument now unwraps, keyed on the
API that served the response — never target.Candidate.CatalogAPI, which
is v4 on the permitted v4-to-v3 fallback path while the response in hand
came from v3. No shape sniffing: that would accept either envelope from
either endpoint and mask a compiler returning the wrong one.

Sources: puppetserver's compiler.rb returns `{ catalog: catalog }` and
master_core.clj's v4-catalog-fn encodes it verbatim; openvox's
api/docs/http_catalog.md shows the bare v3 document.

Edge vertex form. A compiler serializes each edge vertex as a
`Type[title]` reference string (Puppet::Relationship#to_data_hash calls
source.to_s/target.to_s), not the `{type, title}` object of PuppetDB's
wire format v8 — that object form is produced by the PuppetDB terminus,
which converts the strings itself in munge_edges. resourceSpecWire now
accepts both, splitting the string with a faithful port of the terminus's
own resource_ref_to_hash regex: type stops at the first bracket, title
runs greedily to the last, so a composite title such as
`File[/etc/foo[bar]]` survives. Using that exact regex is what makes a
candidate identity line up with the PuppetDB baseline's stored
source_type/source_title, which the same function produced. A
non-matching reference is a reported normalization diagnostic; the Ruby
original yields {nil, nil} there, which this package's contract forbids.

Both doc.go files stated the wrong shape outright and are corrected in
place, citing the compiler and terminus sources rather than the previous
"documented but unverified" framing.

The test fixtures encoded both bugs: internal/compiler served one
unwrapped shape from both endpoints, and cmd/piace's compilerCatalog
built edge vertices as objects no compiler emits. Both are corrected,
with regressions pinning each envelope in both directions, the fallback
path's v3 shape, composite titles, mixed vertex forms, and unparseable
references. One acceptance test was using the catalogs map to inject a
200 `{"error":...}` body; that moved to an explicit rawBodies hatch,
since a real semantic rejection arrives non-2xx.

--debug. Both failures were diagnosable from one fact PIACE never showed:
the response body's top-level keys. An Observer hook on
transport.Client.Do — the single chokepoint every compiler and PuppetDB
request passes through — feeds two options on all three subcommands:

  --debug              method, URL, status, duration, body sizes, content
                       type, and the response's top-level JSON member
                       names, to stderr. Member names, not values, so it
                       stays inside requirements.md 3.5 and is safe for a
                       CI log.
  --debug-dump-dir DIR verbatim request/response bodies, 0600 files in a
                       0700 directory, never a console. Documented in
                       code and README as a deliberate, operator-requested
                       bypass of the redaction boundary rather than as
                       something safe.

Observation is inert: an acceptance test asserts --debug changes neither
stdout nor the exit code.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings August 25, 2026 11:10

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Every /puppet/v3/ route is served by the compiler's embedded Ruby Puppet
request handler, which rejects a request carrying no Accept header before
doing any work ("Missing required Accept header", HTTP 400). Neither the
v3 catalog request nor the v3 file_content request sent one, so both fail
against a real compiler while every fake in the test suite accepted them.
The value is endpoint-specific: application/json for the catalog endpoint,
application/octet-stream for file_content, which rejects application/json
with HTTP 406. The v4 catalog route is served directly and needs no
header, so buildV4Request deliberately still sends none.

Verified against a deployed OpenVox compiler, which also settled two
assumptions the docs carried:

- OpenVox serves POST /puppet/v4/catalog, accepts trusted_facts, and
  honours the request's persistence field. The spec's "OpenVox is v3
  only" stance was wrong; Puppet Server and OpenVox present the same
  contract, and the adapter now documents that it has no
  implementation-specific branch.
- A v3 catalog request has no persistence control. One request for a
  previously unknown certname left PuppetDB holding a factset and a
  catalog under the requested environment, the stored catalog carrying
  the request's own transaction_uuid. The same request over v4 with
  persistence {facts: false, catalog: false} left nothing behind.

The second finding makes a PuppetDB baseline impossible with v3: the
candidate compilation overwrites the very catalog the baseline reads, so
a v3 run destroys its own input and fails on the next target with a
baseline-environment mismatch. Requirements 1.7-1.8 now state that v4
satisfies 1.6 by disabling persistence, and that any target which can
compile over v3 -- catalog_api: v3, or v4 with allow_v3_fallback: true --
requires baseline.source: file. Requirement 1.6 itself is unchanged: v3's
inability to satisfy it is the reason v3 is a degraded path, not a reason
to weaken the invariant.

Also documents the Accept contract and the v4 response envelope in
requirements section 7, adds the auth.conf rules a catalog-reader
certificate needs, and records task 13 for the two parts not implemented
here: rejecting a v3-capable target with a PuppetDB baseline during
configuration resolution, and extending the non-suppressible v3 warning
to state the PuppetDB mutation alongside the $trusted caveat.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@alvagante
alvagante merged commit eb81687 into feat/piace-implementation Aug 25, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants