feat: Add mTLS Interceptor and Channel Wrapper for certificate rotation handling - #18240
feat: Add mTLS Interceptor and Channel Wrapper for certificate rotation handling#18240agrawalradhika-cell wants to merge 2 commits into
Conversation
Implement mTLS Interceptor and Channel Wrapper for automatic certificate rotation and retry logic.
There was a problem hiding this comment.
Code Review
This pull request introduces a new mTLS interceptor and channel wrapper (mtls_interceptor.py) to handle automatic retry logic and certificate rotation for gRPC connections. The review feedback highlights several critical runtime and import-time issues that must be addressed, including missing imports for _LOGGER and _mtls_helper, an undefined parameter create_channel_fn in MTLSRefreshingChannel.__init__, an immediate overwrite of self._channel using an undefined secure_authorized_channel function, and a NameError caused by referencing _BaseCallWrapper as a base class before its definition.
| self._channel = self._create_channel_fn( | ||
| ssl_credentials=new_ssl_credentials, | ||
| client_cert_callback=None | ||
| ) | ||
|
|
||
|
|
||
| self._channel = secure_authorized_channel(**factory_args) | ||
| self._cached_cert = call_cert_bytes |
There was a problem hiding this comment.
The newly created channel assigned to self._channel on line 124 is immediately overwritten on line 130 by a call to secure_authorized_channel(**factory_args). Furthermore, secure_authorized_channel is not defined or imported in this module. Please correct this logic to ensure the channel is properly initialized and not overwritten.
References
- Before flagging a potential NameError or suggesting a fully-qualified import path in a code review, verify all module-level imports to ensure the module or variable is not already imported.
| return self._details | ||
|
|
||
|
|
||
| class _RetryableUnaryResponseFuture(_BaseCallWrapper): |
There was a problem hiding this comment.
The class _BaseCallWrapper is used as a base class for _RetryableUnaryResponseFuture before it is defined (it is defined on line 662). In Python, base classes must be defined before they are referenced in a class definition, otherwise a NameError is raised at import time. Please move the definition of _BaseCallWrapper above _RetryableUnaryResponseFuture.
References
- Before flagging a potential NameError or suggesting a fully-qualified import path in a code review, verify all module-level imports to ensure the module or variable is not already imported.
| import collections | ||
| import threading | ||
| import time | ||
|
|
||
| import grpc | ||
| from google.auth import transport |
There was a problem hiding this comment.
The module uses _LOGGER and _mtls_helper but does not import or define them, which will cause NameError exceptions at runtime. Please import logging and _mtls_helper, and initialize _LOGGER.
import collections
import logging
import threading
import time
import grpc
from google.auth import transport
from google.auth.transport import _mtls_helper
_LOGGER = logging.getLogger(__name__)References
- Before flagging a potential NameError or suggesting a fully-qualified import path in a code review, verify all module-level imports to ensure the module or variable is not already imported.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Implement mTLS Interceptor and Channel Wrapper for automatic certificate rotation and retry logic.
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Fixes #<issue_number_goes_here> 🦕