Skip to content

chore(toolset): add Cilium and Hubble to toolsets - #769

Merged
mainred merged 12 commits into
HolmesGPT:masterfrom
matmerr:matmerr/holmes-cilium
Oct 1, 2025
Merged

mainred merged 12 commits into
HolmesGPT:masterfrom
matmerr:matmerr/holmes-cilium

Conversation

@matmerr

@matmerr matmerr commented Jul 31, 2025

Copy link
Copy Markdown
Contributor

Looking to add the Cilium and Hubble tools for further network diagnostics. Relatively trivial, but in basic experimentation I did find it to be useful.

@CLAassistant

CLAassistant commented Jul 31, 2025 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@coderabbitai

coderabbitai Bot commented Jul 31, 2025 •

Copy link
Copy Markdown
Contributor

Walkthrough

Adds Cilium and Hubble integrations: a new Cilium toolset docs page, an index entry, and a YAML toolset declaration registering cilium/core and hubble/observability with metadata, LLM guidance, prerequisites, timeouts, and exposed CLI tools for diagnostics and flow observability. (48 words)

Changes

Cohort / File(s) Change Summary
Cilium Toolset Documentation
docs/data-sources/builtin-toolsets/cilium.md
Added comprehensive docs covering prerequisites, Holmes/Helm configuration, capabilities for Cilium (core) and Hubble (observability), example commands, use cases, and troubleshooting sequences.
Toolset Index
docs/data-sources/builtin-toolsets/index.md
Added Cilium entry (icon and link) to built-in toolsets list.
Toolset Configuration (YAML)
holmes/plugins/toolsets/cilium.yaml
Added cilium/core and hubble/observability toolset declarations with docs/icons, llm_instructions, prerequisites, default timeout (300s), cli tag, and lists of exposed commands (status, config, sysdump, connectivity tests, hubble observe variants, filters, summaries, security events, etc.).

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant HolmesGPT
    participant CiliumCLI as "cilium CLI"
    participant HubbleCLI as "hubble CLI"

    rect rgb(237,245,255)
    Note over HolmesGPT: cilium/core — diagnostics, status, sysdump
    end
    rect rgb(237,255,245)
    Note over HolmesGPT: hubble/observability — real-time flow observation, filters, summaries
    end

    User->>HolmesGPT: Ask network troubleshooting / observation
    HolmesGPT->>CiliumCLI: Execute cilium/core commands (status, config, sysdump, connectivity test)
    CiliumCLI-->>HolmesGPT: Return diagnostics/state
    HolmesGPT->>HubbleCLI: Execute hubble/observability commands (observe with filters, summaries)
    HubbleCLI-->>HolmesGPT: Return flow/observability data
    HolmesGPT->>User: Present findings and recommended next commands
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested reviewers

  • aantn
  • arikalon1

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed The title succinctly and accurately summarizes the primary change of this pull request by indicating that the Cilium and Hubble toolsets are being added, which aligns directly with the changes in documentation and YAML toolset declarations.
Description Check ✅ Passed The description clearly states the intent to add Cilium and Hubble tools for network diagnostics and is directly relevant to the changes in this pull request.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (4)
docs/data-sources/builtin-toolsets/index.md (1)

14-14: Verify that the simple-cilium icon actually exists in the docs icon set

If the slug is missing the rendered page will show a broken glyph. Double-check the icon pack or switch to a material-design icon that is guaranteed to be present.

docs/data-sources/builtin-toolsets/cilium.md (2)

58-69: markdownlint MD046 – fenced block style

The linter expects indented code blocks inside list items/admonitions. Convert this fenced block to an indented block or prepend it with the correct indentation/fence marker (~~~yaml) to silence the warning.


149-168: Bold text used as pseudo-heading triggers MD036

