Uh oh!
There was an error while loading. Please reload this page.
feat(decdn_node): expose operator-facing config knobs (#29) - #32
Conversation
The decdn-node daemon reads ~60 config fields, but the role only templated the subset upstream ships in its own operator config. Fields with no role knob had to be hand-edited into /etc/decdn/node.toml, which the next `make deploy` silently reverts. Issue #29 is the concrete case: cache.node_to_node_pull_through_enabled gates all serve-time origin pull-through but had no variable. Expose ~27 operator-facing knobs (node-to-node pull-through, settlement thresholds, delivery clamps, cache tuning, blockchain watchers, network, observability) using the role's existing discipline: each defaults to an unset sentinel ("" / []) meaning "omit the key, use the daemon default", so node.toml stays byte-identical unless an operator opts in. An explicit 0/false is emitted (0 is meaningful, e.g. gc_interval_sec = 0 disables the sweep). Every knob is fail-loud validated at deploy time so a bad value never reaches the daemon (which would crash-loop under deny_unknown_fields): bool/integer shape, i64::MAX cap, per-field ranges, cross-field constraints compared against the daemon's default-filled values, and TOML-injection guards on string/list knobs. Bytes/Percent render as bare integers; new [cache] scalars render before [cache.origin] so the sub-table header doesn't absorb them. Add a negative-path molecule scenario (validation) that asserts each bad-value family is rejected by the role's own validation, plus positive coverage in the default scenario for the emit-0/bare-int/bool/two-element-list forms. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Warning Review limit reached
Next review available in:37 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe role adds optional tuning variables, validates their types and ranges, renders them into ChangesTuning knob configuration and validation
Estimated code review effort: 4 (Complex) | ~45 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Code Review
This pull request introduces optional operator-facing tuning knobs for the decdn_node Ansible role, exposing various daemon configuration fields (such as node-to-node pull-through, settlement, delivery-rate clamps, cache tuning, and blockchain watchers) along with robust validation tasks and a negative-path Molecule test scenario. The review feedback highlights a critical, systematic issue across the validation tasks and Jinja2 templates: if an operator explicitly unsets any of these optional variables by setting them to null (which resolves to None in Ansible), the current checks against empty strings ("") will fail. This leads to unexpected validation failures, template rendering crashes (e.g., calling | length on NoneType), or dangerous type coercions (such as coercing None to 0, which would clamp the delivery ceiling to zero and block paid traffic). The reviewer provides actionable recommendations to use robust null/None checks, such as 'in ["", none]' and 'is not none', to safely handle unset variables.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Pull request overview
This PR expands the ansible/roles/decdn_node role to expose a larger set of operator-facingdecdn_* configuration knobs (notably including cache.node_to_node_pull_through_enabled from #29), while keeping upstream defaults unpinned via an “unset sentinel” ("" / []) and adding deploy-time validation to prevent invalid TOML from crash-looping the daemon.
Changes:
- Add ~27 new optional
decdn_*variables (defaulting to""/[]) and emit them intonode.tomlonly when set. - Add fail-loud Ansible validation for optional bool/int/list/string knobs (including cross-field constraints) to block invalid configs before host mutation.
- Extend Molecule coverage with a new negative-path
validationscenario and additional assertions in the default scenario.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| ansible/roles/decdn_node/templates/node.toml.j2 | Emit new optional operator knobs into node.toml, including relay_urls, pull-through, cache tuning, and observability fields. |
| ansible/roles/decdn_node/tasks/main.yml | Add deploy-time assertions to validate optional knob shapes/ranges/cross-field constraints before rendering/starting services. |
| ansible/roles/decdn_node/README.md | Document the new optional tuning knobs and their intended semantics. |
| ansible/roles/decdn_node/defaults/main.yml | Introduce the new decdn_* defaults using the unset-sentinel pattern ("" / []). |
| ansible/molecule/validation/molecule.yml | Add a new Molecule scenario dedicated to negative-path validation behavior. |
| ansible/molecule/validation/converge.yml | Implement negative test cases that must fail specifically at the role’s validation asserts. |
| ansible/molecule/default/converge.yml | Set representative tuning knobs to exercise emission/validation paths in the default scenario. |
| ansible/molecule/default/verify.yml | Assert rendered TOML contains the expected tuning knobs with correct types (e.g., explicit 0, ints vs bools). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
…trings Address code-review feedback on the tuning-knob validation: - null/None now behaves like the "" / [] unset sentinel everywhere (Gemini). An operator writing `decdn_x:` (null) previously fail-loud'd on the shape assert or crash'd `| length` on None; now it omits the key and uses the daemon default, matching Ansible idiom. Scalars use `not in ["", none]` (NOT `| length`, which errors on an integer value); lists use `is not none and | length`; the integer-shape loop is a list expression so null keeps its type instead of becoming the string "None". - Reject backslashes in the interpolated string/URL knobs (Copilot). A backslash in a TOML basic string starts an escape and a trailing `\` escapes the closing quote (a daemon crash-loop); relay_urls/relay_url/user_agent/otlp_endpoint now disallow it. Also validate the previously-unchecked singular decdn_relay_url. Verified: null overrides render as omitted (no crash, default parity holds); backslash/quote/whitespace values are rejected at deploy time; all prior positive/negative cases still hold; ansible-lint + yamllint clean; all four molecule scenarios pass (validation rescued=6). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Uh oh!
There was an error while loading. Please reload this page.
Summary
Closes#29. The
decdn-nodedaemon reads ~60 config fields, but thedecdn_noderole only templated the subset upstream ships in its own operator config (examples/configs/arbitrum-sepolia.toml). A field with no role knob had to be hand-edited into/etc/decdn/node.toml— which the nextmake deployre-renders and silently reverts. Issue #29 is the concrete case:cache.node_to_node_pull_through_enabledgates all serve-time origin pull-through but had nodecdn_*variable.This exposes ~27 operator-facing knobs and closes the same gap for the rest of that surface. Deep-internal tuning with strong upstream defaults (rate limiters, circuit breaker, retry policy, gossip, receipts, prefetch) stays out of scope.
Design
""(or[]) meaning "omit the key, use the daemon's own default".node.tomlstays byte-identical to today unless an operator opts in — no pinning of upstream defaults. An explicit0/falseis emitted (gated on!= "", not| int > 0), because0is meaningful (gc_interval_sec = 0disables the sweep,max_unrecouped_leech_bytes = 0= off).deny_unknown_fields): bool/integer shape, an i64::MAX cap (TOML ints are i64), per-field ranges, cross-field constraints compared against the daemon's default-filled values, and TOML-injection guards (quote/whitespace/newline) on the string/list knobs.Bytes/Percentemit as bare integers (pull_ahead_bytes = 1048576,pull_share_ratio_percent = 400= 4.0×); new[cache]scalars render before[cache.origin]so the sub-table header doesn't absorb them;relay_urlssupersedes the singularrelay_urlwhen both are set.Knobs added
Node-to-node pull-through (#29) · settlement thresholds + auto-close · delivery floor/ceiling + voucher interval · cache tuning (probe holds, GC, pins, user-agent) · blockchain watchers (RPC watchdog, event poll, blacklist poll) · network (
relay_urls,enable_0rtt) · observability (otlp_endpoint, region accounting). Seeroles/decdn_node/defaults/main.yml.Testing
molecule/validationnegative scenario: asserts each bad-value family (cross-field, range, shape, bool, list, string) is rejected by the role's own validation (matched onansible_failed_task.name, so a value slipping past validation → role fails elsewhere → caught).molecule/defaultextended: asserts the emit-0, bare-intBytes/Percent,bool, and two-element-list rendering forms with exact value + type (isinstanceguards).ansible-lint(production profile) +yamllintclean;make build(galaxy collection) OK.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation