Skip to content

feat: add categraf collect-config wizard with plugin templates and metric verification - #2224

Merged
jsers merged 3 commits into
mainfrom
optimize-host-collect-0728
Jul 29, 2026
Merged

feat: add categraf collect-config wizard with plugin templates and metric verification#2224
jsers merged 3 commits into
mainfrom
optimize-host-collect-0728

Conversation

@710leo

@710leo 710leo commented Jul 29, 2026

Copy link
Copy Markdown
Member
image image image

Summary by CodeRabbit

  • New Features
    • Added a guided collection setup workflow for selecting components, configuring TOML or form-based settings, generating installation commands, and verifying incoming metrics.
    • Added searchable, categorized collection component selection with localized interface text.
    • Added collection setup access from the host installation flow, including a next step after successful installation.
    • Added templates for numerous database, middleware, web, network, and system monitoring integrations.
  • Documentation
    • Added a script to synchronize collection templates for consistent updates.

@coderabbitai

coderabbitai Bot commented Jul 29, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Adds synchronized collector templates and a host-page collection wizard. The wizard supports component selection, structured or TOML configuration, command generation, installation handoff, localized UI text, and Prometheus-based metric arrival verification.

Changes

Collector setup

Layer / File(s) Summary
Collector template synchronization
public/n9e-collect-templates/*, scripts/sync_collect_templates.sh
Adds collector TOML templates and a script that synchronizes supported upstream templates into the public directory.
Collector catalog and shared contracts
src/pages/hosts/pages/List/CollectSetup/catalog.ts, src/pages/hosts/services.ts, src/pages/hosts/locale/*
Defines catalog metadata, field schemas, template URLs, collection capability metadata, and localized wizard text.
TOML and command generation
src/pages/hosts/pages/List/CollectSetup/buildToml.ts, buildCommand.ts, *.test.ts
Generates escaped TOML and shell commands, including authentication, base64 encoding, validation, omission rules, and tests.
Metric arrival verification
src/pages/hosts/pages/List/CollectSetup/buildArrivalPromql.ts, useMetricArrival.ts, *.test.ts
Builds escaped arrival-detection PromQL and polls selected datasources using baseline-aware detection states.
Collection wizard and host integration
src/pages/hosts/pages/List/CollectSetup/*, src/pages/hosts/pages/List/List.tsx, InstallCategraf/*, components/CommandBlock.tsx
Adds component picking, form/TOML editing, command display, installation handoff, verification UI, and host-page entry points.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant HostList
  participant CollectSetup
  participant Categraf
  participant Prometheus
  HostList->>CollectSetup: open collection setup
  CollectSetup->>Categraf: generate and run collect command
  Categraf-->>CollectSetup: collect command completes
  CollectSetup->>Prometheus: poll configured metric query
  Prometheus-->>CollectSetup: return reporting idents
  CollectSetup-->>HostList: show detected, waiting, or timeout state
Loading

Possibly related PRs

  • n9e/fe#2202: Both changes extend the hosts-side Categraf installation metadata and probing flow.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title accurately summarizes the main change: a Categraf collect-config wizard with templates and metric verification.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch optimize-host-collect-0728

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 14

🧹 Nitpick comments (1)
src/pages/hosts/pages/List/CollectSetup/useMetricArrival.ts (1)

58-64: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Ref mutated during render (react-doctor no-ref-current-in-render).

Assigning datasourcesRef.current/identsRef.current at render time is flagged by the static analyzer. Under this repo's React 17 (no concurrent rendering), render replay/discard isn't a concern, so this is functionally safe today. If you want to future-proof against a later React upgrade, move these assignments into a useEffect/useLayoutEffect instead.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/pages/hosts/pages/List/CollectSetup/useMetricArrival.ts` around lines 58
- 64, Move the render-time assignments to datasourcesRef.current and
identsRef.current into an appropriate useEffect or useLayoutEffect in the
useMetricArrival hook. Keep datasourceKey and identsKey memoization unchanged,
and ensure the polling logic still reads the latest datasource and ident values
through the refs.

Source: Linters/SAST tools

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@public/n9e-collect-templates/elasticsearch.toml`:
- Around line 20-22: Comment out the username and password entries in the
Elasticsearch template so they are not active by default, preventing
authentication with the known password when only servers are configured. Require
users to explicitly uncomment and configure both credential fields.

In `@public/n9e-collect-templates/ethtool.toml`:
- Line 8: Remove the host-specific ens33 and ens38 values from interface_include
in the ethtool template; leave the example commented out or otherwise make
interface selection derive from the target host so collection does not depend on
fixed interface names.

In `@public/n9e-collect-templates/haproxy.toml`:
- Around line 9-10: Update the TLS settings in
public/n9e-collect-templates/haproxy.toml:9-10 by changing the active ssl_verify
default to true or removing it. In
public/n9e-collect-templates/influxdb.toml:29-36,
public/n9e-collect-templates/tomcat.toml:23-30, and
public/n9e-collect-templates/zookeeper.toml:12-19, change the example
insecure_skip_verify values to false.

In `@public/n9e-collect-templates/kafka.toml`:
- Around line 4-6: Update the comment immediately above the active [[instances]]
block to remove the stale instruction about uncommenting it and instead describe
that the plugin is enabled through this active configuration block.

In `@public/n9e-collect-templates/kubernetes.toml`:
- Around line 37-42: Update the Kubernetes template’s insecure_skip_verify
setting to be disabled by default by commenting it out or setting it to false,
while preserving explicit opt-in for users who require skipped TLS verification.

In `@public/n9e-collect-templates/mongodb.toml`:
- Around line 10-15: Remove the active username and password values from the
MongoDB template by commenting out the username and password entries, while
keeping the surrounding configuration guidance intact. Require users to
explicitly provide secure credentials rather than supplying usable defaults.

In `@public/n9e-collect-templates/nfsclient.toml`:
- Around line 7-25: Correct the user-facing template comments and resynchronize
generated templates: in public/n9e-collect-templates/nfsclient.toml lines 7-25
fix “explictly,” “Symantics,” and “mulitple”; in
public/n9e-collect-templates/nginx_upstream_check.toml lines 16-17 change “it's
is not set” to “it is not set”; in public/n9e-collect-templates/nvidia_smi.toml
lines 16-17 change “date collection” to “data collection”; and in
public/n9e-collect-templates/phpfpm.toml lines 4-6 describe PHP-FPM endpoints
rather than Nginx stub_status URIs. Then rerun the template synchronization
script.

In `@public/n9e-collect-templates/nvidia_smi.toml`:
- Around line 8-10: Update the commented nvidia_smi_command example to remove
insecure SSH options that disable host-key verification and discard known hosts.
Require strict host-key checking and configure a secure, persistent known-hosts
database instead.

In `@public/n9e-collect-templates/postgresql.toml`:
- Around line 42-49: Replace the Oracle-specific v$session example query in the
PostgreSQL template’s commented metrics block with a valid PostgreSQL query
using pg_stat_activity, preserving equivalent session status/type counts where
supported, or remove the example block entirely.

In `@public/n9e-collect-templates/tomcat.toml`:
- Around line 23-30: Update the commented insecure_skip_verify example in the
Optional TLS Config section to use false, so the template defaults to secure
certificate validation while retaining the option for an explicit true opt-out.

In `@public/n9e-collect-templates/zookeeper.toml`:
- Around line 12-19: Update the commented insecure_skip_verify example in the
Optional TLS Config section of zookeeper.toml from true to false, so the example
enables certificate validation by default while retaining the explicit opt-out
setting.

In `@scripts/sync_collect_templates.sh`:
- Around line 45-60: Make the synchronization flow in
scripts/sync_collect_templates.sh transactional: build and validate the complete
template set in a temporary staging directory, fail when no expected catalog
files are found or any copy fails, and only then replace OUT_DIR atomically. Do
not remove the existing published templates before validation succeeds; preserve
the excluded-name filtering and count reporting while ensuring failures cannot
leave a partial destination.

In `@src/pages/hosts/pages/List/CollectSetup/buildCommand.ts`:
- Around line 42-59: The collect commands expose reversible TOML credentials
through the --conf-b64 argv argument. Update buildCollectCommand and
buildManualCollectCommand, together with the corresponding script interface, to
transport the encoded configuration through an environment variable, stdin, or a
short-lived file rather than the command line, while preserving command
functionality and avoiding shell history/process-argument exposure.

In `@src/pages/hosts/pages/List/List.tsx`:
- Around line 993-1011: Expose the machines detected by InstallCategraf through
its detectedExtra handoff, preferably by providing the detected idents to the
render callback or an equivalent onDetected callback. In List.tsx, store those
idents when the install completes and pass them as CollectSetup’s defaultIdents
for the next_entry flow instead of the stale selectedIdents, while preserving
the existing close, refresh, and collect-opening behavior.

---

Nitpick comments:
In `@src/pages/hosts/pages/List/CollectSetup/useMetricArrival.ts`:
- Around line 58-64: Move the render-time assignments to datasourcesRef.current
and identsRef.current into an appropriate useEffect or useLayoutEffect in the
useMetricArrival hook. Keep datasourceKey and identsKey memoization unchanged,
and ensure the polling logic still reads the latest datasource and ident values
through the refs.
🪄 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: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 4bdf7603-480a-4bc5-8c55-9e1bf458f7d9

📥 Commits

Reviewing files that changed from the base of the PR and between 33bb3a3 and 8f2d164.

📒 Files selected for processing (78)
  • public/n9e-collect-templates/apache.toml
  • public/n9e-collect-templates/arp_packet.toml
  • public/n9e-collect-templates/bind.toml
  • public/n9e-collect-templates/cadvisor.toml
  • public/n9e-collect-templates/chrony.toml
  • public/n9e-collect-templates/clickhouse.toml
  • public/n9e-collect-templates/consul.toml
  • public/n9e-collect-templates/dns_query.toml
  • public/n9e-collect-templates/docker.toml
  • public/n9e-collect-templates/elasticsearch.toml
  • public/n9e-collect-templates/ethtool.toml
  • public/n9e-collect-templates/exec.toml
  • public/n9e-collect-templates/filecount.toml
  • public/n9e-collect-templates/greenplum.toml
  • public/n9e-collect-templates/haproxy.toml
  • public/n9e-collect-templates/http_response.toml
  • public/n9e-collect-templates/influxdb.toml
  • public/n9e-collect-templates/iptables.toml
  • public/n9e-collect-templates/jenkins.toml
  • public/n9e-collect-templates/kafka.toml
  • public/n9e-collect-templates/keepalived.toml
  • public/n9e-collect-templates/kubernetes.toml
  • public/n9e-collect-templates/ldap.toml
  • public/n9e-collect-templates/logstash.toml
  • public/n9e-collect-templates/mongodb.toml
  • public/n9e-collect-templates/mtail.toml
  • public/n9e-collect-templates/mysql.toml
  • public/n9e-collect-templates/nats.toml
  • public/n9e-collect-templates/net_response.toml
  • public/n9e-collect-templates/netstat_filter.toml
  • public/n9e-collect-templates/nfsclient.toml
  • public/n9e-collect-templates/nginx.toml
  • public/n9e-collect-templates/nginx_upstream_check.toml
  • public/n9e-collect-templates/nsq.toml
  • public/n9e-collect-templates/ntp.toml
  • public/n9e-collect-templates/nvidia_smi.toml
  • public/n9e-collect-templates/phpfpm.toml
  • public/n9e-collect-templates/ping.toml
  • public/n9e-collect-templates/postgresql.toml
  • public/n9e-collect-templates/procstat.toml
  • public/n9e-collect-templates/prometheus.toml
  • public/n9e-collect-templates/rabbitmq.toml
  • public/n9e-collect-templates/redis.toml
  • public/n9e-collect-templates/redis_sentinel.toml
  • public/n9e-collect-templates/rocketmq_offset.toml
  • public/n9e-collect-templates/smart.toml
  • public/n9e-collect-templates/sqlserver.toml
  • public/n9e-collect-templates/supervisor.toml
  • public/n9e-collect-templates/systemd.toml
  • public/n9e-collect-templates/tengine.toml
  • public/n9e-collect-templates/tomcat.toml
  • public/n9e-collect-templates/traffic_server.toml
  • public/n9e-collect-templates/whois.toml
  • public/n9e-collect-templates/x509_cert.toml
  • public/n9e-collect-templates/zookeeper.toml
  • scripts/sync_collect_templates.sh
  • src/App.tsx
  • src/pages/hosts/locale/en_US.ts
  • src/pages/hosts/locale/ja_JP.ts
  • src/pages/hosts/locale/ru_RU.ts
  • src/pages/hosts/locale/zh_CN.ts
  • src/pages/hosts/locale/zh_HK.ts
  • src/pages/hosts/pages/List/CollectSetup/ComponentPicker.tsx
  • src/pages/hosts/pages/List/CollectSetup/buildArrivalPromql.test.ts
  • src/pages/hosts/pages/List/CollectSetup/buildArrivalPromql.ts
  • src/pages/hosts/pages/List/CollectSetup/buildCommand.test.ts
  • src/pages/hosts/pages/List/CollectSetup/buildCommand.ts
  • src/pages/hosts/pages/List/CollectSetup/buildToml.test.ts
  • src/pages/hosts/pages/List/CollectSetup/buildToml.ts
  • src/pages/hosts/pages/List/CollectSetup/catalog.ts
  • src/pages/hosts/pages/List/CollectSetup/index.tsx
  • src/pages/hosts/pages/List/CollectSetup/useMetricArrival.ts
  • src/pages/hosts/pages/List/InstallCategraf/buildCommand.ts
  • src/pages/hosts/pages/List/InstallCategraf/index.tsx
  • src/pages/hosts/pages/List/List.tsx
  • src/pages/hosts/pages/List/components/CommandBlock.tsx
  • src/pages/hosts/services.ts
  • src/pages/hosts/types.ts

Comment on lines +20 to +22
## HTTP Basic Authentication username and password.
username = "elastic"
password = "password"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Do not ship an active known password in the template.

If a user only fills servers, the collector will attempt authentication with the public, predictable password password. Keep both credential fields commented out and require explicit configuration.

Suggested fix
-username = "elastic"
-password = "password"
+# username = "elastic"
+# password = ""
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
## HTTP Basic Authentication username and password.
username = "elastic"
password = "password"
## HTTP Basic Authentication username and password.
# username = "elastic"
# password = ""
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@public/n9e-collect-templates/elasticsearch.toml` around lines 20 - 22,
Comment out the username and password entries in the Elasticsearch template so
they are not active by default, preventing authentication with the known
password when only servers are configured. Require users to explicitly uncomment
and configure both credential fields.

[[instances]]
## List of interfaces to pull metrics for
# interface_include = ["eth0"]
interface_include = ["ens33","ens38"]

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Remove the host-specific active interface list.

ens33 and ens38 are environment-specific names. On other hosts this template will silently collect no interfaces, so keep the example commented or generate the list from the target host.

Suggested fix
-interface_include = ["ens33","ens38"]
+# interface_include = ["ens33", "ens38"]
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
interface_include = ["ens33","ens38"]
# interface_include = ["ens33", "ens38"]
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@public/n9e-collect-templates/ethtool.toml` at line 8, Remove the
host-specific ens33 and ens38 values from interface_include in the ethtool
template; leave the example commented out or otherwise make interface selection
derive from the target host so collection does not depend on fixed interface
names.

Comment on lines +9 to +10
# Flag that enables SSL certificate verification for the scrape URI
ssl_verify = false

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Files:"
git ls-files 'public/n9e-collect-templates/*.toml' | sed -n '1,120p'

echo
echo "Target TLS settings:"
for f in haproxy influxdb tomcat zookeeper; do
  echo "--- ${f}.toml ---"
  sed -n '1,45p' "public/n9e-collect-templates/${f}.toml"
done

echo
echo "Search for disable/ insecure / ssl_verify settings:"
rg -n "ssl_verify|insecure_skip_verify|use_tls|tls_min_version|tls_ca|tls_cert|tls_key" public/n9e-collect-templates -S || true

Repository: n9e/fe

Length of output: 1901


Use secure TLS verification defaults across the collector templates.

  • public/n9e-collect-templates/haproxy.toml: change active ssl_verify = false to true or omit it so TLS is verified by default.
  • public/n9e-collect-templates/influxdb.toml, tomcat.toml, zookeeper.toml: change the example insecure_skip_verify values to false to avoid exposing a skip-certification setup.
📍 Affects 4 files
  • public/n9e-collect-templates/haproxy.toml#L9-L10 (this comment)
  • public/n9e-collect-templates/influxdb.toml#L29-L36
  • public/n9e-collect-templates/tomcat.toml#L23-L30
  • public/n9e-collect-templates/zookeeper.toml#L12-L19
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@public/n9e-collect-templates/haproxy.toml` around lines 9 - 10, Update the
TLS settings in public/n9e-collect-templates/haproxy.toml:9-10 by changing the
active ssl_verify default to true or removing it. In
public/n9e-collect-templates/influxdb.toml:29-36,
public/n9e-collect-templates/tomcat.toml:23-30, and
public/n9e-collect-templates/zookeeper.toml:12-19, change the example
insecure_skip_verify values to false.

Comment on lines +4 to +6
############################################################################
# !!! uncomment [[instances]] to enable this plugin
[[instances]]

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Remove the stale “uncomment instances” instruction.

[[instances]] is already active on Line 6, so Line 5 contradicts the actual template and may make users think the plugin is disabled. Update the comment to describe the active block.

Suggested change
-# !!! uncomment [[instances]] to enable this plugin
+# Configure the active instance below to enable this plugin.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
############################################################################
# !!! uncomment [[instances]] to enable this plugin
[[instances]]
############################################################################
# Configure the active instance below to enable this plugin.
[[instances]]
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@public/n9e-collect-templates/kafka.toml` around lines 4 - 6, Update the
comment immediately above the active [[instances]] block to remove the stale
instruction about uncommenting it and instead describe that the plugin is
enabled through this active configuration block.

Comment on lines +37 to +42
use_tls = true
# tls_ca = "/etc/categraf/ca.pem"
# tls_cert = "/etc/categraf/cert.pem"
# tls_key = "/etc/categraf/key.pem"
## Use TLS but skip chain & host verification
insecure_skip_verify = true No newline at end of file

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Do not disable kubelet TLS verification by default.

insecure_skip_verify = true makes the generated kubelet connection accept untrusted certificates and hostnames. The Categraf client applies this TLS configuration to its HTTP transport, so users who only fill in url are exposed to man-in-the-middle risk. (raw.githubusercontent.com)

Comment this option out or set it to false; users should explicitly opt in when required.

Suggested change
 use_tls = true
 # tls_ca = "/etc/categraf/ca.pem"
 # tls_cert = "/etc/categraf/cert.pem"
 # tls_key = "/etc/categraf/key.pem"
 ## Use TLS but skip chain & host verification
-insecure_skip_verify = true
+# insecure_skip_verify = true
#!/bin/sh
set -eu
curl -fsSL https://raw.githubusercontent.com/flashcatcloud/categraf/main/inputs/kubernetes/kubernetes.go |
  grep -n -A12 'TLSConfig()'
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@public/n9e-collect-templates/kubernetes.toml` around lines 37 - 42, Update
the Kubernetes template’s insecure_skip_verify setting to be disabled by default
by commenting it out or setting it to false, while preserving explicit opt-in
for users who require skipped TLS verification.

Comment on lines +23 to +30
## Optional TLS Config
# use_tls = false
# tls_min_version = "1.2"
# tls_ca = "/etc/categraf/ca.pem"
# tls_cert = "/etc/categraf/cert.pem"
# tls_key = "/etc/categraf/key.pem"
## Use TLS but skip chain & host verification
# insecure_skip_verify = true

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "## Locate tomcat.toml"
fd -a 'tomcat\.toml$' . || true

echo
echo "## Relevant snippet"
if [ -f public/n9e-collect-templates/tomcat.toml ]; then
  nl -ba public/n9e-collect-templates/tomcat.toml | sed -n '1,80p'
fi

echo
echo "## Similar insecure_skip_verify examples"
rg -n "insecure_skip_verify.*true|use_tls\s*=|tls_" -S . --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**' | head -200

Repository: n9e/fe

Length of output: 1901


Use secure TLS verification in the example.

Change the commented insecure_skip_verify example to false; disabling certificate validation should be an explicit opt-out, not the copy-paste value.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@public/n9e-collect-templates/tomcat.toml` around lines 23 - 30, Update the
commented insecure_skip_verify example in the Optional TLS Config section to use
false, so the template defaults to secure certificate validation while retaining
the option for an explicit true opt-out.

Comment on lines +12 to +19
## Optional TLS Config
# use_tls = false
# tls_min_version = "1.2"
# tls_ca = "/etc/categraf/ca.pem"
# tls_cert = "/etc/categraf/cert.pem"
# tls_key = "/etc/categraf/key.pem"
## Use TLS but skip chain & host verification
# insecure_skip_verify = true No newline at end of file

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== locate file =="
fd -a 'zookeeper\.toml$' . || true

echo "== locate TLS settings in zookeeper templates =="
if [ -f public/n9e-collect-templates/zookeeper.toml ]; then
  nl -ba public/n9e-collect-templates/zookeeper.toml | sed -n '1,80p'
fi

echo "== search insecure_skip_verify occurrences =="
rg -n --hidden --glob '!node_modules' --glob '!dist' --glob '!build' 'insecure_skip_verify|use_tls|tls_ca|tls_cert|tls_key' . || true

echo "== git diff stat/name =="
git diff --stat || true
git diff -- public/n9e-collect-templates/zookeeper.toml || true

Repository: n9e/fe

Length of output: 1901


Use secure TLS verification in the example.

Change the commented insecure_skip_verify example to false; disabling certificate validation should be an explicit opt-out.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@public/n9e-collect-templates/zookeeper.toml` around lines 12 - 19, Update the
commented insecure_skip_verify example in the Optional TLS Config section of
zookeeper.toml from true to false, so the example enables certificate validation
by default while retaining the explicit opt-out setting.

Comment on lines +45 to +60
mkdir -p "$OUT_DIR"
# Full resync: a plugin dropped upstream must not linger here as a stale file.
rm -f "$OUT_DIR"/*.toml

count=0
for dir in "$CATEGRAF_DIR"/conf/input.*/; do
name="$(basename "$dir")"
name="${name#input.}"
excluded "$name" && continue
src="$dir$name.toml"
if [ ! -f "$src" ]; then
echo "skip $name: no $name.toml in $dir" >&2
continue
fi
cp "$src" "$OUT_DIR/$name.toml"
count=$((count + 1))

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Make template synchronization transactional and fail on an empty source.

