Add monitoring in podman deployment

[AlphaOne1]

Summary by CodeRabbit

• New Features

  • Optional full monitoring stack (Prometheus + Grafana) and a metrics endpoint
  • Integrated OpenTelemetry-based telemetry support (metrics/traces/logs); trace-endpoint flag marked deprecated

• Configuration

  • podman Kubernetes support flag
  • Conditional ServiceAccount creation and pod annotations
  • Service ports updated (primary: 8080, metrics: 8081)
  • Exposed image pullPolicy/tag and default resource requests
  • Chart maintainers field added

• Chores

  • Updated dependencies and Release 1.6.2 changelog entry
  • Standardized SPDX header handling in templates
  • Enabled weekly Helm updates via Dependabot

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds Release 1.6.2 changelog and dependency bumps; introduces configurable telemetry (centralized OpenTelemetry SDK, exporters, and metrics serving), a TeeLogHandler, Helm chart enhancements (podmanKube flag, monitoring stack, ServiceAccount), named metrics port/service, and multiple SPDX header/year updates.

Changes

Cohort / File(s)	Summary
Release notes CHANGELOG.md	Added Release 1.6.2 entry noting "added monitoring to deployment configuration" and "update dependencies".
Go module & lint config go.mod, .golangci.yaml, go.sum	Bumped Go dependencies and added OpenTelemetry-related modules; updated golangci-lint OTEL exporter/config entries.
Telemetry core (new/updated) instrumentation.go, main.go, main_test.go	Introduced centralized OTEL setup (setupOTelSDK, builders for resource/tracer/meter/logger, env defaults, error sentinel), updated main to initialize telemetry and serve metrics via serveMetrics; deprecates trace-endpoint flag.
Logging logging.go	Added exported TeeLogHandler implementing multi-handler behavior (Enabled, Handle, WithAttrs, WithGroup).
Helm chart — monitoring & SA helm/templates/monitoring.yaml, helm/templates/serviceaccount.yaml, helm/values.yaml, helm/Chart.yaml	New conditional monitoring stack (Prometheus + Grafana) gated by .Values.monitoring.enabled; new ServiceAccount template and values (podmanKube, monitoring, serviceAccount, metricsPort); added chart maintainers.
Helm templates — ports & helpers helm/templates/deployment.yaml, helm/templates/service.yaml, helm/templates/_helpers.tpl	Added named metrics container port and Service port (.Values.service.metricsPort); introduced relName/podmanKube logic for fullname/instance labels; SPDX headers converted to Helm block comments and indentation/formats adjusted.
Helm SPDX/header adjustments helm/*.license, helm/.helmignore, helm/templates/*, helm/templates/*.license, helm/templates/tests/*	Converted or adjusted top-of-file SPDX headers into block comments or updated years; some license files had header removals.
CI / automation / metadata .github/dependabot.yml, .github/workflows/*, .github/CODEOWNERS, .github/FUNDING.yml, AUTHORS.md, README and assorted docs`	Added Dependabot helm updates; updated SPDX years; minor README note (deprecated trace-endpoint); Chart metadata and docs year updates.
Misc — packaging / docker / static files Dockerfile, .dockerignore, .gitignore, nfpm.yaml.tmpl, docker_root/*, testroot/*, man/*.tmpl, *.md, Makefile, etc.	Widespread SPDX year bumps and minor formatting tweaks across many files; no behavioral changes outside telemetry and Helm changes.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant App as main (sonicweb)
    participant SDK as OTEL SDK (setupOTelSDK)
    participant Exporter as OTLP/Console/Prometheus Exporters
    participant Metrics as Metrics HTTP server (serveMetrics)
    participant Scraper as Prometheus (external)

    App->>SDK: call setupOTelSDK(ctx)
    SDK->>Exporter: configure/exporter(s) per env (traces/metrics/logs)
    SDK-->>App: returns shutdown func + metricHandler
    App->>Metrics: serveMetrics(metricHandler, enablePprof)
    note right of Metrics: metricHandler may be nil (no exporter) or\nPrometheus handler when prometheus exporter selected
    Scraper->>Metrics: scrape /metrics (if prometheus handler present)
    App->>SDK: on shutdown call shutdown(ctx)

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

🐇 I stitched new metrics into chart and code,

Prom and Grafana find their humble abode,
Telemetry hums, logs split like a seam,
I hop, I watch—observability's dream,
Carrots counted, dashboards aglow, off we go.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Add monitoring in podman deployment' directly reflects the main changes: new monitoring.yaml template with Prometheus/Grafana stack, monitoring configuration in values.yaml, and podmanKube support in templates.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

• 📝 Generate docstrings

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

[coderabbitai]

Actionable comments posted: 5

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3cb373c and c1187a1.

⛔ Files ignored due to path filters (1)

• go.sum is excluded by !**/*.sum

📒 Files selected for processing (15)

• CHANGELOG.md (1 hunks)
• go.mod (2 hunks)
• helm/.helmignore (1 hunks)
• helm/.helmignore.license (0 hunks)
• helm/templates/NOTES.txt (1 hunks)
• helm/templates/NOTES.txt.license (0 hunks)
• helm/templates/_helpers.tpl (3 hunks)
• helm/templates/deployment.yaml (2 hunks)
• helm/templates/hpa.yaml (1 hunks)
• helm/templates/ingress.yaml (1 hunks)
• helm/templates/monitoring.yaml (1 hunks)
• helm/templates/service.yaml (2 hunks)
• helm/templates/serviceaccount.yaml (1 hunks)
• helm/templates/tests/test-connection.yaml (1 hunks)
• helm/values.yaml (4 hunks)

💤 Files with no reviewable changes (2)

• helm/templates/NOTES.txt.license
• helm/.helmignore.license

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2025-09-21T14:10:51.227Z

Learnt from: AlphaOne1
Repo: AlphaOne1/sonicweb PR: 193
File: governance.md:1-4
Timestamp: 2025-09-21T14:10:51.227Z
Learning: In the SonicWeb repository, AlphaOne1 prefers to maintain original author attribution in SPDX headers even when it differs from the standardized format used elsewhere in the repo, particularly for governance and foundational documents.

Applied to files:

• helm/.helmignore

🪛 YAMLlint (1.37.1)

helm/templates/service.yaml

[error] 1-1: syntax error: expected the node content, but found '-'

(syntax)

helm/templates/monitoring.yaml

[error] 1-1: syntax error: expected the node content, but found '-'

(syntax)

helm/templates/ingress.yaml

[error] 1-1: syntax error: expected the node content, but found '-'

(syntax)

helm/templates/tests/test-connection.yaml

[error] 1-1: syntax error: expected the node content, but found '-'

(syntax)

helm/templates/hpa.yaml

[error] 1-1: syntax error: expected the node content, but found '-'

(syntax)

helm/templates/deployment.yaml

[error] 1-1: syntax error: expected the node content, but found '-'

(syntax)

helm/templates/serviceaccount.yaml

[error] 1-1: syntax error: expected the node content, but found '-'

(syntax)

🔇 Additional comments (18)

CHANGELOG.md (1)

4-8: Changelog entries align with PR scope.

The new Release 1.6.2 entries accurately reflect the monitoring feature addition and dependency updates made in this PR. Format and structure are consistent with prior releases.

helm/.helmignore (1)

1-2: SPDX headers correctly added.

Licensing metadata is properly formatted and positioned. No functional changes to ignore patterns.

helm/templates/NOTES.txt (1)

1-4: Template-block header conversion is idiomatic Helm syntax.

The SPDX header conversion from plain comments to {{- /* ... */ -}} is the correct approach for Helm template files. This syntax is valid and will render properly. The YAMLlint error about line 1 is a false positive due to pre-template-processing YAML validation.

helm/templates/service.yaml (2)

1-4: Template-block header conversion is valid.

Same idiomatic pattern as other Helm templates. YAMLlint error is a false positive.

---

19-22: No issues found - metricsPort is properly defined and consistently configured.

The metricsPort value is defined in helm/values.yaml (line 72: 8081) and correctly referenced across all templates:

• Service exposes it as a port (service.yaml:19)
• Deployment exposes it as a containerPort (deployment.yaml:49)
• Monitoring scrapes the same port (monitoring.yaml:73)

All configurations are consistent and properly aligned.

helm/templates/tests/test-connection.yaml (1)

1-4: Template header conversion is consistent across files.

Valid Helm template block syntax. YAMLlint error is a false positive.

helm/templates/ingress.yaml (1)

1-4: Header conversion is consistent and valid.

Template block syntax is idiomatic. YAMLlint false positive at line 1 should be ignored.

helm/templates/deployment.yaml (2)

1-4: Template header conversion is valid.

Idiomatic Helm template block syntax. YAMLlint false positive should be ignored.

---

48-50: Metrics port configuration is consistent with service template.

Port name, value reference, and configuration align with the metrics port added to service.yaml. Enables dedicated metrics port for monitoring stack integration.

go.mod (1)

46-46: Updated indirect dependencies verified as secure.

Security verification complete via public vulnerability databases:

• prometheus/common v0.67.2: No public CVE or security advisory found; release includes panic fix and linting cleanup
• prometheus/procfs v0.19.2: No publicly reported CVEs or known vulnerabilities
• google.golang.org/genproto v0.0.0-20251029180050-ab9386a59fda: No public CVE or vendor security advisory for this pseudo-version

The indirect dependency updates are safe to merge.

helm/values.yaml (3)

11-15: LGTM! Well-structured feature flags.

The addition of podmanKube and includeMonitoring flags with opt-in defaults (false) is a good approach for introducing optional functionality.

---

91-101: LGTM! Reasonable resource requests.

The resource requests (100m CPU, 20Mi memory) are appropriate for a lightweight web service, and the detailed comments provide good guidance for users.

---

69-72: Document breaking port change in release notes.

The change from service.port: 80 to service.port: 8080 is indeed a breaking change for existing Helm deployments. Verify that this change is documented in the CHANGELOG or a migration guide, and ensure users upgrading from prior versions are notified of the port modification so they can update their ingress configurations, load balancers, or networking rules accordingly.

helm/templates/hpa.yaml (1)

1-4: LGTM! Consistent header format.

The SPDX header has been correctly converted to Helm template comment syntax, consistent with other templates in this PR. This prevents the header from appearing in rendered manifests.

Note: The YAMLlint syntax error is a false positive; this is valid Helm template syntax.

helm/templates/_helpers.tpl (2)

22-30: LGTM! Well-implemented podmanKube support.

The conditional logic correctly overrides the release name to "SonicWeb" when podmanKube is enabled, ensuring consistent naming for Podman deployments.

---

58-63: LGTM! Consistent instance labeling.

The selector labels correctly apply the same conditional logic, ensuring that labels match the naming convention when podmanKube is enabled.

helm/templates/monitoring.yaml (2)

39-48: LGTM! Prometheus configuration follows best practices.

The Prometheus deployment uses a pinned version (v2.55.0), appropriate retention settings (7d), and proper volume mounts for configuration and storage.

---

66-73: LGTM! Proper Prometheus scrape configuration.

The Prometheus configuration correctly references the SonicWeb service using the template helper for the fullname and the metrics port from values.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Use specific busybox version and consider alternative for dashboard provisioning.

