Skip to content

rrGitServer: accept blob-storage env vars from .Values.env#307

Merged
JatinNanda merged 2 commits into
r2-cleanupfrom
jatin/rrgitserver-validate-env-map
Jun 5, 2026
Merged

rrGitServer: accept blob-storage env vars from .Values.env#307
JatinNanda merged 2 commits into
r2-cleanupfrom
jatin/rrgitserver-validate-env-map

Conversation

@JatinNanda
Copy link
Copy Markdown
Contributor

@JatinNanda JatinNanda commented Jun 5, 2026

Part of the r2-cleanup integration branch (merges into r2-cleanup, then r2 via #303).

Problem

retool.rrGitServer.validateBlobStorage only scanned .Values.environmentVariables and .Values.environmentSecrets for RR_BLOB_STORAGE_PROVIDER / RR_DEFAULT_*. Deployments that set those via the .Values.env map were forced to duplicate the RR_* vars into environmentVariables just to pass the check. And env vars supplied via envFrom (Secret/ConfigMap splat) are invisible at template time, so a valid setup using them had no way to satisfy the guard.

Fix

  1. Range over .Values.env (keyed by var name) in the same opt-out check.
  2. Add rrGitServer.skipBlobStorageValidation (default false) as an explicit escape hatch for sources the chart cannot inspect at template time (e.g. envFrom). The failure message now points at this flag.

Verification (helm template, rrGitServer.enabled=true)

Case Result
env.RR_BLOB_STORAGE_PROVIDER=gcs ✅ passes (previously required duplication)
env.RR_DEFAULT_GCS_BUCKET=... ✅ passes
skipBlobStorageValidation=true, nothing else ✅ bypassed
nothing configured (default) ❌ fails with updated message + envFrom hint
unrelated env.SOME_OTHER=x ❌ still fails (no false-positive)
rrGitServer.enabled=false ✅ no-op

Both values.yaml copies stay in sync; helm lint passes.

🤖 Generated with Claude Code

validateBlobStorage only scanned environmentVariables and environmentSecrets
for RR_BLOB_STORAGE_PROVIDER / RR_DEFAULT_*, so deployments that configure
those via the .Values.env map had to duplicate them into environmentVariables
to satisfy the check. Range over .Values.env (keyed by var name) as well, and
mention env in the doc comment and failure message.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@ryanartecona ryanartecona left a comment

Choose a reason for hiding this comment

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

technically env vars can also come from envFrom, i.e. if you have a secret or configmap that you just want to splat all the keys/values in there into env vars on the backend pods. I don't think we can detect at template time at all in that case, but it might be prudent to also add an explicit disable for this sanity check in that case where it needs to be overridden.

The blob-storage guard can only inspect blobStorage / env /
environmentVariables / environmentSecrets at template time. Env vars injected
via envFrom (a Secret/ConfigMap splat) are invisible to it, so a valid
configuration that supplies RR_BLOB_STORAGE_PROVIDER / RR_DEFAULT_* that way
would fail the check with no way out.

Add rrGitServer.skipBlobStorageValidation (default false) to bypass the check
entirely, and point at it from the failure message.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@JatinNanda
Copy link
Copy Markdown
Contributor Author

technically env vars can also come from envFrom, i.e. if you have a secret or configmap that you just want to splat all the keys/values in there into env vars on the backend pods. I don't think we can detect at template time at all in that case, but it might be prudent to also add an explicit disable for this sanity check in that case where it needs to be overridden.

good call, added a skip

@JatinNanda JatinNanda marked this pull request as ready for review June 5, 2026 19:39
@JatinNanda JatinNanda merged commit b88cfdd into r2-cleanup Jun 5, 2026
12 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