myxce/.kiro/specs/admin-config-tool/research.md
Rongsong Shen a14785108c docs(admin-config-tool): commit spec docs and example configs
design.md, research.md (new), brief.md and requirements.md (updated
during spec authoring: Req 1.6 resolved/superseded by bundling, Req
5.5 added, Req 4.3/4.4 extended for limits.yaml) had been sitting
uncommitted since before implementation began -- the entire 18-task
implementation was built against these documents without them ever
landing in git.

src/configs/*.yaml + schema/*.schema.json: the example config
directory and bundled JSON schemas most of the test suite reads as
"real repo fixtures" (test_config_build.py, test_mapping.py,
test_validation.py, etc.) were likewise never committed -- those
tests would silently skip (skipif guards) rather than run for anyone
else checking out this branch.
2026-07-22 10:11:08 +00:00

27 KiB

Research & Design Decisions: admin-config-tool

Summary

  • Feature: admin-config-tool
  • Discovery Scope: Complex Integration — first Python component in a C/Go codebase, first external client of the controller's raw TLS wire protocol, no in-repo precedent for either.
  • Key Findings:
    • The controller's OP_upgradeConfig network dispatch path is currently broken for every role (standby rejected by ha_dispatch_gate, primary rejected by an inverted ha_is_primary() check in cluster.c) — a controller-side defect discovered during this design's discovery phase, out of this spec's boundary but blocking real E2E validation of config push.
    • Redirect (redirectHost/redirectPort) is wired only for OPCode_join, never OPCode_upgradeConfig — auto-primary-discovery (Req 3) has no live data to act on for config push until the controller is extended.
    • pycapnp handles controller.capnp's $C.mapname(...) annotations without issue (declared, inert metadata under dynamic reflection) — no custom-annotation workaround needed.
    • src/frontend/xcecli/meson.build's go_build-wrapper + custom_target() pattern is a directly reusable template for src/xceadmin/meson.build, resolving brief.md's open question on Python package location/build integration.

Research Log

TLS setup and certificate validation

  • Context: config push needs to establish the same trust model the controller cluster already uses (brief.md Req 2.3-2.4 assumption).
  • Sources Consulted: src/lib/netio.c:1208-1243 (create_ssl_context), src/controller/myapp.c:94-109, src/controller/main.c:374-375,90, src/controller/join.c:13-16.
  • Findings:
    • create_ssl_context(cert_file, key_file) builds a client context (TLS_client_method()) when no cert/key is given, a server context otherwise. myconf->certfile/keyfile resolve to <certdir>/xce.crt/xce.key.
    • The controller's own outbound client context explicitly sets SSL_CTX_set_verify(client_ssl_ctx, SSL_VERIFY_NONE, NULL) (myapp.c:109).
    • No SSL_CTX_set_verify(..., SSL_VERIFY_PEER, ...), X509_check_host, or SSL_get_peer_certificate call exists anywhere in the codebase. validate_host() (join.c:13-16) is an unrelated stub (return 1;) used only in the cluster-join path, not TLS.
    • No minimum TLS version or cipher-suite pinning exists (SSL_CTX_set_min_proto_version/set_ciphersuites not found).
  • Implications: xceadmin should connect with ssl.CERT_NONE/check_hostname=False to match current behavior exactly — this is encryption-only transport security throughout XCE today, not a weakening introduced by this spec. Captured in design.md Security Considerations.

Wire framing byte order

  • Context: brief.md asserted "8-byte uint64 size prefix in host byte order" — needed independent confirmation before committing a Python struct.pack format string.
  • Sources Consulted: src/include/netio.h:30, src/lib/netio.c:111-199, src/lib/request.c:863-938, src/controller/config_repl.c:1925-1968.
  • Findings: io_packet_t.sz (uint64_t) is written/read as raw memory (&pkt->sz, sizeof(...)) directly into SSL_read/SSL_write — no htole64/htobe64/ntohll anywhere in the path. request.c:863-864 states this explicitly in a comment; config_repl.c's xce_probe_config_tag send/recv code (:1925-1968) follows the identical pattern.
  • Implications: Python must use struct.pack("=Q", size) (native byte order), not network byte order (">Q"). Documented in design.md xcecommon.wire.

OP_upgradeConfig dispatch path

  • Context: Verifying the request would actually reach do_upgrade_config_with_bytes() on a real cluster before designing around it as a working dependency.
  • Sources Consulted: src/controller/ha.c:3794-3960 (ha_dispatch_gate), src/controller/cluster.c:238-682 (myreq_handler), src/controller/config_repl.c:1591-1810, src/controller/config_repl.h:347-349, src/controller/testsuite/test_config_repl.c:2042, test_commit_barrier_rollout.c:1484,1531.
  • Findings:
    • ha_dispatch_gate()'s CTRL_STANDBY allow-list (ha.c:3926-3951) does not include OPCode_upgradeConfig (only upgradeConfigAck is listed) — standbys are rejected with -XCE_ERR_STANDBY before reaching the opcode switch.
    • For CTRL_PRIMARY with quorum, the gate falls through (ha.c:3880) — primaries pass the gate.
    • But cluster.c:667-679's in-switch handler does if (ha_is_primary(&c->ha)) { /* reject, -EAGAIN */ } else { handle_upgrade_config(...) } — i.e., it only calls the real handler for a non-primary, the opposite of what passed the gate.
    • Net: standby → rejected at the gate; primary → passes the gate, rejected by the inverted in-switch check. No role reaches handle_upgrade_config()/do_upgrade_config_with_bytes() via the network today. That function is exercised only by unit tests calling it directly.
    • config_repl.h:347-349's doc comment ("handle an OP_upgradeConfig request from an admin tool") and the handler's actual logic (WAL marker commit, push_config_to_standbys, restart signal) both imply it should run on the primary — the cluster.c:671 guard looks inverted/stale relative to both the dispatch gate and the handler's own semantics.
  • Implications: This is a controller-side defect, not an xceadmin gap — out of this spec's boundary (see design.md Boundary Commitments), but it makes real-cluster E2E testing of config push currently impossible. Testing Strategy in design.md treats the mock-controller test suite as load-bearing, not merely a dev-loop convenience, until this is fixed. Recommend logging this as a bug/blocking item outside this spec's implementation tasks.

