acl/agent/connect: close remaining tracked endpoint gaps#124
Open
mbrulatout wants to merge 4 commits into
Open
acl/agent/connect: close remaining tracked endpoint gaps#124mbrulatout wants to merge 4 commits into
mbrulatout wants to merge 4 commits into
Conversation
added 4 commits
July 6, 2026 10:45
Token: add policy/role/authmethod/servicename filters to list(); add service_identities, expiration_time, expiration_ttl, local to create()/update(). Extracted the duplicated policy/role/templated- policy body-building logic (identical in create() and update()) into shared, typed helper functions -- both a DRY cleanup and a fix for a pylint too-many-branches violation the duplication caused. Policy: add update() and delete(), the two CRUD operations that were still missing. Verified against a live Consul instance that Local=true tokens reject a service identity's Datacenters scoping (a real, undocumented-in-the-obvious-place interaction), and that Rules must be passed as a dict (JSON-syntax ACL rules), not a raw HCL string -- matches the existing create() convention.
Adds filter (bexpr) and token to services()/checks(); prune to force_leave() (Consul 1.13+); Kind/Proxy/SocketPath/Locality to Service.register(). Extracted the new register() fields into a small helper to keep it under ruff's mccabe complexity limit -- it was already at the edge before this change. Verified against a live Consul instance that a connect-proxy registration requires a top-level Port or SocketPath (distinct from Proxy.LocalServicePort), and that force-leave on a node Consul has never seen is a real server-side error, not a no-op success.
Adds Connect.CA.update_configuration() for PUT /v1/connect/ca/ configuration, the write counterpart to the existing read-only configuration(). Requires operator:write (elevated from operator:read prior to Consul 1.6.10/1.7.10/1.8.6).
Marks Agent, ACL Token/Policy, and Connect CA as fully covered. Every remaining entry not marked done is an intentional exclusion: Enterprise-only params/endpoints, the deprecated pre-1.9 legacy intentions model, and Agent.monitor()'s documented lack of true streaming.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes out essentially every remaining gap tracked in
ENDPOINT_STATUS.md. After this, everything left unmarked is an intentional exclusion (Enterprise-only, deprecated legacy intentions, orAgent.monitor()'s documented streaming limitation).policy/role/authmethod/servicenamefilters onlist();service_identities,expiration_time,expiration_ttl,localoncreate()/update(). Also extracted the policy/role/templated-policy body-building logic (identical in both methods) into shared typed helpers — a DRY fix that also resolved a pylint too-many-branches violation the duplication caused.update()anddelete()— the only two CRUD gaps left.filter+tokenonservices()/checks();pruneonforce_leave();Kind/Proxy/SocketPath/LocalityonService.register().CA.update_configuration(), the write counterpart to the existing read-onlyconfiguration().Also: removed the unused
features/registryNot part of this PR's diff (it turned out
consul/features/*.pyandtests/test_features.pywere never committed to any branch — confirmed viagit log --all), but worth noting: those files, sitting locally the whole time, accounted for the "6 pre-existing unrelated test failures" mentioned across every prior PR in this series. They're gone now, which is why this PR's own CI should be fully clean rather than showing that familiar noise.Real findings from live-probing
Datacentersscoping when combined withLocal: true("cannot specify a list of datacenters on a local token") — a real, non-obvious interaction, covered by two separate tests instead of one combined one.RulesonACL Policymust be passed as a dict (JSON-syntax ACL rules) that getsjson.dumps()'d once, not a raw HCL string — matches the existing (previously untested) convention increate().connect-proxyservice registration requires a top-levelPortorSocketPath, distinct fromProxy.LocalServicePort.Test plan
pre-commit run --all-files(ruff, ruff format, pylint, mypy) — clean, run from scratch multiple times during development after two real pylint/ruff-mccabe violations were caught and fixed (Token.create/update's branch count,Service.register's cyclomatic complexity)