Two concerns:

1. Using busybox:latest should be replaced with a specific version.
2. Downloading dashboards from grafana.com during init can fail due to network issues or API changes, causing pod startup failures.

Apply this diff to fix the image version:

-                image: busybox:latest
+                image: busybox:1.37.0

Consider these alternatives for dashboard provisioning:

• Pre-package the dashboard JSON in the ConfigMap
• Make dashboard provisioning optional or best-effort (don't fail on download errors)
• Use a more robust init container with curl and error handling

📝 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

	image: busybox:latest
	command:
	- sh
	- -c
	- |
	set -e
	# IDs und Revisions definieren
	# Beispiel: Go Dashboard
	wget -qO - https://grafana.com/api/dashboards/13240/revisions/2/download \
	| sed 's/"${DS_PROMETHEUS}"/"prometheus"/g' \
	> /dashboards/promhttp-go-stats.json
	volumeMounts:
	- name: grafana-dashboards
	mountPath: /dashboards
	image: busybox:1.37.0
	command:
	- sh
	- -c
	- |
	set -e
	# IDs und Revisions definieren
	# Beispiel: Go Dashboard
	wget -qO - https://grafana.com/api/dashboards/13240/revisions/2/download \
	| sed 's/"${DS_PROMETHEUS}"/"prometheus"/g' \
	> /dashboards/promhttp-go-stats.json
	volumeMounts:
	- name: grafana-dashboards
	mountPath: /dashboards

🤖 Prompt for AI Agents

In helm/templates/monitoring.yaml around lines 120-133, the init container uses
image: busybox:latest and directly downloads dashboards which can cause startup
failures; change the image to a specific BusyBox tag (e.g., busybox:1.36.1 or
another vetted version) and avoid relying on a remote download as a blocking
step—either embed the dashboard JSON into a ConfigMap and mount it into
/dashboards, or make the download best-effort (wrap the wget in error-tolerant
logic so the container exits successfully on failure), or replace the init
container with a small image that has curl and retry/backoff/error handling and
exits cleanly if the download fails; implement one of these options and update
the image tag accordingly.

[coderabbitai]

Actionable comments posted: 2

♻️ Duplicate comments (3)

helm/templates/monitoring.yaml (3)

109-109: Pin busybox to a specific version.

Using busybox:latest can lead to unpredictable behavior and deployment failures if the image changes.

(This issue was flagged in a previous review.)

---

120-133: Address dashboard provisioning reliability and busybox image version.

Using busybox:latest is unstable, and downloading dashboards from an external URL as a blocking init step can cause pod startup failures if the API is unavailable or changes.

(These concerns were flagged in a previous review. Consider pre-packaging the dashboard JSON in a ConfigMap or making the download best-effort with error handling.)

---

136-136: Pin Grafana to a specific version.

Using grafana/grafana:latest can lead to unpredictable behavior and breaking changes.

(This issue was flagged in a previous review.)

🧹 Nitpick comments (2)

helm/templates/monitoring.yaml (2)

39-59: Add resource requests and limits to Prometheus container.

Containers without defined resource requests/limits can cause unpredictable behavior and eviction. Define appropriate requests and limits based on your workload.

            containers:
              - name: prometheus
                image: prom/prometheus:v2.55.0
+               resources:
+                 requests:
+                   memory: "512Mi"
+                   cpu: "250m"
+                 limits:
+                   memory: "1Gi"
+                   cpu: "500m"
                args:
                  - --config.file=/etc/prometheus/prometheus.yml

Adjust the memory and CPU values based on your Prometheus scrape load and retention policy.

---

135-151: Add resource requests and limits to Grafana container.

Containers without defined resource requests/limits can cause unpredictable behavior and eviction. Define appropriate requests and limits based on your workload.

            containers:
              - name: grafana
                image: grafana/grafana:latest
+               resources:
+                 requests:
+                   memory: "256Mi"
+                   cpu: "100m"
+                 limits:
+                   memory: "512Mi"
+                   cpu: "250m"
                ports:
                  - containerPort: 3000

Adjust the memory and CPU values based on your Grafana dashboard complexity and expected concurrency.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1ef09fc and 9026849.

⛔ Files ignored due to path filters (1)

• go.sum is excluded by !**/*.sum

📒 Files selected for processing (16)

• .github/dependabot.yml (1 hunks)
• CHANGELOG.md (1 hunks)
• go.mod (2 hunks)
• helm/.helmignore (1 hunks)
• helm/.helmignore.license (0 hunks)
• helm/templates/NOTES.txt (1 hunks)
• helm/templates/NOTES.txt.license (0 hunks)
• helm/templates/_helpers.tpl (3 hunks)
• helm/templates/deployment.yaml (2 hunks)
• helm/templates/hpa.yaml (1 hunks)
• helm/templates/ingress.yaml (1 hunks)
• helm/templates/monitoring.yaml (1 hunks)
• helm/templates/service.yaml (2 hunks)
• helm/templates/serviceaccount.yaml (1 hunks)
• helm/templates/tests/test-connection.yaml (1 hunks)
• helm/values.yaml (4 hunks)

💤 Files with no reviewable changes (2)

• helm/templates/NOTES.txt.license
• helm/.helmignore.license

✅ Files skipped from review due to trivial changes (1)

• helm/.helmignore

🚧 Files skipped from review as they are similar to previous changes (4)

• CHANGELOG.md
• go.mod
• .github/dependabot.yml
• helm/templates/_helpers.tpl

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2025-09-21T14:10:51.227Z

Learnt from: AlphaOne1
Repo: AlphaOne1/sonicweb PR: 193
File: governance.md:1-4
Timestamp: 2025-09-21T14:10:51.227Z
Learning: In the SonicWeb repository, AlphaOne1 prefers to maintain original author attribution in SPDX headers even when it differs from the standardized format used elsewhere in the repo, particularly for governance and foundational documents.

Applied to files:

• helm/templates/NOTES.txt

🪛 YAMLlint (1.37.1)

helm/templates/service.yaml

[error] 1-1: syntax error: expected the node content, but found '-'

(syntax)

helm/templates/deployment.yaml

[error] 1-1: syntax error: expected the node content, but found '-'

(syntax)

helm/templates/hpa.yaml

[error] 1-1: syntax error: expected the node content, but found '-'

(syntax)

helm/templates/ingress.yaml

[error] 1-1: syntax error: expected the node content, but found '-'

(syntax)

helm/templates/monitoring.yaml

[error] 1-1: syntax error: expected the node content, but found '-'

(syntax)

helm/templates/serviceaccount.yaml

[error] 1-1: syntax error: expected the node content, but found '-'

(syntax)

helm/templates/tests/test-connection.yaml

[error] 1-1: syntax error: expected the node content, but found '-'

(syntax)

🔇 Additional comments (12)

helm/templates/NOTES.txt (1)

1-4: SPDX header addition looks good.

The header is properly wrapped in a Helm template block comment and won't be rendered in output manifests.

helm/templates/tests/test-connection.yaml (1)

1-4: Header conversion follows project pattern.

The SPDX header is correctly embedded in a Helm template block comment, consistent with other template files.

helm/templates/service.yaml (1)

19-22: Metrics port definition is correct.

The named metrics port aligns with the corresponding containerPort in deployment.yaml and uses the metricsPort value from values.yaml.

helm/templates/ingress.yaml (1)

1-4: Header change is consistent with other templates.

No functional impact to ingress rendering.

helm/templates/deployment.yaml (1)

48-50: Metrics port correctly exposed in deployment.

The named metrics port aligns with the service.yaml port definition and uses the same metricsPort value.

helm/templates/serviceaccount.yaml (2)

6-18: ServiceAccount template structure is correct.

Conditional creation, helper references, and YAML structure all align with Kubernetes API. Helper names "SonicWeb.serviceAccountName" and "SonicWeb.labels" are correct.

---

1-18: Verify serviceAccount behavior when create is disabled.

The deployment references a ServiceAccount via serviceAccountName: {{ include "SonicWeb.serviceAccountName" . }}, but with serviceAccount.create: false as the default, the ServiceAccount resource won't be created. Verify that:

1. Users who need the ServiceAccount must explicitly set serviceAccount.create: true in their values.
2. When disabled, the deployment falls back to the default service account, which may have insufficient permissions for your use case.

Document this requirement if needed.

helm/templates/hpa.yaml (1)

1-4: Header conversion is consistent.

No functional impact to HPA autoscaling configuration.

helm/values.yaml (4)

11-12: Verify that monitoring stack template exists.

The values add podmanKube and monitoring.enabled flags, but the corresponding template file helm/templates/monitoring.yaml is not provided in this review. The PR summary mentions adding a monitoring stack (Prometheus + Grafana), so verify that:

1. The monitoring.yaml template exists and is included in the PR.
2. The template respects the monitoring.enabled flag to conditionally render the monitoring stack.
3. Documentation is updated to explain the new monitoring setup.

Also applies to: 14-17

---

33-43: ServiceAccount configuration requires explicit opt-in.

With serviceAccount.create: false as the default, users who need a custom ServiceAccount must explicitly enable it. Ensure:

1. Documentation clearly states that users should set serviceAccount.create: true if they need RBAC or custom service account behavior.
2. The default behavior (using the namespace's default service account) is acceptable for your use case.

The automount default of true is reasonable and can be overridden as needed.

---

93-103: Resource requests are now defined.

The addition of defined resource requests (cpu: 100m, memory: 20Mi) is good practice and helps with Kubernetes scheduling and resource management. Limits remain commented out, which is reasonable for a flexible deployment.

---

19-26: Image and ingress configuration improvements look good.

The explicit pullPolicy: IfNotPresent and tag fields provide better control over image management. The expanded ingress configuration with hosts and TLS structure enables easier ingress setup.

Also applies to: 77-91

[coderabbitai]

Actionable comments posted: 1

♻️ Duplicate comments (4)

helm/templates/monitoring.yaml (4)

107-118: Pin busybox image to a specific version.

The init container uses busybox:latest, which can lead to unpredictable behavior and deployment failures if the image changes. Pin to a specific version for reproducibility and consistency across environments.

             initContainers:
               - name: wait-for-prometheus
-                image: busybox:latest
+                image: busybox:1.37.0
                 command:
                   - sh
                   - -c

---

119-133: Replace busybox:latest and improve dashboard provisioning robustness.

Two concerns:

1. Using busybox:latest should be replaced with a specific version (line 120).
2. Downloading dashboards from grafana.com during init container startup (lines 128–130) can fail due to network issues, API rate limits, or service changes, causing pod startup to block indefinitely or fail.

These failures prevent the pod from becoming ready, degrading service availability.

Fix 1: Pin busybox to a specific version

               - name: fetch-grafana-dashboards
-                image: busybox:latest
+                image: busybox:1.37.0
                 command:

Fix 2: Make dashboard provisioning robust to network failures

Consider these alternatives:

• Option A (Recommended): Pre-package dashboards in ConfigMap
  Embed the dashboard JSON directly in a ConfigMap instead of fetching during init. This eliminates network dependency and pod startup delays.

• Option B: Wrap download in error-tolerant logic
  Add error handling so the init container exits cleanly even if the download fails:

  wget -qO - https://grafana.com/api/dashboards/13240/revisions/2/download \
    | sed 's/"${DS_PROMETHEUS}"/"prometheus"/g' \
    > /dashboards/promhttp-go-stats.json || echo "Warning: Failed to fetch dashboard, continuing..."

• Option C: Make dashboards optional via values
  Add a flag like monitoring.grafana.fetchDashboards to conditionally run this init container only when desired.

Implement one of these approaches to decouple pod startup from remote resource availability.

---

135-139: Pin Grafana image to a specific version.

Using grafana/grafana:latest can lead to unpredictable behavior and breaking changes across Grafana minor/major versions. Pin to a specific version for stability and predictable upgrades.

             containers:
               - name: grafana
-                image: grafana/grafana:latest
+                image: grafana/grafana:11.3.1
                 ports:

Consider moving the Grafana version to values.yaml for easier control:

# values.yaml
monitoring:
  grafana:
    image: grafana/grafana:11.3.1

Then reference it in the template:

-                image: grafana/grafana:latest
+                image: {{ .Values.monitoring.grafana.image }}

---

140-144: Replace hardcoded Grafana credentials with Kubernetes Secrets.

Hardcoding admin credentials directly in the deployment template is a critical security vulnerability. These credentials are visible in version control, cannot be rotated without redeploying, and violate least-privilege access patterns. Move credentials to a Kubernetes Secret instead.

Implementation:

1. Create a Secret template (e.g., helm/templates/monitoring-secret.yaml):

{{ if .Values.monitoring.enabled }}
apiVersion: v1
kind: Secret
metadata:
  name: grafana-admin
  labels:
    app: grafana
type: Opaque
stringData:
  admin-user: {{ .Values.monitoring.grafana.adminUser | quote }}
  admin-password: {{ .Values.monitoring.grafana.adminPassword | quote }}
{{ end }}

2. Add to values.yaml:

monitoring:
  grafana:
    adminUser: admin
    adminPassword: ""  # Leave empty for auto-generation or supply via --set during install

3. Update the Grafana container env section in this file:

                 env:
                   - name: GF_SECURITY_ADMIN_USER
-                    value: admin
+                    valueFrom:
+                      secretKeyRef:
+                        name: grafana-admin
+                        key: admin-user
                   - name: GF_SECURITY_ADMIN_PASSWORD
-                    value: admin
+                    valueFrom:
+                      secretKeyRef:
+                        name: grafana-admin
+                        key: admin-password

This ensures credentials are:

• Never stored in version control
• Rotatable independently of code
• Managed via Kubernetes RBAC
• Can be supplied securely at deploy time via helm install --set monitoring.grafana.adminPassword=<secure-value>

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9026849 and 0238e08.

⛔ Files ignored due to path filters (1)

• go.sum is excluded by !**/*.sum

📒 Files selected for processing (16)

• .github/dependabot.yml (1 hunks)
• CHANGELOG.md (1 hunks)
• go.mod (2 hunks)
• helm/.helmignore (1 hunks)
• helm/.helmignore.license (0 hunks)
• helm/templates/NOTES.txt (1 hunks)
• helm/templates/NOTES.txt.license (0 hunks)
• helm/templates/_helpers.tpl (3 hunks)
• helm/templates/deployment.yaml (2 hunks)
• helm/templates/hpa.yaml (1 hunks)
• helm/templates/ingress.yaml (1 hunks)
• helm/templates/monitoring.yaml (1 hunks)
• helm/templates/service.yaml (2 hunks)
• helm/templates/serviceaccount.yaml (1 hunks)
• helm/templates/tests/test-connection.yaml (1 hunks)
• helm/values.yaml (4 hunks)

💤 Files with no reviewable changes (2)

• helm/.helmignore.license
• helm/templates/NOTES.txt.license

🚧 Files skipped from review as they are similar to previous changes (5)

• go.mod
• CHANGELOG.md
• helm/.helmignore
• helm/templates/NOTES.txt
• helm/templates/_helpers.tpl

🧰 Additional context used

🪛 YAMLlint (1.37.1)

helm/templates/tests/test-connection.yaml

[error] 1-1: syntax error: expected the node content, but found '-'

(syntax)

helm/templates/service.yaml

[error] 1-1: syntax error: expected the node content, but found '-'

(syntax)

helm/templates/deployment.yaml

[error] 1-1: syntax error: expected the node content, but found '-'

(syntax)

helm/templates/hpa.yaml

[error] 1-1: syntax error: expected the node content, but found '-'

(syntax)

helm/templates/ingress.yaml

[error] 1-1: syntax error: expected the node content, but found '-'

(syntax)

helm/templates/monitoring.yaml

[error] 1-1: syntax error: expected the node content, but found '-'

(syntax)

helm/templates/serviceaccount.yaml

[error] 1-1: syntax error: expected the node content, but found '-'

(syntax)

⏰ 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). (2)