Redirect wiring scope

  • Context: Requirement 3 (Auto Primary Discovery) assumes redirectHost/redirectPort get populated on a standby's reply to any request, including upgradeConfig.
  • Sources Consulted: src/schema/controller.capnp:573-595 (RequestReply), src/controller/join.c:27-31,64,277.
  • Findings: fill_redirect_reply() is defined at join.c:27-31 and its only production call site is join.c:277, inside the OPCode_join handling path. No other opcode handler calls it.
  • Implications: xceadmin.discovery's redirect-following is implemented generically against the RequestReply schema shape (correct for any opcode that does populate these fields, e.g. join), but will not actually fire for config push until the controller extends fill_redirect_reply()'s call sites to include upgradeConfig. Documented as an Out-of-Boundary item and Revalidation Trigger in design.md.

ProbeConfigTag liveness

  • Context: config push --dry-run (Requirement 6) was planned around reusing ProbeConfigTag.
  • Sources Consulted: src/controller/config_repl.c:1811-1998 (xce_probe_config_tag + doc comment), src/schema/controller.capnp:821-827, ha.c:7409, ha.c:3828,3860,3889,3911,3943 (dispatch-gate allow-lists).
  • Findings: xce_probe_config_tag() is a direct-struct RPC (no Request/Reply wrapper) with only test callers (test_ha_bootstrap.c:259 and a mock in test_ha_bootstrap_support.c). No server-side dispatch case for OPCode_probeConfigTag exists anywhere in cluster.c, despite ha_dispatch_gate listing the opcode as allowed for every role — the allow-list entry is moot because nothing answers it. A stale doc comment at ha.c:7409 claims it's "already called in myapp_run before ha_start_election_async"; no such call exists in myapp.c or anywhere in production code.
  • Implications: config push --dry-run's wire client can and should still be implemented against the documented contract (Requirement 6 remains valid future-facing design), but it has no live counterpart to call today. Reflected in design.md as an Out-of-Boundary item; Requirement 6 stays Nice-to-Have/Follow-up as already scoped in requirements.md.

