-
Notifications
You must be signed in to change notification settings - Fork 60
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
[Integration][AWS] - Fix ExpiredTokenException #1041
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good job on using the sessionDuration in the assume role client. I left a comment on how the aiocache works for the ttl
|
||
async def update_available_access_credentials() -> None: | ||
@cached(ttl=CACHE_DURATION_SECONDS, cache=Cache.MEMORY) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would like to understand what happens when the cache is cleared after the duration? And how is the simple Memory different from using FastAPIs event.attribute functionality?
i guess my question is, since you changed the approach from event based to time based, what happens to the assumed role credentials after the ttl?
Also after the cache is cleared, how does it manage the re-entry. Thus, after the first ttl duration has elapsed, we signal that the credentials needs to be refreshed. Now what happens after the second hour. how does the ttl behave?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cache is meant to expire after exhausting 80% of the session duration;
when the cache is cleared, the next call to update_available_access_credentials
resets the session, producing new credentials thereby extending the expiry time once again...
This can happen at any point within a resync.
TTL, last for the specified time only, update_available_access_credentials
is being called as many times as possible to ensure we know whenever the TTL is close to expiry.
@@ -93,7 +96,9 @@ async def _get_organization_session(self) -> aioboto3.Session | None: | |||
async with application_session.client("sts") as sts_client: | |||
try: | |||
organizations_client = await sts_client.assume_role( | |||
RoleArn=organization_role_arn, RoleSessionName="AssumeRoleSession" | |||
RoleArn=organization_role_arn, | |||
RoleSessionName="AssumeRoleSession", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RoleSessionName="AssumeRoleSession", | |
RoleSessionName="OceanOrgAssumeRoleSession", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added some comments. Please make sure to add tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added some more comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks nice
Description
What
ExpiredTokenException
by replacing the event-based caching system with a time-dependent caching mechanism. The new approach ensures that the role is reassumed and session credentials are refreshed when 80% of the session duration has been used.Why
ExpiredTokenException
, causing session credentials to expire unexpectedly.How
Type of change
Please leave one option from the following and delete the rest:
All tests should be run against the port production environment(using a testing org).
Core testing checklist
Integration testing checklist
examples
folder in the integration directory.Preflight checklist
Screenshots
Include screenshots from your environment showing how the resources of the integration will look.
API Documentation
Provide links to the API documentation used for this integration.