322 task create helm subchart for rspamd service#324
322 task create helm subchart for rspamd service#324Aravinda-HWK wants to merge 9 commits intoLSFLK:mainfrom
Conversation
….yaml for persistence handling
- Introduced Redis chart with service, statefulset, and service account templates. - Added Rspamd chart including service, statefulset, config maps, and network policies. - Implemented liveness and readiness probes for both Redis and Rspamd. - Configured persistence for Redis and Rspamd with volume claim templates. - Created helper templates for both charts to manage naming and labels. - Added README files for both charts with installation and configuration instructions. - Established dependencies between Rspamd and Redis, Unbound, and ClamAV.
… checks and security context
…lates (LSFLK#320) * Create Helm subchart for OpenDKIM service with configuration and templates * Add detailed local testing instructions for OpenDKIM chart * Refactor OpenDKIM Helm chart: remove PVC template and adjust workload.yaml for persistence handling
….yaml for persistence handling
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces three new Helm subcharts—Redis, Unbound, and Rspamd—to provide a comprehensive spam and malware filtering solution. The review identified several critical issues and improvement opportunities: the Rspamd configuration incorrectly attempts to use a hostname in a resolv.conf override, which requires an IP address; the Unbound chart runs with excessive root and privileged security contexts; and the Redis implementation lacks the clustering logic necessary to support more than one replica safely. Furthermore, feedback suggests quoting secret values to prevent parsing errors, implementing more robust timeout parsing in shell scripts, and avoiding the latest image tag to ensure deterministic deployments.
| resolv.conf: | | ||
| nameserver {{ .Values.dependencies.unbound.host }} |
There was a problem hiding this comment.
The nameserver directive in /etc/resolv.conf requires an IP address. Using a hostname like {{ .Values.dependencies.unbound.host }} is invalid and will cause DNS resolution to fail. Additionally, overriding the pod's resolv.conf via subPath is generally discouraged as it breaks standard Kubernetes service discovery and does not receive updates if the ConfigMap changes.
| runAsUser: 0 | ||
| runAsNonRoot: false | ||
|
|
||
| securityContext: | ||
| privileged: true | ||
| allowPrivilegeEscalation: true | ||
| runAsUser: 0 |
There was a problem hiding this comment.
Running Unbound as root (runAsUser: 0) with privileged: true is a significant security risk. Containers should run as non-root whenever possible. If binding to port 53 is required, use the NET_BIND_SERVICE capability instead of full privileges, or run Unbound on a high port and map it via the Service.
| @@ -0,0 +1,73 @@ | |||
| replicaCount: 1 | |||
There was a problem hiding this comment.
The Redis chart is implemented as a basic StatefulSet without clustering or sentinel logic. If replicaCount is set to a value greater than 1, it will result in multiple independent Redis instances, which will cause data inconsistency for Rspamd. This value should remain 1 unless a proper high-availability configuration is implemented.
| {{- include "rspamd.labels" . | nindent 4 }} | ||
| type: Opaque | ||
| stringData: | ||
| password: {{ .Values.webui.password }} |
There was a problem hiding this comment.
The password value should be quoted to ensure that special characters do not cause YAML parsing errors.
password: {{ .Values.webui.password | quote }}| - -c | ||
| - | | ||
| echo "Checking Redis connectivity at {{ .Values.dependencies.redis.host }}:{{ .Values.dependencies.redis.port }}" | ||
| timeout_sec={{ .Values.dependencies.initCheckTimeout | replace "s" "" }} |
There was a problem hiding this comment.
The replace "s" "" logic is fragile. If the initCheckTimeout value uses a different unit (like '1m') or has no unit, the shell script will fail with a syntax error during the numeric comparison. Consider enforcing a numeric value in seconds or using a more robust parsing method.
|
|
||
| image: | ||
| repository: rspamd/rspamd | ||
| tag: "latest" |
There was a problem hiding this comment.
Using the latest tag for container images is discouraged in production as it makes deployments non-deterministic. It is better to use a specific version tag.
1 participant
📌 Description
This PR is to create the rspamd helm chart along with unbound and redis charts.
🔍 Changes Made
✅ Checklist (Email System)
🧪 Testing Instructions
📷 Screenshots / Logs (if applicable)