-
Couldn't load subscription status.
- Fork 6
SLT-1103: Drupal chart: Add support for mailpit, deprecate mailhog #741
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…ng drupal.env as otherwise mailpit deployment has incorrect name and kubernetes services are not created.
…release-name>-mailpit-smtp:25 as that's the service/port that mailpit creates for SMTP communications.
…is not created and php containers fail to start.
…th to mailpit-http service.
…going through otherwise.
…ginx config to proxy mailpit-http under Drupal's /mailpit path.
…just varnish rules.
…ifies mailpit and mailhog are not enabled at the same time. Add deprecation notes for mailhog.
5255230 to
265f89b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This pull request replaces Mailhog with Mailpit in the Drupal chart to address maintenance concerns and ensure a stable email testing service in development environments. Key changes include:
- Adding Mailpit configuration and dependency while deprecating Mailhog.
- Updating chart templates, tests, and configuration files to support Mailpit.
- Adding validation rules to prevent dual activation and production use of deprecated services.
Reviewed Changes
Copilot reviewed 11 out of 14 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| silta/silta.yml | Added Mailpit service flag |
| charts/drupal/values.yaml | Introduced Mailpit configuration values and overrides |
| charts/drupal/tests/drupal_deployment_test.yaml | Updated test cases to refer to Mailpit |
| charts/drupal/test.values.yaml | Changed Mailhog to Mailpit in test values |
| charts/drupal/templates/varnish-configmap-vcl.yaml | Added proxy bypass for Mailpit URLs |
| charts/drupal/templates/smtp-secret.yaml | Modified condition to include Mailpit |
| charts/drupal/templates/drupal-configmap.yaml | Added redirect from legacy Mailhog to Mailpit |
| charts/drupal/templates/checks.yaml | Inserted checks to prevent enabling both services simultaneously and blocking deprecated usage in production |
| charts/drupal/Chart.yaml | Added Mailpit as a chart dependency |
| .circleci/config.yml | Added Helm repo for the new Mailpit chart dependency |
Files not reviewed (3)
- charts/drupal/templates/NOTES.txt: Language not supported
- charts/drupal/templates/_helpers.tpl: Language not supported
- charts/drupal/values.schema.json: Language not supported
Comments suppressed due to low confidence (1)
charts/drupal/values.yaml:757
- [nitpick] The nested 'mailpit' key inside the main Mailpit configuration block may be confusing. Consider renaming it (e.g., to 'webrootConfig' or similar) for clearer structure.
mailpit:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RTBC. Nice work, thanks @k4lv15 ! 👍
/var/www/html$ env | grep SMTP_ADDRESS
SMTP_ADDRESS=feature-slt-1103-mail-mailpit-smtp:25
Motivation:
Mailhog is an email service we’re using in our development environments (local & Silta non-production) to test out if email functionalities are working properly. Mailhog is not maintained anymore and is causing issues for developers. Therefore, we need to replace it with some alternative. Our current pick is Mailpit.
Changes proposed:
How to test:
helm install drupal-release charts/drupal --set mailpit.enabled=trueor test here: https://app.circleci.com/pipelines/github/wunderio/drupal-project-k8s/6873/workflows/20d2da53-257e-4bc2-8d2e-85bbb7c3eae2/jobs/22713). Test the following:helm unittest ./charts/drupal)/mailhogpath redirects to/mailpitmailpitandmailhogenabled, see that the installation fails (or see screenshot below)