• GitHub Check: FuzzTest (stable, ubuntu-latest)
• GitHub Check: Analyze (go)

🔇 Additional comments (14)

helm/templates/tests/test-connection.yaml (1)

1-4: YAMLlint reports a false positive; header wrapping is correct Helm syntax.

Helm template comments ({{- /* */}}) are the recommended way to document template features, and they are not visible in the rendered output. The YAMLlint error is a known limitation when tools don't understand Helm template directives. The syntax is valid and idiomatic Helm.

.github/dependabot.yml (1)

26-29: Adding Helm chart to Dependabot is a good practice.

This enables automated dependency updates for the Helm chart alongside existing Go and Docker ecosystem monitoring.

helm/templates/hpa.yaml (1)

1-4: YAMLlint false positive; Helm template comment syntax is valid.

Same as the test-connection.yaml file—the {{- /* */ -}} syntax is correct and follows Helm best practices.

helm/templates/serviceaccount.yaml (1)

1-18: ServiceAccount template is correctly implemented and addresses previous feedback.

The template uses the correct helper references (SonicWeb.serviceAccountName and SonicWeb.labels), properly handles optional annotations, and correctly sets automountServiceAccountToken. The conditional creation gate aligns with best practices.

helm/templates/ingress.yaml (1)

1-4: YAMLlint false positive; Helm template comment syntax is valid.

Consistent header wrapping across templates. No functional changes to the Ingress resource.

helm/templates/deployment.yaml (1)

48-50: Metrics port addition is properly configured and aligns with monitoring feature.

The new metrics port (name: metrics, containerPort: 8081) correctly exposes the monitoring endpoint and matches the service port definition (targetPort: metrics). The port value is sourced from .Values.service.metricsPort.

helm/templates/service.yaml (1)

19-22: Metrics service port is correctly defined and coordinates with deployment.

The metrics port entry uses proper port naming and indirection (targetPort: metrics references the deployment port name). Configuration aligns with deployment.yaml and values.yaml definitions.

helm/values.yaml (5)

11-17: Feature flags are properly scoped and default to disabled.

The podmanKube and monitoring.enabled flags are well-designed additions that default to false, ensuring backward compatibility while enabling new functionality for opt-in deployment configurations.

---

33-43: ServiceAccount configuration block is complete and aligned with Kubernetes best practices.

The serviceAccount block with create, automount, annotations, and name fields provides flexibility for RBAC setup, with sensible defaults (create: false, automount: true).

---

71-74: Port configuration is intentional and properly aligned across templates.

The service port change (8080) and new metrics port (8081) are consistent with deployment and service template definitions. These addresses a previous breaking change concern and enable the monitoring feature.

---

93-103: Resource constraints are reasonable for development/light production use.

Adding memory/CPU requests (100m/20Mi) improves Kubernetes scheduler behavior and resource predictability. The commented limits remain available for users who need stricter constraints.

---

1-116: monitoring.yaml template correctly gates monitoring resources with conditional.

The helm/templates/monitoring.yaml file exists and properly uses {{ if .Values.monitoring.enabled}} to conditionally deploy the Prometheus Service, Deployment, ConfigMap, Grafana Service, Deployment, and related ConfigMaps. The conditional correctly closes at the end of the file, preventing all monitoring resources from being deployed when monitoring.enabled: false.

Minor note: Consider using {{- if .Values.monitoring.enabled }} (with dash and space for whitespace control) to align with Helm template best practices.

helm/templates/monitoring.yaml (2)

1-6: Resolve YAMLlint syntax error in SPDX header.

YAMLlint reports a syntax error at line 1 regarding the SPDX header comment format. The Helm template comment {{- /* ... */ -}} may interfere with YAML validation. Verify that the header syntax is compatible with your linting configuration, or move the SPDX comment outside the YAML document structure.

---

60-73: No action required. The configuration is correct as written.

.Values.service.metricsPort is already defined in helm/values.yaml (line 74: metricsPort: 8081) and properly exposed in the service template (helm/templates/service.yaml, lines 19-22) with the named port metrics. The Prometheus scrape target correctly references this configuration.

[coderabbitai]

🛠️ Refactor suggestion | 🟠 Major

Add resource requests and limits to Prometheus Deployment.

The Prometheus Deployment is missing CPU and memory resource requests and limits, which can lead to unpredictable pod behavior, eviction, or node pressure under load. Define appropriate limits to ensure stable operation.

Add resource constraints:

             containers:
               - name: prometheus
                 image: prom/prometheus:v2.55.0
                 args:
                   - --config.file=/etc/prometheus/prometheus.yml
                   - --storage.tsdb.path=/prometheus
                   - --storage.tsdb.retention.time=7d
+                resources:
+                  requests:
+                    cpu: 100m
+                    memory: 256Mi
+                  limits:
+                    cpu: 500m
+                    memory: 512Mi
                 ports:

Consider also replacing emptyDir with a persistent volume for production use to prevent metrics loss on pod restart.

📝 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

	apiVersion: apps/v1
	kind: Deployment
	metadata:
	name: prometheus
	labels:
	app: prometheus
	spec:
	replicas: 1
	selector:
	matchLabels:
	app: prometheus
	template:
	metadata:
	labels:
	app: prometheus
	spec:
	containers:
	- name: prometheus
	image: prom/prometheus:v2.55.0
	args:
	- --config.file=/etc/prometheus/prometheus.yml
	- --storage.tsdb.path=/prometheus
	- --storage.tsdb.retention.time=7d
	ports:
	- containerPort: 9090
	name: web
	volumeMounts:
	- name: prometheus-config
	mountPath: /etc/prometheus
	- name: prometheus-storage
	mountPath: /prometheus
	volumes:
	- name: prometheus-config
	configMap:
	name: prometheus-config
	- name: prometheus-storage
	emptyDir: {}
	apiVersion: apps/v1
	kind: Deployment
	metadata:
	name: prometheus
	labels:
	app: prometheus
	spec:
	replicas: 1
	selector:
	matchLabels:
	app: prometheus
	template:
	metadata:
	labels:
	app: prometheus
	spec:
	containers:
	- name: prometheus
	image: prom/prometheus:v2.55.0
	args:
	- --config.file=/etc/prometheus/prometheus.yml
	- --storage.tsdb.path=/prometheus
	- --storage.tsdb.retention.time=7d
	resources:
	requests:
	cpu: 100m
	memory: 256Mi
	limits:
	cpu: 500m
	memory: 512Mi
	ports:
	- containerPort: 9090
	name: web
	volumeMounts:
	- name: prometheus-config
	mountPath: /etc/prometheus
	- name: prometheus-storage
	mountPath: /prometheus
	volumes:
	- name: prometheus-config
	configMap:
	name: prometheus-config
	- name: prometheus-storage
	emptyDir: {}

