-
Notifications
You must be signed in to change notification settings - Fork 156
Creation of Non-Operator Helm Chart #513
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
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 PR introduces a new Helm chart for Mattermost Enterprise Edition designed for restrictive environments where the Mattermost Operator cannot be deployed due to RBAC limitations. The chart requires external PostgreSQL database and S3-compatible storage, operating entirely within namespace-level permissions.
Key changes:
- Complete standalone Helm chart implementation without operator dependency
- External database and S3-compatible file storage configuration support
- Optional features including dedicated job server, HPA, and NGINX ingress
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| charts/mattermost/Chart.yaml | Chart metadata defining Mattermost v2.7.0 for app v11.0.4 |
| charts/mattermost/.helmignore | Standard Helm ignore patterns for VCS and IDE files |
| charts/mattermost/README.md | Comprehensive installation and configuration documentation |
| charts/mattermost/ci/default-values.yaml | CI test configuration with minimal replica settings |
| charts/mattermost/templates/NOTES.txt | Post-installation instructions and access methods |
| charts/mattermost/templates/_helpers.tpl | Helm template helper functions for naming and API versions |
| charts/mattermost/templates/deployment-mattermost-app.yaml | Main Mattermost application deployment with health probes and configuration |
| charts/mattermost/templates/deployment-mattermost-hpa.yaml | Optional HorizontalPodAutoscaler for automatic scaling |
| charts/mattermost/templates/deployment-mattermost-jobserver.yaml | Optional dedicated job server deployment |
| charts/mattermost/templates/ingress-mattermost-app.yaml | Optional ingress configuration with TLS support |
| charts/mattermost/templates/secret-mattermost-dbsecret.yaml | Database connection secret template |
| charts/mattermost/templates/secret-mattermost-license.yaml | Mattermost license secret template |
| charts/mattermost/templates/service-account.yaml | ServiceAccount with workload identity annotation support |
| charts/mattermost/templates/service-mattermost-app.yaml | Service exposing app, metrics, cluster, and gossip ports |
| charts/mattermost/templates/tests/mattermost-app-config-test.yaml | ConfigMap containing helm test for Mattermost availability |
| charts/mattermost/templates/tests/mattermost-app-test.yaml | Helm test pod using BATS framework |
| charts/mattermost/values.yaml | Default configuration values for all chart components |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…r clarity, bump appVersion to 11.1.1, and set resource limits and requests.
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.
This PR is being reviewed by Cursor Bugbot
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| {{- end }} | ||
| {{- with .Values.global.features.jobserver.extraEnv }} | ||
| {{- toYaml . | nindent 8 }} | ||
| {{- end }} |
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.
Bug: Job server missing file store configuration environment variables
The job server deployment only receives the MM_CONFIG database connection but lacks the S3 file store environment variables (MM_FILESETTINGS_*) that the main app deployment configures. Since these environment variables are runtime overrides and not persisted to the database config, background jobs requiring file storage access (exports, data retention, file cleanup) will fail or use incorrect settings when the dedicated job server is enabled.
Additional Locations (1)
| {{- else }} | ||
| name: {{ include "mattermost.fullname" . }}-mattermost-dbsecret | ||
| key: mattermost.dbsecret | ||
| {{- end }} |
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.
Bug: Inconsistent existingDatabaseSecret condition causes empty secret reference
The condition if .Values.global.features.database.existingDatabaseSecret checks if the map object exists rather than if existingDatabaseSecret.name has a value. Since values.yaml defines existingDatabaseSecret as a map with name: and key: keys (with nil values), the map is non-empty and truthy in Go templates. This causes deployments to use empty name and key values in the secretKeyRef, failing Kubernetes validation. The secret-mattermost-dbsecret.yaml correctly checks .existingDatabaseSecret.name, creating an inconsistency.
Additional Locations (1)
…t Kubernetes versions 1.25 and above, adding conditional metrics for CPU and memory utilization.
| run.sh: |- | ||
| #!/usr/bin/env bats | ||
| @test "Testing Mattermost is accessible" { | ||
| url="http://{{ include "mattermost.fullname" . }}.{{ .Release.Namespace }}.svc.cluster.local:{{ .Values.mattermostApp.service.internalPort }}" |
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.
Bug: Test uses wrong port when connecting to Service
The Helm test URL connects to the Kubernetes Service using internalPort but the Service listens on externalPort. When a client connects to .svc.cluster.local, it uses the Service's exposed port (externalPort), not the container's target port (internalPort). This works by coincidence when both ports equal 8065 (the default), but if externalPort is configured differently (e.g., port 80), the test will fail because it tries to connect to the wrong port.
|
@angeloskyratzakos @andrleite Thanks for your prior review, I've added the example values for GDC. Could you do a re-review before I merge it later today? |
Summary
This PR introduces a new non-operator Helm chart for Mattermost Enterprise Edition, designed for environments with restricted RBAC permissions where the Mattermost Operator cannot be used. The chart provides a lightweight alternative that operates within namespace-only permissions and requires external PostgreSQL database and S3-compatible file storage.
Key features:
The chart includes:
Ticket Link
https://mattermost.atlassian.net/browse/SEC-9017
Note
Adds a standalone Mattermost EE Helm chart with optional ingress, jobserver, HPA, and secrets-based configuration.
charts/mattermostDeployment(app),Service, optionalIngress, optionalHPA, optionalServiceAccount.Deploymentcontrolled viaglobal.features.jobserver.Secrettemplates;_helpers.tpl,NOTES.txt.ConfigMap+ testPod.values.yamlwith site URL, image, autoscaling, S3 file store, env/volumes; CI defaults inci/default-values.yaml.README.mdfor install, config, scaling, and troubleshooting.Written by Cursor Bugbot for commit 2ae0e00. This will update automatically on new commits. Configure here.