feat: Add retry logic when certificate mismatch for existing credentials & Agent Identity workloads #1841

agrawalradhika-cell · 2025-10-16T23:12:08Z

feat: Add retry logic when certificate mismatch for existing credentials & Agent Identity workloads

This change introduces retry support when requests are created for existing credentials and Agent Identities on GKE and Cloud Run Workloads. When 401(Unauthorized) error is created, due to certificate at time of configuration of mTLS channel being different from the current certificate, a retry is added to the request by configuring the mTLS channel with the current certificate.

This change introduces retry support when requests are created for AgentIdentities on GKE and Cloud Run Workloads. Signed-off-by: Radhika Agrawal <agrawalradhika@google.com>

…ion and request Signed-off-by: Radhika Agrawal <agrawalradhika@google.com>

… from mTLS configuration Signed-off-by: Radhika Agrawal <agrawalradhika@google.com>

lsirac · 2025-10-18T14:06:39Z

Is the description accurate? This will apply to existing X509 workloads too?

agrawalradhika-cell · 2025-10-20T22:30:40Z

Is the description accurate? This will apply to existing X509 workloads too?

Updated the description

google/auth/transport/requests.py

google/auth/transport/urllib3.py

google/auth/transport/requests.py

Signed-off-by: Radhika Agrawal <agrawalradhika@google.com>

nbayati · 2025-10-24T00:35:29Z

google/auth/_agent_identity_utils.py

@@ -0,0 +1,188 @@
+# Copyright 2025 Google LLC


Leaving a comment here so we won't forget to wait for the PR #1821 to merge first and do a rebase to ensure we don't have duplicated or conflicting code.

google/auth/transport/urllib3.py

google/auth/transport/requests.py

daniel-sanche

Let a few small comments, but nothing major. I avoided _agent_identity_utils entirely because of conflicts with #1821

I'll take another look when #1821 is merged

google/auth/transport/requests.py

google/auth/transport/urllib3.py

daniel-sanche · 2025-10-30T18:47:31Z

tests/transport/test_requests.py

+        # New certificate and key to simulate rotation.
+        new_cert = b'-----BEGIN CERTIFICATE-----\nMIIDIzCCAgugAwIBAgIJAMfISuBQ5m+5MA0GCSqGSIb3DQEBBQUAMBUxEzARBgNV\nBAMTCnVuaXQtdGVzdHMwHhcNMTExMjA2MTYyNjAyWhcNMjExMjAzMTYyNjAyWjAV\nMRMwEQYDVQQDEwp1bml0LXRlc3RzMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIB\nCgKCAQEA4ej0p7bQ7L/r4rVGUz9RN4VQWoej1Bg1mYWIDYslvKrk1gpj7wZgkdmM\n7oVK2OfgrSj/FCTkInKPqaCR0gD7K80q+mLBrN3PUkDrJQZpvRZIff3/xmVU1Wer\nuQLFJjnFb2dqu0s/FY/2kWiJtBCakXvXEOb7zfbINuayL+MSsCGSdVYsSliS5qQp\ngyDap+8b5fpXZVJkq92hrcNtbkg7hCYUJczt8n9hcCTJCfUpApvaFQ18pe+zpyl4\n+WzkP66I28hniMQyUlA1hBiskT7qiouq0m8IOodhv2fagSZKjOTTU2xkSBc//fy3\nZpsL7WqgsZS7Q+0VRK8gKfqkxg5OYQIDAQABo3YwdDAdBgNVHQ4EFgQU2RQ8yO+O\ngN8oVW2SW7RLrfYd9jEwRQYDVR0jBD4wPIAU2RQ8yO+OgN8oVW2SW7RLrfYd9jGh\nGaQXMBUxEzARBgNVBAMTCnVuaXQtdGVzdHOCCQDHyErgUOZvuTAMBgNVHRMEBTAD\nAQH/MA0GCSqGSIb3DQEBBQUAA4IBAQBRv+M/6+FiVu7KXNjFI5pSN17OcW5QUtPr\nodJMlWrJBtynn/TA1oJlYu3yV5clc/71Vr/AxuX5xGP+IXL32YDF9lTUJXG/uUGk\n+JETpKmQviPbRsvzYhz4pf6ZIOZMc3/GIcNq92ECbseGO+yAgyWUVKMmZM0HqXC9\novNslqe0M8C1sLm1zAR5z/h/litE7/8O2ietija3Q/qtl2TOXJdCA6sgjJX2WUql\nybrC55ct18NKf3qhpcEkGQvFU40rVYApJpi98DiZPYFdx1oBDp/f4uZ3ojpxRVFT\ncDwcJLfNRCPUhormsY7fDS9xSyThiHsW9mjJYdcaKQkwYZ0F11yB\n-----END CERTIFICATE-----\n'
+        new_key = b'-----BEGIN ENCRYPTED PRIVATE KEY-----\nMIHeMEkGCSqGSIb3DQEFDTA8MBsGCSqGSIb3DQEFDDAOBAj9XnJ2h78QVAICCAAw\nHQYJYIZIAWUDBAECBBBeiiOF2LnLzq/wjb/viwMwBIGQk28Zkfj2EIk42bgc7UzC\nSf98qssCVhsIYz0Xa3eSATg8Cpn83YieaBeyxdk/tXTnrOhxMV/vt7T98kWhaGbH\n5Z9CdGVLfes0UFvVJqrlk6vcf2sOnLCGbrn78HS+ayrGOCRSCd/7+dnEiB/7Um1B\nMk6BBJHsLEnZZSHyfrw8jvYgVmcSBy/WdY0pqldD/+4D\n-----END ENCRYPTED PRIVATE KEY-----\n'
+