🤖 Prompt for AI Agents

In helm/templates/monitoring.yaml around lines 23 to 59, the Prometheus
container lacks resource requests and limits; add a resources block under the
prometheus container that sets sensible CPU and memory requests and limits (e.g.
requests to reserve baseline like cpu: "250m", memory: "512Mi" and limits to cap
usage like cpu: "1000m", memory: "2Gi" or values appropriate for your cluster),
ensuring both requests and limits are present to avoid eviction and node
pressure; also replace or provide an alternative to the current emptyDir volume
for prometheus-storage by wiring a persistentVolumeClaim (or note to create a
PVC) for production to prevent metric loss on pod restarts.

[coderabbitai]

Actionable comments posted: 4

♻️ Duplicate comments (1)

helm/templates/monitoring.yaml (1)

39-59: Add resource requests and limits to Prometheus Deployment.

The Prometheus container lacks CPU and memory resource requests and limits, which can lead to unpredictable pod behavior, eviction, or node pressure under load.

Add a resources block:

             containers:
               - name: prometheus
                 image: prom/prometheus:v2.55.0  # after fixing version above
                 args:
                   - --config.file=/etc/prometheus/prometheus.yml
                   - --storage.tsdb.path=/prometheus
                   - --storage.tsdb.retention.time=7d
+                resources:
+                  requests:
+                    cpu: 100m
+                    memory: 256Mi
+                  limits:
+                    cpu: 500m
+                    memory: 512Mi
                 ports:

Additionally, consider replacing emptyDir (line 59) with a PersistentVolumeClaim in production to prevent metric loss on pod restart.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0238e08 and 95f60b0.

📒 Files selected for processing (7)

• helm/Chart.yaml (1 hunks)
• helm/templates/deployment.yaml (2 hunks)
• helm/templates/hpa.yaml (1 hunks)
• helm/templates/ingress.yaml (2 hunks)
• helm/templates/monitoring.yaml (1 hunks)
• helm/templates/service.yaml (1 hunks)
• helm/templates/serviceaccount.yaml (1 hunks)

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2025-09-21T14:10:51.227Z

Learnt from: AlphaOne1
Repo: AlphaOne1/sonicweb PR: 193
File: governance.md:1-4
Timestamp: 2025-09-21T14:10:51.227Z
Learning: In the SonicWeb repository, AlphaOne1 prefers to maintain original author attribution in SPDX headers even when it differs from the standardized format used elsewhere in the repo, particularly for governance and foundational documents.

Applied to files:

• helm/Chart.yaml

🪛 YAMLlint (1.37.1)

helm/templates/service.yaml

[error] 1-1: syntax error: expected the node content, but found '-'

(syntax)

---

[warning] 15-15: too many spaces after hyphen

(hyphens)

---

[warning] 17-17: too many spaces after colon

(colons)

---

[warning] 18-18: too many spaces after colon

(colons)

---

[warning] 19-19: too many spaces after hyphen

(hyphens)

---

[warning] 21-21: too many spaces after colon

(colons)

---

[warning] 22-22: too many spaces after colon

(colons)

helm/templates/hpa.yaml

[error] 1-1: syntax error: expected the node content, but found '-'

(syntax)

---

[warning] 16-16: too many spaces after colon

(colons)

---

[warning] 22-22: too many spaces after hyphen

(hyphens)

helm/templates/monitoring.yaml

[error] 1-1: syntax error: expected the node content, but found '-'

(syntax)

---

[warning] 18-18: too many spaces after hyphen

(hyphens)

---

[warning] 18-18: too many spaces after colon

(colons)

---

[warning] 19-19: too many spaces after colon

(colons)

---

[warning] 21-21: too many spaces after colon

(colons)

---

[warning] 24-24: too many spaces after colon

(colons)

---

[warning] 40-40: too many spaces after hyphen

(hyphens)

---

[warning] 40-40: too many spaces after colon

(colons)

---

[warning] 47-47: too many spaces after hyphen

(hyphens)

---

[warning] 48-48: too many spaces after colon

(colons)

---

[warning] 50-50: too many spaces after hyphen

(hyphens)

---

[warning] 50-50: too many spaces after colon

(colons)

---

[warning] 52-52: too many spaces after hyphen

(hyphens)

---

[warning] 52-52: too many spaces after colon

(colons)

---

[warning] 55-55: too many spaces after hyphen

(hyphens)

---

[warning] 58-58: too many spaces after hyphen

(hyphens)

---

[warning] 62-62: too many spaces after colon

(colons)

---

[warning] 76-76: too many spaces after colon

(colons)

---

[warning] 86-86: too many spaces after hyphen

(hyphens)

---

[warning] 86-86: too many spaces after colon

(colons)

---

[warning] 87-87: too many spaces after colon

(colons)

---

[warning] 89-89: too many spaces after colon

(colons)

---

[warning] 92-92: too many spaces after colon

(colons)

---

[warning] 108-108: too many spaces after hyphen

(hyphens)

---

[warning] 108-108: too many spaces after colon

(colons)

---

[warning] 119-119: too many spaces after hyphen

(hyphens)

---

[warning] 119-119: too many spaces after colon

(colons)

---

[warning] 132-132: too many spaces after hyphen

(hyphens)

---

[warning] 132-132: too many spaces after colon

(colons)

---

[warning] 135-135: too many spaces after hyphen

(hyphens)

---

[warning] 135-135: too many spaces after colon

(colons)

---

[warning] 138-138: too many spaces after hyphen

(hyphens)

---

[warning] 139-139: too many spaces after colon

(colons)

---

[warning] 141-141: too many spaces after hyphen

(hyphens)

---

[warning] 141-141: too many spaces after colon

(colons)

---

[warning] 143-143: too many spaces after hyphen

(hyphens)

---

[warning] 143-143: too many spaces after colon

(colons)

---

[warning] 146-146: too many spaces after hyphen

(hyphens)

---

[warning] 146-146: too many spaces after colon

(colons)

---

[warning] 148-148: too many spaces after hyphen

(hyphens)

---

[warning] 148-148: too many spaces after colon

(colons)

---

[warning] 150-150: too many spaces after hyphen

(hyphens)

---

[warning] 150-150: too many spaces after colon

(colons)

---

[warning] 153-153: too many spaces after hyphen

(hyphens)

---

[warning] 156-156: too many spaces after hyphen

(hyphens)

---

[warning] 159-159: too many spaces after hyphen

(hyphens)

---

[warning] 163-163: too many spaces after colon

(colons)

---

[warning] 180-180: too many spaces after colon

(colons)

helm/templates/deployment.yaml

[error] 1-1: syntax error: expected the node content, but found '-'

(syntax)

helm/templates/ingress.yaml

[error] 1-1: syntax error: expected the node content, but found '-'

(syntax)

helm/templates/serviceaccount.yaml

[error] 1-1: syntax error: expected the node content, but found '-'

(syntax)

⏰ 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). (2)

• GitHub Check: FuzzTest (stable, ubuntu-latest)
• GitHub Check: Analyze (go)

🔇 Additional comments (8)

helm/templates/service.yaml (2)

1-4: YAML linting errors are Helm syntax artifacts and can be ignored.

The error "syntax error: expected the node content, but found '-'" is a YAMLlint false positive for the Helm template comment block {{- /* ... */ -}}, which is valid Helm syntax. YAMLlint cannot parse Helm templating constructs.

---

15-22: Metrics port addition is well-integrated.

The metrics port correctly references {{ .Values.service.metricsPort }} and aligns with the corresponding port definition in deployment.yaml. The port name metrics is appropriate for Prometheus scraping.

helm/templates/ingress.yaml (1)

1-4: Header reformatting is consistent with other templates.

The SPDX header conversion to Helm template block comment is consistent across the Helm templates in this PR.

helm/Chart.yaml (1)

20-22: Maintainers metadata addition is appropriate.

Adding maintainer information to the Helm chart is a best practice and provides proper attribution for chart maintenance. Ensure the contact email is appropriate for your distribution/organization context if this chart is published publicly.

helm/templates/deployment.yaml (1)

44-50: Metrics port integration is correct and consistent.

The metrics port (containerPort 48-50) properly references {{ .Values.service.metricsPort }} and aligns with the service.yaml configuration. Port name "metrics" is appropriate for Prometheus scraping.

helm/templates/serviceaccount.yaml (1)

6-18: ServiceAccount template is correct and well-structured.

