Skip to content

Add Ryot Helm chart and release workflows - #1791

Merged
IgnisDa merged 20 commits into
IgnisDa:mainfrom
NigelVanHattum:feature/helm-chart
Jul 24, 2026
Merged

Add Ryot Helm chart and release workflows#1791
IgnisDa merged 20 commits into
IgnisDa:mainfrom
NigelVanHattum:feature/helm-chart

Conversation

@NigelVanHattum

@NigelVanHattum NigelVanHattum commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

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.

⚠️ One-time setup required by @IgnisDa (before a new release)
helm-release publishes to a gh-pages Helm repo, so:

  1. Create an empty gh-pages branch:
git checkout --orphan gh-pages && git rm -rf . && \
git commit --allow-empty -m "Init gh-pages" && git push origin gh-pages
  1. Enable Pages: Settings → Pages → source Deploy from a branch, branch gh-pages, / (root) — docs.

  2. Settings → Actions → General → Workflow permissions → Read and write (lets release + docs push) — docs.

Closes #1789

Summary by CodeRabbit

  • New Features

    • Added a Helm chart for deploying Ryot to Kubernetes.
    • Supports bundled PostgreSQL or external database configurations.
    • Added configurable ingress, services, health checks, secrets, persistence, and security settings.
    • Added post-install guidance and generated configuration reference documentation.
  • Documentation

    • Documented installation, configuration, database options, secrets, ingress, testing, and validation behavior.
  • Tests

    • Added chart validation, rendering, database connectivity, and runtime health checks.
  • Chores

    • Added automated Helm linting, documentation generation, testing, and release publishing workflows.

@coderabbitai

coderabbitai Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Adds 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.

Changes

Ryot Helm Chart and CI

