feat(spanner): Support dynamic certificate and key rotation in Spanner Omni

[sagnghos]

Summary

This PR adds support for zero-downtime dynamic reloading of client certificates/keys (mTLS) and server root CA certificates in Spanner Omni without requiring application or connection pool restarts.

Changes

• DynamicKeyManager (com.google.cloud.spanner.omni): An X509ExtendedKeyManager that monitors file modification timestamps and lengths, dynamically reloading rotated client certificates and RSA/EC private keys.
• DynamicTrustManager (com.google.cloud.spanner.omni): An X509ExtendedTrustManager that dynamically reloads updated server root CA certificates into an in-memory keystore/trust manager upon file changes.
• SpannerOptions & Connection API:
  • Added Builder.setCaCertificate(String caCertificate) and getCaCertificate() across SpannerOptions, ConnectionProperties, ConnectionOptions, and SpannerPool.
  • Updated Builder.useClientCert(String, String) to use dynamic key management.
• SpannerOmniHelper: Added support for spanner.ca_cert_path and updated mTLS setup detection when client certificates are provided.
• Testing: Added unit tests covering dynamic certificate/key rotation, CA rotation, multi-CA bundles, fallback handling, and options configuration.

Fixes b/562755231

[gemini-code-assist]

Code Review

This pull request introduces dynamic loading and automatic reloading of client certificates, private keys, and server root CA certificates for Spanner Omni instances by implementing DynamicKeyManager and DynamicTrustManager. It also updates SpannerOptions, ConnectionOptions, and related classes to support the new caCertificate configuration. Feedback on these changes highlights critical performance concerns regarding blocking file I/O operations performed on every TLS handshake or trust check, which could block Netty's EventLoop threads; throttling these checks is recommended. Additionally, it is advised to remove the direct dependency on BouncyCastle in DynamicKeyManager to prevent classpath conflicts, relying instead on standard Java APIs for PKCS#8 private keys.

[sagnghos]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces support for configuring a server root CA certificate (caCertificate) for SSL/TLS authentication in Spanner Omni instances, alongside new DynamicKeyManager and DynamicTrustManager classes that dynamically reload certificates and keys from disk upon rotation. Feedback on these changes suggests enhancing private key parsing in DynamicKeyManager to explicitly reject PKCS#1 keys with a clear error message, and ensuring that temporary self-signed certificates created during testing in SpannerOptionsTest are properly cleaned up in a try-finally block to prevent file leaks.

[sagnghos]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces dynamic reloading of client certificates, private keys, and root CA certificates for Spanner Omni instances by implementing DynamicKeyManager and DynamicTrustManager. It also updates SpannerOptions, ConnectionOptions, and SpannerPool to support configuring CA certificates. The review feedback suggests resolving a potential memory leak in SpannerOptions by avoiding the capture of the Builder instance in a lambda, and recommends using System.nanoTime() instead of System.currentTimeMillis() in the dynamic managers to ensure reliable, monotonic interval checks.

[sagnghos]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces dynamic reloading of client certificates, private keys, and server root CA certificates from disk for Spanner Omni instances by implementing DynamicKeyManager and DynamicTrustManager. It also updates SpannerOptions, ConnectionOptions, and SpannerPool to support these configuration options and adds corresponding unit tests. The review feedback suggests improving the private key parsing in DynamicKeyManager to throw a unified exception when both RSA and EC parsing fail, and simplifying the exception handling in the reloadMaterial methods of both dynamic managers by propagating exceptions directly to their callers.

[sagnghos]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces dynamic reloading of client certificates, private keys, and server root CA certificates from disk for Spanner Omni instances. It adds DynamicKeyManager and DynamicTrustManager to automatically reload rotated credentials, and integrates these managers into SpannerOptions, ConnectionOptions, and SpannerPool. Feedback on these changes suggests validating that the private key matches the public key in the certificate chain during rotation to prevent loading mismatched pairs, and optimizing file reads by using the length of the read byte array instead of making separate File.length() system calls.

[sagnghos]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces DynamicKeyManager and DynamicTrustManager to enable dynamic reloading and rotation of client certificates, private keys, and server root CA certificates for Spanner Omni, alongside corresponding configuration updates in SpannerOptions, ConnectionOptions, and SpannerPool. The review feedback highlights critical issues that need to be addressed: a bug where the SSL context is lost during toBuilder() calls because it is not persisted in SpannerOptions, potential race conditions in both dynamic managers when reading files and during TLS handshakes, and a key corruption issue when decoding binary DER private keys using the MIME decoder.

[sagnghos]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces dynamic reloading of client certificates, private keys, and server root CA certificates for SSL/TLS authentication in Spanner Omni by adding DynamicKeyManager and DynamicTrustManager and integrating them across Spanner options and connection properties. Feedback on these changes suggests performing file checks and reloads asynchronously to avoid blocking Netty event loop threads during SSL handshakes, implementing a deterministic eviction policy for key materials instead of relying on unordered map keys, and lazily initializing CertificateFactory instances to reduce overhead from repeated security provider lookups.

[sagnghos]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces dynamic client certificate and root CA certificate reloading for Spanner Omni by adding DynamicKeyManager and DynamicTrustManager, along with corresponding configuration options and tests. The code review highlights a critical issue where the background thread schedulers in both managers will cause thread and memory leaks because their close() methods are never invoked by Netty. To resolve this, it is recommended to remove the background threads entirely and implement rate-limited, on-demand file checks during TLS handshakes. Additionally, the algorithm verification for EC private keys should be updated to support security providers that return 'ECDSA' instead of 'EC'.

