packages/system/talos-log-collector/tests/talos-log-collector_test.yaml (1)

88-102: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider adding test coverage for ServiceAccount and CiliumNetworkPolicy.

The suite covers the receive/forward wiring well, but the ServiceAccount (disabled token automount) and CiliumNetworkPolicy (ingress restricted to host entity) are security-relevant templates with no test coverage. A separate test suite or expanded suite-level templates: list would guard these against regression.

💡 Suggested additional test cases

+  - it: "ServiceAccount has token automount disabled"
+    template: templates/serviceaccount.yaml
+    asserts:
+      - equal:
+          path: automountServiceAccountToken
+          value: false
+
+  - it: "CiliumNetworkPolicy restricts ingress to host entity on listen port"
+    template: templates/networkpolicy.yaml
+    asserts:
+      - hasDocuments:
+          count: 1
+      - matchRegex:
+          path: spec.ingresss[0].toPorts[0].ports[0].port
+          pattern: "5170"

🤖 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 `@packages/system/talos-log-collector/tests/talos-log-collector_test.yaml`
around lines 88 - 102, Add test coverage for the security-relevant templates
that are currently untested: the ServiceAccount should assert token automount is
disabled, and the CiliumNetworkPolicy should assert ingress is restricted to the
host entity. Update the talos-log-collector test suite in
talos-log-collector_test.yaml by either expanding the suite-level templates list
or adding a separate suite that targets the ServiceAccount and
CiliumNetworkPolicy templates, using their template names to keep the checks
stable against refactors.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

packages/system/talos-log-collector/templates/daemonset.yaml (1)

35-69: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Consider adding a liveness probe for the system-node-critical DaemonSet.

Without a livenessProbe, Kubernetes cannot detect a hung Vector process. A TCP probe on the listen port is a minimal safeguard; enabling Vector's internal API for an HTTP health check would be more thorough.

♻️ Suggested liveness probe

         args: ["--config", "/etc/vector/vector.yaml"]
+        livenessProbe:
+          tcpSocket:
+            port: talos
+          initialDelaySeconds: 10
+          periodSeconds: 30
         env:

🤖 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 `@packages/system/talos-log-collector/templates/daemonset.yaml` around lines 35
- 69, Add a livenessProbe to the DaemonSet container in the vector template so
Kubernetes can detect a hung Vector process. Use the existing VECTOR listener
port on the vector container (the same listenPort/hostPort target) and wire it
into the containers spec near the current ports/resources settings. If you
choose a more thorough check later, enable Vector’s internal API and switch to
an HTTP health probe, but for now a TCP probe is the minimal fix.

packages/system/talos-log-collector/tests/talos-log-collector_test.yaml (2)

41-51: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Tolerations test only covers NoSchedule; NoExecute is unverified.

The suite comment states "runs on every node (tolerate everything)" but the assertion only checks the NoSchedule toleration. The DaemonSet template also includes a NoExecute toleration. Adding an assertion for it would complete the coverage claim.

♻️ Suggested additional assertion

       - contains:
           path: spec.template.spec.tolerations
           content:
             effect: NoSchedule
             operator: Exists
+      - contains:
+          path: spec.template.spec.tolerations
+          content:
+            effect: NoExecute
+            operator: Exists

🤖 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 `@packages/system/talos-log-collector/tests/talos-log-collector_test.yaml`
around lines 41 - 51, Add an assertion to the “runs on every node and is
node-critical” test verifying that the DaemonSet tolerations also include
effect: NoExecute with operator: Exists, alongside the existing NoSchedule
assertion.

---

20-27: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider adding a digest-pinning assertion for the image.

The regex verifies the first-party repository but doesn't assert that the image is digest-pinned (sha256:). Other system packages in the repo use patterns like vector:.+sha256: to enforce immutability without coupling to a specific digest. A separate assertion would guard against accidental non-pinned tags.

Based on learnings, in the cozystack/cozystack repo it is acceptable and intentional for image-pin assertions to use tag/digest-agnostic regexes that verify digest pinning (e.g., :.+sha256:) rather than a specific full digest value.

♻️ Suggested additional assertion

   - it: "DaemonSet pulls the first-party vector image from the cozystack registry"
     template: templates/daemonset.yaml
     asserts:
       - isKind:
           of: DaemonSet
       - matchRegex:
           path: spec.template.spec.containers[0].image
           pattern: "^ghcr\\.io/cozystack/cozystack/vector:"
+      - matchRegex:
+          path: spec.template.spec.containers[0].image
+          pattern: "sha256:"

🤖 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 `@packages/system/talos-log-collector/tests/talos-log-collector_test.yaml`
around lines 20 - 27, Add a separate matchRegex assertion for
spec.template.spec.containers[0].image in the DaemonSet test, using a pattern
such as `:.+sha256:` to require a digest-pinned image while remaining
independent of the specific digest.

Source: Learnings

packages/system/talos-log-collector/templates/daemonset.yaml (1)

35-69: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Consider adding a liveness probe for hang detection.

No liveness probe is configured. If Vector hangs (e.g., deadlock or resource exhaustion), the pod stays Running but silently stops collecting logs — particularly impactful for a system-node-critical DaemonSet. Enabling Vector's built-in health endpoint and adding an HTTP liveness probe is a small change with significant reliability benefit.

💡 Proposed liveness probe addition

In configmap.yaml, enable the Vector API (bound to loopback so it's not exposed to other pods):

     data_dir: /vector-data
+    api:
+      enabled: true
+      address: 127.0.0.1:52000
     sources:

In daemonset.yaml, add the probe:

         args: ["--config", "/etc/vector/vector.yaml"]
+        livenessProbe:
+          httpGet:
+            path: /health
+            port: 52000
+            host: 127.0.0.1
+          initialDelaySeconds: 10
+          periodSeconds: 30
         env:

🤖 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 `@packages/system/talos-log-collector/templates/daemonset.yaml` around lines 35
- 69, Add Vector’s built-in health API configuration to the chart’s configmap
and add an HTTP liveness probe to the vector container in the DaemonSet,
targeting the loopback API health endpoint and the configured listen port.
Ensure the probe uses appropriate delay, timeout, and failure thresholds without
exposing the API beyond the node.