feat(gax): support transparent retries during mTLS certificate rotations - #13901
feat(gax): support transparent retries during mTLS certificate rotations#13901macastelaz wants to merge 17 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces support for dynamic mTLS certificate rotation across both gRPC and HTTP/JSON transport channels by tracking certificate fingerprints, hot-swapping active channels, and triggering refreshes upon encountering UnauthenticatedException. The code review highlights several critical issues: a bug in ApiResultRetryAlgorithm that bypasses configured retries for non-retryable UnauthenticatedExceptions, a transient IllegalStateException during channel hot-swapping in RefreshingHttpJsonChannel, a backward compatibility break in CertificateBasedAccess when GOOGLE_API_USE_CLIENT_CERTIFICATE is explicitly set to "true", and a memory leak due to unpruned terminated entries in RefreshingHttpJsonChannel. Additionally, the reviewer recommends replacing synchronization on an AtomicReference with a lock-free read and an explicit ReentrantLock to avoid lock contention.
…ety bugs from multi-agent deep dive
Description
This PR introduces robust dynamic mTLS certificate rotation capabilities for
HTTP/JSON and gRPC transport channels, ensuring that certificates can be
rotated in long-lived environments without prematurely severing active, in-
flight RPCs or streams.
🚀 Core Features & Architectural Updates
• Dynamic Certificate Rotation: Implemented RefreshingHttpJsonChannel and
overhauled the gRPC ChannelPool to support dynamic, thread-safe, hot-swapping
of the underlying active transport channels whenever workload certificates
rotate dynamically on the filesystem.
• Preemptive Drop Mitigation: Refactored the internal channel rotation
pipeline (via refreshAll() and refreshSafely()) so that newly formed
connections are seamlessly brought online while preceding active streams are
cleanly drained and gracefully retired. This mitigates GFE connection drop
errors that previously occurred during hard resource refreshes.
• Core Retry Integration: Aligned streaming algorithm Callables and Retry
mechanisms with the dynamic refresh paradigm to ensure transparent retry
policies are respected, avoiding double-wrapped exceptions when traversing
rotated transports.
🔒 System Hardening & Bug Fixes
During the development of these features, several deep-dive reviews were
conducted over the GAX codebase, resulting in the following critical fixes:
• HTTP/JSON Teardown Thread-Safety: Fixed a race condition in
RefreshingHttpJsonChannel.java where shutdown() was calculating state
dynamically from underlying sub-channels without a lock. This allowed a
concurrent refresh() to spawn completely new channels after teardown began,
permanently leaking the channel pool.
• Outstanding RPC Memory Leak (ChannelPool.java): Fixed an uncontrolled
exception escape hatch in ReleasingClientCall.start(). If a pre-existing
cancellation exception was detected, the method aborted forcefully. This
bypassed onClose and never executed entry.release(), leaving the sub-channel
permanently trapped with an outstanding RPC count and preventing graceful
cleanup during rotations.
• Transport Channel Override Drops: Fixed merge() operations in
GrpcCallContext and HttpJsonCallContext that intentionally dropped custom
outer transportChannel references in favor of strict this.transportChannel
defaults. Context overrides now safely propagate custom overrides.
• Cross-Platform Compatibility: Fixed naively concatenated pathing for
certificates (Windows compatibility) and properly escaped JSON strings inside
CertificateBasedAccess.
mTLS Fail-Open Security Fix (CertificateBasedAccess.java):
JSON), the system previously swallowed the I/O exception, failed-open to
a null filepath, and allowed a standard non-mTLS auth connection without
notifying the developer. The system now correctly fails-closed (crashing
startup by throwing an IllegalStateException) upon parsing failure,
preventing unintentional security downgrade rollbacks.
Infinity Timeout Boundary Enforcement (GrpcCallContext & HttpJsonCallContext):
user limits into downstream libraries). However, a logical flaw permitted
bypassing this if a downstream caller submitted an unconstrained/infinite
timeout limit (represented as null), quietly erasing strict prior
deadlines. Override evaluations now properly reject null expansion
boundaries.
🧪 Testing
• Added and updated comprehensive unit-tests reflecting the thread-safety
fixes inside ChannelPoolTest.java and RefreshingHttpJsonChannelTest.java.
• Corrected edge case test configurations to leverage realistic mocked X.509
certificates to properly exercise deep WorkloadCertificateUtils.
getCertificateFingerprint() filesystem caching mechanisms.