Repository navigation
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughAdds a Kyverno integration: docs (README, nav, index, kyverno.md), a new ChangesKyverno integration
Sequence Diagram(s)sequenceDiagram
participant User
participant Holmes
participant Kubectl as "kubectl / K8s API"
participant Kyverno as "Kyverno controllers / CRs"
participant LLM as "Log summarizer (LLM)"
User->>Holmes: Request Kyverno investigation
Holmes->>Kubectl: Run toolset commands (list/get policies, reports, updatereqs, logs)
Kubectl->>Kyverno: Query CRDs and controller logs
Kyverno-->>Kubectl: Return resources and logs
Kubectl-->>Holmes: Return command outputs
Holmes->>LLM: Send logs + transformer prompt
LLM-->>Holmes: Summarized findings
Holmes-->>User: Present investigation summary and commands
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
✅ Deploy Preview for holmes-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
tests/llm/fixtures/test_ask_holmes/257_kyverno_policy_violations/test_case.yaml (1)
14-14:include_tool_calls: truemay not be needed here.The expected_output already pins very specific values (policy name, resource name, and the
HOLMES-EVAL-KYVERNO-001verification code), which is sufficient to rule out hallucinations. Including tool calls increases eval runtime and prompt size without adding signal.As per coding guidelines: "use
include_tool_calls: trueonly when expected output is too generic to rule out hallucinations - prefer specific value checking when possible".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/llm/fixtures/test_ask_holmes/257_kyverno_policy_violations/test_case.yaml` at line 14, The test fixture currently enables include_tool_calls which isn't necessary because the expected_output already asserts concrete values (policy name, resource name, and verification code); update the test case by removing include_tool_calls or setting include_tool_calls: false in the YAML so tool call logging is not performed, leaving the expected_output assertions intact (refer to the include_tool_calls key in the test_ask_holmes fixture).holmes/plugins/toolsets/kyverno.yaml (3)
57-59: Tool name suggests "get one" but command lists all reports in the namespace.
kyverno_get_policy_reportrunskubectl get policyreport -n {{ namespace }} -o yamlwhich returns every PolicyReport in the namespace, not a specific one. The singular name (paired with the singularkyverno_get_cluster_policy_reportwhich does take areport_name) is likely to confuse the LLM into supplying areport_nametemplate variable that is silently ignored. Either rename tokyverno_list_policy_reports_detailed/kyverno_get_policy_reports, or add an optionalreport_nameparameter so a single report can be fetched.🛠️ Suggested fix
- - name: "kyverno_get_policy_report" - description: "Get the full PolicyReport for a namespace including all individual rule results with violation messages and affected resource names" - command: "kubectl get policyreport -n {{ namespace }} -o yaml 2>&1" + - name: "kyverno_get_policy_report" + description: "Get full PolicyReport YAML for a namespace. If report_name is provided, fetch that specific report; otherwise return all PolicyReports in the namespace, including individual rule results, violation messages, and affected resource names." + command: "kubectl get policyreport{% if report_name %} {{ report_name }}{% endif %} -n {{ namespace }} -o yaml 2>&1"
33-34: Prerequisite check is reasonable; minor robustness suggestion.
kubectl api-resources --api-group=kyverno.io --no-headers | grep -q clusterpoliciescorrectly auto-detects Kyverno presence. One minor consideration: in clusters where the API server is briefly unavailable,kubectl api-resourcescan hang or return an empty list; consider adding--request-timeout=5sso the toolset prerequisite never blocks Holmes startup.🛠️ Suggested fix
- - command: "kubectl api-resources --api-group=kyverno.io --no-headers 2>/dev/null | grep -q clusterpolicies" + - command: "kubectl api-resources --api-group=kyverno.io --no-headers --request-timeout=5s 2>/dev/null | grep -q clusterpolicies"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@holmes/plugins/toolsets/kyverno.yaml` around lines 33 - 34, The prerequisites command that auto-detects Kyverno can hang when the API server is slow; update the command string under the prerequisites key (the existing "kubectl api-resources --api-group=kyverno.io --no-headers 2>/dev/null | grep -q clusterpolicies") to include a short request timeout (e.g., add --request-timeout=5s) while preserving the stderr redirection and grep check so the prerequisite never blocks Holmes startup.
5-5: Self-host the Kyverno logo instead of hot-linkingmain.
https://raw.githubusercontent.com/kyverno/kyverno/main/img/logo.pngfloats with upstream; if Kyverno renames or moves the file the icon will silently 404. Other toolsets in this repo ship their logos underimages/integration_logos/. Add akyverno-icon.png(or pin to a tagged commit/release) and reference it here and fromREADME.md.Based on learnings: "When adding a new integration (toolset), … add logo to images/integration_logos/".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@holmes/plugins/toolsets/kyverno.yaml` at line 5, The kyverno toolset currently hot-links the logo via icon_url pointing to the Kyverno repo main branch; add a local copy under images/integration_logos (e.g., images/integration_logos/kyverno-icon.png) or reference a pinned commit/release URL, then update the icon_url value in holmes/plugins/toolsets/kyverno.yaml to point to that local asset (and update any README.md references to use the same local path), ensuring the symbol to change is the icon_url entry in kyverno.yaml and the README references to the Kyverno logo.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@README.md`:
- Line 66: README.md currently reuses kubernetes-icon.png for Kyverno; add a new
kyverno-icon.png (PNG) into images/integration_logos/ and update the README.md
line that references kubernetes-icon.png to point to
images/integration_logos/kyverno-icon.png and keep the existing link text to
Kyverno; also open docs/why-holmesgpt.md and add Kyverno to the supported
integrations list (a short mention consistent with other entries) so the docs
reflect the new integration.
In
`@tests/llm/fixtures/test_ask_holmes/257_kyverno_policy_violations/test_case.yaml`:
- Around line 52-56: The error message stating "Kyverno failed to become ready
after 120 seconds" is incorrect because the readiness loop uses 60 iterations of
`kubectl wait --timeout=5s` plus `sleep 2`, giving ~7 minutes total; update the
message to reflect the actual timeout (e.g., "after ~7 minutes" or "after 420
seconds") or adjust the loop/iteration count to truly match 120 seconds;
reference the KYVERNO_READY check and the surrounding loop that calls `kubectl
wait --timeout=5s` and `sleep 2` when making the change.
- Around line 25-31: Replace the floating "latest" install with a pinned Kyverno
release (v1.17.1) and make installation idempotent: instead of using the
namespace-only existence check and `kubectl create -f
https://.../latest/.../install.yaml`, verify a Kyverno component/CRD is present
(e.g., check for the kyverno deployment in the kyverno namespace or the
kyvernopolicies CRD) and if absent apply the pinned manifest using an idempotent
method (e.g., `kubectl apply --server-side -f` or `helm upgrade --install`
against the v1.17.1 chart); ensure the code references the pinned URL for
v1.17.1 and swaps `kubectl create -f` for an apply/upgrade path so repeated runs
won’t error when resources already exist.
---
Nitpick comments:
In `@holmes/plugins/toolsets/kyverno.yaml`:
- Around line 33-34: The prerequisites command that auto-detects Kyverno can
hang when the API server is slow; update the command string under the
prerequisites key (the existing "kubectl api-resources --api-group=kyverno.io
--no-headers 2>/dev/null | grep -q clusterpolicies") to include a short request
timeout (e.g., add --request-timeout=5s) while preserving the stderr redirection
and grep check so the prerequisite never blocks Holmes startup.
- Line 5: The kyverno toolset currently hot-links the logo via icon_url pointing
to the Kyverno repo main branch; add a local copy under images/integration_logos
(e.g., images/integration_logos/kyverno-icon.png) or reference a pinned
commit/release URL, then update the icon_url value in
holmes/plugins/toolsets/kyverno.yaml to point to that local asset (and update
any README.md references to use the same local path), ensuring the symbol to
change is the icon_url entry in kyverno.yaml and the README references to the
Kyverno logo.
In
`@tests/llm/fixtures/test_ask_holmes/257_kyverno_policy_violations/test_case.yaml`:
- Line 14: The test fixture currently enables include_tool_calls which isn't
necessary because the expected_output already asserts concrete values (policy
name, resource name, and verification code); update the test case by removing
include_tool_calls or setting include_tool_calls: false in the YAML so tool call
logging is not performed, leaving the expected_output assertions intact (refer
to the include_tool_calls key in the test_ask_holmes fixture).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 143b3845-1300-49e4-832e-84ef44442e0d
📒 Files selected for processing (7)
README.mddocs/data-sources/builtin-toolsets/.nav.ymldocs/data-sources/builtin-toolsets/index.mddocs/data-sources/builtin-toolsets/kyverno.mdholmes/plugins/toolsets/kyverno.yamltests/llm/fixtures/test_ask_holmes/257_kyverno_policy_violations/test_case.yamltests/llm/fixtures/test_ask_holmes/257_kyverno_policy_violations/toolsets.yaml
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
|
Hi there! Thanks for the pr submission. |
Hi @naomi-robusta The generic kubectl/bash toolsets can technically run any kubectl command, but they don't know which commands to run for Kyverno — and that's the core problem this toolset solves. Kyverno violations require a specific traversal: list active policies and their failure action (Enforce vs Audit) → check PolicyReport summaries by namespace → drill into individual violation messages → inspect UpdateRequests for generate/mutate failures → check controller logs. The llm_instructions block encodes this directly into the LLM's context so Holmes follows the right path every time, rather than exploring ad hoc.
|
|
@mailprak |
@naomi-robusta Sure, |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@tests/llm/fixtures/test_ask_holmes/258_kyverno_without_toolset/test_case.yaml`:
- Around line 7-8: Update the expected evaluation strings to use the actual
ClusterPolicy resource name instead of the rule name: replace occurrences like
"Must mention the policy name 'require-team-label'" with the full resource
identifier "ClusterPolicy/require-team-label-258" (and similarly update the
related expectations at the other referenced locations). Locate and edit the
fixture expectations in
test_ask_holmes/258_kyverno_without_toolset/test_case.yaml that reference
'require-team-label' (including the blocks around lines 24-25 and 68-73) so they
assert the exact policy identifier and the exact violating resource name
'unlabeled-api'.
- Around line 39-60: The script currently only waits for
deployment/kyverno-admission-controller, but PolicyReport creation also requires
the reports controller (and ideally the background controller) to be ready;
update the readiness loop to check all three
deployments—kyverno-admission-controller, kyverno-reports-controller, and
kyverno-background-controller—before setting KYVERNO_READY to true (e.g., test
each deployment with kubectl wait and only break when all three succeed), and
keep the existing timeout/failure behavior and diagnostic kubectl get pods call
if readiness fails.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 053aff4d-7747-411a-a9df-4c12864ecfb3
⛔ Files ignored due to path filters (1)
images/integration_logos/kyverno-icon.pngis excluded by!**/*.png
📒 Files selected for processing (2)
tests/llm/fixtures/test_ask_holmes/258_kyverno_without_toolset/test_case.yamltests/llm/fixtures/test_ask_holmes/258_kyverno_without_toolset/toolsets.yaml
✅ Files skipped from review due to trivial changes (1)
- tests/llm/fixtures/test_ask_holmes/258_kyverno_without_toolset/toolsets.yaml
This branch has not been deployed
2 participants
New Features
Added a Kyverno toolset (kyverno/core) exposing comprehensive kubectl-based tools for troubleshooting the Kyverno policy engine, including listing and inspecting ClusterPolicies and namespaced Policies, querying PolicyReport and ClusterPolicyReport summaries and details to surface violation messages and affected resources, inspecting UpdateRequests for generate/mutate-existing rule failures, and streaming admission, background, and reports controller logs with automatic LLM summarization for large log volumes.
The toolset includes guided LLM instructions that enforce a structured investigation order — starting from active policies and failure actions (Enforce vs Audit), narrowing to namespaces with violations via report summaries, then drilling into specific violation messages and resource names — and auto-detects Kyverno's presence via a prerequisite check against the kyverno.io API group.
Documentation
Added comprehensive Kyverno documentation (docs/data-sources/builtin-toolsets/kyverno.md) covering prerequisites, RBAC requirements for kyverno.io and reports.kyverno.io API groups, configuration examples for both Holmes CLI and Robusta Helm Chart deployments, and common use case prompts. Integrated Kyverno into the toolset index page and site navigation.
Tests
Added an LLM evaluation test (tests/llm/fixtures/test_ask_holmes/257_kyverno_policy_violations/) with a complete end-to-end scenario: installs Kyverno if not present, creates a ClusterPolicy in Audit mode that requires a team label on pods, deploys a violating pod (unlabeled-api) without that label, waits for a PolicyReport with failures (with a pod-recreation fallback to force fresh admission evaluation), and verifies that Holmes correctly identifies the policy name, the violating resource, and the embedded verification code HOLMES-EVAL-KYVERNO-001 from the violation message.
Summary by CodeRabbit
New Features
Documentation
Tests