The template correctly uses include "SonicWeb.serviceAccountName" and include "SonicWeb.labels" helpers (the past review's concerns about incorrect helper names have been resolved). Conditional rendering on .Values.serviceAccount.create, optional annotations support, and the automountServiceAccountToken configuration are all appropriate.

helm/templates/hpa.yaml (1)

1-4: Header reformatting is consistent.

The SPDX header conversion to Helm template block comment is consistent with other templates in this PR.

helm/templates/monitoring.yaml (1)

41-41: > Likely an incorrect or invalid review comment.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Pin Grafana image to a specific version.

Using grafana/grafana:latest causes non-reproducible deployments and potential breaking changes. Pin to a stable release (e.g., grafana/grafana:11.3.1). Consider moving the image tag to values.yaml to allow version upgrades through chart values.

-                    image: grafana/grafana:latest
+                    image: grafana/grafana:11.3.1

Also applies to: 136-136

🤖 Prompt for AI Agents

In helm/templates/monitoring.yaml around line 136, the container image is using
the non-reproducible tag grafana/grafana:latest; change this to a fixed version
and make the image configurable via values.yaml. Update values.yaml to add a
grafana.image.name (default "grafana/grafana") and grafana.image.tag (default
"11.3.1"), then modify the template to reference those values (use
.Values.grafana.image.name and .Values.grafana.image.tag) so upgrades can be
controlled from chart values and deployments become reproducible.

[coderabbitai]

Actionable comments posted: 3

♻️ Duplicate comments (4)

helm/templates/monitoring.yaml (4)

39-60: Prometheus has no resource requests/limits; add at least requests.
Without requests/limits, the monitoring pods can destabilize small clusters.

---

107-134: Init container relies on external download; can block startup + reduce reliability.
fetch-grafana-dashboards depends on grafana.com at pod start (Line 128-130). Consider embedding the dashboard JSON in a ConfigMap, or make the download best-effort with retry/backoff and non-fatal failure.

---

108-133: Pin BusyBox to an immutable tag/digest (don’t use busybox:stable).
stable is floating, so this is still non-reproducible.

-                    image: busybox:stable
+                    image: busybox:1.37.0

---

135-150: Grafana image is not patch-pinned (grafana/grafana:12.3).
Recommend pinning to a full patch version (or digest) and ideally moving it into values.yaml for controlled upgrades.

🧹 Nitpick comments (1)

helm/templates/service.yaml (1)

14-22: Prefer standard list formatting (- port:) to avoid “too many spaces after hyphen” warnings.
This is cosmetic, but it reduces noise in CI linting.

-    ports:
-        -   port: {{ .Values.service.port }}
+    ports:
+        - port: {{ .Values.service.port }}
             targetPort: http
             protocol: TCP
             name: http
-        -   port: {{ .Values.service.metricsPort }}
+        - port: {{ .Values.service.metricsPort }}
             targetPort: metrics
             protocol: TCP
             name: metrics

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 95f60b0 and b077b3e.

📒 Files selected for processing (7)

• helm/templates/deployment.yaml (2 hunks)
• helm/templates/hpa.yaml (2 hunks)
• helm/templates/ingress.yaml (2 hunks)
• helm/templates/monitoring.yaml (1 hunks)
• helm/templates/service.yaml (2 hunks)
• helm/templates/serviceaccount.yaml (1 hunks)
• helm/values.yaml (4 hunks)

🧰 Additional context used

🪛 YAMLlint (1.37.1)

helm/templates/serviceaccount.yaml

[error] 1-1: syntax error: expected the node content, but found '-'

(syntax)

helm/templates/monitoring.yaml

[error] 1-1: syntax error: expected the node content, but found '-'

(syntax)

---

[warning] 40-40: too many spaces after hyphen

(hyphens)

---

[warning] 47-47: too many spaces after hyphen

(hyphens)

---

[warning] 50-50: too many spaces after hyphen

(hyphens)

---

[warning] 52-52: too many spaces after hyphen

(hyphens)

---

[warning] 55-55: too many spaces after hyphen

(hyphens)

---

[warning] 58-58: too many spaces after hyphen

(hyphens)

---

[warning] 86-86: too many spaces after hyphen

(hyphens)

---

[warning] 108-108: too many spaces after hyphen

(hyphens)

---

[warning] 119-119: too many spaces after hyphen

(hyphens)

---

[warning] 132-132: too many spaces after hyphen

(hyphens)

---

[warning] 135-135: too many spaces after hyphen

(hyphens)

---

[warning] 138-138: too many spaces after hyphen

(hyphens)

---

[warning] 141-141: too many spaces after hyphen

(hyphens)

---

[warning] 146-146: too many spaces after hyphen

(hyphens)

---

[warning] 152-152: too many spaces after hyphen

(hyphens)

---

[warning] 154-154: too many spaces after hyphen

(hyphens)

---

[warning] 156-156: too many spaces after hyphen

(hyphens)

---

[warning] 159-159: too many spaces after hyphen

(hyphens)

---

[warning] 162-162: too many spaces after hyphen

(hyphens)

---

[warning] 165-165: too many spaces after hyphen

(hyphens)

helm/templates/ingress.yaml

[error] 1-1: syntax error: expected the node content, but found '-'

(syntax)

helm/templates/service.yaml

[error] 1-1: syntax error: expected the node content, but found '-'

(syntax)

---

[warning] 19-19: too many spaces after hyphen

(hyphens)

helm/templates/deployment.yaml

[error] 1-1: syntax error: expected the node content, but found '-'

(syntax)

helm/templates/hpa.yaml

[error] 1-1: syntax error: expected the node content, but found '-'

(syntax)

⏰ 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). (1)

• GitHub Check: Analyze (go)

🔇 Additional comments (9)

helm/templates/service.yaml (1)

1-4: YAMLlint “syntax error” at Line 1 is expected for Helm templates; lint rendered output instead.
Tools like yamllint don’t parse {{- ... -}}. Consider running lint on helm template output or configuring yamllint to ignore templates.

helm/templates/ingress.yaml (2)

1-4: YAMLlint “syntax error” at Line 1 is expected for Helm templates; lint rendered output instead.

---

21-46: The template correctly handles nested range directives with proper YAML indentation. Verification with multiple .Values.ingress.tls[*].hosts and .Values.ingress.hosts[*].paths[*] entries confirms the rendered YAML is valid and structurally sound. No indentation issues detected.

helm/templates/deployment.yaml (2)

1-4: YAMLlint “syntax error” at Line 1 is expected for Helm templates; lint rendered output instead.

---

39-60: Metrics port wiring looks consistent (Service targetPort metrics ↔ container port name metrics).
No functional concerns with the added metrics port itself.

helm/templates/serviceaccount.yaml (2)

1-4: YAMLlint “syntax error” at Line 1 is expected for Helm templates; lint rendered output instead.

---

6-18: Conditional ServiceAccount + helper usage looks correct.
SonicWeb.serviceAccountName / SonicWeb.labels usage is consistent, and automountServiceAccountToken is explicitly controlled.

helm/templates/hpa.yaml (2)

1-4: YAMLlint “syntax error” at Line 1 is expected for Helm templates; lint rendered output instead.

---

20-36: Indentation update for metrics list looks correct.
No behavioral change; conditionals remain intact.

[coderabbitai]

Actionable comments posted: 1

♻️ Duplicate comments (1)

helm/templates/monitoring.yaml (1)

107-133: InitContainers: pin BusyBox + don’t block startup on external Grafana.com download.

Two issues called out in prior reviews still apply here:

• busybox:stable is a floating tag (still non-reproducible).
• The dashboard download is a hard dependency (set -e), so transient network / API changes can keep Grafana stuck in Init.

Suggested direction: pin BusyBox to a vetted version and make dashboard provisioning either (a) packaged as a ConfigMap, or (b) best-effort with bounded retries/timeouts and non-fatal failure.

🧹 Nitpick comments (4)

helm/templates/_helpers.tpl (2)

17-30: podmanKube hardcodes release identity → collision risk + case-sensitive contains edge case.

If .Values.podmanKube=true is ever used with Helm (not just podman kube-generated YAML), forcing $relName = "SonicWeb" and hardcoding it into names/labels can make two installs in the same namespace collide. Also contains $name $relName is case-sensitive and runs before lower, so "sonicweb" vs "SonicWeb" can unexpectedly fall into the printf "%s-%s" path and produce duplicated names.

 {{-     $name := default .Chart.Name .Values.nameOverride }}
-{{-     $relName := .Release.Name }}
+{{-     $relName := .Release.Name }}
 {{-     if .Values.podmanKube }}
 {{-         $relName = "SonicWeb"}}
 {{-     end }}
-{{-     if contains $name $relName }}
+{{-     if contains (lower $name) (lower $relName) }}
 {{-         $relName | trunc 63 | trimSuffix "-" | lower }}
 {{-     else }}
 {{-         printf "%s-%s" $relName $name | trunc 63 | trimSuffix "-" | lower }}
 {{-     end }}

---

56-63: Avoid duplicating “podmanKube relName” logic across helpers.

selectorLabels re-encodes the “SonicWeb instance when podmanKube” rule separately from fullname. Consider centralizing a single helper (e.g., "SonicWeb.relName" / "SonicWeb.instance") to keep naming/label behavior consistent.

helm/templates/monitoring.yaml (2)

6-74: Prometheus deployment needs basic operability knobs (resources + storage).

This ships Prometheus with emptyDir (metrics loss on restart) and no resource requests/limits, which can make it noisy/unstable under node pressure.

                 -   name: prometheus
                     image: prom/prometheus:v3.8.0
@@
                     volumeMounts:
@@
                         -   name: prometheus-storage
                             mountPath: /prometheus
+                    resources:
+                        requests:
+                            cpu: 100m
+                            memory: 256Mi
+                        limits:
+                            cpu: 500m
+                            memory: 512Mi
@@
                 -   name: prometheus-storage
                     emptyDir: { }

Consider wiring an optional PVC via values for non-dev installs.

---

135-176: Move Grafana image tag to values + pin to a patch version; enforce/adminPassword handling.

grafana/grafana:12.3 may still float depending on upstream tagging; prefer x.y.z and make it configurable via values. Also, if adminPassword is required, consider making Helm render fail fast via required.

 stringData:
-    admin-user: {{ .Values.monitoring.grafana.adminUser     | quote }}
-    admin-password: {{ .Values.monitoring.grafana.adminPassword | quote }}
+    admin-user: {{ .Values.monitoring.grafana.adminUser | quote }}
+    admin-password: {{ required "monitoring.grafana.adminPassword must be set" .Values.monitoring.grafana.adminPassword | quote }}

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b077b3e and f637c72.

📒 Files selected for processing (3)

• helm/templates/_helpers.tpl (3 hunks)
• helm/templates/monitoring.yaml (1 hunks)
• helm/values.yaml (4 hunks)

🧰 Additional context used

🪛 YAMLlint (1.37.1)

helm/templates/monitoring.yaml

[error] 1-1: syntax error: expected the node content, but found '-'

(syntax)

---

[warning] 40-40: too many spaces after hyphen

(hyphens)

---

[warning] 47-47: too many spaces after hyphen

(hyphens)

---

[warning] 50-50: too many spaces after hyphen

(hyphens)

---

[warning] 52-52: too many spaces after hyphen

(hyphens)

---

[warning] 55-55: too many spaces after hyphen

(hyphens)

---

[warning] 58-58: too many spaces after hyphen

(hyphens)

---

[warning] 86-86: too many spaces after hyphen

(hyphens)

---

[warning] 108-108: too many spaces after hyphen

(hyphens)

---

[warning] 119-119: too many spaces after hyphen

(hyphens)

---

[warning] 132-132: too many spaces after hyphen

(hyphens)

---

[warning] 135-135: too many spaces after hyphen

(hyphens)

---

[warning] 138-138: too many spaces after hyphen

(hyphens)

---

[warning] 141-141: too many spaces after hyphen

(hyphens)

---

[warning] 146-146: too many spaces after hyphen

(hyphens)

---

[warning] 152-152: too many spaces after hyphen

(hyphens)

---

[warning] 154-154: too many spaces after hyphen

(hyphens)

---

[warning] 156-156: too many spaces after hyphen

(hyphens)

---

[warning] 159-159: too many spaces after hyphen

(hyphens)

---

[warning] 162-162: too many spaces after hyphen

(hyphens)

---

[warning] 165-165: too many spaces after hyphen

(hyphens)

🔇 Additional comments (3)

helm/templates/monitoring.yaml (1)

177-211: Provisioning ConfigMaps look coherent (datasource + dashboards provider).

Names are release-scoped via SonicWeb.fullname, and the Prometheus datasource URL matches the Prometheus Service.

helm/values.yaml (2)

36-47: Verify serviceAccount.automount is actually applied in the rendered ServiceAccount/Pod spec.

Values define it, but ensure the templates set automountServiceAccountToken accordingly (either on the ServiceAccount or Pod spec, depending on your intent).

---

96-106: Good default: requests set while leaving limits optional.

This is a reasonable baseline for small clusters and matches the surrounding guidance.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Fix the --set hint for Grafana password (wrong values path).

The values key is monitoring.grafana.adminPassword, but the comment says --set monitoring.adminPassword=..., which will confuse users.

-        adminPassword: "" # empty, _must_ be provided with --set monitoring.adminPassword=<>
+        adminPassword: "" # empty, _must_ be provided with --set monitoring.grafana.adminPassword=<>

🤖 Prompt for AI Agents

In helm/values.yaml around lines 14 to 21, the inline comment advising how to
set the Grafana password uses the wrong key path; update the comment to
reference the correct values path by changing the hint to use --set
monitoring.grafana.adminPassword=<password> (so users set
monitoring.grafana.adminPassword instead of monitoring.adminPassword).

