chore(chart): align lobu chart to owletto fork (pre-consolidation, byte-identical)#882
Merged
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (20)
📝 WalkthroughWalkthroughThis PR refactors the Lobu Helm chart to shift secret management from chart-generated to externally-provided, consolidates helper templates, standardizes container ports and PVCs, and simplifies operational defaults. All deployments and jobs are updated to reference ChangesHelm Chart Configuration Refactor
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@charts/lobu/templates/embeddings-service.yaml`:
- Line 12: Add the missing embeddings.service.port key to values.yaml so the
templates referencing .Values.embeddings.service.port (used in
embeddings-service.yaml and embeddings-deployment.yaml for containerPort, the
PORT env var, and readiness/liveness probe ports) have a numeric value; update
values.yaml to include an embeddings: section with service: port: 8080 (or the
actual service port your container listens on) to match the containerPort/PORT
used in the deployment.
In `@charts/lobu/templates/NOTES.txt`:
- Around line 4-5: The NOTES.txt uses .Values.ingress.host but the chart defines
.Values.ingress.hosts; update the template (inside the .Values.ingress.enabled
conditional) to reference the hosts array (e.g., use .Values.ingress.hosts[0] or
iterate over .Values.ingress.hosts and render each host) so the installed URL is
populated correctly; ensure you still guard with .Values.ingress.enabled and
handle the case when hosts is empty.
In `@charts/lobu/values.yaml`:
- Around line 11-13: The values.yaml currently sets tag: "latest" which,
combined with pullPolicy: Always, causes mutable image pulls; update the
values.yaml to remove the hardcoded "latest" and leave tag unset (or set it to
an empty string) so the chart can default to the chart appVersion; specifically
change the tag entry (symbol: tag) and ensure pullPolicy remains as-is (symbol:
pullPolicy) so image resolution uses the chart's appVersion rather than
"latest".
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f29622fe-ea28-425e-a35c-e7a7e58edb64
📒 Files selected for processing (19)
.github/workflows/helm-chart.ymlREADME.mdcharts/lobu/Chart.yamlcharts/lobu/templates/NOTES.txtcharts/lobu/templates/_helpers.tplcharts/lobu/templates/app-pvc.yamlcharts/lobu/templates/deployment.yamlcharts/lobu/templates/embeddings-deployment.yamlcharts/lobu/templates/embeddings-pvc.yamlcharts/lobu/templates/embeddings-service.yamlcharts/lobu/templates/ingress.yamlcharts/lobu/templates/migration-job.yamlcharts/lobu/templates/secret.yamlcharts/lobu/templates/service.yamlcharts/lobu/templates/smoke-test-job.yamlcharts/lobu/templates/worker-deployment.yamlcharts/lobu/templates/worker-pvc.yamlcharts/lobu/values.example.yamlcharts/lobu/values.yaml
💤 Files with no reviewable changes (3)
- charts/lobu/values.example.yaml
- charts/lobu/templates/secret.yaml
- .github/workflows/helm-chart.yml
Comment on lines
4
to
+5
| {{- if .Values.ingress.enabled }} | ||
| Application URL: | ||
| {{- range .Values.ingress.hosts }} | ||
| https://{{ . }} | ||
| {{- end }} | ||
| https://{{ .Values.ingress.host }} |
There was a problem hiding this comment.
Use the same ingress key as the chart templates for NOTES URL.
Line 5 references .Values.ingress.host, but the chart changes use .Values.ingress.hosts; this can render an empty/incorrect install URL.
Suggested fix
{{- if .Values.ingress.enabled }}
- https://{{ .Values.ingress.host }}
+ https://{{ first .Values.ingress.hosts }}
{{- else }}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| {{- if .Values.ingress.enabled }} | |
| Application URL: | |
| {{- range .Values.ingress.hosts }} | |
| https://{{ . }} | |
| {{- end }} | |
| https://{{ .Values.ingress.host }} | |
| {{- if .Values.ingress.enabled }} | |
| https://{{ first .Values.ingress.hosts }} |
🤖 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 `@charts/lobu/templates/NOTES.txt` around lines 4 - 5, The NOTES.txt uses
.Values.ingress.host but the chart defines .Values.ingress.hosts; update the
template (inside the .Values.ingress.enabled conditional) to reference the hosts
array (e.g., use .Values.ingress.hosts[0] or iterate over .Values.ingress.hosts
and render each host) so the installed URL is populated correctly; ensure you
still guard with .Values.ingress.enabled and handle the case when hosts is
empty.
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 `@packages/owletto`:
- Line 1: The submodule pointer for packages/owletto must be updated to a commit
that is reachable from owletto/main; go into the submodule (packages/owletto),
fetch origin, find and checkout a commit that is on origin/main (a commit
reachable from owletto/main), then stage the updated gitlink in the superproject
and commit the change. Concretely: run git fetch origin in packages/owletto,
identify a commit on owletto/main (or checkout origin/main), update the
submodule HEAD to that reachable commit, return to the superrepo, git add
packages/owletto and commit the new submodule pointer so CI’s merge-base check
passes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
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 `@packages/landing/src/content/docs/deployment/kubernetes.mdx`:
- Around line 60-69: Remove the duplicated "Create the referenced secrets before
installing" kubectl commands block that re-declares the same secrets already
created earlier; delete the repeated commands that create secrets named
"lobu-db" and "lobu-secrets" (the --from-literal lines for uri, JWT_SECRET,
BETTER_AUTH_SECRET, ANTHROPIC_API_KEY) so only the original "Create secrets"
section remains and keep the following note about setting app.env.PUBLIC_WEB_URL
if needed.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ed94b579-9194-41f8-ae89-d3ae45bc68e0
📒 Files selected for processing (1)
packages/landing/src/content/docs/deployment/kubernetes.mdx
Comment on lines
+60
to
+69
| Create the referenced secrets before installing: | ||
|
|
||
| ```bash | ||
| kubectl -n lobu create secret generic lobu-db \ | ||
| --from-literal=uri='postgresql://USER:PASSWORD@HOST:5432/lobu?sslmode=require' | ||
| kubectl -n lobu create secret generic lobu-secrets \ | ||
| --from-literal=JWT_SECRET='replace-with-a-long-random-value' \ | ||
| --from-literal=BETTER_AUTH_SECRET='replace-with-a-long-random-value' \ | ||
| --from-literal=ANTHROPIC_API_KEY='sk-ant-...' | ||
| ``` |
There was a problem hiding this comment.
Remove duplicate secret creation commands.
These kubectl commands are identical to lines 25-31 in the "Create secrets" section above. Users have already been instructed to create these secrets earlier in the guide, making this duplication confusing and unnecessary.
📝 Proposed fix to remove duplication
-Create the referenced secrets before installing:
-
-```bash
-kubectl -n lobu create secret generic lobu-db \
- --from-literal=uri='postgresql://USER:PASSWORD@HOST:5432/lobu?sslmode=require'
-kubectl -n lobu create secret generic lobu-secrets \
- --from-literal=JWT_SECRET='replace-with-a-long-random-value' \
- --from-literal=BETTER_AUTH_SECRET='replace-with-a-long-random-value' \
- --from-literal=ANTHROPIC_API_KEY='sk-ant-...'
-```
-
If your public URL differs from the first ingress host, also set `app.env.PUBLIC_WEB_URL` explicitly.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| Create the referenced secrets before installing: | |
| ```bash | |
| kubectl -n lobu create secret generic lobu-db \ | |
| --from-literal=uri='postgresql://USER:PASSWORD@HOST:5432/lobu?sslmode=require' | |
| kubectl -n lobu create secret generic lobu-secrets \ | |
| --from-literal=JWT_SECRET='replace-with-a-long-random-value' \ | |
| --from-literal=BETTER_AUTH_SECRET='replace-with-a-long-random-value' \ | |
| --from-literal=ANTHROPIC_API_KEY='sk-ant-...' | |
| ``` | |
| If your public URL differs from the first ingress host, also set `app.env.PUBLIC_WEB_URL` explicitly. |
🤖 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 `@packages/landing/src/content/docs/deployment/kubernetes.mdx` around lines 60
- 69, Remove the duplicated "Create the referenced secrets before installing"
kubectl commands block that re-declares the same secrets already created
earlier; delete the repeated commands that create secrets named "lobu-db" and
"lobu-secrets" (the --from-literal lines for uri, JWT_SECRET,
BETTER_AUTH_SECRET, ANTHROPIC_API_KEY) so only the original "Create secrets"
section remains and keep the following note about setting app.env.PUBLIC_WEB_URL
if needed.
buremba
force-pushed
the
chore/chart-align-v2
branch
from
ef0d79f to
0cc2913
Compare
…te-identical) Replace charts/lobu/* with the contents of packages/owletto/deploy/charts/lobu/ at owletto-main (chart version 0.3.0, last bumped by owletto#179 = lobu#878's worker-smoke gate). The two charts have diverged since lobu's "public Helm chart" landed (#573), and prod's Flux still pulls the owletto fork — every chart change since has needed double-PRs to keep the two in sync. This commit makes `helm template charts/lobu` produce byte-identical output to the owletto fork given any values, so the follow-up Flux repoint is a no-op render diff. The repoint itself happens in a separate owletto PR; deletion of the owletto fork happens in a third PR after prod verifies the new source. No version bump. No behavior change. The chart stays at 0.3.0. Files dropped (orphaned by the alignment): - charts/lobu/templates/secret.yaml — referenced `.Values.secrets.create` and `include "lobu.secretName"`, neither of which exist in the owletto-style values.yaml / _helpers.tpl. Without `secrets.create`, the chart-managed Secret feature was unreachable; production has always used an externally managed Secret via `secretName`. - charts/lobu/values.example.yaml — its walkthrough was for `secrets.create`, which no longer exists. README updated to point at values.yaml + an operator-provided values file instead. CI: - .github/workflows/helm-chart.yml: drop the `Render install example` step that fed values.example.yaml. Verified: - `helm lint charts/lobu` → 0 errors - `helm template ... charts/lobu -f <prod-like-values>` vs same on the owletto fork: 0 line diff - `helm template lobu charts/lobu` (defaults): 0 line diff - `make build-packages` + `make typecheck`: clean
After lobu#882's chart alignment to the owletto fork, the docs were
pointing at:
- charts/lobu/values.example.yaml (file deleted)
- secrets.create / secrets.stringData (feature removed with the
secret.yaml template)
Replace the curl-the-example flow with an inline values.yaml stub and
explicit kubectl-create-secret commands matching the new owletto-style
chart values shape. Drop the Chart-managed-secrets section since the
feature no longer exists.
buremba
force-pushed
the
chore/chart-align-v2
branch
from
0cc2913 to
cfdcb52
Compare
3 tasks
buremba
added a commit
that referenced
this pull request
May 18, 2026
… block (#890) Two CodeRabbit follow-ups from PR #882: 1. charts/lobu/templates/NOTES.txt referenced .Values.ingress.host (singular), but the chart's values schema uses .Values.ingress.hosts (array). Fresh installs rendered the install URL as `https://` with an empty host. Now reads from `index .Values.ingress.hosts 0` when both ingress.enabled and at least one host are set, falls through to the port-forward hint otherwise. 2. packages/landing/.../kubernetes.mdx had the same `kubectl create secret` block twice — once under "Create secrets" and again later under "Install". The second copy is removed and replaced with a back-reference to the earlier section, so users aren't told to do the same thing twice. Verified by `helm install --dry-run --debug --generate-name` on both ingress.enabled=true (renders https://app.example.com) and ingress.enabled=false (renders port-forward).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Step 1 of three consolidating the divergent helm charts. Replaces every overlapping file in
charts/lobu/with the corresponding file frompackages/owletto/deploy/charts/lobu/at owletto-main. The two charts have diverged since lobu's "public Helm chart" landed (#573) and prod's Flux still pulls the owletto fork — every chart change since has needed double-PRs.Verified byte-identical render (modulo Chart.yaml
version+appVersion, which release-please owns):helm template charts/lobuproduces zero-line diffs against the owletto fork on both prod-like values and chart defaults. The follow-up Flux repoint (Step 2, owletto PR) will be a no-op render diff at the kube level.No version bump. No behavior change. Chart.yaml
versionstays at7.1.0andappVersionat7.1.0(release-please-tracked; manifest matches). The actual template + values bytes match owletto/main's chart at 0.3.0.Files dropped (orphaned by the alignment)
charts/lobu/templates/secret.yaml— referenced.Values.secrets.createandinclude "lobu.secretName", neither of which exist in the owletto-stylevalues.yaml/_helpers.tpl. Withoutsecrets.create, the chart-managed Secret feature was unreachable; production has always used an externally managed Secret viasecretName.charts/lobu/values.example.yaml— its walkthrough was forsecrets.create, which no longer exists. README + landing docs updated to point atvalues.yaml+ an operator-provided values file.CI / docs / submodule
.github/workflows/helm-chart.yml: drops theRender install examplestep (no more example file to render).packages/landing/src/content/docs/deployment/kubernetes.mdx: replaces the curl-the-example flow with an inline values.yaml stub, drops theChart-managed secretssection, adds explicitmigrations.enabled: trueto the stub (so fresh installs don't boot app pods that skip migrations against an unmigrated DB), and tells users to always setapp.env.PUBLIC_WEB_URLexplicitly (see Deferred follow-ups below).packages/owlettosubmodule: bumped from780c96de→1c04fa03(current owletto/main). Clears the pre-existingsubmodule-driftCI failure.Deferred follow-ups (after Step 3 lands)
The byte-alignment inherits three existing latent bugs from the owletto fork. None of them are introduced by this PR — they're already shipping in prod's chart source. Production is unaffected because the prod overlay overrides the relevant values. Self-hosters following the updated
kubernetes.mdxset them explicitly. Fixing them in this PR would diverge from byte-identical and risk Step 2's no-op repoint claim.BASE_URL→PUBLIC_WEB_URL. Server code (packages/server/src/auth/base-url.ts,utils/public-origin.ts) readsPUBLIC_WEB_URL; the chart's deployment template derivesBASE_URLfromingress.hosts[0], which the server ignores. Prod overlay setsPUBLIC_WEB_URLexplicitly. Self-hosters must overrideapp.env.PUBLIC_WEB_URLuntil the follow-up lands.ingress.enableddefault should befalse. Current default renders an Ingress with emptyrules:on fresh installs. Prod overlay setshosts:so prod is fine. Self-hosters must either setingress.hosts:or overrideingress.enabled: false.secrets.createfeature gone. No known OCI consumers (semver-aware clients stay on 7.x and never auto-upgrade into the renumbered chart territory; chart version stays at 7.1.0 here anyway).What this PR explicitly does NOT do
release-please-config.json. release-please still ownscharts/lobu/Chart.yaml#versionand$.appVersion.app.allowMultiReplica: trueto the prod overlay defensively.Test plan
helm lint charts/lobu→ 0 errors (1 INFO about icon)helm template summaries-app charts/lobu -f <prod-like-values>vs same on the owletto fork: 0 line diffhelm template lobu charts/lobu(defaults) vs same on owletto fork: 0 line diffmake build-packagescleanmake typecheckcleansubmodule-driftCI: passing (was failing pre-PR)Codex review verdict
Two passes: 58% → 45%. Both passes flag the deferred follow-ups above as merge blockers. Treated as policy disagreement, not new risk: every flagged behavior already exists in prod's chart source, production is unaffected, and fixing them in this PR would defeat the byte-alignment goal that makes Step 2's repoint safe. Merging on policy.
Summary by CodeRabbit
New Features
Documentation
Chores