This repository was archived by the owner on Mar 26, 2026. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 80
feat: auto-enable mTLS when supported certificates are detected #2472
Merged
daniel-sanche
merged 37 commits into
googleapis:main
from
agrawalradhika-cell:set-mtls-flag-true
Nov 12, 2025
Merged
Changes from all commits
Commits
Show all changes
37 commits
Select commit
Hold shift + click to select a range
f7b051b
feat: Use the mTLS helper which enables mTLS if GOOGLE_API_USE_CLIENT…
agrawalradhika-cell b74faea
feat: Add tests accordingly for the python client library
agrawalradhika-cell eb53969
chore: Update the client side usage of mtls_helper.check_use_client_c…
agrawalradhika-cell cdfc98c
chore: minor indentation fixes
agrawalradhika-cell a89ad6c
fix: minor indentation
agrawalradhika-cell e5a7653
fix: Minor typos and update tests to be full coverage
agrawalradhika-cell 9ad7951
update goldens
daniel-sanche 7f3b1bb
chore: Minor typos, indentation and refactor testcase
agrawalradhika-cell c630687
chore: Update mock name for test
agrawalradhika-cell b7cbb55
fix: fix the ads-templates by removing old logic
agrawalradhika-cell 8c01845
fix: Update the gapic to use public wrapper from google-auth for mTLS…
agrawalradhika-cell 38ca78b
chore: Update the import check for should_use_client_cert
agrawalradhika-cell 21b76fe
chore: minor lint not detected by nox
agrawalradhika-cell d17f2d4
fix: minor assignment fix
agrawalradhika-cell 51fefe4
fix: Correction in code and update test
agrawalradhika-cell 7f4cb05
chore: updated goldens
daniel-sanche eb5ee27
chore: Update docstrings
agrawalradhika-cell fd60e7f
fix: fix minor lint issue that is not detected by nox, and caught by …
agrawalradhika-cell 0d33d2f
chore: Update tests to increase test coverage
agrawalradhika-cell d637226
fix: fix the ads-template file
agrawalradhika-cell 04f7869
fix: minor lint fix
agrawalradhika-cell ba88591
fix: fix tests
agrawalradhika-cell f0c6b1f
updated goldens
daniel-sanche 8a94a60
fix: fix typo in testcase
agrawalradhika-cell e30868e
updated goldens
daniel-sanche 70f271b
chore: Add testcases to increase test coverage
agrawalradhika-cell ffc6b5e
chore: Add more testcases to increase coverage to 100%
agrawalradhika-cell 29cb163
fix: Add pragma: NOCOVER as test coverage missing 100%, but locally a…
agrawalradhika-cell 763b50e
chore: updated goldens
daniel-sanche 9006e57
fix: Add pragma: NO cover to help with test coverage
agrawalradhika-cell d91d08d
fix: Add pragma: NO COVER to test coverage missing lines
agrawalradhika-cell 39ba8c0
fix: remove pragma: no Cover as coverage unaffected
agrawalradhika-cell 6413a07
added no cover line to ads_templates
daniel-sanche 1b6d208
Merge branch 'main' into set-mtls-flag-true
daniel-sanche a1a1b73
added no cover statement
daniel-sanche ad1d486
Merge branch 'main' into set-mtls-flag-true
daniel-sanche 8b4a5cc
updated goldens
daniel-sanche File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -183,6 +183,32 @@ class {{ service.client_name }}(metaclass={{ service.client_name }}Meta): | |
|
|
||
| _DEFAULT_UNIVERSE = "googleapis.com" | ||
|
|
||
| @staticmethod | ||
| def _use_client_cert_effective(): | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should file a bug to follow up on moving this to a shared file. Each service will have this function repeated which can cause bloat in the generated code when there are many services.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure, opened at https://github.com/googleapis/gapic-generator-python/issues/2485 |
||
| """Returns whether client certificate should be used for mTLS if the | ||
| google-auth version supports should_use_client_cert automatic mTLS enablement. | ||
|
|
||
| Alternatively, read from the GOOGLE_API_USE_CLIENT_CERTIFICATE env var. | ||
|
|
||
| Returns: | ||
| bool: whether client certificate should be used for mTLS | ||
| Raises: | ||
| ValueError: (If using a version of google-auth without should_use_client_cert and | ||
| GOOGLE_API_USE_CLIENT_CERTIFICATE is set to an unexpected value.) | ||
agrawalradhika-cell marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| """ | ||
| # check if google-auth version supports should_use_client_cert for automatic mTLS enablement | ||
| if hasattr(mtls, "should_use_client_cert"): # pragma: NO COVER | ||
| return mtls.should_use_client_cert() | ||
| else: # pragma: NO COVER | ||
| # if unsupported, fallback to reading from env var | ||
| use_client_cert_str = os.getenv("GOOGLE_API_USE_CLIENT_CERTIFICATE", "false").lower() | ||
| if use_client_cert_str not in ("true", "false"): | ||
| raise ValueError( | ||
| "Environment variable `GOOGLE_API_USE_CLIENT_CERTIFICATE` must be" | ||
| " either `true` or `false`" | ||
| ) | ||
| return use_client_cert_str == "true" | ||
|
|
||
| @classmethod | ||
| def from_service_account_info(cls, info: dict, *args, **kwargs): | ||
| """Creates an instance of this client using the provided credentials | ||
|
|
@@ -297,16 +323,14 @@ class {{ service.client_name }}(metaclass={{ service.client_name }}Meta): | |
| DeprecationWarning) | ||
| if client_options is None: | ||
| client_options = client_options_lib.ClientOptions() | ||
| use_client_cert = os.getenv("GOOGLE_API_USE_CLIENT_CERTIFICATE", "false") | ||
| use_client_cert = {{ service.client_name }}._use_client_cert_effective() | ||
| use_mtls_endpoint = os.getenv("GOOGLE_API_USE_MTLS_ENDPOINT", "auto") | ||
| if use_client_cert not in ("true", "false"): | ||
| raise ValueError("Environment variable `GOOGLE_API_USE_CLIENT_CERTIFICATE` must be either `true` or `false`") | ||
| if use_mtls_endpoint not in ("auto", "never", "always"): | ||
| raise MutualTLSChannelError("Environment variable `GOOGLE_API_USE_MTLS_ENDPOINT` must be `never`, `auto` or `always`") | ||
|
|
||
| # Figure out the client cert source to use. | ||
| client_cert_source = None | ||
| if use_client_cert == "true": | ||
| if use_client_cert: | ||
| if client_options.client_cert_source: | ||
| client_cert_source = client_options.client_cert_source | ||
| elif mtls.has_default_client_cert_source(): | ||
|
|
@@ -336,14 +360,12 @@ class {{ service.client_name }}(metaclass={{ service.client_name }}Meta): | |
| google.auth.exceptions.MutualTLSChannelError: If GOOGLE_API_USE_MTLS_ENDPOINT | ||
| is not any of ["auto", "never", "always"]. | ||
| """ | ||
| use_client_cert = os.getenv("GOOGLE_API_USE_CLIENT_CERTIFICATE", "false").lower() | ||
| use_client_cert = {{ service.client_name }}._use_client_cert_effective() | ||
| use_mtls_endpoint = os.getenv("GOOGLE_API_USE_MTLS_ENDPOINT", "auto").lower() | ||
| universe_domain_env = os.getenv("GOOGLE_CLOUD_UNIVERSE_DOMAIN") | ||
| if use_client_cert not in ("true", "false"): | ||
| raise ValueError("Environment variable `GOOGLE_API_USE_CLIENT_CERTIFICATE` must be either `true` or `false`") | ||
| if use_mtls_endpoint not in ("auto", "never", "always"): | ||
| raise MutualTLSChannelError("Environment variable `GOOGLE_API_USE_MTLS_ENDPOINT` must be `never`, `auto` or `always`") | ||
| return use_client_cert == "true", use_mtls_endpoint, universe_domain_env | ||
| return use_client_cert, use_mtls_endpoint, universe_domain_env | ||
|
|
||
| @staticmethod | ||
| def _get_client_cert_source(provided_cert_source, use_cert_flag): | ||
|
|
||
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test (and others) branch on
hasattr(google.auth.transport.mtls, "should_use_client_cert"). I think it's important both sides of the branch are covered.IIUC, I think it will be, because of the testing/constraints files. We run tests against both the most recent supported version of dependencies, and the oldest version, so that will cover both branches.
I'd probably prefer to mention this detail in a comment above this kind of test, something like:
But I'm not sure if this is worth re-generating all the files to add. Just wanted to note it here at least
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is for ads-templates, don't think they are related to goldens
As plan is to move it off, but I'll add all the testcases here too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Checked with Daniel, this was for adding comments not testcases, can skip