[coderabbitai]

Actionable comments posted: 4

Fix all issues with AI Agents 🤖

In @instrumentation.go:
- Around line 311-313: Remove the unused nolint directive by deleting the
trailing "//nolint:nilnil" comments from the return statements that return three
nils (the branches that check envExporters == OTLPExporterNone or envExporters
== "" and the other similar branches found elsewhere), leaving the returns as
"return nil, nil, nil"; apply the same deletion for the other occurrences of the
"//nolint:nilnil" comment in this file (the other return nil, nil, nil
branches).
- Line 36: Update the OpenTelemetry semantic conventions import in
instrumentation.go to the allowed version by replacing the import path
"go.opentelemetry.io/otel/semconv/v1.37.0" with
"go.opentelemetry.io/otel/semconv/v1.38.0" so it matches the depguard policy;
locate the import statement that references semconv (the line currently
importing semconv "go.opentelemetry.io/otel/semconv/v1.37.0") and change only
the version suffix to v1.38.0.
- Around line 317-319: The environment variable check uses the wrong name;
update the check that reads OTEL_EXPORTER_OTLP_METRIC_PROTOCOL to
OTEL_EXPORTER_OTLP_METRICS_PROTOCOL so the specialized protocol override is
recognized; modify the conditional that assigns to the variable protocol (the
block with specializedProtocol := os.Getenv(...)) to use the plural
OTEL_EXPORTER_OTLP_METRICS_PROTOCOL and keep the rest of the logic unchanged.
- Around line 260-268: The code appends a possibly-nil exp to traceExporters
when an unknown OTEL_TRACES_EXPORTER is specified; update the logic so after
selecting/creating exp you check if exp == nil and return a descriptive error
(e.g., "unknown traces exporter: %v") instead of appending nil, and only append
non-nil exporters to traceExporters; apply the same nil-check-and-error pattern
to newMeterProvider and newLoggerProvider for their respective exporter
variables/slices so unknown exporter types do not result in nil entries.

🧹 Nitpick comments (8)

logging.go (1)

67-78: LGTM: Correct error aggregation and record isolation.

The implementation correctly:

• Clones records before forwarding to prevent handlers from interfering with each other
• Attempts to call all handlers even if some fail
• Aggregates errors using errors.Join, which returns nil when no errors occur

The error slice starts with capacity 0, which is reasonable since errors are the exception case. Pre-allocating len(t.Handler) would optimize for the failure path at the cost of memory in the common success path.

helm/templates/ingress.yaml (1)

24-28: Indentation style is unconventional but valid.

The use of - (dash followed by 3 spaces) before hosts: and the subsequent indentation levels produce valid YAML after template rendering. The secretName will correctly appear as a sibling of hosts under each TLS entry.

Consider standardizing to the more common 2-space indentation pattern for consistency with typical Helm chart conventions, but this is not blocking.

helm/templates/monitoring.yaml (3)

39-59: Add resource requests and limits to Prometheus container.

The Prometheus container lacks resource constraints, which can lead to unpredictable pod behavior or node pressure under load.

🔎 Proposed fix

             containers:
                 -   name: prometheus
                     image: prom/prometheus:v3.8.0
                     args:
                         - --config.file=/etc/prometheus/prometheus.yml
                         - --storage.tsdb.path=/prometheus
                         - --storage.tsdb.retention.time=7d
+                    resources:
+                        requests:
+                            cpu: 100m
+                            memory: 256Mi
+                        limits:
+                            cpu: 500m
+                            memory: 512Mi
                     ports:

Also consider replacing emptyDir (line 59) with a PersistentVolumeClaim for production use to prevent metrics loss on pod restart.

---

119-133: Network-dependent init container may cause pod startup failures.

The init container fetches a dashboard from grafana.com as a blocking step with set -e. Network issues, API changes, or rate limiting will cause startup failures.

Consider making the download best-effort to prevent pod startup failures:

🔎 Proposed fix

                 -   name: fetch-grafana-dashboards
                     image: busybox:stable
                     command:
                         - sh
                         - -c
                         - |
-                            set -e
+                            # Best-effort download - don't fail pod startup on network issues
                             # IDs und Revisions definieren
                             # Beispiel: Go Dashboard
-                            wget -qO - https://grafana.com/api/dashboards/13240/revisions/2/download  \
+                            wget -qO - https://grafana.com/api/dashboards/13240/revisions/2/download 2>/dev/null \
                               | sed 's/"${DS_PROMETHEUS}"/"prometheus"/g' \
-                              > /dashboards/promhttp-go-stats.json
+                              > /dashboards/promhttp-go-stats.json || echo '{}' > /dashboards/promhttp-go-stats.json

Alternatively, pre-package the dashboard JSON in a ConfigMap to eliminate the network dependency entirely.

---

134-139: Consider pinning Grafana to a patch version.

Using grafana/grafana:12.3 is better than latest, but pinning to a specific patch version (e.g., 12.3.0) ensures fully reproducible deployments.

main.go (3)

214-254: High cognitive complexity in telemetry setup block.

The pipeline flags high complexity (nestif: 11) for this block. The deprecation handling logic with nested conditionals contributes to this.

Consider extracting the deprecation handling into a helper function:

🔎 Proposed refactor

// handleDeprecatedTraceEndpoint handles the deprecated trace-endpoint parameter
func handleDeprecatedTraceEndpoint(traceEndpoint string) error {
    if len(traceEndpoint) == 0 {
        return nil
    }

    if value, isSet := os.LookupEnv("OTEL_EXPORTER_OTLP_TRACES_ENDPOINT"); isSet && value != traceEndpoint {
        slog.Warn("deprecated trace-endpoint parameter is set, "+
            "differs from OTEL_EXPORTER_OTLP_TRACES_ENDPOINT, and takes precedence",
            slog.String("trace-endpoint", traceEndpoint),
            slog.String("OTEL_EXPORTER_OTLP_TRACES_ENDPOINT", value))
    }

    if err := os.Setenv("OTEL_EXPORTER_OTLP_TRACES_ENDPOINT", traceEndpoint); err != nil {
        return fmt.Errorf("could not set OTEL_EXPORTER_OTLP_TRACES_ENDPOINT: %w", err)
    }

    slog.Warn("trace-endpoint parameter is deprecated, " +
        "please use environment variable OTEL_EXPORTER_OTLP_TRACES_ENDPOINT instead")
    return nil
}

---

244-250: Extract magic number to a named constant.

The shutdown timeout of 5*time.Second should be a named constant for clarity and maintainability.

🔎 Proposed fix

Add a constant at the top of the file:

// ShutdownTimeout is the timeout for graceful shutdown of telemetry.
const ShutdownTimeout = 5 * time.Second

