Skip to content

fix: follow-ups from the review of #130-#136 - #137

Merged
thiras merged 3 commits into
mainfrom
fix/review-followups-130-136
Oct 10, 2026
Merged

thiras merged 3 commits into
mainfrom
fix/review-followups-130-136

Conversation

@thiras

@thiras thiras commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Fixes from a multi-agent review of the last seven PRs (#130–#136).

Docs that broke a deploy when followed

  • compose/README.md: an fs origin's compose.override.yaml was never loaded, because every command passes -f compose/compose.yaml. The README now says to add -f compose/compose.override.yaml to every command.
  • cloud-init/user-data-publisher.yaml:
    • removed a stale comment that told the operator to add a second decdn_nodes group;
    • set ansible_connection: local (and the interpreter) on sponsord_hosts.hosts.localhost, so a sponsor alone no longer tries SSH.

Code

  • ansible/Makefile → scripts/limit-guard.sh, run before every playbook target. It refuses a run where the playbook selects no host, with or without a LIMIT. It also refuses a LIMIT that includes any host no play selects, and names those hosts: deploy-node LIMIT=<an origin>, LIMIT='node-1,origin-1' and the reverse used to exit 0 having skipped them. For a group LIMIT that mixes kinds, subtract the uncovered group (LIMIT='eu:!decdn_origin_nodes') or use make deploy.
  • scripts/release.sh: the decdn.node constraint check now runs on every publisher-collection release, not just the first.
  • cloud-init/tests/lint.py: the origin backend keys may be set only in decdn_origin_nodes.vars. A sponsord_* group or a host var could override them and still pass the lint.

Tests (tests/scripts-test.sh)

  • site.yml and publisher.yml play composition: each node runs once, cache nodes in node.yml, origins in origin.yml.
  • node.yml/origin.yml pre_tasks are all tagged always.
  • Every playbook target: an exact check that make -n runs the guard, then its own playbook scoped to LIMIT. This replaces greps that matched the guard line.
  • limit-guard.sh: partial, mixed-group, empty-inventory, unknown-host and --syntax-check cases.
  • release-collection.yml: the Galaxy step runs before publish and passes --no-deps.
  • A later publisher release above its decdn.node constraint is refused.
  • The publisher template cut down to a sponsor alone and to an origin alone, both through the lint.
  • Three lint variants with the origin backend set outside decdn_origin_nodes.vars.

Doc and comment accuracy

  • cloud-init/README.md: the Stage 2 description and the lint's list now cover origins.
  • ansible/galaxy/publisher/README.md: the RPC URL arrives as an EnvironmentFile, not a credential; only the relay and DNS roles refuse held ports.
  • compose/README.md: config init --origin also takes file:/// and s3:// (checked against upstream v0.0.1).
  • ansible/README.md: the firewall holes list now includes the relay's and the DNS server's.
  • docs/publishers.md and docs/lifecycle.md: the prune condition is now "leaves CapacityBond's active set" (ADR 011).
  • Five files said "imported by site.yml"; they now say publisher.yml.
  • bootstrap.sh, lint.py and baseline-plays.awk: comments corrected.
  • AGENTS.md and RELEASING.md: wording fixes.

Test plan

  • make test-scripts passes. Only the upstream-mirror generators are skipped, since they need UPSTREAM.
  • make lint
  • make lint-cloud-init
  • The sponsor-alone test fails against the old template.
  • The always-tag query reports a pre_task with the tag removed.
  • Mutations: dropping the limit from a recipe, or the guard from a target, fails make test-scripts.
  • CI

🤖 Generated with Claude Code

- cloud-init: the publisher template sets ansible_connection on sponsord_hosts
  too, so a sponsor alone deploys; drop the stale "add a decdn_nodes group"
  comment; the lint pins the origin backend keys to decdn_origin_nodes.vars
- ansible/Makefile: refuse a LIMIT that matches no host of the target's playbook
  (deploy-node LIMIT=<origin> exited 0 having done nothing)
- release.sh: check decdn.publisher's decdn.node constraint on every release
- tests: site.yml/publisher.yml play composition, always-tagged guards, the
  LIMIT guard, the later-release constraint, the cut-down publisher template
- docs: compose override needs -f, config init --origin forms, cloud-init
  README stage 2 and lint list, publisher README credentials and ports,
  firewall holes, prune condition (ADR 011), stale site.yml comments

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 10, 2026 19:19

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.

🟡 Changes recommended

The release guard can accept unpublished dependencies, and optional origin backend variables remain insufficiently restricted.

2 open findings
What changed in this PR

Follow-up fixes for PRs #130–#136, improving deployment safeguards, cloud-init validation, release checks, tests, and documentation.

Changes:

  • Adds Ansible LIMIT guards and broader playbook tests.
  • Tightens cloud-init inventory validation and sponsor-only support.
  • Updates release dependency handling and operational documentation.
File Description
tests/​scripts-test.sh Expands release, cloud-init, and playbook tests.
scripts/​release.sh Checks node dependency constraints for every publisher release.
RELEASING.md Documents collection dependency sequencing.
docs/​publishers.md Corrects secret and origin-pruning guidance.
docs/​lifecycle.md Clarifies inactive-origin pruning conditions.
compose/​README.md Corrects origin configuration and override usage.
cloud-init/​user-data-publisher.yaml Supports sponsor-only local execution.
cloud-init/​tests/​lint.py Restricts origin backend variable placement.
cloud-init/​README.md Updates origin and baseline validation guidance.
cloud-init/​bootstrap.sh Corrects origin validation commentary.
cloud-init/​baseline-plays.awk Documents origin baseline play handling.
ansible/​README.md Documents firewall ports and LIMIT safeguards.
ansible/​playbooks/​sponsord.yml Corrects importer documentation.
ansible/​playbooks/​iroh_relay.yml Corrects importer documentation.
ansible/​playbooks/​iroh_dns_server.yml Corrects importer documentation.
ansible/​Makefile Rejects limits matching no playbook hosts.
ansible/​inventory/​group_vars/​iroh_relay_hosts.yml Corrects publisher entry-point reference.
ansible/​inventory/​group_vars/​iroh_dns_server_hosts.yml Corrects publisher entry-point reference.
ansible/​galaxy/​publisher/​README.md Corrects port and credential behavior.
ansible/​galaxy/​publisher/​galaxy.yml Updates dependency-check documentation.
AGENTS.md Aligns repository guidance with deployment paths.
.github/​workflows/​molecule.yml Updates scenario timing commentary.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread cloud-init/tests/lint.py Outdated
Comment thread scripts/release.sh
thiras and others added 2 commits October 10, 2026 22:51
…ependency

- cloud-init lint: every decdn_cache_origin* key (optional ones too) may be
  set only in decdn_origin_nodes.vars, not just the required fields
- release-collection.yml: before uploading decdn.publisher, require that
  Galaxy resolves the decdn.node its MANIFEST.json declares; a git tag is
  not an upload (PUBLISH_ENABLED off, or a failed publish)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- scripts/limit-guard.sh (from ansible/Makefile's limit_guard): refuse a run
  in which the playbook selects no host (with or without LIMIT), and a LIMIT
  that includes any host no play selects, naming each; --syntax-check skips it
- tests: one exact check per playbook target (guard, then the scoped run),
  replacing greps the guard line matched and that could SIGPIPE make; the
  guard's partial, empty-inventory, unknown-host and --syntax-check cases;
  the Galaxy step's order and --no-deps
- release-collection.yml: setup-python in the publish job (PEP 668); the
  Galaxy refusal no longer assumes the version is missing
- docs: config init's s3 region, release.sh header, secret.yml S3 keys,
  conditional firewall holes, ssh_allow_cidrs for a sponsor alone, lint and
  CONTRIBUTING summaries, stale site.yml comments

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@thiras
thiras merged commit f76d026 into main Oct 10, 2026
55 checks passed
@thiras
thiras deleted the fix/review-followups-130-136 branch October 10, 2026 20:25
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