Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
21a0e94
Add revoke Vault token functionality to OIDC lookup creds
fincamd Mar 11, 2026
a10e229
Add linter suggestions
fincamd Mar 16, 2026
62aedd4
Add more unit tests
fincamd Mar 16, 2026
5d36c79
Refactor tests
fincamd Mar 16, 2026
a7ed1c8
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Mar 16, 2026
9b6e940
Address pre-commit failures
fincamd Mar 16, 2026
40c5606
Add test case for kv_backend v2
fincamd Mar 16, 2026
6f02418
Add more test cases
fincamd Mar 17, 2026
5f0703a
Use urllib join instead of ospath join
fincamd Mar 23, 2026
0aaed5f
Move token definition out of try block
fincamd Mar 23, 2026
3da915f
Simplify kv_backend to meet pre-commit linter
fincamd Mar 23, 2026
025669d
Fix token typing linter
fincamd Mar 23, 2026
c50577c
Remove unused parameter in test token empty
fincamd Mar 23, 2026
52931cb
Add context manager to manage vault token
fincamd Mar 24, 2026
c7674ca
Address linter issues and restore removed comments
fincamd Mar 25, 2026
f0185c3
Addressed PR review suggestions
fincamd Mar 26, 2026
97c080a
Fix typevar definition in decorator function for mypy
fincamd Mar 27, 2026
cb1dc4f
Made utility revoke_self_token function private
fincamd Mar 27, 2026
e66bbd8
Set default value for namespace and cacert calling revoke_self_token
fincamd Mar 27, 2026
a1beb25
Use autospec in the test mocks
fincamd Mar 27, 2026
1758c9b
Remove suppress exception context
fincamd Mar 27, 2026
58ba8ac
Use backend function reference instead of getattr lookup in tests
fincamd Mar 27, 2026
705a527
Use less generic name in a test kwargs var
fincamd Mar 27, 2026
942d309
Use a more realistic exception being raised in tests
fincamd Mar 27, 2026
c14ad50
Add back lost comment
fincamd Mar 27, 2026
60ad1c8
📝 Record the detached param spec justification
webknjaz Mar 27, 2026
d4fd375
✨ Enforce `_revoke_self_token` to be kwarg-only
webknjaz Mar 27, 2026
2832592
💅 Pass token injection in decorator explicitly
webknjaz Mar 27, 2026
105f3c0
💅 Concat token type in decorated callable def
webknjaz Mar 27, 2026
39eecf0
🧪 Let MyPy know that `token` is internal
webknjaz Mar 30, 2026
4f6b847
🧪 Patch `requests.Session` though actual module
webknjaz Mar 30, 2026
bff3686
🧪💅 Move most in-test mock defs into fixtures
webknjaz Mar 30, 2026
400d016
🧪 Use indentity check for test function params
webknjaz Mar 30, 2026
f2801f8
Unify mock @ test_backend_revokes_oidc_token
webknjaz Mar 31, 2026
8350839
💅 Replace token arg injection w/ `contextvars`
webknjaz Mar 31, 2026
0abbb41
🧪 Suppress WPS201 in `hashivault` module
webknjaz Mar 31, 2026
170c75d
🧪 Let pylint know private access is ok in tests
webknjaz Mar 31, 2026
f5203c6
🧪 Make MyPy allow private attr access in tests
webknjaz Mar 31, 2026
09eb6e5
🩹📝 Suppress types that Sphinx can't resolve
webknjaz Mar 31, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .flake8
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ per-file-ignores =
src/awx_plugins/credentials/dsv.py: ANN003, ANN201, D100, D103, P103, WPS210
# NOTE: `awx_plugins.credentials.github_app` may need restructuring to comply with WPS 202.
src/awx_plugins/credentials/github_app.py: WPS202
src/awx_plugins/credentials/hashivault.py: ANN003, ANN201, B950, C901, CCR001, D100, D103, LN001, N400, WPS202, WPS204, WPS210, WPS221, WPS223, WPS229, WPS231, WPS232, WPS331, WPS336, WPS337, WPS432, WPS454
src/awx_plugins/credentials/hashivault.py: ANN003, ANN201, B950, C901, CCR001, D100, D103, LN001, N400, WPS201, WPS202, WPS204, WPS210, WPS221, WPS223, WPS229, WPS231, WPS232, WPS331, WPS336, WPS337, WPS432, WPS454
src/awx_plugins/credentials/injectors.py: ANN001, ANN201, ANN202, C408, D100, D103, WPS110, WPS111, WPS202, WPS210, WPS347, WPS433, WPS440
src/awx_plugins/credentials/plugin.py: ANN001, ANN002, ANN101, ANN201, ANN204, B010, D100, D101, D103, D105, D107, D205, D400, E731, WPS432, WPS433, WPS440, WPS442, WPS601
src/awx_plugins/credentials/plugins.py: B950,D100, D101, D103, D105, D107, D205, D400, LN001, WPS204, WPS229, WPS433, WPS440
Expand Down
9 changes: 9 additions & 0 deletions .mypy.ini
Original file line number Diff line number Diff line change
Expand Up @@ -111,3 +111,12 @@ disallow_any_expr = false
disallow_any_expr = false
# fails on `@hypothesis.given()`:
disallow_any_decorated = false

