Add Ryot Helm chart and release workflows - #1791
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a complete Ryot Helm chart with bundled or external database configuration, Kubernetes templates, validation and runtime tests, documentation, and GitHub Actions for linting, testing, documentation generation, and release publishing. ChangesRyot Helm Chart and CI
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Helm
participant ryotValidate as "ryot.validate"
participant Kubernetes
participant HelmTest as "helm test"
Helm->>ryotValidate: validate values
ryotValidate-->>Helm: render or fail
Helm->>Kubernetes: install Ryot and optional PostgreSQL
HelmTest->>Kubernetes: run HTTP and database test Pods
Kubernetes-->>HelmTest: return health and connectivity results
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 18
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
helm/ryot/tests/rendering_test.yaml (1)
160-184: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the persistence-disabled volume-mount path.
Given the emptyDir/volumeMounts mismatch flagged in
postgres-statefulset.yaml(Lines 48-52, 89-93), consider adding an assertion here thatspec.template.spec.containers[0].volumeMountscontains thedatamount whenpostgres.persistence.enabled: false, to guard against regressions once fixed.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@helm/ryot/tests/rendering_test.yaml` around lines 160 - 184, Add a test case in rendering_test.yaml for the bundled postgres StatefulSet when postgres.persistence.enabled is false, using templates/postgres-statefulset.yaml and the existing postgres-StatefulSet setup. In the new assertion block, verify that spec.template.spec.containers[0].volumeMounts still includes the data mount so the emptyDir/volumeMounts behavior stays covered. Use the existing postgres-statefulset render test structure and the postgres.persistence.enabled flag to locate and extend the right scenario.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/helm-docs.yml:
- Line 11: The workflow-level contents permission is too broad in this helm-docs
workflow. Move the contents: write setting from the top-level permissions block
into the generate job’s permissions and add a brief comment there explaining it
is needed so the job can commit the regenerated VALUES.md back to the PR branch.
Use the generate job and its permissions block as the locating symbols when
making the change.
- Around line 22-51: The helm-docs workflow currently tries to commit and push
regenerated VALUES.md back to the PR branch unconditionally, which will fail on
forked pull_request events because GITHUB_TOKEN is read-only. Update the
helm-docs workflow job around the checkout and “Commit regenerated docs” steps
to skip the push path for forked PRs by checking whether
github.event.pull_request.head.repo.full_name matches github.repository, and
only run git commit/git push when it is safe; otherwise log a clear message or
use a fallback action instead of pushing directly.
- Line 22: The workflow currently uses actions/checkout by tag, so update the
helm-docs workflow to pin the checkout step to a specific commit SHA instead of
v4. Locate the existing uses: actions/checkout reference in the workflow and
replace it with the immutable SHA-pinned form to keep the action version locked.
In @.github/workflows/helm-guardrails.yml:
- Around line 3-8: Add a concurrency configuration to the helm-guardrails
workflow so newer runs cancel any in-progress runs for the same pull request.
Update the workflow definition alongside the existing pull_request and
workflow_dispatch triggers, using a stable concurrency group based on the
workflow and PR context so superseded checks are automatically canceled.
- Line 17: Pin the third-party actions used by this workflow to immutable commit
SHAs instead of version tags, and update the checkout step in the
helm-guardrails job to disable persisted credentials since this job does not
push. Locate the action references in the workflow by their names,
actions/checkout and azure/setup-helm, and replace the tagged versions with
SHA-pinned references while adding persist-credentials: false to the checkout
configuration.
In @.github/workflows/helm-lint.yml:
- Line 17: Pin the third-party GitHub Actions used in the helm-lint workflow to
immutable commit SHAs instead of mutable tags, and update both the
actions/checkout and azure/setup-helm references accordingly. Locate the
workflow steps by the actions/checkout and azure/setup-helm entries, replace the
version tags with full SHAs, and add persist-credentials: false to the checkout
step since this job does not push changes.
- Around line 3-8: The helm-lint workflow currently reruns on every push without
cancelling older in-flight executions. Add a concurrency configuration to the
workflow so runs from the same pull request share a stable group and set them to
cancel superseded runs; update the top-level workflow definition alongside the
existing on/workflow_dispatch settings.
In @.github/workflows/helm-release.yml:
- Around line 3-6: Add a concurrency group to the GitHub Actions workflow so
manual workflow_dispatch runs cannot overlap with an in-flight release published
run. Update the workflow’s top-level concurrency settings in the same workflow
that defines the on release/workflow_dispatch triggers, using a stable group key
tied to the workflow name or ref and enabling cancel-in-progress as appropriate.
Locate the workflow by its release and workflow_dispatch triggers and add the
concurrency block alongside them.
- Line 9: The workflow-level permissions are too broad and undocumented; in the
helm release workflow, add a brief comment explaining that chart-releaser-action
needs write access to publish packaged charts/index to gh-pages and create
releases, and move contents: write down to the specific job scope if possible.
Update the relevant workflow job that runs chart-releaser-action so the
permission is attached there rather than at the top level.
- Line 15: The helm release workflow uses mutable third-party action tags, which
should be pinned to commit SHAs for the release path. Update the actions
referenced in the workflow, including actions/checkout, azure/setup-helm, and
helm/chart-releaser-action, to fixed commit SHAs so the release job does not
depend on floating versions.
In @.github/workflows/helm-test.yml:
- Line 17: The workflow uses floating third-party action tags, so update the
helm-test job to pin actions/checkout, azure/setup-helm, and helm/kind-action to
specific commit SHAs instead of version tags. While touching the checkout step,
set persist-credentials to false since this job does not push changes. Use the
existing action references in the workflow to replace the unpinned uses entries
consistently.
- Around line 3-8: Add a concurrency group to the helm-test workflow so newer
runs cancel older superseded ones. Update the workflow definition in the
helm-test GitHub Actions file by adding a concurrency block tied to the pull
request or ref context, and set cancel-in-progress to true so stale kind-cluster
jobs are aborted when a new run starts.
In `@helm/ryot/README.md`:
- Around line 75-79: Clarify the “Individual components” section in README to
reflect that non-password fields are not always plain env values: in the chart’s
component-mode flow, each component can be sourced either inline or from an
existing secret via existingSecret and existingSecretKey. Update the wording
near the DATABASE_URL description to distinguish chart-managed Secret handling
for inline password from secret-backed sourcing for the other fields, using the
same “Individual components” and existingSecret/existingSecretKey terminology.
- Around line 223-225: The README table for postgres.auth.password shows an
unsafe default that conflicts with the chart behavior. Update the documentation
entry in the Helm README to remove the `postgres` default and state that users
must provide a password or set `existingSecret`, keeping it consistent with the
chart’s validation and values handling.
In `@helm/ryot/templates/_helpers.tpl`:
- Around line 209-231: The DATABASE_URL composition in ryot.databaseEnv can
break when postgres.auth or externalDatabase values contain URL-sensitive
characters because the env interpolation in the bundled-postgres and
externalDb.componentMode branches does not percent-encode them. Update the
helper to either build the URL from encoded components or avoid composing it
from raw env vars, and make sure the externalDatabase fields referenced through
ryot.dbComponentEnv are documented as requiring URL-safe values just like
postgres.auth.password.
In `@helm/ryot/templates/deployment.yaml`:
- Around line 71-77: The secret reference checks in the deployment template are
using inline required calls instead of the chart’s centralized validation
helper. Update the secretEnvFrom loop to route validation through ryot.validate
and/or ryot.assertSecretRef, matching the existing pattern used elsewhere in the
chart, and keep the existing secretEnvFrom key/existingSecret fields covered by
that shared helper.
In `@helm/ryot/templates/postgres-statefulset.yaml`:
- Around line 48-52: The PostgreSQL StatefulSet template currently only renders
volumeMounts inside the postgres.persistence.enabled branch, so the data
emptyDir added when persistence is disabled is never mounted. Update the
postgres-statefulset.yaml logic around the volumeMounts and data volume
definitions so the container always mounts the data volume: use the configured
mountPath when persistence is enabled, and also add a volumeMount for the
emptyDir when persistence is disabled. Keep the fix localized to the postgres
persistence conditional blocks and the data volume handling.
In `@helm/ryot/values.yaml`:
- Around line 166-185: The external database connection fields in the values
schema lack the same URL-safety warning already documented for
postgres.auth.password, even though ryot.databaseEnv builds DATABASE_URL from
host, username, and password without escaping. Update the externalDatabase.host,
externalDatabase.username, and externalDatabase.password entries in values.yaml
to document that these values must be URL-safe (especially for special
characters like @, /, and :), matching the existing guidance for
postgres.auth.password.
---
Outside diff comments:
In `@helm/ryot/tests/rendering_test.yaml`:
- Around line 160-184: Add a test case in rendering_test.yaml for the bundled
postgres StatefulSet when postgres.persistence.enabled is false, using
templates/postgres-statefulset.yaml and the existing postgres-StatefulSet setup.
In the new assertion block, verify that
spec.template.spec.containers[0].volumeMounts still includes the data mount so
the emptyDir/volumeMounts behavior stays covered. Use the existing
postgres-statefulset render test structure and the postgres.persistence.enabled
flag to locate and extend the right scenario.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 67ea063b-ed7b-40fa-bde9-7decb6984b73
📒 Files selected for processing (27)
.github/workflows/helm-docs.yml.github/workflows/helm-guardrails.yml.github/workflows/helm-lint.yml.github/workflows/helm-release.yml.github/workflows/helm-test.yml.gitignorehelm/ryot/.helmignorehelm/ryot/Chart.yamlhelm/ryot/README.mdhelm/ryot/VALUES.mdhelm/ryot/templates/NOTES.txthelm/ryot/templates/_helpers.tplhelm/ryot/templates/configmap.yamlhelm/ryot/templates/deployment.yamlhelm/ryot/templates/ingress.yamlhelm/ryot/templates/postgres-secret.yamlhelm/ryot/templates/postgres-service.yamlhelm/ryot/templates/postgres-statefulset.yamlhelm/ryot/templates/secret.yamlhelm/ryot/templates/service.yamlhelm/ryot/templates/tests/test-database-url.yamlhelm/ryot/templates/tests/test-http-connection.yamlhelm/ryot/tests/database_url_test.yamlhelm/ryot/tests/guardrails_test.yamlhelm/ryot/tests/rendering_test.yamlhelm/ryot/tests/runtime_pods_test.yamlhelm/ryot/values.yaml
|
Thanks for this! Initial glance at the PR looks good. I will take a look in detail when I get time. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/helm-release.yml:
- Around line 11-15: The concurrency guard in the helm-release workflow is keyed
off github.ref, which lets workflow_dispatch and release: published runs use
different groups and overlap. Update the concurrency.group setting in the
workflow so it uses a fixed shared key for all publish runs, keeping
cancel-in-progress false, and leave the release serialization behavior
centralized in that concurrency block.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9f044e8b-101c-4c3d-9d2b-89f77a74afce
📒 Files selected for processing (8)
.github/workflows/helm-guardrails.yml.github/workflows/helm-lint.yml.github/workflows/helm-release.yml.github/workflows/helm-test.ymlhelm/ryot/README.mdhelm/ryot/templates/postgres-statefulset.yamlhelm/ryot/tests/rendering_test.yamlhelm/ryot/values.yaml
|
@IgnisDa I have gone through all the comments that CodeRabbit made and adjusted the things were it had a valid point. The Helm-Docs however, I do require your opinion on. Let me know if you have any more questions. |
|
Hey @IgnisDa , Did you have time to review this? |
|
Hey I’ll take a look at this today. Sorry for the delay.
…On Mon, 13 Jul 2026 at 8:51 PM, NigelVanHattum ***@***.***> wrote:
*NigelVanHattum* left a comment (IgnisDa/ryot#1791)
<#1791 (comment)>
Hey @IgnisDa <https://github.com/IgnisDa> ,
Did you have time to review this?
—
Reply to this email directly, view it on GitHub
<#1791?email_source=notifications&email_token=AOQ5PNGK4SITRGJ3HX5QP5L5ET46VA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTIOJVHE2TMMRQHA42M4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLDGN5XXIZLSL5RWY2LDNM#issuecomment-4959562089>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AOQ5PNBEF3CNHFTVSWNWIE35ET46VAVCNFSNUABFKJSXA33TNF2G64TZHM3DEMZQHA2TMNJXHNEXG43VMU5TINZYHE3DIMBZGY42C5QC>
.
Triage notifications, keep track of coding agent tasks and review pull
requests on the go with GitHub Mobile for iOS
<https://github.com/notifications/mobile/ios/AOQ5PNGBWDT3OYWYXFHIKJ35ET46VA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTIOJVHE2TMMRQHA42M4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJKTGN5XXIZLSL5UW64Y>
and Android
<https://github.com/notifications/mobile/android/AOQ5PNGPXG4QMXWL2SHTXAD5ET46VA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTIOJVHE2TMMRQHA42M4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLTGN5XXIZLSL5QW4ZDSN5UWI>.
Download it today!
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
Can we move the chart from a new top-level It's a mechanical rename, nothing chart-breaking. Every spot that currently hardcodes the
No functional changes needed beyond the path swap. |
|
Doc gap: this PR doesn't touch Could you add a section here mirroring the existing ones - a short blurb plus the |
|
Related structural suggestion: the 4 PR-triggered helm workflows (helm-lint.yml, helm-guardrails.yml, helm-docs.yml, helm-test.yml) could collapse into a single This wouldn't change what reviewers see - each job still reports as its own separate check in the PR's checks list regardless of which file it lives in, so nothing is lost there. One nice side effect: merging gives a clean way to address the still-open "scope One tradeoff worth knowing before treating this as a clear win: today each of the four can be |
|
One more idea before I do the one-time gh-pages/Pages setup this PR asks for: since we already publish Docker images to That would mean:
Downside is Open to switching |
|
@NigelVanHattum I am done with the review. Please take a look at my comments when you have time. THanks! |
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/helm-docs.yml (1)
1-17:⚠️ Potential issue | 🟠 MajorFork-PR push failure still unresolved.
This still triggers on
pull_requestand checks out/pushes to${{ github.head_ref }}(Lines 26, 50). As already discussed on this file, forked-repo PRs get a read-onlyGITHUB_TOKENregardless ofcontents: write, so the commit/push step will fail for every external contributor touchingci/helm/**. The maintainer previously agreed to switch to apush: branches: [main]trigger instead — that change doesn't appear to be reflected here yet.Also applies to: 22-53
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/helm-docs.yml around lines 1 - 17, The Helm Docs workflow must stop running on pull_request events because fork PR tokens cannot push changes. Update the workflow trigger to run on push to the main branch, and adjust the checkout/commit flow around the existing github.head_ref usage so it targets the main branch rather than a PR head.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ci/helm/ryot/templates/_helpers.tpl`:
- Around line 69-70: Update the ryot.postgres.fullname helper to truncate the
base ryot.fullname before appending “-postgres”, ensuring the resulting resource
name never exceeds 63 characters while preserving the suffix and existing naming
behavior.
In `@ci/helm/ryot/templates/deployment.yaml`:
- Around line 64-77: Add validation for entries in both .Values.secretEnv and
.Values.secretEnvFrom, rejecting keys that match chart-managed environment names
such as SERVER_ADMIN_ACCESS_TOKEN, SERVER_PRO_KEY, and the database secret
variables. Apply the validation before rendering each dynamic env entry, using
Helm’s existing required/fail validation pattern, while preserving rendering for
non-reserved names.
In `@ci/helm/ryot/templates/postgres-secret.yaml`:
- Around line 10-11: The postgres-password template currently overwrites the
chart-managed credential on every Helm release, breaking upgrades when persisted
PostgreSQL data still uses the old password. Update the postgres-secret.yaml
handling to preserve the existing secret value across upgrades and reject or
explicitly block attempts to change the bundled password; document a separate
external-secret management path or provide an explicit password-rotation
mechanism if chart-owned rotation is supported.
In `@ci/helm/ryot/VALUES.md`:
- Line 5: Update the chart description in VALUES.md to use the hyphenated
compound adjective “self-hosted” instead of “self hosted,” leaving the rest of
the user-facing text unchanged.
---
Outside diff comments:
In @.github/workflows/helm-docs.yml:
- Around line 1-17: The Helm Docs workflow must stop running on pull_request
events because fork PR tokens cannot push changes. Update the workflow trigger
to run on push to the main branch, and adjust the checkout/commit flow around
the existing github.head_ref usage so it targets the main branch rather than a
PR head.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c06e9be9-7f35-461b-9fbd-6432b5f53154
📒 Files selected for processing (27)
.github/workflows/helm-docs.yml.github/workflows/helm-guardrails.yml.github/workflows/helm-lint.yml.github/workflows/helm-release.yml.github/workflows/helm-test.yml.gitignoreci/helm/ryot/.helmignoreci/helm/ryot/Chart.yamlci/helm/ryot/README.mdci/helm/ryot/VALUES.mdci/helm/ryot/templates/NOTES.txtci/helm/ryot/templates/_helpers.tplci/helm/ryot/templates/configmap.yamlci/helm/ryot/templates/deployment.yamlci/helm/ryot/templates/ingress.yamlci/helm/ryot/templates/postgres-secret.yamlci/helm/ryot/templates/postgres-service.yamlci/helm/ryot/templates/postgres-statefulset.yamlci/helm/ryot/templates/secret.yamlci/helm/ryot/templates/service.yamlci/helm/ryot/templates/tests/test-database-url.yamlci/helm/ryot/templates/tests/test-http-connection.yamlci/helm/ryot/tests/database_url_test.yamlci/helm/ryot/tests/guardrails_test.yamlci/helm/ryot/tests/rendering_test.yamlci/helm/ryot/tests/runtime_pods_test.yamlci/helm/ryot/values.yaml
Agreed; I have merged 3 of the helm workflows. I've kept the helm-docs as a separate, because this will be moved to the main branch after a PR (see discussion) |
With the help of AI I was able to set it up. I've never tested this workflow, so not sure what more would be required. This would be something we need to test. |
|
@IgnisDa Thanks for the review. I've taken multiple steps to work on them. A few open topics:
Just let me know how you want to tackle both open topics. |
|
@NigelVanHattum I think this PR is done and ready to be merged. Only thing that remains to be tested is the actual thing being published to ghcr. i will do that on main itself after it is merged. Should I go ahead and merge it? |
|
Sounds good! |
|
i have merged this. will cut a new release within a few days. |
|
thanks for the feature! |
Adds a helm chart in helm/ryot/ — deploy Ryot on Kubernetes, mirroring the Docker Compose setup.
Chart: Deployment + Service + optional Ingress; optional bundled PostgreSQL (StatefulSet), or bring-your-own DB (full DATABASE_URL or individual host/port/db/user/password, inline or from secrets). Sensitive values always via Secret; DB password never written into a manifest literal. Render-time validation for required config.
CI (.github/workflows/, gated to helm/**): helm-lint, helm-guardrails (helm-unittest), helm-test (kind + helm test), helm-docs (regenerates VALUES.md), helm-release (chart-releaser on GitHub Release, version from tag).
Usage/values: helm/ryot/README.md.
helm-release publishes to a gh-pages Helm repo, so:
Enable Pages: Settings → Pages → source Deploy from a branch, branch gh-pages, / (root) — docs.
Settings → Actions → General → Workflow permissions → Read and write (lets release + docs push) — docs.
Closes #1789
Summary by CodeRabbit
New Features
Documentation
Tests
Chores