No existing external-client precedent

  • Sources Consulted: src/tools/ (xce-build.sh, deploy-ha.sh, go_build, appimage/entry.sh), src/controller/testsuite/test.c (built as xce_test), src/frontend/xcecli/common/request/request.go, repo-wide find . -iname "*.py".
  • Findings: src/tools/ has no socket/capnp client. xce_test is a WAL/snapshot file-inspection CLI with zero network code. xcecli (Go) speaks HTTP/JSON to apiserver, never the raw wire protocol. No Python file exists anywhere in the repository.
  • Implications: xceadmin is the first client of this wire protocol from outside a C daemon — there is no reference implementation to diff against; correctness rests entirely on independently re-deriving the framing/union contract from the C source (done above), not on copying an existing pattern.

Build tooling precedent for a non-C, non-Meson-native component

  • Sources Consulted: src/tools/xce-build.sh:52,63-131, .claude/skills/xce-build/SKILL.md, .claude/skills/xce-deploy-ha/SKILL.md, src/nixSupport/xce.nix:18-70, src/frontend/xcecli/meson.build, src/meson.build:34-35,84-86, src/meson.options:3-6.
  • Findings: xce-build.sh dispatches on component name via a case statement; apiserver/xcecli branches additionally shell out to make test/go test. xcecli's meson.build wraps go build via a custom_target() calling a generic tools/go_build script, gated by the component meson option (combo: all|xcecore|xcecli) and conditionally included via if component != 'xcecore': subdir('frontend/xcecli'). xce.nix's nativeBuildInputs has zero Python packages today — confirmed genuinely absent, not just undocumented.
  • Implications: src/xceadmin/meson.build should mirror xcecli's pattern exactly (a new tools/py_build wrapper + custom_target() + a component choice or unconditional subdir('xceadmin')), resolving brief.md's open question #9 (Python package layout) in favor of src/xceadmin/, consistent with structure.md's "each major subsystem lives under src/<component>/" convention.

