fix: helm chart fail-closed gates, latent DATABASE_URL bug, deploy-path docs#11
Merged
fix: helm chart fail-closed gates, latent DATABASE_URL bug, deploy-path docs#11
Conversation
…th docs helm: - postgres.enabled=true now uses pgvector/pgvector:pg18 so the dev-mode toggle no longer crash-loops on the CREATE EXTENSION vector migration - redis.authToken is required when redis.enabled=true (mirrors the Compose --requirepass guard so Valkey can't boot unauthenticated) - _helpers databaseUrl: capture `required` returns into $_, otherwise externalDatabase deploys rendered "pg.example.comtestpostgresql://..." -- the assertion's return value was prepending into the URL string - redis probes use valkey-cli with --no-auth-warning to match Compose - NOTES.txt: drop redundant `or` wrap; app-configmap: drop redundant default since values.yaml already defaults sslMode cloud platforms: - cloudrun README step 7: REDIS_TLS=false / DATABASE_SSL=disable to match basic-tier Memorystore + Cloud SQL Auth Proxy unix socket - fly.toml: replace ghcr.io/yawlabs/mcp-hosting:latest (unfetchable by Fly) with a registry.fly.io placeholder + comment - fly bootstrap: stop swallowing real `fly postgres attach` failures (capture exit, only forgive "already attached"); slugify whoami; run `fly certs add` for non-default domains docs: - README: bump 0.1.0 -> 0.2.0 to match Chart.yaml - upgrade.md: replace MCP_HOSTING_IMAGE_TAG rollback recipe with docker-compose.override.yml block (the env var did nothing) - backup-restore.md: filenames mcp-hosting-backup-* -> mcp-hosting-* to match what scripts/backup.sh actually writes ci: - deploy-test: skip Python setup on the helm matrix entry - test.sh: pass --set redis.authToken=... to helm lint/template so the new required gate doesn't break CI Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
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
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
A
/full-passaudit surfaced a mix of latent bugs and silent footguns across the Helm chart, the Fly + Cloud Run paths, and the operator-facing docs. This PR fixes them in one cohesive batch.Worth flagging loudly
_helpers.tpldatabaseUrl was rendering"pg.example.comtestpostgresql://..."for every external-database deploy becauserequiredreturns its asserted value, which got concatenated before the URL string. Captured into$_ :=so it works as a pure assertion.redis.authTokenis now required at render time whenredis.enabled=true. The previous chart rendered Valkey with no--requirepassand the app pod with noREDIS_AUTH_TOKENif the value was left at its default empty string -- silently fail-open, asymmetric to the Compose path's${REDIS_AUTH_TOKEN:?...}guard. Justifies av0.2.1cut.Helm
postgres.enabled=true->pgvector/pgvector:pg18(dev-mode toggle no longer crash-loops onCREATE EXTENSION vector).redis.authTokenrequired when in-cluster Valkey is enabled.valkey-cliwith--no-auth-warningto match Compose.NOTES.txtredundantor;app-configmapredundantdefault; both cleaned up.Cloud Run
REDIS_TLS=true,DATABASE_SSL=requirewhich contradicted the basic-tier Memorystore + Cloud SQL Auth Proxy unix-socket provisioning the same doc walks the operator through. NowREDIS_TLS=false,DATABASE_SSL=disablewith explainer notes for when to flip them back.Fly
fly.tomlno longer points at the unfetchable private GHCR image; placeholder + comment direct the operator to mirror via bootstrap.sh or set--imageon deploy.fly postgres attach | grep -v ... || truewas swallowing real failures underset -euo pipefail(auth, network, 5xx) and leaving DATABASE_URL unset. Now captures exit code and only forgivesalready attached.whoamislugified so corp-SSO usernames don't generate invalid Fly app names.fly certs addnow runs for non-default domains.Docs
--version 0.1.0->0.2.0to matchChart.yaml.docs/upgrade.md: removed theMCP_HOSTING_IMAGE_TAG=v0.8.0rollback recipe (no interpolation indocker-compose.yml, the env var did nothing); replaced with adocker-compose.override.ymlblock.docs/backup-restore.md: filename referencesmcp-hosting-backup-*->mcp-hosting-*to matchscripts/backup.sh.CI
deploy-test.yml: skip Python setup on the helm matrix entry (kubeconform is a Go binary).test.sh: pass--set redis.authToken=...tohelm lintandhelm templateso the new required gate doesn't break CI.Test plan
helm lint --strictpasses.helm templaterenders cleanly with full value set (646 lines, 0 errors).helm templatefails closed whenredis.authTokenis omitted.helm templatefails closed whenexternalDatabase.passwordis omitted.postgresql://user:pass@host:port/db?sslmode=require.pgvector/pgvector:pg18.bash -npasses on all five shell scripts.fly.tomlparses + has required keys.cloudrun/service.yamlparses + has correct Knative shape.Validate Templates(helm-lint, yaml-lint, docker-compose-validate, shellcheck) passes (verified by this PR's CI run).🤖 Generated with Claude Code