feat!: remove bundled Bitnami PostgreSQL and MySQL sub-chart dependen…#295
feat!: remove bundled Bitnami PostgreSQL and MySQL sub-chart dependen…#295
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThe OpenFGA Helm chart version is bumped from 0.2.57 to 0.3.0, removing bundled PostgreSQL and MySQL subchart dependencies. Validation templates enforce configuration checks to prevent use of deprecated flags. New CI values files provide embedded Kubernetes manifests for MySQL and PostgreSQL deployments. Bitnami repository references are removed from GitHub workflows. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 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)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
The datastore.uriSecret template hardcodes the secret key to "uri", so the CI test secrets must use "uri" instead of "OPENFGA_DATASTORE_URI".
The post-install hook migration job creates a deadlock: Helm waits for the deployment to be ready before running hooks, but the deployment needs the migration to pass readiness checks. Using initContainer migration avoids this by running the migration inside the pod.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@charts/openfga/values.yaml`:
- Line 375: The example secret key name in values.yaml uses
OPENFGA_DATASTORE_URI but the chart (the helper that resolves
datastore.uriSecret) expects the secret key to be uri; update the example secret
entries to use key name "uri" (or add an "uri" key mapping) wherever
OPENFGA_DATASTORE_URI is shown so that the secret matches the
datastore.uriSecret behavior (ensure both occurrences are changed).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0917f666-be4c-41a7-8efa-255515de0f81
⛔ Files ignored due to path filters (1)
charts/openfga/Chart.lockis excluded by!**/*.lock
📒 Files selected for processing (8)
.github/workflows/release.yml.github/workflows/test.ymlcharts/openfga/Chart.yamlcharts/openfga/ci/mysql-values.yamlcharts/openfga/ci/postgres-values.yamlcharts/openfga/templates/_helpers.tplcharts/openfga/templates/validation.yamlcharts/openfga/values.yaml
💤 Files with no reviewable changes (2)
- .github/workflows/release.yml
- .github/workflows/test.yml
There was a problem hiding this comment.
Pull request overview
This PR updates the openfga Helm chart for a breaking v0.3.0 release by removing the bundled Bitnami PostgreSQL/MySQL subcharts and shifting users to external database provisioning.
Changes:
- Bumps chart version to
0.3.0and removes Bitnamipostgresql/mysqldependencies from the chart. - Adds Helm template validation to hard-fail upgrades that still set
postgresql.enabled/mysql.enabled. - Updates values/CI examples and CI workflows to reflect the removal of the Bitnami repo/dependencies.
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| charts/openfga/values.yaml | Removes subchart values and adds external DB dev/test examples under extraObjects. |
| charts/openfga/templates/validation.yaml | Adds upgrade-time validation to prevent silent ignores of removed subchart flags. |
| charts/openfga/templates/_helpers.tpl | Adjusts secret-creation helper after removing Bitnami dependency integration. |
| charts/openfga/ci/postgres-values.yaml | Adds CI install values to stand up a dev/test PostgreSQL via extraObjects. |
| charts/openfga/ci/mysql-values.yaml | Adds CI install values to stand up a dev/test MySQL via extraObjects. |
| charts/openfga/Chart.yaml | Bumps chart version and removes Bitnami DB dependencies. |
| charts/openfga/Chart.lock | Regenerates lockfile after dependency removal. |
| .github/workflows/test.yml | Removes Bitnami repo add step from chart testing workflow. |
| .github/workflows/release.yml | Removes Bitnami repo add step from release workflow. |
Comments suppressed due to low confidence (1)
charts/openfga/Chart.yaml:22
- After removing the Bitnami PostgreSQL/MySQL dependencies, the chart docs and schema should be updated to match:
charts/openfga/README.mdstill instructs users to setpostgresql.enabled/mysql.enabled, andvalues.schema.jsonstill defines those deprecated objects. Consider removing or clearly deprecating those schema entries and updating the README examples to use external DB +datastore.uri/datastore.uriSecretso upgrades don’t get conflicting guidance.
url: https://openfga.github.io/helm-charts/pgp-public-key.asc
dependencies:
- name: common
version: "2.13.3"
repository: oci://registry-1.docker.io/bitnamicharts
tags:
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| */}} | ||
| {{- define "openfga.createSecret" -}} | ||
| {{- if not (or .Values.global.postgresql.auth.existingSecret .Values.auth.existingSecret) -}} | ||
| {{- if not .Values.auth.existingSecret -}} |
There was a problem hiding this comment.
openfga.createSecret now references .Values.auth.existingSecret, but this chart’s values use datastore.existingSecret/datastore.uriSecret and there is no auth block in values.yaml/values.schema.json. Either update this helper to check the correct datastore secret values, or remove the helper if it’s no longer used, to avoid future template errors if it gets referenced.
| {{- if not .Values.auth.existingSecret -}} | |
| {{- if and (not .Values.datastore.existingSecret) (not .Values.datastore.uriSecret) -}} |
The chart reads key 'uri' from the secret when datastore.uriSecret is set, not 'OPENFGA_DATASTORE_URI'. Fix the commented examples to match.
Show users how to configure datastore.uriSecret alongside the extraObjects Secret examples so they can copy a working setup.
This helper referenced .Values.auth.existingSecret which does not exist in the chart's values schema. It was never called by any template.
Replace postgresql.enabled/mysql.enabled instructions with datastore.uri examples and point users to extraObjects examples in values.yaml for dev/test database setups.
…cies
Description
What problem is being solved?
How is it being solved?
What changes are made to solve it?
References
Review Checklist
mainSummary by CodeRabbit
Release Notes
Breaking Changes
datastore.uriordatastore.uriSecret.New Features
Chores