Lines such as **"My pods can't communicate"** are flagged because emphasis is used instead of a heading. Convert them to real headings (e.g. #### My pods can't communicate) to keep the document hierarchy clean.

holmes/plugins/toolsets/cilium.yaml (1)

118-126: Doc/implementation drift – flow count

The docs promise “last 1000 flows”, but the command here uses --last 100. Align the numbers (or make it a configurable parameter) to avoid user confusion.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5fd1a3c and d335d4ab6a8495f53ebb8361fe4dc97c9e81ac2b.

📒 Files selected for processing (4)
  • docs/data-sources/builtin-toolsets/cilium.md (1 hunks)
  • docs/data-sources/builtin-toolsets/index.md (1 hunks)
  • holmes/plugins/toolsets/cilium.yaml (1 hunks)
  • holmes/plugins/toolsets/cilium/cilium.yaml (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
holmes/plugins/toolsets/**/*.yaml

📄 CodeRabbit Inference Engine (CLAUDE.md)

holmes/plugins/toolsets/**/*.yaml: Toolsets: holmes/plugins/toolsets/{name}.yaml or {name}/
Bash toolset validates commands for safety

Files:

  • holmes/plugins/toolsets/cilium.yaml
  • holmes/plugins/toolsets/cilium/cilium.yaml
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
PR: robusta-dev/holmesgpt#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T14:10:31.590Z
Learning: Applies to holmes/plugins/toolsets/**/*.yaml : Toolsets: holmes/plugins/toolsets/{name}.yaml or {name}/
Learnt from: CR
PR: robusta-dev/holmesgpt#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T14:10:31.590Z
Learning: PRs require maintainer approval
Learnt from: nherment
PR: robusta-dev/holmesgpt#408
File: holmes/plugins/toolsets/kubernetes_logs.py:90-97
Timestamp: 2025-05-15T05:13:43.169Z
Learning: In the Kubernetes logs toolset for Holmes, both current and previous logs are intentionally fetched and combined for each pod, even though this requires more API calls. This design ensures all logs are captured even when pods restart but retain their name, providing complete diagnostic information.
📚 Learning: applies to holmes/plugins/toolsets/**/*.yaml : toolsets: holmes/plugins/toolsets/{name}.yaml or {nam...
Learnt from: CR
PR: robusta-dev/holmesgpt#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T14:10:31.590Z
Learning: Applies to holmes/plugins/toolsets/**/*.yaml : Toolsets: holmes/plugins/toolsets/{name}.yaml or {name}/

Applied to files:

  • docs/data-sources/builtin-toolsets/index.md
  • docs/data-sources/builtin-toolsets/cilium.md
  • holmes/plugins/toolsets/cilium.yaml
  • holmes/plugins/toolsets/cilium/cilium.yaml
📚 Learning: in the kubernetes logs toolset for holmes, both current and previous logs are intentionally fetched ...
Learnt from: nherment
PR: robusta-dev/holmesgpt#408
File: holmes/plugins/toolsets/kubernetes_logs.py:90-97
Timestamp: 2025-05-15T05:13:43.169Z
Learning: In the Kubernetes logs toolset for Holmes, both current and previous logs are intentionally fetched and combined for each pod, even though this requires more API calls. This design ensures all logs are captured even when pods restart but retain their name, providing complete diagnostic information.

Applied to files:

  • docs/data-sources/builtin-toolsets/cilium.md
  • holmes/plugins/toolsets/cilium.yaml
  • holmes/plugins/toolsets/cilium/cilium.yaml
📚 Learning: applies to holmes/plugins/toolsets/**/*.yaml : bash toolset validates commands for safety...
Learnt from: CR
PR: robusta-dev/holmesgpt#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T14:10:31.590Z
Learning: Applies to holmes/plugins/toolsets/**/*.yaml : Bash toolset validates commands for safety

Applied to files:

  • holmes/plugins/toolsets/cilium.yaml
  • holmes/plugins/toolsets/cilium/cilium.yaml
🪛 markdownlint-cli2 (0.17.2)
docs/data-sources/builtin-toolsets/cilium.md

58-58: Code block style
Expected: indented; Actual: fenced

(MD046, code-block-style)


149-149: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


155-155: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


160-160: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


165-165: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)

🔇 Additional comments (2)
holmes/plugins/toolsets/cilium/cilium.yaml (1)

94-99: Possible CLI flag mismatch

cilium endpoint get {{ endpoint_id }} -o jsonpath='{.policy}' – older versions use --jsonpath rather than -o jsonpath. Verify against the Cilium CLI you ship; an invalid flag will cause the tool to fail.

holmes/plugins/toolsets/cilium.yaml (1)

214-218: agent-events sub-command might be invalid

Hubble typically exposes agent events via --type agent-event (singular). Confirm that hubble observe agent-events … works on the target CLI version.

Comment thread holmes/plugins/toolsets/cilium.yaml
Comment thread holmes/plugins/toolsets/cilium/cilium.yaml Outdated
Comment thread holmes/plugins/toolsets/cilium/cilium.yaml Outdated
@matmerr
matmerr force-pushed the matmerr/holmes-cilium branch from d335d4a to 217aaa9 Compare July 31, 2025 22:02

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (3)
docs/data-sources/builtin-toolsets/cilium.md (3)

1-3: Title should reflect both Cilium and Hubble to avoid confusion

The page covers two toolsets (cilium/core and hubble/observability), yet the H1 only mentions Cilium. Consider updating to something like # Cilium & Hubble so users immediately understand both are documented here.


58-69: Code-block style violates project markdown-lint rule MD046

The project expects indented blocks, but the Advanced Configuration YAML uses fenced blocks. Either switch to indented style or disable MD046 for this block.

-```yaml
-toolsets:
-  cilium/core:
-    enabled: true
-    config:
-      timeout: 60  # Command timeout in seconds
-  hubble/observability:
-    enabled: true
-    config:
-      max_flows: 10000  # Maximum flows to observe
-      timeout: 120      # Extended timeout for flow monitoring
-```
+
+    toolsets:
+      cilium/core:
+        enabled: true
+        config:
+          timeout: 60    # Command timeout in seconds
+      hubble/observability:
+        enabled: true
+        config:
+          max_flows: 10000  # Maximum flows to observe
+          timeout: 120      # Extended timeout for flow monitoring

149-169: Use headings instead of emphasized text for troubleshooting scenarios (MD036)

Markdown-lint flags the bold strings used as titles. Convert them to H4 headers for consistency and better anchor links.

-**"My pods can't communicate"**
+#### My pods can't communicate
...
-**"DNS resolution is failing"**
+#### DNS resolution is failing
...
-**"Network policies aren't working"**
+#### Network policies aren't working
...
-**"Load balancing issues"**
+#### Load balancing issues
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d335d4ab6a8495f53ebb8361fe4dc97c9e81ac2b and 217aaa9.

📒 Files selected for processing (3)
  • docs/data-sources/builtin-toolsets/cilium.md (1 hunks)
  • docs/data-sources/builtin-toolsets/index.md (1 hunks)
  • holmes/plugins/toolsets/cilium.yaml (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • docs/data-sources/builtin-toolsets/index.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • holmes/plugins/toolsets/cilium.yaml
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
PR: robusta-dev/holmesgpt#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T14:10:31.590Z
Learning: Applies to holmes/plugins/toolsets/**/*.yaml : Toolsets: holmes/plugins/toolsets/{name}.yaml or {name}/
Learnt from: CR
PR: robusta-dev/holmesgpt#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T14:10:31.590Z
Learning: PRs require maintainer approval
Learnt from: nherment
PR: robusta-dev/holmesgpt#408
File: holmes/plugins/toolsets/kubernetes_logs.py:90-97
Timestamp: 2025-05-15T05:13:43.169Z
Learning: In the Kubernetes logs toolset for Holmes, both current and previous logs are intentionally fetched and combined for each pod, even though this requires more API calls. This design ensures all logs are captured even when pods restart but retain their name, providing complete diagnostic information.
📚 Learning: applies to holmes/plugins/toolsets/**/*.yaml : toolsets: holmes/plugins/toolsets/{name}.yaml or {nam...
Learnt from: CR
PR: robusta-dev/holmesgpt#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T14:10:31.590Z
Learning: Applies to holmes/plugins/toolsets/**/*.yaml : Toolsets: holmes/plugins/toolsets/{name}.yaml or {name}/

Applied to files:

  • docs/data-sources/builtin-toolsets/cilium.md
📚 Learning: in the kubernetes logs toolset for holmes, both current and previous logs are intentionally fetched ...
Learnt from: nherment
PR: robusta-dev/holmesgpt#408
File: holmes/plugins/toolsets/kubernetes_logs.py:90-97
Timestamp: 2025-05-15T05:13:43.169Z
Learning: In the Kubernetes logs toolset for Holmes, both current and previous logs are intentionally fetched and combined for each pod, even though this requires more API calls. This design ensures all logs are captured even when pods restart but retain their name, providing complete diagnostic information.

Applied to files:

  • docs/data-sources/builtin-toolsets/cilium.md
🪛 markdownlint-cli2 (0.17.2)
docs/data-sources/builtin-toolsets/cilium.md

58-58: Code block style
Expected: indented; Actual: fenced

(MD046, code-block-style)


149-149: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


155-155: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


160-160: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


165-165: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)

🔇 Additional comments (1)
docs/data-sources/builtin-toolsets/cilium.md (1)

39-53: Verify include-file paths actually exist

The --8<-- "snippets/toolset_refresh_warning.md" and "snippets/helm_upgrade_command.md" directives will break the build if those snippet files are missing or mis-pathed. Double-check that both snippet files are present under docs/snippets/.

arikalon1
arikalon1 previously approved these changes Aug 1, 2025

@arikalon1 arikalon1 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the contribution @matmerr , looks great!

Left a minor comment

Comment thread holmes/plugins/toolsets/cilium.yaml Outdated
Comment thread holmes/plugins/toolsets/cilium.yaml Outdated
@arikalon1

Copy link
Copy Markdown
Collaborator

@matmerr can you please sign the CLA, so we can merge it?

Comment thread holmes/plugins/toolsets/cilium.yaml Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (2)
holmes/plugins/toolsets/cilium/cilium.yaml (2)

1-6: Duplicate toolset file still present – runtime clash risk

Previous review already pointed out that cilium/core and hubble/observability are defined both here and in holmes/plugins/toolsets/cilium.yaml. Nothing changed, so the loader will still pick one copy nondeterministically and shadow the other. Delete the redundant file or merge the definitions into a single location.


104-112: Hard-coded 30 s timeout re-appears – breaks user configurability

The two monitor commands ignore the config.timeout setting that the rest of the toolset honours. The same fix suggested earlier still applies.

-        command: "timeout 30 cilium monitor"
+        command: "timeout {{ config.timeout | default(30) }} cilium monitor"

-        command: "timeout 30 cilium monitor --verbose"
+        command: "timeout {{ config.timeout | default(30) }} cilium monitor --verbose"
🧹 Nitpick comments (1)
holmes/plugins/toolsets/cilium/cilium.yaml (1)

57-60: Missing timeout on namespace-scoped connectivity test

cilium_connectivity_test_namespace can run several minutes yet lacks a timeout guard unlike the global test. Mirror the pattern for consistency:

-        command: "cilium connectivity test --test-namespace {{ namespace }}"
+        command: "timeout {{ config.timeout | default(300) }} cilium connectivity test --test-namespace {{ namespace }}"
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 217aaa9 and 8bb994a727434fd259e74ca96ac895b1f5a55557.

📒 Files selected for processing (1)
  • holmes/plugins/toolsets/cilium/cilium.yaml (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
holmes/plugins/toolsets/**/*.yaml

📄 CodeRabbit Inference Engine (CLAUDE.md)

Toolsets must be placed in holmes/plugins/toolsets/{name}.yaml or {name}/

Files:

  • holmes/plugins/toolsets/cilium/cilium.yaml
🧠 Learnings (3)
📓 Common learnings
Learnt from: nherment
PR: robusta-dev/holmesgpt#408
File: holmes/plugins/toolsets/kubernetes_logs.py:90-97
Timestamp: 2025-05-15T05:13:43.169Z
Learning: In the Kubernetes logs toolset for Holmes, both current and previous logs are intentionally fetched and combined for each pod, even though this requires more API calls. This design ensures all logs are captured even when pods restart but retain their name, providing complete diagnostic information.
Learnt from: CR
PR: robusta-dev/holmesgpt#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T07:25:36.018Z
Learning: Applies to holmes/plugins/toolsets/**/*.yaml : Toolsets must be placed in holmes/plugins/toolsets/{name}.yaml or {name}/
📚 Learning: applies to holmes/plugins/toolsets/**/*.yaml : toolsets must be placed in holmes/plugins/toolsets/{n...
Learnt from: CR
PR: robusta-dev/holmesgpt#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T07:25:36.018Z
Learning: Applies to holmes/plugins/toolsets/**/*.yaml : Toolsets must be placed in holmes/plugins/toolsets/{name}.yaml or {name}/

Applied to files:

  • holmes/plugins/toolsets/cilium/cilium.yaml
📚 Learning: in the kubernetes logs toolset for holmes, both current and previous logs are intentionally fetched ...
Learnt from: nherment
PR: robusta-dev/holmesgpt#408
File: holmes/plugins/toolsets/kubernetes_logs.py:90-97
Timestamp: 2025-05-15T05:13:43.169Z
Learning: In the Kubernetes logs toolset for Holmes, both current and previous logs are intentionally fetched and combined for each pod, even though this requires more API calls. This design ensures all logs are captured even when pods restart but retain their name, providing complete diagnostic information.

Applied to files:

  • holmes/plugins/toolsets/cilium/cilium.yaml

Comment thread holmes/plugins/toolsets/cilium/cilium.yaml Outdated
@matmerr
matmerr force-pushed the matmerr/holmes-cilium branch 2 times, most recently from 190c90f to d870885 Compare August 4, 2025 22:29

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (1)
holmes/plugins/toolsets/cilium/cilium.yaml (1)

105-112: Inconsistent timeout style in monitor commands

cilium_monitor and cilium_monitor_verbose are the only tools that hard-code 30 instead of using {{ config.timeout }} like the rest of the file. This breaks the single-source-of-truth intention of the config.timeout setting.

-        command: "timeout 30 cilium monitor"
+        command: "timeout {{ config.timeout | default(30) }} cilium monitor"

-        command: "timeout 30 cilium monitor --verbose"
+        command: "timeout {{ config.timeout | default(30) }} cilium monitor --verbose"
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 190c90fc2cf77e9e722fe258f8c1afc3107bcee3 and d870885c7fb7a222684e097ea21233d99ad17609.

📒 Files selected for processing (2)
  • holmes/plugins/toolsets/cilium.yaml (1 hunks)
  • holmes/plugins/toolsets/cilium/cilium.yaml (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • holmes/plugins/toolsets/cilium.yaml
🧰 Additional context used
📓 Path-based instructions (1)
holmes/plugins/toolsets/**/*.yaml

📄 CodeRabbit Inference Engine (CLAUDE.md)

Toolsets must be placed in holmes/plugins/toolsets/{name}.yaml or {name}/

Files:

  • holmes/plugins/toolsets/cilium/cilium.yaml
🧠 Learnings (3)
📓 Common learnings
Learnt from: nherment
PR: robusta-dev/holmesgpt#408
File: holmes/plugins/toolsets/kubernetes_logs.py:90-97
Timestamp: 2025-05-15T05:13:43.169Z
Learning: In the Kubernetes logs toolset for Holmes, both current and previous logs are intentionally fetched and combined for each pod, even though this requires more API calls. This design ensures all logs are captured even when pods restart but retain their name, providing complete diagnostic information.
Learnt from: CR
PR: robusta-dev/holmesgpt#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T07:25:36.018Z
Learning: Applies to holmes/plugins/toolsets/**/*.yaml : Toolsets must be placed in holmes/plugins/toolsets/{name}.yaml or {name}/
📚 Learning: applies to holmes/plugins/toolsets/**/*.yaml : toolsets must be placed in holmes/plugins/toolsets/{n...
Learnt from: CR
PR: robusta-dev/holmesgpt#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T07:25:36.018Z
Learning: Applies to holmes/plugins/toolsets/**/*.yaml : Toolsets must be placed in holmes/plugins/toolsets/{name}.yaml or {name}/

Applied to files:

  • holmes/plugins/toolsets/cilium/cilium.yaml
📚 Learning: in the kubernetes logs toolset for holmes, both current and previous logs are intentionally fetched ...
Learnt from: nherment
PR: robusta-dev/holmesgpt#408
File: holmes/plugins/toolsets/kubernetes_logs.py:90-97
Timestamp: 2025-05-15T05:13:43.169Z
Learning: In the Kubernetes logs toolset for Holmes, both current and previous logs are intentionally fetched and combined for each pod, even though this requires more API calls. This design ensures all logs are captured even when pods restart but retain their name, providing complete diagnostic information.

Applied to files:

  • holmes/plugins/toolsets/cilium/cilium.yaml

Comment thread holmes/plugins/toolsets/cilium/cilium.yaml Outdated
Comment thread holmes/plugins/toolsets/cilium/cilium.yaml Outdated
@matmerr
matmerr force-pushed the matmerr/holmes-cilium branch from d870885 to cd174d3 Compare August 4, 2025 22:38
@matmerr

matmerr commented Aug 12, 2025

Copy link
Copy Markdown
Contributor Author

@arikalon1 looks like build is waiting to be kicked off

@matmerr

matmerr commented Aug 26, 2025

Copy link
Copy Markdown
Contributor Author

@aantn any additional feedback?

@aantn

aantn commented Aug 29, 2025

Copy link
Copy Markdown
Collaborator

Hi @matmerr, sorry on the delay! I was out of office. Will look soon.

Comment thread holmes/plugins/toolsets/cilium.yaml Outdated
Comment thread holmes/plugins/toolsets/cilium.yaml Outdated
Comment thread holmes/plugins/toolsets/cilium.yaml Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

♻️ Duplicate comments (2)
holmes/plugins/toolsets/cilium.yaml (2)

1-6: Duplicate toolset file likely exists (keep only one canonical definition)

Previous review already flagged a duplicate cilium/cilium.yaml. Please remove one to avoid loader conflicts.


144-146: Description says “last 100” but command fetches 1000; also heavy on tokens

Align with 100 to reduce output and match copy.

-        description: "Observe network flows in real-time (last 100 flows)"
-        command: "hubble observe --last 1000"
+        description: "Observe network flows in real-time (last 100 flows)"
+        command: "hubble observe --last 100"
🧹 Nitpick comments (3)
holmes/plugins/toolsets/cilium.yaml (3)

147-150: Bound follow output to protect token budgets

Cap backlog and follow duration.

-        command: "timeout {{ config.timeout | default(30) }} hubble observe --follow"
+        command: "timeout {{ config.timeout | default(120) }} hubble observe --since 1m --follow --output compact"

5-5: Host icons locally to avoid external breakage

External URLs can 404 or change. Prefer hosting under the docs site assets.

Also applies to: 108-108


224-231: Distinguish policy/audit events from generic packet drops

  • holmes/plugins/toolsets/cilium.yaml (224–231): if you mean policy/audit denials, change hubble_observe_security_events to:
    hubble observe --type policy-verdict --verdict DROPPED --last 100
    If you mean generic dropped packets instead, use:
    hubble observe --type drop --last 100 (drop_reason / drop_reason_desc available in JSON)
  • Leave hubble_observe_policy_verdicts as "--type policy-verdict --last 100" for allows+denies (or add --verdict if you want only denies).
    Also applies to lines 177–180.
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 58a7f91 and e919582.

📒 Files selected for processing (1)
  • holmes/plugins/toolsets/cilium.yaml (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
holmes/plugins/toolsets/**/*.yaml

📄 CodeRabbit inference engine (CLAUDE.md)

Define toolsets as YAML files under holmes/plugins/toolsets/{name}.yaml or subdirectories

Files:

  • holmes/plugins/toolsets/cilium.yaml
🧠 Learnings (1)
📚 Learning: 2025-09-08T11:12:28.846Z
Learnt from: CR
PR: robusta-dev/holmesgpt#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-08T11:12:28.846Z
Learning: Applies to holmes/plugins/toolsets/**/*.yaml : Define toolsets as YAML files under holmes/plugins/toolsets/{name}.yaml or subdirectories

Applied to files:

  • holmes/plugins/toolsets/cilium.yaml
🔇 Additional comments (5)
holmes/plugins/toolsets/cilium.yaml (5)

28-30: Good: tagged as CLI-only

Accurately reflects that binaries must exist on the runner.

Also applies to: 133-135


31-34: Good: prerequisite and default timeout on Cilium toolset

Reasonable defaults, matches the “slow test” note.


96-99: No action needed — "cilium context" is a valid cilium-cli subcommand.
Verified against the cilium-cli README and Cilium docs.


215-218: No change required — hubble list nodes is correct.
Verified against Cilium Hubble CLI docs; keep the command as written.


100-104: Guard cilium-cli subcommands by version/availability

Multicast ("cilium multicast list group") was added Aug 2024; BGP commands ("cilium bgp peers"/"cilium bgp routes") landed with the BGP control‑plane around Cilium v1.16 (July 2024); "cilium features status" and "cilium encryption status" are in current cilium-cli releases. Detect cilium-cli version or probe subcommand availability (e.g. "cilium --help" or "cilium version") and skip/soft‑fail with a clear message if unavailable.

Applies to holmes/plugins/toolsets/cilium.yaml — lines 67–85 and 100–104.

Comment thread holmes/plugins/toolsets/cilium.yaml Outdated
Comment thread holmes/plugins/toolsets/cilium.yaml
Comment thread holmes/plugins/toolsets/cilium.yaml
Comment thread holmes/plugins/toolsets/cilium.yaml
mainred
mainred previously approved these changes Sep 25, 2025
@mainred
mainred enabled auto-merge (squash) September 27, 2025 04:44

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
docs/data-sources/builtin-toolsets/cilium.md (1)

149-169: Promote troubleshooting labels to real headings (with blank lines)

The bolded phrases (**"My pods can't communicate"**, etc.) act as section headings, but using emphasis like this violates our markdown lint (MD036) and the project style. Replace them with actual headings (e.g., #### My pods can't communicate) and insert a blank line before the numbered steps so the docs build passes and renders correctly.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a97ef57 and 3900f8f.

📒 Files selected for processing (1)
  • docs/data-sources/builtin-toolsets/cilium.md (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
docs/**/*.md

📄 CodeRabbit inference engine (CLAUDE.md)

In MkDocs docs, always add a blank line between a header/bold text and a following list to render correctly

Files:

  • docs/data-sources/builtin-toolsets/cilium.md
🪛 markdownlint-cli2 (0.18.1)
docs/data-sources/builtin-toolsets/cilium.md

58-58: Code block style
Expected: indented; Actual: fenced

(MD046, code-block-style)


149-149: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


155-155: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


160-160: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


165-165: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: build (3.12)
  • GitHub Check: build (3.11)
  • GitHub Check: build (3.10)

Comment thread docs/data-sources/builtin-toolsets/cilium.md
@mainred
mainred merged commit 188a97d into HolmesGPT:master Oct 1, 2025
7 checks passed
@Sheeproid

Copy link
Copy Markdown
Collaborator

Hi @matmerr , I'm Tomer from the Holmes team.
We are getting more requests lately for network related tooling.

I've sent you an email re. your contribution to learn more - just making sure you got it?
Have a great weekend

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants