-
Notifications
You must be signed in to change notification settings - Fork 1.8k
feat(auth): [aiohttp] Add mTLS reconfiguration logic when certificate mismatch for existing credentials & Agent Identity workloads #18224
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
Open
agrawalradhika-cell
wants to merge
61
commits into
main
Choose a base branch
from
cert-rotation-aiohttp
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
61 commits
Select commit
Hold shift + click to select a range
94a1d95
feat: Add retry for cert rotation handling
agrawalradhika-cell 420447c
chore: Add tests for MTLS certificate rotation behavior
agrawalradhika-cell 907cf00
Update packages/google-auth/tests/transport/aio/test_sessions_mtls.py
agrawalradhika-cell cc850b1
Improve error handling for mTLS reconfiguration
agrawalradhika-cell a44acb0
fix: Rename test_cert_rotation_failure to test_cert_rotation_failure_…
agrawalradhika-cell 984e47c
chore: Refactor MTLS parameter check on unauthorized response o use a…
agrawalradhika-cell 30341bc
chore: Reset mTLS init task upon client certificate change
agrawalradhika-cell 1c068dc
fix: fix the lint errors
agrawalradhika-cell 6fb1e86
chore: Refactor mTLS channel reconfiguration logic for adding mTLS ch…
agrawalradhika-cell 2cdfe2d
chore: Add mTLS rotation lock for certificate management
agrawalradhika-cell d734731
chore: Log mTLS channel reconfiguration failure as error
agrawalradhika-cell 97e91d0
chore: Refactor mTLS handling for unauthorized responses
agrawalradhika-cell d0da58b
fix: Remove unnecessary continue statement after mTLS configuration.
agrawalradhika-cell 825426d
fix: Fix cert rotation tests and improve error handling
agrawalradhika-cell 63e587c
fix: fix unit tests for the checks
agrawalradhika-cell 71b3bf5
fix: Fix unit tests for the change
agrawalradhika-cell 7d92d30
test: remove fragile async caplog assertions
agrawalradhika-cell 8b2efcf
fix: Add error handling for credential refresh failures
agrawalradhika-cell a4d0405
fix: Fix lint errors
agrawalradhika-cell 2d52a21
chore: Refactor mTLS endpoint handling in sessions.py
agrawalradhika-cell 2806f4f
chore: Reorder response closing logic for clarity
agrawalradhika-cell d5426f2
chore: Handle additional exception during credential refresh
agrawalradhika-cell 968a9fd
fix: Modify mTLS rotation lock initialization
agrawalradhika-cell 9d1a690
fix: Handle response closure in mTLS error handling
agrawalradhika-cell 85d4a76
Fix: Fix improperly falling through to the credential refresh logic.
agrawalradhika-cell d1c6512
chore: Track and close old auth requests in sessions.py
agrawalradhika-cell b28caea
fix: Adjust max_allowed_time based on elapsed time
agrawalradhika-cell f176eed
chore: Add client_cert_callback to transport session
agrawalradhika-cell a436abe
chore: Enhance check_parameters_for_unauthorized_response with callback
agrawalradhika-cell fbee990
fix: Add test for certificate rotation lock contention
agrawalradhika-cell b310e27
fix: Enhance MTLS session tests with various scenarios
agrawalradhika-cell 55f1ad4
Fix: Fix lint and unit tetsts
agrawalradhika-cell 554a571
fix: fix unit tests for tests_sessions
agrawalradhika-cell eb28f81
chore: Refactor mTLS channel configuration callback
agrawalradhika-cell 221810e
fix: Import urllib.parse instead of urllib
agrawalradhika-cell 445c576
fix: Format mTLS channel configuration for readability
agrawalradhika-cell 6b0edd3
fix: Fix test name for mTLS certificate matching
agrawalradhika-cell b6e30b0
fix: Refactor type annotations and error handling
agrawalradhika-cell 7c32ec9
chore: Change exception type in test for MTLS session
agrawalradhika-cell f98c347
Fix duplicate isinstance check for data type
agrawalradhika-cell c97bfdf
fix: Format error message for MutualTLSChannelError
agrawalradhika-cell 2790119
chore: Refactor mTLS handling and improve timeout logic
agrawalradhika-cell 498bcd6
chore: Implement mTLS check counter for configuration management
agrawalradhika-cell 242359a
fix: Implement test for cert rotation lock contention
agrawalradhika-cell 8c867af
fix: Refactor mTLS configuration and error handling
agrawalradhika-cell 8942a12
fix: Fix indentation for asyncio test decorator
agrawalradhika-cell eb95f3c
fix: Enhance tests for MTLS session certificate rotation
agrawalradhika-cell 50ba485
fix: Remove stale_cert assignment in sessions.py
agrawalradhika-cell 0e8950f
fix: Update sessions.py for lint
agrawalradhika-cell bc8bebb
fix: fix lint errors in sessions.py
agrawalradhika-cell 2b57c11
Rename MTLS_URL_PREFIXES to _MTLS_URL_PREFIXES and typecasting fix
agrawalradhika-cell a149eb7
fix: Refactor request headers handling in sessions.py
agrawalradhika-cell e66a723
fix: Fix client certificate callback handling in mTLS
agrawalradhika-cell 314d974
feat: Implement mTLS parameter check and fingerprinting
agrawalradhika-cell cf308b8
fix: Refactor MTLS parameter checking in sessions.py
agrawalradhika-cell 7794506
fix: Update mock patch for MTLS check parameters
agrawalradhika-cell 931bdfd
fix: Refactor mTLS channel reconfiguration logic for lint
agrawalradhika-cell 9bd8e8f
fix: Fix lint errors
agrawalradhika-cell e8ee7b6
fix: fix the unit tests based on the code changes
agrawalradhika-cell 7379d8a
fix: Log when credentials do not implement refresh method
agrawalradhika-cell 48cf5d2
chore: Refactor authentication retry logic in sessions.py
agrawalradhika-cell File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
Oops, something went wrong.
Oops, something went wrong.
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.
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.
Every rotation appends the old
AiohttpRequesttoself._old_auth_requests. These requests stay open untilsession.close(). For long running services with periodic certificate rotation, this list grows without bounds, leaking connection pools and open sockets. Prune or close old sessions in the background after in flight requests finish.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.
Thanks for the feedback!
It was an intentional decision to tie the cleanup of old sessions to AsyncAuthorizedSession.close() for a few practical reasons:
Given the infrequency of rotations and the fact that idle sockets naturally time out and close on their own, accepting a slow-growing list of empty session objects seemed like the safest tradeoff compared to the complexity and risk of implementing background garbage collection.
Let me know if you still have concerns here
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 think we can solve this cleanly without background tasks. Since rotations occur hours apart and
_do_configure()is already an async coroutine, can we consider boundingself._old_auth_requeststo two generations and awaitingclose()in-line on the oldest entry when appending a new one?This gives in-flight requests on the immediately displaced session ample time to complete, and bounds retention.