Skip to content

[MOSIP-37808] Updated DB attributes of keymanager#552

Merged
ckm007 merged 1 commit into
mosip:developfrom
abhishek8shankar:MOSIP-37808
May 25, 2026
Merged

[MOSIP-37808] Updated DB attributes of keymanager#552
ckm007 merged 1 commit into
mosip:developfrom
abhishek8shankar:MOSIP-37808

Conversation

@abhishek8shankar
Copy link
Copy Markdown
Member

@abhishek8shankar abhishek8shankar commented May 19, 2026

Summary by CodeRabbit

  • Chores
    • Database deployment infrastructure updated to use parameterized configuration variables for database names, user credentials, and password management instead of hardcoded values. This enables flexible and repeatable deployments across different environments. New deployment configuration property added for enhanced environment customization.

Review Change Stack

Signed-off-by: Abhi <abhishek.shankarcs@gmail.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 19, 2026

Walkthrough

Database deployment automation updated to accept parameterized database name and database user role at deploy-time. Hardcoded mosip_keymgr database and keymgruser role replaced with variable placeholders across configuration, shell orchestration, and all SQL setup/teardown scripts.

Changes

Database deployment parameterization

Layer / File(s) Summary
Deployment configuration
db_scripts/mosip_keymgr/deploy.properties
Added DB_UNAME property set to keymgruser to externalize the database username identifier.
Deploy script variable injection
db_scripts/mosip_keymgr/deploy.sh
Shell script updated to pass mosipdbname and dbuname as psql variables (-v mosipdbname=$MOSIP_DB_NAME, -v dbuname=$DB_UNAME) to all SQL script invocations. Added connection termination logging before pg_terminate_backend call.
SQL script parameterization
db_scripts/mosip_keymgr/drop_db.sql, drop_role.sql, role_dbuser.sql, db.sql, ddl.sql, grants.sql, dml.sql
All SQL scripts updated to use :mosipdbname placeholder for database references and :dbuname placeholder for role references, replacing previous hardcoded mosip_keymgr and keymgruser values across CREATE DATABASE, DROP DATABASE, CREATE ROLE, GRANT, and \c connect directives.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 Hop along the parameterized path,
Where mosipdbname and dbuname flow free,
No more hardcoded names to extract,
Just variables dancing in psql decree! 🐇

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main changes: converting hardcoded database attributes (database name, username) to parameterized variables across all deployment scripts in the keymanager module.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 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 `@db_scripts/mosip_keymgr/deploy.sh`:
- Line 31: The psql invocation currently embeds the DBUSER_PWD with escaped
single quotes and leaves shell variables unquoted; update the psql command to
quote all shell expansions (e.g. use "$SU_USER_PWD", "$SU_USER", "$DB_SERVERIP",
"$DB_PORT", "$DEFAULT_DB_NAME", "$DBUSER_PWD", "$DB_UNAME") to avoid
word-splitting/pathname expansion, and pass the password into the SQL using SQL
literal quoting (use -v dbuserpwd="$DBUSER_PWD" in the psql args and reference
it inside role_dbuser.sql as :'dbuserpwd') so single quotes and special
characters in the password are handled safely.

In `@db_scripts/mosip_keymgr/drop_db.sql`:
- Line 1: The DROP statement uses an unquoted psql variable (:mosipdbname) which
does raw text substitution and can break for special characters; update the
identifier usage to the psql identifier-quoted form by replacing occurrences of
:mosipdbname with the identifier-quoted variable (use :"mosipdbname") so the
database name is safely quoted when executing DROP DATABASE IF EXISTS.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 437c6be6-91d6-4024-8433-f32e279a74cb

📥 Commits

Reviewing files that changed from the base of the PR and between 1ded8fd and 89b12f0.

📒 Files selected for processing (9)
  • db_scripts/mosip_keymgr/db.sql
  • db_scripts/mosip_keymgr/ddl.sql
  • db_scripts/mosip_keymgr/deploy.properties
  • db_scripts/mosip_keymgr/deploy.sh
  • db_scripts/mosip_keymgr/dml.sql
  • db_scripts/mosip_keymgr/drop_db.sql
  • db_scripts/mosip_keymgr/drop_role.sql
  • db_scripts/mosip_keymgr/grants.sql
  • db_scripts/mosip_keymgr/role_dbuser.sql

Comment thread db_scripts/mosip_keymgr/deploy.sh
Comment thread db_scripts/mosip_keymgr/drop_db.sql
@ckm007 ckm007 merged commit 81cf9e8 into mosip:develop May 25, 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