Layer / File(s) Summary
Chart metadata and values schema
ci/helm/ryot/Chart.yaml, ci/helm/ryot/values.yaml, ci/helm/ryot/.helmignore, .gitignore
Defines chart metadata, packaging exclusions, application and secret values, bundled or external database settings, ingress, probes, security, resources, and scheduling options.
Template helpers and validation
ci/helm/ryot/templates/_helpers.tpl
Adds naming, labels, image, secret, database environment, secret-reference, and fail-fast validation helpers.
Application workload templates
ci/helm/ryot/templates/configmap.yaml, secret.yaml, deployment.yaml, service.yaml, ingress.yaml, NOTES.txt
Renders application configuration, secrets, Deployment, Service, optional Ingress, environment wiring, and installation notes.
Bundled PostgreSQL templates
ci/helm/ryot/templates/postgres-*
Adds conditional PostgreSQL credentials, headless Service, StatefulSet, health probes, security and scheduling options, and ephemeral or persistent storage.
Render-time and runtime tests
ci/helm/ryot/templates/tests/*, ci/helm/ryot/tests/*
Tests database URL modes, secret references, validation failures, rendered resources, Helm test hooks, HTTP health checks, and database connectivity.
Chart documentation
ci/helm/ryot/README.md, ci/helm/ryot/VALUES.md
Documents installation, configuration modes, validation behavior, runtime checks, chart values, and generated values reference content.
Helm CI workflows
.github/workflows/helm-*.yml
Adds workflows for documentation generation, linting, strict unit tests, kind-based Helm tests, and release chart publication.

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
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: adding the Ryot Helm chart and supporting release workflows.
Linked Issues check ✅ Passed The PR delivers a Helm chart plus GitHub Pages/chart-releaser release support, matching issue #1789's request.
Out of Scope Changes check ✅ Passed The added workflows, chart files, tests, and docs all support Helm chart packaging and publishing; no unrelated changes stand out.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 win

Add 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 that spec.template.spec.containers[0].volumeMounts contains the data mount when postgres.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

📥 Commits

Reviewing files that changed from the base of the PR and between 51d9484 and 6ebe3bd.

📒 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
  • .gitignore
  • helm/ryot/.helmignore
  • helm/ryot/Chart.yaml
  • helm/ryot/README.md
  • helm/ryot/VALUES.md
  • helm/ryot/templates/NOTES.txt
  • helm/ryot/templates/_helpers.tpl
  • helm/ryot/templates/configmap.yaml
  • helm/ryot/templates/deployment.yaml
  • helm/ryot/templates/ingress.yaml
  • helm/ryot/templates/postgres-secret.yaml
  • helm/ryot/templates/postgres-service.yaml
  • helm/ryot/templates/postgres-statefulset.yaml
  • helm/ryot/templates/secret.yaml
  • helm/ryot/templates/service.yaml
  • helm/ryot/templates/tests/test-database-url.yaml
  • helm/ryot/templates/tests/test-http-connection.yaml
  • helm/ryot/tests/database_url_test.yaml
  • helm/ryot/tests/guardrails_test.yaml
  • helm/ryot/tests/rendering_test.yaml
  • helm/ryot/tests/runtime_pods_test.yaml
  • helm/ryot/values.yaml

Comment thread .github/workflows/helm-docs.yml Outdated
Comment thread .github/workflows/helm-docs.yml Outdated
Comment thread .github/workflows/helm-docs.yml Outdated
Comment thread .github/workflows/helm-guardrails.yml Outdated
Comment thread .github/workflows/helm-guardrails.yml Outdated
Comment thread ci/helm/ryot/README.md
Comment thread ci/helm/ryot/templates/_helpers.tpl
Comment thread ci/helm/ryot/templates/deployment.yaml
Comment thread helm/ryot/templates/postgres-statefulset.yaml Outdated
Comment thread ci/helm/ryot/values.yaml
@IgnisDa

IgnisDa commented Jul 2, 2026

Copy link
Copy Markdown
Owner

Thanks for this! Initial glance at the PR looks good. I will take a look in detail when I get time.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between 99c71f3 and 4d99bc5.

📒 Files selected for processing (8)
  • .github/workflows/helm-guardrails.yml
  • .github/workflows/helm-lint.yml
  • .github/workflows/helm-release.yml
  • .github/workflows/helm-test.yml
  • helm/ryot/README.md
  • helm/ryot/templates/postgres-statefulset.yaml
  • helm/ryot/tests/rendering_test.yaml
  • helm/ryot/values.yaml

Comment thread .github/workflows/helm-release.yml
@NigelVanHattum

Copy link
Copy Markdown
Contributor Author

@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 NigelVanHattum changed the title Feature/helm chart Add Ryot Helm chart and release workflows Jul 2, 2026
@NigelVanHattum

Copy link
Copy Markdown
Contributor Author

Hey @IgnisDa ,

Did you have time to review this?

@IgnisDa

IgnisDa commented Jul 13, 2026 via email

Copy link
Copy Markdown
Owner

Comment thread .github/workflows/helm-release.yml
@IgnisDa

IgnisDa commented Jul 13, 2026

Copy link
Copy Markdown
Owner

Can we move the chart from a new top-level helm/ directory into ci/helm/ instead? We already keep deployment-adjacent tooling (the Caddyfile, fly.toml, the container run script) under ci/, and Helm itself has no opinion on where the chart directory lives - helm install ./ci/helm/ryot works identically to ./helm/ryot.

It's a mechanical rename, nothing chart-breaking. Every spot that currently hardcodes the helm/ path:

  • paths: filters in helm-lint.yml, helm-test.yml, helm-guardrails.yml, helm-docs.yml (helm/** -> ci/helm/**)
  • charts_dir: helm in helm-release.yml -> ci/helm
  • the two yq calls against helm/ryot/Chart.yaml in helm-release.yml
  • --chart-search-root helm and the git status/git add helm/ryot/VALUES.md lines in helm-docs.yml
  • the helm lint/helm template/helm unittest/helm install invocations in helm-lint.yml/helm-guardrails.yml/helm-test.yml
  • the !helm/**/templates negation patterns in .gitignore
  • the install examples inside helm/ryot/README.md itself

No functional changes needed beyond the path swap.

@IgnisDa

IgnisDa commented Jul 13, 2026