Then use it:

 defer func() {
-    shutdownCtx, shutdownCancel := context.WithTimeout(context.Background(), 5*time.Second)
+    shutdownCtx, shutdownCancel := context.WithTimeout(context.Background(), ShutdownTimeout)
     defer shutdownCancel()

---

226-239: Use exitFunc instead of os.Exit for consistency.

The file defines exitFunc for testability (line 38), but these error paths use os.Exit(1) directly, making them difficult to test.

🔎 Proposed fix

 if err := os.Setenv("OTEL_EXPORTER_OTLP_TRACES_ENDPOINT", config.TraceEndpoint); err != nil {
     slog.Error("could not set OTEL_EXPORTER_OTLP_TRACES_ENDPOINT",
         slog.String("error", err.Error()))
-    os.Exit(1)
+    exitFunc(1)
 }
 ...
 if err != nil {
     slog.Error("failed to initialize OTEL SDK", slog.String("error", err.Error()))
-    os.Exit(1)
+    exitFunc(1)
 }

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f637c72 and 1dd2432.

⛔ Files ignored due to path filters (3)

• docker_root/favicon.svg is excluded by !**/*.svg
• go.sum is excluded by !**/*.sum
• sonicweb_logo.svg is excluded by !**/*.svg

📒 Files selected for processing (55)

• .dockerignore
• .github/CODEOWNERS
• .github/FUNDING.yml
• .github/dependabot.yml
• .github/workflows/compliance.yml
• .github/workflows/dependency-review.yml
• .github/workflows/release.yml
• .github/workflows/scorecard.yml
• .github/workflows/security.yml
• .github/workflows/test.yml
• .gitignore
• .golangci.yaml
• .markdownlint.json.license
• .vscode/settings.json.license
• AUTHORS.md
• CHANGELOG.md
• CODE_OF_CONDUCT.md
• CONTRIBUTING.md
• Dockerfile
• GOVERNANCE.md
• Makefile
• README.md
• SECURITY.md
• docker_root/apple-touch-icon.png.license
• docker_root/favicon.ico.license
• docker_root/index.html
• go.mod
• go.sum.license
• helm/.helmignore
• helm/Chart.yaml
• helm/templates/NOTES.txt
• helm/templates/_helpers.tpl
• helm/templates/deployment.yaml
• helm/templates/hpa.yaml
• helm/templates/ingress.yaml
• helm/templates/monitoring.yaml
• helm/templates/service.yaml
• helm/templates/serviceaccount.yaml
• helm/templates/tests/test-connection.yaml
• helm/values.yaml
• instrumentation.go
• logging.go
• logo.tmpl.license
• main.go
• main_test.go
• man/sonicweb.1.tmpl
• man/sonicweb_de.1.tmpl
• man/sonicweb_es.1.tmpl
• middlewares.go
• nfpm.yaml.tmpl
• server.go
• testroot/index.html
• testroot/spa.html
• testroot/testHeaders.conf.license
• tls.go

✅ Files skipped from review due to trivial changes (29)

• .github/workflows/test.yml
• SECURITY.md
• main_test.go
• docker_root/index.html
• .github/CODEOWNERS
• man/sonicweb.1.tmpl
• man/sonicweb_es.1.tmpl
• tls.go
• man/sonicweb_de.1.tmpl
• .github/workflows/security.yml
• .github/workflows/compliance.yml
• .dockerignore
• CONTRIBUTING.md
• docker_root/favicon.ico.license
• .github/workflows/dependency-review.yml
• testroot/spa.html
• CODE_OF_CONDUCT.md
• logo.tmpl.license
• .markdownlint.json.license
• Makefile
• testroot/index.html
• GOVERNANCE.md
• .github/FUNDING.yml
• testroot/testHeaders.conf.license
• nfpm.yaml.tmpl
• server.go
• docker_root/apple-touch-icon.png.license
• .gitignore
• .vscode/settings.json.license

🚧 Files skipped from review as they are similar to previous changes (6)

• helm/templates/_helpers.tpl
• helm/templates/NOTES.txt
• go.mod
• helm/.helmignore
• helm/Chart.yaml
• helm/values.yaml

🧰 Additional context used

🧠 Learnings (3)

📚 Learning: 2025-09-21T14:10:51.227Z

Learnt from: AlphaOne1
Repo: AlphaOne1/sonicweb PR: 193
File: governance.md:1-4
Timestamp: 2025-09-21T14:10:51.227Z
Learning: In the SonicWeb repository, AlphaOne1 prefers to maintain original author attribution in SPDX headers even when it differs from the standardized format used elsewhere in the repo, particularly for governance and foundational documents.

Applied to files:

• AUTHORS.md
• Dockerfile
• .github/dependabot.yml
• go.sum.license
• README.md

📚 Learning: 2025-09-21T12:04:15.890Z

Learnt from: AlphaOne1
Repo: AlphaOne1/sonicweb PR: 192
File: .github/workflows/compliance.yml:71-71
Timestamp: 2025-09-21T12:04:15.890Z
Learning: In the SonicWeb project's compliance workflow (.github/workflows/compliance.yml), AlphaOne1 prefers to check ALL commits for DCO compliance without limiting to 200 commits, even on large initial pushes, prioritizing thoroughness over performance.

Applied to files:

• .github/workflows/scorecard.yml
• .github/dependabot.yml

📚 Learning: 2025-09-17T17:34:47.959Z

Learnt from: AlphaOne1
Repo: AlphaOne1/sonicweb PR: 181
File: docker_root/index.html:79-85
Timestamp: 2025-09-17T17:34:47.959Z
Learning: In the sonicweb project, AlphaOne1 controls all Markdown content at build time (like README.md in Docker images), so DOMPurify sanitization is not required for their controlled markdown rendering scenarios. They acknowledge DOMPurify should be used with unknown/untrusted Markdown content.

Applied to files:

• README.md

🪛 GitHub Actions: Security

main.go

[error] 182-182: gocognit: cognitive complexity 32 of func main is high (> 30).

---

[error] 245-245: mnd: Magic number: 5, in detected.

---

[error] 214-214: nestif: If config.EnableTelemetry has complex nested blocks (complexity: 11).

instrumentation.go

[error] 36-36: depguard: import 'go.opentelemetry.io/otel/semconv/v1.37.0' is not allowed from list 'main'.

---

[error] 50-50: funlen: Function 'setupOTelSDK' has too many statements (41 > 40).

---

[error] 306-306: funlen: Function 'newMeterProvider' is too long (62 > 60).

---

[error] 160-160: ireturn: newPropagator returns interface (go.opentelemetry.io/otel/propagation.TextMapPropagator).

---

[error] 312-312: nolintlint: directive //nolint:nilnil is unused for linter "nilnil".

🪛 YAMLlint (1.37.1)

helm/templates/serviceaccount.yaml

[error] 1-1: syntax error: expected the node content, but found '-'

(syntax)

helm/templates/deployment.yaml

[error] 1-1: syntax error: expected the node content, but found '-'

(syntax)

helm/templates/service.yaml

[error] 1-1: syntax error: expected the node content, but found '-'

(syntax)

---

[warning] 19-19: too many spaces after hyphen

(hyphens)

helm/templates/monitoring.yaml

[error] 1-1: syntax error: expected the node content, but found '-'

(syntax)

---

[warning] 40-40: too many spaces after hyphen

(hyphens)

---

[warning] 47-47: too many spaces after hyphen

(hyphens)

---

[warning] 50-50: too many spaces after hyphen

(hyphens)

---

[warning] 52-52: too many spaces after hyphen

(hyphens)

---

[warning] 55-55: too many spaces after hyphen

(hyphens)

---

[warning] 58-58: too many spaces after hyphen

(hyphens)

---

[warning] 86-86: too many spaces after hyphen

(hyphens)

---

[warning] 108-108: too many spaces after hyphen

(hyphens)

---

[warning] 119-119: too many spaces after hyphen

(hyphens)

---

[warning] 132-132: too many spaces after hyphen

(hyphens)

---

[warning] 135-135: too many spaces after hyphen

(hyphens)

---

[warning] 138-138: too many spaces after hyphen

(hyphens)

---

[warning] 141-141: too many spaces after hyphen

(hyphens)

---

[warning] 146-146: too many spaces after hyphen

(hyphens)

---

[warning] 152-152: too many spaces after hyphen

(hyphens)

---

[warning] 154-154: too many spaces after hyphen

(hyphens)

---

[warning] 156-156: too many spaces after hyphen

(hyphens)

---

[warning] 159-159: too many spaces after hyphen

(hyphens)

---

[warning] 162-162: too many spaces after hyphen

(hyphens)

---

[warning] 165-165: too many spaces after hyphen

(hyphens)

helm/templates/hpa.yaml

[error] 1-1: syntax error: expected the node content, but found '-'

(syntax)

helm/templates/ingress.yaml

[error] 1-1: syntax error: expected the node content, but found '-'

(syntax)

helm/templates/tests/test-connection.yaml

[error] 1-1: syntax error: expected the node content, but found '-'

(syntax)

⏰ 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). (1)

• GitHub Check: Analyze (go)

🔇 Additional comments (40)

middlewares.go (1)

1-1: LGTM! Copyright year updated appropriately.

The copyright year update to 2026 is correct and aligns with the project-wide update mentioned in the commit messages.

helm/templates/serviceaccount.yaml (4)

1-4: SPDX header is correct; ignore the YAMLlint error.

The Helm comment syntax {{- /* ... */ -}} is valid for Go templates but not recognized by YAMLlint, resulting in a false positive. The SPDX header and copyright year (2026) are properly formatted.

---

6-12: LGTM! Previous issue resolved.

The template helper references have been correctly updated to use SonicWeb.serviceAccountName and SonicWeb.labels. The conditional rendering pattern and metadata structure follow Helm best practices.

---

13-16: LGTM!

The optional annotations block correctly uses the with guard and proper indentation. This is a standard Helm pattern for conditionally rendering user-provided values.

---

17-17: LGTM!

The automountServiceAccountToken field is correctly positioned at the root level of the ServiceAccount resource and follows standard Helm templating patterns.

.github/dependabot.yml (2)

1-1: LGTM! Copyright year updated appropriately.

The SPDX copyright year update to 2026 aligns with the project-wide update mentioned in the commit messages.

---

26-29: LGTM! Helm dependency updates correctly configured.

The new Helm package-ecosystem block is properly configured with a weekly update schedule, and the /helm directory contains a valid Helm chart structure (Chart.yaml, templates, values.yaml) that Dependabot can process.

logging.go (4)

57-65: LGTM: Correct OR semantics for multi-handler enablement.

The method correctly returns true if any underlying handler is enabled at the specified level, ensuring that records are processed for all interested handlers. The short-circuit evaluation is efficient.

---

80-88: LGTM: Correct attribute propagation with proper immutability.

The method correctly propagates attributes to all underlying handlers and returns a new TeeLogHandler instance, maintaining the immutability pattern expected of slog.Handler implementations. Pre-allocation of the slice with the correct capacity is efficient.

---

90-98: LGTM: Correct group propagation with proper immutability.

The method correctly propagates the group name to all underlying handlers and returns a new TeeLogHandler instance, maintaining the immutability pattern. The implementation mirrors WithAttrs appropriately.

---

53-55: The original review comment is incorrect and should not be applied.

The suggested refactor to unexport the Handler field and add a constructor would break existing code. The codebase initializes TeeLogHandler directly in instrumentation.go:123 using struct literal syntax: &TeeLogHandler{Handler: []slog.Handler{...}}. Making Handler unexported would cause this initialization to fail compilation.

Additionally, the concern about external mutation is not supported by the actual usage patterns in the codebase—the Handler field is only set during initialization and never mutated externally. The methods properly manage handlers within the type's scope.

Likely an incorrect or invalid review comment.

.golangci.yaml (4)

1-1: LGTM! Copyright year updated appropriately.

The copyright year has been updated to 2026, which is consistent with the current date.

---

71-71: LGTM! OpenTelemetry slog bridge integration.

The addition of the otelslog bridge enables integration between OpenTelemetry and Go's structured logging, which aligns with the monitoring objectives of this PR.

---

75-84: No issues found. All OpenTelemetry exporter packages listed in .golangci.yaml (lines 75-84) are correctly specified and available in go.mod with locked versions (spanning v0.15.0 to v1.39.0). The package paths follow standard OpenTelemetry naming conventions.

---

85-91: Update semconv version to v1.37.0 in .golangci.yaml.

The .golangci.yaml file at line 91 references go.opentelemetry.io/otel/semconv/v1.38.0, but v1.38.0 does not exist. The latest available version is v1.37.0, which is already used in the codebase (see instrumentation.go). Update the reference to v1.37.0 to match the actual dependency version.

⛔ Skipped due to learnings

Learnt from: AlphaOne1
Repo: AlphaOne1/sonicweb PR: 176
File: go.mod:46-46
Timestamp: 2025-09-14T02:05:57.237Z
Learning: The go.opentelemetry.io/auto/sdk v1.2.0 dependency causes build errors on 386 architecture. Version v1.1.0 should be used instead to maintain compatibility with 386 builds in multi-arch Docker configurations.

Dockerfile (1)

1-1: LGTM!

Copyright year update to 2026 is appropriate for the new calendar year.

AUTHORS.md (1)

1-1: LGTM!

Copyright year update aligns with the repo-wide year bump to 2026.

go.sum.license (1)

1-1: LGTM!

Copyright year update consistent with other files in this PR.

.github/workflows/scorecard.yml (1)

1-1: LGTM!

Copyright year update. Workflow logic and pinned action versions remain unchanged.

README.md (2)

2-2: LGTM!

Copyright year update to 2026.

---

97-121: LGTM!

Documentation table updates are well-structured:

• Table alignment improves readability
• The -trace-endpoint deprecation notice (line 116) correctly guides users toward the new OTEL_*_ENDPOINT environment variable approach, aligning with the telemetry refactoring in this PR

helm/templates/tests/test-connection.yaml (1)

1-4: LGTM!

The SPDX header is correctly wrapped in Helm template comment syntax ({{- /* ... */ -}}). This will be stripped during template rendering.

The YAMLlint error is a false positive—YAMLlint doesn't parse Helm template directives. The resulting manifest after helm template will be valid YAML.

helm/templates/ingress.yaml (2)

1-4: LGTM!

The SPDX header is correctly wrapped in Helm template comment syntax. The YAMLlint error is a false positive—it doesn't understand Helm template directives.

---

33-45: Ingress rules structure is valid.

The deep indentation in the rules block mirrors the TLS section's style. After template rendering, this produces a valid Kubernetes Ingress spec with properly nested host, http.paths, and backend.service structures.

helm/templates/service.yaml (2)

1-4: LGTM: Header template formatting is correct.

The SPDX header is properly wrapped in a Helm template comment block to prevent it from appearing in rendered manifests. The copyright year 2026 appears to be a project-wide update across all files.

---

15-22: LGTM: Metrics port correctly added.

The metrics port is properly configured with the same structure as the http port. The consistent indentation style (extra spaces after hyphens) aligns both port entries for readability.

Note: YAMLlint's "too many spaces after hyphen" warning on line 19 can be safely ignored—the formatting is intentional and consistent across both port entries.

helm/templates/hpa.yaml (2)

1-4: LGTM: Header formatting consistent with other templates.

The SPDX header follows the same template comment pattern applied across all Helm templates in this PR.

---

22-35: LGTM: Indentation alignment.

The indentation adjustments improve consistency across the metrics blocks without changing functionality.

.github/workflows/release.yml (1)

1-1: LGTM: Copyright year update.

Consistent with the project-wide copyright year update to 2026 across all files.

