Add fixture for AWS client configured with secondary test credentials #8520
Conversation
This file contains 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
viren-nadkarni
force-pushed
the
secondary-client-fixture
branch
from
981841a
to
2bc484b
Compare
viren-nadkarni
added
the
semver: patch
viren-nadkarni
force-pushed
the
secondary-client-fixture
branch
2 times, most recently
from
4d02d38
to
7083c45
Compare
viren-nadkarni
force-pushed
the
secondary-client-fixture
branch
from
7083c45
to
ac0ddee
Compare
viren-nadkarni
commented
Jun 20, 2023
| return self._get_client( | ||
| service_name=service_name, | ||
| region_name=region_name or config.region_name or self._get_region(), | ||
| use_ssl=self._use_ssl, | ||
| verify=self._verify, | ||
| endpoint_url=endpoint_url, | ||
| aws_access_key_id=aws_access_key_id or TEST_AWS_ACCESS_KEY_ID, |
There was a problem hiding this comment.
We don't want to use the test credentials in non-test scenarios. This is part of the effort to make accounts explicit and avoid silent fallbacks, making cross-account bugs tiresome to debug.
viren-nadkarni
commented
Jun 20, 2023
| @pytest.fixture(scope="class") | ||
| def user_arn(self, aws_client): | ||
| return aws_client.sts.get_caller_identity()["Arn"] | ||
|
|
There was a problem hiding this comment.
Fixture is moved out of class scope to global scope so that it can be used in all test classes.
viren-nadkarni
force-pushed
the
secondary-client-fixture
branch
from
bdbc30d
to
1c582e5
Compare
viren-nadkarni
requested review from
thrau,
baermat,
bentsku,
macnev2013,
joe4dev and
dominikschubert
as code owners
viren-nadkarni
removed request for
dominikschubert,
thrau,
joe4dev,
macnev2013,
bentsku and
baermat
viren-nadkarni
force-pushed
the
secondary-client-fixture
branch
from
d2c4edb
to
1219349
Compare
dfangl
approved these changes
Jul 13, 2023
localstack/testing/aws/util.py
Outdated
| @@ -192,10 +211,19 @@ def base_aws_client_factory(session: boto3.Session) -> ClientFactory: | |||
| if not config: | |||
| config = botocore.config.Config() | |||
|
|
|||
| # Prevent this fixture from using the region configured in system config | |||
| # Prevent this fixture from using the credentials configured in system config | |||
There was a problem hiding this comment.
Not sure about this change, nothing changed in the logic here?
| """ | ||
| Returns an AWS client configured with secondary test credentials. | ||
| """ | ||
| return secondary_testing_aws_client(aws_client_factory) |
There was a problem hiding this comment.
I think we should at some point separate the client factories as well, and set the credentials differently in different sessions. Can wait for a second iteration, though.
There was a problem hiding this comment.
Noted in my techdebt backlog 👍
viren-nadkarni
commented
Jul 14, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR introduces
secondary_aws_clientfixture which works exactly like theaws_clientfixture except it is configured with secondary test credentials.It is meant to be used in cross-accounts test scenarios.
This PR also migrates existing cross-accounts tests to use this new fixture.