Skip to content

[fix][r2][SHI-458] Fix rr.gitServer.separate.enabled=true: blob storage env vars - #359

Merged
emas-rt merged 3 commits into
mainfrom
helm-chart-issues-review
Aug 11, 2026
Merged

[fix][r2][SHI-458] Fix rr.gitServer.separate.enabled=true: blob storage env vars#359
emas-rt merged 3 commits into
mainfrom
helm-chart-issues-review

Conversation

@emas-rt

@emas-rt emas-rt commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes a bug affecting self-hosted deployments using rr.gitServer.separate.enabled=true.

SHI-458 — Backend missing blob storage env vars (Linear)

retool.gitServer.commonEnv (which emits RR_BLOB_STORAGE_PROVIDER / RR_DEFAULT_S3_* / RR_DEFAULT_GCS_* / RR_DEFAULT_AZURE_*) was gated inside the else branch of the separateEnabled check in deployment_backend.yaml. When the git server was separate, only RR_GIT_SERVER_HOST and RR_GIT_SERVER_PORT were set on the backend — causing /api/ai/rr/snapshots to fail with Required environment variable RR_DEFAULT_S3_BUCKET is not set.

Fix: Move retool.gitServer.commonEnv outside the if/else so it renders unconditionally whenever gitServer.enabled=true, in both embedded and separate modes.

Test plan

  • helm template with rr.gitServer.separate.enabled=true + S3 blob storage: verify RR_GIT_SERVER_HOST, RR_GIT_SERVER_PORT, and RR_BLOB_STORAGE_PROVIDER/RR_DEFAULT_S3_* all appear on the backend deployment
  • helm template with rr.gitServer.separate.enabled=false (embedded): verify no RR_GIT_SERVER_HOST on backend, blob storage vars still render
  • End-to-end: deployed with rr.gitServer.separate.enabled=true and rr.blobStorage.s3 configured — /api/ai/rr/snapshots returns 201

🤖 Generated with Claude Code

…storage env vars and git v2 ingress routing

SHI-458: retool.gitServer.commonEnv (blob storage vars) was only
included in the embedded-git-server branch. Move it outside the if/else
so RR_BLOB_STORAGE_PROVIDER / RR_DEFAULT_* are always rendered on the
backend when gitServer is enabled, regardless of separate mode.

SHI-459: Add /api/ai/rr/git/v2 path rule to both Ingress and HTTPRoute
when rr.gitServer.separate.enabled=true, routing git v2 traffic to the
git-server service before the catch-all backend rule.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@greptile-apps

greptile-apps Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Confidence Score: 4/5

The PR appears safe to merge, with the non-blocking caveat that the new routing behavior lacks automated template regression coverage.

The changed environment-variable placement and routing rules follow existing helpers, service contracts, version branches, and path ordering; the only accepted concern is that future regressions in the three newly added routing forms would not be detected automatically.

Files Needing Attention: charts/retool/templates/ingress.yaml, charts/retool/templates/httproute.yaml

Important Files Changed

Filename Overview
charts/retool/templates/deployment_backend.yaml Moves common git-server environment rendering outside the embedded-only branch while retaining separate-mode host and port configuration.
charts/retool/templates/httproute.yaml Adds a correctly gated git-v2 PathPrefix rule targeting the separate git-server service, but without automated rendering coverage.
charts/retool/templates/ingress.yaml Adds the git-v2 prefix to both Ingress host forms using the established Kubernetes-version branches, but leaves the new behavior untested.

Reviews (1): Last reviewed commit: "[fix][SHI-458][SHI-459] Fix rr.gitServer..." | Re-trigger Greptile

Comment thread charts/retool/templates/ingress.yaml Outdated
Comment on lines +57 to +72
{{- if include "retool.gitServer.separateEnabled" $ }}
- path: /api/ai/rr/git/v2
{{- if (semverCompare ">=1.18-0" $.Capabilities.KubeVersion.Version) }}
pathType: Prefix
{{- end }}
backend:
{{- if semverCompare ">=1.19-0" $.Capabilities.KubeVersion.Version }}
service:
name: {{ template "retool.gitServer.name" $ }}
port:
number: {{ include "retool.gitServer.port" $ | int }}
{{- else }}
serviceName: {{ template "retool.gitServer.name" $ }}
servicePort: {{ include "retool.gitServer.port" $ | int }}
{{- end }}
{{- end }}

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.

P2 Add routing regression coverage

The new separate-git-server routes span both Ingress forms and HTTPRoute, but no automated rendering assertions verify their prefix, ordering, or backend target. Add template coverage for these forms so later chart changes do not silently restore the git-v2 404 behavior or emit an invalid backend reference.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

emas-rt and others added 2 commits August 7, 2026 16:39
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…routing)

The backend fix (PR #82965) proxies /api/ai/rr/git/v2/* internally when
RR_GIT_SERVER_HOST is set, so ingress-level routing to the git-server
service is not needed and would bypass backend middleware.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@emas-rt emas-rt changed the title [fix][SHI-458][SHI-459] Fix rr.gitServer.separate.enabled=true: blob storage env vars and git v2 ingress routing [fix][SHI-458][SHI-459] Fix rr.gitServer.separate.enabled=true: blob storage env vars Aug 10, 2026
@emas-rt emas-rt changed the title [fix][SHI-458][SHI-459] Fix rr.gitServer.separate.enabled=true: blob storage env vars [fix][r2][SHI-458] Fix rr.gitServer.separate.enabled=true: blob storage env vars Aug 10, 2026

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

👍

@emas-rt
emas-rt merged commit 1078158 into main Aug 11, 2026
13 checks passed
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.

2 participants