CHANGELOG.md (2)

1-3: LGTM: Copyright header updated.

Copyright year consistent with updates across the repository.

---

4-9: LGTM: Release entry appropriately documents changes.

The Release 1.6.2 entry clearly summarizes the monitoring additions and dependency updates, following the established changelog format.

helm/templates/deployment.yaml (4)

1-4: LGTM: Header formatting consistent.

SPDX header properly wrapped in Helm template comments, matching the pattern across all templates.

---

39-50: LGTM: Metrics port correctly configured.

The container spec properly exposes both the http and metrics ports. The metrics port configuration uses .Values.service.metricsPort and the port name "metrics" correctly matches the targetPort reference in the Service definition.

---

51-56: LGTM: Probe and resource indentation updated.

The indentation adjustments for probes and resources maintain consistency with the restructured container specification.

---

58-60: LGTM: volumeMounts indentation correct.

The volumeMounts block is properly indented to reflect its position within the container specification, with the 24-space nindent aligning with other container-level configurations.

helm/templates/monitoring.yaml (1)

6-73: LGTM on the Prometheus stack configuration.

The Prometheus Service, Deployment, and ConfigMap use proper fullname scoping with {{ include "SonicWeb.fullname" . }} to avoid naming collisions. The scrape configuration correctly references the SonicWeb service metrics port.

main.go (1)

335-336: LGTM on metrics serving integration.

The serveMetrics goroutine correctly passes the metricHandler (which may be nil if Prometheus exporter is not configured) and the pprof flag.

instrumentation.go (3)

39-46: LGTM on exported constants.

The OTLP protocol and exporter type constants provide clear, reusable identifiers for configuration.

---

452-456: LGTM on serveMetrics nil-safety.

The function correctly handles the case where metricHandler is nil (when Prometheus exporter is not configured) by returning early if both metrics and pprof are disabled.

---

118-129: Logger provider integration looks good.

The dual logging setup via TeeLogHandler correctly sends logs to both the default handler and the OpenTelemetry slog bridge. TeeLogHandler is properly defined in logging.go with all necessary slog.Handler interface methods.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check depguard configuration for semconv
echo "=== Checking .golangci.yml for depguard and semconv rules ==="
rg -n "semconv" .golangci.yml -A 3 -B 3 || echo "No semconv found in .golangci.yml"

echo -e "\n=== Checking go.mod for semconv version ==="
rg -n "semconv" go.mod

echo -e "\n=== Checking depguard section in .golangci.yml ==="
rg -n "depguard" .golangci.yml -A 20 -B 2 || echo "No depguard section found"

Repository: AlphaOne1/sonicweb

Length of output: 405

---

🏁 Script executed:

#!/bin/bash
# Find all config files
echo "=== Finding configuration files ==="
find . -name ".golangci.yml" -o -name ".golangci.yaml" -o -name "golangci.yml" 2>/dev/null | head -20

echo -e "\n=== Checking instrumentation.go ==="
find . -name "instrumentation.go" -type f 2>/dev/null | head -5

# Read go.mod
echo -e "\n=== Reading go.mod (checking for semconv) ==="
if [ -f "go.mod" ]; then
  cat go.mod | grep -i semconv || echo "No semconv import found in go.mod"
else
  echo "go.mod not found"
fi

Repository: AlphaOne1/sonicweb

Length of output: 253

---

🏁 Script executed:

#!/bin/bash
echo "=== Reading .golangci.yaml for depguard rules ==="
cat .golangci.yaml

echo -e "\n=== Reading instrumentation.go around line 36 ==="
head -50 instrumentation.go | cat -n

echo -e "\n=== Searching go.mod for all OpenTelemetry references ==="
grep -i "opentelemetry\|otel" go.mod || echo "No otel refs found in go.mod"

Repository: AlphaOne1/sonicweb

Length of output: 8255

---

Update semconv version from v1.37.0 to v1.38.0.

Line 36 imports semconv/v1.37.0, but the depguard configuration only allows semconv/v1.38.0. Change the import to match the allowed version.

🧰 Tools

🪛 GitHub Actions: Security

[error] 36-36: depguard: import 'go.opentelemetry.io/otel/semconv/v1.37.0' is not allowed from list 'main'.

🤖 Prompt for AI Agents

In @instrumentation.go around line 36, Update the OpenTelemetry semantic
conventions import in instrumentation.go to the allowed version by replacing the
import path "go.opentelemetry.io/otel/semconv/v1.37.0" with
"go.opentelemetry.io/otel/semconv/v1.38.0" so it matches the depguard policy;
locate the import statement that references semconv (the line currently
importing semconv "go.opentelemetry.io/otel/semconv/v1.37.0") and change only
the version suffix to v1.38.0.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Unknown exporter types result in nil being appended to the slice.

If an unknown exporter type is specified in OTEL_TRACES_EXPORTER, the exp variable remains nil and is appended to traceExporters. This will cause issues when creating the tracer provider.

🔎 Proposed fix

+       default:
+           slog.Warn("unsupported trace exporter", slog.String("exporter", exporter))
+           continue
        }

        if err != nil {
            return nil, fmt.Errorf("could not instantiate trace exporter %v with protocol %v: %w",
                exporter, protocol, err)
        }

+       if exp == nil {
+           continue
+       }
+
        traceExporters = append(traceExporters, exp)
    }

Apply the same fix pattern to newMeterProvider (lines 323-354) and newLoggerProvider (lines 409-432).

📝 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

	}
	if err != nil {
	return nil, fmt.Errorf("could not instantiate trace exporter %v with protocol %v: %w",
	exporter, protocol, err)
	}
	traceExporters = append(traceExporters, exp)
	}
	}
	default:
	slog.Warn("unsupported trace exporter", slog.String("exporter", exporter))
	continue
	}
	if err != nil {
	return nil, fmt.Errorf("could not instantiate trace exporter %v with protocol %v: %w",
	exporter, protocol, err)
	}
	if exp == nil {
	continue
	}
	traceExporters = append(traceExporters, exp)
	}

🤖 Prompt for AI Agents

In @instrumentation.go around lines 260-268, The code appends a possibly-nil exp
to traceExporters when an unknown OTEL_TRACES_EXPORTER is specified; update the
logic so after selecting/creating exp you check if exp == nil and return a
descriptive error (e.g., "unknown traces exporter: %v") instead of appending
nil, and only append non-nil exporters to traceExporters; apply the same
nil-check-and-error pattern to newMeterProvider and newLoggerProvider for their
respective exporter variables/slices so unknown exporter types do not result in
nil entries.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Remove unused nolint directive.

The pipeline reports the //nolint:nilnil directive on line 312 is unused, indicating the nilnil linter is not enabled.

🔎 Proposed fix

 if envExporters == OTLPExporterNone || envExporters == "" {
-    return nil, nil, nil //nolint:nilnil // it is completely valid to have no provider set
+    return nil, nil, nil // it is completely valid to have no provider set
 }

Apply the same fix to lines 235 and 400 if the nilnil linter is not enabled in your configuration.

📝 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

	if envExporters == OTLPExporterNone || envExporters == "" {
	return nil, nil, nil //nolint:nilnil // it is completely valid to have no provider set
	}
	if envExporters == OTLPExporterNone || envExporters == "" {
	return nil, nil, nil // it is completely valid to have no provider set
	}

🧰 Tools

🪛 GitHub Actions: Security

[error] 312-312: nolintlint: directive //nolint:nilnil is unused for linter "nilnil".

🤖 Prompt for AI Agents

In @instrumentation.go around lines 311-313, Remove the unused nolint directive
by deleting the trailing "//nolint:nilnil" comments from the return statements
that return three nils (the branches that check envExporters == OTLPExporterNone
or envExporters == "" and the other similar branches found elsewhere), leaving
the returns as "return nil, nil, nil"; apply the same deletion for the other
occurrences of the "//nolint:nilnil" comment in this file (the other return nil,
nil, nil branches).

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

OpenTelemetry OTEL_EXPORTER_OTLP_METRICS_PROTOCOL environment variable specification

💡 Result:

Short specification:

• Name: OTEL_EXPORTER_OTLP_METRICS_PROTOCOL.
• Purpose: selects the OTLP transport protocol used for metrics. [1][2]
• Supported values: "grpc", "http/protobuf", "http/json". [1][2]
• Default: SDK-dependent; the spec recommends "http/protobuf" unless an SDK has a strong reason to use "grpc". [1][2]
• Notes: SDKs SHOULD support both "grpc" and "http/protobuf" and MUST support at least one (if only one is supported it SHOULD be "http/protobuf"). [1]

Sources: OpenTelemetry specification and OTLP exporter docs. [1] (opentelemetry.io)

Citations:

• 1: https://opentelemetry.io/docs/specs/otel/protocol/exporter/?utm_source=openai

---

🏁 Script executed:

# Search for references to OTEL_EXPORTER_OTLP_METRIC in the instrumentation.go file and surrounding context
rg "OTEL_EXPORTER_OTLP_METRIC" -B 3 -A 3

Repository: AlphaOne1/sonicweb

Length of output: 1827

---

🏁 Script executed:

# Check if there are other protocol env vars defined in the codebase for comparison
rg "OTEL_EXPORTER_OTLP.*PROTOCOL" -B 1 -A 1

Repository: AlphaOne1/sonicweb

Length of output: 1628

---

🏁 Script executed:

# Find and read the instrumentation.go file to understand the full context
fd "instrumentation.go" -x wc -l {}

Repository: AlphaOne1/sonicweb

Length of output: 86

---

Typo in environment variable name: METRIC should be METRICS.

The OpenTelemetry specification and this file's own documentation use OTEL_EXPORTER_OTLP_METRICS_PROTOCOL (plural). This matches the pattern used for traces (OTEL_EXPORTER_OTLP_TRACES_PROTOCOL) and logs (OTEL_EXPORTER_OTLP_LOGS_PROTOCOL). The current code using the singular form will cause the specialized protocol override to be ignored.

🔎 Proposed fix

-if specializedProtocol := os.Getenv("OTEL_EXPORTER_OTLP_METRIC_PROTOCOL"); specializedProtocol != "" {
+if specializedProtocol := os.Getenv("OTEL_EXPORTER_OTLP_METRICS_PROTOCOL"); specializedProtocol != "" {
     protocol = specializedProtocol
 }

📝 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

	if specializedProtocol := os.Getenv("OTEL_EXPORTER_OTLP_METRIC_PROTOCOL"); specializedProtocol != "" {
	protocol = specializedProtocol
	}
	if specializedProtocol := os.Getenv("OTEL_EXPORTER_OTLP_METRICS_PROTOCOL"); specializedProtocol != "" {
	protocol = specializedProtocol
	}

🤖 Prompt for AI Agents

In @instrumentation.go around lines 317-319, The environment variable check uses
the wrong name; update the check that reads OTEL_EXPORTER_OTLP_METRIC_PROTOCOL
to OTEL_EXPORTER_OTLP_METRICS_PROTOCOL so the specialized protocol override is
recognized; modify the conditional that assigns to the variable protocol (the
block with specializedProtocol := os.Getenv(...)) to use the plural
OTEL_EXPORTER_OTLP_METRICS_PROTOCOL and keep the rest of the logic unchanged.