It looks like these certificates are copy/pasted all over the tests. Can we make them shared constants or something? Or maybe a test fixture?

Signed-off-by: Radhika Agrawal <agrawalradhika@google.com>

daniel-sanche · 2025-11-21T22:10:33Z

google/auth/transport/requests.py

            and _credential_refresh_attempt < self._max_refresh_attempts
        ):
-
+            if response.status_code == 401:


it might be good to have a comment above this explaining what 401 represents

daniel-sanche · 2025-11-21T22:11:37Z

google/auth/transport/requests.py

+                        _LOGGER.info(
+                            "Skipping reconfiguration of mTLS channel because the client"
+                            " certificate has not changed."
+                        )


I'd suggest pulling out all this new code into a helper. The request method is getting pretty long and hard to follow

daniel-sanche · 2025-11-21T22:18:18Z

google/auth/transport/requests.py

+                    )
+                    cert_obj = _agent_identity_utils.parse_certificate(
+                        call_cert_callback_result[0]
+                    )


It looks like the parser can raise ValueErrors for invalid data. Are we ok with raising an exception here?

If so, we should declare it in the docstring

daniel-sanche · 2025-11-21T22:21:54Z

google/auth/transport/requests.py

+                if self.is_mtls:
+                    call_cert_callback_result = (
+                        _agent_identity_utils.call_client_cert_callback()
+                    )


seeing an untyped call_cert_callback_result and then indexing into it later makes me a bit nervous

I'd suggest either unpacking this as two values:

call_cert_bytes, call_key_bytes = ( _agent_identity_utils.call_client_cert_callback() )

or giving it a type annotation:

call_cert_callback_result: tuple[bytes, bytes] = ( _agent_identity_utils.call_client_cert_callback() )

daniel-sanche · 2025-11-21T22:24:50Z

google/auth/transport/requests.py

+    @property
+    def cached_cert(self):
+        """Returns the cached client certificate."""
+        return self._cached_cert


Are you sure this is something that needs to be publicly accessible?

If so, can you add a type here? Either using a type annotation, or just describing it in the doctoring?

daniel-sanche · 2025-11-21T22:27:06Z

google/auth/transport/urllib3.py


+        use_mtls = False
+        if self._is_mtls == True:
+            if "mtls.googleapis.com" in url or "mtls.sandbox.googleapis.com" in url:


nit: consider extracting these out

MTLS_URL_PREFIXES = ["mtls.googleapis.com", "mtls.sandbox.googleapis.com"] ... use_mtls = any([prefix in url for prefix in MTLS_URL_PREFIXES])

daniel-sanche · 2025-11-21T22:30:13Z

google/auth/transport/urllib3.py

        ):
