Conversation
improve clickhouse config
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/release-helm.yml (1)
102-102: Strip trailing whitespace to satisfy YAML-lintLines 102 and 120 contain stray spaces. They do not break the workflow but fail CI linters.
-102␠␠ +102 … -120␠␠ +120Also applies to: 120-120
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/release-helm.yml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/release-helm.yml
108-108: the runner of "softprops/action-gh-release@v1" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 YAMLlint (1.37.1)
.github/workflows/release-helm.yml
[error] 102-102: trailing spaces
(trailing-spaces)
[error] 120-120: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms (23)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 10)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (9, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (10, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 10)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: typecheck / typecheck
- GitHub Check: check-broken-links
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (2)
.github/workflows/release-helm.yml (2)
108-108: Updatesoftprops/action-gh-releaseto a Node ≥ 20 build
softprops/action-gh-release@v1ships a Node 12 runner, which GitHub is actively deprecating; new workflows will soon refuse to run it (see actionlint warning).
Pin to a more recent tag/commit (e.g.softprops/action-gh-release@v2or a specific SHA) that targets Node 20 to avoid sudden breakage.
21-23: 👍 Least-privilegepermissionsblocks address previous security finding
Both jobs now declare explicit, minimal scopes (contents: read/write,packages: write), resolving the earlier Advanced Security alert about unrestrictedGITHUB_TOKEN.Also applies to: 51-53
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (5)
hosting/k8s/helm/templates/_helpers.tpl (1)
103-105: Security issue resolved - SSL mode is now configurable.The previous security concern about hardcoded
sslmode=disablehas been properly addressed. The connection strings now use configurable SSL mode values with secure defaults (prefer).hosting/k8s/helm/templates/ingress.yaml (1)
10-11: Annotations duplication issue resolved.The previous concern about duplicate annotations has been properly addressed by using the
trigger-v4.ingress.annotationshelper function, which merges annotations from multiple sources without conflicts.hosting/k8s/helm/templates/postgresql.yaml (1)
81-102: Duplicate volumeClaimTemplates issue resolved.The previous concern about duplicate
volumeClaimTemplatesblocks has been properly addressed. The template now has a single, properly structured conditional block that either creates persistent volume claims or uses emptyDir volumes based on configuration.hosting/k8s/helm/templates/tests/test-clickhouse.yaml (1)
19-19: Security and reliability issues resolved.Both previous concerns have been properly addressed:
- Credentials are now passed securely via
--useroption instead of being embedded in the URL- Image is pinned to a specific version (
curlimages/curl:8.14.1) instead of usinglatesthosting/k8s/helm/templates/tests/test-webapp.yaml (1)
13-13: Image version pinning issue resolved.The previous concern about using
curlimages/curl:latesthas been properly addressed by pinning to a specific version (curlimages/curl:8.14.1), ensuring reproducible tests.
🧹 Nitpick comments (12)
hosting/k8s/helm/templates/tests/test-electric.yaml (2)
8-9: Addhook-delete-policyto prevent orphaned test PodsSuccessful test Pods will linger unless explicitly cleaned up.
Annotating with a delete-policy keeps the namespace tidy and avoids quota pressure.annotations: "helm.sh/hook": test + "helm.sh/hook-delete-policy": before-hook-creation,hook-succeeded
18-20: Harden the curl check with retries & timeoutsA single timeout spike will mark the test failed even though the service might be healthy.
Adding retry / timeout flags provides a small resilience boost while keeping the test fast.- curl -f http://{{ include "trigger-v4.fullname" . }}-electric:{{ .Values.electric.service.port }}/api/status + curl -fsS --retry 3 --max-time 5 \ + http://{{ include "trigger-v4.fullname" . }}-electric:{{ .Values.electric.service.port }}/api/statushosting/k8s/helm/templates/tests/test-supervisor.yaml (2)
8-9: Mirror hook-delete-policy here as wellSame motivation as for the Electric test – keeps the cluster clean.
annotations: "helm.sh/hook": test + "helm.sh/hook-delete-policy": before-hook-creation,hook-succeeded
18-20: Add curl resiliency flags- curl -f http://{{ include "trigger-v4.fullname" . }}-supervisor:{{ .Values.supervisor.service.ports.metrics }}/metrics + curl -fsS --retry 3 --max-time 5 \ + http://{{ include "trigger-v4.fullname" . }}-supervisor:{{ .Values.supervisor.service.ports.metrics }}/metricshosting/k8s/helm/templates/tests/test-minio.yaml (2)
8-9: Add delete policy for test Podannotations: "helm.sh/hook": test + "helm.sh/hook-delete-policy": before-hook-creation,hook-succeeded
18-20: Retry / timeout for curl- curl -f http://{{ include "trigger-v4.fullname" . }}-minio:{{ .Values.minio.service.ports.api }}/minio/health/live + curl -fsS --retry 3 --max-time 5 \ + http://{{ include "trigger-v4.fullname" . }}-minio:{{ .Values.minio.service.ports.api }}/minio/health/livehosting/k8s/helm/templates/secrets.yaml (1)
10-16: Consider switching tostringDatafor easier plaintext authoringUsing
datarequires callers to pre-base64 every value, which is error-prone.
stringDatalets users supply plain strings which Helm will encode for them, reducing sharp edges.-type: Opaque -data: +type: Opaque +stringData:(Helm will still render correct base64 in the final manifest.)
hosting/k8s/helm/templates/webapp.yaml (2)
44-48: Remove trailing space & keepcommand/argsYAML-validThere is a trailing space after
command:which trips YAML linters.
While here, make the list form explicit to avoid accidental string coercion.- command: - - ./scripts/entrypoint.sh + command: + - ./scripts/entrypoint.sh
283-284: Add a final newline for POSIX complianceSeveral tools complain when files lack a terminating newline; easy win for lint cleanliness.
hosting/k8s/helm/templates/supervisor.yaml (3)
78-86: Init container: add read-only filesystem & non-root by defaultYou already dropped root privileges—nice. Consider sealing it completely with a read-only root FS:
securityContext: runAsUser: 1000 + readOnlyRootFilesystem: trueSmall change, big defense-in-depth gain.
117-214: Environment block becoming unwieldyMore than 90 env vars in a single block affects readability & diffability.
Consider extracting groups (e.g.,kubernetes,metrics) into named ConfigMaps and mounting them viaenvFrom, or templating a helper that iterates keys.Keeps this template maintainable as config grows.
263-263: Missing newline at EOFYAML lint flags the absence; add a trailing newline for POSIX friendliness.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
docs/self-hosting/docker.mdx(1 hunks)docs/self-hosting/kubernetes.mdx(1 hunks)hosting/k8s/helm/templates/_helpers.tpl(1 hunks)hosting/k8s/helm/templates/ingress.yaml(1 hunks)hosting/k8s/helm/templates/postgresql.yaml(1 hunks)hosting/k8s/helm/templates/secrets.yaml(1 hunks)hosting/k8s/helm/templates/supervisor.yaml(1 hunks)hosting/k8s/helm/templates/tests/test-clickhouse.yaml(1 hunks)hosting/k8s/helm/templates/tests/test-electric.yaml(1 hunks)hosting/k8s/helm/templates/tests/test-minio.yaml(1 hunks)hosting/k8s/helm/templates/tests/test-supervisor.yaml(1 hunks)hosting/k8s/helm/templates/tests/test-webapp.yaml(1 hunks)hosting/k8s/helm/templates/webapp.yaml(1 hunks)hosting/k8s/helm/values-production-example.yaml(1 hunks)hosting/k8s/helm/values.yaml(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- docs/self-hosting/docker.mdx
- docs/self-hosting/kubernetes.mdx
🚧 Files skipped from review as they are similar to previous changes (2)
- hosting/k8s/helm/values-production-example.yaml
- hosting/k8s/helm/values.yaml
🧰 Additional context used
🪛 YAMLlint (1.37.1)
hosting/k8s/helm/templates/ingress.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
[error] 51-51: no new line character at the end of file
(new-line-at-end-of-file)
hosting/k8s/helm/templates/postgresql.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
[warning] 18-18: wrong indentation: expected 4 but found 6
(indentation)
[error] 138-138: no new line character at the end of file
(new-line-at-end-of-file)
hosting/k8s/helm/templates/secrets.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
hosting/k8s/helm/templates/supervisor.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
[error] 263-263: no new line character at the end of file
(new-line-at-end-of-file)
hosting/k8s/helm/templates/tests/test-clickhouse.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
hosting/k8s/helm/templates/tests/test-electric.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
hosting/k8s/helm/templates/tests/test-minio.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
hosting/k8s/helm/templates/tests/test-supervisor.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
hosting/k8s/helm/templates/tests/test-webapp.yaml
[error] 4-4: syntax error: expected , but found ''
(syntax)
hosting/k8s/helm/templates/webapp.yaml
[warning] 7-7: wrong indentation: expected 2 but found 4
(indentation)
[warning] 16-16: wrong indentation: expected 4 but found 6
(indentation)
[error] 46-46: trailing spaces
(trailing-spaces)
[error] 284-284: no new line character at the end of file
(new-line-at-end-of-file)
[error] 4-4: syntax error: expected , but found ''
(syntax)
⏰ Context from checks skipped due to timeout of 90000ms (25)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (10, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (9, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 10)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 10)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 10)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (8)
hosting/k8s/helm/templates/_helpers.tpl (1)
1-253: Comprehensive helper template looks well-structured.The helper template provides a robust set of utility functions for:
- Consistent naming and labeling across resources
- Configurable image references with registry override support
- Secure connection string generation with proper SSL configuration
- Flexible service account and authentication handling
The implementation follows Helm best practices and enables proper separation of concerns.
hosting/k8s/helm/templates/ingress.yaml (1)
1-51: Well-structured ingress template with proper conditional rendering.The ingress template includes:
- Conditional deployment based on
.Values.ingress.enabled- Proper TLS configuration with multiple host support
- Flexible path configuration with sensible defaults
- Integration with cert-manager and external-dns through helper functions
The implementation follows Kubernetes ingress best practices.
hosting/k8s/helm/templates/postgresql.yaml (2)
50-55: PostgreSQL configuration includes logical replication setup.The PostgreSQL container is configured with
wal_level=logical, which is essential for logical replication features. This shows proper consideration for advanced PostgreSQL use cases.
1-138: Comprehensive PostgreSQL deployment with proper service configuration.The template provides:
- StatefulSet with configurable persistence and resources
- Proper security context and probe configurations
- Both headless and standard services for different access patterns
- Consistent labeling and naming through helper functions
The implementation follows StatefulSet best practices for database deployments.
hosting/k8s/helm/templates/tests/test-clickhouse.yaml (1)
1-21: Well-implemented Helm test for ClickHouse health verification.The test provides:
- Conditional execution based on ClickHouse configuration
- Secure credential handling
- Clear test output with status messages
- Proper Helm test hook annotation
The implementation ensures reliable health checking for the ClickHouse service.
hosting/k8s/helm/templates/tests/test-webapp.yaml (1)
1-19: Simple and effective webapp health check test.The test provides:
- Health endpoint verification via HTTP request
- Clear test output with status messages
- Proper Helm test hook annotation
- Pinned image version for reliability
The implementation provides reliable health verification for the webapp service.
hosting/k8s/helm/templates/secrets.yaml (1)
27-27: 👍 Trim fix applied – resolves prior basic-auth issueThe newline-trimming we flagged previously is now in place.
No further action needed here.hosting/k8s/helm/templates/supervisor.yaml (1)
80-81: Image tag lacks immutability
busybox:1.35is mutable in public registries. Pin to a digest (or distroless) to guarantee reproducible builds and avoid surprise CVE regressions.Example:
- image: busybox:1.35 + image: busybox@sha256:<digest>
2 participants
No description provided.