# It is sometimes necessary to access "officially-private" attributes in our
# own tests for the purposes of assertion, mocking and spying on objects. And
# that's fine since it's within the same repository fully under our control,
# and so we don't need to have MyPy complain about those in this specific
# context. This is why we disable `attr-defined` in our tests [1].
#
# [1] https://til.codeinthehole.com/posts/how-to-handle-convenience-imports-with-mypy/
disable_error_code = attr-defined
6 changes: 6 additions & 0 deletions docs/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,12 @@
'py:class',
'awx_plugins.interfaces._temporary_private_credential_api.Credential',
),
( # generic return type variable:
'py:class',
'awx_plugins.credentials.hashivault._RT',
),
('py:class', '_PT'), # generic ParamSpec type variable
('py:class', '_contextvars.ContextVar'), # unresolved context var type
('py:class', 'EnvVarsType'),
]

Expand Down
178 changes: 139 additions & 39 deletions src/awx_plugins/credentials/hashivault.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,14 @@
# FIXME: the following violations must be addressed gradually and unignored
# mypy: disable-error-code="arg-type, no-untyped-call, no-untyped-def"

import contextlib as _ctx
import contextvars as _ctx_vars
import functools as _functools
import os
import pathlib
import time
import typing as _t
from collections import abc as _abc
from urllib.parse import urljoin