The script deletes the published templates before validating or completing the copy. An empty/incompatible checkout can therefore remove all templates and exit with synced 0, while a later cp failure leaves a partial directory. Since the wizard fetches these files by catalog name, this can cause runtime template-load failures.

Stage the complete output in a temporary directory, validate the expected catalog files, then replace the destination only after success.

Suggested approach
+stage_dir="$(mktemp -d "${OUT_DIR}.staging.XXXXXX")"
+trap 'rm -rf "$stage_dir"' EXIT
-rm -f "$OUT_DIR"/*.toml
...
-cp "$src" "$OUT_DIR/$name.toml"
+cp "$src" "$stage_dir/$name.toml"
...
+if [ "$count" -eq 0 ]; then
+    echo "no templates found in $CATEGRAF_DIR" >&2
+    exit 1
+fi
+rm -rf "$OUT_DIR"
+mv "$stage_dir" "$OUT_DIR"
+trap - EXIT
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
mkdir -p "$OUT_DIR"
# Full resync: a plugin dropped upstream must not linger here as a stale file.
rm -f "$OUT_DIR"/*.toml
count=0
for dir in "$CATEGRAF_DIR"/conf/input.*/; do
name="$(basename "$dir")"
name="${name#input.}"
excluded "$name" && continue
src="$dir$name.toml"
if [ ! -f "$src" ]; then
echo "skip $name: no $name.toml in $dir" >&2
continue
fi
cp "$src" "$OUT_DIR/$name.toml"
count=$((count + 1))
stage_dir="$(mktemp -d "${OUT_DIR}.staging.XXXXXX")"
trap 'rm -rf "$stage_dir"' EXIT
mkdir -p "$OUT_DIR"
# Full resync: a plugin dropped upstream must not linger here as a stale file.
rm -f "$OUT_DIR"/*.toml
count=0
for dir in "$CATEGRAF_DIR"/conf/input.*/; do
name="$(basename "$dir")"
name="${name#input.}"
excluded "$name" && continue
src="$dir$name.toml"
if [ ! -f "$src" ]; then
echo "skip $name: no $name.toml in $dir" >&2
continue
fi
cp "$src" "$stage_dir/$name.toml"
count=$((count + 1))
done
if [ "$count" -eq 0 ]; then
echo "no templates found in $CATEGRAF_DIR" >&2
exit 1
fi
rm -rf "$OUT_DIR"
mv "$stage_dir" "$OUT_DIR"
trap - EXIT
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/sync_collect_templates.sh` around lines 45 - 60, Make the
synchronization flow in scripts/sync_collect_templates.sh transactional: build
and validate the complete template set in a temporary staging directory, fail
when no expected catalog files are found or any copy fails, and only then
replace OUT_DIR atomically. Do not remove the existing published templates
before validation succeeds; preserve the excluded-name filtering and count
reporting while ensuring failures cannot leave a partial destination.

Comment on lines +42 to +59
export function buildCollectCommand(options: BuildCollectCommandOptions): string {
const addr = normalizeServerAddr(options.serverAddr);
if (!addr || !_.trim(options.toml)) return '';
const conf = shellQuote(toBase64(options.toml));
return `curl -sSfL${buildCurlAuth(options)} ${shellQuote(`${addr}${COLLECT_SCRIPT_PATH}`)} | sudo bash -s -- --input ${shellQuote(options.input)} --conf-b64 ${conf}`;
}

/** 不放心直接 pipe 到 bash 的用户,可以先下载审阅再执行 */
export function buildManualCollectCommand(options: BuildCollectCommandOptions): string {
const addr = normalizeServerAddr(options.serverAddr);
if (!addr || !_.trim(options.toml)) return '';
const conf = shellQuote(toBase64(options.toml));
return [
`curl -sSfL${buildCurlAuth(options)} ${shellQuote(`${addr}${COLLECT_SCRIPT_PATH}`)} -o collect-config.sh`,
'less collect-config.sh',
`sudo bash collect-config.sh --input ${shellQuote(options.input)} --conf-b64 ${conf}`,
].join('\n');
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift

Base64-encoded TOML (with credentials) is embedded in argv — visible via ps//proc/shell history.

buildCollectCommand and buildManualCollectCommand both pass the full plugin config as --conf-b64 <payload> on the command line. Base64 is reversible in one step, so this doesn't achieve the "passwords aren't nakedly in the command" goal stated in the comment at Line 9 — any local user (or anything scraping ps aux//proc/<pid>/cmdline while the install runs) can recover DB/service credentials that may be present in the TOML (mysql/redis/postgresql passwords, kafka sasl_password, tomcat/rabbitmq passwords, etc., per catalog.ts).

Consider transporting the config via an env var (not shown by default ps) or having the script read it from stdin/a short-lived file instead of argv.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/pages/hosts/pages/List/CollectSetup/buildCommand.ts` around lines 42 -
59, The collect commands expose reversible TOML credentials through the
--conf-b64 argv argument. Update buildCollectCommand and
buildManualCollectCommand, together with the corresponding script interface, to
transport the encoded configuration through an environment variable, stdin, or a
short-lived file rather than the command line, while preserving command
functionality and avoiding shell history/process-argument exposure.

