fix(ci): import profiler ci setuptools - #17713
Closed
hebaalazzeh wants to merge 48 commits into
Closed
Conversation
…rs.py Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
…al client_helpers methods
Splits the monolithic client_helpers.py into routing.py, client_cert.py, config_helpers.py, and method_helpers.py to improve readability and maintainability. Updates gapic_v1 exports and test imports accordingly.
…and enable REST tests without grpc
…re_deps_from_source in noxfile
Co-authored-by: Anthonios Partheniou <partheniou@google.com>
…ers.py Co-authored-by: Anthonios Partheniou <partheniou@google.com>
Co-authored-by: Anthonios Partheniou <partheniou@google.com>
Co-authored-by: Anthonios Partheniou <partheniou@google.com>
Co-authored-by: Anthonios Partheniou <partheniou@google.com>
Co-authored-by: Anthonios Partheniou <partheniou@google.com>
…n-grpc environments
hebaalazzeh
changed the title
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces several helper modules under google/api_core/gapic_v1 to handle client certificates, environment variable parsing, request ID generation, and routing/endpoint resolution for mTLS and universe domains, along with comprehensive unit tests. The feedback suggests raising a MutualTLSChannelError in _get_client_cert_source if mTLS is enabled but no client certificate source is provided or found, rather than silently falling back to standard TLS.
Comment on lines
+70
to
+79
| client_cert_source = None | ||
| if use_cert_flag: | ||
| if provided_cert_source: | ||
| client_cert_source = provided_cert_source | ||
| elif ( | ||
| hasattr(mtls, "has_default_client_cert_source") | ||
| and mtls.has_default_client_cert_source() | ||
| ): | ||
| client_cert_source = mtls.default_client_cert_source() | ||
| return client_cert_source |
Contributor
There was a problem hiding this comment.
According to the general rules, we should not silently fall back to standard TLS if mutual TLS (mTLS) configuration or certificate loading fails. If use_cert_flag is True but no client certificate source can be found (neither provided nor default), we should raise a MutualTLSChannelError to fail fast and avoid security risks or confusing downstream authorization errors.
Suggested change
| client_cert_source = None | |
| if use_cert_flag: | |
| if provided_cert_source: | |
| client_cert_source = provided_cert_source | |
| elif ( | |
| hasattr(mtls, "has_default_client_cert_source") | |
| and mtls.has_default_client_cert_source() | |
| ): | |
| client_cert_source = mtls.default_client_cert_source() | |
| return client_cert_source | |
| client_cert_source = None | |
| if use_cert_flag: | |
| if provided_cert_source: | |
| client_cert_source = provided_cert_source | |
| elif ( | |
| hasattr(mtls, "has_default_client_cert_source") | |
| and mtls.has_default_client_cert_source() | |
| ): | |
| client_cert_source = mtls.default_client_cert_source() | |
| else: | |
| from google.auth.exceptions import MutualTLSChannelError | |
| raise MutualTLSChannelError( | |
| "Client certificate source is required when mTLS is enabled, " | |
| "but no client certificate source was provided or found." | |
| ) | |
| return client_cert_source |
References
- Do not silently fall back to standard TLS if mutual TLS (mTLS) configuration or certificate loading fails. Silently falling back can introduce security vulnerabilities or cause confusing downstream authorization errors; instead, fail fast by raising an appropriate exception.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Error
The import-profile CI workflow fails on Python 3.15 with:
This PR installs setuptools inside the temporary .venv-profiler virtualenv created for profiling. This ensures the profiler's namespace package resolution logic works correctly on newer Python versions (3.12+) where setuptools is no longer installed by default.