from awx_plugins.interfaces._temporary_private_django_api import ( # noqa: WPS436
Expand All @@ -16,6 +21,14 @@
from .plugin import CertFiles, CredentialPlugin, raise_for_status


_AUTH_TOKEN: _ctx_vars.ContextVar[str] = _ctx_vars.ContextVar('_AUTH_TOKEN')
"""Authentication token for use in plugin handlers."""


class _EmptyKwargs(_t.TypedDict):
"""Schema for zero keyword arguments."""


# Base input fields
url_field: _types.FieldDict = {
'id': 'url',
Expand Down Expand Up @@ -481,6 +494,76 @@ def workload_identity_auth(**kwargs):
return {'role': kwargs.get('jwt_role'), 'jwt': workload_identity_token}


def _revoke_self_token(
*,
vault_token: str,
url: str,
namespace: str,
cacert: str | None = None,
) -> None:
"""Revoke the passed-in Vault token."""
url = urljoin(url, 'v1/auth/token/revoke-self')
sess = requests.Session()
sess.headers['X-Vault-Token'] = vault_token
if namespace != '':
sess.headers['X-Vault-Namespace'] = namespace
with CertFiles(cacert) as cert:
resp = sess.post(url, verify=cert, timeout=30)
resp.raise_for_status()


@_ctx.contextmanager
def _vault_token(**kwargs: str) -> _abc.Iterator[str]:
"""Context manager that yields a Vault token and revokes it on exit if obtained via workload identity."""
is_oidc_context = 'workload_identity_token' in kwargs
token = handle_auth(**kwargs)
try:
yield token
finally:
# Only revoke tokens obtained via OIDC authentication
if is_oidc_context:
_revoke_self_token(
vault_token=token,
url=kwargs['url'],
namespace=kwargs.get('namespace', ''),
cacert=kwargs.get('cacert'),
)


@_ctx.contextmanager
def _token_in_context(token: str, /) -> _abc.Iterator[None]:
"""Set a token for the execution context lifetime."""
var_ctx_token = _AUTH_TOKEN.set(token)
try:
yield
finally:
_AUTH_TOKEN.reset(var_ctx_token)


# Param Spec to represent decorated function parameters
_PT = _t.ParamSpec( # FIXME: Use [_RT, **_PT] in the signature in Python 3.12
'_PT',
)
# TypeVar to represent decorated function return type
_RT = _t.TypeVar('_RT')


def _inject_auth_token_with_revocation(
decorated_function: _t.Callable[_PT, _RT],
/,
) -> _t.Callable[_PT, _RT]:
@_functools.wraps(decorated_function)
def _decorate_the_function_with_revocation( # noqa: WPS430 -- in-decorator
*args: _PT.args,
**kwargs: _PT.kwargs,
) -> _RT:
with _vault_token(**kwargs) as token:
with _token_in_context(token):
return decorated_function(*args, **kwargs)

return _decorate_the_function_with_revocation


def method_auth(**kwargs):
# get auth method specific params
request_kwargs = {'json': kwargs['auth_param'], 'timeout': 30}
Expand Down Expand Up @@ -522,32 +605,39 @@ def method_auth(**kwargs):
return token


def kv_backend(**kwargs):
token = handle_auth(**kwargs)
url = kwargs['url']
secret_path = kwargs['secret_path']
secret_backend = kwargs.get('secret_backend')
secret_key = kwargs.get('secret_key')
cacert = kwargs.get('cacert')
api_version = kwargs['api_version']

@_inject_auth_token_with_revocation
# NOTE: The "too many args" rules of flake8 and pylint are disabled due to such
# NOTE: many arguments being a common public plugin API at the moment.
# pylint: disable-next=too-many-arguments
def kv_backend( # noqa: WPS211 -- the same as too-many-arguments
*,
url: str,
api_version: str,
secret_path: str,
secret_key: str | None = None,
secret_backend: str | None = None,
secret_version: str | None = None,
cacert: str | None = None,
namespace: str | None = None,
**_discarded_kwargs: _t.Unpack[_EmptyKwargs],
) -> str:
request_kwargs = {
'timeout': 30,
'allow_redirects': False,
}

sess = requests.Session()
sess.mount(url, requests.adapters.HTTPAdapter(max_retries=5))
sess.headers['Authorization'] = f'Bearer {token}'
sess.headers['Authorization'] = f'Bearer {_AUTH_TOKEN.get()}'
# Compatibility header for older installs of Hashicorp Vault
sess.headers['X-Vault-Token'] = token
if kwargs.get('namespace'):
sess.headers['X-Vault-Namespace'] = kwargs['namespace']
sess.headers['X-Vault-Token'] = _AUTH_TOKEN.get()
if namespace:
sess.headers['X-Vault-Namespace'] = namespace

if api_version == 'v2':
if kwargs.get('secret_version'):
if secret_version:
request_kwargs['params'] = { # type: ignore[assignment] # FIXME
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The lost comment needs to be re-explained, obviously. However, I advise against indenting entire function bodies.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This comment was removed to make the linter happy because that line was triggering the "comment line too long"

Copy link
Copy Markdown
Contributor Author

@fincamd fincamd Mar 27, 2026

Choose a reason for hiding this comment

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

Added back here 0b60950

'version': kwargs['secret_version'],
'version': secret_version,
}
if secret_backend:
path_segments = [secret_backend, 'data', secret_path]
Expand All @@ -556,7 +646,6 @@ def kv_backend(**kwargs):
mount_point, *path = pathlib.Path(
secret_path.lstrip(os.sep),
).parts
'/'.join(path)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

It's a bit hard to tell without ignoring whitespace (https://github.com/ansible/awx-plugins/pull/176/changes?w=1), but this looks like a good catch - '/'.join(path) was just dead code so just that line is deleted:

try:
mount_point, *path = pathlib.Path(
secret_path.lstrip(os.sep),
).parts
'/'.join(path)
except Exception:

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes! That was dead code, I deleted it in this branch

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Let's not make unrelated functional or formatting changes in this branch.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Well, it's just a line deletion... I think we can go forward. It was just dead code anyways

except Exception:
mount_point, path = secret_path, []
# https://www.vaultproject.io/api/secret/kv/kv-v2.html#read-secret-version
Expand Down Expand Up @@ -593,40 +682,51 @@ def kv_backend(**kwargs):
)
and ('data' in json['data'])
):
return json['data']['data'][secret_key]
return json['data'][secret_key]
return str(json['data']['data'][secret_key])
return str(json['data'][secret_key])
except KeyError:
raise RuntimeError(f'{secret_key} is not present at {secret_path}')
return json['data']