Comment on lines +993 to +1011
detectedExtra={
installMeta.collect ? (
<Button
size='small'
type='primary'
onClick={() => {
// 承接安装引导:机器上报后顺手进入采集配置,安装弹窗关闭时正常走刷新逻辑
setInstallVisible(false);
setRefreshFlag(_.uniqueId('refreshFlag_'));
setCollectVisible(true);
}}
>
{t('collect.next_entry')}
</Button>
) : undefined
}
/>
)}
{collectVisible && installMeta?.collect && <CollectSetup meta={installMeta} defaultIdents={selectedIdents} onClose={() => setCollectVisible(false)} />}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

CollectSetup's defaultIdents doesn't carry forward the newly detected machines.

The "next_entry" CTA (Line 999 comment: seamlessly continue from install to collect-config) closes InstallCategraf and opens CollectSetup, but defaultIdents={selectedIdents} at Line 1011 uses the host table's pre-existing checkbox selection — not the machine(s) InstallCategraf's useTargetArrival() just detected (newIdents, which never leaves InstallCategraf). The newly-installed host the user is trying to configure won't be pre-selected in step 3's target picker; the user has to find and select it manually, defeating the stated purpose of this handoff.

Consider exposing the detected idents from InstallCategraf (e.g. change detectedExtra to a render-prop (idents: string[]) => React.ReactNode, or add an onDetected: (idents: string[]) => void callback) so List.tsx can thread them into CollectSetup's defaultIdents instead of selectedIdents.