pycapnp schema loading and message encoding

  • Context: Confirm controller.capnp's C-specific annotations ($C.mapname(...)) don't break pycapnp's schema compilation, and confirm the right API surface for non-RPC encode/decode.
  • Sources Consulted: pycapnp GitHub/README, pycapnp CHANGELOG, Quickstart (2.2.4 docs), capnp.pyx source.
  • Findings: $C.mapname(...) is a properly declared annotation (via c.capnp's annotation mapname @0x...), not an unknown construct — pycapnp's DynamicStruct reflection parses it as inert metadata; the only real failure mode is an unresolved /c.capnp import path, handled via capnp.load(path, imports=[schema_dir]). msg.write(f) calls f.fileno() and writes to the raw fd, bypassing any Python-level wrapper — safe for open(path, 'wb'), unsafe for a socket. msg.to_bytes()/Struct.from_bytes() are pure in-memory and the correct choice for the socket path.
  • Implications: No custom annotation-stripping preprocessing needed before schema load. xcecommon.wire must use from_bytes()/to_bytes() exclusively (never .write()/.read() on a live socket) — captured as an explicit constraint in design.md.

pycapnp + Python ssl module composition

  • Findings: pycapnp's async/KJ-loop machinery (AsyncIoStream, capnp.run()) is scoped to its RPC/vat-network layer (TwoPartyClient/Server) and does not intrude on plain synchronous from_bytes()/to_bytes() usage. ssl.SSLContext().wrap_socket() gives a normal SSLSocket whose .recv()/.sendall() perform the framing at the plaintext layer; pycapnp never touches the socket object directly in this design.
  • Implications: Confirms the xcecommon.wire design (manual framing over ssl.SSLSocket, in-memory capnp encode/decode) has no hidden event-loop conflict.

click and pytest conventions

  • Findings: Current stable click (8.3.x/8.4.x) idiom for grouped subcommands: @click.group() → @cli.group() → @config.command(), giving xceadmin config build/xceadmin config push naturally. pytest-regressions' file_regression fixture (binary-capable, deliberate --force-regen updates) is the more actively maintained golden-file library versus pytest-golden (YAML-only, no binary support) — evaluated but not adopted (see Design Decisions) in favor of a plain assert produced == expected_bytes fixture comparison, since the byte-equality need here doesn't need the extra dependency's ergonomics.

Architecture Pattern Evaluation

Option Description Strengths Risks / Limitations Notes
Layered CLI over shared library (selected) Thin click CLI (xceadmin) over a protocol/schema/mapping library (xcecommon) with no CLI dependency Matches project's inward-dependency convention; xcecommon directly reusable by future Python xcecli (brief.md Future Roadmap); each layer independently testable None material for this scope —
Single-file script One xceadmin.py doing everything Fastest to write initially Not reusable by future xcecli; violates project's "one domain per file" convention (structure.md); harder to unit-test wire framing in isolation Rejected
Two separate pip distributions (xceadmin, xcecommon) from day one Split packaging immediately Cleaner dependency graph if xcecommon gets a second consumer soon No second consumer exists yet; premature packaging overhead (Simplification lens) Deferred, not rejected — revisit when Python xcecli work starts

Design Decisions

Decision: src/xceadmin/ as the package location

  • Context: brief.md left this as Open Design Question #9 (candidates: src/xceadmin/, tools/xceadmin/, top-level xceadmin/).
  • Alternatives Considered: 1) tools/xceadmin/ alongside deploy-ha.sh. 2) top-level xceadmin/. 3) src/xceadmin/.
  • Selected Approach: src/xceadmin/, mirroring src/frontend/'s existing precedent for a non-C, non-Meson-native language living directly under src/ with its own build wrapper.
  • Rationale: structure.md's "Organization Philosophy" states "each major subsystem lives under src/<component>/" — xcecli (Go) already establishes that this applies across languages, not just C. Discovery confirmed a directly reusable custom_target() + wrapper-script pattern from xcecli/meson.build.
  • Trade-offs: None material — this is the path of least resistance given an existing, working precedent.
  • Follow-up: Confirm during implementation whether component meson option needs a new xceadmin choice or can use unconditional subdir() inclusion (both patterns exist for xcecli depending on option value — see src/meson.build:84-86).

Decision: plain byte-equality golden-file tests, not pytest-regressions

  • Context: External research surfaced pytest-regressions as the more idiomatic 2025/2026-era binary golden-file library versus brief.md's unspecified "compare binary, verify SHA-256."
  • Alternatives Considered: 1) pytest-regressions (file_regression.check(..., binary=True)). 2) pytest-golden (YAML-only, ruled out — no binary support). 3) Plain assert produced_bytes == open(expected_path, 'rb').read() in a parametrized fixture.
  • Selected Approach: Option 3 — plain assertion.
  • Rationale: pytest-regressions is not yet in this project's nix-pinned dependency list (brief.md's Nix Environment section) and would need independent nixpkgs-availability verification before adoption; the actual requirement (9.1-9.3, byte-for-byte determinism against a committed fixture) doesn't need file_regression's deliberate-update ergonomics for a small, stable fixture set (Simplification lens).
  • Trade-offs: Loses --force-regen's one-flag golden-file update convenience; gains zero new dependencies to verify/pin.
  • Follow-up: Revisit if the fixture set grows large enough that manual golden-file regeneration becomes a recurring pain point.