-
-            _LOGGER.info(
+          if response.status == 401:


This code looks almost identical to the code in requests.py. I already suggested pulling it into a helper, can you use the same helper here?

daniel-sanche · 2025-11-21T22:36:49Z

google/auth/transport/requests.py

+                            )
+                        except Exception as e:
+                            _LOGGER.error("Failed to reconfigure mTLS channel: %s", e)
+                            raise e


what exceptions do we expect here? The docstring only mentions google.auth.exceptions.MutualTLSChannelError (This may be a good place for a raise from, and raise MutualTLSChannelError here)

agrawalradhika-cell added 2 commits October 15, 2025 16:50

feat: Add retry logic when certificate mismatch for Agent Identity

4201d20

This change introduces retry support when requests are created for AgentIdentities on GKE and Cloud Run Workloads. Signed-off-by: Radhika Agrawal <agrawalradhika@google.com>

feat: Add unit tests for cert rotation when mismatch for mTLS connect…

4b4124f

…ion and request Signed-off-by: Radhika Agrawal <agrawalradhika@google.com>

agrawalradhika-cell changed the title ~~Cert rotation x509~~ feat: Add retry logic when certificate mismatch for Agent Identity Oct 16, 2025

agrawalradhika-cell marked this pull request as ready for review October 17, 2025 01:17

agrawalradhika-cell requested review from a team as code owners October 17, 2025 01:17

agrawalradhika-cell marked this pull request as draft October 17, 2025 01:17

feat: Update urllib3 and it's unit test for cert rotation if mismatch…

04be65b

… from mTLS configuration Signed-off-by: Radhika Agrawal <agrawalradhika@google.com>

agrawalradhika-cell marked this pull request as ready for review October 17, 2025 19:54

agrawalradhika-cell changed the title ~~feat: Add retry logic when certificate mismatch for Agent Identity~~ feat: Add retry logic when certificate mismatch for existing credentials & Agent Identity workloads Oct 20, 2025

sai-sunder-s reviewed Oct 20, 2025

View reviewed changes

google/auth/transport/requests.py Outdated Show resolved Hide resolved

google/auth/transport/requests.py Outdated Show resolved Hide resolved

google/auth/transport/requests.py Outdated Show resolved Hide resolved

google/auth/transport/urllib3.py Outdated Show resolved Hide resolved

nbayati reviewed Oct 22, 2025

View reviewed changes

google/auth/transport/requests.py Outdated Show resolved Hide resolved

agrawalradhika-cell added 3 commits October 22, 2025 15:03

Merge branch 'googleapis:main' into cert-rotation-x509

4c33b9f

fix: Fix the indentations and make changes according to review comments

95f201d

Signed-off-by: Radhika Agrawal <agrawalradhika@google.com>

fix: fix the indentations on requests.py and urllib3.py

4a5e142

Signed-off-by: Radhika Agrawal <agrawalradhika@google.com>

agrawalradhika-cell requested review from nbayati and sai-sunder-s October 23, 2025 22:42

nbayati requested changes Oct 24, 2025

View reviewed changes

nbayati added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Oct 28, 2025

sai-sunder-s reviewed Oct 30, 2025

View reviewed changes

google/auth/transport/requests.py Show resolved Hide resolved

google/auth/transport/requests.py Show resolved Hide resolved

sai-sunder-s previously approved these changes Oct 30, 2025

View reviewed changes

daniel-sanche reviewed Oct 30, 2025

View reviewed changes

fix: minor fixes for indentation and docstring

e292e9c

Signed-off-by: Radhika Agrawal <agrawalradhika@google.com>

agrawalradhika-cell dismissed sai-sunder-s’s stale review via e292e9c November 21, 2025 08:44

Merge branch 'main' into cert-rotation-x509

e6f1214

daniel-sanche reviewed Nov 21, 2025

View reviewed changes

feat: Add retry logic when certificate mismatch for existing credentials & Agent Identity workloads #1841

Are you sure you want to change the base?

feat: Add retry logic when certificate mismatch for existing credentials & Agent Identity workloads #1841

Uh oh!

Conversation

agrawalradhika-cell commented Oct 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lsirac commented Oct 18, 2025

Uh oh!

agrawalradhika-cell commented Oct 20, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

daniel-sanche left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

daniel-sanche Nov 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

agrawalradhika-cell commented Oct 16, 2025 •

edited

Loading

daniel-sanche Nov 21, 2025 •

edited

Loading