Skip to content

feat(network): canonical template reconciliation + CIDR-aware inventory (network epic)#98

Open
pparage wants to merge 8 commits into
devfrom
feature/network-provisioning
Open

feat(network): canonical template reconciliation + CIDR-aware inventory (network epic)#98
pparage wants to merge 8 commits into
devfrom
feature/network-provisioning

Conversation

@pparage

@pparage pparage commented Jun 11, 2026

Copy link
Copy Markdown
Member

What

Backend half of the "drawn networks actually provision" epic — makes the canvas's network definitions authoritative end-to-end. Five commits, all TDD, full overlay+core suites green (the one unrelated test_alembic failure is pre-existing: system alembic on SQLAlchemy 1.x vs project 2.x).

  1. d4edbd7 / 673ad00 — reconcile expand_replication to the canonical schema (fix(compose): expand_replication reads network templates from config, but canonical schema defines them node-level #84). Network templates (cidr_template/bridge_template/gateway_template) are now read at node level and rendered with the canonical Jinja {{ bridge_base + team_id }} form (legacy {N+team_id} still accepted). Byte-parity with the deployer-ui TS expander, enforced by the shared vector harness. Fixes a 3-way schema↔impl divergence (location, placeholder syntax, vlan field).
  2. b5afd2bbridge_base null/non-int falls back to 140 (TS parity). Caught by a Sonnet verification pass: Python crashed where TS defaulted. Also adds node-level gateway_template to the schema/Pydantic (it was rendered but would fail strict validation).
  3. b779a05 — derive ansible_host from the bound network CIDR (fix(deploy): derive inventory ansible_host from topology instead of hardcoded 192.168.x scheme #73). A host NIC's node_ref resolves to its network's cidr (per-team rendered via the same canonical renderer). Precedence: explicit NIC ip > node_ref CIDR > legacy 192.168.{bridge_base+team} fallback.
  4. ca063ab — emit per-host cloud-init net vars + r42_network_map. Backend becomes the single source of resolved network values so the _universal playbook stops recomputing them: each host carries r42_ci_netmask/r42_ci_gateway/r42_net_bridge; all.vars.r42_network_map carries per-network/per-team resolved bridge/cidr/gateway.

Companion PRs / context

  • range42-playbooks#90 consumes r42_network_map + the per-host ci vars (slices 4/5). It depends on this PR.
  • deployer-ui counterparts (TS expander, schema, vectors) are already on dev per that repo's direct-commit convention.
  • Verified by two Sonnet review agents (parity + regressions); both real findings fixed here.

Test

pytest tests/overlay tests/core green (excluding the pre-existing test_alembic env failure). Deployer-ui full unit suite: 539 passed.

pparage added 5 commits June 11, 2026 10:14
Byte-parity with the deployer-ui TS expander; resolves gateway_template
to gateway per team id. Verified by the shared expand_replication vector
03_per_team_gateway_template.
…rk-template form

Byte-parity with the deployer-ui TS expander: read cidr/bridge/gateway
templates at node level (per range42.schema.json) and render the canonical
Jinja {{ bridge_base + team_id }} form, keeping legacy {N+team_id} support.
Threads bridge_base from the document. Resolves #84 schema<->impl
divergence; verified by the shared expand_replication vector.
… regen Node

Non-numeric bridge_base previously crashed the Python expander with a
TypeError where the TS expander defaulted to 140; now both agree. Regenerates
generated.py to include node-level gateway_template. Verified by the shared
expand_replication vectors.
A host NIC's node_ref now resolves to its network node's cidr (rendering a
node-level cidr_template per team via the canonical renderer), and the host
ip is derived inside that subnet instead of the hardcoded 192.168.{bridge_base}
scheme. Precedence: explicit NIC ip > node_ref CIDR > hardcoded fallback, so
topologies without networks[] are unchanged. Honors canvas-drawn custom CIDRs.
Backend becomes the single source of resolved network values so the
_universal playbook stops recomputing them. Each host now carries
r42_ci_netmask/r42_ci_gateway/r42_net_bridge (derived from its bound network,
else the legacy 192.168.{bridge_base+team} scheme), and all.vars carries
r42_network_map (per network id + team -> resolved bridge/cidr/gateway).
Prereq for the playbook slices 4/5 (network-aware bridge create + cloud-init).

@chatgpt-codex-connector chatgpt-codex-connector Bot 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ca063abb10

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread app/schemas/generated.py
Comment on lines +168 to +173
gateway_template: Annotated[
str | None,
Field(
description="Gateway IP template, e.g., 192.168.{{ bridge_base + team_id }}.1 (network kind only)"
),
] = None

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Allow static network fields in the generated schema

The new inventory path reads cidr, bridge, and gateway on network nodes (and the added tests create those topologies), but this model still has extra="forbid" and only adds gateway_template here. Since validate_project validates base docs with CatalogEntry.model_validate, any catalog that uses a static custom CIDR/gateway is reported invalid before it can be deployed; add the materialized fields to the schema alongside the templates.

Useful? React with 👍 / 👎.

…way nodes

Addresses PR review (Codex + subagent):
- team_count is now a global extravar so the proxmox-cli provisioning plays
  see it (set_fact on localhost is host-scoped). Fixes deploy-fatal undefined.
- Regenerate Node to include materialized cidr/bridge/gateway (network kind),
  so a catalog authoring a static custom CIDR passes CatalogEntry validation
  (Node has extra=forbid; expand_replication also emits these fields).
@pparage

pparage commented Jun 11, 2026

Copy link
Copy Markdown
Member Author

Addressed review in 526a25b: team_count is now a global extravar (the proxmox-cli plays can't see a localhost set_fact), and Node now declares materialized cidr/bridge/gateway so static-CIDR catalogs pass CatalogEntry validation.

pparage added 2 commits June 11, 2026 14:43
Returns the raw PVE guest config (net0/net1/ipconfig*, ...) via the host
token over httpx. The v1 VM list omits per-NIC detail, so the UI import flow
needs this to reconstruct bridge/network edges for imported machines.
The proxmox_controller role's node-network tasks (bridge create/list) read
proxmox_api_host/user/token_id/token_secret as plain vars, but deploy_trigger
never provided them (generated inventory has no token) — so a live _universal
deploy could not authenticate to Proxmox. Derive them from the target host
(token_ref 'user!tokenid=secret') into extravars, and add the secret to the
redaction tainted-set. Surfaced by live testing the network epic.
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.

1 participant