-
Notifications
You must be signed in to change notification settings - Fork 4
feat(formance): add unified Helm chart for CE and EE deployments #275
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughAdds a Formance Helm chart: new NOTES post-install template and helpers for naming/labels and Enterprise Edition validation, plus chart README and root README entry update; removes demo chart README, Earthfile, and a .gitignore line. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
Comment |
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
charts/formance/README.md (1)
227-570: Fix markdown formatting issues in "Other Values" section.The static analysis flagged two markdown issues:
- Line 307: Bare URL (should be wrapped in markdown link syntax
[text](url))- Line 543: Table column count mismatch (5 cells instead of 4)
Additionally, the "Other Values" section contains numerous entries (lines 231–570) with missing or empty descriptions. These appear to be auto-generated schema artifacts that reduce documentation clarity. Since this README is auto-generated by helm-docs, consider either:
- Adding descriptions to these entries in the source values.yaml or values.schema.json
- Filtering out zero-description entries during generation to keep the README focused on documented values
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
charts/formance/values.schema.jsonis excluded by!**/*.json
📒 Files selected for processing (2)
README.mdcharts/formance/README.md
🧰 Additional context used
🪛 LanguageTool
charts/formance/README.md
[grammar] ~1-~1: Ensure spelling is correct
Context: # formance 
[style] ~111-~111: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...---------| | global.platform | object | `{"consoleV3":{"enabled":true,"host":"console.{{ .Values.global.serviceHost }}","scheme":"https"},"membership":{"host":"membership.{{ .Values.global.serviceHost }}","oidc":{"host":"dex.{{ .Values.global.serviceHost }}","scheme":"https"...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🪛 markdownlint-cli2 (0.18.1)
charts/formance/README.md
307-307: Bare URL used
(MD034, no-bare-urls)
543-543: Table column count
Expected: 4; Actual: 5; Too many cells, extra data will be missing
(MD056, table-column-count)
🔇 Additional comments (1)
README.md (1)
12-12: LGTM!The new Formance chart entry is correctly formatted, properly positioned in the table, and includes all required metadata (version 1.0.0, description, and Artifact Hub badge). The addition follows the established pattern for other chart entries.
Add a new unified 'formance' chart that simplifies deployment by combining: - regions (always installed): operator, operator-crds, versions - cloudprem (EE only): control-plane (membership, portal, console-v3) Features: - Single chart for both Community and Enterprise editions - Centralized configuration in values.yaml - EE validation (requires licence.token and licence.clusterID) - Helm tests for operator and control-plane health checks - Profiles for minimal CE and EE configurations - JSON Schema for values validation Usage: - CE: helm install formance formance/formance - EE: helm install formance formance/formance --set ee.enabled=true ... Refs: EN-197
Generated by pre-commit hooks: - Add README.md for formance chart (helm-docs) - Update values.schema.json (helm schema) - Update root README.md with formance chart reference Refs: EN-197
7c70f17 to
ab22c97
Compare
Add post-install/post-upgrade hooks to Versions resources to ensure they are created after the operator CRDs are installed. This fixes the "no matches for kind Versions" error during fresh installs. Refs: EN-197
- Remove standalone charts/demo in favor of integrated demo templates - Add demo templates (postgresql, stack, settings) to formance chart - Use official postgres:16-alpine image instead of Bitnami (now paid) - Remove postgresql bitnami dependency from Chart.yaml - Fix postgresql StatefulSet template for proper YAML structure BREAKING CHANGE: charts/demo is removed, use formance chart with demo.enabled=true and postgresql.enabled=true instead
- Add wait-for-postgresql hook to ensure DB is ready before stack creation - Re-add helm hooks to demo CRDs (without hook-delete-policy to avoid blocking) - Enable postgresql by default (only need --set demo.enabled=true now) - Add replicas setting (default: 2) for demo deployments - Remove payments module from demo (requires temporal.dsn) - Update NOTES.txt to show demo stack info when demo mode is enabled
- Move PostgreSQL from demo/ to root templates (shared by all) - Configure cloudprem.membership to use formance's PostgreSQL - Set global.postgresql.host to point to our PostgreSQL - Add ingress configuration to demo Gateway when serviceHost is set - Update NOTES.txt to show demo stack URL - Simplify settings.yaml (same PostgreSQL for CE and EE) This ensures a single PostgreSQL instance is used by: - Demo stack (Ledger, Gateway) - Membership/Dex (in EE mode)
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
charts/formance/README.md (1)
340-360: Fix bare URL in documentation table.Line 345 contains an unformatted URL that should be wrapped in Markdown link syntax. The static analysis tool flagged this as a style violation (MD034, no-bare-urls).
🔧 Proposed fix
-| cloudprem.membership.config.auth.tokenValidity | object | `{"accessToken":"5m","refreshToken":"72h"}` | According to "nsuµmh" And https://github.com/spf13/cast/blob/e9ba3ce83919192b29c67da5bec158ce024fdcdb/caste.go#L61C3-L61C3 | +| cloudprem.membership.config.auth.tokenValidity | object | `{"accessToken":"5m","refreshToken":"72h"}` | According to "nsuµmh" and [cast library](https://github.com/spf13/cast/blob/e9ba3ce83919192b29c67da5bec158ce024fdcdb/caste.go#L61C3-L61C3) |
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (15)
charts/demo/Chart.lockis excluded by!**/*.lock,!**/*.lockcharts/demo/Chart.yamlis excluded by!**/*.yamlcharts/demo/templates/settings.yamlis excluded by!**/*.yamlcharts/demo/templates/stacks.yamlis excluded by!**/*.yamlcharts/demo/values.schema.jsonis excluded by!**/*.jsoncharts/demo/values.yamlis excluded by!**/*.yamlcharts/formance/Chart.lockis excluded by!**/*.lock,!**/*.lockcharts/formance/Chart.yamlis excluded by!**/*.yamlcharts/formance/templates/demo/settings.yamlis excluded by!**/*.yamlcharts/formance/templates/demo/stack.yamlis excluded by!**/*.yamlcharts/formance/templates/demo/wait-for-postgresql.yamlis excluded by!**/*.yamlcharts/formance/templates/postgresql.yamlis excluded by!**/*.yamlcharts/formance/values.schema.jsonis excluded by!**/*.jsoncharts/formance/values.yamlis excluded by!**/*.yamlcharts/regions/templates/versions.yamlis excluded by!**/*.yaml
📒 Files selected for processing (6)
README.mdcharts/demo/.gitignorecharts/demo/Earthfilecharts/demo/README.mdcharts/formance/README.mdcharts/formance/templates/NOTES.txt
💤 Files with no reviewable changes (3)
- charts/demo/.gitignore
- charts/demo/README.md
- charts/demo/Earthfile
🚧 Files skipped from review as they are similar to previous changes (1)
- README.md
🧰 Additional context used
🪛 LanguageTool
charts/formance/README.md
[style] ~111-~111: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...---------| | global.platform | object | `{"consoleV3":{"enabled":true,"host":"console.{{ .Values.global.serviceHost }}","scheme":"https"},"membership":{"host":"membership.{{ .Values.global.serviceHost }}","oidc":{"host":"dex.{{ .Values.global.serviceHost }}","scheme":"https"...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
charts/formance/templates/NOTES.txt
[style] ~57-~57: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...tps://{{ .Values.demo.stack.name }}.{{ .Values.global.serviceHost }} {{- end }} Check...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🪛 markdownlint-cli2 (0.18.1)
charts/formance/README.md
345-345: Bare URL used
(MD034, no-bare-urls)
582-582: Table column count
Expected: 4; Actual: 5; Too many cells, extra data will be missing
(MD056, table-column-count)
🔇 Additional comments (7)
charts/formance/README.md (1)
570-585: Verify table column alignment at line 582.The static analysis tool reports that line 582 has 5 cells when 4 are expected. This suggests a malformed table row that could break Markdown rendering. Please verify the row structure around this line is correct, as it appears to be in the "Other Values" section listing additional configuration keys.
charts/formance/templates/NOTES.txt (6)
1-11: Verify helper functions are defined in _helpers.tpl.The template uses
include "formance.validateEE"andinclude "formance.edition"at lines 1 and 10. Per the AI summary, these helpers should be defined incharts/formance/templates/_helpers.tpl, but this file is not included in the provided context. Ensure both helpers are properly implemented and tested.
13-33: LGTM: EE/CE conditional messaging is clear.The conditional logic properly distinguishes between Enterprise Edition and Community Edition deployments, with appropriate messaging for each mode. The warning about
global.serviceHostwhen URLs cannot be displayed is helpful for users.
45-67: LGTM: Demo stack section is well-structured.The demo section correctly gates on
demo.enabledand provides clear status-checking and access instructions. The conditional rendering of the access URL based onglobal.serviceHostmirrors the EE section's pattern for consistency.
69-78: LGTM: PostgreSQL section provides useful connection details.The PostgreSQL configuration output correctly gates on
postgresql.enabledand exposes the connection endpoint, username, and database name, making it easier for users to connect externally if needed.
80-99: LGTM: Stack creation guidance is helpful for CE users.The conditional block (when demo is not enabled) provides a clear YAML example for users to create their first stack. Versioning it with
v3.1aligns with the demo stack version default in the README, which is sensible for consistency.
101-116: LGTM: Closing sections provide clear next steps.The Helm tests, documentation links, and footer sections are well-organized and direct users to the right resources (docs, GitHub, support). URLs are correctly formatted in Markdown.
Summary
Add a new unified
formancechart that simplifies deployment by combining:Features
values.yamllicence.tokenandlicence.clusterID)Usage
Test plan
helm lintpasseshelm templategenerates correct resources for CEhelm templategenerates correct resources for EERefs: EN-197