Fixes #1830 Merge v1.3.0 into develop for PRE-REG#1062
Merged
Conversation
Signed-off-by: GOKULRAJ136 <110164849+GOKULRAJ136@users.noreply.github.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughBumps modules to 1.4.0-SNAPSHOT, reconfigures Maven publishing/profile behavior, replaces kernel PDF generator with Apache PDFBox (Loader.loadPDF), adds a new prereg-captcha Helm chart, refactors Helm envFrom/values, updates SQL migrations for idempotency, and refreshes docs and CI workflow entries. ChangesProject Versioning, Dependencies, and Maven Publishing
PDF Processing Library Migration
Helm Charts: New Captcha Chart and Template Updates
Database Upgrade and Rollback Scripts
Documentation, CI/CD, and Deployment Infrastructure
🎯 4 (Complex) | ⏱️ ~40 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
Signed-off-by: GOKULRAJ136 <110164849+GOKULRAJ136@users.noreply.github.com>
Signed-off-by: GOKULRAJ136 <110164849+GOKULRAJ136@users.noreply.github.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pre-registration/pre-registration-application-service/src/main/java/io/mosip/preregistration/application/service/util/DocumentServiceUtil.java (1)
409-416:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd an upload size guard before buffering the PDF stream in
isPasswordProtectedFile.
DocumentServicecallsserviceUtil.isPasswordProtectedFile(file)beforeserviceUtil.fileSizeCheck(file.getSize()), andisPasswordProtectedFilecurrently buffers the entire PDF viaLoader.loadPDF(inputStream.readAllBytes())—so oversized uploads can spike heap usage/OOM risk. AddfileSizeCheck(file.getSize())(or the same threshold logic) at the start of the PDF/password path beforereadAllBytes().🤖 Prompt for 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. In `@pre-registration/pre-registration-application-service/src/main/java/io/mosip/preregistration/application/service/util/DocumentServiceUtil.java` around lines 409 - 416, isPasswordProtectedFile buffers the entire PDF via Loader.loadPDF(inputStream.readAllBytes()) before any size validation, risking OOM for oversized uploads; add an upload size guard at the start of the PDF/password path by invoking the existing fileSizeCheck(file.getSize()) (or replicating its threshold logic) before reading the stream, i.e., check file.getSize() right after detecting a PDF content type in isPasswordProtectedFile and bail out/throw if it exceeds the limit so Loader.loadPDF is only called for allowed-size files.
🤖 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 `@AGENTS.md`:
- Around line 81-90: The fenced architecture block in AGENTS.md is missing a
language identifier; update the opening triple-backtick for the block labeled
"controller/ → ..." to include a language tag (e.g., change ``` to ```text) so
the markdown linter/CI accepts it—locate the fenced architecture block and add
the language identifier after the opening backticks.
In `@db_upgrade_scripts/mosip_prereg/sql/1.2.0.3_to_1.3.0_rollback.sql`:
- Line 2: The header comment text "-- Below script required to rollback from
1.3.0-beta.1 to 1.3.0." is incorrect for this file; update that comment to
reflect the actual rollback direction implied by the filename
(1.2.0.3_to_1.3.0_rollback.sql) by changing it to something like "-- Below
script required to rollback from 1.3.0 to 1.2.0.3." so the comment follows the
<from-version>_to_<to-version>_rollback.sql convention.
- Around line 10-11: Rollback currently drops CREATE_TIME before making
START_TIME NOT NULL which can fail; modify the rollback in the
BATCH_STEP_EXECUTION section to (1) backfill any NULL START_TIME values using
the CREATE_TIME values (update rows where START_TIME IS NULL to set START_TIME =
CREATE_TIME), (2) then alter the START_TIME column to SET NOT NULL, and only
after that (3) drop the CREATE_TIME column; ensure the SQL statements reference
table BATCH_STEP_EXECUTION and columns START_TIME and CREATE_TIME in that order.
In `@db_upgrade_scripts/mosip_prereg/sql/1.2.0.3_to_1.3.0_upgrade.sql`:
- Line 2: Update the header comment so the transition matches the filename:
replace the incorrect "1.3.0-beta.1 to 1.3.0" text with the correct migration
path "1.2.0.3 to 1.3.0" in the SQL file's top comment (the line currently
reading "-- Below script required to upgrade from 1.3.0-beta.1 to 1.3.0");
ensure the header follows the existing
<from-version>_to_<to-version>_upgrade.sql convention used by
db_upgrade_scripts/mosip_prereg/sql/**/*.sql.
In `@deploy/prereg/delete.sh`:
- Line 10: The helm delete command for the prereg captcha release is not
idempotent and will abort the script under set -e if the release is absent;
update the invocation that targets the "prereg-captcha" release (the helm -n
prereg delete prereg-captcha call) to use Helm's uninstall with ignore-not-found
(e.g., helm -n prereg uninstall prereg-captcha --ignore-not-found) so the
command succeeds harmlessly when the release is already removed and the rest of
the cleanup continues.
In `@helm/prereg-application/templates/deployment.yaml`:
- Around line 108-110: The template currently assumes .Values.extraEnvVarsSecret
is a scalar and always renders a single secretRef.name, which breaks when users
supply the legacy list form; update the block to handle both types by checking
the value kind (use kindIs "slice" or similar) and: if it's a slice, range over
.Values.extraEnvVarsSecret and render a secretRef entry for each item using the
existing include "common.tplvalues.render" (pass each item as "value");
otherwise render the single secretRef.name as today. Reference
.Values.extraEnvVarsSecret and the include "common.tplvalues.render" when making
the change.
In `@helm/prereg-batchjob/templates/deployment.yaml`:
- Around line 108-110: The envFrom secretRef currently assumes
.Values.extraEnvVarsSecret is a scalar and breaks when users supply a list;
update the template around envFrom/secretRef to handle both scalar and list
forms: check the kind/type of .Values.extraEnvVarsSecret (e.g. using kindOf or
typeIs) and if it is a string render a single secretRef.name with the existing
include "common.tplvalues.render" call, otherwise if it is a list range over the
list and render one secretRef block per item (applying the same include
rendering for each element). Ensure you reference .Values.extraEnvVarsSecret,
envFrom and the include "common.tplvalues.render" so both backward-compatible
scalar and list overrides are supported.
In `@helm/prereg-captcha/.gitignore`:
- Line 2: Replace the incorrect gitignore pattern "Charts.lock" with the correct
"Chart.lock" entry in the prereg-captcha chart's .gitignore so the generated
Helm lockfile is ignored consistently with other charts; update the line
containing "Charts.lock" to read "Chart.lock".
In `@helm/prereg-captcha/templates/deployment.yaml`:
- Around line 57-72: The initContainers block contains unresolved placeholders
in the volume-permissions init container: replace the scaffold token
%%commands%% with a concrete permission command (e.g., a shell command that
chowns/chmods the target path using the intended UID/GID), and replace the
placeholder volumeMount name and mountPath (currently "foo" and "bar") with the
actual mount name and path used by persistence (e.g., name "data" and mountPath
"/data") so the init container actually touches the persistent volume; also add
the corresponding volumes entry and the main container's volumeMount for the
same "data" volume when .Values.persistence.enabled is true so the volume is
mounted both for init and the app. Ensure these changes are guarded by the
existing .Values.volumePermissions.enabled and .Values.persistence.enabled
conditionals and use the image and resources from .Values.volumePermissions.
In `@helm/prereg-captcha/values.yaml`:
- Around line 127-130: The values file uses containerSecurityContext.runAsUser
as the string "mosip" which will be emitted directly into
securityContext.runAsUser (in templates/deployment.yaml via omit
.Values.containerSecurityContext "enabled" | toYaml) and must be a numeric UID;
change containerSecurityContext.runAsUser in values.yaml to a numeric UID when
containerSecurityContext.enabled=true, and introduce a separate value (e.g.,
containerUser or env.container_user) for the container_user environment
variable; update the deployment template to use the new containerUser value for
the env var while leaving securityContext.runAsUser sourced from
containerSecurityContext.runAsUser so UIDs and usernames are not conflated.
In `@helm/prereg-datasync/templates/deployment.yaml`:
- Around line 108-110: The template currently assumes .Values.extraEnvVarsSecret
is a scalar when rendering secretRef.name via include "common.tplvalues.render",
which breaks upgrades if users provided a list; update the deployment template
to handle both scalar and list inputs by checking the value kind (e.g., using
kindIs or kindOf) for .Values.extraEnvVarsSecret and: if it's a list/slice,
select the appropriate element (typically the first item) and pass that into
include "common.tplvalues.render"; if it's a scalar, pass it through as
before—this change should be applied where secretRef.name is set so both list
and scalar extraEnvVarsSecret values produce valid names.
In `@pre-registration/pom.xml`:
- Around line 211-220: Remove the duplicate spring-boot-maven-plugin entry under
build/pluginManagement/plugins: keep a single
<artifactId>org.springframework.boot:spring-boot-maven-plugin</artifactId> entry
(preferably the one using ${spring.boot.maven.plugin.version}) and delete the
hardcoded <version>3.2.3</version> block; ensure only one plugin block named
spring-boot-maven-plugin remains in pluginManagement so the effective
configuration is unambiguous.
In `@pre-registration/pre-registration-application-service/Dockerfile`:
- Line 89: The CMD in the Dockerfile places -jar before the JVM -D system
properties so those -D values become application args; update the command that
launches pre-registration-application-service.jar to move all -D... options
(loader.path="${loader_path_env}",
spring.cloud.config.label="${spring_config_label_env}",
spring.profiles.active="${active_profile_env}",
spring.cloud.config.uri="${spring_config_url_env}") before -jar so they are
passed as JVM system properties rather than application arguments, preserving
existing quoting and the jar filename pre-registration-application-service.jar.
In `@pre-registration/pre-registration-batchjob/Dockerfile`:
- Around line 84-86: The Dockerfile CMD builds a java invocation where JVM
system properties (e.g., -Dloader.path="${loader_path_env}",
-Dspring.cloud.config.label="${spring_config_label_env}",
-Dspring.profiles.active="${active_profile_env}",
-Dspring.cloud.config.uri="${spring_config_url_env}") appear after the -jar flag
and are incorrectly treated as the jar filename; move all -D... system
properties and any JVM flags so they appear before the -jar option and ensure
the jar file (pre-registration-batchjob.jar) is the argument immediately
following -jar while preserving the existing JVM flags and environment variables
(loader_path_env, spring_config_label_env, active_profile_env,
spring_config_url_env).
In `@pre-registration/pre-registration-captcha-service/Dockerfile`:
- Around line 83-84: The CMD in the Dockerfile places JVM system properties
after -jar so they become application args and not JVM -D properties; update the
Dockerfile CMD (the java command invoking pre-registration-captcha-service.jar)
to move all -Dspring.* and -Dspring.profiles.active and other JVM flags before
the -jar option so the Spring system properties (e.g.,
-Dspring.cloud.config.uri, -Dspring.cloud.config.label,
-Dspring.profiles.active) are recognized by the JVM as system properties at
startup.
In `@pre-registration/pre-registration-captcha-service/pom.xml`:
- Around line 166-175: The openapi-doc-generate-profile contains two identical
central-publishing-maven-plugin entries; remove the duplicated <plugin> block
for artifactId central-publishing-maven-plugin and keep a single plugin
declaration (preserving <version>, <extensions>, and the <configuration> with
publishingServerId and autoPublish) inside the openapi-doc-generate-profile so
Maven doesn't merge/duplicate plugin configuration.
In `@pre-registration/pre-registration-datasync-service/Dockerfile`:
- Around line 91-92: The JVM/Spring system properties (e.g., -Dloader.path,
-Dspring.cloud.config.label, -Dspring.profiles.active,
-Dspring.cloud.config.uri) are placed after -jar and are thus passed as
application arguments; move all -D... flags to appear before -jar in the java
command in the Dockerfile so they are treated as JVM/system properties (keep the
existing JVM X- flags in place and ensure the sequence is: java [X- flags] [
-D... system props ] -jar pre-registration-datasync-service.jar).
In `@README.md`:
- Around line 131-135: Update the stale Docker image tags in the README example
lines that pull mosipid/pre-registration-application-service,
mosipid/pre-registration-datasync-service, mosipid/pre-registration-batchjob,
and mosipid/pre-registration-booking-service: replace the old 1.3.0-beta.2 tag
with the currently supported release tag(s) used by our deployment (e.g., the
current stable version or a variable like {CURRENT_TAG}), ensuring all four
docker pull lines use the same updated tag and reflect the documented release
channel.
- Around line 1-2: The README badge and service links reference the old release
branch "release-1.3.x"; update all badge URLs and any service links in the
README that point to release-1.3.x (including the top badges and the links
around lines referenced as 25-30) to the active release branch for this PR
(e.g., "release-1.4.x") or to the canonical release variable if you prefer
dynamic linking; search for the literal "release-1.3.x" in README.md, replace
with "release-1.4.x", and verify the linked CI/badge targets (GitHub Actions and
SonarCloud URLs) resolve correctly after the change.
---
Outside diff comments:
In
`@pre-registration/pre-registration-application-service/src/main/java/io/mosip/preregistration/application/service/util/DocumentServiceUtil.java`:
- Around line 409-416: isPasswordProtectedFile buffers the entire PDF via
Loader.loadPDF(inputStream.readAllBytes()) before any size validation, risking
OOM for oversized uploads; add an upload size guard at the start of the
PDF/password path by invoking the existing fileSizeCheck(file.getSize()) (or
replicating its threshold logic) before reading the stream, i.e., check
file.getSize() right after detecting a PDF content type in
isPasswordProtectedFile and bail out/throw if it exceeds the limit so
Loader.loadPDF is only called for allowed-size files.
🪄 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: be4234f4-0c39-4c90-9cc8-9f05867ff821
📒 Files selected for processing (45)
.github/workflows/push-trigger.ymlAGENTS.mdNOTICEREADME.mdTHIRD-PARTY-NOTICES.txtapi-test/pom.xmldb_upgrade_scripts/mosip_prereg/sql/1.2.0.1_to_1.2.0.2_rollback.sqldb_upgrade_scripts/mosip_prereg/sql/1.2.0.1_to_1.2.0.2_upgrade.sqldb_upgrade_scripts/mosip_prereg/sql/1.2.0.2_to_1.2.0.3_rollback.sqldb_upgrade_scripts/mosip_prereg/sql/1.2.0.2_to_1.2.0.3_upgrade.sqldb_upgrade_scripts/mosip_prereg/sql/1.2.0.3_to_1.3.0_rollback.sqldb_upgrade_scripts/mosip_prereg/sql/1.2.0.3_to_1.3.0_upgrade.sqldeploy/prereg-apitestrig/README.mddeploy/prereg-apitestrig/install.shdeploy/prereg/delete.shhelm/prereg-application/templates/deployment.yamlhelm/prereg-application/values.yamlhelm/prereg-batchjob/templates/deployment.yamlhelm/prereg-batchjob/values.yamlhelm/prereg-captcha/.gitignorehelm/prereg-captcha/.helmignorehelm/prereg-captcha/Chart.yamlhelm/prereg-captcha/README.mdhelm/prereg-captcha/templates/NOTES.txthelm/prereg-captcha/templates/_helpers.tplhelm/prereg-captcha/templates/deployment.yamlhelm/prereg-captcha/templates/extra-list.yamlhelm/prereg-captcha/templates/service-account.yamlhelm/prereg-captcha/templates/service.yamlhelm/prereg-captcha/templates/servicemonitor.yamlhelm/prereg-captcha/templates/virtualservice.yamlhelm/prereg-captcha/values.yamlhelm/prereg-datasync/templates/deployment.yamlhelm/prereg-datasync/values.yamlpre-registration/pom.xmlpre-registration/pre-registration-application-service/Dockerfilepre-registration/pre-registration-application-service/pom.xmlpre-registration/pre-registration-application-service/src/main/java/io/mosip/preregistration/application/service/util/DocumentServiceUtil.javapre-registration/pre-registration-batchjob/Dockerfilepre-registration/pre-registration-batchjob/pom.xmlpre-registration/pre-registration-captcha-service/Dockerfilepre-registration/pre-registration-captcha-service/pom.xmlpre-registration/pre-registration-core/pom.xmlpre-registration/pre-registration-datasync-service/Dockerfilepre-registration/pre-registration-datasync-service/pom.xml
Signed-off-by: GOKULRAJ136 <110164849+GOKULRAJ136@users.noreply.github.com>
Signed-off-by: GOKULRAJ136 <110164849+GOKULRAJ136@users.noreply.github.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
pre-registration/pre-registration-captcha-service/pom.xml (1)
64-67: 💤 Low valuePre-existing duplicate
spring-cloud-starter-bootstrapdependency.This duplicate dependency declaration exists in the merged content. It doesn't cause functional issues (Maven ignores duplicates), but could be cleaned up in a future housekeeping PR.
🤖 Prompt for 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. In `@pre-registration/pre-registration-captcha-service/pom.xml` around lines 64 - 67, The pom.xml contains a duplicate dependency block for the artifactId "spring-cloud-starter-bootstrap"; remove the redundant <dependency> element that repeats org.springframework.cloud:spring-cloud-starter-bootstrap so only one declaration remains (search for the dependency entry with artifactId "spring-cloud-starter-bootstrap" and delete the duplicate block).
🤖 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.
Nitpick comments:
In `@pre-registration/pre-registration-captcha-service/pom.xml`:
- Around line 64-67: The pom.xml contains a duplicate dependency block for the
artifactId "spring-cloud-starter-bootstrap"; remove the redundant <dependency>
element that repeats org.springframework.cloud:spring-cloud-starter-bootstrap so
only one declaration remains (search for the dependency entry with artifactId
"spring-cloud-starter-bootstrap" and delete the duplicate block).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 025d7486-f257-45b1-b29e-15db8a46ca9b
📒 Files selected for processing (5)
README.mdapi-test/pom.xmlpre-registration/pre-registration-application-service/pom.xmlpre-registration/pre-registration-captcha-service/pom.xmlpre-registration/pre-registration-datasync-service/pom.xml
✅ Files skipped from review due to trivial changes (1)
- README.md
🚧 Files skipped from review as they are similar to previous changes (2)
- pre-registration/pre-registration-application-service/pom.xml
- pre-registration/pre-registration-datasync-service/pom.xml
Signed-off-by: GOKULRAJ136 <110164849+GOKULRAJ136@users.noreply.github.com>
ashok-ksharma
approved these changes
Jun 5, 2026
ckm007
approved these changes
Jun 5, 2026
9 tasks
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
3 participants
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 by CodeRabbit
New Features
Documentation
Configuration Updates
CI/CD
Database