[sagnghos]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces DynamicKeyManager and DynamicTrustManager to dynamically reload client certificates, private keys, and server root CA certificates from disk for Spanner Omni. It updates SpannerOptions, ConnectionOptions, ConnectionProperties, and SpannerPool to support and propagate these certificate configurations. Feedback on the changes suggests failing fast in SpannerPool with an IllegalArgumentException if only one of clientCertificate or clientCertificateKey is provided, rather than silently ignoring them.

[sagnghos]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces dynamic reloading of client certificates, private keys, and server root CA certificates from disk for Spanner Omni instances by adding DynamicKeyManager and DynamicTrustManager. It updates SpannerOptions, ConnectionOptions, and SpannerPool to support and propagate these certificate configurations, along with adding comprehensive unit tests. The review feedback recommends optimizing the checkAndReload() methods in both DynamicKeyManager and DynamicTrustManager by replacing lock.lock() with lock.tryLock(). This prevents concurrent threads, such as Netty event loop threads, from blocking sequentially on disk I/O during TLS handshakes.

[sagnghos]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces support for dynamically reloading client certificates, private keys, and server root CA certificates for SSL/TLS authentication in Spanner Omni. It adds DynamicKeyManager and DynamicTrustManager to automatically reload these credentials when modified on disk, and updates SpannerOptions, ConnectionOptions, and SpannerPool to support these new configurations. The feedback highlights a concurrency issue in both DynamicKeyManager and DynamicTrustManager where blocking file system checks are performed before acquiring a lock. To prevent thread blocking and latency spikes under high concurrency, the lock should be acquired before executing these I/O operations.

[sagnghos]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces dynamic reloading of client certificates, private keys, and root CA certificates from disk for Spanner Omni instances. It implements DynamicKeyManager and DynamicTrustManager to automatically reload rotated credentials, and updates SpannerOptions, ConnectionOptions, and SpannerPool to support these new capabilities along with a new caCertificate configuration option. The review feedback suggests a minor improvement in DynamicKeyManager to use Collection.removeIf for a cleaner and more idiomatic eviction of old certificate aliases.

[sagnghos]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces support for dynamic SSL/TLS certificate and key rotation in Spanner Omni. It adds DynamicKeyManager and DynamicTrustManager to handle automatic reloading of client certificates, private keys, and server root CA certificates from disk. It also updates SpannerOptions, ConnectionOptions, and SpannerPool to support CA certificate configuration. The review feedback suggests two optimizations in DynamicKeyManager: first, checking for binary DER private keys before performing expensive UTF-8 string decoding, and second, using Base64.getMimeDecoder() to parse PEM content more efficiently instead of manually stripping whitespace with regex.

[sagnghos]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces dynamic SSL/TLS credential reloading for Spanner Omni by adding DynamicKeyManager and DynamicTrustManager. These classes monitor and reload client certificates, private keys, and root CA certificates from disk when they are rotated. The configuration options have been integrated into SpannerOptions, ConnectionOptions, and SpannerPool, along with comprehensive unit tests. The review feedback highlights a potential issue in DynamicKeyManager where the key material eviction logic could inadvertently remove unrelated keys from the map due to unsafe string parsing, and suggests a more robust filtering implementation.

[sagnghos]

/gemini review

[gemini-code-assist]

Code Review

This pull request adds support for dynamically reloading client certificates, private keys, and root CA certificates from disk for Spanner Omni instances by introducing DynamicKeyManager and DynamicTrustManager. It also integrates these managers into SpannerOptions, connection options, and connection pooling. The review feedback recommends adding a null check in DynamicTrustManager.getAcceptedIssuers() to avoid potential NullPointerExceptions, and optimizing several tests by replacing Thread.sleep delays with manual file modification updates using setLastModified() to speed up test execution.

[sagnghos]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces dynamic SSL/TLS certificate and key reloading for Spanner Omni. It adds DynamicKeyManager and DynamicTrustManager to automatically reload client certificates, private keys, and root CA certificates from disk when they are rotated or modified. Additionally, it exposes new configuration options (clientCertificate, clientCertificateKey, caCertificate) across SpannerOptions, ConnectionOptions, and SpannerPool, supported by comprehensive unit tests. The feedback suggests explicitly using a standard keystore type like "JKS" instead of KeyStore.getDefaultType() in DynamicTrustManager to prevent potential initialization failures in environments with non-standard default keystore configurations.

[sagnghos]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces support for configuring a server root CA certificate (caCertificate) for Spanner Omni instances, enabling secure SSL/TLS authentication. It implements DynamicKeyManager and DynamicTrustManager to dynamically load and automatically reload client certificates, private keys, and root CA certificates from disk upon rotation or modification. Comprehensive unit tests are added to verify these behaviors. Feedback on the changes suggests using KeyStore.getDefaultType() instead of "JKS" in DynamicTrustManager for FIPS compliance, and cloning the accepted issuers array in getAcceptedIssuers() to prevent external modification of internal state.

[sagnghos]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces support for dynamic SSL/TLS certificate and key rotation in Spanner Omni. It adds DynamicKeyManager and DynamicTrustManager to automatically reload client certificates, private keys, and server root CA certificates from disk when modified. Additionally, it exposes configuration options for these certificates through SpannerOptions, ConnectionOptions, and connection properties, along with comprehensive unit tests validating the dynamic reloading, throttling, and fallback behaviors. I have no feedback to provide as there are no review comments.