def ssh_backend(**kwargs):
token = handle_auth(**kwargs)
url = urljoin(kwargs['url'], 'v1')
secret_path = kwargs['secret_path']
role = kwargs['role']
cacert = kwargs.get('cacert')
raise RuntimeError(
f'{secret_key} is not present at {secret_path}',
)
return str(json['data'])


@_inject_auth_token_with_revocation
# NOTE: The "too many args" rules of flake8 and pylint are disabled due to such
# NOTE: many arguments being a common public plugin API at the moment.
# pylint: disable-next=too-many-arguments
def ssh_backend( # noqa: WPS211 -- the same as too-many-arguments
*,
url: str,
secret_path: str,
role: str,
public_key: str,
cacert: str | None = None,
namespace: str | None = None,
valid_principals: str | None = None,
**_discarded_kwargs: _t.Unpack[_EmptyKwargs],
) -> str:
url = urljoin(url, 'v1')

request_kwargs = {
'timeout': 30,
'allow_redirects': False,
'json': {
'public_key': public_key,
},
}

request_kwargs['json'] = { # type: ignore[assignment] # FIXME
'public_key': kwargs['public_key'],
}
if kwargs.get('valid_principals'):
if valid_principals:
request_kwargs['json'][ # type: ignore[index] # FIXME
'valid_principals'
] = kwargs['valid_principals']
] = valid_principals

sess = requests.Session()
sess.mount(url, requests.adapters.HTTPAdapter(max_retries=5))
sess.headers['Authorization'] = f'Bearer {token}'
if kwargs.get('namespace'):
sess.headers['X-Vault-Namespace'] = kwargs['namespace']
sess.headers['Authorization'] = f'Bearer {_AUTH_TOKEN.get()}'
if namespace:
sess.headers['X-Vault-Namespace'] = namespace
# Compatibility header for older installs of Hashicorp Vault
sess.headers['X-Vault-Token'] = token
sess.headers['X-Vault-Token'] = _AUTH_TOKEN.get()
# https://www.vaultproject.io/api/secret/ssh/index.html#sign-ssh-key
request_url = '/'.join([url, secret_path, 'sign', role]).rstrip('/')

Expand All @@ -642,7 +742,7 @@ def ssh_backend(**kwargs):
else:
break
raise_for_status(resp)
return resp.json()['data']['signed_key']
return str(resp.json()['data']['signed_key'])


hashivault_kv_plugin = CredentialPlugin(
Expand Down
Loading
Loading