fix: construct cloud run url for agent card when using: adk deploy cloud_run --a2a #2155
Conversation
andrewlarimer
force-pushed
the
fix/construct-cloud-run-url-for-agent-card-when-using-adk-deploy-cloud_run---a2a
branch
from
dad7a81 to
c4145be
Compare
|
I like this solution (thanks Andrew!) but I don't really like the We talked off-thread about Perhaps better isolating the effect of this change to just the |
|
@DeanChensj Please help review this PR |
src/google/adk/cli/cli_deploy.py
Outdated
| @@ -239,6 +239,8 @@ def to_cloud_run( | |||
| log_level.lower() if log_level else verbosity, | |||
| '--labels', | |||
| 'created-by=adk', | |||
| '--set-env-vars', | |||
| 'GOOGLE_CLOUD_RUN_K_SERVICE=1' | |||
There was a problem hiding this comment.
ADK core maintainers, is this the right config you want to key off?
| @@ -1102,7 +1103,15 @@ async def _get_a2a_runner_async() -> Runner: | |||
| logger.info("Setting up A2A agent: %s", app_name) | |||
|
|
|||
| try: | |||
| a2a_rpc_path = f"http://{host}:{port}/a2a/{app_name}" | |||
| if "GOOGLE_CLOUD_RUN_K_SERVICE" in os.environ: | |||
There was a problem hiding this comment.
if derive the host URL
else use the default host:port
There was a problem hiding this comment.
also such logic should not be a2a specific, features like allowed origin also needs to allowlist the actual host name. we should not create discrepancy here.
…hen-using-adk-deploy-cloud_run---a2a
|
actually we don't need to do something specific to cloud run. we need to support 0.0.0.0 as host name when we start api server anyway. this is a valid command that we should support. a2a sdk is working on some feature so that agent card can be dynamically built from request. in the case of 0.0.0.0 the host name in rpc url should be whatever from the request. I think this PR is kind of workaround or hacky. Will let @Jacksunwei who developed cloud run deployment feature to review. |
| @@ -317,7 +318,15 @@ async def _get_a2a_runner_async() -> Runner: | |||
| logger.info("Setting up A2A agent: %s", app_name) | |||
|
|
|||
| try: | |||
| a2a_rpc_path = f"http://{host}:{port}/a2a/{app_name}" | |||
| if "GOOGLE_CLOUD_RUN_K_SERVICE" in os.environ: | |||
There was a problem hiding this comment.
we should not put cloud run specific logic into fast api server
There was a problem hiding this comment.
We could construct the URL in the to_cloud_run function and pass that as the env variable for fast api server to look for and use if it's present.
Right now when using |
|
I think this is a needed feature, but the approach is too specific to cloud run. When deploying an agent with a2a, I'd expect user needs to publish with a different url other than the cloud_run k service in real case. I think we can decouple the flag with cloud run. Something like: As for deploying to cloud run, we should allow user to specify the url, |
That makes sense to me. Would you like me to implement that @Jacksunwei, or is that something you'd prefer to implement? |
|
Yes, please. We love your contribution 😀 |
How does |
Currently, using adk deploy cloud_run --a2a invokes the deployment with adk api_server, however the URL on the agent card (even if the correct URL is provided) is overwritten by https://0.0.0.0:port/a2a/agent_name.
In order for another agent, such as an ADK RemoteA2aAgent to use the Agent Card and access the agent, the URL needs to be a correct Cloud Run service URL.
This fix sets a new IS_DEPLOY_TO_CLOUD_RUN env variable in the Dockerfile, and updates the api_server such that if that env variable is present, it constructs the Cloud Run service URL appropriately to update the url on the Agent Card.
This has been tested by modifying the Dockerfile to deploy to cloud run with this env variable and copy the modified fast_api.py script into the Cloud Run container. The hosted agent card then displays the correct URL, and an ADK RemoteA2aAgent can access the remote agent.
Feel free to ping me for test steps in a lab environment, if that is helpful.