Add Ryot Helm chart and release workflows#1791
Conversation
|
Caution Review failedAn error occurred during the review process. Please try again later. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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
| - uses: actions/checkout@v4 | ||
| with: | ||
| # Check out the PR branch itself so the regenerated docs can be | ||
| # pushed back to it. | ||
| ref: ${{ github.head_ref }} | ||
|
|
||
| - name: Install helm-docs | ||
| env: | ||
| HELM_DOCS_VERSION: "1.14.2" | ||
| run: | | ||
| curl -sSLo helm-docs.tar.gz \ | ||
| "https://github.com/norwoodj/helm-docs/releases/download/v${HELM_DOCS_VERSION}/helm-docs_${HELM_DOCS_VERSION}_Linux_x86_64.tar.gz" | ||
| tar -xf helm-docs.tar.gz helm-docs | ||
| sudo mv helm-docs /usr/local/bin/helm-docs | ||
| helm-docs --version | ||
|
|
||
| - name: Generate VALUES.md | ||
| # Writes helm/ryot/VALUES.md from the annotations in values.yaml and | ||
| # leaves the hand-written README.md untouched. | ||
| run: helm-docs --chart-search-root helm --output-file VALUES.md | ||
|
|
||
| - name: Commit regenerated docs | ||
| run: | | ||
| if [ -n "$(git status --porcelain helm/ryot/VALUES.md)" ]; then | ||
| git config user.name "github-actions[bot]" | ||
| git config user.email "41898282+github-actions[bot]@users.noreply.github.com" | ||
| git add helm/ryot/VALUES.md | ||
| git commit -m "docs(helm): regenerate VALUES.md [skip ci]" | ||
| git push | ||
| else |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Workflow will fail to push regenerated docs on fork PRs.
For pull_request events from forked repositories, GitHub always issues a read-only GITHUB_TOKEN, regardless of the permissions: contents: write declared here. Since this job checks out head_ref and attempts git push (line 50) directly against the PR branch, every external contributor's PR touching helm/** will fail this job at the push step.
Consider skipping the push (or the whole job) when the PR is from a fork, e.g. gate on github.event.pull_request.head.repo.full_name == github.repository, and surface a friendly message/log or use a bot PR-comment fallback instead.
🧰 Tools
🪛 zizmor (1.26.1)
[warning] 22-26: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 22-22: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 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 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.
There was a problem hiding this comment.
@IgnisDa this is a valid point. This workflow automatically generates the documentation (values and defaults) for every PR. This makes sure the documentation is always up-to-date. How would you want to tackle this?
My suggestion would be to run this after the PR merge (so pushes on main), so change trigger from pull_request → push: branches: [main] with paths: helm/**. Will commit straight to main.
One caveat to flag: if main is a protected branch requiring PRs/reviews, a direct github-actions push will be rejected. Two ways around it:
- allow the github-actions bot / apps to bypass the branch protection for this path, or
- instead of pushing to main, have the workflow open a small "docs: update VALUES.md" PR (e.g. via peter-evans/create-pull-request).
Do you want a direct push (simplest, needs a protection exception) or auto-PR? Or maybe skip the docs all together?
There was a problem hiding this comment.
@NigelVanHattum, thanks for engaging with this — it's really a call for @IgnisDa to make on repo conventions, but here's a comparison to help the decision:
Option A — Guard the existing pull_request trigger (smallest change)
Keep everything as-is, but gate the "Commit regenerated docs" step (and ideally skip the whole job early) on:
if: github.event.pull_request.head.repo.full_name == github.repositoryFork PRs simply skip the auto-commit; the docs job would report success but no push. Downside: fork-originated PRs will have stale VALUES.md until a maintainer manually regenerates it or pushes a commit on top before merge. You could pair this with a helm-lint-style check that fails (rather than auto-fixes) when VALUES.md is out of date on fork PRs, prompting the contributor to run helm-docs locally.
Option B — Trigger on push to main (your suggestion)
Cleaner mental model (no fork token limitations at all, since it only runs after merge), but as you noted, if main is protected requiring PR review, this bot push will be rejected outright unless there's a bypass. Also means VALUES.md will be briefly out-of-sync with values.yaml between merge and the follow-up auto-commit — acceptable for docs, but worth knowing.
Option C — Auto-PR via peter-evans/create-pull-request
Goes through normal review, works fine with protected branches without needing a bypass rule, but adds a second PR per helm change, which may be noisy for a docs-only regeneration.
If IgnisDa wants zero maintenance overhead and doesn't mind occasional stale docs on external contributions, Option A is the least invasive fix to just the reported issue. If main isn't protected (or a bot-bypass rule is easy to add), Option B is simplest long-term. Option C is safest for protected branches but noisiest.
Once the direction is picked, happy to draft the corresponding workflow diff.
🧠 Learnings used
Learnt from: NigelVanHattum
Repo: IgnisDa/ryot PR: 1791
File: .github/workflows/helm-docs.yml:22-22
Timestamp: 2026-07-01T22:59:48.659Z
Learning: In IgnisDa/ryot GitHub Actions workflow files, do not require third-party actions (e.g., `actions/checkout`) to be pinned to specific commit SHAs. If a reviewer suggests SHA pinning for security, treat it as a low-priority nitpick that may be declined, reflecting the maintainer’s preference to avoid the resulting maintenance burden.
|
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. |
NigelVanHattum
changed the title
2 participants
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
VALUES.md, and improved post-install guidance with database connection instructions..gitignoretemplate exceptions.