-
Notifications
You must be signed in to change notification settings - Fork 303
[ROB-2694] holmes-oauth #2047
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
[ROB-2694] holmes-oauth #2047
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -12,6 +12,7 @@ | |||||||||||||||||||||||
| HolmesChatParams, | ||||||||||||||||||||||||
| HolmesConversationParams, | ||||||||||||||||||||||||
| HolmesIssueChatParams, | ||||||||||||||||||||||||
| HolmesOAuthParams, | ||||||||||||||||||||||||
| ResourceInfo, | ||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||
| from robusta.core.model.events import ExecutionBaseEvent | ||||||||||||||||||||||||
|
|
@@ -385,6 +386,45 @@ def holmes_chat(event: ExecutionBaseEvent, params: HolmesChatParams): | |||||||||||||||||||||||
| handle_holmes_error(e) | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| @action | ||||||||||||||||||||||||
| def holmes_oauth(event: ExecutionBaseEvent, params: HolmesOAuthParams): | ||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||
| Forwards OAuth callback data to Holmes for token exchange and storage. | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| This action is a thin pass-through — the actual OAuth contract is defined | ||||||||||||||||||||||||
| in Holmes. Params use extra=allow so Holmes can evolve the contract | ||||||||||||||||||||||||
| (e.g. add new fields) without requiring runner changes. | ||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||
| holmes_url = HolmesDiscovery.find_holmes_url(params.holmes_url) | ||||||||||||||||||||||||
| if not holmes_url: | ||||||||||||||||||||||||
| raise ActionException( | ||||||||||||||||||||||||
| ErrorCodes.HOLMES_DISCOVERY_FAILED, | ||||||||||||||||||||||||
| "Robusta couldn't connect to the Holmes client.", | ||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||
| params_dict = params.dict(exclude={"holmes_url", "model"}) | ||||||||||||||||||||||||
| result = requests.post( | ||||||||||||||||||||||||
| f"{holmes_url}/api/oauth/callback", | ||||||||||||||||||||||||
| json=params_dict, | ||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||
|
Comment on lines
+407
to
+410
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Verify outbound requests in this module and whether they set explicit timeouts.
rg -nP 'requests\.post\(' src/robusta/core/playbooks/internal/ai_integration.py -A6 -B2Repository: robusta-dev/robusta Length of output: 2804 Add an explicit timeout to the OAuth callback request. Line 407 issues an outbound HTTP call without Proposed fix+HOLMES_OAUTH_TIMEOUT = (5, 30) # connect, read (seconds)
+
result = requests.post(
f"{holmes_url}/api/oauth/callback",
json=params_dict,
+ timeout=HOLMES_OAUTH_TIMEOUT,
)📝 Committable suggestion
Suggested change
🧰 Tools🪛 Ruff (0.15.9)[error] 407-407: Probable use of (S113) 🤖 Prompt for AI Agents |
||||||||||||||||||||||||
| result.raise_for_status() | ||||||||||||||||||||||||
| response_data = result.json() | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| finding = Finding( | ||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we need this finding? |
||||||||||||||||||||||||
| title="Holmes OAuth", | ||||||||||||||||||||||||
| aggregation_key="HolmesOAuthResult", | ||||||||||||||||||||||||
| subject=FindingSubject(subject_type=FindingSubjectType.TYPE_NONE), | ||||||||||||||||||||||||
| finding_type=FindingType.AI_ANALYSIS, | ||||||||||||||||||||||||
| failure=not response_data.get("success", False), | ||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||
| event.add_finding(finding) | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| except Exception as e: | ||||||||||||||||||||||||
| logging.exception("Failed to complete Holmes OAuth callback") | ||||||||||||||||||||||||
| handle_holmes_error(e) | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| def stream_and_render_graphs(url, holmes_req, event): | ||||||||||||||||||||||||
| with requests.post( | ||||||||||||||||||||||||
| url, | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Those are the only 2 var in HolmesParams why we inheriting it only to exclude those fields?