Skip to content

feat: Add mTLS Interceptor and Channel Wrapper for certificate rotation handling - #18240

Draft
agrawalradhika-cell wants to merge 2 commits into
mainfrom
add-mtls-interceptor
Draft

feat: Add mTLS Interceptor and Channel Wrapper for certificate rotation handling#18240
agrawalradhika-cell wants to merge 2 commits into
mainfrom
add-mtls-interceptor

Conversation

@agrawalradhika-cell

Copy link
Copy Markdown
Contributor

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:

  • Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)

Fixes #<issue_number_goes_here> 🦕

Implement mTLS Interceptor and Channel Wrapper for automatic certificate rotation and retry logic.
@agrawalradhika-cell
agrawalradhika-cell requested review from a team as code owners August 27, 2026 19:14

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

Comment thread packages/google-auth/google/auth/transport/mtls_interceptor.py Outdated
Comment on lines +124 to +131
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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

critical

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
  1. 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):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

critical

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
  1. 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.

Comment on lines +3 to +8
import collections
import threading
import time

import grpc
from google.auth import transport

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

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
  1. 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant