feat(charts)!: add matomo and clickhouse#706
Merged
Merged
Conversation
mberlofa
added
enhancement
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR adds two new Helm charts, ClickHouse and Matomo, with chart metadata, values contracts, helpers and validation, workload and networking templates, tests, scenario values, and documentation. ChangesClickHouse Helm Chart
Matomo Helm Chart
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
Standards Check (GR-079) — PASSEvery changed chart fully passes standards-check. |
This was referenced Jul 7, 2026
Contributor
🟢 Security Scan:
|
| Framework | Score |
|---|---|
| MITRE + NSA + SOC2 | 89.39394% |
✅ Security posture acceptable.
Contributor
🟢 Security Scan:
|
| Framework | Score |
|---|---|
| MITRE + NSA + SOC2 | 84.783554% |
✅ Security posture acceptable.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
♻️ Duplicate comments (1)
charts/matomo/templates/cronjob-archiver.yaml (1)
44-45: 🎯 Functional Correctness | 🔴 Critical | 🏗️ Heavy liftSame
host:portconcatenation issue asdeployment.yaml.This duplicates the
MATOMO_DATABASE_HOSTconstruction flagged incharts/matomo/templates/deployment.yaml(lines 72-73); the archiver job will fail to connect to the database for the same reason if a non-default port is configured. Fixing this in one place (e.g., a shared helper) would avoid the two definitions drifting.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@charts/matomo/templates/cronjob-archiver.yaml` around lines 44 - 45, The MATOMO_DATABASE_HOST value in the cronjob archiver template repeats the same broken host:port concatenation used elsewhere, so it should be updated to use the same corrected database host formatting as deployment.yaml. Fix the construction in the archiver’s MATOMO_DATABASE_HOST entry by reusing a shared helper or the same logic used by the deployment template, and ensure only the host is used where the container expects a host value. Reference the MATOMO_DATABASE_HOST setting and the matomo.databaseHost/matomo.databasePort helpers to keep both templates consistent.
🧹 Nitpick comments (11)
charts/matomo/templates/networkpolicy.yaml (1)
29-47: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winDatabase egress rule allows traffic to any destination on the DB port.
The egress rule at Lines 38-40 restricts only by port, not destination, since there's no
toselector. Combined withextraTobeing optional, workloads can reach any external host on the database port by default, weakening the isolation NetworkPolicy is meant to provide.Consider requiring a
toselector (via a configurable external DB CIDR/selector value) or documenting this permissive default so operators know to lock it down for production.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@charts/matomo/templates/networkpolicy.yaml` around lines 29 - 47, The egress rule in the network policy template is too permissive because the database port rule in the egress block under the networkPolicy.egress.enabled path has no destination selector. Update the matomo networkpolicy.yaml logic around the database egress entry to require a configurable `to` target (for example via a DB CIDR/selector value) or otherwise make the default explicit and documented; keep the existing `extraTo` and `extraEgress` handling but ensure the main database egress rule is not open to any destination by default.charts/matomo/templates/servicemonitor.yaml (1)
18-19: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHardcoded metrics path — consider making configurable.
Path
/matomo.phpis hardcoded rather than sourced from values, unlike the ClickHouse chart's.Values.metrics.pathpattern shown in the snippet. If Matomo's metrics/health endpoint could change or is user-configurable (e.g., a subdirectory install), this locks it in.♻️ Suggested change (verify a matching values.schema.json / values.yaml key exists first)
endpoints: - port: http - path: /matomo.php + path: {{ .Values.metrics.path | default "/matomo.php" | quote }} interval: {{ .Values.metrics.serviceMonitor.interval }}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@charts/matomo/templates/servicemonitor.yaml` around lines 18 - 19, The ServiceMonitor in the Matomo chart hardcodes the probe path, so update the servicemonitor template to read the endpoint from chart values instead of using /matomo.php directly. Use the existing values-driven pattern from the chart’s templating (for example a metrics path value in values.yaml/values.schema.json) and reference the same key from the ServiceMonitor resource so users can override it for custom installs or subdirectory setups.charts/matomo/templates/extra-manifests.yaml (1)
1-6: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winInconsistent with the generic extra-manifests pattern.
The
charts/generic/templates/extra-manifests.yamlreference implementation guards against a nil list (| default list) and pipes throughtplto allow templated values (e.g.,{{ .Release.Name }}) inside user-supplied manifests. This version omits both, so extra manifests here can't reference chart values/templates and relies on implicit nil-range safety.Based on cross-file evidence from `charts/generic/templates/extra-manifests.yaml`, which uses `{{- range .Values.extraManifests | default list }}` and `{{ tpl (toYaml .) $ }}`.♻️ Align with the canonical generic template
{{/* SPDX-License-Identifier: Apache-2.0 */}} -{{- range .Values.extraManifests }} +{{- range .Values.extraManifests | default list }} --- -{{ toYaml . }} +{{ tpl (toYaml .) $ }} {{- end }}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@charts/matomo/templates/extra-manifests.yaml` around lines 1 - 6, The extra-manifests template is missing the canonical safeguards and templating support used by the generic chart. Update the extra-manifests loop in the templates/extra-manifests.yaml logic to default .Values.extraManifests to an empty list before ranging, and render each manifest through tpl after toYaml so user-supplied manifests can reference chart values like .Release.Name. Keep the existing behavior aligned with the generic extra-manifests pattern.charts/matomo/tests/routing_test.yaml (1)
3-5: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing default-off test for HTTPRoute.
The suite verifies ingress doesn't render by default but has no equivalent assertion for
gateway-httproute.yaml, even though it's included intemplates.Suggested addition
- it: does not render ingress by default template: ingress.yaml asserts: - hasDocuments: count: 0 + - it: does not render HTTPRoute by default + template: gateway-httproute.yaml + asserts: + - hasDocuments: + count: 0🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@charts/matomo/tests/routing_test.yaml` around lines 3 - 5, The routing test suite currently checks the default-off behavior for ingress but does not assert the same for gateway-httproute.yaml even though it is listed in templates. Update the routing test fixture and assertions in routing_test.yaml to explicitly verify that the HTTPRoute template does not render by default, using the gateway-httproute.yaml template entry as the target to locate the missing coverage.charts/clickhouse/tests/validation_test.yaml (1)
5-43: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd coverage for the remaining
clickhouse.validatebranches.Good coverage overall, but two fail branches in
charts/clickhouse/templates/_helpers.tpl(Lines 94, 99) aren't exercised:
replicaCount: 0(thelt ... 1branch, distinct from thegt ... 1branch already tested at Line 6-11)podLabelsoverridingapp.kubernetes.io/name(onlyapp.kubernetes.io/instanceis tested at Lines 12-18)Since these are hard validation gates that block chart installs, missing coverage risks silent regressions if the fail conditions are ever refactored.
✅ Suggested additional test cases
+ - it: blocks replicaCount below minimum + set: + replicaCount: 0 + asserts: + - failedTemplate: + errorMessage: "replicaCount must be at least 1" + - it: blocks name label override + set: + podLabels: + app.kubernetes.io/name: bad + asserts: + - failedTemplate: + errorMessage: podLabels must not override app.kubernetes.io/name🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@charts/clickhouse/tests/validation_test.yaml` around lines 5 - 43, Add test coverage for the remaining clickhouse.validate failure branches in the validation suite. Extend the tests in validation_test.yaml to exercise the `clickhouse.validate` logic in `_helpers.tpl` by adding a case for `replicaCount: 0` to hit the `lt ... 1` path, and another case where `podLabels` overrides `app.kubernetes.io/name` to verify that selector label override is rejected. Use the existing `failedTemplate` pattern alongside the current `replicaCount`, `podLabels`, and `clickhouse.validate` checks.charts/matomo/values.yaml (1)
217-219: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winNo default securityContext/podSecurityContext hardening.
Both
securityContextandpodSecurityContextdefault to empty objects, so the Matomo Apache container runs withoutrunAsNonRoot, dropped capabilities, or a read-only root filesystem by default. Given this chart targets production use (per DESIGN/README goals), consider shipping sane secure defaults (e.g.,runAsNonRoot: true,allowPrivilegeEscalation: false,capabilities.drop: [ALL]), while leaving them overridable.Example defaults
-podSecurityContext: {} - -securityContext: {} +podSecurityContext: + fsGroup: 33 + +securityContext: + allowPrivilegeEscalation: false + capabilities: + drop: ["ALL"]🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@charts/matomo/values.yaml` around lines 217 - 219, The Matomo chart defaults leave both securityContext and podSecurityContext empty, so the Apache container starts without basic hardening. Update the default values in the values.yaml entries for podSecurityContext and securityContext to include secure baseline settings such as runAsNonRoot, allowPrivilegeEscalation disabled, and dropped capabilities, while keeping them configurable for users who need different settings.charts/matomo/tests/validation_test.yaml (1)
5-28: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider adding test cases for the remaining
matomo.validatebranches.Current suite covers only 3 of the ~7 fail branches in
matomo.validate(podLabels override, ingress hosts, externalSecrets items). Missing coverage:gatewayAPI.httpRoutesempty,metrics.serviceMonitor.intervalmissing, external-DB-without-password, andexternalSecrets.items[].specmissingsecretStoreRef/data/dataFrom.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@charts/matomo/tests/validation_test.yaml` around lines 5 - 28, Add validation tests to cover the remaining failing branches in matomo.validate, since the current suite only exercises a subset of the guards. Extend validation_test.yaml with cases for gatewayAPI.httpRoutes being empty, metrics.serviceMonitor.interval being omitted, external DB configured without a password, and externalSecrets.items[].spec missing secretStoreRef, data, or dataFrom. Use the existing failedTemplate pattern in the matomo.validate test file so each branch is asserted by its exact errorMessage.charts/matomo/templates/cronjob-archiver.yaml (1)
41-54: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated DB connection env block.
The
MATOMO_DATABASE_*env definitions here are a near-exact copy ofdeployment.yaml's block (lines 72-82). Extracting a shared_helpers.tpltemplate (e.g.,matomo.databaseEnv) for the env list would reduce drift risk between the Deployment and CronJob — directly relevant given the host/port bug above exists in both copies today.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@charts/matomo/templates/cronjob-archiver.yaml` around lines 41 - 54, The `MATOMO_DATABASE_*` environment block in the CronJob is duplicated from the Deployment, so it should be centralized to avoid drift. Extract the shared env list into a reusable helper in `_helpers.tpl` (for example, a `matomo.databaseEnv` helper) and update both `cronjob-archiver.yaml` and `deployment.yaml` to include that helper instead of maintaining separate copies. Use the existing `MATOMO_DATABASE_HOST`, `MATOMO_DATABASE_USERNAME`, `MATOMO_DATABASE_DBNAME`, and `MATOMO_DATABASE_PASSWORD` definitions as the symbols to consolidate.charts/clickhouse/tests/service_test.yaml (1)
10-20: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider adding coverage for the conditional metrics port.
Tests cover http/tcp ports but not the
metricsport that's conditionally rendered when.Values.metrics.enabledis true.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@charts/clickhouse/tests/service_test.yaml` around lines 10 - 20, Add test coverage for the conditionally rendered metrics port in the ClickHouse service tests. Update the existing service rendering assertions in service_test.yaml, and/or add a new case that enables .Values.metrics.enabled so the rendered Service can be checked for the metrics port alongside the existing http and tcp ports. Use the service template’s port fields as the anchor for verification so the test clearly covers the conditional behavior.charts/clickhouse/templates/pdb.yaml (1)
2-2: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePDB condition is unreachable given the chart's single-replica constraint.
clickhouse.validatefails the render wheneverreplicaCount > 1, so thegt (int .Values.replicaCount) 1guard here can never be true - this PDB template will never render under the chart's supported configuration. SincepodDisruptionBudget.enabled: trueis the default, consider clarifying in docs/values comments that PDB is a no-op for this single-node chart, or removing the dead conditional.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@charts/clickhouse/templates/pdb.yaml` at line 2, The PDB guard in the clickhouse PDB template is dead code because clickhouse.validate already rejects any replicaCount greater than 1, so this branch can never render. Update the chart behavior by either removing the unreachable gt (int .Values.replicaCount) 1 check from the podDisruptionBudget template or clarifying in the values/docs for podDisruptionBudget.enabled that it is a no-op for this single-replica chart; use clickhouse.validate and the PDB template condition as the symbols to locate the fix.charts/clickhouse/tests/pdb_test.yaml (1)
1-9: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winOnly the "no PDB rendered" branch is tested.
The suite verifies
pdb.yamlrenders zero documents in the default/standalone case, but there's no assertion covering the positive path (podDisruptionBudget.enabled: trueandreplicaCount > 1), which would validateminAvailable/maxUnavailableselection logic in the template.Suggested additional test case
suite: clickhouse pdb templates: - pdb.yaml tests: - it: does not render for standalone default asserts: - hasDocuments: count: 0 + - it: renders when enabled with multiple replicas + set: + replicaCount: 3 + podDisruptionBudget: + enabled: true + minAvailable: 1 + asserts: + - hasDocuments: + count: 1 + - equal: + path: spec.minAvailable + value: 1🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@charts/clickhouse/tests/pdb_test.yaml` around lines 1 - 9, The clickhouse PDB test suite only covers the no-render case, so add a positive test for the pdb.yaml template when podDisruptionBudget.enabled is true and replicaCount is greater than 1. Use the existing pdb.yaml suite and its assertions to verify a document is rendered and that the template selects the expected minAvailable or maxUnavailable branch. Reference the pdb.yaml template and the current standalone default test case when adding the new assertion.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@charts/clickhouse/README.md`:
- Around line 91-95: The Kubescape command in the README uses a Windows-style
backslash path that breaks in POSIX shells. Update the documented command in the
clickhouse README to use the shell-safe forward-slash form for the rendered YAML
path, or explicitly mark the example as PowerShell-only. Keep the change focused
on the Kubescape scan example so readers can run it reliably in the intended
shell context.
In `@charts/clickhouse/templates/networkpolicy.yaml`:
- Around line 19-41: The default ingress fallback in the NetworkPolicy template
currently uses a blanket namespaceSelector, which opens traffic from every
namespace when ingressFrom or metricsFrom are unset. Update the
networkpolicy.yaml logic around the ingress and metrics rules to default to
same-namespace only by omitting the from block or using a local podSelector
without a namespaceSelector, while preserving explicit overrides from
.Values.networkPolicy.ingressFrom and .Values.networkPolicy.metricsFrom.
In `@charts/clickhouse/templates/statefulset.yaml`:
- Around line 25-29: The StatefulSet pod template checksum only reflects
configmap changes, so updates to the ClickHouse password secret will not trigger
a rollout and pods can keep a stale env var value. Update the annotations in the
StatefulSet template to include a checksum for the Secret used by
CLICKHOUSE_PASSWORD, alongside the existing checksum/config, and make sure the
secret is part of the pod template hash so changes to the referenced secret
content cause a rolling restart. Locate the pod template annotations and the
CLICKHOUSE_PASSWORD secretKeyRef wiring in the StatefulSet manifest when
applying the fix.
In `@charts/matomo/README.md`:
- Line 104: The Kubescape example command uses a Windows-style path that will
fail in bash shells; update the README example to use the forward-slash form for
the rendered manifest path. Adjust the command under the Matomo example so the
`kubescape scan` invocation references `.tmp/matomo-render.yaml` instead of the
backslash path.
In `@charts/matomo/templates/_helpers.tpl`:
- Around line 167-181: The external database validation in matomo.validate is
too narrow because it only enforces the secret when archiver.enabled is true,
but the main Deployment always consumes MATOMO_DATABASE_PASSWORD from the
database secret. Update the matomo.validate helper to require
database.external.password, database.external.existingSecret, or an
ExternalSecret target whenever databaseMode is "external", regardless of
archiver.enabled, and keep the existing fail message aligned with the new
unconditional check.
In `@charts/matomo/templates/cronjob-archiver.yaml`:
- Around line 32-56: Add the missing security hardening to the archiver
container in cronjob-archiver.yaml by applying the same securityContext pattern
used by the main matomo container. Update the archive container spec to
reference the existing .Values.securityContext (or the chart’s equivalent helper
if used elsewhere) so the PHP console job runs with the same protections as the
main workload. Use the archive container definition and the matomo deployment
container securityContext as the reference points.
In `@charts/matomo/templates/deployment.yaml`:
- Line 24: The deployment checksum annotation currently only tracks the
ConfigMap, so password changes from MATOMO_DATABASE_PASSWORD will not trigger a
rollout. Update the checksum logic in the deployment template to also include
the rendered Secret used by the Matomo pod (the one referenced by the
secretKeyRef for the database password), so changes to credentials through Helm
values or ExternalSecret refreshes cause a rolling restart.
- Around line 72-73: The MATOMO_DATABASE_HOST environment value is being
rendered as host:port, but the Matomo container expects only the hostname here.
Update the deployment template so the MATOMO_DATABASE_HOST entry uses the
database host helper alone, and keep any port configuration in config.ini.php
instead; use the existing matomo.databaseHost and matomo.databasePort symbols to
locate the affected template logic.
In `@charts/matomo/templates/tests/test-connection.yaml`:
- Around line 12-21: Add a timeout to the Matomo connection test so the Helm
test cannot hang forever if the service is unresponsive. Update the test hook in
the connection test manifest used by the curl container so the `command` for the
`curl` check enforces a bounded wait, and consider adding a pod-level
deadline/timeout setting in the test spec as well. Use the existing `curl -fsS`
probe against `matomo.php` as the place to apply the timeout, keeping the
current readiness check behavior intact while ensuring `helm test` can fail
fast.
---
Duplicate comments:
In `@charts/matomo/templates/cronjob-archiver.yaml`:
- Around line 44-45: The MATOMO_DATABASE_HOST value in the cronjob archiver
template repeats the same broken host:port concatenation used elsewhere, so it
should be updated to use the same corrected database host formatting as
deployment.yaml. Fix the construction in the archiver’s MATOMO_DATABASE_HOST
entry by reusing a shared helper or the same logic used by the deployment
template, and ensure only the host is used where the container expects a host
value. Reference the MATOMO_DATABASE_HOST setting and the
matomo.databaseHost/matomo.databasePort helpers to keep both templates
consistent.
---
Nitpick comments:
In `@charts/clickhouse/templates/pdb.yaml`:
- Line 2: The PDB guard in the clickhouse PDB template is dead code because
clickhouse.validate already rejects any replicaCount greater than 1, so this
branch can never render. Update the chart behavior by either removing the
unreachable gt (int .Values.replicaCount) 1 check from the podDisruptionBudget
template or clarifying in the values/docs for podDisruptionBudget.enabled that
it is a no-op for this single-replica chart; use clickhouse.validate and the PDB
template condition as the symbols to locate the fix.
In `@charts/clickhouse/tests/pdb_test.yaml`:
- Around line 1-9: The clickhouse PDB test suite only covers the no-render case,
so add a positive test for the pdb.yaml template when
podDisruptionBudget.enabled is true and replicaCount is greater than 1. Use the
existing pdb.yaml suite and its assertions to verify a document is rendered and
that the template selects the expected minAvailable or maxUnavailable branch.
Reference the pdb.yaml template and the current standalone default test case
when adding the new assertion.
In `@charts/clickhouse/tests/service_test.yaml`:
- Around line 10-20: Add test coverage for the conditionally rendered metrics
port in the ClickHouse service tests. Update the existing service rendering
assertions in service_test.yaml, and/or add a new case that enables
.Values.metrics.enabled so the rendered Service can be checked for the metrics
port alongside the existing http and tcp ports. Use the service template’s port
fields as the anchor for verification so the test clearly covers the conditional
behavior.
In `@charts/clickhouse/tests/validation_test.yaml`:
- Around line 5-43: Add test coverage for the remaining clickhouse.validate
failure branches in the validation suite. Extend the tests in
validation_test.yaml to exercise the `clickhouse.validate` logic in
`_helpers.tpl` by adding a case for `replicaCount: 0` to hit the `lt ... 1`
path, and another case where `podLabels` overrides `app.kubernetes.io/name` to
verify that selector label override is rejected. Use the existing
`failedTemplate` pattern alongside the current `replicaCount`, `podLabels`, and
`clickhouse.validate` checks.
In `@charts/matomo/templates/cronjob-archiver.yaml`:
- Around line 41-54: The `MATOMO_DATABASE_*` environment block in the CronJob is
duplicated from the Deployment, so it should be centralized to avoid drift.
Extract the shared env list into a reusable helper in `_helpers.tpl` (for
example, a `matomo.databaseEnv` helper) and update both `cronjob-archiver.yaml`
and `deployment.yaml` to include that helper instead of maintaining separate
copies. Use the existing `MATOMO_DATABASE_HOST`, `MATOMO_DATABASE_USERNAME`,
`MATOMO_DATABASE_DBNAME`, and `MATOMO_DATABASE_PASSWORD` definitions as the
symbols to consolidate.
In `@charts/matomo/templates/extra-manifests.yaml`:
- Around line 1-6: The extra-manifests template is missing the canonical
safeguards and templating support used by the generic chart. Update the
extra-manifests loop in the templates/extra-manifests.yaml logic to default
.Values.extraManifests to an empty list before ranging, and render each manifest
through tpl after toYaml so user-supplied manifests can reference chart values
like .Release.Name. Keep the existing behavior aligned with the generic
extra-manifests pattern.
In `@charts/matomo/templates/networkpolicy.yaml`:
- Around line 29-47: The egress rule in the network policy template is too
permissive because the database port rule in the egress block under the
networkPolicy.egress.enabled path has no destination selector. Update the matomo
networkpolicy.yaml logic around the database egress entry to require a
configurable `to` target (for example via a DB CIDR/selector value) or otherwise
make the default explicit and documented; keep the existing `extraTo` and
`extraEgress` handling but ensure the main database egress rule is not open to
any destination by default.
In `@charts/matomo/templates/servicemonitor.yaml`:
- Around line 18-19: The ServiceMonitor in the Matomo chart hardcodes the probe
path, so update the servicemonitor template to read the endpoint from chart
values instead of using /matomo.php directly. Use the existing values-driven
pattern from the chart’s templating (for example a metrics path value in
values.yaml/values.schema.json) and reference the same key from the
ServiceMonitor resource so users can override it for custom installs or
subdirectory setups.
In `@charts/matomo/tests/routing_test.yaml`:
- Around line 3-5: The routing test suite currently checks the default-off
behavior for ingress but does not assert the same for gateway-httproute.yaml
even though it is listed in templates. Update the routing test fixture and
assertions in routing_test.yaml to explicitly verify that the HTTPRoute template
does not render by default, using the gateway-httproute.yaml template entry as
the target to locate the missing coverage.
In `@charts/matomo/tests/validation_test.yaml`:
- Around line 5-28: Add validation tests to cover the remaining failing branches
in matomo.validate, since the current suite only exercises a subset of the
guards. Extend validation_test.yaml with cases for gatewayAPI.httpRoutes being
empty, metrics.serviceMonitor.interval being omitted, external DB configured
without a password, and externalSecrets.items[].spec missing secretStoreRef,
data, or dataFrom. Use the existing failedTemplate pattern in the
matomo.validate test file so each branch is asserted by its exact errorMessage.
In `@charts/matomo/values.yaml`:
- Around line 217-219: The Matomo chart defaults leave both securityContext and
podSecurityContext empty, so the Apache container starts without basic
hardening. Update the default values in the values.yaml entries for
podSecurityContext and securityContext to include secure baseline settings such
as runAsNonRoot, allowPrivilegeEscalation disabled, and dropped capabilities,
while keeping them configurable for users who need different settings.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: ad88ce15-6046-42b4-9ca9-41cd05f5dd58
📒 Files selected for processing (92)
charts/clickhouse/.helmignorecharts/clickhouse/Chart.yamlcharts/clickhouse/DESIGN.mdcharts/clickhouse/README.mdcharts/clickhouse/ci/auth-values.yamlcharts/clickhouse/ci/default-values.yamlcharts/clickhouse/ci/dual-stack.yamlcharts/clickhouse/ci/ephemeral-values.yamlcharts/clickhouse/ci/external-secrets-values.yamlcharts/clickhouse/ci/metrics-values.yamlcharts/clickhouse/ci/network-policy-values.yamlcharts/clickhouse/ci/production-values.yamlcharts/clickhouse/docs/external-secrets.mdcharts/clickhouse/docs/observability.mdcharts/clickhouse/docs/production.mdcharts/clickhouse/examples/external-secrets.yamlcharts/clickhouse/examples/metrics.yamlcharts/clickhouse/examples/production.yamlcharts/clickhouse/examples/standalone.yamlcharts/clickhouse/templates/NOTES.txtcharts/clickhouse/templates/_helpers.tplcharts/clickhouse/templates/configmap.yamlcharts/clickhouse/templates/externalsecret.yamlcharts/clickhouse/templates/extra-manifests.yamlcharts/clickhouse/templates/networkpolicy.yamlcharts/clickhouse/templates/pdb.yamlcharts/clickhouse/templates/secret.yamlcharts/clickhouse/templates/service.yamlcharts/clickhouse/templates/serviceaccount.yamlcharts/clickhouse/templates/servicemonitor.yamlcharts/clickhouse/templates/statefulset.yamlcharts/clickhouse/templates/tests/test-connection.yamlcharts/clickhouse/templates/validate.yamlcharts/clickhouse/tests/externalsecret_test.yamlcharts/clickhouse/tests/metrics_test.yamlcharts/clickhouse/tests/networkpolicy_test.yamlcharts/clickhouse/tests/pdb_test.yamlcharts/clickhouse/tests/secret_test.yamlcharts/clickhouse/tests/service_test.yamlcharts/clickhouse/tests/statefulset_test.yamlcharts/clickhouse/tests/test_connection_test.yamlcharts/clickhouse/tests/validation_test.yamlcharts/clickhouse/values.schema.jsoncharts/clickhouse/values.yamlcharts/matomo/.helmignorecharts/matomo/Chart.yamlcharts/matomo/DESIGN.mdcharts/matomo/README.mdcharts/matomo/ci/default-values.yamlcharts/matomo/ci/dual-stack.yamlcharts/matomo/ci/external-db-values.yamlcharts/matomo/ci/external-secrets-values.yamlcharts/matomo/ci/gateway-api-values.yamlcharts/matomo/ci/ingress-values.yamlcharts/matomo/ci/network-policy-values.yamlcharts/matomo/ci/production-values.yamlcharts/matomo/docs/external-secrets.mdcharts/matomo/docs/networking.mdcharts/matomo/docs/production.mdcharts/matomo/examples/external-database.yamlcharts/matomo/examples/gateway-api.yamlcharts/matomo/examples/production.yamlcharts/matomo/examples/simple.yamlcharts/matomo/templates/NOTES.txtcharts/matomo/templates/_helpers.tplcharts/matomo/templates/configmap.yamlcharts/matomo/templates/cronjob-archiver.yamlcharts/matomo/templates/deployment.yamlcharts/matomo/templates/externalsecret.yamlcharts/matomo/templates/extra-manifests.yamlcharts/matomo/templates/gateway-httproute.yamlcharts/matomo/templates/ingress.yamlcharts/matomo/templates/networkpolicy.yamlcharts/matomo/templates/pdb.yamlcharts/matomo/templates/pvc.yamlcharts/matomo/templates/secret.yamlcharts/matomo/templates/service.yamlcharts/matomo/templates/serviceaccount.yamlcharts/matomo/templates/servicemonitor.yamlcharts/matomo/templates/tests/test-connection.yamlcharts/matomo/templates/validate.yamlcharts/matomo/tests/archiver_test.yamlcharts/matomo/tests/deployment_test.yamlcharts/matomo/tests/externalsecret_test.yamlcharts/matomo/tests/networkpolicy_test.yamlcharts/matomo/tests/pdb_test.yamlcharts/matomo/tests/routing_test.yamlcharts/matomo/tests/service_test.yamlcharts/matomo/tests/test_connection_test.yamlcharts/matomo/tests/validation_test.yamlcharts/matomo/values.schema.jsoncharts/matomo/values.yaml
mberlofa
force-pushed
the
feat/matomo-clickhouse-new-charts
branch
from
0f5c3cd to
dd56a71
Compare
mberlofa
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
charts/matomo/templates/networkpolicy.yaml (1)
38-44: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winDatabase egress rule allows any destination on that port.
The database egress rule has no
to:selector, so it permits egress to the database port on any destination (not just the actual DB). This weakens the egress restriction's value —extraTois optional, so by default nothing constrains the destination. Consider documenting this limitation clearly (e.g., in DESIGN.md/README) and recommending users populatenetworkPolicy.egress.extraTofor stricter isolation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@charts/matomo/templates/networkpolicy.yaml` around lines 38 - 44, The NetworkPolicy egress rule for the database port in the matomo chart currently allows traffic to any destination because it has ports without a destination selector. Update the chart documentation/comments around the network policy handling to clearly state this limitation and point users to configure networkPolicy.egress.extraTo for stricter isolation; reference the networkpolicy template and the egress.extraTo values path so the guidance is easy to find even if the template changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@charts/clickhouse/templates/tests/test-connection.yaml`:
- Around line 12-21: The connection test in the curl-based Helm test can hang
indefinitely if the service is unresponsive. Update the test pod definition and
the curl invocation in the test-connection template so the `curl` command has a
bounded execution time, using the existing test container setup in the `curl`
container spec. Keep the same `clickhouse.fullname` ping check, but add an
explicit timeout to ensure `helm test` and CI fail fast instead of waiting
forever.
---
Nitpick comments:
In `@charts/matomo/templates/networkpolicy.yaml`:
- Around line 38-44: The NetworkPolicy egress rule for the database port in the
matomo chart currently allows traffic to any destination because it has ports
without a destination selector. Update the chart documentation/comments around
the network policy handling to clearly state this limitation and point users to
configure networkPolicy.egress.extraTo for stricter isolation; reference the
networkpolicy template and the egress.extraTo values path so the guidance is
easy to find even if the template changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e8731dc6-2e76-4609-b4f3-32bfd353d333
📒 Files selected for processing (92)
charts/clickhouse/.helmignorecharts/clickhouse/Chart.yamlcharts/clickhouse/DESIGN.mdcharts/clickhouse/README.mdcharts/clickhouse/ci/auth-values.yamlcharts/clickhouse/ci/default-values.yamlcharts/clickhouse/ci/dual-stack.yamlcharts/clickhouse/ci/ephemeral-values.yamlcharts/clickhouse/ci/external-secrets-values.yamlcharts/clickhouse/ci/metrics-values.yamlcharts/clickhouse/ci/network-policy-values.yamlcharts/clickhouse/ci/production-values.yamlcharts/clickhouse/docs/external-secrets.mdcharts/clickhouse/docs/observability.mdcharts/clickhouse/docs/production.mdcharts/clickhouse/examples/external-secrets.yamlcharts/clickhouse/examples/metrics.yamlcharts/clickhouse/examples/production.yamlcharts/clickhouse/examples/standalone.yamlcharts/clickhouse/templates/NOTES.txtcharts/clickhouse/templates/_helpers.tplcharts/clickhouse/templates/configmap.yamlcharts/clickhouse/templates/externalsecret.yamlcharts/clickhouse/templates/extra-manifests.yamlcharts/clickhouse/templates/networkpolicy.yamlcharts/clickhouse/templates/pdb.yamlcharts/clickhouse/templates/secret.yamlcharts/clickhouse/templates/service.yamlcharts/clickhouse/templates/serviceaccount.yamlcharts/clickhouse/templates/servicemonitor.yamlcharts/clickhouse/templates/statefulset.yamlcharts/clickhouse/templates/tests/test-connection.yamlcharts/clickhouse/templates/validate.yamlcharts/clickhouse/tests/externalsecret_test.yamlcharts/clickhouse/tests/metrics_test.yamlcharts/clickhouse/tests/networkpolicy_test.yamlcharts/clickhouse/tests/pdb_test.yamlcharts/clickhouse/tests/secret_test.yamlcharts/clickhouse/tests/service_test.yamlcharts/clickhouse/tests/statefulset_test.yamlcharts/clickhouse/tests/test_connection_test.yamlcharts/clickhouse/tests/validation_test.yamlcharts/clickhouse/values.schema.jsoncharts/clickhouse/values.yamlcharts/matomo/.helmignorecharts/matomo/Chart.yamlcharts/matomo/DESIGN.mdcharts/matomo/README.mdcharts/matomo/ci/default-values.yamlcharts/matomo/ci/dual-stack.yamlcharts/matomo/ci/external-db-values.yamlcharts/matomo/ci/external-secrets-values.yamlcharts/matomo/ci/gateway-api-values.yamlcharts/matomo/ci/ingress-values.yamlcharts/matomo/ci/network-policy-values.yamlcharts/matomo/ci/production-values.yamlcharts/matomo/docs/external-secrets.mdcharts/matomo/docs/networking.mdcharts/matomo/docs/production.mdcharts/matomo/examples/external-database.yamlcharts/matomo/examples/gateway-api.yamlcharts/matomo/examples/production.yamlcharts/matomo/examples/simple.yamlcharts/matomo/templates/NOTES.txtcharts/matomo/templates/_helpers.tplcharts/matomo/templates/configmap.yamlcharts/matomo/templates/cronjob-archiver.yamlcharts/matomo/templates/deployment.yamlcharts/matomo/templates/externalsecret.yamlcharts/matomo/templates/extra-manifests.yamlcharts/matomo/templates/gateway-httproute.yamlcharts/matomo/templates/ingress.yamlcharts/matomo/templates/networkpolicy.yamlcharts/matomo/templates/pdb.yamlcharts/matomo/templates/pvc.yamlcharts/matomo/templates/secret.yamlcharts/matomo/templates/service.yamlcharts/matomo/templates/serviceaccount.yamlcharts/matomo/templates/servicemonitor.yamlcharts/matomo/templates/tests/test-connection.yamlcharts/matomo/templates/validate.yamlcharts/matomo/tests/archiver_test.yamlcharts/matomo/tests/deployment_test.yamlcharts/matomo/tests/externalsecret_test.yamlcharts/matomo/tests/networkpolicy_test.yamlcharts/matomo/tests/pdb_test.yamlcharts/matomo/tests/routing_test.yamlcharts/matomo/tests/service_test.yamlcharts/matomo/tests/test_connection_test.yamlcharts/matomo/tests/validation_test.yamlcharts/matomo/values.schema.jsoncharts/matomo/values.yaml
✅ Files skipped from review due to trivial changes (25)
- charts/matomo/.helmignore
- charts/matomo/Chart.yaml
- charts/matomo/tests/externalsecret_test.yaml
- charts/matomo/tests/service_test.yaml
- charts/matomo/docs/external-secrets.md
- charts/matomo/examples/gateway-api.yaml
- charts/clickhouse/ci/metrics-values.yaml
- charts/matomo/ci/default-values.yaml
- charts/matomo/tests/pdb_test.yaml
- charts/clickhouse/.helmignore
- charts/clickhouse/Chart.yaml
- charts/matomo/docs/production.md
- charts/clickhouse/ci/auth-values.yaml
- charts/clickhouse/DESIGN.md
- charts/matomo/README.md
- charts/clickhouse/tests/metrics_test.yaml
- charts/clickhouse/values.yaml
- charts/clickhouse/ci/default-values.yaml
- charts/clickhouse/docs/observability.md
- charts/clickhouse/ci/ephemeral-values.yaml
- charts/matomo/examples/external-database.yaml
- charts/matomo/examples/simple.yaml
- charts/clickhouse/README.md
- charts/matomo/docs/networking.md
- charts/clickhouse/docs/external-secrets.md
🚧 Files skipped from review as they are similar to previous changes (36)
- charts/matomo/ci/dual-stack.yaml
- charts/matomo/tests/deployment_test.yaml
- charts/matomo/tests/archiver_test.yaml
- charts/clickhouse/tests/pdb_test.yaml
- charts/clickhouse/tests/validation_test.yaml
- charts/clickhouse/ci/production-values.yaml
- charts/matomo/ci/network-policy-values.yaml
- charts/matomo/tests/routing_test.yaml
- charts/clickhouse/examples/metrics.yaml
- charts/matomo/ci/ingress-values.yaml
- charts/clickhouse/tests/secret_test.yaml
- charts/matomo/tests/networkpolicy_test.yaml
- charts/clickhouse/tests/service_test.yaml
- charts/clickhouse/ci/dual-stack.yaml
- charts/clickhouse/tests/externalsecret_test.yaml
- charts/clickhouse/examples/standalone.yaml
- charts/clickhouse/tests/statefulset_test.yaml
- charts/clickhouse/examples/external-secrets.yaml
- charts/clickhouse/tests/test_connection_test.yaml
- charts/clickhouse/ci/network-policy-values.yaml
- charts/matomo/ci/external-db-values.yaml
- charts/matomo/values.schema.json
- charts/matomo/ci/external-secrets-values.yaml
- charts/matomo/ci/production-values.yaml
- charts/matomo/ci/gateway-api-values.yaml
- charts/clickhouse/tests/networkpolicy_test.yaml
- charts/matomo/tests/validation_test.yaml
- charts/clickhouse/examples/production.yaml
- charts/clickhouse/docs/production.md
- charts/matomo/tests/test_connection_test.yaml
- charts/clickhouse/ci/external-secrets-values.yaml
- charts/matomo/examples/production.yaml
- charts/clickhouse/templates/_helpers.tpl
- charts/matomo/templates/_helpers.tpl
- charts/matomo/values.yaml
- charts/clickhouse/values.schema.json
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@charts/matomo/templates/_helpers.tpl`:
- Line 172: The validation in the helper template is too broad because it relies
on the global ExternalSecrets flag instead of confirming a secret will actually
be created for the database password. Update the guard in the helper that checks
external database settings to require an ExternalSecret whose rendered target
matches matomo.databaseSecretName, rather than just
.Values.externalSecrets.enabled. Keep the existing existingSecret/password
checks, but make the failure condition pass only when a database-targeting
ExternalSecret is configured.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 36f5c423-9496-4ddf-a746-cef10191eca3
📒 Files selected for processing (14)
charts/clickhouse/README.mdcharts/clickhouse/templates/networkpolicy.yamlcharts/clickhouse/templates/statefulset.yamlcharts/clickhouse/tests/networkpolicy_test.yamlcharts/clickhouse/tests/statefulset_test.yamlcharts/matomo/README.mdcharts/matomo/templates/_helpers.tplcharts/matomo/templates/cronjob-archiver.yamlcharts/matomo/templates/deployment.yamlcharts/matomo/templates/tests/test-connection.yamlcharts/matomo/tests/archiver_test.yamlcharts/matomo/tests/deployment_test.yamlcharts/matomo/tests/test_connection_test.yamlcharts/matomo/tests/validation_test.yaml
✅ Files skipped from review due to trivial changes (2)
- charts/matomo/README.md
- charts/clickhouse/README.md
🚧 Files skipped from review as they are similar to previous changes (9)
- charts/matomo/tests/deployment_test.yaml
- charts/matomo/tests/validation_test.yaml
- charts/matomo/templates/tests/test-connection.yaml
- charts/matomo/tests/test_connection_test.yaml
- charts/clickhouse/tests/networkpolicy_test.yaml
- charts/matomo/templates/deployment.yaml
- charts/matomo/templates/cronjob-archiver.yaml
- charts/clickhouse/tests/statefulset_test.yaml
- charts/clickhouse/templates/statefulset.yaml
mberlofa
added a commit
to helmforgedev/site
that referenced
this pull request
Jul 7, 2026
## Summary - Adds Matomo and ClickHouse chart documentation pages. - Adds catalog, navigation, and playground entries for both charts. - Adds Matomo and ClickHouse icons. - Normalizes existing chart icons that were blocking the HelmForge icon audit. ## Validation - npm run format:check - npm run lint - npm run build - make site-sync-check CHART=matomo - make site-sync-check CHART=clickhouse - make org-check Companion charts PR: helmforgedev/charts#706. Companion profile PR: helmforgedev/.github#13. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added support for two new charts: **ClickHouse** and **Matomo**. * Expanded the playground with setup options for both charts. * Updated navigation so both charts appear in the appropriate categories. * **Documentation** * Added detailed documentation pages for **ClickHouse** and **Matomo**, including installation, configuration examples, troubleshooting, and values references. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This was referenced Jul 7, 2026
Closed
mberlofa
added a commit
to helmforgedev/.github
that referenced
this pull request
Jul 8, 2026
## Summary - Updates the organization profile chart count from 87 to 89. - Keeps the profile aligned with the new Matomo and ClickHouse charts. ## Validation - make release-check REPO=.github - make attribution-check REPO=.github - make md-lint REPO=.github - make org-check Companion charts PR: helmforgedev/charts#706. Companion site PR: helmforgedev/site#368.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
docker.io/library/matomo:5.11.2-apacheimage.docker.io/clickhouse/clickhouse-server:25.8.28.1image.Issues
Resolves #654.
Resolves #705.
Companion PRs
Validation
Chart image verification:
make image-verify IMAGE=docker.io/library/matomo:5.11.2-apachemake image-verify IMAGE=docker.io/clickhouse/clickhouse-server:25.8.28.1Matomo:
make validate-chart CHART=matomo->matomo: FULLY VALIDATED (18 layers)ClickHouse:
make validate-chart CHART=clickhouse->clickhouse: FULLY VALIDATED (18 layers)Repository gates:
make preflightmake standards-check CHART=matomomake standards-check CHART=clickhousenode scripts/charts/template-standards-check.js --chart matomonode scripts/charts/template-standards-check.js --chart clickhousemake deps-check CHART=matomomake deps-check CHART=clickhousemake site-sync-check CHART=matomomake site-sync-check CHART=clickhousemake org-checkNotes
ClickHouse intentionally blocks
replicaCount > 1; replicated or sharded deployments should use ClickHouse Operator or Altinity operator workflows.Summary by CodeRabbit