🐛 Sketch of the fix
 interface Props {
   meta: CategrafInstallMeta;
   /** detected 表示期间确实有新机器上报,父级据此决定是否刷新列表 */
   onClose: (detected: boolean) => void;
   /** 检测到新机器后展示的追加动作(如「下一步:配置采集」CTA) */
-  detectedExtra?: React.ReactNode;
+  detectedExtra?: (newIdents: string[]) => React.ReactNode;
 }
-              description={
-                <>
-                  <div>{t('install.detected_next')}</div>
-                  {detectedExtra ? <div className='mt-2'>{detectedExtra}</div> : null}
-                </>
-              }
+              description={
+                <>
+                  <div>{t('install.detected_next')}</div>
+                  {detectedExtra ? <div className='mt-2'>{detectedExtra(newIdents)}</div> : null}
+                </>
+              }
-          detectedExtra={
-            installMeta.collect ? (
+          detectedExtra={
+            installMeta.collect
+              ? (newIdents) => (
               <Button
                 size='small'
                 type='primary'
                 onClick={() => {
                   setInstallVisible(false);
                   setRefreshFlag(_.uniqueId('refreshFlag_'));
+                  setDetectedIdents(newIdents);
                   setCollectVisible(true);
                 }}
               >
                 {t('collect.next_entry')}
               </Button>
-            ) : undefined
+            )
+              : undefined
           }
-      {collectVisible && installMeta?.collect && <CollectSetup meta={installMeta} defaultIdents={selectedIdents} onClose={() => setCollectVisible(false)} />}
+      {collectVisible && installMeta?.collect && (
+        <CollectSetup meta={installMeta} defaultIdents={detectedIdents.length > 0 ? detectedIdents : selectedIdents} onClose={() => setCollectVisible(false)} />
+      )}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/pages/hosts/pages/List/List.tsx` around lines 993 - 1011, Expose the
machines detected by InstallCategraf through its detectedExtra handoff,
preferably by providing the detected idents to the render callback or an
equivalent onDetected callback. In List.tsx, store those idents when the install
completes and pass them as CollectSetup’s defaultIdents for the next_entry flow
instead of the stale selectedIdents, while preserving the existing close,
refresh, and collect-opening behavior.

@jsers
jsers merged commit 5509efa into main Jul 29, 2026
1 check passed
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.

2 participants