Copy link
Copy Markdown
Owner

Doc gap: this PR doesn't touch apps/docs/src/deployment.md. That's the docs site's "Deployment" page, and it currently lists every alternative way to deploy Ryot beyond docker compose - Railway, Dokku, Fly, Cosmos - each with a short blurb and setup steps. There's no Kubernetes/Helm entry, so anyone browsing docs.ryot.io has no way to discover this chart exists at all; they'd only find it by digging into the GitHub repo directly.

Could you add a section here mirroring the existing ones - a short blurb plus the helm repo add / helm install quickstart (same as helm/ryot/README.md), and a link to VALUES.md for the full options list?

@IgnisDa

IgnisDa commented Jul 13, 2026

Copy link
Copy Markdown
Owner

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 helm-ci.yml with four named jobs (lint, unittest, docs, test) sharing one on: pull_request: paths: helm/** / workflow_dispatch block, instead of repeating that trigger boilerplate across four files.

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. helm-release.yml should stay separate: different trigger (release: published vs pull_request), different concurrency semantics (serialize + never cancel, vs. cancel-on-push), no reason to couple release publishing with PR checks.

One nice side effect: merging gives a clean way to address the still-open "scope contents: write to job level" comment on helm-docs.yml elsewhere in this PR - in a merged file the workflow-level permissions: can default to contents: read, with only the docs job overriding to write.

One tradeoff worth knowing before treating this as a clear win: today each of the four can be workflow_dispatch-triggered independently from the Actions UI; merged into one file, a manual dispatch runs all four jobs at once unless an input is added to select just one. This is acceptable.

@IgnisDa

IgnisDa commented Jul 13, 2026

Copy link
Copy Markdown
Owner

One more idea before I do the one-time gh-pages/Pages setup this PR asks for: since we already publish Docker images to ghcr.io/ignisda/ryot, what do you think about pushing the chart to the same registry as an OCI artifact instead (helm push ryot-<version>.tgz oci://ghcr.io/ignisda/charts, with a helm registry login ghcr.io step beforehand) rather than the classic index.yaml + gh-pages route via chart-releaser-action?

That would mean:

  • No gh-pages branch or GitHub Pages enablement needed. I do not want github pages since this will block the gh-pages branch for helm charts when it might be used for documentation in the future if needed.
  • No "Read and write" default workflow permissions change - helm-release.yml would just need packages: write for that one job, same as the existing docker build job already has
  • Users install via helm install oci://ghcr.io/ignisda/charts/ryot --version x.y.z instead of helm repo add + helm install

Downside is oci:// charts aren't discoverable via helm search repo the way a classic repo index is, and it needs Helm 3.8+ for OCI support - probably a non-issue at this point (OCI support has been on by default since 3.8 / mid-2022), but flagging it for completeness.

Open to switching helm-release.yml over to this instead of gh-pages? Curious what you think given you already built out the chart-releaser path.

@IgnisDa

IgnisDa commented Jul 13, 2026

Copy link
Copy Markdown
Owner

@NigelVanHattum I am done with the review. Please take a look at my comments when you have time. THanks!

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 | 🟠 Major

Fork-PR push failure still unresolved.

This still triggers on pull_request and checks out/pushes to ${{ github.head_ref }} (Lines 26, 50). As already discussed on this file, forked-repo PRs get a read-only GITHUB_TOKEN regardless of contents: write, so the commit/push step will fail for every external contributor touching ci/helm/**. The maintainer previously agreed to switch to a push: 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

📥 Commits

Reviewing files that changed from the base of the PR and between 82da437 and 43f0ee9.

📒 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
  • .gitignore
  • ci/helm/ryot/.helmignore
  • ci/helm/ryot/Chart.yaml
  • ci/helm/ryot/README.md
  • ci/helm/ryot/VALUES.md
  • ci/helm/ryot/templates/NOTES.txt
  • ci/helm/ryot/templates/_helpers.tpl
  • ci/helm/ryot/templates/configmap.yaml
  • ci/helm/ryot/templates/deployment.yaml
  • ci/helm/ryot/templates/ingress.yaml
  • ci/helm/ryot/templates/postgres-secret.yaml
  • ci/helm/ryot/templates/postgres-service.yaml
  • ci/helm/ryot/templates/postgres-statefulset.yaml
  • ci/helm/ryot/templates/secret.yaml
  • ci/helm/ryot/templates/service.yaml
  • ci/helm/ryot/templates/tests/test-database-url.yaml
  • ci/helm/ryot/templates/tests/test-http-connection.yaml
  • ci/helm/ryot/tests/database_url_test.yaml
  • ci/helm/ryot/tests/guardrails_test.yaml
  • ci/helm/ryot/tests/rendering_test.yaml
  • ci/helm/ryot/tests/runtime_pods_test.yaml
  • ci/helm/ryot/values.yaml

Comment thread ci/helm/ryot/templates/_helpers.tpl Outdated
Comment thread ci/helm/ryot/templates/deployment.yaml
Comment thread ci/helm/ryot/templates/postgres-secret.yaml
Comment thread ci/helm/ryot/VALUES.md Outdated
@NigelVanHattum

Copy link
Copy Markdown
Contributor Author

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 helm-ci.yml with four named jobs (lint, unittest, docs, test) sharing one on: pull_request: paths: helm/** / workflow_dispatch block, instead of repeating that trigger boilerplate across four files.

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. helm-release.yml should stay separate: different trigger (release: published vs pull_request), different concurrency semantics (serialize + never cancel, vs. cancel-on-push), no reason to couple release publishing with PR checks.

One nice side effect: merging gives a clean way to address the still-open "scope contents: write to job level" comment on helm-docs.yml elsewhere in this PR - in a merged file the workflow-level permissions: can default to contents: read, with only the docs job overriding to write.

One tradeoff worth knowing before treating this as a clear win: today each of the four can be workflow_dispatch-triggered independently from the Actions UI; merged into one file, a manual dispatch runs all four jobs at once unless an input is added to select just one. This is acceptable.

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)

@NigelVanHattum

Copy link
Copy Markdown
Contributor Author

One more idea before I do the one-time gh-pages/Pages setup this PR asks for: since we already publish Docker images to ghcr.io/ignisda/ryot, what do you think about pushing the chart to the same registry as an OCI artifact instead (helm push ryot-<version>.tgz oci://ghcr.io/ignisda/charts, with a helm registry login ghcr.io step beforehand) rather than the classic index.yaml + gh-pages route via chart-releaser-action?

That would mean:

  • No gh-pages branch or GitHub Pages enablement needed. I do not want github pages since this will block the gh-pages branch for helm charts when it might be used for documentation in the future if needed.
  • No "Read and write" default workflow permissions change - helm-release.yml would just need packages: write for that one job, same as the existing docker build job already has
  • Users install via helm install oci://ghcr.io/ignisda/charts/ryot --version x.y.z instead of helm repo add + helm install

Downside is oci:// charts aren't discoverable via helm search repo the way a classic repo index is, and it needs Helm 3.8+ for OCI support - probably a non-issue at this point (OCI support has been on by default since 3.8 / mid-2022), but flagging it for completeness.

Open to switching helm-release.yml over to this instead of gh-pages? Curious what you think given you already built out the chart-releaser path.

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.

@NigelVanHattum

Copy link
Copy Markdown
Contributor Author

@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.

@IgnisDa

IgnisDa commented Jul 24, 2026

Copy link
Copy Markdown
Owner

@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?

@NigelVanHattum

Copy link
Copy Markdown
Contributor Author

Sounds good!

@IgnisDa
IgnisDa merged commit 3afe499 into IgnisDa:main Jul 24, 2026
13 checks passed
@IgnisDa

IgnisDa commented Jul 24, 2026

Copy link
Copy Markdown
Owner

i have merged this. will cut a new release within a few days.

@IgnisDa

IgnisDa commented Jul 24, 2026

Copy link
Copy Markdown
Owner

thanks for the feature!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature Request] Helm chart

2 participants