feat: Add retry logic when certificate mismatch for existing credentials & Agent Identity workloads #1841
+322
−5
Conversation
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 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>
|
Is the description accurate? This will apply to existing X509 workloads too? |
Updated the description |
|
|
||
| if self._is_mtls: | ||
| mtls_adapter = _MutualTlsAdapter(cert, key) | ||
| self._cached_cert = lambda: (cert) |
There was a problem hiding this comment.
did you intend to put this line under the if condition?
Comment on lines
+627
to
+628
| _mtls_helper.get_client_ssl_credentials(generate_encrypted_key=True) | ||
| ) |
| if cached_fingerprint != current_cert_fingerprint: | ||
| try: | ||
| _LOGGER.info("Client certificate has changed, reconfiguring mTLS channel.") | ||
| self.configure_mtls_channel(self.call_client_cert_callback) |
There was a problem hiding this comment.
self.call_client_cert_callback is called twice, once to check and once to set. can you reuse from the check?
Comment on lines
+494
to
+511
| def call_client_cert_callback(self): | ||
| """Calls the current client cert callback and returns the certificate and key.""" | ||
| _, cert_bytes, key_bytes, passphrase = ( | ||
| _mtls_helper.get_client_ssl_credentials(generate_encrypted_key=True) | ||
| ) | ||
| return cert_bytes, key_bytes | ||
|
|
||
| def get_cached_cert_fingerprint(self): | ||
| """Returns the fingerprint of the cached certificate.""" | ||
| if self._cached_cert: | ||
| cached_cert_fingerprint = ( | ||
| _agent_identity_utils.calculate_certificate_fingerprint( | ||
| self._cached_cert() | ||
| ) | ||
| ) | ||
| else: | ||
| raise ValueError("mTLS connection is not configured.") | ||
| return cached_cert_fingerprint |
There was a problem hiding this comment.
these 2 methods are same as in requests.py. can you put them in some utils or modify existing utils to take in the value of self._cached_cert
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
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.