fix(auth): align mTLS discovery and enforce fail-fast transport configuration.#17470
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces auto-enablement of mTLS when workload certificates are discovered (including at a well-known SPIFFE path) and changes the library's behavior to fail fast with a MutualTLSChannelError instead of silently falling back to standard TLS when mTLS is expected but certificates cannot be loaded. The review feedback suggests several improvements: aligning the environment variable checks in _agent_identity_utils.py to treat any value other than 'true' as disabled, correcting a misleading exception message in sessions.py when a custom request handler is used, and making the SPIFFE certificate path check more robust by using path.isfile and handling potential OSError exceptions.
|
Summary of Changes Aligned with PR 17387:
New fixes addressing PR 17470 comments:
|
nbayati
left a comment
There was a problem hiding this comment.
Just left a comment on the docstring update, otherwise LGTM!
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors mTLS configuration and client certificate checks across multiple transports (aio, requests, urllib3) to respect explicit opt-outs and handle custom transports gracefully by setting _is_mtls to False and logging warnings. It also improves error handling for missing workload certificate paths and introduces corresponding unit tests. The reviewer feedback recommends correcting a docstring mismatch in a new test that incorrectly states an exception is raised, and improving test isolation by clearing all relevant environment variables in the auto-enablement test.
This PR resolves two mTLS issues:
(1) it synchronizes the transport and token discovery paths to check for workload certs and library support, preventing mismatched bound tokens, and
(2) it enforces fail-fast error behavior in all transports if mTLS configuration fails, avoiding silent fallback to insecure TLS. These align token and transport states securely.