HDDS-14816. Add Recon AI Assistant backend foundation with Google Gemini integration.#27
HDDS-14816. Add Recon AI Assistant backend foundation with Google Gemini integration.#27priyeshkaratha wants to merge 1 commit intomasterfrom
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a Recon Chatbot feature that enables users to query Ozone cluster information using LLMs like Gemini, OpenAI, and Anthropic. The implementation includes a ChatbotAgent for orchestration, a ToolExecutor for executing internal Recon API calls with paging support, and a security layer for managing API keys via JCEKS. Feedback focuses on improving security by avoiding hardcoded placeholders and URL-based API keys, as well as enhancing configurability for the Recon base URL and model-to-client routing logic.
| # Enable Recon Chatbot for testing | ||
| OZONE-SITE.XML_ozone.recon.chatbot.enabled=true | ||
| OZONE-SITE.XML_ozone.recon.chatbot.provider=gemini | ||
| OZONE-SITE.XML_ozone.recon.chatbot.gemini.api.key=YOUR_API_KEY_HERE |
There was a problem hiding this comment.
The API key is hardcoded as a placeholder. For production or even shared development environments, it's crucial to manage API keys securely, for example, using environment variables or a secrets management system, rather than embedding them directly in configuration files. This prevents accidental exposure of sensitive credentials.
| public ToolExecutor(OzoneConfiguration configuration) { | ||
| // Get Recon base URL from configuration | ||
| // Default to localhost for local development | ||
| this.reconBaseUrl = "http://localhost:9888"; |
There was a problem hiding this comment.
The Recon base URL is hardcoded to http://localhost:9888. This should be configurable via OzoneConfiguration to allow deployment in environments where Recon is not running on localhost or uses a different port.
| this.reconBaseUrl = "http://localhost:9888"; | |
| this.reconBaseUrl = configuration.get("ozone.recon.address", "http://localhost:9888"); |
| throw new LLMException("No API key configured for provider 'gemini'."); | ||
| } | ||
|
|
||
| String url = getBaseUrl() + "/v1beta/models/" + model + ":generateContent?key=" + resolvedKey; |
There was a problem hiding this comment.
The Gemini API key is passed directly in the URL as a query parameter. While this might be dictated by the Gemini API design, it's generally less secure than using HTTP headers for sensitive information, as URL parameters can be exposed in server logs or network monitoring.
| if (requestModel != null) { | ||
| if (requestModel.contains(":")) { | ||
| String[] parts = requestModel.split(":", 2); | ||
| String prefix = parts[0].toLowerCase(); | ||
| if (clients.containsKey(prefix)) { | ||
| return clients.get(prefix); | ||
| } | ||
| } | ||
|
|
||
| String m = requestModel.toLowerCase(); | ||
| if (m.startsWith("gpt-") || m.startsWith("o1") || m.startsWith("o3")) { | ||
| return clients.get("openai"); | ||
| } | ||
| if (m.startsWith("gemini")) { | ||
| return clients.get("gemini"); | ||
| } | ||
| if (m.startsWith("claude")) { | ||
| return clients.get("anthropic"); | ||
| } |
There was a problem hiding this comment.
The resolveClient method uses hardcoded string prefixes (e.g., "gpt-", "gemini", "claude") to determine which LLM client to use. This approach can be brittle and require code changes if new models are introduced with different naming conventions or if the preferred client for a model changes. Consider a more flexible and configurable mapping for model-to-client resolution.
2 participants
What changes were proposed in this pull request?
Provide a one-liner summary of the changes in the PR Title field above.
It should be in the form of
HDDS-1234. Short summary of the change.Please describe your PR in detail:
perspective not just for the reviewer.
the Jira's description if the jira is well defined.
issue investigation, github discussion, etc.
Examples of well-written pull requests:
What is the link to the Apache JIRA
Please create an issue in ASF JIRA before opening a pull request, and you need to set the title of the pull
request which starts with the corresponding JIRA issue number. (e.g. HDDS-XXXX. Fix a typo in YYY.)
If you do not have an ASF Jira account yet, please follow the first-time contributor
instructions in the Jira guideline.
(Please replace this section with the link to the Apache JIRA)
How was this patch tested?
(Please explain how this patch was tested. Ex: unit tests, manual tests, workflow run on the fork git repo.)
(If this patch involves UI changes, please attach a screenshot; otherwise, remove this.)