[INFRA-119] Plane-EE: feat: Add Email Service#98
Conversation
## Walkthrough
This update introduces a configurable email service to the plane-enterprise Helm chart. It adds new values, questions, and conditional Kubernetes manifests for deploying, configuring, and securing the email service. The chart version is incremented, and resources for certificates, secrets, config maps, and deployments are included, all gated by an enablement flag. Additionally, certificate issuer creation logic is extended to consider email service enablement.
## Changes
| File(s) | Change Summary |
|------------------------------------------------------------------------------------------------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| charts/plane-enterprise/Chart.yaml | Incremented chart version from 1.2.2 to 1.2.3. |
| charts/plane-enterprise/questions.yml | Added a new "Email Service" configuration section with variables for image, enablement, resource limits, environment variables, and SSL settings. Conditional subquestions and visibility logic included. |
| charts/plane-enterprise/values.yaml | Introduced `email_service` configuration under `services` and `email_service_envs` under `env` for email service enablement, resource limits, image, webhook URL, and SMTP domain. |
| charts/plane-enterprise/templates/certs/email-certs.yaml | Added a new template for a cert-manager `Certificate` resource, conditionally created for the email service when enabled and an SMTP domain is specified. |
| charts/plane-enterprise/templates/config-secrets/email-env.yaml | Added a template for a Kubernetes ConfigMap for the email service, conditionally created. The ConfigMap stores SMTP domain and embedded blacklist/spam files, plus fixed internal API URLs. |
| charts/plane-enterprise/templates/workloads/email.deployment.yaml | Added a conditional template for deploying the email service, including a Service (LoadBalancer) and Deployment with configurable image, resources, environment, volume mounts, and annotations for reloads. |
| charts/plane-enterprise/templates/certs/cert-issuers.yaml | Modified conditional logic for rendering certificate issuer secret and issuer resources to include email service enablement alongside ingress enablement. |
## Sequence Diagram(s)
```mermaid
sequenceDiagram
participant User
participant Helm
participant Kubernetes API
participant Email Service Pod
participant ConfigMap/Secret
participant Cert-Manager
User->>Helm: Enable email_service and set config
Helm->>Kubernetes API: Deploy email Service, Deployment, ConfigMap, Certificate (if enabled)
Kubernetes API->>ConfigMap/Secret: Create email-env resources
Kubernetes API->>Cert-Manager: Create Certificate for SMTP domain (if needed)
Kubernetes API->>Email Service Pod: Start pod with mounted secrets, config, and certs
Email Service Pod->>User: Ready to handle email traffic (SMTP/SMTPS)Suggested reviewers
Poem
|
akshat5302
changed the title
|
Pull Request Linked with Plane Work Items
Comment Automatically Generated by Plane |
akshat5302
changed the title
…to add-email-service
…es to use artifacts.plane.so for consistency across configurations.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (11)
charts/plane-enterprise/templates/certs/email-certs.yaml (1)
10-10: Remove trailing whitespace
Line 10 contains trailing spaces which YAMLLint flags; please remove them to avoid formatting errors. Also add a newline at EOF to satisfy POSIX conventions.🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 10-10: trailing spaces
(trailing-spaces)
charts/plane-enterprise/values.yaml (1)
154-160: Add standard pullPolicy and cluster IP settings
All other services define apullPolicy(e.g.IfNotPresent) andassign_cluster_ipflag. Consider extending the email service block for consistency:email_service: enabled: false # or true, per above replicas: 1 memory_limit: 1000Mi cpu_limit: 500m image: artifacts.plane.so/makeplane/email-commercial pullPolicy: IfNotPresent # ← new assign_cluster_ip: false # ← newcharts/plane-enterprise/templates/config-secrets/email-env.yaml (1)
14-16: Remove trailing spaces and add EOF newline
Lines 14 and 15 end with extraneous spaces; please strip them. Also ensure the file ends with a single newline character.🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 14-14: trailing spaces
(trailing-spaces)
[error] 15-15: trailing spaces
(trailing-spaces)
charts/plane-enterprise/questions.yml (1)
71-71: Remove trailing whitespace
Line 71 has trailing spaces; please remove to adhere to YAMLLint conventions.🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 71-71: trailing spaces
(trailing-spaces)
charts/plane-enterprise/templates/workloads/email.deployment.yaml (7)
3-26: Service manifest review: Consider configurability of Service type.The Service resource exposes SMTP ports via a LoadBalancer, which may incur cloud-provider costs. To increase flexibility, consider making the Service
typeconfigurable (e.g.,.Values.services.email_service.serviceType) with a sensible default (LoadBalancer) so users can opt forClusterIPorNodePortin different environments.
28-36: Deployment metadata: Addmetadata.labelsfor Deployment.Best practice dictates that
Deployment.metadata.labelsmatchspec.selector.matchLabelsandspec.template.metadata.labels, which improves discoverability in dashboards and promotes consistency. Consider adding:metadata: labels: app.name: {{ .Release.Namespace }}-{{ .Release.Name }}-email-appunder the
Deploymentmetadata block.
44-45: Remove trailing spaces and fix indentation underlabels.Lines 44 and 45 have trailing whitespace and misaligned indentation. Align the
app.nameentry two spaces underlabelsand remove trailing spaces:- labels: - app.name: {{ .Release.Namespace }}-{{ .Release.Name }}-email-app + labels: + app.name: {{ .Release.Namespace }}-{{ .Release.Name }}-email-app🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 44-44: trailing spaces
(trailing-spaces)
[error] 45-45: trailing spaces
(trailing-spaces)
69-77: VolumeMount indentation: Fix alignment undervolumeMounts.Static analysis flagged inconsistent indentation at lines 70–77. Align all volume mounts two spaces under
volumeMounts:- volumeMounts: - - name: tls-cert - mountPath: /opt/email/keys + volumeMounts: + - name: tls-cert + mountPath: /opt/email/keys ...Ensure each
- name:entry is indented exactly two spaces undervolumeMounts.🧰 Tools
🪛 YAMLlint (1.37.1)
[warning] 70-70: wrong indentation: expected 8 but found 10
(indentation)
78-86: Volume spec indentation and secretName consistency.The
volumesblock has incorrect indentation (line 79). Also, double-check thatsecretName: {{ .Release.Name }}-mail-tls-secretmatches the name used in the certificate template (email-certs.yaml). Align indentation so that:volumes: - name: spam-blacklist configMap: name: {{ .Release.Name }}-email-vars - name: tls-cert secret: secretName: {{ .Release.Name }}-mail-tls-secret items: - key: tls.crt path: cert.pem - key: tls.key path: key.pem optional: false🧰 Tools
🪛 YAMLlint (1.37.1)
[warning] 79-79: wrong indentation: expected 6 but found 8
(indentation)
91-92: Remove deprecatedserviceAccountfield.
serviceAccountNameis sufficient for binding a ServiceAccount to the Pod. TheserviceAccountfield is deprecated and can be removed to avoid confusion:- serviceAccount: {{ .Release.Name }}-srv-account serviceAccountName: {{ .Release.Name }}-srv-account
94-94: Add newline at end of file.YAML files should end with a newline to satisfy POSIX standards and tools like
yamllint. Please add an empty line after{{- end }}.🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 94-94: no new line character at the end of file
(new-line-at-end-of-file)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (6)
charts/plane-enterprise/Chart.yaml(1 hunks)charts/plane-enterprise/questions.yml(2 hunks)charts/plane-enterprise/templates/certs/email-certs.yaml(1 hunks)charts/plane-enterprise/templates/config-secrets/email-env.yaml(1 hunks)charts/plane-enterprise/templates/workloads/email.deployment.yaml(1 hunks)charts/plane-enterprise/values.yaml(2 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
charts/plane-enterprise/templates/certs/email-certs.yaml
[error] 10-10: trailing spaces
(trailing-spaces)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
[error] 15-15: no new line character at the end of file
(new-line-at-end-of-file)
charts/plane-enterprise/templates/config-secrets/email-env.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
[error] 14-14: trailing spaces
(trailing-spaces)
[error] 15-15: trailing spaces
(trailing-spaces)
[error] 36-36: no new line character at the end of file
(new-line-at-end-of-file)
charts/plane-enterprise/questions.yml
[error] 71-71: trailing spaces
(trailing-spaces)
charts/plane-enterprise/templates/workloads/email.deployment.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
[error] 44-44: trailing spaces
(trailing-spaces)
[error] 45-45: trailing spaces
(trailing-spaces)
[warning] 63-63: wrong indentation: expected 8 but found 10
(indentation)
[warning] 70-70: wrong indentation: expected 8 but found 10
(indentation)
[warning] 79-79: wrong indentation: expected 6 but found 8
(indentation)
[error] 94-94: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (3)
charts/plane-enterprise/Chart.yaml (1)
8-8: Approve version bump
The chart version has been correctly incremented from 1.2.2 to 1.2.3 to reflect the new email service additions.charts/plane-enterprise/templates/workloads/email.deployment.yaml (2)
1-2: Conditional rendering check: Ensure Helm conditional is correct.The
{{- if .Values.services.email_service.enabled }}directive properly gates the Service and Deployment resources. This matches the intended enablement flag and follows Helm templating best practices.🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
62-68: Verify environment ConfigMap/Secret names match.Ensure that
{{ .Release.Name }}-email-varsand{{ .Release.Name }}-email-secretsare created in theemail-env.yamltemplate and include the expected keys (spam-blacklists and credentials). A mismatch here could lead to pod startup failures.🧰 Tools
🪛 YAMLlint (1.37.1)
[warning] 63-63: wrong indentation: expected 8 but found 10
(indentation)
| containers: | ||
| - name: {{ .Release.Name }}-email-app | ||
| image: {{ .Values.services.email_service.image | default "artifacts.plane.so/makeplane/email-commercial" }}:{{ .Values.planeVersion }} | ||
| securityContext: | ||
| runAsUser: 100 | ||
| stdin: true | ||
| tty: true | ||
| resources: | ||
| requests: | ||
| memory: "50Mi" | ||
| cpu: "50m" | ||
| limits: | ||
| memory: {{ .Values.services.email_service.memory_limit | default "1000Mi" | quote }} | ||
| cpu: {{ .Values.services.email_service.cpu_limit | default "500m" | quote}} | ||
| envFrom: |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding health probes and configurable resource requests.
Currently, the container lacks liveness/readiness probes and has hardcoded resource requests. For production readiness, add livenessProbe and readinessProbe, and make the requests (CPU/memory) configurable via chart values (e.g., .Values.services.email_service.requests).
🤖 Prompt for AI Agents
In charts/plane-enterprise/templates/workloads/email.deployment.yaml between
lines 48 and 62, the container definition lacks liveness and readiness probes
and has hardcoded resource requests. To fix this, add `livenessProbe` and
`readinessProbe` sections with appropriate probe configurations, and update the
resource requests for CPU and memory to be configurable via chart values such as
`.Values.services.email_service.requests.cpu` and
`.Values.services.email_service.requests.memory` instead of hardcoded values.
…and questions, update email secrets to use ConfigMap, and add readiness probe to email deployment.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
charts/plane-enterprise/questions.yml (1)
432-476: Validate SSL question variables usage
The SSL fields (ssl.issuer,ssl.server,ssl.email,ssl.token) are defined at the top level and will collide with the global ingressssl.*settings. Please confirm whether the email service should consume the globalssl.*values or introduce a dedicatedservices.email_service.ssl.*block—and updateemail-certs.yamlaccordingly.charts/plane-enterprise/templates/workloads/email.deployment.yaml (1)
55-73: Add livenessProbe and make requests configurable
For production readiness, include alivenessProbealongside the existingreadinessProbeand expose resourcerequestsvia chart values (e.g.,.Values.services.email_service.requests.cpu/.memory) instead of hardcoding them.
🧹 Nitpick comments (5)
charts/plane-enterprise/templates/config-secrets/email-env.yaml (2)
10-11: Remove trailing whitespace
There's an extra space at the end of theEMAIL_SAVE_ENDPOINTline that may trigger lint warnings.
22-22: Add newline at end of file
Ensure the file ends with a newline to satisfy POSIX conventions and avoid linter errors.🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 22-22: no new line character at the end of file
(new-line-at-end-of-file)
charts/plane-enterprise/questions.yml (1)
71-71: Remove trailing whitespace
There is a trailing space on this blank line that can cause lint warnings.🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 71-71: trailing spaces
(trailing-spaces)
charts/plane-enterprise/templates/workloads/email.deployment.yaml (2)
44-45: Remove trailing whitespace
There are trailing spaces on these lines which may lead to lint failures; please clean them up.🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 44-44: trailing spaces
(trailing-spaces)
[error] 45-45: trailing spaces
(trailing-spaces)
102-102: Add newline at end of file
Ensure the file ends with a newline to comply with POSIX standards and avoid linter warnings.🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 102-102: no new line character at the end of file
(new-line-at-end-of-file)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
charts/plane-enterprise/questions.yml(2 hunks)charts/plane-enterprise/templates/config-secrets/email-env.yaml(1 hunks)charts/plane-enterprise/templates/workloads/email.deployment.yaml(1 hunks)charts/plane-enterprise/values.yaml(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- charts/plane-enterprise/values.yaml
🧰 Additional context used
🪛 YAMLlint (1.37.1)
charts/plane-enterprise/templates/workloads/email.deployment.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
[error] 44-44: trailing spaces
(trailing-spaces)
[error] 45-45: trailing spaces
(trailing-spaces)
[warning] 74-74: wrong indentation: expected 8 but found 10
(indentation)
[warning] 78-78: wrong indentation: expected 8 but found 10
(indentation)
[warning] 87-87: wrong indentation: expected 6 but found 8
(indentation)
[error] 102-102: no new line character at the end of file
(new-line-at-end-of-file)
charts/plane-enterprise/questions.yml
[error] 71-71: trailing spaces
(trailing-spaces)
charts/plane-enterprise/templates/config-secrets/email-env.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
[error] 20-20: trailing spaces
(trailing-spaces)
[error] 22-22: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (3)
charts/plane-enterprise/templates/config-secrets/email-env.yaml (1)
3-13: ConfigMap resource definition is correct
The ConfigMap is conditionally created and properly populates SMTP domain, internal API endpoints, and embedded blacklists when the email service is enabled.charts/plane-enterprise/questions.yml (1)
63-68: Approve new Docker image variable
Theservices.email_service.imageentry is correctly added as a required subquestion under the Plane Version group, matching existing service definitions.charts/plane-enterprise/templates/workloads/email.deployment.yaml (1)
3-13: Service resource definition is correct
The LoadBalancer service exposing SMTP ports withexternalTrafficPolicy: Localis well-configured and aligns with best practices for email ingress.
…ingress or email service configuration
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
charts/plane-enterprise/templates/certs/cert-issuers.yaml (1)
1-1: YAMLLint False Positive on Helm Directive
YAMLLint may flag the leading{{- if … }}on column 1 as a syntax error. To keep CI clean, consider disabling YAMLLint for this template or adding a file-level directive, e.g.:# yamllint disable🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
charts/plane-enterprise/templates/certs/cert-issuers.yaml(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
charts/plane-enterprise/templates/certs/cert-issuers.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
| @@ -1,4 +1,4 @@ | |||
| {{- if and .Values.ingress.enabled .Values.ssl.createIssuer (empty .Values.ssl.tls_secret_name) }} | |||
| {{- if and (or .Values.ingress.enabled .Values.services.email_service.enabled) .Values.ssl.createIssuer (empty .Values.ssl.tls_secret_name) }} | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Conditional Logic Expanded to Include Email Service
The addition of .Values.services.email_service.enabled to the or clause correctly extends the ACME issuer provisioning to the new email-service component without breaking the existing ingress path.
Please verify that the path services.email_service.enabled is defined in your values.yaml. You can run:
🏁 Script executed:
#!/bin/bash
# Confirm the email_service flag path in values.yaml
rg -n 'services\.email_service\.enabled' charts/plane-enterprise/values.yamlLength of output: 78
Let's search for any email_service or email-service keys in your values file:
🏁 Script executed:
#!/bin/bash
# Look for email service flags in values.yaml
echo "Checking for 'email_service' occurrences:"
rg -n 'email_service' -C2 charts/plane-enterprise/values.yaml
echo
echo "Checking for 'email-service' occurrences:"
rg -n 'email-service' -C2 charts/plane-enterprise/values.yamlLength of output: 519
Incorrect Values Path in Certificate Issuer Conditional
It looks like the template is checking .Values.services.email_service.enabled, but your values.yaml defines email_service at the root, not under services. This means the email‐service branch will never be picked up.
Please update one of the following:
- Move
email_service:under aservices:block incharts/plane-enterprise/values.yaml, or - Change the template at
charts/plane-enterprise/templates/certs/cert-issuers.yamlto:- {{- if and (or .Values.ingress.enabled .Values.services.email_service.enabled) .Values.ssl.createIssuer (empty .Values.ssl.tls_secret_name) }} + {{- if and (or .Values.ingress.enabled .Values.email_service.enabled) .Values.ssl.createIssuer (empty .Values.ssl.tls_secret_name) }}
Locations to check:
- charts/plane-enterprise/values.yaml (ensure the path you choose matches the template)
- charts/plane-enterprise/templates/certs/cert-issuers.yaml (update the conditional)
📝 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.
| {{- if and (or .Values.ingress.enabled .Values.services.email_service.enabled) .Values.ssl.createIssuer (empty .Values.ssl.tls_secret_name) }} | |
| {{- if and (or .Values.ingress.enabled .Values.email_service.enabled) .Values.ssl.createIssuer (empty .Values.ssl.tls_secret_name) }} |
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
🤖 Prompt for AI Agents
In charts/plane-enterprise/templates/certs/cert-issuers.yaml at line 1, the
conditional references .Values.services.email_service.enabled, but your
values.yaml defines email_service at the root level, not under services. To fix
this, either move the email_service block under services in
charts/plane-enterprise/values.yaml or update the conditional in
cert-issuers.yaml to check .Values.email_service.enabled instead, ensuring the
paths are consistent between the template and values file.
…ingress configuration
charts/plane-enterprise/templates/config-secrets/email-env.yaml
Outdated
Show resolved
Hide resolved
2 participants
Description
Type of Change
Summary by CodeRabbit