Fix alembic migration crash on a password containing a percent-encoded char - #87
Conversation
…d char
alembic/env.py's config.set_main_option("sqlalchemy.url", PG_CONNECTION)
writes the value through a ConfigParser, which treats any literal "%" as
the start of %(name)s interpolation syntax. install.sh's urlencode()
correctly percent-encodes special characters in POSTGRES_PASSWORD before
embedding it in PG_CONNECTION (e.g. "@" -> "%40") — a completely valid,
necessary escape for the connection URL itself, but ConfigParser has no
idea that's what it is and raises "invalid interpolation syntax" the
moment such a password is used, crashing every alembic invocation
(upgrade, and the stamp-head retry fallback in entrypoint.sh, identically)
before any migration or DB connection is even attempted.
Fix: escape "%" as "%%" before calling set_main_option(). ConfigParser
un-escapes "%%" back to "%" on read (verified directly: round-tripping
the exact failing connection string through configparser.set()+get()
with this escape produces byte-identical output), so this doesn't change
the actual URL passed to SQLAlchemy — it only satisfies ConfigParser's
own write-time validation.
Root-caused from a live crash: ValueError: invalid interpolation syntax
in 'postgresql+psycopg://mosip:nexus%40123@nexus-postgres:5432/mosipnexus'
at position 32.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A5KE4fZxHqbeiDaUJenSfU
Signed-off-by: ckm007 <chandrakeshavmishra@gmail.com>
WalkthroughAlembic now preserves percent-encoded PostgreSQL credentials. Nexus server and UI installers now use local ChangesNexus configuration and deployment
Estimated code review effort: 2 (Simple) | ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Both install.sh scripts took the values-file as a positional argument (3rd for nexus-server, 2nd for nexus-ui), requiring empty "" placeholders for any earlier arg you wanted to skip — easy to get wrong (e.g. ./install.sh my-values.yaml silently puts it into $1/kubeconfig instead, and the values file is never applied, with no error). Just pass -f my-values.yaml directly to helm upgrade --install instead — the file must exist next to the script now (run from deploy/nexus-server/ or deploy/nexus-ui/), matching the my-values.yaml convention already documented in README.md/.gitignore. Also updates nexus-server/install.sh's default namespace to "nexus" (matches the target cluster's actual namespace). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A5KE4fZxHqbeiDaUJenSfU Signed-off-by: ckm007 <chandrakeshavmishra@gmail.com>
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 `@MosipNexus/deploy/nexus-server/install.sh`:
- Line 36: Update the namespace handling around the NS release configuration to
provide an explicit migration or legacy-release guard when moving from
mosip-nexus to nexus, preventing creation of a second stack. Ensure delete.sh
cleanup handles resources/releases in both namespaces so legacy mosip-nexus
deployments are not left behind.
🪄 Autofix
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: 557834b1-a155-4248-9df9-01ef020c6267
📒 Files selected for processing (3)
MosipNexus/Server/alembic/env.pyMosipNexus/deploy/nexus-server/install.shMosipNexus/deploy/nexus-ui/install.sh
Summary
alembic/env.py'sconfig.set_main_option("sqlalchemy.url", PG_CONNECTION)writes the value through aConfigParser, which treats any literal%as the start of%(name)sinterpolation syntax.install.sh'surlencode()correctly percent-encodes special characters inPOSTGRES_PASSWORDbefore embedding it inPG_CONNECTION(e.g.@→%40) — a valid, necessary escape for the connection URL itself — butConfigParserhas no idea that's what it is and raisesinvalid interpolation syntaxthe moment such a password is used. This crashes every alembic invocation before any migration or DB connection is attempted, including thestamp headretry fallback inentrypoint.sh(identically, since it hits the same line).Live crash this fixes:
Fix
Escape
%as%%before callingset_main_option().ConfigParserun-escapes%%back to%on read, so this doesn't change the actual URL passed to SQLAlchemy — verified directly by round-tripping the exact failing connection string throughconfigparser.set()+.get()with this escape, producing byte-identical output.Verification
python3 -m py_compile— cleanConfigParser().set('s','k', url.replace('%','%%'))then.get('s','k')returns the originalurlunchangedentrypoint.shstamp-and-retry fallback in this PR — flagged separately, left as-is per explicit requesthttps://claude.ai/code/session_01A5KE4fZxHqbeiDaUJenSfU
Summary by CodeRabbit
Bug Fixes
Deployment
nexusnamespace and require a localmy-values.yamlconfiguration file.