Repository navigation
feat(decdn_node)!: re-sync config schema, contracts and install path with decdn d306cc5c - #36
Merged
Merged
Conversation
…with decdn d306cc5c The repo had fallen ~180 commits behind decdn/decdn and was broken in two independent ways that fail at different stages of a deploy: 1. The pinned release does not exist. `git ls-remote --tags` on decdn/decdn returns zero tags — release.yml fires on a `v[0-9]*` tag push and has never run — so `make deploy` 404'd on the first get_url. The default install method is now `manual`, and the release-mode assert names the cause. 2. The role rendered 18 node.toml keys the daemon rejects. Every upstream config section is `#[serde(deny_unknown_fields)]` with no serde aliases, so a stale key is a startup crash-loop, and `content_blacklist_address` had become required (an absent value is a fail-open compliance trap, ADR 011/031). Upstream causes: the shared payment pool replaced pairwise channels, [gossip] was deleted, the example configs were dropped in favour of `decdn config init`, and all fourteen contracts were redeployed (deployBlock 11613778). BREAKING CHANGE: `decdn_payment_channel_address` is now `decdn_payment_pool_address`, `decdn_buyer_deposit_micro_usdc` is now `decdn_buyer_working_deposit_micro_usdc`, and `decdn_content_blacklist_address` is required. Sixteen variables backing deleted upstream keys are removed (`decdn_enable_0rtt`, `decdn_delivery_ceiling`, `decdn_voucher_interval_mb`, the three `*_from_block` knobs, the settlement auto-close pair, the speculative-pull trio, and the per-tarball sha256 pins). See ansible/galaxy/CHANGELOG.md for the full upgrade note. Also in this change: - Full schema parity: [network.discovery], [cache.tinylfu], [cache.serve_economics], [cache.origin_retry], [cache.circuit_breaker], [[cache.origins]], [security], [load_shed], [dht/probe.rate_limit], [receipts] and [content] are now rendered. - `decdn config validate` runs against the installed binary after templating, so schema drift fails the deploy with the daemon's own error instead of crash-looping the service. This is the durable guard: the hand-mirrored Ansible asserts are what drifted in the first place. - Release integrity: SHA256SUMS is verified against a detached signature using a vendored keyring, parsed via --status-fd so an EXPIRED or REVOKED key is rejected (plain `gpg --verify` exits 0 for both). Each archive is matched to exactly one manifest entry rather than counting ': OK' lines. - The RPC URL is sourced from the 0600 env file rather than passed via Ansible's `environment:`, which is a shell prefix and would put the embedded API key in the target's process table. - New `molecule/schema` scenario: asserts every rendered config PATH exists upstream, from an inventory generated by files/gen-schema-keys.py. Paths, not leaf names — `sketch_bytes` is legal at cache.tinylfu.sketch_bytes and a startup failure at cache.sketch_bytes. - Validation scenario grows to 11 negative cases, including one proving the config gate itself still fails a deploy. Verified: all 5 molecule scenarios pass (default idempotent); ansible-lint production profile clean; a real decdn 0.1.1 binary validates the host_vars, maximal and multi-origin renders (exit 0); all 8 addresses match contracts/deployments/421614.json. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JrEoHSWL8rdZTB8FuT9pAM
There was a problem hiding this comment.
Pull request overview
This PR re-syncs the decdn_node Ansible role (and its molecule coverage/docs) to match upstream decdn/decdn config schema and deployment realities, including a switch to defaulting installs to manual due to lack of upstream release tags. It also adds a stronger post-template validation gate (decdn config validate) and release artifact integrity verification when release mode is used.
Changes:
- Reworked
node.tomlrendering + variable model to match upstream schema (new sections, removed keys, renamed contracts, required ContentBlacklist). - Hardened install/validation: signed
SHA256SUMSverification for release installs and an authoritative post-templatedecdn config validategate. - Expanded molecule scenarios (new
schema+validationimprovements) and updated repo documentation accordingly.
Reviewed changes
Copilot reviewed 31 out of 32 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Updates high-level install description (signed tarball vs local build) and clarifies onboarding scope. |
| ansible/roles/decdn_node/templates/node.toml.j2 | Major schema re-sync: new tables/keys, new origin forms, and updated blockchain/payment settings. |
| ansible/roles/decdn_node/templates/decdn-node.service.j2 | Adds ExecReload documentation and wiring for SIGHUP-based config reload. |
| ansible/roles/decdn_node/tasks/main.yml | Updates required vars/contracts, adds extensive validation for new knobs, and adds decdn config validate gate. |
| ansible/roles/decdn_node/tasks/install.yml | Reworks release install to verify signed SHA256SUMS and use a private staging dir; adds CLI version backstop. |
| ansible/roles/decdn_node/README.md | Updates role docs for new schema, install modes, onboarding CLI, and new knobs (incl. S3 credential guidance). |
| ansible/roles/decdn_node/handlers/main.yml | Adds a reload handler alongside restart. |
| ansible/roles/decdn_node/files/decdn-release-KEYS.asc | Adds vendored GPG keyring used to authenticate release manifests. |
| ansible/roles/decdn_node/defaults/main.yml | Updates defaults to new schema + variables; sets install method default to manual; adds new config knobs. |
| ansible/README.md | Updates deployment docs to reflect new required contracts, onboarding CLI, and validation gate. |
| ansible/molecule/validation/prepare.yml | Stages placeholder wallet material so the config-gate case can reach the end of the role in CI. |
| ansible/molecule/validation/molecule.yml | Adds prepare step to the validation scenario sequence. |
| ansible/molecule/validation/converge.yml | Expands validation cases (enum/dict/float/bounded-range + config-gate regression guard) and updates vars. |
| ansible/molecule/slow-readiness/converge.yml | Updates required contract vars and ContentBlacklist in scenario inputs. |
| ansible/molecule/schema/verify.yml | Adds schema-keyset drift checker and breadth assertions; adds secret-leak guard for rpc_url in node.toml. |
| ansible/molecule/schema/README.md | Documents purpose/maintenance of the schema drift scenario. |
| ansible/molecule/schema/prepare.yml | Prepares placeholder wallet material for schema scenario role execution. |
| ansible/molecule/schema/molecule.yml | Introduces the new schema-drift molecule scenario. |
| ansible/molecule/schema/files/schema-keys.txt | Adds generated upstream config-path inventory for keyset drift detection. |
| ansible/molecule/schema/files/gen-schema-keys.py | Adds generator script to rebuild schema-keys inventory from an upstream checkout. |
| ansible/molecule/schema/converge.yml | Renders a “maximal knobs” config and a multi-origin variant to exercise keyset coverage. |
| ansible/molecule/generate-keystore/converge.yml | Updates required contract vars and ContentBlacklist in scenario inputs. |
| ansible/molecule/default/verify.yml | Updates rendered-value assertions for the new schema and adds checks for new sub-tables and float rendering. |
| ansible/molecule/default/files/decdn-node-stub | Extends stub to support decdn config validate (TOML syntax only) and a forced-failure marker. |
| ansible/molecule/default/converge.yml | Updates default scenario vars to match new required contracts and new schema knobs. |
| ansible/Makefile | Documents the full molecule scenario set (including new schema and validation). |
| ansible/inventory/host_vars/decdn-node-1/secret.yml.example | Expands secret template to include decdn_extra_env guidance (e.g., AWS credentials). |
| ansible/inventory/host_vars/decdn-node-1/main.yml | Updates example host_vars for new contracts, manual install default, and new schema fields. |
| ansible/galaxy/CHANGELOG.md | Documents breaking variable renames/removals and new integrity/validation features. |
| AGENTS.md | Updates repo guidance to reflect signed release verification and new schema coupling guards. |
| .gitignore | Ignores .ansible/ scaffolding created by tooling. |
| .github/workflows/molecule.yml | Updates workflow comments to reflect the expanded molecule scenario set. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The `kics` job has been failing on every PR even though the scan itself is
clean (0 CRITICAL / 0 HIGH). The failure is in Checkmarx/kics-github-action's
wrapper, not in our Ansible tree.
Root cause: the action's entrypoint runs the scan, discards its exit code into
`KICS_EXIT_CODE`, then unconditionally does
apk add --update nodejs npm && npm ci && npm run build && node dist/index.js
and the step's exit status is that last `node` invocation. That `apk add` is an
*unpinned* fetch into a *digest-pinned* wolfi-base image, so the two have now
drifted apart: Chainguard's current nodejs-26 needs GLIBC_2.44, the pinned base
ships older, and node dies with
node: /usr/lib/libm.so.6: version `GLIBC_2.44' not found (required by node)
three times over (npm is a node script too). The step therefore fails no matter
what the scan found. Upstream has no fix — the only commit past our pinned SHA
just removes a Dependabot config — and the inputs can't disable that code path.
Fix: drop the action and drive the KICS *engine* image directly via
`make security`, which the repo already used for local scans. CI and local runs
are now the same command, there is no Node layer, and the unpinned runtime
`apk add` is gone — one digest-pinned artifact (`KICS_IMAGE`) instead of a
pinned image that fetches unpinned packages. The gate is unchanged in
substance: the engine's own `--fail-on high` exit code (verified: exit 0 with
0 HIGH, exit 40 when a finding meets the threshold).
Also:
- `-w /repo` so findings carry repo-relative paths (`ansible/...`) rather than
`../../repo/ansible/...`; this is what the summary prints and what the
(still-commented) SARIF code-scanning upload would need.
- `--report-formats json,sarif` in the Makefile so the local run produces the
same artifacts CI uploads.
- A "Summarise KICS findings" step replaces the action's `enable_jobs_summary`.
- Drop the now-dead Dependabot ignore for the action; refresh the
supply-chain notes in ci.yml, CONTRIBUTING.md and the Makefile.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DEQihoRTxzVdi2wpPb9Apq
… discovery
Two defects found in review of this PR's own changes.
1. install.yml matched manifest entries with `grep -F " $f"`, a substring
match on the filename. Verified against a scratch manifest:
- a sibling entry that merely STARTS with $f (e.g. "$f.sig") makes the
count 2, so a perfectly good manifest aborts with the misleading
"covers it twice";
- the `hash *name` binary-mode form (`sha256sum -b`) matches nothing at
all, count 0, "does not cover this archive".
Now matches the filename FIELD exactly via awk string equality, skipping
the hash and the two-character " " / " *" separator. Tested across six
manifest shapes (normal, .sig sibling, binary mode, missing tarball,
genuine duplicate, wrong hash) — each now lands on the right verdict.
Note this was never a verification hole: SHA256SUMS is GPG-verified before
this step, and every wrong path already failed closed (the count guard, or
sha256sum --check on an absent file). The defect is availability — a
legitimate upstream manifest could block the install.
2. node.toml.j2 nested `dns_origin` inside the `pkarr_url` branch, so the
resolve-only mode that defaults/main.yml and the assert in tasks/main.yml
both call legal ("dns_origin alone is fine — that is a resolve-only node")
rendered to nothing: no [network.discovery] table, and the daemon silently
fell back to n0 discovery. dns_origin was also dropped when combined with
static peers. Both keys are now emitted independently, and the table is
emitted when any of the three mechanisms is set.
Guarded by a new "resolve-only discovery variant" play in the schema
scenario, following the multi-origin variant's pattern: a separate render
to its own path, plus explicit presence/absence assertions — the schema
checker alone cannot see a key that was never emitted. Verified red/green:
with the old template the scenario fails on "Assert dns_origin renders
without pkarr_url"; with the fix, schema passes end to end.
The role README said the pair must be "set together or not at all", which
contradicted the role's own assert. Corrected.
No CHANGELOG entry: both defects were introduced in this PR's unreleased
0.1.0 content and never shipped.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DEQihoRTxzVdi2wpPb9Apq
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
This repo had fallen ~180 commits behind
decdn/decdnand was broken in two independent ways that fail at different stages of a deploy:git ls-remote --tagsondecdn/decdnreturns zero tags —release.ymlfires on av[0-9]*tag push and has never run.make deploy404'd on the firstget_url, before reaching anything else.node.tomlkeys the daemon rejects. Every upstream config section is#[serde(deny_unknown_fields)]with no serde aliases, so a stale key is a startup crash-loop — andcontent_blacklist_addresshad become required (an absent value is a fail-open compliance trap, ADR 011/031).Upstream causes: the shared payment pool replaced pairwise channels,
[gossip]was deleted, example configs were dropped in favour ofdecdn config init, and all fourteen contracts were redeployed (deployBlock11613778).Breaking changes
decdn_payment_channel_addressdecdn_payment_pool_addressdecdn_buyer_deposit_micro_usdcdecdn_buyer_working_deposit_micro_usdcdecdn_content_blacklist_addressoptionalSixteen variables backing deleted upstream keys are removed (
decdn_enable_0rtt,decdn_delivery_ceiling,decdn_voucher_interval_mb, the three*_from_blockknobs, the settlement auto-close pair, the speculative-pull trio, the per-tarball sha256 pins). Full upgrade note inansible/galaxy/CHANGELOG.md.The default install method is now
manual, since there is no upstream release to download.What else is in here
[network.discovery],[cache.tinylfu],[cache.serve_economics],[cache.origin_retry],[cache.circuit_breaker],[[cache.origins]],[security],[load_shed],[dht/probe.rate_limit],[receipts],[content].decdn config validatenow runs against the installed binary after templating. The hand-mirrored Ansible asserts are precisely what drifted; this is the durable fix.SHA256SUMSverified against its detached signature using a vendored keyring, parsed via--status-fdso an expired or revoked key is rejected (plaingpg --verifyexits 0 for both). Each archive is matched to exactly one manifest entry rather than counting': OK'lines.environment:, which is a shell prefix and would place the embedded API key in the target's process table.molecule/schema— a new scenario asserting every rendered config path exists upstream, from an inventory generated byfiles/gen-schema-keys.py. Paths, not leaf names:sketch_bytesis legal atcache.tinylfu.sketch_bytesand a startup failure atcache.sketch_bytes.Review notes
This branch was reviewed by the PR-review toolkit, which found 9 real defects in the first draft — including that the schema guard checked leaf names (so the exact hazard the template header warns about passed clean),
make checkdying in release mode becausetempfilehas no check-mode support, and the config gate silently no-op'ing when norcwas produced. All were confirmed empirically and fixed; see the commit body.Verification
defaultis idempotentansible-lintproduction profile: 0 failures; yamllint + markdownlint cleancontracts/deployments/421614.jsonKnown, accepted
make securityreports one new MEDIUM ("Communication Over HTTP") on the loopback readiness probe. It is a false positive — the target is127.0.0.1, the daemon serves metrics without TLS, and this PR adds an assert enforcing loopback binding. KICS's inline ignore directives have no effect on this query in the pinned engine, so it is left unsuppressed with a comment rather than carrying a no-op directive.--fail-on highstill exits 0 andkics-results/is gitignored. I could not determine what made KICS newly resolve it; the probe task itself is unchanged.🤖 Generated with Claude Code
https://claude.ai/code/session_01JrEoHSWL8rdZTB8FuT9pAM