Decision: generic (opcode-agnostic) redirect-following and wire exchange

  • Context: Discovery found fill_redirect_reply() only wired for OPCode_join, and two distinct message shapes (Request/Reply-wrapped vs. ProbeConfigTagRequest/Reply direct-struct) both need send/receive.
  • Alternatives Considered: 1) An upgradeConfig-specific redirect function, built now and generalized later if dry-run/other opcodes need it. 2) One generic follow_redirects() against the RequestReply shape, and one generic send_request()/framing pair reused by both message shapes.
  • Selected Approach: Option 2, generic from the start.
  • Rationale: Generalization lens — both consumers (upgradeConfig today, join-shaped mocks in tests, potentially dry-run's direct-struct variant) are instances of the same "read RequestReply-shaped fields, frame-and-exchange bytes" problem; building the opcode-specific version first and generalizing later would mean deleting code, not adding it (the redirect logic and framing logic are trivial regardless — no meaningful cost to being generic from day one).
  • Trade-offs: None material.
  • Follow-up: None.

Decision: push_config()/probe_config() split into two functions, not one boolean-flagged function (Design Review correction, 2026-07-21, second pass)

  • Context: A second /kiro-validate-design pass found the first draft's push_config(..., dry_run: bool) -> PushResult implied a dry-run reply has acked_count, which ProbeConfigTagReply (only a configTag field) does not have — the shared return type would misrepresent one of the two call shapes.
  • Alternatives Considered: 1) Keep one function, make PushResult fields Optional for the dry-run case. 2) Two functions, two result types.
  • Selected Approach: Option 2 — push_config() -> PushResult and probe_config() -> DryRunResult.
  • Rationale: The two wire messages and replies share no fields; a boolean flag silently switching which fields are populated is a Type Safety anti-pattern this project's design-principles.md implicitly warns against ("discriminated unions for error handling" — same spirit applies to result shapes).
  • Trade-offs: None material — strictly a clarity improvement over the first draft.
  • Follow-up: None.

Decision: schema files are packaged as package_data, not referenced from src/schema/ at runtime

  • Context: User question after design approval — does pip install . (Req 8.2, non-editable) actually work? It doesn't as originally drafted: non-editable installs copy files into site-packages, severing any fixed relative path back to src/schema/controller.capnp. A __file__-relative walk (e.g. Path(__file__).parents[3] / "schema") only survives editable installs, where __file__ still resolves into the checked-out repo tree.
  • Alternatives Considered: 1) Require --schema-path on every invocation (no default) — rejected, bad ergonomics for the common case. 2) __file__-relative resolution — rejected, breaks for pip install .. 3) Package a synced copy of the schema as package_data, resolved via importlib.resources (packaging-aware, correct for both install modes).
  • Selected Approach: Option 3. src/schema/{controller,c}.capnp synced (cp) into src/xceadmin/src/xcecommon/schema_files/ by xce-build.sh's xceadmin) case on every build, packaged via pyproject.toml package_data, resolved at runtime via importlib.resources.files("xcecommon") / "schema_files" / "controller.capnp".
  • Rationale: importlib.resources is the standard-library-correct way to locate packaged non-code files regardless of install mode (editable, wheel-installed, zipped) — it doesn't care where site-packages physically is. src/schema/ remains the single source of truth (this spec doesn't own it, doesn't hand-edit the copy); the sync step is mechanical and runs every build, so it can't drift stale.
  • Trade-offs: One more build-step to keep in sync (mitigated: it's a single cp, not a generation/compilation step); the packaged copy is momentarily stale between a controller.capnp edit and the next xce-build xceadmin run — acceptable, matches how any other build artifact in this repo already works (nothing is watched/hot-reloaded).
  • Follow-up: None.

Risks & Mitigations

  • Controller OP_upgradeConfig dispatch defect (cluster.c:667-679 vs ha.c:3926-3951) — no role can process a push today. Mitigation: documented as an explicit Out-of-Boundary/Revalidation-Trigger item in design.md; config push implementation and testing proceed against the documented contract via a mock controller; real-cluster E2E is blocked until a separate fix (outside this spec) lands. Recommend raising this as its own tracked defect.
  • Redirect not wired for upgradeConfig — auto-discovery (Requirement 3) is untestable end-to-end against a real cluster for this opcode specifically. Mitigation: generic implementation is correct and forward-compatible; mock-controller tests exercise the logic using synthetic redirectHost/redirectPort replies.
  • ProbeConfigTag has no server handler — config push --dry-run cannot be validated live. Mitigation: already Nice-to-Have/Follow-up in requirements.md; client implemented against the schema contract, tested via mock only.
  • First Python component in the repo — nix/build/CI integration has no precedent to copy exactly (only an analogous one, xcecli's Go integration). Mitigation: xcecli's meson.build/custom_target() pattern is close enough structurally to de-risk this; flagged for extra scrutiny during task review.
  • pycapnp C-extension build inside nix — pip install pycapnp normally compiles a C extension against capnproto; getting this wrong would break nix develop. Mitigation: use python3Packages.pycapnp from nixpkgs (pre-built against nix-provided capnproto), not a bare pip install pycapnp, exactly as brief.md's Nix Environment section already specifies.

Decision: xceadmin is not wired into meson (Design Review correction, 2026-07-21)

  • Context: /kiro-validate-design review flagged that the initial design draft's src/xceadmin/meson.build (mirroring xcecli's custom_target()) was architecturally unsound — Go's pattern gates on a single static-binary output; pip install -e . mutates a venv/site-packages tree with no single trackable output for meson to gate on.
  • Alternatives Considered: 1) Force it through meson anyway via a stamp-file custom_target(). 2) Skip meson entirely, dispatch purely from xce-build.sh's case statement.
  • Selected Approach: Option 2.
  • Rationale: brief.md's own harness sketch (xce-build.sh's xceadmin) case) already specified direct pip install/pytest dispatch with no meson compile step — the meson integration was an unjustified addition during design drafting, not something the original harness plan called for.
  • Trade-offs: xceadmin doesn't participate in meson compile -C build <targets> the way C/Go components do; acceptable since it has no C-extension compile step of its own that meson would need to orchestrate (pycapnp's C extension is pip/nix's concern, not this project's meson graph).
  • Follow-up: None — resolved at design time, component meson option is unchanged.

