feat: Integrate mTLS support into Agent Registry API calls#5861
feat: Integrate mTLS support into Agent Registry API calls#5861agrawalradhika-cell wants to merge 9 commits into
Conversation
|
Response from ADK Triaging Agent Hello @agrawalradhika-cell, thank you for creating this PR! While reviewing your PR, I noticed a few items from our contribution guidelines are currently missing or incomplete:
Providing this information will help our reviewers review your PR more efficiently. Thank you! |
|
Hi @agrawalradhika-cell , Thank you for your contribution! We appreciate you taking the time to submit this pull request. Your PR has been received by the team and is currently under review. We will provide feedback as soon as we have an update to share. |
|
Hi @Jacksunwei , can you please review this. |
| except httpx.HTTPStatusError as e: | ||
| # Using AuthorizedSession for internal API calls to handle mTLS/Auth. | ||
| response = session.get( | ||
| url, headers=self._get_auth_headers(), params=params |
There was a problem hiding this comment.
we shouldn't be manually injecting _get_auth_headers() into every request (I think it defeats the point of AuthorizedSession)
perhaps we should have something like
response = session.get(
url, headers=self._get_request_headers(), params=params
)
There was a problem hiding this comment.
I am not able to follow exactly what you mean by this? Do you mean something like response = session.get( url, params=params )
Can you please elaborate? Thanks!
There was a problem hiding this comment.
something like this:
headers = {}
quota_project_id = getattr(self._credentials, "quota_project_id", None) or self.project_id
if quota_project_id:
headers["x-goog-user-project"] = quota_project_id
try:
# Let AuthorizedSession handle Authorization/Refresh automatically
response = session.get(url, headers=headers, params=params)
response.raise_for_status()
return response.json()
fix: add client_cert_source in configure_mtls_channel
fce1c73 to
aa51512
Compare
| ) -> Dict[str, Any]: | ||
| """Helper function to make GET requests to the Agent Registry API.""" | ||
| # Determine if mTLS should be used | ||
| session = requests_auth.AuthorizedSession(credentials=self._credentials) |
There was a problem hiding this comment.
creating a new AuthorizedSession on every request could be a performance bottleneck - it would be better to instantiate and configure the session once during initialization (in the init) and reuse it
Link to Issue or Description of Change
1. Link to an existing issue (if applicable):
2. Or, if no issue exists, describe the change:
Problem:
The current
AgentRegistryclient useshttpx.Clientfor API requests, which does not inherently support the mTLS (mutual TLS) requirements and automatic endpoint selection needed for secure Google API interactions.Solution:
Integrated mTLS support by transitioning the
AgentRegistryclient to usegoogle.auth.transport.requests.AuthorizedSession. This change allows the client to:GOOGLE_API_USE_CLIENT_CERTIFICATEandGOOGLE_API_USE_MTLS_ENDPOINT.agentregistry.googleapis.com) and mTLS (agentregistry.mtls.googleapis.com) endpoints.Testing Plan
Please describe the tests that you ran to verify your changes. This is required
for all PRs that are not small documentation or typo fixes.
Unit Tests:
Please include a summary of passed
pytestresults.Manual End-to-End (E2E) Tests:
Checklist
Additional context
Add any other context or screenshots about the feature request here.