Skip to content

api: round out Agent, Operator, Catalog, Health, Coordinate, Event endpoints#123

Merged
mbrulatout merged 9 commits into
criteo:masterfrom
mbrulatout:phase3-round-out-endpoints
Jul 6, 2026
Merged

api: round out Agent, Operator, Catalog, Health, Coordinate, Event endpoints#123
mbrulatout merged 9 commits into
criteo:masterfrom
mbrulatout:phase3-round-out-endpoints

Conversation

@mbrulatout

Copy link
Copy Markdown
Contributor

Summary

Phase 3 of the ongoing API-parity work (follow-up to #120/#121/#122) — closes out the remaining gaps tracked in ENDPOINT_STATUS.md for six existing endpoint groups, each as its own commit:

  • Agentleave(), reload(), metrics() (JSON + Prometheus), monitor() (single-shot, not true streaming — documented limitation), and a nested Agent.Token class for all five PUT /v1/agent/token/:type variants.
  • Operator — raft peer removal/leader-transfer, autopilot config/health, keyring list/install/use/remove, usage. CE-only, per the earlier "no Enterprise" decision.
  • Catalogfilter/peer/merge-central-config params, new gateway_services(), SkipNodeUpdate/TaggedAddresses on register().
  • Healthfilter/peer/merge-central-config params, new ingress().
  • Coordinatenode() read, update() write.
  • Eventdc on fire(), node/service/tag on list().
  • Transport layerHTTPClient.delete() gained a data param (needed by DELETE /v1/operator/keyring, which requires a JSON body — every other write method already had this, delete() was the one gap).

Bugs this surfaced (same pattern found twice)

PUT /v1/operator/autopilot/configuration with cas behaves exactly like Config.set()'s cas support from #121: always returns HTTP 200, with actual success/failure encoded as a JSON boolean body. Using status-only CB.boolean() there would have silently reported cas failures as successes — caught by a real integration test, fixed to use CB.json() matching the established Config precedent.

Other real findings from live-probing (not just docs)

  • GET /v1/coordinate/node/:node 404s in a fresh dev-mode Consul until a coordinate is actually written for that node — LAN coordinates aren't pre-populated by gossip the way coordinate/datacenters (WAN) is.
  • Event.list()'s node/service/tag params are accepted without error but have no observable filtering effect at list-time in a single-node topology — they appear to only gate gossip propagation at fire-time. Tests assert the real behavior, not the docs' implication.
  • /catalog/services' bexpr tag selector is ServiceTags, not Tags.
  • /catalog/node/:node's filter evaluates against each entry of the Services map, not the top-level document.
  • Health.ingress() deliberately excludes peer/merge-central-config even though Consul doesn't error if you pass them — the canonical docs explicitly say ingress doesn't support them, so the client matches the documented contract over the lenient runtime behavior.

Test plan

  • mypy consul/ --no-sqlite-cache — clean (32 source files)
  • ruff check . / ruff format --diff . — clean
  • New/extended integration tests across all six areas, run against real dockerized Consul (1.20.6/1.21.5/1.22.0) — 385 passing
  • Full existing test suite re-run — no regressions (only the 6 pre-existing failures in the untracked, unrelated tests/test_features.py)
  • Agent.monitor()'s test is intentionally scoped to the fast-fail path only (invalid loglevel → 400) — the success path is a genuinely unbounded blocking call with no per-request timeout anywhere in this library's transport layer, and there is no safe way to bound it from a test without either leaking a thread (Python's concurrent.futures atexit handler joins worker threads unconditionally, which would hang interpreter shutdown) or adding timeout support to std.py/aio.py, which is out of scope here.

Mathias Brulatout added 9 commits July 3, 2026 10:50
get()/put()/post() all accept a request body, but delete() didn't,
even though some Consul DELETE endpoints require one (e.g. DELETE
/v1/operator/keyring needs {"Key": ...} -- a bodyless request gets
rejected with "Request decode failed: EOF"). Widens delete() to
match, defaulting to "" so every existing caller is unaffected.
Adds Agent.leave(), Agent.reload(), Agent.metrics() (JSON and
Prometheus formats), Agent.monitor() (single-shot, not true
streaming -- documented limitation, see docstring), and a nested
Agent.Token class covering PUT /v1/agent/token/:type for all five
token types (default/agent/agent_recovery/replication/
config_file_service_registration).
Adds raft_remove_peer(), raft_transfer_leader(), autopilot
configuration read/write (with cas support), autopilot_health()
(handles the real 429-with-body "unhealthy" response instead of
raising), keyring list/install/use/remove, and usage(). CE-only,
per the earlier decision to exclude Enterprise entirely (License,
Area, Network Segments, and autopilot state's Enterprise fields are
intentionally not implemented).

Found and fixed the same CB.boolean()-hides-the-real-answer bug as
config.py's cas support: PUT .../autopilot/configuration with cas
always returns HTTP 200, encoding actual success/failure as a JSON
boolean body -- using CB.json() there instead, matching the
Config.delete() precedent.
Adds filter (bexpr) to nodes/services/service/connect/node, peer
and merge-central-config to service/connect, a new
gateway_services() method, and SkipNodeUpdate/TaggedAddresses
support on register(). Uncommented and extended
tests/api/test_catalog.py, which previously had no active tests.
Adds filter (bexpr) to node/checks/service/connect/state, peer and
merge-central-config to service/connect, and a new ingress() method
for GET /v1/health/ingress/:service (deliberately without peer,
matching Consul's documented ingress limitations rather than its
more lenient runtime behavior).
Adds Coordinate.node() (GET /v1/coordinate/node/:node) and
Coordinate.update() (PUT /v1/coordinate/update). Note: node()
404s in a fresh single-node dev-mode Consul until a coordinate has
actually been written for that node -- LAN coordinates aren't
pre-populated by gossip the way WAN datacenter coordinates are.
Adds dc to fire() and node/service/tag filters to list(). Verified
live that node/service/tag actually only take effect at fire time
(gossip propagation to matching agents), not as a list-time filter
on already-stored events -- tests assert accordingly rather than
asserting unverified filtering behavior.
Marks Agent, Operator, Catalog, Health, Coordinate, and Event as
fully covered per the endpoints added in the preceding commits.
CI caught what my local mypy/ruff-only verification missed: adding
filter/peer/merge-central-config pushed _service to 13 branches,
over pylint's default limit of 12. Extracted the tag-normalization
logic into a small typed _tag_params() static helper, which also
required a full type signature to satisfy disallow_untyped_calls.
@mbrulatout
mbrulatout merged commit 62fd8b5 into criteo:master Jul 6, 2026
5 checks passed
@mbrulatout
mbrulatout deleted the phase3-round-out-endpoints branch July 6, 2026 08: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