Decision: bundle the 7 JSON Schema files too, not just controller.capnp (2026-07-21, post-approval user question)

  • Context: After design approval, user asked whether the JSON schemas used to validate YAML input should also ship with xceadmin, the same way controller.capnp does. Initial answer was "no, they're operator-supplied input per Req 1.1/1.6" — but on reconsideration, that framing didn't hold up: the JSON schemas are the formal spec of what xcecommon.mapping's hardcoded field-extraction logic (groupname, exclude_members, resname, …) actually expects, not independent operator content. An operator-managed copy that drifts from the shipped mapping code risks a silent failure mode — validation passes on YAML that then maps wrong (or rejects YAML that would map fine) — arguably worse than the capnp case (hard crash) because nothing raises. No requirement or brief text ever described org-custom validation schemas as an actual use case; the configs_dir/schema/ layout in brief.md's original sketch was inherited unreflectively from src/configs/'s example layout, not a deliberate feature.
  • Alternatives Considered: 1) Keep operator-supplied, configs_dir/schema/ required (original design). 2) Bundle as package_data, same mechanism as controller.capnp, configs_dir/schema/ becomes irrelevant.
  • Selected Approach: Option 2 — confirmed with the user directly (AskUserQuestion, "Yes, bundle" selected over "Keep operator-supplied").
  • Rationale: Consistency with the capnp decision's own rationale (version-lock a tool-internal contract to the release that consumes it); removes an entire operator-facing requirement (maintaining a schema/ folder they didn't author) for zero documented benefit forfeited.
  • Trade-offs: No mechanism left for an org to enforce custom/stricter YAML validation rules — acceptable, since none was ever a stated requirement; can be revisited as a follow-up feature (e.g. a future --extra-schema overlay) if a real need surfaces.
  • Process note: this reopened an approved acceptance criterion (Requirement 1.6) — amended requirements.md directly (added 5.5, rewrote 1.6 as "resolved, superseded") rather than looping back through /kiro-spec-requirements, since the user's direct confirmation in this conversation constitutes the approval for this specific amendment.
  • Follow-up: None.

References