Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add assumerole #24

Open
wants to merge 67 commits into
base: devel
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
67 commits
Select commit Hold shift + click to select a range
764bb69
Add a credential plugin to implement AWS AssumeRole functionality
derekwaters Aug 29, 2024
5d19ff1
Fix EoF
derekwaters Aug 29, 2024
cf6d481
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Aug 29, 2024
24ce59c
add doc strings and adjust entry point in pyproject toml
thedoubl3j Aug 29, 2024
b50e003
move to monkeypatch for test, add deps and more clean up
thedoubl3j Sep 5, 2024
35e5dcb
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Sep 5, 2024
656ecb7
fix some linting and add ignores
thedoubl3j Sep 5, 2024
847d081
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Sep 5, 2024
1807b76
update accesssecrettest to add typing
thedoubl3j Sep 6, 2024
648e8f7
update arnonly test with typing
thedoubl3j Sep 6, 2024
4dd176a
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Sep 6, 2024
c6a99ae
remove datetime as its found in stdlib
thedoubl3j Sep 6, 2024
fb2a28c
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Sep 6, 2024
5268832
update inputs field to fit formatting
thedoubl3j Sep 6, 2024
d9435eb
update assumerole and rebase
thedoubl3j Sep 26, 2024
836b5c9
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Sep 26, 2024
e949dd2
parameterize tests and fix more linting
thedoubl3j Sep 27, 2024
2d48e82
linter updates, update pre-commit with new deps, update expired logic
thedoubl3j Oct 1, 2024
ef334bc
more linter fixes, add some ignores and fix toml dep
thedoubl3j Oct 1, 2024
814e586
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Oct 1, 2024
93a28bd
Update .flake8
thedoubl3j Oct 2, 2024
1345871
Update tests/importable_test.py
thedoubl3j Oct 2, 2024
a0737ef
Fix pytest params nesting
webknjaz Oct 3, 2024
29aee1e
Update pyproject.toml
thedoubl3j Oct 7, 2024
86eef44
Update src/awx_plugins/credentials/aws_assumerole.py
thedoubl3j Oct 7, 2024
ea9d0e7
Apply suggestions from code review
thedoubl3j Oct 8, 2024
f1e4035
more linter fixes, add some ignores and fix toml dep
thedoubl3j Oct 1, 2024
4e7cc43
rebase and include fixes from sviat
thedoubl3j Oct 8, 2024
499c494
Move cred plugin test rule ignores to the first flake8 entry
webknjaz Oct 8, 2024
87cd182
Import gettext_noop from the interfaces dep
webknjaz Oct 8, 2024
5d346ff
Reference existing `client_err` in exception handling
webknjaz Oct 8, 2024
3f4340c
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Oct 8, 2024
3e44826
Mention that boto3 is also a dep of the new plugin
webknjaz Oct 8, 2024
b308b67
List botocore after boto3 in the deps
webknjaz Oct 8, 2024
a132eeb
Fix kwargs variants definition in tests
webknjaz Oct 8, 2024
d441571
fix imports and prep for consuming sviat suggestions
thedoubl3j Oct 8, 2024
9883c3d
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Oct 8, 2024
b565e0f
Apply suggestions from code review from @webknjaz
thedoubl3j Oct 8, 2024
e876ff0
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Oct 8, 2024
bf1ec34
Use `Never` from `typing`
webknjaz Oct 9, 2024
753b6af
Import missing `CredentialsTypeDef`
webknjaz Oct 9, 2024
d5f99d6
Make the import line fit the limit
webknjaz Oct 9, 2024
b1a2129
Quote conditionally imported types
webknjaz Oct 9, 2024
9430333
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Oct 9, 2024
e93e917
Quote type concats
webknjaz Oct 9, 2024
4ada473
Fix boto3 type refs in docs
webknjaz Oct 9, 2024
0b1e5ef
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Oct 9, 2024
fb85b7e
Unbreak the docstring
webknjaz Oct 9, 2024
444d121
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Oct 9, 2024
74b9206
Fix DAR @ Sphinx config
webknjaz Oct 9, 2024
c4dd94d
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Oct 9, 2024
50fe83e
Install `botocore` type stubs in addition to `boto3`
webknjaz Oct 9, 2024
803c659
Merge the kwargs dicts
webknjaz Oct 9, 2024
83ab18e
Simplify extra kwargs
webknjaz Oct 9, 2024
3f8bb9e
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Oct 9, 2024
669ba01
Drop static kwargs from params
webknjaz Oct 9, 2024
881976f
Clarify that kwargs are actually creds
webknjaz Oct 9, 2024
53fb4a2
Add ids to assumerole id tests
webknjaz Oct 9, 2024
f7db82c
Fixup: pass ids to the parametrize decorator
webknjaz Oct 9, 2024
9842a9b
update types, fix imports to resolve modules not found
thedoubl3j Oct 9, 2024
c75d33c
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Oct 9, 2024
ff886aa
Drop the unnecessary django stubs
webknjaz Oct 10, 2024
0dcc430
Drop the duplicate cred plugin import
webknjaz Oct 10, 2024
0eb6e86
Keep none creds args expected
webknjaz Oct 10, 2024
164b54a
Pass the required external id in tests
webknjaz Oct 10, 2024
db9e869
Apply suggestions from code review
thedoubl3j Oct 10, 2024
cbeea1c
linter updates, update pre-commit with new deps, update expired logic
thedoubl3j Oct 1, 2024
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
19 changes: 10 additions & 9 deletions .flake8
Original file line number Diff line number Diff line change
Expand Up @@ -111,19 +111,20 @@ per-file-ignores =
tests/**.py: DAR, DCO020, S101, S105, S108, S404, S603, WPS202, WPS210, WPS430, WPS436, WPS441, WPS442, WPS450

# The following ignores must be fixed and the entries removed from this config:
src/awx_plugins/credentials/aim.py: ANN003, ANN201, B950, CCR001, D100, D103, LN001, Q003, WPS210, WPS221, WPS223, WPS231, WPS336, WPS432
src/awx_plugins/credentials/aws_secretsmanager.py: ANN003, ANN201, D100, D103, WPS111, WPS210, WPS329, WPS529
src/awx_plugins/credentials/azure_kv.py: ANN003, ANN201, D100, D103, WPS111, WPS361, WPS421
src/awx_plugins/credentials/centrify_vault.py: ANN003, ANN201, D100, D103, N802, P101, WPS210, WPS229
src/awx_plugins/credentials/conjur.py: ANN003, ANN201, B950, D100, D103, E800, P101, WPS111, WPS210, WPS229, WPS432, WPS440
src/awx_plugins/credentials/dsv.py: ANN003, ANN201, D100, D103, P103, WPS210
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/injectors.py: ANN001, ANN201, ANN202, C408, D100, D103, WPS110, WPS111, WPS202, WPS210, WPS347, WPS433, WPS440
src/awx_plugins/credentials/aim.py: ANN003, ANN201, B950, CCR001, D100, D103, LN001, Q003, WPS210, WPS221, WPS223, WPS226, WPS231, WPS336, WPS432
src/awx_plugins/credentials/aws_secretsmanager.py: ANN003, ANN201, D100, D103, WPS111, WPS210, WPS226, WPS329, WPS529
src/awx_plugins/credentials/aws_assumerole.py: WPS226, WPS332
thedoubl3j marked this conversation as resolved.
Show resolved Hide resolved
src/awx_plugins/credentials/azure_kv.py: ANN003, ANN201, D100, D103, WPS111, WPS226, WPS361, WPS421
src/awx_plugins/credentials/centrify_vault.py: ANN003, ANN201, D100, D103, N802, P101, WPS210, WPS226, WPS229
src/awx_plugins/credentials/conjur.py: ANN003, ANN201, B950, D100, D103, E800, P101, WPS111, WPS210, WPS226, WPS229, WPS432, WPS440
src/awx_plugins/credentials/dsv.py: ANN003, ANN201, D100, D103, P103, WPS210, WPS226
src/awx_plugins/credentials/hashivault.py: ANN003, ANN201, B950, C901, CCR001, D100, D103, LN001, N400, WPS202, WPS204, WPS210, WPS221, WPS223, WPS226, WPS229, WPS231, WPS232, WPS331, WPS336, WPS337, WPS432, WPS454
src/awx_plugins/credentials/injectors.py: ANN001, ANN201, ANN202, C408, D100, D103, WPS110, WPS111, WPS202, WPS210, WPS226, WPS347, WPS433, WPS440
src/awx_plugins/credentials/plugin.py: ANN001, ANN002, ANN101, ANN201, ANN204, B010, D100, D101, D103, D105, D107, D205, D400, E731, WPS115, WPS432, WPS433, WPS440, WPS442, WPS601
src/awx_plugins/credentials/plugins.py: B950,D100, D101, D103, D105, D107, D205, D400, LN001, WPS204, WPS229, WPS433, WPS440
src/awx_plugins/credentials/tss.py: ANN003, ANN201, D100, D103, E712, WPS433, WPS440, WPS503
src/awx_plugins/inventory/plugins.py: ANN001, ANN002, ANN003, ANN101, ANN102, ANN201, ANN202, ANN206, B950, C812, C819, D100, D101, D102, D205, D209, D400, D401, LN001, LN002, N801, WPS110, WPS111, WPS202, WPS210, WPS214, WPS301, WPS319, WPS324, WPS331, WPS336, WPS337, WPS338, WPS347, WPS421, WPS433, WPS450, WPS510, WPS529
tests/credential_plugins_test.py: ANN101, B017, C419, D100, D102, D103, D205, D209, D400, DAR, PT011, S105, WPS111, WPS117, WPS118, WPS202, WPS352, WPS421, WPS433, WPS507
tests/credential_plugins_test.py: ANN101, B017, C419, D100, D102, D103, D205, D209, D400, DAR, PT011, S105, WPS111, WPS117, WPS118, WPS202, WPS352, WPS421, WPS430, WPS433, WPS507
tests/importable_test.py: ANN101, DAR

# Count the number of occurrences of each error/warning code and print a report:
Expand Down
9 changes: 6 additions & 3 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,8 @@ repos:
@ git+https://github.com/ansible/awx_plugins.interfaces.git@ad4a965
- azure-identity # needed by credentials.azure_kv
- azure-keyvault # needed by credentials.azure_kv
- boto3-stubs # needed by credentials.awx_secretsmanager
- boto3-stubs[sts] # needed by credentials.awx_secretsmanager
webknjaz marked this conversation as resolved.
Show resolved Hide resolved
- botocore-stubs # needed by credentials.awx_secretsmanager
- lxml # dep of `--txt-report`, `--cobertura-xml-report` & `--html-report`
- msrestazure # needed by credentials.azure_kv
- pytest
Expand Down Expand Up @@ -227,7 +228,8 @@ repos:
@ git+https://github.com/ansible/awx_plugins.interfaces.git@ad4a965
- azure-identity # needed by credentials.azure_kv
- azure-keyvault # needed by credentials.azure_kv
- boto3-stubs # needed by credentials.awx_secretsmanager
- boto3-stubs[sts] # needed by credentials.awx_secretsmanager
webknjaz marked this conversation as resolved.
Show resolved Hide resolved
- botocore-stubs # needed by credentials.awx_secretsmanager
- lxml # dep of `--txt-report`, `--cobertura-xml-report` & `--html-report`
- msrestazure # needed by credentials.azure_kv
- pytest
Expand Down Expand Up @@ -257,7 +259,8 @@ repos:
@ git+https://github.com/ansible/awx_plugins.interfaces.git@ad4a965
- azure-identity # needed by credentials.azure_kv
- azure-keyvault # needed by credentials.azure_kv
- boto3-stubs # needed by credentials.awx_secretsmanager
- boto3-stubs[sts] # needed by credentials.awx_secretsmanager
webknjaz marked this conversation as resolved.
Show resolved Hide resolved
- botocore-stubs # needed by credentials.awx_secretsmanager
- lxml # dep of `--txt-report`, `--cobertura-xml-report` & `--html-report`
- msrestazure # needed by credentials.azure_kv
- pytest
Expand Down
54 changes: 54 additions & 0 deletions docs/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,15 @@
from pathlib import Path
from tomllib import loads as _parse_toml

from sphinx.addnodes import pending_xref
from sphinx.application import Sphinx
from sphinx.environment import BuildEnvironment


# isort: split

from docutils.nodes import literal, reference


# -- Path setup --------------------------------------------------------------

Expand Down Expand Up @@ -195,3 +204,48 @@
nitpick_ignore = [
# temporarily listed ('role', 'reference') pairs that Sphinx cannot resolve
]


def _replace_missing_boto3_reference(
app: Sphinx,
env: BuildEnvironment,
node: pending_xref,
contnode: literal,
) -> reference | None:
if (node.get('refdomain'), node.get('reftype')) != ('py', 'class'):
return None

boto3_type_uri_map = {
'AssumeRoleResponseTypeDef': 'type_defs/#assumeroleresponsetypedef',
'CredentialsTypeDef': 'type_defs/#credentialstypedef',
'STSClient': 'client/#stsclient',
}
ref_target = node.get('reftarget', '')

try:
return reference(
ref_target,
ref_target,
internal=False,
refuri='https://youtype.github.io/boto3_stubs_docs/'
f'mypy_boto3_sts/{boto3_type_uri_map[ref_target]}',
)
except KeyError:
return None


def setup(app: Sphinx) -> dict[str, bool | str]:
"""Register project-local Sphinx extension-API customizations.

:param app: Initialized Sphinx app instance.
:type app: Sphinx
:returns: Extension metadata.
:rtype: dict[str, bool | str]
"""
app.connect('missing-reference', _replace_missing_boto3_reference)

return {
'parallel_read_safe': True,
'parallel_write_safe': True,
'version': release,
}
5 changes: 4 additions & 1 deletion pyproject.toml
thedoubl3j marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,13 @@ dependencies = [ # runtime deps # https://packaging.python.org/en/latest/guide
"PyYAML", # credentials.injectors, inventory.plugins
"azure-identity", # credentials.azure_kv
"azure-keyvault", # credentials.azure_kv
"boto3", # credentials.awx_secretsmanager
"boto3", # credentials.aws_assume_role, credentials.awx_secretsmanager
"botocore", # credentials.aws_assume_role, credentials.awx_secretsmanager
"msrestazure", # credentials.azure_kv
"python-dsv-sdk >= 1.0.4", # credentials.thycotic_dsv
"python-tss-sdk >= 1.2.1", # credentials.thycotic_tss
"requests", # credentials.aim, credentials.centrify_vault, credentials.conjur, credentials.hashivault
"botocore.execeptions", # credentials.aws_assume_role
]
classifiers = [ # Allowlist: https://pypi.org/classifiers/
"Development Status :: 1 - Planning",
Expand Down Expand Up @@ -83,6 +85,7 @@ centrify_vault_kv = "awx_plugins.credentials.centrify_vault:centrify_plugin"
thycotic_dsv = "awx_plugins.credentials.dsv:dsv_plugin"
thycotic_tss = "awx_plugins.credentials.tss:tss_plugin"
aws_secretsmanager_credential = "awx_plugins.credentials.aws_secretsmanager:aws_secretmanager_plugin"
aws_assume_role = "awx_plugins.credentials.aws_assume_role:aws_assume_role_plugin"

[project.entry-points."awx_plugins.inventory"] # new entry points group name
azure-rm = "awx_plugins.inventory.plugins:azure_rm"
Expand Down
182 changes: 182 additions & 0 deletions src/awx_plugins/credentials/aws_assumerole.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,182 @@
"""This module provides integration with AWS AssumeRole functionality."""

import hashlib
import typing
from datetime import datetime

from awx_plugins.interfaces._temporary_private_django_api import ( # noqa: WPS436
gettext_noop as _,
)

import boto3
from botocore.exceptions import ClientError


if typing.TYPE_CHECKING:
from mypy_boto3_sts.client import STSClient
from mypy_boto3_sts.type_defs import (
AssumeRoleResponseTypeDef,
CredentialsTypeDef,
)

from .plugin import CredentialPlugin
thedoubl3j marked this conversation as resolved.
Show resolved Hide resolved


_aws_cred_cache: dict[
str,
'CredentialsTypeDef | dict[typing.Never, typing.Never]',
] | dict[typing.Never, typing.Never] = {}


assume_role_inputs = {
'fields': [
{
'id': 'access_key',
'label': _('AWS Access Key'),
'type': 'string',
'secret': True,
'help_text': _(
'The optional AWS access key for'
'the user who will assume the role.',
),
},
{
'id': 'secret_key',
'label': 'AWS Secret Key',
'type': 'string',
'secret': True,
'help_text': _(
'The optional AWS secret key for the'
'user who will assume the role.',
),
},
{
'id': 'external_id',
'label': 'External ID',
'type': 'string',
'help_text': _(
'The optional External ID which will'
'be provided to the assume role API.',
),
},
],
'metadata': [
{
'id': 'identifier',
'label': 'Identifier',
'type': 'string',
'help_text': _(
'The name of the key in the assumed AWS role'
'to fetch [AccessKeyId | SecretAccessKey | SessionToken].',
),
},
],
'required': [
'role_arn',
],
}


def aws_assumerole_getcreds(
access_key: str | None,
secret_key: str | None,
role_arn: str,
external_id: int,
) -> 'CredentialsTypeDef | dict[typing.Never, typing.Never]':
"""Return the credentials for use.

:param access_key: The AWS access key ID.
:type access_key: str
:param secret_key: The AWS secret access key.
:type secret_key: str
:param role_arn: The ARN received from AWS.
:type role_arn: str
:param external_id: The external ID received from AWS.
:type external_id: int
:returns: The credentials received from AWS.
:rtype: dict
:raises ValueError: If the client response is bad.
"""
connection: 'STSClient' = boto3.client(
service_name='sts',
# The following EE creds are read from the env if they are not passed:
aws_access_key_id=access_key, # defaults to `None` in the lib
aws_secret_access_key=secret_key, # defaults to `None` in the lib
)
try:
response: 'AssumeRoleResponseTypeDef' = connection.assume_role(
RoleArn=role_arn,
RoleSessionName='AAP_AWS_Role_Session1',
ExternalId=external_id,
)
except ClientError as client_err:
raise ValueError(
Copy link
Member

Choose a reason for hiding this comment

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

ValueError does not seem like a good fit for this kind of error. It's probably a RuntimeError or something like that.

Suggested change
raise ValueError(
raise RuntimeError(

Or perhaps a

Suggested change
raise ValueError(
raise LookupError(

It might also be a

Suggested change
raise ValueError(
raise ConnectionError(

ValueError would signal to the caller that they passed something wrong into the function call. But this message seems to imply it's an unexpected platform problem or something like that.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think any of these make sense and agree value isn't the correct thing in place. Only one I can see kinda not fitting is connection since errors could or could not be connection related. IMO lookup makes the most since we the actual function call is going and "lookin " something up but I am not dead set on that.

Copy link
Member

Choose a reason for hiding this comment

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

Yep. That's what I often use. And the calling code would need to handle it and turn into whatever awx interface currently wants, which seems to be a ValueError, looking into the function below.

f'Got a bad client response from AWS: {client_err.message}.',
) from client_err

return response.get('Credentials', {})


def aws_assumerole_backend(
access_key: str | None,
secret_key: str | None,
role_arn: str,
external_id: int,
identifier: str,
) -> dict:
"""Contact AWS to assume a given role for the user.

:param access_key: The AWS access key ID.
:type access_key: str
:param secret_key: The AWS secret access key.
:type secret_key: str
:param role_arn: The ARN received from AWS.
:type role_arn: str
:param external_id: The external ID received from AWS.
:type external_id: int
:param identifier: The identifier to fetch from the assumed role.
:type identifier: str
:raises ValueError: If the identifier is not found.
:returns: The identifier fetched from the assumed role.
:rtype: dict
"""
# Generate a unique SHA256 hash for combo of user access key and ARN
# This should allow two users requesting the same ARN role to have
# separate credentials, and should allow the same user to request
# multiple roles.
credential_key_hash = hashlib.sha256(
(str(access_key or '') + role_arn).encode('utf-8'),
)
credential_key = credential_key_hash.hexdigest()

credentials = _aws_cred_cache.get(credential_key, {})

# If there are no credentials for this user/ARN *or* the credentials
# we have in the cache have expired, then we need to contact AWS again.
creds_expired = (
(creds_expire_at := credentials.get('Expiration')) and
creds_expire_at < datetime.now(credentials['Expiration'].tzinfo)
)
if creds_expired:

credentials = aws_assumerole_getcreds(
access_key, secret_key, role_arn, external_id,
)

_aws_cred_cache[credential_key] = credentials

credentials = _aws_cred_cache.get(credential_key, {})

try:
return credentials[identifier]
except KeyError as key_err:
raise ValueError(
Copy link
Member

Choose a reason for hiding this comment

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

The same concern goes for this exception. Although I imagine that awx expects a ValueError so we can't change it w/o defining the interface first...

Copy link
Member Author

Choose a reason for hiding this comment

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

that is my thought as well, looking at the actual error message makes it seem like this should be dev focused since we are the ones going and getting this actual value with the user inputs but could be helpful for debugging if someone hits this. kinda leaves a good flag as where things could have fallen apart?

Copy link
Member

Choose a reason for hiding this comment

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

Technically, all the exceptions are targeting the devs, so are the tracebacks. But if awx uses this somehow, we can't change it to a LookupError, I think.

f'Could not find a value for {identifier}.',
) from key_err


aws_assumerole_plugin = CredentialPlugin(
'AWS Assume Role Plugin',
inputs=assume_role_inputs,
backend=aws_assumerole_backend,
)
Loading
Loading