Skip to content

Conversation

@Prafulrakhade
Copy link
Member

@Prafulrakhade Prafulrakhade commented Dec 13, 2025

Summary by CodeRabbit

  • Chores
    • Improved deployment process with enhanced configuration copying during installation steps
    • Transitioned configuration management from secret-based to configmap-based approach for better deployment flexibility
    • Optimized installation workflow for MOSIP captcha service deployment

✏️ Tip: You can customize this high-level summary in your review settings.

Signed-off-by: Prafulrakhade <prafulrakhade02@gmail.com>
@coderabbitai
Copy link

coderabbitai bot commented Dec 13, 2025

Walkthrough

The changes refactor the captcha module's installation process by centralizing configmap copying operations. The local copy utility is replaced with a shared utility from the parent utils directory, and the installation script now invokes configmap copy operations before proceeding with Helm deployment, removing previous secret-based copying logic.

Changes

Cohort / File(s) Summary
Captcha Installation Integration
deployment/v3/mosip/captcha/install.sh
Added configmap copy step in installation flow: echoes status, removes CRLF, and executes copy_cm.sh before Helm install.
Configmap Copy Utility
deployment/v3/mosip/captcha/copy_cm.sh, deployment/v3/mosip/captcha/copy_cm_func.sh
Updated copy_cm.sh to reference shared utility from ../../utils/copy_cm_func.sh and introduced DST_NS variable. Removed local copy_cm_func.sh as it is now sourced from shared utils directory. Changed from copying secrets to copying two configmaps (artifactory-share and config-server-share).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Path updates to shared utility location
  • New variable introduction (DST_NS) and script invocation in install flow
  • Resource type shift from secrets to configmaps with corresponding parameter adjustments

Possibly related PRs

Suggested reviewers

  • ckm007

Poem

🐰 A captcha's delight, configmaps to share,
Through namespace paths with bunny care,
From one home to another they leap,
Installation flows, secure and deep! 🔐

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title check ❓ Inconclusive The title references a ticket (MOSIP-44048) and mentions 'bug fix for GA release', but lacks specificity about what the actual bug or fix entails. The changes involve restructuring configmap copy logic in deployment scripts, which is not clearly conveyed by the generic title. Consider using a more descriptive title such as 'MOSIP-44048 Refactor captcha configmap copy logic' or similar to better reflect the specific changes made to the deployment scripts.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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

@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: 1

🧹 Nitpick comments (1)
deployment/v3/mosip/captcha/copy_cm.sh (1)

6-10: Path and utility interface are correct; validation is optional rather than critical.

The relative path ../../utils/copy_cm_func.sh correctly resolves to deployment/v3/utils/copy_cm_func.sh, which exists and properly supports configmap operations. The utility accepts the parameters passed by the script in the correct order (resource type, configmap name, source namespace, destination namespace).

The script's use of set -e and set -o errexit provides sufficient error handling for utility execution failures. While adding an explicit file existence check before invoking the utility would improve clarity of error messages, it is not critical given that the parent utility exists in the expected location and the script will exit on any utility failure.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 39b3d01 and 19a027a.

📒 Files selected for processing (3)
  • deployment/v3/mosip/captcha/copy_cm.sh (1 hunks)
  • deployment/v3/mosip/captcha/copy_cm_func.sh (0 hunks)
  • deployment/v3/mosip/captcha/install.sh (1 hunks)
💤 Files with no reviewable changes (1)
  • deployment/v3/mosip/captcha/copy_cm_func.sh

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.

1 participant