Skip to content

Add support for node identities to token operations#119

Merged
mougams merged 1 commit into
criteo:masterfrom
rsommer:feat/acl_support_node_identities
Jul 6, 2026
Merged

Add support for node identities to token operations#119
mougams merged 1 commit into
criteo:masterfrom
rsommer:feat/acl_support_node_identities

Conversation

@rsommer

@rsommer rsommer commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

feat(acl): support node identities in create and update methods.

Both methods gained support for specifying a list of node identities when creating or updating a token. The node identities have to be given in the form of ["node01:dc1", "node02:dc1] etc.

@rsommer
rsommer force-pushed the feat/acl_support_node_identities branch 2 times, most recently from 9e26765 to 0dea0e0 Compare June 26, 2026 12:54
@rsommer

rsommer commented Jun 29, 2026

Copy link
Copy Markdown
Contributor Author

Locally linting succeeds:

❯ tox -e lint
lint: venv> .venv/bin/uv venv -p ~/GIT/github.com/rsommer/py-consul/.venv/bin/python3 --allow-existing '--prompt=py-consul[lint]' --python-preference system /tmp/.tox-build-current/unknown/lint
lint: install_deps> .venv/bin/uv pip install -r tests-requirements.txt
.pkg: venv> .venv/bin/uv venv -p ~/GIT/github.com/rsommer/py-consul/.venv/bin/python3 --allow-existing '--prompt=py-consul[.pkg]' --python-preference system /tmp/.tox-build-current/unknown/.pkg
.pkg: install_requires> .venv/bin/uv pip install 'setuptools>=40.8.0'
.pkg: _optional_hooks> python ~/GIT/github.com/rsommer/py-consul/.venv/lib/python3.13/site-packages/pyproject_api/_backend.py True setuptools.build_meta __legacy__
.pkg: get_requires_for_build_sdist> python ~/GIT/github.com/rsommer/py-consul/.venv/lib/python3.13/site-packages/pyproject_api/_backend.py True setuptools.build_meta __legacy__
.pkg: get_requires_for_build_wheel> python ~/GIT/github.com/rsommer/py-consul/.venv/lib/python3.13/site-packages/pyproject_api/_backend.py True setuptools.build_meta __legacy__
.pkg: prepare_metadata_for_build_wheel> python ~/GIT/github.com/rsommer/py-consul/.venv/lib/python3.13/site-packages/pyproject_api/_backend.py True setuptools.build_meta __legacy__
.pkg: build_sdist> python ~/GIT/github.com/rsommer/py-consul/.venv/lib/python3.13/site-packages/pyproject_api/_backend.py True setuptools.build_meta __legacy__
lint: install_package_deps> .venv/bin/uv pip install 'requests>=2.0'
lint: install_package> .venv/bin/uv pip install --reinstall --no-deps py-consul@/tmp/.tox-build-current/unknown/.tmp/package/1/py_consul-1.7.1.tar.gz
lint: commands[0]> pre-commit run --all-files
ruff.....................................................................Passed
ruff format..............................................................Passed
pylint static code analyzer..............................................Passed
mypy.....................................................................Passed
  lint: OK (82.72=setup[6.81]+cmd[75.90] seconds)
  congratulations :) (82.77 seconds)

@mougams mougams left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thank you for your patch - a comment though

Comment thread consul/api/acl/token.py Outdated
@rsommer
rsommer force-pushed the feat/acl_support_node_identities branch from 0dea0e0 to bf6d5c5 Compare June 29, 2026 08:37
@rsommer
rsommer requested a review from mougams June 29, 2026 08:56

@mougams mougams left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@mougams

mougams commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

@rsommer could you rebase and push again ? All the tests are failing due to mypy. Tried unsuccessfully a failed jobs retrigger 😕

@mougams

mougams commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

@rsommer could you rebase and push again ? All the tests are failing due to mypy. Tried unsuccessfully a failed jobs retrigger 😕

nvm it should be fixed once this one #120 is approved and merged.

@rsommer
rsommer force-pushed the feat/acl_support_node_identities branch from bf6d5c5 to e037db1 Compare July 5, 2026 15:54
@rsommer

rsommer commented Jul 5, 2026

Copy link
Copy Markdown
Contributor Author

@rsommer could you rebase and push again ? All the tests are failing due to mypy. Tried unsuccessfully a failed jobs retrigger 😕

Done, all checks have passed ;)

Comment thread consul/api/acl/token.py Outdated
import builtins


def parse_identities(node_identities: list[str] | None) -> dict[typing.Any, typing.Any]:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sorry, just saw it - could you replace the return type by

dict[str, list[dict[str, str]]]

to make it more accurate.

feat(acl): support node identities in create and update methods.

Both methods gained support for specifying a list of node identities
when creating or updating a token. The node identities have to be given
in the form of `["node01:dc1", "node02:dc1]` etc.
@rsommer
rsommer force-pushed the feat/acl_support_node_identities branch from e037db1 to 2509062 Compare July 6, 2026 08:18
@mougams
mougams merged commit 84c33e7 into criteo:master Jul 6, 2026
5 checks passed
@mougams

mougams commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

All merged - thx @rsommer

@rsommer
rsommer deleted the feat/acl_support_node_identities branch July 6, 2026 11:06
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